Display refactor and SH8601 support

[abrondijk]

Description

The main purpose of this PR is to add support for the SH8601 OLED display driver.

As this is an inherently different display driver, it requires a few changes to the display and display_driver components:

• Refactored LcdInitCmd struct to have configurable delays per command.
• Moved set_brightness & get_brightness functions to the display driver itself.
  This is because OLED displays don't have a backlight of course, and thus its brightness is controlled through the SPI interface rather than a PWM signal.
• Make implementing the send_lines function a requirement for the following:
• Change lcd_write function to a dedicated write_command function.
  For the currently supported display drivers, sending commands is rather trivial. But this OLED driver (and from what I've found so far, a lot of QSPI drivers), handles sending commands quite a bit differently compared to writing to gram.
• Have the DC pin be optional
  This OLED driver does not have a DC signal exposed

Motivation and Context

For Smartknob-HA, we are exploring the possibility of swapping the display to this OLED one.

How has this been tested?

This has been tested on the Lilygo T-Encoder Pro.

The changes above, and the actual QSPI functions, can be found here, mainly the writeCommand, sendLines and initialize functions.

If you agree to the changes I've made to the components, I will update the driver example, and also apply the above changes to the already existing drivers and components using them.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation Update
• Hardware (schematic, board, system design) change
• Software change

Checklist:

• My change requires a change to the documentation.
• I have added / updated the documentation related to this change via either README or WIKI

Software

• I have added tests to cover my changes.
• I have updated the .github/workflows/build.yml file to add my new test to the automated cloud build github action.
• All new and existing tests passed.
• My code follows the code style of this project.

Hardware

• I have updated the design files (schematic, board, libraries).
• I have attached the PDFs of the SCH / BRD to this PR
• I have updated the design output (GERBER, BOM) files.

[abrondijk]

@finger563 Let me know what you think :)

[finger563]

@abrondijk this sounds like a good change - please go ahead and update the other display drivers accordingly and once you lmk it's ready I'll test on my systems. This design pattern should work well with my recent change removing gamma configuration from the driver itself since that may be display / board dependent. Def like the idea of officially adding OLED support to espp 🚀

[finger563]

@abrondijk is there a way the LED could still be part of the display, but just be optional? such that we don't use the LED / initialize at all if the provided GPIO_NUM is -1? That way it's still part of the display rather than the display driver?

[abrondijk]

@finger563 It is possible, but that would mean another 2 functions to pass to the display constructor, for displays that don't have a PWM controlled backlight (set & get).

As it is right now, I'm not entirely happy anyway, with the LED instance having to be a pointer (because of the lack of constructor).
I'll try to find a better solution.

[abrondijk]

@finger563 In my most recent commit, I've moved the brightness back to the display component, and added 2 functions to the config struct.
Does this seem alright to you?

[abrondijk]

On another note, I'm not entirely sure whats causing the static analysis to fail. Do you have any idea?

[finger563]

On another note, I'm not entirely sure whats causing the static analysis to fail. Do you have any idea?

I think I've probably got it misconfigured - I think it's complaining because it can't find the file in the upstream repo since this PR is from a fork. It's worked on other PRs from forks before so I think it just has to do with this PR adding a new file.

I thought I'd configured it properly to analyze fork PRs but I'll take another look.

[finger563]

I agree with your comments on cluttering the display config - so I think what we can do is similar to what we did for tasks and other components: multiple configs. This will make the caller code clearer since we can have a config specifically for LED PWM, and a config specifically for more general control with function callbacks.

Hope that makes sense and keeps the code easy to use and (hopefully) harder to misuse.

[finger563]

Locally, I've updated the examples to fix the &trans[1] issue, and that seems to have resolved the garbage, so the code seems to be working fine on my end :)

[finger563]

Once you fix the examples and address the unresolved comments, please Update the docs (add the new file to the doc/Doxyfile alongside the existing display-drivers includes and please add it to the doc/en/display/display_drivers.rst alongside the others.

Once those are done, I think this will be good to merge :)

