Skip to content

improve bootup behaviour for boot presets#5686

Open
DedeHai wants to merge 11 commits into
wled:mainfrom
DedeHai:improve_bootup_colors
Open

improve bootup behaviour for boot presets#5686
DedeHai wants to merge 11 commits into
wled:mainfrom
DedeHai:improve_bootup_colors

Conversation

@DedeHai

@DedeHai DedeHai commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator

changes made:

  • full refactor of the bootup behaviour regarding "bootup preset" and "turn on at boot" here is how it works now:

    • if turn on at boot is set, LEDs turn on unconditionally to the set default brightness
    • if no prest is applied, it still fades from black at the set default transition time instead of instantly turning on (same as it was in 0.14)
    • if a preset is applied it fades to that preset on all segments, starting off black
    • if any transition mode other than fade is set, LEDs start off at "default brightness" instead of fading from black if "turn on at boot" is set (not acutally sure why on "fade" transition it starts off black but it works)
  • Segments created by a bootup presets no longer use the DEFAULT_COLOR but start out black: segments created within the first 5s after bootup default to black

  • DEFAULT_COLOR used for new segments is now the same as the bootup color (AA which is 170 changed to A0 which is 160)

TLDR;
no matter if "turn on at boot" is now set or not, boot up presets are applied correctly. If "turn on at boot" is set, LEDs always turn on.

Summary by CodeRabbit

  • Bug Fixes
    • Improved boot behavior so LEDs reliably begin from black, including when no relay is configured.
    • Streamlined startup brightness and boot preset initialization for more consistent first-frame behavior.
  • UI/UX
    • Updated the bootup brightness field label to “Bootup brightness”.
  • Behavior / Visual Defaults
    • Adjusted the default warm orange shade and ensured newly created auto segments and certain default segments start with the expected primary color.
  • Style
    • Changed the initial stored primary color state to start from black.

@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • ✅ Review completed - (🔄 Check again to review again)

Walkthrough

DEFAULT_COLOR hex value is corrected from 0xFFAA00 to 0xFFA000. colPri[] global initializer is zeroed. The Segment constructor gains a boot-time guard that forces colors[0] to BLACK while millis() is below a 2-second threshold (plus optional delay). beginStrip() startup sequencing is restructured around relay presence and bootPreset application. A UI label is renamed.

Changes

Boot-up LED color initialization

Layer / File(s) Summary
Color constants and initial global state
wled00/FX.h, wled00/wled.h
DEFAULT_COLOR updated to 0xFFA000; colPri[] global initializer changed from orange-like values to all zeros; Segment colors initializer gains a descriptive comment indicating warm orange.
Segment constructor boot-time black enforcement
wled00/FX.h
Constructor computes bootupTime (2000ms + optional WLED_BOOTUPDELAY) and forces colors[0] to BLACK while millis() is below that threshold, preventing fade-in from the default orange during early boot.
beginStrip() startup sequencing and UI label
wled00/wled.cpp, wled00/data/settings_leds.htm
strip.show() is called early when no relay pin is present; brightness initialization starts from zero then sets to briS when turnOnAtBoot is true or briLast=briS when false; bootPreset > 0 handling is isolated into its own branch that applies the preset then forces active segments to BLACK; transitionDelayDefault restoration is now conditional on bootPreset == 0. Settings label for bootup brightness field renamed from "with brightness" to "Bootup brightness".

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • wled/WLED#5315: Modifies the same WLED::beginStrip() function with relay-presence–gated startup initialization logic, directly overlapping the refactored code path.
  • wled/WLED#4827: Changes to wled00/wled.cpp and wled00/wled.h around beginStrip() boot-preset handling and colPri initialization overlap directly with this PR's boot-color sequencing modifications.

Suggested labels

enhancement

Suggested reviewers

  • softhack007
  • willmmiles
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'improve bootup behaviour for boot presets' directly addresses the primary objective of this PR, which is to refactor and improve bootup behavior specifically for boot preset handling.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]

This comment was marked as outdated.

@DedeHai

DedeHai commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator Author

@coderabbitai please check for any edge cases that may have a changed behaviour and come up with a test plan to verify this PR works as intended.

@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get the PR diff
gh pr diff 5686 --repo wled/WLED

