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

drivers/lcd: add CMD sequence feature #19918

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

gschorcht
Copy link
Contributor

Contribution description

The PR provides the capability to execute a sequence of LCD controller commands defined in a byte array as CLP tuples (command, length, parameter). This can be used for example

  • to send an initialization sequence to a controller that is not supported by a controller-specific driver or
  • to write specific configuration parameters that are not covered by the controller-specific driver.

The PR introduces the pseudomodule lcd_init_sequence to use the initialization command sequence defined in the driver parameter set instead of the the controller-specific initialization function.

The format of a CLP tuple is:

  • one byte the command index
  • one byte the length of the following parameters, i.e. the number of bytes representing the parameters for the command. If the command has no parameters, the length is 0.
  • n bytes the parameters of the command.

For testing, tests/drivers/st77xx has been extended to use initialize the LCD controller by a intialization command sequence if module lcd_init_sequence is enabled.

Testing procedure

tests/drivers/st77xx should work with enabled lcd_init_sequence:

USEMODULE=lcd_init_sequence BOARD=adafruit-pybadge make -C tests/drivers/st77xx flash
USEMODULE=lcd_init_sequence BOARD=esp32s3-usb-otg make -j8 -C tests/drivers/st77xx flash

Issues/PRs references

@github-actions github-actions bot added Area: tests Area: tests and testing framework Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration labels Sep 8, 2023
@gschorcht gschorcht added Type: new feature The issue requests / The PR implemements a new feature for RIOT CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Sep 8, 2023
@riot-ci
Copy link

riot-ci commented Sep 8, 2023

Murdock results

✔️ PASSED

02feadd fixup! drivers/lcd: add lcd_write_cmd_sequence function

Success Failures Total Runtime
10248 0 10249 17m:14s

Artifacts

@gschorcht gschorcht force-pushed the drivers/lcd_cmd_sequence branch from 0e7ec13 to 10f11ca Compare September 8, 2023 11:36
@gschorcht gschorcht added this to the Release 2023.10 milestone Sep 15, 2023

#include "st7735_internal.h"

static const uint8_t st7735_default_init[] = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think those should live in the driver, not copy & pasted between applications/boards.
Or is there anything application specific about this?

(Sure it makes sense being able to supply an out-of-tree command sequence, but if we already have some known good init sequences for some boards, they should be easily usable)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These init sequences are actually only intended for test and demonstration purposes. They have the disadvantage that they use the parameter macros directly and cannot be parameterized by the parameter set of the device. Furthermore, most of the commands in these example sequences only configure the default values after resetting the display anyway and are actually not needed at all. Last, the default init sequences are already implemented by the driver for the ST77xx variants.

drivers/include/lcd.h Outdated Show resolved Hide resolved
@gschorcht gschorcht force-pushed the drivers/lcd_cmd_sequence branch from 9f8ca48 to 5d6a28c Compare September 20, 2023 23:13
@gschorcht
Copy link
Contributor Author

I had to rebase the PR to resolve conflicts.

@gschorcht
Copy link
Contributor Author

I had to rebase the PR.

@gschorcht gschorcht force-pushed the drivers/lcd_cmd_sequence branch from 5d6a28c to fbbcb56 Compare September 22, 2023 15:20
@benpicco benpicco removed this from the Release 2023.10 milestone Jan 2, 2024
@gschorcht gschorcht force-pushed the drivers/lcd_cmd_sequence branch from fbbcb56 to 52926ab Compare November 22, 2024 13:02
@github-actions github-actions bot removed the Area: Kconfig Area: Kconfig integration label Nov 22, 2024
@benpicco benpicco requested review from Enoch247 and removed request for smlng November 22, 2024 13:03
@Enoch247
Copy link
Contributor

When would using a CMD seq be the preferred method vs adding or extending the LCD specific drivers?

@gschorcht
Copy link
Contributor Author

After taking a year off, I am slowly trying to resume my work on RIOT-OS and am simply rebasing my old PRs to start with.

@gschorcht
Copy link
Contributor Author

When would using a CMD seq be the preferred method vs adding or extending the LCD specific drivers?

Sometimes you will only find the sequence of bytes that represent the command sequence for initialization in data sheets, manufacturer examples or reference implementations.In this case, such a function is helpful.

@Enoch247
Copy link
Contributor

When would using a CMD seq be the preferred method vs adding or extending the LCD specific drivers?

Sometimes you will only find the sequence of bytes that represent the command sequence for initialization in data sheets, manufacturer examples or reference implementations.In this case, such a function is helpful.

@Enoch247 Enoch247 closed this Dec 14, 2024
@Enoch247 Enoch247 reopened this Dec 14, 2024
@Enoch247
Copy link
Contributor

Sorry, I didn't mean to close this. I pressed the wrong button. Re-opening.

@Enoch247
Copy link
Contributor

The part I am unclear on is; what functionality does this PR provide that is not already possible with the existing code. It appears to be a another way to do what is already possible.

If it does not provide new functionality, than an explanation of why the duplicate method is desirable to add would be helpful.

@gschorcht
Copy link
Contributor Author

The part I am unclear on is; what functionality does this PR provide that is not already possible with the existing code. It appears to be a another way to do what is already possible.

If it does not provide new functionality, than an explanation of why the duplicate method is desirable to add would be helpful.

Instead of using an endless sequence of calls to lcd_ll_write_cmd when you need a specific command sequence, you can simply define the command sequence as a byte array, for example if you have to override the initialization sequence of the controller specific initialization for your display.

It corresponds to the approach as it is done in the Adafriut ST7735 Library: https://github.com/adafruit/Adafruit-ST7735-Library/blob/master/Adafruit_ST7735.cpp#L44-L209

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: drivers Area: Device drivers Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants