Some effect options not being synchronized/syncing does not always work

[remixmark]

What happened?

Many sync settings from the same effect do not synchronize and on occasion the effect all together does not synchronize. I have 5 esp32's sync'd. Typically they do not sync when I have the new PS playlist selected.

To Reproduce Bug

I'm not sure if this is because of the effect I have on my playlist or not, but it seems to be a problem with the new PS effects as that's all I have in this playlist.

Create playlist w/ only PS effects and adjust the effect parameters so they're not the defaults.

Expected Behavior

Sync'ing should apply to all parameters and the effect should be identical on all sync'd esp32's.

Install Method

Binary from WLED.me

What version of WLED?

16.0.0-beta

Which microcontroller/board are you seeing the problem on?

ESP32

Relevant log/trace output

Anything else?

No response

Code of Conduct

• I agree to follow this project's Code of Conduct

[remixmark]

Adding sync screenshots:

[softhack007]

@remixmark does it work better when you set the checkmark on "segment options" in sync settings?

[DedeHai]

I am not too familiar with sync as I do not use it, but doesn't "Effects" only sync the FX number?
edit: I just rememberd I added a fix to send out the checkmarks, so there is code syncing the FX parameters, but I do not know when or how it is triggered.
edit2: this #5239

[softhack007]

@coderabbitai please check the code related to udp sync (udp.cpp notify() and parseNotifyPacket() ). Assume that sender and receiver are the latest wled version, so both will use the newest packet version. compare with UI paths that set effect sliders/checkboxes interactively.

Identify credible scenarios that lead to loss / overwriting of effect options. Assume that all sync settings are correctly configured so udp level communication works.

[remixmark]

@remixmark does it work better when you set the checkmark on "segment options" in sync settings?

No same issue, seems just the first 2 (the top ones) effect options only get sync'd.

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get the full udp.cpp file
fd -e cpp udp wled00 --exec cat {}

Length of output: 40021

---

Good, I have the full udp.cpp. Now let me check how the UI path handles effect sliders/checkboxes, and inspect the segment options bit layout for comparison.

---

🏁 Script executed:

#!/bin/bash
# Find deserializeSegment in json.cpp and inspect the relevant section
rg -n "custom1|custom2|custom3|check1|check2|check3|\"fx\"|\"sx\"|\"ix\"|\"c1\"|\"c2\"|\"c3\"|\"o1\"|\"o2\"|\"o3\"" wled00/json.cpp | head -80

echo "---OPTIONS BITS in FX.h---"
rg -n "SELECTED|REVERSED|SEG_OPT|options|freeze|mirror|transpose|map1D2D" wled00/FX.h | grep -i "bit\|0x\|<<\|#define.*SEG\|option" | head -60

echo "---SEGMENT struct fields---"
rg -n "custom1\|custom2\|custom3\|check1\|check2\|check3\|map1D2D" wled00/FX.h | head -30

echo "---deserializeSegment function header---"
rg -n "deserializeSegment\|void.*eSegment\|bool.*eSegment" wled00/json.cpp | head -10

Length of output: 2567

---

🏁 Script executed:

#!/bin/bash
echo "---SEG_OPTION defines---"
rg -n "SEG_OPTION\|#define.*0x00\|#define.*<<" wled00/FX.h | head -40

echo "---full options bit defines block---"
sed -n '130,165p' wled00/FX.h

echo "---deserializeSegment around map1D2D and options---"
sed -n '275,340p' wled00/json.cpp

echo "---notify() options packing (lines around udpOut[9+ofs])---"
rg -n "options\|0x8F\|0x0071\|0b000" wled00/udp.cpp | head -30

Length of output: 3807

---

🏁 Script executed:

#!/bin/bash
echo "---receiveSegmentOptions default and usages---"
rg -n "receiveSegmentOptions\|receiveSegment" wled00/ --type cpp | head -30

echo "---blendMode in udp.cpp---"
rg -n "blendMode\|blend" wled00/udp.cpp | head -20

echo "---blendMode in FX.h struct---"
rg -n "blendMode\|blending" wled00/FX.h | head -20

echo "---notify options 0x8F mask comment or context---"
sed -n '88,105p' wled00/udp.cpp

echo "---parseNotifyPacket options merge context---"
sed -n '210,245p' wled00/udp.cpp

echo "---REVERSE_Y_2D in FX.h---"
rg -n "REVERSE_Y\|reverse_y\|0x40\|0x0040" wled00/FX.h | head -20

Length of output: 2654

---

🏁 Script executed:

#!/bin/bash
echo "---receiveSegmentOptions in wled.h---"
rg -n "receiveSegment\|receiveNotif" wled00/wled.h | head -20

echo "---options bit accessors in FX.h---"
rg -n "reverse_y\|mirror_y\|transpose\|map1D2D\|blendMode" wled00/FX.h | head -30

echo "---options field accessors (getOption/setOption area)---"
sed -n '620,700p' wled00/FX.h

echo "---Segment struct member layout (options area)---"
sed -n '420,445p' wled00/FX.h

Length of output: 7151

---

@softhack007 Here is my analysis of the UDP sync path vs the UI path for effect options, based on reading both notify() and parseNotifyPacket() against the segment struct and deserializeSegment():

---

Bug 1: reverse_y (REVERSE_Y_2D) is always transmitted as 0 — always cleared on receiver

This is the clearest hard bug.

In notify() (sender):

udpOut[9+ofs] = selseg.options & 0x8F;   // 0x8F = 0b10001111

0x8F covers bits: 7 (mirror_y), 3 (mirror), 2 (on), 1 (reverse), 0 (selected).
Bit 6 (reverse_y = 0x40) is not included in 0x8F, so it is always transmitted as 0.

In parseNotifyPacket() (receiver, version > 11):

selseg.options = (selseg.options & 0b0000000000110001U)
               | ((uint16_t)udpIn[28+ofs] << 8)
               | (udpIn[9+ofs] & 0b11001110U);   // 0xCE: bits 7,6,3,2,1

The receiver mask 0b11001110 does include bit 6 (reverse_y). But since the sender never sets it, bit 6 is always 0 in the received byte → reverse_y is always cleared on the receiver after any sync, regardless of what was configured.

Fix: Change sender mask from 0x8F to 0xCF (add bit 6):

udpOut[9+ofs] = selseg.options & 0xCF;   // 0xCF = 0b11001111, adds reverse_y

---

Bug 2: blendMode is never transmitted or received

The UI path (deserializeSegment) sets blendMode via getVal(elem["bm"], seg.blendMode). There is no blendMode field anywhere in notify() or parseNotifyPacket(). The receiver always keeps its own local blendMode regardless of what the sender has set. When an effect depends on a specific blending style, the receiver will render differently. This field needs to be added to the per-segment block (currently at 36 bytes, UDP_SEG_SIZE).

---

Scenario 3: custom1/custom2/custom3/check1/check2/check3 are silently dropped when "Segment options" is not checked

When receiveSegmentOptions = false, the per-segment loop hits an early continue after calling setGeometry():

if (!receiveSegmentOptions) {
    selseg.setGeometry(...);
    strip.resume();
    continue;  // no options, no effects, no custom sliders
}

Execution then falls to the "simple effect sync" path, which only applies mode, speed, and intensity to selected segments — it never touches custom1, custom2, custom3, check1, check2, or check3.

So a user who enables "Effects" sync but does not enable "Segment options" will get mode/speed/intensity synced but all effect-specific parameters (sliders, checkboxes) are silently discarded. Many effects behave completely differently with wrong slider/checkbox values.

The UI path always applies all of them together whenever the segment is updated.

---

Scenario 4: The 1-second send-dedup window causes lost updates for fast changes

if (millis() - notificationSentTime < 1000) return;

If the receiver itself sent a notification within the past second (e.g., a button press, or a relay in a chain), it ignores all incoming packets for that full second. Any changes the sender makes within that window are silently dropped. This is intentional for loop prevention but means rapid successive changes on the sender (e.g., scrubbing a slider) can result in only the first or none of the intermediate states being received.

