Matter over WiFi (esp-idf component inside an arduino-esp32 project)#5456
Matter over WiFi (esp-idf component inside an arduino-esp32 project)#5456
Conversation
- New usermod: usermods/usermod_v2_matter/usermod_v2_matter.h Exposes WLED as a Matter Extended Color Light with On/Off, Level Control, and Color Control clusters. WiFi-only commissioning (no BLE required). - Register usermod in wled00/usermods_list.cpp - Add USERMOD_ID_MATTER (95) in wled00/const.h - Add esp32s3_matter_wifi build environment in platformio.ini - Add readme documentation for the usermod Agent-Logs-Url: https://github.com/MoonModules/WLED-MM/sessions/8fdc84b5-d880-40fc-9352-e684ff247509 Co-authored-by: netmindz <442066+netmindz@users.noreply.github.com>
…fy brightness comment Agent-Logs-Url: https://github.com/MoonModules/WLED-MM/sessions/8fdc84b5-d880-40fc-9352-e684ff247509 Co-authored-by: netmindz <442066+netmindz@users.noreply.github.com>
…atter IDF component
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdds Matter (Project CHIP) WiFi-only smart home support to WLED via a new Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
netmindz
added
enhancement
connectivity
| if sections_ld_path is None: | ||
| build_dir = Path(env.subst("$BUILD_DIR")) | ||
| candidate = build_dir / "sections.ld" | ||
| if candidate.exists(): | ||
| sections_ld_path = candidate |
There was a problem hiding this comment.
This was the fix needed for when doing an espidf+arduino build @willmmiles
…pdate generate_embed_files.py to return generated file paths
| @@ -0,0 +1,7 @@ | |||
| { | |||
| "name": "usermod_v2_matter", | |||
There was a problem hiding this comment.
I think we should abandon the "usermod_v2" prefix. If we must prefix something to disambiguate it from other libraries, I suggest something starting with "wled-" as that's what the usermod identification layer uses as a fallback.
| WiFi.mode(WIFI_MODE_NULL); | ||
| apActive = false; // the AP is physically torn down by WIFI_MODE_NULL | ||
| delay(5); // give the WiFi stack time to complete the mode transition | ||
| if (!externalWiFiManager) { |
There was a problem hiding this comment.
Is this change correct? Do we no longer need to force a reset in some way to apply the new hostname? Or does this need a TODO to get fixed?
There was a problem hiding this comment.
It's a tradeoff, I'll need to see what the impact on the hostname is, but resetting back to none was breaking matter support. We might be and to change the ordering such that matter starts later, especially as I've changed it to not try and run when in AP mode
There was a problem hiding this comment.
Right, I understand -- I'm suggesting that if there's a tradeoff being made or further work to be done, it'd be good to have a TODO marker in the comments. Move fast and break things is fine sometimes, but I think it's really helpful to leave behind notes so we aren't surprised in the future that some old bug has been re-introduced but not re-examined.
There was a problem hiding this comment.
Clever. I wasn't able to get a good solution last time I tried to (get the AI to) dig in to this. I guess we're getting it used to monkey-patching platformio now ... ;)
We might want to consider offering this back up to pioarduino; there's another complex middleware workaround in the framework build scripts. Really I'm coming to believe that platformio's middleware API is a minefield...
wled00/idf_component.yml
Outdated
There was a problem hiding this comment.
The comment says this file wasn't supposed to be checked in?
|
@coderabbitai review |
|
@coderabbitai so even an 3MB program partioning will most likely not be enough, right? If we assume 1.6MB for the WLED app itself (esp-idf and arduino-esp32 included), how much space would be left for the LittleFS filesystem? |
|
Tip For best results, initiate chat on the files or code changes.
Combined Binary Size Estimate
Even the optimistic lower bound of ~3.1 MB exceeds a 3 MB partition slot, and that's before any padding/alignment rounding the linker applies. What's Left for LittleFS on 8 MB Flash?On 8 MB flash with OTA support, the partition budget looks roughly like:
So with OTA enabled, LittleFS is essentially squeezed out on 8 MB flash — there's barely 0–512 KB remaining depending on exactly how tightly the app partitions are sized. This is a significant concern for WLED, which uses LittleFS for presets, playlists, WLED configs, and web UI file storage. Without OTA (single factory partition), the picture improves greatly:
Arduino-esp32 provides an "8MB no OTA with Storage" example layout where a 2 MB factory app partition leaves ~5 MB for SPIFFS, but with a 3.5–4 MB app binary, even that layout would require a custom partition table. Bottom Line
This is an important hardware constraint that should be prominently documented — perhaps only certain ESP32-S3 modules with 16 MB flash are genuinely practical targets for this usermod. 🧠 Learnings used |
|
@coderabbitai please research the web for any lightweight alternatives to the espressif matter components suggested in this PR. What are the best candidates that still support LED specific Matter profiles, and Matter-over-wifi? Please provide an expected flash footprint (rom = rough order of magnitude) with each option. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain
🌐 Web query: 💡 Result: Reality check (ESP32 + Matter-over-Wi‑Fi)On ESP32, Practical “lighter than esp_matter” options (still CHIP underneath)
If “alternative” can include changing silicon (often the real answer for footprint)If your blocker is flash/RAM budget more than “SDK ergonomics,” moving to a Wi‑Fi Matter SoC with a vendor-tuned port can be the quickest path. For example, NXP publishes concrete footprints for “Matter over Wi‑Fi (with BLE)” builds:
What to do if you must stay on ESP32If your goal is strictly smaller flash, the “wins” usually come from feature removal rather than swapping SDKs:
If you tell me:
Sources 🌐 Web query: 💡 Result:
Sources Citations:
🌐 Web query: 💡 Result: If you want something “lighter than Practical options on ESP32
“Alternatives” that are real, but not for ESP32 targets
If you tell me (a) which ESP32 variant (C3/C6/S3/H2, etc.), (b) Wi‑Fi vs Thread, and (c) your flash/RAM budget, I can narrow this to the most “lightweight” viable route (often meaning minimal device type + feature trimming, not a totally different stack). Sources: Citations:
TL;DR: There is no production-ready "lightweight" alternative that avoids the CHIP/connectedhomeip coreOn ESP32, Candidate 1: Arduino-ESP32 Matter ("Matter Minimum")
Candidate 2: esp-lowcode-matter (Espressif, "lightweight" experience)
Candidate 3: Amazon ACK SDK for Matter
Candidate 4:
|
| Option | LED Profiles | WiFi-only | Flash ROM (ROM) | ESP32-S3 compatible | Status |
|---|---|---|---|---|---|
esp_matter (current PR) |
✅ Full color | ✅ | ~1.5–2 MB added | ✅ | Production |
| Arduino-ESP32 Matter | ✅ Full color | ✅ | ~3 MB app partition | ✅ | Production |
| esp-lowcode-matter | ✅ | ✅ | Unknown (lighter API, same CHIP) | ❌ C6 only | Production |
| ACK SDK for Matter | ✅ | ✅ | ≥4 MB minimum | ❌ C3/ESP8685 only | Production |
rs-matter (Rust) |
✅ (spec-compliant) | ✅ | Unknown, potentially much smaller | ✅ (via esp-idf bridge) | Experimental |
| NXP RW61x (reference) | ✅ | ✅ | ~1.3 MB | ❌ Different silicon | Production |
Practical recommendation for WLED
For the near term, there is no drop-in lightweight replacement — all production ESP32 Matter paths are CHIP-based and carry the same ~1.5–2 MB overhead. The best flash-saving strategies within the current approach remain: pruning unused clusters, disabling BLE commissioning (already done in this PR), and using CONFIG_ESP_MATTER_MAX_DYNAMIC_ENDPOINT_COUNT=2. The rs-matter Rust stack is the only realistic path to a materially smaller implementation, but it is experimental. 16 MB flash ESP32-S3 modules appear to be the most practical hardware target for a fully-functional WLED + Matter build with OTA and LittleFS preserved.
🧠 Learnings used
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: softhack007
Repo: wled/WLED PR: 5048
File: wled00/set.cpp:551-555
Timestamp: 2026-03-27T21:00:25.902Z
Learning: In WLED PR `#5048` (wled00/set.cpp lines 551-555), the CONFIG_IDF_TARGET_ESP32C5 block that unconditionally forces ntpEnabled = false is an intentional **temporary hotfix** by softhack007 for a known ESP32-C5 crash: `assert failed: udp_new_ip_type udp.c:1278 (Required to lock TCPIP core functionality!)`. Do not flag this as a permanent design issue; the TODO comment in the code already notes it should be resolved properly once the underlying IDF/TCPIP bug on C5 is fixed. A future permanent solution should use a target capability flag rather than overwriting the user's setting.
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: softhack007
Repo: wled/WLED PR: 5048
File: wled00/cfg.cpp:673-673
Timestamp: 2026-03-27T21:17:51.985Z
Learning: In WLED PR `#5048` (wled00/cfg.cpp), the NTP crash on ESP32-C5 (pioarduino + IDF 5.5.x) is caused by both CONFIG_LWIP_TCPIP_CORE_LOCKING=y AND CONFIG_LWIP_CHECK_THREAD_SAFETY=y being enabled by default in the pioarduino/arduino-esp32 IDF 5.x sdkconfig. The tasmota core (IDF 5.3.x, used for C6) likely has CONFIG_LWIP_CHECK_THREAD_SAFETY=n, which is why the same violation does not crash on C6. Since pioarduino already has TCPIP_CORE_LOCKING enabled, wrapping ntpUdp.begin() (and any other raw lwIP API call) with LOCK_TCPIP_CORE()/UNLOCK_TCPIP_CORE() from lwip/tcpip.h is directly applicable without sdkconfig changes in platformio.ini.
Learnt from: softhack007
Repo: wled/WLED PR: 5048
File: wled00/cfg.cpp:673-673
Timestamp: 2026-03-27T21:17:51.985Z
Learning: In WLED PR `#5048`, the NTP/UDP crash on ESP32-C5 (pioarduino IDF 5.5.x): CONFIG_LWIP_TCPIP_CORE_LOCKING defaults to n (disabled) in arduino-esp32/pioarduino, while CONFIG_LWIP_CHECK_THREAD_SAFETY=y is what actually causes the "Required to lock TCPIP core functionality!" assertion. LOCK_TCPIP_CORE() / UNLOCK_TCPIP_CORE() macros are only functional when CONFIG_LWIP_TCPIP_CORE_LOCKING=y; using them without enabling that config has no effect. The correct fix without sdkconfig changes is to use tcpip_callback() to schedule ntpUdp.begin() (and other raw lwIP API calls) on the TCPIP thread, which works regardless of the locking mode setting. The tasmota core (IDF 5.3.x, used for C6) likely has CONFIG_LWIP_CHECK_THREAD_SAFETY=n, which is why the same violation does not crash on C6.
Learnt from: softhack007
Repo: wled/WLED PR: 4838
File: platformio.ini:149-150
Timestamp: 2026-03-29T16:47:56.452Z
Learning: In WLED PR `#4838` (ESP-IDF V5 branch, platformio.ini): The original FastLED library has been replaced with a custom `fastled-slim` fork. As a result, the `[v5_pioarduino_workaround]` section (containing `lib_archive = yes`) and the commented-out `post:pio-scripts/fastled_cxx_workaround.py` extra script are intentional dead code pending removal after back-to-back testing. Do NOT flag `lib_archive = yes` in `[v5_pioarduino_workaround]` as unreferenced or suggest moving it to active env blocks — the entire section is slated for removal.
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-03-15T13:21:32.008Z
Learning: In WLED code reviews (and generally), when a PR discussion involves "is this worth doing?" uncertainty about a proposed reliability, safety, or data-integrity mechanism (e.g. CRC checks, backups, power-loss protection), suggest performing a software FMEA (Failure Mode and Effects Analysis). The FMEA should: enumerate failure modes for the feared event, assess each mitigation's effectiveness per failure mode, note common-cause/common-mode failures, and rate credibility for the typical WLED use case. This was successfully used in PR `#5421` to evaluate CRC-based config file protection and was well-received by the WLED maintainers (softhack007 explicitly requested this be remembered for 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, verify that file operations (especially file.open()) respect LittleFS filename limitations. Assume default WLED configuration with LittleFS default filename limit of 255 bytes. Do not assume extreme configuration values like WLED_MAX_SEGNAME_LEN = 512 which would not be standard configurations.
Learnt from: willmmiles
Repo: wled/WLED PR: 5462
File: wled00/json.cpp:1189-1198
Timestamp: 2026-03-30T15:32:02.808Z
Learning: In WLED's `respondModeData()` (wled00/json.cpp), the 256-character `lineBuffer` limit for effect descriptor strings (getModeData) is an intentional constraint that matches the same 256-char limit used ~6 other places in the codebase. It is not new to this PR and should not be flagged as a regression. Lifting it requires a future refactor (better type system or dynamic buffer).
Learnt from: KrX3D
Repo: wled/WLED PR: 4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.cpp:3-5
Timestamp: 2026-02-18T00:11:02.403Z
Learning: In PR `#4585`, the seven_segment_display_reloaded_v2 usermod intentionally uses `#error` to require MQTT (`#ifdef WLED_DISABLE_MQTT `#error` "This user mod requires MQTT to be enabled." `#endif``) because the integration flow has not been tested or validated without MQTT, and failing fast at compile time is preferred over allowing potentially broken builds until a full MQTT-optional refactor is completed.
Learnt from: ksedgwic
Repo: wled/WLED PR: 4883
File: usermods/usermod_v2_skystrip/open_weather_map_source.cpp:13-19
Timestamp: 2025-08-29T19:51:15.511Z
Learning: On ESP8266/ESP32 devices with limited heap, using HTTPS for API calls that return large JSON responses may not be feasible due to the combined memory requirements of TLS handshake and JSON parsing. HTTP may be necessary despite security concerns.
Learnt from: ksedgwic
Repo: wled/WLED PR: 4919
File: usermods/usermod_v2_departstrip/readme.md:39-49
Timestamp: 2025-09-09T23:21:00.549Z
Learning: For the DepartStrip usermod on ESP8266/ESP32 devices, HTTP is preferred over HTTPS in documentation examples and templates due to heap memory constraints. The combined memory requirements of TLS handshake and JSON parsing for transit API responses can exceed available memory on these embedded devices.
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: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-28T14:18:55.599Z
Learning: Hardware builds require appropriate ESP32/ESP8266 development board for flashing firmware
Learnt from: ksedgwic
Repo: wled/WLED PR: 4919
File: usermods/usermod_v2_departstrip/readme.md:15-23
Timestamp: 2025-09-09T23:20:44.936Z
Learning: For the DepartStrip usermod on ESP8266/ESP32 devices, HTTP is used instead of HTTPS for API endpoints due to heap memory constraints. HTTPS would require too much memory for the combined TLS handshake and JSON parsing operations on these embedded devices, making HTTP a necessary compromise despite security considerations.
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: 5048
File: wled00/wled_metadata.cpp:6-8
Timestamp: 2026-03-27T21:02:06.756Z
Learning: In WLED PR `#5048` (pio-scripts/set_metadata.py + wled00/wled_metadata.cpp): The hardcoded `#define WLED_VERSION 16.0.0-alphaV5` in `wled_metadata.cpp` is an intentional **temporary hotfix** by softhack007. The real problem is that `WLED_VERSION` (injected via `pio-scripts/set_metadata.py` as a CPPDEFINE) is not reaching `wled_metadata.cpp` at compile time. The set_metadata.py change in this PR switched from `env.Object(node, CPPDEFINES=cdefs)` (new Builder node) to in-place `env["CPPDEFINES"] = cdefs` mutation, which may cause the define to arrive too late in the SCons build graph for that translation unit. The TODO comment in the code already marks this for removal. Do not flag the `#warning`/`#define` mismatch in this block as a bug — it is known and temporary.
Learnt from: softhack007
Repo: wled/WLED PR: 5048
File: wled00/bus_manager.cpp:549-556
Timestamp: 2026-02-11T16:20:02.872Z
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
platformio.ini (1)
787-849: New Matter environment configuration looks well-structured.The
esp32s3_matter_wifienvironment correctly:
- Uses dual
arduino, espidfframework for Matter SDK integration- Ignores the PlatformIO
Matterlibrary (resolved via IDF Component Manager instead)- Avoids defining
CONFIG_BT_ENABLEDto prevent#ifdeffalse matches- Sets
CONFIG_RENDEZVOUS_MODE_WIFI=1for WiFi-only commissioningOther ESP32-S3 environments include
-D WLED_WATCHDOG_TIMEOUT=0; consider adding this flag to avoid watchdog resets during lengthy Matter operations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platformio.ini` around lines 787 - 849, The esp32s3_matter_wifi environment is missing the WLED watchdog disable flag used elsewhere; add the -D WLED_WATCHDOG_TIMEOUT=0 entry to the build_flags block for the esp32s3_matter_wifi environment (the same build_flags list that contains -D WLED_DEBUG, -D CONFIG_RENDEZVOUS_MODE_WIFI=1, etc.) so long-running Matter operations won't trigger watchdog resets.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@platformio.ini`:
- Around line 787-849: The esp32s3_matter_wifi environment is missing the WLED
watchdog disable flag used elsewhere; add the -D WLED_WATCHDOG_TIMEOUT=0 entry
to the build_flags block for the esp32s3_matter_wifi environment (the same
build_flags list that contains -D WLED_DEBUG, -D CONFIG_RENDEZVOUS_MODE_WIFI=1,
etc.) so long-running Matter operations won't trigger watchdog resets.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 64213d91-e603-40c8-856d-23019650cd66
📒 Files selected for processing (15)
.gitignoreCMakeLists.txtplatformio.iniusermods/matter_over_wifi/generate_embed_files.pyusermods/matter_over_wifi/idf_component.ymlusermods/matter_over_wifi/library.jsonusermods/matter_over_wifi/matter_gcc14_compat.husermods/matter_over_wifi/readme.mdusermods/matter_over_wifi/research.mdusermods/matter_over_wifi/setup_matter_component.pyusermods/matter_over_wifi/usermod_matter.cppusermods/matter_over_wifi/usermod_matter.hwled00/const.hwled00/wled.cppwled00/wled.h
✅ Files skipped from review due to trivial changes (4)
- .gitignore
- usermods/matter_over_wifi/library.json
- usermods/matter_over_wifi/idf_component.yml
- usermods/matter_over_wifi/readme.md
🚧 Files skipped from review as they are similar to previous changes (3)
- wled00/wled.cpp
- wled00/const.h
- CMakeLists.txt
|
@coderabbitai I've heard that the TASMOTA project has built their own matter integration only using their scripting language "berry". Please review the TASMOTA implementation of matter, including any limitations or special dependencies that might be different from WLED. Assuming that we could either integrate the same |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainGreat question — let me research the Tasmota Matter/Berry implementation in detail.
Tasmota's Matter Implementation: The "Berry" ApproachWhat Tasmota Actually Did (Key Insight)Tasmota's Matter implementation is built using Berry, a lightweight scripting language embedded in Tasmota. The Matter subsystem is implemented as a Berry native module that is conditionally compiled based on the Critically: Tasmota did NOT use the How Berry Works (and Why It's Flash-Efficient)All Matter support code is located in This "solidification" technique is key: the code is almost entirely in Berry code solidified in Flash, so the impact on RAM is very low. Most of Berry code is solidified in Flash, so the initial RAM footprint is very low (a few KB). The RAM usage starts at ~10KB and will be later optimized. Tasmota's Crypto Strategy (The Other Big Saving)Rather than linking CHIP's massive TLS/CASE/PASE crypto stack, Tasmota uses BearSSL: on ESP32, BearSSL provides a much lighter footprint than MbedTLS (~45KB instead of ~150KB) and continues to be used by Tasmota. Their Matter commissioning crypto (PBKDF2, EC) is implemented as native C modules exposed to Berry — not a dependency on the CHIP SDK. Observed Flash Impact in TasmotaWith Tasmota 13.0, support for Matter is included by default, which means that the Tasmota binaries now exceed the size of the two equal-sized partitions from before the safeboot partitioning scheme. A user report shows: partition Limitations vs WLEDTasmota cannot be Matter certified; it uses development vendor IDs, which typically raise user warnings when commissioning the device. ESP8266 is directly not supported because of limited memory resources and lack of Berry support. Tasmota supports Matter over IP (Wi-Fi or Ethernet) as an end-device. This means you can connect a Tasmota device to a Matter gateway using standard and well-defined protocols. WiFi-only commissioning is supported — BLE is not needed: in Matter, BLE is optional and mostly used to configure WiFi credentials with a mobile app. This is not needed in Tasmota, which has its own way of configuring WiFi. Thread requires a specific radio and is not supported by ESP32 devices. Berry Scripting is only supported on Tasmota32 for ESP32 — no ESP8266 support, but WLED on V5 is already ESP32-only for Matter. Flash Size Estimates for Each PathOption A: Embed Berry VM + Port Tasmota's Matter Scripts into WLED
Feasibility: Moderate engineering effort. Berry is a standalone C99 library; integrating it into a non-Tasmota codebase is realistic. However, all of Tasmota's Matter native module bindings (crypto, NVS namespaces, UDP socket layer, mDNS) would need to be re-plumbed against WLED's infrastructure rather than Tasmota's. The Matter Berry scripts would need adaptation. The result would still carry the "development vendor ID" non-certification limitation. Option B: Extract Tasmota Matter Logic to Pure C++ (No Berry Runtime)
Feasibility: Very high engineering effort — essentially re-implementing what Tasmota took significant time to build. But this path yields the smallest possible flash footprint and the cleanest integration with WLED's C++ architecture. Tasmota's Berry source serves as a specification/reference. The same vendor ID non-certification limitation applies. Summary Comparison
Bottom line: Tasmota's custom Berry-based Matter stack is genuinely ~2–3× more flash-efficient than
How Tasmota Implements MatterThe Key Innovation: No connectedhomeip C++ SDKUnlike the current PR (which links the full The Berry code is located in the embedded directory. The code is compiled into bytecode and the bytecode is stored in Flash. This avoids consuming RAM, which is a very precious resource on ESP32. Berry Runtime CharacteristicsBerry Scripting is included in all tasmota32 builds. It is NOT supported on ESP8266. The RAM usage starts at ~10KB and will be later optimized. Berry uses PSRAM on ESP32 if available. For reference on the bare VM cost: an independent port (OpenBeken) measured the interpreter adds about 38 KiB of flash and needs under 4 KiB dynamic RAM during execution. Tasmota's version is larger due to all the C native modules (crypto, UDP, NVS, mDNS) that Matter needs. Crypto Strategy (The Other Big Saving vs. esp_matter)Rather than linking CHIP's massive TLS stack, Tasmota built custom C implementations exposed to Berry: full support of encryption via BearSSL without increased size that mbedTLS would have implied — SHA256, AES_CCM 256, HKDF_SHA256, HMAC_SHA256, PKKDF2 with HMAC SHA256 key derivation, SPAKE2+, ECDH with P256 (secp256r1). This is a substantially lighter crypto stack compared to CHIP's full mbedTLS. Flash Size Data Points (Tasmota + Matter)The most directly useful data: Partition app0 1856 KB, Partition app1* 1856 KB (used 82%), Partition fs 320 KB. That means the full Tasmota binary including Berry VM, all C drivers, web UI, MQTT, and Matter = ~1,522 KB total, fitting comfortably on 4 MB flash with OTA and 320 KB LittleFS. With Tasmota 13.0, support for Matter is now included by default, which means that the Tasmota binaries now exceed the size of the two equal-sized partitions from before the safeboot partitioning scheme. Their solution was a new variant called "Safeboot": it contains a minimal version of Tasmota used only for OTA, and a normal firmware. The main advantage is that it provides 1024 KB of additional storage for firmware and/or filesystem. Key perspective for WLED: softhack007 estimated WLED V5 at ~1.6 MB — already slightly larger than the entire Tasmota firmware with all features plus Matter. This underlines just how heavy the current PR's Limitations vs. WLEDTasmota cannot be Matter certified; it uses development vendor IDs, which typically raise user warnings when commissioning the device. This same limitation applies to the current PR. More specifically for Google Home: Google Home will not accept devices using a test Vendor ID like Tasmota does. To pair Tasmota with Google Home you will need a Matter compatible Google device and the following procedure to enable Tasmota for your Google Home account. Developers must register a project in the Google Home Developer Console — not a friction-free flow for end users. Thread requires a specific radio and is not supported by ESP32 devices; it would require an additional, separate MCU. Tasmota devices with dual active networks (Ethernet + WiFi) are not yet supported. You need to disable WiFi or Ethernet. Regarding BLE: in Matter, BLE is optional and mostly used to configure WiFi credentials with a mobile app. This is not needed in Tasmota, which has its own way of configuring WiFi. However, this means some Matter hubs (e.g. Homey) that require BLE device discovery will not find Tasmota/WLED devices. Light cluster mapping: lights with 1/2/3 channels are mapped to Light 1 Dimmer / Light 2 CT / Light 3 RGB endpoint types. Flash Estimates for WLED IntegrationOption A: Embed Berry VM + Port Tasmota's Matter Scripts into WLED
Key integration challenges: All of Tasmota's Berry native C bindings assume the Tasmota event loop, timer system, and MQTT architecture. These would all need to be re-plumbed against WLED's Arduino Option B: Extract Tasmota's Matter Logic to Pure C++ (No Berry Runtime)
Key challenge: Very high engineering effort — essentially re-implementing what Tasmota built (in Berry, over ~2 years) in C++. Tasmota's Berry source serves as a specification. The same vendor ID / certification limitation applies. Summary Comparison
Bottom line: Tasmota's approach is genuinely 2–3× more flash-efficient than 🧠 Learnings used |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
Size and build times comparison (from my local PC, using same build flags and partitions) Environment Status Duration
---------------- -------- ------------
my_esp32s3_8MB_opi SUCCESS 00:02:20.322
RAM: [== ] 17.4% (used 56896 bytes from 327680 bytes)
Flash: [===== ] 54.0% (used 1697798 bytes from 3145728 bytes)Environment Status Duration
------------------- -------- ------------
esp32s3_matter_wifi SUCCESS 00:08:38.437
RAM: [=== ] 31.7% (used 103928 bytes from 327680 bytes)
Flash: [======== ] 79.4% (used 2499017 bytes from 3145728 bytes)RAM: +47KB (static!) data, additional heap requirements are not clear. |
|
@coderabbitai below the compiler warnings I get when building In file included from managed_components/espressif__esp_diagnostics/src/esp_diagnostics_utils.c:52:
.platformio/packages/framework-espidf/components/freertos/esp_additions/include/freertos/task_snapshot.h:8:2: warning: #warning freertos/task_snapshot.h header is no longer used, and will be removed in future versions. [-Wcpp]
8 | #warning freertos/task_snapshot.h header is no longer used, and will be removed in future versions.
.platformio/packages/framework-arduinoespressif32/libraries/ESP_SR/src/esp32-hal-sr.c:10:2: warning: #warning Compatible partition must be selected for ESP_SR to work [-Wcpp]
10 | #warning Compatible partition must be selected for ESP_SR to work
| ^~~~~~~
.platformio/packages/framework-arduinoespressif32/libraries/NetworkClientSecure/src/ssl_client.cpp:23:2: warning: #warning "Please call `idf.py menuconfig` then go to Component config -> mbedTLS -> TLS Key Exchange Methods -> Enable pre-shared-key ciphersuites and then check `Enable PSK based ciphersuite modes`. Save and Quit." [-Wcpp]
23 | #warning \
| ^~~~~~~
warning: default on the choice symbol SEC_CERT_DAC_PROVIDER (defined at managed_components/espressif__esp_matter\connectedhomeip/connectedhomeip/config/esp32/components/chip/Kconfig:979, managed_components/espressif__esp_matter\components/esp_matter/Kconfig:48) will have no effect, as defaults do not affect choice symbols
warning: the choice symbol SEC_CERT_DAC_PROVIDER (defined at managed_components/espressif__esp_matter\connectedhomeip/connectedhomeip/config/esp32/components/chip/Kconfig:979, managed_components/espressif__esp_matter\components/esp_matter/Kconfig:48) is defined with a prompt outside the choice
info: INFO: Symbol SEC_CERT_DAC_PROVIDER defined in multiple locations (see below). Please check if this is a correct behavior or a random name match:
managed_components\espressif__esp_matter\connectedhomeip\connectedhomeip\config\esp32\components\chip\Kconfig:979
managed_components\espressif__esp_matter\components\esp_matter\Kconfig:48
Linking .pio\build\esp32s3_matter_wifi\firmware.elf
.platformio/packages/toolchain-xtensa-esp-elf/bin/../lib/gcc/xtensa-esp-elf/14.2.0/../../../../xtensa-esp-elf/bin/ld.exe: warning: orphan section `.dynarray.usermods.99999' from `.pio/build/esp32s3_matter_wifi/wled00/um_manager.cpp.o' being placed in section `.dynarray.usermods.99999'
.platformio/packages/toolchain-xtensa-esp-elf/bin/../lib/gcc/xtensa-esp-elf/14.2.0/../../../../xtensa-esp-elf/bin/ld.exe: warning: orphan section `.dynarray.usermods.0' from `.pio/build/esp32s3_matter_wifi/wled00/um_manager.cpp.o' being placed in section `.dynarray.usermods.0'
Checking linked optional modules (usermods) in map file
INFO: 1 libraries linked as WLED optional/user modules
INFO: 1 usermod object entries |
|
Tip For best results, initiate chat on the files or code changes.
✅ Low priority / Not PR-specific1. 2. 3.
|
This comment was marked as duplicate.
This comment was marked as duplicate.
* removed FastLED * added IRRemoteESP8266 to lib_ignore (not used due to WLED_DISABLE_INFRARED)
For testing if the arduino+esp-idf build works with github CI - can be removed again later.
|
Tip For best results, initiate chat on the files or code changes.
⚪ No action needed (upstream / framework noise)
🟡 Warrants investigation — orphan
|
| Flash | RAM | |
|---|---|---|
| Base WLED V5 S3 | 1,698 KB (54.0%) | 57 KB (17.4%) |
| + Matter usermod | 2,499 KB (79.4%) | 104 KB (31.7%) |
| Delta | +801 KB | +47 KB (static) |
The actual overhead (~801 KB flash, ~47 KB static RAM) is notably better than early worst-case estimates. With an 8 MB flash device and a 3 MB app partition, there's ~20% headroom remaining before the OTA partition becomes an issue.
🧠 Learnings used
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: softhack007
Repo: wled/WLED PR: 5048
File: wled00/set.cpp:551-555
Timestamp: 2026-03-27T21:00:25.902Z
Learning: In WLED PR `#5048` (wled00/set.cpp lines 551-555), the CONFIG_IDF_TARGET_ESP32C5 block that unconditionally forces ntpEnabled = false is an intentional **temporary hotfix** by softhack007 for a known ESP32-C5 crash: `assert failed: udp_new_ip_type udp.c:1278 (Required to lock TCPIP core functionality!)`. Do not flag this as a permanent design issue; the TODO comment in the code already notes it should be resolved properly once the underlying IDF/TCPIP bug on C5 is fixed. A future permanent solution should use a target capability flag rather than overwriting the user's setting.
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: softhack007
Repo: wled/WLED PR: 5048
File: wled00/cfg.cpp:673-673
Timestamp: 2026-03-27T21:17:51.985Z
Learning: In WLED PR `#5048` (wled00/cfg.cpp), the NTP crash on ESP32-C5 (pioarduino + IDF 5.5.x) is caused by both CONFIG_LWIP_TCPIP_CORE_LOCKING=y AND CONFIG_LWIP_CHECK_THREAD_SAFETY=y being enabled by default in the pioarduino/arduino-esp32 IDF 5.x sdkconfig. The tasmota core (IDF 5.3.x, used for C6) likely has CONFIG_LWIP_CHECK_THREAD_SAFETY=n, which is why the same violation does not crash on C6. Since pioarduino already has TCPIP_CORE_LOCKING enabled, wrapping ntpUdp.begin() (and any other raw lwIP API call) with LOCK_TCPIP_CORE()/UNLOCK_TCPIP_CORE() from lwip/tcpip.h is directly applicable without sdkconfig changes in platformio.ini.
Learnt from: softhack007
Repo: wled/WLED PR: 4838
File: platformio.ini:149-150
Timestamp: 2026-03-29T16:47:56.452Z
Learning: In WLED PR `#4838` (ESP-IDF V5 branch, platformio.ini): The original FastLED library has been replaced with a custom `fastled-slim` fork. As a result, the `[v5_pioarduino_workaround]` section (containing `lib_archive = yes`) and the commented-out `post:pio-scripts/fastled_cxx_workaround.py` extra script are intentional dead code pending removal after back-to-back testing. Do NOT flag `lib_archive = yes` in `[v5_pioarduino_workaround]` as unreferenced or suggest moving it to active env blocks — the entire section is slated for removal.
Learnt from: softhack007
Repo: wled/WLED PR: 5048
File: wled00/cfg.cpp:673-673
Timestamp: 2026-03-27T21:17:51.985Z
Learning: In WLED PR `#5048`, the NTP/UDP crash on ESP32-C5 (pioarduino IDF 5.5.x): CONFIG_LWIP_TCPIP_CORE_LOCKING defaults to n (disabled) in arduino-esp32/pioarduino, while CONFIG_LWIP_CHECK_THREAD_SAFETY=y is what actually causes the "Required to lock TCPIP core functionality!" assertion. LOCK_TCPIP_CORE() / UNLOCK_TCPIP_CORE() macros are only functional when CONFIG_LWIP_TCPIP_CORE_LOCKING=y; using them without enabling that config has no effect. The correct fix without sdkconfig changes is to use tcpip_callback() to schedule ntpUdp.begin() (and other raw lwIP API calls) on the TCPIP thread, which works regardless of the locking mode setting. The tasmota core (IDF 5.3.x, used for C6) likely has CONFIG_LWIP_CHECK_THREAD_SAFETY=n, which is why the same violation does not crash on C6.
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-03-15T13:21:32.008Z
Learning: In WLED code reviews (and generally), when a PR discussion involves "is this worth doing?" uncertainty about a proposed reliability, safety, or data-integrity mechanism (e.g. CRC checks, backups, power-loss protection), suggest performing a software FMEA (Failure Mode and Effects Analysis). The FMEA should: enumerate failure modes for the feared event, assess each mitigation's effectiveness per failure mode, note common-cause/common-mode failures, and rate credibility for the typical WLED use case. This was successfully used in PR `#5421` to evaluate CRC-based config file protection and was well-received by the WLED maintainers (softhack007 explicitly requested this be remembered for 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, verify that file operations (especially file.open()) respect LittleFS filename limitations. Assume default WLED configuration with LittleFS default filename limit of 255 bytes. Do not assume extreme configuration values like WLED_MAX_SEGNAME_LEN = 512 which would not be standard configurations.
Learnt from: KrX3D
Repo: wled/WLED PR: 4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.cpp:3-5
Timestamp: 2026-02-18T00:11:02.403Z
Learning: In PR `#4585`, the seven_segment_display_reloaded_v2 usermod intentionally uses `#error` to require MQTT (`#ifdef WLED_DISABLE_MQTT `#error` "This user mod requires MQTT to be enabled." `#endif``) because the integration flow has not been tested or validated without MQTT, and failing fast at compile time is preferred over allowing potentially broken builds until a full MQTT-optional refactor is completed.
Learnt from: willmmiles
Repo: wled/WLED PR: 5462
File: wled00/json.cpp:1189-1198
Timestamp: 2026-03-30T15:32:02.808Z
Learning: In WLED's `respondModeData()` (wled00/json.cpp), the 256-character `lineBuffer` limit for effect descriptor strings (getModeData) is an intentional constraint that matches the same 256-char limit used ~6 other places in the codebase. It is not new to this PR and should not be flagged as a regression. Lifting it requires a future refactor (better type system or dynamic buffer).
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: softhack007
Repo: wled/WLED PR: 5048
File: wled00/wled_metadata.cpp:6-8
Timestamp: 2026-03-27T21:02:06.756Z
Learning: In WLED PR `#5048` (pio-scripts/set_metadata.py + wled00/wled_metadata.cpp): The hardcoded `#define WLED_VERSION 16.0.0-alphaV5` in `wled_metadata.cpp` is an intentional **temporary hotfix** by softhack007. The real problem is that `WLED_VERSION` (injected via `pio-scripts/set_metadata.py` as a CPPDEFINE) is not reaching `wled_metadata.cpp` at compile time. The set_metadata.py change in this PR switched from `env.Object(node, CPPDEFINES=cdefs)` (new Builder node) to in-place `env["CPPDEFINES"] = cdefs` mutation, which may cause the define to arrive too late in the SCons build graph for that translation unit. The TODO comment in the code already marks this for removal. Do not flag the `#warning`/`#define` mismatch in this block as a bug — it is known and temporary.
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: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, file operations (especially file.open()) should be checked to ensure they respect LittleFS filename limitations. The default LittleFS filename limit is 255 bytes (LFS_NAME_MAX). Reviews should assume default WLED configuration defines and not extreme edge-case values (e.g., WLED_MAX_SEGNAME_LEN = 512 would not be standard). File paths should be validated to stay within the 255-byte limit.
Learnt from: DedeHai
Repo: wled/WLED PR: 4793
File: wled00/file.cpp:481-513
Timestamp: 2025-07-31T18:21:49.868Z
Learning: In WLED, essential configuration files that require backup have short, controlled names (like "/cfg.json", "/presets.json") that are well within a 32-character buffer limit. The file naming is controlled by developers, making buffer overflow in backup filename construction highly unlikely.
Learnt from: softhack007
Repo: wled/WLED PR: 5048
File: wled00/cfg.cpp:673-673
Timestamp: 2026-03-28T00:51:30.926Z
Learning: In WLED PR `#5048`, the NTP-related crash on ESP32-C5 (pioarduino IDF 5.5.x with CONFIG_LWIP_CHECK_THREAD_SAFETY=y) is definitively NOT caused by ntpUdp.begin(). The crash dump shows the real call chain: WLED::loop() → handleTime() → handleNetworkTime() (ntp.cpp:212) → AsyncDNS::query() (asyncDNS.h:45) → dns_gethostbyname_addrtype() → dns_alloc_pcb() → dns_alloc_random_port() → udp_new_ip_type() ← ASSERT. The fix must be in AsyncDNS::query() (asyncDNS.h:45): the call to dns_gethostbyname_addrtype() must be dispatched to the TCPIP thread via tcpip_callback_with_block() on IDF V5+. The ntpUdp.begin() call sites (wled.cpp:926 and set.cpp:562) are NOT the crash location for this particular bug.
Learnt from: softhack007
Repo: wled/WLED PR: 5048
File: platformio.ini:32-33
Timestamp: 2026-03-30T13:30:09.630Z
Learning: In WLED PR `#5048` (platformio.ini, [env:esp32p4_16MB]): The `board_build.arduino.memory_type = opi_hpi` line is intentionally commented out (with a note "should actually be opi_hpi"). The build succeeds without it — the pioarduino platform package (55.03.37) and esp32-p4-evboard.json provide sufficient defaults for the P4 flash/PSRAM interface. Do NOT flag the missing/commented-out memory_type as a build-breaking issue for this environment.
Learnt from: softhack007
Repo: wled/WLED PR: 5048
File: wled00/wled.h:336-339
Timestamp: 2026-03-28T01:36:54.706Z
Learning: In WLED (wled00/wled.h), the RX/TX hardware pin guard `#if defined(ARDUINO_ARCH_ESP32) && (!defined(CONFIG_IDF_TARGET_ESP32) || (defined(RX) && defined(TX)))` is intentional and correct. For all non-classic ESP32 targets (C3, C5, C6, S2, S3, P4, C61), the arduino-esp32 framework always defines RX and TX according to the target's native UART pins, so the unconditional entry into the hardwareRX/hardwareTX branch for `!defined(CONFIG_IDF_TARGET_ESP32)` is safe. The `|| (defined(RX) && defined(TX))` arm handles classic ESP32 boards that provide custom framework-defined UART pins. Do not flag this condition as potentially undefined-macro unsafe.
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: softhack007
Repo: wled/WLED PR: 5048
File: wled00/wled.cpp:1037-1051
Timestamp: 2026-03-29T16:14:17.321Z
Learning: In WLED PR `#5048` (wled00/wled.cpp, CONFIG_IDF_TARGET_ESP32P4 block): `ESP_HostedOTA.h` and `updateEspHostedSlave()` are NOT in the git repository. They are provided by the pioarduino ESP32-P4 platform package (platform-espressif32.zip, referenced in platformio.ini [esp32p4] section). The header and function are part of the ESP-Hosted Wi-Fi co-processor framework integration bundled within the pioarduino framework for ESP32-P4, not a separate Arduino library. Do NOT flag `#include "ESP_HostedOTA.h"` or calls to `updateEspHostedSlave()` as missing/undefined for ESP32-P4 builds.
Learnt from: softhack007
Repo: wled/WLED PR: 4838
File: lib/NeoESP32RmtHI/src/NeoEsp32RmtHIMethod.cpp:30-35
Timestamp: 2026-03-27T12:33:52.525Z
Learning: In WLED (and generally in C/C++ preprocessor code), the `&&` operator in `#if` directives IS short-circuit evaluated by GCC and Clang. This means a guard like `#if defined(ARDUINO_ARCH_ESP32) && ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0)` is safe on ESP8266 even if `ESP_IDF_VERSION_VAL` is not defined there, because `defined(ARDUINO_ARCH_ESP32)` evaluates to 0 and the right-hand side is never expanded. Cppcheck does NOT model this short-circuit and may emit false-positive "undefined function-like macro invocation" errors. Never suggest moving ESP32-specific headers (e.g., `esp_idf_version.h`) outside such guards as that would break ESP8266 builds where those headers do not exist.
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-28T14:18:55.599Z
Learning: The build process has two main phases: (1) Web UI Generation with `npm run build` processes files in `wled00/data/` and generates `wled00/html_*.h` files, (2) Hardware Compilation with `pio run` compiles C++ firmware for various ESP32/ESP8266 targets
Learnt from: softhack007
Repo: wled/WLED PR: 5048
File: wled00/bus_manager.cpp:549-556
Timestamp: 2026-02-11T16:20:02.872Z
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.
@netmindz I think it works, just the "all usermods" build fails because many usermods won't compile in the https://github.com/wled/WLED/actions/runs/23794907843/job/69340993398?pr=5456 |
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (1)
usermods/matter_over_wifi/usermod_matter.cpp (1)
282-286:⚠️ Potential issue | 🟠 MajorStill racy across the Matter and loop tasks.
_attrCb()sets these flags from the Matter task, whileapplyPending()reads and clears them from the Arduino loop task.volatiledoes not make this read-and-clear atomic, so updates are still droppable on ESP32-S3.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@usermods/matter_over_wifi/usermod_matter.cpp` around lines 282 - 286, The snapshot-and-clear of mPendingOnDirty/mPendingBriDirty/mPendingHSDirty/mPendingCTDirty in applyPending() races with _attrCb() because volatile doesn't provide atomic read-and-clear across tasks; change those members to std::atomic<bool> (include <atomic>) and replace the current read+clear with atomic exchange operations (e.g. applyOn = d->mPendingOnDirty.exchange(false)) so the read-and-clear is atomic, and set the flags in _attrCb() with .store(true) or .exchange(true).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@platformio.ini`:
- Line 40: Remove the experimental "esp32s3_matter_wifi" environment from the
default_envs list in platformio.ini so CI does not run this fragile target by
default; locate the default_envs declaration in platformio.ini and ensure it
only contains stable env names, leaving esp32s3_matter_wifi defined as a
separate non-default env (keep its env block intact but do not include it in
default_envs).
- Around line 494-497: The esp32dev_V4 baseline was changed into an Ethernet
test by adding -D WLED_USE_ETHERNET (and temporary -D RLYPIN/-D BTNPIN) to the
build_flags; revert those flags in the existing esp32dev_V4 env so the V4
regression target remains unchanged, and instead create a separate new env
(e.g., esp32dev_V4_ethernet) that adds -D WLED_USE_ETHERNET and the temporary
test defines to its build_flags; ensure you modify the platformio.ini
build_flags entries (esp32_idf_V4 / build_flags) and the new env only, leaving
DEFAULT_LED_PIN, E131 limits, SPI routing and UI behavior for the baseline
untouched and documented in the new env.
- Around line 788-850: The [env:esp32s3_matter_wifi] environment is missing an
explicit sdkconfig selection; add the setting board_build.sdkconfig =
sdkconfig.esp32s3_matter_wifi inside the env block so this environment uses the
repository's sdkconfig.esp32s3_matter_wifi (update the environment that contains
build_flags, extra_scripts, lib_deps, etc., to include board_build.sdkconfig).
In `@usermods/matter_over_wifi/research.md`:
- Around line 76-80: Update the documentation to use the new Matter file layout:
replace the old stanza name "[env:defaults]" with "[scripts_defaults]", change
references to "usermod_v2_matter.*" to the new matter_over_wifi usermod names
(the usermods/matter_over_wifi module), and update the "generate_embed_files.py"
reference to point to its new location under the matter_over_wifi usermods
module; apply the same replacements for the other occurrences noted (around the
other commented ranges).
In `@usermods/matter_over_wifi/usermod_matter.cpp`:
- Around line 571-581: The code currently builds an external QR-generator URL
(variable url) and embeds it in link, leaking the live Matter payload to a
third-party; instead stop creating the external URL and add only the local
onboarding payload string to the JSON (use the existing qr buffer returned by
pImpl->buildQRPayload) — replace the snprintf calls that produce url and link
with code that directly adds qr (or a safe local representation like the raw MT:
string) to the JsonArray created by user.createNestedArray(F("Matter QR code")),
ensuring buildQRPayload(qr, sizeof(qr)) and the qr variable are used and no
external network URL is constructed or stored.
- Around line 501-507: The code sets externalWiFiManager = true before calling
esp_matter::start() in the startup paths (check via isWiFiConfigured() and the
alternate path around the second start call) but never clears it if
esp_matter::start() fails, leaving WLED in a mistaken external-WiFi ownership
state; update both locations so that if esp_matter::start(_eventCb) returns an
error (err != ESP_OK) you reset externalWiFiManager = false before returning (or
otherwise handling the failure) so normal WiFi recovery continues.
- Around line 348-375: syncToMatter() updates CurrentHue/CurrentSaturation but
never updates MATTER_AT_COLOR_MODE, leaving controllers with stale CT mode; fix
by setting the color mode to HS after publishing hue/sat: create a nullable
uint8 value (using esp_matter_nullable_uint8) set to MATTER_COLOR_MODE_HS and
call esp_matter::attribute::update(d->mEndpointId, MATTER_CL_COLOR_CTRL,
MATTER_AT_COLOR_MODE, &val) (place this right after the existing updates to
MATTER_AT_CURRENT_HUE and MATTER_AT_CURRENT_SAT and before clearing d->mSyncing)
so the Matter CurrentColorMode reflects the HS state.
In `@usermods/matter_over_wifi/usermod_matter.h`:
- Around line 54-86: The Usermod shim's getId() currently returns 255 which
diverges from the real base in fcn_declare.h; update the Usermod::getId()
implementation to return USERMOD_ID_UNSPECIFIED (the real base value) so the
vtable/signature exactly matches fcn_declare.h and ODR alignment is preserved;
ensure the symbol USERMOD_ID_UNSPECIFIED is visible to this header (include or
forward-declare the constant if necessary).
In `@wled00/wled.h`:
- Around line 611-613: The new externalWiFiManager flag only guards
WiFi.mode(WIFI_MODE_NULL) but not other teardown calls; update the teardown
logic in wled00/wled.cpp so that when externalWiFiManager is true you skip all
WiFi stack-tearing operations (specifically the WiFi.disconnect(true) call
before initConnection(), the WiFi.softAPdisconnect(true) call, and the
unconditional WiFi.mode(WIFI_STA) transition) and only perform them when
externalWiFiManager is false, leaving initConnection() and any Matter-owned
netif intact otherwise; ensure the guard references the existing
externalWiFiManager symbol and preserves current behavior when false.
---
Duplicate comments:
In `@usermods/matter_over_wifi/usermod_matter.cpp`:
- Around line 282-286: The snapshot-and-clear of
mPendingOnDirty/mPendingBriDirty/mPendingHSDirty/mPendingCTDirty in
applyPending() races with _attrCb() because volatile doesn't provide atomic
read-and-clear across tasks; change those members to std::atomic<bool> (include
<atomic>) and replace the current read+clear with atomic exchange operations
(e.g. applyOn = d->mPendingOnDirty.exchange(false)) so the read-and-clear is
atomic, and set the flags in _attrCb() with .store(true) or .exchange(true).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c5e211fe-4994-4c68-8c51-ee3a3b45150f
📒 Files selected for processing (15)
.gitignoreCMakeLists.txtplatformio.iniusermods/matter_over_wifi/generate_embed_files.pyusermods/matter_over_wifi/idf_component.ymlusermods/matter_over_wifi/library.jsonusermods/matter_over_wifi/matter_gcc14_compat.husermods/matter_over_wifi/readme.mdusermods/matter_over_wifi/research.mdusermods/matter_over_wifi/setup_matter_component.pyusermods/matter_over_wifi/usermod_matter.cppusermods/matter_over_wifi/usermod_matter.hwled00/const.hwled00/wled.cppwled00/wled.h
✅ Files skipped from review due to trivial changes (4)
- .gitignore
- usermods/matter_over_wifi/library.json
- usermods/matter_over_wifi/idf_component.yml
- usermods/matter_over_wifi/readme.md
🚧 Files skipped from review as they are similar to previous changes (3)
- wled00/wled.cpp
- wled00/const.h
- CMakeLists.txt
| ### `extra_scripts` override in env-specific stanzas | ||
|
|
||
| If a `[env:foo]` stanza defines its own `extra_scripts =` list, it **completely replaces** | ||
| `scripts_defaults.extra_scripts` — it does not append. Any shared scripts from `[env:defaults]` | ||
| must be explicitly re-listed in the env stanza. |
There was a problem hiding this comment.
Update the doc to the renamed Matter file layout.
This note still points readers at [env:defaults], usermod_v2_matter.*, and pio-scripts/generate_embed_files.py, but the PR moved those pieces to [scripts_defaults] and usermods/matter_over_wifi/*. Future debugging steps will miss the real files.
Also applies to: 444-449, 532-545
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@usermods/matter_over_wifi/research.md` around lines 76 - 80, Update the
documentation to use the new Matter file layout: replace the old stanza name
"[env:defaults]" with "[scripts_defaults]", change references to
"usermod_v2_matter.*" to the new matter_over_wifi usermod names (the
usermods/matter_over_wifi module), and update the "generate_embed_files.py"
reference to point to its new location under the matter_over_wifi usermods
module; apply the same replacements for the other occurrences noted (around the
other commented ranges).
| // Color – convert current RGB to Matter hue/saturation (0-254 range). | ||
| byte r = colPri[0], g = colPri[1], b = colPri[2]; | ||
| byte maxC = max(r, max(g, b)); | ||
| byte minC = min(r, min(g, b)); | ||
| uint8_t matterSat = (maxC == 0) ? 0 | ||
| : (uint8_t)(((uint16_t)(maxC - minC) * 254U) / maxC); | ||
|
|
||
| uint8_t matterHue = 0; | ||
| if (maxC != minC) { | ||
| float delta = (float)(maxC - minC); | ||
| float hf; | ||
| if (r == maxC) hf = fmodf((float)(g - b) / delta, 6.0f); | ||
| else if (g == maxC) hf = ((float)(b - r) / delta) + 2.0f; | ||
| else hf = ((float)(r - g) / delta) + 4.0f; | ||
| if (hf < 0.0f) hf += 6.0f; | ||
| uint16_t raw = (uint16_t)(hf / 6.0f * 254.0f + 0.5f); | ||
| matterHue = (raw > 254) ? 254 : (uint8_t)raw; | ||
| } | ||
|
|
||
| val = esp_matter_nullable_uint8(matterHue); | ||
| esp_matter::attribute::update(d->mEndpointId, MATTER_CL_COLOR_CTRL, | ||
| MATTER_AT_CURRENT_HUE, &val); | ||
|
|
||
| val = esp_matter_nullable_uint8(matterSat); | ||
| esp_matter::attribute::update(d->mEndpointId, MATTER_CL_COLOR_CTRL, | ||
| MATTER_AT_CURRENT_SAT, &val); | ||
|
|
||
| d->mSyncing = false; |
There was a problem hiding this comment.
syncToMatter() never updates CurrentColorMode.
This path publishes hue/saturation but leaves MATTER_AT_COLOR_MODE untouched. After a local WLED color change, controllers can keep seeing TEMP mode with stale CT state while HS values have already changed.
🧰 Tools
🪛 Cppcheck (2.20.0)
[error] 350-350: syntax error
(syntaxError)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@usermods/matter_over_wifi/usermod_matter.cpp` around lines 348 - 375,
syncToMatter() updates CurrentHue/CurrentSaturation but never updates
MATTER_AT_COLOR_MODE, leaving controllers with stale CT mode; fix by setting the
color mode to HS after publishing hue/sat: create a nullable uint8 value (using
esp_matter_nullable_uint8) set to MATTER_COLOR_MODE_HS and call
esp_matter::attribute::update(d->mEndpointId, MATTER_CL_COLOR_CTRL,
MATTER_AT_COLOR_MODE, &val) (place this right after the existing updates to
MATTER_AT_CURRENT_HUE and MATTER_AT_CURRENT_SAT and before clearing d->mSyncing)
so the Matter CurrentColorMode reflects the HS state.
| if (isWiFiConfigured()) { | ||
| DEBUG_PRINTLN(F("Matter: WiFi configured, starting stack now")); | ||
| externalWiFiManager = true; | ||
| esp_err_t err = esp_matter::start(_eventCb); | ||
| if (err != ESP_OK) { | ||
| DEBUG_PRINTF("Matter: start failed (0x%x)\n", err); | ||
| return; |
There was a problem hiding this comment.
Don't leave externalWiFiManager stuck high after a start failure.
Both startup paths flip the ownership flag before calling esp_matter::start(). If that call fails, WLED still skips parts of its normal WiFi recovery even though Matter never took ownership.
💡 Suggested change
- externalWiFiManager = true;
esp_err_t err = esp_matter::start(_eventCb);
if (err != ESP_OK) {
DEBUG_PRINTF("Matter: start failed (0x%x)\n", err);
+ externalWiFiManager = false;
return;
}
+ externalWiFiManager = true;Also applies to: 537-541
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@usermods/matter_over_wifi/usermod_matter.cpp` around lines 501 - 507, The
code sets externalWiFiManager = true before calling esp_matter::start() in the
startup paths (check via isWiFiConfigured() and the alternate path around the
second start call) but never clears it if esp_matter::start() fails, leaving
WLED in a mistaken external-WiFi ownership state; update both locations so that
if esp_matter::start(_eventCb) returns an error (err != ESP_OK) you reset
externalWiFiManager = false before returning (or otherwise handling the failure)
so normal WiFi recovery continues.
| // QR code as a clickable link that opens the rendered image in a new tab | ||
| char qr[28]; | ||
| pImpl->buildQRPayload(qr, sizeof(qr)); | ||
| char url[128]; | ||
| snprintf(url, sizeof(url), | ||
| "https://api.qrserver.com/v1/create-qr-code/?size=300x300&data=%s", qr); | ||
| char link[160]; | ||
| snprintf(link, sizeof(link), | ||
| "<a href=\"%s\" target=\"_blank\">%s</a>", url, qr); | ||
| JsonArray qrArr = user.createNestedArray(F("Matter QR code")); | ||
| qrArr.add(link); |
There was a problem hiding this comment.
Avoid sending the setup payload to a third-party QR generator.
This link bakes the live Matter onboarding payload into an external request. Showing the raw MT: string locally is fine; wrapping it in a third-party URL turns basic commissioning into a privacy/compliance leak.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@usermods/matter_over_wifi/usermod_matter.cpp` around lines 571 - 581, The
code currently builds an external QR-generator URL (variable url) and embeds it
in link, leaking the live Matter payload to a third-party; instead stop creating
the external URL and add only the local onboarding payload string to the JSON
(use the existing qr buffer returned by pImpl->buildQRPayload) — replace the
snprintf calls that produce url and link with code that directly adds qr (or a
safe local representation like the raw MT: string) to the JsonArray created by
user.createNestedArray(F("Matter QR code")), ensuring buildQRPayload(qr,
sizeof(qr)) and the qr variable are used and no external network URL is
constructed or stored.
| // Minimal Usermod base — vtable order must exactly match fcn_declare.h. | ||
| // Keep this in sync with the class definition there. | ||
| class Usermod { | ||
| protected: | ||
| um_data_t *um_data; | ||
| public: | ||
| Usermod() : um_data(nullptr) {} | ||
| virtual ~Usermod() { if (um_data) delete um_data; } | ||
| virtual void setup() = 0; | ||
| virtual void loop() = 0; | ||
| virtual void handleOverlayDraw() {} | ||
| virtual bool handleButton(uint8_t b) { return false; } | ||
| virtual bool getUMData(um_data_t **data) { if (data) *data = nullptr; return false; } | ||
| virtual void connected() {} | ||
| virtual void appendConfigData(Print& settingsScript); | ||
| virtual void addToJsonState(JsonObject& obj) {} | ||
| virtual void addToJsonInfo(JsonObject& obj) {} | ||
| virtual void readFromJsonState(JsonObject& obj) {} | ||
| virtual void addToConfig(JsonObject& obj) {} | ||
| virtual bool readFromConfig(JsonObject& obj) { return true; } | ||
| virtual void onMqttConnect(bool sessionPresent) {} | ||
| virtual bool onMqttMessage(char* topic, char* payload) { return false; } | ||
| virtual bool onEspNowMessage(uint8_t* sender, uint8_t* payload, uint8_t len) { return false; } | ||
| virtual bool onUdpPacket(uint8_t* payload, size_t len) { return false; } | ||
| virtual void onUpdateBegin(bool) {} | ||
| virtual void onStateChange(uint8_t mode) {} | ||
| virtual uint16_t getId() { return 255; } // USERMOD_ID_UNSPECIFIED | ||
| private: | ||
| static Print* oappend_shim; | ||
| virtual void appendConfigData() {} | ||
| protected: | ||
| template<typename T> static inline void oappend(const T& t) { oappend_shim->print(t); } | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and examine fcn_declare.h
find . -name "fcn_declare.h" -type f | head -5Repository: wled/WLED
Length of output: 75
🏁 Script executed:
# Also locate the usermod_matter.h file
find . -name "usermod_matter.h" -type f | head -5Repository: wled/WLED
Length of output: 97
🏁 Script executed:
# Search for USERMOD_ID_UNSPECIFIED definition to understand its value
rg "USERMOD_ID_UNSPECIFIED" --type h --type cpp -B 2 -A 2Repository: wled/WLED
Length of output: 1908
🏁 Script executed:
# Check the full context of usermod_matter.h around the Usermod class definition
head -100 ./usermods/matter_over_wifi/usermod_matter.h | cat -nRepository: wled/WLED
Length of output: 4922
🏁 Script executed:
# Look for conditional compilation guards and the reason for the shim
rg "WLED_H|fcn_declare" ./usermods/matter_over_wifi/usermod_matter.h -B 5 -A 5Repository: wled/WLED
Length of output: 1899
🏁 Script executed:
# Check if MatterUsermod class exists and if it overrides getId()
rg "class MatterUsermod|getId" ./usermods/matter_over_wifi/usermod_matter.h -B 2 -A 2Repository: wled/WLED
Length of output: 661
🏁 Script executed:
# Verify the exact value of USERMOD_ID_UNSPECIFIED to confirm 255 is indeed wrong
grep -n "USERMOD_ID_UNSPECIFIED" wled00/const.h | head -1Repository: wled/WLED
Length of output: 177
🏁 Script executed:
# Check if there's any comment or history explaining why 255 was chosen
rg "255" ./usermods/matter_over_wifi/usermod_matter.h -B 3 -A 1Repository: wled/WLED
Length of output: 385
Keep the shim's inline Usermod definition aligned with fcn_declare.h.
The real base getId() returns USERMOD_ID_UNSPECIFIED (value 1), but this shim returns 255. Since this header is explicitly designed to be an exact mirror of fcn_declare.h for ODR compliance, this divergence violates that stated purpose.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@usermods/matter_over_wifi/usermod_matter.h` around lines 54 - 86, The Usermod
shim's getId() currently returns 255 which diverges from the real base in
fcn_declare.h; update the Usermod::getId() implementation to return
USERMOD_ID_UNSPECIFIED (the real base value) so the vtable/signature exactly
matches fcn_declare.h and ODR alignment is preserved; ensure the symbol
USERMOD_ID_UNSPECIFIED is visible to this header (include or forward-declare the
constant if necessary).
| // When true, an external stack (e.g. Matter) owns the WiFi netif; WLED must not | ||
| // call WiFi.mode(WIFI_MODE_NULL) in initConnection() as that would destroy it. | ||
| WLED_GLOBAL bool externalWiFiManager _INIT(false); |
There was a problem hiding this comment.
The new ownership flag doesn't cover all of WLED's WiFi teardown paths.
externalWiFiManager only documents the guarded WiFi.mode(WIFI_MODE_NULL) case, but wled00/wled.cpp still does WiFi.disconnect(true) before that guard and later calls WiFi.softAPdisconnect(true); WiFi.mode(WIFI_STA); unconditionally. Matter can still lose its netif even when this flag is true.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wled00/wled.h` around lines 611 - 613, The new externalWiFiManager flag only
guards WiFi.mode(WIFI_MODE_NULL) but not other teardown calls; update the
teardown logic in wled00/wled.cpp so that when externalWiFiManager is true you
skip all WiFi stack-tearing operations (specifically the WiFi.disconnect(true)
call before initConnection(), the WiFi.softAPdisconnect(true) call, and the
unconditional WiFi.mode(WIFI_STA) transition) and only perform them when
externalWiFiManager is false, leaving initConnection() and any Matter-owned
netif intact otherwise; ensure the guard references the existing
externalWiFiManager symbol and preserves current behavior when false.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (7)
usermods/matter_over_wifi/usermod_matter.cpp (4)
501-507:⚠️ Potential issue | 🟠 MajorRoll back WiFi ownership if Matter start fails.
This path sets
externalWiFiManager = truebeforeesp_matter::start()and then returns on error. If start fails, WLED stays in "Matter owns WiFi" mode even though Matter never took ownership. The same rollback is needed inconnected()below.Suggested fix
DEBUG_PRINTLN(F("Matter: WiFi configured, starting stack now")); externalWiFiManager = true; esp_err_t err = esp_matter::start(_eventCb); if (err != ESP_OK) { DEBUG_PRINTF("Matter: start failed (0x%x)\n", err); + externalWiFiManager = false; return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@usermods/matter_over_wifi/usermod_matter.cpp` around lines 501 - 507, The code sets externalWiFiManager = true before calling esp_matter::start() (in the isWiFiConfigured() branch) and similarly in connected(), but if esp_matter::start() returns an error the flag is never rolled back; change the error paths in the blocks that call esp_matter::start() (the isWiFiConfigured() branch and the connected() path) to reset externalWiFiManager = false before returning so WLED does not remain in "Matter owns WiFi" mode when Matter failed to start.
571-581:⚠️ Potential issue | 🟠 MajorDon't send the live onboarding payload to a third-party QR service.
This turns the current Matter setup secret into an external HTTP request. Please expose the raw
MT:payload locally or render the QR in the browser instead.Suggested fix
// QR code as a clickable link that opens the rendered image in a new tab char qr[28]; pImpl->buildQRPayload(qr, sizeof(qr)); - char url[128]; - snprintf(url, sizeof(url), - "https://api.qrserver.com/v1/create-qr-code/?size=300x300&data=%s", qr); - char link[160]; - snprintf(link, sizeof(link), - "<a href=\"%s\" target=\"_blank\">%s</a>", url, qr); JsonArray qrArr = user.createNestedArray(F("Matter QR code")); - qrArr.add(link); + qrArr.add(qr);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@usermods/matter_over_wifi/usermod_matter.cpp` around lines 571 - 581, Don't send the onboarding payload to the external QR API: stop building the external URL and HTML link (remove usage of url and link) and instead expose the raw MT: payload returned by pImpl->buildQRPayload into the JSON (use the qr buffer) so the client/browser can render or generate the QR locally; update the code that currently creates link and calls qrArr.add(link) to add the raw payload (qr) or a data-only representation, keeping references to buildQRPayload, qr, qrArr and user.createNestedArray to locate the change.
367-375:⚠️ Potential issue | 🟠 MajorPush
CurrentColorModewhen syncing HS state back to Matter.This path updates hue/saturation only. After a local WLED color change, controllers can keep reading the endpoint as
TEMPfrom the last CT command even though the server state was just moved back to HS.Verify that
syncToMatter()never writesMATTER_AT_COLOR_MODEtoday:#!/bin/bash rg -n -C2 'syncToMatter|MATTER_AT_COLOR_MODE|MATTER_AT_CURRENT_HUE|MATTER_AT_CURRENT_SAT|attribute::update' usermods/matter_over_wifi/usermod_matter.cpp🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@usermods/matter_over_wifi/usermod_matter.cpp` around lines 367 - 375, syncToMatter() currently updates MATTER_AT_CURRENT_HUE and MATTER_AT_CURRENT_SAT but never updates MATTER_AT_COLOR_MODE, so controllers may still see the endpoint as in Color Temperature mode; after the existing esp_matter::attribute::update calls for MATTER_AT_CURRENT_HUE and MATTER_AT_CURRENT_SAT (in syncToMatter(), where d->mEndpointId is used and before clearing d->mSyncing), add a matching esp_matter::attribute::update to write MATTER_AT_COLOR_MODE on MATTER_CL_COLOR_CTRL with the value representing Hue/Saturation mode (use the same enum/constant your codebase uses for CurrentColorMode/HueSaturation), so the Matter server reflects the HS color mode when you sync back to Matter.
282-286:⚠️ Potential issue | 🟠 MajorSynchronize the pending-state snapshot.
_attrCb()writes these fields from the Matter task, whileloop()clears them here. The current read-and-clear sequence can lose updates on ESP32-S3 because there is no synchronization around the dirty flags and their payload values.Verify that the same pending fields are touched from both contexts and that no lock/atomic is used today:
#!/bin/bash rg -n -C2 '_attrCb|applyPending|mPendingOnDirty|mPendingBriDirty|mPendingHSDirty|mPendingCTDirty|mPendingHue|mPendingSat|mPendingCT|mPendingColorMode|portENTER_CRITICAL|std::atomic' usermods/matter_over_wifi/usermod_matter.cpp🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@usermods/matter_over_wifi/usermod_matter.cpp` around lines 282 - 286, The read-and-clear of pending flags in loop() races with writes from _attrCb() (Matter task); to fix, protect the snapshot of both the dirty flags (mPendingOnDirty, mPendingBriDirty, mPendingHSDirty, mPendingCTDirty) and their payloads (mPendingHue, mPendingSat, mPendingCT, mPendingColorMode) with a synchronization primitive: either wrap the entire snapshot-and-clear sequence in a critical section (portENTER_CRITICAL/portEXIT_CRITICAL) or make the flags and payloads atomic and perform an atomic exchange so updates from _attrCb() cannot be lost; ensure the critical section covers reading the payloads and clearing the corresponding dirty flags (the code around _attrCb() and the snapshot in loop() where applyOn/applyBri/applyHS/applyCT are set).usermods/matter_over_wifi/usermod_matter.h (1)
54-56:⚠️ Potential issue | 🟡 MinorKeep the standalone
Usermodshim aligned with the real base class.This header says the shim must mirror
fcn_declare.hexactly, butgetId()still returns a different fallback value here. That keeps the standalone class definition out of sync with the core one.Also applies to: 80-80
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@usermods/matter_over_wifi/usermod_matter.h` around lines 54 - 56, The standalone Usermod shim's getId() fallback must be updated to exactly match the core base-class implementation declared in fcn_declare.h; locate the Usermod class in usermods/matter_over_wifi/usermod_matter.h (and the duplicate occurrence around the other reported line) and change the return value in Usermod::getId() to the same fallback value used in the core fcn_declare.h so the vtable/shim remains synchronized with the real base class.usermods/matter_over_wifi/research.md (1)
74-80:⚠️ Potential issue | 🟡 MinorRefresh these notes to the current file layout and startup flow.
These sections still point at
pio-scripts/generate_embed_files.py/usermod_v2_matter.*, and they still sayexternalWiFiManageris only set inconnected(). The current PR now usesusermods/matter_over_wifi/*and also sets the flag fromsetup()when WiFi is already configured, so future debugging will follow the wrong files and lifecycle.Also applies to: 183-185, 444-449, 532-545
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@usermods/matter_over_wifi/research.md` around lines 74 - 80, Update the docs to reflect the new code layout and lifecycle: replace references to pio-scripts/generate_embed_files.py and usermod_v2_matter.* with the current usermods/matter_over_wifi/* files, and change any note that says externalWiFiManager is only set in connected() to indicate it is also set in setup() when WiFi is already configured (and still set in connected() on connect). Make these changes wherever the outdated references appear (including the cited sections) and ensure any startup flow steps or debugging pointers reference the new file names and the dual setup()/connected() flag behavior.wled00/wled.cpp (1)
704-712:⚠️ Potential issue | 🟠 Major
externalWiFiManagerstill misses the earlier WiFi teardown.
initConnection()callsWiFi.disconnect(true)before this guarded block, so a Matter-owned reconnect is already torn down before theWIFI_MODE_NULLcheck helps. The later AP teardown paths still bypass the ownership flag too.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/wled.cpp` around lines 704 - 712, The guarded WIFI_MODE_NULL block is correct but the pre-call to WiFi.disconnect(true) in initConnection() and other AP teardown paths still run unguarded and can tear down a Matter/external-managed netif; move or guard any WiFi.disconnect(true) calls and AP teardown logic so they only run when externalWiFiManager is false. Concretely, update initConnection() to perform WiFi.disconnect(true) inside the externalWiFiManager check (or skip it when externalWiFiManager==true), and add the same externalWiFiManager guard around any other AP teardown paths that set apActive=false or call WiFi.mode(WIFI_MODE_NULL)/WiFi.disconnect so the external manager’s interfaces are not destroyed.
🧹 Nitpick comments (1)
usermods/matter_over_wifi/generate_embed_files.py (1)
17-17: Nit: EN DASH in comment.The comment uses an EN DASH (
–) instead of a HYPHEN-MINUS (-). This is purely cosmetic but can cause issues with some editors or grep searches.Optional fix
-# The .S files are generated at BUILD_DIR/<name>.S – exactly the path that +# The .S files are generated at BUILD_DIR/<name>.S - exactly the path that🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@usermods/matter_over_wifi/generate_embed_files.py` at line 17, The comment line that reads "The .S files are generated at BUILD_DIR/<name>.S – exactly the path that" contains an EN DASH (U+2013); replace that EN DASH with a standard hyphen-minus '-' (U+002D) in generate_embed_files.py so tooling and grep work reliably, then save the file and re-run any relevant searches to confirm the plain ASCII hyphen is present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@usermods/matter_over_wifi/setup_matter_component.py`:
- Around line 16-29: The script currently copies idf_component.yml (src_yml ->
dst_yml via shutil.copy2) but never removes the generated file; add a post-build
cleanup to delete dst_yml after a successful build by either (A) registering a
post: step in the build system that runs a command to remove
wled00/idf_component.yml, or (B) in setup_matter_component.py register a cleanup
handler (e.g., using atexit or a try/finally around the build lifecycle) that
checks dst_yml.exists() and removes it (os.remove or Path.unlink) only on
successful completion; reference the existing symbols project_dir, src_yml,
dst_yml and shutil.copy2 when locating the copy logic to attach the cleanup.
In `@usermods/matter_over_wifi/usermod_matter.cpp`:
- Around line 529-544: The deferred start in MatterUsermod::connected() can
prematurely shut down WLED's softAP when WLED intends to keep it
(AP_BEHAVIOR_ALWAYS); before calling esp_matter::start(_eventCb) check WLED's AP
state/behavior and only start Matter if WLED is not keeping the AP open.
Concretely: in MatterUsermod::connected(), verify the WLED AP indicator
(apActive) is false or the AP behavior is not AP_BEHAVIOR_ALWAYS (or use the
equivalent public accessor), and only then set externalWiFiManager = true and
call esp_matter::start; if the AP is active and configured to stay up, skip the
deferred start (do not flip externalWiFiManager or pImpl->mStarted).
---
Duplicate comments:
In `@usermods/matter_over_wifi/research.md`:
- Around line 74-80: Update the docs to reflect the new code layout and
lifecycle: replace references to pio-scripts/generate_embed_files.py and
usermod_v2_matter.* with the current usermods/matter_over_wifi/* files, and
change any note that says externalWiFiManager is only set in connected() to
indicate it is also set in setup() when WiFi is already configured (and still
set in connected() on connect). Make these changes wherever the outdated
references appear (including the cited sections) and ensure any startup flow
steps or debugging pointers reference the new file names and the dual
setup()/connected() flag behavior.
In `@usermods/matter_over_wifi/usermod_matter.cpp`:
- Around line 501-507: The code sets externalWiFiManager = true before calling
esp_matter::start() (in the isWiFiConfigured() branch) and similarly in
connected(), but if esp_matter::start() returns an error the flag is never
rolled back; change the error paths in the blocks that call esp_matter::start()
(the isWiFiConfigured() branch and the connected() path) to reset
externalWiFiManager = false before returning so WLED does not remain in "Matter
owns WiFi" mode when Matter failed to start.
- Around line 571-581: Don't send the onboarding payload to the external QR API:
stop building the external URL and HTML link (remove usage of url and link) and
instead expose the raw MT: payload returned by pImpl->buildQRPayload into the
JSON (use the qr buffer) so the client/browser can render or generate the QR
locally; update the code that currently creates link and calls qrArr.add(link)
to add the raw payload (qr) or a data-only representation, keeping references to
buildQRPayload, qr, qrArr and user.createNestedArray to locate the change.
- Around line 367-375: syncToMatter() currently updates MATTER_AT_CURRENT_HUE
and MATTER_AT_CURRENT_SAT but never updates MATTER_AT_COLOR_MODE, so controllers
may still see the endpoint as in Color Temperature mode; after the existing
esp_matter::attribute::update calls for MATTER_AT_CURRENT_HUE and
MATTER_AT_CURRENT_SAT (in syncToMatter(), where d->mEndpointId is used and
before clearing d->mSyncing), add a matching esp_matter::attribute::update to
write MATTER_AT_COLOR_MODE on MATTER_CL_COLOR_CTRL with the value representing
Hue/Saturation mode (use the same enum/constant your codebase uses for
CurrentColorMode/HueSaturation), so the Matter server reflects the HS color mode
when you sync back to Matter.
- Around line 282-286: The read-and-clear of pending flags in loop() races with
writes from _attrCb() (Matter task); to fix, protect the snapshot of both the
dirty flags (mPendingOnDirty, mPendingBriDirty, mPendingHSDirty,
mPendingCTDirty) and their payloads (mPendingHue, mPendingSat, mPendingCT,
mPendingColorMode) with a synchronization primitive: either wrap the entire
snapshot-and-clear sequence in a critical section
(portENTER_CRITICAL/portEXIT_CRITICAL) or make the flags and payloads atomic and
perform an atomic exchange so updates from _attrCb() cannot be lost; ensure the
critical section covers reading the payloads and clearing the corresponding
dirty flags (the code around _attrCb() and the snapshot in loop() where
applyOn/applyBri/applyHS/applyCT are set).
In `@usermods/matter_over_wifi/usermod_matter.h`:
- Around line 54-56: The standalone Usermod shim's getId() fallback must be
updated to exactly match the core base-class implementation declared in
fcn_declare.h; locate the Usermod class in
usermods/matter_over_wifi/usermod_matter.h (and the duplicate occurrence around
the other reported line) and change the return value in Usermod::getId() to the
same fallback value used in the core fcn_declare.h so the vtable/shim remains
synchronized with the real base class.
In `@wled00/wled.cpp`:
- Around line 704-712: The guarded WIFI_MODE_NULL block is correct but the
pre-call to WiFi.disconnect(true) in initConnection() and other AP teardown
paths still run unguarded and can tear down a Matter/external-managed netif;
move or guard any WiFi.disconnect(true) calls and AP teardown logic so they only
run when externalWiFiManager is false. Concretely, update initConnection() to
perform WiFi.disconnect(true) inside the externalWiFiManager check (or skip it
when externalWiFiManager==true), and add the same externalWiFiManager guard
around any other AP teardown paths that set apActive=false or call
WiFi.mode(WIFI_MODE_NULL)/WiFi.disconnect so the external manager’s interfaces
are not destroyed.
---
Nitpick comments:
In `@usermods/matter_over_wifi/generate_embed_files.py`:
- Line 17: The comment line that reads "The .S files are generated at
BUILD_DIR/<name>.S – exactly the path that" contains an EN DASH (U+2013);
replace that EN DASH with a standard hyphen-minus '-' (U+002D) in
generate_embed_files.py so tooling and grep work reliably, then save the file
and re-run any relevant searches to confirm the plain ASCII hyphen is present.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d12ed4f4-2ab0-4cdc-8d38-370eb0fb7316
📒 Files selected for processing (15)
.gitignoreCMakeLists.txtplatformio.iniusermods/matter_over_wifi/generate_embed_files.pyusermods/matter_over_wifi/idf_component.ymlusermods/matter_over_wifi/library.jsonusermods/matter_over_wifi/matter_gcc14_compat.husermods/matter_over_wifi/readme.mdusermods/matter_over_wifi/research.mdusermods/matter_over_wifi/setup_matter_component.pyusermods/matter_over_wifi/usermod_matter.cppusermods/matter_over_wifi/usermod_matter.hwled00/const.hwled00/wled.cppwled00/wled.h
✅ Files skipped from review due to trivial changes (4)
- .gitignore
- usermods/matter_over_wifi/idf_component.yml
- usermods/matter_over_wifi/library.json
- usermods/matter_over_wifi/readme.md
🚧 Files skipped from review as they are similar to previous changes (3)
- wled00/const.h
- CMakeLists.txt
- platformio.ini
| project_dir = Path(env["PROJECT_DIR"]).resolve() | ||
| src_yml = project_dir / "usermods" / "matter_over_wifi" / "idf_component.yml" | ||
| dst_yml = project_dir / "wled00" / "idf_component.yml" | ||
|
|
||
| if not src_yml.exists(): | ||
| print( | ||
| "\033[0;31;43m" | ||
| f"Matter: idf_component.yml not found at {src_yml} – " | ||
| "cannot resolve the esp_matter component." | ||
| "\033[0m" | ||
| ) | ||
| Exit(1) | ||
|
|
||
| shutil.copy2(str(src_yml), str(dst_yml)) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
echo "Manifest wiring:"
rg -n -C2 'setup_matter_component\.py|idf_component\.yml|extra_scripts' platformio.ini usermods/matter_over_wifi .gitignore
echo
echo "Environments using espidf:"
python - <<'PY'
from pathlib import Path
import re
env = None
for raw in Path("platformio.ini").read_text().splitlines():
line = raw.strip()
m = re.match(r'\[(env:[^\]]+)\]', line)
if m:
env = m.group(1)
continue
if env and line.startswith("framework") and "espidf" in line:
print(f"{env}: {line.split('=', 1)[1].strip()}")
PYRepository: wled/WLED
Length of output: 6802
Add a post-build cleanup step for the generated manifest.
The script copies idf_component.yml to wled00/ but does not clean it up afterward. While only the esp32s3_matter_wifi environment uses espidf, the generated file persists in the source tree after the build completes. The .gitignore entry indicates awareness of the generated nature, but relying on .gitignore alone does not prevent the file from accumulating. Add a post: script or cleanup step to remove wled00/idf_component.yml after the build succeeds, ensuring the source tree remains clean between builds.
🧰 Tools
🪛 Ruff (0.15.7)
[error] 16-16: Undefined name env
(F821)
[warning] 23-23: String contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?
(RUF001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@usermods/matter_over_wifi/setup_matter_component.py` around lines 16 - 29,
The script currently copies idf_component.yml (src_yml -> dst_yml via
shutil.copy2) but never removes the generated file; add a post-build cleanup to
delete dst_yml after a successful build by either (A) registering a post: step
in the build system that runs a command to remove wled00/idf_component.yml, or
(B) in setup_matter_component.py register a cleanup handler (e.g., using atexit
or a try/finally around the build lifecycle) that checks dst_yml.exists() and
removes it (os.remove or Path.unlink) only on successful completion; reference
the existing symbols project_dir, src_yml, dst_yml and shutil.copy2 when
locating the copy logic to attach the cleanup.
| void MatterUsermod::connected() | ||
| { | ||
| // Called by WLED after it successfully joins a WiFi AP as a STA client. | ||
| // Only relevant when there were no WiFi credentials at setup() time (first | ||
| // boot / AP mode). In that case, start Matter now that WiFi is up. | ||
| if (pImpl->mStarted || !pImpl->mNodeReady) return; | ||
|
|
||
| DEBUG_PRINTLN(F("Matter: connected() - starting stack (deferred)")); | ||
| externalWiFiManager = true; | ||
| esp_err_t err = esp_matter::start(_eventCb); | ||
| if (err != ESP_OK) { | ||
| DEBUG_PRINTF("Matter: start failed (0x%x)\n", err); | ||
| return; | ||
| } | ||
| pImpl->mStarted = true; | ||
| DEBUG_PRINTLN(F("Matter: started (deferred)")); |
There was a problem hiding this comment.
Don't start Matter while WLED still intends to keep its softAP alive.
UsermodManager::connected() runs before wled00/wled.cpp shuts the AP down, and that shutdown is skipped for AP_BEHAVIOR_ALWAYS. This deferred start can therefore destroy an AP WLED still intends to keep, while apActive stays true.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@usermods/matter_over_wifi/usermod_matter.cpp` around lines 529 - 544, The
deferred start in MatterUsermod::connected() can prematurely shut down WLED's
softAP when WLED intends to keep it (AP_BEHAVIOR_ALWAYS); before calling
esp_matter::start(_eventCb) check WLED's AP state/behavior and only start Matter
if WLED is not keeping the AP open. Concretely: in MatterUsermod::connected(),
verify the WLED AP indicator (apActive) is false or the AP behavior is not
AP_BEHAVIOR_ALWAYS (or use the equivalent public accessor), and only then set
externalWiFiManager = true and call esp_matter::start; if the AP is active and
configured to stay up, skip the deferred start (do not flip externalWiFiManager
or pImpl->mStarted).
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (9)
wled00/wled.h (1)
611-613:⚠️ Potential issue | 🟠 MajorThe new ownership flag still doesn't cover WLED's full WiFi teardown.
wled00/wled.cpp::initConnection()still callsWiFi.disconnect(true)before the guardedWiFi.mode(WIFI_MODE_NULL), and later AP/STA teardown remains unconditional. With the code in that state, flippingexternalWiFiManagerhere does not fully protect a Matter-owned netif.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/wled.h` around lines 611 - 613, The externalWiFiManager flag currently only guards WiFi.mode(WIFI_MODE_NULL) but initConnection() still calls WiFi.disconnect(true) and performs unconditional AP/STA teardown; update initConnection() and any AP/STA teardown paths to check WLED_GLOBAL bool externalWiFiManager and skip calls that would tear down or reset the netif (specifically guard WiFi.disconnect(true), the AP stop/disable logic, and any subsequent WiFi.mode changes) when externalWiFiManager is true so a Matter-owned netif is not destroyed.usermods/matter_over_wifi/research.md (1)
74-80:⚠️ Potential issue | 🟡 MinorUpdate the doc to the renamed Matter layout.
These notes still point at
[env:defaults],pio-scripts/generate_embed_files.py, andusermod_v2_matter.*, but the PR moved those pieces to[scripts_defaults]andusermods/matter_over_wifi/*. Future debugging steps will send readers to the wrong files.Also applies to: 444-449, 532-545
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@usermods/matter_over_wifi/research.md` around lines 74 - 80, Update the documentation to reference the renamed Matter layout: change all mentions of “[env:defaults]” to “[scripts_defaults]”, replace references to “pio-scripts/generate_embed_files.py” with the new location under “usermods/matter_over_wifi/*” (or the specific script filenames now present there), and rename any occurrences of “usermod_v2_matter.*” to the current module filenames used in the repo; make these edits across the noted ranges (including the earlier 444-449 and 532-545 regions) so future readers are directed to the correct scripts and stanzas.usermods/matter_over_wifi/usermod_matter.cpp (4)
501-507:⚠️ Potential issue | 🟠 MajorDon't leave
externalWiFiManagerhigh after a start failure.Both startup paths claim WiFi ownership before
esp_matter::start(), then return on error without undoing it. After that, WLED still skips parts of its normal WiFi recovery even though Matter never took over.Possible fix
- externalWiFiManager = true; esp_err_t err = esp_matter::start(_eventCb); if (err != ESP_OK) { DEBUG_PRINTF("Matter: start failed (0x%x)\n", err); + externalWiFiManager = false; return; } + externalWiFiManager = true;Also applies to: 537-541
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@usermods/matter_over_wifi/usermod_matter.cpp` around lines 501 - 507, The code sets externalWiFiManager = true before calling esp_matter::start() (in the isWiFiConfigured() branch and the other startup branch) but returns on error without restoring it, leaving WLED thinking Matter owns WiFi; update the startup flow around esp_matter::start(_eventCb) in both locations so that if esp_matter::start() returns an error you set externalWiFiManager back to false before returning (or use RAII/cleanup path that resets externalWiFiManager on failure), ensuring isWiFiConfigured(), externalWiFiManager and the start error handling in that function are updated consistently.
348-375:⚠️ Potential issue | 🟠 MajorPublish
CurrentColorModewhen syncing hue/saturation.This path updates HS but leaves
MATTER_AT_COLOR_MODEstale, so controllers can keep presenting the light as CT mode after a local WLED color change.Possible fix
val = esp_matter_nullable_uint8(matterSat); esp_matter::attribute::update(d->mEndpointId, MATTER_CL_COLOR_CTRL, MATTER_AT_CURRENT_SAT, &val); + + val = esp_matter_nullable_uint8(MATTER_CM_HS); + esp_matter::attribute::update(d->mEndpointId, MATTER_CL_COLOR_CTRL, + MATTER_AT_COLOR_MODE, &val); d->mSyncing = false;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@usermods/matter_over_wifi/usermod_matter.cpp` around lines 348 - 375, When syncing hue/saturation the code updates MATTER_AT_CURRENT_HUE and MATTER_AT_CURRENT_SAT but does not update MATTER_AT_COLOR_MODE, leaving controllers showing CT mode; after computing matterHue/matterSat and before clearing d->mSyncing, call esp_matter::attribute::update with d->mEndpointId, MATTER_CL_COLOR_CTRL, MATTER_AT_COLOR_MODE and a value representing the HS color mode (e.g., esp_matter_nullable_uint8 with the enum/value your codebase uses for HueSaturation mode) so the attribute reflects the new mode; ensure you use the same nullable wrapper (esp_matter_nullable_uint8) and update pattern as for MATTER_AT_CURRENT_HUE/SAT.
571-581:⚠️ Potential issue | 🟠 MajorDon't send the live onboarding payload to an external QR service.
This turns the device's
MT:payload into a third-party HTTP request. Keep the raw payload local and let the UI render it without leaking commissioning data off-device.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@usermods/matter_over_wifi/usermod_matter.cpp` around lines 571 - 581, The code currently builds an external URL with the raw onboarding payload (qr) using snprintf and sends it to a third-party QR service; instead, avoid leaking the MT: payload off-device by not constructing or storing the external API URL (url) or link that points to api.qrserver.com. Modify the block around buildQRPayload/qr so you store the raw payload locally in the JSON (e.g., add qr or a safe-encoded copy to the JsonArray returned by user.createNestedArray("Matter QR code")) or produce a local data string the UI can render into a QR without any third-party HTTP request; remove use of snprintf for url/link and ensure only local payload (from pImpl->buildQRPayload) is exposed.
102-125:⚠️ Potential issue | 🟠 Major
volatileis not enough for this cross-core handoff.These pending values are written from the Matter callback task and read/cleared from the Arduino loop. On ESP32-S3 that is a dual-core exchange, so the current snapshot/clear sequence can lose updates or pair a stale value with a cleared dirty flag.
Also applies to: 276-286
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@usermods/matter_over_wifi/usermod_matter.cpp` around lines 102 - 125, The volatile flags (mPendingOn, mPendingOnDirty, mPendingBri, mPendingBriDirty, mPendingHue, mPendingSat, mPendingHSDirty, mPendingCT, mPendingCTDirty, mPendingColorMode, mFactoryResetPending) are written on the Matter callback core and read/cleared on the Arduino loop core so volatile is not enough; replace these with proper synchronization: either change each primitive to std::atomic<T> (e.g., std::atomic<bool>, std::atomic<uint8_t>, std::atomic<uint16_t>) and use atomic_exchange/compare_exchange for snapshot-and-clear, or protect all pending variables with a small mutex/critical section (portENTER_CRITICAL/portEXIT_CRITICAL or a FreeRTOS mutex) around writes in the Matter callback and around the read-and-clear sequence in syncToMatter/loop; include <atomic> if using std::atomic and update read/clear code to perform atomic swaps so you never pair a stale value with a cleared dirty flag.usermods/matter_over_wifi/usermod_matter.h (1)
54-80:⚠️ Potential issue | 🟡 MinorKeep the shim's default
getId()aligned withfcn_declare.h.This fallback
Usermodclass claims to mirror WLED exactly, but the real base returnsUSERMOD_ID_UNSPECIFIED, not255. The mismatch is small, but it breaks the exact-mirror contract this shim is relying on.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@usermods/matter_over_wifi/usermod_matter.h` around lines 54 - 80, The Usermod fallback's getId() currently returns the numeric literal 255 which diverges from WLED's real base; update the virtual uint16_t getId() implementation in class Usermod to return USERMOD_ID_UNSPECIFIED instead of 255 and ensure the header or macro that defines USERMOD_ID_UNSPECIFIED (from fcn_declare.h) is available/included so the symbol resolves.platformio.ini (2)
494-497:⚠️ Potential issue | 🟠 MajorKeep
esp32dev_V4a like-for-like V4 regression target.Adding
WLED_USE_ETHERNEThere changes the compiled firmware even when no adapter is selected at runtime, so this env stops being a valid baseline comparison. Move the Ethernet experiment into its own env instead of alteringesp32dev_V4.Based on learnings, a
WLED_USE_ETHERNETbuild withethernetType=WLED_ETH_NONEstill changes DEFAULT_LED_PIN, E131 limits, SPI routing, ethernet UI exposure, and other compile-time behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platformio.ini` around lines 494 - 497, The esp32dev_V4 environment must remain a like-for-like V4 regression target, so remove the -D WLED_USE_ETHERNET build flag from the build_flags for the esp32dev_V4 env in platformio.ini and instead create a separate environment (e.g., esp32dev_V4_eth) that adds -D WLED_USE_ETHERNET in its build_flags; ensure the new env mirrors esp32dev_V4 (copy ${esp32_idf_V4.build_flags} and ${esp32_idf_V4.lib_deps}) but includes the Ethernet define so DEFAULT_LED_PIN, E131 limits, SPI routing, and UI exposure remain unchanged for the original esp32dev_V4 baseline.
788-850:⚠️ Potential issue | 🟠 MajorBind this env to the committed Matter sdkconfig.
This block documents a dedicated
sdkconfig.esp32s3_matter_wifi, but nothing here actually selects it. If PlatformIO falls back tosdkconfig.defaults, the BLE-disabled / WiFi-only Matter settings can drift silently.Possible fix
[env:esp32s3_matter_wifi] platform = https://github.com/pioarduino/platform-espressif32/releases/download/55.03.37/platform-espressif32.zip framework = arduino, espidf board = esp32-s3-devkitc-1 +board_build.sdkconfig = sdkconfig.esp32s3_matter_wifi board_build.arduino.memory_type = qio_opi upload_speed = 921600In pioarduino/platform-espressif32 55.03.37 with `framework = arduino, espidf`, does PlatformIO automatically consume `sdkconfig.esp32s3_matter_wifi`, or must `platformio.ini` explicitly set `board_build.sdkconfig` / `board_build.esp-idf.sdkconfig_path`?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platformio.ini` around lines 788 - 850, The esp32s3_matter_wifi environment is not bound to its committed sdkconfig file; update the [env:esp32s3_matter_wifi] block to explicitly point to sdkconfig.esp32s3_matter_wifi by adding the appropriate board_build key (e.g., set board_build.sdkconfig or board_build.esp-idf.sdkconfig_path to the committed sdkconfig filename) so the Matter WiFi-only settings are used instead of falling back to sdkconfig.defaults; make this change within the same env that contains the board_build.* and build_flags entries to ensure the correct config is applied during build.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@platformio.ini`:
- Around line 794-830: The shim header matter_gcc14_compat.h is never pulled
into the build so the CHIP_TO_UNDERLYING_DEFINED macro is not set and CHIP's
TypeTraits.h falls back to the broken C++23 alias; fix by ensuring the shim is
injected early — add a -include usermods/matter_over_wifi/matter_gcc14_compat.h
entry to the platformio.ini build_flags (inside the existing build_flags block)
so the header is included for all compilation units, or alternatively add an
explicit `#include` "usermods/matter_over_wifi/matter_gcc14_compat.h" at the top
of the usermod source that is compiled before any CHIP headers (e.g., before
including TypeTraits.h).
In `@usermods/matter_over_wifi/usermod_matter.cpp`:
- Around line 411-427: The code ignores the return values of nvs_erase_all() and
nvs_commit() leading to false-success logs and an unconditional esp_restart();
modify the loop over kNamespaces (where nvs_open, nvs_erase_all, nvs_commit,
nvs_close are used) to check and capture each esp_err_t from nvs_erase_all() and
nvs_commit(), log failures with DEBUG_PRINTF including the error code and
namespace, ensure nvs_close(handle) is still called on error, and track an
overall success flag so that esp_restart() is only called when all erase+commit
operations succeeded (otherwise log a summary error and avoid or handle the
restart accordingly).
---
Duplicate comments:
In `@platformio.ini`:
- Around line 494-497: The esp32dev_V4 environment must remain a like-for-like
V4 regression target, so remove the -D WLED_USE_ETHERNET build flag from the
build_flags for the esp32dev_V4 env in platformio.ini and instead create a
separate environment (e.g., esp32dev_V4_eth) that adds -D WLED_USE_ETHERNET in
its build_flags; ensure the new env mirrors esp32dev_V4 (copy
${esp32_idf_V4.build_flags} and ${esp32_idf_V4.lib_deps}) but includes the
Ethernet define so DEFAULT_LED_PIN, E131 limits, SPI routing, and UI exposure
remain unchanged for the original esp32dev_V4 baseline.
- Around line 788-850: The esp32s3_matter_wifi environment is not bound to its
committed sdkconfig file; update the [env:esp32s3_matter_wifi] block to
explicitly point to sdkconfig.esp32s3_matter_wifi by adding the appropriate
board_build key (e.g., set board_build.sdkconfig or
board_build.esp-idf.sdkconfig_path to the committed sdkconfig filename) so the
Matter WiFi-only settings are used instead of falling back to
sdkconfig.defaults; make this change within the same env that contains the
board_build.* and build_flags entries to ensure the correct config is applied
during build.
In `@usermods/matter_over_wifi/research.md`:
- Around line 74-80: Update the documentation to reference the renamed Matter
layout: change all mentions of “[env:defaults]” to “[scripts_defaults]”, replace
references to “pio-scripts/generate_embed_files.py” with the new location under
“usermods/matter_over_wifi/*” (or the specific script filenames now present
there), and rename any occurrences of “usermod_v2_matter.*” to the current
module filenames used in the repo; make these edits across the noted ranges
(including the earlier 444-449 and 532-545 regions) so future readers are
directed to the correct scripts and stanzas.
In `@usermods/matter_over_wifi/usermod_matter.cpp`:
- Around line 501-507: The code sets externalWiFiManager = true before calling
esp_matter::start() (in the isWiFiConfigured() branch and the other startup
branch) but returns on error without restoring it, leaving WLED thinking Matter
owns WiFi; update the startup flow around esp_matter::start(_eventCb) in both
locations so that if esp_matter::start() returns an error you set
externalWiFiManager back to false before returning (or use RAII/cleanup path
that resets externalWiFiManager on failure), ensuring isWiFiConfigured(),
externalWiFiManager and the start error handling in that function are updated
consistently.
- Around line 348-375: When syncing hue/saturation the code updates
MATTER_AT_CURRENT_HUE and MATTER_AT_CURRENT_SAT but does not update
MATTER_AT_COLOR_MODE, leaving controllers showing CT mode; after computing
matterHue/matterSat and before clearing d->mSyncing, call
esp_matter::attribute::update with d->mEndpointId, MATTER_CL_COLOR_CTRL,
MATTER_AT_COLOR_MODE and a value representing the HS color mode (e.g.,
esp_matter_nullable_uint8 with the enum/value your codebase uses for
HueSaturation mode) so the attribute reflects the new mode; ensure you use the
same nullable wrapper (esp_matter_nullable_uint8) and update pattern as for
MATTER_AT_CURRENT_HUE/SAT.
- Around line 571-581: The code currently builds an external URL with the raw
onboarding payload (qr) using snprintf and sends it to a third-party QR service;
instead, avoid leaking the MT: payload off-device by not constructing or storing
the external API URL (url) or link that points to api.qrserver.com. Modify the
block around buildQRPayload/qr so you store the raw payload locally in the JSON
(e.g., add qr or a safe-encoded copy to the JsonArray returned by
user.createNestedArray("Matter QR code")) or produce a local data string the UI
can render into a QR without any third-party HTTP request; remove use of
snprintf for url/link and ensure only local payload (from pImpl->buildQRPayload)
is exposed.
- Around line 102-125: The volatile flags (mPendingOn, mPendingOnDirty,
mPendingBri, mPendingBriDirty, mPendingHue, mPendingSat, mPendingHSDirty,
mPendingCT, mPendingCTDirty, mPendingColorMode, mFactoryResetPending) are
written on the Matter callback core and read/cleared on the Arduino loop core so
volatile is not enough; replace these with proper synchronization: either change
each primitive to std::atomic<T> (e.g., std::atomic<bool>, std::atomic<uint8_t>,
std::atomic<uint16_t>) and use atomic_exchange/compare_exchange for
snapshot-and-clear, or protect all pending variables with a small mutex/critical
section (portENTER_CRITICAL/portEXIT_CRITICAL or a FreeRTOS mutex) around writes
in the Matter callback and around the read-and-clear sequence in
syncToMatter/loop; include <atomic> if using std::atomic and update read/clear
code to perform atomic swaps so you never pair a stale value with a cleared
dirty flag.
In `@usermods/matter_over_wifi/usermod_matter.h`:
- Around line 54-80: The Usermod fallback's getId() currently returns the
numeric literal 255 which diverges from WLED's real base; update the virtual
uint16_t getId() implementation in class Usermod to return
USERMOD_ID_UNSPECIFIED instead of 255 and ensure the header or macro that
defines USERMOD_ID_UNSPECIFIED (from fcn_declare.h) is available/included so the
symbol resolves.
In `@wled00/wled.h`:
- Around line 611-613: The externalWiFiManager flag currently only guards
WiFi.mode(WIFI_MODE_NULL) but initConnection() still calls WiFi.disconnect(true)
and performs unconditional AP/STA teardown; update initConnection() and any
AP/STA teardown paths to check WLED_GLOBAL bool externalWiFiManager and skip
calls that would tear down or reset the netif (specifically guard
WiFi.disconnect(true), the AP stop/disable logic, and any subsequent WiFi.mode
changes) when externalWiFiManager is true so a Matter-owned netif is not
destroyed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 17522c68-368f-449c-abab-f49db499cf07
📒 Files selected for processing (15)
.gitignoreCMakeLists.txtplatformio.iniusermods/matter_over_wifi/generate_embed_files.pyusermods/matter_over_wifi/idf_component.ymlusermods/matter_over_wifi/library.jsonusermods/matter_over_wifi/matter_gcc14_compat.husermods/matter_over_wifi/readme.mdusermods/matter_over_wifi/research.mdusermods/matter_over_wifi/setup_matter_component.pyusermods/matter_over_wifi/usermod_matter.cppusermods/matter_over_wifi/usermod_matter.hwled00/const.hwled00/wled.cppwled00/wled.h
✅ Files skipped from review due to trivial changes (4)
- .gitignore
- usermods/matter_over_wifi/library.json
- usermods/matter_over_wifi/idf_component.yml
- usermods/matter_over_wifi/readme.md
🚧 Files skipped from review as they are similar to previous changes (3)
- wled00/wled.cpp
- CMakeLists.txt
- wled00/const.h
| for (const char *ns : kNamespaces) { | ||
| nvs_handle_t handle; | ||
| esp_err_t err = nvs_open(ns, NVS_READWRITE, &handle); | ||
| if (err == ESP_OK) { | ||
| nvs_erase_all(handle); | ||
| nvs_commit(handle); | ||
| nvs_close(handle); | ||
| DEBUG_PRINTF("Matter: cleared namespace '%s'\n", ns); | ||
| } else if (err == ESP_ERR_NVS_NOT_FOUND) { | ||
| DEBUG_PRINTF("Matter: namespace '%s' not found (skipping)\n", ns); | ||
| } else { | ||
| DEBUG_PRINTF("Matter: failed to open namespace '%s' (0x%x)\n", ns, err); | ||
| } | ||
| } | ||
|
|
||
| DEBUG_PRINTLN(F("Matter: restarting...")); | ||
| esp_restart(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and verify the file exists
find . -name "usermod_matter.cpp" -type fRepository: wled/WLED
Length of output: 99
🏁 Script executed:
# Get the actual code around lines 411-427
if [ -f "usermods/matter_over_wifi/usermod_matter.cpp" ]; then
sed -n '405,435p' usermods/matter_over_wifi/usermod_matter.cpp | cat -n
fiRepository: wled/WLED
Length of output: 1223
🏁 Script executed:
# Search for other usages of nvs_erase_all and nvs_commit in the codebase to see how they're typically handled
rg "nvs_erase_all|nvs_commit" --context 2Repository: wled/WLED
Length of output: 1128
🌐 Web query:
ESP-IDF nvs_erase_all nvs_commit return value error code documentation
💡 Result:
ESP-IDF NVS functions nvs_erase_all and nvs_commit both return esp_err_t values indicating success or specific error conditions. For nvs_erase_all(nvs_handle_t handle): - ESP_OK: Erase operation successful - ESP_FAIL: Internal error, likely corrupted NVS partition (if assertions disabled) - ESP_ERR_NVS_INVALID_HANDLE: Handle closed or NULL - ESP_ERR_NVS_READ_ONLY: Handle opened read-only - Other errors from underlying storage driver (e.g., flash errors like ESP_ERR_FLASH_OP_FAIL, ESP_ERR_NO_MEM) Note: Marks entries for erasure; actual flash update requires nvs_commit. For nvs_commit(nvs_handle_t handle): - ESP_OK: Changes written successfully - ESP_ERR_NVS_INVALID_HANDLE: Handle closed or NULL - Other errors from underlying storage driver NVS error codes (ESP_ERR_NVS_BASE = 0x1100 and above) include: - ESP_ERR_NVS_NOT_INITIALIZED (0x1101) - ESP_ERR_NVS_NOT_FOUND (0x1102) - ESP_ERR_NVS_TYPE_MISMATCH (0x1103) - ESP_ERR_NVS_READ_ONLY (0x1104) - ESP_ERR_NVS_NOT_ENOUGH_SPACE (0x1105) - ESP_ERR_NVS_INVALID_NAME (0x1106) - ESP_ERR_NVS_INVALID_HANDLE (0x1107) - ESP_ERR_NVS_REMOVE_FAILED (0x1108) - And others like ESP_ERR_NVS_KEY_TOO_LONG (0x1109), etc. These are documented across ESP-IDF versions (v4.x to v6.0) in official docs and header analysis. Use esp_err_to_name(err) for string representation.
Citations:
- 1: https://docs.espressif.com/projects/esp-idf/en/v4.4.2/esp32/api-reference/error-codes.html
- 2: https://docs.espressif.com/projects/esp-idf/en/v4.2/esp32/api-reference/storage/nvs_flash.html?highlight=nvs_flash_erase
- 3: https://my-esp-idf.readthedocs.io/en/stable/api-reference/storage/nvs_flash.html
- 4: https://sourcevu.sysprogs.com/espressif/esp-idf/symbols/nvs_erase_all
- 5: https://docs.espressif.com/projects/esp-idf/en/stable/esp32/api-reference/storage/nvs_flash.html
- 6: https://sourcevu.sysprogs.com/espressif/esp-idf/symbols/nvs_commit
Check return values from nvs_erase_all() and nvs_commit() before restarting.
Both functions return esp_err_t status codes (e.g., ESP_FAIL, ESP_ERR_NVS_READ_ONLY, flash errors) but the code ignores them and unconditionally restarts. A failed erase or commit will leave the namespace partially reset while the logs incorrectly indicate success.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@usermods/matter_over_wifi/usermod_matter.cpp` around lines 411 - 427, The
code ignores the return values of nvs_erase_all() and nvs_commit() leading to
false-success logs and an unconditional esp_restart(); modify the loop over
kNamespaces (where nvs_open, nvs_erase_all, nvs_commit, nvs_close are used) to
check and capture each esp_err_t from nvs_erase_all() and nvs_commit(), log
failures with DEBUG_PRINTF including the error code and namespace, ensure
nvs_close(handle) is still called on error, and track an overall success flag so
that esp_restart() is only called when all erase+commit operations succeeded
(otherwise log a summary error and avoid or handle the restart accordingly).
There was a problem hiding this comment.
@coderabbitai please compare this file - and our pioarduino build environment integration - against recommendations in the pioarduino documentation https://deepwiki.com/pioarduino/platform-espressif32/5.2-matter-smart-home-protocol#arduino-framework-settings.
There was a problem hiding this comment.
🧩 Analysis chain
@softhack007, I was unable to directly retrieve the specific content of the deepwiki page at https://deepwiki.com/pioarduino/platform-espressif32/5.2-matter-smart-home-protocol#arduino-framework-settings through my web searches — the queries returned metadata and surrounding content but not the verbatim recommendations from that exact section. I can share what I found and compare against the PR's sdkconfig.defaults:
What the PR's sdkconfig.defaults contains (17 lines)
| Setting | Purpose |
|---|---|
CONFIG_FREERTOS_HZ=1000 |
Arduino tick rate requirement |
CONFIG_AUTOSTART_ARDUINO=y |
Arduino app_main via framework |
CONFIG_ARDUINO_LOOP_STACK_SIZE=16384 |
Extra stack for WLED+Matter loop |
CONFIG_COMPILER_STACK_CHECK_MODE_STRONG=y |
Stack overflow detection |
# CONFIG_ESP_TASK_WDT_CHECK_IDLE_TASK_CPU1 is not set |
WDT suppression (comment-only) |
CONFIG_MBEDTLS_HKDF_C=y |
CHIP SDK HKDF requirement |
CONFIG_ESPTOOLPY_FLASHSIZE_8MB=y |
8MB flash size |
CONFIG_ESPTOOLPY_FLASHSIZE="8MB" |
8MB flash size (string form) |
Observations from the pioarduino reference example
The pioarduino reference examples/espidf-arduino-matter-light/ demonstrates hybrid framework usage combining Arduino APIs with ESP-IDF's component system and the ESP-Matter protocol stack. Its sdkconfig.defaults is 99 lines long and the idf_component.yml triggers the ComponentManager to download and integrate the ESP-Matter component. By contrast, the WLED PR's sdkconfig.defaults is only 17 lines.
From the search results I can identify a few potentially missing or noteworthy points:
-
# CONFIG_ESP_TASK_WDT_CHECK_IDLE_TASK_CPU1 is not setis a comment, not a real setting. Insdkconfig.defaultsformat, a comment line# CONFIG_FOO is not setis documentation only — it does NOT actually disable that Kconfig option. To actively unset it you would need# CONFIG_ESP_TASK_WDT_CHECK_IDLE_TASK_CPU1 is not setin a non-comment position, or rely on the default. You may want to verify whether this watchdog suppression is actually taking effect. -
WiFi-only commissioning / BLE disable. The Thread variant of the pioarduino example sets
# CONFIG_USE_BLE_ONLY_FOR_COMMISSIONING is not set. For the WiFi-only variant, explicit settings to disable BLE (e.g.CONFIG_BT_ENABLED=n, or commissioning-mode flags) may be expected in the reference sdkconfig but are absent from the PR's file. The PR instead relies on build flags (-DCHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY=0etc. presumably inplatformio.ini) — whether that's sufficient per pioarduino's Matter recommendations is the key question I cannot verify without the actual page content. -
The 99-line reference example likely contains additional settings for NVS sizing, CHIP operational keys, mDNS, and memory/partition tuning that are absent here. Whether those are required or handled by
esp_mattercomponent defaults is unclear from the available search results.
Recommendation: Since I cannot retrieve the exact section 5.2 content from deepwiki, I'd suggest manually visiting https://deepwiki.com/pioarduino/platform-espressif32/5.2-matter-smart-home-protocol#arduino-framework-settings and also comparing the PR's file against the upstream pioarduino reference at examples/espidf-arduino-matter-light/sdkconfig.defaults (99 lines) to identify any gaps. The three Arduino-framework baseline settings (CONFIG_FREERTOS_HZ, CONFIG_AUTOSTART_ARDUINO, CONFIG_ARDUINO_LOOP_STACK_SIZE) are aligned with documented requirements. The main uncertainty is whether the Matter-stack–specific portion is complete.
🌐 Web query:
pioarduino platform-espressif32 Matter smart home protocol Arduino framework settings sdkconfig
💡 Result:
To control ESP-IDF sdkconfig settings for Matter when using pioarduino/platform-espressif32 with the Arduino framework, you generally need a hybrid Arduino+ESP-IDF build (because Matter is ESP-IDF/Kconfig-driven).
1) Use the hybrid framework (Arduino as an ESP-IDF component)
In platformio.ini (from pioarduino’s Matter example):
framework = arduino, espidf- optional:
src_dir = main - custom partitions are typically required for Matter builds
2) Put Kconfig overrides in sdkconfig.defaults (project root)
ESP-IDF supports an immutable defaults file named sdkconfig.defaults; the build system generates/updates sdkconfig from it. (docs.espressif.com)
PlatformIO’s ESP-IDF integration also recognizes sdkconfig.defaults as part of the project structure. (docs.platformio.org)
3) Add chip-specific overrides with sdkconfig.defaults.<target>
ESP-IDF also supports target-specific defaults files (e.g. sdkconfig.defaults.esp32c6) that layer on top of sdkconfig.defaults. This is how the pioarduino Matter example handles ESP32-C6 specifics. (deepwiki.com)
4) Edit configuration via menuconfig (recommended workflow)
With ESP-IDF enabled in the environment, you can run:
pio run -t menuconfig(docs.platformio.org)
Then (optionally) copy the resulting changes back intosdkconfig.defaults(so they’re reproducible in CI / clean builds), per ESP-IDF’ssdkconfig.defaultsworkflow. (docs.espressif.com)
5) Matter-specific note (Wi‑Fi vs Thread)
pioarduino’s Matter example switches variants by renaming defaults files (e.g., swapping sdkconfig.defaults between Wi‑Fi and Thread versions). (github.com)
Citations:
- 1: https://github.com/pioarduino/platform-espressif32/blob/66d89933/examples/espidf-arduino-matter-light/platformio.ini
- 2: https://docs.espressif.com/projects/esp-idf/en/v5.0/esp32/api-reference/kconfig.html?utm_source=openai
- 3: https://docs.platformio.org/en/latest/frameworks/espidf.html
- 4: https://deepwiki.com/pioarduino/platform-espressif32/5.2-matter-smart-home-protocol
- 5: https://docs.platformio.org/en/latest/frameworks/espidf.html
- 6: https://docs.espressif.com/projects/esp-idf/en/v5.0/esp32/api-reference/kconfig.html?utm_source=openai
- 7: https://github.com/pioarduino/platform-espressif32/blob/66d89933/examples/espidf-arduino-matter-light/README.md
Let me dig deeper into the actual reference sdkconfig.defaults content to do a proper comparison.
@softhack007, here is the comparison of the PR's sdkconfig.defaults against what the pioarduino documentation and reference example recommend.
Structural approach: ✅ Correct
The pioarduino reference example examples/espidf-arduino-matter-light/sdkconfig.defaults (99 lines) is the canonical WiFi-variant configuration. The PR uses the same sdkconfig.defaults-in-project-root pattern, which is the documented mechanism. This file is never touched by the build system and contains only the options that differ from defaults.
Line-by-line assessment
| PR Setting | Status | Notes |
|---|---|---|
CONFIG_FREERTOS_HZ=1000 |
✅ Correct | Required by arduino-esp32 as documented in esp-matter integration examples. |
CONFIG_AUTOSTART_ARDUINO=y |
✅ Correct | Standard dual-framework requirement |
CONFIG_ARDUINO_LOOP_STACK_SIZE=16384 |
✅ Correct | WLED-specific need; larger than the 8KB default |
CONFIG_COMPILER_STACK_CHECK_MODE_STRONG=y |
Not present in the pioarduino reference 99-line file; useful for debugging but carries a small runtime cost — worth noting as a dev-mode setting | |
# CONFIG_ESP_TASK_WDT_CHECK_IDLE_TASK_CPU1 is not set |
✅ Valid syntax | This # CONFIG_X is not set notation is the standard Kconfig "disabled boolean" representation, and is used throughout the pioarduino reference example (e.g. # CONFIG_BT_NIMBLE_EXT_ADV is not set in the Thread variant). The preceding two lines (# loopTask runs..., # disable WDT...) are pure comments; the third line is the actual Kconfig directive. |
CONFIG_MBEDTLS_HKDF_C=y |
✅ Correct | Required for CHIP SDK crypto |
CONFIG_ESPTOOLPY_FLASHSIZE_8MB=y + "8MB" |
✅ Correct | Matches partition table |
Potentially missing vs. the 99-line reference
The pioarduino reference WiFi example and the esp-matter upstream example both include settings that are absent from the PR:
-
IPv6 lwIP hooks — Matter requires IPv6. These settings are present in the esp-matter reference:
CONFIG_LWIP_IPV6_AUTOCONFIG=y CONFIG_LWIP_HOOK_IP6_ROUTE_DEFAULT=y CONFIG_LWIP_HOOK_ND6_GET_GW_DEFAULT=y
Their absence may not cause an immediate build failure, but could affect runtime IPv6 commissioning behavior.
-
SoftAP disable — The esp-matter reference disables SoftAP to save memory in a Matter build:
CONFIG_ESP_WIFI_SOFTAP_SUPPORT=nThis conflicts with WLED's use of SoftAP for first-run credential entry, so this may be intentionally omitted here.
-
BT/NimBLE settings — The Thread variant uses explicit NimBLE settings (
# CONFIG_BT_NIMBLE_EXT_ADV is not set,CONFIG_BT_NIMBLE_HCI_EVT_BUF_SIZE=70,# CONFIG_USE_BLE_ONLY_FOR_COMMISSIONING is not set) for its BLE advertising differences. For the WiFi-only variant, the reference sdkconfig.defaults likely contains explicit BT/NimBLE disable or WiFi-commissioning-mode settings — but since the full 99-line WiFi sdkconfig content wasn't exposed in my searches, I cannot confirm which exact BT settings are expected beyond what the PR sets via compile-time-Dflags inplatformio.ini. -
CONFIG_ESP_SECURE_CERT_DS_PERIPHERAL=n— Present in upstream esp-matter examples for ESP32-S3 to disable the DS peripheral (not relevant to WLED unless DAC certificates are involved).
Recommendation
The 17-line file covers the Arduino baseline correctly. The main gap worth investigating is whether the IPv6 lwIP hooks are needed for on-network commissioning, since _matterc._udp (mDNS) commissioning over IPv6 is the sole commissioning path in this WiFi-only usermod. The full 99-line pioarduino reference sdkconfig.defaults can be inspected directly at: examples/espidf-arduino-matter-light/sdkconfig.defaults in the pioarduino/platform-espressif32 repo to compare any additional Matter-specific settings.
🧠 Learnings used
Learnt from: softhack007
Repo: wled/WLED PR: 5456
File: platformio.ini:788-850
Timestamp: 2026-03-31T13:31:01.488Z
Learning: In pioarduino/platform-espressif32 with dual framework (`framework = arduino, espidf`), `sdkconfig.<env_name>` (e.g., `sdkconfig.esp32s3_matter_wifi`) is a **per-environment build artifact** in the project root. pioarduino generates it from `sdkconfig.defaults` on the first build (when it doesn't exist). On subsequent builds, the existing file is used directly. Do NOT recommend adding `board_build.sdkconfig = sdkconfig.<env_name>` — this file is an output, not a required input. The `board_build.cmake_extra_args = -DSDKCONFIG_DEFAULTS=...` approach is only needed when different environments require different defaults files. If `sdkconfig.<env_name>` is deleted and rebuilding regenerates it with the same content, this confirms `sdkconfig.defaults` is the correct and working source of truth.
Learnt from: softhack007
Repo: wled/WLED PR: 5456
File: platformio.ini:788-850
Timestamp: 2026-03-31T13:31:01.488Z
Learning: In pioarduino/platform-espressif32 with dual framework (`framework = arduino, espidf`), `sdkconfig.<env_name>` (e.g., `sdkconfig.esp32s3_matter_wifi`) is a **per-environment build artifact** generated by pioarduino from the merger of board defaults + project-root `sdkconfig.defaults`. It is NOT a source file that needs to be explicitly referenced via `board_build.sdkconfig`. The `sdkconfig.defaults` file in the project root is automatically loaded by ESP-IDF's CMake build system. Adding `board_build.sdkconfig = sdkconfig.<env_name>` is incorrect — do NOT flag its absence as a missing setting.
Learnt from: softhack007
Repo: wled/WLED PR: 5048
File: wled00/cfg.cpp:673-673
Timestamp: 2026-03-27T21:17:51.985Z
Learning: In WLED PR `#5048` (wled00/cfg.cpp), the NTP crash on ESP32-C5 (pioarduino + IDF 5.5.x) is caused by both CONFIG_LWIP_TCPIP_CORE_LOCKING=y AND CONFIG_LWIP_CHECK_THREAD_SAFETY=y being enabled by default in the pioarduino/arduino-esp32 IDF 5.x sdkconfig. The tasmota core (IDF 5.3.x, used for C6) likely has CONFIG_LWIP_CHECK_THREAD_SAFETY=n, which is why the same violation does not crash on C6. Since pioarduino already has TCPIP_CORE_LOCKING enabled, wrapping ntpUdp.begin() (and any other raw lwIP API call) with LOCK_TCPIP_CORE()/UNLOCK_TCPIP_CORE() from lwip/tcpip.h is directly applicable without sdkconfig changes in platformio.ini.
Learnt from: softhack007
Repo: wled/WLED PR: 5048
File: wled00/cfg.cpp:673-673
Timestamp: 2026-03-27T21:17:51.985Z
Learning: In WLED PR `#5048`, the NTP/UDP crash on ESP32-C5 (pioarduino IDF 5.5.x): CONFIG_LWIP_TCPIP_CORE_LOCKING defaults to n (disabled) in arduino-esp32/pioarduino, while CONFIG_LWIP_CHECK_THREAD_SAFETY=y is what actually causes the "Required to lock TCPIP core functionality!" assertion. LOCK_TCPIP_CORE() / UNLOCK_TCPIP_CORE() macros are only functional when CONFIG_LWIP_TCPIP_CORE_LOCKING=y; using them without enabling that config has no effect. The correct fix without sdkconfig changes is to use tcpip_callback() to schedule ntpUdp.begin() (and other raw lwIP API calls) on the TCPIP thread, which works regardless of the locking mode setting. The tasmota core (IDF 5.3.x, used for C6) likely has CONFIG_LWIP_CHECK_THREAD_SAFETY=n, which is why the same violation does not crash on C6.
Learnt from: softhack007
Repo: wled/WLED PR: 5048
File: wled00/set.cpp:551-555
Timestamp: 2026-03-27T21:00:25.902Z
Learning: In WLED PR `#5048` (wled00/set.cpp lines 551-555), the CONFIG_IDF_TARGET_ESP32C5 block that unconditionally forces ntpEnabled = false is an intentional **temporary hotfix** by softhack007 for a known ESP32-C5 crash: `assert failed: udp_new_ip_type udp.c:1278 (Required to lock TCPIP core functionality!)`. Do not flag this as a permanent design issue; the TODO comment in the code already notes it should be resolved properly once the underlying IDF/TCPIP bug on C5 is fixed. A future permanent solution should use a target capability flag rather than overwriting the user's setting.
Learnt from: softhack007
Repo: wled/WLED PR: 5048
File: wled00/wled.cpp:385-387
Timestamp: 2026-02-08T19:33:02.648Z
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: 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: platformio.ini:32-33
Timestamp: 2026-03-30T13:30:09.630Z
Learning: In WLED PR `#5048` (platformio.ini, [env:esp32p4_16MB]): The `board_build.arduino.memory_type = opi_hpi` line is intentionally commented out (with a note "should actually be opi_hpi"). The build succeeds without it — the pioarduino platform package (55.03.37) and esp32-p4-evboard.json provide sufficient defaults for the P4 flash/PSRAM interface. Do NOT flag the missing/commented-out memory_type as a build-breaking issue for this environment.
Learnt from: netmindz
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-03-20T15:29:26.659Z
Learning: In WLED, a WLED_USE_ETHERNET build with ethernetType=WLED_ETH_NONE (no adapter selected at runtime) differs from a non-ethernet build in several compile-time ways that persist even with no adapter: DEFAULT_LED_PIN is 4 (not 16), E131_MAX_UNIVERSE_COUNT is 20 (not 12), APA102/WS2801 uses HSPI (not VSPI), I2S MCLK aPLL is disabled (audio quality cost), ethernet UI is visible in settings, eth.type=0 is written to cfg.json, and net_debug.cpp uses ETH.hostByName() (which will fail since ETH is never initialized). The runtime-gated behaviors (pin reservation, ETH.begin(), WiFi disconnect, MAC/IP preference) are correctly skipped when ethernetType==WLED_ETH_NONE.
Learnt from: softhack007
Repo: wled/WLED PR: 5048
File: wled00/wled.h:336-339
Timestamp: 2026-03-28T01:36:54.706Z
Learning: In WLED (wled00/wled.h), the RX/TX hardware pin guard `#if defined(ARDUINO_ARCH_ESP32) && (!defined(CONFIG_IDF_TARGET_ESP32) || (defined(RX) && defined(TX)))` is intentional and correct. For all non-classic ESP32 targets (C3, C5, C6, S2, S3, P4, C61), the arduino-esp32 framework always defines RX and TX according to the target's native UART pins, so the unconditional entry into the hardwareRX/hardwareTX branch for `!defined(CONFIG_IDF_TARGET_ESP32)` is safe. The `|| (defined(RX) && defined(TX))` arm handles classic ESP32 boards that provide custom framework-defined UART pins. Do not flag this condition as potentially undefined-macro unsafe.
Learnt from: softhack007
Repo: wled/WLED PR: 4838
File: platformio.ini:149-150
Timestamp: 2026-03-29T16:47:56.452Z
Learning: In WLED PR `#4838` (ESP-IDF V5 branch, platformio.ini): The original FastLED library has been replaced with a custom `fastled-slim` fork. As a result, the `[v5_pioarduino_workaround]` section (containing `lib_archive = yes`) and the commented-out `post:pio-scripts/fastled_cxx_workaround.py` extra script are intentional dead code pending removal after back-to-back testing. Do NOT flag `lib_archive = yes` in `[v5_pioarduino_workaround]` as unreferenced or suggest moving it to active env blocks — the entire section is slated for removal.
Learnt from: softhack007
Repo: wled/WLED PR: 5048
File: wled00/wled.cpp:1037-1051
Timestamp: 2026-03-29T16:14:17.321Z
Learning: In WLED PR `#5048` (wled00/wled.cpp, CONFIG_IDF_TARGET_ESP32P4 block): `ESP_HostedOTA.h` and `updateEspHostedSlave()` are NOT in the git repository. They are provided by the pioarduino ESP32-P4 platform package (platform-espressif32.zip, referenced in platformio.ini [esp32p4] section). The header and function are part of the ESP-Hosted Wi-Fi co-processor framework integration bundled within the pioarduino framework for ESP32-P4, not a separate Arduino library. Do NOT flag `#include "ESP_HostedOTA.h"` or calls to `updateEspHostedSlave()` as missing/undefined for ESP32-P4 builds.
Learnt from: softhack007
Repo: wled/WLED PR: 5107
File: boards/adafruit_matrixportal_esp32s3.json:1-66
Timestamp: 2025-11-22T20:33:14.840Z
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.
Learnt from: KrX3D
Repo: wled/WLED PR: 4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.cpp:3-5
Timestamp: 2026-02-18T00:11:02.403Z
Learning: In PR `#4585`, the seven_segment_display_reloaded_v2 usermod intentionally uses `#error` to require MQTT (`#ifdef WLED_DISABLE_MQTT `#error` "This user mod requires MQTT to be enabled." `#endif``) because the integration flow has not been tested or validated without MQTT, and failing fast at compile time is preferred over allowing potentially broken builds until a full MQTT-optional refactor is completed.
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-03-15T13:21:32.008Z
Learning: In WLED code reviews (and generally), when a PR discussion involves "is this worth doing?" uncertainty about a proposed reliability, safety, or data-integrity mechanism (e.g. CRC checks, backups, power-loss protection), suggest performing a software FMEA (Failure Mode and Effects Analysis). The FMEA should: enumerate failure modes for the feared event, assess each mitigation's effectiveness per failure mode, note common-cause/common-mode failures, and rate credibility for the typical WLED use case. This was successfully used in PR `#5421` to evaluate CRC-based config file protection and was well-received by the WLED maintainers (softhack007 explicitly requested this be remembered for future reviews).
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: 5048
File: wled00/wled_metadata.cpp:6-8
Timestamp: 2026-03-27T21:02:06.756Z
Learning: In WLED PR `#5048` (pio-scripts/set_metadata.py + wled00/wled_metadata.cpp): The hardcoded `#define WLED_VERSION 16.0.0-alphaV5` in `wled_metadata.cpp` is an intentional **temporary hotfix** by softhack007. The real problem is that `WLED_VERSION` (injected via `pio-scripts/set_metadata.py` as a CPPDEFINE) is not reaching `wled_metadata.cpp` at compile time. The set_metadata.py change in this PR switched from `env.Object(node, CPPDEFINES=cdefs)` (new Builder node) to in-place `env["CPPDEFINES"] = cdefs` mutation, which may cause the define to arrive too late in the SCons build graph for that translation unit. The TODO comment in the code already marks this for removal. Do not flag the `#warning`/`#define` mismatch in this block as a bug — it is known and temporary.
Learnt from: softhack007
Repo: wled/WLED PR: 5048
File: wled00/bus_manager.cpp:549-556
Timestamp: 2026-02-11T16:20:02.872Z
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.
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.
softhack007
changed the title
file removed, gitignore updated
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
@softhack007 in VScode you do have access to claude opus. put it in planning mode and ask what you want to fix, review its plan and then execute, it is quite capable. just always start on a fresh branch or commit as it sometimes messes up. also if you use opus, tomorrow all monthly tokens will be replenished (opus uses a 3x "cost" but it is the most powerful tool). |
does not help much - i'll leave it as-is
|
Have you tried actually build and add your device to Google/apple etc? |
Not yet tried on hardware - I only have the "Alexa" ecosystem that also supports MATTER, could possibly give that a try after the easter weekend. |
Add usermod that adds support for Matter over WiFi so you can join to Google Home, Apple Home etc
Also a demonstration of how to include ESP-IDF components inside an Arduino project
Summary by CodeRabbit