drivers: rename st7735 to more generic st77xx #19825

gschorcht · 2023-07-13T13:39:42Z

Contribution description

This PR provides the following changes:

renames the driver st7735 to st77xx
renames the test st7735 to st77xx
models controller variants as pseudomodules st7735, st7789 and st7796
removes the buggy initialization as a workaround to use reset defaults, see issue drivers/st7735: faulty driver initialization #19818
adds backward compatibility header files for ST7735_PARAM_* symbols
adds a test board for compilation test of backward compatibility
updates the corresponding board definitions

The PR should solve the remaining dependency issues in KConfig we had by using st7735 module for different controller variants. The backward compatibility header files should work for boards that still use ST7735_PARAM_* in their board definitions so that the board defintions at user's side use shouldn't be affected.

~~To be compilable, the PR includes PR #19824.~~

Testing procedure

Green CI
tests/drivers/disp_dev and tests/drivers/st77xx should still work for all boards using a ST77xx display.
The PR was already tested for these tests for:
- adafruit-pybadge
- esp32s2-lilygo-ttgo-t8
- esp32s3-usb-otg
- sipeed-longan-nano

Issues/PRs references

Workaround for issue #19818
Preqruisite for PR #19827

riot-ci · 2023-07-13T15:18:38Z

Murdock results

✔️ PASSED

020861b drivers/lcd.h: fixes and improvements of documentation

Success	Failures	Total	Runtime
7968	0	7968	14m:07s

Artifacts

Documentation preview

boards/adafruit-pybadge/Makefile.dep

aabadie · 2023-07-14T12:24:56Z

boards/sipeed-longan-nano/Makefile.dep

+  ifneq (,$(filter disp_dev,$(USEMODULE)))
+    USEMODULE += st77xx
+  endif
+
+  ifneq (,$(filter st77xx,$(USEMODULE)))
+    # indicate that a ST7735 is used
+    USEMODULE += st7735
+  endif


Suggested change

ifneq (,$(filter disp_dev,$(USEMODULE)))

USEMODULE += st77xx

endif

ifneq (,$(filter st77xx,$(USEMODULE)))

# indicate that a ST7735 is used

USEMODULE += st7735

endif

ifneq (,$(filter disp_dev,$(USEMODULE)))

USEMODULE += st7735

endif

I did it by intention in that way. Modeling consistent dependencies in Kconfig and the makefiles are really a pain 😟 It took me a number of iterations to get them working. First, I did it exactly in that way you suggested using such a DRIVER variable in the test and selecting directly the driver variant by the board, but I couldn't get it working in that way.

Selecting directly the variant if disp_dev is enabled and not if just the driver st77xx is enabled will cause inconsistencies in tests/drivers/st77xx. Kconfig pulls in the driver by CONFIG_MODULE_ST77XX=y in app.config.test and the HAVE_ST* symbols then enable the specific variant that a given board has. The Makefile dependencies however would always use st7735 by default because of the DRIVER variable. To compile it with the other variants in the CI would then require

either to separate tests/driver/st77* test apps for each variant to be able to set the DRIVER ?= st77* variable and CONFIG_MODULE_ST77*=y (however the code duplication makes definitely no sense)

or to set the DRIVER variable explicitly for all boards that use a certain variant other than the default, for example:

# define the driver to be used for selected boards ifneq (,$(filter stm32l496g-disco esp32s3-usb-otg esp32s2-lilygo-ttgo-t8,$(BOARD))) DRIVER := st7789 endif ifneq (,$(filter esp32s3-wt32-sc01-plus,$(BOARD))) DRIVER := l3g4200d_ng endif

To be honest, isn't it more logical that if an app says "I select the display driver st77xx whatever variant is used" that the board says "Ok, I have this variant" or it uses the default. This corresponds to the approach as it is modeled in Kconfig.

aabadie · 2023-07-14T12:26:49Z

drivers/st77xx/Makefile.dep

+ifeq (,$(filter st7789 st7796,$(USEMODULE)))
+  # enable st7735 as default if no other module is enabled
+  USEMODULE += st7735
+endif


This block should go in drivers/Makefile.dep