echo "===== wled.cpp segment ====="
gh api repos/wled/WLED/contents/wled00/wled.cpp?ref=refs/pull/5686/head --jq '.content' | base64 -d | grep -n "bootPreset\|turnOnAtBoot\|briS\|colPri\|transitionDelay\|beginStrip\|BLACK\|DEFAULT_COLOR\|bootTime\|CALL_MODE_INIT" -A2 -B2

echo "===== FX.h segment ====="
gh api repos/wled/WLED/contents/wled00/FX.h?ref=refs/pull/5686/head --jq '.content' | base64 -d | grep -n "DEFAULT_COLOR\|bootTime\|bootupDelay\|colors\[0\]\|BLACK\|WLED_BOOTUPDELAY" -A5 -B2

echo "===== wled.h colPri ====="
gh api repos/wled/WLED/contents/wled00/wled.h?ref=refs/pull/5686/head --jq '.content' | base64 -d | grep -n "colPri\|colSec" -A2 -B2

Length of output: 11599


🏁 Script executed:

#!/bin/bash
# Get full beginStrip function from PR branch
gh api repos/wled/WLED/contents/wled00/wled.cpp?ref=refs/pull/5686/head --jq '.content' | base64 -d | grep -n "beginStrip\|void WLED::begin" -A 80 | head -120

echo "=====FX.h Segment constructor====="
gh api repos/wled/WLED/contents/wled00/FX.h?ref=refs/pull/5686/head --jq '.content' | base64 -d | grep -n "Segment()\|Segment ::\|bootTime\|bootupDelay\|colors\[0\]" -A 10 -B 5 | head -80

Length of output: 9859



@DedeHai Here's a thorough edge-case analysis and test plan for this PR.


Edge Case Analysis

EC-1: turnOnAtBoot = true + bootPreset > 0 (New Combined Path)

Changed behavior: Previously these were separate branches. Now both execute sequentially: bri = briS, colPri set to orange, then immediately overwritten to {0,0,0,0} before applyPreset(). The net result is bri > 0 with the preset colors—which is the intended behavior—but the intermediate orange colPri is wasted work. More importantly, confirm the preset's own brightness (if any) wins over briS.

EC-2: turnOnAtBoot = false + bootPreset > 0

Changed behavior: bri stays 0. applyPreset() is called but whether LEDs turn on depends on whether applyPreset sets bri. If the preset doesn't embed a brightness value, LEDs remain off. Then handlePresets() in setup() re-applies the preset—double application may replay side-effects (playlists, random seeds, etc.).

EC-3: Transition not restored when bootPreset > 0

strip.setTransition(transitionDelayDefault) is now inside the else branch, so it is never called when a boot preset is set. colorUpdated(CALL_MODE_INIT) therefore runs with transition = 0. If a fade-in from black is desired for the boot preset path, the transitionDelayDefault needs to be restored before colorUpdated or the fade must come from handlePresets() in setup().

EC-4: briLast No Longer Set in beginStrip()

Old code: briLast = briS; bri = 0;. New code: briLast is never assigned in beginStrip(). The first power-toggle (button/API) will restore to whatever briLast was deserialized from config, which may be 0, causing "turn on" to appear broken.

EC-5: strip.fill(BLACK) Removed

Old code explicitly filled the strip with BLACK when turnOnAtBoot = false. New code relies on bri = 0 to keep LEDs visually dark. On hardware where setBrightness(0) isn't perfectly zero (PWM floor, etc.) or where a relay isn't used and stale frame buffer data exists, pixels might briefly flicker.

EC-6: strip.show() Called Before turnOnAtBoot Check (Relay Absent)

strip.show() at bri = 0 is now always called when rlyPin < 0, regardless of turnOnAtBoot. Previously it was only inside the turnOnAtBoot = false branch. This means on turnOnAtBoot = true + no relay, the strip is shown at bri = 0 first and then turned on. This is correct behavior but adds one redundant show call.

EC-7: 5-Second Boot Window Affects User-Created Segments

Any segment created within 5000ms + WLED_BOOTUPDELAY starts with colors[0] = BLACK. If a user reacts quickly and adds a segment via the UI within 5 seconds of boot, it silently starts black instead of the expected orange. With large WLED_BOOTUPDELAY values this window can extend significantly (e.g., 35 seconds with a 30s delay).

