Usermod for ESP32-C3 Super Mini OLED 72x40, rebased to v0.16.x#5475
Open
doccaz wants to merge 10 commits into
Open
Usermod for ESP32-C3 Super Mini OLED 72x40, rebased to v0.16.x#5475doccaz wants to merge 10 commits into
doccaz wants to merge 10 commits into
Conversation
Port of the OLED 72x40 usermod from PR wled#5335 (originally targeting 0_15_x) to the v0.16.x main branch. Addresses all review feedback from softhack007: - Use WLED global I2C pins instead of hardcoded SDA/SCL - Remove WiFi.setTxPower() workaround (use -DLOLIN_WIFI_FIX instead) - Replace blocking delay(5000) splash with non-blocking state machine - Fix strip.isUpdating() guard to prevent display stalling - Remove direct btnPin[]/buttonType[] modification; use configurable buttonIndex with handleButton() instead - Fix bootButtonPin config persistence - Fix brightness graph off-by-one rendering bug - Use getJsonValue() 3-arg form to prevent config defaults drift - Migrate to v0.16.x usermod format (REGISTER_USERMOD, custom_usermods, appendConfigData with oappend) - Assign USERMOD_ID 59 (next available on main)
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds a new WLED usermod "OLED_72x40" that drives a U8g2 SSD1306 I2C display (72x40 UI in 128x64 buffer), implements a non-blocking splash→dashboard state machine, inactivity power-save, optional heartbeat GPIO, persistent config, registration, and a library manifest plus README. ChangesOLED_72x40 Usermod
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@usermods/OLED_72x40/OLED_72x40.cpp`:
- Around line 135-150: The LED heartbeat in loop() currently manipulates ledPin
before validating the usermod state; update loop() so it first returns if
!enabled || !initDone || !u8g2, or alternatively include those same guards in
the LED heartbeat condition, so digitalWrite()/analogWrite() are only called
when the usermod is enabled and initialized; locate the heartbeat code
referencing ledPin and ledTimer inside loop() and add the enabled/initDone/u8g2
checks (or move the heartbeat block below the existing if (!enabled || !initDone
|| !u8g2) return;) to prevent driving a GPIO when initialization failed.
- Around line 90-112: The code is incorrectly using the persisted usermod
setting enabled as a runtime error flag and leaking allocated resources on init
failure; change the init paths in the OLED_72x40 usermod so you do NOT modify
the persisted enabled flag on I2C or display begin() failures (leave enabled
untouched), introduce or use a separate runtime flag such as initialized or
available to reflect whether the display is usable, and on any early failure
after allocating ledPin call PinManager::releasePin(ledPin) (and reset ledPin =
-1) so the heartbeat pin is not permanently reserved; also ensure u8g2 is
deleted and nulled on failure while keeping enabled for config persistence and
use addToConfig() to serialize only the persisted enabled flag.
In `@usermods/OLED_72x40/README.md`:
- Around line 82-91: The example custom_usermods list is not minimal and
incorrectly enables the unrelated audioreactive usermod; update the example so
custom_usermods only includes OLED_72x40 (remove audioreactive) so readers won't
pull in unintended dependencies — edit the README example line containing
custom_usermods and ensure it reads only "custom_usermods = OLED_72x40"
(referencing the custom_usermods, audioreactive, and OLED_72x40 symbols).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1538640b-a040-4371-8f2e-036dd7c41d53
⛔ Files ignored due to path filters (1)
usermods/OLED_72x40/esp32-c3-super-mini.pngis excluded by!**/*.png
📒 Files selected for processing (4)
usermods/OLED_72x40/OLED_72x40.cppusermods/OLED_72x40/README.mdusermods/OLED_72x40/library.jsonwled00/const.h
- Add setupFailed flag to avoid persisting init failures in the enabled config setting (which would silently disable the usermod on next config save) - Deallocate heartbeat LED pin on u8g2->begin() failure to prevent pin leak - Move loop() early-return guard above heartbeat code so GPIO is not driven when usermod failed to initialize - Remove unrelated audioreactive from README custom_usermods example
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@usermods/OLED_72x40/OLED_72x40.cpp`:
- Around line 97-103: The code currently mutates the persisted configuration
variable ledPin when allocation/init fails; instead introduce a separate runtime
variable (e.g., allocatedLedPin or runtimeLedPin) and use that for calls to
PinManager::allocatePin(ledPin,...), pinMode(allocatedLedPin,...), and any pin
release logic; do not change the persisted ledPin value so addToConfig()
continues to save the original user setting. Update all places that currently
set ledPin = -1 on allocation/failure or set pinMode/release to operate on the
new runtime variable (keep ledPin read-only for config), and ensure
addToConfig() and any config serialization reference the original ledPin, not
the runtime variable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 61e8c3cf-c87a-4590-b3b9-ee783dbdfbb7
📒 Files selected for processing (2)
usermods/OLED_72x40/OLED_72x40.cppusermods/OLED_72x40/README.md
Separate ledPin (persisted config value) from activeLedPin (runtime allocation state). Previously, ledPin was set to -1 on allocation failure, which caused addToConfig() to persist -1 and erase the user's configured heartbeat pin.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
usermods/OLED_72x40/OLED_72x40.cpp (1)
71-78: Hoist the repeated defaults into named constants.
60000/60,28,24,0, and8are duplicated between the member initializers and thegetJsonValue()fallbacks. A future tweak to only one side will reintroduce config drift.Based on learnings: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR.
Also applies to: 247-257
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@usermods/OLED_72x40/OLED_72x40.cpp` around lines 71 - 78, Several numeric literals (60000, 28, 24, 0, 8 and the 72 array size) are duplicated between the member initializers (screenTimeoutMS, xOff, yOff, buttonIndex, ledPin, vValues) and the getJsonValue() fallback values; hoist these into named constants (e.g., DEFAULT_SCREEN_TIMEOUT_MS, DEFAULT_SCREEN_TIMEOUT_S if needed, DEFAULT_X_OFFSET, DEFAULT_Y_OFFSET, DEFAULT_BUTTON_INDEX, DEFAULT_LED_PIN, DEFAULT_VVALUES_SIZE) and use those constants both when initializing the members and as the fallback arguments inside getJsonValue(); update any other repeated literals in the same file (including the section referenced around getJsonValue usages) to use the new constants so defaults remain single-source-of-truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@usermods/OLED_72x40/OLED_72x40.cpp`:
- Around line 106-114: When releasing the heartbeat LED GPIO (activeLedPin)
ensure the pin is no longer electrically driven before calling
PinManager::deallocatePin; specifically, before the deallocation path in the
u8g2->begin() failure block and the other release path around lines ~258-264,
first stop driving the pin (e.g., drive it to a safe state and set it to input)
and then call PinManager::deallocatePin(activeLedPin, PinOwner::UM_Unspecified),
set activeLedPin = -1, delete u8g2 and set setupFailed = true as currently done;
update both places that currently only call PinManager::deallocatePin to perform
the “stop driving” step first to avoid leaving the GPIO actively driven.
---
Nitpick comments:
In `@usermods/OLED_72x40/OLED_72x40.cpp`:
- Around line 71-78: Several numeric literals (60000, 28, 24, 0, 8 and the 72
array size) are duplicated between the member initializers (screenTimeoutMS,
xOff, yOff, buttonIndex, ledPin, vValues) and the getJsonValue() fallback
values; hoist these into named constants (e.g., DEFAULT_SCREEN_TIMEOUT_MS,
DEFAULT_SCREEN_TIMEOUT_S if needed, DEFAULT_X_OFFSET, DEFAULT_Y_OFFSET,
DEFAULT_BUTTON_INDEX, DEFAULT_LED_PIN, DEFAULT_VVALUES_SIZE) and use those
constants both when initializing the members and as the fallback arguments
inside getJsonValue(); update any other repeated literals in the same file
(including the section referenced around getJsonValue usages) to use the new
constants so defaults remain single-source-of-truth.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 65f2ecfd-544e-419f-a828-2266044ca3b2
📒 Files selected for processing (1)
usermods/OLED_72x40/OLED_72x40.cpp
Add pinMode(activeLedPin, INPUT) before PinManager::deallocatePin() in both failure/reconfig paths so the pin is not left electrically driven after release.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@usermods/OLED_72x40/OLED_72x40.cpp`:
- Around line 133-160: When transitioning from the splash to the dashboard in
loop(), reset the inactivity timer so the dashboard isn't immediately turned
off: when you set displayState = DASHBOARD after checking millis() -
splashStartTime >= SPLASH_DURATION, also assign lastInteraction = millis() (or
equivalent) to update the inactivity timestamp; touch the block that uses
splashStartTime, displayState, SPLASH, SPLASH_DURATION, DASHBOARD and
lastInteraction to implement this change.
- Around line 244-269: readFromConfig updates the enabled flag but doesn’t bring
the live hardware into sync; update readFromConfig to handle enabled
transitions: detect oldEnabled vs enabled, and if enabled changed from
false->true call the same initialization path used by setup() (or extract the
setup body into a reusable method e.g., initHardware/initDisplay that sets
initDone, allocates activeLedPin via PinManager and calls pinMode, and
initializes the OLED), and if true->false call a teardown method (e.g.,
deinitHardware/deinitDisplay) that clears initDone, resets/inputs activeLedPin
and deallocates it via PinManager, and ensures display is powered down; reuse
symbols enabled, initDone, setup(), activeLedPin, ledPin, and PinManager to
implement this so enabling after boot or setupFailed initializes correctly and
disabling immediately cleans up hardware before any further reconfiguration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 40c4230f-702a-4ecb-8d8b-7e8957bc727c
📒 Files selected for processing (1)
usermods/OLED_72x40/OLED_72x40.cpp
- Reset lastInteraction when splash ends so the dashboard isn't immediately blanked if sleepTimeout < 5 seconds - Handle enabled->disabled transition in readFromConfig: power-save the display, release and tristate the heartbeat LED pin - Guard LED pin reallocation with enabled check so pins aren't allocated when usermod is disabled
Saving config triggers a state change event which was waking the display immediately after the teardown disabled it. Add the same enabled/setupFailed/initDone guards to onStateChange() and handleButton() so they are no-ops when the usermod is disabled. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@usermods/OLED_72x40/OLED_72x40.cpp`:
- Around line 192-194: The buffer lineBuffer used with
extractModeName(effectCurrent, JSON_mode_names, lineBuffer, 16) must be
initialized before the call to avoid drawStr reading uninitialized data when
extractModeName returns 0; initialize lineBuffer (e.g., set lineBuffer[0] = '\0'
or zero-fill the 17-byte array) immediately after its declaration (char
lineBuffer[17];) and before calling extractModeName, then call
u8g2->drawStr(currentX, currentY + 7, lineBuffer) as before.
- Around line 215-221: The onStateChange(uint8_t mode) callback (and the other
wake callbacks around lines 224-230) can re-power the display even after
readFromConfig() disabled the usermod; add an early guard that returns
immediately when the usermod is disabled (e.g., if (!enabled) return; or if
(!isEnabled()) return;) before touching u8g2, displayState, lastInteraction, or
lastUpdate so callbacks remain inert while the usermod is disabled.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: efb3be77-4499-48c7-88a7-f877f6eafbbb
📒 Files selected for processing (1)
usermods/OLED_72x40/OLED_72x40.cpp
Zero-initialize lineBuffer so drawStr() doesn't read garbage if extractModeName() returns 0 for an out-of-range mode index.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
usermods/OLED_72x40/OLED_72x40.cpp (1)
123-127: Consider extracting the panel/layout constants.
128,64,72,40, and the fixed y-baselines are repeated in several coordinate calculations. Giving themstatic constexprnames would make the flip/layout math easier to audit.Based on learnings, replacing meaningful magic numbers with named constants improves maintainability and helps avoid layout regressions.
Also applies to: 187-209
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@usermods/OLED_72x40/OLED_72x40.cpp` around lines 123 - 127, Extract the repeated layout numbers into named constants and use them in the flip math: define static constexpr ints like SCREEN_WIDTH (128), SCREEN_HEIGHT (64), PANEL_WIDTH (72), PANEL_HEIGHT (40), LOGO_WIDTH (16) and optional X_BASELINE/Y_BASELINE to replace the raw literals used when computing splashY and textX; update calculations that reference flipDisplay, xOff, yOff, splashY and textX (and the bitmap draw call u8g2->drawBitmap and subsequent layout code around lines 187-209) to use those constants so the flip/layout math is clearer and consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@usermods/OLED_72x40/OLED_72x40.cpp`:
- Around line 264-286: The code only attempts to allocate activeLedPin when
oldLedPin != ledPin, so if the usermod was disabled (which clears activeLedPin)
but the configured ledPin didn't change you never retry allocation; change the
logic to also attempt allocation whenever enabled && initDone && activeLedPin <
0. Concretely, keep the existing branch that handles pin changes (if oldLedPin
!= ledPin) but add (or replace with) a separate branch that runs when enabled &&
initDone && activeLedPin < 0 && ledPin >= 0 and calls
PinManager::allocatePin(ledPin, true, PinOwner::UM_Unspecified), sets
activeLedPin and pinMode(activeLedPin, OUTPUT) on success; ensure you still
deallocate any previous activeLedPin before allocating (using pinMode(...,
INPUT) and PinManager::deallocatePin) as in the current code paths.
---
Nitpick comments:
In `@usermods/OLED_72x40/OLED_72x40.cpp`:
- Around line 123-127: Extract the repeated layout numbers into named constants
and use them in the flip math: define static constexpr ints like SCREEN_WIDTH
(128), SCREEN_HEIGHT (64), PANEL_WIDTH (72), PANEL_HEIGHT (40), LOGO_WIDTH (16)
and optional X_BASELINE/Y_BASELINE to replace the raw literals used when
computing splashY and textX; update calculations that reference flipDisplay,
xOff, yOff, splashY and textX (and the bitmap draw call u8g2->drawBitmap and
subsequent layout code around lines 187-209) to use those constants so the
flip/layout math is clearer and consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 63276b47-3a0d-4afb-9a23-3726d8b22d4d
📒 Files selected for processing (1)
usermods/OLED_72x40/OLED_72x40.cpp
When the usermod is disabled, activeLedPin is cleared. If re-enabled without changing the configured pin, the reallocation condition (oldLedPin != ledPin) was false, leaving the heartbeat permanently off. Now also triggers when ledPin is configured but activeLedPin is unset.
DedeHai
reviewed
Apr 6, 2026
Avoids pulling in the math library, saving flash. Uses WLED's integer-based sin_approx() from wled_math.cpp as suggested by DedeHai.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
usermods/OLED_72x40/OLED_72x40.cpp (1)
251-289:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winWake the panel on config/API updates.
When the OLED is already asleep,
readFromConfig()applies the new settings but leaveslastInteractionstale anddisplayStateatOFF, so the timeout guard inloop()keeps short-circuiting and the updated config stays invisible until some unrelated button/state event happens.Suggested fix
if (enabled && initDone && (oldLedPin != ledPin || (ledPin >= 0 && activeLedPin < 0))) { if (activeLedPin >= 0) { pinMode(activeLedPin, INPUT); PinManager::deallocatePin(activeLedPin, PinOwner::UM_Unspecified); } activeLedPin = -1; if (ledPin >= 0 && PinManager::allocatePin(ledPin, true, PinOwner::UM_Unspecified)) { activeLedPin = ledPin; pinMode(activeLedPin, OUTPUT); } } + if (enabled && initDone && u8g2) { + lastInteraction = millis(); + lastUpdate = 0; + if (displayState == OFF) { + u8g2->setPowerSave(0); + displayState = DASHBOARD; + } + } + if (initDone && u8g2) u8g2->setFlipMode(flipDisplay ? 1 : 0); return configComplete;As per coding guidelines, "sleep/blanking is inactivity-based and wakes on button interaction or settings/API changes."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@usermods/OLED_72x40/OLED_72x40.cpp` around lines 251 - 289, readFromConfig() updates settings while the panel may be asleep; ensure it wakes the display so new config is visible by updating lastInteraction and clearing power-save. After applying config (e.g. where initDone is true and enabled is true), if displayState == OFF then set lastInteraction = millis(), set displayState = ON, and if (u8g2) call u8g2->setPowerSave(0); also handle the enabled transition from false->true similarly (the block handling enabled && initDone ... or after reallocation of activeLedPin are good insertion points). This uses the symbols readFromConfig, lastInteraction, displayState, OFF, ON, u8g2, initDone, and enabled.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@usermods/OLED_72x40/OLED_72x40.cpp`:
- Around line 251-289: readFromConfig() updates settings while the panel may be
asleep; ensure it wakes the display so new config is visible by updating
lastInteraction and clearing power-save. After applying config (e.g. where
initDone is true and enabled is true), if displayState == OFF then set
lastInteraction = millis(), set displayState = ON, and if (u8g2) call
u8g2->setPowerSave(0); also handle the enabled transition from false->true
similarly (the block handling enabled && initDone ... or after reallocation of
activeLedPin are good insertion points). This uses the symbols readFromConfig,
lastInteraction, displayState, OFF, ON, u8g2, initDone, and enabled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8275a29f-f8a1-4d42-b7bb-7b693373cd46
📒 Files selected for processing (1)
usermods/OLED_72x40/OLED_72x40.cpp
coderabbitai
Bot
added
the
documentation
softhack007
reviewed
May 23, 2026
| -D I2CSCLPIN=6 ; OLED I2C SCL pin | ||
| -D DATA_PINS=10 ; LED strip data output pin | ||
| -D BTNPIN=9 ; Onboard boot button used as pushbutton | ||
| -D BTNTYPE=BTN_TYPE_PUSH |
Member
There was a problem hiding this comment.
@coderabbitai please check if these defines are aligned with the build_flags supported in the WLED core (wled00/).
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check each build flag used in the README against wled00/ source
echo "=== I2CSDAPIN ==="
rg -n "I2CSDAPIN" --type cpp --type h -C2
echo "=== I2CSCLPIN ==="
rg -n "I2CSCLPIN" --type cpp --type h -C2
echo "=== DATA_PINS ==="
rg -n "DATA_PINS" --type cpp --type h -C2
echo "=== BTNPIN ==="
rg -n "BTNPIN" --type cpp --type h -C2
echo "=== BTNTYPE ==="
rg -n "\bBTNTYPE\b" --type cpp --type h -C2
echo "=== BTN_TYPE_PUSH ==="
rg -n "BTN_TYPE_PUSH" --type cpp --type h -C2
echo "=== Check platformio.ini or wled00/cfg.h for pin defines ==="
fd "platformio.ini" --exec cat {}
Length of output: 42309
@softhack007 — all five build flags in the README example are correctly recognized by the WLED core:
| Flag | Core location | Status |
|---|---|---|
-D I2CSDAPIN=5 |
wled00/wled.h (initialises i2c_sda), wled00/const.h (sets HW_PIN_SDA) |
✅ Valid |
-D I2CSCLPIN=6 |
wled00/wled.h (initialises i2c_scl), wled00/const.h (sets HW_PIN_SCL) |
✅ Valid |
-D DATA_PINS=10 |
wled00/cfg.cpp (consumed for default LED bus setup) |
✅ Valid |
-D BTNPIN=9 |
wled00/wled.h (default = 0), wled00/cfg.cpp |
✅ Valid |
-D BTNTYPE=BTN_TYPE_PUSH |
wled00/wled.h, wled00/cfg.cpp |
✅ Valid, but redundant |
One issue worth noting: -D BTNTYPE=BTN_TYPE_PUSH is redundant. In wled00/wled.h lines 291–293:
`#ifndef` BTNTYPE
`#define` BTNTYPE BTN_TYPE_PUSH
`#endif`BTN_TYPE_PUSH is already the default when BTNTYPE is not defined. It won't break anything, but it's unnecessary noise in the example. The README could simply omit that line, or at least add a comment clarifying it's only needed if a non-default type is wanted.
Also worth noting for completeness: the example env extends env:esp32c3dev, which already includes -DLOLIN_WIFI_FIX in the base platformio.ini, so that flag is inherited automatically — consistent with the PR's stated goal of removing the WiFi workaround and relying on -DLOLIN_WIFI_FIX from the base environment.
softhack007
reviewed
May 23, 2026
| u8g2->setPowerSave(0); | ||
| displayState = DASHBOARD; | ||
| } | ||
| return false; // let WLED core handle the actual button action |
Member
There was a problem hiding this comment.
should this be return true; ?
softhack007
reviewed
May 23, 2026
| } | ||
|
|
||
| u8g2 = new U8G2_SSD1306_128X64_NONAME_F_HW_I2C(U8G2_R0, U8X8_PIN_NONE); | ||
|
|
Contributor
There was a problem hiding this comment.
@softhack007 — U8G2_R0 is the display rotation argument passed to the U8g2 constructor. Specifically, it means no rotation, landscape orientation. The full set of valid values is:
| Value | Meaning |
|---|---|
U8G2_R0 |
No rotation, landscape |
U8G2_R1 |
90° clockwise rotation |
U8G2_R2 |
180° clockwise rotation |
U8G2_R3 |
270° clockwise rotation |
U8G2_MIRROR |
No rotation, landscape, display content mirrored (v2.6.x+) |
U8G2_MIRROR_VERTICAL |
Display content vertically mirrored (v2.29.x+) |
So in this usermod, U8G2_R0 starts the display in its default unrotated landscape mode. The 180° flip (for when the board is mounted upside down) is then handled separately at runtime via the flipDisplay config flag and U8g2's setFlipMode() call — which keeps the hardware rotation fixed while allowing a software-level flip without recreating the driver object.
🧠 Learnings used
Learnt from: softhack007
Repo: wled/WLED PR: 5355
File: wled00/util.cpp:635-638
Timestamp: 2026-02-07T16:06:08.677Z
Learning: PSRAM-related compilation guards should enable PSRAM code only for ESP32 variants that actually include PSRAM: ESP32-C61, ESP32-C5, and ESP32-P4. Exclude ESP32-C3, ESP32-C6, and ESP8266 from these guards. Apply this rule across the codebase (not just wled00/util.cpp) by reviewing and updating PSRAM guards/macros in all relevant files (C/C++ headers and sources).
Learnt from: softhack007
Repo: wled/WLED PR: 4838
File: lib/NeoESP32RmtHI/src/NeoEsp32RmtHIMethod.cpp:30-35
Timestamp: 2026-03-27T12:33:48.499Z
Learning: In C/C++ preprocessor conditionals (`#if`, `#elif`) GCC/Clang treat `&&` as short-circuit evaluated during preprocessing. This means guards like `#if defined(ARDUINO_ARCH_ESP32) && ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0)` are safe even if the macro/function-like macro on the RHS (e.g., `ESP_IDF_VERSION_VAL`) is not defined on some targets, because the RHS will not be expanded when the LHS is false (e.g., `defined(...)` evaluates to 0). During code review, avoid flagging such cases as “undefined function-like macro invocation” if they are protected by short-circuiting `defined(...) && ...`/`||` logic; some tools like cppcheck may not model this and can produce false positives. Also, don’t suggest refactoring that moves ESP32-specific includes/headers (e.g., `esp_idf_version.h`) outside of these guarded preprocessor blocks, since that will break targets (e.g., ESP8266) where the headers don’t exist.
Learnt from: softhack007
Repo: wled/WLED PR: 4843
File: usermods/elastic_collisions/Elastic_Collisions.cpp:1-1
Timestamp: 2026-05-06T22:10:27.517Z
Learning: In C/C++, do not flag precedence issues for expressions that mix additive operators (`+`, `-`) with shift operators (`<<`, `>>`) based on the assumption that shifts bind tighter. Per C/C++ operator precedence, `+`/`-` have higher precedence than `<<`/`>>` (e.g., `x - edge0 << 8` parses as `(x - edge0) << 8`). When reviewing WLED (and other) fixed-point/bit-manipulation code, confirm the intended parse using cppreference before reporting a precedence bug for mixed `-`/`+` and `<<`/`>>` expressions.
Learnt from: softhack007
Repo: wled/WLED PR: 5599
File: usermods/audioreactive/audio_reactive.cpp:1303-1308
Timestamp: 2026-05-11T16:00:13.574Z
Learning: When targeting arduino-esp32 v3.x (ESP-IDF 5.x), treat `WiFiUDP::flush()` as deprecated and ineffective for RX draining; it should not be used to clear/drain the UDP receive buffer. Instead, use `WiFiUDP::clear()`, which was introduced in arduino-esp32 v3.0 and is marked as the deprecated replacement via `NetworkUdp.h`.
If the code must compile and behave correctly across both arduino-esp32 v2.x (ESP-IDF < 5) and v3.x (ESP-IDF >= 5), use conditional compilation:
- for `ESP_IDF_VERSION_MAJOR < 5`: use `udp.flush()`
- for `ESP_IDF_VERSION_MAJOR >= 5`: use `udp.clear()`
Do not flag `WiFiUDP::clear()` usages for ESP-IDF >= 5 as non-existent—this is the documented correct replacement.
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
4 participants
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.
Port of the OLED 72x40 usermod from PR #5335 (originally targeting 0_15_x) to the v0.16.x main branch. Addresses all review feedback from softhack007:
Summary by CodeRabbit
New Features
Documentation
Chores