I don't like that the drivers/Makefile.dep is blown up more an more with driver specific dependencies. The goal should be to keep drivers/Makefile.dep as simple and small as possible and to resolve driver specific dependencies in the driver implementation. That was a further reason why I selected st77xx instead of st77* when disp_dev is enabled. In that case, the driver's Makefile.dep is used to resolve the driver specific dependencies.

tests/drivers/st77xx/Makefile

gschorcht · 2023-07-17T13:29:35Z

Just for information, I have alread working here 😉

8 bit parallel interface (as used for example by esp32s3-wt32-sc01-plus)
16 bit parallel interface (as used for example by stm32l496g-disco)

The parallel interface on STM32 is GPIO-driven at the moment which is as slow as SPI serial interface display. That's why I'm experimenting a bit to use the FMC instead.

Since the signal LCD_RST uses the STM32L152-based MFX (Multi-Function eXtender) on stm32l496-disco, I have an experimental driver for the MFX as a by-product by which the orange LED (LD1) is already controlled.

Since the parallel interfaces are implemented in drivers/lcd and independent on the controller, I should be able to provide them independent on this PR 😄 But since the board dependency description depend on this PR it might be better to wait for it.

gschorcht · 2023-07-21T12:26:42Z

@aabadie How do we proceed with the required changes of the dependencies? I had tried to explain why I defined the dependencies exactly this way. For short

to prevent to blow-up drivers/Makefile.dep driver details and
to be able to use the same test app for all variants.

19824: boards/sipeed_longan_nano: separate board definition for Sipeed Longan Nano TFT r=benpicco a=gschorcht ### Contribution description This PR provides a minimal separate board definition for the Sipeed Longan Nano version with TFT display which is just an extension of `boards/sipeed-longan-nano` with enabled TFT display module. From the lessons we had to learn with the Kconfig modelling of optional hardware, the TFT version of the Sipeed Longan Nano board has been split off into its own board definition based on the existing Siepeed Longan Nano board. Commits ba29a5e, 237819e, 6d8b56d and c5faf34 are small cleanups of peripheral configurations and could be split from this PR as follow-up PR (changes +70 -36). ### Testing procedure Green CI ``` BOARD=sipeed-longan-nano-tft make -j8 -C tests/drivers/st77xx flash ``` should work ### Issues/PRs references Follow up to PR #19813 and PR #19814 Prerequisite for PR #19825 and PR #19827 19855: gnrc_static: fix static PID assignment r=benpicco a=benpicco Co-authored-by: Gunar Schorcht <[email protected]> Co-authored-by: Benjamin Valentin <[email protected]>

19824: boards/sipeed_longan_nano: separate board definition for Sipeed Longan Nano TFT r=benpicco a=gschorcht ### Contribution description This PR provides a minimal separate board definition for the Sipeed Longan Nano version with TFT display which is just an extension of `boards/sipeed-longan-nano` with enabled TFT display module. From the lessons we had to learn with the Kconfig modelling of optional hardware, the TFT version of the Sipeed Longan Nano board has been split off into its own board definition based on the existing Siepeed Longan Nano board. Commits ba29a5e, 237819e, 6d8b56d and c5faf34 are small cleanups of peripheral configurations and could be split from this PR as follow-up PR (changes +70 -36). ### Testing procedure Green CI ``` BOARD=sipeed-longan-nano-tft make -j8 -C tests/drivers/st77xx flash ``` should work ### Issues/PRs references Follow up to PR #19813 and PR #19814 Prerequisite for PR #19825 and PR #19827 Co-authored-by: Gunar Schorcht <[email protected]>

Instead of using a wrong intialization command sequence for power and frame control, default values after reset are used.

aabadie · 2023-08-24T12:44:23Z

drivers/st77xx/Kconfig.st7735

+
+config MODULE_ST7735
+    bool "ST7735 display"
+    default y if !MODULE_ST7789 && !MODULE_ST7796


Why not use the following?

Suggested change

default y if !MODULE_ST7789 && !MODULE_ST7796

default y if HAVE_ST7735

like for other variants.