EC-8: Segment Copy Constructor Not Updated

The boot-time black logic is only in the default constructor body. The copy constructor (Segment(const Segment &orig)) and move constructor are not affected, so copied segments during transition setup retain the source's color, not black. This is probably fine but worth confirming that no boot-time segment copy occurs before beginStrip() completes.

EC-9: colors[1] / colors[2] Not Zeroed After Preset Application

After applyPreset(), the code forces colors[0] = BLACK for all active segments but leaves colors[1] and colors[2] as set by the preset. Effects using secondary/tertiary colors will not fade from black for those channels.

EC-10: First-Time Install (briS = 0)

bri = briS ? briS : 128 correctly handles this fallback. ✅

EC-11: Non-Fade Transition Mode

Per PR description, LEDs should start at default brightness immediately. Verify that when transitionDelayDefault > 0 but mode is not FADE, the segment shows the preset state directly without a black-to-color animation.

EC-12: Boot Preset Is a Playlist

handlePresets() in setup() calls handlePlaylist() after handlePresets(). Since applyPreset() is already called in beginStrip(), the playlist is started twice. Verify no duplicate timer registration or doubled first-entry playback.


Test Plan

Setup Matrix

Each test should be run with a fresh config (factory reset) and with a saved config unless noted.

# turnOnAtBoot bootPreset rlyPin transitionDelayDefault Notes
T-1 false 0 none 700ms (default) Baseline: all off at boot
T-2 true 0 none 700ms LEDs on at briS, fade from black
T-3 true 0 none 0 Instant on, no fade
T-4 false N (valid preset) none 700ms Preset applied, LEDs may be off
T-5 true N (valid preset) none 700ms LEDs on + preset fades in from black
T-6 true N (valid preset) none 0 Instant preset, no fade
T-7 true 0 configured 700ms Relay controls on/off, then fade
T-8 false N configured 700ms Relay stays off, preset applied silently
T-9 true N (playlist preset) none 700ms Playlist boot preset
T-10 true 0 none 700ms briS = 0 (first install fallback)

Test Cases

T-1: All Off at Boot

  1. Set turnOnAtBoot = false, bootPreset = 0.
  2. Power cycle.
  3. Expected: All LEDs remain off. No flicker on first frame.
  4. Check: bri = 0, no relay command issued, strip dark.

T-2: Turn-On Fade from Black

  1. Set turnOnAtBoot = true, bootPreset = 0, default orange color saved, transitionDelayDefault = 700ms.
  2. Power cycle.
  3. Expected: LEDs fade from black to briS brightness in orange over ~700ms.
  4. Check: No flash of orange before fade starts; brightness reaches briS exactly.

T-3: Instant Turn-On (No Fade)

  1. Set turnOnAtBoot = true, bootPreset = 0, transitionDelayDefault = 0.
  2. Power cycle.
  3. Expected: LEDs turn on instantly at briS, no fade transition.
  4. Check: No visible ramp-up from zero.

T-4: Boot Preset Only (LEDs Stay Off?)

  1. Set turnOnAtBoot = false, bootPreset = N (preset with explicit brightness embedded).
  2. Power cycle.
  3. Expected (verify): Does the preset turn on the LEDs if it has a bri field? Or do LEDs stay off because bri = 0?
  4. Check: bri value after boot; compare with and without brightness saved in the preset.

T-5: Turn-On + Boot Preset Fade

  1. Set turnOnAtBoot = true, bootPreset = N, transitionDelayDefault = 700ms.
  2. Power cycle.
  3. Expected: LEDs start black, fade to the preset's colors at briS brightness over 700ms.
  4. Check: No orange flash before fade; all segment channels (col0, col1, col2) fade correctly; briS is respected vs. preset brightness.

T-6: Turn-On + Boot Preset Instant (No Fade)

  1. Set turnOnAtBoot = true, bootPreset = N, transitionDelayDefault = 0.
  2. Power cycle.
  3. Expected: Preset applied immediately at briS with no fade.
  4. Check: Same as T-5 but instantaneous.

