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)
Contributor
WalkthroughAdds ESP32-C5 and ESP32-C6 build environments and PlatformIO config; expands many CONFIG_IDF_TARGET_* guards across sources so C5/C6 (and P4) are treated like C3 for I2S/touch/PSRAM/DMX/pin-safety and other platform-specific logic. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
netmindz
commented
Nov 8, 2025
Comment on lines
+14
to
+28
| ;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 |
Member
Author
There was a problem hiding this comment.
TODO: revert before merge, here to save CI build time
Member
Author
|
@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 ....)
softhack007
reviewed
Nov 8, 2025
Member
@netmindz thanks, I'll try to help once I'm finished with some open ends in WLED-MM 😁 december |
Member
Author
|
Possibly also worth looking at #4626 to see if there are any changes there that are helpful |
1 task
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
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.
* use WLED_HAVE_RTC_MEMORY_HEAP * fix typo in comment
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as duplicate.
This comment was marked as duplicate.
show "not connected" instead of "unknown"
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
usermods/audioreactive/audio_reactive.cpp (1)
1377-1440:⚠️ Potential issue | 🟡 MinorMirror the new C5 analog/PDM exclusions in
audio_source.hline 8.The
.cppfile excludesCONFIG_IDF_TARGET_ESP32C5from analog mic (line 1424) and PDM (lines 1377, 1411) support. However,audio_source.hline 8 still includes the deprecated ADC headers for C5:`#if` !defined(CONFIG_IDF_TARGET_ESP32S2) && !defined(CONFIG_IDF_TARGET_ESP32S3) && !defined(CONFIG_IDF_TARGET_ESP32C3) `#include` <driver/adc_deprecated.h> `#include` <driver/adc_types_deprecated.h> `#endif`Add
&& !defined(CONFIG_IDF_TARGET_ESP32C5)to this guard to prevent the header mismatch.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@usermods/audioreactive/audio_reactive.cpp` around lines 1377 - 1440, The preprocessor guard in audio_source.h still allows including deprecated ADC headers for ESP32C5 even though audio_reactive.cpp excludes CONFIG_IDF_TARGET_ESP32C5 for ADC/PDM; update the `#if` that currently reads `#if` !defined(CONFIG_IDF_TARGET_ESP32S2) && !defined(CONFIG_IDF_TARGET_ESP32S3) && !defined(CONFIG_IDF_TARGET_ESP32C3) to also exclude CONFIG_IDF_TARGET_ESP32C5 so the includes for <driver/adc_deprecated.h> and <driver/adc_types_deprecated.h> are skipped on C5, matching the behavior in audio_reactive.cpp and the switch cases (functions/classes referenced: audioSource initialization, I2SAdcSource, I2SSource, and the CONFIG_IDF_TARGET_ESP32C5 macro).
🧹 Nitpick comments (6)
wled00/dmx_output.cpp (1)
72-75: This C5/C6/C61 branch is currently dead code.
wled00/wled.hstill hard-stops DMX builds for these targets before the DMX headers are included, so extending this condition does not affect current behavior. I’d either key this off the same centralized capability macro or leave only the actually supported targets here to keep the DMX support matrix in one place.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/dmx_output.cpp` around lines 72 - 75, The preprocessor branch around dmx.init/dmx.initWrite uses ESP32C3/C5/C6/C61 macros that are currently dead because the upstream DMX include is prevented for those targets; update the conditional to use the centralized DMX capability macro (the same one used in wled.h) or else remove the unsupported target macros and only keep the targets that actually enable DMX today; edit the block containing dmx.init and dmx.initWrite so it references that central capability macro (or the minimal set of supported target macros) to keep the DMX support matrix consistent (symbols to look for: dmx.init, dmx.initWrite and the existing preprocessor condition around them).usermods/audioreactive/audio_source.h (1)
25-32: If these MCUs are intentionally unsupported, fail the build instead of warning.On targets with
SOC_I2S_NUM >= 1, this still compiles the usermod after a#warning, even though the message says the MCU is unsupported. If the plan is to keep C5/C6/C61 disabled until the port is validated, switch the warning path to#erroror guard the usermod out earlier.⚙️ Small tightening
- `#warning` This audio reactive usermod does not support your MCU yet. + `#error` This audio reactive usermod does not support your MCU yet.Based on learnings: failing fast at compile time is preferred over allowing potentially broken builds until a full refactor is completed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@usermods/audioreactive/audio_source.h` around lines 25 - 32, The `#warning` path currently allows compilation for targets matching the MCU list when SOC_I2S_NUM >= 1 despite being unsupported; change the logic in audio_source.h so that the unsupported MCU branch fails the build instead of emitting a warning—replace the `#warning` with `#error` (or move the entire unsupported-MCU check earlier) so that targets matched by the conditionals (the block using CONFIG_IDF_TARGET_ESP32C2 / ESP32C5 / ESP32C6 / ESP32C61 / ESP32H2 / ESP8266 / ESP8265 and the SOC_I2S_NUM check) cause a compile-time error, preventing the usermod from being built on those MCUs until a validated port exists.wled00/pin_manager.cpp (2)
257-277: Keep the P4 connector mask separate from the MCU-wide validity rules.Most
isPinOk()branches only reject silicon-reserved pins, but this one also rejects pins just because they are not exposed on one specific board. Since the guard isCONFIG_IDF_TARGET_ESP32P4, any other P4 board definition will inherit those DevKit-specific exclusions too. A board-level mask/table would age better than hardcoding connector availability in the MCU path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/pin_manager.cpp` around lines 257 - 277, The CONFIG_IDF_TARGET_ESP32P4 block in isPinOk() mixes MCU-reserved pins with board-specific (DevKit P4 connector) exclusions (e.g., gpio==9, ranges >13&&<20, >23&&<26, >27&&<32, >33&&<36, >38&&<45, >48&&<53, gpio==54); extract those connector-specific checks out of the CONFIG_IDF_TARGET_ESP32P4 branch and implement a separate board-level connector mask/table (e.g., isPinOnP4Connector() or p4_connector_mask[]) that is applied only for the specific DevKit board build/config, leaving the MCU-wide silicon-reserved checks in the existing CONFIG_IDF_TARGET_ESP32P4 code path and calling the new board-level filter from isPinOk() where board definitions are handled.
415-427: Update the function comment to match the new ADC2 behavior.After this change, Line 403 is no longer true for ESP32-S3 and the generic ADC2-capable path, because both can now return
truefor ADC2 pins. Keeping the comment in sync will prevent the next cleanup from “fixing” an intentional behavior. Based on learnings: the ADC2/WiFi hard block applies only to classic ESP32; S2/S3 and newer oneshot paths are expected to stay enabled.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/pin_manager.cpp` around lines 415 - 427, Update the function comment above the ADC-capability check (near where adc_channel is computed via digitalPinToAnalogChannel) to reflect that ADC2 pins may now return true on ESP32-S3 and on the generic ADC2-capable path; state that the ADC2/WiFi hard block applies only to classic ESP32 (not S2/S3 or newer oneshot paths), and that checks using SOC_ADC_CHANNEL_NUM, SOC_ADC_PERIPH_NUM and SOC_ADC_MAX_CHANNEL_NUM can legitimately allow ADC2 channels to be treated as usable. Ensure the comment mentions the specific behavior for ESP32-S3 (channels 0-9) and the generic branch logic that permits ADC2 channels to return true so future cleanups don’t revert this intentional behavior.wled00/wled_boards.h (1)
18-25: Consider turning the placeholder capability TODOs into named macros now.This header is already the central capability map. Giving the RMT/DMX entries real names here would make it easier for new call sites to depend on a capability flag instead of spelling out target lists again.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/wled_boards.h` around lines 18 - 25, Add concrete capability macro definitions for the placeholder TODOs so callers can `#ifdef` on capability flags instead of repeating target lists: define macros like WLED_HAVE_TOUCH, WLED_HAVE_I2S0_LEDS, WLED_HAVE_I2S1_LEDS, WLED_ALLOW_LOLIN_WIFI_FIX, plus two descriptive RMT/DMX capability macros (e.g., WLED_RMT_LIMITED_CHANNELS and WLED_HAVE_SPARKFUN_DMX) in this central header; ensure each macro is conditionally set based on existing platform macros used elsewhere (so code in button.cpp, bus_wrapper.h, wled.h and dmx_output.cpp can simply `#ifdef` WLED_HAVE_TOUCH / WLED_HAVE_I2S0_LEDS / WLED_HAVE_I2S1_LEDS / WLED_RMT_LIMITED_CHANNELS / WLED_HAVE_SPARKFUN_DMX / WLED_ALLOW_LOLIN_WIFI_FIX).wled00/wled.cpp (1)
552-557: Consider extracting WiFi band-mode application into a small helper.The same
setBandMode + failure logpattern appears in three places with only mode/context differences.♻️ Proposed refactor
+#if defined(ARDUINO_ARCH_ESP32) && (ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 4, 2)) +static inline void applyWiFiBandMode(wifi_band_mode_t mode, const char* where) { + if (!WiFi.setBandMode(mode)) { + DEBUG_PRINTF_P(PSTR("%s: WiFi band configuration failed!\n"), where); + } +} +#endif- if (!WiFi.setBandMode(WIFI_BAND_MODE_AUTO)) { // WIFI_BAND_MODE_AUTO = 5GHz+2.4GHz; WIFI_BAND_MODE_5G_ONLY, WIFI_BAND_MODE_2G_ONLY - DEBUG_PRINTLN(F("setup(): Wifi band configuration failed!\n")); - } + applyWiFiBandMode(WIFI_BAND_MODE_AUTO, "setup()");- if (!WiFi.setBandMode(WIFI_BAND_MODE_AUTO)) { - DEBUG_PRINTLN(F("initAP(): Wifi band configuration failed!\n")); - } + applyWiFiBandMode(WIFI_BAND_MODE_AUTO, "initAP()");- if (!WiFi.setBandMode((wifi_band_mode_t)wifiBandMode)) { - DEBUG_PRINTLN(F("initConnection(): WiFi band configuration failed!")); - } + applyWiFiBandMode((wifi_band_mode_t)wifiBandMode, "initConnection()");Also applies to: 692-699, 787-791
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/wled.cpp` around lines 552 - 557, Extract the repeated pattern that calls WiFi.setBandMode(...) and logs failure into a small helper like applyWifiBandMode(mode, context) that calls WiFi.setBandMode(mode), returns the boolean result, and logs a contextual failure message with DEBUG_PRINTLN (include the provided context string in the log). Replace the three duplicated blocks that call WiFi.setBandMode(...) followed by DEBUG_PRINTLN(F("... Wifi band configuration failed!\n")) with calls to applyWifiBandMode(...) passing the appropriate WIFI_BAND_MODE_* value and a short context string (e.g., "setup", or the specific init step) so the same behavior is preserved and code is deduplicated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@platformio.ini`:
- Around line 32-33: The default_envs entry esp32p4_16MB is included without a
configured memory interface and must be removed from default_envs or explicitly
configured; either delete esp32p4_16MB from the default_envs list, or restore
and set board_build.arduino.memory_type = opi_hpi (and ensure the pinned
esp32-p4-evboard.json platform/package supports that memory_type) so CI builds
succeed for that target.
---
Outside diff comments:
In `@usermods/audioreactive/audio_reactive.cpp`:
- Around line 1377-1440: The preprocessor guard in audio_source.h still allows
including deprecated ADC headers for ESP32C5 even though audio_reactive.cpp
excludes CONFIG_IDF_TARGET_ESP32C5 for ADC/PDM; update the `#if` that currently
reads `#if` !defined(CONFIG_IDF_TARGET_ESP32S2) &&
!defined(CONFIG_IDF_TARGET_ESP32S3) && !defined(CONFIG_IDF_TARGET_ESP32C3) to
also exclude CONFIG_IDF_TARGET_ESP32C5 so the includes for
<driver/adc_deprecated.h> and <driver/adc_types_deprecated.h> are skipped on C5,
matching the behavior in audio_reactive.cpp and the switch cases
(functions/classes referenced: audioSource initialization, I2SAdcSource,
I2SSource, and the CONFIG_IDF_TARGET_ESP32C5 macro).
---
Nitpick comments:
In `@usermods/audioreactive/audio_source.h`:
- Around line 25-32: The `#warning` path currently allows compilation for targets
matching the MCU list when SOC_I2S_NUM >= 1 despite being unsupported; change
the logic in audio_source.h so that the unsupported MCU branch fails the build
instead of emitting a warning—replace the `#warning` with `#error` (or move the
entire unsupported-MCU check earlier) so that targets matched by the
conditionals (the block using CONFIG_IDF_TARGET_ESP32C2 / ESP32C5 / ESP32C6 /
ESP32C61 / ESP32H2 / ESP8266 / ESP8265 and the SOC_I2S_NUM check) cause a
compile-time error, preventing the usermod from being built on those MCUs until
a validated port exists.
In `@wled00/dmx_output.cpp`:
- Around line 72-75: The preprocessor branch around dmx.init/dmx.initWrite uses
ESP32C3/C5/C6/C61 macros that are currently dead because the upstream DMX
include is prevented for those targets; update the conditional to use the
centralized DMX capability macro (the same one used in wled.h) or else remove
the unsupported target macros and only keep the targets that actually enable DMX
today; edit the block containing dmx.init and dmx.initWrite so it references
that central capability macro (or the minimal set of supported target macros) to
keep the DMX support matrix consistent (symbols to look for: dmx.init,
dmx.initWrite and the existing preprocessor condition around them).
In `@wled00/pin_manager.cpp`:
- Around line 257-277: The CONFIG_IDF_TARGET_ESP32P4 block in isPinOk() mixes
MCU-reserved pins with board-specific (DevKit P4 connector) exclusions (e.g.,
gpio==9, ranges >13&&<20, >23&&<26, >27&&<32, >33&&<36, >38&&<45, >48&&<53,
gpio==54); extract those connector-specific checks out of the
CONFIG_IDF_TARGET_ESP32P4 branch and implement a separate board-level connector
mask/table (e.g., isPinOnP4Connector() or p4_connector_mask[]) that is applied
only for the specific DevKit board build/config, leaving the MCU-wide
silicon-reserved checks in the existing CONFIG_IDF_TARGET_ESP32P4 code path and
calling the new board-level filter from isPinOk() where board definitions are
handled.
- Around line 415-427: Update the function comment above the ADC-capability
check (near where adc_channel is computed via digitalPinToAnalogChannel) to
reflect that ADC2 pins may now return true on ESP32-S3 and on the generic
ADC2-capable path; state that the ADC2/WiFi hard block applies only to classic
ESP32 (not S2/S3 or newer oneshot paths), and that checks using
SOC_ADC_CHANNEL_NUM, SOC_ADC_PERIPH_NUM and SOC_ADC_MAX_CHANNEL_NUM can
legitimately allow ADC2 channels to be treated as usable. Ensure the comment
mentions the specific behavior for ESP32-S3 (channels 0-9) and the generic
branch logic that permits ADC2 channels to return true so future cleanups don’t
revert this intentional behavior.
In `@wled00/wled_boards.h`:
- Around line 18-25: Add concrete capability macro definitions for the
placeholder TODOs so callers can `#ifdef` on capability flags instead of repeating
target lists: define macros like WLED_HAVE_TOUCH, WLED_HAVE_I2S0_LEDS,
WLED_HAVE_I2S1_LEDS, WLED_ALLOW_LOLIN_WIFI_FIX, plus two descriptive RMT/DMX
capability macros (e.g., WLED_RMT_LIMITED_CHANNELS and WLED_HAVE_SPARKFUN_DMX)
in this central header; ensure each macro is conditionally set based on existing
platform macros used elsewhere (so code in button.cpp, bus_wrapper.h, wled.h and
dmx_output.cpp can simply `#ifdef` WLED_HAVE_TOUCH / WLED_HAVE_I2S0_LEDS /
WLED_HAVE_I2S1_LEDS / WLED_RMT_LIMITED_CHANNELS / WLED_HAVE_SPARKFUN_DMX /
WLED_ALLOW_LOLIN_WIFI_FIX).
In `@wled00/wled.cpp`:
- Around line 552-557: Extract the repeated pattern that calls
WiFi.setBandMode(...) and logs failure into a small helper like
applyWifiBandMode(mode, context) that calls WiFi.setBandMode(mode), returns the
boolean result, and logs a contextual failure message with DEBUG_PRINTLN
(include the provided context string in the log). Replace the three duplicated
blocks that call WiFi.setBandMode(...) followed by DEBUG_PRINTLN(F("... Wifi
band configuration failed!\n")) with calls to applyWifiBandMode(...) passing the
appropriate WIFI_BAND_MODE_* value and a short context string (e.g., "setup", or
the specific init step) so the same behavior is preserved and code is
deduplicated.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d67c3ad0-944a-4b0a-b652-4318d6e3cdb2
📒 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 (3)
- wled00/ota_update.cpp
- wled00/data/index.js
- wled00/src/dependencies/dmx/ESPDMX.cpp
🚧 Files skipped from review as they are similar to previous changes (13)
- wled00/improv.cpp
- wled00/src/dependencies/dmx/SparkFunDMX.cpp
- wled00/network.cpp
- wled00/FXparticleSystem.cpp
- wled00/button.cpp
- wled00/set.cpp
- wled00/FX_fcn.cpp
- wled00/util.cpp
- wled00/wled.h
- wled00/cfg.cpp
- wled00/json.cpp
- wled00/bus_wrapper.h
- wled00/const.h
* removed unnecessary arduino.memory_type * removed board_build.f_cpu (set in board manifest.json) * added example debug flags
Contributor
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 (1)
wled00/FX_fcn.cpp (1)
1166-1186:⚠️ Potential issue | 🟠 MajorUse assigned
iType(not requesteddriverType) for I2S DMA estimation.Line 1166-Line 1186 infer
useParallelI2Sfrom requesteddriverType, and Line 1205 treatsI_NONE(0) as I2S due to parity. This can overestimate DMA memory and trip the placeholder path at Line 1217 for configs that didn’t actually get I2S.🔧 Suggested fix
- // validate the bus config: count I2S buses and check if they meet requirements - unsigned i2sBusCount = 0; - - for (const auto &bus : busConfigs) { - if (Bus::isDigital(bus.type) && !Bus::is2Pin(bus.type)) { - digitalCount++; - if (bus.driverType == 1) - i2sBusCount++; - } - } - DEBUG_PRINTF_P(PSTR("Digital buses: %u, I2S buses: %u\n"), digitalCount, i2sBusCount); - - // Determine parallel vs single I2S usage (used for memory calculation only) - bool useParallelI2S = false; - `#if` defined(CONFIG_IDF_TARGET_ESP32S3) - // ESP32-S3 always uses parallel LCD driver for I2S - if (i2sBusCount > 0) { - useParallelI2S = true; - } - `#else` - if (i2sBusCount > 1) { - useParallelI2S = true; - } - `#endif` + // Determine bus iType first; memory sizing must use assigned drivers. + for (auto &bus : busConfigs) { + bus.iType = BusManager::getI(bus.type, bus.pins, bus.driverType); + } + + unsigned i2sBusCount = 0; + for (const auto &bus : busConfigs) { + if (Bus::isDigital(bus.type) && !Bus::is2Pin(bus.type)) { + digitalCount++; + if (bus.iType != I_NONE && ((bus.iType & 0x01) == 0)) i2sBusCount++; + } + } + DEBUG_PRINTF_P(PSTR("Digital buses: %u, I2S buses: %u\n"), digitalCount, i2sBusCount); + + bool useParallelI2S = false; + `#if` defined(CONFIG_IDF_TARGET_ESP32S3) + useParallelI2S = (i2sBusCount > 0); + `#else` + useParallelI2S = (i2sBusCount > 1); + `#endif` ... - for (auto &bus : busConfigs) { - // assign bus types: call to getI() determines bus types/drivers, allocates and tracks polybus channels - // store the result in iType for later use during bus creation (getI() must only be called once per BusConfig) - // note: this needs to be determined for all buses prior to creating them as it also determines parallel I2S usage - bus.iType = BusManager::getI(bus.type, bus.pins, bus.driverType); - } for (auto &bus : busConfigs) { ... - bool usesI2S = (bus.iType & 0x01) == 0; // I2S bus types are even numbered, can't use bus.driverType == 1 as getI() may have defaulted to RMT + bool usesI2S = (bus.iType != I_NONE) && ((bus.iType & 0x01) == 0); // I2S bus types are even numberedBased on learnings: In WLED, both parallel I2S and single I2S modes require DMA accounting based on which buses actually use I2S hardware.
Also applies to: 1193-1206, 1213-1217
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/FX_fcn.cpp` around lines 1166 - 1186, The code currently increments i2sBusCount based on the requested driverType; instead use the bus's assigned iType (the actual instantiated driver) when determining I2S usage so DMA memory is estimated only for buses that truly use I2S hardware—update the loop over busConfigs (and the other occurrences noted around the blocks at 1193-1206 and 1213-1217) to check bus.iType against the I2S types (not bus.driverType or I_NONE parity) when incrementing i2sBusCount and when computing useParallelI2S, leaving the ESP32-S3 conditional logic intact but driven by assigned iType.
♻️ Duplicate comments (1)
wled00/wled.cpp (1)
492-501:⚠️ Potential issue | 🟡 MinorHandle
esp_read_mac()failure before derivingescapedMac.At Line 495 / Line 497,
esp_read_mac()return values are ignored. If the call fails,escapedMaccan still be overwritten from zeroed bytes, which risks non-unique identity values (AP SSID/topic suffixes).Suggested patch
if (escapedMac == "000000000000") { uint8_t mac[6] = {0}; + esp_err_t macErr = ESP_FAIL; `#if` defined(CONFIG_IDF_TARGET_ESP32P4) // P4 does not have on-chip WIFI, use ethernet MAC - esp_read_mac(mac, ESP_MAC_ETH); + macErr = esp_read_mac(mac, ESP_MAC_ETH); `#else` - esp_read_mac(mac, ESP_MAC_WIFI_STA); + macErr = esp_read_mac(mac, ESP_MAC_WIFI_STA); `#endif` - char buf[13]; - sprintf(buf, "%02x%02x%02x%02x%02x%02x", mac[0], mac[1], mac[2], mac[3], mac[4], mac[5]); - escapedMac = buf; + if (macErr == ESP_OK) { + char buf[13]; + snprintf(buf, sizeof(buf), "%02x%02x%02x%02x%02x%02x", mac[0], mac[1], mac[2], mac[3], mac[4], mac[5]); + escapedMac = buf; + } else { + DEBUG_PRINTF_P(PSTR("esp_read_mac failed: %d\n"), macErr); + } }#!/bin/bash # Verify esp_read_mac result handling in the current tree sed -n '488,503p' wled00/wled.cpp echo "----" rg -n 'esp_read_mac\s*\(' wled00/wled.cpp -C2 echo "----" rg -n '=\s*esp_read_mac\s*\(' wled00/wled.cpp -C2 # Expected: # - Each esp_read_mac(...) call is assigned to esp_err_t # - A subsequent check for ESP_OK exists before formatting/assigning escapedMac🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/wled.cpp` around lines 492 - 501, The esp_read_mac() return value is ignored, so escapedMac may be derived from a zero MAC; modify the block that calls esp_read_mac(...) to store its return into an esp_err_t (or int) result, check that the result equals ESP_OK before using the mac[] buffer and formatting into escapedMac, and on failure skip assigning escapedMac (or log/handle the error) so we do not overwrite escapedMac with a zero MAC; update the branch containing esp_read_mac(...), the local mac[] and sprintf into buf, and the assignment escapedMac = buf to only occur after the ESP_OK check.
🧹 Nitpick comments (2)
wled00/pin_manager.cpp (1)
239-255: Make the USB/JTAG pin guard independent of board-package defaults.These branches only reserve the USB/JTAG pins when
ARDUINO_USB_CDC_ON_BOOTorARDUINO_USB_DFU_ON_BOOTevaluates to1, but the new C5/C6 envs inplatformio.inionly set the CDC flag and never define the DFU flag. That leaves the protection here partly controlled by external board.json defaults, so a platform package change can silently make GPIO 12/13/14 look user-assignable again. Please set both macros explicitly in the envs, or collapse this behind a repo-owned capability define.Also applies to: 264-266
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/pin_manager.cpp` around lines 239 - 255, The pin-guard currently depends on external ARDUINO_USB_CDC_ON_BOOT/ARDUINO_USB_DFU_ON_BOOT macros which can be altered by board-package defaults, so update pin_manager.cpp to stop relying solely on those: add a repo-owned capability macro (e.g. WLED_PROTECT_USB_JTAG) and use it in the USB-JTAG checks (replace the existing `#if` ARDUINO_USB_CDC_ON_BOOT == 1 || ARDUINO_USB_DFU_ON_BOOT == 1 blocks around the gpio == 12/13/14 checks) or, alternatively, ensure both ARDUINO_USB_CDC_ON_BOOT and ARDUINO_USB_DFU_ON_BOOT are explicitly set to 1 in the project envs for all C5/C6 platformio.ini envs so the protection for GPIO 12/13/14 is always applied; make the same change for the other occurrence noted in the diff.wled00/cfg.cpp (1)
166-170: Redundant preprocessor check inside already-guarded block.The
#if defined(ARDUINO_ARCH_ESP32) && ...on line 166 is inside an#ifdef ARDUINO_ARCH_ESP32block (line 164), making theARDUINO_ARCH_ESP32check redundant.♻️ Suggested simplification
`#ifdef` ARDUINO_ARCH_ESP32 CJSON(txPower, wifi[F("txpwr")]); - `#if` defined(ARDUINO_ARCH_ESP32) && (ESP_IDF_VERSION_MAJOR > 4) + `#if` ESP_IDF_VERSION_MAJOR > 4 txPower = min(max((int)txPower, (int)WIFI_POWER_2dBm), (int)WIFI_POWER_21dBm); // V5 allows WIFI_POWER_21dBm = 84 ... WIFI_POWER_MINUS_1dBm = -4 `#else` txPower = min(max((int)txPower, (int)WIFI_POWER_2dBm), (int)WIFI_POWER_19_5dBm); `#endif` `#endif`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/cfg.cpp` around lines 166 - 170, The inner preprocessor check redundantly re-tests ARDUINO_ARCH_ESP32 inside an outer `#ifdef` ARDUINO_ARCH_ESP32 block; simplify the condition by replacing the inner "#if defined(ARDUINO_ARCH_ESP32) && (ESP_IDF_VERSION_MAJOR > 4)" with a direct check of ESP_IDF_VERSION_MAJOR (e.g., "#if (ESP_IDF_VERSION_MAJOR > 4)"), keeping the two branches that clamp txPower using WIFI_POWER_21dBm and WIFI_POWER_19_5dBm respectively; update the block around txPower so only the ESP_IDF_VERSION_MAJOR test remains while preserving the existing min/max logic.
🤖 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/FX_fcn.cpp`:
- Around line 1166-1186: The code currently increments i2sBusCount based on the
requested driverType; instead use the bus's assigned iType (the actual
instantiated driver) when determining I2S usage so DMA memory is estimated only
for buses that truly use I2S hardware—update the loop over busConfigs (and the
other occurrences noted around the blocks at 1193-1206 and 1213-1217) to check
bus.iType against the I2S types (not bus.driverType or I_NONE parity) when
incrementing i2sBusCount and when computing useParallelI2S, leaving the ESP32-S3
conditional logic intact but driven by assigned iType.
---
Duplicate comments:
In `@wled00/wled.cpp`:
- Around line 492-501: The esp_read_mac() return value is ignored, so escapedMac
may be derived from a zero MAC; modify the block that calls esp_read_mac(...) to
store its return into an esp_err_t (or int) result, check that the result equals
ESP_OK before using the mac[] buffer and formatting into escapedMac, and on
failure skip assigning escapedMac (or log/handle the error) so we do not
overwrite escapedMac with a zero MAC; update the branch containing
esp_read_mac(...), the local mac[] and sprintf into buf, and the assignment
escapedMac = buf to only occur after the ESP_OK check.
---
Nitpick comments:
In `@wled00/cfg.cpp`:
- Around line 166-170: The inner preprocessor check redundantly re-tests
ARDUINO_ARCH_ESP32 inside an outer `#ifdef` ARDUINO_ARCH_ESP32 block; simplify the
condition by replacing the inner "#if defined(ARDUINO_ARCH_ESP32) &&
(ESP_IDF_VERSION_MAJOR > 4)" with a direct check of ESP_IDF_VERSION_MAJOR (e.g.,
"#if (ESP_IDF_VERSION_MAJOR > 4)"), keeping the two branches that clamp txPower
using WIFI_POWER_21dBm and WIFI_POWER_19_5dBm respectively; update the block
around txPower so only the ESP_IDF_VERSION_MAJOR test remains while preserving
the existing min/max logic.
In `@wled00/pin_manager.cpp`:
- Around line 239-255: The pin-guard currently depends on external
ARDUINO_USB_CDC_ON_BOOT/ARDUINO_USB_DFU_ON_BOOT macros which can be altered by
board-package defaults, so update pin_manager.cpp to stop relying solely on
those: add a repo-owned capability macro (e.g. WLED_PROTECT_USB_JTAG) and use it
in the USB-JTAG checks (replace the existing `#if` ARDUINO_USB_CDC_ON_BOOT == 1 ||
ARDUINO_USB_DFU_ON_BOOT == 1 blocks around the gpio == 12/13/14 checks) or,
alternatively, ensure both ARDUINO_USB_CDC_ON_BOOT and ARDUINO_USB_DFU_ON_BOOT
are explicitly set to 1 in the project envs for all C5/C6 platformio.ini envs so
the protection for GPIO 12/13/14 is always applied; make the same change for the
other occurrence noted in the diff.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 401fcb8d-bb7e-4f97-8972-b5c089925999
📒 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 (4)
- usermods/audioreactive/audio_source.h
- wled00/ota_update.cpp
- wled00/src/dependencies/dmx/ESPDMX.cpp
- wled00/set.cpp
🚧 Files skipped from review as they are similar to previous changes (8)
- wled00/dmx_output.cpp
- wled00/improv.cpp
- wled00/src/dependencies/dmx/SparkFunDMX.cpp
- wled00/button.cpp
- wled00/FXparticleSystem.cpp
- wled00/wled.h
- wled00/const.h
- platformio.ini
only one usecase: WLED_MAX_DIGITAL_CHANNELS=0 on C5/C6 will prevent very frequent RMT timeout warnings.
1 task
coderabbitai
Bot
added
enhancement
hardware
Contributor
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
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
6 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.
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
New Features
Bug Fixes