Just to model the dependencies exactly as defined in the makefiles: https://github.com/RIOT-OS/RIOT/blob/2c0c1caa33158436b6906c441f34d2f279b33e94/drivers/st77xx/Makefile.dep#L9-L13 Do you think that your suggestion would also give the same results. If so, I'm fine with it.

I don't think it would result in the same.

If I am understanding this modelling correctly the following statements would be made:

Multiple variants of ST77xx can be explicitly selected

Selecting ST77xx would default to ST7735 unless a board declares a variant with HAVE_

Using the suggestion would mean that boards would not have a default and require explicit selection or HAVE_ST7735.

If only one variant can be used (due to implementation constraints) then probably better to use a choice, this makes the default more scalable.

If only one variant can be used (due to implementation constraints) then probably better to use a choice, this makes the default more scalable.

It is intentionally a selection. I don't know if it really makes sense or there is a use case where a board has two or more displays with different ST77xx variants. But to make this possible in principle, there can be multiple ST77xx displays and the user can select multiple variants.

If I am understanding this modelling correctly the following statements would be made:

Multiple variants of ST77xx can be explicitly selected

Selecting ST77xx would default to ST7735 unless a board declares a variant with HAVE_

Exactly.

@aabadie Are you fine now with the Kconfig dependency modelling? This was the last point open for this PR.

@aabadie Are you fine now with the Kconfig dependency modelling? This was the last point open for this PR.

I'm fine with the changes. Let's get this in!

aabadie · 2023-08-24T12:50:44Z

tests/drivers/st77xx/app.config.test

@@ -1,5 +1,5 @@
 # this file enables modules defined in Kconfig. Do not use this file for
 # application configuration. This is only needed during migration.
-CONFIG_MODULE_ST7735=y
+CONFIG_MODULE_ST77XX=y


I think a Kconfig file is needed in the application directory to select the right module driver based on the type of board, like it is done in the Makefile.

config APPLICATION select MODULE_ST7789 if BOARD_with_st7789 select MODULE_ST7796 if BOARD_with_st7796 select MODULE_ST7735 if !BOARD_with_st7789 && !BOARD_with_st7796

(untested)

Do you mean a specific board by BOARD_with_st7789 and BOARD_with_st7796?

I'm not sure, but wouldn't it also work with the following?

config APPLICATION select MODULE_ST7789 if HAVE_ST7789 select MODULE_ST7796 if HAVE_ST7796 select MODULE_ST7735 if !HAVE_ST7789 && !HAVE_ST7796

I think a Kconfig file is needed in the application directory to select the right module driver based on the type of board, like it is done in the Makefile.

@aabadie I'm still a bit unsure about the right way to do Kconfig modeling. Do we agree that the following approach is the right one from the user's point of view?

(Top) → Drivers → Display Device Drivers → ST77xx display driver [ ] ST7735 display [*] ST7789 display [ ] ST7796 display

That is, the user selects Drivers -> Display Device Drivers -> ST77xx display driver and the default variants is/are selected by the corresponding HAVE_ST77* features, e.g:

config MODULE_ST7789 bool "ST7789 display" default y if HAVE_ST7789 depends on MODULE_ST77XX

but wouldn't it also work with the following?

That seems better indeed. I don't know if that is good though. Maybe @MrKevinWeiss can tell

The current state seems correct to me.

"I want a generic st77xx device", if a board has it declared than it will select it by default and if not, fallback to the default st7735.