T-7: Relay + Turn-On + Boot Preset

  1. Configure relay pin, set turnOnAtBoot = true, bootPreset = N.
  2. Power cycle.
  3. Expected: Relay fires, LEDs turn on after relay delay, preset fades in.
  4. Check: strip.show() is NOT called before relay activates; no premature LED output.

T-8: Power Toggle After Boot (briLast regression)

  1. Any configuration.
  2. Power cycle, wait for boot to complete.
  3. Toggle LEDs off via button or API.
  4. Toggle LEDs back on.
  5. Expected: LEDs restore to the expected brightness (briS or preset brightness).
  6. Check: Confirm briLast holds the correct value—not 0—after boot.

T-9: Boot Playlist Preset

  1. Set bootPreset = N where N is a playlist preset.
  2. Power cycle.
  3. Expected: Playlist starts, advances entries correctly, no duplicate first-entry playback.
  4. Check: Playlist timing is correct; first entry isn't played twice.

T-10: First-Install (briS = 0)

  1. Factory reset (or manually set briS = 0 in config).
  2. Set turnOnAtBoot = true.
  3. Power cycle.
  4. Expected: LEDs turn on at brightness 128 (fallback).
  5. Check: Not dark; confirms bri = briS ? briS : 128 fallback works.

T-11: New Segment Within 5-Second Boot Window

  1. Power cycle, immediately (< 5 seconds) go to the segment editor and add a new segment.
  2. Expected: New segment starts black instead of orange.
  3. Check (UX regression): Does the user see unexpected black? Is this behavior acceptable? After 5 seconds, verify new segments correctly start in orange.

T-12: Multi-Segment Boot Preset

  1. Configure a preset that defines 3+ segments with different colors.
  2. Set bootPreset = N, turnOnAtBoot = true.
  3. Power cycle.
  4. Expected: All active segments fade from black to their respective preset colors. Inactive segments remain black/off.
  5. Check: colors[1] and colors[2] also transition correctly (not just colors[0]).

T-13: Non-Fade Transition Mode

  1. Set the transition mode to a non-fade type (e.g., Wipe), bootPreset = N, turnOnAtBoot = true.
  2. Power cycle.
  3. Expected: Per PR description, LEDs start at default brightness immediately without fading from black.
  4. Check: No black-to-color animation; LEDs snap to preset state.

T-14: WLED_BOOTUPDELAY Defined

  1. Compile with WLED_BOOTUPDELAY = 10000 (10s).
  2. Power cycle; wait 8 seconds, add a segment.
  3. Expected: Segment still starts black (within 15s window).
  4. Wait 16 seconds, add another segment.
  5. Expected: Segment starts in orange (outside window).

The most critical cases to verify first are EC-3 (transition not restored for boot preset path, affecting whether fade-in actually occurs), EC-4 (briLast not set, affecting power toggle), and EC-2 (whether bri stays 0 when turnOnAtBoot = false + bootPreset > 0 renders the preset invisible until handlePresets() in setup()).

@DedeHai

DedeHai commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator Author

