-
Notifications
You must be signed in to change notification settings - Fork 7
Add support for ssd1315 displays #11
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
base: main
Are you sure you want to change the base?
Conversation
|
Thank you for the PR. Lets improve PR title to something like Lets improve PR description to something like: Finally it helps if you mentions it's tested on hardware and what specific hardware.. And also squash commits, unless it makes sense to have separate ones. Buenos tardes y gracias! |
bettio
left a comment
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.
I just left 2 minor comments, also I agree with @petermm
Everything else looks good, thanks for the contribution. I'm looking forward merging this PR.
5b8792d to
92dc9ee
Compare
92dc9ee to
f3f4721
Compare
ssd1306_display_driver.c
Outdated
| #define CHAR_WIDTH 8 | ||
|
|
||
| #define I2C_ADDRESS 0x3C | ||
| #define I2C_CHUNK_SIZE 64 |
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.
I2C_CHUNK_SIZE is not used anywhere, so let's remove line.
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.
solved
petermm
left a comment
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.
Tested ssd1306 on simulator, as working.. so LGTM besides that stray I2C_CHUNK_SIZE
ssd1306_display_driver.c
Outdated
| i2c_master_write_byte(cmd, 0xAD, true); // Internal IREF Setting | ||
| i2c_master_write_byte(cmd, 0x10, true); // Internal Iref | ||
|
|
||
| i2c_master_write_byte(cmd, 0xAF, true); // Display ON |
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.
this 0xAF is redundant as it's being sent later CMD_DISPLAY_ON for all displays..
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.
solved
This PR adds supporf for ssd1315 displays. ssd1315 is mostly compatible with ssd1306 drivers, but it has some small differences, such as requiring a special initialization sequence and needing an explicit column address reset when updating. I also increased the timeout window from 10 to 100, to fix this error i was getting: `E (1225) SSD1306: I2C write failed for display update: 0x107` [(a timeout)](https://docs.espressif.com/projects/esp-idf/en/stable/esp32/api-reference/error-codes.html) The initialization sequence is based on the one used in [u8g2](https://github.com/olikraus/u8g2/blob/dbd338af5d57b219e48ea1ffdb97c4153668676c/csrc/u8x8_d_ssd1315_128x64_noname.c). Tested on an ESP32.
b253671 to
b937a15
Compare

This PR adds supporf for ssd1315 displays. ssd1315 is mostly compatible with ssd1306 drivers, but it has some small differences, such as requiring a special initialization sequence and needing an explicit column address reset when updating.
I also increased the timeout window from 10 to 100, to fix this error i was getting:
E (1225) SSD1306: I2C write failed for display update: 0x107(a timeout)The initialization sequence is based on the one used in u8g2.
Tested on an ESP32.