---

Scenario 5: Segment-count mismatch corrupts segment ID mapping when receiveSegmentBounds = false

When receiveSegmentBounds is false and the receiver has inactive segments interspersed, the code tries to compensate:

if (!selseg.isActive()) {
    inactiveSegs++;
    continue;
} else {
    id += inactiveSegs;  // adjust id
}

This adjustment is wrong in general. Suppose the sender has active segments [0, 1, 2], and the receiver has active segments [0, 2] (segment 1 is inactive). For sender's segment 1: local id=1 is inactive → inactiveSegs=1, skipped. For sender's segment 2: local id=2, adjusted to id=3 which may not exist. Segment 2 data is applied to the wrong or nonexistent slot. In practice this means a single segment-numbering gap between sender and receiver permanently misaligns all subsequent segment updates.

---

Summary table

Field	Sent by notify()	Received by parseNotifyPacket()	UI path
mode, speed, intensity	✅	✅ (both paths)	✅
custom1/2/3, check1/2/3	✅ (v12)	✅ only if receiveSegmentOptions=true AND applyEffects=true	✅ always
reverse_y	❌ (sender masks it out with 0x8F)	reads it (0b11001110) — always clears it	✅
mirror_y, mirror, reverse, on	✅	✅ (v12)	✅
transpose, map1D2D, soundSim	✅ (high byte udpOut[28+ofs])	✅ (v12)	✅
blendMode	❌ missing entirely	❌ missing entirely	✅
palette	✅	✅ only if receiveNotificationPalette	✅

The two actionable bugs are the 0x8F mask (drop-in fix: 0xCF) and the missing blendMode field.

---

🧠 Learnings used

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: BobLoeffler68
Repo: wled/WLED PR: 5440
File: usermods/user_fx/user_fx.cpp:1307-1327
Timestamp: 2026-03-24T07:59:28.661Z
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: BobLoeffler68
Repo: wled/WLED PR: 5440
File: usermods/user_fx/user_fx.cpp:1304-1313
Timestamp: 2026-03-25T07:03:35.475Z
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: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-03-25T12:25:11.610Z
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: 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: 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: 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: softhack007
Repo: wled/WLED PR: 5443
File: wled00/FX_fcn.cpp:1277-1277
Timestamp: 2026-03-24T12:10:32.630Z
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: DedeHai
Repo: wled/WLED PR: 4615
File: wled00/FX.cpp:10824-10824
Timestamp: 2026-03-29T06:08:02.547Z
Learning: WLED: In wled00/FX.cpp::mode_slow_transition(), the change-detection logic intentionally compares data->currentCCT to SEGMENT.cct (not data->endCCT). SEGMENT.cct is set to currentCCT at the end of each call; comparing to endCCT would re-initialize the transition on each frame and stall CCT blending. Do not propose changing this.

---

Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-04-08T20:15:18.188Z
Learning: In WLED effects (wled00/FX.cpp, usermods/user_fx/user_fx.cpp, etc.), the preferred safe pattern for elapsed-time checks with uint32_t timestamps is: store `last_time = now` when the event occurs, then check `(now - last_time) >= delay`. This is always safe against uint32_t wraparound. An alternative that also works is `stored_time = now + delay` combined with the comparison `(int32_t)(now - stored_time) >= 0` (safe for delays < 2^31 ms). The naive pattern `stored_time = now + delay; if (now >= stored_time)` is NOT safe against overflow: when `now + delay` wraps past 2^32, `stored_time` becomes numerically smaller than `now` and the condition fires immediately. Do not flag assignments of the form `last_time = now` as needing wraparound fixes; they are correct. Do flag comparisons of the form `now >= stored_time` where stored_time is a future target.

---

Learnt from: willmmiles
Repo: wled/WLED PR: 4859
File: usermods/udp_name_sync/udp_name_sync.cpp:62-70
Timestamp: 2025-08-21T00:00:17.196Z
Learning: In WLED codebase, UDP packet processing code must never rely on null termination being present on the wire and must always validate packet size first. The codebase already has some vulnerabilities where malformed data can crash the software, so avoiding introduction of new ones is a priority.

---

Learnt from: netmindz
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-02-20T17:38:19.745Z
Learning: In WLED DDP packet handling (wled00/e131.cpp), the destination field should be validated to reject non-display packets. Packets with destination=DDP_ID_STATUS (251) or destination=DDP_ID_CONFIG (250) should be rejected and not processed as LED display data. Only packets with destination=DDP_ID_DISPLAY (1) should be accepted for LED output. WLED doesn't distinguish between multiple display IDs (e.g., display 1 vs display 2), but it must distinguish between DISPLAY packets versus STATUS/CONFIG packets.

---

Learnt from: DedeHai
Repo: wled/WLED PR: 4997
File: wled00/e131.cpp:33-44
Timestamp: 2025-10-10T18:34:06.550Z
Learning: In WLED's DDP packet handling (ws.cpp and e131.cpp), only prevent out-of-bounds memory access rather than enforcing DDP spec compliance. Don't check the 1440-byte spec limit—accept out-of-spec packets assuming correct encoding. The bounds check `maxDataIndex = c + numLeds * ddpChannelsPerLed; if (maxDataIndex > dataLen) reject` is sufficient and already covers the timecode flag case (when c=4) without needing separate validation.

[softhack007]

@coderabbitai please check if sync via esp-now may behave differently from normal udp sync using wifi.

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find and read udp.cpp
fd -e cpp -e c "udp" --full-path | head -20

Length of output: 55

---

🏁 Script executed:

#!/bin/bash
# Also look for notify and parseNotifyPacket
rg -n "notify\|parseNotify\|WLED_SYNC\|notifyPacket" --type cpp -l | head -20

Length of output: 0

---

🏁 Script executed:

#!/bin/bash
cat wled00/udp.cpp

Length of output: 40021

---

Good, I have the full udp.cpp. Now let me examine the Segment options bit layout and the UI/JSON paths for comparison.

---

🏁 Script executed:

#!/bin/bash
# Check Segment options bit definitions in FX.h or similar headers
rg -n "SEG_OPT\|options.*bit\|reverse_y\|mirror_y\|transpose\|check1\|check2\|check3\|custom1\|custom2\|custom3" wled00/FX.h | head -80

Length of output: 0

---

🏁 Script executed:

#!/bin/bash
# Check how custom1/2/3 and check1/2/3 are set via HTTP API (set.cpp) and JSON API (json.cpp)
rg -n "custom1\|custom2\|custom3\|check1\|check2\|check3" wled00/set.cpp wled00/json.cpp | head -80

Length of output: 0

---

🏁 Script executed:

#!/bin/bash
# Find key files
fd -e h "FX" | head -10
fd -e cpp "set|json" --full-path | grep wled | head -10

Length of output: 87

---

🏁 Script executed:

#!/bin/bash
# Search for options/check/custom definitions broadly
rg -n "check1\|check2\|check3\|custom1\|custom2\|custom3" -l | head -20

Length of output: 0

---

🏁 Script executed:

#!/bin/bash
# Look for Segment struct and options bit definitions in FX.h
grep -n "check1\|check2\|check3\|custom1\|custom2\|custom3\|SEG_OPTION\|options\b" wled00/FX.h | head -80

Length of output: 1028

---

🏁 Script executed:

#!/bin/bash
# Search in json.cpp and set.cpp for effect slider/checkbox handling
grep -n "check1\|check2\|check3\|custom1\|custom2\|custom3" wled00/json.cpp wled00/set.cpp | head -80

Length of output: 3305

---

🏁 Script executed:

#!/bin/bash
# Get SEG_OPTION bit constants
grep -n "SEG_OPTION\|#define.*SEG\b" wled00/FX.h | head -40

Length of output: 176

---

🏁 Script executed:

#!/bin/bash
# Get context around the deserializeSegment function in json.cpp - mode/speed/intensity handling
sed -n '240,340p' wled00/json.cpp

Length of output: 3137

---

🏁 Script executed:

#!/bin/bash
# Get the options bit write in set.cpp context
sed -n '1090,1160p' wled00/set.cpp

Length of output: 3206

---

🏁 Script executed:

#!/bin/bash
# Get full Segment struct and options bit layout
sed -n '135,170p' wled00/FX.h

Length of output: 1595

---

🏁 Script executed:

#!/bin/bash
# Get options-related members and bit accessors
sed -n '425,470p' wled00/FX.h

Length of output: 2472

---

🏁 Script executed:

#!/bin/bash
# Check how stateUpdated triggers notify - what callMode is used
grep -n "stateUpdated\|CALL_MODE_DIRECT\|CALL_MODE_NO_NOTIFY\|callMode" wled00/json.cpp wled00/set.cpp | head -40

Length of output: 842

---

@softhack007 Here is a thorough analysis of the notify() / parseNotifyPacket() paths versus the UI (JSON/HTTP) paths.

---

Bug A — reverse_y is silently zeroed on every v12 sync (HIGH)

In notify():

udpOut[9+ofs] = selseg.options & 0x8F; // "selected, mirrored, on, reversed, reverse_y (for 2D)"

The comment mentions reverse_y, but 0x8F = 0b10001111 covers only bits 7, 3, 2, 1, 0 (mirror_y, mirror, on, reverse, selected). Bit 6 (reverse_y = 0x40) is absent from the mask.

In parseNotifyPacket() (version > 11):

selseg.options = (selseg.options & 0b0000000000110001U)
               | ((uint16_t)udpIn[28+ofs] << 8)
               | (udpIn[9+ofs] & 0b11001110U);  // 0xCE = bits 7,6,3,2,1

The receiver reads bit 6 of udpIn[9] and writes it into reverse_y. Since the sender never sets bit 6, the receiver's reverse_y is forced to 0 on every sync, regardless of its original state or what the sender has.

UI path (json.cpp): seg.reverse_y is set directly with no such masking/suppression.

---

Bug B — custom sliders + checkboxes dropped when receiveNotificationEffects = false (HIGH)

The per-segment slider/checkbox write is guarded by applyEffects:

if (version > 11) {
    // options applied unconditionally …
    if (applyEffects) {          // ← gate
        selseg.custom1 = udpIn[29+ofs];
        selseg.custom2 = udpIn[30+ofs];
        selseg.custom3 = udpIn[31+ofs] & 0x1F;
        selseg.check1  = (udpIn[31+ofs]>>5) & 0x1;
        selseg.check2  = (udpIn[31+ofs]>>6) & 0x1;
        selseg.check3  = (udpIn[31+ofs]>>7) & 0x1;
    }
}

where:

bool applyEffects = (receiveNotificationEffects || !someSel);

If the user enables "Sync Segment Options" but disables "Sync Effects", someSel is true and receiveNotificationEffects is false → applyEffects = false → custom1/2/3 and check1/2/3 are silently dropped even though the data was sent and the options-sync path was entered.

UI path: json.cpp calls getVal(elem["c1"], seg.custom1) etc. unconditionally alongside fx/sx/ix—there is no separate "apply effects" gate for custom sliders.

---

Bug C — custom sliders + checkboxes never reachable when receiveSegmentOptions = false (MEDIUM)

When receiveSegmentOptions = false, the per-segment loop either:

• calls continue immediately after setGeometry (geometry-only path), or
• is skipped entirely (old path for version < 11).

The "simple effect sync" fallback at the bottom then runs:

if ((applyEffects || receiveNotificationPalette) && (version < 11 || !receiveSegmentOptions)) {
    for (...) {
        seg.setMode(udpIn[8]);
        seg.speed = udpIn[9];
        if (version > 2) seg.intensity = udpIn[16];
    }
}

This reads the main-segment header bytes (8, 9, 16), not per-segment slider bytes. custom1/2/3 and check1/2/3 have no representation in the legacy header and are never applied on this path regardless of version.

Additionally, this fallback applies to locally selected segments, not the specific segments the sender intended.

---

Bug D — Segment ID mismatch when inactive segments exist (MEDIUM)

In notify() the sent ID is the sequential count of active segments (s), not the actual segment array index (i):

size_t s = 0;
for (size_t i = 0; i < nsegs; i++) {
    if (!selseg.isActive()) continue;
    udpOut[0+ofs] = s;   // ← sequential active-segment counter, not array index
    ...
    ++s;
}

In parseNotifyPacket() the received ID is used as a direct array index:

unsigned id = udpIn[0+ofs];
Segment& selseg = strip.getSegment(id);  // treats id as array index

If either side has inactive segments interspersed in the array, IDs are misaligned and effects land on the wrong segments.

The inactiveSegs compensation exists but is dead code: selseg is already bound to strip.getSegment(id) before id += inactiveSegs executes, so the adjustment never affects which segment is updated.

---

Bug E — blendMode not included in the packet (LOW)

Segment::blendMode is serialised by json.cpp and set via the UI/JSON path, but is absent from the UDP packet structure entirely.

---

Bug F — Dead first options write for version 12 (cosmetic)

For every v12 per-segment, options is written twice:

// first write (for v11 compat):
selseg.options = (selseg.options & 0x0071U) | (udpIn[9+ofs] & 0x0E);
// immediately overwritten inside if (version > 11):
selseg.options = (selseg.options & 0b0000000000110001U) | ((uint16_t)udpIn[28+ofs]<<8) | (udpIn[9+ofs] & 0b11001110U);

The first write is no-op for v12 but it also transiently preserves reverse_y from local state — which the second write then clears (due to Bug A).

---

Root-cause summary for the original issue (effect slider/checkbox sync)

The scenario most likely responsible for the screenshots in the issue:

Condition	Result
receiveSegmentOptions=false (default)	custom1/2/3, check1/2/3 can never be applied — no code path reaches them
receiveSegmentOptions=true, receiveNotificationEffects=false	Sliders/checks silently dropped by the applyEffects gate
receiveSegmentOptions=true, receiveNotificationEffects=true	Works, except reverse_y always clobbered to 0 (Bug A) and inactive-segment topology mismatch (Bug D)

The fix for the most common case (Bug B) would be to allow custom sliders/checks through whenever receiveSegmentOptions is true, independently of applyEffects — mirroring what the JSON path does.

---

🧠 Learnings used

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: BobLoeffler68
Repo: wled/WLED PR: 5440
File: usermods/user_fx/user_fx.cpp:1307-1327
Timestamp: 2026-03-24T07:59:28.661Z
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: BobLoeffler68
Repo: wled/WLED PR: 5440
File: usermods/user_fx/user_fx.cpp:1304-1313
Timestamp: 2026-03-25T07:03:35.475Z
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: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-03-25T12:25:11.610Z
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: 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: 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: 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: softhack007
Repo: wled/WLED PR: 5443
File: wled00/FX_fcn.cpp:1277-1277
Timestamp: 2026-03-24T12:10:32.630Z
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: DedeHai
Repo: wled/WLED PR: 4615
File: wled00/FX.cpp:10824-10824
Timestamp: 2026-03-29T06:08:02.547Z
Learning: WLED: In wled00/FX.cpp::mode_slow_transition(), the change-detection logic intentionally compares data->currentCCT to SEGMENT.cct (not data->endCCT). SEGMENT.cct is set to currentCCT at the end of each call; comparing to endCCT would re-initialize the transition on each frame and stall CCT blending. Do not propose changing this.

---

Learnt from: willmmiles
Repo: wled/WLED PR: 4859
File: usermods/udp_name_sync/udp_name_sync.cpp:62-70
Timestamp: 2025-08-21T00:00:17.196Z
Learning: In WLED codebase, UDP packet processing code must never rely on null termination being present on the wire and must always validate packet size first. The codebase already has some vulnerabilities where malformed data can crash the software, so avoiding introduction of new ones is a priority.

---

Learnt from: netmindz
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-02-20T17:38:19.745Z
Learning: In WLED DDP packet handling (wled00/e131.cpp), the destination field should be validated to reject non-display packets. Packets with destination=DDP_ID_STATUS (251) or destination=DDP_ID_CONFIG (250) should be rejected and not processed as LED display data. Only packets with destination=DDP_ID_DISPLAY (1) should be accepted for LED output. WLED doesn't distinguish between multiple display IDs (e.g., display 1 vs display 2), but it must distinguish between DISPLAY packets versus STATUS/CONFIG packets.

---

Learnt from: DedeHai
Repo: wled/WLED PR: 4997
File: wled00/e131.cpp:33-44
Timestamp: 2025-10-10T18:34:06.550Z
Learning: In WLED's DDP packet handling (ws.cpp and e131.cpp), only prevent out-of-bounds memory access rather than enforcing DDP spec compliance. Don't check the 1440-byte spec limit—accept out-of-spec packets assuming correct encoding. The bounds check `maxDataIndex = c + numLeds * ddpChannelsPerLed; if (maxDataIndex > dataLen) reject` is sufficient and already covers the timecode flag case (when c=4) without needing separate validation.

---

Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-04-08T20:15:18.188Z
Learning: In WLED effects (wled00/FX.cpp, usermods/user_fx/user_fx.cpp, etc.), the preferred safe pattern for elapsed-time checks with uint32_t timestamps is: store `last_time = now` when the event occurs, then check `(now - last_time) >= delay`. This is always safe against uint32_t wraparound. An alternative that also works is `stored_time = now + delay` combined with the comparison `(int32_t)(now - stored_time) >= 0` (safe for delays < 2^31 ms). The naive pattern `stored_time = now + delay; if (now >= stored_time)` is NOT safe against overflow: when `now + delay` wraps past 2^32, `stored_time` becomes numerically smaller than `now` and the condition fires immediately. Do not flag assignments of the form `last_time = now` as needing wraparound fixes; they are correct. Do flag comparisons of the form `now >= stored_time` where stored_time is a future target.

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "---Find ESP-NOW files---"
fd -e cpp -e h -e ino espnow --exec echo {}
fd -i espnow --exec echo {}
fd -i "esp_now\|espnow\|esp-now" --exec echo {}

echo "---Search for espnow in all source files---"
rg -rn -i "espnow\|esp_now\|ESP_NOW\|ESPNOW" --type cpp -l

echo "---Search in all files---"
rg -rn -i "espnow\|esp_now" -l | head -20

Length of output: 94

---

🏁 Script executed:

#!/bin/bash
echo "---Broader search for ESP-NOW keywords---"
rg -rn -i "esp.now\|espnow\|esp_now" -l 2>/dev/null | head -30

echo "---Search for 'now' in sync context---"
rg -rn "NOW\|espnow" wled00/ 2>/dev/null | head -30

