Display driver API sample

[vanwinkeljan]

This PR introduces a generic display shield sample as long as a board supports arduino shields this sample should work for suche board.

[zephyrbot]

All checks are passing now.

checkpatch (informational only, not a failure)

-:205: WARNING:MACRO_WITH_FLOW_CONTROL: Macros with flow control statements should be avoided
#205: FILE: samples/drivers/display/src/main.c:25:
+#define RETURN_FROM_MAIN(exit_code) return

- total: 0 errors, 1 warnings, 424 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

Your patch has style problems, please review.

NOTE: Ignored message types: AVOID_EXTERNS BRACES CONFIG_EXPERIMENTAL CONST_STRUCT DATE_TIME FILE_PATH_CHANGES MINMAX NETWORKING_BLOCK_COMMENT_STYLE PRINTK_WITHOUT_KERN_LEVEL SPLIT_STRING VOLATILE

NOTE: If any of the errors are false positives, please report
      them to the maintainers.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

[jfischer-no]

I suggest to use CONFIG_LVGL_DISPLAY_DEV_NAME as it is already there.

[vanwinkeljan]

That is not an option CONFIG_LVGL_DISPLAY_DEV_NAME can only be set if LVGL is used else this config is not available.

[jfischer-no]

@vanwinkeljan Why not make it more generic and move to samples/drivers/? I guess your plan is also to remove samples/display/ili9340 and samples/display/st7789?

[dbkinder]

change if to If (beginning of a sentence)

[dbkinder]

Let's fix some issues with this rewrite:

This sample makes use of Arduino-based display shields and should work on any
board supporting such a shield, for example:

- :ref:`adafruit_2_8_tft_touch_v2`
- :ref:`ssd1306_128x32_shield`
- :ref:`ssd1306_128x64_shield`

[dbkinder]

change build to built

[vanwinkeljan]

@vanwinkeljan Why not make it more generic and move to samples/drivers/? I guess your plan is also to remove samples/display/ili9340 and samples/display/st7789?

@jfischer-phytec-iot thx for the suggestion will take care of this in the next push.

[erwango]

This addition is not matching the definition of shield: a HW component that could be added on top of a board thanks to compatible connector.
Agree with @jfischer-phytec-iot , it would be better proposed as a sample/driver.

[vanwinkeljan]

@erwango The sample was already moved to samples/drivers/display_shield

[MaureenHelm]

@vanwinkeljan Why not make it more generic and move to samples/drivers/? I guess your plan is also to remove samples/display/ili9340 and samples/display/st7789?

@jfischer-phytec-iot thx for the suggestion will take care of this in the next push.

This looks more like a general display driver sample than a shield sample, so I think it belongs in samples/drivers/display. It should work on mimxrt1050_evk, which doesn't need a shield.

[vanwinkeljan]

@MaureenHelm Valid point, only remaining problem is how do we get the correct display name in the sample.

An option would be to add a dts_fixup.h file to the sample to map all display name defines to a well known define that we can use in the sample.

[jfischer-no]

Valid point, only remaining problem is how do we get the correct display name in the sample.

I am to implement similar mechanism that network stack has to discover the network interfaces or what we have for the usb classes.

[vanwinkeljan]

@jfischer-phytec-iot Any suggestion how we proceed with this PR, eg. proceed with a dts_fixup.h file until your mechanism is in place, ... ?

[MaureenHelm]

only remaining problem is how do we get the correct display name in the sample.

What about a common dts alias like we do for led0?

[jfischer-no]

only remaining problem is how do we get the correct display name in the sample.

What about a common dts alias like we do for led0?

Would probably not work for the configuration like reel board + ssd1306_128x32 or mimxrt1050 + waveshare_epaper.

[jfischer-no]

@jfischer-phytec-iot Any suggestion how we proceed with this PR, eg. proceed with a dts_fixup.h file until your mechanism is in place, ... ?

I think yes if that with dts_fixup.h works and is acceptable.

[dbkinder]

Change to Below

[dbkinder]

for consistency, use lowercase for the color names (red, green, blue)

[dbkinder]

What should you do if the LCD is "endian swapped"? Is there a configuration or code change needed?

[vanwinkeljan]

As this is a generic display sample there is no clear solution to this issue and it is up to the user to figure it out. Could be a Kconfig that is not set correctly or a plain bug in the code.

[vanwinkeljan]

Updated PR

• Renamed sample to display instead of display_shield
• Added support for mimxrt1050_evk (including build in CI)
• Run sample in CI for native_posix + dummy display
• Added waveshare_epaper_gdeh0213b1 to CI
• Removed shield specific dts_fixup.h
• Added dts_fixup.h to sample until display discovery mechanism is available

[dbkinder]

That's funny. My previous comments ask what someone should do if indeed the LCD is endian swapped and @MaureenHelm hit this. So maybe this is a common enough problem to say more here about what the developer might need to do in this situation?

[dbkinder]

docs LGTM, thanks.

[vanwinkeljan]

@MaureenHelm @erwango could you please revisit your reviews?

[erwango]

@vanwinkeljan,approved.
Btw, can you provide inputs on #20934.
I've modified shield Kconfig system, so I had to rewrite some shields configuration.
I have some doubts on the display shields configuration I proposed (particularly where to draw the line between application and shield configuration), and it seems like this change is perfect to test the proposal.

[vanwinkeljan]

@erwango I will have a look at the other PR

[jfischer-no]

block it to have time for review and test

[jfischer-no]

LGTM, thanks. Tested with ssd1306, mimxrt1050, st7789v, ssd16xx. As next we should remove samples/display/ili9340 and sample/display/st7789v.

[jfischer-no]

@MaureenHelm Please revise.