I tested all the suggested cases and made some improvements. this is how it works now, copy from source code comment for reference:

  • if turnOnAtBoot is false, strip is set to black. It will fade in to startup brightness and orange when turned on
  • if a bootup preset is set, it will fade to that preset if it has on:true set (to default brightness) or to that preset's brightness if set
  • if turnOnAtBoot is true the LEDs will fade in to orange and default brightness
  • if a booup preset is set, it will start at the default brightness except if "fade" transition is used, then it will still fade from black
  • there is no way to have LEDs off at boot and upon turn-on have them immediatel jump to a target brightness but users can use a playlist to do that

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@wled00/wled.cpp`:
- Line 624: Fix the typo in the comment on line 624 where "booup" is misspelled.
Change "booup" to "bootup" in the comment that starts with "if a booup preset is
set" to correctly read "if a bootup preset is set".
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 3ae61826-11cf-47e0-bd66-338208522b86

📥 Commits

Reviewing files that changed from the base of the PR and between 598e08c and 9b03b78.

📒 Files selected for processing (1)
  • wled00/wled.cpp

Comment thread wled00/wled.cpp Outdated
@softhack007 softhack007 added this to the 16.1 milestone Jun 19, 2026
@DedeHai

DedeHai commented Jun 20, 2026

Copy link
Copy Markdown
Collaborator Author

@coderabbitai full review, take the test plan above into account and check if there is inconsistencies for edge cases.

@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "===== Full wled.cpp beginStrip function ====="
gh api repos/wled/WLED/contents/wled00/wled.cpp?ref=refs/pull/5686/head --jq '.content' | base64 -d | grep -n "" | grep -A 120 "beginStrip"

echo ""
echo "===== Full Segment constructor in FX.h ====="
gh api repos/wled/WLED/contents/wled00/FX.h?ref=refs/pull/5686/head --jq '.content' | base64 -d | grep -n "" | grep -A 40 "Segment()"

echo ""
echo "===== wled.h colPri / colSec / briLast / briS ====="
gh api repos/wled/WLED/contents/wled00/wled.h?ref=refs/pull/5686/head --jq '.content' | base64 -d | grep -n "colPri\|colSec\|briLast\|briS\b" -A 1 -B 1

echo ""
echo "===== setup() in wled.cpp - handlePresets call context ====="
gh api repos/wled/WLED/contents/wled00/wled.cpp?ref=refs/pull/5686/head --jq '.content' | base64 -d | grep -n "" | grep -A 60 "void WLED::setup"

Length of output: 21332


🏁 Script executed:

#!/bin/bash
echo "===== Segment constructor full body (FX.h around lines 564-640) ====="
gh api repos/wled/WLED/contents/wled00/FX.h?ref=refs/pull/5686/head --jq '.content' | base64 -d | sed -n '564,640p'

echo ""
echo "===== beginStrip full (wled.cpp lines 606-646) ====="
gh api repos/wled/WLED/contents/wled00/wled.cpp?ref=refs/pull/5686/head --jq '.content' | base64 -d | sed -n '606,650p'

echo ""
echo "===== Check for colors[0] BLACK forcing after applyPreset in wled.cpp ====="
gh api repos/wled/WLED/contents/wled00/wled.cpp?ref=refs/pull/5686/head --jq '.content' | base64 -d | grep -n "colors\[0\]\|BLACK\|applyPreset\|bootPreset\|WLED_BOOTUPDELAY\|bootupTime\|bootTime" -A 2 -B 2

echo ""
echo "===== Check transitionDelayDefault restoration and bootPreset coloring in wled.cpp ====="
gh api repos/wled/WLED/contents/wled00/wled.cpp?ref=refs/pull/5686/head --jq '.content' | base64 -d | grep -n "transitionDelay\|CALL_MODE_INIT\|colorUpdated" -A 2 -B 2

echo ""
echo "===== Check FX.h for bootupTime / WLED_BOOTUPDELAY in constructor ====="
gh api repos/wled/WLED/contents/wled00/FX.h?ref=refs/pull/5686/head --jq '.content' | base64 -d | grep -n "bootupTime\|WLED_BOOTUPDELAY\|millis\|colors\[0\].*BLACK\|colors\[0\] = BLACK" -A 3 -B 3

Length of output: 12574



@DedeHai Here is the full review with findings mapped against the test plan.


Full Review — Boot Behavior Refactor (#5686)

✅ What is working correctly

Test Verdict Notes
T-1 (off at boot, no preset) bri=0, strip shown via strip.show() (no-relay path), orange color pre-loaded for when user turns on
T-2 (turn-on fade, no preset) bri=briS, transition restored via strip.setTransition(transitionDelayDefault), segment starts black (constructor), fades to orange
T-3 (instant turn-on, no preset, trans=0) Transition is 0, colorUpdated applies immediately
T-8 (power toggle / briLast) briLast=briS set in the else branch (line 631); briLast defaults to 128 in wled.h otherwise
T-10 (first install, briS=0) bri = briS where briS is 128 by default — the guard bri = briS ? briS : 128 from old code is no longer needed since UI prevents 0; initialize is fine
EC-8 (copy constructor) Boot-time black only in default constructor; copy/move constructors unaffected
EC-9 (colors[1]/[2]) Constructor initializes to {DEFAULT_COLOR, BLACK, BLACK} so secondary/tertiary are already black

⚠️ Issues Found


🔴 Issue 1 — PR Description Says "5 seconds", Code Uses 2 Seconds

FX.h, line 599:

uint32_t bootupTime = 2000; // 2s should be more than enough to init bootup-segments to black

The PR objectives state: "Segments created within the first 5 seconds after bootup default to black" — but the code uses 2000ms. The comment also says "2s". One of them is wrong. If real hardware with WLED_BOOTUPDELAY=0 takes more than 2 seconds to reach beginStrip() in edge cases (slow flash, heavy usermod setup), segments created during setup() before beginStrip() completes would fall outside the 2s window and not get the black override.

Recommendation: Decide on the correct value (2s or 5s), update both the code and the PR description/comments to match.


🔴 Issue 2 — Boot Preset Fade Depends on Preset's Own Stored Transition Value (EC-3)

In beginStrip():

strip.setTransition(0); // line 609 — zeroed for all paths

if (bootPreset > 0) {
    applyPreset(bootPreset, CALL_MODE_INIT);  // may or may not restore transition
}
else {
    // ...
    strip.setTransition(transitionDelayDefault);  // only restored here
}

colorUpdated(CALL_MODE_INIT); // runs with transition=0 if preset didn't set one

strip.setTransition(transitionDelayDefault) is only restored in the else branch. For the bootPreset > 0 path, whether a fade-from-black occurs depends entirely on whether applyPreset() sets a transition value from the preset's stored JSON. If the preset was saved without an explicit transition field, the transition stays 0 and the boot preset appears instantly even when the user expects a fade.

The PR description says "if 'fade' transition is used, then it will still fade from black" — but users who rely on transitionDelayDefault (set globally in LED settings) would not get the fade unless their preset was captured with transition data.

Recommendation: Either restore transitionDelayDefault before colorUpdated also in the boot preset branch, or document clearly that the boot preset fade requires the preset to have been saved with an explicit transition value.


🟡 Issue 3 — Boot Preset Applied Twice (EC-2, EC-12)

beginStrip() calls applyPreset(bootPreset, CALL_MODE_INIT) at line 634.
Then setup() (lines 506–510) calls handlePresets() which applies the same boot preset again:

if (bootPreset > 0) {
    handlePresets();   // re-applies boot preset
    handlePlaylist();
    handlePresets();   // applies again for anything queued
}

For regular presets this is likely benign (idempotent color/effect state). For playlists (T-9), the playlist is started in beginStrip via applyPreset, then handlePlaylist() advances it, and handlePresets() may start it again. Whether this causes the first entry to play twice depends on playlist timer state. The PR author confirmed T-9 was tested, but it's worth verifying with a playlist preset that has a very short first-entry duration (e.g., 500ms) where double-firing would be noticeable.

Recommendation: Consider skipping the applyPreset call in beginStrip and relying solely on the handlePresets() call in setup(), or add a guard that avoids re-applying if the preset was already applied during beginStrip.


🟡 Issue 4 — briLast Not Synced to Preset Brightness After Boot (EC-4 refinement)

When turnOnAtBoot = true and bootPreset > 0:

  1. bri = briS (line 629)
  2. applyPreset() may set bri to the preset's stored brightness (e.g., 200)
  3. briLast is never updated in this path

Result: After boot, bri = 200 (from preset) but briLast remains whatever was in config (e.g., 128). When the user toggles off and back on, brightness jumps back to 128, not 200. This is surprising if the boot preset was intentionally configured to run at a specific brightness.

Recommendation: After applyPreset, if bri > 0, set briLast = bri to ensure power-toggle restores to the preset's brightness.


🟢 Issue 5 — Typo in Source Comment

wled.cpp, line 624:

//   if a booup preset is set, ...

Should be bootup.


Test Case Matrix Summary

Case EC Verdict
T-4 (off + boot preset, preset has on:true) EC-2 ✅ Works via applyPreset setting bri
T-5 (on + boot preset, fade) EC-3 ⚠️ Fade depends on preset's stored transition
T-6 (on + boot preset, no fade) EC-3 ✅ Instant (transition=0)
T-7 (relay + boot preset) EC-6 strip.show() skipped when rlyPin ≥ 0
T-9 (playlist boot preset) EC-12 ⚠️ Double application; needs short-duration playlist test
T-11 (new segment < 2s window) EC-7 ✅ Black (constructor), but "5s" in PR description is incorrect — actual is 2s
T-12 (multi-segment preset) EC-9 ✅ colors[1]/[2] initialized to BLACK by constructor
T-13 (non-fade transition mode) EC-11 ✅ Per design: no fade from black for non-fade transitions
T-14 (WLED_BOOTUPDELAY compile) EC-7 bootupTime += WLED_BOOTUPDELAY handles this correctly

The most important items to address before merge are Issue 1 (2s vs 5s discrepancy) and Issue 2 (silent dependency on preset-stored transition for the boot fade). Issues 3 and 4 are lower priority but worth noting.

[full_review]


✏️ Learnings added
Learnt from: DedeHai
URL: https://github.com/wled/WLED/pull/5686

Timestamp: 2026-06-20T07:56:49.662Z
Learning: In WLED PR `#5686` (boot behavior refactor), the `Segment` default constructor in `wled00/FX.h` forces `colors[0] = BLACK` when `millis() < 2000 + WLED_BOOTUPDELAY`. This is the mechanism for the boot-time fade-from-black. The value 2000ms was used in code though the PR description stated 5 seconds — actual is 2 seconds.

