-
Notifications
You must be signed in to change notification settings - Fork 7.4k
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
Support SSD1306 128x32 (IDFGH-11296) #12450
Conversation
👋 Welcome storoj, thank you for your first contribution to 📘 Please check Contributions Guide for the contribution checklist, information regarding code and documentation style, testing and other topics. 🖊️ Please also make sure you have read and signed the Contributor License Agreement for espressif/esp-idf project. Pull request review and merge process you can expectEspressif develops the ESP-IDF project in an internal repository (Gitlab). We do welcome contributions in the form of bug reports, feature requests and pull requests via this public GitHub repository.
|
This change fixes the issue #9277. |
Sorry, there's one more initialization command that's missing. I will update my PR shortly. |
a1d7062
to
5a130d5
Compare
SSD1306 may have different sizes and resolutions: 128x32, 128x64, 96x16 and others. `SSD1306_CMD_SET_MULTIPLEX` (0xA8) initialization command effectively sets the height of the display's resolution. The value for this command is the height in pixels minus one. ``` Set MUX ratio to N+1 MUX N=A[5:0] : from 16MUX to 64MUX, RESET= 111111b (i.e. 63d, 64MUX) A[5:0] from 0 to 14 are invalid entry. ``` `SSD1306_CMD_SET_COMPINS` (0xDA) configures the communication pins. According to the datasheet it should be `0x2` with other optional bits set. ``` A[4]=0b, Sequential COM pin configuration A[4]=1b(RESET), Alternative COM pin configuration A[5]=0b(RESET), Disable COM Left/Right remap A[5]=1b, Enable COM Left/Right remap ``` * https://github.com/adafruit/Adafruit_SSD1306; * https://www.digikey.com/htmldatasheets/production/2047793/0/0/1/ssd1306.html
5a130d5
to
8f75c4c
Compare
* | ||
* esp_lcd_panel_ssd1306_config_t ssd1306_config = { | ||
* .width = 128, | ||
* .height = 32 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems like this example usage is different from the driver code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But compared to the
typedef struct {
/**
* @brief Multiplex ratio: (0..63)
*
* Display's height minus one.
*/
uint8_t mux_ratio;
/**
* @brief Enables alternative COM pin configuration.
*
* When unset then Sequential COM pin configuration is used.
*/
bool com_pin_alt;
/**
* @brief COM Left/Right remap.
*/
bool com_lr_remap;
} esp_lcd_panel_ssd1306_config_t;
This example version looks more intuitive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@suda-morris thanks for your comments, good catch on mismatched documentation comment. I had several iterations on the driver and forgot to update the docs.
First I wanted to provide simpler interface with just width
and height
to make it easier to use. However later I reconsidered this approach because:
- width seems to never be needed;
- I'm not sure that height is enough to reliably configure a display;
- ... which may be even more confusing to developers when they first try the driver with the default settings (128x64), it fails, then they find this config, set the correct values and it fails again because for example it's a display from another manufacturer. In that case it will be quite hard to override lcd commands sent to display.
I decided to switch to more "generic" (but less intuitive) API to provide developers more flexibility while still being high-level enough to not send raw lcd commands. Maybe I should have mentioned typical config parameters for the most popular display sizes (128x64, 128x32, 96x16 and maybe others) in the documentation header.
Or it could be a good idea to provide public constants like
extern const esp_lcd_panel_ssd1306_config_t esp_lcd_panel_ssd1306_128x32 = {
.mux_ratio = 0x2,
.com_pin_alt = false,
.com_lr_remap = false
}
... but I'm not quite sure about naming and overall coding style, that's my very first touch of the embedded area.
@suda-morris what do you think I should prefer:
- mention typical configs for different screens in the documentation comment;
- provide constants with configs for known display types;
I lean towards the constants path. If you agree could you please guide me on the coding style and naming I should use? I struggle with that so far.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @storoj I would prefer the first approach. As you also said, different manufacturers may require different settings. We can't provide the setting for all known display types in esp-idf as well, because that's the job of a BSP package. Ideally, the user will define his own BSP package like these, and in there, he can predefine the value of esp_lcd_panel_ssd1306_config_t according to his manufacture.
@storoj Please install and use pre-comit hook for pass tests: https://docs.espressif.com/projects/esp-idf/en/latest/esp32/contribute/install-pre-commit-hook.html |
sha=8f75c4c5249c3a85d1e6e89cd7bd744183155d35 |
merged in c4dc16c |
For those that don't want to use another library just to get their 32px stuff going until this is this code is actually released and included everywhere... I just did this workaround that worked fine to me (stole the code from the esp-bsp SSD1306 library if it looks familiar): https://github.com/RobustoFramework/Robusto/blob/main/components/robusto/ui/src/robusto_32px.c I call it here, just after the initialization to overwrite some of the settings the normal flow sends:
It is not very invasive and easy to remove when done. |
SSD1306 may have different sizes and resolutions: 128x32, 128x64, 96x16
and others.
SSD1306_CMD_SET_MULTIPLEX
(0xA8) initialization command effectivelysets the height of the display's resolution. The value for this command
is the height in pixels minus one.
SSD1306_CMD_SET_COMPINS
(0xDA) configures the communication pins.According to the datasheet it should be
0x2
with other optional bitsset.