[ssd1306] suggest a different SPI mode to reduce visual glitching#6217
[ssd1306] suggest a different SPI mode to reduce visual glitching#6217swoboda1337 merged 1 commit intoesphome:currentfrom
Conversation
There was a problem hiding this comment.
As this is a feature matched with a PR in https://github.com/esphome/esphome, please target your PR to the next branch and rebase.
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
✅ Deploy Preview for esphome ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
I kept the reference but remove the link so now the bot is fine :) |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdded two TIP blocks to the SSD1306 display documentation noting potential SPI mode conflicts that can corrupt display content; documents that the datasheet specifies MODE3 but some drivers use MODE0 and recommends trying Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/content/docs/components/display/ssd1306.mdx (2)
133-137: Use a docs callout block for this SPI-mode advisory.This reads like cautionary guidance; formatting it as a
NOTE/TIPimproves scannability and consistency.As per coding guidelines: "Use GitHub-flavored alert syntax (
> [!NOTE],> [!WARNING],> [!TIP]) for alert boxes in MDX files".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/content/docs/components/display/ssd1306.mdx` around lines 133 - 137, Convert the advisory paragraph about SPI mode into a GitHub-flavored MDX callout (e.g., > [!TIP] or > [!NOTE]) rather than plain text: wrap the existing SPI-mode guidance about MODE3/MODE0 and the suggested spi_mode: MODE0 workaround in a single callout block, keep all technical details and the link to /components/spi#spi-modes intact, and ensure the callout label matches tone (use [!TIP] or [!NOTE]) so it renders as an alert box in the SSD1306 docs content.
136-136: Clarify wherespi_mode: MODE0must be configured.Line 136 tells users to add
spi_mode, but doesn’t show the exact YAML scope (spi:block). A tiny snippet here would prevent misconfiguration.Proposed doc tweak
- Some users reported that adding a manual `spi_mode: MODE0` helps reduce glitching. + Some users reported that explicitly setting SPI mode in the `spi:` block to `MODE0` helps reduce glitching. + + ```yaml + spi: + clk_pin: D0 + mosi_pin: D1 + spi_mode: MODE0 + ```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/content/docs/components/display/ssd1306.mdx` at line 136, Clarify that spi_mode must be set under the spi: YAML mapping by adding a small example showing the spi: block with clk_pin, mosi_pin and spi_mode: MODE0 so users know the exact scope to place spi_mode (reference symbols: spi:, spi_mode, clk_pin, mosi_pin, MODE0); insert the tiny YAML snippet immediately after the sentence mentioning "add `spi_mode`" to demonstrate the correct placement and prevent misconfiguration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/content/docs/components/display/ssd1306.mdx`:
- Around line 133-135: Fix the two copy issues in the SSD1306 doc: replace the
phrase "presence of other SPI devices" where the sentence currently reads
"presence of other SPI devices" missing "the" by updating the sentence to "the
presence of other SPI devices", and change the project name casing from
"esphome" to "ESPHome" (the lowercase "esphome" token in the paragraph about SPI
modes/SSD1306). Locate and update the corresponding strings in
src/content/docs/components/display/ssd1306.mdx (the sentence mentioning SPI
devices and the occurrence of "esphome") so the grammar and casing are
corrected.
---
Nitpick comments:
In `@src/content/docs/components/display/ssd1306.mdx`:
- Around line 133-137: Convert the advisory paragraph about SPI mode into a
GitHub-flavored MDX callout (e.g., > [!TIP] or > [!NOTE]) rather than plain
text: wrap the existing SPI-mode guidance about MODE3/MODE0 and the suggested
spi_mode: MODE0 workaround in a single callout block, keep all technical details
and the link to /components/spi#spi-modes intact, and ensure the callout label
matches tone (use [!TIP] or [!NOTE]) so it renders as an alert box in the
SSD1306 docs content.
- Line 136: Clarify that spi_mode must be set under the spi: YAML mapping by
adding a small example showing the spi: block with clk_pin, mosi_pin and
spi_mode: MODE0 so users know the exact scope to place spi_mode (reference
symbols: spi:, spi_mode, clk_pin, mosi_pin, MODE0); insert the tiny YAML snippet
immediately after the sentence mentioning "add `spi_mode`" to demonstrate the
correct placement and prevent misconfiguration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 697f2eeb-a678-4b49-a0e7-960a0f3605d9
📒 Files selected for processing (1)
src/content/docs/components/display/ssd1306.mdx
There was a problem hiding this comment.
Pull request overview
Adds a troubleshooting note to the SSD1306 SPI display documentation about potential display corruption when sharing an SPI bus with other devices. Suggests that users try setting spi_mode: MODE0 as a workaround.
Changes:
- Added a paragraph to the SSD1306 SPI section describing SPI mode conflicts and a potential workaround using
MODE0
You can also share your feedback on Copilot code review. Take the survey.
e098c33 to
eb1f056
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/content/docs/components/display/ssd1306.mdx (1)
134-137: Soften the claim to keep the troubleshooting advice hardware-agnostic.At Line 137, “helps reduce glitching” reads as broadly guaranteed. Consider “may help reduce glitching” to match the anecdotal/context-dependent framing in Line 134–136.
Suggested wording tweak
-> use `MODE0` for accessing the display. Using `spi_mode: MODE0` helps reduce glitching. +> use `MODE0` for accessing the display. Using `spi_mode: MODE0` may help reduce glitching.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/content/docs/components/display/ssd1306.mdx` around lines 134 - 137, The sentence “Using `spi_mode: MODE0` helps reduce glitching.” should be softened to reflect anecdotal evidence; update that phrase to “may help reduce glitching” so the advice remains hardware-agnostic and consistent with the preceding conditional wording about interference and user reports (referencing the existing mention of `spi_mode: MODE0` and the surrounding troubleshooting paragraph).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/content/docs/components/display/ssd1306.mdx`:
- Around line 134-137: The sentence “Using `spi_mode: MODE0` helps reduce
glitching.” should be softened to reflect anecdotal evidence; update that phrase
to “may help reduce glitching” so the advice remains hardware-agnostic and
consistent with the preceding conditional wording about interference and user
reports (referencing the existing mention of `spi_mode: MODE0` and the
surrounding troubleshooting paragraph).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2d1a6a98-aa78-46cc-bc40-99dcbb775380
📒 Files selected for processing (1)
src/content/docs/components/display/ssd1306.mdx
I'm not a huge fan of these weasel words, but I have seen visual glitches on this display when combined with other SPI devices (see esphome/esphome#13683). On Discord, people have suggested experimenting with a different SPI mode, and indeed, that solved the problem for me. I am not suggesting to change this chip's SPI mode (the picture in the datasheet is not 100% correct, but there are strong hints that mode3 is the intended mode). At the same time, other implementations like the Adafruit one use mode 0 successfully. So, at a suggestion of @swoboda1337, let's at least document these findings.
eb1f056 to
bd1f3a1
Compare
|
Thanks! |
I'm not a huge fan of these weasel words, but I have seen visual glitches on this display when combined with other SPI devices (see esphome pr 13683). On Discord, people have suggested experimenting with a different SPI mode, and indeed, that solved the problem for me.
I am not suggesting to change this chip's SPI mode (the picture in the datasheet is not 100% correct, but there are strong hints that mode3 is the intended mode). At the same time, other implementations like the Adafruit one use mode 0 successfully.
So, at a suggestion of @swoboda1337, let's at least document these findings.
Checklist
currentbecause this is a fix, change and/or adjustment in the current documentation and is not for a new component or feature.