Open
Conversation
Contributor
WalkthroughFetches Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@wled00/data/index.js`:
- Around line 3416-3427: The IR_TYPES object in wled00/data/index.js is
misaligned with the ir.cpp remote type enum; update IR_TYPES so its keys match
ir.cpp: keep 0 as null, then set 1 -> "24-key", 2 -> "24-key-v2" (CW/WW/CT
control), 3 -> "40-key", 4 -> "44-key", 5 -> "21-key", 6 -> "6-key", 7 ->
"9-key", and 8 -> "json-remote" and remove the unused 9 entry so reported
telemetry matches the ir.cpp switch cases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+3416
to
+3427
| const IR_TYPES = { | ||
| 0: null, // not configured — omit field entirely | ||
| 1: "3-key", | ||
| 2: "24-key", | ||
| 3: "24-key-v2", | ||
| 4: "40-key", | ||
| 5: "44-key", | ||
| 6: "21-key", | ||
| 7: "6-key", | ||
| 8: "9-key", | ||
| 9: "json-remote", | ||
| }; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify IR remote type definitions and their numeric codes
echo "=== Searching for IR remote type definitions in ir.cpp ==="
rg -n -C3 'switch.*irEnabled|case [0-9]:.*decode|irEnabled' wled00/ir.cpp
echo ""
echo "=== Searching for IR type constants ==="
rg -n 'IR_TYPE|irEnabled.*=' --type=cpp --type=h
echo ""
echo "=== Searching for IR type enum or defines ==="
ast-grep --pattern 'enum $_ { $$$ IR $$$ }'Repository: wled/WLED
Length of output: 2561
Fix IR_TYPES mapping—all values are misaligned with actual ir.cpp remote type definitions.
The ir.cpp switch statement (lines 677–691) defines IR remote types as:
1→ 24-key remote2→ 24-key with CW/WW/CT control3→ 40-key remote4→ 44-key remote5→ 21-key remote6→ 6-key learning remote7→ 9-key remote8→ ir.json configurable
The IR_TYPES mapping has these values shifted; for example, IR_TYPES[1]="3-key" should be "24-key", IR_TYPES[3]="24-key-v2" should be "40-key", etc. Additionally, IR_TYPES[9] has no corresponding case in ir.cpp. This causes incorrect IR remote types to be reported to the telemetry endpoint.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wled00/data/index.js` around lines 3416 - 3427, The IR_TYPES object in
wled00/data/index.js is misaligned with the ir.cpp remote type enum; update
IR_TYPES so its keys match ir.cpp: keep 0 as null, then set 1 -> "24-key", 2 ->
"24-key-v2" (CW/WW/CT control), 3 -> "40-key", 4 -> "44-key", 5 -> "21-key", 6
-> "6-key", 7 -> "9-key", and 8 -> "json-remote" and remove the unused 9 entry
so reported telemetry matches the ir.cpp switch cases.
Member
There was a problem hiding this comment.
The AI here is correct - as far as I can tell, there's no "3-key" setting, something has inserted an invalid value. If this code was original authored by another AI, and you've still got the context accessible, I'd be curious to ask what it was thinking -- is a 3-key remote commonly added by OEMs?
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== IR remote type switch in ir.cpp ==="
rg -n -A2 -B1 'case [0-9]+:' wled00/ir.cpp | head -80
echo ""
echo "=== IR settings / type defines ==="
rg -n 'irEnabled|IR_TYPE|irType|ir.*type' --type=h | head -40
echo ""
echo "=== cfg.cpp IR type storage ==="
rg -n -C2 'irEnabled|ir.*type|irType' wled00/cfg.cpp | head -40
echo ""
echo "=== usermods or other files referencing IR type names ==="
rg -rn '3.key\|24.key\|40.key\|44.key\|21.key\|6.key\|9.key' --type=h --type=cpp | head -30
Length of output: 4875
🌐 Web query:
"3-key" IR remote LED controller WLED common OEM
💡 Result:
A very common “3‑key” (3 physical buttons on the controller) Wi‑Fi + IR RGB constant‑voltage inline controller is the Skydance/iSkyDance F3 series, widely rebranded—especially F3‑D(WT) (Tuya/Wi‑Fi + IR + 3‑button). It’s specified as a 3‑channel constant‑voltage RGB controller with “3‑button operation (on/off, mode, color adjustment)” and IR control. [1] [2]
For WLED specifically: WLED has built-in profiles for several common multi-key IR remotes (24/40/44/21/6-key), and for anything else (including “3‑key” remotes) you generally use WLED’s “JSON IR remote” to map whatever IR codes your remote sends. [3]
Sources:
[1] SuperLightingLED F3‑D(WT) “3‑Button WiFi & IR RGB LED Controller” manual/spec PDF
[2] iSkyDance/Skydance product comparison PDF listing “3 Button IR Remote Control RGB Constant Voltage LED Controller” / F3‑D(WT)
[3] WLED documentation: Infrared / Supported IR remotes / JSON IR remote
@willmmiles — great question. Looking at the ir.cpp switch statement, here's what actually happened:
irEnabled values 1–8 in WLED map as follows:
| Value | Handler | Description |
|---|---|---|
| 1 | decodeIR24 / decodeIR24OLD |
24-key remote |
| 2 | decodeIR24CT |
24-key with CW/WW/CT keys |
| 3 | decodeIR40 |
40-key remote |
| 4 | decodeIR44 |
44-key remote |
| 5 | decodeIR21 |
21-key remote |
| 6 | decodeIR6 |
6-key learning remote |
| 7 | decodeIR9 |
9-key remote |
| 8 | (handled above switch) | JSON/ir.json remote |
The "3-key" entry inserted at index 1 caused every real value to be shifted by one, with 9: "json-remote" left dangling beyond WLED's defined range.
As to the likely "thinking": 3-key IR remotes do exist in the wild (e.g. the Skydance F3 series, a common OEM constant-voltage RGB controller with a 3-button panel + IR), but WLED has no built-in decoder for them — those would be handled via json-remote (case 8). So it appears to be a hallucination that mixed general LED controller knowledge with WLED's specific enum, then cascaded all real entries incorrectly.
The corrected mapping for @netmindz should be:
const IR_TYPES = {
0: null, // not configured — omit field entirely
1: "24-key", // white 24-key remote
2: "24-key-ct", // white 24-key with CW/WW/CT keys
3: "40-key", // blue 40-key remote
4: "44-key", // white 44-key remote
5: "21-key", // white 21-key remote
6: "6-key", // black 6-key learning remote
7: "9-key", // 9-key remote
8: "json-remote", // ir.json configurable remote
};🧠 Learnings used
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: 5442
File: wled00/bus_manager.cpp:462-468
Timestamp: 2026-03-23T23:33:45.480Z
Learning: In WLED's TYPE_ANALOG_2CH_HBRIDGE (wled00/bus_manager.cpp), the dead zone at the CCT midpoint (level=127 and level=128 both producing _data[0]=0, _data[1]=0) is intentional by design. The map() blending is directional: WW channel (_data[0]) is brightest at level=0 and fades to 0 at level=127; CW channel (_data[1]) starts at 0 at level=128 and is brightest at level=255. The zero-output at the midpoint is the H-bridge polarity transition boundary, ensuring no overlap between the two polarity directions. Do not flag this dead zone as a bug in future reviews.
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit. This improves code maintainability and reduces the risk of inconsistencies.
Learnt from: softhack007
Repo: wled/WLED PR: 5457
File: usermods/zigbee_rgb_light/usermod_zigbee_rgb_light.h:0-0
Timestamp: 2026-03-31T17:30:57.269Z
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: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with defined constants when meaningful constants exist in the codebase. For example, suggest replacing hardcoded "32" with WLED_MAX_SEGNAME_LEN if the context relates to segment name length limits.
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: 0
File: :0-0
Timestamp: 2026-03-08T00:57:36.134Z
Learning: In WLED (wled00/cfg.cpp), `deserializeConfig()` is called with an empty JsonObject when cfg.json doesn't exist on fresh install. Any value read without the ArduinoJSON `|` fallback operator or CJSON macro will override correct constructor defaults with 0/null. Known affected values: `strip.setTargetFps(hw_led["fps"])` at line 179 (sets FPS to 0/unlimited instead of WLED_FPS=42). Fix: `strip.setTargetFps(hw_led["fps"] | WLED_FPS)`. The gamma issue (gammaCorrectCol/Bri) had the same root cause and was fixed in commit d1d9dec402 (Jan 2026) using inline `| default` fallbacks.
Learnt from: 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: 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: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-28T14:18:55.599Z
Learning: Applies to wled00/data/**/*.js : When updating Web UI files in `wled00/data/`, make use of common functions available in `wled00/data/common.js` where possible
Learnt from: willmmiles
Repo: wled/WLED PR: 5462
File: wled00/json.cpp:1189-1198
Timestamp: 2026-03-30T15:32:02.808Z
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: 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: netmindz
Repo: wled/WLED PR: 5093
File: wled00/util.cpp:1159-1182
Timestamp: 2025-11-20T00:04:04.829Z
Learning: In WLED PR `#5093`, the deviceId feature is designed for opt-in usage reporting that tracks only version/upgrade information (non-behavioral data), not user activity patterns. The deterministic salt approach (MAC + "WLED" + chip model/revision) is acceptable for this limited use case, as correlating MAC addresses to version history represents minimal privacy risk compared to behavioral tracking.
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-03-29T20:58:20.383Z
Learning: In WLED PR `#5048`, /json/net is generated by wled00/json.cpp::serializeNetworks(), which now includes ssid, rssi, bssid, channel, and enc for each scanned network. Any “Ch undefined” in the UI applies only to legacy builds or empty-scan states.
Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/include/NeoEsp32RmtHIMethod.h:173-180
Timestamp: 2025-09-02T01:56:43.841Z
Learning: willmmiles prefers to maintain consistency with upstream NeoPixelBus patterns (like unchecked malloc in construct() methods) rather than diverging until improvements are made upstream first, to minimize maintenance burden and keep the codebase aligned.
Learnt from: softhack007
Repo: wled/WLED PR: 5456
File: platformio.ini:794-830
Timestamp: 2026-03-31T13:41:45.126Z
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: 5456
File: platformio.ini:794-830
Timestamp: 2026-03-31T13:41:45.126Z
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. Do NOT flag the missing `-include` for `matter_gcc14_compat.h` as a build issue.
Learnt from: KrX3D
Repo: wled/WLED PR: 4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.cpp:3-5
Timestamp: 2026-02-18T00:11:02.403Z
Learning: In PR `#4585`, the seven_segment_display_reloaded_v2 usermod intentionally uses `#error` to require MQTT (`#ifdef WLED_DISABLE_MQTT `#error` "This user mod requires MQTT to be enabled." `#endif``) because the integration flow has not been tested or validated without MQTT, and failing fast at compile time is preferred over allowing potentially broken builds until a full MQTT-optional refactor is completed.
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-03-15T13:21:32.008Z
Learning: In WLED code reviews (and generally), when a PR discussion involves "is this worth doing?" uncertainty about a proposed reliability, safety, or data-integrity mechanism (e.g. CRC checks, backups, power-loss protection), suggest performing a software FMEA (Failure Mode and Effects Analysis). The FMEA should: enumerate failure modes for the feared event, assess each mitigation's effectiveness per failure mode, note common-cause/common-mode failures, and rate credibility for the typical WLED use case. This was successfully used in PR `#5421` to evaluate CRC-based config file protection and was well-received by the WLED maintainers (softhack007 explicitly requested this be remembered for future reviews).
Learnt from: softhack007
Repo: wled/WLED PR: 5381
File: wled00/data/update.htm:2-2
Timestamp: 2026-02-19T13:24:19.009Z
Learning: In the WLED repo, do not commit generated embedded header files (e.g., html_*.h, js_iro.h). These are automatically regenerated during the build from wled00/data/. Only commit source web UI files in wled00/data/ with extensions .htm, .html, .css, or .js (e.g., update.htm and other source UI files).
…s, peripherals, and integrations
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wled00/data/index.js (1)
3525-3528:⚠️ Potential issue | 🟠 MajorDon't mark failed reports as handled.
This shared
catchalso covers the fresh/json/infoand/json/cfgreads. CallingupdateVersionInfo(...)here suppresses future prompts even when nothing reached the telemetry API, which also contradicts Line 3522. Only persist the new version after a successful POST or an explicit user opt-out.Possible fix
.catch(e => { console.log('Failed to report upgrade', e); showToast('Report failed', true); - updateVersionInfo(info.ver, false, !!alwaysReport); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/data/index.js` around lines 3525 - 3528, The catch handler that logs 'Failed to report upgrade' currently calls updateVersionInfo(info.ver, false, !!alwaysReport) which incorrectly marks the version as handled even when telemetry POST or subsequent /json/info and /json/cfg reads failed; modify the error path in the promise chain around the POST (the .catch(e => { ... }) that contains showToast and console.log) to remove the updateVersionInfo(...) call so it is only invoked after a successful POST response or when the user explicitly opts out, or alternatively add a success flag and call updateVersionInfo(info.ver, false, !!alwaysReport) only inside the successful POST response handler (not in the shared catch) to avoid suppressing future prompts; ensure references to updateVersionInfo, showToast and the failing .catch are updated accordingly.
♻️ Duplicate comments (1)
wled00/data/index.js (1)
3416-3427:⚠️ Potential issue | 🟠 MajorFix the
IR_TYPESlookup; it's still shifted by one.
cfgData.hw?.ir?.typeis used verbatim here, so every non-zero IR remote is reported under the wrong name today.json-remotealso belongs on code8, not9.Possible fix
const IR_TYPES = { - 1: "3-key", - 2: "24-key", - 3: "24-key-v2", - 4: "40-key", - 5: "44-key", - 6: "21-key", - 7: "6-key", - 8: "9-key", - 9: "json-remote", + 1: "24-key", + 2: "24-key-v2", + 3: "40-key", + 4: "44-key", + 5: "21-key", + 6: "6-key", + 7: "9-key", + 8: "json-remote", };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/data/index.js` around lines 3416 - 3427, The IR_TYPES lookup is misaligned with cfgData.hw?.ir?.type so non-zero IR types are reported wrong; update IR_TYPES (the constant named IR_TYPES) so the values align with their numeric codes — specifically move "json-remote" to key 8 and ensure "9-key" is at key 9 (i.e., correct the shifted mapping so cfgData.hw?.ir?.type indexes the right label).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@wled00/data/index.js`:
- Around line 3470-3478: The peripherals array collapses button topology to a
boolean via .some(b => b.type !== 0); instead compute the actual physical button
count from cfgData.hw?.btn?.ins (e.g. const buttonCount = (cfgData.hw?.btn?.ins
?? []).filter(b => b.type !== 0).length), use buttonCount > 0 to include
"buttons" in the peripherals list, and expose the count as a separate field
(e.g. buttonCount or buttons_count) in the payload so single-button and
multi-button installs remain distinguishable; update references to
cfgData.hw?.btn?.ins and remove the .some(...) usage.
- Around line 3429-3434: Replace the parallel Promise.all fetches with a
serialized flow: use the existing info argument if present (the local info
variable/parameter) or fetch getURL('/json/info') first, await its JSON, then
fetch getURL('/json/cfg') and await its JSON; remove Promise.all usage (and any
references to infoData/cfgData from the parallel result) and instead use
sequential variables (e.g., info and cfg) so the /json/cfg request only runs
after /json/info completes to avoid overlapping startup requests.
- Around line 3458-3467: The gamma feature is being detected by comparing
cfgData.light?.gc?.col > 1.0 which treats the boolean as numeric and prevents
"gamma" from being reported; update the ledFeatures construction (the array
assigned to ledFeatures) to treat gamma flags as booleans — check
cfgData.light?.gc?.col || cfgData.light?.gc?.bri (or similar boolean fields like
gammaCorrectCol/gammaCorrectBri) instead of numeric comparison so that "gamma"
is included when either gamma correction flag is true.
---
Outside diff comments:
In `@wled00/data/index.js`:
- Around line 3525-3528: The catch handler that logs 'Failed to report upgrade'
currently calls updateVersionInfo(info.ver, false, !!alwaysReport) which
incorrectly marks the version as handled even when telemetry POST or subsequent
/json/info and /json/cfg reads failed; modify the error path in the promise
chain around the POST (the .catch(e => { ... }) that contains showToast and
console.log) to remove the updateVersionInfo(...) call so it is only invoked
after a successful POST response or when the user explicitly opts out, or
alternatively add a success flag and call updateVersionInfo(info.ver, false,
!!alwaysReport) only inside the successful POST response handler (not in the
shared catch) to avoid suppressing future prompts; ensure references to
updateVersionInfo, showToast and the failing .catch are updated accordingly.
---
Duplicate comments:
In `@wled00/data/index.js`:
- Around line 3416-3427: The IR_TYPES lookup is misaligned with
cfgData.hw?.ir?.type so non-zero IR types are reported wrong; update IR_TYPES
(the constant named IR_TYPES) so the values align with their numeric codes —
specifically move "json-remote" to key 8 and ensure "9-key" is at key 9 (i.e.,
correct the shifted mapping so cfgData.hw?.ir?.type indexes the right label).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+3429
to
+3434
| // Fetch fresh data from /json/info and /json/cfg endpoints | ||
| Promise.all([ | ||
| fetch(getURL('/json/info'), {method: 'get'}).then(res => res.json()), | ||
| fetch(getURL('/json/cfg'), {method: 'get'}).then(res => res.json()) | ||
| ]) | ||
| .then(([infoData, cfgData]) => { |
Contributor
There was a problem hiding this comment.
Don't reintroduce parallel startup fetches.
Lines 287-295 already serialize device requests to avoid 503s on low-heap hardware. Promise.all() here can overlap that init path in the auto-report flow and bring the same failure mode back. Reuse the info argument and fetch /json/cfg afterward, or serialize both requests.
Possible fix
- Promise.all([
- fetch(getURL('/json/info'), {method: 'get'}).then(res => res.json()),
- fetch(getURL('/json/cfg'), {method: 'get'}).then(res => res.json())
- ])
- .then(([infoData, cfgData]) => {
+ const infoData = info;
+ fetch(getURL('/json/cfg'), {method: 'get'})
+ .then(res => res.ok ? res.json() : Promise.reject(new Error('Failed to fetch /json/cfg')))
+ .then(cfgData => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wled00/data/index.js` around lines 3429 - 3434, Replace the parallel
Promise.all fetches with a serialized flow: use the existing info argument if
present (the local info variable/parameter) or fetch getURL('/json/info') first,
await its JSON, then fetch getURL('/json/cfg') and await its JSON; remove
Promise.all usage (and any references to infoData/cfgData from the parallel
result) and instead use sequential variables (e.g., info and cfg) so the
/json/cfg request only runs after /json/info completes to avoid overlapping
startup requests.
Member
There was a problem hiding this comment.
While it's unlikely that progress reporting ends up overlapping with the initial load, I agree with the AI that it would be better to avoid parallel requests unless you're prepared to handle 503s -- especially since the progress reporting is not particularly latency sensitive.
Contributor
There was a problem hiding this comment.
@willmmiles, agreed — serializing is the right call here. The info object is already passed into reportUpgradeEvent(), so there's no need to re-fetch /json/info at all. Here's the recommended fix:
- Promise.all([
- fetch(getURL('/json/info'), {method: 'get'}).then(res => res.json()),
- fetch(getURL('/json/cfg'), {method: 'get'}).then(res => res.json())
- ])
- .then(([infoData, cfgData]) => {
+ const infoData = info;
+ fetch(getURL('/json/cfg'), {method: 'get'})
+ .then(res => res.ok ? res.json() : Promise.reject(new Error('Failed to fetch /json/cfg')))
+ .then(cfgData => {This eliminates the parallel fetch entirely — reusing the already-available info argument as infoData and fetching only /json/cfg as a single sequential request. Since telemetry reporting is not latency-sensitive, there's no downside to this approach.
🧠 Learnings used
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-28T14:18:55.599Z
Learning: Applies to wled00/data/**/*.js : When updating Web UI files in `wled00/data/`, make use of common functions available in `wled00/data/common.js` where possible
Learnt from: willmmiles
Repo: wled/WLED PR: 5462
File: wled00/json.cpp:1189-1198
Timestamp: 2026-03-30T15:32:02.808Z
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: 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: netmindz
Repo: wled/WLED PR: 5093
File: wled00/util.cpp:1159-1182
Timestamp: 2025-11-20T00:04:04.829Z
Learning: In WLED PR `#5093`, the deviceId feature is designed for opt-in usage reporting that tracks only version/upgrade information (non-behavioral data), not user activity patterns. The deterministic salt approach (MAC + "WLED" + chip model/revision) is acceptable for this limited use case, as correlating MAC addresses to version history represents minimal privacy risk compared to behavioral tracking.
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-03-29T20:58:20.383Z
Learning: In WLED PR `#5048`, /json/net is generated by wled00/json.cpp::serializeNetworks(), which now includes ssid, rssi, bssid, channel, and enc for each scanned network. Any “Ch undefined” in the UI applies only to legacy builds or empty-scan states.
Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/include/NeoEsp32RmtHIMethod.h:173-180
Timestamp: 2025-09-02T01:56:43.841Z
Learning: willmmiles prefers to maintain consistency with upstream NeoPixelBus patterns (like unchecked malloc in construct() methods) rather than diverging until improvements are made upstream first, to minimize maintenance burden and keep the codebase aligned.
Learnt from: softhack007
Repo: wled/WLED PR: 5456
File: platformio.ini:794-830
Timestamp: 2026-03-31T13:41:45.126Z
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: 5456
File: platformio.ini:794-830
Timestamp: 2026-03-31T13:41:45.126Z
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. Do NOT flag the missing `-include` for `matter_gcc14_compat.h` as a build issue.
Learnt from: softhack007
Repo: wled/WLED PR: 5457
File: usermods/zigbee_rgb_light/usermod_zigbee_rgb_light.h:0-0
Timestamp: 2026-03-31T17:30:57.269Z
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: 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: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit. This improves code maintainability and reduces the risk of inconsistencies.
Learnt from: KrX3D
Repo: wled/WLED PR: 4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.cpp:3-5
Timestamp: 2026-02-18T00:11:02.403Z
Learning: In PR `#4585`, the seven_segment_display_reloaded_v2 usermod intentionally uses `#error` to require MQTT (`#ifdef WLED_DISABLE_MQTT `#error` "This user mod requires MQTT to be enabled." `#endif``) because the integration flow has not been tested or validated without MQTT, and failing fast at compile time is preferred over allowing potentially broken builds until a full MQTT-optional refactor is completed.
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-03-15T13:21:32.008Z
Learning: In WLED code reviews (and generally), when a PR discussion involves "is this worth doing?" uncertainty about a proposed reliability, safety, or data-integrity mechanism (e.g. CRC checks, backups, power-loss protection), suggest performing a software FMEA (Failure Mode and Effects Analysis). The FMEA should: enumerate failure modes for the feared event, assess each mitigation's effectiveness per failure mode, note common-cause/common-mode failures, and rate credibility for the typical WLED use case. This was successfully used in PR `#5421` to evaluate CRC-based config file protection and was well-received by the WLED maintainers (softhack007 explicitly requested this be remembered for future reviews).
Learnt from: softhack007
Repo: wled/WLED PR: 5381
File: wled00/data/update.htm:2-2
Timestamp: 2026-02-19T13:24:19.009Z
Learning: In the WLED repo, do not commit generated embedded header files (e.g., html_*.h, js_iro.h). These are automatically regenerated during the build from wled00/data/. Only commit source web UI files in wled00/data/ with extensions .htm, .html, .css, or .js (e.g., update.htm and other source UI files).
Comment on lines
+3458
to
+3467
| ledFeatures: [ | ||
| ...(infoData.leds?.lc & 0x02 ? ["rgbw"] : []), | ||
| ...(infoData.leds?.lc & 0x04 ? ["cct"] : []), | ||
| ...((infoData.leds?.maxpwr ?? 0) > 0 ? ["abl"] : []), | ||
| ...(cfgData.hw?.led?.cr ? ["cct-from-rgb"] : []), | ||
| ...(cfgData.hw?.led?.cct ? ["white-balance"] : []), | ||
| ...((cfgData.light?.gc?.col ?? 1.0) > 1.0 ? ["gamma"] : []), | ||
| ...(cfgData.light?.aseg ? ["auto-segments"] : []), | ||
| ...((cfgData.light?.nl?.mode ?? 0) > 0 ? ["nightlight"] : []), | ||
| ], |
Contributor
There was a problem hiding this comment.
Use the gamma flag as a boolean.
cfgData.light?.gc?.col is a boolean enable field. Comparing it to > 1.0 coerces true to 1, so "gamma" never gets reported when gamma correction is enabled. If the intent is device-level gamma support, check gc.col || gc.bri instead.
Based on learnings, gammaCorrectCol/Bri are config booleans in wled00/cfg.cpp, not numeric gamma values.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wled00/data/index.js` around lines 3458 - 3467, The gamma feature is being
detected by comparing cfgData.light?.gc?.col > 1.0 which treats the boolean as
numeric and prevents "gamma" from being reported; update the ledFeatures
construction (the array assigned to ledFeatures) to treat gamma flags as
booleans — check cfgData.light?.gc?.col || cfgData.light?.gc?.bri (or similar
boolean fields like gammaCorrectCol/gammaCorrectBri) instead of numeric
comparison so that "gamma" is included when either gamma correction flag is
true.
Comment on lines
+3470
to
+3478
| peripherals: [ | ||
| ...((cfgData.hw?.relay?.pin ?? -1) >= 0 ? ["relay"] : []), | ||
| ...((cfgData.hw?.btn?.ins ?? []).some(b => b.type !== 0) ? ["buttons"] : []), | ||
| ...((cfgData.hw?.if?.['i2c-pin']?.[0] ?? -1) >= 0 ? ["i2c"] : []), | ||
| ...((cfgData.hw?.if?.['spi-pin']?.[0] ?? -1) >= 0 ? ["spi"] : []), | ||
| ...((cfgData.eth?.type ?? 0) > 0 ? ["ethernet"] : []), | ||
| ...((cfgData.if?.live?.dmx?.inputRxPin ?? 0) > 0 ? ["dmx-input"] : []), | ||
| ...((cfgData.hw?.ir?.type ?? 0) > 0 ? ["ir-remote"] : []), | ||
| ], |
Contributor
There was a problem hiding this comment.
Don't collapse button topology to a boolean.
cfgData.hw?.btn?.ins is already the source for button topology, but .some(b => b.type !== 0) throws the count away. Issue #5444 explicitly asks for physical button count, so single-button and multi-button installs will be indistinguishable in this payload.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wled00/data/index.js` around lines 3470 - 3478, The peripherals array
collapses button topology to a boolean via .some(b => b.type !== 0); instead
compute the actual physical button count from cfgData.hw?.btn?.ins (e.g. const
buttonCount = (cfgData.hw?.btn?.ins ?? []).filter(b => b.type !== 0).length),
use buttonCount > 0 to include "buttons" in the peripherals list, and expose the
count as a separate field (e.g. buttonCount or buttons_count) in the payload so
single-button and multi-button installs remain distinguishable; update
references to cfgData.hw?.btn?.ins and remove the .some(...) usage.
willmmiles
reviewed
Apr 4, 2026
Comment on lines
+3429
to
+3434
| // Fetch fresh data from /json/info and /json/cfg endpoints | ||
| Promise.all([ | ||
| fetch(getURL('/json/info'), {method: 'get'}).then(res => res.json()), | ||
| fetch(getURL('/json/cfg'), {method: 'get'}).then(res => res.json()) | ||
| ]) | ||
| .then(([infoData, cfgData]) => { |
Member
There was a problem hiding this comment.
While it's unlikely that progress reporting ends up overlapping with the initial load, I agree with the AI that it would be better to avoid parallel requests unless you're prepared to handle 503s -- especially since the progress reporting is not particularly latency sensitive.
Comment on lines
+3473
to
+3474
| ...((cfgData.hw?.if?.['i2c-pin']?.[0] ?? -1) >= 0 ? ["i2c"] : []), | ||
| ...((cfgData.hw?.if?.['spi-pin']?.[0] ?? -1) >= 0 ? ["spi"] : []), |
Member
There was a problem hiding this comment.
Are these diagnostic? IIRC some of the pin settings default to values from the Arduino platform, depending on the specific board selected. I'm pretty sure that at least some of my test boards come up with some of these pre-set -- I distinctly remember having to unassign them because that's where I'd hooked up my LEDs..
Comment on lines
+3416
to
+3427
| const IR_TYPES = { | ||
| 0: null, // not configured — omit field entirely | ||
| 1: "3-key", | ||
| 2: "24-key", | ||
| 3: "24-key-v2", | ||
| 4: "40-key", | ||
| 5: "44-key", | ||
| 6: "21-key", | ||
| 7: "6-key", | ||
| 8: "9-key", | ||
| 9: "json-remote", | ||
| }; |
Member
There was a problem hiding this comment.
The AI here is correct - as far as I can tell, there's no "3-key" setting, something has inserted an invalid value. If this code was original authored by another AI, and you've still got the context accessible, I'd be curious to ask what it was thinking -- is a 3-key remote commonly added by OEMs?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5444
Summary by CodeRabbit