🚀 Add new MCU support (C6 and C5) to the ESP-IDF V5 builds #5048
Conversation
due to Arduino 3.0.1, a few updated libraries are required: * Tasmota Platform - official platfomio lacks arduino support for C6 * FastLED (latest + C6 build patches) * AsyncTCP (latest + C6 build patches) * AsyncWebServer (latest + C6 build patches) * NeoPixelBus (lastest)
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the 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 |
platformio.ini
Outdated
| ;nodemcuv2 | ||
| ;esp8266_2m | ||
| ;esp01_1m_full | ||
| ;nodemcuv2_160 | ||
| ;esp8266_2m_160 | ||
| ;esp01_1m_full_160 | ||
| ;nodemcuv2_compat | ||
| ;esp8266_2m_compat | ||
| ;esp01_1m_full_compat | ||
| ;esp32dev | ||
| ;esp32dev_debug | ||
| ;esp32_eth | ||
| ;esp32_wrover | ||
| ; lolin_s2_mini ;; TODO: disabled NeoEsp32RmtMethodIsr | ||
| esp32c3dev | ||
| ;esp32c3dev |
There was a problem hiding this comment.
TODO: revert before merge, here to save CI build time
|
@softhack007 I've started trying to cherry-pick your changes over, quite a few don't apply due to the changes with MM, but I'll try and resolve the merge conflicts for those that are simple |
* more debug output * added my own fork of FastLED ( looks like more bugs to solve ....)
@netmindz thanks, I'll try to help once I'm finished with some open ends in WLED-MM 😁 december |
|
Possibly also worth looking at #4626 to see if there are any changes there that are helpful |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
![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.
Thanks, can you change to using |
Main objective is to fnd out how far away we are from a build for the -P4. I'll clean up the "if def" mess later.
|
@softhack007 on it :) |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
Adds a user-configurable "WiFi band" dropdown to the WiFi settings page: 2.4 GHz only, 5 GHz only, or Auto (both). Defaults to Auto. Uses WiFi.setBandMode() from the Arduino ESP32 core (requires IDF 5.4.2+), guarded by #ifdef SOC_WIFI_SUPPORT_5G. Replaces the hardcoded WIFI_BAND_MODE_AUTO call in V5-C6 with a user-selectable setting. Changes: - wled.h: new wifiBandMode variable (default WIFI_BAND_MODE_AUTO) - cfg.cpp: serialize/deserialize with input validation (1-3 range) - set.cpp: handle BM form param with validation, trigger reconnect - xml.cpp: populate dropdown value, hide on non-5G chips - wled.cpp: pass wifiBandMode to WiFi.setBandMode() in initConnection() - settings_wifi.htm: dropdown in Experimental section Tested on ESP32-C5 hardware - all three modes work correctly.
| node, | ||
| CPPDEFINES=cdefs | ||
| ) | ||
| # Return the node unmodified - middleware should not create new build targets |
There was a problem hiding this comment.
ToDO: this is a temporary hack - the root cause is solved in the latest pioarduino (and tasmota, too?). Remember to revert this change once we upgrade to latest platform packages.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
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 (3)
wled00/FX_fcn.cpp (1)
1194-1211:⚠️ Potential issue | 🟠 MajorFix single‑I2S DMA memory estimation for ESP32/S2.
usesI2Sstill assumes single‑I2S is the first digital bus (and doesn’t account for S2 single‑I2S), which underestimatesmaxI2Sonce multiple digital buses are configured. With single‑I2S now mapped to the last bus, the DMA memory estimate should key off that bus index to avoid allowing oversized configs through the MAX_LED_MEMORY check.Based on learnings: In WLED PR `#5267`, the single‑I2S bus moved to the last bus (ESP32 bus 8, ESP32‑S2 bus 4), so memory calculations must account for that specific bus index.🔧 Suggested adjustment (align single‑I2S bus index)
- `#if` defined(CONFIG_IDF_TARGET_ESP32) || defined(CONFIG_IDF_TARGET_ESP32S3) - const bool usesI2S = ((useParallelI2S && digitalCount <= 8) || (!useParallelI2S && digitalCount == 1)); - `#elif` defined(CONFIG_IDF_TARGET_ESP32S2) - const bool usesI2S = (useParallelI2S && digitalCount <= 8); + `#if` defined(CONFIG_IDF_TARGET_ESP32) + const unsigned singleI2SBus = 9; // bus 8 (last) when single I2S + const bool usesI2S = (useParallelI2S && digitalCount <= 8) || (!useParallelI2S && digitalCount == singleI2SBus); + `#elif` defined(CONFIG_IDF_TARGET_ESP32S3) + const bool usesI2S = (useParallelI2S && digitalCount <= 8) || (!useParallelI2S && digitalCount == 1); + `#elif` defined(CONFIG_IDF_TARGET_ESP32S2) + const bool usesI2S = (useParallelI2S && digitalCount <= 8) || (!useParallelI2S && digitalCount == 5); // bus 4 (last) when single I2Swled00/wled.h (2)
158-165:⚠️ Potential issue | 🟡 MinorDMX error message should mention C61.
The guard includes ESP32‑C61, but the compile‑time error text doesn’t.
✏️ Suggested message update
- `#error` "DMX output is not supported on ESP32-C5/C6/P4 (esp_dmx library excluded)" + `#error` "DMX output is not supported on ESP32-C5/C6/C61/P4 (esp_dmx library excluded)"- `#error` "DMX output is not supported on ESP32-C5/C6/P4 (esp_dmx library excluded)" + `#error` "DMX output is not supported on ESP32-C5/C6/C61/P4 (esp_dmx library excluded)"Also applies to: 480-483
334-344:⚠️ Potential issue | 🔴 CriticalGuard RX/TX usage for non‑classic ESP32 targets.
The current guard logic has a flaw: for non-classic ESP32s (S2/S3/C3/C5/C6/C61/P4), the condition
!defined(CONFIG_IDF_TARGET_ESP32)short-circuits the OR to TRUE, bypassing the RX/TX existence check. Since some Arduino-ESP32 board variants don't define these macros, this will cause compilation failures. Require explicit RX/TX presence for all ESP32 targets:-#if defined(ARDUINO_ARCH_ESP32) && (!defined(CONFIG_IDF_TARGET_ESP32) || (defined(RX) && defined(TX))) +#if defined(ARDUINO_ARCH_ESP32) && defined(RX) && defined(TX) constexpr uint8_t hardwareRX = RX; constexpr uint8_t hardwareTX = TX; `#else` // use defaults for RX/TX constexpr uint8_t hardwareRX = 3; constexpr uint8_t hardwareTX = 1; `#endif`
🤖 Fix all issues with AI agents
In `@wled00/wled_boards.h`:
- Around line 130-136: The comment marking RTC memory as "TBC" is misleading
because util.cpp (around util.cpp:701) already uses MALLOC_CAP_RTCRAM for
conditional allocation; update wled_boards.h by removing the "TBC" text and
either (a) leave `#define` WLED_HAVE_RTC_MEMORY_HEAP 1 as a confirmed capability
(matching how ESP32-C61 is documented) or (b) gate the definition behind the
proper IDF config macro (e.g., only define WLED_HAVE_RTC_MEMORY_HEAP when
CONFIG_IDF_TARGET_ESP32P4 or the appropriate ESP-IDF capability macro is
present); ensure the comment style matches other board entries and that
WLED_HAVE_RTC_MEMORY_HEAP is treated consistently with util.cpp usage.
🧹 Nitpick comments (2)
wled00/ota_update.cpp (1)
18-23: Keep bootloader offset single‑sourced.Now that offsets vary by target, consider wiring the OTA context to
BOOTLOADER_OFFSETto prevent future mismatches.Suggested follow-up
- const uint32_t bootloaderOffset = 0x1000; + const uint32_t bootloaderOffset = BOOTLOADER_OFFSET;platformio.ini (1)
392-420: Pin the NeoPixelBus C5 fork to a commit hash instead of a branch ref.
Branch pointers can move or be deleted, breaking reproducible builds. Use a commit hash or tag for CI artifacts. This aligns with the pattern used elsewhere in the file (e.g.,GifDecoder.git#bc3af189b6b1e06946569f6b4287f0b79a860f8e).
* remove last scan results when changing band mode * set band mode before starting a new scan
allow max Tx power = 21 dBm if framework supports it. If not supported, silently clamp to 19.5 dBm.
according to espressif docs, this is "maximum TX power" not "TX power"
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
* shortened another mega-ifdef * added missing CONFIG_IDF_TARGET_ESP32C61
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
usermods/audioreactive/audio_reactive.cpp (1)
1207-1232:⚠️ Potential issue | 🔴 CriticalAdd ESP32-C6 to hardware capability guards.
Both guards need C6 added:
- Line 1207: C6 doesn't support PDM microphones
- Line 1223: C6 doesn't support ADC over I2S (only classic ESP32 does)
Proposed fix
- `#if` !defined(CONFIG_IDF_TARGET_ESP32S2) && !defined(CONFIG_IDF_TARGET_ESP32C3) && !defined(CONFIG_IDF_TARGET_ESP32C5) + `#if` !defined(CONFIG_IDF_TARGET_ESP32S2) && !defined(CONFIG_IDF_TARGET_ESP32C3) && !defined(CONFIG_IDF_TARGET_ESP32C5) && !defined(CONFIG_IDF_TARGET_ESP32C6) case 5: // ... PDM case `#endif` // ... - `#if` !defined(CONFIG_IDF_TARGET_ESP32S2) && !defined(CONFIG_IDF_TARGET_ESP32C3) && !defined(CONFIG_IDF_TARGET_ESP32C5) && !defined(CONFIG_IDF_TARGET_ESP32S3) + `#if` !defined(CONFIG_IDF_TARGET_ESP32S2) && !defined(CONFIG_IDF_TARGET_ESP32C3) && !defined(CONFIG_IDF_TARGET_ESP32C5) && !defined(CONFIG_IDF_TARGET_ESP32C6) && !defined(CONFIG_IDF_TARGET_ESP32S3) // ADC over I2S is only possible on "classic" ESP32 case 0:wled00/FX_fcn.cpp (1)
1194-1211:⚠️ Potential issue | 🟠 MajorFix single‑I2S memory accounting to match bus placement.
WhenuseParallelI2Sis false, this still assumes the first digital bus uses I2S (digitalCount == 1). With single‑I2S moved to the last bus,maxI2Scan be computed from the wrong bus and under‑estimate DMA size, letting configs slip past theMAX_LED_MEMORYcheck. If S3 uses a different index, adjust accordingly.🔧 Proposed fix (align single‑I2S to the last bus)
- const bool usesI2S = ((useParallelI2S && digitalCount <= 8) || (!useParallelI2S && digitalCount == 1)); + const bool usesI2S = useParallelI2S + ? (digitalCount <= 8) + : (digitalCount == 9); // single I2S lives on last bus (digitalCount=9) @@ - const bool usesI2S = (useParallelI2S && digitalCount <= 8); + const bool usesI2S = useParallelI2S + ? (digitalCount <= 8) + : (digitalCount == 5); // single I2S lives on last bus (digitalCount=5)Based on learnings: In WLED PR
#5267, the bus allocation strategy for single I2S mode was changed: ESP32 now uses single I2S on bus 8 (the last bus, digitalCount=9) instead of bus 0; ESP32-S2 uses single I2S on bus 4 (digitalCount=5). Memory calculation logic must account for which specific bus uses I2S hardware, not just check digitalCount <= 8.wled00/wled.h (2)
158-165:⚠️ Potential issue | 🟡 MinorFix DMX guard indentation and clarify the C61 error text.
Indent nested preprocessor directives by two spaces and include C61 in the
#errormessage to match the guard list. As per coding guidelines: Use 2-space indentation for non-generated C++ header files (.h).🛠️ Proposed fix
`#ifdef` WLED_ENABLE_DMX - `#if` defined(CONFIG_IDF_TARGET_ESP32C5) || defined(CONFIG_IDF_TARGET_ESP32C6) || defined(CONFIG_IDF_TARGET_ESP32C61) || defined(CONFIG_IDF_TARGET_ESP32P4) - `#error` "DMX output is not supported on ESP32-C5/C6/P4 (esp_dmx library excluded)" - `#elif` defined(ESP8266) || defined(CONFIG_IDF_TARGET_ESP32C3)|| defined(CONFIG_IDF_TARGET_ESP32S2) - `#include` "src/dependencies/dmx/ESPDMX.h" - `#else` //ESP32 - `#include` "src/dependencies/dmx/SparkFunDMX.h" - `#endif` + `#if` defined(CONFIG_IDF_TARGET_ESP32C5) || defined(CONFIG_IDF_TARGET_ESP32C6) || defined(CONFIG_IDF_TARGET_ESP32C61) || defined(CONFIG_IDF_TARGET_ESP32P4) + `#error` "DMX output is not supported on ESP32-C5/C6/C61/P4 (esp_dmx library excluded)" + `#elif` defined(ESP8266) || defined(CONFIG_IDF_TARGET_ESP32C3)|| defined(CONFIG_IDF_TARGET_ESP32S2) + `#include` "src/dependencies/dmx/ESPDMX.h" + `#else` //ESP32 + `#include` "src/dependencies/dmx/SparkFunDMX.h" + `#endif` `#endif` @@ `#ifdef` WLED_ENABLE_DMX - `#if` defined(CONFIG_IDF_TARGET_ESP32C5) || defined(CONFIG_IDF_TARGET_ESP32C6) || defined(CONFIG_IDF_TARGET_ESP32C61) || defined(CONFIG_IDF_TARGET_ESP32P4) - `#error` "DMX output is not supported on ESP32-C5/C6/P4 (esp_dmx library excluded)" - `#elif` defined(ESP8266) || defined(CONFIG_IDF_TARGET_ESP32C3) || defined(CONFIG_IDF_TARGET_ESP32C6) || defined(CONFIG_IDF_TARGET_ESP32S2) - WLED_GLOBAL DMXESPSerial dmx; - `#else` //ESP32 - WLED_GLOBAL SparkFunDMX dmx; - `#endif` + `#if` defined(CONFIG_IDF_TARGET_ESP32C5) || defined(CONFIG_IDF_TARGET_ESP32C6) || defined(CONFIG_IDF_TARGET_ESP32C61) || defined(CONFIG_IDF_TARGET_ESP32P4) + `#error` "DMX output is not supported on ESP32-C5/C6/C61/P4 (esp_dmx library excluded)" + `#elif` defined(ESP8266) || defined(CONFIG_IDF_TARGET_ESP32C3) || defined(CONFIG_IDF_TARGET_ESP32C6) || defined(CONFIG_IDF_TARGET_ESP32S2) + WLED_GLOBAL DMXESPSerial dmx; + `#else` //ESP32 + WLED_GLOBAL SparkFunDMX dmx; + `#endif` `#endif`Also applies to: 480-487
334-344:⚠️ Potential issue | 🟠 MajorFix RX/TX detection:
defined()cannot detect C++ constants.In Arduino-ESP32,
RX/TXare defined as C++ constants (static const uint8_t) inpins_arduino.h, not preprocessor macros. The#if defined(RX)check cannot detect them. This causes the condition to always use hardcoded defaults (3, 1) for non-classic ESP32 variants, ignoring their actual UART0 pins (e.g., C3: 20/21, C5: 12/11, C6: 17/16, S2/S3: 44/43). Either use runtime assignment or include the actual Arduino headers to access the constants properly.
🤖 Fix all issues with AI agents
In `@usermods/audioreactive/audio_reactive.cpp`:
- Around line 1800-1803: The analog microphone JSON serialization blocks (e.g.,
the creation of JsonObject amic via top.createNestedObject and assignments of
amic["pin"] and related analog-mic fields) are missing ESP32-C6 in their
exclusion preprocessor guards; update the conditional macros around these
sections (the guard at the block creating amic and the subsequent analog-mic
serialization blocks) to also exclude CONFIG_IDF_TARGET_ESP32C6 (or switch to
the positive CONFIG_IDF_TARGET_ESP32 guard to target classic ESP32 only) so the
analog mic fields are omitted for ESP32S2, ESP32C3, ESP32C5, ESP32S3 and ESP32C6
consistently.
In `@wled00/bus_manager.cpp`:
- Around line 553-560: The TODO comment contains typos and an incorrect target
name; update the comment near the ledc_set_duty_and_update call (and the
surrounding LEDC/LEDC.channel_group comments) to fix spelling ("impemented" ->
"implemented") and correct the target reference ("P5" -> "P4" or "ESP32P4" to
match the `#if` condition). Ensure the TODO clearly references the LEDC struct
behavior and dithering support for ESP32P4 and leaves the rest of the technical
notes unchanged.
In `@wled00/wled.h`:
- Around line 427-429: The extern declaration for the global useParallelI2S in
bus_manager.cpp must be guarded to match its definition in wled.h; wrap the
extern bool useParallelI2S declaration with the same conditional compile guards
used for the definition (i.e., only declare it when building for ESP32 with
WLED_HAS_PARALLEL_I2S) so non-parallel I2S targets do not get an unresolved
extern reference.
🧹 Nitpick comments (2)
usermods/audioreactive/audio_reactive.cpp (1)
1163-1232: Consider centralizing hardware capability guards.The PR objectives mention
wled_boards.hwas added to centralize capability flags. Rather than repeating long#if !defined(...)chains throughout this file, consider defining capability macros like:// In wled_boards.h `#if` defined(CONFIG_IDF_TARGET_ESP32) `#define` WLED_HAS_ANALOG_MIC 1 `#define` WLED_HAS_PDM_MIC 1 `#elif` defined(CONFIG_IDF_TARGET_ESP32S3) `#define` WLED_HAS_PDM_MIC 1 `#endif`Then guards become simple:
#ifdef WLED_HAS_ANALOG_MIC— cleaner, future-proof, and consistent across files.wled00/ws.cpp (1)
83-96: Optional: name the DDP header length constant.The two bounds checks use the
10literal; extracting it avoids magic numbers and keeps the checks in sync.♻️ Suggested update
constexpr uint8_t BINARY_PROTOCOL_E131 = P_E131; // = 0, untested! constexpr uint8_t BINARY_PROTOCOL_ARTNET = P_ARTNET; // = 1, untested! constexpr uint8_t BINARY_PROTOCOL_DDP = P_DDP; // = 2 +constexpr size_t DDP_HEADER_LEN = 10; // bytes (excluding protocol byte) ... - if (len < size_t(10 + offset)) return; // DDP header is 10 bytes (+1 protocol byte) + if (len < size_t(DDP_HEADER_LEN + offset)) return; // DDP header is 10 bytes (+1 protocol byte) ... - if (len < size_t(10 + offset + ddpDataLen)) return; // not enough data, prevent out of bounds read + if (len < size_t(DDP_HEADER_LEN + offset + ddpDataLen)) return; // not enough data, prevent out of bounds readBased on learnings: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit.
| #if defined(CONFIG_IDF_TARGET_ESP32C5) || defined(CONFIG_IDF_TARGET_ESP32C61) || defined(CONFIG_IDF_TARGET_ESP32P4) | ||
| // the .duty_init.duty member seems to only affect fade operations, and its necessary to also trigger an update with | ||
| // LEDC.channel_group[gr].channel[ch].conf0.para_up = 1; | ||
| // --> research latest (V5.5.x) esp-idf documentation on how to set the duty cycle registers (by API calls?). | ||
| // https://docs.espressif.com/projects/esp-idf/en/v5.5.2/esp32c5/api-reference/peripherals/ledc.html#_CPPv424ledc_set_duty_and_update11ledc_mode_t14ledc_channel_t8uint32_t8uint32_t | ||
| // LEDC.channel_group[gr].channel[ch].duty_init.duty = duty << ((!dithering)*4); // C5 LEDC struct uses duty_init, but requires additional steps to activate | ||
| // TODO: find out if / how dithering support can be impemented on P5 | ||
| ledc_set_duty_and_update((ledc_mode_t)gr, (ledc_channel_t)ch, duty >> bitShift, hPoint >> bitShift); |
There was a problem hiding this comment.
Fix typos in the TODO comment.
“P5” and “impemented” look accidental; update to the actual targets and spelling.
✏️ Suggested fix
- // TODO: find out if / how dithering support can be impemented on P5
+ // TODO: find out if / how dithering support can be implemented on C5/C61/P4🤖 Prompt for AI Agents
In `@wled00/bus_manager.cpp` around lines 553 - 560, The TODO comment contains
typos and an incorrect target name; update the comment near the
ledc_set_duty_and_update call (and the surrounding LEDC/LEDC.channel_group
comments) to fix spelling ("impemented" -> "implemented") and correct the target
reference ("P5" -> "P4" or "ESP32P4" to match the `#if` condition). Ensure the
TODO clearly references the LEDC struct behavior and dithering support for
ESP32P4 and leaves the rest of the technical notes unchanged.
There was a problem hiding this comment.
see also #4838 (comment)
ToDO: LEDC (analog aka PWM LEDs) will need a complete rewrite for V5. LEDC channel now managed by the framework internally. Dithering possibly won't work as before, either.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Fetch the referenced PR `#4838` discussion comment to understand the context
gh pr view 4838 --repo wled/WLED --comments | grep -A 20 -B 5 "r2793934593" || echo "Discussion not found in PR comments, trying API"
# Also check the specific discussion thread
gh api repos/wled/WLED/pulls/4838/comments | jq '.[] | select(.id == 2793934593) | {body: .body, user: .user.login, created_at: .created_at}'
Length of output: 47
@softhack007, thank you for the context! That's very helpful to know.
Given that the entire LEDC subsystem needs a rewrite for V5 (with the framework now managing channels internally and dithering potentially requiring a different approach), the typo fix and TODO comment are indeed less critical—this code is transitional for the C5/C6/P4 targets until the V5 LEDC rewrite is completed.
I'll note this architectural requirement for future reference. The current ledc_set_duty_and_update() approach for C5/C61/P4 (vs. direct LEDC struct access for other targets) is a reasonable temporary solution while the proper V5 HAL-based implementation is being developed.
🧬 Code Graph Analysis Results
- File: wled00/pin_manager.cpp
-
Line range 206-206
- isPinOk(byte gpio, bool output)
bool PinManager::isPinOk(byte gpio, bool output) { if (gpio >= WLED_NUM_PINS) return false; // catch error case, to avoid array out-of-bounds access `#ifdef` ARDUINO_ARCH_ESP32 if (digitalPinIsValid(gpio)) { `#if` defined(CONFIG_IDF_TARGET_ESP32C3) // strapping pins: 2, 8, & 9 if (gpio > 11 && gpio < 18) return false; // 11-17 SPI FLASH `#if` ARDUINO_USB_CDC_ON_BOOT == 1 || ARDUINO_USB_DFU_ON_BOOT == 1 if (gpio > 17 && gpio < 20) return false; // 18-19 USB-JTAG `#endif` `#elif` defined(CONFIG_IDF_TARGET_ESP32S3) // strapping pins: 0, 45 & 46 if (gpio > 21 && gpio < 33) return false; // 22 to 32: not connected + SPI FLASH // JTAG: GPIO39-42 are usually used for inline debugging // GPIO46 is input only and pulled down `#elif` defined(CONFIG_IDF_TARGET_ESP32C5) // strapping pins: (2), 7, 25, 26, 27, 28 (GPIO2 is not a strapping pin; it's used only for external JTAG when GPIO7 selects it) // GPIO 0-15 directly usable, 16-22 are for SPI flash if (gpio > 15 && gpio < 23) return false; // 16-22 SPI FLASH `#if` ARDUINO_USB_CDC_ON_BOOT == 1 || ARDUINO_USB_DFU_ON_BOOT == 1 if (gpio == 13 || gpio == 14) return false; // 13-14 USB-JTAG `#endif` `#elif` defined(CONFIG_IDF_TARGET_ESP32C6) // strapping pins: 4, 5, 8, 9, 15 (GPIO4/MTMS and GPIO5/MTDI are also strapping pins) // GPIO 0-23 directly usable, 24-30 are for SPI flash if (gpio > 23 && gpio < 31) return false; // 24-30 SPI FLASH `#if` ARDUINO_USB_CDC_ON_BOOT == 1 || ARDUINO_USB_DFU_ON_BOOT == 1 if (gpio == 12 || gpio == 13) return false; // 12-13 USB-JTAG `#endif` `#else` if ((strncmp_P(PSTR("ESP32-U4WDH"), ESP.getChipModel(), 11) == 0) || // this is the correct identifier, but.... (strncmp_P(PSTR("ESP32-PICO-D"), ESP.getChipModel(), 12) == 0)) { // https://github.com/espressif/arduino-esp32/issues/10683 // this chip has 4 MB of internal Flash and different packaging, so available pins are different! if ((gpio > 5 && gpio < 9) || gpio == 11) return false; // U4WDH/PICO-D2 & PICO-D4: GPIO 6, 7, 8, 11 are used for SPI flash; 9 & 10 are free if (gpio == 16 || gpio == 17) return false; // U4WDH/PICO-D?: GPIO 16 and 17 are used for PSRAM } else if (strncmp_P(PSTR("ESP32-PICO-V3"), ESP.getChipModel(), 13) == 0) { if (gpio == 6 || gpio == 11) return false; // PICO-V3: uses GPIO 6 and 11 for flash if (strstr_P(ESP.getChipModel(), PSTR("V3-02")) != nullptr && (gpio == 9 || gpio == 10)) return false; // PICO-V3-02: uses GPIO 9 and 10 for PSRAM; 7, 8 are free } else { // for classic ESP32 (non-mini) modules, these are the SPI flash pins if (gpio > 5 && gpio < 12) return false; //SPI flash pins } if (gpio == 16) return !psramFound(); // PSRAM pins on modules with off-package or in-package PSRAM if (gpio == 17) { if (strncmp_P(PSTR("ESP32-D0WDR2-V3"), ESP.getChipModel(), 15) == 0) { return true; } else { return !psramFound(); // PSRAM pins on modules with in-package PSRAM } } `#endif` if (output) return digitalPinCanOutput(gpio); else return true; } `#else` if (gpio < 6) return true; if (gpio < 12) return false; //SPI flash pins if (gpio < 17) return true; `#endif` return false; } -
Line range 298-323
- PinManager::allocateLedc(byte channels)
void PinManager::allocateLedc(byte channels) { if (channels > WLED_MAX_ANALOG_CHANNELS || channels == 0) return 255; unsigned ca = 0; for (unsigned i = 0; i < WLED_MAX_ANALOG_CHANNELS; i++) { if (bitRead(ledcAlloc, i)) { //found occupied pin ca = 0; } else { // if we have PWM CCT bus allocation (2 channels) we need to make sure both channels share the same timer // for phase shifting purposes (otherwise phase shifts may not be accurate) if (channels == 2) { // will skip odd channel for first channel for phase shifting if (ca == 0 && i % 2 == 0) ca++; // even LEDC channels is 1st PWM channel if (ca == 1 && i % 2 == 1) ca++; // odd LEDC channel is 2nd PWM channel } else ca++; } if (ca >= channels) { //enough free channels unsigned in = (i + 1) - ca; for (unsigned j = 0; j < ca; j++) { bitWrite(ledcAlloc, in+j, true); } return in; } } return 255; //not enough consecutive free LEDC channels } -
Line range 325-329
- PinManager::deallocateLedc(byte pos, byte channels)
void PinManager::deallocateLedc(byte pos, byte channels) { for (unsigned j = pos; j < pos + channels && j < WLED_MAX_ANALOG_CHANNELS; j++) { bitWrite(ledcAlloc, j, false); } }
-
✏️ Learnings added
Learnt from: softhack007
Repo: wled/WLED PR: 5048
File: wled00/bus_manager.cpp:549-556
Timestamp: 2026-02-11T16:19:58.009Z
Learning: ESP-IDF V5 requires a complete rewrite of the LEDC (PWM/analog LED) subsystem in WLED. The framework now manages LEDC channels internally (unlike V4 where WLED managed them directly), and dithering functionality may not work as it did before. Direct LEDC struct access (e.g., `LEDC.channel_group[gr].channel[ch].duty.duty`) used in the current implementation will need to be replaced with proper HAL API calls. This affects `wled00/bus_manager.cpp` BusPwm class and related pin manager LEDC allocation code.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: DedeHai
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-01-03T16:42:26.035Z
Learning: In WLED, both parallel I2S and single I2S modes require additional DMA buffers (4× the pixel buffer size). The memory calculation must account for which specific buses use I2S hardware, not just check digitalCount <= 8, because single I2S mode places the I2S bus at the END (ESP32 bus 8 with digitalCount=9, S2 bus 4 with digitalCount=5).
Learnt from: DedeHai
Repo: wled/WLED PR: 4939
File: wled00/FX_fcn.cpp:1176-1187
Timestamp: 2025-09-16T18:08:42.848Z
Learning: In WLED finalizeInit() bus creation (wled00/FX_fcn.cpp), intentionally allowing memory overruns when bus configurations exceed MAX_LED_MEMORY is a deliberate design choice. The trade-off prioritizes creating buses with reduced LED counts over completely failing to create buses, which would cause no LED output and UI failures. This approach forces users to update configurations after migrating to version 0.16 while maintaining basic functionality.
Learnt from: DedeHai
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-01-03T16:42:26.035Z
Learning: In WLED PR `#5267`, the bus allocation strategy for single I2S mode was changed: ESP32 now uses single I2S on bus 8 (the last bus, digitalCount=9) instead of bus 0; ESP32-S2 uses single I2S on bus 4 (digitalCount=5). Memory calculation logic must account for which specific bus uses I2S in single mode, not just check digitalCount <= 8.
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-12-28T14:06:48.772Z
Learning: In WLED post-commit ee9ac947, the rendering pipeline uses per-segment buffers and per-pixel bus updates. Unmapped (0xFFFF) mapping entries are now skipped in WS2812FX::show() (no “clear to black”), which can leave physical gap LEDs with stale/random colors unless they are explicitly cleared. This is a behavior change from pre-0.16 where a full physical buffer was effectively refreshed each frame.
Learnt from: DedeHai
Repo: wled/WLED PR: 4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR `#4798`, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.
Learnt from: freakintoddles2
Repo: wled/WLED PR: 4904
File: wled00/FX.cpp:3915-3930
Timestamp: 2025-09-02T19:42:57.475Z
Learning: WLED wled00/FX.cpp::mode_percent — For time-based options (% of Minute/Hour/Day), behavior is clock-like: at rollover (elapsed==0) intentionally show SEGLEN lit for one tick to represent completion, then move to 1 LED at the next second. This is by design (not a progress bar) and should not be “fixed”.
Learnt from: mval-sg
Repo: wled/WLED PR: 4876
File: wled00/wled_eeprom.cpp:0-0
Timestamp: 2025-09-01T10:26:17.959Z
Learning: In WLED PR `#4876`, the DMXStartLED EEPROM backward compatibility issue was partially addressed by keeping it at address 2550 and reading it as a 16-bit value, with DMXChannelsValue array moved to addresses 2552-2566. This maintains compatibility with pre-0.11 EEPROM layouts for DMXStartLED, though legacy "Set to 255" (code 6) configurations may still need migration logic.
Learnt from: mval-sg
Repo: wled/WLED PR: 4876
File: wled00/xml.cpp:0-0
Timestamp: 2025-08-28T08:09:20.630Z
Learning: The WLED codebase has opportunities for refactoring hardcoded array bounds (like the "15" used for DMX channels) to use sizeof(array)/sizeof(array[0]) for more maintainable code, but such changes should be done consistently across the entire codebase in a dedicated refactoring effort.
Learnt from: blazoncek
Repo: wled/WLED PR: 5140
File: wled00/data/settings_time.htm:66-76
Timestamp: 2025-12-01T07:01:16.949Z
Learning: In WLED PR `#5134`, the fix for macros being initialized with the enable bit set only handles new configurations, not existing ones. If there is a bug in timer/macro handling code (e.g., in settings_time.htm), it must be fixed to work correctly for existing configurations as well.
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit. This improves code maintainability and reduces the risk of inconsistencies.
Learnt from: BobLoeffler68
Repo: wled/WLED PR: 5109
File: wled00/FX.cpp:3174-3343
Timestamp: 2025-11-27T06:33:11.436Z
Learning: WLED Ants effect (wled00/FX.cpp): The author prefers the current velocity initialization using hw_random16(1000, 5000)/5000.0f, resulting in an effective range of ~3.6–10.0 (with VELOCITY_MIN=2.0, VELOCITY_MAX=10.0), and wants the code kept as-is with comments updated to document this behavior. Avoid suggesting changes to span the full 2.0–10.0 range in future reviews.
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with defined constants when meaningful constants exist in the codebase. For example, suggest replacing hardcoded "32" with WLED_MAX_SEGNAME_LEN if the context relates to segment name length limits.
Learnt from: DedeHai
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-01-03T14:38:09.949Z
Learning: In WLED code reviews, check that FastLED noise function aliases (`inoise8`, `inoise16`, etc.) are not used. These should be replaced with the proper WLED functions (`perlin8`, `perlin16`, etc.) as the `inoise*` functions are only aliases kept for compatibility reasons.
Learnt from: DedeHai
Repo: wled/WLED PR: 4997
File: wled00/e131.cpp:33-44
Timestamp: 2025-10-10T18:34:06.550Z
Learning: In WLED's DDP packet handling (ws.cpp and e131.cpp), only prevent out-of-bounds memory access rather than enforcing DDP spec compliance. Don't check the 1440-byte spec limit—accept out-of-spec packets assuming correct encoding. The bounds check `maxDataIndex = c + numLeds * ddpChannelsPerLed; if (maxDataIndex > dataLen) reject` is sufficient and already covers the timecode flag case (when c=4) without needing separate validation.
Learnt from: DedeHai
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-01-03T14:38:09.949Z
Learning: In WLED code reviews, check that FastLED trigonometric function aliases are not used. Replace `sin8()` and `cos8()` with `sin8_t()` and `cos8_t()`. Replace `sin16()` and `cos16()` with `sin16_t()` and `cos16_t()`. For float versions, replace `sinf()` and `cosf()` with `sin_approx()` and `cos_approx()`. See wled_math.cpp for the proper WLED implementations.
Learnt from: BobLoeffler68
Repo: wled/WLED PR: 5293
File: usermods/user_fx/user_fx.cpp:149-151
Timestamp: 2026-01-24T18:51:09.740Z
Learning: In WLED's Spinning Wheel effect (usermods/user_fx/user_fx.cpp), the color scale calculation (SEGENV.aux1) must match the position modulo range. When position uses `pos = (pos_fixed >> 16) % SEGLEN`, the color scale must be `aux1 = (255 << 8) / SEGLEN` to distribute the palette evenly across the full position range. Using vstripLen for color scaling while position uses SEGLEN causes palette colors to cycle too quickly and appear incomplete.
Learnt from: DedeHai
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-01-13T21:23:35.514Z
Learning: In WLED, the global `paletteBlend` variable (wled.h:603) and the `WS2812FX::paletteBlend` member (FX.h:940) are duplicates without synchronization code. The global is loaded/saved in cfg.cpp and set via UI in set.cpp, but never copied to the strip member. This is the only such case in the codebase; other settings are either strip-only members (autoSegments, correctWB, cctFromRgb, isMatrix) or global-only (gammaCorrectCol/Bri/Val, blendingStyle).
Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/include/NeoEsp32RmtHIMethod.h:31-36
Timestamp: 2025-09-02T01:45:58.047Z
Learning: For the NeoEsp32RmtHI driver, RISC-V ESP32-C3 support is currently disabled via bus_wrapper.h rather than compile-time guards, as the maintainer willmmiles is working on resolving underlying nested interrupt issues and prefers to centralize the workaround in one location.
Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/src/NeoEsp32RmtHI.S:8-16
Timestamp: 2025-09-02T02:27:39.220Z
Learning: The ESP32 macro is defined when compiling code on all ESP32 variants (ESP32, ESP32-S2, ESP32-S3, ESP32-C3, etc.) in the Arduino framework, providing consistent library compatibility across the entire ESP32 family. For target-specific detection, CONFIG_IDF_TARGET_* macros should be used instead.
Learnt from: softhack007
Repo: wled/WLED PR: 5048
File: wled00/wled.cpp:385-387
Timestamp: 2026-02-08T19:32:56.086Z
Learning: In WLED ESP32 code, when checking whether USB CDC serial initialization delay is needed, rely on the ARDUINO_USB_CDC_ON_BOOT flag rather than explicit CONFIG_IDF_TARGET checks, because not all development boards for chips with native USB capability (ESP32-C3/C5/C6/S2/S3/H2/P4) actually use the native USB—many have external UART bridge chips instead. The ARDUINO_USB_CDC_ON_BOOT flag correctly indicates the board's actual USB configuration.
Learnt from: softhack007
Repo: wled/WLED PR: 5355
File: wled00/util.cpp:635-638
Timestamp: 2026-02-07T16:06:14.651Z
Learning: ESP32-C61, ESP32-C5, and ESP32-P4 all support external PSRAM. Only ESP32-C3, ESP32-C6, and ESP8266 do not support PSRAM and should be excluded from PSRAM-related compilation guards in WLED.
these variables are defined in wled.h, which is included already.
|
@coderabbitai please remember for future reviews: check that all global variables in .cpp files are well-defined (via .h file or via "extern"). Verify that declarations are neither duplicated, nor implicitly declared as "int". |
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.
Sorry, something went wrong.
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 (3)
usermods/audioreactive/audio_source.h (1)
25-33:⚠️ Potential issue | 🟡 MinorUpdate the unsupported-target message to match the expanded guard.
The guard now includes ESP32‑C61 (and other targets already in the list), but the message only mentions C2/C3, which is misleading during build failures/warnings.
🔧 Suggested tweak
- `#error` This audio reactive usermod does not support ESP32-C2 or ESP32-C3. + `#error` This audio reactive usermod does not support this target. ... - `#warning` This audio reactive usermod does not support ESP32-C2 and ESP32-C3. + `#warning` This audio reactive usermod does not support this target.wled00/data/settings_wifi.htm (1)
72-80:⚠️ Potential issue | 🟡 MinorGuard against missing channel values in scan results.
Some scans omit
channel, which would render “Ch undefined” in the UI.🛠️ Suggested adjustment
- option.textContent = `${networks[i].ssid} (${networks[i].rssi} dBm, Ch ${networks[i].channel})`; // [${networks[i].bssid.replaceAll(':','')}] + const ch = Number.isFinite(networks[i].channel) ? `, Ch ${networks[i].channel}` : ""; + option.textContent = `${networks[i].ssid} (${networks[i].rssi} dBm${ch})`; // [${networks[i].bssid.replaceAll(':','')}]wled00/FX_fcn.cpp (1)
1163-1184:⚠️ Potential issue | 🟠 MajorFix I2S DMA buffer estimation for single‑I2S mode.
When
useParallelI2Sis false,usesI2Sonly triggers on the first digital bus. The current bus allocation places single‑I2S on the last digital bus, somaxI2Scan be underestimated and the MAX_LED_MEMORY check can pass when it shouldn’t. Track the total digital bus count and use that to detect the last bus.🐛 Suggested fix
- digitalCount = 0; + const unsigned totalDigitalBuses = digitalCount; + digitalCount = 0;- const bool usesI2S = ((useParallelI2S && digitalCount <= 8) || (!useParallelI2S && digitalCount == 1)); + const bool usesI2S = ((useParallelI2S && digitalCount <= 8) || (!useParallelI2S && digitalCount == totalDigitalBuses));- const bool usesI2S = (useParallelI2S && digitalCount <= 8); + const bool usesI2S = (useParallelI2S && digitalCount <= 8) || (!useParallelI2S && digitalCount == totalDigitalBuses);Based on learnings: In WLED, both parallel I2S and single I2S modes require additional DMA buffers (4× the pixel buffer size). The memory calculation must account for which specific buses use I2S hardware, not just check digitalCount <= 8, because single I2S mode places the I2S bus at the END (ESP32 bus 8 with digitalCount=9, S2 bus 4 with digitalCount=5).
Also applies to: 1194-1209
🤖 Fix all issues with AI agents
In `@wled00/data/settings_wifi.htm`:
- Around line 253-258: You changed the WiFi UI (wled00/data/settings_wifi.htm —
elements with id "bm" and "tx"), so regenerate the embedded UI headers by
running the project's UI build step (npm run build) to update the generated
headers used in firmware builds; after running the build verify the generated
header files changed and include them in the PR so the firmware build stays in
sync with settings_wifi.htm.
🧹 Nitpick comments (2)
wled00/bus_wrapper.h (1)
509-513: Track the TODO to simplify the long guard.
Happy to help refactor this into a helper macro or wrapper if you want to tackle it now.wled00/ota_update.cpp (1)
18-23: Bootloader offsets and ESP32-C5 chip ID verified as correct.The values for ESP32-C5 (chip_id
0x0017, bootloader offset0x2000) and ESP32-C61 (bootloader offset0x0000) match ESP-IDF documentation. Bootloader offsets are already properly hoisted into theBOOTLOADER_OFFSETconstant.Optional: Consider defining chip ID constants. The chip IDs are currently hardcoded in the validation checks (lines 415–450), though they are documented in comments (lines 405–413). Replacing hardcoded values like
0x0017with named constants (e.g.,ESP32C5_CHIP_ID = 0x0017) would improve maintainability and reduce drift risk if ESP-IDF updates these values.
5 participants
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
Summary by CodeRabbit
New Features
Bug Fixes / Compatibility
Chores