Learnt from: DedeHai
URL: https://github.com/wled/WLED/pull/5686

Timestamp: 2026-06-20T07:56:49.662Z
Learning: In WLED PR `#5686` (boot behavior refactor), `strip.setTransition(transitionDelayDefault)` is only restored in the `else` (no boot preset) branch of `beginStrip()` in `wled00/wled.cpp`. For the `bootPreset > 0` path, the transition stays 0 unless `applyPreset()` sets it from the preset's stored JSON data. The fade-from-black for boot presets depends on whether the preset was saved with an explicit transition value.

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.

@DedeHai

DedeHai commented Jun 20, 2026

Copy link
Copy Markdown
Collaborator Author

note: the rabbit is wrong about most things except the default transition time.

@DedeHai

DedeHai commented Jun 20, 2026

Copy link
Copy Markdown
Collaborator Author

TODO: remove the 2s boot timeout and move the orange color to set.cpp

@DedeHai

DedeHai commented Jun 21, 2026

Copy link
Copy Markdown
Collaborator Author

@coderabbitai if I change the segment constructor to use all black i.e. colors{BLACK,BLACK,BLACK} but still want newly created segments that are created from UI and not a preset to be orange, what is the best way to set colors[0] = DEFAULT_COLOR in that newly created segment? I think this also might apply to to makeAutoSegments().

