🚀 Add new MCU support (-C6,-C5,-P4) to the ESP-IDF V5 builds#5048
Conversation
due to Arduino 3.0.1, a few updated libraries are required: * Tasmota Platform - official platfomio lacks arduino support for C6 * FastLED (latest + C6 build patches) * AsyncTCP (latest + C6 build patches) * AsyncWebServer (latest + C6 build patches) * NeoPixelBus (lastest)
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (8)
WalkthroughAdds ESP32-C5, ESP32-C6, and ESP32-P4 build environments to PlatformIO, introduces a new ChangesESP32-C5/C6/P4 support and WiFi 5G band selection
Sequence DiagramsequenceDiagram
participant UI as UI: Band Selector
participant SetCpp as set.cpp: BM handler
participant CfgCpp as cfg.cpp: Config store
participant WledCpp as wled.cpp: Init/Connect
participant WiFiAPI as ESP32 WiFi API
participant JsonCpp as json.cpp: Serialize
UI->>SetCpp: POST BM=band_mode value
SetCpp->>SetCpp: validate, update wifiBandMode
SetCpp->>WiFiAPI: WiFi.scanDelete()
CfgCpp->>CfgCpp: deserializeConfig parse wifi.band
WledCpp->>WledCpp: setup() / initAP() / initConnection()
WledCpp->>WiFiAPI: WiFi.setBandMode(wifiBandMode)
WiFiAPI-->>WledCpp: STA connected
WledCpp->>WiFiAPI: WiFi.channel()<br/>WiFi.getBand()
JsonCpp->>JsonCpp: serialize band/channel<br/>to JSON info
JsonCpp-->>UI: emit serializeInfo<br/>with band field
CfgCpp->>CfgCpp: serializeConfig write wifi.band
Estimated code review effort🎯 4 (Complex) | ⏱️ ~90 minutes Possibly related PRs
Suggested labels
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. 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 |
| ;nodemcuv2 | ||
| ;esp8266_2m | ||
| ;esp01_1m_full | ||
| ;nodemcuv2_160 | ||
| ;esp8266_2m_160 | ||
| ;esp01_1m_full_160 | ||
| ;nodemcuv2_compat | ||
| ;esp8266_2m_compat | ||
| ;esp01_1m_full_compat | ||
| ;esp32dev | ||
| ;esp32dev_debug | ||
| ;esp32_eth | ||
| ;esp32_wrover | ||
| ; lolin_s2_mini ;; TODO: disabled NeoEsp32RmtMethodIsr | ||
| esp32c3dev | ||
| ;esp32c3dev |
There was a problem hiding this comment.
TODO: revert before merge, here to save CI build time
|
@softhack007 I've started trying to cherry-pick your changes over, quite a few don't apply due to the changes with MM, but I'll try and resolve the merge conflicts for those that are simple |
* more debug output * added my own fork of FastLED ( looks like more bugs to solve ....)
@netmindz thanks, I'll try to help once I'm finished with some open ends in WLED-MM 😁 december |
|
Possibly also worth looking at #4626 to see if there are any changes there that are helpful |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
also disabled some core debug flags
this was a temporary hack needed one year ago; FastLED should be compatible with -C6 now.
trying to reduce compile errors in bus_wrapper.h ... plus some preparation for P4 support
similar to C3
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
wled00/pin_manager.cpp (1)
403-428:⚠️ Potential issue | 🟡 MinorUpdate the stale ADC1-only comment.
Line 403 still says only ADC1 is usable, but the new S3 and generic paths intentionally accept ADC2 where supported. Please narrow that warning to classic ESP32 so future readers do not “fix” this back incorrectly.
📝 Proposed comment update
- // Check ADC capability: only ADC1 channels can be used (ADC2 channels are not usable when WiFi is active) + // Check ADC capability. On classic ESP32 only ADC1 is usable while WiFi is active; + // newer ESP32 targets may safely expose ADC2 through the oneshot/Arduino analogRead path.Based on learnings, the ADC2/WiFi hard block applies only to classic ESP32; ESP32-S2/S3 and newer oneshot paths can safely expose ADC2 for WLED’s
analogRead()usage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/pin_manager.cpp` around lines 403 - 428, The top comment "Check ADC capability: only ADC1 channels can be used (ADC2 channels are not usable when WiFi is active)" is stale; update it to specify that the ADC2/WiFi hard block applies only to classic ESP32 (CONFIG_IDF_TARGET_ESP32) and that the other branches (CONFIG_IDF_TARGET_ESP32S2, CONFIG_IDF_TARGET_ESP32S3 and the generic path using SOC_ADC_CHANNEL_NUM/SOC_ADC_PERIPH_NUM and digitalPinToAnalogChannel) intentionally accept ADC2 where supported for analogRead(). Locate the comment and the surrounding checks (digitalPinToAnalogChannel, SOC_ADC_CHANNEL_NUM, SOC_ADC_PERIPH_NUM) in pin_manager.cpp and replace the generic warning with a targeted note about classic ESP32 only.wled00/const.h (2)
538-559:⚠️ Potential issue | 🟠 MajorKeep C5/C61
MAX_LEDSaligned with the non-PSRAM memory cap.Line 538 allows 4096 LEDs on non-PSRAM C5/C61, but Line 550 caps their pixel/bus memory at 28 KB. A single 4096-LED bus can exceed that once global and segment buffers are counted, causing the
FX_fcn.cppmemory guard to replace it with a placeholder bus instead of driving LEDs.Proposed cap alignment for non-PSRAM C5/C61
`#ifndef` MAX_LEDS `#ifdef` ESP8266 `#define` MAX_LEDS 1536 //can't rely on memory limit to limit this to 1536 LEDs - `#elif` defined(CONFIG_IDF_TARGET_ESP32S2) - `#define` MAX_LEDS 2048 //due to memory constraints S2 + `#elif` defined(CONFIG_IDF_TARGET_ESP32S2) || ((defined(CONFIG_IDF_TARGET_ESP32C5) || defined(CONFIG_IDF_TARGET_ESP32C61)) && !defined(BOARD_HAS_PSRAM)) + `#define` MAX_LEDS 2048 // due to memory constraints on S2 and non-PSRAM C5/C61 `#elif` defined(CONFIG_IDF_TARGET_ESP32C3) || defined(CONFIG_IDF_TARGET_ESP32C5) || defined(CONFIG_IDF_TARGET_ESP32C6) || defined(CONFIG_IDF_TARGET_ESP32C61) `#define` MAX_LEDS 4096 `#else` `#define` MAX_LEDS 16384 // classic esp32, S3 and P4 can take more🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/const.h` around lines 538 - 559, The MAX_LEDS macro allows 4096 for CONFIG_IDF_TARGET_ESP32C5/ESP32C61 but MAX_LED_MEMORY for those targets without PSRAM is only 28KB, so reduce MAX_LEDS when BOARD_HAS_PSRAM is not defined to match the non-PSRAM memory cap to avoid FX_fcn.cpp falling back to a placeholder bus; update the MAX_LEDS definition block (the `#elif` branch that checks CONFIG_IDF_TARGET_ESP32C5 and CONFIG_IDF_TARGET_ESP32C61) to conditionally define a lower MAX_LEDS when BOARD_HAS_PSRAM is not defined and keep the larger value when BOARD_HAS_PSRAM is present, ensuring the MAX_LEDS and MAX_LED_MEMORY constraints are consistent for C5/C61 targets.
674-680:⚠️ Potential issue | 🟠 MajorUse GPIO1 as the default LED pin for ESP32-C5 to avoid its bootstrap strapping pin GPIO2.
GPIO2 is confirmed as a C5 bootstrap strapping pin per Espressif documentation (ESP-IDF GPIO docs and boot configuration specs). Using GPIO2 as the default LED pin could cause boot failures if GPIO2 is driven high by LED wiring before the bootloader runs. GPIO1 is safe on C5—it is within the usable 0–15 range and is not a strapping, reserved, or USB-JTAG pin.
Suggested C5-specific branch
`#if` defined(ESP8266) || defined(CONFIG_IDF_TARGET_ESP32C3) || defined(CONFIG_IDF_TARGET_ESP32C5) || defined(CONFIG_IDF_TARGET_ESP32C6) || defined(CONFIG_IDF_TARGET_ESP32C61) || defined(CONFIG_IDF_TARGET_ESP32P4) `#ifdef` WLED_ENABLE_DMX `#define` DEFAULT_LED_PIN 1 `#warning` "Compiling with DMX. The default LED pin has been changed to pin 1." `#else` - `#define` DEFAULT_LED_PIN 2 // GPIO2 (D4) on Wemos D1 mini compatible boards, safe to use on any board + `#if` defined(CONFIG_IDF_TARGET_ESP32C5) + `#define` DEFAULT_LED_PIN 1 // avoid GPIO2, which is a C5 bootstrap strapping pin + `#else` + `#define` DEFAULT_LED_PIN 2 // GPIO2 (D4) on Wemos D1 mini compatible boards + `#endif` `#endif`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/const.h` around lines 674 - 680, The DEFAULT_LED_PIN selection block should avoid using GPIO2 for ESP32-C5 because GPIO2 is a bootstrap strapping pin; update the preprocessor logic so that when CONFIG_IDF_TARGET_ESP32C5 is defined (or the combined condition that includes it) the DEFAULT_LED_PIN is set to 1 instead of 2; modify the conditional around DEFAULT_LED_PIN (the `#if` defined(...) / `#ifdef` WLED_ENABLE_DMX block that defines DEFAULT_LED_PIN) to special-case CONFIG_IDF_TARGET_ESP32C5 (keeping the existing DMX override behavior) so the symbol DEFAULT_LED_PIN resolves to 1 for ESP32-C5 while leaving other targets unchanged.
♻️ Duplicate comments (1)
wled00/data/settings_wifi.htm (1)
284-287:⚠️ Potential issue | 🟡 MinorKeep high TX power options aligned with supported builds.
If IDF v4 artifacts still serve this page, values
80,82, and84can be selected but are clamped back to78inset.cpp, so the UI and persisted setting diverge. Gate or remove these options for non-V5 builds, or confirm this branch no longer ships the page with IDF v4 firmware.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/data/settings_wifi.htm` around lines 284 - 287, The TX power dropdown in settings_wifi.htm exposes values 84/82/80 which are clamped to 78 in set.cpp causing UI/persist divergence; update the TX <select name="TX"> options to match supported builds by either removing or gating 84/82/80 behind the V5-only build flag (the same compile-time or template flag used to select the V5 framework) or replace those option values with the effective clamped value (78) so the UI matches set.cpp behavior; reference the TX select in settings_wifi.htm and the clamping logic in set.cpp to implement the guard or value alignment.
🧹 Nitpick comments (3)
wled00/wled_boards.h (2)
44-45: Rename macro to use all-uppercase for consistency with coding guidelines.
WLED_HAVE_FAST_int_DIVIDEviolates the UPPER_CASE macro naming convention. Rename toWLED_HAVE_FAST_INT_DIVIDEthroughout the codebase (4 definitions inwled_boards.hand 9 uses acrossFX_fcn.cppandFXparticleSystem.cpp).♻️ Proposed direction
- `#define` WLED_HAVE_FAST_int_DIVIDE 1 + `#define` WLED_HAVE_FAST_INT_DIVIDE 1Update all preprocessor conditionals from
#if !defined(WLED_HAVE_FAST_int_DIVIDE)to#if !defined(WLED_HAVE_FAST_INT_DIVIDE).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/wled_boards.h` around lines 44 - 45, Rename the macro WLED_HAVE_FAST_int_DIVIDE to WLED_HAVE_FAST_INT_DIVIDE everywhere: update the four definitions in wled_boards.h and all uses (preprocessor checks and conditionals) in FX_fcn.cpp and FXparticleSystem.cpp, changing occurrences like `#if` !defined(WLED_HAVE_FAST_int_DIVIDE) (and any direct uses) to `#if` !defined(WLED_HAVE_FAST_INT_DIVIDE) (and use the new name in `#define` and `#ifdef/`#ifndef checks) so the macro follows the UPPER_CASE convention consistently across the codebase.
9-10: Consider explicitly includingsoc/soc_caps.hto clarify the dependency onSOC_CPU_CORES_NUM.
wled_boards.husesSOC_CPU_CORES_NUM(lines 53, 69, 143), which is defined insoc/soc_caps.h. The current code works because the macro is guarded by#if defined(CONFIG_IDF_TARGET_ESP32xx)checks and is indirectly included via Arduino/ESP-IDF headers. However, adding an explicit include makes the dependency visible and follows the "include what you use" principle. This is a defensive improvement for clarity and future-proofing.Suggested enhancement
`#define` WLED_BOARDS_H `#include` "NodeStruct.h" // to get generic NODE_TYPE_IDs + +#if defined(ARDUINO_ARCH_ESP32) + `#include` "soc/soc_caps.h" +#endif🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/wled_boards.h` around lines 9 - 10, wled_boards.h uses SOC_CPU_CORES_NUM but does not explicitly include the header that defines it; add an explicit `#include` "soc/soc_caps.h" at the top of wled_boards.h (near the existing includes like "NodeStruct.h") so the dependency on SOC_CPU_CORES_NUM is visible and robust across build configurations; ensure the include is guarded by the same target checks if desired (e.g., only relevant when CONFIG_IDF_TARGET_ESP32xx) to avoid accidental platform leakage.wled00/bus_wrapper.h (1)
236-236: Prefer the capability macro for the remaining I2S typedef guard.Since this file now includes
wled_boards.h, this target exclusion list can useWLED_HAS_PARALLEL_I2Stoo, avoiding another place to update when board capability coverage changes.♻️ Proposed cleanup
-#elif !defined(CONFIG_IDF_TARGET_ESP32C3) && !defined(CONFIG_IDF_TARGET_ESP32C5) && !defined(CONFIG_IDF_TARGET_ESP32C6) && !defined(CONFIG_IDF_TARGET_ESP32C61) && !defined(CONFIG_IDF_TARGET_ESP32P4) +#elif defined(WLED_HAS_PARALLEL_I2S)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/bus_wrapper.h` at line 236, Replace the long target-exclusion preprocessor condition used as the I2S typedef guard with the capability macro: detect and remove the current `#elif` condition that lists CONFIG_IDF_TARGET_ESP32C3/ESP32C5/ESP32C6/ESP32C61/ESP32P4 and instead use a check against WLED_HAS_PARALLEL_I2S (negated if needed) so the guard becomes driven by the board capability macro defined via wled_boards.h; update the conditional surrounding the typedef in bus_wrapper.h to use WLED_HAS_PARALLEL_I2S and ensure the include order still brings in wled_boards.h before the guard.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@wled00/const.h`:
- Around line 538-559: The MAX_LEDS macro allows 4096 for
CONFIG_IDF_TARGET_ESP32C5/ESP32C61 but MAX_LED_MEMORY for those targets without
PSRAM is only 28KB, so reduce MAX_LEDS when BOARD_HAS_PSRAM is not defined to
match the non-PSRAM memory cap to avoid FX_fcn.cpp falling back to a placeholder
bus; update the MAX_LEDS definition block (the `#elif` branch that checks
CONFIG_IDF_TARGET_ESP32C5 and CONFIG_IDF_TARGET_ESP32C61) to conditionally
define a lower MAX_LEDS when BOARD_HAS_PSRAM is not defined and keep the larger
value when BOARD_HAS_PSRAM is present, ensuring the MAX_LEDS and MAX_LED_MEMORY
constraints are consistent for C5/C61 targets.
- Around line 674-680: The DEFAULT_LED_PIN selection block should avoid using
GPIO2 for ESP32-C5 because GPIO2 is a bootstrap strapping pin; update the
preprocessor logic so that when CONFIG_IDF_TARGET_ESP32C5 is defined (or the
combined condition that includes it) the DEFAULT_LED_PIN is set to 1 instead of
2; modify the conditional around DEFAULT_LED_PIN (the `#if` defined(...) / `#ifdef`
WLED_ENABLE_DMX block that defines DEFAULT_LED_PIN) to special-case
CONFIG_IDF_TARGET_ESP32C5 (keeping the existing DMX override behavior) so the
symbol DEFAULT_LED_PIN resolves to 1 for ESP32-C5 while leaving other targets
unchanged.
In `@wled00/pin_manager.cpp`:
- Around line 403-428: The top comment "Check ADC capability: only ADC1 channels
can be used (ADC2 channels are not usable when WiFi is active)" is stale; update
it to specify that the ADC2/WiFi hard block applies only to classic ESP32
(CONFIG_IDF_TARGET_ESP32) and that the other branches
(CONFIG_IDF_TARGET_ESP32S2, CONFIG_IDF_TARGET_ESP32S3 and the generic path using
SOC_ADC_CHANNEL_NUM/SOC_ADC_PERIPH_NUM and digitalPinToAnalogChannel)
intentionally accept ADC2 where supported for analogRead(). Locate the comment
and the surrounding checks (digitalPinToAnalogChannel, SOC_ADC_CHANNEL_NUM,
SOC_ADC_PERIPH_NUM) in pin_manager.cpp and replace the generic warning with a
targeted note about classic ESP32 only.
---
Duplicate comments:
In `@wled00/data/settings_wifi.htm`:
- Around line 284-287: The TX power dropdown in settings_wifi.htm exposes values
84/82/80 which are clamped to 78 in set.cpp causing UI/persist divergence;
update the TX <select name="TX"> options to match supported builds by either
removing or gating 84/82/80 behind the V5-only build flag (the same compile-time
or template flag used to select the V5 framework) or replace those option values
with the effective clamped value (78) so the UI matches set.cpp behavior;
reference the TX select in settings_wifi.htm and the clamping logic in set.cpp
to implement the guard or value alignment.
---
Nitpick comments:
In `@wled00/bus_wrapper.h`:
- Line 236: Replace the long target-exclusion preprocessor condition used as the
I2S typedef guard with the capability macro: detect and remove the current `#elif`
condition that lists CONFIG_IDF_TARGET_ESP32C3/ESP32C5/ESP32C6/ESP32C61/ESP32P4
and instead use a check against WLED_HAS_PARALLEL_I2S (negated if needed) so the
guard becomes driven by the board capability macro defined via wled_boards.h;
update the conditional surrounding the typedef in bus_wrapper.h to use
WLED_HAS_PARALLEL_I2S and ensure the include order still brings in wled_boards.h
before the guard.
In `@wled00/wled_boards.h`:
- Around line 44-45: Rename the macro WLED_HAVE_FAST_int_DIVIDE to
WLED_HAVE_FAST_INT_DIVIDE everywhere: update the four definitions in
wled_boards.h and all uses (preprocessor checks and conditionals) in FX_fcn.cpp
and FXparticleSystem.cpp, changing occurrences like `#if`
!defined(WLED_HAVE_FAST_int_DIVIDE) (and any direct uses) to `#if`
!defined(WLED_HAVE_FAST_INT_DIVIDE) (and use the new name in `#define` and
`#ifdef/`#ifndef checks) so the macro follows the UPPER_CASE convention
consistently across the codebase.
- Around line 9-10: wled_boards.h uses SOC_CPU_CORES_NUM but does not explicitly
include the header that defines it; add an explicit `#include` "soc/soc_caps.h" at
the top of wled_boards.h (near the existing includes like "NodeStruct.h") so the
dependency on SOC_CPU_CORES_NUM is visible and robust across build
configurations; ensure the include is guarded by the same target checks if
desired (e.g., only relevant when CONFIG_IDF_TARGET_ESP32xx) to avoid accidental
platform leakage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2f2256f6-01e0-49d4-a985-29de3f912cfb
📒 Files selected for processing (31)
platformio.iniusermods/audioreactive/audio_reactive.cppusermods/audioreactive/audio_source.hwled00/FX.hwled00/FX_fcn.cppwled00/FXparticleSystem.cppwled00/bus_manager.cppwled00/bus_wrapper.hwled00/button.cppwled00/cfg.cppwled00/const.hwled00/data/index.jswled00/data/settings_wifi.htmwled00/dmx_output.cppwled00/fcn_declare.hwled00/improv.cppwled00/json.cppwled00/network.cppwled00/ota_update.cppwled00/pin_manager.cppwled00/set.cppwled00/src/dependencies/dmx/ESPDMX.cppwled00/src/dependencies/dmx/SparkFunDMX.cppwled00/udp.cppwled00/util.cppwled00/wled.cppwled00/wled.hwled00/wled_boards.hwled00/wled_metadata.cppwled00/ws.cppwled00/xml.cpp
✅ Files skipped from review due to trivial changes (2)
- wled00/ota_update.cpp
- wled00/src/dependencies/dmx/ESPDMX.cpp
🚧 Files skipped from review as they are similar to previous changes (13)
- wled00/improv.cpp
- wled00/dmx_output.cpp
- wled00/src/dependencies/dmx/SparkFunDMX.cpp
- wled00/button.cpp
- wled00/bus_manager.cpp
- wled00/FXparticleSystem.cpp
- wled00/util.cpp
- wled00/cfg.cpp
- wled00/data/index.js
- wled00/json.cpp
- usermods/audioreactive/audio_reactive.cpp
- wled00/FX_fcn.cpp
- platformio.ini
|
@willmmiles @softhack007 @DedeHai - Are we good to merge this into V5 then or are there any bits we want to clean up about this branch before it gets merged in with the mega V5 changeset? |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@usermods/audioreactive/audio_reactive.cpp`:
- Around line 469-471: In the integer FFT path around the FFT_Magnitude
assignment, the scaled value computed on line 469 (FFT_Magnitude =
FFT_Magnitude_int * 512) is immediately overwritten by line 471 (FFT_Magnitude =
FFT_Magnitude_int), negating the scaling operation. Remove the redundant
assignment on line 471 that overwrites the scaled FFT_Magnitude value, keeping
only the scaled assignment from line 469 and the FFT_MajorPeak_int assignment.
In `@wled00/data/index.js`:
- Around line 3547-3551: The catch handler for the upgrade report call is
invoking updateVersionInfo(info.ver, false, !!alwaysReport) even when the report
fails, which marks the version as updated and prevents retries after transient
failures. This is inconsistent with how non-OK responses are handled in the
success path. Remove the updateVersionInfo call from the catch block so that
failed reports are not marked as complete and can be retried on subsequent
attempts.
In `@wled00/FX_fcn.cpp`:
- Around line 1311-1313: The BFRALLOC_ENFORCE_PSRAM flag in the allocate_buffer
call for _pixels allocation is causing the pointer to remain null on non-PSRAM
targets, effectively disabling rendering. Remove the BFRALLOC_ENFORCE_PSRAM flag
from the allocate_buffer call while keeping the other flags
(BFRALLOC_NOBYTEACCESS and BFRALLOC_CLEAR), allowing the allocation to use PSRAM
when available but gracefully fall back to regular DRAM on non-PSRAM targets
instead of enforcing PSRAM only.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 83f345e7-7017-4b71-a664-ca09b9222106
📒 Files selected for processing (30)
platformio.iniusermods/audioreactive/audio_reactive.cppusermods/audioreactive/audio_source.hwled00/FX.hwled00/FX_fcn.cppwled00/FXparticleSystem.cppwled00/bus_manager.cppwled00/bus_wrapper.hwled00/button.cppwled00/cfg.cppwled00/const.hwled00/data/index.jswled00/data/settings_wifi.htmwled00/dmx_output.cppwled00/fcn_declare.hwled00/improv.cppwled00/json.cppwled00/network.cppwled00/ota_update.cppwled00/pin_manager.cppwled00/set.cppwled00/src/dependencies/dmx/ESPDMX.cppwled00/src/dependencies/dmx/SparkFunDMX.cppwled00/udp.cppwled00/util.cppwled00/wled.cppwled00/wled.hwled00/wled_boards.hwled00/wled_metadata.cppwled00/xml.cpp
💤 Files with no reviewable changes (15)
- wled00/ota_update.cpp
- wled00/improv.cpp
- wled00/wled_metadata.cpp
- wled00/src/dependencies/dmx/ESPDMX.cpp
- wled00/xml.cpp
- wled00/network.cpp
- wled00/src/dependencies/dmx/SparkFunDMX.cpp
- wled00/udp.cpp
- wled00/wled_boards.h
- wled00/pin_manager.cpp
- wled00/set.cpp
- wled00/json.cpp
- wled00/wled.h
- wled00/wled.cpp
- wled00/util.cpp
🚧 Files skipped from review as they are similar to previous changes (2)
- wled00/FX.h
- wled00/dmx_output.cpp
|
I did not yet really look into much V5 stuff, so dont wait for me. I can help with post merge cleanup and testing once its done. |
|
I'd like to do a short scan for remaining experimental stuff, and missing "ToDO" comments. |
|
@coderabbitai full review |
This comment was marked as resolved.
This comment was marked as resolved.
sprintf can overwrite past the end of buffer.
|
@coderabbitai please look for pending ToDo's and open ends listed in the PR description or in comments. Check if there is a matching "// toDo: " style comment in the source code for each item that is really still unsolved in the code. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="wled/WLED"
pr="5048"
echo "=== PR metadata/body (first 240 lines) ==="
gh pr view "$pr" --repo "$repo" --json title,body,url,author,headRefName,baseRefName \
--jq '"URL: \(.url)\nTitle: \(.title)\nAuthor: \(.author.login)\nHead: \(.headRefName) -> \(.baseRefName)\n\nBODY:\n\(.body)"' | sed -n '1,240p'
echo
echo "=== PR comments mentioning TODO/open/pending/unsupported/disabled/C5/C6/P4/LEDC/NeoPixelBus/NTP/OTA/MAC/DMX/AudioReactive/Usermod ==="
gh pr view "$pr" --repo "$repo" --comments --json comments \
--jq '.comments[] | select((.body|test("(?i)(todo|to-do|open point|open end|pending|unsupported|disabled|C5|C6|P4|LEDC|NeoPixelBus|NTP|OTA|MAC|DMX|AudioReactive|Usermod)"))) | "COMMENT_URL: \(.url)\nAUTHOR: \(.author.login)\nDATE: \(.createdAt)\nBODY:\n\(.body)\n---"' \
| sed -n '1,360p'
echo
echo "=== Source TODO / ToDO / toDo / open-end markers (excluding generated html_*.h where possible) ==="
rg -n --hidden -S -C 1 \
-g '!wled00/html_*.h' \
-g '!**/.git/**' \
'(?i)\bto\s*do\b|TODO|ToDO|toDo|open end|open point|temporarily|temporary|not supported|unsupported|disabled until|remove once|add back|fix once|fix .*later|pending' \
platformio.ini wled00 usermods pio-scripts .vscode 2>/dev/null \
| sed -n '1,500p'
echo
echo "=== Focused source snippets for known C5/C6/P4 open points ==="
patterns=(
'ESP32-C6 update not supported|C6 update|BOOTLOADER_OFFSET|esp_read_mac|esp_wifi_remote_get_mac|ESP_MAC_WIFI_STA|ESP32P4|CONFIG_IDF_TARGET_ESP32P4'
'CONFIG_IDF_TARGET_ESP32C5|ntpEnabled|AsyncDNS|dns_gethostbyname|tcpip_callback|Required to lock TCPIP'
'NeoPixelBus|jonny190|wled-c5-stable|WLED_MAX_DIGITAL_CHANNELS|WLED_MAX_RMT_CHANNELS'
'LEDC\.channel_group|ledc_set_duty_and_update|LEDC_USE_APB_CLK|LEDC_USE_PLL_DIV_CLK|SOC_LEDC'
'AudioReactive|audioreactive|WLED_DISABLE_AUDIO|analogmic'
'WLED_ENABLE_DMX|esp_dmx|dmx_input|SparkFunDMX|ESPDMX'
'serializeNetworks|node\["channel"\]|settings_leds|appendGPIOinfo|chip'
)
for p in "${patterns[@]}"; do
echo
echo "--- pattern: $p ---"
rg -n -S -C 3 "$p" platformio.ini wled00 usermods 2>/dev/null | sed -n '1,160p'
done
echo
echo "=== Compact list of files with TODO-style markers ==="
rg -n --hidden -S \
-g '!wled00/html_*.h' \
-g '!**/.git/**' \
'(?i)\bto\s*do\b|TODO|ToDO|toDo|open end|open point|temporarily|disabled until|remove once|add back|fix once' \
platformio.ini wled00 usermods pio-scripts .vscode 2>/dev/null \
| awk -F: '{print $1}' | sort | uniq -c | sort -nrLength of output: 138247 The PR description image shows VS Code with the PlatformIO task tree open and Upload and Monitor selected, specifically for esp32c6dev_4MB.
Open points with adequate source markers
Items that need clarification or a missing source TODO
Additional source TODOs found that are relevant to this PR/V5 cleanupThese are not all listed in the PR open-points summary, but they look worth triaging before merge:
Suggested minimal cleanup before merge
|
due to lack of "native" on-chip wifi radio
|
@coderabbitai please go through all changed after commit 01076c8, and check if any change still requires adjustments for building with esp-idf V5, or for new features of this branch - including but not limited to WLEDNetwork, wled_boards.h, missing #ifdefs for new MCUs. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
BASE="01076c82378d245ae4f7d339d8fd65211626bd31"
echo "=== Repository / branch context ==="
git rev-parse --show-toplevel
git status --short --branch
echo "HEAD: $(git rev-parse HEAD)"
echo "BASE: $BASE"
git cat-file -e "$BASE^{commit}" && echo "Base commit exists"
echo
echo "=== Diff stat after base commit ==="
git diff --stat "$BASE"..HEAD | sed -n '1,220p'
echo
echo "=== Changed files after base commit ==="
git diff --name-status "$BASE"..HEAD | sed -n '1,260p'
echo
echo "=== Changed files grouped by extension / area ==="
git diff --name-only "$BASE"..HEAD | python3 - <<'PY'
import sys, collections, os
files=[l.strip() for l in sys.stdin if l.strip()]
areas=collections.Counter()
exts=collections.Counter()
for f in files:
top=f.split('/')[0]
areas[top]+=1
ext=os.path.splitext(f)[1] or '<none>'
exts[ext]+=1
print("Areas:")
for k,v in areas.most_common():
print(f" {k}: {v}")
print("Extensions:")
for k,v in exts.most_common():
print(f" {k}: {v}")
PY
echo
echo "=== Focus files if changed ==="
for f in \
"wled00/WLEDNetwork.h" \
"wled00/WLEDNetwork.cpp" \
"wled00/network.cpp" \
"wled00/net_debug.cpp" \
"wled00/wled_boards.h" \
"wled00/wled.h" \
"wled00/wled.cpp" \
"wled00/const.h" \
"wled00/bus_manager.cpp" \
"wled00/bus_wrapper.h" \
"wled00/json.cpp" \
"wled00/pin_manager.cpp" \
"wled00/ota_update.cpp" \
"wled00/set.cpp" \
"wled00/cfg.cpp" \
"platformio.ini"
do
if git diff --name-only "$BASE"..HEAD -- "$f" | grep -q .; then
echo "--- CHANGED: $f ---"
git diff --unified=80 "$BASE"..HEAD -- "$f" | sed -n '1,260p'
fi
done
echo
echo "=== All changed hunks mentioning ESP-IDF V5 / new MCUs / capability macros / networking ==="
git diff --unified=8 "$BASE"..HEAD -- \
':!wled00/html_*.h' \
| rg -n -i -C 4 \
'ESP_IDF|IDF_VERSION|CONFIG_IDF_TARGET_ESP32(C5|C6|C61|P4|S2|S3|C3)|ARDUINO_ARCH_ESP32|SOC_|WLED_BOARD|wled_boards|WLEDNetwork|Network|Ethernet|ETH|WiFi|OTA|MAC|LEDC|RMT|I2S|PARLIO|TOUCH|DMX|ESPNOW|ESP-NOW|AudioReactive|audioreactive|NeoPixelBus|NeoEsp32RmtHI|TODO|ToDO|unsupported|temporar|disable' \
| sed -n '1,1000p'
echo
echo "=== Target guard inventory in changed files ==="
changed=$(git diff --name-only "$BASE"..HEAD | tr '\n' ' ')
if [ -n "$changed" ]; then
rg -n -S \
'CONFIG_IDF_TARGET_ESP32(C3|C5|C6|C61|P4|S2|S3)|CONFIG_IDF_TARGET_ESP32\b|ARDUINO_ARCH_ESP32|ESP8266|ESP_IDF_VERSION|SOC_[A-Z0-9_]+' \
$changed \
2>/dev/null | sed -n '1,900p'
fi
echo
echo "=== Potential missing-new-MCU guards: changed conditionals that mention old/new ESP32 subsets ==="
python3 - <<'PY'
import subprocess, re, sys
base="01076c82378d245ae4f7d339d8fd65211626bd31"
files=subprocess.check_output(["git","diff","--name-only",base+"..HEAD"], text=True).splitlines()
targets=["ESP32C3","ESP32C5","ESP32C6","ESP32C61","ESP32P4","ESP32S2","ESP32S3"]
pat=re.compile(r'^\s*#\s*(if|ifdef|ifndef|elif)\b.*(CONFIG_IDF_TARGET|ARDUINO_ARCH_ESP32|ESP32C|ESP32S|ESP32P|ESP8266|ESP_IDF_VERSION|SOC_)')
for f in files:
if not (f.endswith((".h",".hpp",".cpp",".c",".ino",".ini",".S",".js",".htm",".html"))):
continue
try:
txt=open(f, encoding="utf-8", errors="ignore").read().splitlines()
except FileNotFoundError:
continue
hits=[]
for i,l in enumerate(txt,1):
if pat.search(l):
mentioned=[t for t in targets if t in l]
# flag selective target conditionals as worth human inspection
if mentioned or "CONFIG_IDF_TARGET_ESP32" in l or "ARDUINO_ARCH_ESP32" in l:
hits.append((i,l.rstrip()))
if hits:
print(f"--- {f} ---")
for i,l in hits[:80]:
print(f"{i}: {l}")
if len(hits)>80:
print(f"... {len(hits)-80} more")
PY
echo
echo "=== ESP-IDF V5 API risk scan in changed files ==="
# Direct register access, deprecated APIs, old ADC/DAC/LEDC/RMT/ETH patterns, raw lwIP, old WiFi APIs.
if [ -n "$changed" ]; then
rg -n -S -C 3 \
'LEDC\.|ledc_channel_config_t|ledc_timer_config_t|ledc_set_duty|ledc_update_duty|LEDC_USE_APB_CLK|LEDC_HIGH_SPEED_MODE|rmt_|RMT_CHANNEL|i2s_|I2S[01]|periph_module|adc1_|adc2_|dac_|tcpip_|dns_gethostbyname|udp_new|LOCK_TCPIP|flush\(\)|esp_read_mac|ESP_MAC_WIFI_STA|ESP_MAC_ETH|esp_wifi_remote_get_mac|ETH\.|WiFi\.|NetworkUDP|AsyncDNS|ESP_ERROR_CHECK|esp_efuse|esp_chip_info' \
$changed 2>/dev/null | sed -n '1,1000p'
fi
echo
echo "=== WLEDNetwork symbols and usages in repository ==="
rg -n -S -C 3 \
'class WLEDNetwork|WLEDNetwork::|NetworkClass|NetworkUDP|WLEDNetwork|Network\.|network\.' \
wled00 \
2>/dev/null | sed -n '1,900p'
echo
echo "=== wled_boards.h full content if present ==="
if [ -f wled00/wled_boards.h ]; then
nl -ba wled00/wled_boards.h | sed -n '1,260p'
fi
echo
echo "=== TODO/open markers added or touched after base commit ==="
git diff --unified=8 "$BASE"..HEAD -- ':!wled00/html_*.h' \
| rg -n -i -C 4 '^\+.*(TODO|ToDO|toDo|open end|open point|temporar|unsupported|not supported|disabled|fix later|remove once|add back|not yet tested|validate|pending)' \
| sed -n '1,700p'
echo
echo "=== Compile-guard sanity: includes of ESP-IDF headers in changed files ==="
if [ -n "$changed" ]; then
rg -n -S -C 3 \
'#\s*include\s*[<"](esp_|driver/|soc/|hal/|lwip/|freertos/|esp_wifi_remote|ESP_Hosted|Network|WiFi|ETH|Update|esp_ota)' \
$changed 2>/dev/null | sed -n '1,700p'
fi
echo
echo "=== Files changed after base with full line counts ==="
for f in $(git diff --name-only "$BASE"..HEAD); do
if [ -f "$f" ]; then
printf "%7s %s\n" "$(wc -l < "$f")" "$f"
else
echo "deleted $f"
fi
done | sed -n '1,260p'Length of output: 15659 🏁 Script executed: #!/bin/bash
set -u
BASE="01076c82378d245ae4f7d339d8fd65211626bd31"
CHANGED_FILE_LIST="/tmp/wled_changed_after_base.txt"
git diff --name-only "$BASE"..HEAD > "$CHANGED_FILE_LIST"
echo "=== Context ==="
echo "HEAD: $(git rev-parse HEAD)"
echo "BASE: $BASE"
echo "Changed files: $(wc -l < "$CHANGED_FILE_LIST")"
echo
echo "=== Changed files grouped by area and extension ==="
python3 - "$CHANGED_FILE_LIST" <<'PY'
import sys, collections, os
path=sys.argv[1]
files=[l.strip() for l in open(path, encoding="utf-8") if l.strip()]
areas=collections.Counter()
exts=collections.Counter()
for f in files:
areas[f.split('/')[0]] += 1
exts[os.path.splitext(f)[1] or '<none>'] += 1
print("Areas:")
for k,v in areas.most_common():
print(f" {k}: {v}")
print("Extensions:")
for k,v in exts.most_common():
print(f" {k}: {v}")
PY
echo
echo "=== Focused diffs for core / V5 / networking / MCU files ==="
while IFS= read -r f; do
case "$f" in
platformio.ini|wled00/network.cpp|wled00/net_debug.cpp|wled00/WLEDNetwork.*|wled00/wled_boards.h|wled00/wled.h|wled00/wled.cpp|wled00/const.h|wled00/bus_manager.cpp|wled00/bus_manager.h|wled00/bus_wrapper.h|wled00/json.cpp|wled00/pin_manager.cpp|wled00/pin_manager.h|wled00/ota_update.cpp|wled00/ota_update.h|wled00/set.cpp|wled00/cfg.cpp|wled00/e131.cpp|wled00/src/dependencies/e131/ESPAsyncE131.cpp|wled00/src/dependencies/e131/ESPAsyncE131.h|wled00/improv.cpp|wled00/remote.cpp|wled00/util.cpp|wled00/fcn_declare.h|wled00/dmx_input.cpp|wled00/dmx_input.h)
echo
echo "--- DIFF: $f ---"
git diff --unified=45 "$BASE"..HEAD -- "$f" | sed -n '1,360p'
;;
esac
done < "$CHANGED_FILE_LIST"
echo
echo "=== Added/changed lines mentioning ESP-IDF V5, new MCUs, capabilities, or networking ==="
git diff --unified=6 "$BASE"..HEAD -- ':!wled00/html_*.h' \
| rg -n -i -C 3 \
'^[+ -].*(ESP_IDF|IDF_VERSION|CONFIG_IDF_TARGET_ESP32(C5|C6|C61|P4|S2|S3|C3)|ARDUINO_ARCH_ESP32|SOC_|WLED_BOARD|wled_boards|WLEDNetwork|NetworkUDP|NetworkClient|NetworkServer|WLEDNetwork|Ethernet|ETH\.|WiFi\.|OTA|MAC|LEDC|RMT|I2S|PARLIO|TOUCH|DMX|ESPNOW|ESP-NOW|AudioReactive|audioreactive|NeoPixelBus|NeoEsp32RmtHI|TODO|ToDO|unsupported|temporar|disable)' \
| sed -n '1,1200p' || true
echo
echo "=== Target guard inventory in changed source/config files ==="
while IFS= read -r f; do
[ -f "$f" ] || continue
case "$f" in
*.h|*.hpp|*.cpp|*.c|*.ino|*.S|*.ini|*.htm|*.html|*.js)
rg -n -S \
'CONFIG_IDF_TARGET_ESP32(C3|C5|C6|C61|P4|S2|S3)|CONFIG_IDF_TARGET_ESP32\b|ARDUINO_ARCH_ESP32|ESP8266|ESP_IDF_VERSION|SOC_[A-Z0-9_]+' \
"$f" 2>/dev/null || true
;;
esac
done < "$CHANGED_FILE_LIST" | sed -n '1,1200p'
echo
echo "=== Selective target conditionals requiring human inspection ==="
python3 - "$CHANGED_FILE_LIST" <<'PY'
import sys, re, os
files=[l.strip() for l in open(sys.argv[1], encoding="utf-8") if l.strip()]
pat=re.compile(r'^\s*#\s*(if|ifdef|ifndef|elif)\b.*(CONFIG_IDF_TARGET|ARDUINO_ARCH_ESP32|ESP32C|ESP32S|ESP32P|ESP8266|ESP_IDF_VERSION|SOC_)')
for f in files:
if not os.path.exists(f) or not f.endswith((".h",".hpp",".cpp",".c",".ino",".ini",".S",".js",".htm",".html")):
continue
hits=[]
for i,l in enumerate(open(f, encoding="utf-8", errors="ignore"),1):
if pat.search(l):
hits.append((i,l.rstrip()))
if hits:
print(f"--- {f} ---")
for i,l in hits[:120]:
print(f"{i}: {l}")
if len(hits)>120:
print(f"... {len(hits)-120} more")
PY
echo
echo "=== ESP-IDF V5 API risk scan in changed files ==="
while IFS= read -r f; do
[ -f "$f" ] || continue
case "$f" in
*.h|*.hpp|*.cpp|*.c|*.ino|*.S)
rg -n -S -C 3 \
'LEDC\.|ledc_channel_config_t|ledc_timer_config_t|ledc_set_duty|ledc_update_duty|LEDC_USE_APB_CLK|LEDC_HIGH_SPEED_MODE|rmt_|RMT_CHANNEL|i2s_|I2S[01]|periph_module|PERIPH_|adc1_|adc2_|dac_|tcpip_|dns_gethostbyname|udp_new|LOCK_TCPIP|flush\(\)|esp_read_mac|ESP_MAC_WIFI_STA|ESP_MAC_ETH|esp_wifi_remote_get_mac|ETH\.|WiFi\.|NetworkUDP|AsyncDNS|ESP_ERROR_CHECK|esp_efuse|esp_chip_info|esp_ota|Update\.|esp_app_desc|esp_partition' \
"$f" 2>/dev/null || true
;;
esac
done < "$CHANGED_FILE_LIST" | sed -n '1,1400p'
echo
echo "=== WLEDNetwork / Network compatibility symbols and usages ==="
rg -n -S -C 3 \
'class WLEDNetwork|WLEDNetwork::|WLEDNetwork|NetworkClass|NetworkUDP|NetworkClient|NetworkServer|Network\.|network\.|Ethernet|ETH\.|WiFi\.' \
wled00 \
2>/dev/null | sed -n '1,1200p' || true
echo
echo "=== wled_boards.h content (for current HEAD) ==="
if [ -f wled00/wled_boards.h ]; then
nl -ba wled00/wled_boards.h | sed -n '1,280p'
else
echo "wled00/wled_boards.h not present"
fi
echo
echo "=== TODO/open markers added or touched after base commit ==="
git diff --unified=6 "$BASE"..HEAD -- ':!wled00/html_*.h' \
| rg -n -i -C 3 '^\+.*(TODO|ToDO|toDo|open end|open point|temporar|unsupported|not supported|disabled|fix later|remove once|add back|not yet tested|validate|pending)' \
| sed -n '1,1000p' || true
echo
echo "=== ESP-IDF/framework includes in changed source files ==="
while IFS= read -r f; do
[ -f "$f" ] || continue
case "$f" in
*.h|*.hpp|*.cpp|*.c|*.ino|*.S)
rg -n -S -C 3 \
'#\s*include\s*[<"](esp_|driver/|soc/|hal/|lwip/|freertos/|esp_wifi_remote|ESP_Hosted|Network|WiFi|ETH|Update|esp_ota|AsyncUDP)' \
"$f" 2>/dev/null || true
;;
esac
done < "$CHANGED_FILE_LIST" | sed -n '1,1000p'
echo
echo "=== Changed files with current line counts ==="
while IFS= read -r f; do
if [ -f "$f" ]; then
printf "%7s %s\n" "$(wc -l < "$f")" "$f"
else
echo "deleted $f"
fi
done < "$CHANGED_FILE_LIST" | sed -n '1,260p'Length of output: 271090 🏁 Script executed: #!/bin/bash
set -u
BASE="01076c82378d245ae4f7d339d8fd65211626bd31"
echo "=== Current line snippets for likely findings ==="
echo
echo "--- bus_manager.cpp LEDC V5 target conditional ---"
nl -ba wled00/bus_manager.cpp | sed -n '560,620p'
echo
echo "--- ESPAsyncE131.cpp parsePacket length checks ---"
nl -ba wled00/src/dependencies/e131/ESPAsyncE131.cpp | sed -n '70,135p'
echo
echo "--- e131.cpp DDP packet bounds / time flag ---"
nl -ba wled00/e131.cpp | sed -n '1,115p'
echo
echo "--- e131.cpp Art-Net / E1.31 packetLen clamps ---"
nl -ba wled00/e131.cpp | sed -n '115,205p'
echo
echo "--- bus_wrapper.h first bytes / first lines ---"
python3 - <<'PY'
from pathlib import Path
p=Path("wled00/bus_wrapper.h")
b=p.read_bytes()[:12]
print("first bytes:", b)
for i,line in enumerate(p.read_text(encoding="utf-8-sig", errors="replace").splitlines()[:5],1):
print(f"{i}: {line!r}")
PY
echo
echo "--- Changed WLEDNetwork references after base ---"
git diff --unified=8 "$BASE"..HEAD -- ':!wled00/html_*.h' \
| rg -n -i -C 4 'WLEDNetwork|NetworkUDP|NetworkClient|NetworkServer|../network/Network|network/Network|WLEDNetwork\.' || true
echo
echo "--- Current WLEDNetwork/Network usages in core hot paths ---"
rg -n -S -C 2 'WLEDNetwork\.|NetworkUDP|NetworkClient|NetworkServer|`#include` "../network/Network.h"|`#include` ".*Network' wled00 \
| sed -n '1,220p'
echo
echo "--- Target-specific conditionals in current core files likely related to new MCUs ---"
for f in wled00/bus_manager.cpp wled00/bus_wrapper.h wled00/wled.cpp wled00/wled.h wled00/const.h wled00/util.cpp wled00/json.cpp wled00/remote.cpp wled00/improv.cpp wled00/ota_update.cpp; do
echo "--- $f ---"
rg -n -S 'CONFIG_IDF_TARGET_ESP32(C3|C5|C6|C61|P4|S2|S3)|ESP_IDF_VERSION|SOC_|WLED_HAS_|WLED_HAVE_|ESP_MAC_|esp_wifi_remote' "$f" || true
doneLength of output: 21375 The PR description image shows VS Code with the PLATFORMIO task tree open and Upload and Monitor selected, with the tooltip “Upload and Monitor (esp32c6dev_4MB)”.
Findings1. ESP32-C6 is missing from the LEDC V5 API pathIn `#if` defined(CONFIG_IDF_TARGET_ESP32C5) || defined(CONFIG_IDF_TARGET_ESP32C61) || defined(CONFIG_IDF_TARGET_ESP32P4)This omits Given the branch’s V5 LEDC notes and the existing avoidance of direct LEDC.channel_group[gr].channel[ch].duty.duty = ...
LEDC.channel_group[gr].channel[ch].hpoint.hpoint = ...Suggested adjustment: `#if` defined(CONFIG_IDF_TARGET_ESP32C5) || defined(CONFIG_IDF_TARGET_ESP32C6) || defined(CONFIG_IDF_TARGET_ESP32C61) || defined(CONFIG_IDF_TARGET_ESP32P4)This is the main V5/new-MCU compatibility issue I’d address before merge. 2. Short Art-Net UDP packets can still be read before length validation
memcmp(sbuff->art_id, ESPAsyncE131::ART_ID, sizeof(sbuff->art_id))
sbuff->art_opcodebefore proving the packet is at least 8/10 bytes long.
Suggested fix pattern: if (protocol == P_ARTNET) {
if (pktLen < 10) {
error = true;
} else {
if (memcmp(sbuff->art_id, ESPAsyncE131::ART_ID, sizeof(sbuff->art_id))) error = true;
if (sbuff->art_opcode != ARTNET_OPCODE_OPDMX && sbuff->art_opcode != ARTNET_OPCODE_OPPOLL) error = true;
}
}This is not V5-specific, but it is in the post-base changes and affects UDP ingress robustness. 3. DDP timecode offset is no longer included in bounds/pixel-count validationIn unsigned c = 0;
if (p->flags & DDP_FLAGS_TIME) c = 4;and correctly checks: if (packetLen < DDP_HEADER_LEN + c + dataLen) return;But the later bounds calculation was changed from including unsigned maxDataIndex = numLeds * ddpChannelsPerLed;while the loop still reads from For packets with Safer pattern: if (dataLen < c) return;
const unsigned payloadLen = dataLen - c;
unsigned stop = start + payloadLen / ddpChannelsPerLed;
unsigned numLeds = stop - start;
unsigned maxDataIndex = c + numLeds * ddpChannelsPerLed;
if (maxDataIndex > dataLen) return;This preserves the intended “bounds-only, don’t over-enforce DDP spec” behavior while making timecode packets safe. 4. Cleanup:
|
* Remove UTF-8 BOM chars from bus_wrapper.h * add ESP32-C6 to the LEDC V5 API path
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
OK, code seems ready for merging back into V5 👍 |
C6 needs the newer ESP-IDF, so have one branch for those changes and keep this one just for the C6 specific things
Building on top of V5
Inspired by C6 Experiments
Help Us with Testing
If you own a C6 or C5 board, we like to hear from your experiences! Basic functions are working on our dev boards, but any problem you find will help us stabilize the C5/C6 versions faster. If you find an error, please create an issue ticket (bug report).
Important: Our V5 Branch is Experimental (early stage of development)
You can find a summary of currently open ends here (open ends for C5, C6), here (modernization opportunities) and here (generic problems with V5).
Using Development Tools (VSCode) - recommended
You can build your own C5 or C6 firmware using our standard development environment (VSCode+platformio) on your own computer. Use Platformio "upload and monitor" to write firmware to your board.
Experimental Binaries
In case you are not feeling comfortable using development tools, you can find some firmware binaries that are automatically created by GitHub after each commit.
WLED CI / wled_build / Build Environments (esp32c6dev_4MB) (push)Help Us with Coding
Helpful information and migration guidance
coding guides
We should make sure that the "V5" branch will still compile for "V4" as a backup solution.
When adding code that only works in the new framework, it should be conditionally compiled, like in the examples below
related
Here's the comprehensive summary of open points for the C6/C5/P4 port, organized by WLED feature area:
📋 Open Points Summary for ESP32-C6/C5/P4 Support
🌐 Networking
NTP
OTA Updates
❌ ESP32-C6: Not supported yet (comment)
❌ ESP32-P4: MAC address retrieval fails (comment)
E (27) system_api: 0 mac type is incorrect (not found)esp_wifi_remote_get_mac()fromesp_wifi_remote_api.h(see esp-idf#16251)ESP-NOW
"ToDO: temporarily disabled, until we find a solution for esp-now build errors with -C6"MQTT
"TODO: remove once we have updated library for V5"💡 LED Drivers
NeoPixelBus
❌ ESP32-C5: Critical RMT channel bug (comment, analysis)
rmt_new_tx_channel()allocates channels without TX capability validation@jonny190to fix NeoPixelBus fork (lines 70-122 in NeoEsp32RmtXMethod.h)🔧 ESP32-C5: Custom fork maintenance (platformio.ini:412-418)
"TODO: remove the temporarily Override below once NeoPixelBus has official support for -C5"NeoEsp32RmtHI
"ToDO: check if NeoESP32RmtHI is still needed with V5 (see discussion in PR#4838)"🎛️ Hardware Features
Infrared (IR)
"TODO: remove once we have updated library for V5"/"TODO: add back"-D WLED_DISABLE_INFRAREDDMX
❌ ESP32-C5 Input: Not supported (platformio.ini:422)
🔧 V4 regression test: Compile errors (platformio.ini:675, 685-686)
"TODO: fix lots of compile errors in dmx_input.cpp"PWM/LEDC (Analog LEDs)
LEDC_USE_PLL_DIV_CLK(no APB clock)duty_r.duty_r, C5 duty range is now inclusive#4838🎨 Effects & Usermods
Audio Reactive
"TODO: add back audioreactive once V5 compatible"Usermods (General)
"TODO: disabled until the core is building"/"ToDO: fix usermods build once the main V5 build works without errors and warnings"🏗️ Build & Configuration
S2/S3 Special Flash Boards
"TODO: disabled NeoEsp32RmtMethodIsr"Chip Identification
🔧 Code Quality & Modernization
Deprecated APIs
esp_adc/adc_oneshot.handdriver/dac_oneshot.h'virtual void NetworkUDP::flush()' is deprecated: Use clear() insteadflush()withclear()Global Variable Declarations
Timezone* tz(ntp.cpp:14) — used in util.cpp, cfg.cppdecode_results results(ir.cpp:11) — used in wled.cpp, json.cpp, colors.cpp, FXparticleSystem.cppuint32_t lastValidCode(ir.cpp:13) — used in remote.cpp✅ Completed/Verified
📊 Priority Summary
Total open items: 20
This summary will be continuously updated as issues are resolved. Items marked ✅ are completed; 🔧 are in progress; ❌ are blockers;⚠️ are technical debt.
Summary by CodeRabbit