19884: drivers/touch_dev_gestures: add gesture recognition for touch devices r=aabadie a=gschorcht ### Contribution description This PR adds simple gesture recognition for touch devices accessed via the generic Touch Device API. It can be used in conjunction with device drivers that use either interrupts or polling mode. It supports up to two touches and the following gestures: - Single and double tap at given position - Long press and release given position - Moving while pressed with current position - Swipe left, right, up and down - Zoom in (spread) and out (pinch) Gesture recognition has been tested with: - [x] `stm32f746g-disco` (works out of the box) - [x] `stm32f723e-disco` (works out of the box) - [x] `stm32f429i-disc1` (works on top of PR #19885) - [x] `stm32l496g-disco` (works with my local LCD display changes waiting for PR #19825, not yet provided) - [x] `esp32s3-wt32-sc01-plus` (new board, not yet provided) ### Testing procedure Flash `tests/drivers/touch_dev_gestures` to a board with touch pane, for example: ``` BOARD=stm32f746g-disco make -j8 -C tests/drivers/touch_dev_gestures/ flash ``` PR #19885 is required for the `stm32f429i-disc1` board. The output should look like this: ``` main(): This is RIOT! (Version: 2023.10-devel-121-g81c5c-drivers/touch_dev_gestures) Single Tap X: 255, Y:154 Single Tap X: 253, Y:153 Double Tap X: 253, Y:149 Swipe right Swipe down Swipe left Swipe up Pressed X: 257, Y:155 Moving X: 257, Y:155 Moving X: 257, Y:155 Moving X: 259, Y:156 Moving X: 262, Y:157 Moving X: 266, Y:158 Moving X: 269, Y:160 Moving X: 273, Y:162 Moving X: 276, Y:165 Moving X: 278, Y:167 Moving X: 278, Y:169 Moving X: 278, Y:169 Released X: 279, Y:172 ``` ### Issues/PRs references Co-authored-by: Gunar Schorcht <[email protected]>

aabadie

please squash!

If the controller-specific driver supports multiple controller variants, the variant has to be specified in the configuration parameter set.

If a board definition already used the ST7735 driver, `st7735*.h` header files and `ST7735_*` macros were used in the board definitions to define the default configuration parameter set. For backward compatibility these header files are kept and the `ST7735_*` macros are mapped to the `ST77XX_*` macros if they are defined.

gschorcht · 2023-09-06T09:58:40Z

@aabadie 👍👍👍 Great, thanks for the review. I have squashed the PR.

aabadie · 2023-09-06T10:18:44Z

bors merge

bors · 2023-09-06T13:58:13Z

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

19827: drivers/st77xx: implement initialization r=aabadie a=gschorcht ### Contribution description This PR implements correct initialization for ST7735, ST7789 and ST7796. A number of configuration parameters are exposed via Kconfig. The user can decide whether to use custom configuration parameters or reset defaults. ~To be compilable, the PR includes PR #19824 and PR #19825~ ### Testing procedure - Green CI - `tests/drivers/disp_dev` and `tests/drivers/st77xx` should still work for all boards using a ST77xx display. - The PR was already tested for these tests for: - [x] `adafruit-pybadge` - [x] `esp32s2-lilygo-ttgo-t8` - [x] `esp32s3-usb-otg` - [x] `sipeed-longan-nano` ### Issues/PRs references ~Depends on #19825~ Fixes #19818 Co-authored-by: Gunar Schorcht <[email protected]>

gschorcht requested review from leandrolanzieri, jia200x, MrKevinWeiss, smlng, aabadie, MichelRottleuthner and fjmolinas as code owners July 13, 2023 13:39

github-actions bot added Area: doc Area: Documentation Area: tests Area: tests and testing framework Area: drivers Area: Device drivers Area: boards Area: Board ports Area: sys Area: System Area: Kconfig Area: Kconfig integration labels Jul 13, 2023

This was referenced Jul 13, 2023

drivers/st77xx: implement initialization #19827

Merged

drivers/st7735: split off separate driver modules for st7789 and st7796 #19828

Closed

gschorcht force-pushed the drivers/st77xx_generic branch 2 times, most recently from bca322e to 0aac967 Compare July 13, 2023 22:37

aabadie reviewed Jul 14, 2023

View reviewed changes

boards/adafruit-pybadge/Makefile.dep Outdated Show resolved Hide resolved

aabadie reviewed Jul 14, 2023

View reviewed changes

tests/drivers/st77xx/Makefile Outdated Show resolved Hide resolved

gschorcht mentioned this pull request Aug 6, 2023

boards/sipeed_longan_nano: separate board definition for Sipeed Longan Nano TFT #19824

Merged

drivers/st7735: remove buggy initialization, use reset defaults

12f441f

Instead of using a wrong intialization command sequence for power and frame control, default values after reset are used.

