Created SH1107 driver for quantum painter #24724
Conversation
…ng SH1106 driver.
Steve973
force-pushed
the
quantum-painter-sh1107-driver
branch
from
a40f821 to
2889006
Compare
|
LGTM. |
| typedef struct sh1107_device_t { | ||
| oled_panel_painter_device_t oled; | ||
|
|
||
| uint8_t framebuffer[SURFACE_REQUIRED_BUFFER_BYTE_SIZE(128, 128, 1)]; |
There was a problem hiding this comment.
Having the framebuffer size hardcoded to the maximum looks wrong (even though the typical MCUs with which the QP code is used are usually not that resource restricted).
Options that I could think of:
- Add some defines which could default to the maximum size, but could be reduced if desired.
- Provide the buffer externally in
make_*_devicefunctions, similar to what [QP] Support for LS0XX displays #21844 does (but maybe with more error checking — e.g., pass the buffer size too, then check the passed size against the value calculated from the display dimensions).
Opinions?
There was a problem hiding this comment.
Tested with a couple of (supposedly) SH1107 displays which I have currently (more may be coming in a couple of weeks):
- 1.5" 128×128 SPI OLED — works (note that this particular display has a somewhat weird pinout which does not match most other 7-pin 128×64 or 64×128 SPI OLEDs);
- 0.96" 64×128 I2C OLED (not really from that listing, but looks exactly like that, and the original listing disappeared since 2020) — works if the suggested changes are applied.
Co-authored-by: Sergey Vlasov <[email protected]>
|
@Steve973 I am willing/offering to contribute to this PR (resolving requested changes and updating documentation) if you don't have the capacity right now to move forward here. |
There was a problem hiding this comment.
Looks like the only remaining concern is the framebuffer size which is hardcoded to the maximum possible size supported by the controller.
Do we accept the driver as it is (which also matches what the existing SH1106 driver does), and then add some way to reduce the RAM consumption for smaller displays later (updating all such drivers at the same time)?
|
I tried to make it exactly like the 1106 driver, and only introduced the single change that is needed for 1107. But I'm fine with whatever people think is best. If anyone else wants to handle any of the changes, then that's fine with me, too. I submitted this because Drashna suggested that I should. |
Given this isn't targeting AVR I'm not too concerned, so if you think it's necessary I'd suggest a follow-up PR. |
Co-authored-by: Sergey Vlasov <[email protected]>
Description
Created a SH1107 driver for Quantum Painter based on the existing SH1106 driver.
This is a small/simple change, since SH1107 and SH1106 are quite similar.
Types of Changes
Issues Fixed or Closed by This PR
Checklist