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 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)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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
platformio.ini
Outdated
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
Draft
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.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (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.
these variables are defined in wled.h, which is included already.
Member
|
@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.
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 (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.
Member
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 3433 🏁 Script executed: Length of output: 7821
🚨 Breaking Differences Requiring Conditional Code1️⃣ Channel Count (affects max analog LEDs)
Impact: 2️⃣ Timer Bit Width (duty resolution)
Impact: Duty calculations need 3️⃣ Clock Sources (critical API change!)
Breaking: Code using
4️⃣ High-Speed Mode
5️⃣ New Features (C5/C6/P4 only)SOC_LEDC_GAMMA_CURVE_FADE_SUPPORTED (1) // Hardware gamma correction
SOC_LEDC_GAMMA_CURVE_FADE_RANGE_MAX (16) // 16-step gamma curves
SOC_LEDC_FADE_PARAMS_BIT_WIDTH (10) // 10-bit fade params
SOC_LEDC_SUPPORT_SLEEP_RETENTION (1) // Preserve state in sleepOpportunity: Could leverage hardware gamma on C5/C6/P4 for smoother fades. 6️⃣ Fade Stop Support
Impact: 📋 Recommended Conditional Compilation Strategy// In bus_manager.cpp or BusPwm class:
// 1. Channel count
`#if` SOC_LEDC_CHANNEL_NUM == 6
`#define` WLED_MAX_ANALOG_CHANNELS 6 // C3/C5/C6
`#else`
`#define` WLED_MAX_ANALOG_CHANNELS 8 // ESP32/S2/S3/P4
`#endif`
// 2. Clock source selection
`#ifdef` SOC_LEDC_SUPPORT_PLL_DIV_CLOCK
ledc_timer.clk_cfg = LEDC_USE_PLL_DIV_CLK; // C5/C6/P4
`#else`
ledc_timer.clk_cfg = LEDC_USE_APB_CLK; // ESP32/S2/S3/C3
`#endif`
// 3. Speed mode
`#ifdef` SOC_LEDC_SUPPORT_HS_MODE
ledc_channel.speed_mode = LEDC_HIGH_SPEED_MODE; // Only ESP32 classic
`#else`
ledc_channel.speed_mode = LEDC_LOW_SPEED_MODE; // All others
`#endif`
// 4. Duty resolution (use SOC macro directly)
ledc_timer.duty_resolution = SOC_LEDC_TIMER_BIT_WIDTH;
// 5. Fade stop (if using fade API)
`#ifdef` SOC_LEDC_SUPPORT_FADE_STOP
ledc_fade_stop(LEDC_LOW_SPEED_MODE, channel);
`#endif`
|
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 (4)
wled00/dmx_output.cpp (1)
71-76:⚠️ Potential issue | 🟡 MinorFix indentation of the preprocessor block in
initDMXOutput().The
#if/#else/#endiflines are indented by one space; this file requires 2-space indentation.🔧 Proposed fix
void initDMXOutput() { - `#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_ESP32S2) + `#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_ESP32S2) dmx.init(512); // initialize with bus length - `#else` + `#else` dmx.initWrite(512); // initialize with bus length - `#endif` + `#endif` }As per coding guidelines:
wled00/**/*.cpp: Use 2-space indentation for C++ source files (.cpp).usermods/audioreactive/audio_source.h (1)
25-33:⚠️ Potential issue | 🟡 MinorAlign the warning text with the expanded target guard.
The guard now includes C5/C6/C61/H2, but the warning/error still says only C2/C3, which is confusing for builders on the newly added targets.
💬 Suggested wording update
- `#error` This audio reactive usermod does not support ESP32-C2 or ESP32-C3. + `#error` This audio reactive usermod is not supported on this MCU (C2/C3/C5/C6/C61/H2). ... - `#warning` This audio reactive usermod does not support ESP32-C2 and ESP32-C3. + `#warning` This audio reactive usermod is not supported on this MCU (C2/C3/C5/C6/C61/H2).wled00/FX_fcn.cpp (1)
1194-1211:⚠️ Potential issue | 🟠 MajorFix I2S memory estimation for single‑I2S bus placement.
usesI2Sstill assumes the single‑I2S bus is the first digital bus (digitalCount == 1). The bus allocation strategy now places single‑I2S on the last bus (ESP32 bus 8, ESP32‑S2 bus 4), so the I2S DMA memory estimate can be skipped entirely and undercountmaxI2S.🔧 Example adjustment
-#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) || defined(CONFIG_IDF_TARGET_ESP32S3) + const bool usesI2S = (useParallelI2S && digitalCount <= 8) || + (!useParallelI2S && digitalCount == 9); +#elif defined(CONFIG_IDF_TARGET_ESP32S2) + const bool usesI2S = (useParallelI2S && digitalCount <= 8) || + (!useParallelI2S && digitalCount == 5);Based on learnings, “single I2S” now uses the last bus (ESP32 bus 8 / S2 bus 4), so memory accounting must key off that bus index instead of
digitalCount == 1.wled00/wled.h (1)
334-344:⚠️ Potential issue | 🔴 CriticalGuard RX/TX usage on boards that don't define those macros.
The current logic silently uses RX=3 and TX=1 for any non-classic ESP32 variant that doesn't define those macros. However, non-classic ESP32 variants (ESP32-C3, ESP32-C5, ESP32-C6, etc.) have different UART0 pins—falling back to 3/1 for those variants is incorrect. For example, ESP32-C3 UART0 defaults to GPIO20/21, not GPIO3/1.
The fix should guard RX/TX usage explicitly and only provide the 3/1 fallback for classic ESP32. Add an error directive for variants without RX/TX defined to prevent silent pin misconfigurations.
🔧 Proposed hardening
-#if defined(ARDUINO_ARCH_ESP32) && (!defined(CONFIG_IDF_TARGET_ESP32) || (defined(RX) && defined(TX))) - // use RX/TX as set by the framework - these boards do _not_ have RX=3 and TX=1 - 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 +#if defined(ARDUINO_ARCH_ESP32) && defined(RX) && defined(TX) + // use RX/TX as set by the framework - these boards do _not_ have RX=3 and TX=1 + constexpr uint8_t hardwareRX = RX; + constexpr uint8_t hardwareTX = TX; +#elif defined(CONFIG_IDF_TARGET_ESP32) + // use defaults for RX/TX on classic ESP32 + constexpr uint8_t hardwareRX = 3; + constexpr uint8_t hardwareTX = 1; +#else + `#error` "RX/TX are not defined for this ESP32 variant. Please define RX/TX (or provide a board-specific default)." +#endif
🧹 Nitpick comments (4)
wled00/ws.cpp (1)
86-99: Replace the DDP header magic number and align types.
10is a protocol constant; making it a namedconstexprimproves readability and avoids repeating magic numbers. While you’re here, consider makingoffsetasize_tto avoid mixed signed/unsigned arithmetic.♻️ Suggested tweak
- constexpr int offset = 1; // offset to skip protocol byte + constexpr size_t offset = 1; // offset to skip protocol byte + constexpr size_t DDP_HEADER_LEN = 10; switch (data[0]) { @@ - if (len < size_t(10 + offset)) return; // DDP header is 10 bytes (+1 protocol byte) + if (len < DDP_HEADER_LEN + offset) return; // DDP header is 10 bytes (+1 protocol byte) size_t ddpDataLen = (data[8+offset] << 8) | data[9+offset]; // data length in bytes from DDP header @@ - if (len < size_t(10 + offset + ddpDataLen)) return; // not enough data, prevent out of bounds read + if (len < 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 meaningful constants exist in the codebase."
wled00/udp.cpp (1)
723-743: Consider usingWLED_BOARDto avoid the growing#elifchain.Now that
wled_boards.hprovides a canonicalWLED_BOARD, this can be simplified and kept in sync automatically.♻️ Possible refactor
-#ifdef ESP8266 - data[38] = NODE_TYPE_ID_ESP8266; -#elif defined(CONFIG_IDF_TARGET_ESP32C5) - data[38] = NODE_TYPE_ID_ESP32C5; -#elif defined(CONFIG_IDF_TARGET_ESP32C6) - data[38] = NODE_TYPE_ID_ESP32C6; -#elif defined(CONFIG_IDF_TARGET_ESP32C61) - data[38] = NODE_TYPE_ID_ESP32C61; -#elif defined(CONFIG_IDF_TARGET_ESP32C3) - data[38] = NODE_TYPE_ID_ESP32C3; -#elif defined(CONFIG_IDF_TARGET_ESP32S3) - data[38] = NODE_TYPE_ID_ESP32S3; -#elif defined(CONFIG_IDF_TARGET_ESP32S2) - data[38] = NODE_TYPE_ID_ESP32S2; -#elif defined(CONFIG_IDF_TARGET_ESP32P4) - data[38] = NODE_TYPE_ID_ESP32P4; -#elif defined(ARDUINO_ARCH_ESP32) - data[38] = NODE_TYPE_ID_ESP32; -#else - data[38] = NODE_TYPE_ID_UNDEFINED; -#endif + data[38] = WLED_BOARD;wled00/FXparticleSystem.cpp (1)
502-510: Update fast-divide comments to match the capability flag.The guard is now macro-driven, but the inline comments still name specific targets. Consider making the comment macro-based to avoid drift.
✏️ Suggested comment tweak (apply to each affected block)
- `#else` // division is faster on ESP32, S2 and S3 + `#else` // division is faster when WLED_HAVE_FAST_int_DIVIDE is setAlso applies to: 516-528, 931-937, 959-969, 1412-1424, 1705-1709, 1726-1732
wled00/wled.cpp (1)
989-996: Consider adding IDF version guard forWiFi.getBand()for consistency.The guard
#if defined(ARDUINO_ARCH_ESP32) && SOC_WIFI_SUPPORT_5Gdiffers from the guards used forWiFi.setBandMode()at lines 537 and 748, which also checkESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 4, 2). While in practiceSOC_WIFI_SUPPORT_5Gcurrently implies IDF 5.4+ (since ESP32-C5 is the only chip defining it), adding the version check would maintain consistency and future-proof the code.♻️ Optional: Add version guard for consistency
- `#if` defined(ARDUINO_ARCH_ESP32) && SOC_WIFI_SUPPORT_5G + `#if` defined(ARDUINO_ARCH_ESP32) && defined(SOC_WIFI_SUPPORT_5G) && (ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 4, 2)) auto wifiBand = WiFi.getBand(); DEBUG_PRINT(wifiBand == WIFI_BAND_2G ? F(" (2.4GHz)") : (wifiBand == WIFI_BAND_5G ? F(" (5GHz)"): F(" (other)"))); `#else`
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain
🌐 Web query: 💡 Result: In ESP-IDF v5.5.2, the LEDC (LED PWM) feature flags and limits are exposed via SoC capability macros in:
Common “is LEDC present?” macro
ESP32 (classic) LEDC capability macros (examples)On ESP32,
Newer targets may expose different/additional LEDC macros (example: ESP32-C5)On ESP32-C5, the LEDC capability set includes (among others) [2]:
If you tell me your exact target (esp32 / esp32s2 / esp32s3 / esp32c3 / esp32c6 / esp32h2 / esp32c5, etc.), I can list the exact Sources: ESP-IDF Programming Guide (v5.5.2) SoC capabilities pages for ESP32 and ESP32-C5 [1][2]. 🌐 Web query: 💡 Result: In ESP-IDF commit
Practical impact: any code (especially low-level/LL code) that directly referenced Sources: Citations: 🌐 Web query: 💡 Result: ESP-IDF commit What changed for “LEDC duty” (relevant to ESP32-C5)
Practical impact
Sources: commit page and diff context [1][2], ESP-IDF LEDC docs (latest / ESP32-C5) [3][4]. 🏁 Script executed: Length of output: 3433
📊 LEDC Capabilities Comparison Table
🚨 Critical Breaking Changes for WLED1️⃣ Clock Source API (BREAKING)// ❌ OLD CODE (won't compile on C5/C6/P4):
ledc_timer.clk_cfg = LEDC_USE_APB_CLK;
// ✅ NEW CODE:
`#ifdef` SOC_LEDC_SUPPORT_PLL_DIV_CLOCK
ledc_timer.clk_cfg = LEDC_USE_PLL_DIV_CLK; // C5/C6/P4
`#else`
ledc_timer.clk_cfg = LEDC_USE_APB_CLK; // ESP32/S2/S3/C3
`#endif`2️⃣ Duty Field Access (C5/P4 specific)ESP32-P4 (commit fbdb9413): // ❌ OLD: duty_r.duty
uint32_t duty = LEDC.channel_group[gr].channel[ch].duty_r.duty;
// ✅ NEW on P4: duty_r.duty_r
uint32_t duty = LEDC.channel_group[gr].channel[ch].duty_r.duty_r;ESP32-C5 (commit 33fa42ca):
3️⃣ Channel Count// Use SOC macro instead of hardcoded 8:
`#define` WLED_MAX_ANALOG_CHANNELS SOC_LEDC_CHANNEL_NUM // 6 on C3/C5/C6, 8 on others4️⃣ High-Speed Mode`#ifdef` SOC_LEDC_SUPPORT_HS_MODE
ledc_channel.speed_mode = LEDC_HIGH_SPEED_MODE; // Only ESP32 classic
`#else`
ledc_channel.speed_mode = LEDC_LOW_SPEED_MODE; // All others
`#endif`💡 Recommended Code Pattern for bus_manager.cpp// 1. Clock source selection
`#ifdef` SOC_LEDC_SUPPORT_PLL_DIV_CLOCK
ledc_timer_config_t timer_conf = {
.speed_mode = LEDC_LOW_SPEED_MODE,
.duty_resolution = SOC_LEDC_TIMER_BIT_WIDTH, // Use SOC macro
.timer_num = LEDC_TIMER_0,
.freq_hz = 5000,
.clk_cfg = LEDC_USE_PLL_DIV_CLK // C5/C6/P4
};
`#else`
ledc_timer_config_t timer_conf = {
.speed_mode = LEDC_LOW_SPEED_MODE,
.duty_resolution = SOC_LEDC_TIMER_BIT_WIDTH,
.timer_num = LEDC_TIMER_0,
.freq_hz = 5000,
.clk_cfg = LEDC_USE_APB_CLK // ESP32/S2/S3/C3
};
`#endif`
// 2. Duty calculation (account for C5 inclusive range)
uint32_t max_duty = (1 << SOC_LEDC_TIMER_BIT_WIDTH); // Now inclusive on C5!
uint32_t duty = (brightness * max_duty) / 255;
// 3. Set duty using HAL API (avoid direct struct access on IDF V5)
ledc_set_duty(LEDC_LOW_SPEED_MODE, channel, duty);
ledc_update_duty(LEDC_LOW_SPEED_MODE, channel);
// 4. Optional: leverage hardware gamma on C5/C6/P4
`#ifdef` SOC_LEDC_GAMMA_CURVE_FADE_SUPPORTED
// Could use ledc_set_duty_with_gamma() for smoother fades
`#endif`🎯 Summary of Must-Fix Issues
All of these are compile/runtime blockers that must be addressed for C5/C6/P4 support. The good news: all can be handled with conditional compilation using 🧠 Learnings used |
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)
usermods/audioreactive/audio_source.h (1)
25-33:⚠️ Potential issue | 🟡 MinorUpdate warning message to reflect all guarded targets, not just C2/C3.
ESP32-C61 correctly belongs in this guard (single-core, no hardware FPU). However, the warning text still only mentions C2 and C3, while the guard now includes C5, C6, C61, H2, and ESP8266/ESP8265. Either update the message to list all affected targets or refactor to use capability-based checks (e.g.,
SOC_CPU_HAS_FPU/SOC_CPU_CORES_NUM) for clarity.wled00/FX_fcn.cpp (1)
1194-1211:⚠️ Potential issue | 🟠 MajorFix single‑I2S memory accounting for ESP32/S2.
The
usesI2Scondition assumes single‑I2S uses the first digital bus (digitalCount == 1) and disables it entirely on S2. Current bus allocation puts single‑I2S on the last digital bus (ESP32 bus 8, S2 bus 4), so this underestimatesmaxI2Sand can let allocations exceedMAX_LED_MEMORY.Consider tracking the total digital bus count before the loop and using that to detect the last bus when
useParallelI2S == false.🔧 Suggested fix
- digitalCount = 0; + unsigned totalDigitalBusses = digitalCount; + digitalCount = 0; ... - const bool usesI2S = ((useParallelI2S && digitalCount <= 8) || (!useParallelI2S && digitalCount == 1)); + const bool usesI2S = (useParallelI2S && digitalCount <= 8) || + (!useParallelI2S && digitalCount == totalDigitalBusses); ... - const bool usesI2S = (useParallelI2S && digitalCount <= 8); + const bool usesI2S = (useParallelI2S && digitalCount <= 8) || + (!useParallelI2S && digitalCount == totalDigitalBusses);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 in single mode, not just check digitalCount <= 8.”wled00/wled.h (1)
334-344:⚠️ Potential issue | 🟠 MajorRX/TX macro selection will fail builds for non-classic ESP32 variants without RX/TX definitions.
The condition incorrectly assumes RX/TX macros are available for all non-classic ESP32 targets (C3/C5/C6/etc.), even though Arduino-ESP32 doesn't guarantee these specific macro names across all board variants. Gate strictly on
defined(RX) && defined(TX)to prevent referencing undefined macros:🔧 Suggested fix
-#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` constexpr uint8_t hardwareRX = 3; constexpr uint8_t hardwareTX = 1; `#endif`
temporary workaround, to prevent that more than the possible 2 digital outputs are configured by users
Member
|
ToDO: rework chip identification code in settings_leds.htm WLED/wled00/data/settings_leds.htm Lines 66 to 71 in cffc57e ==> Instead of guessing the chip type from some parameters, we should simply add this info - possibly as a short string, similar to appendGPIOinfo Line 95 in cffc57e |
Member
|
@coderabbitai a user has reported problems with two ws2812b led strings on two pins
I want to know if this is a problem in the patched NeoPixelBus driver, or a problem in our handling of RMT channels in bus_manager.cpp and bus_wrapper.h. Please review both the WLED code for -C5 that's related to ws2812b led outputs, and the patched NeoPixelBus driver for -C5, and try to find the root cause of the user's problem. |
This comment was marked as duplicate.
This comment was marked as duplicate.
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 9495 🏁 Script executed: Length of output: 154 🏁 Script executed: Length of output: 5125 🏁 Script executed: Length of output: 135 🏁 Script executed: Length of output: 4504 🏁 Script executed: Length of output: 6314
🔍 AnalysisESP32-C5 Hardware LimitationFrom ESP-IDF 5.5.2 `#define` SOC_RMT_TX_CANDIDATES_PER_GROUP 2 /*!< Only 2 channels can transmit */
`#define` SOC_RMT_RX_CANDIDATES_PER_GROUP 2 /*!< Only 2 channels can receive */
`#define` SOC_RMT_CHANNELS_PER_GROUP 4 /*!< 4 total channels */Critical: ESP32-C5 has 4 RMT channels, but only channels 0 and 1 can transmit. Channels 2-3 are RX-only. WLED Code Status ✅Your WLED code is correct:
The Problem
|
Member
@coderabbitai the neopixelbus C5 fork can be reached with this commit: https://github.com/jonny190/NeoPixelBus/tree/103cfb585ef8efcdd6645ebb9938bf7012d093fd Please complete your analysis with an inspection of the source code in this branch. |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 285 🏁 Script executed: Length of output: 251 🏁 Script executed: Length of output: 254 🏁 Script executed: Length of output: 3643 🏁 Script executed: Length of output: 3110 🏁 Script executed: Length of output: 589 🏁 Script executed: Length of output: 3330 🏁 Script executed: Length of output: 1789 🏁 Script executed: Length of output: 2252
🐛 Critical Issues Found in NeoPixelBus C5 Fork1️⃣ Constructor Ignores Channel ParameterLocation: Line 70-75 in NeoEsp32RmtMethodBase(uint8_t pin, uint16_t pixelCount, size_t elementSize, size_t settingsSize,
NeoBusChannel channel = NeoBusChannel_0) :
_sizeData(pixelCount* elementSize + settingsSize),
_pin(pin),
_channel(NULL) // ❌ BUG: 'channel' parameter is NEVER used!
{
construct();
}WLED passes 2️⃣ Dynamic Channel Allocation Without RestrictionsLocation: Lines 104-116 in rmt_tx_channel_config_t config = {};
config.clk_src = RMT_CLK_SRC_DEFAULT;
config.gpio_num = static_cast<gpio_num_t>(_pin); // ✅ GPIO set
// ❌ NO channel specification!
config.mem_block_symbols = 192;
config.resolution_hz = T_SPEED::RmtTicksPerSecond;
config.trans_queue_depth = 4;
config.flags.invert_out = T_INVERTED::Inverted;
config.flags.with_dma = false;
ret += rmt_new_tx_channel(&config, &_channel); // ❌ Allocates ANY available channelThe problem: 3️⃣ Error Handling DisabledLocation: Lines 122-124 // if (ret) {
// AddLog(2,"RMT: initialized with error code: %u on pin: %u",ret, _pin);
// }Initialization failures are silently ignored, so users never see that the second bus failed. 4️⃣ ESP32-C5 Not in Supported Targets ListLocation: Line 362 `#if` defined(NEOPIXEL_ESP32_RMT_DEFAULT) || defined(CONFIG_IDF_TARGET_ESP32S2) || \
defined(CONFIG_IDF_TARGET_ESP32C3) || defined(CONFIG_IDF_TARGET_ESP32C6) || \
defined(CONFIG_IDF_TARGET_ESP32S3)ESP32-C5 is missing! This may cause it to fall back to a different code path. 📋 Why Second Output Fails
✅ Required Fixes for NeoPixelBus ForkFix 1: Use Channel Parameter (Quick Fix)// In constructor - store the channel number:
_channelNum(static_cast<uint8_t>(channel)) // Add this member
// In Initialize() - force specific channel allocation:
`#if` defined(CONFIG_IDF_TARGET_ESP32C5)
// C5 only supports TX on channels 0-1
if (_channelNum > 1) {
ESP_LOGE(TAG, "ESP32-C5 only supports RMT TX on channels 0-1!");
return;
}
`#endif`Fix 2: Add ESP32-C5 to Target List`#if` defined(NEOPIXEL_ESP32_RMT_DEFAULT) || defined(CONFIG_IDF_TARGET_ESP32S2) || \
defined(CONFIG_IDF_TARGET_ESP32C3) || defined(CONFIG_IDF_TARGET_ESP32C5) || \ // ADD THIS
defined(CONFIG_IDF_TARGET_ESP32C6) || defined(CONFIG_IDF_TARGET_ESP32S3)Fix 3: Enable Error Loggingret += rmt_new_tx_channel(&config, &_channel);
if (ret != ESP_OK) {
ESP_LOGE(TAG, "RMT channel init failed: 0x%x on pin %u", ret, _pin);
}Fix 4: Validate TX Capability`#if` defined(SOC_RMT_TX_CANDIDATES_PER_GROUP)
// Ensure we're using a TX-capable channel on C5/C6
if (_channelNum >= SOC_RMT_TX_CANDIDATES_PER_GROUP) {
ESP_LOGE(TAG, "Channel %u cannot transmit on this chip", _channelNum);
return;
}
`#endif`🔧 Temporary WLED WorkaroundUntil the NeoPixelBus fork is fixed, limit ESP32-C5 to 1 WS2812B output: In `#if` defined(CONFIG_IDF_TARGET_ESP32C3) || defined(CONFIG_IDF_TARGET_ESP32C6)
`#define` WLED_MAX_DIGITAL_CHANNELS 2
`#elif` defined(CONFIG_IDF_TARGET_ESP32C5)
`#define` WLED_MAX_DIGITAL_CHANNELS 1 // ⚠️ Driver bug: second channel failsAdd a warning comment in the UI or documentation that C5 currently supports only 1 digital output until the driver is fixed. 🎯 Action Items
The good news: WLED code is correct — this is purely a NeoPixelBus driver issue that can be fixed with the above changes! 🚀 🧠 Learnings used |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 221 🏁 Script executed: Length of output: 11426 🏁 Script executed: Length of output: 2389
🐛 Critical Bug in NeoEsp32RmtXMethod.hThe Problem:Line 70-73 (Constructor): NeoEsp32RmtMethodBase(..., NeoBusChannel channel = NeoBusChannel_0) :
_sizeData(...),
_pin(pin),
_channel(NULL) // ❌ BUG: Ignores the 'channel' parameter!Line 112 (Initialize): ret += rmt_new_tx_channel(&config, &_channel); // ❌ Auto-allocates without constraintsLines 120-122 (Error handling): // if (ret) {
// AddLog(2,"RMT: initialized with error code: %u on pin: %u",ret, _pin);
// }
// ❌ BUG: Error logging is commented out!🔍 What's Happening:
✅ The Fix:Option 1: Use the channel parameter (cleanest)// In constructor (line 70-73):
NeoEsp32RmtMethodBase(..., NeoBusChannel channel = NeoBusChannel_0) :
_sizeData(...),
_pin(pin),
_channel(NULL),
_channelNum(static_cast<uint8_t>(channel)) // Store channel number
{ ... }
// In Initialize() (line 105-112):
void Initialize()
{
esp_err_t ret = ESP_OK;
rmt_tx_channel_config_t config = {};
config.clk_src = RMT_CLK_SRC_DEFAULT;
config.gpio_num = static_cast<gpio_num_t>(_pin);
`#ifdef` CONFIG_IDF_TARGET_ESP32C5
// C5: Force TX-capable channels (0-1 only)
if (_channelNum > 1) {
ESP_LOGE(TAG, "ESP32-C5 only supports RMT TX channels 0-1");
return;
}
`#endif`
config.mem_block_symbols = 192;
config.resolution_hz = T_SPEED::RmtTicksPerSecond;
config.trans_queue_depth = 4;
config.flags.invert_out = T_INVERTED::Inverted;
config.flags.with_dma = false;
ret = rmt_new_tx_channel(&config, &_channel);
// CRITICAL: Enable error logging!
if (ret != ESP_OK) {
ESP_LOGE(TAG, "RMT channel %d init failed on pin %u: error %d",
_channelNum, _pin, ret);
return;
}
// ... rest of initialization
}Option 2: Add C5-specific validation (minimal change)// In Initialize(), after rmt_new_tx_channel:
ret = rmt_new_tx_channel(&config, &_channel);
`#ifdef` CONFIG_IDF_TARGET_ESP32C5
// Verify we got a TX-capable channel
// (ESP-IDF internal channel ID is opaque, so we check initialization success)
if (ret != ESP_OK) {
ESP_LOGE(TAG, "RMT TX channel allocation failed on pin %u", _pin);
return;
}
`#endif`
// Enable error checking everywhere:
if (ret != ESP_OK) {
ESP_LOGE(TAG, "RMT initialized with error code: %d on pin: %u", ret, _pin);
}📋 Immediate Actions:
Temporary workaround for users: Only use 1 WS2812B output until the driver is fixed. Would you like me to prepare a complete patch for the driver? 🧠 Learnings used |
Member
|
@jonny190 there seems to be a bug in your NPB fork, please check #5048 (comment). |
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
5 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
Summary by CodeRabbit
New Features
Bug Fixes / Compatibility
Chores