echo "---List all files in wled00---"
ls wled00/*.cpp wled00/*.h 2>/dev/null | head -40

echo "---List usermods---"
ls usermods/ 2>/dev/null | head -50

echo "---Search for ESP-NOW in all .cpp .h files---"
grep -rl -i "esp.now\|espnow" --include="*.cpp" --include="*.h" --include="*.ino" . 2>/dev/null | head -20

echo "---Search the word 'espnow' in any file---"
grep -rl "espnow\|ESP_NOW\|esp_now" . 2>/dev/null | head -20

Length of output: 2000

---

🏁 Script executed:

#!/bin/bash
echo "---espnow in udp.cpp---"
grep -n -i "espnow\|esp_now" wled00/udp.cpp

echo "---espnow in wled.h---"
grep -n -i "espnow\|esp_now" wled00/wled.h

echo "---espnow in const.h---"
grep -n -i "espnow\|esp_now" wled00/const.h

echo "---espnow in wled.cpp---"
grep -n -i "espnow\|esp_now" wled00/wled.cpp

echo "---espnow in cfg.cpp---"
grep -n -i "espnow\|esp_now" wled00/cfg.cpp

echo "---espnow in remote.cpp---"
grep -n -i "espnow\|esp_now" wled00/remote.cpp

echo "---espnow in fcn_declare.h---"
grep -n -i "espnow\|esp_now" wled00/fcn_declare.h

Length of output: 4428

---

🏁 Script executed:

#!/bin/bash
echo "---PartialEspNowPacket struct and notify() ESP-NOW block---"
sed -n '1,50p' wled00/udp.cpp

echo "---notify() ESP-NOW send section---"
sed -n '140,200p' wled00/udp.cpp

echo "---espNowReceiveCB full---"
sed -n '899,970p' wled00/udp.cpp

echo "---remote.cpp ESP-NOW full---"
cat wled00/remote.cpp | head -250

Length of output: 16837

---

🏁 Script executed:

#!/bin/bash
echo "---espNowReceiveCB continuation and parseNotifyPacket call---"
sed -n '955,1010p' wled00/udp.cpp

echo "---parseNotifyPacket first 20 lines---"
grep -n "parseNotifyPacket" wled00/udp.cpp | head -10

echo "---notificationSentTime check in parseNotifyPacket---"
sed -n '195,230p' wled00/udp.cpp

Length of output: 3540

---

@softhack007 Here is the comparison of ESP-NOW sync vs. normal UDP/WiFi sync:

---

Key structural similarity

The payload is identical. notify() builds the same udpOut[] buffer for both paths, then branches. ESP-NOW copies that buffer into PartialEspNowPacket chunks and sends via quickEspNow.send(). On the receiving side, espNowReceiveCB() reassembles the chunks and calls the same parseNotifyPacket(). This means all bugs from the previous analysis carry over unchanged:

• reverse_y always transmitted as 0 (sender mask 0x8F)
• blendMode never transmitted
• custom1/2/3, check1/2/3 silently dropped when receiveSegmentOptions = false

---

Differences between ESP-NOW and WiFi UDP sync

1. ESP-NOW only activates without WiFi

if (len < 3 || !broadcast || buffer->magic != 'W' || !useESPNowSync || WLED_CONNECTED) {
    return;  // silently drops packet if WiFi is connected
}

ESP-NOW sync is a WiFi-less fallback only. If the receiver has a WiFi connection, all incoming ESP-NOW sync packets are silently discarded. The two modes are mutually exclusive on the receiver side.

2. Sender allowlist (linked_remotes)

The receiver requires the sender's MAC address to be in linked_remotes. Any WLED device not on that list is silently ignored, even if it sends a correctly formatted packet with matching magic bytes. WiFi UDP has no equivalent — any device in the same sync group can send and be processed.

3. Stricter rate limiting: 250 ms on top of the 1 s dedup

ESP-NOW has an extra throttle in the callback:

if (millis() - lastProcessed > 250) {
    parseNotifyPacket(udpIn);
    lastProcessed = millis();
} else {
    // silently ignored
}

This 250 ms gate fires before the 1000 ms sent-self check inside parseNotifyPacket. A receiver running ESP-NOW will process at most ~4 sync messages per second (vs. WiFi UDP which only has the 1 s sent-self protection). Fast slider scrubbing on the sender will see more dropped updates under ESP-NOW.

4. Multi-packet fragmentation with fragile abort-on-gap behavior

The first ESP-NOW frame carries the 41-byte global header plus up to 5 segments (~247 bytes). With 6+ segments, additional frames are sent. The receiver expects frames numbered 0, 1, 2, ... strictly in order:

} else {
    // any out of order packet or incorrectly sized packet [...] will abort
    if (udpIn) free(udpIn);
    udpIn = nullptr; packetsReceived = 0; segsReceived = 0;
    return;
}

Any lost, duplicated, or reordered continuation frame silently aborts the entire reassembly. There is no retry or recovery. Under typical conditions (small segment counts) this is not an issue, but setups with many segments are at risk.

5. Ring buffer overflow for >17 segments

The sender code itself has a warning comment:

// WARNING: this will only work for up to 3 messages (~17 segments) as QuickESPNOW only has a ring buffer capable of holding 3 queued messages

If a setup has more than ~17 segments, the sender silently drops the overflow frames. The receiver reassembly will abort (wrong packet sequence), and the sync is completely lost for that update.

6. Static-variable concurrency hazard in callback context

The reassembly state in espNowReceiveCB() uses static local variables (udpIn, packetsReceived, segsReceived, lastProcessed), and the callback runs in the WiFi/interrupt context with no mutex. If two senders transmit simultaneously, or a new message arrives before the current reassembly completes, the static state is silently corrupted. The WiFi UDP path processes packets sequentially in the main loop and has no equivalent race.

---

Summary

Property	WiFi UDP	ESP-NOW
When active	WiFi connected	WiFi not connected
Packet payload	udpOut[]	Same udpOut[], chunked
Sender authentication	None (sync group only)	MAC allowlist (linked_remotes)
Rate limit	1 s sent-self dedup	250 ms throttle + 1 s sent-self dedup
Fragmentation	Single UDP datagram	Multi-frame with abort-on-gap
Max segments	All	~17 practical limit
Concurrency safety	Main loop, sequential	Callback, shared static state, no mutex
Bugs carried over	reverse_y, blendMode, custom sliders	Same bugs (same parseNotifyPacket)

The most operationally significant difference for the issue reporter is the WLED_CONNECTED exclusion — if their devices are on WiFi, ESP-NOW sync is dead-on-arrival. The 250 ms throttle is a secondary concern for fast changes.

---

🧠 Learnings used

Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-03-25T12:25:11.610Z
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: 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: 5440
File: usermods/user_fx/user_fx.cpp:1304-1313
Timestamp: 2026-03-25T07:03:35.475Z
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: 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: softhack007
Repo: wled/WLED PR: 5443
File: wled00/FX_fcn.cpp:1277-1277
Timestamp: 2026-03-24T12:10:32.630Z
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: DedeHai
Repo: wled/WLED PR: 5464
File: wled00/FX_fcn.cpp:1699-1701
Timestamp: 2026-04-09T07:26:14.510Z
Learning: In WLED (wled00/util.cpp), `allocate_buffer()` processes `BFRALLOC_NOBYTEACCESS` in an `if` with an `else` that covers all other flags. On classic ESP32, this means `BFRALLOC_ENFORCE_PSRAM` and `BFRALLOC_PREFER_PSRAM` are silently ignored when `BFRALLOC_NOBYTEACCESS` is also set — IRAM (MALLOC_CAP_32BIT) is always tried first. Combining `BFRALLOC_CLEAR | BFRALLOC_NOBYTEACCESS` is unsafe: the allocator's internal `memset` at util.cpp:901 will perform byte-level access on a potentially IRAM-resident buffer. This affects `Segment::pixels` in `setGeometry()` (FX.h) and `_pixels` in `finalizeInit()` (FX_fcn.cpp). Additionally, `memcpy` on `Segment::pixels` in the copy constructor and copy assignment (FX_fcn.cpp lines ~73 and ~116) is also a byte-level violation when the buffer is in IRAM. The safe pattern is element-wise 32-bit access (for loops over `uint32_t*`) rather than `memset`/`memcpy` for all `BFRALLOC_NOBYTEACCESS` buffers.

---

Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-04-10T16:10:42.477Z
Learning: In WLED (wled/WLED), the segments vector (`strip.segments`) is highly volatile: preset loads, effect changes, and JSON state updates in json.cpp can add/remove/reorder segments and change their active status at any time. Segments have no stable persistent identifier — only their ordinal index in the vector, which also changes. Code that tries to maintain a discovery-time binding from an external protocol (e.g., Espalexa/Alexa) to a specific segment using ordinal-among-active-segments as a key will silently map to wrong segments after any preset load. A correct implementation must store the init-time segment-index-to-device mapping explicitly and guard against the segment at that index changing identity after a vector rebuild.

---

Learnt from: softhack007
Repo: wled/WLED PR: 5443
File: wled00/FX_fcn.cpp:1277-1277
Timestamp: 2026-03-24T12:13:21.670Z
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: softhack007
Repo: wled/WLED PR: 5508
File: wled00/FX.cpp:4137-4142
Timestamp: 2026-04-16T09:49:58.587Z
Learning: WLED overlay behavior: Effects must operate via SEGMENT.* APIs (reads/writes) — global frame reads (strip.getPixelColor*) are reserved for copy_* style post‑processing. Blending between segments is performed later in WS2812FX::blendSegment(); effects should not try to composite with lower layers themselves.

---

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: DedeHai
Repo: wled/WLED PR: 4997
File: wled00/e131.cpp:33-44
Timestamp: 2025-10-10T18:34:06.550Z
Learning: In WLED's DDP packet handling (ws.cpp and e131.cpp), only prevent out-of-bounds memory access rather than enforcing DDP spec compliance. Don't check the 1440-byte spec limit—accept out-of-spec packets assuming correct encoding. The bounds check `maxDataIndex = c + numLeds * ddpChannelsPerLed; if (maxDataIndex > dataLen) reject` is sufficient and already covers the timecode flag case (when c=4) without needing separate validation.

---

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: 5048
File: wled00/set.cpp:551-555
Timestamp: 2026-03-27T21:00:25.902Z
Learning: In WLED PR `#5048` (wled00/set.cpp lines 551-555), the CONFIG_IDF_TARGET_ESP32C5 block that unconditionally forces ntpEnabled = false is an intentional **temporary hotfix** by softhack007 for a known ESP32-C5 crash: `assert failed: udp_new_ip_type udp.c:1278 (Required to lock TCPIP core functionality!)`. Do not flag this as a permanent design issue; the TODO comment in the code already notes it should be resolved properly once the underlying IDF/TCPIP bug on C5 is fixed. A future permanent solution should use a target capability flag rather than overwriting the user's setting.

---

Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-04-08T20:15:18.188Z
Learning: In WLED effects (wled00/FX.cpp, usermods/user_fx/user_fx.cpp, etc.), the preferred safe pattern for elapsed-time checks with uint32_t timestamps is: store `last_time = now` when the event occurs, then check `(now - last_time) >= delay`. This is always safe against uint32_t wraparound. An alternative that also works is `stored_time = now + delay` combined with the comparison `(int32_t)(now - stored_time) >= 0` (safe for delays < 2^31 ms). The naive pattern `stored_time = now + delay; if (now >= stored_time)` is NOT safe against overflow: when `now + delay` wraps past 2^32, `stored_time` becomes numerically smaller than `now` and the condition fires immediately. Do not flag assignments of the form `last_time = now` as needing wraparound fixes; they are correct. Do flag comparisons of the form `now >= stored_time` where stored_time is a future target.

---

Learnt from: willmmiles
Repo: wled/WLED PR: 4859
File: usermods/udp_name_sync/udp_name_sync.cpp:62-70
Timestamp: 2025-08-21T00:00:17.196Z
Learning: In WLED codebase, UDP packet processing code must never rely on null termination being present on the wire and must always validate packet size first. The codebase already has some vulnerabilities where malformed data can crash the software, so avoiding introduction of new ones is a priority.

---

Learnt from: DedeHai
Repo: wled/WLED PR: 5464
File: wled00/FX_fcn.cpp:1699-1701
Timestamp: 2026-04-09T09:14:32.589Z
Learning: In WLED, pixel buffers allocated with `BFRALLOC_NOBYTEACCESS` (both `WS2812FX::_pixels` and `Segment::pixels`) are intentionally placed in 32-bit-only accessible IRAM by design. Buffers up to ~1000 LEDs (~4 kB) easily fit within IRAM's ~50 kB pool. `memset` and `memcpy` on these buffers work correctly in practice because the Xtensa newlib assembly implementations use 32-bit stores for word-aligned, word-multiple-size buffers — satisfying the IRAM hardware constraint as an implementation property of the assembly code, which is unlikely to change across IDF versions. The risk of a future violation exists but is accepted as very low. Do not flag `memset`/`memcpy` usage on `BFRALLOC_NOBYTEACCESS` pixel buffers as a critical bug in WLED reviews; note it as a low-risk accepted pattern.

[DedeHai]

to me this sounds like a bug but it could be intentional. but why would sync FX only sync speed and intensity and not the other sliders? so either this was forgotten when the new sliders were added (0.13?) or it was an intentional choice to no sync them.

[softhack007]

Execution then falls to the "simple effect sync" path, which only applies mode, speed, and intensity to selected segments — it never touches custom1, custom2, custom3, check1, check2, or check3.

This might be something to chase further down ... but then it would be a bug that is ages old - "custom" and "check" fields exist since wled 0.14.0, maybe they even existed in 0.13.x already.

[softhack007]

@coderabbitai please look thought the git history of the code involved in notify() and parseNotifyPacket(), and checking if any of your bug scenarios are regressions since wled 0.14.3.

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find version tags near 0.14.3
git tag | grep -E "0\.14|0\.15|0\.16" | sort -V | head -40

echo "---Find commit for 0.14.3---"
git log --oneline --decorate | grep -i "0\.14\|0\.15" | head -20

echo "---Git log of udp.cpp (last 60 commits)---"
git log --oneline --follow -- wled00/udp.cpp | head -60

echo "---Git log with dates of udp.cpp---"
git log --format="%h %ad %s" --date=short --follow -- wled00/udp.cpp | head -60

Length of output: 10613

---

🏁 Script executed:

#!/bin/bash
# Find when custom1/check1 UDP fields were added - search history
git log --all --oneline --follow -S "custom1" -- wled00/udp.cpp | head -20
echo "---check1 history---"
git log --all --oneline --follow -S "check1" -- wled00/udp.cpp | head -20
echo "---0x8F history---"
git log --all --oneline --follow -S "0x8F" -- wled00/udp.cpp | head -20
echo "---reverse_y in udp.cpp history---"
git log --all --oneline --follow -S "reverse_y" -- wled00/udp.cpp | head -20
echo "---version > 11 history---"
git log --all --oneline --follow -S "version > 11" -- wled00/udp.cpp | head -20
echo "---UDP_SEG_SIZE history---"
git log --all --oneline --follow -S "UDP_SEG_SIZE" -- wled00/udp.cpp | head -20

Length of output: 873

---

🏁 Script executed:

#!/bin/bash
echo "=== Date of 0.14.3 commit ==="
git show --format="%H %ad %s" --date=short -s 6d278994

echo "=== Date/content of 3bae3aa9 (new sliders/2D sync) ==="
git show --format="%H %ad %s" --date=short -s 3bae3aa9

echo "=== Is 3bae3aa9 an ancestor of 0.14.3? ==="
git merge-base --is-ancestor 3bae3aa9 6d278994 && echo "YES - predates 0.14.3" || echo "NO - NOT in 0.14.3 lineage"

echo "=== What did 787d8a73 (fix FX checkmark sync) change? ==="
git show 787d8a73 -- wled00/udp.cpp | grep "^[+-]" | grep -v "^---\|^+++" | head -60

echo "=== What did 96700fe3 (Fix for udp sync `#3487`) change? ==="
git show 96700fe3 -- wled00/udp.cpp | grep "^[+-]" | grep -v "^---\|^+++" | head -60

echo "=== What did 367a4e17 (Fix udp sync `#3487`) change? ==="
git show 367a4e17 -- wled00/udp.cpp | grep "^[+-]" | grep -v "^---\|^+++" | head -60

echo "=== What did 347ec91b (notify() bugfix) change? ==="
git show 347ec91b -- wled00/udp.cpp | grep "^[+-]" | grep -v "^---\|^+++" | head -60

Length of output: 2075

---

🏁 Script executed:

#!/bin/bash
echo "=== Original 3bae3aa9 diff - notify() and parseNotifyPacket() changes ==="
git show 3bae3aa9 -- wled00/udp.cpp | grep "^[+-]" | grep -v "^---\|^+++" | head -120

echo "=== State of udp.cpp at 0.14.3 commit - options masking section ==="
git show 6d278994:wled00/udp.cpp | grep -n "0x8F\|custom1\|custom2\|check1\|check2\|check3\|reverse_y\|0b110\|version.*11\|applyEffects\|receiveSegment" | head -40

echo "=== State of 0x8F mask and version>11 section at 0.14.3 ==="
git show 6d278994:wled00/udp.cpp | sed -n '80,130p'

Length of output: 7221

---

@softhack007 Here are the results from tracing the git history. The 0.14.3 release is commit 6d278994 dated 2024-03-29, and 3bae3aa9 (2022-10-31, "UDP Sync fix - sync new sliders - sync 2D options") is confirmed as an ancestor.

---

Results: Which bugs are regressions since 0.14.3?

None of the previously identified bugs A/B/C are regressions — they all predate 0.14.3

Bug	First introduced	At 0.14.3?	Fixed after 0.14.3?
A: reverse_y sender zeroed (0x8F mask)	3bae3aa9 2022-10-31	✅ present	❌ not fixed
A: receiver clears reverse_y (mask mismatch)	367a4e17 2023-10-27	✅ present	❌ not fixed
B: custom sliders behind applyEffects gate	3bae3aa9 2022-10-31	✅ present	❌ not fixed
C: custom sliders unreachable without receiveSegmentOptions	3bae3aa9 2022-10-31	✅ present	❌ not fixed
check2/check3 never synced (copy-paste bug)	3bae3aa9 2022-10-31	✅ present	✅ fixed by PR #5239 (2025-12-29)
E: blendMode absent from packet	ee9ac947 2025-04-22	❌ (field didn't exist)	❌ never added

---

The check2/check3 copy-paste bug deserves special attention

This is the bug most directly explaining the issue reporter's symptom ("only the first 2 effect options get sync'd"). The original 3bae3aa9 commit introduced this copy-paste error that survived until PR #5239:

// State at 0.14.3 (present since 2022-10-31):
selseg.check1  = (udpIn[31+ofs]>>5) & 0x1;
selseg.check1  = (udpIn[31+ofs]>>6) & 0x1;  // BUG: should be check2 — overwrites check1
selseg.check1  = (udpIn[31+ofs]>>7) & 0x1;  // BUG: should be check3 — overwrites check1

// Fixed by 787d8a73 (PR `#5239`, 2025-12-29):
selseg.check1  = (udpIn[31+ofs]>>5) & 0x1;
selseg.check2  = (udpIn[31+ofs]>>6) & 0x1;  // ✅
selseg.check3  = (udpIn[31+ofs]>>7) & 0x1;  // ✅

So with the current main branch code, check2/check3 are now correctly received — but only when both receiveSegmentOptions=true and receiveNotificationEffects=true (Bug B gate is still in place).

---

The reverse_y mask asymmetry was introduced between 3bae3aa9 and 0.14.3

• 3bae3aa9 original receiver mask: & 0x8E → did NOT include bit 6 (reverse_y) on receive side either. Internally consistent.
• 367a4e17/96700fe3 (2023-10-27, still before 0.14.3) changed the receiver to & 0b11001110U (added bit 6), but the sender was not updated from 0x8F. This created the sender/receiver mismatch — the receiver now zeroes reverse_y on every sync rather than preserving it.

---

Bug E (blendMode) is a post-0.14.3 omission

blendMode was added as a Segment member in ee9ac947 (2025-04-22). It has never been added to the UDP sync packet. Since 0.14.3 predates that commit, this is not a regression from 0.14.3 — it's a new field that was simply never wired into sync when the blending overhaul landed.

---

Summary for 0.14.3 baseline

All of the core sync issues — custom sliders and checkmarks gated behind receiveSegmentOptions, the applyEffects gate, and reverse_y silently cleared — were already present at 0.14.3. The only post-0.14.3 fix applied to this area was PR #5239 correcting the check2/check3 copy-paste bug in December 2025.

---

🧠 Learnings used

Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-03-25T12:25:11.610Z
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: 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: 5440
File: usermods/user_fx/user_fx.cpp:1304-1313
Timestamp: 2026-03-25T07:03:35.475Z
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: 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: softhack007
Repo: wled/WLED PR: 5443
File: wled00/FX_fcn.cpp:1277-1277
Timestamp: 2026-03-24T12:10:32.630Z
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: DedeHai
Repo: wled/WLED PR: 5464
File: wled00/FX_fcn.cpp:1699-1701
Timestamp: 2026-04-09T07:26:14.510Z
Learning: In WLED (wled00/util.cpp), `allocate_buffer()` processes `BFRALLOC_NOBYTEACCESS` in an `if` with an `else` that covers all other flags. On classic ESP32, this means `BFRALLOC_ENFORCE_PSRAM` and `BFRALLOC_PREFER_PSRAM` are silently ignored when `BFRALLOC_NOBYTEACCESS` is also set — IRAM (MALLOC_CAP_32BIT) is always tried first. Combining `BFRALLOC_CLEAR | BFRALLOC_NOBYTEACCESS` is unsafe: the allocator's internal `memset` at util.cpp:901 will perform byte-level access on a potentially IRAM-resident buffer. This affects `Segment::pixels` in `setGeometry()` (FX.h) and `_pixels` in `finalizeInit()` (FX_fcn.cpp). Additionally, `memcpy` on `Segment::pixels` in the copy constructor and copy assignment (FX_fcn.cpp lines ~73 and ~116) is also a byte-level violation when the buffer is in IRAM. The safe pattern is element-wise 32-bit access (for loops over `uint32_t*`) rather than `memset`/`memcpy` for all `BFRALLOC_NOBYTEACCESS` buffers.

---

Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-04-10T16:10:42.477Z
Learning: In WLED (wled/WLED), the segments vector (`strip.segments`) is highly volatile: preset loads, effect changes, and JSON state updates in json.cpp can add/remove/reorder segments and change their active status at any time. Segments have no stable persistent identifier — only their ordinal index in the vector, which also changes. Code that tries to maintain a discovery-time binding from an external protocol (e.g., Espalexa/Alexa) to a specific segment using ordinal-among-active-segments as a key will silently map to wrong segments after any preset load. A correct implementation must store the init-time segment-index-to-device mapping explicitly and guard against the segment at that index changing identity after a vector rebuild.

---

Learnt from: softhack007
Repo: wled/WLED PR: 5443
File: wled00/FX_fcn.cpp:1277-1277
Timestamp: 2026-03-24T12:13:21.670Z
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: softhack007
Repo: wled/WLED PR: 5508
File: wled00/FX.cpp:4137-4142
Timestamp: 2026-04-16T09:49:58.587Z
Learning: WLED overlay behavior: Effects must operate via SEGMENT.* APIs (reads/writes) — global frame reads (strip.getPixelColor*) are reserved for copy_* style post‑processing. Blending between segments is performed later in WS2812FX::blendSegment(); effects should not try to composite with lower layers themselves.

---

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: DedeHai
Repo: wled/WLED PR: 4997
File: wled00/e131.cpp:33-44
Timestamp: 2025-10-10T18:34:06.550Z
Learning: In WLED's DDP packet handling (ws.cpp and e131.cpp), only prevent out-of-bounds memory access rather than enforcing DDP spec compliance. Don't check the 1440-byte spec limit—accept out-of-spec packets assuming correct encoding. The bounds check `maxDataIndex = c + numLeds * ddpChannelsPerLed; if (maxDataIndex > dataLen) reject` is sufficient and already covers the timecode flag case (when c=4) without needing separate validation.

---

Learnt from: DedeHai
Repo: wled/WLED PR: 5464
File: wled00/FX_fcn.cpp:1699-1701
Timestamp: 2026-04-09T09:14:32.589Z
Learning: In WLED, pixel buffers allocated with `BFRALLOC_NOBYTEACCESS` (both `WS2812FX::_pixels` and `Segment::pixels`) are intentionally placed in 32-bit-only accessible IRAM by design. Buffers up to ~1000 LEDs (~4 kB) easily fit within IRAM's ~50 kB pool. `memset` and `memcpy` on these buffers work correctly in practice because the Xtensa newlib assembly implementations use 32-bit stores for word-aligned, word-multiple-size buffers — satisfying the IRAM hardware constraint as an implementation property of the assembly code, which is unlikely to change across IDF versions. The risk of a future violation exists but is accepted as very low. Do not flag `memset`/`memcpy` usage on `BFRALLOC_NOBYTEACCESS` pixel buffers as a critical bug in WLED reviews; note it as a low-risk accepted pattern.

---

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: 5048
File: wled00/cfg.cpp:673-673
Timestamp: 2026-03-28T00:51:30.926Z
Learning: In WLED PR `#5048`, the NTP-related crash on ESP32-C5 (pioarduino IDF 5.5.x with CONFIG_LWIP_CHECK_THREAD_SAFETY=y) is definitively NOT caused by ntpUdp.begin(). The crash dump shows the real call chain: WLED::loop() → handleTime() → handleNetworkTime() (ntp.cpp:212) → AsyncDNS::query() (asyncDNS.h:45) → dns_gethostbyname_addrtype() → dns_alloc_pcb() → dns_alloc_random_port() → udp_new_ip_type() ← ASSERT. The fix must be in AsyncDNS::query() (asyncDNS.h:45): the call to dns_gethostbyname_addrtype() must be dispatched to the TCPIP thread via tcpip_callback_with_block() on IDF V5+. The ntpUdp.begin() call sites (wled.cpp:926 and set.cpp:562) are NOT the crash location for this particular bug.

---

Learnt from: softhack007
Repo: wled/WLED PR: 5048
File: wled00/cfg.cpp:673-673
Timestamp: 2026-03-27T21:17:51.985Z
Learning: In WLED PR `#5048`, the NTP/UDP crash on ESP32-C5 (pioarduino IDF 5.5.x): CONFIG_LWIP_TCPIP_CORE_LOCKING defaults to n (disabled) in arduino-esp32/pioarduino, while CONFIG_LWIP_CHECK_THREAD_SAFETY=y is what actually causes the "Required to lock TCPIP core functionality!" assertion. LOCK_TCPIP_CORE() / UNLOCK_TCPIP_CORE() macros are only functional when CONFIG_LWIP_TCPIP_CORE_LOCKING=y; using them without enabling that config has no effect. The correct fix without sdkconfig changes is to use tcpip_callback() to schedule ntpUdp.begin() (and other raw lwIP API calls) on the TCPIP thread, which works regardless of the locking mode setting. The tasmota core (IDF 5.3.x, used for C6) likely has CONFIG_LWIP_CHECK_THREAD_SAFETY=n, which is why the same violation does not crash on C6.

---

Learnt from: softhack007
Repo: wled/WLED PR: 5048
File: wled00/set.cpp:551-555
Timestamp: 2026-03-27T21:00:25.902Z
Learning: In WLED PR `#5048` (wled00/set.cpp lines 551-555), the CONFIG_IDF_TARGET_ESP32C5 block that unconditionally forces ntpEnabled = false is an intentional **temporary hotfix** by softhack007 for a known ESP32-C5 crash: `assert failed: udp_new_ip_type udp.c:1278 (Required to lock TCPIP core functionality!)`. Do not flag this as a permanent design issue; the TODO comment in the code already notes it should be resolved properly once the underlying IDF/TCPIP bug on C5 is fixed. A future permanent solution should use a target capability flag rather than overwriting the user's setting.

---

Learnt from: willmmiles
Repo: wled/WLED PR: 4859
File: usermods/udp_name_sync/udp_name_sync.cpp:62-70
Timestamp: 2025-08-21T00:00:17.196Z
Learning: In WLED codebase, UDP packet processing code must never rely on null termination being present on the wire and must always validate packet size first. The codebase already has some vulnerabilities where malformed data can crash the software, so avoiding introduction of new ones is a priority.

---

Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-04-08T20:15:18.188Z
Learning: In WLED effects (wled00/FX.cpp, usermods/user_fx/user_fx.cpp, etc.), the preferred safe pattern for elapsed-time checks with uint32_t timestamps is: store `last_time = now` when the event occurs, then check `(now - last_time) >= delay`. This is always safe against uint32_t wraparound. An alternative that also works is `stored_time = now + delay` combined with the comparison `(int32_t)(now - stored_time) >= 0` (safe for delays < 2^31 ms). The naive pattern `stored_time = now + delay; if (now >= stored_time)` is NOT safe against overflow: when `now + delay` wraps past 2^32, `stored_time` becomes numerically smaller than `now` and the condition fires immediately. Do not flag assignments of the form `last_time = now` as needing wraparound fixes; they are correct. Do flag comparisons of the form `now >= stored_time` where stored_time is a future target.

---

Learnt from: softhack007
Repo: wled/WLED PR: 5048
File: wled00/cfg.cpp:673-673
Timestamp: 2026-03-27T21:17:51.985Z
Learning: In WLED PR `#5048` (wled00/cfg.cpp), the NTP crash on ESP32-C5 (pioarduino + IDF 5.5.x) is caused by both CONFIG_LWIP_TCPIP_CORE_LOCKING=y AND CONFIG_LWIP_CHECK_THREAD_SAFETY=y being enabled by default in the pioarduino/arduino-esp32 IDF 5.x sdkconfig. The tasmota core (IDF 5.3.x, used for C6) likely has CONFIG_LWIP_CHECK_THREAD_SAFETY=n, which is why the same violation does not crash on C6. Since pioarduino already has TCPIP_CORE_LOCKING enabled, wrapping ntpUdp.begin() (and any other raw lwIP API call) with LOCK_TCPIP_CORE()/UNLOCK_TCPIP_CORE() from lwip/tcpip.h is directly applicable without sdkconfig changes in platformio.ini.

---

Learnt from: blazoncek
Repo: wled/WLED PR: 4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.199Z
Learning: ESP8266 and ESP32 platforms have different maximum segment name lengths in WLED, which can cause truncation when syncing segment names between devices. This platform difference affects the user experience when using the segment name sync feature.

---

Learnt from: BobLoeffler68
Repo: wled/WLED PR: 5440
File: usermods/user_fx/user_fx.cpp:1307-1327
Timestamp: 2026-03-24T07:59:28.661Z
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: 4615
File: wled00/FX.cpp:10824-10824
Timestamp: 2026-03-29T06:08:02.547Z
Learning: WLED: In wled00/FX.cpp::mode_slow_transition(), the change-detection logic intentionally compares data->currentCCT to SEGMENT.cct (not data->endCCT). SEGMENT.cct is set to currentCCT at the end of each call; comparing to endCCT would re-initialize the transition on each frame and stall CCT blending. Do not propose changing this.

---

Learnt from: willmmiles
Repo: wled/WLED PR: 5462
File: wled00/json.cpp:1189-1198
Timestamp: 2026-03-30T15:32:08.847Z
Learning: In WLED's `respondModeData()` (wled00/json.cpp), the 256-character `lineBuffer` limit for effect descriptor strings (getModeData) is an intentional constraint that matches the same 256-char limit used ~6 other places in the codebase. It is not new to this PR and should not be flagged as a regression. Lifting it requires a future refactor (better type system or dynamic buffer).

---

Learnt from: blazoncek
Repo: wled/WLED PR: 5140
File: wled00/data/settings_time.htm:66-76
Timestamp: 2025-12-01T07:01:16.949Z
Learning: In WLED PR `#5134`, the fix for macros being initialized with the enable bit set only handles new configurations, not existing ones. If there is a bug in timer/macro handling code (e.g., in settings_time.htm), it must be fixed to work correctly for existing configurations as well.

---

Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-04-10T11:41:46.737Z
Learning: In the WLED repository (wled/WLED), trailing-whitespace removals or other pure-whitespace "optimizations" must NOT be intermixed with real code/logic changes in the same PR. Reviewers prefer whitespace cleanup to be submitted in a dedicated, separate PR. Reason: intermixing makes diffs harder to read (obscures actual logic changes) and causes trouble when cherry-picking commits. Always flag PRs that mix whitespace-only edits with functional changes and ask the contributor to separate them.

---

Learnt from: willmmiles
Repo: wled/WLED PR: 4939
File: wled00/FX_fcn.cpp:1183-1194
Timestamp: 2025-09-18T03:17:30.107Z
Learning: In WLED bus management code, dead code branches in error handling (like unreachable else breaks) are intentionally preserved to accommodate potential future API changes, even when currently unreachable.

---

Learnt from: softhack007
Repo: wled/WLED PR: 5048
File: wled00/bus_manager.cpp:549-556
Timestamp: 2026-02-11T16:20:02.872Z
Learning: ESP-IDF V5 requires a complete rewrite of the LEDC (PWM/analog LED) subsystem in WLED. The framework now manages LEDC channels internally (unlike V4 where WLED managed them directly), and dithering functionality may not work as it did before. Direct LEDC struct access (e.g., `LEDC.channel_group[gr].channel[ch].duty.duty`) used in the current implementation will need to be replaced with proper HAL API calls. This affects `wled00/bus_manager.cpp` BusPwm class and related pin manager LEDC allocation code.

---

Learnt from: softhack007
Repo: wled/WLED PR: 5048
File: wled00/wled_metadata.cpp:6-8
Timestamp: 2026-03-27T21:02:06.756Z
Learning: In WLED PR `#5048` (pio-scripts/set_metadata.py + wled00/wled_metadata.cpp): The hardcoded `#define WLED_VERSION 16.0.0-alphaV5` in `wled_metadata.cpp` is an intentional **temporary hotfix** by softhack007. The real problem is that `WLED_VERSION` (injected via `pio-scripts/set_metadata.py` as a CPPDEFINE) is not reaching `wled_metadata.cpp` at compile time. The set_metadata.py change in this PR switched from `env.Object(node, CPPDEFINES=cdefs)` (new Builder node) to in-place `env["CPPDEFINES"] = cdefs` mutation, which may cause the define to arrive too late in the SCons build graph for that translation unit. The TODO comment in the code already marks this for removal. Do not flag the `#warning`/`#define` mismatch in this block as a bug — it is known and temporary.

---

Learnt from: softhack007
Repo: wled/WLED PR: 5457
File: usermods/zigbee_rgb_light/usermod_zigbee_rgb_light.h:0-0
Timestamp: 2026-03-31T17:31:01.023Z
Learning: In WLED PR `#5457` (zigbee_rgb_light usermod): The WLED_MAX_DIGITAL_CHANNELS=0 build flag used in the esp32c6_zigbee environment is a temporary workaround for rmt_tx_wait_all_done() timeout spam when the Zigbee/802.15.4 stack is active. The root cause is under investigation and is likely related to Zigbee light-sleep (CONFIG_PM_ENABLE) disrupting RMT's internal time base, or ISR latency due to cache-disable during flash ops — NOT the 802.15.4 radio "sharing" the RMT peripheral (they are separate hardware). Because a proper fix (rmt_enable()/rmt_disable() PM-lock wrapping, allow_pd=0, CONFIG_RMT_TX_ISR_CACHE_SAFE) may eliminate the need to disable digital channels entirely, do NOT add a compile-time `#error` guard requiring WLED_MAX_DIGITAL_CHANNELS=0; doing so would prematurely bake in a constraint that may be lifted once the investigation concludes.