less restrictive DDP header acceptance, add safety checks to all protocols #5547

softhack007 · 2026-04-28T09:56:09Z

@coderabbitai please check if WLED handling of sequence numbers is compatible with DDP protocol specs. Accepting an "outdated" packet may be acceptable, but a wrong rejection should be avoided, even when more than 15 packets are needed to transmit a full "frame". Also check against the real implementation of known DDP senders like LedFx, hyperion, xlights, OpenRGB, FPP.

Let me research the DDP protocol spec and how known senders handle sequence numbers.

@softhack007 Here is a detailed analysis of WLED's DDP sequence number handling against the spec and known senders.

DDP Sequence Number Spec

The DDP spec (byte 1) defines nnnn as a sequence number from 1–15, or zero if not used, and states the sequence number should be incremented with each new packet sent. A sender can send duplicate packets with the same sequence number and DDP header for redundancy. The sequence number is ignored if zero.

Key point: the sequence number is per-packet (not per-frame), and wraps in the ring 1→2→…→15→1.

WLED's Rejection Logic

WLED's current code (after this PR):

int sn = p->sequenceNum & 0xF; // 0 = disabled if (sn) { if (lastPushSeq > 5) { if (sn > (lastPushSeq - 5) && sn < lastPushSeq) return; // reject window: 5 slots before last push } else { if (sn > (10 + lastPushSeq) || sn < lastPushSeq) return; } }

lastPushSeq is the sequence number of the last push packet received. The window rejects any packet whose sn falls in the 5 slots immediately preceding lastPushSeq, on the assumption those are late-arriving packets from the previous frame.

Where This Breaks Down

The 5-slot window hardcodes an assumption of ≤5 packets per frame.

Consider a large installation with, e.g., 5760 LEDs (12 packets of 480 RGB pixels @ 1440 bytes each, a common xLights packet size):

Previous frame push: lastPushSeq = 11

Reject window: sn ∈ {7, 8, 9, 10}

New frame packets arrive in sequence: 12, 13, 14, 15, 1, 2, 3, 4, 5, 6, 7, 8 → push at sn = 9

Packets with sn = 7 and sn = 8 from the new frame fall inside the reject window and are wrongly dropped, causing visual corruption/flicker on those LED ranges mid-frame.

This means for any installation needing roughly >10 packets per frame (with sequence numbers enabled), current-frame packets will be wrongly rejected. OpenRGB's DDP implementation uses sequence numbers from 1–15 per spec, and xLights defaults to 1440 bytes per DDP packet, which is 480 RGB pixels — meaning only ~7–8 packets are needed before wrapping issues arise for mid-size installs.

Known Sender Behaviors

Sender Seq Numbers Risk

xLights Increments per packet, 1–15 🔴 Affected for >~10 packets/frame

OpenRGB 4-bit nibble per spec 🔴 Same

FPP Standard DDP, per-packet 🔴 Same

LedFx / Hyperion Typically use seq=0 (disabled) or low packet counts 🟡 Likely safe

Senders that set sn = 0 (disabled) bypass this check entirely and are unaffected.

Summary of Issues

Wrong rejections are possible: For frames requiring >~10 packets with sequence numbers enabled, valid new-frame packets whose sn falls in the 5-slot window behind lastPushSeq (after wrap) are silently dropped. The PR comment "if more are used and packets are very late, they are still accepted" is not fully accurate for this wrap-around case.

The 15-value ring is too small: With only 15 sequence numbers and a 5-slot window, there's only room for about 10 "safe" slots per frame. Any frame needing more packets will cycle through the reject window.

Safe path exists: If the sender sets sn = 0 (sequence checking disabled), WLED's code correctly skips all rejection.

Recommendations

Best fix: Increase the "assumed max lag" from 5 to a smaller window (e.g., 2–3) to reduce false rejections, or better yet, track the sequence number of the last accepted packet (not just the push packet) and reject only obvious backwards steps.

Safe guard: Consider documenting that this feature is unreliable for frames requiring >10 packets and advising senders (or WLED's own DDP broadcast) to use sn = 0 for large installations.

Least risk: If in doubt, the entire out-of-sequence rejection for DDP could be disabled (set sn = 0 in WLED's own output), since DDP spec explicitly allows ignoring it. The spec does not mandate receivers implement rejection.

🧠 Learnings used

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: 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: softhack007 Repo: wled/WLED PR: 5547 File: wled00/e131.cpp:33-40 Timestamp: 2026-04-28T09:41:31.415Z Learning: In WLED DDP packet handling (wled00/e131.cpp), the destination filtering uses an explicit deny-list (reject DDP_ID_CONTROL, DDP_ID_STATUS, DDP_ID_CONFIG) rather than an allow-list (only DDP_ID_DISPLAY). This is intentional for maximum compatibility — devices may use non-standard or future destination IDs that should still drive LED output. Do not suggest switching to an allow-list (`p->destination != DDP_ID_DISPLAY`) in future reviews of this code.

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-13T09:36:13.994Z Learning: WLED serial streaming frame maxima: - Adalight: protocol max 65,536 LEDs (count = hi<<8 | lo, plus 1), but WLED’s uint16_t parser caps it at 65,535. - TPM2 serial (RGB): protocol max payload 65,535 bytes → 21,845 LEDs (bytes/3); effective output always limited by configured LED count and MAX_LEDS.

Learnt from: mval-sg Repo: wled/WLED PR: 4876 File: wled00/xml.cpp:0-0 Timestamp: 2025-08-28T08:09:20.630Z Learning: The WLED codebase has opportunities for refactoring hardcoded array bounds (like the "15" used for DMX channels) to use sizeof(array)/sizeof(array[0]) for more maintainable code, but such changes should be done consistently across the entire codebase in a dedicated refactoring effort.

Learnt from: mval-sg Repo: wled/WLED PR: 4876 File: wled00/wled_eeprom.cpp:0-0 Timestamp: 2025-09-01T10:26:17.959Z Learning: In WLED PR `#4876`, the DMXStartLED EEPROM backward compatibility issue was partially addressed by keeping it at address 2550 and reading it as a 16-bit value, with DMXChannelsValue array moved to addresses 2552-2566. This maintains compatibility with pre-0.11 EEPROM layouts for DMXStartLED, though legacy "Set to 255" (code 6) configurations may still need migration logic.

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

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: 2025-12-28T14:06:48.772Z Learning: In WLED post-commit ee9ac947, the rendering pipeline uses per-segment buffers and per-pixel bus updates. Unmapped (0xFFFF) mapping entries are now skipped in WS2812FX::show() (no “clear to black”), which can leave physical gap LEDs with stale/random colors unless they are explicitly cleared. This is a behavior change from pre-0.16 where a full physical buffer was effectively refreshed each frame.

Learnt from: softhack007 Repo: wled/WLED PR: 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: 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: softhack007 Repo: wled/WLED PR: 5436 File: wled00/fcn_declare.h:394-395 Timestamp: 2026-03-22T00:09:16.015Z Learning: In WLED (wled00/fcn_declare.h), the `prepareHostname(char* hostname, size_t maxLen)` signature change from the old `prepareHostname(char* hostname)` is intentional and breaking. There is no compatibility shim; any usermod or downstream caller must explicitly pass the buffer size. Do not suggest a default-size shim for this function.

Learnt from: BobLoeffler68 Repo: wled/WLED PR: 4891 File: wled00/FX.cpp:3333-3349 Timestamp: 2025-08-31T03:38:14.114Z Learning: WLED PacMan effect (wled00/FX.cpp): Keep pacmancharacters_t position fields as signed int (not int16_t). Maintainer preference (blazoncek) prioritizes avoiding potential overhead/regressions over minor RAM savings. Avoid type shrinking here unless memory pressure is demonstrated.

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: 5482 File: wled00/wled.h:579-579 Timestamp: 2026-04-08T21:25:44.777Z Learning: In WLED PR `#5482` (wled00/wled.h), the `denyWsecUpload` flag is confirmed to default to `false`. The complementary defence against malformed wsec.json content (bootloop/brick risk from malformed JSON) is handled by PR `#5484`, which hardens JSON file reading/parsing at load time. Do not suggest moving `denyWsecUpload` default to `true` or replacing JSON validation with upload blocking — both mechanisms serve different purposes and should coexist.

Learnt from: softhack007 Repo: wled/WLED PR: 5456 File: platformio.ini:794-830 Timestamp: 2026-03-31T13:42:00.444Z Learning: In WLED PR `#5456` (Matter over WiFi usermod, CMakeLists.txt + platformio.ini): The GCC 14 chip::to_underlying compatibility issue with CHIP SDK's TypeTraits.h is fixed by building in gnu++20 mode (not gnu++2b/gnu++23). CMakeLists.txt uses `idf_build_replace_option_from_property` to swap `-std=gnu++2b` for `-std=gnu++20` when the matter usermod is present. The `matter_gcc14_compat.h` shim file (which pre-defines `chip::to_underlying` and sets `CHIP_TO_UNDERLYING_DEFINED`) is dead code under this configuration — it is never included anywhere. TypeTraits.h's broken C++23 `using std::to_underlying` alias path is only taken in gnu++23 mode; in gnu++20 mode CHIP defines its own `chip::to_underlying` function template normally, so no shim is needed. Additionally, upstream connectedhomeip TypeTraits.h already has the fix built-in. ESP-IDF v5.5 uses GCC 14.2.0. Do NOT flag the missing `-include` for `matter_gcc14_compat.h` as a build issue.

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: 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: 5355 File: wled00/util.cpp:635-638 Timestamp: 2026-02-07T16:06:08.677Z Learning: PSRAM-related compilation guards should enable PSRAM code only for ESP32 variants that actually include PSRAM: ESP32-C61, ESP32-C5, and ESP32-P4. Exclude ESP32-C3, ESP32-C6, and ESP8266 from these guards. Apply this rule across the codebase (not just wled00/util.cpp) by reviewing and updating PSRAM guards/macros in all relevant files (C/C++ headers and sources).

Learnt from: softhack007 Repo: wled/WLED PR: 4893 File: wled00/set.cpp:95-95 Timestamp: 2026-03-14T20:56:46.543Z Learning: Guideline: Ensure WiFi hostname is set after WiFi.mode() but before WiFi.begin() to avoid default esp-XXXXXX hostname being used in DHCP. This ordering only takes effect after the STA interface exists (so avoid placing hostname setting before initConnection). In WLED, place the hostname configuration inside initConnection() (after WiFi.disconnect(true) and before WiFi.begin()) rather than in earlier boot code like deserializeConfig(). This rule should be applied in code reviews for WLED’s network initialization paths in wled00/*.cpp, and note that on ESP8266 the ordering is less strict but still desirable for consistency.

Learnt from: softhack007 Repo: wled/WLED PR: 4838 File: lib/NeoESP32RmtHI/src/NeoEsp32RmtHIMethod.cpp:30-35 Timestamp: 2026-03-27T12:33:48.499Z Learning: In C/C++ preprocessor conditionals (`#if`, `#elif`) GCC/Clang treat `&&` as short-circuit evaluated during preprocessing. This means guards like `#if defined(ARDUINO_ARCH_ESP32) && ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0)` are safe even if the macro/function-like macro on the RHS (e.g., `ESP_IDF_VERSION_VAL`) is not defined on some targets, because the RHS will not be expanded when the LHS is false (e.g., `defined(...)` evaluates to 0). During code review, avoid flagging such cases as “undefined function-like macro invocation” if they are protected by short-circuiting `defined(...) && ...`/`||` logic; some tools like cppcheck may not model this and can produce false positives. Also, don’t suggest refactoring that moves ESP32-specific includes/headers (e.g., `esp_idf_version.h`) outside of these guarded preprocessor blocks, since that will break targets (e.g., ESP8266) where the headers don’t exist.

Learnt from: softhack007 Repo: wled/WLED PR: 5048 File: wled00/cfg.cpp:673-673 Timestamp: 2026-03-27T21:17:45.980Z Learning: When calling raw lwIP APIs (e.g., around `ntpUdp.begin()` or any `lwIP`/`tcpip`-layer call) in this codebase on ESP32 Arduino-ESP32 platforms where core locking/checking is enabled, wrap the lwIP call with `LOCK_TCPIP_CORE()` / `UNLOCK_TCPIP_CORE()` from `lwip/tcpip.h`. This prevents thread-safety/core-violation crashes without requiring `sdkconfig` changes.

Learnt from: softhack007 Repo: wled/WLED PR: 5048 File: wled00/cfg.cpp:673-673 Timestamp: 2026-03-27T21:17:45.980Z Learning: When using lwIP “raw” APIs in WLED on ESP32 (Arduino-ESP32 / IDF 5.5+), don’t rely on LOCK_TCPIP_CORE()/UNLOCK_TCPIP_CORE() unless CONFIG_LWIP_TCPIP_CORE_LOCKING=y is guaranteed. CONFIG_LWIP_CHECK_THREAD_SAFETY=y can trigger the assertion “Required to lock TCPIP core functionality!” when raw lwIP calls occur off the TCPIP thread. If the locking mode isn’t enabled (or can’t be changed via sdkconfig), schedule lwIP work (e.g., ntpUdp.begin() and similar raw lwIP calls) via tcpip_callback() so it runs on the TCPIP thread; this should work regardless of the locking-mode setting. Review any similar raw lwIP usage for correct thread/context handling.

Learnt from: softhack007 Repo: wled/WLED PR: 5048 File: wled00/wled.cpp:698-700 Timestamp: 2026-03-28T01:37:15.541Z Learning: In this WLED codebase, when using `DEBUG_PRINTLN(F("..."))`, an explicit trailing `\n` inside the `F("...")` string (e.g., `DEBUG_PRINTLN(F("Warning!\n"))`) may be intentional to create a blank line in debug output as a visual separator. During code review, do not automatically flag these as “double newline” or recommend removing the `\n`—first verify with the author/context that the extra newline is deliberate.

Learnt from: willmmiles Repo: wled/WLED PR: 5462 File: wled00/json.cpp:1189-1198 Timestamp: 2026-03-30T15:32:02.808Z Learning: When working with WLED strings stored in PROGMEM on ESP8266/ESP32, assume you can’t use `strchr_P` for character scanning. If you need to scan a PROGMEM string for a character (e.g., searching for '@' within effect data), review changes to ensure they perform an explicit RAM copy first (e.g., via `strncpy_P` into a buffer) and then scan in RAM. Avoid proposing incremental/streaming PROGMEM character-by-character scans without a RAM buffer on these targets, since it’s impractical with the available APIs.

Learnt from: DedeHai Repo: wled/WLED PR: 5464 File: wled00/FX_fcn.cpp:1699-1701 Timestamp: 2026-04-09T07:26:09.816Z Learning: In WLED’s WLED code, if a pixel/buffer allocation uses `BFRALLOC_NOBYTEACCESS` (and especially on classic ESP32 where byte-level access to IRAM-resident buffers is unsafe), avoid using byte-wise operations like `memset`/`memcpy` on that buffer. Specifically, do not combine `BFRALLOC_CLEAR | BFRALLOC_NOBYTEACCESS` and do not perform `memcpy`/`memset` over `Segment::pixels` (e.g., in `setGeometry()`/`finalizeInit()` or copy ctor/assignment). Instead, use element-wise 32-bit access (loop over `uint32_t*` and assign/copy per element) to ensure 32-bit access only.

Learnt from: DedeHai Repo: wled/WLED PR: 5464 File: wled00/FX_fcn.cpp:1699-1701 Timestamp: 2026-04-09T09:14:29.218Z Learning: In WLED, when reviewing ESP32 code that uses pixel buffers allocated with `BFRALLOC_NOBYTEACCESS` (e.g., `WS2812FX::_pixels` and `Segment::pixels`), treat `memset`/`memcpy` on those buffers as an intentionally low-risk, accepted pattern. These buffers are designed to be IRAM-safe for 32-bit-only access, and the Xtensa newlib assembly implementations perform 32-bit word stores for word-aligned, word-multiple-sized buffers (so the IRAM constraint is satisfied in practice). Do not flag `memset`/`memcpy` on `BFRALLOC_NOBYTEACCESS` pixel buffers as a critical bug; if any concern arises, downgrade severity to low and focus on potential future regressions rather than an immediate correctness violation.

Learnt from: softhack007 Repo: wled/WLED PR: 5508 File: wled00/FX.cpp:4137-4142 Timestamp: 2026-04-16T09:49:58.587Z Learning: In WLED effect/overlay implementations (C++ under wled00), ensure effects operate through the SEGMENT.* read/write APIs for their own pixels/overlay state. Reserve global frame reads/writes like strip.getPixelColor* for copy_* style post-processing only. Do not manually composite/blend across lower layers inside the effect; segment-to-segment blending is handled later by WS2812FX::blendSegment(), so effects should produce their per-segment contributions without trying to composite with underlying layers themselves.

@softhack007 I looked at this code in detail, it is not perfect but rejecting "late packets" seems reasonable. if large setups are needed, just disable this option in the sync settings (it is disabled by default), it is not possible to have both, late packet rejection AND a large setup as the sequence number for DDP is limited to 1-15 and to check for late packets, a max window of 7 would be possible.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

@DedeHai @coderabbitai I was actually wondering if we could make improvements for #5412 while working on the code.

The DDP spec reads like they wanted to support packet re-transmit within one "push frame", so the sequence counter could be used to identify duplicates (in a history window of 15 packets max). I think dups (same frame) are not a problem in WLED, because they simply cause a re-painting of some pixels. You're right that rejecting late packets - from the previous "push frame" - might be useful in some setups - especially with high latency / unstable WiFi and with lower pixel counts. My fear is that users will simply enable "Skip out-of-sequence packets" (which makes sense for other protocols like Art-Net), and later complain that a bigger setup always misses pixel updates when sequence checks are enabled.

I'm not sure atm where is the "breaking limit" (matrix size) when sequence checking will start to mis-behave. But as this is a legacy "buggy feature" I could live with a use-as-is decision 🤔, so we still support the previous "proper use" cases.

What do you think?

🧩 Analysis chain

@softhack007, here's my analysis of the sequence number situation and whether improvements are practical for this PR.

Exact Breaking-Point Calculation

The rejection window always excludes exactly 4 sequence numbers as "late from previous frame":

When lastPushSeq > 5: rejects sn ∈ (lastPushSeq−5, lastPushSeq) — e.g. with push at sn=10, rejects {6,7,8,9}

When lastPushSeq ≤ 5: rejects via sn > (10+lastPushSeq) || sn < lastPushSeq — e.g. with push at sn=3, rejects {14,15,1,2}

So: 15 total − 1 (zero/disabled) − 4 (rejected window) = 10 safe data-packet slots + 1 for the push packet itself = 11 packets max per frame without false-rejection.

At xLights' default 1440 bytes/packet:

RGB (3 ch): 480 LEDs/packet × 11 = ~5,280 LEDs max

RGBW (4 ch): 360 LEDs/packet × 11 = ~3,960 LEDs max

For typical WLED ESP32 setups that max out around 1,000–2,500 LEDs, this is usually fine. The problematic zone is large matrix setups (2,000–8,000+ LEDs with RGBW), which is exactly the target use case for #5412.

Can We Improve It?

Several options with different trade-offs:

Option Window size Safe slots RGB LED limit Con

Current code 4 10+push ~5,280 Fails for large RGBW or >5K RGB installs

Reduce window to 2 2 12+push ~6,240 Less protection against genuinely late packets

Track last-accepted sn (not push sn) 4 10+push ~5,280 Better accuracy but same fundamental limit

Disable DDP seq checking entirely — 15 unlimited No protection, consistent with sn=0 path

The fundamental constraint is the DDP spec's 4-bit sequence number (1-15 ring). No algorithm can safely distinguish "late from previous frame" from "early in new frame" with only 15 values and an unbounded number of packets per frame.

Recommendation

Given that:

The feature is disabled by default (e131SkipOutOfSequence), as @DedeHai noted

Duplicates within a frame are indeed harmless (just repaint)

The true late-packet problem (high-latency WiFi) tends to occur on small setups, not large ones that need >10 packets/frame

The breaking limit (~5,280 RGB / ~3,960 RGBW) exceeds most WLED deployments

I'd suggest accepting use-as-is for this PR but adding a comment near the rejection logic that documents the limit, so future contributors understand why the feature can't safely be extended to large setups:

// Note: with a 15-value (1–15) DDP sequence ring and a 4-slot rejection window, // frames requiring more than ~10 packets may experience false rejections. // "Skip out-of-sequence packets" (e131SkipOutOfSequence) is disabled by default; // for large installations it should remain disabled. if (e131SkipOutOfSequence && lastPushSeq) {

This way the behavior is documented, and users/maintainers know the safe operating envelope without any code risk from changing the window algorithm. The #5412 improvement (if it relates to large-matrix sequence behavior) could then be tracked as a separate, more surgical change.

🏁 Script executed:

#!/bin/bash gh issue view 5412 --repo wled/WLED 2>/dev/null | head -60

Length of output: 0

🌐 Web query:

WLED github issue 5412 DDP sequence

💡 Result:

GitHub issue wled/WLED #5412 (opened Mar 9, 2026) reports a potential bug in WLED’s DDP packet sequence / out‑of‑order handling when the setting “skip out-of sequence packets” is enabled. [1]

In wled00/e131.cpp, WLED uses only the low 4 bits of the DDP sequenceNum (sequenceNum & 0xF). [1]

The code comment says it’s “assuming 4 packets max. before push”, and the logic effectively uses a small window (it compares against lastPushSeq and a 5-sequence range) to reject “late packets belonging to previous frame”. [1]

The issue concern: if a single “frame” is split across more than ~4–5 DDP packets (e.g., large installations like HUB75 64×64 streaming), then valid packets that arrive slightly late/out-of-order could be discarded, potentially causing corrupted / incomplete LED output. The issue asks to verify whether that “4 packets max before push” assumption is actually valid per DDP expectations. [1]

DDP itself is the UDP protocol WLED listens for on port 4048. [2]

Sources: [1] [2]

@softhack007, confirmed — issue #5412 reports exactly this problem: large installations like HUB75 64×64 streaming split a single frame across more than ~4–5 DDP packets, and the code's "4 packets max before push" assumption causes valid packets to be discarded. My earlier analysis lines up with the issue report.

To summarize the math concisely:

The rejection window always excludes exactly 4 sequence numbers (the code checks sn > (lastPushSeq−5) && sn < lastPushSeq, which spans 4 values, not 5).

That leaves 10 safe data-packet slots + 1 push packet = 11 total per frame before false rejections begin.

At 480 RGB LEDs per 1440-byte DDP packet: ~5,280 RGB / ~3,960 RGBW LEDs is the breaking point.

A 64×64 HUB75 matrix = 4,096 pixels — right at the RGBW edge.

For this PR, I'd suggest the pragmatic "use-as-is + document the limit" approach, since:

The feature is off by default (e131SkipOutOfSequence)

Late-packet problems (high-latency WiFi) typically affect small setups, not large ones that need >11 packets/frame

No algorithm can do better given DDP's 4-bit (1–15) sequence ring without cross-frame state

A targeted comment referencing #5412 would help future contributors understand the constraint:

//reject late packets belonging to previous frame (assuming 4 packets max. before push, if more are used and packets are very late, they are still accepted) // Note: the 4-bit DDP sequence ring (1-15) limits safe use of this feature to frames requiring // ≤10 DDP packets (~5280 RGB / ~3960 RGBW LEDs at 1440 bytes/packet). See issue `#5412`. // For larger installations, keep "Skip out-of-sequence packets" (e131SkipOutOfSequence) disabled. if (e131SkipOutOfSequence && lastPushSeq) {

This keeps the PR focused on the safety fixes it targets, and creates a clear breadcrumb for whoever addresses #5412 properly in a future dedicated PR.

🧠 Learnings used

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: 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: softhack007 Repo: wled/WLED PR: 5547 File: wled00/e131.cpp:33-40 Timestamp: 2026-04-28T09:41:31.415Z Learning: In WLED DDP packet handling (wled00/e131.cpp), the destination filtering uses an explicit deny-list (reject DDP_ID_CONTROL, DDP_ID_STATUS, DDP_ID_CONFIG) rather than an allow-list (only DDP_ID_DISPLAY). This is intentional for maximum compatibility — devices may use non-standard or future destination IDs that should still drive LED output. Do not suggest switching to an allow-list (`p->destination != DDP_ID_DISPLAY`) in future reviews of this code.

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

Learnt from: softhack007 Repo: wled/WLED PR: 5482 File: wled00/wled.h:579-579 Timestamp: 2026-04-08T21:25:44.777Z Learning: In WLED PR `#5482` (wled00/wled.h), the `denyWsecUpload` flag is confirmed to default to `false`. The complementary defence against malformed wsec.json content (bootloop/brick risk from malformed JSON) is handled by PR `#5484`, which hardens JSON file reading/parsing at load time. Do not suggest moving `denyWsecUpload` default to `true` or replacing JSON validation with upload blocking — both mechanisms serve different purposes and should coexist.

Learnt from: DedeHai Repo: wled/WLED PR: 5464 File: wled00/FX_fcn.cpp:1699-1701 Timestamp: 2026-04-09T07:26:09.816Z Learning: In WLED’s WLED code, if a pixel/buffer allocation uses `BFRALLOC_NOBYTEACCESS` (and especially on classic ESP32 where byte-level access to IRAM-resident buffers is unsafe), avoid using byte-wise operations like `memset`/`memcpy` on that buffer. Specifically, do not combine `BFRALLOC_CLEAR | BFRALLOC_NOBYTEACCESS` and do not perform `memcpy`/`memset` over `Segment::pixels` (e.g., in `setGeometry()`/`finalizeInit()` or copy ctor/assignment). Instead, use element-wise 32-bit access (loop over `uint32_t*` and assign/copy per element) to ensure 32-bit access only.

Learnt from: DedeHai Repo: wled/WLED PR: 4798 File: wled00/FX.cpp:7531-7533 Timestamp: 2025-08-26T11:51:21.817Z Learning: In WLED PR `#4798`, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.

Learnt from: softhack007 Repo: wled/WLED PR: 0 File: :0-0 Timestamp: 2025-12-28T14:06:48.772Z Learning: In WLED post-commit ee9ac947, the rendering pipeline uses per-segment buffers and per-pixel bus updates. Unmapped (0xFFFF) mapping entries are now skipped in WS2812FX::show() (no “clear to black”), which can leave physical gap LEDs with stale/random colors unless they are explicitly cleared. This is a behavior change from pre-0.16 where a full physical buffer was effectively refreshed each frame.

Learnt from: softhack007 Repo: wled/WLED PR: 0 File: :0-0 Timestamp: 2026-04-13T09:36:13.994Z Learning: WLED serial streaming frame maxima: - Adalight: protocol max 65,536 LEDs (count = hi<<8 | lo, plus 1), but WLED’s uint16_t parser caps it at 65,535. - TPM2 serial (RGB): protocol max payload 65,535 bytes → 21,845 LEDs (bytes/3); effective output always limited by configured LED count and MAX_LEDS.

Learnt from: DedeHai Repo: wled/WLED PR: 5464 File: wled00/FX_fcn.cpp:1699-1701 Timestamp: 2026-04-09T09:14:29.218Z Learning: In WLED, when reviewing ESP32 code that uses pixel buffers allocated with `BFRALLOC_NOBYTEACCESS` (e.g., `WS2812FX::_pixels` and `Segment::pixels`), treat `memset`/`memcpy` on those buffers as an intentionally low-risk, accepted pattern. These buffers are designed to be IRAM-safe for 32-bit-only access, and the Xtensa newlib assembly implementations perform 32-bit word stores for word-aligned, word-multiple-sized buffers (so the IRAM constraint is satisfied in practice). Do not flag `memset`/`memcpy` on `BFRALLOC_NOBYTEACCESS` pixel buffers as a critical bug; if any concern arises, downgrade severity to low and focus on potential future regressions rather than an immediate correctness violation.

Learnt from: mval-sg Repo: wled/WLED PR: 4876 File: wled00/xml.cpp:0-0 Timestamp: 2025-08-28T08:09:20.630Z Learning: The WLED codebase has opportunities for refactoring hardcoded array bounds (like the "15" used for DMX channels) to use sizeof(array)/sizeof(array[0]) for more maintainable code, but such changes should be done consistently across the entire codebase in a dedicated refactoring effort.

Learnt from: softhack007 Repo: wled/WLED PR: 5436 File: wled00/fcn_declare.h:394-395 Timestamp: 2026-03-22T00:09:16.015Z Learning: In WLED (wled00/fcn_declare.h), the `prepareHostname(char* hostname, size_t maxLen)` signature change from the old `prepareHostname(char* hostname)` is intentional and breaking. There is no compatibility shim; any usermod or downstream caller must explicitly pass the buffer size. Do not suggest a default-size shim for this function.

Learnt from: BobLoeffler68 Repo: wled/WLED PR: 4891 File: wled00/FX.cpp:3333-3349 Timestamp: 2025-08-31T03:38:14.114Z Learning: WLED PacMan effect (wled00/FX.cpp): Keep pacmancharacters_t position fields as signed int (not int16_t). Maintainer preference (blazoncek) prioritizes avoiding potential overhead/regressions over minor RAM savings. Avoid type shrinking here unless memory pressure is demonstrated.

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: mval-sg Repo: wled/WLED PR: 4876 File: wled00/wled_eeprom.cpp:0-0 Timestamp: 2025-09-01T10:26:17.959Z Learning: In WLED PR `#4876`, the DMXStartLED EEPROM backward compatibility issue was partially addressed by keeping it at address 2550 and reading it as a 16-bit value, with DMXChannelsValue array moved to addresses 2552-2566. This maintains compatibility with pre-0.11 EEPROM layouts for DMXStartLED, though legacy "Set to 255" (code 6) configurations may still need migration logic.

Learnt from: softhack007 Repo: wled/WLED PR: 5456 File: platformio.ini:794-830 Timestamp: 2026-03-31T13:42:00.444Z Learning: In WLED PR `#5456` (Matter over WiFi usermod, CMakeLists.txt + platformio.ini): The GCC 14 chip::to_underlying compatibility issue with CHIP SDK's TypeTraits.h is fixed by building in gnu++20 mode (not gnu++2b/gnu++23). CMakeLists.txt uses `idf_build_replace_option_from_property` to swap `-std=gnu++2b` for `-std=gnu++20` when the matter usermod is present. The `matter_gcc14_compat.h` shim file (which pre-defines `chip::to_underlying` and sets `CHIP_TO_UNDERLYING_DEFINED`) is dead code under this configuration — it is never included anywhere. TypeTraits.h's broken C++23 `using std::to_underlying` alias path is only taken in gnu++23 mode; in gnu++20 mode CHIP defines its own `chip::to_underlying` function template normally, so no shim is needed. Additionally, upstream connectedhomeip TypeTraits.h already has the fix built-in. ESP-IDF v5.5 uses GCC 14.2.0. Do NOT flag the missing `-include` for `matter_gcc14_compat.h` as a build issue.

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: 5355 File: wled00/util.cpp:635-638 Timestamp: 2026-02-07T16:06:08.677Z Learning: PSRAM-related compilation guards should enable PSRAM code only for ESP32 variants that actually include PSRAM: ESP32-C61, ESP32-C5, and ESP32-P4. Exclude ESP32-C3, ESP32-C6, and ESP8266 from these guards. Apply this rule across the codebase (not just wled00/util.cpp) by reviewing and updating PSRAM guards/macros in all relevant files (C/C++ headers and sources).

Learnt from: softhack007 Repo: wled/WLED PR: 4893 File: wled00/set.cpp:95-95 Timestamp: 2026-03-14T20:56:46.543Z Learning: Guideline: Ensure WiFi hostname is set after WiFi.mode() but before WiFi.begin() to avoid default esp-XXXXXX hostname being used in DHCP. This ordering only takes effect after the STA interface exists (so avoid placing hostname setting before initConnection). In WLED, place the hostname configuration inside initConnection() (after WiFi.disconnect(true) and before WiFi.begin()) rather than in earlier boot code like deserializeConfig(). This rule should be applied in code reviews for WLED’s network initialization paths in wled00/*.cpp, and note that on ESP8266 the ordering is less strict but still desirable for consistency.

Learnt from: softhack007 Repo: wled/WLED PR: 4838 File: lib/NeoESP32RmtHI/src/NeoEsp32RmtHIMethod.cpp:30-35 Timestamp: 2026-03-27T12:33:48.499Z Learning: In C/C++ preprocessor conditionals (`#if`, `#elif`) GCC/Clang treat `&&` as short-circuit evaluated during preprocessing. This means guards like `#if defined(ARDUINO_ARCH_ESP32) && ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0)` are safe even if the macro/function-like macro on the RHS (e.g., `ESP_IDF_VERSION_VAL`) is not defined on some targets, because the RHS will not be expanded when the LHS is false (e.g., `defined(...)` evaluates to 0). During code review, avoid flagging such cases as “undefined function-like macro invocation” if they are protected by short-circuiting `defined(...) && ...`/`||` logic; some tools like cppcheck may not model this and can produce false positives. Also, don’t suggest refactoring that moves ESP32-specific includes/headers (e.g., `esp_idf_version.h`) outside of these guarded preprocessor blocks, since that will break targets (e.g., ESP8266) where the headers don’t exist.

Learnt from: softhack007 Repo: wled/WLED PR: 5048 File: wled00/cfg.cpp:673-673 Timestamp: 2026-03-27T21:17:45.980Z Learning: When calling raw lwIP APIs (e.g., around `ntpUdp.begin()` or any `lwIP`/`tcpip`-layer call) in this codebase on ESP32 Arduino-ESP32 platforms where core locking/checking is enabled, wrap the lwIP call with `LOCK_TCPIP_CORE()` / `UNLOCK_TCPIP_CORE()` from `lwip/tcpip.h`. This prevents thread-safety/core-violation crashes without requiring `sdkconfig` changes.

Learnt from: softhack007 Repo: wled/WLED PR: 5048 File: wled00/cfg.cpp:673-673 Timestamp: 2026-03-27T21:17:45.980Z Learning: When using lwIP “raw” APIs in WLED on ESP32 (Arduino-ESP32 / IDF 5.5+), don’t rely on LOCK_TCPIP_CORE()/UNLOCK_TCPIP_CORE() unless CONFIG_LWIP_TCPIP_CORE_LOCKING=y is guaranteed. CONFIG_LWIP_CHECK_THREAD_SAFETY=y can trigger the assertion “Required to lock TCPIP core functionality!” when raw lwIP calls occur off the TCPIP thread. If the locking mode isn’t enabled (or can’t be changed via sdkconfig), schedule lwIP work (e.g., ntpUdp.begin() and similar raw lwIP calls) via tcpip_callback() so it runs on the TCPIP thread; this should work regardless of the locking-mode setting. Review any similar raw lwIP usage for correct thread/context handling.

Learnt from: softhack007 Repo: wled/WLED PR: 5048 File: wled00/wled.cpp:698-700 Timestamp: 2026-03-28T01:37:15.541Z Learning: In this WLED codebase, when using `DEBUG_PRINTLN(F("..."))`, an explicit trailing `\n` inside the `F("...")` string (e.g., `DEBUG_PRINTLN(F("Warning!\n"))`) may be intentional to create a blank line in debug output as a visual separator. During code review, do not automatically flag these as “double newline” or recommend removing the `\n`—first verify with the author/context that the extra newline is deliberate.

Learnt from: willmmiles Repo: wled/WLED PR: 5462 File: wled00/json.cpp:1189-1198 Timestamp: 2026-03-30T15:32:02.808Z Learning: When working with WLED strings stored in PROGMEM on ESP8266/ESP32, assume you can’t use `strchr_P` for character scanning. If you need to scan a PROGMEM string for a character (e.g., searching for '@' within effect data), review changes to ensure they perform an explicit RAM copy first (e.g., via `strncpy_P` into a buffer) and then scan in RAM. Avoid proposing incremental/streaming PROGMEM character-by-character scans without a RAM buffer on these targets, since it’s impractical with the available APIs.

Learnt from: softhack007 Repo: wled/WLED PR: 5508 File: wled00/FX.cpp:4137-4142 Timestamp: 2026-04-16T09:49:58.587Z Learning: In WLED effect/overlay implementations (C++ under wled00), ensure effects operate through the SEGMENT.* read/write APIs for their own pixels/overlay state. Reserve global frame reads/writes like strip.getPixelColor* for copy_* style post-processing only. Do not manually composite/blend across lower layers inside the effect; segment-to-segment blending is handled later by WS2812FX::blendSegment(), so effects should produce their per-segment contributions without trying to composite with underlying layers themselves.

Ok, so with RGB (not RGBW which is totally uncommon in large displays, and not supported for HUB75), false rejections start above ~ 82x64 pixels.

It's not perfect and the fix is to uncheck that option, its more a question of documentation. Until looking into this code for this PR I did not even know that option existed :)
We can always refine at a later point if this becomes an issue with growing number of HUB75 users. The key point of this PR is to make DDP as compatible as we dare to at the cost of accepting wrong packets that lead to a screwed up display (which will be rare).

What I am most concerned about in this PR is rejecting valid packets in non DDP protocols, which may be the case if the rabbit is correct in its statement above on artnet packets.

so if we can not verify that to be working in a real setup, I'd rather comment out those packet size checks for now as this PR will block 16.0 release until done.

Yes, good point. As testing is very hard to do, especially we don't know much about all the possible senders - maybe this PR should strictly focus on "prevent array bounds violations" and on DDP. Which means, any clamping that is not driven by array/buffer bounds should be commented out.

-Original file line number
+Diff line change
@@ Expand Up / @@ -200,7 +200,7 @@ function sendDDP(ws, start, len, colors) { @@
     		let pkt = new Uint8Array(11 + dLen); // DDP header is 10 bytes, plus 1 byte for WLED websocket protocol indicator
     		pkt[0] = 0x02; // DDP protocol indicator for WLED websocket. Note: below DDP protocol bytes are offset by 1
     		pkt[1] = 0x40; // flags: 0x40 = no push, 0x41 = push (i.e. render), note: this is DDP protocol byte 0
-    		pkt[2] = 0x00; // reserved
+    		pkt[2] = 0x00; // upper nibble is reserved, lower nibble is sequence number, if set to 0 no sequence checking is done (if enabled)
     		pkt[3] = 0x0B; // RGB, 8bit per channel
     		pkt[4] = 0x01; // destination id (not used but 0x01 is default output)
     		pkt[5] = (off >> 24) & 255; // DDP protocol 4-7 is offset
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -100,7 +100,7 @@ void initDMXInput(); @@
     void handleDMXInput();
     //e131.cpp
-    void handleE131Packet(e131_packet_t* p, IPAddress clientIP, byte protocol);
+    void handleE131Packet(e131_packet_t* p, IPAddress clientIP, byte protocol, size_t packetLen);
     void handleDMXData(uint16_t uni, uint16_t dmxChannels, uint8_t* e131_data, uint8_t mde, uint8_t previousUniverses);
     // void handleArtnetPollReply(IPAddress ipAddress);                                          // local function, only used in e131.cpp
     // void prepareArtnetPollReply(ArtPollReply* reply);                                         // local function, only used in e131.cpp
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up @@
     void ESPAsyncE131::parsePacket(AsyncUDPPacket _packet) {
       bool error = false;
       uint8_t protocol = P_E131;
+      const size_t pktLen = _packet.length();
       e131_packet_t *sbuff = reinterpret_cast<e131_packet_t *>(_packet.data());
-    	//E1.31 packet identifier ("ACS-E1.17")
-      if (memcmp(sbuff->acn_id, ESPAsyncE131::ACN_ID, sizeof(sbuff->acn_id)))
-        protocol = P_ARTNET;
-    	if (protocol == P_ARTNET) {
-    		if (memcmp(sbuff->art_id, ESPAsyncE131::ART_ID, sizeof(sbuff->art_id)))
-    			error = true; //not "Art-Net"
-    		if (sbuff->art_opcode != ARTNET_OPCODE_OPDMX && sbuff->art_opcode != ARTNET_OPCODE_OPPOLL)
-    			error = true; //not a DMX or poll packet
-    	} else { //E1.31 error handling
-    		if (htonl(sbuff->root_vector) != ESPAsyncE131::VECTOR_ROOT)
-    			error = true;
-    		if (htonl(sbuff->frame_vector) != ESPAsyncE131::VECTOR_FRAME)
-    			error = true;
-    		if (sbuff->dmp_vector != ESPAsyncE131::VECTOR_DMP)
-    			error = true;
-    		if (sbuff->property_values[0] != 0)
-    			error = true;
-    	}
+      // E1.31 packet identifier ("ACS-E1.17"), need at least 16 bytes to safely read acn_id (offset 4, length 12).
+      if (pktLen >= 16) {
+        if (memcmp(sbuff->acn_id, ESPAsyncE131::ACN_ID, sizeof(sbuff->acn_id)))
+          protocol = P_ARTNET;
+      }
+      if (protocol == P_ARTNET) {
+        if (memcmp(sbuff->art_id, ESPAsyncE131::ART_ID, sizeof(sbuff->art_id)))
+          error = true; //not "Art-Net"
+        if (sbuff->art_opcode != ARTNET_OPCODE_OPDMX && sbuff->art_opcode != ARTNET_OPCODE_OPPOLL)
+          error = true; //not a DMX or poll packet
+      } else { //E1.31 error handling
+        if (pktLen < 126) { // need up to property_values[0] at offset 125
+          error = true;
+        } else {
+          if (htonl(sbuff->root_vector) != ESPAsyncE131::VECTOR_ROOT)
+            error = true;
+          if (htonl(sbuff->frame_vector) != ESPAsyncE131::VECTOR_FRAME)
+            error = true;
+          if (sbuff->dmp_vector != ESPAsyncE131::VECTOR_DMP)
+            error = true;
+          if (sbuff->property_values[0] != 0)
+            error = true;
+        }
+      }
       if (error && _packet.localPort() == DDP_DEFAULT_PORT) { //DDP packet
         error = false;
         protocol = P_DDP;
       }
       if (!error) {
-        _callback(sbuff, _packet.remoteIP(), protocol);
+        _callback(sbuff, _packet.remoteIP(), protocol, pktLen);
       }
     }

-Original file line number
+Diff line change
@@ Expand Up / @@ -64,10 +64,14 @@ typedef struct ip_addr ip4_addr_t; @@
     #define DDP_TYPE_RGB24  0x0B // 00 001 011 (RGB , 8 bits per channel, 3 channels)
     #define DDP_TYPE_RGBW32 0x1B // 00 011 011 (RGBW, 8 bits per channel, 4 channels)
+    #define DDP_TYPE_LEGACY 0x01 // 00 000 001 legacy RGB 8bit definition
-    #define DDP_ID_DISPLAY  1
-    #define DDP_ID_CONFIG   250
-    #define DDP_ID_STATUS   251
+    // DDP Source or Destination ID (header byte 3)
+    #define DDP_ID_DISPLAY  1    // default output device
+    #define DDP_ID_CONTROL  246  // JSON control (not implemented)
+    #define DDP_ID_CONFIG   250  // JSON config (not implemented)
+    #define DDP_ID_STATUS   251  // JSON status (not implemented)
+    #define DDP_ID_ALL      255  // all devices
     #define ARTNET_OPCODE_OPDMX 0x5000
     #define ARTNET_OPCODE_OPPOLL 0x2000
@@ Expand Down Expand Up / @@ -212,7 +216,7 @@ typedef union { @@
     } ArtPollReply;
     // new packet callback
-    typedef void (*e131_packet_callback_function) (e131_packet_t* p, IPAddress clientIP, byte protocol);
+    typedef void (*e131_packet_callback_function) (e131_packet_t* p, IPAddress clientIP, byte protocol, size_t packetLen);
     class ESPAsyncE131 {
      private:
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up @@
             // write the header
             /*0*/ddpUdp.write(flags);
+            // TODO: sequence number should be 1-15 as 0 means "unused", it has no bad consequences other than out of sequence packet may be accepted
             /*1*/ddpUdp.write(sequenceNumber++ & 0x0F); // sequence may be unnecessary unless we are sending twice (as requested in Sync settings)
             /*2*/ddpUdp.write(isRGBW ?  DDP_TYPE_RGBW32 : DDP_TYPE_RGB24);
             /*3*/ddpUdp.write(DDP_ID_DISPLAY);
@@ Expand Down @@

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

less restrictive DDP header acceptance, add safety checks to all protocols #5547

Uh oh!

Diff view

Diff view

There are no files selected for viewing

softhack007 Apr 28, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot Apr 28, 2026

Uh oh!

DedeHai Apr 28, 2026

Uh oh!

coderabbitai Bot Apr 28, 2026

Uh oh!

softhack007 Apr 28, 2026

Uh oh!

coderabbitai Bot Apr 28, 2026

Uh oh!

softhack007 Apr 28, 2026 •

edited

Loading

Uh oh!

DedeHai Apr 28, 2026

Uh oh!

DedeHai Apr 28, 2026

Uh oh!

softhack007 Apr 28, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

-Original file line number
+Diff line change
@@ Expand Up @@
             if (!data || len < offset+1) return; // catch invalid / single-byte payload
             switch (data[0]) {
               case BINARY_PROTOCOL_E131:
-                handleE131Packet((e131_packet_t*)&data[offset], client->remoteIP(), P_E131);
+                handleE131Packet((e131_packet_t*)&data[offset], client->remoteIP(), P_E131, len - offset);
                 break;
               case BINARY_PROTOCOL_ARTNET:
-                handleE131Packet((e131_packet_t*)&data[offset], client->remoteIP(), P_ARTNET);
+                handleE131Packet((e131_packet_t*)&data[offset], client->remoteIP(), P_ARTNET, len - offset);
                 break;
               case BINARY_PROTOCOL_DDP:
-                if (len < 10 + offset) return; // DDP header is 10 bytes (+1 protocol byte)
-                size_t ddpDataLen = (data[8+offset] << 8) | data[9+offset]; // data length in bytes from DDP header
-                uint8_t flags = data[0+offset];
-                if ((flags & DDP_FLAGS_TIME) ) ddpDataLen += 4; // timecode flag adds 4 bytes to data length
-                if (len < (10 + offset + ddpDataLen)) return; // not enough data, prevent out of bounds read
-                // could be a valid DDP packet, forward to handler
-                handleE131Packet((e131_packet_t*)&data[offset], client->remoteIP(), P_DDP);
+                handleE131Packet((e131_packet_t*)&data[offset], client->remoteIP(), P_DDP, len - offset);
             }
           }
         } else {
@@ Expand Down @@

Uh oh!

less restrictive DDP header acceptance, add safety checks to all protocols #5547

Uh oh!

less restrictive DDP header acceptance, add safety checks to all protocols #5547

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

softhack007 Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Apr 28, 2026

Choose a reason for hiding this comment

DDP Sequence Number Spec

WLED's Rejection Logic

Where This Breaks Down

Known Sender Behaviors

Summary of Issues

Recommendations

Uh oh!

DedeHai Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

softhack007 Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Apr 28, 2026

Choose a reason for hiding this comment

Exact Breaking-Point Calculation

Can We Improve It?

Recommendation

Uh oh!

softhack007 Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DedeHai Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

DedeHai Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

softhack007 Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

softhack007 Apr 28, 2026 •

edited

Loading

softhack007 Apr 28, 2026 •

edited

Loading

softhack007 Apr 28, 2026 •

edited

Loading