[esp32-C6 and esp-idf V5 branch] Fix ESP32-C6 GPIO pin validation and runtime compatibility #5114
Conversation
- Fix IPAddress comparison with 0U causing null pointer dereference in initConnection() - Add bounds checking for selectedWiFi index before array access - Add ESP32-C6 conditional for touch sensor exclusion (C6 has no capacitive touch) - Add deferResponse() wrapper for C6's patched ESPAsyncWebServer - Add early return in esp32RMTInvertIdle() for C6 (no RMT support) - Add ESP32-C6 BitBang bus creation in bus_wrapper.h - Update platformio.ini with proper C6 build flags and dependencies - Use patched FastLED, AsyncTCP, and ESPAsyncWebServer forks for C6 support
|
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 You can disable this status message by setting the WalkthroughThis PR adds ESP32-C6 hardware support to WLED with platform-specific conditionals, updates key dependencies (QuickESPNow, QuickDebug, AsyncTCP, GifDecoder), introduces a platform-abstracted response deferral mechanism, and improves WiFi connectivity index validation. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
|
This PR is all over the place. C6 is not even supported. Can you elaborate what you are trying to do here? |
There was a problem hiding this comment.
Hi @karan-vk,
Thanks for your contribution to our esp32-C6 activities.
I have a few questions and some modifications you did seems to be very temporary. Especially I would like to hear the opinion of @willmmiles about your changes related to neoPixelBus and espAsyncWebserver.
A few other changes seem to be cosmetic and not strictly need for ESP32-C6. Please check and remove unrelated modifications. My intention is simply to keep the scope of the C6 branch clean, to facilitate later merging into main.
|
|
||
| // RMT driver selection | ||
| #if !defined(WLED_USE_SHARED_RMT) && !defined(__riscv) | ||
| #if defined(CONFIG_IDF_TARGET_ESP32C6) |
There was a problem hiding this comment.
@willmmiles this is a kind of preprocessor hack, that actually hides " NeoEsp32BitBang" under the NeoEsp32RmtMethod label. Not sure if we should do it this way, what do you think?
wled00/fcn_declare.h
Outdated
| void serveSettings(AsyncWebServerRequest* request, bool post = false); | ||
| void serveSettingsJS(AsyncWebServerRequest* request); | ||
|
|
||
| // ESP32-C6 compatibility: deferResponse not available in patched ESPAsyncWebServer fork |
There was a problem hiding this comment.
I think I'd like to see a PR to our web server with any necessary fixes, not a removal of the new features.
There was a problem hiding this comment.
that's a good point - could be that the V5 branch is still making reference to my own fork, where i've fixed a few incompatibilities more than a year ago. If V5 is still using my fork, no problem i can make a PR to our webserver.
| } | ||
|
|
||
| if (!requestJSONBufferLock(17)) { | ||
| request->deferResponse(); |
There was a problem hiding this comment.
is this specific to C6? If unrelated, please revert.
wled00/network.cpp
Outdated
| DEBUG_PRINTF_P(PSTR("WiFi: Found %d SSIDs. @ %lus\n"), status, millis()/1000); | ||
| int rssi = -9999; | ||
| int selected = selectedWiFi; | ||
| int selected = (selectedWiFi < multiWiFi.size()) ? selectedWiFi : 0; // ensure valid starting index |
There was a problem hiding this comment.
what's the reason for this change?
There was a problem hiding this comment.
This fixes a crash that was occurring on ESP32-C6 during WiFi connection. The issue is that `selectedWiFi` (global `int`) can be out of bounds if the WiFi list changes, or if `findWiFi()` returns -1 during a scan. Without bounds checking, accessing `multiWiFi[selectedWiFi]` causes undefined behavior. This is a defensive fix that benefits all platforms but was discovered while testing C6.
wled00/wled.h
Outdated
|
|
||
| // server library objects | ||
| #if defined(CONFIG_IDF_TARGET_ESP32C6) | ||
| // ESP32-C6 uses patched ESPAsyncWebServer fork with simpler constructor |
There was a problem hiding this comment.
@willmmiles what do you think - is this specific to C6 or maybe specific for ESP-IDF v5?
| bool isConfig = false; | ||
|
|
||
| if (!requestJSONBufferLock(14)) { | ||
| request->deferResponse(); |
| bitbank2/AnimatedGIF@^1.4.7 | ||
| https://github.com/Aircoookie/GifDecoder#bc3af189b6b1e06946569f6b4287f0b79a860f8e | ||
| https://github.com/netmindz/esp_dmx/#esp-idf-v5-fixes | ||
| ${env.lib_deps} |
There was a problem hiding this comment.
this change may break other builds. Please explain why you deleted ${env.lib_deps}.
There was a problem hiding this comment.
The `${env.lib_deps}` was removed because it was causing dependency conflicts with the IDF5-specific libraries. The IDF5 section now explicitly lists all required dependencies to ensure compatibility. The key differences for IDF5 are:
- `AsyncTCP @ 3.4.7` instead of 3.4.6 (IDF5 compatible)
- Patched FastLED fork for C6
- Patched ESPAsyncWebServer v2.4.2
If this breaks other builds, I can restore `${env.lib_deps}` and find another way to handle the C6-specific dependencies. Would you prefer I split these into a C6-only section?
There was a problem hiding this comment.
Let me think about it - basicially we should follow the same approach as the "V4" builds - with an env specific to ESP-IDF V5, and a C6 specific one that "extends=" the V5 environment.
softhack007
changed the title
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
.vscode/extensions.json (1)
3-3: Revert unrelated VSCode extension change.As noted in the previous review, adding the PlatformIO IDE extension recommendation is unrelated to the ESP32-C6 hardware support objectives of this PR. Please revert this change.
platformio.ini (1)
265-268: Reconsider GifDecoder commit hash usage.As noted in the previous review (line 268), using a long commit hash
bc3af189b6b1e06946569f6b4287f0b79a860f8einstead of a stable version tag is unnecessary and not specific to ESP32-C6. This pattern appears in bothesp32_all_variantsandesp32_idf_V5sections, suggesting it is a general IDF5 update. Using commit hashes can make builds fragile and harder to maintain. Consider reverting to a published version tag (e.g.,@^1.x.y) or documenting why this specific commit is required.wled00/wled.h (1)
906-911: ESP32-C6 specific AsyncWebServer initialization.The conditional initialization correctly accommodates the simpler constructor in the patched ESPAsyncWebServer fork used for ESP32-C6. The compile-time guard ensures no runtime overhead.
Note: A previous reviewer asked whether this should be
CONFIG_IDF_TARGET_ESP32C6-specific or more broadly ESP-IDF v5-specific. If other IDF5 targets (e.g., future chips) use the same patched fork, this guard may need adjustment.wled00/bus_wrapper.h (1)
248-256: BitBang fallback for ESP32-C6 via macro redefinition.This preprocessor approach redefines
NeoEsp32RmtMethod(x)to expand toNeoEsp32BitBang##x##Methodfor ESP32-C6, allowing existing RMT bus type identifiers to transparently use BitBang. While unconventional, this approach minimizes code duplication across the large switch-case blocks. The comment at line 249 documents the rationale.
🧹 Nitpick comments (2)
wled00/network.cpp (1)
298-307: Hardenselectedindex initialization and simplify type handlingThe clamping to
0whenselectedWiFi≥multiWiFi.size()is a good safety improvement and answers the previous “why this change?” concern: it prevents out‑of‑range access when the stored index drifts beyond the configured WiFi list.To make this fully robust and avoid any signed/unsigned quirks, consider basing
selectedonsize_tand casting once, so negativeselectedWiFi(if it ever occurs) also falls back cleanly:- int rssi = -9999; - int selected = (selectedWiFi < multiWiFi.size()) ? selectedWiFi : 0; // ensure valid starting index + int rssi = -9999; + size_t selected = (static_cast<size_t>(selectedWiFi) < multiWiFi.size()) + ? static_cast<size_t>(selectedWiFi) + : 0; // ensure valid starting index @@ - // find the WiFi with the strongest signal (but keep priority of entry if signal difference is not big) - if (foundBSSID || (n < (unsigned)selected && WiFi.RSSI(o) > rssi-10) || WiFi.RSSI(o) > rssi) { + // find the WiFi with the strongest signal (but keep priority of entry if signal difference is not big) + if (foundBSSID || (n < selected && WiFi.RSSI(o) > rssi-10) || WiFi.RSSI(o) > rssi) {This keeps the new behavior but makes the index mathematically safe regardless of the underlying type of
selectedWiFiand removes the need for the cast inside the loop.wled00/const.h (1)
65-69: ESP32-C6 sharing the C3 digital-channel limits looks appropriateExtending the
CONFIG_IDF_TARGET_ESP32C3branch to includeCONFIG_IDF_TARGET_ESP32C6so both useWLED_MAX_DIGITAL_CHANNELS 2matches the rest of this PR (BitBang on C6, no parallel I2S) and keeps resource limits conservative. Comment text still mentions “2 RMT, 6 LEDC” but that’s cosmetic and can be updated later if desired.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
.vscode/extensions.json(1 hunks)platformio.ini(8 hunks)wled00/bus_manager.cpp(2 hunks)wled00/bus_wrapper.h(14 hunks)wled00/button.cpp(1 hunks)wled00/const.h(1 hunks)wled00/fcn_declare.h(1 hunks)wled00/json.cpp(1 hunks)wled00/network.cpp(1 hunks)wled00/pin_manager.cpp(1 hunks)wled00/set.cpp(1 hunks)wled00/wled.cpp(3 hunks)wled00/wled.h(1 hunks)wled00/wled_server.cpp(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
wled00/**/!(html_*)*.h
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation for non-generated C++ header files (.h)
Files:
wled00/fcn_declare.hwled00/wled.hwled00/bus_wrapper.hwled00/const.h
wled00/**/*.cpp
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation for C++ source files (.cpp)
Files:
wled00/set.cppwled00/json.cppwled00/bus_manager.cppwled00/wled.cppwled00/wled_server.cppwled00/pin_manager.cppwled00/network.cppwled00/button.cpp
platformio.ini
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use platformio.ini as the single source of truth for hardware build targets and settings
Files:
platformio.ini
🧠 Learnings (36)
📓 Common learnings
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: 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: 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: blazoncek
Repo: wled/WLED PR: 4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.199Z
Learning: ESP8266 and ESP32 platforms have different maximum segment name lengths in WLED, which can cause truncation when syncing segment names between devices. This platform difference affects the user experience when using the segment name sync feature.
Learnt from: ksedgwic
Repo: wled/WLED PR: 4883
File: usermods/usermod_v2_skystrip/rest_json_client.h:6-14
Timestamp: 2025-08-29T00:26:15.808Z
Learning: WLED uses a vendored ArduinoJson library (version 6) located at "src/dependencies/json/ArduinoJson-v6.h" which is included through wled.h. Usermods should not directly include ArduinoJson headers but instead rely on wled.h for ArduinoJson symbols. The standard pattern is to include wled.h and use JsonObject, JsonArray, DynamicJsonDocument, etc. without additional includes.
Learnt from: netmindz
Repo: wled/WLED PR: 5093
File: wled00/util.cpp:1159-1182
Timestamp: 2025-11-20T00:04:04.829Z
Learning: In WLED PR #5093, the deviceId feature is designed for opt-in usage reporting that tracks only version/upgrade information (non-behavioral data), not user activity patterns. The deterministic salt approach (MAC + "WLED" + chip model/revision) is acceptable for this limited use case, as correlating MAC addresses to version history represents minimal privacy risk compared to behavioral tracking.
Learnt from: willmmiles
Repo: wled/WLED PR: 4928
File: wled00/FX_fcn.cpp:1181-1181
Timestamp: 2025-09-18T02:59:33.666Z
Learning: In WLED, getFreeHeapSize() is a platform-agnostic function defined in fcn_declare.h that provides accurate free heap reporting across ESP32 and ESP8266 platforms. On ESP32, it uses heap_caps_get_free_size(MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT) instead of ESP.getFreeHeap() because the latter can include other memory types. On ESP8266, it uses ESP.getFreeHeap().
Learnt from: ksedgwic
Repo: wled/WLED PR: 4883
File: usermods/usermod_v2_skystrip/rest_json_client.cpp:40-46
Timestamp: 2025-08-29T00:22:34.115Z
Learning: For ESP32/ESP8266 HTTP requests in WLED, set timeouts on the underlying WiFiClient (using client.setTimeout()) before calling http_.begin(), rather than using HTTPClient::setTimeout(). This pattern is used successfully in the Klipper usermod to prevent connection hangs.
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: blazoncek
Repo: wled/WLED PR: 4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.200Z
Learning: In WLED, maximum segment name length varies by platform:
- ESP8266: 32 characters (WLED_MAX_SEGNAME_LEN = 32)
- ESP32: 64 characters (WLED_MAX_SEGNAME_LEN = 64)
This platform difference can cause truncation when syncing longer names from ESP32 to ESP8266. Additionally, the WLED UI has limitations regarding modified maximum segment name lengths.
📚 Learning: 2025-08-29T01:34:34.358Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4853
File: wled00/util.cpp:779-781
Timestamp: 2025-08-29T01:34:34.358Z
Learning: On ESP8266 systems, avoid adding no-op stub functions across translation units due to limited code memory constraints, as the compiler cannot inline away the function calls, resulting in wasteful memory usage.
Applied to files:
wled00/fcn_declare.hwled00/bus_manager.cppwled00/wled.hwled00/pin_manager.cppplatformio.ini
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to wled00/html_*.h : DO NOT edit generated embedded web header files (wled00/html_*.h)
Applied to files:
wled00/fcn_declare.hwled00/wled.hwled00/wled.cppwled00/const.h
📚 Learning: 2025-09-02T01:45:58.047Z
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.
Applied to files:
wled00/bus_manager.cppwled00/wled.hwled00/bus_wrapper.hwled00/pin_manager.cppplatformio.ini
📚 Learning: 2025-09-18T03:17:30.107Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4939
File: wled00/FX_fcn.cpp:1183-1194
Timestamp: 2025-09-18T03:17:30.107Z
Learning: In WLED bus management code, dead code branches in error handling (like unreachable else breaks) are intentionally preserved to accommodate potential future API changes, even when currently unreachable.
Applied to files:
wled00/bus_manager.cppwled00/bus_wrapper.hwled00/pin_manager.cpp
📚 Learning: 2025-09-18T03:17:30.107Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4939
File: wled00/FX_fcn.cpp:1183-1194
Timestamp: 2025-09-18T03:17:30.107Z
Learning: In WLED BusManager (wled00/FX_fcn.cpp), direct access to BusManager::busses is part of the intended public API design, not an encapsulation violation. The wrapper accessor methods are considered unnecessary obfuscation that should be removed in future refactoring.
Applied to files:
wled00/bus_manager.cppwled00/bus_wrapper.h
📚 Learning: 2025-09-16T18:08:42.848Z
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.
Applied to files:
wled00/wled.hwled00/bus_wrapper.hwled00/const.h
📚 Learning: 2025-02-19T12:43:34.199Z
Learnt from: blazoncek
Repo: wled/WLED PR: 4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.199Z
Learning: ESP8266 and ESP32 platforms have different maximum segment name lengths in WLED, which can cause truncation when syncing segment names between devices. This platform difference affects the user experience when using the segment name sync feature.
Applied to files:
wled00/wled.hwled00/const.hplatformio.ini
📚 Learning: 2025-11-14T13:37:30.955Z
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.
Applied to files:
wled00/wled.hwled00/wled.cppwled00/const.h
📚 Learning: 2025-11-14T13:37:11.994Z
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.
Applied to files:
wled00/wled.hwled00/wled.cppwled00/const.h
📚 Learning: 2025-11-16T19:40:46.260Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4926
File: wled00/FX.cpp:4727-4730
Timestamp: 2025-11-16T19:40:46.260Z
Learning: WLED AuroraWave (wled00/FX.cpp): wave_start and wave_end intentionally use int16_t; segments longer than 32k LEDs are not supported (bounded by MAX_LEDS), so widening to 32-bit is unnecessary.
Applied to files:
wled00/wled.hwled00/wled.cppwled00/const.h
📚 Learning: 2025-08-29T00:26:15.808Z
Learnt from: ksedgwic
Repo: wled/WLED PR: 4883
File: usermods/usermod_v2_skystrip/rest_json_client.h:6-14
Timestamp: 2025-08-29T00:26:15.808Z
Learning: WLED uses a vendored ArduinoJson library (version 6) located at "src/dependencies/json/ArduinoJson-v6.h" which is included through wled.h. Usermods should not directly include ArduinoJson headers but instead rely on wled.h for ArduinoJson symbols. The standard pattern is to include wled.h and use JsonObject, JsonArray, DynamicJsonDocument, etc. without additional includes.
Applied to files:
wled00/wled.hwled00/wled_server.cppplatformio.ini
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to wled00/data/** : For web UI changes, edit files only under wled00/data (not firmware or generated files)
Applied to files:
wled00/wled.h
📚 Learning: 2025-04-26T12:51:04.993Z
Learnt from: KrX3D
Repo: wled/WLED PR: 4664
File: wled00/syslog.cpp:50-51
Timestamp: 2025-04-26T12:51:04.993Z
Learning: In the WLED codebase, syslog configuration variables (syslogHost, syslogPort, syslogProtocol, syslogFacility, syslogSeverity) are defined as global variables in wled.h and are accessed directly in the SyslogPrinter implementation, rather than being class members.
Applied to files:
wled00/wled.h
📚 Learning: 2025-08-28T08:09:20.630Z
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.
Applied to files:
wled00/wled.hwled00/const.h
📚 Learning: 2025-08-29T00:22:34.115Z
Learnt from: ksedgwic
Repo: wled/WLED PR: 4883
File: usermods/usermod_v2_skystrip/rest_json_client.cpp:40-46
Timestamp: 2025-08-29T00:22:34.115Z
Learning: For ESP32/ESP8266 HTTP requests in WLED, set timeouts on the underlying WiFiClient (using client.setTimeout()) before calling http_.begin(), rather than using HTTPClient::setTimeout(). This pattern is used successfully in the Klipper usermod to prevent connection hangs.
Applied to files:
wled00/wled.h
📚 Learning: 2025-09-02T02:27:39.220Z
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.
Applied to files:
wled00/wled.hwled00/bus_wrapper.hwled00/pin_manager.cppwled00/const.hplatformio.ini
📚 Learning: 2025-09-21T20:44:09.502Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4928
File: wled00/bus_wrapper.h:1248-1275
Timestamp: 2025-09-21T20:44:09.502Z
Learning: In WLED bus_wrapper.h, the _useParallelI2S flag is used for selecting the correct function pointer (between parallel I2S and non-parallel I2S bus types) rather than for changing memory calculations. Both parallel and non-parallel I2S configurations have the same memory requirements, so memUsage() correctly doesn't differentiate based on this flag.
Applied to files:
wled00/bus_wrapper.hwled00/const.h
📚 Learning: 2025-09-02T01:56:43.841Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/include/NeoEsp32RmtHIMethod.h:173-180
Timestamp: 2025-09-02T01:56:43.841Z
Learning: willmmiles prefers to maintain consistency with upstream NeoPixelBus patterns (like unchecked malloc in construct() methods) rather than diverging until improvements are made upstream first, to minimize maintenance burden and keep the codebase aligned.
Applied to files:
wled00/bus_wrapper.h
📚 Learning: 2025-09-02T02:15:44.324Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/include/NeoEsp32RmtHIMethod.h:87-91
Timestamp: 2025-09-02T02:15:44.324Z
Learning: ESP_ERROR_CHECK_WITHOUT_ABORT_SILENT_TIMEOUT is part of the NeoPixelBus library API and can be safely depended upon when NeoPixelBus is a declared dependency.
Applied to files:
wled00/bus_wrapper.hplatformio.ini
📚 Learning: 2025-09-18T03:17:30.107Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4939
File: wled00/FX_fcn.cpp:1183-1194
Timestamp: 2025-09-18T03:17:30.107Z
Learning: In WLED finalizeInit() bus creation (wled00/FX_fcn.cpp), digitalCount is intentionally incremented even for placeholder buses to maintain consistent resource mapping across all bus configurations.
Applied to files:
wled00/bus_wrapper.hwled00/const.h
📚 Learning: 2025-09-28T09:53:42.670Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4939
File: wled00/data/index.js:2406-2413
Timestamp: 2025-09-28T09:53:42.670Z
Learning: In WLED bus management, bus objects can exist with valid configuration data even when isOK() returns false. The getPins() method can be safely called on buses that are not OK, as the bus object and its configuration exist independently of the isOK() status, which specifically indicates whether the NeoPixel bus was successfully created.
Applied to files:
wled00/bus_wrapper.h
📚 Learning: 2025-09-02T01:48:16.409Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/include/NeoEsp32RmtHIMethod.h:72-85
Timestamp: 2025-09-02T01:48:16.409Z
Learning: In NeoPixelBus driver code, ESP_ERROR_CHECK (which can abort) is used in destructors when driver uninstall failures indicate hardware is in an unsafe configuration, as such errors are considered unrecoverable hardware safety issues rather than typical software errors.
Applied to files:
wled00/bus_wrapper.h
📚 Learning: 2025-11-22T12:12:53.486Z
Learnt from: DedeHai
Repo: wled/WLED PR: 5105
File: wled00/FX.h:968-968
Timestamp: 2025-11-22T12:12:53.486Z
Learning: In WLED's WS2812FX class (wled00/FX.h), the _pixels array stores LED colors indexed by unmapped (logical) LED positions, not physical positions. The getPixelColor(n) function intentionally checks if the logical LED at index n maps to a valid physical position via getMappedPixelIndex(n), then accesses _pixels[n] with the unmapped index. If the mapping is invalid (e.g., -1 for gaps in LED maps, or out of bounds), it returns 0 (black). This allows peek preview to correctly show gaps in LED mappings as black pixels. The mapping from logical to physical positions is only applied when writing to the hardware bus, not when reading from the pixel buffer.
<!-- [add_learning]
Applied to files:
wled00/bus_wrapper.h
📚 Learning: 2025-09-11T04:36:42.869Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4754
File: usermods/user_fx/README.md:169-179
Timestamp: 2025-09-11T04:36:42.869Z
Learning: In WLED, SEGENV.data is already typed as uint8_t* and can be accessed directly without casting when storing per-pixel state data.
Applied to files:
wled00/bus_wrapper.h
📚 Learning: 2025-10-10T18:34:06.550Z
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.
Applied to files:
wled00/wled.cppwled00/const.h
📚 Learning: 2025-08-31T03:38:14.114Z
Learnt from: BobLoeffler68
Repo: wled/WLED PR: 4891
File: wled00/FX.cpp:3333-3349
Timestamp: 2025-08-31T03:38:14.114Z
Learning: WLED PacMan effect (wled00/FX.cpp): Keep pacmancharacters_t position fields as signed int (not int16_t). Maintainer preference (blazoncek) prioritizes avoiding potential overhead/regressions over minor RAM savings. Avoid type shrinking here unless memory pressure is demonstrated.
Applied to files:
wled00/wled.cpp
📚 Learning: 2025-04-26T19:19:07.600Z
Learnt from: blazoncek
Repo: wled/WLED PR: 4658
File: wled00/const.h:140-141
Timestamp: 2025-04-26T19:19:07.600Z
Learning: In WLED, the WLED_MAX_PANELS macro is intentionally defined as a fixed constant value (18) with no redefinition mechanism, making it "unoverridable" - there's no need for a static assertion to check its maximum value.
Applied to files:
wled00/const.h
📚 Learning: 2025-06-15T09:59:52.720Z
Learnt from: netmindz
Repo: wled/WLED PR: 4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Applied to files:
wled00/const.h
📚 Learning: 2025-10-20T09:38:51.997Z
Learnt from: blazoncek
Repo: wled/WLED PR: 4995
File: wled00/FX.cpp:5223-5226
Timestamp: 2025-10-20T09:38:51.997Z
Learning: WLED matrices: each dimension (SEG_W, SEG_H) is limited to ≤255; 256 or larger per side is not supported/feasible on ESP32, so effects should assume per-dimension max 255.
Applied to files:
wled00/const.h
📚 Learning: 2025-03-29T19:21:52.806Z
Learnt from: ComicoX
Repo: wled/WLED PR: 4626
File: wled00/button.cpp:116-118
Timestamp: 2025-03-29T19:21:52.806Z
Learning: ESP32-C6 does not have built-in touch sensors, unlike other ESP32 variants.
Applied to files:
wled00/button.cpp
📚 Learning: 2025-11-22T20:33:14.800Z
Learnt from: softhack007
Repo: wled/WLED PR: 5107
File: boards/adafruit_matrixportal_esp32s3.json:1-66
Timestamp: 2025-11-22T20:33:14.800Z
Learning: In WLED, board JSON files in the boards/ directory may reference partition files that don't exist or have slightly different names than actual files in tools/. This is intentional because platformio.ini or platformio_override.sample.ini explicitly override the partition file using the board_build.partitions setting, which takes precedence over the board JSON partition reference. This pattern exists in multiple local board definitions and should not be flagged as an issue.
Applied to files:
platformio.ini
📚 Learning: 2025-08-29T00:26:15.808Z
Learnt from: ksedgwic
Repo: wled/WLED PR: 4883
File: usermods/usermod_v2_skystrip/rest_json_client.h:6-14
Timestamp: 2025-08-29T00:26:15.808Z
Learning: In WLED projects, ArduinoJson.h is not directly included via #include <ArduinoJson.h> - the ArduinoJson symbols are made available through the WLED build system and wled.h transitive includes, so explicitly adding #include <ArduinoJson.h> is not necessary and may not work.
Applied to files:
platformio.ini
📚 Learning: 2025-07-06T16:15:24.051Z
Learnt from: Jason2866
Repo: wled/WLED PR: 4167
File: platformio.ini:427-430
Timestamp: 2025-07-06T16:15:24.051Z
Learning: In espressif Arduino framework, `-fno-lto` is set as a standard default in the `platformio-build.py` script. Therefore, using `-fno-lto` in `build_unflags` is not only valid but necessary to remove this default setting before enabling LTO with `-flto=auto` or `-flto=4` in `build_flags`.
Applied to files:
platformio.ini
📚 Learning: 2025-07-06T16:11:05.896Z
Learnt from: Jason2866
Repo: wled/WLED PR: 4167
File: platformio.ini:427-430
Timestamp: 2025-07-06T16:11:05.896Z
Learning: In PlatformIO, using `-fno-lto` in `build_unflags` is a valid pattern to remove potentially existing `-fno-lto` flags from the build system before adding `-flto=auto` in `build_flags`. This ensures that LTO can be enabled without conflicts from default build system flags that might disable LTO.
Applied to files:
platformio.ini
📚 Learning: 2025-02-19T12:43:34.200Z
Learnt from: blazoncek
Repo: wled/WLED PR: 4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.200Z
Learning: In WLED, maximum segment name length varies by platform:
- ESP8266: 32 characters (WLED_MAX_SEGNAME_LEN = 32)
- ESP32: 64 characters (WLED_MAX_SEGNAME_LEN = 64)
This platform difference can cause truncation when syncing longer names from ESP32 to ESP8266. Additionally, the WLED UI has limitations regarding modified maximum segment name lengths.
Applied to files:
platformio.ini
🧬 Code graph analysis (5)
wled00/set.cpp (1)
wled00/fcn_declare.h (1)
deferResponse(553-560)
wled00/json.cpp (1)
wled00/fcn_declare.h (1)
deferResponse(553-560)
wled00/wled.cpp (1)
wled00/network.cpp (2)
findWiFi(285-318)findWiFi(285-285)
wled00/wled_server.cpp (1)
wled00/fcn_declare.h (1)
deferResponse(553-560)
wled00/button.cpp (1)
usermods/audioreactive/audio_reactive.cpp (1)
ARDUINO_ARCH_ESP32(554-1707)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: wled_build / Build Environments (esp32c6dev_4MB)
🔇 Additional comments (20)
platformio.ini (6)
175-176: Clarify QuickESPNow version bump rationale.The dependency was updated from
gmag11/QuickESPNow @ ~0.7.0togmag11/QuickEspNow@^0.8.1, but the reason for this version bump is unclear. Is this required for IDF5 compatibility, ESP32-C6 support, or a general update? As noted in the previous review, this needs explicit justification.
195-195: Document the purpose of QuickDebug addition.Line 195 adds
gmag11/QuickDebug@^0.7.0but there is no documentation of its purpose. Is this required for ESP32-C6 debugging, IDF5 compatibility, or optional tooling? Please add an inline comment or justify this in the PR description.
304-319: IDF5 compatibility updates are well-structured.Lines 304–319 appropriately disable ESP-NOW for IDF5 via the
-D WLED_DISABLE_ESPNOWflag and add QuickEspNow tolib_ignoreto prevent conflicts. The dependency updates (AsyncTCP 3.4.7, patched FastLED, ESPAsyncWebServer fork, QuickDebug) are justified for IDF5 compatibility. The structure is sound.
374-382: C6-specific configuration is sound.Lines 374–382 correctly configure ESP32-C6 with BitBang method for NeoPixel (RMT unsupported), disable FastLED's RMT driver via
-D FASTLED_NO_FASTLED, and use a C6-patched AsyncTCP fork. The comment on line 381 appropriately clarifies that NeoPixelBus is already included via${env.lib_deps}to avoid duplication. Thelib_depsreference on line 382 correctly includes base dependencies.
396-396: Verify escaped WLED_RELEASE_NAME syntax in platformio.ini.Lines 396, 426, and 427 use escaped quotes in build flags:
-D WLED_RELEASE_NAME=\"ESP32-C6_8MB\". While this pattern appears elsewhere in the file (e.g., lines 459, 488), confirm that backslash-escaping is the correct syntax for platformio.ini build flags with string values, especially when used in macro definitions.Also applies to: 426-427
410-413: lib_ignore entries are well-justified.Lines 410–413 appropriately exclude IRremoteESP8266 (to speed compilation with
WLED_DISABLE_INFRARED) and QuickEspNow (ESP-NOW incompatibility with C6/IDF5). Inline comments clearly document the rationale.wled00/button.cpp (1)
110-119: Correctly exclude ESP32-C6 from touch-button pathsAdding
!defined(CONFIG_IDF_TARGET_ESP32C6)to the ESP32 touch block is consistent with ESP32‑C6 lacking touch sensors and prevents calling touch APIs on that target. Behavior for other variants (ESP32, S2, S3, C3) is unchanged.wled00/bus_manager.cpp (1)
1245-1279: Early-return for ESP32-C6 inesp32RMTInvertIdle()is consistent with BitBang-only C6 supportGuarding
BusManager::esp32RMTInvertIdle()with#if defined(CONFIG_IDF_TARGET_ESP32C6)and returning immediately on C6 cleanly disables RMT idle-level tweaking on a target where you’re not using RMT for LEDs, while preserving existing behavior on other ESP32 variants. That aligns with the BitBang routing for C6 introduced elsewhere in the PR.wled00/set.cpp (1)
652-657: Use ofdeferResponse(request)here is the right ESP32‑C6-compatible abstractionSwitching from
request->deferResponse()to the newdeferResponse(request)wrapper means:
- Non‑C6 targets still defer the response when the JSON buffer lock isn’t available.
- ESP32‑C6 now returns a clear HTTP 503 instead of relying on
deferResponse, which isn’t supported in the patched AsyncWebServer fork.That matches the intent of the helper in
fcn_declare.hand avoids C6‑specific build/runtime issues on this path.wled00/json.cpp (1)
1126-1129: Consistent deferral/error handling viadeferResponse(request)Aligning
serveJson()with the newdeferResponse(request)helper keeps behavior the same on existing targets (deferred JSON response when the lock is busy) and gives ESP32‑C6 a defined fallback (503 “Server busy”) instead of invoking an unsupporteddeferResponsemethod. This is consistent with the other call sites updated in the PR.wled00/fcn_declare.h (1)
551-560: InlinedeferResponsehelper cleanly encapsulates the ESP32‑C6 limitationThe new inline wrapper:
- Preserves
request->deferResponse()semantics on platforms where it exists.- Provides a sane fallback on ESP32‑C6 by immediately sending a 503 “Server busy” response, avoiding compile/runtime issues with the patched AsyncWebServer fork.
Centralizing this in
fcn_declare.halso keeps call sites simple and avoids scattering C6#ifguards. If you ever need different status codes or content types per caller, this will be the right place to extend.wled00/pin_manager.cpp (1)
235-242: ESP32‑C6 pin restrictions inisPinOk()match documented hardware usageThe new
CONFIG_IDF_TARGET_ESP32C6branch:
- Rejects GPIO 24–30 (SPI flash), and
- Optionally rejects GPIO 12 and 13 when USB‑CDC/DFU on boot is enabled (USB‑JTAG pins),
while leaving the remaining GPIO range to
digitalPinCanOutput()for final validation. This is in line with Espressif’s C6 pin recommendations and keeps flash/USB pins out of general use without over‑restricting strapping pins (8, 9, 15), which are only documented in comments.wled00/wled_server.cpp (1)
314-316: Platform-abstracted deferral wrapper looks correct.The switch from
request->deferResponse()to the free functiondeferResponse(request)correctly abstracts the platform difference. On ESP32-C6, wheredeferResponse()is unavailable in the patched ESPAsyncWebServer fork, the wrapper sends a 503 response as a fallback. This is an appropriate "server busy" response when the JSON buffer lock cannot be acquired.wled00/wled.cpp (3)
661-664: Good defensive bounds checking for WiFi index.The bounds check at line 662 correctly guards against
selectedWiFibeing out of range before accessingmultiWiFi[selectedWiFi]. SincefindWiFi()can return negative values during scanning (as shown in the relevant code snippet from network.cpp), clamping to 0 prevents out-of-bounds access.The explicit
IPAddress((uint32_t)0)comparison is more robust than comparing to literal zero.
798-800: Correct handling of findWiFi() return value.Capturing the result in a local variable and only assigning to
selectedWiFiwhen non-negative correctly prevents scan-in-progress status codes from corrupting the WiFi selection index.
837-839: Consistent pattern applied on disconnect.Same correct pattern as above for the disconnect reconnection path.
wled00/bus_wrapper.h (4)
234-234: Correct exclusion of ESP32-C6 from I2S1 bus types.ESP32-C6, like ESP32-C3, lacks I2S1 support, so excluding it from these type definitions is correct.
551-579: ESP32-C6 BitBang constructors use 2-arg form (no channel).BitBang methods don't require a channel parameter, so the simplified
new B_32_RN_*_*(len, pins[0])constructors are correct for C6. The#elseblock preserves the 3-arg form with(NeoBusChannel)channelfor other targets that use actual RMT.
1373-1376: ESP32-C6 limited to 2 transmit channels like C3.Grouping C6 with C3 for the 2-channel limit is correct per ESP32-C6 hardware capabilities.
453-466: Consistent I2S1/parallel exclusions for C6.All I2S1 and parallel I2S code paths correctly exclude ESP32-C6 alongside ESP32-C3. The guards are applied consistently across
begin(),create(),show(),canShow(),setPixelColor(),getPixelColor(),cleanup(),getDataSize(), andmemUsage().
|
sorry, my bad. thanks for explaining I did not notice its not for main branch and only saw changes unrelated to the description. |
- Revert VSCode extension changes - Remove unused QuickDebug dependency - Revert GifDecoder hash to short form - Harden selectedWiFi index calculation in network.cpp
- Upgrade to newer ESPAsyncWebServer that has deferResponse() and queue features - Remove deferResponse() wrapper function from fcn_declare.h - Remove AsyncWebServer constructor conditional from wled.h - Revert json.cpp, set.cpp, wled_server.cpp to use native deferResponse() This addresses @willmmiles feedback to use the proper library instead of workarounds.
|
Switched to Aircoookie/ESPAsyncWebServer#v2.4.2 which has deferResponse() built-in - removed the wrapper code. Build works! |
|
Tested with esp32_wrover build - not breaking other builds. |
|
Btw, I think we can move from platformio to pioarduino. C6 and P4 are not officially supported in platformio. But pioarduino does support the new boards. |
- esp32c6 section now properly extends esp32_idf_V5 like other variants (S2, C3) - Removed duplicate lib_deps, now inherits from esp32_idf_V5 - Fixed comment typo (ESP32-C3 -> ESP32-C6) - Use full GifDecoder hash for git fetch compatibility Tested: esp32c6dev_4MB and esp32_wrover both build successfully.
|
Restructured esp32c6 to properly extend esp32_idf_V5 like the other variants. Tested both esp32c6dev_4MB and esp32_wrover - both build fine. |
|
@karan-vk I have updated the base Sorry for the re-work, my goal is still to merge your changes, but they need to be compatible with our latest Edit: there might be errors in |
4 participants
Summary
Problem
The V5-C6 branch had several issues preventing proper operation on ESP32-C6:
Incorrect GPIO restrictions: The
isPinOk()function inpin_manager.cppwas missing ESP32-C6 specific handling, causing it to fall through to classic ESP32 logic which blocks GPIO 6-11 as "SPI flash pins". On ESP32-C6, the flash pins are actually GPIO 24-30, so GPIO 8 (and other low-numbered GPIOs) should be available for LED configuration.Runtime crashes: IPAddress comparison with
0Ucausing issues, array bounds violations withselectedWiFiindex.Missing platform exclusions: ESP32-C6 does not support capacitive touch sensors, RMT for NeoPixel (uses BitBang), or I2S1 parallel buses.
ESPAsyncWebServer API differences: The patched fork for C6 does not have
deferResponse().Changes
Pin Manager (
pin_manager.cpp)#elif defined(CONFIG_IDF_TARGET_ESP32C6)block toisPinOk()Bus Manager & Wrapper
esp32RMTInvertIdle()for C6Runtime Fixes
wled.cppto avoid null pointer issuesselectedWiFiindexdeferResponse()wrapper function for C6 compatibilityBuild Configuration (
platformio.ini)Features NOT Working on ESP32-C6
The following features are currently disabled or non-functional on ESP32-C6:
Hardware Limitations (ESP32-C6 vs other ESP32 variants)
Software/Library Limitations (may be resolved in future)
Testing
Successfully tested and verified working on:
Both boards now:
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.