@coderabbitai

coderabbitai Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "===== makeAutoSegments in FX_fcn.cpp ====="
gh api repos/wled/WLED/contents/wled00/FX_fcn.cpp?ref=refs/pull/5686/head --jq '.content' | base64 -d | grep -n "makeAutoSegments\|setSegment\|addSegment\|Segment(" -A 5 -B 2 | head -80

echo ""
echo "===== Segment creation in json.cpp ====="
gh api repos/wled/WLED/contents/wled00/json.cpp?ref=refs/pull/5686/head --jq '.content' | base64 -d | grep -n "setSegment\|addSegment\|Segment(\|DEFAULT_COLOR\|colors\[0\]" -A 5 -B 2 | head -100

echo ""
echo "===== setSegment / addSegment signatures in FX.h ====="
gh api repos/wled/WLED/contents/wled00/FX.h?ref=refs/pull/5686/head --jq '.content' | base64 -d | grep -n "setSegment\|addSegment" -A 3 -B 1 | head -60

echo ""
echo "===== Where makeAutoSegments is called ====="
gh api repos/wled/WLED/contents/wled00/FX_fcn.cpp?ref=refs/pull/5686/head --jq '.content' | base64 -d | grep -n "makeAutoSegments" -A 2 -B 2
gh api repos/wled/WLED/contents/wled00/wled.cpp?ref=refs/pull/5686/head --jq '.content' | base64 -d | grep -n "makeAutoSegments" -A 2 -B 2

Length of output: 11351



@DedeHai There are two distinct creation paths to handle — here's the cleanest approach for each:


Path 1: Segment created from the UI via the JSON API (json.cpp)

deserializeSegment() already has everything you need — presetId == 0 means it's a UI/API call, and newSeg == true means the segment was just appended. Simply set the color right after strip.appendSegment():

// json.cpp, inside deserializeSegment()
if (id >= strip.getSegmentsNum()) {
  if (stop <= 0) return false;
  strip.appendSegment(0, strip.getLengthTotal());
  id = strip.getSegmentsNum()-1;
  newSeg = true;
}

Segment& seg = strip.getSegment(id);

// NEW: UI-created segments start orange; preset-applied segments stay black
if (newSeg && presetId == 0) {
  seg.colors[0] = DEFAULT_COLOR;
}

This is zero-overhead and self-documenting — presetId == 0 is already the established signal for "not from a preset".


Path 2: makeAutoSegments() called from set.cpp (user changed LED config)

This is the "move orange to set.cpp" you noted. makeAutoSegments() is called in two contexts:

Call site Segments should be
beginStrip() (boot) BLACK — preset/boot logic takes over
set.cpp after finalizeInit() Orange — user reconfigured LEDs

The simplest fix is to set the color in set.cpp after the makeAutoSegments() call, so the boot path is unaffected:

// set.cpp, after strip.makeAutoSegments() (line ~222)
if (aligned) {
  strip.makeAutoSegments();
  // newly auto-created segments should show default color in the UI
  for (size_t s = 0; s < strip.getSegmentsNum(); s++) {
    Segment& seg = strip.getSegment(s);
    if (seg.isActive() && seg.colors[0] == BLACK)
      seg.colors[0] = DEFAULT_COLOR;
  }
} else {
  strip.fixInvalidSegments();
}

The colors[0] == BLACK guard avoids overwriting segments that already have a user-set color (e.g., if only one bus was added and existing segments are untouched).


Summary

With these two changes, the constructor can safely become {BLACK, BLACK, BLACK} and the entire millis() boot-window hack can be removed. The orange color is now set explicitly only where it makes semantic sense: in the UI API layer (json.cpp) and in the user-triggered LED reconfiguration path (set.cpp), not in the constructor.


✏️ Learnings added
Learnt from: DedeHai
URL: https://github.com/wled/WLED/pull/5686

Timestamp: 2026-06-21T06:31:20.156Z
Learning: In WLED (wled00/json.cpp), `deserializeSegment()` has a `presetId` parameter (default 0) and a local `newSeg` bool that is set to `true` when `appendSegment()` is called. The combination `newSeg == true && presetId == 0` reliably identifies a segment newly created from the UI/API (not from a preset). This is the correct injection point for setting `colors[0] = DEFAULT_COLOR` on UI-created segments.

Learnt from: DedeHai
URL: https://github.com/wled/WLED/pull/5686

Timestamp: 2026-06-21T06:31:20.156Z
Learning: In WLED (wled00/FX_fcn.cpp), `makeAutoSegments()` is called from two places: `beginStrip()` during boot (segments should remain BLACK) and `set.cpp` after `finalizeInit()` when the user reconfigures LEDs (segments should start orange/DEFAULT_COLOR). The orange color assignment for auto-segments should be done in `set.cpp` after the `makeAutoSegments()` call, not inside `makeAutoSegments()` itself, to avoid affecting boot-time segment initialization.

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@wled00/FX_fcn.cpp`:
- Around line 1994-1996: The loop at line 1994 uses the variable s as the upper
bound, but when s equals 0, a fallback segment can still be created at line
1990, meaning the actual segment count is now greater than s. This causes the
loop to skip execution entirely, leaving the fallback segment's colors[0]
uninitialized to the default orange color. Replace the loop condition that uses
s with the actual segment count after the fallback creation logic (use the
current size of the _segments array) to ensure all segments including the
fallback are initialized with the default color.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b7783d8c-211f-4664-be91-b7a6fa92159b

📥 Commits

Reviewing files that changed from the base of the PR and between 83cdb46 and 503b30e.

📒 Files selected for processing (5)
  • wled00/FX.h
  • wled00/FX_fcn.cpp
  • wled00/data/settings_leds.htm
  • wled00/json.cpp
  • wled00/wled.cpp
✅ Files skipped from review due to trivial changes (1)
  • wled00/data/settings_leds.htm

Comment thread wled00/FX_fcn.cpp Outdated
Comment thread wled00/wled.cpp Outdated
@DedeHai

DedeHai commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator Author

good to merge from my side. @willmmiles you mentioned breaking changes but I can't really think of anything bad...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants