Conversation
- Create usermods/zigbee_rgb_light/usermod_zigbee_rgb_light.h with full implementation: FreeRTOS task for Zigbee stack, attribute set callbacks for On/Off + Level Control + Color Control clusters, colorHStoRGB() conversion, and config persistence via addToConfig/readFromConfig - Create usermods/zigbee_rgb_light/README.md with hardware requirements, enable instructions, WiFi/Zigbee coexistence notes, and platformio_override.ini example - Create wled00/usermods_list.cpp to conditionally include/register the usermod under #ifdef USERMOD_ZIGBEE_RGB_LIGHT - Add USERMOD_ID_ZIGBEE_RGB_LIGHT (59) to wled00/const.h Co-authored-by: netmindz <442066+netmindz@users.noreply.github.com> Agent-Logs-Url: https://github.com/wled/WLED/sessions/fb0cc4ff-60ec-4e31-ba9d-19027c40294a
Address code review feedback: protect shared state between the Zigbee FreeRTOS task and the WLED main loop with a FreeRTOS mutex. The attribute callback and applyState() now acquire zbStateMutex before reading/writing powerOn, zbBrightness, zbHue, zbSaturation. Co-authored-by: netmindz <442066+netmindz@users.noreply.github.com> Agent-Logs-Url: https://github.com/wled/WLED/sessions/fb0cc4ff-60ec-4e31-ba9d-19027c40294a
- Remove wled00/usermods_list.cpp (old registration approach) - Add usermods/zigbee_rgb_light/library.json with Zigbee lib deps - Rename .h to .cpp following other usermod conventions - Update README to document custom_usermods instead of build flags Co-authored-by: netmindz <442066+netmindz@users.noreply.github.com> Agent-Logs-Url: https://github.com/wled/WLED/sessions/e8f06557-562a-4982-84fc-298a483eb548
Co-authored-by: netmindz <442066+netmindz@users.noreply.github.com> Agent-Logs-Url: https://github.com/wled/WLED/sessions/e8f06557-562a-4982-84fc-298a483eb548
Use a custom linker section to hold dynamic arrays such as the usermod list. This provides an extensible solution for wider use in the future (such as FX or Bus drivers), as well as IDF v5 compatibility.
Replace the old dynarray.py (which appended tools/dynarray.ld as a separate linker script) with the V5 version that patches sections.ld in-place, injecting .dynarray entries inside the existing .flash.rodata output section. This avoids creating an extra ELF PT_LOAD segment that caused the ESP-IDF v5 bootloader to assert: rom_index < 2. Also update validate_modules.py to the V5 version (nm-based validation), add wled00/dynarray.h (generic DECLARE_DYNARRAY/DYNARRAY_MEMBER macros), and delete the now-obsolete tools/dynarray.ld and tools/esp8266_rodata.ld.
Move esp_coex_wifi_i154_enable() from the ZigbeeRGBLightUsermod constructor to setup(). The constructor runs during static initialization (do_global_ctors), before the IDF coexistence subsystem is ready, causing a null pointer dereference (Load access fault, MTVAL=0x10). Moving it to setup() is safe because UsermodManager::setup() runs before WiFi.mode(WIFI_STA) in wled.cpp. Also add library.json and zigbee_rgb_light.cpp for the custom_usermods + REGISTER_USERMOD() pattern used by the V5 dynarray build system.
Wrap all WLED_MAX_DIGITAL_CHANNELS #define directives in const.h with #ifndef guards so that a -D flag on the compiler command line takes precedence without triggering redefinition warnings. This is needed for the Zigbee build which sets -DWLED_MAX_DIGITAL_CHANNELS=0 to disable RMT-based LED buses (the 802.15.4 radio shares the RMT peripheral).
Add custom_usermods = zigbee_rgb_light so the V5 build system automatically discovers and links the Zigbee usermod via library.json and REGISTER_USERMOD().
|
Important Review skippedDraft detected. 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:
WalkthroughIntroduces a new Zigbee RGB Light usermod for ESP32-C6 that integrates WLED with the ESP Zigbee stack to expose the device as a Zigbee HA Color Dimmable Light endpoint. Includes build configuration, library dependencies, comprehensive documentation, and supporting core framework updates. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
netmindz
added
enhancement
connectivity
|
@coderabbitai review |
…reconnect WLED's initConnection() calls WiFi.mode(WIFI_MODE_NULL) during WiFi reconnection, which tears down the WiFi stack and resets the 802.15.4 coexistence arbiter configuration. This caused intermittent WiFi drops when the Zigbee radio was active. Changes: - Re-call esp_coex_wifi_i154_enable() and re-apply IEEE802154_MIDDLE priority in connected() override on every WiFi reconnect - Fix HUE_ECHO_WINDOW_MS constant from 5000 to 1500 to match actual usage in echo suppression logic - Replace hardcoded 1500ms values in zbUpdateState() with the HUE_ECHO_WINDOW_MS constant Test results improved from 22/31 to 28/31 (best run), with sustained HTTP going from complete failure to 10/10 at avg 0.89s response time.
|
@coderabbitai how much flash (program size) increase should we expect from integrating the ESP32 Zigbee stack? |
|
Tip For best results, initiate chat on the files or code changes.
Estimated flash increase: ~300–500 KB of additional program code. Here's the breakdown:
Bottom line: for a definitive number, the cleanest approach would be to run 🧠 Learnings used |
This comment was marked as duplicate.
This comment was marked as duplicate.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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/zigbee_rgb_light/README.md`:
- Around line 16-18: Update the README wording to remove the implication that
WS2812B/SK6812/NeoPixel strips are supported natively; explicitly state that the
shipped build compiles with -DWLED_MAX_DIGITAL_CHANNELS=0 so direct digital LED
buses are disabled, and document that onboard analog RGB/RGBW outputs (PWM/LEDC)
are available for non-addressable LEDs or that addressable pixel strips require
an external controller; reference the existing lines mentioning "ESP32-C6" and
"Standard WS2812B / SK6812 / NeoPixel" so you replace or augment that text with
the clarified phrasing.
In `@usermods/zigbee_rgb_light/research-phillips-hue.md`:
- Around line 155-159: Update the "WLED Zigbee usermod
(`/home/will/netmindz/WLED/`)" section to reflect current state: either mark the
paragraph as historical or change wording from present tense ("on hold", "still
has Router mode", "pairing is fully working") to accurately state that the
implementation now initializes ESP_ZB_DEVICE_TYPE_ED and no longer runs as a
Router; likewise update the similar notes referenced around the other block
(mentions of Router mode and on-hold status) so the header, the WLED usermod
description, and any statements about pairing and Router mode consistently
reflect the current implementation.
In `@usermods/zigbee_rgb_light/research.md`:
- Around line 19-22: Update the Threading Model note to reflect the current
default stack size by replacing the stale "8KB" reference for zigbeeTask with
the current ZIGBEE_TASK_STACK_SIZE default (16384) and/or annotate that 8KB was
an earlier baseline; ensure the text still mentions zigbeeTask,
ZIGBEE_TASK_STACK_SIZE, zbStateMutex and that the Zigbee stack runs in its own
FreeRTOS task, and optionally add a brief parenthetical noting the historical
8KB baseline for clarity.
In `@usermods/zigbee_rgb_light/test_zigbee_hue.py`:
- Around line 28-33: Remove the real Hue bridge credentials and local IPs from
the usage example in test_zigbee_hue.py by replacing the concrete values for the
CLI flags (--bridge-ip, --api-key, --light-id, --wled-ip) with clearly marked
placeholders (e.g. <BRIDGE_IP>, <API_KEY>, <LIGHT_ID>, <WLED_IP>), and update
the matching README example to use the same placeholders so no real credentials
are checked in; ensure the placeholder names match the flags so readers can
substitute their own values.
In `@usermods/zigbee_rgb_light/usermod_zigbee_rgb_light.h`:
- Around line 1061-1072: zbBindCallback is counting every bind response as
completed even on failure; change it to only increment s_zb_binds_completed when
the status indicates success (check the esp_zb_zdp_status_t success value) and
do not advance to scheduling reporting
(esp_zb_scheduler_alarm(zbSetupReportingConfig,...)) unless s_zb_binds_completed
reaches s_zb_binds_needed from successful binds. In the failure branch, log the
failure with the cluster and status and avoid incrementing the counter
(optionally track failures separately or retry), ensuring the reporting config
is only scheduled after the required number of successful binds.
- Around line 283-292: prepareReport currently drops the snapshot if
xSemaphoreTake(zbStateMutex, pdMS_TO_TICKS(10)) fails, which causes
onStateChange->zbReportStateViaScheduler() to later see reportPending==false and
never report; instead, when the mutex take fails, retry or defer capturing the
snapshot by scheduling zbReportStateViaScheduler() (or the existing scheduler
task used in onStateChange) to run shortly so it can take the mutex and set
reportPowerOn/reportBri/reportColorX/reportColorY/reportPending; apply the same
change to the duplicate block around lines 860-870 so both prepare-report paths
will reschedule a deferred attempt instead of dropping the snapshot.
- Around line 18-20: This header must fail compilation when built for
CONFIG_IDF_TARGET_ESP32C6 while digital channels are enabled; add a preprocessor
guard in usermod_zigbee_rgb_light.h that checks WLED_MAX_DIGITAL_CHANNELS and
emits a compile-time error if it is not zero (e.g. when
CONFIG_IDF_TARGET_ESP32C6 is defined and WLED_MAX_DIGITAL_CHANNELS != 0) so the
build stops early instead of producing a runtime RMT/802.15.4 conflict.
🪄 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: 44d4a46a-3b32-4198-813c-58c6524daf53
📒 Files selected for processing (5)
usermods/zigbee_rgb_light/README.mdusermods/zigbee_rgb_light/research-phillips-hue.mdusermods/zigbee_rgb_light/research.mdusermods/zigbee_rgb_light/test_zigbee_hue.pyusermods/zigbee_rgb_light/usermod_zigbee_rgb_light.h
| ### Threading Model | ||
|
|
||
| The Zigbee stack runs in its own FreeRTOS task (`zigbeeTask`, 8KB stack, priority 5). The WLED main loop runs on the Arduino core. Communication between the two is via shared `volatile` state variables protected by a FreeRTOS mutex (`zbStateMutex`). | ||
|
|
There was a problem hiding this comment.
The task-size note is stale.
This still documents zigbeeTask as an 8 KB task, but the implementation now defaults ZIGBEE_TASK_STACK_SIZE to 16384. Since this file is positioned as the current architecture reference, please update the note or call out that 8 KB was only an earlier baseline.
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 20-20: Blank line inside blockquote
(MD028, no-blanks-blockquote)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@usermods/zigbee_rgb_light/research.md` around lines 19 - 22, Update the
Threading Model note to reflect the current default stack size by replacing the
stale "8KB" reference for zigbeeTask with the current ZIGBEE_TASK_STACK_SIZE
default (16384) and/or annotate that 8KB was an earlier baseline; ensure the
text still mentions zigbeeTask, ZIGBEE_TASK_STACK_SIZE, zbStateMutex and that
the Zigbee stack runs in its own FreeRTOS task, and optionally add a brief
parenthetical noting the historical 8KB baseline for clarity.
| static void zbBindCallback(esp_zb_zdp_status_t status, void *user_ctx) | ||
| { | ||
| uint16_t cluster = (uint16_t)(uintptr_t)user_ctx; | ||
| ESP_LOGD("ZigbeeRGB", "Bind response for cluster 0x%04x: status=0x%02x", cluster, status); | ||
|
|
||
| s_zb_binds_completed++; | ||
| if (s_zb_binds_completed >= s_zb_binds_needed) { | ||
| ESP_LOGD("ZigbeeRGB", "All %d bindings completed, configuring reporting...", | ||
| s_zb_binds_needed); | ||
| // Schedule reporting config in Zigbee task context | ||
| esp_zb_scheduler_alarm(zbSetupReportingConfig, 0, 100); | ||
| } |
There was a problem hiding this comment.
Failed binds are being counted as completed.
zbBindCallback() increments s_zb_binds_completed for every Bind_rsp, including failure statuses. A failed cluster bind can therefore still show 3/3 in /json/info and trigger reporting setup with no valid destination for that cluster.
🛠️ Suggested fix
static void zbBindCallback(esp_zb_zdp_status_t status, void *user_ctx)
{
uint16_t cluster = (uint16_t)(uintptr_t)user_ctx;
ESP_LOGD("ZigbeeRGB", "Bind response for cluster 0x%04x: status=0x%02x", cluster, status);
+ if (status != ESP_ZB_ZDP_STATUS_SUCCESS) {
+ ESP_LOGW("ZigbeeRGB", "Bind failed for cluster 0x%04x: status=0x%02x", cluster, status);
+ return;
+ }
+
s_zb_binds_completed++;
- if (s_zb_binds_completed >= s_zb_binds_needed) {
+ if (s_zb_binds_completed == s_zb_binds_needed) {
ESP_LOGD("ZigbeeRGB", "All %d bindings completed, configuring reporting...",
s_zb_binds_needed);
// Schedule reporting config in Zigbee task context📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| static void zbBindCallback(esp_zb_zdp_status_t status, void *user_ctx) | |
| { | |
| uint16_t cluster = (uint16_t)(uintptr_t)user_ctx; | |
| ESP_LOGD("ZigbeeRGB", "Bind response for cluster 0x%04x: status=0x%02x", cluster, status); | |
| s_zb_binds_completed++; | |
| if (s_zb_binds_completed >= s_zb_binds_needed) { | |
| ESP_LOGD("ZigbeeRGB", "All %d bindings completed, configuring reporting...", | |
| s_zb_binds_needed); | |
| // Schedule reporting config in Zigbee task context | |
| esp_zb_scheduler_alarm(zbSetupReportingConfig, 0, 100); | |
| } | |
| static void zbBindCallback(esp_zb_zdp_status_t status, void *user_ctx) | |
| { | |
| uint16_t cluster = (uint16_t)(uintptr_t)user_ctx; | |
| ESP_LOGD("ZigbeeRGB", "Bind response for cluster 0x%04x: status=0x%02x", cluster, status); | |
| if (status != ESP_ZB_ZDP_STATUS_SUCCESS) { | |
| ESP_LOGW("ZigbeeRGB", "Bind failed for cluster 0x%04x: status=0x%02x", cluster, status); | |
| return; | |
| } | |
| s_zb_binds_completed++; | |
| if (s_zb_binds_completed == s_zb_binds_needed) { | |
| ESP_LOGD("ZigbeeRGB", "All %d bindings completed, configuring reporting...", | |
| s_zb_binds_needed); | |
| // Schedule reporting config in Zigbee task context | |
| esp_zb_scheduler_alarm(zbSetupReportingConfig, 0, 100); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@usermods/zigbee_rgb_light/usermod_zigbee_rgb_light.h` around lines 1061 -
1072, zbBindCallback is counting every bind response as completed even on
failure; change it to only increment s_zb_binds_completed when the status
indicates success (check the esp_zb_zdp_status_t success value) and do not
advance to scheduling reporting
(esp_zb_scheduler_alarm(zbSetupReportingConfig,...)) unless s_zb_binds_completed
reaches s_zb_binds_needed from successful binds. In the failure branch, log the
failure with the cluster and status and avoid incrementing the counter
(optionally track failures separately or retry), ensuring the reporting config
is only scheduled after the required number of successful binds.
|
size and build time comparison (same partitions, same pioarduino platform version) Environment Status Duration
------------------ -------- ------------
esp32c6__no_zigbee SUCCESS 00:02:00.921
RAM: [=== ] 26.4% (used 86496 bytes from 327680 bytes)
Flash: [====== ] 55.8% (used 1755424 bytes from 3145728 bytes)Environment Status Duration
-------------- -------- ------------
esp32c6_zigbee SUCCESS 00:01:51.029
RAM: [=== ] 31.0% (used 101680 bytes from 327680 bytes)
Flash: [====== ] 64.5% (used 2030420 bytes from 3145728 bytes)RAM (static) +15 KB |
* remove unneeded wled00/idf_component.yml * comment out flash extra_images hack (these files are always flashed)
|
what is the reason it is already at 1.7meg without zigbee? |
| ; and the coexistence call are visible on the serial console. | ||
| -DCORE_DEBUG_LEVEL=3 | ||
| ; Disable RMT-based digital LED buses: the Zigbee 802.15.4 radio on C6 | ||
| ; shares the RMT peripheral, causing rmt_tx_wait_all_done() timeout spam. |
There was a problem hiding this comment.
@coderabbitai please crosscheck this statement with the official espressif documentation for C5 and C6.
Is sharing of RMT only a limitation for ZigBee, or is this a general concern that also applies to WiFi radio? Does espressif recommend workarounds?
There was a problem hiding this comment.
🧩 Analysis chain
@softhack007, here are my findings after cross-checking with official Espressif documentation:
The comment's claim vs. what official docs say
The comment states:
"the Zigbee 802.15.4 radio on C6 shares the RMT peripheral"
This is technically inaccurate per the official Espressif documentation. The 802.15.4 MAC on the ESP32-C6 is a dedicated baseband/MAC hardware block — it is entirely separate from the RMT (Remote Control Transceiver) peripheral, which is a general-purpose pulse-generation peripheral used for WS2812/NeoPixel LED strips, IR, etc.
What IS officially documented: RF path sharing
According to the ESP32-C6 datasheet, "Wi-Fi, Bluetooth and 802.15.4 coexist with each other and share the same antenna."
The ESP-IDF RF Coexistence guide confirms: "ESP boards now support three modules: Bluetooth (BT & BLE), IEEE 802.15.4 (Thread / Zigbee), and Wi-Fi. Each type of board has only one 2.4 GHz ISM band RF module, shared by two or three modules. Consequently, a module cannot receive or transmit data while another module is engaged. In such scenarios, ESP32-C6 employs the time-division multiplexing method to manage the reception and transmission of packets."
So: it is the 2.4 GHz RF path (antenna/transceiver) that is shared, not the RMT peripheral. This applies equally to WiFi, BLE, and 802.15.4.
Is this Zigbee-specific, or does it also apply to WiFi?
This is not Zigbee-specific. The coexistence docs note: "Routers in Thread and Zigbee networks maintain unsynchronized links with their neighbors, requiring continuous signal reception. With only a single RF path, increased Wi-Fi or BLE traffic may lead to higher packet loss rates for Thread and Zigbee communications. To build a Wi-Fi based Thread Border Router or Zigbee Gateway product, we recommend using a dual-SoC solution (e.g., ESP32-S3 + ESP32-H2) with separate antennas."
Practical issues reported in the esp-zigbee-sdk tracker confirm this: connecting to WiFi while Zigbee is active causes problems; even when ordering is correct, "the WiFi connection was nearly unusable, i.e. client requests weren't received and ping drop rate was at around 80%."
For the ESP32-C5, it also integrates both WiFi and 802.15.4 (the ESP32-C5 is a single-core CPU model with 802.15.4 radio and WiFi), so the same single-antenna RF coexistence limitation applies.
Why is there a rmt_tx_wait_all_done() timeout then?
The RMT timeout is not caused by the 802.15.4 radio using the RMT peripheral. The likely actual cause is that the 802.15.4/ZBOSS stack's high-priority ISRs or tasks preempt the RMT DMA-completion interrupt callbacks, causing the RMT driver's internal timeout to trigger while waiting for the DMA transfer to be acknowledged. This is an interrupt-latency / task-scheduling conflict, not a peripheral-level resource sharing conflict.
The comment in the code should be clarified. A more accurate description would be:
; Disable RMT-based digital LED buses: the Zigbee 802.15.4 stack on C6
; uses high-priority ISRs that can delay RMT DMA completion callbacks,
; causing rmt_tx_wait_all_done() timeout spam.Summary
| Claim | Accuracy |
|---|---|
| 802.15.4 and RMT share the same RF path | ✅ Correct (but that's the antenna/transceiver, not RMT) |
| 802.15.4 radio uses/shares the RMT peripheral | ❌ Not substantiated by official docs |
| WiFi also shares the RF path with 802.15.4 | ✅ Yes, this is a general RF coexistence concern, not Zigbee-specific |
| Espressif recommends dual-SoC for gateways | ✅ Yes, officially recommended for router/gateway use cases |
esp_coex_wifi_i154_enable() is the workaround |
✅ Yes, that's the SW coexistence workaround for single-chip WiFi+Zigbee |
The comment in platformio_override.ini should be corrected to describe the ISR latency / task scheduling issue rather than claiming peripheral sharing.
ᓚᘏᗢ
🌐 Web query:
ESP32-C6 RMT peripheral shared 802.15.4 Zigbee radio conflict Espressif
💡 Result:
On ESP32‑C6, the RMT peripheral is not shared with (and does not “conflict” with) the IEEE 802.15.4 Zigbee/Thread radio in the sense of a single hardware block being multiplexed between them.
What is shared on ESP32‑C6 is the 2.4 GHz RF subsystem/antenna time between Wi‑Fi, BLE, and IEEE 802.15.4, and Espressif handles this via the coexistence mechanism (largely automatic, with optional software coexist configuration in ESP‑IDF). [1] The ESP32‑C6 datasheet lists RMT as a normal peripheral and 802.15.4 as a separate radio/MAC capability. [2]
If you’re seeing problems when using Zigbee/Thread alongside something that uses RMT (e.g., IR/LED timing), it’s more likely due to:
- RF coexistence constraints when Wi‑Fi/BLE/802.15.4 run concurrently (not RMT). [1]
- A known RMT erratum (e.g., RMT‑176 in continuous TX idle-level behavior), which is unrelated to 802.15.4 but can look like “RMT instability.” [3]
- Typical timing/clock/PM/interrupt priority interactions (RMT driver notes it may hold APB frequency via PM lock for timing stability). [4]
Sources:
[1] ESP‑IDF “RF Coexistence – ESP32‑C6” guide. (docs.espressif.com)
[2] ESP32‑C6 Series Datasheet (802.15.4 + RMT listed as separate features). (documentation.espressif.com)
[3] ESP32‑C6 Series SoC Errata (includes RMT‑176). (documentation.espressif.com)
[4] ESP‑IDF RMT peripheral documentation (timing/PM lock notes). (docs.espressif.com)
Citations:
- 1: https://docs.espressif.com/projects/esp-idf/en/v5.1.3/esp32c6/api-guides/coexist.html?utm_source=openai
- 2: https://documentation.espressif.com/esp32-c6_datasheet_en.html?utm_source=openai
- 3: https://documentation.espressif.com/esp-chip-errata/en/latest/esp32c6/index.html?utm_source=openai
- 4: https://docs.espressif.com/projects/esp-idf/en/v5.2/esp32c6/api-reference/peripherals/rmt.html?utm_source=openai
🌐 Web query:
ESP32-C6 ESP32-C5 RMT coexistence WiFi Zigbee peripheral sharing
💡 Result:
- ESP32-C6 and ESP32-C5 have a single 2.4 GHz RF resource that is shared by Wi‑Fi, BLE, and IEEE 802.15.4 (Thread/Zigbee), so they cannot truly run Wi‑Fi RX/TX and Zigbee/Thread RX/TX simultaneously. Coexistence is handled by time‑division multiplexing + priority arbitration in Espressif’s coexistence module. (docs.espressif.com)
- On ESP32‑C5, Espressif documents that 802.15.4 “normal receive” is the lowest priority, meaning Wi‑Fi/BLE can preempt it; only some 802.15.4 operations (e.g., TX/ACK/time‑critical ops) request higher priority, but arbitration still depends on what Wi‑Fi/BLE are doing at that moment. (docs.espressif.com)
- For product designs that need a reliable “hub/gateway” behavior (continuous Wi‑Fi + continuous Zigbee/Thread), Espressif’s Zigbee guidance commonly points to using separate Wi‑Fi and 802.15.4 SoCs (i.e., don’t assume one chip can behave like two independent radios at full performance). (docs.espressif.com)
- RMT peripheral “sharing” is separate from RF coexistence: RMT is a digital pulse TX/RX peripheral (LED/IR-style waveforms, etc.) and is not the mechanism used for Wi‑Fi/Zigbee coexistence. Practically, you only need to manage RMT like any other peripheral (GPIO matrix/pin conflicts, DMA/interrupt load), while Wi‑Fi/Zigbee sharing is governed by the RF coexistence rules above. (docs.espressif.com)
Citations:
- 1: https://docs.espressif.com/projects/esp-idf/en/v5.1.3/esp32c6/api-guides/coexist.html?utm_source=openai
- 2: https://docs.espressif.com/projects/esp-idf/en/latest/esp32c5/api-guides/coexist.html?utm_source=openai
- 3: https://docs.espressif.com/projects/esp-zigbee-sdk/en/latest/esp32c6/introduction.html?utm_source=openai
- 4: https://docs.espressif.com/projects/esp-idf/en/v5.1.3/esp32c6/api-guides/coexist.html?utm_source=openai
🌐 Web query:
ESP32-C6 rmt_tx_wait_all_done timeout 802.15.4 zigbee peripheral conflict
💡 Result:
rmt_tx_wait_all_done() timing out on ESP32‑C6 while running Zigbee (802.15.4) is usually not a “Zigbee peripheral steals RMT” conflict. 802.15.4 only shares the RF (time-division multiplexed with Wi‑Fi/BLE), not the RMT peripheral. [1][2]
What commonly does cause the timeout is that the RMT TX “done” condition never gets serviced/finishes because of power management / light-sleep or delayed interrupts:
- Light-sleep / PM clock changes: With
CONFIG_PM_ENABLE, the system may adjust/disable clocks before sleep, which can break RMT’s internal time base unless the driver’s PM lock is held. The RMT driver acquires the PM lock inrmt_enable()and releases it inrmt_disable(), and transactions between them are “guaranteed” to be stable. [3] - RMT powered down in light sleep:
rmt_tx_channel_config_t::allow_pdcontrols whether RMT can be powered down in light sleep (with context save/restore). If Zigbee enables light sleep aggressively, powering down at the wrong time can make a “wait done” timeout more likely. [4] - ISR delayed when cache is disabled (flash operations): By default RMT interrupts can be deferred while cache is disabled (e.g., flash write/erase), so “transaction done” isn’t handled in time; ESP-IDF provides
CONFIG_RMT_TX_ISR_CACHE_SAFEto address this. [5] - Infinite/looping TX: If you configured an infinite loop transaction,
rmt_tx_wait_all_done()can block forever (or hit your timeout). [6]
Practical mitigations to try (most-to-least common):
- Wrap TX with
rmt_enable()… transmit …rmt_tx_wait_all_done()…rmt_disable()so the driver holds the PM lock during the whole operation. [3] - If using Zigbee light-sleep, avoid entering light sleep while an RMT TX is in progress (or temporarily disable light sleep around the TX), and consider
allow_pd = 0for the RMT channel. [4] - If you do any flash writes/erases while driving RMT (directly or via another component), enable
CONFIG_RMT_TX_ISR_CACHE_SAFE. [5] - Confirm you didn’t set infinite loop mode for the TX. [6]
If you share your ESP-IDF version, whether Zigbee light sleep is enabled, and your RMT channel config (clock source, allow_pd, loop count), I can narrow it to the most likely single cause.
Sources
[1] ESP-IDF “RF Coexistence” (ESP32‑C6) overview (time-division multiplexing of RF among Wi‑Fi/BLE/802.15.4). (docs.espressif.com)
[2] ESP32‑C6 datasheet (Wi‑Fi, BLE, 802.15.4 share same RF/antenna; RMT is a separate peripheral). (documentation.espressif.com)
[3] ESP-IDF RMT “Power Management” section (PM clock changes; driver PM lock acquired in rmt_enable()/released in rmt_disable()). (docs.espressif.com)
[4] ESP-IDF RMT allow_pd (power down peripheral in light sleep). (docs.espressif.com)
[5] ESP-IDF RMT “Cache Safe” note (interrupt deferred when cache disabled; cache-safe Kconfig options). (docs.espressif.com)
[6] ESP-IDF RMT API note for rmt_tx_wait_all_done() (can block forever if transaction can’t finish, e.g., infinite loop). (docs.espressif.com)
Citations:
- 1: https://docs.espressif.com/projects/esp-idf/en/v5.1.3/esp32c6/api-guides/coexist.html
- 2: https://documentation.espressif.com/api/resource/doc/file/lYZaqv1x/FILE/esp32-c6_datasheet_en.pdf?utm_source=openai
- 3: https://docs.espressif.com/projects/esp-idf/en/stable/esp32s2/api-reference/peripherals/rmt.html
- 4: https://docs.espressif.com/projects/esp-idf/en/stable/esp32s2/api-reference/peripherals/rmt.html
- 5: https://docs.espressif.com/projects/esp-idf/en/stable/esp32s2/api-reference/peripherals/rmt.html
- 6: https://docs.espressif.com/projects/esp-idf/en/v5.3.4/esp32s2/api-reference/peripherals/rmt.html
@softhack007, here are the crosschecked findings:
The PR comment's claim vs. official Espressif documentation
The comment states: "the Zigbee 802.15.4 radio on C6 shares the RMT peripheral" — this is inaccurate per the official ESP-IDF and datasheet documentation.
On ESP32-C6, the RMT peripheral is not shared with the IEEE 802.15.4 Zigbee/Thread radio. What is shared is the 2.4 GHz RF subsystem/antenna time between Wi-Fi, BLE, and IEEE 802.15.4, handled via the coexistence mechanism. The ESP32-C6 datasheet lists RMT as a normal peripheral and 802.15.4 as a separate radio/MAC capability.
Is this Zigbee-specific, or does it also affect WiFi?
This is not Zigbee-specific. ESP32-C6 and ESP32-C5 have a single 2.4 GHz RF resource shared by Wi-Fi, BLE, and IEEE 802.15.4 (Thread/Zigbee), so they cannot truly run Wi-Fi RX/TX and Zigbee/Thread RX/TX simultaneously — coexistence is handled by time-division multiplexing + priority arbitration.
On ESP32-C5 specifically, Espressif documents that 802.15.4 "normal receive" is the lowest priority, meaning Wi-Fi/BLE can preempt it; only some 802.15.4 operations (e.g., TX/ACK/time-critical ops) request higher priority.
RMT peripheral "sharing" is entirely separate from RF coexistence: RMT is a digital pulse TX/RX peripheral and is not the mechanism used for Wi-Fi/Zigbee coexistence. You only need to manage RMT like any other peripheral (GPIO matrix/pin conflicts, DMA/interrupt load), while Wi-Fi/Zigbee sharing is governed by the RF coexistence rules.
Why is there actually a rmt_tx_wait_all_done() timeout?
rmt_tx_wait_all_done() timing out on ESP32-C6 while running Zigbee is not a "Zigbee peripheral steals RMT" conflict — 802.15.4 only shares the RF, not the RMT peripheral. The most likely actual causes are:
- Light-sleep / PM clock changes: With
CONFIG_PM_ENABLE, the system may adjust/disable clocks before sleep, which can break RMT's internal time base unless the driver's PM lock is held. The RMT driver acquires the PM lock inrmt_enable()and releases it inrmt_disable(). Thermt_tx_channel_config_t::allow_pdflag controls whether RMT can be powered down in light sleep — if Zigbee enables aggressive light sleep, powering down at the wrong moment makes armt_tx_wait_all_done()timeout more likely. - ISR delayed when cache is disabled (e.g., during flash write/erase): RMT interrupts can be deferred while cache is disabled, so "transaction done" isn't handled in time. ESP-IDF provides
CONFIG_RMT_TX_ISR_CACHE_SAFEto address this.
Espressif-recommended workarounds
For product designs that need reliable hub/gateway behavior (continuous Wi-Fi + continuous Zigbee/Thread), Espressif's Zigbee guidance recommends using separate Wi-Fi and 802.15.4 SoCs rather than assuming one chip can behave like two independent radios at full performance.
For the rmt_tx_wait_all_done() timeout specifically, the recommended mitigations are:
- Wrap TX with
rmt_enable()→ transmit →rmt_tx_wait_all_done()→rmt_disable()so the driver holds the PM lock throughout. - Set
allow_pd = 0for the RMT channel if Zigbee light-sleep is active. - Enable
CONFIG_RMT_TX_ISR_CACHE_SAFEif flash operations occur concurrently.
Summary
| Claim | Accuracy per official docs |
|---|---|
| 802.15.4 shares the RMT peripheral with WLED LED buses | ❌ Not documented; RMT is a separate, unrelated peripheral |
| 802.15.4, WiFi, and BLE share the 2.4 GHz RF path | ✅ Correct and well-documented; applies equally to WiFi |
rmt_tx_wait_all_done() timeout is caused by Zigbee light-sleep/PM |
✅ Most likely actual cause per ESP-IDF docs |
| Espressif recommends dual-SoC for reliable WiFi+Zigbee gateway | ✅ Yes, officially recommended |
| ESP32-C5 has the same single-RF coexistence limitation | ✅ Yes, same behaviour documented |
The comment in platformio_override.ini should be corrected — the RMT timeout is most likely a light-sleep / PM clock interaction, not the 802.15.4 radio "sharing" the RMT peripheral.
ᓚᘏᗢ
✏️ Learnings added
Learnt from: softhack007
Repo: wled/WLED PR: 5457
File: platformio_override.ini:23-23
Timestamp: 2026-03-31T17:05:44.026Z
Learning: In WLED PR `#5457` (platformio_override.ini, esp32c6_zigbee): The comment "Zigbee 802.15.4 radio on C6 shares the RMT peripheral" is factually incorrect per official Espressif documentation. The ESP32-C6 802.15.4 radio does NOT use or share the RMT peripheral. What IS shared (documented in ESP32-C6 datasheet and ESP-IDF RF coexistence guide) is the single 2.4 GHz RF path (antenna/transceiver), shared among WiFi, BLE, and 802.15.4 via time-division multiplexing — this applies equally to WiFi, not just Zigbee. The rmt_tx_wait_all_done() timeout spam is most likely caused by Zigbee light-sleep enabling aggressive power management (CONFIG_PM_ENABLE) that disrupts RMT's internal time base, or ISR latency due to cache-disable during flash ops. Mitigations: wrap TX with rmt_enable()/rmt_disable() for PM lock, set allow_pd=0, or enable CONFIG_RMT_TX_ISR_CACHE_SAFE. For reliable WiFi+Zigbee gateway operation, Espressif officially recommends a dual-SoC solution (e.g., ESP32-S3 + ESP32-H2). ESP32-C5 has the same single-RF coexistence limitation as C6.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: 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: 5311
File: platformio.ini:395-395
Timestamp: 2026-01-31T18:19:33.055Z
Learning: In WLED, usermods can be referenced as out-of-tree library dependencies (e.g., via lib_deps pointing to external repositories) while the `custom_usermods` setting in platformio.ini controls which in-tree usermods are enabled. Setting `custom_usermods = ` (empty) only disables in-tree usermods; out-of-tree usermods pulled via library dependencies remain active.
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: 5456
File: platformio.ini:794-830
Timestamp: 2026-03-31T13:41:45.126Z
Learning: In WLED PR `#5456` (Matter over WiFi usermod, CMakeLists.txt + platformio.ini): The GCC 14 chip::to_underlying compatibility issue with CHIP SDK's TypeTraits.h is fixed by building in gnu++20 mode (not gnu++2b/gnu++23). CMakeLists.txt uses `idf_build_replace_option_from_property` to swap `-std=gnu++2b` for `-std=gnu++20` when the matter usermod is present. The `matter_gcc14_compat.h` shim file (which pre-defines `chip::to_underlying` and sets `CHIP_TO_UNDERLYING_DEFINED`) is dead code under this configuration — it is never included anywhere. TypeTraits.h's broken C++23 `using std::to_underlying` alias path is only taken in gnu++23 mode; in gnu++20 mode CHIP defines its own `chip::to_underlying` function template normally, so no shim is needed. Additionally, upstream connectedhomeip TypeTraits.h already has the fix built-in. ESP-IDF v5.5 uses GCC 14.2.0. Do NOT flag the missing `-include` for `matter_gcc14_compat.h` as a build issue.
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: softhack007
Repo: wled/WLED PR: 5456
File: platformio.ini:794-830
Timestamp: 2026-03-31T13:41:45.126Z
Learning: In WLED PR `#5456` (Matter over WiFi usermod, CMakeLists.txt + platformio.ini): The GCC 14 chip::to_underlying compatibility issue with CHIP SDK's TypeTraits.h is fixed by building in gnu++20 mode (not gnu++2b/gnu++23). CMakeLists.txt uses `idf_build_replace_option_from_property` to swap `-std=gnu++2b` for `-std=gnu++20` when the matter usermod is present. The `matter_gcc14_compat.h` shim file (which pre-defines `chip::to_underlying` and sets `CHIP_TO_UNDERLYING_DEFINED`) is dead code under this configuration — it is never included anywhere. TypeTraits.h's broken C++23 `using std::to_underlying` alias path is only taken in gnu++23 mode; in gnu++20 mode CHIP defines its own `chip::to_underlying` function template normally, so no shim is needed. Do NOT flag the missing `-include` for `matter_gcc14_compat.h` as a build issue.
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: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-28T14:18:55.599Z
Learning: Applies to platformio.ini : In platformio.ini, hardware build targets are configured in `default_envs` and must succeed for all targets during CI/CD validation
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: 4838
File: wled00/const.h:90-100
Timestamp: 2026-03-28T01:26:38.240Z
Learning: In WLED (wled00/const.h), ESP32-C5 and ESP32-C6 each have 4 RMT channels, but they are split into 2 TX and 2 RX channels. Only TX channels can drive LED strip outputs, so WLED_MAX_RMT_CHANNELS must be set to 2 for both C5 and C6 — the same as ESP32-C3. Do not suggest raising WLED_MAX_RMT_CHANNELS above 2 for C5/C6 based on their total RMT channel count.
Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/include/NeoEsp32RmtHIMethod.h:31-36
Timestamp: 2025-09-02T01:45:58.047Z
Learning: For the NeoEsp32RmtHI driver, RISC-V ESP32-C3 support is currently disabled via bus_wrapper.h rather than compile-time guards, as the maintainer willmmiles is working on resolving underlying nested interrupt issues and prefers to centralize the workaround in one location.
Learnt from: 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/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/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: 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/json.cpp:1112-1123
Timestamp: 2026-03-29T20:24:09.490Z
Learning: In WLED PR `#5048` (wled00/json.cpp serializePins()), ESP32-C5 strapping pins are:
- MTMS (GPIO number to be confirmed per silicon rev), MTDI=GPIO3, GPIO7, GPIO25, GPIO26, GPIO27, GPIO28.
- GPIO3 (=MTDI) is a strapping pin for SDIO sampling/driving clock edge configuration (together with GPIO25).
- GPIO26 is a strapping pin for chip boot mode selection (together with GPIO27 and GPIO28).
- The code flagging gpio==2, gpio==3, gpio==7, gpio==25, gpio==26 as PIN_CAP_BOOTSTRAP (plus gpio==27 and gpio==28 with PIN_CAP_BOOT) is CORRECT for ESP32-C5.
- Do NOT suggest removing GPIO3 or GPIO26 from the C5 strapping-pin block — both are legitimate strapping pins per the ESP32-C5 datasheet and DevKitC-1 user guide.
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: 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: 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: willmmiles
Repo: wled/WLED PR: 4623
File: usermods/word-clock-matrix/word-clock-matrix.cpp:332-332
Timestamp: 2025-03-29T01:22:54.617Z
Learning: In the WLED project, hardcoded usermod IDs (like 500 in the WordClockMatrix's getId() method) are intentionally used as part of a strategy to avoid modifying core code. There are plans to remove numeric usermod IDs entirely in the future.
Learnt from: KrX3D
Repo: wled/WLED PR: 4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.cpp:612-617
Timestamp: 2026-02-18T00:41:15.185Z
Learning: In the seven_segment_display_reloaded_v2 usermod (PR `#4585`), the `addToConfig()` method intentionally calls `_updateMQTT()` to publish MQTT state on every config save. This behavior is by design and should not be flagged as an issue.
experimental
| reportTriggerCount++; | ||
|
|
||
| // Store pending Zigbee report | ||
| prepareReport(newPower, newBri, newX, newY); |
There was a problem hiding this comment.
this function should return true/false, depending on the mutex acquisition in prepareReport. Only update reportTriggerCount when prepare was successful.
if (prepareReport(newPower, newBri, newX, newY)) reportTriggerCount++;
| uint8_t rBri; | ||
| uint16_t rX, rY; | ||
|
|
||
| if (self->zbStateMutex && xSemaphoreTake(self->zbStateMutex, pdMS_TO_TICKS(10)) == pdTRUE) { |
There was a problem hiding this comment.
same comment here - 10ms seems much too short
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (3)
usermods/zigbee_rgb_light/test_zigbee_hue.py (1)
28-33:⚠️ Potential issue | 🟠 MajorRemove the checked-in Hue credential and LAN details.
This usage block still contains a real-looking API key plus private bridge/WLED addresses and light ID. Replace them with placeholders here, and keep the matching README example in sync, before merge.
🔐 Suggested fix
python3 test_zigbee_hue.py \\ - --bridge-ip 192.168.178.216 \\ - --api-key XBfT0n000WWp2FV6DxcOnbhcxV5X7SFlKpB53Bix \\ - --light-id 23 \\ - --wled-ip 192.168.178.107 + --bridge-ip <bridge-ip> \\ + --api-key <api-key> \\ + --light-id <light-id> \\ + --wled-ip <wled-ip>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@usermods/zigbee_rgb_light/test_zigbee_hue.py` around lines 28 - 33, The usage block in test_zigbee_hue.py currently contains sensitive/real-looking credentials and LAN addresses; replace the bridge IP, api key, light id, and wled IP with non-sensitive placeholders (e.g., <BRIDGE_IP>, <API_KEY>, <LIGHT_ID>, <WLED_IP>) in the Usage section and ensure the matching README example is updated to use the same placeholders so both stay in sync; locate the Usage text in test_zigbee_hue.py and the corresponding README example and perform the substitutions.usermods/zigbee_rgb_light/research-phillips-hue.md (1)
155-159:⚠️ Potential issue | 🟡 MinorThese notes need to be marked as historical.
They still describe the WLED usermod as “on hold” and “still has Router mode”, while the same document now says the implementation is fully working and the code runs in ED mode. Please rewrite these in past tense or label them as superseded experiments.
Also applies to: 204-205
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@usermods/zigbee_rgb_light/research-phillips-hue.md` around lines 155 - 159, Update the section mentioning "WLED Zigbee usermod (`/home/will/netmindz/WLED/`)" so the notes are marked historical: change present-tense phrases like "on hold" and "still has Router mode" to past-tense (e.g., "was on hold", "used Router mode") or add a clear "Historical / Superseded experiment" prefix or header to that paragraph; similarly update the references to "HA Color Dimmable Light endpoint" and the Router/ED mode statements (and the duplicate phrases at the other occurrence around lines 204–205) to indicate they are superseded, and ensure the text states that the current implementation runs in ED mode and that the older notes are archival.usermods/zigbee_rgb_light/usermod_zigbee_rgb_light.h (1)
283-293:⚠️ Potential issue | 🟠 MajorDon't drop WLED→Zigbee reports when the mutex is busy.
If Line 285 misses the 10 ms lock window,
onStateChange()still scheduleszbReportStateViaScheduler(), butreportPendingwas never set. The scheduled callback then exits immediately, so that state change is never reported. Please defer and retry the snapshot instead of silently returning.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@usermods/zigbee_rgb_light/usermod_zigbee_rgb_light.h` around lines 283 - 293, prepareReport currently drops the report if zbStateMutex isn't acquired within 10ms which causes zbReportStateViaScheduler to find reportPending unset; change prepareReport so that if xSemaphoreTake(zbStateMutex, pdMS_TO_TICKS(10)) fails it does not silently return but instead re-schedules a retry (do not busy-wait): use the existing scheduling mechanism (the same scheduler that invokes zbReportStateViaScheduler) to enqueue a short-delayed retry of prepareReport (or a small helper like scheduleDeferredReport) so the snapshot is attempted again until the mutex is acquired and reportPending is set; reference prepareReport, zbStateMutex, xSemaphoreTake, reportPending and zbReportStateViaScheduler in your change.
🤖 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/zigbee_rgb_light/README.md`:
- Line 21: Update the README text that claims "802.15.4 radio shares the RMT
peripheral on the ESP32-C6" to instead state that the ESP32-C6 uses a single
shared 2.4 GHz RF path under coexistence arbitration and that the RMT conflict
statement is an implementation/workaround constraint (not silicon-level fact);
keep the practical guidance to use -DWLED_MAX_DIGITAL_CHANNELS=0 and analog/PWM
outputs as a workaround for digital LED buses (WS2812B via RMT), and revise the
duplicated passages (the current note and the section around the other
occurrence) to reflect the corrected coexistence explanation and refer to
Espressif coexistence arbitration rather than RMT sharing.
In `@usermods/zigbee_rgb_light/research-phillips-hue.md`:
- Around line 547-558: The security API matrix is outdated: update the table
rows for esp_zb_secur_TC_standard_distributed_key_set() and
esp_zb_secur_network_min_join_lqi_set() to reflect that they have been
used/tested by the TL;DR and current usermod (or alternatively mark the entire
table as "historical / may be stale"), and add a brief note linking that these
APIs are relied upon by the usermod so future readers won't treat them as
unverified; ensure the changed entries reference the exact function names
esp_zb_secur_TC_standard_distributed_key_set() and
esp_zb_secur_network_min_join_lqi_set().
In `@usermods/zigbee_rgb_light/research.md`:
- Around line 57-63: Update the documentation to remove the incorrect claim that
the ESP32-C6 802.15.4 radio shares the RMT peripheral; instead state that the
documented shared resource is the single 2.4 GHz RF path and that RMT-based LED
buses can still cause `rmt_tx_wait_all_done()` timeouts in some builds, so the
suggested workaround (`-DWLED_MAX_DIGITAL_CHANNELS=0` in build_flags) remains
valid; reference `rmt_tx_wait_all_done()`, the build flag
`-DWLED_MAX_DIGITAL_CHANNELS=0`, and the file `wled00/const.h` in the
explanation so readers can locate the related code and macros.
In `@usermods/zigbee_rgb_light/usermod_zigbee_rgb_light.h`:
- Around line 462-473: The snapshot-and-clear race: don't clear stateChanged in
loop() before applyState() can act; instead, have loop() take zbStateMutex, copy
state into locals (as it already does) but do NOT set stateChanged = false there
— instead, change applyState() to accept/return a success flag (or reacquire
zbStateMutex and clear stateChanged only after a successful send) so that
applyState() clears stateChanged only when the Zigbee command actually succeeds;
reference zbStateMutex, stateChanged, loop(), and applyState() when making this
change.
- Around line 1020-1035: zbUpdateState currently drops inbound updates when
xSemaphoreTake(self->zbStateMutex, pdMS_TO_TICKS(10)) times out, losing parsed
commands; change it so a failed mutex take does not silently discard the update:
if the semaphore take fails, copy the incoming parameters (power, bri, colorX,
colorY, useXY) into a thread-safe fallback (either push a small struct to a
FreeRTOS queue like zbUpdateQueue or set an atomic/pendingUpdate structure) and
return, or increase/loop the timeout until queued—ensure the consumer (the task
that normally holds zbStateMutex) will apply queued pending updates and set
stateChanged; reference zbUpdateState, s_zb_instance, zbStateMutex,
stateChanged, and ensure the fallback is safe without holding zbStateMutex.
---
Duplicate comments:
In `@usermods/zigbee_rgb_light/research-phillips-hue.md`:
- Around line 155-159: Update the section mentioning "WLED Zigbee usermod
(`/home/will/netmindz/WLED/`)" so the notes are marked historical: change
present-tense phrases like "on hold" and "still has Router mode" to past-tense
(e.g., "was on hold", "used Router mode") or add a clear "Historical /
Superseded experiment" prefix or header to that paragraph; similarly update the
references to "HA Color Dimmable Light endpoint" and the Router/ED mode
statements (and the duplicate phrases at the other occurrence around lines
204–205) to indicate they are superseded, and ensure the text states that the
current implementation runs in ED mode and that the older notes are archival.
In `@usermods/zigbee_rgb_light/test_zigbee_hue.py`:
- Around line 28-33: The usage block in test_zigbee_hue.py currently contains
sensitive/real-looking credentials and LAN addresses; replace the bridge IP, api
key, light id, and wled IP with non-sensitive placeholders (e.g., <BRIDGE_IP>,
<API_KEY>, <LIGHT_ID>, <WLED_IP>) in the Usage section and ensure the matching
README example is updated to use the same placeholders so both stay in sync;
locate the Usage text in test_zigbee_hue.py and the corresponding README example
and perform the substitutions.
In `@usermods/zigbee_rgb_light/usermod_zigbee_rgb_light.h`:
- Around line 283-293: prepareReport currently drops the report if zbStateMutex
isn't acquired within 10ms which causes zbReportStateViaScheduler to find
reportPending unset; change prepareReport so that if
xSemaphoreTake(zbStateMutex, pdMS_TO_TICKS(10)) fails it does not silently
return but instead re-schedules a retry (do not busy-wait): use the existing
scheduling mechanism (the same scheduler that invokes zbReportStateViaScheduler)
to enqueue a short-delayed retry of prepareReport (or a small helper like
scheduleDeferredReport) so the snapshot is attempted again until the mutex is
acquired and reportPending is set; reference prepareReport, zbStateMutex,
xSemaphoreTake, reportPending and zbReportStateViaScheduler in your change.
🪄 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: d97c3554-eb81-4673-bed3-68daa097a665
📒 Files selected for processing (6)
platformio_override.iniusermods/zigbee_rgb_light/README.mdusermods/zigbee_rgb_light/research-phillips-hue.mdusermods/zigbee_rgb_light/research.mdusermods/zigbee_rgb_light/test_zigbee_hue.pyusermods/zigbee_rgb_light/usermod_zigbee_rgb_light.h
✅ Files skipped from review due to trivial changes (1)
- platformio_override.ini
|
|
||
| > **Note:** Classic ESP32, ESP32-S2, ESP32-S3, ESP32-C3, and ESP8266 do **not** have an 802.15.4 radio and cannot run this usermod. | ||
|
|
||
| > **Note:** The 802.15.4 radio shares the RMT peripheral on the ESP32-C6. Digital LED buses (WS2812B via RMT) will conflict -- use `-DWLED_MAX_DIGITAL_CHANNELS=0` and analog/PWM outputs instead. |
There was a problem hiding this comment.
The coexistence explanation is technically wrong here.
Lines 21 and 265 describe 802.15.4 as sharing RMT and WiFi/802.15.4 as having separate radios. The current Espressif docs describe a single shared 2.4 GHz RF path under coexistence arbitration. Please keep the digital-output limitation as a workaround/implementation constraint, not as a silicon RMT-sharing fact.
Based on learnings, the ESP32-C6 802.15.4 radio does NOT use or share the RMT peripheral; the documented shared resource is the single 2.4 GHz RF path.
Also applies to: 263-270
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@usermods/zigbee_rgb_light/README.md` at line 21, Update the README text that
claims "802.15.4 radio shares the RMT peripheral on the ESP32-C6" to instead
state that the ESP32-C6 uses a single shared 2.4 GHz RF path under coexistence
arbitration and that the RMT conflict statement is an implementation/workaround
constraint (not silicon-level fact); keep the practical guidance to use
-DWLED_MAX_DIGITAL_CHANNELS=0 and analog/PWM outputs as a workaround for digital
LED buses (WS2812B via RMT), and revise the duplicated passages (the current
note and the section around the other occurrence) to reflect the corrected
coexistence explanation and refer to Espressif coexistence arbitration rather
than RMT sharing.
| | API | Purpose | Tested? | | ||
| |-----|---------|---------| | ||
| | `esp_zb_secur_TC_standard_preconfigure_key_set()` | Set TC pre-configured key | Yes, no effect | | ||
| | `esp_zb_secur_link_key_exchange_required_set(false)` | Skip TC link key exchange | Yes, inconclusive (needs re-test) | | ||
| | `esp_zb_secur_TC_standard_distributed_key_set()` | Set distributed network key | No | | ||
| | `esp_zb_secur_network_security_enable()` | Enable/disable NWK security | No | | ||
| | `esp_zb_secur_network_key_set()` | Set NWK key directly | No | | ||
| | `esp_zb_secur_network_min_join_lqi_set()` | Set min LQI for joining | No | | ||
| | `esp_zb_secur_ic_set()` | Set install code | Yes, returns ESP_FAIL | | ||
| | `esp_zb_secur_ic_str_set()` | Set install code (string) | No | | ||
| | `esp_zb_secur_multi_TC_standard_preconfigure_key_add()` | Add TC key to list | No | | ||
| | `esp_zb_secur_multi_standard_distributed_key_add()` | Add distributed key to list | No | |
There was a problem hiding this comment.
The security API matrix is stale.
This table still marks esp_zb_secur_TC_standard_distributed_key_set() and esp_zb_secur_network_min_join_lqi_set() as untested, even though the TL;DR and the current usermod depend on both. Update the table or label it as historical so future debugging doesn't treat working fixes as speculative.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~549-~549: Do not mix variants of the same word (‘pre-configure’ and ‘preconfigure’) within a single text.
Context: ...andard_preconfigure_key_set()| Set TC pre-configured key | Yes, no effect | |esp_zb_secur_...
(EN_WORD_COHERENCY)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@usermods/zigbee_rgb_light/research-phillips-hue.md` around lines 547 - 558,
The security API matrix is outdated: update the table rows for
esp_zb_secur_TC_standard_distributed_key_set() and
esp_zb_secur_network_min_join_lqi_set() to reflect that they have been
used/tested by the TL;DR and current usermod (or alternatively mark the entire
table as "historical / may be stale"), and add a brief note linking that these
APIs are relied upon by the usermod so future readers won't treat them as
unverified; ensure the changed entries reference the exact function names
esp_zb_secur_TC_standard_distributed_key_set() and
esp_zb_secur_network_min_join_lqi_set().
| ### 2. RMT Peripheral Conflict | ||
|
|
||
| The ESP32-C6 has only 2 RMT channels. The 802.15.4 radio shares the RMT peripheral, causing `rmt_tx_wait_all_done() flush timeout` errors if any RMT-based LED bus is active. | ||
|
|
||
| Solution: set `-DWLED_MAX_DIGITAL_CHANNELS=0` in `build_flags` to prevent creation of RMT/I2S LED buses. Use analog (PWM/LEDC) outputs or an external SPI LED controller instead. | ||
|
|
||
| The `#ifndef` guard in `wled00/const.h` allows this command-line override to take effect without redefinition warnings. |
There was a problem hiding this comment.
Please stop describing this as an RMT-sharing hardware conflict.
The workaround may still be required for this build, but the documented shared resource on C6 is the single 2.4 GHz RF path, not the RMT peripheral. Describing it as “802.15.4 shares RMT” points future debugging at the wrong subsystem.
Based on learnings, the ESP32-C6 802.15.4 radio does NOT use or share the RMT peripheral; the documented shared resource is the single 2.4 GHz RF path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@usermods/zigbee_rgb_light/research.md` around lines 57 - 63, Update the
documentation to remove the incorrect claim that the ESP32-C6 802.15.4 radio
shares the RMT peripheral; instead state that the documented shared resource is
the single 2.4 GHz RF path and that RMT-based LED buses can still cause
`rmt_tx_wait_all_done()` timeouts in some builds, so the suggested workaround
(`-DWLED_MAX_DIGITAL_CHANNELS=0` in build_flags) remains valid; reference
`rmt_tx_wait_all_done()`, the build flag `-DWLED_MAX_DIGITAL_CHANNELS=0`, and
the file `wled00/const.h` in the explanation so readers can locate the related
code and macros.
| if (zbStateMutex && xSemaphoreTake(zbStateMutex, pdMS_TO_TICKS(10)) == pdTRUE) { | ||
| localPower = powerOn; | ||
| localBri = zbBrightness; | ||
| localHue = zbHue; | ||
| localSat = zbSaturation; | ||
| localX = zbColorX; | ||
| localY = zbColorY; | ||
| localUseXY = zbUseXY; | ||
| xSemaphoreGive(zbStateMutex); | ||
| } else { | ||
| return; // could not acquire mutex, try again next loop | ||
| } |
There was a problem hiding this comment.
Clear stateChanged only after the snapshot succeeds.
loop() clears the flag before applyState() reacquires zbStateMutex. If applyState() hits the early-return path at Line 471, the pending Zigbee command is lost with stateChanged == false, so it will never be retried.
Also applies to: 827-835
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@usermods/zigbee_rgb_light/usermod_zigbee_rgb_light.h` around lines 462 - 473,
The snapshot-and-clear race: don't clear stateChanged in loop() before
applyState() can act; instead, have loop() take zbStateMutex, copy state into
locals (as it already does) but do NOT set stateChanged = false there — instead,
change applyState() to accept/return a success flag (or reacquire zbStateMutex
and clear stateChanged only after a successful send) so that applyState() clears
stateChanged only when the Zigbee command actually succeeds; reference
zbStateMutex, stateChanged, loop(), and applyState() when making this change.
| void zbUpdateState(bool power, uint8_t bri, uint16_t colorX, uint16_t colorY, bool useXY) | ||
| { | ||
| if (!s_zb_instance) return; | ||
| auto *self = s_zb_instance; | ||
|
|
||
| if (self->zbStateMutex && xSemaphoreTake(self->zbStateMutex, pdMS_TO_TICKS(10)) == pdTRUE) { | ||
| self->powerOn = power; | ||
| self->zbBrightness = bri; | ||
| if (useXY) { | ||
| self->zbColorX = colorX; | ||
| self->zbColorY = colorY; | ||
| } | ||
| self->zbUseXY = useXY; | ||
| self->stateChanged = true; | ||
| xSemaphoreGive(self->zbStateMutex); | ||
| } |
There was a problem hiding this comment.
Don't silently discard inbound Zigbee commands on lock contention.
Most raw-command paths funnel through zbUpdateState(). When Line 1025 times out, the frame has already been parsed, but no retry is queued and stateChanged stays false, so on/off/level/color updates can sporadically disappear under contention.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@usermods/zigbee_rgb_light/usermod_zigbee_rgb_light.h` around lines 1020 -
1035, zbUpdateState currently drops inbound updates when
xSemaphoreTake(self->zbStateMutex, pdMS_TO_TICKS(10)) times out, losing parsed
commands; change it so a failed mutex take does not silently discard the update:
if the semaphore take fails, copy the incoming parameters (power, bri, colorX,
colorY, useXY) into a thread-safe fallback (either push a small struct to a
FreeRTOS queue like zbUpdateQueue or set an atomic/pendingUpdate structure) and
return, or increase/loop the timeout until queued—ensure the consumer (the task
that normally holds zbStateMutex) will apply queued pending updates and set
stateChanged; reference zbUpdateState, s_zb_instance, zbStateMutex,
stateChanged, and ensure the fallback is safe without holding zbStateMutex.
| if (!s_zb_instance) return; | ||
| auto *self = s_zb_instance; | ||
|
|
||
| if (self->zbStateMutex && xSemaphoreTake(self->zbStateMutex, pdMS_TO_TICKS(10)) == pdTRUE) { |
There was a problem hiding this comment.
extend timeout - 10ms is easy to miss when the MCU is very busy.
@DedeHai most likely from the pioarduino framework - it is somewhat bigger than the tasmota framework. |
the missing lines
| SemaphoreHandle_t zbStateMutex = nullptr; | ||
|
|
||
| // Zigbee→WLED state — written from the Zigbee task, read in loop() | ||
| volatile bool stateChanged = false; |
There was a problem hiding this comment.
this shadows a global variable, better find a unique name.
| zbColorY = *(const uint16_t *)message->attribute.data.value; | ||
| zbUseXY = true; | ||
| ESP_LOGD("ZigbeeRGB", "Color Y: 0x%04x", (unsigned)zbColorY); | ||
| stateChanged = true; |
There was a problem hiding this comment.
This if ... else if .... could be more readable as a switch-case (nested in the surrounding one)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (10)
usermods/zigbee_rgb_light/usermod_zigbee_rgb_light.h (4)
310-310:⚠️ Potential issue | 🟠 MajorIncrease mutex timeouts in Zigbee hot paths; 10 ms is too tight.
These lock windows are short enough to miss during normal Zigbee/ZBOSS work, causing sporadic command/report loss.
🛠️ Suggested fix
+#ifndef ZB_STATE_MUTEX_TIMEOUT_MS +#define ZB_STATE_MUTEX_TIMEOUT_MS 200 +#endif ... - xSemaphoreTake(zbStateMutex, pdMS_TO_TICKS(10)) + xSemaphoreTake(zbStateMutex, pdMS_TO_TICKS(ZB_STATE_MUTEX_TIMEOUT_MS))Based on learnings,
xSemaphoreTake(zbStateMutex, ...)should use ~100–200 ms in this usermod.Also applies to: 462-462, 963-963, 1025-1025, 1444-1444, 1486-1486, 1528-1528
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@usermods/zigbee_rgb_light/usermod_zigbee_rgb_light.h` at line 310, The semaphore timeout used when taking zbStateMutex is too short (pdMS_TO_TICKS(10)) and causes missed locks; update every xSemaphoreTake(zbStateMutex, pdMS_TO_TICKS(10)) call in usermod_zigbee_rgb_light.h (including the occurrences around zbStateMutex checks at the lines referenced) to use a larger timeout such as pdMS_TO_TICKS(200) (or 100–200 ms) so Zigbee/ZBOSS hot paths have sufficient lock window; ensure all instances (the calls at the noted duplicates) are changed consistently to the new timeout constant.
1061-1072:⚠️ Potential issue | 🟠 MajorCount only successful bind responses before scheduling reporting config.
Line 1066 increments completion for all
Bind_rspstatuses. A failed cluster bind can still trigger reporting setup and misleadingN/3diagnostics.🛠️ Suggested fix
static void zbBindCallback(esp_zb_zdp_status_t status, void *user_ctx) { uint16_t cluster = (uint16_t)(uintptr_t)user_ctx; ESP_LOGD("ZigbeeRGB", "Bind response for cluster 0x%04x: status=0x%02x", cluster, status); + if (status != ESP_ZB_ZDP_STATUS_SUCCESS) { + ESP_LOGW("ZigbeeRGB", "Bind failed for cluster 0x%04x: status=0x%02x", cluster, status); + return; + } + s_zb_binds_completed++; - if (s_zb_binds_completed >= s_zb_binds_needed) { + if (s_zb_binds_completed == s_zb_binds_needed) { ESP_LOGD("ZigbeeRGB", "All %d bindings completed, configuring reporting...", s_zb_binds_needed); esp_zb_scheduler_alarm(zbSetupReportingConfig, 0, 100); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@usermods/zigbee_rgb_light/usermod_zigbee_rgb_light.h` around lines 1061 - 1072, The callback zbBindCallback currently increments s_zb_binds_completed for every Bind_rsp regardless of status; change it to only increment (and log a success) when the esp_zb_zdp_status_t status indicates success (e.g., compare against ESP_ZB_ZDP_STATUS_SUCCESS), and only when that success count reaches s_zb_binds_needed call esp_zb_scheduler_alarm(zbSetupReportingConfig, 0, 100); for non-success statuses, log the failure (including cluster and status) but do not increment s_zb_binds_completed or trigger reporting setup.
827-835:⚠️ Potential issue | 🟠 MajorClear
stateChangedonly after a successful apply snapshot.Line 830 clears the flag before
applyState()runs, butapplyState()can early-return at Line 472. That can drop inbound Zigbee commands.Also applies to: 462-473
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@usermods/zigbee_rgb_light/usermod_zigbee_rgb_light.h` around lines 827 - 835, The code clears stateChanged before calling applyState(), which may early-return and cause dropped Zigbee commands; change the logic so stateChanged is only cleared after a successful snapshot application: acquire zbStateMutex with xSemaphoreTake, read stateChanged into pending, release the mutex, call applyState(), and only if applyState() indicates success (or does not early-return) re-acquire zbStateMutex and set stateChanged = false (then xSemaphoreGive); alternatively modify applyState() (or make it return a bool) so the caller can detect success and clear stateChanged afterwards; update uses of stateChanged clearing at both the shown block and the similar block around lines 462-473 to follow this pattern (referencing zbStateMutex, xSemaphoreTake, stateChanged, applyState, xSemaphoreGive).
283-293:⚠️ Potential issue | 🟠 MajorDo not drop WLED→Zigbee reports when
prepareReport()misses the mutex.At Line 285, lock contention causes a silent drop; Line 870 still schedules the callback, which then sees no pending snapshot and exits. That loses state sync permanently for that trigger.
🛠️ Suggested fix
- void prepareReport(bool power, uint8_t bri, uint16_t x, uint16_t y) + bool prepareReport(bool power, uint8_t bri, uint16_t x, uint16_t y) { - if (zbStateMutex && xSemaphoreTake(zbStateMutex, pdMS_TO_TICKS(10)) == pdTRUE) { + if (zbStateMutex && xSemaphoreTake(zbStateMutex, pdMS_TO_TICKS(200)) == pdTRUE) { reportPowerOn = power; reportBri = bri; reportColorX = x; reportColorY = y; reportPending = true; xSemaphoreGive(zbStateMutex); + return true; } + return false; } ... - prepareReport(newPower, newBri, newX, newY); - esp_zb_scheduler_alarm(zbReportStateViaScheduler, 0, 10); + if (prepareReport(newPower, newBri, newX, newY)) { + esp_zb_scheduler_alarm(zbReportStateViaScheduler, 0, 10); + } else { + esp_zb_scheduler_alarm(zbReportStateViaScheduler, 0, 50); // deferred retry path + }Based on learnings, Zigbee task load can exceed 10 ms; timeouts here should be at least ~100–200 ms.
Also applies to: 860-871
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@usermods/zigbee_rgb_light/usermod_zigbee_rgb_light.h` around lines 283 - 293, prepareReport silently drops WLED→Zigbee reports when xSemaphoreTake times out; change the mutex acquisition to a longer timeout and make the write atomic/guaranteed: in prepareReport() replace pdMS_TO_TICKS(10) with a larger wait (e.g., pdMS_TO_TICKS(200) or portMAX_DELAY) or loop/retry until taken, then set reportPowerOn/reportBri/reportColorX/reportColorY/reportPending and give the mutex; also ensure the scheduled callback code that reads reportPending (the callback referenced around lines ~860-871) relies on the guaranteed write and does not treat a missed mutex as a successful schedule.usermods/zigbee_rgb_light/README.md (1)
21-21:⚠️ Potential issue | 🟡 MinorAlign coexistence text with actual C6 architecture (RF-path sharing, not RMT sharing).
These lines currently mix two inaccurate statements (“RMT shared” and “separate radios”), which can mislead debugging and deployment decisions.
Based on learnings, ESP32-C6 WiFi/BLE/802.15.4 share one 2.4 GHz RF path under coexistence arbitration; 802.15.4 does not share the RMT peripheral.
Also applies to: 261-261, 265-265
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@usermods/zigbee_rgb_light/README.md` at line 21, The README note incorrectly states that the 802.15.4 radio "shares the RMT peripheral" on ESP32-C6; update the wording in the README (the sentence containing "802.15.4 radio shares the RMT peripheral on the ESP32-C6" and the duplicated lines at 261 and 265) to reflect that WiFi/BLE/802.15.4 share a single 2.4 GHz RF path under coexistence arbitration (not the RMT peripheral), remove the RMT-sharing claim, and keep the practical guidance (e.g., recommend using -DWLED_MAX_DIGITAL_CHANNELS=0 and analog/PWM outputs) while clarifying the correct cause as RF-path coexistence rather than RMT resource conflict.usermods/zigbee_rgb_light/research.md (2)
21-21:⚠️ Potential issue | 🟡 MinorUpdate the stale stack-size note for
zigbeeTask.Line 21 still documents an 8KB stack, but the implementation default is now
ZIGBEE_TASK_STACK_SIZE = 16384, so this reference is outdated.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@usermods/zigbee_rgb_light/research.md` at line 21, The documentation mentions zigbeeTask uses an 8KB stack but the implementation now sets ZIGBEE_TASK_STACK_SIZE = 16384; update the note to reflect the current default stack size (16KB) and, if helpful, reference the ZIGBEE_TASK_STACK_SIZE constant so the doc stays in sync with code; ensure the sentence about task communication still references zigbeeTask and zbStateMutex but replaces "8KB" with "16KB (ZIGBEE_TASK_STACK_SIZE = 16384)" or similar.
57-63:⚠️ Potential issue | 🟡 MinorCorrect the RMT/coexistence explanation to avoid a wrong root-cause narrative.
Lines 59 and 93 describe this as an 802.15.4↔RMT hardware-sharing conflict; that is inaccurate and can mislead future debugging.
Based on learnings, the ESP32-C6 802.15.4 radio does not share the RMT peripheral; the shared resource is the single 2.4 GHz RF path under coexistence arbitration.
Also applies to: 93-94
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@usermods/zigbee_rgb_light/research.md` around lines 57 - 63, Update the "2. RMT Peripheral Conflict" explanation to remove the incorrect claim that the 802.15.4 radio shares the RMT peripheral and instead state that the conflict arises from the single 2.4 GHz RF path under coexistence arbitration; keep the recommendation to set -DWLED_MAX_DIGITAL_CHANNELS=0 and the note about the `#ifndef` guard in wled00/const.h, and apply the same correction to the duplicate text around lines referenced (the paragraph at lines ~93-94) so both places accurately describe the RF/coexistence root cause rather than an RMT hardware sharing issue.usermods/zigbee_rgb_light/test_zigbee_hue.py (1)
30-33:⚠️ Potential issue | 🟠 MajorReplace real-looking CLI credential values with placeholders.
The usage block should not publish concrete bridge/API/device identifiers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@usermods/zigbee_rgb_light/test_zigbee_hue.py` around lines 30 - 33, The usage block exposes real-looking credentials; replace the concrete values after the CLI flags --bridge-ip, --api-key, --light-id, and --wled-ip in the test usage/sample command with non-sensitive placeholders (e.g., <BRIDGE_IP>, <API_KEY>, <LIGHT_ID>, <WLED_IP> or example values like 192.0.2.1, API_KEY, 1, 192.0.2.2) so the test shows safe example inputs instead of real identifiers; update the sample invocation in test_zigbee_hue.py accordingly.usermods/zigbee_rgb_light/research-phillips-hue.md (2)
547-555:⚠️ Potential issue | 🟡 MinorUpdate the security API matrix to reflect current validated behavior.
esp_zb_secur_TC_standard_distributed_key_set()andesp_zb_secur_network_min_join_lqi_set()are still marked “No,” but the document and implementation now depend on them.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@usermods/zigbee_rgb_light/research-phillips-hue.md` around lines 547 - 555, Update the security API matrix row entries to reflect validated behavior: change esp_zb_secur_TC_standard_distributed_key_set() from "No" to "Yes (used by implementation)" and change esp_zb_secur_network_min_join_lqi_set() from "No" to "Yes (required by implementation)"; keep esp_zb_secur_network_security_enable(), esp_zb_secur_network_key_set() as "No" if still untested and leave esp_zb_secur_ic_set() marked with its observed "Yes, returns ESP_FAIL" behavior. Ensure you edit the table entries for the exact function names esp_zb_secur_TC_standard_distributed_key_set() and esp_zb_secur_network_min_join_lqi_set() so the document accurately reflects that the implementation depends on them.
155-160:⚠️ Potential issue | 🟡 MinorMark the outdated “on hold / router mode” notes as historical (or update them).
These lines conflict with the file’s own current-status section and now read as stale implementation state.
Also applies to: 204-205
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@usermods/zigbee_rgb_light/research-phillips-hue.md` around lines 155 - 160, The paragraph under the section "### 2. WLED Zigbee usermod" that says "Currently on hold until the standalone test succeeds" and the related "on hold / router mode" notes are stale; update them to reflect current status or mark them explicitly as historical. Edit the "### 2. WLED Zigbee usermod" block (and the duplicate notes later in the file) to either remove the outdated status, replace it with the current-status text from the file's existing "current-status" section, or prepend a clear "Historical note (DATE):" label so readers know the note is archival rather than current. Ensure you update both occurrences of the same wording.
🤖 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/zigbee_rgb_light/README.md`:
- Around line 283-287: The README example exposes real-looking bridge
credentials and hosts in the sample command invoking test_zigbee_hue.py; replace
the literal values for --bridge-ip, --api-key, --light-id and --wled-ip with
non-sensitive placeholders (e.g. <BRIDGE_IP>, <API_KEY>, <LIGHT_ID>, <WLED_IP>)
and update the descriptive text to indicate these are placeholders to be
replaced by the user. Ensure the command line in README.md uses those
placeholders for the test_zigbee_hue.py invocation and remove any real-looking
secrets or IP addresses.
---
Duplicate comments:
In `@usermods/zigbee_rgb_light/README.md`:
- Line 21: The README note incorrectly states that the 802.15.4 radio "shares
the RMT peripheral" on ESP32-C6; update the wording in the README (the sentence
containing "802.15.4 radio shares the RMT peripheral on the ESP32-C6" and the
duplicated lines at 261 and 265) to reflect that WiFi/BLE/802.15.4 share a
single 2.4 GHz RF path under coexistence arbitration (not the RMT peripheral),
remove the RMT-sharing claim, and keep the practical guidance (e.g., recommend
using -DWLED_MAX_DIGITAL_CHANNELS=0 and analog/PWM outputs) while clarifying the
correct cause as RF-path coexistence rather than RMT resource conflict.
In `@usermods/zigbee_rgb_light/research-phillips-hue.md`:
- Around line 547-555: Update the security API matrix row entries to reflect
validated behavior: change esp_zb_secur_TC_standard_distributed_key_set() from
"No" to "Yes (used by implementation)" and change
esp_zb_secur_network_min_join_lqi_set() from "No" to "Yes (required by
implementation)"; keep esp_zb_secur_network_security_enable(),
esp_zb_secur_network_key_set() as "No" if still untested and leave
esp_zb_secur_ic_set() marked with its observed "Yes, returns ESP_FAIL" behavior.
Ensure you edit the table entries for the exact function names
esp_zb_secur_TC_standard_distributed_key_set() and
esp_zb_secur_network_min_join_lqi_set() so the document accurately reflects that
the implementation depends on them.
- Around line 155-160: The paragraph under the section "### 2. WLED Zigbee
usermod" that says "Currently on hold until the standalone test succeeds" and
the related "on hold / router mode" notes are stale; update them to reflect
current status or mark them explicitly as historical. Edit the "### 2. WLED
Zigbee usermod" block (and the duplicate notes later in the file) to either
remove the outdated status, replace it with the current-status text from the
file's existing "current-status" section, or prepend a clear "Historical note
(DATE):" label so readers know the note is archival rather than current. Ensure
you update both occurrences of the same wording.
In `@usermods/zigbee_rgb_light/research.md`:
- Line 21: The documentation mentions zigbeeTask uses an 8KB stack but the
implementation now sets ZIGBEE_TASK_STACK_SIZE = 16384; update the note to
reflect the current default stack size (16KB) and, if helpful, reference the
ZIGBEE_TASK_STACK_SIZE constant so the doc stays in sync with code; ensure the
sentence about task communication still references zigbeeTask and zbStateMutex
but replaces "8KB" with "16KB (ZIGBEE_TASK_STACK_SIZE = 16384)" or similar.
- Around line 57-63: Update the "2. RMT Peripheral Conflict" explanation to
remove the incorrect claim that the 802.15.4 radio shares the RMT peripheral and
instead state that the conflict arises from the single 2.4 GHz RF path under
coexistence arbitration; keep the recommendation to set
-DWLED_MAX_DIGITAL_CHANNELS=0 and the note about the `#ifndef` guard in
wled00/const.h, and apply the same correction to the duplicate text around lines
referenced (the paragraph at lines ~93-94) so both places accurately describe
the RF/coexistence root cause rather than an RMT hardware sharing issue.
In `@usermods/zigbee_rgb_light/test_zigbee_hue.py`:
- Around line 30-33: The usage block exposes real-looking credentials; replace
the concrete values after the CLI flags --bridge-ip, --api-key, --light-id, and
--wled-ip in the test usage/sample command with non-sensitive placeholders
(e.g., <BRIDGE_IP>, <API_KEY>, <LIGHT_ID>, <WLED_IP> or example values like
192.0.2.1, API_KEY, 1, 192.0.2.2) so the test shows safe example inputs instead
of real identifiers; update the sample invocation in test_zigbee_hue.py
accordingly.
In `@usermods/zigbee_rgb_light/usermod_zigbee_rgb_light.h`:
- Line 310: The semaphore timeout used when taking zbStateMutex is too short
(pdMS_TO_TICKS(10)) and causes missed locks; update every
xSemaphoreTake(zbStateMutex, pdMS_TO_TICKS(10)) call in
usermod_zigbee_rgb_light.h (including the occurrences around zbStateMutex checks
at the lines referenced) to use a larger timeout such as pdMS_TO_TICKS(200) (or
100–200 ms) so Zigbee/ZBOSS hot paths have sufficient lock window; ensure all
instances (the calls at the noted duplicates) are changed consistently to the
new timeout constant.
- Around line 1061-1072: The callback zbBindCallback currently increments
s_zb_binds_completed for every Bind_rsp regardless of status; change it to only
increment (and log a success) when the esp_zb_zdp_status_t status indicates
success (e.g., compare against ESP_ZB_ZDP_STATUS_SUCCESS), and only when that
success count reaches s_zb_binds_needed call
esp_zb_scheduler_alarm(zbSetupReportingConfig, 0, 100); for non-success
statuses, log the failure (including cluster and status) but do not increment
s_zb_binds_completed or trigger reporting setup.
- Around line 827-835: The code clears stateChanged before calling applyState(),
which may early-return and cause dropped Zigbee commands; change the logic so
stateChanged is only cleared after a successful snapshot application: acquire
zbStateMutex with xSemaphoreTake, read stateChanged into pending, release the
mutex, call applyState(), and only if applyState() indicates success (or does
not early-return) re-acquire zbStateMutex and set stateChanged = false (then
xSemaphoreGive); alternatively modify applyState() (or make it return a bool) so
the caller can detect success and clear stateChanged afterwards; update uses of
stateChanged clearing at both the shown block and the similar block around lines
462-473 to follow this pattern (referencing zbStateMutex, xSemaphoreTake,
stateChanged, applyState, xSemaphoreGive).
- Around line 283-293: prepareReport silently drops WLED→Zigbee reports when
xSemaphoreTake times out; change the mutex acquisition to a longer timeout and
make the write atomic/guaranteed: in prepareReport() replace pdMS_TO_TICKS(10)
with a larger wait (e.g., pdMS_TO_TICKS(200) or portMAX_DELAY) or loop/retry
until taken, then set
reportPowerOn/reportBri/reportColorX/reportColorY/reportPending and give the
mutex; also ensure the scheduled callback code that reads reportPending (the
callback referenced around lines ~860-871) relies on the guaranteed write and
does not treat a missed mutex as a successful schedule.
🪄 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: cc0ae929-ea0c-4330-827e-be8dbb79bfc0
📒 Files selected for processing (6)
platformio_override.iniusermods/zigbee_rgb_light/README.mdusermods/zigbee_rgb_light/research-phillips-hue.mdusermods/zigbee_rgb_light/research.mdusermods/zigbee_rgb_light/test_zigbee_hue.pyusermods/zigbee_rgb_light/usermod_zigbee_rgb_light.h
✅ Files skipped from review due to trivial changes (1)
- platformio_override.ini
| python3 usermods/zigbee_rgb_light/test_zigbee_hue.py \ | ||
| --bridge-ip 192.168.178.216 \ | ||
| --api-key XBfT0n000WWp2FV6DxcOnbhcxV5X7SFlKpB53Bix \ | ||
| --light-id 23 \ | ||
| --wled-ip 192.168.178.107 |
There was a problem hiding this comment.
Remove live-looking bridge credentials and host details from README examples.
These should be placeholders to avoid exposing environment-specific secrets and identifiers.
🔐 Suggested fix
python3 usermods/zigbee_rgb_light/test_zigbee_hue.py \
- --bridge-ip 192.168.178.216 \
- --api-key XBfT0n000WWp2FV6DxcOnbhcxV5X7SFlKpB53Bix \
- --light-id 23 \
- --wled-ip 192.168.178.107
+ --bridge-ip <bridge-ip> \
+ --api-key <api-key> \
+ --light-id <light-id> \
+ --wled-ip <wled-ip>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@usermods/zigbee_rgb_light/README.md` around lines 283 - 287, The README
example exposes real-looking bridge credentials and hosts in the sample command
invoking test_zigbee_hue.py; replace the literal values for --bridge-ip,
--api-key, --light-id and --wled-ip with non-sensitive placeholders (e.g.
<BRIDGE_IP>, <API_KEY>, <LIGHT_ID>, <WLED_IP>) and update the descriptive text
to indicate these are placeholders to be replaced by the user. Ensure the
command line in README.md uses those placeholders for the test_zigbee_hue.py
invocation and remove any real-looking secrets or IP addresses.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
I did some more comparisons C6, tasmota (always uses -flto)
RAM: [== ] 23.5% (used 77028 bytes from 327680 bytes)
Flash: [==== ] 39.8% (used 1251682 bytes from 3145728 bytes)
C6, pioarduino
RAM: [=== ] 26.1% (used 85472 bytes from 327680 bytes)
Flash: [===== ] 54.5% (used 1713996 bytes from 3145728 bytes)
C6, pioarduino -flto
RAM: [=== ] 26.1% (used 85448 bytes from 327680 bytes)
Flash: [===== ] 52.1% (used 1639958 bytes from 3145728 bytes)-> pioarduino instead of tasmota: +450KB C6 pioarduino, zigbee
RAM: [=== ] 30.7% (used 100656 bytes from 327680 bytes)
Flash: [====== ] 63.2% (used 1989024 bytes from 3145728 bytes)
C6 pioarduino -flto, zigbee
RAM: [=== ] 30.7% (used 100640 bytes from 327680 bytes)
Flash: [====== ] 60.9% (used 1914582 bytes from 3145728 bytes)-> RAM increases by +15KB, plus 16KB stack for the zigbee task (maybe more due to dynamic heap alloc) -> even with -flto, C6 with 4MB is only possible without OTA partition on the high-perf or debug side C6 pioarduino, zigbee, -O2 (fastest)
RAM: [=== ] 30.7% (used 100656 bytes from 327680 bytes)
Flash: [======= ] 68.9% (used 2168892 bytes from 3145728 bytes)
C6 pioarduino, zigbee, full debug
RAM: [=== ] 30.7% (used 100736 bytes from 327680 bytes)
Flash: [======= ] 72.2% (used 2270762 bytes from 3145728 bytes) |
|
I think @willmmiles has plans for using the coredump partition as part of automated error reporting |
Work in progress - awaiting hardware
Others have tested and been able to add the device to HA and control the on of and brightness
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Tests