Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

arm/stm32f401rc-rs485: Add support to LCD 16x2 with I2C Backpack #13565

Merged
merged 1 commit into from
Sep 21, 2024

Conversation

rcsim
Copy link
Contributor

@rcsim rcsim commented Sep 21, 2024

Summary

Add support to LCD 16x2 with I2C Backpack

Impact

Users will be able to use LCD 16x2 with I2C Backpack on SMT32F401RC-RS485 Board

Testing

Configure lcd1602
Build and Flash
Run slcd example:

nsh> slcd "Hello NuttX"
Opening /dev/slcd0 for read/write access
Attributes:
  rows: 2 columns: 16 nbars: 0
  max contrast: 0 max brightness: 1
Clear screen
WRITING:
0000: 1b5b46                                                            .[F
Set brightness to 1
Print [Hello NuttX]
WRITING:
0000: 1b5b471b5b30304c1b5b4548656c6c6f 204e75747458                     .[G.[00L.[EHello  NuttX
Test complete
nsh>

@github-actions github-actions bot added Area: Documentation Improvements or additions to documentation Area: Board support Board support issues Size: M The size of the change in this PR is medium labels Sep 21, 2024
@nuttxpr
Copy link

nuttxpr commented Sep 21, 2024

[Experimental Bot, please feedback here]

The PR as presented does not meet the NuttX requirements fully. Here's why:

Missing Information:

  • Summary:

    • Why is this change necessary? Is it a new feature request? Are there existing issues with LCD support?
    • What functional part is changed? Be specific - mention the driver(s) modified or added.
    • How does the change work? Explain the technical implementation (e.g., "Added an I2C driver for the LCD backpack and integrated it with the framebuffer driver").
  • Impact:

    • The current description is too vague.
    • Which architectures/boards are affected specifically?
    • Build impact? Are there new Kconfig options?
    • Documentation? Was any added/updated to reflect the new LCD support?
  • Testing:

    • Insufficient detail!
    • Build Hosts: List the specific OS, CPU architecture, and compiler versions used for testing.
    • Target(s): Provide the exact architecture (e.g., armv7-m), board name (e.g., stm32f4discovery), and the NuttX configuration used (e.g., stm32f4discovery_defconfig).
    • "Testing logs before change" are irrelevant. There wouldn't have been LCD output before this feature.

Recommendations:

  1. Expand the Summary: Provide a clear, technical overview of the changes made.
  2. Be Specific about Impact: Clearly list all affected architectures, boards, drivers, Kconfig options, and documentation.
  3. Detailed Testing Info: Follow the template, providing complete information about your build and target environments.

Example Improvement:

## Summary

This PR adds support for 16x2 LCD displays with I2C backpacks to the STM32F401RC-RS485 board. 
It achieves this by:

1. **Adding a new I2C driver** for the specific LCD backpack used (mention the chip/model).
2. **Integrating the LCD driver** with the NuttX framebuffer driver to enable text output. 
3. **Adding a new board configuration option** (`CONFIG_VIDEO_LCD1602_I2C`) to enable this feature.

This change addresses [link to NuttX issue or feature request, if applicable].

## Impact

* **New feature:** Support for 16x2 LCDs with I2C backpacks.
* **Impact on user:** Users with the supported LCD and board can now use it for display output.
* **Impact on build:**
    * New Kconfig option: `CONFIG_VIDEO_LCD1602_I2C`
* **Impact on hardware:**
    * **Architectures:** ARM (specifically ARMv7-M)
    * **Boards:** STM32F401RC-RS485
* **Impact on documentation:**
    * Added a new section to the STM32F401RC-RS485 board documentation explaining how to configure and use the LCD.
* **Impact on security:** None
* **Impact on compatibility:** None 

## Testing

I confirm that changes are verified and work as intended:

* **Build Host(s):**
    * OS: Ubuntu 20.04
    * CPU: Intel Core i7-10700K
    * Compiler: GCC 9.4.0
* **Target(s):**
    * Architecture: armv7-m
    * Board:  stm32f401rc-rs485
    * Configuration: `stm32f401rc_rs485_defconfig` with  `CONFIG_VIDEO_LCD1602_I2C=y`

**Testing logs after change:**

nsh> slcd "Hello NuttX"
Opening /dev/slcd0 for read/write access
Attributes:
rows: 2 columns: 16 nbars: 0
max contrast: 0 max brightness: 1
Clear screen
WRITING:
0000: 1b5b46 .[F
Set brightness to 1
Print [Hello NuttX]
WRITING:
0000: 1b5b471b5b30304c1b5b4548656c6c6f 204e75747458 .[G.[00L.[EHello NuttX
Test complete
nsh>

By providing this level of detail, you make it much easier for the NuttX maintainers to understand, review, and ultimately merge your contribution.

@acassis acassis merged commit a5754a2 into apache:master Sep 21, 2024
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Board support Board support issues Area: Documentation Improvements or additions to documentation Size: M The size of the change in this PR is medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants