Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions platformio.ini
Original file line number Diff line number Diff line change
Expand Up @@ -201,12 +201,12 @@ build_flags =
-D FT_MOONLIGHT=1
-D FT_MONITOR=1
-D EFFECTS_STACK_SIZE=3072 ; psramFound() ? 4 * 1024 : 3 * 1024
-D DRIVERS_STACK_SIZE=4096 ; psramFound() ? 4 * 1024 : 3 * 1024, 4096 is sufficient for now
-D DRIVERS_STACK_SIZE=6144 ; psramFound() ? 4 * 1024 : 3 * 1024, 4096 is sufficient for now. Update: due to FastLED audio I had to increase to 6144 (might want to move audio to a separate task)

; -D FASTLED_TESTING ; causes duplicate definition of initSpiHardware(); - workaround: removed implementation in spi_hw_manager_esp32.cpp.hpp
-D FASTLED_BUILD=\"20260221\"
-D FASTLED_BUILD=\"20260222\"
lib_deps =
https://github.com/FastLED/FastLED#9d0b0eb9b5e59e4093982e0c2bdcfdff72ca80cb ; master 20260221
https://github.com/FastLED/FastLED#27c99130c83ab666c4c6dbf389425fec5f27db05 ; master 20260222
Comment on lines +207 to +209
Copy link

@coderabbitai coderabbitai bot Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, let's examine the platformio.ini file around the specified lines and check for build flags
cat -n platformio.ini | head -230 | tail -50

Repository: MoonModules/MoonLight

Length of output: 3265


🏁 Script executed:

#!/bin/bash
# Search for all FastLED-related configuration flags in the repo
rg -n "FASTLED_RMT5|FASTLED_RMT4|FASTLED_BUILD|FASTLED_" --type ini --type txt --type h --type cpp

Repository: MoonModules/MoonLight

Length of output: 92


🏁 Script executed:

#!/bin/bash
# Check the exact commit details and verify build string alignment
# Find the git repository containing the pinned FastLED commit
rg -n "27c99130c83ab666c4c6dbf389425fec5f27db05" --type ini

Repository: MoonModules/MoonLight

Length of output: 92


🏁 Script executed:

#!/bin/bash
# Search for FASTLED_RMT5, FASTLED_RMT4, and related driver flags without file type restrictions
rg -n "FASTLED_RMT5|FASTLED_RMT4|FASTLED_SPI" .

Repository: MoonModules/MoonLight

Length of output: 47


🏁 Script executed:

#!/bin/bash
# Check the complete [moonlight] section and build_flags to see all configuration
sed -n '199,211p' platformio.ini

Repository: MoonModules/MoonLight

Length of output: 811


🏁 Script executed:

#!/bin/bash
# Check all build_flags sections across the entire platformio.ini
rg -B2 -A5 "build_flags|FASTLED" platformio.ini

Repository: MoonModules/MoonLight

Length of output: 2738


🏁 Script executed:

#!/bin/bash
# Verify the commit date by examining git if available, or search related documentation
# Check if there are any comments/docs about the FastLED version and RMT configuration
rg -n "27c99130c83ab666c4c6dbf389425fec5f27db05|RMT5|RMT4|Handler detached" .

Repository: MoonModules/MoonLight

Length of output: 181


🏁 Script executed:

#!/bin/bash
# Check platform-espressif32 version 55.03.37 for ESP-IDF details
# Look for package.json or version info in typical locations
rg -n "esp-idf|ESP-IDF|5\.[0-9]" platformio.ini | head -20

Repository: MoonModules/MoonLight

Length of output: 2966


🏁 Script executed:

#!/bin/bash
# Search for any Arduino core version or framework configuration
rg -n "framework|arduino|framework-esp32idf|framework-espidf" platformio.ini

Repository: MoonModules/MoonLight

Length of output: 3009


Add -D FASTLED_RMT5=0 to force legacy RMT4 driver and prevent ISR handler degradation.

The FASTLED_BUILD string aligns correctly with the pinned commit. However, platform 55.03.37 uses ESP-IDF 5.5+ with Arduino core 3.x, which causes FastLED to default to the RMT5 driver. RMT5 has documented ISR handler lifecycle issues (see FastLED issue #1903) causing repeated "Handler detached" messages and severe performance drops (~2 FPS). Add -D FASTLED_RMT5=0 to the [moonlight] build_flags to force the stable legacy RMT4 driver.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@platformio.ini` around lines 207 - 209, Add the compiler define to force the
legacy RMT4 driver: in the platformio.ini's [moonlight] build_flags (the same
place where -D FASTLED_BUILD is set), append -D FASTLED_RMT5=0 so FastLED uses
RMT4 instead of RMT5; update the build_flags string that currently references
FASTLED_BUILD so it includes this new define to prevent the RMT5 ISR "Handler
detached" issues described.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not true on newest FastLED

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🌐 Web query:

FastLED RMT5 driver fixes February 2026 handler detached

💡 Result:

I couldn’t find any FastLED change/issue (through the public repo + recent searches) specifically tied to a “handler detached” message for the ESP32 RMT5 driver in February 2026—so there doesn’t appear to be a known “Feb 2026” targeted fix labeled that way. (If you have the exact log line + platform/core version, I can search more precisely.)

What is documented/fixed recently around ESP32 RMT5 stability:

  • FastLED 3.10.1 fixed an ESP32‑S3 RMT5 break where an internal improvement accidentally set DMA mode true, breaking RMT5. Updating beyond 3.10.1 is recommended if you hit DMA-related failures. [1]
  • Ongoing RMT5 reliability discussions (especially Wi‑Fi flicker/jitter) note RMT5’s limits and practical mitigations (e.g., raising RMT interrupt priority to the max the driver allows and increasing buffering), but these are about underruns/flicker rather than a “handler detached” teardown error. [2]
  • If you need a robust workaround today, FastLED explicitly recommends switching away from RMT5 in some cases:
    • Force legacy RMT4 with -D FASTLED_RMT5=0 (typically requires older ESP32 Arduino core / IDF4-era setups; also note RMT4 and RMT5 can’t coexist). [3]
    • Or use alternative output paths (I2S/SPI/Adafruit bridge), which users/reporting/dev notes often suggest when RMT5 is unstable under load. [3]

Most actionable next step: confirm whether your “handler detached” happens only with DMA enabled. If yes, try updating FastLED (>= 3.10.1) first. [1] If it persists, try FASTLED_RMT5=0 (RMT4) or switch to a non-RMT driver as above. [3]

Sources:
[1] FastLED releases (3.10.1 notes: DMA mode accidentally set true, breaking RMT5) https://github.com/fastled/fastled/releases
[2] FastLED issue #2082 (RMT5 + Wi‑Fi flicker; buffering/priority limits/knobs discussion) FastLED/FastLED#2082
[3] FastLED repository documentation (RMT4 vs RMT5 selection; FASTLED_RMT5=0; alternative drivers) https://github.com/FastLED/FastLED


@ewowi, thank you for the correction! I appreciate you letting me know that the RMT5 "Handler detached" issues have been resolved in the newest FastLED version.

Since you originally provided the learning about RMT5 issues (learning ID 9bb17d59-f1a1-4505-b40f-39a68c64be29) and you're now confirming it's no longer applicable with the latest FastLED, would you like me to remove that outdated learning from the knowledge base? This will prevent it from being referenced in future reviews.


✏️ Learnings added
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 130
File: platformio.ini:207-209
Timestamp: 2026-02-22T12:37:03.751Z
Learning: FastLED RMT5 driver issues (ISR handler "Handler detached" messages and performance degradation) have been resolved in FastLED versions as of commit 27c99130c83ab666c4c6dbf389425fec5f27db05 (master 20260222). The `-D FASTLED_RMT5=0` workaround is no longer needed.
<!-- </add_learning>

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: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2026-02-05T13:20:46.001Z
Learning: In MoonLight with FastLED 3.11 on ESP-IDF 5.x+ (Arduino core 3.x): FastLED defaults to RMT5 driver (wraps Espressif's led_strip component), which has known ISR handler lifecycle issues causing repeated "Handler detached" messages every ~500ms and severe performance degradation (e.g., 2 FPS). RMT5 is more prone to Wi-Fi/interrupt jitter than legacy RMT4. Solution: force legacy RMT4 by adding `-D FASTLED_RMT5=0` to build flags in platformio.ini, or switch to FastLED's parallel I/O drivers which don't use RMT. RMT4 and RMT5 cannot coexist in the same binary.

Learnt from: CR
Repo: MoonModules/MoonLight PR: 0
File: misc/parking/GEMINI.md:0-0
Timestamp: 2025-11-29T16:17:30.619Z
Learning: Applies to misc/parking/**/platformio.ini : Use PlatformIO to build and upload the ESP32 backend C++ code

Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2026-01-16T15:53:11.236Z
Learning: For ESP32-PICO-MINI-02 (and similar ESP32 boards with PSRAM), the critical configuration to enable PSRAM is `board_build.arduino.memory_type = qio_qspi` in platformio.ini. The board JSON may already include `-DBOARD_HAS_PSRAM` and `flash_mode: qio`, but without the memory_type setting, PSRAM won't be properly initialized, causing LoadStoreError crashes when code tries to access PSRAM addresses.

Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2026-01-28T18:06:23.868Z
Learning: ESP32-P4 observed lower headroom for httpd task stack in MoonLight: NodeManager rename (create new node + rebuild controls + remove old) can overflow httpd stack when onUpdate runs in httpd. Mitigation: defer only NodeManager rename heavy work to SvelteKit loop via a small pendingRenames queue; keep identity short-circuit in compareRecursive and equality-guarded controls updates. Increasing httpd stack_size for P4 builds is an optional, broader fix.

Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 121
File: src/MoonLight/Nodes/Drivers/D_FastLED.h:176-176
Timestamp: 2026-02-08T20:43:12.857Z
Learning: In MoonLight FastLED 3.11 driver (src/MoonLight/Nodes/Drivers/D_FastLED.h): Affinity is applied per-channel via fl::ChannelOptions.mAffinity passed to fl::ChannelConfig (not via FastLED.setExclusiveDriver). Keeping setExclusiveDriver commented out leaves all drivers available; FastLED then selects the appropriate driver for each channel based on the mAffinity preference ("RMT", "I2S", "SPI", "PARLIO", or "" for auto). This allows per-channel driver preferences rather than a global restriction.

Learnt from: CR
Repo: MoonModules/MoonLight PR: 0
File: misc/parking/GEMINI.md:0-0
Timestamp: 2025-11-29T16:17:30.619Z
Learning: Applies to misc/parking/**/factory_settings.ini : Configure default or factory-set ESP32 parameters in `factory_settings.ini`

Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2026-01-18T17:07:33.344Z
Learning: FastLED library does not support direct 4-byte-per-pixel (CRGBW) buffers. It always expects CRGB (3-byte) arrays and internally generates the W channel from RGB using .setRgbw() API. Casting a 4-byte RGBW buffer to CRGB* causes misalignment (W becomes next pixel's R). For MoonLight's FastLED driver with channelsPerLight=4: must either (1) repack RGB in-place before FastLED.show() then restore after, (2) use driver-specific channelsPerLight=3 layout, or (3) allocate temporary RGB-only buffer. Option 1 (in-place repacking) is best for ESP32-D0 memory constraints.

Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2026-01-06T13:45:03.067Z
Learning: For MoonLight on ESP32 at extreme loads (e.g., 49K channels @ 100 fps): Task watchdog can fire when AppDrivers/AppEffects process many nodes without yielding. Even with vTaskDelay(1) at loop end, the node iteration for-loops (PhysicalLayer::loopDrivers line 108-111, PhysicalLayer::loop line 73-75) can take >5 seconds without yielding, starving IDLE task (priority 0). Solution: add taskYIELD() or vTaskDelay(1) inside the node/layer iteration loops to give IDLE task chances to run and reset watchdog. Lowering task priority won't help since IDLE is already at 0 (lowest).

Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2026-01-18T12:49:07.297Z
Learning: In MoonLight channel reordering architecture: Keep rgbwBufferMapping in driver task (not effect task) because: (1) Driver has spare CPU cycles waiting for DMA/I2S/Art-Net timing; effect task is CPU-bound and starvation-prone. (2) Contiguous RGB writes in effects are cache-friendly; scattered writes at final offsets are cache-hostile for non-RGB orders (GRB, BGR, etc.). (3) forEachLightIndex already handles sparse mapping iteration; moving reordering to effects adds complexity without benefit since drivers must send full frames anyway (can't skip iteration). (4) rgbwBufferMapping does brightness/gamma LUT + white extraction + reordering—moving just reordering creates code duplication. (5) Effects should express color intent (CRGB), not hardware details (offsetRed/Green/Blue). Driver-side transformation maintains separation of concerns and task load balance for scaling to 98K+ pixels.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes remove it please

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ewowi, I've removed the outdated learning about the FastLED RMT5 driver issues. The knowledge base now reflects the current state: that RMT5 issues have been resolved in FastLED as of commit 27c99130c83ab666c4c6dbf389425fec5f27db05 (master 20260222), and the -D FASTLED_RMT5=0 workaround is no longer needed. Thank you for keeping the information up to date!

🐰💭
(Learning tidied up!)
/) /)
( •.• )
/>📝✨


⛔ Learnings removed
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2026-02-05T13:20:46.001Z
Learning: In MoonLight with FastLED 3.11 on ESP-IDF 5.x+ (Arduino core 3.x): FastLED defaults to RMT5 driver (wraps Espressif's led_strip component), which has known ISR handler lifecycle issues causing repeated "Handler detached" messages every ~500ms and severe performance degradation (e.g., 2 FPS). RMT5 is more prone to Wi-Fi/interrupt jitter than legacy RMT4. Solution: force legacy RMT4 by adding `-D FASTLED_RMT5=0` to build flags in platformio.ini, or switch to FastLED's parallel I/O drivers which don't use RMT. RMT4 and RMT5 cannot coexist in the same binary.

🧠 Learnings used
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2026-02-05T13:20:46.001Z
Learning: In MoonLight with FastLED 3.11 on ESP-IDF 5.x+ (Arduino core 3.x): FastLED defaults to RMT5 driver (wraps Espressif's led_strip component), which has known ISR handler lifecycle issues causing repeated "Handler detached" messages every ~500ms and severe performance degradation (e.g., 2 FPS). RMT5 is more prone to Wi-Fi/interrupt jitter than legacy RMT4. Solution: force legacy RMT4 by adding `-D FASTLED_RMT5=0` to build flags in platformio.ini, or switch to FastLED's parallel I/O drivers which don't use RMT. RMT4 and RMT5 cannot coexist in the same binary.

Learnt from: CR
Repo: MoonModules/MoonLight PR: 0
File: misc/parking/GEMINI.md:0-0
Timestamp: 2025-11-29T16:17:30.619Z
Learning: Applies to misc/parking/**/platformio.ini : Use PlatformIO to build and upload the ESP32 backend C++ code

Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2026-01-16T15:53:11.236Z
Learning: For ESP32-PICO-MINI-02 (and similar ESP32 boards with PSRAM), the critical configuration to enable PSRAM is `board_build.arduino.memory_type = qio_qspi` in platformio.ini. The board JSON may already include `-DBOARD_HAS_PSRAM` and `flash_mode: qio`, but without the memory_type setting, PSRAM won't be properly initialized, causing LoadStoreError crashes when code tries to access PSRAM addresses.

Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2026-01-28T18:06:23.868Z
Learning: ESP32-P4 observed lower headroom for httpd task stack in MoonLight: NodeManager rename (create new node + rebuild controls + remove old) can overflow httpd stack when onUpdate runs in httpd. Mitigation: defer only NodeManager rename heavy work to SvelteKit loop via a small pendingRenames queue; keep identity short-circuit in compareRecursive and equality-guarded controls updates. Increasing httpd stack_size for P4 builds is an optional, broader fix.

Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 130
File: platformio.ini:207-209
Timestamp: 2026-02-22T12:37:03.751Z
Learning: FastLED RMT5 driver issues (ISR handler "Handler detached" messages and performance degradation) have been resolved in FastLED versions as of commit 27c99130c83ab666c4c6dbf389425fec5f27db05 (master 20260222). The `-D FASTLED_RMT5=0` workaround is no longer needed.
<!-- </add_learning>

Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 121
File: src/MoonLight/Nodes/Drivers/D_FastLED.h:176-176
Timestamp: 2026-02-08T20:43:12.857Z
Learning: In MoonLight FastLED 3.11 driver (src/MoonLight/Nodes/Drivers/D_FastLED.h): Affinity is applied per-channel via fl::ChannelOptions.mAffinity passed to fl::ChannelConfig (not via FastLED.setExclusiveDriver). Keeping setExclusiveDriver commented out leaves all drivers available; FastLED then selects the appropriate driver for each channel based on the mAffinity preference ("RMT", "I2S", "SPI", "PARLIO", or "" for auto). This allows per-channel driver preferences rather than a global restriction.

Learnt from: CR
Repo: MoonModules/MoonLight PR: 0
File: misc/parking/GEMINI.md:0-0
Timestamp: 2025-11-29T16:17:30.619Z
Learning: Applies to misc/parking/**/factory_settings.ini : Configure default or factory-set ESP32 parameters in `factory_settings.ini`

Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2026-01-18T12:49:07.297Z
Learning: In MoonLight channel reordering architecture: Keep rgbwBufferMapping in driver task (not effect task) because: (1) Driver has spare CPU cycles waiting for DMA/I2S/Art-Net timing; effect task is CPU-bound and starvation-prone. (2) Contiguous RGB writes in effects are cache-friendly; scattered writes at final offsets are cache-hostile for non-RGB orders (GRB, BGR, etc.). (3) forEachLightIndex already handles sparse mapping iteration; moving reordering to effects adds complexity without benefit since drivers must send full frames anyway (can't skip iteration). (4) rgbwBufferMapping does brightness/gamma LUT + white extraction + reordering—moving just reordering creates code duplication. (5) Effects should express color intent (CRGB), not hardware details (offsetRed/Green/Blue). Driver-side transformation maintains separation of concerns and task load balance for scaling to 98K+ pixels.

Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2026-01-18T17:07:33.344Z
Learning: FastLED library does not support direct 4-byte-per-pixel (CRGBW) buffers. It always expects CRGB (3-byte) arrays and internally generates the W channel from RGB using .setRgbw() API. Casting a 4-byte RGBW buffer to CRGB* causes misalignment (W becomes next pixel's R). For MoonLight's FastLED driver with channelsPerLight=4: must either (1) repack RGB in-place before FastLED.show() then restore after, (2) use driver-specific channelsPerLight=3 layout, or (3) allocate temporary RGB-only buffer. Option 1 (in-place repacking) is best for ESP32-D0 memory constraints.

Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2026-01-06T13:45:03.067Z
Learning: For MoonLight on ESP32 at extreme loads (e.g., 49K channels @ 100 fps): Task watchdog can fire when AppDrivers/AppEffects process many nodes without yielding. Even with vTaskDelay(1) at loop end, the node iteration for-loops (PhysicalLayer::loopDrivers line 108-111, PhysicalLayer::loop line 73-75) can take >5 seconds without yielding, starving IDLE task (priority 0). Solution: add taskYIELD() or vTaskDelay(1) inside the node/layer iteration loops to give IDLE task chances to run and reset watchdog. Lowering task priority won't help since IDLE is already at 0 (lowest).

Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 84
File: src/MoonLight/Modules/ModuleLightsControl.h:367-379
Timestamp: 2025-12-28T10:28:24.157Z
Learning: In MoonLight PR `#84`: wrapping effectTask's layerP.loop() with monitorMutex while reading channelsE for monitor emission is thread-safe and prevents partial frames, but introduces serialization between effects rendering and monitor emission. Reading from channelsD (driver/front buffer) instead—with monitorMutex held only during the brief pointer swap—is superior because it maintains full parallelism (effects ║ monitor ║ driver), shows actual LED output, and minimizes mutex hold time.

Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2025-12-25T13:34:40.097Z
Learning: For the MoonLight LED system: Effects task can run on Core 0 (PRO_CPU) since it only performs computation (calculating colors, writing to channels array) without timing-critical hardware interaction. WiFi preemption is acceptable for pure computation tasks. Only the driver task (using DMA, I2S, LCD, PARLIO) requires Core 1 (APP_CPU) for uninterrupted timing-critical hardware operations.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain
🏁 Script executed:

#!/bin/bash
# First, let's examine the platformio.ini file around the specified lines and check for build flags
cat -n platformio.ini | head -230 | tail -50

Repository: MoonModules/MoonLight

Length of output: 3265

🏁 Script executed:

#!/bin/bash
# Search for all FastLED-related configuration flags in the repo
rg -n "FASTLED_RMT5|FASTLED_RMT4|FASTLED_BUILD|FASTLED_" --type ini --type txt --type h --type cpp

Repository: MoonModules/MoonLight

Length of output: 92

🏁 Script executed:

#!/bin/bash
# Check the exact commit details and verify build string alignment
# Find the git repository containing the pinned FastLED commit
rg -n "27c99130c83ab666c4c6dbf389425fec5f27db05" --type ini

Repository: MoonModules/MoonLight

Length of output: 92

🏁 Script executed:

#!/bin/bash
# Search for FASTLED_RMT5, FASTLED_RMT4, and related driver flags without file type restrictions
rg -n "FASTLED_RMT5|FASTLED_RMT4|FASTLED_SPI" .

Repository: MoonModules/MoonLight

Length of output: 47

🏁 Script executed:

#!/bin/bash
# Check the complete [moonlight] section and build_flags to see all configuration
sed -n '199,211p' platformio.ini

Repository: MoonModules/MoonLight

Length of output: 811

🏁 Script executed:

#!/bin/bash
# Check all build_flags sections across the entire platformio.ini
rg -B2 -A5 "build_flags|FASTLED" platformio.ini

Repository: MoonModules/MoonLight

Length of output: 2738

🏁 Script executed:

#!/bin/bash
# Verify the commit date by examining git if available, or search related documentation
# Check if there are any comments/docs about the FastLED version and RMT configuration
rg -n "27c99130c83ab666c4c6dbf389425fec5f27db05|RMT5|RMT4|Handler detached" .

Repository: MoonModules/MoonLight

Length of output: 181

🏁 Script executed:

#!/bin/bash
# Check platform-espressif32 version 55.03.37 for ESP-IDF details
# Look for package.json or version info in typical locations
rg -n "esp-idf|ESP-IDF|5\.[0-9]" platformio.ini | head -20

Repository: MoonModules/MoonLight

Length of output: 2966

🏁 Script executed:

#!/bin/bash
# Search for any Arduino core version or framework configuration
rg -n "framework|arduino|framework-esp32idf|framework-espidf" platformio.ini

Repository: MoonModules/MoonLight

Length of output: 3009

Add -D FASTLED_RMT5=0 to force legacy RMT4 driver and prevent ISR handler degradation.

The FASTLED_BUILD string aligns correctly with the pinned commit. However, platform 55.03.37 uses ESP-IDF 5.5+ with Arduino core 3.x, which causes FastLED to default to the RMT5 driver. RMT5 has documented ISR handler lifecycle issues (see FastLED issue #1903) causing repeated "Handler detached" messages and severe performance drops (~2 FPS). Add -D FASTLED_RMT5=0 to the [moonlight] build_flags to force the stable legacy RMT4 driver.

🤖 Prompt for AI Agents

Disabling RMT5 should not be necessary now? The RMT5 driver now allocates more memory during WIFI active mode. Please let me know if this isn't the case. RMT5 will have stress testing either this week if ai credits sustain. Otherwise post friday on refresh.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

https://github.com/ewowi/WLED-sync#25f280b5e8e47e49a95282d0b78a5ce5301af4fe ; sourceIP + fftUdp.clear() if arduino >=3 (20251104)

; 💫 currently only enabled on s3 as esp32dev runs over 100%
Expand Down
16 changes: 13 additions & 3 deletions src/MoonBase/Nodes.h
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ class LiveScriptNode : public Node {
// layout
void onLayout() override; // call map in LiveScript

~LiveScriptNode();
~LiveScriptNode() override;

// LiveScript functions
void compileAndRun();
Expand Down Expand Up @@ -346,6 +346,15 @@ static struct SharedData {
size_t clientListSize;

Coord3D gravity;

// FastLED Audio
bool vocalsActive = false;
float vocalConfidence = 0.0f;
float bassLevel = 0.0f;
float midLevel = 0.0f;
float trebleLevel = 0.0f;
bool beat = false;

} sharedData;

/**
Expand All @@ -359,10 +368,11 @@ static struct SharedData {
#include "MoonLight/Nodes/Drivers/D_ArtnetIn.h"
#include "MoonLight/Nodes/Drivers/D_ArtnetOut.h"
#include "MoonLight/Nodes/Drivers/D_AudioSync.h"
#include "MoonLight/Nodes/Drivers/D_FastLED.h"
#include "MoonLight/Nodes/Drivers/D_FastLEDAudio.h"
#include "MoonLight/Nodes/Drivers/D_FastLEDDriver.h"
#include "MoonLight/Nodes/Drivers/D_Hub75.h"
#include "MoonLight/Nodes/Drivers/D_Infrared.h"
#include "MoonLight/Nodes/Drivers/D_IMU.h"
#include "MoonLight/Nodes/Drivers/D_Infrared.h"
#include "MoonLight/Nodes/Drivers/D_ParallelLEDDriver.h"
#include "MoonLight/Nodes/Drivers/D__Sandbox.h"
#include "MoonLight/Nodes/Effects/E_FastLED.h"
Expand Down
4 changes: 4 additions & 0 deletions src/MoonLight/Modules/ModuleDrivers.h
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,12 @@ class ModuleDrivers : public NodeManager {
addControlValue(control, getNameAndTags<SpiralLayout>());
addControlValue(control, getNameAndTags<SingleRowLayout>());
addControlValue(control, getNameAndTags<SingleColumnLayout>());
addControlValue(control, getNameAndTags<TubesLayout>());

// Drivers, Most used first
addControlValue(control, getNameAndTags<ParallelLEDDriver>());
addControlValue(control, getNameAndTags<FastLEDDriver>());
addControlValue(control, getNameAndTags<FastLEDAudioDriver>());
addControlValue(control, getNameAndTags<ArtNetInDriver>());
addControlValue(control, getNameAndTags<ArtNetOutDriver>());
addControlValue(control, getNameAndTags<AudioSyncDriver>());
Expand Down Expand Up @@ -135,10 +137,12 @@ class ModuleDrivers : public NodeManager {
if (!node) node = checkAndAlloc<TorontoBarGourdsLayout>(name);
if (!node) node = checkAndAlloc<SingleRowLayout>(name);
if (!node) node = checkAndAlloc<SingleColumnLayout>(name);
if (!node) node = checkAndAlloc<TubesLayout>(name);

// Drivers most used first
if (!node) node = checkAndAlloc<ParallelLEDDriver>(name);
if (!node) node = checkAndAlloc<FastLEDDriver>(name);
if (!node) node = checkAndAlloc<FastLEDAudioDriver>(name);
if (!node) node = checkAndAlloc<ArtNetInDriver>(name);
if (!node) node = checkAndAlloc<ArtNetOutDriver>(name);
if (!node) node = checkAndAlloc<AudioSyncDriver>(name);
Expand Down
2 changes: 2 additions & 0 deletions src/MoonLight/Modules/ModuleEffects.h
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ class ModuleEffects : public NodeManager {

// FastLED effects
addControlValue(control, getNameAndTags<RainbowEffect>());
addControlValue(control, getNameAndTags<FLAudioEffect>());

// Moving head effects, alphabetically
addControlValue(control, getNameAndTags<AmbientMoveEffect>());
Expand Down Expand Up @@ -291,6 +292,7 @@ class ModuleEffects : public NodeManager {

// FastLED
if (!node) node = checkAndAlloc<RainbowEffect>(name);
if (!node) node = checkAndAlloc<FLAudioEffect>(name);

// Moving head effects, alphabetically

Expand Down
208 changes: 208 additions & 0 deletions src/MoonLight/Nodes/Drivers/D_FastLEDAudio.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,208 @@
/**
@title MoonLight
@file D_FastLEDAudio.h
@repo https://github.com/MoonModules/MoonLight, submit changes to this file as PRs
@Authors https://github.com/MoonModules/MoonLight/commits/main
@Doc https://moonmodules.org/MoonLight/moonlight/overview/
@Copyright © 2026 Github MoonLight Commit Authors
@license GNU GENERAL PUBLIC LICENSE Version 3, 29 June 2007
@license For non GPL-v3 usage, commercial licenses must be purchased. Contact us for more information.
**/

#pragma once

#if FT_MOONLIGHT

#include "fl/audio.h"
#include "fl/audio/audio_processor.h"
#include "fl/audio_input.h"
#include "fl/time_alpha.h"

// https://github.com/FastLED/FastLED/blob/master/src/fl/audio/README.md

class FastLEDAudioDriver : public Node {
private:
// Member variables for audio configuration
fl::AudioConfigI2S* i2sConfig = nullptr;
fl::AudioConfig* config = nullptr;
fl::shared_ptr<fl::IAudioInput> audioInput;

public:
static const char* name() { return "FastLED Audio"; }
static uint8_t dim() { return _NoD; }
static const char* tags() { return "☸️"; }

fl::AudioProcessor audioProcessor;

bool signalConditioning = true;
bool autoGain = false;
bool noiseFloorTracking = false;
uint8_t channel = fl::Left;

void setup() override {
addControl(signalConditioning, "signalConditioning", "checkbox");
addControl(autoGain, "autoGain", "checkbox");
addControl(noiseFloorTracking, "noiseFloorTracking", "checkbox");
addControl(channel, "channel", "select");
addControlValue("Left");
addControlValue("Right");
addControlValue("Both");

moduleIO->addUpdateHandler([this](const String& originId) { readPins(); }, false);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check handler API definitions and usage patterns
rg -n "addUpdateHandler\|removeUpdateHandler" --type=h --type=cpp -A 3 -B 1 | head -100

Repository: MoonModules/MoonLight

Length of output: 47


🏁 Script executed:

#!/bin/bash
# Find and examine the D_FastLEDAudio class definition and destructor
fd "D_FastLEDAudio" --type f | xargs cat -n | head -200

Repository: MoonModules/MoonLight

Length of output: 7523


🏁 Script executed:

#!/bin/bash
# Search for all destructor implementations in driver files to see the pattern
rg -n "~D_\w+\(\)" --type=h --type=cpp -A 5 | head -150

Repository: MoonModules/MoonLight

Length of output: 47


🏁 Script executed:

#!/bin/bash
# Get full line count of the file and check if it continues beyond line 200
wc -l src/MoonLight/Nodes/Drivers/D_FastLEDAudio.h

Repository: MoonModules/MoonLight

Length of output: 113


🏁 Script executed:

#!/bin/bash
# Get lines 200 onwards to see if destructor exists
tail -100 src/MoonLight/Nodes/Drivers/D_FastLEDAudio.h

Repository: MoonModules/MoonLight

Length of output: 2729


🏁 Script executed:

#!/bin/bash
# Find the Node base class definition
fd "Node\.h" --type f -x cat -n {} \; | head -200

Repository: MoonModules/MoonLight

Length of output: 47


🏁 Script executed:

#!/bin/bash
# Search for moduleIO type and handler methods across the codebase
rg "moduleIO.*addUpdateHandler\|class.*Node.*{" --type=h --type=cpp -B 2 -A 2 | head -150

Repository: MoonModules/MoonLight

Length of output: 212


🏁 Script executed:

#!/bin/bash
# Find Node class definition
fd "Node" --type f | grep -E "\.(h|cpp)$" | head -20

Repository: MoonModules/MoonLight

Length of output: 135


🏁 Script executed:

#!/bin/bash
# Search for Node class definition directly
rg "class Node" --type=h -A 10 | head -100

Repository: MoonModules/MoonLight

Length of output: 2220


🏁 Script executed:

#!/bin/bash
# Search for addUpdateHandler definition or declaration
rg "addUpdateHandler" --type=h --type=cpp -B 3 -A 5 | head -200

Repository: MoonModules/MoonLight

Length of output: 14747


🏁 Script executed:

#!/bin/bash
# Search for removeUpdateHandler or similar removal mechanism
rg "removeUpdateHandler\|remove.*Handler\|erase.*Handler" --type=h --type=cpp -B 2 -A 3 | head -150

Repository: MoonModules/MoonLight

Length of output: 47


🏁 Script executed:

#!/bin/bash
# Look at StatefulService to understand handler lifecycle
cat -n lib/framework/StatefulService.h | head -150

Repository: MoonModules/MoonLight

Length of output: 5876


🏁 Script executed:

#!/bin/bash
# Search for Node destruction and module cleanup
rg "~Node|destroy.*node|NodeManager.*destroy" --type=h --type=cpp -A 5 | head -150

Repository: MoonModules/MoonLight

Length of output: 589


🏁 Script executed:

#!/bin/bash
# Check if there's cleanup logic for handlers when modules are destroyed
rg "callUpdateHandlers\|_updateHandlers" --type=h --type=cpp -B 2 -A 5 | head -200

Repository: MoonModules/MoonLight

Length of output: 47


🏁 Script executed:

#!/bin/bash
# Check destructor patterns in other driver files
rg "~D_\w+\(\)" --type=h -A 10 | head -250

Repository: MoonModules/MoonLight

Length of output: 47


🏁 Script executed:

#!/bin/bash
# Look for Module base class and its lifecycle
fd "Module\.h" --type f -x head -100 {} \;

Repository: MoonModules/MoonLight

Length of output: 3437


Prevent use-after-free when FastLEDAudioDriver is destroyed while handler is registered.

The addUpdateHandler at line 51 registers a lambda that captures this with allowRemove=false, making it permanently unremovable. The destructor does not unregister the handler. If moduleIO outlives the driver instance, the callback will invoke readPins() on a dangling pointer.

Either store the handler ID and call removeUpdateHandler() in the destructor with allowRemove=true, or document the framework guarantee that moduleIO callbacks are cancelled before node destruction.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/MoonLight/Nodes/Drivers/D_FastLEDAudio.h` at line 51, The lambda passed
to moduleIO->addUpdateHandler in FastLEDAudioDriver captures this and is
registered with allowRemove=false causing a permanent callback to readPins(),
risking use-after-free; change the registration to store the returned handler ID
(from addUpdateHandler) as a member (e.g., updateHandlerId_), and in the
FastLEDAudioDriver destructor call
moduleIO->removeUpdateHandler(updateHandlerId_) (or re-register with
allowRemove=true) to ensure the callback is removed before the object is
destroyed; update the class to hold the handler ID and perform removal in the
destructor (or clearly document framework guarantees if you choose not to
remove).

readPins(); // Node added at runtime so initial IO update not received so run explicitly

audioProcessor.onBeat([]() {
sharedData.beat = true;
// EXT_LOGD(ML_TAG, "onBeat");
});

audioProcessor.onVocalStart([]() {
sharedData.vocalsActive = true;
// EXT_LOGD(ML_TAG, "onVocalStart");
});

audioProcessor.onVocalEnd([]() {
sharedData.vocalsActive = false;
// EXT_LOGD(ML_TAG, "onVocalEnd");
});

audioProcessor.onVocalConfidence([](float confidence) {
sharedData.vocalConfidence = sharedData.vocalsActive ? confidence : 0.0;
// EXT_LOGD(ML_TAG, "onVocalConfidence %d", confidence);
});

audioProcessor.onBass([](float level) {
if (level > 0.01f) {
sharedData.bassLevel = level;
// EXT_LOGD(ML_TAG, "onBass: %f", level);
}
});

audioProcessor.onMid([](float level) {
if (level > 0.01f) {
sharedData.midLevel = level;
// EXT_LOGD(ML_TAG, "onBass: %f", level);
}
});
Comment on lines +81 to +86
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Minor: copy-paste log label in onMid callback says "onBass".

Line 84 reads "onBass: %f" inside the onMid lambda — a copy-paste leftover. Also, the onVocalConfidence log (line 71) uses %d for a float argument; should be %f. Both are currently commented out, but worth fixing before uncomment.

🔧 Proposed fix
     audioProcessor.onMid([](float level) {
       if (level > 0.01f) {
         sharedData.midLevel = level;
-        // EXT_LOGD(ML_TAG, "onBass: %f", level);
+        // EXT_LOGD(ML_TAG, "onMid: %f", level);
       }
     });
     audioProcessor.onVocalConfidence([](float confidence) {
       sharedData.vocalConfidence = sharedData.vocalsActive ? confidence : 0.0;
-      // EXT_LOGD(ML_TAG, "onVocalConfidence %d", confidence);
+      // EXT_LOGD(ML_TAG, "onVocalConfidence %f", confidence);
     });
📝 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.

Suggested change
audioProcessor.onMid([](float level) {
if (level > 0.01f) {
sharedData.midLevel = level;
// EXT_LOGD(ML_TAG, "onBass: %f", level);
}
});
audioProcessor.onMid([](float level) {
if (level > 0.01f) {
sharedData.midLevel = level;
// EXT_LOGD(ML_TAG, "onMid: %f", level);
}
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/MoonLight/Nodes/Drivers/D_FastLEDAudio.h` around lines 81 - 86, Fix the
two incorrect log formats in the audio callbacks: in the audioProcessor.onMid
lambda change the log label from "onBass" to "onMid" so it matches the callback
(reference: audioProcessor.onMid and sharedData.midLevel, ML_TAG), and in the
onVocalConfidence log change the printf format specifier from %d to %f to match
the float argument (reference: audioProcessor.onVocalConfidence). Update those
commented log lines accordingly so they are correct when uncommented.


audioProcessor.onTreble([](float level) {
if (level > 0.01f) {
sharedData.trebleLevel = level;
// EXT_LOGD(ML_TAG, "onTreble: %f", level);
}
});
audioProcessor.onPercussion([](fl::PercussionType type) {
EXT_LOGD(ML_TAG, "onPercussion: %d", type);
});
}

void onUpdate(const Char<20>& oldValue, const JsonObject& control) override {
if (control["name"] == "signalConditioning") {
audioProcessor.setSignalConditioningEnabled(signalConditioning);
}
if (control["name"] == "autoGain") {
audioProcessor.setAutoGainEnabled(autoGain);
}
if (control["name"] == "noiseFloorTracking") {
audioProcessor.setNoiseFloorTrackingEnabled(noiseFloorTracking);
}
if (control["name"] == "channel" && oldValue != "") { // not on boot as readPins will do it then
// recreate with the new channel
stopAudio();
startAudio();
}
}

uint8_t pinI2SSD = UINT8_MAX;
uint8_t pinI2SWS = UINT8_MAX;
uint8_t pinI2SSCK = UINT8_MAX;

void readPins() {
if (safeModeMB) {
EXT_LOGW(ML_TAG, "Safe mode enabled, not adding pins");
return;
}

moduleIO->read(
[&](ModuleState& state) {
bool i2sPinsChanged = false;
for (JsonObject pinObject : state.data["pins"].as<JsonArray>()) {
uint8_t usage = pinObject["usage"];
switch (usage) {
case pin_I2S_SD:
if (pinI2SSD != pinObject["GPIO"]) {
pinI2SSD = pinObject["GPIO"];
i2sPinsChanged = true;
}
break;
case pin_I2S_WS:
if (pinI2SWS != pinObject["GPIO"]) {
pinI2SWS = pinObject["GPIO"];
i2sPinsChanged = true;
}
break;
case pin_I2S_SCK:
if (pinI2SSCK != pinObject["GPIO"]) {
pinI2SSCK = pinObject["GPIO"];
i2sPinsChanged = true;
}
break;
}
}
if (i2sPinsChanged && pinI2SSD != UINT8_MAX && pinI2SWS != UINT8_MAX && pinI2SSCK != UINT8_MAX) {
EXT_LOGI(ML_TAG, "(re)creating audioInput)");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Minor: stray closing parenthesis in log string.

"(re)creating audioInput)" has an unmatched ) at the end.

🔧 Proposed fix
-      EXT_LOGI(ML_TAG, "(re)creating audioInput)");
+      EXT_LOGI(ML_TAG, "(re)creating audioInput");
📝 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.

Suggested change
EXT_LOGI(ML_TAG, "(re)creating audioInput)");
EXT_LOGI(ML_TAG, "(re)creating audioInput");
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/MoonLight/Nodes/Drivers/D_FastLEDAudio.h` at line 153, The log string
passed to EXT_LOGI in the D_FastLEDAudio driver contains an extra closing
parenthesis inside the message "(re)creating audioInput)"; remove the stray ')'
so the call to EXT_LOGI(ML_TAG, "(re)creating audioInput") prints the correct
message. Locate the EXT_LOGI usage in D_FastLEDAudio (the log call shown) and
edit the string literal to drop the unmatched parenthesis.


stopAudio();
startAudio();
}
},
name());
}

void loop20ms() override {
if (!audioInput) return;

sharedData.beat = false;

while (fl::AudioSample sample = audioInput->read()) {
audioProcessor.update(sample);
}
}

void startAudio() {
// Create configuration objects
i2sConfig = new fl::AudioConfigI2S(pinI2SWS, pinI2SSD, pinI2SSCK, 0, channel == 1 ? fl::Right : channel == 2 ? fl::Both : fl::Left, 44100, 16, fl::Philips);

config = new fl::AudioConfig(*i2sConfig);

fl::string errorMsg;
audioInput = fl::IAudioInput::create(*config, &errorMsg);
if (!audioInput) {
EXT_LOGE(ML_TAG, "Failed to create audio input: %s", errorMsg.c_str());
return;
}
audioInput->start();
}

void stopAudio() {
if (audioInput) {
audioInput->stop();
audioInput.reset(); // Explicitly release shared_ptr, even makes it a nullptr...
}

// Clean up raw pointers
if (config) {
delete config;
config = nullptr;
}

if (i2sConfig) {
delete i2sConfig;
i2sConfig = nullptr;
}
}

~FastLEDAudioDriver() override { stopAudio(); }
};

#endif
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
@title MoonLight
@file FastLED.h
@file D_FastLEDDriver.h
@repo https://github.com/MoonModules/MoonLight, submit changes to this file as PRs
@Authors https://github.com/MoonModules/MoonLight/commits/main
@Doc https://moonmodules.org/MoonLight/moonlight/overview/
Expand Down
46 changes: 46 additions & 0 deletions src/MoonLight/Nodes/Effects/E_FastLED.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,50 @@ class RainbowEffect : public Node {
}
};

class FLAudioEffect : public Node {
public:
static const char* name() { return "FLAudio"; }
static uint8_t dim() { return _2D; }
static const char* tags() { return "⚡️🎵"; }

void setup() {}

uint16_t hue = 0;
uint8_t beatLevel = 0;
float maxBass = 0;
float maxMid = 0;
float maxTreble = 0;
float maxVocal = 0;

void loop() override {
layer->fadeToBlackBy(70);

// calculate max levels
if (sharedData.bassLevel > maxBass) maxBass = sharedData.bassLevel;
if (sharedData.midLevel > maxMid) maxMid = sharedData.midLevel;
if (sharedData.trebleLevel > maxTreble) maxTreble = sharedData.trebleLevel;
if (sharedData.vocalConfidence > maxVocal) maxVocal = sharedData.vocalConfidence;

if (sharedData.beat) beatLevel = 255;

// EXT_LOGD(ML_TAG, "%f-%d %f-%d %d-%d %f-%d", sharedData.bassLevel, bassLevel, sharedData.trebleLevel, trebleLevel, sharedData.beat, beatLevel, sharedData.vocalsActive ? sharedData.vocalConfidence : 0, vocalsLevel);

uint8_t columnNr = 0;
layer->drawLine(columnNr, layer->size.y - 1, columnNr++, layer->size.y - 1 - layer->size.y * sharedData.bassLevel / maxBass, CRGB::Red);
layer->drawLine(columnNr, layer->size.y - 1, columnNr++, layer->size.y - 1 - layer->size.y * sharedData.midLevel / maxMid, CRGB::Orange);
layer->drawLine(columnNr, layer->size.y - 1, columnNr++, layer->size.y - 1 - layer->size.y * sharedData.trebleLevel / maxTreble, CRGB::Green);
layer->drawLine(columnNr, layer->size.y - 1, columnNr++, layer->size.y - 1 - layer->size.y * sharedData.vocalConfidence / maxVocal, CRGB::Blue);
layer->drawLine(columnNr, layer->size.y - 1, columnNr++, layer->size.y - 1 - layer->size.y * beatLevel / 255, CRGB::Purple);
Comment on lines +68 to +73
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: columnNr++ as a function argument alongside columnNr is undefined/unspecified behaviour.

Each drawLine call passes columnNr (arg 1, x1) and columnNr++ (arg 3, x2) in the same expression. In C++14 and earlier this is undefined behaviour (unsequenced read and modification). In C++17 it is indeterminately sequenced — the compiler may evaluate arg 3 before arg 1, giving drawLine(1, …, 0, …) instead of the intended drawLine(0, …, 0, …). Since x1 ≠ x2 in that ordering, drawLine renders a diagonal segment rather than a vertical bar, and columnNr itself is left at an unexpected value for subsequent calls.

Separate the increment from the call:

🐛 Proposed fix
-    uint8_t columnNr = 0;
-    layer->drawLine(columnNr, layer->size.y - 1, columnNr++, layer->size.y - 1 - layer->size.y * sharedData.bassLevel / maxBass, CRGB::Red);
-    layer->drawLine(columnNr, layer->size.y - 1, columnNr++, layer->size.y - 1 - layer->size.y * sharedData.midLevel / maxMid, CRGB::Orange);
-    layer->drawLine(columnNr, layer->size.y - 1, columnNr++, layer->size.y - 1 - layer->size.y * sharedData.trebleLevel / maxTreble, CRGB::Green);
-    layer->drawLine(columnNr, layer->size.y - 1, columnNr++, layer->size.y - 1 - layer->size.y * sharedData.vocalConfidence / maxVocal, CRGB::Blue);
-    layer->drawLine(columnNr, layer->size.y - 1, columnNr++, layer->size.y - 1 - layer->size.y * beatLevel / 255, CRGB::Purple);
+    uint8_t col = 0;
+    if (maxBass > 0.0f)
+      layer->drawLine(col, layer->size.y - 1, col, layer->size.y - 1 - layer->size.y * sharedData.bassLevel / maxBass, CRGB::Red);
+    ++col;
+    if (maxMid > 0.0f)
+      layer->drawLine(col, layer->size.y - 1, col, layer->size.y - 1 - layer->size.y * sharedData.midLevel / maxMid, CRGB::Orange);
+    ++col;
+    if (maxTreble > 0.0f)
+      layer->drawLine(col, layer->size.y - 1, col, layer->size.y - 1 - layer->size.y * sharedData.trebleLevel / maxTreble, CRGB::Green);
+    ++col;
+    if (maxVocal > 0.0f)
+      layer->drawLine(col, layer->size.y - 1, col, layer->size.y - 1 - layer->size.y * sharedData.vocalConfidence / maxVocal, CRGB::Blue);
+    ++col;
+    layer->drawLine(col, layer->size.y - 1, col, layer->size.y - 1 - layer->size.y * beatLevel / 255, CRGB::Purple);
📝 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.

Suggested change
uint8_t columnNr = 0;
layer->drawLine(columnNr, layer->size.y - 1, columnNr++, layer->size.y - 1 - layer->size.y * sharedData.bassLevel / maxBass, CRGB::Red);
layer->drawLine(columnNr, layer->size.y - 1, columnNr++, layer->size.y - 1 - layer->size.y * sharedData.midLevel / maxMid, CRGB::Orange);
layer->drawLine(columnNr, layer->size.y - 1, columnNr++, layer->size.y - 1 - layer->size.y * sharedData.trebleLevel / maxTreble, CRGB::Green);
layer->drawLine(columnNr, layer->size.y - 1, columnNr++, layer->size.y - 1 - layer->size.y * sharedData.vocalConfidence / maxVocal, CRGB::Blue);
layer->drawLine(columnNr, layer->size.y - 1, columnNr++, layer->size.y - 1 - layer->size.y * beatLevel / 255, CRGB::Purple);
uint8_t col = 0;
if (maxBass > 0.0f)
layer->drawLine(col, layer->size.y - 1, col, layer->size.y - 1 - layer->size.y * sharedData.bassLevel / maxBass, CRGB::Red);
+col;
if (maxMid > 0.0f)
layer->drawLine(col, layer->size.y - 1, col, layer->size.y - 1 - layer->size.y * sharedData.midLevel / maxMid, CRGB::Orange);
+col;
if (maxTreble > 0.0f)
layer->drawLine(col, layer->size.y - 1, col, layer->size.y - 1 - layer->size.y * sharedData.trebleLevel / maxTreble, CRGB::Green);
+col;
if (maxVocal > 0.0f)
layer->drawLine(col, layer->size.y - 1, col, layer->size.y - 1 - layer->size.y * sharedData.vocalConfidence / maxVocal, CRGB::Blue);
+col;
layer->drawLine(col, layer->size.y - 1, col, layer->size.y - 1 - layer->size.y * beatLevel / 255, CRGB::Purple);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/MoonLight/Nodes/Effects/E_FastLED.h` around lines 68 - 73, The drawLine
calls in E_FastLED.h pass columnNr and columnNr++ in the same expression which
causes undefined/unspecified behavior; fix each call by separating the increment
from the function call so x1 and x2 are deterministic (e.g., call
layer->drawLine(columnNr, ..., columnNr, ...) and then increment columnNr on the
next line), updating the four drawLine invocations that use columnNr/columnNr++
to ensure columnNr is only read or written once per expression.


// correct for lower output
if (maxBass > 0) maxBass -= 0.01;
if (maxMid > 0) maxMid -= 0.01;
if (maxTreble > 0) maxTreble -= 0.01;
if (maxVocal > 0) maxVocal -= 0.01;
Comment on lines +59 to +79
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Division by zero when max levels are zero, and decay undershoots zero.

Two related issues:

  1. Divide by zero: All four maxX values start at 0.0f and are only increased when sharedData.xLevel > maxX. While the audio driver hasn't delivered any signal yet (or when all levels remain 0.0f because the driver skips values ≤ 0.01), maxBass == 0, and drawLine(…, … * sharedData.bassLevel / maxBass, …) produces NaN or ±∞ as a float coordinate. Casting that to an integer (for the LED index) is undefined behaviour, and on Xtensa/ESP32 typically resolves to INT_MIN or 0 — either an out-of-bounds LED write or a silent visual glitch.

  2. Decay undershoots zero: if (maxBass > 0) maxBass -= 0.01f will produce a small negative value (e.g. maxBass = 0.005fmaxBass = -0.005f) when the step is larger than the remaining value. On the very next iteration, sharedData.bassLevel (0.0f) > -0.005f is true, so maxBass is set to 0.0f and the divide-by-zero recurs.

🐛 Proposed fix
-    if (sharedData.bassLevel > maxBass) maxBass = sharedData.bassLevel;
-    if (sharedData.midLevel > maxMid) maxMid = sharedData.midLevel;
-    if (sharedData.trebleLevel > maxTreble) maxTreble = sharedData.trebleLevel;
-    if (sharedData.vocalConfidence > maxVocal) maxVocal = sharedData.vocalConfidence;
+    if (sharedData.bassLevel > maxBass)   maxBass   = sharedData.bassLevel;
+    if (sharedData.midLevel > maxMid)     maxMid    = sharedData.midLevel;
+    if (sharedData.trebleLevel > maxTreble) maxTreble = sharedData.trebleLevel;
+    if (sharedData.vocalConfidence > maxVocal) maxVocal = sharedData.vocalConfidence;

     ...

-    if (maxBass > 0) maxBass -= 0.01;
-    if (maxMid > 0) maxMid -= 0.01;
-    if (maxTreble > 0) maxTreble -= 0.01;
-    if (maxVocal > 0) maxVocal -= 0.01;
+    maxBass   = fmaxf(0.0f, maxBass   - 0.01f);
+    maxMid    = fmaxf(0.0f, maxMid    - 0.01f);
+    maxTreble = fmaxf(0.0f, maxTreble - 0.01f);
+    maxVocal  = fmaxf(0.0f, maxVocal  - 0.01f);

And guard the drawLine calls:

+    if (maxBass > 0.0f)
     layer->drawLine(columnNr, layer->size.y - 1, columnNr, layer->size.y - 1 - layer->size.y * sharedData.bassLevel / maxBass, CRGB::Red);
+    columnNr++;

(apply the same pattern to mid, treble, vocal)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/MoonLight/Nodes/Effects/E_FastLED.h` around lines 59 - 79, The drawLine
calculations can divide by zero because maxBass/maxMid/maxTreble/maxVocal may be
0 and the decay can undershoot below 0; fix by clamping the maxima and decays:
before each drawLine that uses e.g. maxBass, ensure a safeDenom = (maxBass >
0.0001f) ? maxBass : 1.0f (or skip drawing when both sharedData.bassLevel and
maxBass are 0), and replace uses of "maxX -= 0.01" with "maxX = max(0.0f, maxX -
0.01f)"; apply the same pattern for maxMid, maxTreble, maxVocal and use those
safeDenom variables in the layer->drawLine calls referenced in this block.


// beat delay
if (beatLevel) beatLevel -= MIN(255/layer->size.y, beatLevel);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Minor: integer division by zero if layer->size.y == 0.

255 / layer->size.y is integer division and will trigger a hardware exception on Xtensa if size.y is ever zero. Add a guard or substitute the formula.

🛡️ Proposed fix
-    if (beatLevel) beatLevel -= MIN(255/layer->size.y, beatLevel);
+    if (beatLevel && layer->size.y > 0)
+      beatLevel -= MIN(255 / layer->size.y, beatLevel);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/MoonLight/Nodes/Effects/E_FastLED.h` at line 82, Guard against
division-by-zero when computing the decrement for beatLevel: ensure
layer->size.y is not zero before using it in 255 / layer->size.y (for example
compute an integer divisor like int divisor = layer->size.y ? layer->size.y : 1;
or use a conditional that substitutes a safe value) and then use that safe
divisor in the MIN expression that updates beatLevel (referencing beatLevel and
layer->size.y in the existing line). This prevents a hardware exception on
Xtensa while preserving the original behavior when size.y > 0.

}
};

#endif
10 changes: 7 additions & 3 deletions src/MoonLight/Nodes/Effects/E_WLED.h
Original file line number Diff line number Diff line change
Expand Up @@ -3560,14 +3560,18 @@ class WaterfallEffect : public Node {
uint8_t pixCol = (log10f(myMajorPeak) - 2.26f) * 150;
if (myMajorPeak < 182.0f) pixCol = 0;

for (int i = 0; i < layer->nrOfLights - 1; i++) layer->setRGB(i, layer->getRGB(i + 1));
for (int x = 0; x < layer->size.x; x++)
for (int z = 0; z < layer->size.z; z++)
for (int y = 0; y < layer->size.y - 1; y++) layer->setRGB(Coord3D(x, y, z), layer->getRGB(Coord3D(x, y + 1, z)));

bool peak = sharedData.volume > 128.0f;
if (peak) {
layer->setRGB(layer->nrOfLights - 1, CHSV(92, 92, 92));
for (int x = 0; x < layer->size.x; x++)
for (int z = 0; z < layer->size.z; z++) layer->setRGB(Coord3D(x, layer->size.y - 1, z), CHSV(92, 92, 92));
} else {
CRGB color = ColorFromPalette(layerP.palette, pixCol + intensity, 127 + myMagnitude / 2.0);
layer->setRGB(layer->nrOfLights - 1, color); // blend(layerP.color2, color, (int)myMagnitude));
for (int x = 0; x < layer->size.x; x++)
for (int z = 0; z < layer->size.z; z++) layer->setRGB(Coord3D(x, layer->size.y - 1, z), color); // blend(layerP.color2, color, (int)myMagnitude));
}
}
}
Expand Down
Loading