Conversation
… during transition much nicer look and no more color inaccuracies
|
Not tried it, but sounds exactly what one of my customers wanted from my lamps |
|
it also was requested somewhere before, I saw it more than once but could not find the references. In a nutshell this is the "sunrise FX" but instead of a fixed sunrise pattern, it allows to use any palette/color as a starting point and as a destination. I am not sure it is very easy to use though: if you just call it once it does nothing but initialize, so you have to use it either in a playlist or in a "chain of presets" |
WalkthroughAdds a new FX mode "Slow Transition" that performs long-duration palette transitions using per-segment persistent data, adjusts real-time progress tracking, and updates palette rendering. Also changes Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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. 📋 Issue PlannerBuilt with CodeRabbit's Coding Plans for faster development and fewer bugs. View plan used: ✨ Finishing Touches🧪 Generate unit tests (beta)
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}
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 `@wled00/FX.cpp`:
- Around line 10830-10836: The transition uses the sync-adjusted clock
(strip.now) causing jumps; change all uses of strip.now in this sunrise-like
effect to the real-time millis() clock: when initializing the timer set
*startTime = millis(); compute current = millis(); then use elapsed = current -
*startTime (with uint32_t arithmetic) to calculate expectedSteps (leave
totalSteps and duration logic unchanged). Replace references to strip.now in the
startTime assignment and the elapsed calculation so this mode mirrors
mode_sunrise() behavior and is not affected by strip time sync adjustments.
- Around line 10797-10801: The preset handoff currently repeatedly calls
applyPreset() each render after the fade completes; modify the handoff code that
uses slow_transition_data (SlowTransitionData / slow_transition_data) so it sets
a one-shot latch (e.g., a boolean flag) when the transition finishes, invokes
applyPreset() only once, and immediately return from the render path after
performing that single handoff to avoid continued rendering with stale data;
apply the same one-shot latch + early-return pattern to the other two handoff
blocks referenced (the similar blocks around the other locations).
- Around line 10839-10849: The sweep branch (when SEGMENT.check2 is true) jumps
*stepsDone to expectedSteps and only applies the final microstep, losing
intermediate palette-entry updates; change the logic in the block using
*stepsDone, expectedSteps, data->currentPalette, data->startPalette,
data->endPalette, CRGB/CRGBW and color_blend so that you advance step-by-step
for any skipped microsteps: iterate from the previous stepsDone+1 up to
expectedSteps and for each step compute i = step % 16 and blendAmount = step /
16 and update data->currentPalette[i] accordingly (instead of applying only the
final step), then set *stepsDone = expectedSteps after the loop.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c313c770-8556-4149-b475-3255b2f43376
📒 Files selected for processing (3)
wled00/FX.cppwled00/FX.hwled00/FX_fcn.cpp
wled00/FX.cpp
Outdated
| data->currentPalette = data->endPalette; // set to end palette (sweep may not have set all entries) | ||
| if (SEGMENT.intensity > 0) { | ||
| uint8_t targetPreset = SEGMENT.intensity; | ||
| applyPreset(targetPreset, CALL_MODE_DIRECT_CHANGE); // apply preset (if it exists) |
There was a problem hiding this comment.
@willmmiles what do you think of this line? i.e. applying a preset from an FX? this is the only thing I find questionable about this PR but it works. If you see no bigger issues with this, I will fix it up and merge.
if there is potential issues in the future, we should do it differently: as soon as this one is out, people will use it and it will need to be maintained - or broken.
There was a problem hiding this comment.
Technically it is safe in this context. Contrary to the name, applyPreset() doesn't actually apply the preset, it just schedules it to be applied on the next pass through the main loop. There's no queue, though, so it may interact strangely if there's a button press or http call in flight at the same time. (That's not new though.)
Architecturally, I don't love the interface design -- adding another system for time-based preset firing in parallel with playlists and timer slots feels like we're asking for subsystems to trip over each other. I also recognize that there's value in ensuring synchronicity between overlay effects and play state (ie. without having to make sure that the playlist entry duration exactly matches the effect ramp time).
Although now that I've typed it out, it occurs to me that maybe having an effect that reads the remaining playlist step duration might solve this problem just as well -- and then the "next preset to select" can be left to the playlist logic, with all the expanded power that has (random preset selection, etc.). What do you think?
There was a problem hiding this comment.
the point of this PR is to enable VERY slow transitions, playlist have a 999s "per entry" time limit where as this PR aims for 255min. It suffers from the same discrepancies noted in the copy FX of an FX stepping over its (architectural) boundaries, which I also do not like.
While the copy FX could be moved to the segment, the slow transition can't, at least I see no simple way to do it.
There was a problem hiding this comment.
the point of this PR is to enable VERY slow transitions, playlist have a 999s "per entry" time limit
I think the documentation might be out of date on that - as far as I can tell from the playlist code, the current limitation is 6553 seconds (109 minutes, 1.82 hours). I'd support removing that limitation - using a 32-bit integer for playlist times isn't a huge memory cost.
It suffers from the same discrepancies noted in the copy FX of an FX stepping over its (architectural) boundaries, which I also do not like.
While the copy FX could be moved to the segment, the slow transition can't, at least I see no simple way to do it.
I am actually a fan of the copy FX implementation! I agree it's a different "kind" of effect, and I would be entirely on board with putting it in a different category in the UI, but I think the ability of our engine to support meta-FX like that is a sign of a good design. I like the idea of a fully composable rendering pipeline. (So many ideas, so little time...)
If we lifted the playlist time limitation, do you think it would be better use "playlist progress" instead of directly applying the next effect?
There was a problem hiding this comment.
If we lifted the playlist time limitation, do you think it would be better use "playlist progress" instead of directly applying the next effect?
Absoluteley. The playlist has another limit applied, the 999s is real. Will check where that is applied and see if it can be lifted. If playlists allow for longer durations, there is no need for this FX to be able to trigger presets.
edit: turns out if you enter 9999 it truncates to 999 and not the max allowed value 6553
I am actually a fan of the copy FX implementation! I agree it's a different "kind" of effect, and I would be entirely on board with putting it in a different category in the UI, but I think the ability of our engine to support meta-FX like that is a sign of a good design. I like the idea of a fully composable rendering pipeline. (So many ideas, so little time...)
I also like the concept of the copy FX, its a very useful tool to have and I would not want to remove it. Just the fact that it accesses other segment's buffers might become an issue at one point.
|
@coderabbitai please trace through the code from UI to preset handling and check where limits to playlist item duration are applied. Check if removing the restriction and making the duration a uint32 is possible or if there are potential side-effects. Provide a suggestion on how to implement longer playlist item duration. |
This comment was marked as outdated.
This comment was marked as outdated.
|
@coderabbitai in playlists there is also a transition between entries. explain how the transition is applied and what time limits are set and how they are restricted. |
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
wled00/FX.cpp (1)
10795-10799:⚠️ Potential issue | 🟠 MajorSweep progress still collapses skipped microsteps.
aux0represents different units in sweep and non-sweep mode. In the sweep branch, this code jumps straight toexpectedStepsand only applies the last microstep, so dropped service ticks skip visible sweep steps. Turning Sweep off mid-transition also leaves a stale per-entry palette because the non-sweep state is never rebuilt from elapsed time.🐛 One way to fix it
typedef struct SlowTransitionData { CRGBPalette16 startPalette; // initial palette CRGBPalette16 currentPalette; // blended palette for current frame, need permanent storage so we can start from this if target changes mid transition CRGBPalette16 endPalette; // target palette + bool wasSweep; // previous sweep-mode state } slow_transition_data; @@ if (changed || SEGMENT.call == 0) { @@ *startSpeed = SEGMENT.speed; *startTime = millis(); // set start time note: intentionally not using strip.now as this is based on real time + data->wasSweep = SEGMENT.check2; } @@ - if (*stepsDone > expectedSteps) - *stepsDone = expectedSteps;// in case sweep was disabled mid transition - - if (*stepsDone < expectedSteps) { - *stepsDone = expectedSteps; // jump to expected steps to make sure timing is correct (need up to 4080 frames, at 20fps that is ~200 seconds) - if (SEGMENT.check2) { - // sweep: one palette entry at a time - uint8_t i = *stepsDone % 16; - uint8_t blendAmount = *stepsDone / 16; - data->currentPalette[i] = CRGB(color_blend(CRGBW(data->startPalette[i]), CRGBW(data->endPalette[i]), blendAmount)); - } else { - // full palette at once - uint8_t blendAmount = (uint8_t)*stepsDone; - for (uint8_t i = 0; i < 16; i++) { - data->currentPalette[i] = CRGB(color_blend(CRGBW(data->startPalette[i]), CRGBW(data->endPalette[i]), blendAmount)); - } - } + if (data->wasSweep != SEGMENT.check2) { + data->wasSweep = SEGMENT.check2; + if (!SEGMENT.check2) { + *stepsDone = expectedSteps; + uint8_t blendAmount = (uint8_t)*stepsDone; + for (uint8_t i = 0; i < 16; i++) { + data->currentPalette[i] = CRGB(color_blend(CRGBW(data->startPalette[i]), CRGBW(data->endPalette[i]), blendAmount)); + } + } else { + *stepsDone = expectedSteps; // currentPalette is already correct from full-palette mode + } + } else if (SEGMENT.check2) { + if (*stepsDone > expectedSteps) *stepsDone = 0; // init/reset + while (*stepsDone < expectedSteps) { + ++(*stepsDone); + uint8_t i = *stepsDone % 16; + uint8_t blendAmount = *stepsDone / 16; + data->currentPalette[i] = CRGB(color_blend(CRGBW(data->startPalette[i]), CRGBW(data->endPalette[i]), blendAmount)); + } + } else if (*stepsDone != expectedSteps) { + *stepsDone = expectedSteps; + uint8_t blendAmount = (uint8_t)*stepsDone; + for (uint8_t i = 0; i < 16; i++) { + data->currentPalette[i] = CRGB(color_blend(CRGBW(data->startPalette[i]), CRGBW(data->endPalette[i]), blendAmount)); + } + } + + if (*stepsDone >= totalSteps) { + // transition complete, apply end palette + data->currentPalette = data->endPalette; // set to end palette (sweep may not have set all entries) - if (*stepsDone >= totalSteps) { - // transition complete, apply end palette - data->currentPalette = data->endPalette; // set to end palette (sweep may not have set all entries) - } }Also applies to: 10836-10856
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/FX.cpp` around lines 10795 - 10799, The sweep branch uses aux0 as microstep accumulator and jumps directly to expectedSteps, applying only the final microstep which collapses skipped ticks; fix by advancing aux0 through each missed micro-microstep and applying incremental blends so each microstep contributes to currentPalette (fields in slow_transition_data: startPalette, currentPalette, endPalette), rather than a single jump to expectedSteps. Also, when switching from sweep to non-sweep, rebuild currentPalette from elapsed time (compute accumulated microsteps from aux0/expectedSteps and blend start->end accordingly) so the per-entry palette isn't left stale; update the logic around aux0, expectedSteps, and currentPalette to handle both modes consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@wled00/FX.cpp`:
- Line 10865: The control label for the slow transition metadata
(_data_FX_MODE_SLOW_TRANSITION) doesn't surface the special-case that speed==0
means a 10s test run; update the PROGMEM string value for
_data_FX_MODE_SLOW_TRANSITION to include a short hint such as " (0 = 10s)" or
"0=10s" adjacent to "Time (min)" so the UI shows the shortcut—edit the string
literal to add that text without changing the parameter order or other metadata
fields.
---
Duplicate comments:
In `@wled00/FX.cpp`:
- Around line 10795-10799: The sweep branch uses aux0 as microstep accumulator
and jumps directly to expectedSteps, applying only the final microstep which
collapses skipped ticks; fix by advancing aux0 through each missed
micro-microstep and applying incremental blends so each microstep contributes to
currentPalette (fields in slow_transition_data: startPalette, currentPalette,
endPalette), rather than a single jump to expectedSteps. Also, when switching
from sweep to non-sweep, rebuild currentPalette from elapsed time (compute
accumulated microsteps from aux0/expectedSteps and blend start->end accordingly)
so the per-entry palette isn't left stale; update the logic around aux0,
expectedSteps, and currentPalette to handle both modes consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| SEGMENT.setPixelColor(i, palcol.color32); | ||
| } | ||
| } | ||
| static const char _data_FX_MODE_SLOW_TRANSITION[] PROGMEM = "Slow Transition@Time (min),,,,,,Sweep;3;!;0;pal=2,sx=0,ix=0"; |
There was a problem hiding this comment.
Expose the 0 = 10s shortcut in the control label.
Line 10831 treats speed == 0 as a 10-second test run, but the metadata still says minutes only, so the special case is hard to discover from the UI.
📝 Suggested tweak
-static const char _data_FX_MODE_SLOW_TRANSITION[] PROGMEM = "Slow Transition@Time (min),,,,,,Sweep;3;!;0;pal=2,sx=0,ix=0";
+static const char _data_FX_MODE_SLOW_TRANSITION[] PROGMEM = "Slow Transition@Time (min, 0=10s),,,,,,Sweep;3;!;0;pal=2,sx=0,ix=0";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wled00/FX.cpp` at line 10865, The control label for the slow transition
metadata (_data_FX_MODE_SLOW_TRANSITION) doesn't surface the special-case that
speed==0 means a 10s test run; update the PROGMEM string value for
_data_FX_MODE_SLOW_TRANSITION to include a short hint such as " (0 = 10s)" or
"0=10s" adjacent to "Time (min)" so the UI shows the shortcut—edit the string
literal to add that text without changing the parameter order or other metadata
fields.
There was a problem hiding this comment.
this is intentional and left to the user to discover
There was a problem hiding this comment.
@DedeHai Got it — we’ll keep the “0 = 10s” shortcut implicit for Slow Transition as intended and won’t suggest surfacing it in the label again.
✏️ Learnings added
Learnt from: DedeHai
Repo: wled/WLED PR: 5379
File: wled00/FX.cpp:10865-10865
Timestamp: 2026-03-27T18:33:31.305Z
Learning: Repo: wled/WLED
File: wled00/FX.cpp
Context: Slow Transition effect (mode_slow_transition) UI metadata string _data_FX_MODE_SLOW_TRANSITION
Learning: The “Time (min)” control intentionally does not advertise the special case “0 = 10s” in the label; this shortcut is meant to be discovered by users. Do not suggest adding “0=10s” (or similar) to the label in future reviews.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-03-25T12:25:11.598Z
Learning: In WLED's `Segment::startTransition()` (wled00/FX_fcn.cpp:284), when `isInTransition()` is true and `_t->_oldSegment` already exists, the function silently returns without updating `_t->_start` or `_t->_bri`. This causes a bug where rapid successive on/off toggles during a non-FADE blending transition (e.g., fairy dust) leave the transition clock stale: by the time of the second re-trigger, elapsed time may already exceed `_dur`, so `updateTransitionProgress()` sets `_progress = 0xFFFF` immediately on the next service tick and `stopTransition()` fires — the blending effect never plays. The fix is to always reset `_t->_start = millis()`, `_t->_dur = dur`, and `_t->_bri = currentBri()` (current visible brightness) in the `isInTransition()` branch, regardless of whether `_oldSegment` exists.
Learnt from: BobLoeffler68
Repo: wled/WLED PR: 5293
File: usermods/user_fx/user_fx.cpp:149-151
Timestamp: 2026-01-24T18:51:09.740Z
Learning: In WLED's Spinning Wheel effect (usermods/user_fx/user_fx.cpp), the color scale calculation (SEGENV.aux1) must match the position modulo range. When position uses `pos = (pos_fixed >> 16) % SEGLEN`, the color scale must be `aux1 = (255 << 8) / SEGLEN` to distribute the palette evenly across the full position range. Using vstripLen for color scaling while position uses SEGLEN causes palette colors to cycle too quickly and appear incomplete.
Learnt from: DedeHai
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-01-13T21:23:35.514Z
Learning: In WLED, the global `paletteBlend` variable (wled.h:603) and the `WS2812FX::paletteBlend` member (FX.h:940) are duplicates without synchronization code. The global is loaded/saved in cfg.cpp and set via UI in set.cpp, but never copied to the strip member. This is the only such case in the codebase; other settings are either strip-only members (autoSegments, correctWB, cctFromRgb, isMatrix) or global-only (gammaCorrectCol/Bri/Val, blendingStyle).
Learnt from: freakintoddles2
Repo: wled/WLED PR: 4904
File: wled00/FX.cpp:3915-3930
Timestamp: 2025-09-02T19:42:57.475Z
Learning: WLED wled00/FX.cpp::mode_percent — For time-based options (% of Minute/Hour/Day), behavior is clock-like: at rollover (elapsed==0) intentionally show SEGLEN lit for one tick to represent completion, then move to 1 LED at the next second. This is by design (not a progress bar) and should not be “fixed”.
Learnt from: netmindz
Repo: wled/WLED PR: 4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: softhack007
Repo: wled/WLED PR: 5443
File: wled00/FX_fcn.cpp:1277-1277
Timestamp: 2026-03-24T12:10:29.964Z
Learning: In WLED's `WS2812FX::service()` (wled00/FX_fcn.cpp), the old condition `|| (doShow && seg.mode == FX_MODE_STATIC)` was an **inclusion** guard — it caused FX_MODE_STATIC to render only when another segment had already set doShow=true. It did NOT skip or protect FX_MODE_STATIC from rendering. The PR `#5443` simplification removes this condition, meaning FX_MODE_STATIC now renders on every `timeToShow` tick uniformly. This is intentional and not a regression. Do not flag FX_MODE_STATIC special-casing as missing in future reviews of this function.
Learnt from: netmindz
Repo: wled/WLED PR: 4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: DedeHai
Repo: wled/WLED PR: 4926
File: wled00/FX.cpp:4727-4730
Timestamp: 2025-11-16T19:40:46.260Z
Learning: WLED AuroraWave (wled00/FX.cpp): wave_start and wave_end intentionally use int16_t; segments longer than 32k LEDs are not supported (bounded by MAX_LEDS), so widening to 32-bit is unnecessary.
Learnt from: BobLoeffler68
Repo: wled/WLED PR: 5440
File: usermods/user_fx/user_fx.cpp:1304-1313
Timestamp: 2026-03-25T07:03:31.345Z
Learning: In WLED `mode_dissolveplus` (usermods/user_fx/user_fx.cpp), using `hw_random16(SEGLEN)` to select the survivor pixel index is correct and safe for this 1D-only effect. The 0xFFFF unmapped-entry concern from the physical bus mapping does not apply to 1D segments because virtual indices 0..SEGLEN-1 always map to valid physical LEDs without gaps. Do not flag this as a bug in future reviews of 1D effects.
Learnt from: BobLoeffler68
Repo: wled/WLED PR: 5440
File: usermods/user_fx/user_fx.cpp:1307-1327
Timestamp: 2026-03-24T07:59:25.586Z
Learning: In WLED's Dissolve Plus effect (usermods/user_fx/user_fx.cpp, mode_dissolveplus), when the "Last one" checkbox (check3/lastOneMode) is turned off while the effect is in PHASE_PAUSE_SURVIVOR, the phase is intentionally NOT immediately reset. The pause expires naturally via the normal delay timing (a few hundred ticks), after which the filling phase begins. PHASE_DISSOLVE_SURVIVOR is reset to PHASE_DISSOLVE and PHASE_FILL_SURVIVOR is reset to PHASE_FILL on toggle-off, but PHASE_PAUSE_SURVIVOR is left to expire on its own. Do not flag this as a bug in future reviews.
Learnt from: DedeHai
Repo: wled/WLED PR: 5372
File: wled00/FX.cpp:6489-6489
Timestamp: 2026-02-26T20:49:50.973Z
Learning: In WLED PR `#5372` (wled/WLED), mode_2Dscrollingtext in wled00/FX.cpp: the UI control backed by SEGMENT.custom2 is intentionally labeled “Font size”. It selects among predefined fonts whose pixel dimensions inherently define size; maintainers prefer to keep the label as “Font size” rather than “Font”.
Learnt from: BobLoeffler68
Repo: wled/WLED PR: 5109
File: wled00/FX.cpp:3174-3343
Timestamp: 2025-11-27T06:33:11.436Z
Learning: WLED Ants effect (wled00/FX.cpp): The author prefers the current velocity initialization using hw_random16(1000, 5000)/5000.0f, resulting in an effective range of ~3.6–10.0 (with VELOCITY_MIN=2.0, VELOCITY_MAX=10.0), and wants the code kept as-is with comments updated to document this behavior. Avoid suggesting changes to span the full 2.0–10.0 range in future reviews.
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: 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. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit. This improves code maintainability and reduces the risk of inconsistencies.
Learnt from: DedeHai
Repo: wled/WLED PR: 4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR `#4798`, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-03-08T00:57:36.134Z
Learning: In WLED (wled00/cfg.cpp), `deserializeConfig()` is called with an empty JsonObject when cfg.json doesn't exist on fresh install. Any value read without the ArduinoJSON `|` fallback operator or CJSON macro will override correct constructor defaults with 0/null. Known affected values: `strip.setTargetFps(hw_led["fps"])` at line 179 (sets FPS to 0/unlimited instead of WLED_FPS=42). Fix: `strip.setTargetFps(hw_led["fps"] | WLED_FPS)`. The gamma issue (gammaCorrectCol/Bri) had the same root cause and was fixed in commit d1d9dec402 (Jan 2026) using inline `| default` fallbacks.
Learnt from: freakintoddles2
Repo: wled/WLED PR: 4904
File: wled00/FX.cpp:3901-3913
Timestamp: 2025-09-02T19:36:49.513Z
Learning: In FX.cpp mode_percent time-based options (% of Minute/Hour/Day), do not guard against invalid RTC; showing a full bar with unsynced time is intentional to avoid confusing UX.
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-12-28T14:06:48.772Z
Learning: In WLED post-commit ee9ac947, the rendering pipeline uses per-segment buffers and per-pixel bus updates. Unmapped (0xFFFF) mapping entries are now skipped in WS2812FX::show() (no “clear to black”), which can leave physical gap LEDs with stale/random colors unless they are explicitly cleared. This is a behavior change from pre-0.16 where a full physical buffer was effectively refreshed each frame.
Learnt from: softhack007
Repo: wled/WLED PR: 5443
File: wled00/FX_fcn.cpp:1277-1277
Timestamp: 2026-03-24T12:13:17.908Z
Learning: In WLED's `WS2812FX::service()` (wled00/FX_fcn.cpp), `seg.freeze` means "do not run the effect function (_mode[seg.mode]())" — it does NOT mean "skip show()". A frozen segment's pixel buffer can still be updated externally (e.g., realtime control or single-pixel JSON API). `strip.trigger()` is the primary mechanism to flush those externally written pixels to the LEDs on the next service tick. Therefore, frozen segments must remain part of the `doShow`/`show()` path, and it is architecturally wrong to exclude frozen segments from `doShow`. Do not suggest skipping frozen segments from the show path in future reviews.
Learnt from: DedeHai
Repo: wled/WLED PR: 4889
File: wled00/json.cpp:310-310
Timestamp: 2026-03-21T18:12:09.437Z
Learning: In WLED's `deserializeSegment()` (wled00/json.cpp), the blend mode field `seg.blendMode` is intentionally written without a post-read clamp (`getVal(elem["bm"], seg.blendMode)`). Out-of-range or unsupported blend mode values are handled safely in `WS2812FX::blendSegment()` (wled00/FX_fcn.cpp), which defaults to mode 0 for any unsupported value via a bounds check against the `BLENDMODES` constant. Do not flag the missing clamp in deserializeSegment as a bug in future reviews.
Learnt from: BobLoeffler68
Repo: wled/WLED PR: 5293
File: usermods/user_fx/user_fx.cpp:167-171
Timestamp: 2026-01-13T03:07:20.560Z
Learning: In WLED effects (usermods/user_fx/user_fx.cpp and similar), when storing future timestamps, assignments like `stored_time = now + delay` are correct and handle uint32_t wraparound properly. Only comparison statements need to use subtraction-based elapsed-time checks (e.g., `(now - stored_time) >= delay`). Do not flag assignment statements as needing fixes for wraparound handling.
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: 4893
File: wled00/set.cpp:95-95
Timestamp: 2026-03-14T20:56:46.543Z
Learning: Guideline: Ensure WiFi hostname is set after WiFi.mode() but before WiFi.begin() to avoid default esp-XXXXXX hostname being used in DHCP. This ordering only takes effect after the STA interface exists (so avoid placing hostname setting before initConnection). In WLED, place the hostname configuration inside initConnection() (after WiFi.disconnect(true) and before WiFi.begin()) rather than in earlier boot code like deserializeConfig(). This rule should be applied in code reviews for WLED’s network initialization paths in wled00/*.cpp, and note that on ESP8266 the ordering is less strict but still desirable for consistency.
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.
4 participants
A new effect that is purely made for very slow transitions, up to 255 minutes.
It also allows to call a preset after the transition is done by setting intensity slider to the preset number to be applied after the transition has finished.
Using segment layering, this also allows it to be a "mask" although that takes a bit of tinkering with the preset json.
One scenario possible with this effect would be this:
The FX only transitions palettes (colors if palette 0 or any color derived palette is used), brightness slider transitions normally i.e. fast. If a color/palette is changed mid transition, it starts a new transition towards that color with the current state as a starting point so transitions never jump.
p.s.
I also remvoed the return value from
Segment::loadPalette()` as the target palette is passed by reference and the return value is never even used anymore.Fixes #5375
Summary by CodeRabbit
New Features
Bug Fixes / Behavior