gschorcht force-pushed the drivers/st77xx_generic branch 2 times, most recently from 7da778c to 2c0c1ca Compare August 8, 2023 05:12

github-actions bot removed the Area: doc Area: Documentation label Aug 8, 2023

gschorcht mentioned this pull request Aug 14, 2023

drivers/touch_dev_gestures: add gesture recognition for touch devices #19884

Merged

5 tasks

aabadie reviewed Aug 24, 2023

View reviewed changes

aabadie approved these changes Sep 6, 2023

View reviewed changes

gschorcht added 11 commits September 6, 2023 11:52

drivers/lcd: support of controller variant parameter

ef7234b

If the controller-specific driver supports multiple controller variants, the variant has to be specified in the configuration parameter set.

drivers: rename st7735 driver to more generic st77xx

b5a7101

tests/drivers: rename st3577 test to more generic st77xx

f64890a

sys/auto_init/screen: rename st7735 to more generic st77xx

b5b4ad4

boards/adafruit-pybadge: update for st77xx driver

5406793

boards/esp32s2-lilygo-ttgo-t8: update for st77xx driver

f8c9770

boards/esp32s3-usb-otg: update for st77xx driver

1e99e9a

boards/sipeed-longan-nano: update for st77xx driver

cb1c972

drivers/lcd.h: move doc to module section

7f28ab8

drivers/lcd.h: fixes and improvements of documentation

020861b

gschorcht force-pushed the drivers/st77xx_generic branch from 2c0c1ca to 020861b Compare September 6, 2023 09:55

bors bot merged commit 85c7bd9 into RIOT-OS:master Sep 6, 2023
23 checks passed

gschorcht deleted the drivers/st77xx_generic branch September 6, 2023 14:45

MrKevinWeiss added this to the Release 2023.10 milestone Nov 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drivers: rename st7735 to more generic st77xx #19825

drivers: rename st7735 to more generic st77xx #19825

gschorcht commented Jul 13, 2023 •

edited

Loading

riot-ci commented Jul 13, 2023 •

edited

Loading

aabadie Jul 14, 2023

gschorcht Jul 15, 2023 •

edited

Loading

aabadie Jul 14, 2023

gschorcht Jul 15, 2023 •

edited

Loading

gschorcht commented Jul 17, 2023

gschorcht commented Jul 21, 2023

aabadie Aug 24, 2023

gschorcht Aug 25, 2023

MrKevinWeiss Aug 30, 2023

gschorcht Aug 30, 2023

gschorcht Aug 30, 2023

gschorcht Sep 4, 2023

aabadie Sep 6, 2023

aabadie Aug 24, 2023 •

edited

Loading

gschorcht Aug 25, 2023

gschorcht Aug 27, 2023

aabadie Aug 28, 2023

MrKevinWeiss Aug 30, 2023

aabadie left a comment

gschorcht commented Sep 6, 2023

aabadie commented Sep 6, 2023

bors bot commented Sep 6, 2023

	default y if !MODULE_ST7789 && !MODULE_ST7796
	default y if HAVE_ST7735

drivers: rename st7735 to more generic st77xx #19825

drivers: rename st7735 to more generic st77xx #19825

Conversation

gschorcht commented Jul 13, 2023 • edited Loading

Contribution description

Testing procedure

Issues/PRs references

riot-ci commented Jul 13, 2023 • edited Loading

Murdock results

Artifacts

Choose a reason for hiding this comment

gschorcht Jul 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gschorcht Jul 15, 2023 • edited Loading

Choose a reason for hiding this comment

gschorcht commented Jul 17, 2023

gschorcht commented Jul 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aabadie Aug 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aabadie left a comment

Choose a reason for hiding this comment

gschorcht commented Sep 6, 2023

aabadie commented Sep 6, 2023

bors bot commented Sep 6, 2023

gschorcht commented Jul 13, 2023 •

edited

Loading

riot-ci commented Jul 13, 2023 •

edited

Loading

gschorcht Jul 15, 2023 •

edited

Loading

gschorcht Jul 15, 2023 •

edited

Loading

aabadie Aug 24, 2023 •

edited

Loading