[abrondijk]

@finger563 I believe I have implemented all requested changes.
The LcdInitCommands refactor was quite a bit more than I initially thought, so I'm afraid you'll have to retest the changes on your hardware.

[finger563]

@finger563 I believe I have implemented all requested changes.
The LcdInitCommands refactor was quite a bit more than I initially thought, so I'm afraid you'll have to retest the changes on your hardware.

Thanks @abrondijk ! I'll test on some of my displays soon.

Can you please update the doc folder accordingly, and if possible, can you get a video or still image of the example running on your encoder board and add it to the PR and the example readme?

[abrondijk]

@finger563 I believe I have implemented all requested changes.
The LcdInitCommands refactor was quite a bit more than I initially thought, so I'm afraid you'll have to retest the changes on your hardware.

Thanks @abrondijk ! I'll test on some of my displays soon.

Can you please update the doc folder accordingly, and if possible, can you get a video or still image of the example running on your encoder board and add it to the PR and the example readme?

I'll take a photo and video later today!
Can you give me a hint on what I should update in the doc directory?
I tried building the docs, but after building the docker image I'm getting quite a lot of python errors

[finger563]

@finger563 I believe I have implemented all requested changes.
The LcdInitCommands refactor was quite a bit more than I initially thought, so I'm afraid you'll have to retest the changes on your hardware.

Thanks @abrondijk ! I'll test on some of my displays soon.

Can you please update the doc folder accordingly, and if possible, can you get a video or still image of the example running on your encoder board and add it to the PR and the example readme?

I'll take a photo and video later today!
Can you give me a hint on what I should update in the doc directory?
I tried building the docs, but after building the docker image I'm getting quite a lot of python errors

Hey @abrondijk add the new file to the doc/Doxyfile alongside the existing display-drivers includes and please add it to the doc/en/display/display_drivers.rst alongside the others. That should be all that's required. The docs build but produce a lot of errors, at some point I need to go back and do a pass on the docs to clean them up 😅

[abrondijk]

@finger563 Is that not what I did here?

[abrondijk]

Sidenote, I will slightly adjust the demo gui, as otherwise the label with the iteration count is barely visible on round displays (like this one).

[finger563]

@finger563 Is that not what I did here?

Ah yes you did, my bad I missed that 😅

[abrondijk]

Pipelines should succeed now, sorry about that.

[abrondijk]

@finger563 I took a photo and video, where do you want them in the repo? In the display_driver example directory?

[finger563]

@finger563 I took a photo and video, where do you want them in the repo? In the display_driver example directory?

@abrondijk I usually just upload them into the PR as a GitHub asset so they're not in the repo, then once they're uploaded I copy the markdown it generated into the example readme :)

[abrondijk]

Fair enough! Here you go:

SH8601.mp4

[finger563]

Tested:

• t-dongle-s3
• esp-box
• t-deck
• seeed-studio-round-display
• matouch-rotary-display

The only thing I don't like is now there are a lot of warnings for missing initializer for member ...::parameters. We can remove these warnings by default constructing / setting default for the parameter vector to be {}, so please do that. Once that's done I'll merge it 🚀

[finger563]

Just add the default {} to the DisplayInitCmd::parameters, and this will be good to merge!

Thanks for doing this!

As mentioned in the previous comments, once this is merged I'll start the work to do a refactor for the display drivers so they inherit from a base DisplayDriver class.

[abrondijk]

Done!
Thank you as well for your time and patience. We greatly appreciate your library!

As for the upcoming refactor, I have some thoughts/ideas/comments and would of course be happy to help. Would you like them here, or do you want to open an issue/pr for that?

[finger563]

Done!
Thank you as well for your time and patience. We greatly appreciate your library!

As for the upcoming refactor, I have some thoughts/ideas/comments and would of course be happy to help. Would you like them here, or do you want to open an issue/pr for that?

I actually made a discord so that it's easier to have discussions for things like this :)

Link is in espp readme