Conversation
- pio.ini: include latest FastLED - memcpy: (void *) on CRGB - CRGB + CRGB not defined (replaced by += workaround) - MAX -> max - D_FastLED.h: remove pun 19 and 20 from CONFIG_IDF_TARGET_ESP32S3
WalkthroughUpdated build flags and FastLED dependency; replaced several C-style Changes
Sequence Diagram(s)sequenceDiagram
participant Init as InitRoutine
participant Layout as LayoutBuilder
participant ChannelCfg as ChannelConfigFactory
participant Engine as EngineSelector
participant FastLED as FastLED_Manager
participant Logger as DiagnosticsLogger
Init->>Layout: compute layout & targets
Layout->>ChannelCfg: build timing/config per target (pin, timing, span)
ChannelCfg->>Engine: select engine (I2S / PARLIO) based on platform
Engine-->>ChannelCfg: return engine instance
ChannelCfg->>FastLED: create channel & FastLED.add(channel)
FastLED->>FastLED: register driver, set exclusivity
FastLED->>Logger: report driver count/info
Logger-->>Init: log driver diagnostics
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/MoonLight/Nodes/Drivers/D_FastLED.h (1)
158-166:⚠️ Potential issue | 🔴 CriticalRemove unguarded GPIO11 cases—not RMT5-specific but a universal hardware conflict on ESP32.
GPIO11 (along with GPIO6–10) is part of the on-module SPI flash bus on standard ESP32 variants and cannot be used for general GPIO. The template instantiation of
addLeds<..., 11>will fail at compile time regardless of build profile. Remove or comment out the GPIO11 cases entirely (as already done for GPIO6–10 in other sections of this file) rather than guarding by RMT5. The constraint is hardware-level, not FastLED RMT5-specific.Suggested fix: remove or comment out GPIO11 cases
- case 11: - FastLED.addLeds<ML_CHIPSET, 11 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; - break; + // case 11: FastLED.addLeds<ML_CHIPSET, 11 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; break; // 11-17 reserved for SPI FLASH
🤖 Fix all issues with AI agents
In `@src/MoonLight/Layers/VirtualLayer.cpp`:
- Line 293: The use of max(nrOfLights, indexV + 1) causes a template deduction
error due to mixed types (indexV+1 is int); fix by casting the second operand to
the same type as nrOfLights (nrOfLights_t) so the overload resolves, e.g.
replace with nrOfLights = max(nrOfLights, static_cast<nrOfLights_t>(indexV +
1)); ensure this change is applied where nrOfLights, indexV and nrOfLights_t are
used together (VirtualLayer.cpp).
In `@src/MoonLight/Nodes/Effects/E_MoonLight.h`:
- Line 1613: The setup() override builds the control range using layer->size but
mistakenly uses layer->size.x three times, shrinking bounds for non‑cubic
layouts; update the max expression in the addControl call (in setup) to compute
the maximum of layer->size.x, layer->size.y and layer->size.z (e.g., nested max
of x, y, then z) and subtract 1 as before so the pos control uses the true
largest dimension.
|
@coderabbitai, how on earth do I get Stack canary watchpoint triggered (AppDrivers) when the stack is 1820 bytes just before the exception? [ 4478][D][PhysicalLayer.cpp:248] onLayoutPost(): [💫] AppDrivers (1820) pass 1 mp:0 #:256 / 768 s:16,16,1 Backtrace: 0x40384f6c:0x3fcc3c00 0x40384ee1:0x3fcc3c30 0x40376da3:0x3fcc3c50 0x40376dbd:0x3fcc3c80 0x40376f7c:0x3fcc3ca0 0x403862dd:0x3fcc3cc0 0x42006866:0x3fcc3ce0 0x42005065:0x3fcc3d00 0x42008096:0x3fcc3d20 0x420081ca:0x3fcc3dc0 0x42016b82:0x3fcc3e30 0x42016f5b:0x3fcc3eb0 0x42018485:0x3fcc3fd0 0x420188d1:0x3fcc41a0 0x4201b0fd:0x3fcc42b0 0x4201b386:0x3fcc4750 0x4200b07a:0x3fcc4780 0x4200b135:0x3fcc4810 0x42197b9d:0x3fcc4840 0x42005535:0x3fcc4860 0x4200b2fd:0x3fcc4880 0x420038ce:0x3fcc4940 0x420039d5:0x3fcc4960 0x42089645:0x3fcc4980 0x420855f1:0x3fcc49f0 0x42088db0:0x3fcc4a40 0x40380119:0x3fcc4a80 |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 3594 🏁 Script executed: Length of output: 543 🏁 Script executed: Length of output: 5702
The ProblemFrom platformio.ini (line 152): -D DRIVERS_STACK_SIZE=4096Your AppDrivers task has only 4KB of stack, but FastLED 3.11 needs significantly more than 3.10.3 did. Understanding the 1820 Bytes "Remaining"The confusing part: ESP-IDF places guard bytes (canaries) at the stack boundaries. When your code writes past the stack limit, it overwrites these canaries. The watchpoint doesn't fire immediately—it fires later when malloc/free checks the canaries during heap operations. In your case:
Why FastLED 3.11 BreaksThe backtrace reveals the culprit:
FastLED 3.11's SPI channel engine is much more complex than the old RMT-only driver, with deeper call stacks and heap allocations for string operations. SolutionIncrease the drivers stack size in platformio.ini: -D DRIVERS_STACK_SIZE=8192 ; was 4096 - FastLED 3.11 needs more for ChannelEngineSpiFor comparison, you already have:
Given the complexity of FastLED 3.11's channel management and the 25-frame backtrace, 8192 bytes (8KB) is appropriate for the driver task. 🧠 Learnings used |
|
@coderabbitai, I am in the process of replacing the FastLED RMT drivers with the new parallel IO drivers, but still on the RMT Drivers I get dramatic low performance of 2 FPS. And I get this in the logging: [ 30198][D][isr_esp32_idf5.hpp:355] detach_handler(): [fl_isr_esp32] Handler detached Can you check what is going on? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 31743 🌐 Web query: 💡 Result: This “ISR handler detached” / RMT-ISR weirdness on ESP32 with FastLED 3.11 is almost always caused by two different RMT stacks getting initialized in the same firmware (legacy RMT APIs vs the newer IDF RMT/ Key points behind it:
What to do (practical fixes):
Sources: ESP-IDF RMT interrupt handler restrictions [1], FastLED’s “RMT4 and RMT5 cannot coexist” warning and how to force RMT5/RMT4 [2]. [1] (docs.espressif.com) Citations:
🌐 Web query: 💡 Result: I wasn’t able to retrieve the actual diff / commit message for commit What I can confirm from FastLED’s current docs/issues is what the RMT-related change-set in this era is about:
If you paste the commit’s diff (or link the commit URL), I can tell you precisely what Sources: [1] FastLED repo README/docs (ESP32 RMT driver configuration) (github.com), [2] FastLED issue Citations:
🏁 Script executed: Length of output: 3685 🏁 Script executed: Length of output: 4607
The ProblemOn ESP-IDF 5.x+ (your platform 55.03.35), FastLED 3.11 defaults to RMT5, which wraps Espressif's Your platformio.ini (line 52) uses: platform = https://github.com/pioarduino/platform-espressif32/releases/download/55.03.35/platform-espressif32.zipThis is ESP-IDF v5.5.x / Arduino v3.3.5, so FastLED automatically selects RMT5. Why It's BreakingRMT5 is more prone to Wi-Fi/interrupt jitter than the legacy RMT4 implementation, and the repeated ISR detachment suggests FastLED's RMT5 driver is continuously deallocating/reallocating resources—destroying performance. Solution: Force Legacy RMT4 DriverAdd this build flag to the [moonlight]
build_flags =
-D FT_MOONLIGHT=1
-D FT_MONITOR=1
-D EFFECTS_STACK_SIZE=3072
-D DRIVERS_STACK_SIZE=4096
-D FASTLED_RMT5=0 ; ← Force legacy RMT4 driver (more stable than RMT5 on IDF 5.x+)
; FastLED pre-compiled settings:
-D ML_CHIPSET=WS2812B
-D ML_COLOR_ORDER=GRBThis forces FastLED to use the legacy RMT4 driver instead of RMT5, avoiding the ISR handler issues. Alternative: Switch to Parallel I/OSince you mentioned transitioning to parallel I/O drivers, that's an even better long-term solution—parallel I/O doesn't use RMT at all, avoiding these conflicts entirely. Note: You may also want to increase ✏️ Learnings added
🧠 Learnings used |
- pio.ini: increase httpd stack 5120->6144 and drivers stack from 4096 to 5120 - FastLED: remove gpio 11 for esp32-do (part of spi flash)
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/MoonLight/Nodes/Drivers/D_FastLED.h`:
- Around line 127-131: The FastLED path misinterprets MoonLight's
4-byte-per-light buffers (layerP.lights.channelsD) as CRGB (3-byte) causing
misaligned pixels; update the FastLED setup around fl::ChannelConfig /
fl::makeTimingConfig usage to first check layerP.lights.header.channelsPerLight
and only use CRGB/fastled when it's 3, otherwise perform RGB repacking like
parlio.cpp (convert 4-byte RGBW to contiguous 3-byte RGB for the LEDs, call
FastLED.show(), then restore or use a temporary buffer), ensuring startLed
indexing into CRGB uses the repacked buffer so the white channel no longer
overwrites the next pixel.
- Around line 133-145: The FastLED exclusive driver is being set after creating
the channel which doesn't affect the already-instantiated engine; move the
FastLED.setExclusiveDriver(...) call to before creating the channel and
instantiate the channel with the explicit engine templates (use
fl::Channel::create<fl::ChannelEngineSpi>(config) on targets that need SPI and
fl::Channel::create<fl::ChannelEnginePARLIO>(config) for PARLIO) so the correct
engine is chosen at creation time; then call FastLED.addLedChannel(channel) as
before.
🧹 Nitpick comments (2)
src/MoonLight/Nodes/Drivers/D_FastLED.h (2)
16-22: Avoid relying on transitive includes for channel APIs.
These commented includes are either dead code or a hint that explicit channel headers are needed. If you intend to use explicit engines (SPI/PARLIO), re‑enable the required headers; otherwise delete the commented block to reduce clutter.
151-156: Cache driver infos once to avoid repeated allocations.
FastLED.getDriverInfos()is called on every loop iteration; if it allocates or builds a temporary list, this is wasteful. Cache the result once outside the loop and iterate it.Possible cleanup
- for (size_t i = 0; i < FastLED.getDriverCount(); i++) { - auto drivers = FastLED.getDriverInfos(); - auto& info = drivers[i]; + auto drivers = FastLED.getDriverInfos(); + for (size_t i = 0; i < drivers.size(); i++) { + auto& info = drivers[i]; EXT_LOGD(ML_TAG, "Driver: %s, Priority: %d, Enabled: %s", info.name.c_str(), info.priority, info.enabled ? "yes" : "no"); }
| //https://github.com/FastLED/FastLED/blob/master/src/fl/channels/README.md | ||
|
|
||
| #if CONFIG_IDF_TARGET_ESP32S2 | ||
| case 0: | ||
| FastLED.addLeds<ML_CHIPSET, 0 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| case 1: | ||
| FastLED.addLeds<ML_CHIPSET, 1 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| case 2: | ||
| FastLED.addLeds<ML_CHIPSET, 2 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| case 3: | ||
| FastLED.addLeds<ML_CHIPSET, 3 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| case 4: | ||
| FastLED.addLeds<ML_CHIPSET, 4 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| case 5: | ||
| FastLED.addLeds<ML_CHIPSET, 5 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| case 6: | ||
| FastLED.addLeds<ML_CHIPSET, 6 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| case 7: | ||
| FastLED.addLeds<ML_CHIPSET, 7 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| case 8: | ||
| FastLED.addLeds<ML_CHIPSET, 8 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| case 9: | ||
| FastLED.addLeds<ML_CHIPSET, 9 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| case 10: | ||
| FastLED.addLeds<ML_CHIPSET, 10 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| case 11: | ||
| FastLED.addLeds<ML_CHIPSET, 11 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| case 12: | ||
| FastLED.addLeds<ML_CHIPSET, 12 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| case 13: | ||
| FastLED.addLeds<ML_CHIPSET, 13 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| case 14: | ||
| FastLED.addLeds<ML_CHIPSET, 14 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| case 15: | ||
| FastLED.addLeds<ML_CHIPSET, 15 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| case 16: | ||
| FastLED.addLeds<ML_CHIPSET, 16 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| case 17: | ||
| FastLED.addLeds<ML_CHIPSET, 17 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| case 18: | ||
| FastLED.addLeds<ML_CHIPSET, 18 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| #if !ARDUINO_USB_CDC_ON_BOOT | ||
| // 19 + 20 = USB HWCDC. reserved for USB port when ARDUINO_USB_CDC_ON_BOOT=1 | ||
| case 19: | ||
| FastLED.addLeds<ML_CHIPSET, 19 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| case 20: | ||
| FastLED.addLeds<ML_CHIPSET, 20 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| #endif | ||
| case 21: | ||
| FastLED.addLeds<ML_CHIPSET, 21 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| // 22 to 32: not connected, or reserved for SPI FLASH | ||
| // case 22: FastLED.addLeds<ML_CHIPSET, 22 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; break; | ||
| // case 23: FastLED.addLeds<ML_CHIPSET, 23 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; break; | ||
| // case 24: FastLED.addLeds<ML_CHIPSET, 24 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; break; | ||
| // case 25: FastLED.addLeds<ML_CHIPSET, 25 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; break; | ||
| #if !defined(BOARD_HAS_PSRAM) | ||
| // 26-32 = reserved for PSRAM | ||
| case 26: | ||
| FastLED.addLeds<ML_CHIPSET, 26 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| // case 27: FastLED.addLeds<ML_CHIPSET, 27 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; break; | ||
| // case 28: FastLED.addLeds<ML_CHIPSET, 28 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; break; | ||
| // case 29: FastLED.addLeds<ML_CHIPSET, 29 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; break; | ||
| // case 30: FastLED.addLeds<ML_CHIPSET, 30 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; break; | ||
| // case 31: FastLED.addLeds<ML_CHIPSET, 31 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; break; | ||
| // case 32: FastLED.addLeds<ML_CHIPSET, 32 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; break; | ||
| #endif | ||
| case 33: | ||
| FastLED.addLeds<ML_CHIPSET, 33 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| case 34: | ||
| FastLED.addLeds<ML_CHIPSET, 34 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| case 35: | ||
| FastLED.addLeds<ML_CHIPSET, 35 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| case 36: | ||
| FastLED.addLeds<ML_CHIPSET, 36 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| case 37: | ||
| FastLED.addLeds<ML_CHIPSET, 37 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| case 38: | ||
| FastLED.addLeds<ML_CHIPSET, 38 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| case 39: | ||
| FastLED.addLeds<ML_CHIPSET, 39 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| case 40: | ||
| FastLED.addLeds<ML_CHIPSET, 40 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| case 41: | ||
| FastLED.addLeds<ML_CHIPSET, 41 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| case 42: | ||
| FastLED.addLeds<ML_CHIPSET, 42 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| case 43: | ||
| FastLED.addLeds<ML_CHIPSET, 43 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| case 44: | ||
| FastLED.addLeds<ML_CHIPSET, 44 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| case 45: | ||
| FastLED.addLeds<ML_CHIPSET, 45 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; | ||
| break; | ||
| // 46 input-only | ||
| // case 46: FastLED.addLeds<ML_CHIPSET, 46 COLOR_ORDER_ARG>(leds, startLed, nrOfLights).setCorrection(TypicalLEDStrip) RGBW_CALL; break; | ||
| #endif // CONFIG_IDF_TARGET_ESP32S2 | ||
| fl::ChipsetTimingConfig timing = fl::makeTimingConfig<fl::TIMING_WS2812_800KHZ>(); | ||
| fl::ChannelConfig config(pins[pinIndex], timing, fl::span<CRGB>(&leds[startLed], layerP.ledsPerPin[pinIndex]), GRB); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "D_FastLED.h" -type fRepository: MoonModules/MoonLight
Length of output: 106
🏁 Script executed:
cat -n src/MoonLight/Nodes/Drivers/D_FastLED.h | head -150Repository: MoonModules/MoonLight
Length of output: 6778
🏁 Script executed:
cat -n src/MoonLight/Nodes/Drivers/D_FastLED.h | sed -n '115,150p'Repository: MoonModules/MoonLight
Length of output: 1722
🏁 Script executed:
rg "channelsPerLight|channelsD|RGBW" src/MoonLight/Nodes/Drivers/D_FastLED.h -A 3 -B 3Repository: MoonModules/MoonLight
Length of output: 263
🏁 Script executed:
rg "channelsPerLight" src/MoonLight/Layers/ -lRepository: MoonModules/MoonLight
Length of output: 214
🏁 Script executed:
grep -n "channelsPerLight" src/MoonLight/Layers/VirtualLayer.h | head -20Repository: MoonModules/MoonLight
Length of output: 2498
🏁 Script executed:
grep -n "channelsD\|offsetRGBW" src/MoonLight/Layers/VirtualLayer.h | head -20Repository: MoonModules/MoonLight
Length of output: 2574
🏁 Script executed:
sed -n '1,100p' src/MoonLight/Layers/VirtualLayer.h | cat -nRepository: MoonModules/MoonLight
Length of output: 3467
🏁 Script executed:
grep -n "channelsPerLight == 3\|channelsPerLight == 4\|if.*RGB\|if.*RGBW" src/MoonLight/Nodes/Drivers/D_FastLED.hRepository: MoonModules/MoonLight
Length of output: 47
🏁 Script executed:
grep -rn "repack\|FastLED.*RGB\|FastLED.*RGBW" src/MoonLight/Nodes/Drivers/ --include="*.h" --include="*.cpp"Repository: MoonModules/MoonLight
Length of output: 1253
🏁 Script executed:
sed -n '119,150p' src/MoonLight/Nodes/Drivers/D_FastLED.h | cat -nRepository: MoonModules/MoonLight
Length of output: 1596
🏁 Script executed:
grep -rn "channelsPerLight\s*=\s*4\|RGBW.*FastLED" src/ --include="*.h" --include="*.cpp"Repository: MoonModules/MoonLight
Length of output: 253
🏁 Script executed:
grep -rn "FastLED\|D_FastLED" src/ --include="*.cpp" | grep -i "driver\|init" | head -20Repository: MoonModules/MoonLight
Length of output: 47
🏁 Script executed:
find src -name "*.h" -exec grep -l "ML_CHANNELS_PER_LIGHT\|CHANNELS_PER_LIGHT" {} \;Repository: MoonModules/MoonLight
Length of output: 47
🏁 Script executed:
sed -n '445,475p' src/MoonBase/Nodes.cppRepository: MoonModules/MoonLight
Length of output: 925
🏁 Script executed:
sed -n '450,470p' src/MoonBase/Nodes.cpp | cat -nRepository: MoonModules/MoonLight
Length of output: 801
Gate FastLED to RGB-only devices or add buffer repacking for RGBW.
The cast (CRGB*)layerP.lights.channelsD misaligns RGBW data: MoonLight stores 4-byte RGBW buffers (R, G, B, W per light), but CRGB expects 3-byte elements. When startLed is calculated as light count and used to index CRGB pointers, the arithmetic is off—the white channel overwrites the next pixel's red.
MoonLight supports multiple RGBW presets (RGBW, GRBW, WRGB with channelsPerLight=4), so this path can be reached. Either add a runtime guard if (layerP.lights.header.channelsPerLight == 3) to skip FastLED for 4-channel devices, or repack the RGB data in-place before FastLED.show() then restore, following the pattern used in parlio.cpp.
🤖 Prompt for AI Agents
In `@src/MoonLight/Nodes/Drivers/D_FastLED.h` around lines 127 - 131, The FastLED
path misinterprets MoonLight's 4-byte-per-light buffers
(layerP.lights.channelsD) as CRGB (3-byte) causing misaligned pixels; update the
FastLED setup around fl::ChannelConfig / fl::makeTimingConfig usage to first
check layerP.lights.header.channelsPerLight and only use CRGB/fastled when it's
3, otherwise perform RGB repacking like parlio.cpp (convert 4-byte RGBW to
contiguous 3-byte RGB for the LEDs, call FastLED.show(), then restore or use a
temporary buffer), ensuring startLed indexing into CRGB uses the repacked buffer
so the white channel no longer overwrites the next pixel.
| #if CONFIG_IDF_TARGET_ESP32 || CONFIG_IDF_TARGET_ESP32S2 || CONFIG_IDF_TARGET_ESP32S3 | ||
| // fl::ChannelPtr channel = fl::Channel::create<fl::ChannelEngineSpi>(config); | ||
| fl::ChannelPtr channel = fl::Channel::create(config); | ||
| FastLED.setExclusiveDriver("SPI"); | ||
| #else | ||
| // auto channel = fl::Channel::create<fl::ChannelEnginePARLIO>(config); | ||
| fl::ChannelPtr channel = fl::Channel::create(config); | ||
| FastLED.setExclusiveDriver("PARLIO"); | ||
| #endif | ||
|
|
||
| default: | ||
| EXT_LOGW(ML_TAG, "FastLEDPin assignment: pin not supported %d", pins[pinIndex]); | ||
| } // switch pins[pinIndex] | ||
| // Register with FastLED | ||
| FastLED.addLedChannel(channel); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the full context of the D_FastLED.h file around the flagged lines
fd D_FastLED.hRepository: MoonModules/MoonLight
Length of output: 104
🏁 Script executed:
# Search for FastLED Channel::create calls and setExclusiveDriver usage in the codebase
rg "Channel::create|setExclusiveDriver" -t cpp -t h -B 2 -A 2Repository: MoonModules/MoonLight
Length of output: 1042
🌐 Web query:
FastLED 3.11 Channel::create exclusive driver behavior order of operations
💡 Result:
In FastLED’s new ESP32 “channels” architecture (3.10.x and newer, including 3.11), driver choice is evaluated at channel/engine creation time and (per the FastLED docs) any driver enable/exclusive changes are applied on the next FastLED.show(). So the practical order-of-operations is: configure driver policy → ensure channels/engines get (re)created under that policy → show(). [1]
What “exclusive driver” does
FastLED.setExclusiveDriver("RMT")(or"SPI","PARLIO", etc.) disables all other drivers (including future ones) and prevents the normal “try highest priority, then fall back” selection behavior. [1]- If the exclusive driver can’t allocate resources (channels/hosts/memory), you’ll generally see repeated warnings rather than a silent fallback to another driver (because you told FastLED not to use any other driver). This matches real-world reports when forcing drivers during channel creation. [2]
The order that matters (what to do in your sketch)
For deterministic behavior with Channel::create / channel allocation:
- Call
FastLED.setExclusiveDriver(...)insetup()before the firstFastLED.show()(best practice). - Then do your
FastLED.addLeds(...)registrations. - Only then call
FastLED.show()(first show is where the driver selection/allocation is guaranteed to be “locked in” under the exclusive policy). [1]
You can call setExclusiveDriver() after addLeds() as long as it’s still before the first show(), because the docs state the change takes effect on the next show(). [1]
If you change exclusivity after channels already exist
Because the policy applies on the “next show()” [1], switching exclusivity mid-run may or may not force a full teardown/recreate of already-created channel engines (that detail is implementation-specific and can vary by driver). If you need a hard guarantee, the reliable approach is: set exclusivity before the first show().
Sources: FastLED runtime driver control / exclusive driver semantics and “next show()” behavior [1]; community report of forcing a driver via setExclusiveDriver() and observing channel-engine allocation warnings/behavior [2].
Move FastLED.setExclusiveDriver() before channel creation and use explicit engine templates.
In FastLED 3.11, driver selection is locked at channel creation time. Setting the exclusive driver after instantiation does not retroactively change the already-created channel engine—it only affects the policy for the next show(). To guarantee SPI or PARLIO is selected from the start (avoiding the RMT5 default on ESP-IDF 5.x+), call setExclusiveDriver() first, then create the channel with an explicit engine template.
Suggested change
`#if` CONFIG_IDF_TARGET_ESP32 || CONFIG_IDF_TARGET_ESP32S2 || CONFIG_IDF_TARGET_ESP32S3
- fl::ChannelPtr channel = fl::Channel::create(config);
- FastLED.setExclusiveDriver("SPI");
+ FastLED.setExclusiveDriver("SPI");
+ fl::ChannelPtr channel = fl::Channel::create<fl::ChannelEngineSpi>(config);
`#else`
- fl::ChannelPtr channel = fl::Channel::create(config);
- FastLED.setExclusiveDriver("PARLIO");
+ FastLED.setExclusiveDriver("PARLIO");
+ fl::ChannelPtr channel = fl::Channel::create<fl::ChannelEnginePARLIO>(config);
`#endif`📝 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.
| #if CONFIG_IDF_TARGET_ESP32 || CONFIG_IDF_TARGET_ESP32S2 || CONFIG_IDF_TARGET_ESP32S3 | |
| // fl::ChannelPtr channel = fl::Channel::create<fl::ChannelEngineSpi>(config); | |
| fl::ChannelPtr channel = fl::Channel::create(config); | |
| FastLED.setExclusiveDriver("SPI"); | |
| #else | |
| // auto channel = fl::Channel::create<fl::ChannelEnginePARLIO>(config); | |
| fl::ChannelPtr channel = fl::Channel::create(config); | |
| FastLED.setExclusiveDriver("PARLIO"); | |
| #endif | |
| default: | |
| EXT_LOGW(ML_TAG, "FastLEDPin assignment: pin not supported %d", pins[pinIndex]); | |
| } // switch pins[pinIndex] | |
| // Register with FastLED | |
| FastLED.addLedChannel(channel); | |
| `#if` CONFIG_IDF_TARGET_ESP32 || CONFIG_IDF_TARGET_ESP32S2 || CONFIG_IDF_TARGET_ESP32S3 | |
| // fl::ChannelPtr channel = fl::Channel::create<fl::ChannelEngineSpi>(config); | |
| FastLED.setExclusiveDriver("SPI"); | |
| fl::ChannelPtr channel = fl::Channel::create<fl::ChannelEngineSpi>(config); | |
| `#else` | |
| // auto channel = fl::Channel::create<fl::ChannelEnginePARLIO>(config); | |
| FastLED.setExclusiveDriver("PARLIO"); | |
| fl::ChannelPtr channel = fl::Channel::create<fl::ChannelEnginePARLIO>(config); | |
| `#endif` | |
| // Register with FastLED | |
| FastLED.addLedChannel(channel); |
🤖 Prompt for AI Agents
In `@src/MoonLight/Nodes/Drivers/D_FastLED.h` around lines 133 - 145, The FastLED
exclusive driver is being set after creating the channel which doesn't affect
the already-instantiated engine; move the FastLED.setExclusiveDriver(...) call
to before creating the channel and instantiate the channel with the explicit
engine templates (use fl::Channel::create<fl::ChannelEngineSpi>(config) on
targets that need SPI and fl::Channel::create<fl::ChannelEnginePARLIO>(config)
for PARLIO) so the correct engine is chosen at creation time; then call
FastLED.addLedChannel(channel) as before.
pio.ini / esp32-S3.ini / D-FastLED.h: cleanup old FastLED directives, include latest FastLED Layouts: correct CarLightsLayout
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
platformio.ini (1)
148-156:⚠️ Potential issue | 🟠 MajorMissing
-D FASTLED_RMT5=0build flag to prevent RMT5 ISR issues.Per known issues with FastLED 3.11 on ESP-IDF 5.x, the RMT5 driver (default) causes repeated "Handler detached" messages and ~2 FPS performance. While the code now uses
setExclusiveDriver("I2S")/"PARLIO", adding-D FASTLED_RMT5=0as a belt-and-suspenders measure ensures RMT5 is compiled out entirely, preventing any accidental fallback.Proposed addition
[moonlight] build_flags = -D FT_MOONLIGHT=1 -D FT_MONITOR=1 -D EFFECTS_STACK_SIZE=3072 ; psramFound() ? 4 * 1024 : 3 * 1024 -D DRIVERS_STACK_SIZE=8192 ; FastLED 3.11 needs more stack + -D FASTLED_RMT5=0 ; Force legacy RMT4, avoid RMT5 ISR lifecycle issues on ESP-IDF 5.xBased on learnings: "FastLED defaults to RMT5 driver... which has known ISR handler lifecycle issues causing repeated 'Handler detached' messages every ~500ms and severe performance degradation (e.g., 2 FPS). Solution: force legacy RMT4 by adding
-D FASTLED_RMT5=0to build flags in platformio.ini."
🤖 Fix all issues with AI agents
In `@platformio.ini`:
- Line 152: The DRIVERS_STACK_SIZE value is a typo (4196) and must be increased
to 8192 to avoid stack canary crashes with deeper FastLED 3.11 call chains;
update the DRIVERS_STACK_SIZE definition (replace DRIVERS_STACK_SIZE=4196) to
DRIVERS_STACK_SIZE=8192 in the platform configuration and update the adjacent
inline comment to reflect the 8KB size, then rebuild and run the AppDrivers task
to verify the stack-related crash is resolved.
In `@src/MoonLight/Nodes/Drivers/D_FastLED.h`:
- Around line 106-110: The FastLED driver name "I2S" is invalid and will fail at
runtime; update calls to FastLED.setExclusiveDriver(...) (and any
ChannelOptions.mAffinity uses) to use a supported driver name — replace "I2S"
with "PARLIO" for the ESP32/ESP32S2/ESP32S3 branches (and optionally the other
branches for consistency) so the calls in the D_FastLED.h code use a valid
driver ("PARLIO", "SPI", or "RMT") with "PARLIO" recommended to avoid RMT ISR
issues.
🧹 Nitpick comments (4)
platformio.ini (1)
156-156: Pinning to a commit hash onmasteris fragile for production.Using a floating commit on
master(f413fee...) means there's no official release guaranteeing API stability. This is fine for development/testing, but consider pinning to a tagged release once FastLED 3.11 is officially released.src/MoonLight/Nodes/Drivers/D_FastLED.h (3)
140-145:getDriverInfos()is fetched on every loop iteration instead of once.Minor inefficiency:
driversis re-fetched inside theforbody on each iteration. Pull it out before the loop.Proposed fix
// Check which drivers are available + auto drivers = FastLED.getDriverInfos(); for (size_t i = 0; i < FastLED.getDriverCount(); i++) { - auto drivers = FastLED.getDriverInfos(); auto& info = drivers[i]; EXT_LOGD(ML_TAG, "Driver: %s, Priority: %d, Enabled: %s", info.name.c_str(), info.priority, info.enabled ? "yes" : "no"); }
16-22: Remove commented-out includes — they add noise without value.These commented-out
#includedirectives for channel/engine headers are dead code. If they're kept as documentation of which headers exist, a brief comment explaining why they're not needed would be more helpful.
131-136: Debug logging inside the per-pin loop may be noisy in production.The controller-walk logging on every pin iteration is useful during bringup but will spam logs in normal operation. Consider guarding with
VERBOSElevel or removing once the channel API is validated.
| ; -D ML_CHIPSET=SK6812 ; 🚧 | ||
| -D ML_COLOR_ORDER=GRB ; define your color order here if needed (why WS2812 has GRB instead of RGB?) Only for FastLED, PD software configurable | ||
| ; -D ML_RGBW=1 ; define whether to enable RGBW (1 = yes, 0 = no) ... could be derived from ML_CHIPSET??? | ||
| -D DRIVERS_STACK_SIZE=4196 ; psramFound() ? 4 * 1024 : 3 * 1024 |
There was a problem hiding this comment.
DRIVERS_STACK_SIZE=4196 looks like a typo — should be 8192.
The PR discussion identifies 4096 as insufficient for FastLED 3.11's deeper call chains (stack canary crash). 4196 is only 100 bytes more — almost certainly a typo rather than an intentional increase. The recommended fix from the crash analysis is 8192 (8 KB).
Proposed fix
- -D DRIVERS_STACK_SIZE=4196 ; psramFound() ? 4 * 1024 : 3 * 1024
+ -D DRIVERS_STACK_SIZE=8192 ; FastLED 3.11 needs more stack (was 4096, see PR `#121` crash analysis)Based on learnings: "FastLED 3.11 introduces deeper call chains and additional allocations (ChannelEngineSpi, channel creation/acquire, beginTransmission/batchedTransmission, logging/formatting). The project's AppDrivers task was built with DRIVERS_STACK_SIZE=4096 (4KB) in platformio.ini, which is insufficient."
📝 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.
| -D DRIVERS_STACK_SIZE=4196 ; psramFound() ? 4 * 1024 : 3 * 1024 | |
| -D DRIVERS_STACK_SIZE=8192 ; FastLED 3.11 needs more stack (was 4096, see PR `#121` crash analysis) |
🤖 Prompt for AI Agents
In `@platformio.ini` at line 152, The DRIVERS_STACK_SIZE value is a typo (4196)
and must be increased to 8192 to avoid stack canary crashes with deeper FastLED
3.11 call chains; update the DRIVERS_STACK_SIZE definition (replace
DRIVERS_STACK_SIZE=4196) to DRIVERS_STACK_SIZE=8192 in the platform
configuration and update the adjacent inline comment to reflect the 8KB size,
then rebuild and run the AppDrivers task to verify the stack-related crash is
resolved.
| // https://github.com/FastLED/FastLED/blob/master/src/fl/channels/README.md | ||
| fl::ChipsetTimingConfig timing = fl::makeTimingConfig<fl::TIMING_WS2812_800KHZ>(); | ||
| CRGB* leds = (CRGB*)layerP.lights.channelsD; | ||
| uint16_t startLed = 0; | ||
| #if CONFIG_IDF_TARGET_ESP32 || CONFIG_IDF_TARGET_ESP32S2 || CONFIG_IDF_TARGET_ESP32S3 | ||
| FastLED.setExclusiveDriver("I2S"); | ||
| #else | ||
| FastLED.setExclusiveDriver("PARLIO"); | ||
| #endif |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "chipSet|colorOrder|ML_CHIPSET|ML_COLOR_ORDER" --type=h --type=cpp -g '!node_modules' -C2Repository: MoonModules/MoonLight
Length of output: 820
🏁 Script executed:
sed -n '95,130p' src/MoonLight/Nodes/Drivers/D_FastLED.hRepository: MoonModules/MoonLight
Length of output: 1470
Hardcoded WS2812 timing and GRB color order removes previously available configurability.
The code has hardcoded fl::TIMING_WS2812_800KHZ (line 103) and GRB (line 127). Evidence shows these were previously configurable: UI controls for colorOrder, clockPin, and latchPin exist but are commented out in src/MoonBase/Nodes.cpp lines 183-185, and the prior implementation (line 131 comment) supported configurable chipsets and color orders.
This change prevents non-WS2812 chipsets (APA102/SK9822 with clock signals, WS2811 400KHz variants) and non-GRB color orders (RGB, BRG, etc.) from being used. If this is an intentional simplification, uncomment and document the decision. Otherwise, restore configurability via the UI.
| #if CONFIG_IDF_TARGET_ESP32 || CONFIG_IDF_TARGET_ESP32S2 || CONFIG_IDF_TARGET_ESP32S3 | ||
| FastLED.setExclusiveDriver("I2S"); | ||
| #else | ||
| FastLED.setExclusiveDriver("PARLIO"); | ||
| #endif |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
FastLED 3.11 setExclusiveDriver API valid driver names documentation
💡 Result:
In FastLED’s runtime driver-control API (introduced in v3.10.x and present in current FastLED), FastLED.setExclusiveDriver(name) expects the registered ESP32 driver name string. The documented driver names are: "PARLIO", "SPI", "RMT". [1]
If you want to be version/platform-safe (including FastLED 3.11), query what your build actually registered at runtime and use those names:
FastLED.getDriverInfo()→ list of driver records includingdriver.name(the exact string to pass) [1]FastLED.getDriverCount(),FastLED.isDriverEnabled("RMT"), etc. [1]
Source documentation: the FastLED GitHub README section “Runtime Driver Control” (shows setExclusiveDriver("RMT") and the available driver table/names). [1]
References
- FastLED/FastLED GitHub README (“FastLED 3.10.4: Runtime Driver Control”, driver table and examples). (github.com)
Citations:
🏁 Script executed:
# Find and examine the D_FastLED.h file to see lines 115-120
find . -name "D_FastLED.h" -type fRepository: MoonModules/MoonLight
Length of output: 106
🏁 Script executed:
cat -n ./src/MoonLight/Nodes/Drivers/D_FastLED.h | sed -n '100,125p'Repository: MoonModules/MoonLight
Length of output: 1334
Replace "I2S" with a valid driver name — "I2S" is not recognized by FastLED 3.11.
FastLED 3.11's setExclusiveDriver() and ChannelOptions.mAffinity only accept: "PARLIO", "SPI", or "RMT". The code uses "I2S" at lines 107 and 117, which will fail at runtime. Use "PARLIO" or "SPI" instead. "PARLIO" is the safer choice for ESP32/S2/S3 to avoid RMT driver ISR stability issues. This also applies to the else branches at lines 109 and 119 if consistency is needed across all platforms.
🤖 Prompt for AI Agents
In `@src/MoonLight/Nodes/Drivers/D_FastLED.h` around lines 106 - 110, The FastLED
driver name "I2S" is invalid and will fail at runtime; update calls to
FastLED.setExclusiveDriver(...) (and any ChannelOptions.mAffinity uses) to use a
supported driver name — replace "I2S" with "PARLIO" for the
ESP32/ESP32S2/ESP32S3 branches (and optionally the other branches for
consistency) so the calls in the D_FastLED.h code use a valid driver ("PARLIO",
"SPI", or "RMT") with "PARLIO" recommended to avoid RMT ISR issues.
1 participant
Summary by CodeRabbit
Bug Fixes
Chores
UI/Layout