Conversation
Backend ======= - Add D_FastLEDAudio.h Add FastLED audio variables to SharedData - WIP - Add Tubes Layout - Add FastLEDAudioDriver - Add FLAudioEffect
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ✏️ Tip: You can disable in-progress messages and the fortune message in your review settings. Tip CodeRabbit can scan for known vulnerabilities in your dependencies using OSV Scanner.OSV Scanner will automatically detect and report security vulnerabilities in your project's dependencies. No additional configuration is required. WalkthroughAdds FastLED audio support: new I2S-based FastLEDAudioDriver that populates new shared audio fields, a reactive FLAudioEffect, a TubesLayout, registration updates, minor config bumps, and small refactors in existing FastLED driver/effects and main startup behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant I2S as I2S Audio HW
participant AudioDrv as FastLEDAudioDriver
participant Proc as AudioProcessor
participant Shared as SharedData
participant Effect as FLAudioEffect
participant LEDs as LED Renderer
I2S->>AudioDrv: provide audio samples
AudioDrv->>Proc: feed samples
Proc->>Proc: detect beat, vocals, freq bands
Proc->>Shared: set beat, vocalsActive, vocalConfidence, bassLevel, trebleLevel
Effect->>Shared: read audio fields
alt vocalsActive
Effect->>Effect: run vocal-driven rainbow animation
else beat
Effect->>Effect: trigger beat visuals, set beatLevel
else
Effect->>Effect: idle/random noise rendering
end
Effect->>LEDs: render pixels
AudioDrv->>Shared: clear transient beat flag
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
src/MoonLight/Nodes/Drivers/D_FastLEDAudio.h (4)
39-42: Unused member variablescurrentBPMandonsetCount.
currentBPMandonsetCountare declared but never read or written anywhere in this class. If they're planned for future use, consider removing them until needed to keep the interface clean (YAGNI).🤖 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 39 - 42, The members currentBPM (float) and onsetCount (uint32_t) in the D_FastLEDAudio class are declared but never used; remove these two unused member declarations from the class in D_FastLEDAudio.h to keep the interface clean (or, if they are intentionally planned, add a clear TODO comment and minimal read/write usage to avoid unused-variable warnings). Locate the declarations named currentBPM and onsetCount in the D_FastLEDAudio class and delete them (or replace with a commented TODO) and rebuild to ensure no references remain.
21-28: Hardcoded I2S pins and static globals in a header file.Two concerns:
Pin configuration: The
#definemacros for I2S pins (6, 4, 5) are hardcoded and will silently shadow or collide with any same-named macros elsewhere. Consider making these configurable (e.g., viaonUpdate/UI controls, as done for other drivers) or at minimum wrapping them in#ifndefguards.Static globals in a header:
i2sConfig,config, andaudioInputarestaticat file scope — each translation unit including this header gets its own copy. If this header is included from multiple.cppfiles, you'll have duplicated state and theaudioInputshared_ptr won't actually be shared. If single-TU inclusion is guaranteed by the build, this works but is fragile.Consider moving the static variables inside the class (as
static inlinemembers or into the.cppfile), and making pin assignments configurable or at least guarded:- `#define` I2S_CLK_PIN 6 - `#define` I2S_WS_PIN 4 - `#define` I2S_SD_PIN 5 - `#define` I2S_CHANNEL fl::Left - -static fl::AudioConfigI2S i2sConfig(I2S_WS_PIN, I2S_SD_PIN, I2S_CLK_PIN, 0, I2S_CHANNEL, 44100, 16, fl::Philips); -static fl::AudioConfig config(i2sConfig); -static fl::shared_ptr<fl::IAudioInput> 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` around lines 21 - 28, The header hardcodes I2S_PIN macros (I2S_CLK_PIN, I2S_WS_PIN, I2S_SD_PIN) and declares file-scope static objects (i2sConfig, config, audioInput) which cause macro collisions and duplicated state across TUs; change the pin defines to be configurable or at minimum wrap them with `#ifndef` guards (so existing builds can override them) and move i2sConfig, config, and audioInput out of the header into the implementation (or declare them as static inline class members) so there is a single shared instance — update the driver class to accept pin/runtime config (e.g., via onUpdate/UI controls) and instantiate the AudioConfigI2S and shared_ptr<fl::IAudioInput> in the .cpp or as inline static members instead of file-scope statics in the header.
51-83: Callback capture lists are inconsistent — consider uniformity.
onBeat,onVocalStart,onVocalEndcapture[&](by reference), which capturesthisand all locals. These callbacks only accesssharedData(global) and member variables (beatCount,lastBeatTime).onVocalConfidence,onBass,onTrebleuse[](no capture) and only accesssharedData(global).The
[&]captures are safe as long as theFastLEDAudioDriverinstance outlivesaudioProcessor(which it does since it's a member). However, mixing capture styles is slightly inconsistent. TheonVocalStart/onVocalEndlambdas don't need[&]since they only touch the globalsharedData.🤖 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 51 - 83, The callbacks use mixed capture styles; make them explicit and consistent: change audioProcessor.onBeat's lambda to capture [this] (it modifies member variables beatCount and lastBeatTime) and keep audioProcessor.onVocalStart / onVocalEnd as [this] if they will ever touch members (or change them to [] if they only touch the global sharedData), and leave audioProcessor.onVocalConfidence, onBass, onTreble as [] (no capture) since they only use the global sharedData; update the capture lists for the lambdas referenced by onBeat, onVocalStart, onVocalEnd, onVocalConfidence, onBass, and onTreble accordingly so each lambda explicitly captures only what it needs.
94-94: Stop audio input in destructor to release hardware resources.The
audioInputstatic shared_ptr is started insetup()but never stopped. When the driver is destroyed, the I2S hardware resource remains active. CallaudioInput->stop()(designed per FastLED API for this purpose) to properly release the resource:♻️ Proposed fix
- ~FastLEDAudioDriver() override {} + ~FastLEDAudioDriver() override { + if (audioInput) { + audioInput->stop(); + } + }🤖 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 94, The destructor ~FastLEDAudioDriver() currently does nothing but must stop the shared audioInput resource started in setup(); update the destructor for FastLEDAudioDriver to check the static std::shared_ptr audioInput and call audioInput->stop() (and optionally reset the pointer) to release the I2S hardware resource; locate the static member audioInput and the setup() method for context and ensure the destructor calls audioInput->stop() only if the pointer is non-null.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@platformio.ini`:
- Around line 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.
In `@src/MoonLight/Nodes/Drivers/D_FastLEDAudio.h`:
- Around line 47-49: Check the result of fl::IAudioInput::create by verifying
audioInput and errorMsg before calling audioInput->start(); if create fails, log
the descriptive errorMsg (or a fallback message) and avoid calling start so you
don't dereference a null pointer. Add a null guard in the class's loop20ms()
(e.g., if (!audioInput) return;) to skip audio processing when audioInput wasn't
created. Ensure all locations that use audioInput (construction/start and
loop20ms) handle the null case consistently and do not assume successful
creation.
- Around line 86-92: The beat flag is being cleared at the end of loop20ms(), so
any onBeat-set sharedData.beat is erased before FLAudioEffect::loop() can
observe it; move the sharedData.beat = false reset to the start of loop20ms()
(before the while loop that calls audioInput->read() and
audioProcessor.update(sample)) so the flag can be set by the onBeat callback
during processing and persist until the next cycle.
---
Nitpick comments:
In `@src/MoonLight/Nodes/Drivers/D_FastLEDAudio.h`:
- Around line 39-42: The members currentBPM (float) and onsetCount (uint32_t) in
the D_FastLEDAudio class are declared but never used; remove these two unused
member declarations from the class in D_FastLEDAudio.h to keep the interface
clean (or, if they are intentionally planned, add a clear TODO comment and
minimal read/write usage to avoid unused-variable warnings). Locate the
declarations named currentBPM and onsetCount in the D_FastLEDAudio class and
delete them (or replace with a commented TODO) and rebuild to ensure no
references remain.
- Around line 21-28: The header hardcodes I2S_PIN macros (I2S_CLK_PIN,
I2S_WS_PIN, I2S_SD_PIN) and declares file-scope static objects (i2sConfig,
config, audioInput) which cause macro collisions and duplicated state across
TUs; change the pin defines to be configurable or at minimum wrap them with
`#ifndef` guards (so existing builds can override them) and move i2sConfig,
config, and audioInput out of the header into the implementation (or declare
them as static inline class members) so there is a single shared instance —
update the driver class to accept pin/runtime config (e.g., via onUpdate/UI
controls) and instantiate the AudioConfigI2S and shared_ptr<fl::IAudioInput> in
the .cpp or as inline static members instead of file-scope statics in the
header.
- Around line 51-83: The callbacks use mixed capture styles; make them explicit
and consistent: change audioProcessor.onBeat's lambda to capture [this] (it
modifies member variables beatCount and lastBeatTime) and keep
audioProcessor.onVocalStart / onVocalEnd as [this] if they will ever touch
members (or change them to [] if they only touch the global sharedData), and
leave audioProcessor.onVocalConfidence, onBass, onTreble as [] (no capture)
since they only use the global sharedData; update the capture lists for the
lambdas referenced by onBeat, onVocalStart, onVocalEnd, onVocalConfidence,
onBass, and onTreble accordingly so each lambda explicitly captures only what it
needs.
- Line 94: The destructor ~FastLEDAudioDriver() currently does nothing but must
stop the shared audioInput resource started in setup(); update the destructor
for FastLEDAudioDriver to check the static std::shared_ptr audioInput and call
audioInput->stop() (and optionally reset the pointer) to release the I2S
hardware resource; locate the static member audioInput and the setup() method
for context and ensure the destructor calls audioInput->stop() only if the
pointer is non-null.
| -D FASTLED_BUILD=\"20260222\" | ||
| lib_deps = | ||
| https://github.com/FastLED/FastLED#9d0b0eb9b5e59e4093982e0c2bdcfdff72ca80cb ; master 20260221 | ||
| https://github.com/FastLED/FastLED#27c99130c83ab666c4c6dbf389425fec5f27db05 ; master 20260222 |
There was a problem hiding this comment.
🧩 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 -50Repository: 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 cppRepository: 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 iniRepository: 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.iniRepository: 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.iniRepository: 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 -20Repository: 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.iniRepository: 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.
There was a problem hiding this comment.
Not true on newest FastLED
There was a problem hiding this comment.
🧩 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]
- Force legacy RMT4 with
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.
There was a problem hiding this comment.
Yes remove it please
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
⚠️ 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 -50Repository: 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 cppRepository: 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 iniRepository: 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.iniRepository: 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.iniRepository: 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 -20Repository: 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.iniRepository: MoonModules/MoonLight
Length of output: 3009
Add
-D FASTLED_RMT5=0to 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=0to 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.
There was a problem hiding this comment.
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!
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/MoonBase/Nodes.h (1)
350-356: Optional: prefer0.0fliterals for float member initializers.
float vocalConfidence = 0,float bassLevel = 0, andfloat trebleLevel = 0are valid C++ but using integer literals for float members is less explicit than0.0f.✨ Proposed style fix
- float vocalConfidence = 0; - float bassLevel = 0; - float trebleLevel = 0; + float vocalConfidence = 0.0f; + float bassLevel = 0.0f; + float trebleLevel = 0.0f;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/MoonBase/Nodes.h` around lines 350 - 356, Update the float member initializers to use explicit float literals: change the initializers for vocalConfidence, bassLevel, and trebleLevel from integer zero (e.g. = 0) to 0.0f; keep the boolean members vocalsActive and beat unchanged. This ensures types are explicit for members vocalConfidence, bassLevel, and trebleLevel in the class/struct where they are declared.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/MoonBase/Nodes.h`:
- Around line 350-356: Update the float member initializers to use explicit
float literals: change the initializers for vocalConfidence, bassLevel, and
trebleLevel from integer zero (e.g. = 0) to 0.0f; keep the boolean members
vocalsActive and beat unchanged. This ensures types are explicit for members
vocalConfidence, bassLevel, and trebleLevel in the class/struct where they are
declared.
|
@coderabbitai, can you verify this issue FastLED/FastLED#2170 and compare it with the commits in this PR and come with a review summary and propose follow up actions to do in this PR |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🌐 Web query: 💡 Result: FastLED/FastLED Issue Source: [1] 🏁 Script executed: Length of output: 10286
FastLED Issue
|
|
@coderabbitai, regarding
How can I bring these in the FastLEDAudioDriver class: create them in setup() and destroy in the destructor |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 2830
Recommended Approach (using pointers)class FastLEDAudioDriver : public Node {
private:
// Member variables for audio configuration
fl::unique_ptr<fl::AudioConfigI2S> i2sConfig;
fl::unique_ptr<fl::AudioConfig> config;
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;
void setup() override {
Node::setup(); // !!
// Create configuration objects
i2sConfig = fl::make_unique<fl::AudioConfigI2S>(
I2S_WS_PIN, I2S_SD_PIN, I2S_CLK_PIN, 0,
I2S_CHANNEL, 44100, 16, fl::Philips);
config = fl::make_unique<fl::AudioConfig>(*i2sConfig);
// Create audio input using the config
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();
// ... rest of your setup code (callbacks, etc.)
}
void loop20ms() override {
if (!audioInput) return;
sharedData.beat = false;
while (fl::AudioSample sample = audioInput->read()) {
audioProcessor.update(sample);
}
}
~FastLEDAudioDriver() override {
if (audioInput) {
audioInput->stop();
}
// unique_ptr will automatically clean up i2sConfig and config
}
};Alternative: If
|
FastLED Audio driver - Move static variables inside class - add controls (gain etc) - add onUpdate to set controls - add readPins to get I2S pins from IO module - add initAudio and stopAudio - destructor FLAudio effect - add Mid
2 participants
Summary by CodeRabbit
New Features
Improvements