Repository navigation
Fix SSD1306 128x32: wrong column-start address + batched I2C page writes - #1
Merged
Merged
Conversation
Fixes daisyaudio#634. SSD130xDriver::Update() (and the equivalent switches in SSD1307Driver) set the page column-start command to 0x12 (column 32) for height==32 panels instead of the standard 0x10 (column 0) used by the default case and every reference SSD1306 driver. Every page write started 32 columns in: the intended content shifted 32 columns right, and the last 32 columns wrapped back around to the left edge, garbling whatever was drawn there. Confirmed independently by three reports on daisyaudio#634 (including a maintainer) against SSD130x4WireSpi128x32Driver; the same class also backs the I2C 128x32 alias. Also fixes the same line in SSD1307Driver::TransferPageDma() for consistency, though no public alias currently instantiates that class at height==32.
SSD130xI2CTransport::SendData() issued a full I2C transaction (START+address+ACK+STOP) per byte -- 512 transactions for one full 128x32 frame (4 pages x 128 bytes), plus ~12 more for per-page setup commands. The SSD1306 auto-increments its column pointer for every data byte that follows a single 0x40 prefix within a transaction, so there's no need to restart the bus per byte. Batches a page's data into one transaction (page bytes + the 0x40 prefix); measured on a 128x32 panel, this cut a full-screen Update() from ~30-40ms to a few ms. Separate from the column-start fix in the previous commit -- this is a transport-layer performance change, not a correctness fix, so it's easy to drop if unwanted.
Author
|
Probably worth to mention here as a reference, this issue has been open since 2024 at daisyaudio#634 |
Author
|
Since this was a rather small tweak I thought doing it via a small PR would be reversible if needed, so i went ahead and already merged it. But on second thought, maybe I should've checked with @bleeptools first? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two fixes to the vendored SSD1306 driver, found during TouchPlaited's OLED bring-up:
high_column_addrwas0x12(column 32) forheight==32instead of the standard0x10(column 0), so every page write started 32 columns in and wrapped. Matches electro-smith/libDaisy#634, confirmed independently by a libDaisy maintainer and two other users.Also opening this against electro-smith upstream directly — merging here first so TouchPlaited's submodule pin has a real ref to fetch from instead of an unpublished local commit.