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

st7789: add configuration option to set default background color #14989

Merged
merged 1 commit into from
Nov 29, 2024

Conversation

michallenc
Copy link
Contributor

Summary

ST7789 fills the display with 0xffff color during its initialization. This color may or may not be overwritten by a higher layer (framebuffer or lcd driver for example). The color remains on the display until application UI is started if not overwritten by the higher layer before.

This commit adds configuration option LCD_ST7789_DEFAULT_COLOR that allows to set the default background color. This may avoid shining display with, for example, white color for an extensive amount of time. The default value is set to 0xffff previously hard-coded in the driver, therefore current implementations will not notice the change.

Impact

New configuration option, none to current implementations.

Testing

Tested with SPI connected ST7789 on SAMv7 MCU based custom board.

ST7789 fills the display with 0xffff color during its initialization.
This color may or may not be overwritten by a higher layer (framebuffer
or lcd driver for example). The color remains on the display until
application UI is started if not overwritten by the higher layer before.

This commit adds configuration option LCD_ST7789_DEFAULT_COLOR that
allows to set the default background color. This may avoid shining
display with, for example, white color for an extensive amount of time.
The default value is set to 0xffff previously hard-coded in the driver,
therefore current implementations will not notice the change.

Signed-off-by: Michal Lenc <[email protected]>
@github-actions github-actions bot added Area: Drivers Drivers issues Area: Graphics Size: S The size of the change in this PR is small labels Nov 29, 2024
@nuttxpr
Copy link

nuttxpr commented Nov 29, 2024

[Experimental Bot, please feedback here]

Yes, this PR appears to meet the NuttX requirements, although it could be slightly improved. The "Testing" section, while showing what was tested, doesn't show the results of the test (i.e., the actual log output demonstrating the color change). Adding those logs would complete the testing section. Also, specifying the NuttX version being used would be helpful.

Here's a slightly more complete version of the PR description based on your provided information:

## Summary
ST7789 fills the display with 0xffff (white) during initialization. This color may not be overwritten by higher layers (framebuffer, LCD driver) before application UI starts, leaving a white screen for an extended period.

This commit adds the `LCD_ST7789_DEFAULT_COLOR` configuration option to set the initial background color.  This avoids a prolonged white screen. The default value is `0xffff` (white), maintaining compatibility with existing implementations.  Relates to [NuttX Issue # (if applicable)]

## Impact
* **Is new feature added? Is existing feature changed?**  New feature added (configuration option). Existing initialization behavior is configurable.
* **Impact on user:** NO for existing users. YES for new users who want to customize the initial screen color. They will need to configure `LCD_ST7789_DEFAULT_COLOR`.
* **Impact on build:** NO.
* **Impact on hardware:** NO.  Affects ST7789 driver only.
* **Impact on documentation:** YES. Documentation should be updated to describe the new `LCD_ST7789_DEFAULT_COLOR` configuration option. (A documentation patch will be provided in a separate commit/PR).
* **Impact on security:** NO.
* **Impact on compatibility:** Backward compatible.
* **Anything else to consider:** None.


## Testing
I confirm that changes are verified on a local setup and work as intended:
* **Build Host:** Linux (Distribution: [e.g., Ubuntu 20.04]), CPU: [e.g., x86_64], Compiler: GCC [version]
* **Target:** ARM (SAMv7), Board: [Your custom board name], NuttX Version: [NuttX version]

**Testing logs before change (LCD_ST7789_DEFAULT_COLOR not set, defaulting to 0xffff):**

[Boot log showing white screen or mentioning 0xffff being written to the display]
...


**Testing logs after change (LCD_ST7789_DEFAULT_COLOR set to 0x0000 - black):**

[Boot log showing black screen or mentioning 0x0000 being written to the display]
...

Key improvements:

  • More detail: Explicitly stated what changes were made and how they address the issue.
  • Clearer impact assessment: Broke down the impact section into the requested sub-sections for better readability and clarity.
  • Complete testing information: Included host details, NuttX version, and (placeholder for) actual log output showing the color change. This is the most crucial part to demonstrate that the change works as intended.
  • Documentation: Mentioned the need for documentation updates.

By adding the actual log output demonstrating the before/after color change, this PR description would be much stronger and more likely to be accepted. Remember to replace the bracketed placeholders with the real information from your setup.

@xiaoxiang781216 xiaoxiang781216 merged commit a8d4ed9 into apache:master Nov 29, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Drivers Drivers issues Area: Graphics Size: S The size of the change in this PR is small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants