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
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
* mbedtls_sha1_... functions have different names * use CONFIG_SOC_ADC_RTC_MAX_BITWIDTH * legacy adc calibration API is not supported on new MCUs (-C5, -C6, -C61, -P4)
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
* 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
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
Release Notes
New Features
Bug Fixes
Configuration