Conversation
firmware ======= - pio.ini: latest FastLED backend ======= - Nodes sharedData: update FL Audio data - FastLED audio: comment onX, add polling in loop - FL Audio effect: update - Solid effect: add palette on columns
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughRefactors audio/shared data to Changes
Sequence Diagram(s)sequenceDiagram
participant AudioHW as Audio Input
participant AudioProc as AudioProcessor
participant Driver as D_FastLEDAudio
participant Shared as SharedData
participant Effect as FastLED/MoonLight Effects
AudioHW->>AudioProc: stream samples
AudioProc->>AudioProc: analyze (EQ, volume, BPM, beats, vocals, percussion)
AudioProc->>Driver: provide getters (eq bins, volume, BPM, confidences, percussion states)
Driver->>Shared: write fl_bands[], fl_volume, fl_bassLevel/fl_midLevel/fl_trebleLevel, fl_bpm, fl_beat/fl_beatConfidence, fl_hihat/fl_kick/fl_snare/fl_tom, fl_vocalConfidence
Effect->>Shared: read fl_* fields each frame
Effect->>LEDs: render visuals based on fl_* values
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
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 (1)
src/MoonLight/Nodes/Drivers/D_FastLEDAudio.h (1)
192-192: Hardcoded normalization factor may produce inconsistent volume scaling.The previous callback-based approach used a dynamic
volumeNormFactorfrom the equalizer, but this is now hardcoded to0.1. If the audio processor's output range varies, volume readings may become inconsistent. Consider documenting why this fixed value was chosen, or expose it as a control if tuning is needed.🤖 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 192, The hardcoded const float norm = 0.1 in D_FastLEDAudio.h causes inconsistent scaling; restore the previous dynamic normalization by calling audioProcessor.getEqVolumeNormFactor() and use a safe fallback when that value is near zero (e.g., if volumeNormFactor > 1e-6 ? volumeNormFactor : 1.0f), or expose the normalization as a configurable parameter on the driver class so it can be tuned; update the line that defines norm and add a comment documenting the chosen fallback behavior and why a fallback is needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/MoonLight/Nodes/Drivers/D_FastLEDAudio.h`:
- Line 208: sharedData.fl_beat is never set true because audioProcessor.isBeat()
is unimplemented; either mark this as intentional with a TODO comment or derive
fl_beat from the existing confidence metric so the FastLED beat logic can work.
Change the code around the commented-out audioProcessor.isBeat() call: if you
want a temporary working beat detector set sharedData.fl_beat =
(sharedData.fl_beatConfidence > X) using a small configurable threshold X (or a
named constant like BEAT_CONF_THRESHOLD) so E_FastLED's beat checks
(sharedData.fl_beat) can trigger, otherwise add a clear TODO comment near
audioProcessor.isBeat() and the line that resets sharedData.fl_beat to false
explaining beat detection is pending upstream implementation.
In `@src/MoonLight/Nodes/Effects/E_FastLED.h`:
- Line 91: The calculation inside the call to layer->drawLine uses integer
division (beatLevel / 255) which collapses the scale to 0 or 1; change the
expression to use floating-point division (e.g., cast beatLevel or 255 to float
or multiply first) so the height multiplier is a fractional value, and ensure
the resulting product with layer->size.y is converted back to an integer/pixel
coordinate before passing to layer->drawLine (refer to the layer->drawLine(...)
call and the beatLevel/255 expression).
- Around line 72-82: The new BPM column uses sharedData.fl_bpm directly, but
fl_bpm is an absolute BPM (e.g., 60–180) and must be normalized to 0..1 before
multiplying by layer->size.y; change the code that draws the BPM bar (the
layer->drawLine call that references sharedData.fl_bpm and columnNr) to compute
a normalized value like: normalizedBpm = clamp((sharedData.fl_bpm - bpmMin) /
(bpmMax - bpmMin), 0.0f, 1.0f) (choose sensible bpmMin/bpmMax, e.g., 60 and 180
or app config), then use layer->size.y * normalizedBpm for the end Y coordinate
and keep the same color/column logic.
---
Nitpick comments:
In `@src/MoonLight/Nodes/Drivers/D_FastLEDAudio.h`:
- Line 192: The hardcoded const float norm = 0.1 in D_FastLEDAudio.h causes
inconsistent scaling; restore the previous dynamic normalization by calling
audioProcessor.getEqVolumeNormFactor() and use a safe fallback when that value
is near zero (e.g., if volumeNormFactor > 1e-6 ? volumeNormFactor : 1.0f), or
expose the normalization as a configurable parameter on the driver class so it
can be tuned; update the line that defines norm and add a comment documenting
the chosen fallback behavior and why a fallback is needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7328f284-3e14-4df4-b22e-42f95880887e
📒 Files selected for processing (6)
platformio.inisrc/MoonBase/Modules/ModuleDevices.hsrc/MoonBase/Nodes.hsrc/MoonLight/Nodes/Drivers/D_FastLEDAudio.hsrc/MoonLight/Nodes/Effects/E_FastLED.hsrc/MoonLight/Nodes/Effects/E_MoonLight.h
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/moonlight/overview.md (1)
26-27:⚠️ Potential issue | 🟡 MinorFix typos in surrounding context.
While these lines weren't modified in this PR, there are two typos worth correcting:
- Line 26: "enthousiasts" → "enthusiasts"
- Line 27: "currenyly" → "currently"
📝 Proposed fixes
-MoonLight is created by [MoonModules](https://moonmodules.org/). MoonModules is a collective of light artist, hardware- and software developers. We love creating DIY lighting solutions based on microcontrollers, mainly ESP32. Our work can be found on GitHub and we use mainly [Discord]([Discord](https://discord.com/channels/700041398778331156/1203994211301728296)) for communications. We develop our products for a large community of light enthousiasts and make products we show to the world, e.g. on art-exhibitions, in venues, or at festivals. See also [about](https://moonmodules.org/about/). -Our flagship product is WLED-MM, a fork of WLED focussing on driving more LEDs, new platforms (e.g. ESP32-P4), better audio and so on. As currenyly most of the MoonModules team are also core developers of WLED, WLED-MM functionality is slowly moving to WLED. +MoonLight is created by [MoonModules](https://moonmodules.org/). MoonModules is a collective of light artist, hardware- and software developers. We love creating DIY lighting solutions based on microcontrollers, mainly ESP32. Our work can be found on GitHub and we use mainly [Discord]([Discord](https://discord.com/channels/700041398778331156/1203994211301728296)) for communications. We develop our products for a large community of light enthusiasts and make products we show to the world, e.g. on art-exhibitions, in venues, or at festivals. See also [about](https://moonmodules.org/about/). +Our flagship product is WLED-MM, a fork of WLED focussing on driving more LEDs, new platforms (e.g. ESP32-P4), better audio and so on. As currently most of the MoonModules team are also core developers of WLED, WLED-MM functionality is slowly moving to WLED.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/moonlight/overview.md` around lines 26 - 27, The doc text in overview.md contains typos: replace "enthousiasts" with "enthusiasts" and "currenyly" with "currently" in the MoonLight description paragraph (the lines mentioning MoonModules community and WLED-MM); update those two words so the sentence reads correctly while preserving the surrounding phrasing and links.docs/index.md (1)
156-156:⚠️ Potential issue | 🟡 MinorFix typo: "Januari" → "January".
The month name is misspelled.
📝 Proposed fix
-### Version 0.8.0 - Januari 2026 +### Version 0.8.0 - January 2026🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/index.md` at line 156, Typo in the release header: change the month name in the header "### Version 0.8.0 - Januari 2026" to "January" so it reads "### Version 0.8.0 - January 2026"; locate the header string in docs/index.md and correct the spelling.
🧹 Nitpick comments (1)
src/MoonLight/Nodes/Drivers/D_FastLEDAudio.h (1)
57-118: Drop the retired callback scaffolding once polling is the only path.This block still mentions removed
sharedDatafields likevocalsActiveandpercussionType, which makes it harder to see which data flow is authoritative now. I'd replace the whole section with one short note that per-frame polling superseded the callback path.🤖 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 57 - 118, Remove the commented-out callback scaffolding related to audioProcessor (onBeat, onVocalStart, onVocalEnd, onVocalConfidence, onBass, onMid, onTreble, onPercussion, onKick, onSnare, onHiHat, onTom) and any references to retired sharedData fields (vocalsActive, percussionType, vocalConfidence, etc.); replace the block with a single concise comment stating that per-frame polling is now the authoritative data flow and that the callback path has been retired so future readers won’t be confused by stale fields or handlers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/gettingstarted/installer.md`:
- Around line 47-51: The table header labeling the stable release still reads
"Release v0.8.0" and should be updated to "Release v0.9.0"; locate the table
header that precedes the device rows (the rows including esp32-d0,
esp32-d0-16mb, esp32-d0-pico2, esp32-p4-nano, esp32-s3-n8r8v and their
<esp-web-install-button> manifest entries) and change the visible stable release
label from v0.8.0 to v0.9.0 so the stable manifest buttons are correctly
described.
- Around line 47-51: Table labels for several boards drift from the actual
section/product names; update the visible names in the table cells so they
exactly match the linked section headings (e.g. change "Dig2Go, DigUno, Dig
Quad" to "Dig2Go, Dig Uno, Dig Quad", change "Dig Octa" if mismatched, and
change "Lightcrafter16" to "LightCrafter16", also ensure "DigNext2" / "Dig Next
2" and any other variants match their target headers). Edit the rows containing
the esp32-d0, esp32-d0-16mb, esp32-d0-pico2 and esp32-s3-n8r8v table cells that
list products and image captions (the visible labels next to the images and
links) to exactly match the linked section titles referenced in the anchor links
(e.g., the text inside the table cell that currently reads "DigUno" -> "Dig
Uno", "Lightcrafter16" -> "LightCrafter16").
In `@docs/moonbase/inputoutput.md`:
- Line 67: The heading rename removed the target referenced by the existing
in-page link "#board-details"; restore the fragment by either reverting or
adding the anchor: restore the original heading text "Board details" (or add an
explicit HTML anchor/tag like <a id="board-details"></a> immediately before the
"QuinLed boards" heading) or update the link at the earlier reference to match
the new heading fragment (e.g., change "#board-details" to the slug generated
from "QuinLed boards"); locate the heading text "QuinLed boards" and the in-page
link referencing "#board-details" to apply the fix.
- Line 133: The image markdown line showing
"../firmware/installer/images/esp32-s3-lightcrafter16.jpg" uses the wrong alt
text "SE-16p"; update that markdown image tag so the alt text reads
"LightCrafter16" (e.g., change `![SE-16p]` to `![LightCrafter16]`) to correctly
identify the product.
- Line 100: Update the image alt text to match the correct board name: change
"MCH ESP32-P4 shield" to "MHC ESP32-P4 shield" in the Markdown image tag (the
line containing {:
style="width: 200px"}), so the screen-reader text matches the section title.
In `@docs/moonlight/drivers.md`:
- Around line 37-40: Update the user-facing copy in the drivers table rows for
"FastLED Audio" and "WLED Audio" to use the hyphenated form "audio-reactive"
instead of "audio reactive"; locate the two rows labeled FastLED Audio and WLED
Audio in docs/moonlight/drivers.md and change occurrences of "audio reactive
effects" or "Listens to audio sent ... and allows audio reactive effects" to
"audio-reactive effects" so the terminology is consistent and hyphenated.
In `@src/MoonLight/Nodes/Drivers/D_FastLEDAudio.h`:
- Around line 185-191: The loop currently calls audioInput->read() only once per
pass, which leaves the FastLED audio input buffer mostly unprocessed; change the
logic in the D_FastLEDAudio driver to drain all available samples each iteration
by restoring the while loop that repeatedly reads and calls
audioProcessor.update(sample) (i.e. while (fl::AudioSample sample =
audioInput->read()) { audioProcessor.update(sample); }) or alternatively use
audioInput->readAll() to fetch the full batch and process each sample with
audioProcessor.update; ensure you still check sample.isValid() or otherwise
handle empty reads when using readAll().
In `@src/MoonLight/Nodes/Effects/E_FastLED.h`:
- Around line 71-107: The new visualization writes columns 0..12 without
checking layer->size.x, which can overflow VirtualLayer; modify FLAudioEffect's
render code to either early-return when layer->size.x < 13 (require minimum
width) or guard each emission by stopping when columnNr >= layer->size.x — i.e.
before every drawLine/setRGB call check columnNr against layer->size.x (or break
out of the emission sequence) so functions drawLine and setRGB never execute
with an out-of-bounds columnNr; keep the existing columnNr progression and
colors but ensure each write is conditioned on the layer width.
---
Outside diff comments:
In `@docs/index.md`:
- Line 156: Typo in the release header: change the month name in the header "###
Version 0.8.0 - Januari 2026" to "January" so it reads "### Version 0.8.0 -
January 2026"; locate the header string in docs/index.md and correct the
spelling.
In `@docs/moonlight/overview.md`:
- Around line 26-27: The doc text in overview.md contains typos: replace
"enthousiasts" with "enthusiasts" and "currenyly" with "currently" in the
MoonLight description paragraph (the lines mentioning MoonModules community and
WLED-MM); update those two words so the sentence reads correctly while
preserving the surrounding phrasing and links.
---
Nitpick comments:
In `@src/MoonLight/Nodes/Drivers/D_FastLEDAudio.h`:
- Around line 57-118: Remove the commented-out callback scaffolding related to
audioProcessor (onBeat, onVocalStart, onVocalEnd, onVocalConfidence, onBass,
onMid, onTreble, onPercussion, onKick, onSnare, onHiHat, onTom) and any
references to retired sharedData fields (vocalsActive, percussionType,
vocalConfidence, etc.); replace the block with a single concise comment stating
that per-frame polling is now the authoritative data flow and that the callback
path has been retired so future readers won’t be confused by stale fields or
handlers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 08ea5422-2924-40a9-af35-a8911b1d8e22
⛔ Files ignored due to path filters (2)
docs/media/moonlight/drivers/FastLED-Audio.pngis excluded by!**/*.pngdocs/media/moonlight/drivers/FastLED-Driver.pngis excluded by!**/*.png
📒 Files selected for processing (9)
README.mddocs/gettingstarted/installer.mddocs/index.mddocs/moonbase/inputoutput.mddocs/moonlight/drivers.mddocs/moonlight/overview.mdmkdocs.ymlsrc/MoonLight/Nodes/Drivers/D_FastLEDAudio.hsrc/MoonLight/Nodes/Effects/E_FastLED.h
✅ Files skipped from review due to trivial changes (2)
- mkdocs.yml
- README.md
docs/gettingstarted/installer.md
Outdated
| | esp32-d0 | {: style="width:100px"} | <esp-web-install-button manifest="https://moonmodules.org/MoonLight/firmware/installer/manifest_esp32-d0.json"></esp-web-install-button> | <esp-web-install-button manifest="https://moonmodules.org/MoonLight/firmware/installer/manifest_esp32-d0-nightly.json"></esp-web-install-button> | [Dig2Go, DigUno, Dig Quad](../../moonbase/inputoutput/#dig2go-dig-uno-dig-quad):<br>{: style="width:100px"}<br>{: style="width:100px"}<br>{: style="width:100px"} | | ||
| | esp32-d0-16mb | {: style="width:100px"} | <esp-web-install-button manifest="https://moonmodules.org/MoonLight/firmware/installer/manifest_esp32-d0-16mb.json"></esp-web-install-button> | <esp-web-install-button manifest="https://moonmodules.org/MoonLight/firmware/installer/manifest_esp32-d0-16mb-nightly.json"></esp-web-install-button> | [Dig Octa](../../moonbase/inputoutput/#dig-octa):<br>{: style="width:100px"}<br>[Serg ESP32](https://www.tindie.com/products/serg74/esp32-wroom-usb-c-d1-mini32-form-factor-board){:target="_blank"} and [Shield](https://www.tindie.com/products/serg74/wled-shield-board-for-addressable-leds)<br>/i/93057/products/2021-08-14T14%3A44%3A14.418Z-shield_v3-1.jpg?1628927139){: style="width:100px"} | | ||
| | [esp32-d0-pico2](https://documentation.espressif.com/esp32-pico-mini-02_datasheet_en.pdf) | {: style="width:100px"} | <esp-web-install-button manifest="https://moonmodules.org/MoonLight/firmware/installer/manifest_esp32-d0-pico2.json"></esp-web-install-button> | <esp-web-install-button manifest="https://moonmodules.org/MoonLight/firmware/installer/manifest_esp32-d0-pico2-nightly.json"></esp-web-install-button> | [DigNext2](../../moonbase/inputoutput/#dig-next-2):<br>{: style="width:100px"}<br>[MHC V57 PRO](../../moonbase/inputoutput/#mhc-v57-pro):<br>{: style="width:100px"}| | ||
| | esp32-p4-nano | {: style="width:100px"} | First Install<br> [esp32-c6-firmware-update](https://esp32-c6-firmware-update.github.io)<br><esp-web-install-button manifest="https://moonmodules.org/MoonLight/firmware/installer/manifest_esp32-p4-nano.json"></esp-web-install-button> | <esp-web-install-button manifest="https://moonmodules.org/MoonLight/firmware/installer/manifest_esp32-p4-nano-nightly.json"></esp-web-install-button> | [Waveshare](https://www.waveshare.com/esp32-p4-nano.htm){:target="_blank"}<br>[MHC ESP32-P4 shield](../../moonbase/inputoutput/#mhc-esp32-p4-shield):<br>{: style="width:100px"} | | ||
| | esp32-s3-n8r8v | {: style="width:100px"} | <esp-web-install-button manifest="https://moonmodules.org/MoonLight/firmware/installer/manifest_esp32-s3-n8r8v.json"></esp-web-install-button> | <esp-web-install-button manifest="https://moonmodules.org/MoonLight/firmware/installer/manifest_esp32-s3-n8r8v-nightly.json">Nightly</esp-web-install-button> | SE-16p<br>{: style="width:100px"} | | ||
| | esp32-s3-n8r8v | {: style="width:100px"} | <esp-web-install-button manifest="https://moonmodules.org/MoonLight/firmware/installer/manifest_esp32-s3-n8r8v.json"></esp-web-install-button> | <esp-web-install-button manifest="https://moonmodules.org/MoonLight/firmware/installer/manifest_esp32-s3-n8r8v-nightly.json">Nightly</esp-web-install-button> | [SE-16p](../../moonbase/inputoutput/#se16-v1)<br>{: style="width:100px"}<br>[Lightcrafter16](../../moonbase/inputoutput/#lightcrafter16)<br>{: style="width:100px"} | |
There was a problem hiding this comment.
Update the stable release label to v0.9.0.
These rows are being refreshed for the v0.9.0 prep, but the table header next to them still says Release v0.8.0, which will mislabel the stable manifests.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/gettingstarted/installer.md` around lines 47 - 51, The table header
labeling the stable release still reads "Release v0.8.0" and should be updated
to "Release v0.9.0"; locate the table header that precedes the device rows (the
rows including esp32-d0, esp32-d0-16mb, esp32-d0-pico2, esp32-p4-nano,
esp32-s3-n8r8v and their <esp-web-install-button> manifest entries) and change
the visible stable release label from v0.8.0 to v0.9.0 so the stable manifest
buttons are correctly described.
Keep the board names consistent with the linked sections.
The visible labels drift from the actual product/section names here (DigUno vs Dig Uno, Lightcrafter16 vs LightCrafter16).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/gettingstarted/installer.md` around lines 47 - 51, Table labels for
several boards drift from the actual section/product names; update the visible
names in the table cells so they exactly match the linked section headings (e.g.
change "Dig2Go, DigUno, Dig Quad" to "Dig2Go, Dig Uno, Dig Quad", change "Dig
Octa" if mismatched, and change "Lightcrafter16" to "LightCrafter16", also
ensure "DigNext2" / "Dig Next 2" and any other variants match their target
headers). Edit the rows containing the esp32-d0, esp32-d0-16mb, esp32-d0-pico2
and esp32-s3-n8r8v table cells that list products and image captions (the
visible labels next to the images and links) to exactly match the linked section
titles referenced in the anchor links (e.g., the text inside the table cell that
currently reads "DigUno" -> "Dig Uno", "Lightcrafter16" -> "LightCrafter16").
docs/moonbase/inputoutput.md
Outdated
| - Device (DVC): All of the above in a box with connectors | ||
|
|
||
| ## Board details | ||
| ## QuinLed boards |
There was a problem hiding this comment.
Restore the old in-page anchor or update the existing link.
Renaming this heading removes the #board-details target still referenced at Line 13, so that link now jumps nowhere.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/moonbase/inputoutput.md` at line 67, The heading rename removed the
target referenced by the existing in-page link "#board-details"; restore the
fragment by either reverting or adding the anchor: restore the original heading
text "Board details" (or add an explicit HTML anchor/tag like <a
id="board-details"></a> immediately before the "QuinLed boards" heading) or
update the link at the earlier reference to match the new heading fragment
(e.g., change "#board-details" to the slug generated from "QuinLed boards");
locate the heading text "QuinLed boards" and the in-page link referencing
"#board-details" to apply the fix.
| // while (fl::AudioSample sample = audioInput->read()) { | ||
| // audioProcessor.update(sample); | ||
| // } | ||
|
|
||
| while (fl::AudioSample sample = audioInput->read()) { | ||
| fl::AudioSample sample = audioInput->read(); | ||
| if (sample.isValid()) { | ||
| audioProcessor.update(sample); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
commit=6d92ea02e5a82da54406e29304ea9217d1c62c08
base="https://raw.githubusercontent.com/FastLED/FastLED/$commit"
for file in src/fl/audio_input.h src/fl/audio/audio_processor.h src/fl/audio/README.md; do
echo "===== $file ====="
curl -fsSL "$base/$file" | grep -nE 'class IAudioInput|struct AudioSample|read\(|update\(' -C2 || true
doneRepository: MoonModules/MoonLight
Length of output: 4350
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get the full file to understand audio task scheduling and prior implementation
echo "===== Full D_FastLEDAudio.h file ====="
wc -l src/MoonLight/Nodes/Drivers/D_FastLEDAudio.hRepository: MoonModules/MoonLight
Length of output: 152
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the entire file (should be reasonable size)
cat -n src/MoonLight/Nodes/Drivers/D_FastLEDAudio.hRepository: MoonModules/MoonLight
Length of output: 10251
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for audio-related task setup and buffering elsewhere in the codebase
echo "===== Search for audio task scheduling patterns ====="
rg -i "audio.*task|schedule.*audio|every_ms.*audio" --type cpp --type h -B2 -A2
echo ""
echo "===== Search for loop patterns with audio ====="
rg "while.*read|for.*read" --type cpp --type h src/MoonLight/Nodes/Drivers/ -B1 -A1Repository: MoonModules/MoonLight
Length of output: 118
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check git history for the audio loop changes
echo "===== Git history for D_FastLEDAudio.h around lines 185-191 ====="
git log --oneline -20 -- src/MoonLight/Nodes/Drivers/D_FastLEDAudio.h
echo ""
echo "===== Recent changes to the loop pattern ====="
git log -p -5 -- src/MoonLight/Nodes/Drivers/D_FastLEDAudio.h | grep -A10 -B10 "while.*read\|fl::AudioSample sample" | head -100Repository: MoonModules/MoonLight
Length of output: 232
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check loop() call frequency and main render loop
echo "===== Search for loop call frequency and timing ====="
rg "loop\(\)|FPS|frame" --type cpp --type h src/MoonLight/Nodes/Drivers/ -B1 -A1 | head -50
echo ""
echo "===== Check if there are any audio-related issues or TODOs ====="
rg -i "todo|fixme|bug|issue.*audio|undersample|drain" --type cpp --type h src/MoonLight/Nodes/Drivers/D_FastLEDAudio.hRepository: MoonModules/MoonLight
Length of output: 3416
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get detailed commit info
echo "===== Commit c24b790 details ====="
git show c24b790 -- src/MoonLight/Nodes/Drivers/D_FastLEDAudio.h | head -150Repository: MoonModules/MoonLight
Length of output: 5234
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check how frequently loop() is called in the main render flow
echo "===== Search for main loop/frame structure ====="
fd -e cpp -e h "Main|main" src/MoonLight --type f | head -5
echo ""
echo "===== Check MoonLight module/framework structure ====="
fd -e h "Node|Driver" src/MoonLight --maxdepth 2 --type f | head -10
echo ""
echo "===== Look for any frame/render loop orchestration ====="
rg "void loop|FastLED.show|show\(\)" --type cpp --type h src/MoonLight --max-count=5Repository: MoonModules/MoonLight
Length of output: 3225
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the layer/module structure to understand loop() call frequency
echo "===== PhysicalLayer loop structure ====="
grep -n "loopDriver\|loop20ms\|loop()" src/MoonLight/Layers/PhysicalLayer.h | head -20
echo ""
echo "===== ModuleEffects loop frequency ====="
grep -n "loop\|getFps\|20ms" src/MoonLight/Modules/ModuleEffects.h | head -15
echo ""
echo "===== Check if there's a frame rate or loop cadence defined ====="
rg "getFps|loop.*Hz|FPS.*=" --type cpp --type h src/MoonLight --max-count=10Repository: MoonModules/MoonLight
Length of output: 666
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the learnings more carefully - it mentions frame/monitor synchronization
# Let's understand the actual loop cadence from PhysicalLayer
echo "===== PhysicalLayer.h full context around loop/driver calls ====="
cat -n src/MoonLight/Layers/PhysicalLayer.h | sed -n '1,150p'Repository: MoonModules/MoonLight
Length of output: 7675
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Try to fetch the actual audio_input.h implementation from FastLED
commit=6d92ea02e5a82da54406e29304ea9217d1c62c08
base="https://raw.githubusercontent.com/FastLED/FastLED/$commit"
echo "===== Attempting to retrieve audio_input.h with read() implementation ====="
curl -fsSL "$base/src/fl/audio_input.h" 2>&1 | head -200Repository: MoonModules/MoonLight
Length of output: 256
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if there's any documentation or comments about buffer behavior
# Also check loop frequency by looking at how effects/drivers are orchestrated
echo "===== Search for NodeManager loop orchestration ====="
fd -type f -name "NodeManager*" src/MoonLight
echo ""
echo "===== Check if there's FPS or loop frequency config ====="
rg "loop.*frequency|loop.*Hz|every.*ms|task.*every" --type cpp --type h src/MoonLight/Layers --max-count=5Repository: MoonModules/MoonLight
Length of output: 306
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get the actual audio_input.h from FastLED
commit=6d92ea02e5a82da54406e29304ea9217d1c62c08
base="https://raw.githubusercontent.com/FastLED/FastLED/$commit"
echo "===== FastLED audio/input.h (the actual implementation) ====="
curl -fsSL "$base/src/fl/audio/input.h" 2>&1 | head -300Repository: MoonModules/MoonLight
Length of output: 9448
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check how loop frequency is determined in the codebase
echo "===== Search for loop orchestration and frequency ====="
rg "loopDrivers|loop\(\)" --type h src/MoonLight/Modules/ModuleEffects.h -B2 -A2 | head -40
echo ""
echo "===== Check PhysicalLayer loop implementation ====="
fd "PhysicalLayer" -type f src/MoonLight/Layers/ 2>/dev/null | xargs ls -laRepository: MoonModules/MoonLight
Length of output: 120
Drain all buffered audio samples per loop iteration.
The FastLED audio input uses a background task/interrupt to fill a sample buffer. Calling audioInput->read() only once per iteration (lines 189–191) drains a single sample while leaving the remaining buffered samples unprocessed. With 44.1 kHz input and typical loop cadence (~20 ms), roughly 800+ samples accumulate and are discarded each frame, causing severe data loss and degraded EQ/beat/BPM detection.
Restore the commented while loop (lines 185–187) to drain all available samples from the buffer each pass:
Proposed fix
- fl::AudioSample sample = audioInput->read();
- if (sample.isValid()) {
- audioProcessor.update(sample);
- }
+ while (fl::AudioSample sample = audioInput->read()) {
+ if (sample.isValid()) {
+ audioProcessor.update(sample);
+ }
+ }Alternatively, use the built-in readAll() convenience method to drain the buffer into a vector and process all samples in a single call.
🤖 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 185 - 191, The
loop currently calls audioInput->read() only once per pass, which leaves the
FastLED audio input buffer mostly unprocessed; change the logic in the
D_FastLEDAudio driver to drain all available samples each iteration by restoring
the while loop that repeatedly reads and calls audioProcessor.update(sample)
(i.e. while (fl::AudioSample sample = audioInput->read()) {
audioProcessor.update(sample); }) or alternatively use audioInput->readAll() to
fetch the full batch and process each sample with audioProcessor.update; ensure
you still check sample.isValid() or otherwise handle empty reads when using
readAll().
| uint8_t columnNr = 0; | ||
| layer->drawLine(columnNr, layer->size.y - 1, columnNr, layer->size.y - 1 - layer->size.y * sharedData.bassLevel, CRGB::Red); | ||
| layer->drawLine(columnNr, layer->size.y - 1, columnNr, layer->size.y - 1 - layer->size.y * sharedData.fl_bassLevel, CRGB::Red); | ||
| columnNr++; | ||
| layer->drawLine(columnNr, layer->size.y - 1, columnNr, layer->size.y - 1 - layer->size.y * sharedData.midLevel, CRGB::Orange); | ||
| layer->drawLine(columnNr, layer->size.y - 1, columnNr, layer->size.y - 1 - layer->size.y * sharedData.fl_midLevel, CRGB::Orange); | ||
| columnNr++; | ||
| layer->drawLine(columnNr, layer->size.y - 1, columnNr, layer->size.y - 1 - layer->size.y * sharedData.trebleLevel, CRGB::Green); | ||
| layer->drawLine(columnNr, layer->size.y - 1, columnNr, layer->size.y - 1 - layer->size.y * sharedData.fl_trebleLevel, CRGB::Green); | ||
| columnNr++; | ||
| layer->drawLine(columnNr, layer->size.y - 1, columnNr, layer->size.y - 1 - layer->size.y * sharedData.vocalConfidence, CRGB::Blue); | ||
|
|
||
| layer->drawLine(columnNr, layer->size.y - 1, columnNr, layer->size.y - 1 - layer->size.y * sharedData.volume, CRGB::Yellow); | ||
| columnNr++; | ||
| // Normalize BPM to 0-1 range (assuming typical range 60-200 BPM) | ||
| float normalizedBpm = constrain(sharedData.fl_bpm, 60.0f, 200.0f); | ||
| normalizedBpm = (normalizedBpm - 60.0f) / 140.0f; | ||
| layer->drawLine(columnNr, layer->size.y - 1, columnNr, layer->size.y - 1 - layer->size.y * normalizedBpm, CRGB::Yellow); | ||
| columnNr++; | ||
|
|
||
| // vocal | ||
| layer->drawLine(columnNr, layer->size.y - 1, columnNr, layer->size.y - 1 - layer->size.y * sharedData.fl_vocalConfidence, CRGB::Purple); | ||
| columnNr++; | ||
|
|
||
| // beat | ||
| layer->drawLine(columnNr, layer->size.y - 1, columnNr++, layer->size.y - 1 - layer->size.y * beatLevel / 255, CRGB::Purple); | ||
| if (sharedData.beat) layer->setRGB(Coord3D(columnNr, layer->size.y - 1), CRGB::Purple); | ||
| layer->drawLine(columnNr, layer->size.y - 1, columnNr, layer->size.y - 1 - layer->size.y * sharedData.fl_beatConfidence, CRGB::Blue); | ||
| columnNr++; | ||
| layer->drawLine(columnNr, layer->size.y - 1, columnNr, layer->size.y - 1 - layer->size.y * beatLevel / 255.0f, CRGB::Blue); | ||
| columnNr++; | ||
| if (sharedData.fl_beat) layer->setRGB(Coord3D(columnNr, layer->size.y - 1), CRGB::Blue); | ||
| columnNr++; | ||
|
|
||
| // percussion | ||
| if (sharedData.percussionType != UINT8_MAX) { | ||
| uint16_t percussionCol = columnNr + sharedData.percussionType; | ||
| if (percussionCol < layer->size.x) { | ||
| layer->setRGB(Coord3D(percussionCol, layer->size.y - 1), CRGB::Cyan); | ||
| } | ||
| } | ||
| if (sharedData.fl_hihat) layer->setRGB(Coord3D(columnNr, layer->size.y - 1), CRGB::Cyan); | ||
| columnNr++; | ||
| if (sharedData.fl_kick) layer->setRGB(Coord3D(columnNr, layer->size.y - 1), CRGB::Cyan); | ||
| columnNr++; | ||
| if (sharedData.fl_snare) layer->setRGB(Coord3D(columnNr, layer->size.y - 1), CRGB::Cyan); | ||
| columnNr++; | ||
| if (sharedData.fl_tom) layer->setRGB(Coord3D(columnNr, layer->size.y - 1), CRGB::Cyan); | ||
|
|
There was a problem hiding this comment.
Guard the new visualization columns on narrow layouts.
Lines 72-106 now draw into columns 0..12, but the effect never checks that layer->size.x >= 13. On smaller 2D layouts, those drawLine/setRGB calls will run past the logical width, and in MoonLight that is not a harmless no-op because the VirtualLayer hot path intentionally skips bounds checks for performance. Either require a minimum width for FLAudioEffect or stop emitting columns once columnNr == layer->size.x.
Based on learnings, in VirtualLayer::setLight and VirtualLayer::getLight bounds checking is intentionally disabled for performance when driving very large LED counts, so effect developers are responsible for staying within bounds.
🤖 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 71 - 107, The new
visualization writes columns 0..12 without checking layer->size.x, which can
overflow VirtualLayer; modify FLAudioEffect's render code to either early-return
when layer->size.x < 13 (require minimum width) or guard each emission by
stopping when columnNr >= layer->size.x — i.e. before every drawLine/setRGB call
check columnNr against layer->size.x (or break out of the emission sequence) so
functions drawLine and setRGB never execute with an out-of-bounds columnNr; keep
the existing columnNr progression and colors but ensure each write is
conditioned on the layer width.
Header4 , in .md files results in inconsistent formatting of the header . E.g. #### Context in Nodes.md isContextin the html, so probably h4 is not defined ? Check other header levels and propose right formatting for header level 4 |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 21045 🏁 Script executed: Length of output: 20206
Fix: Add an /* Enhanced Headings */
.md-content h2 {
/* existing ... */
}
.md-content h3 {
color: var(--moonlight-primary);
font-weight: 600;
}
/* ADD THIS */
.md-content h4 {
color: var(--moonlight-accent);
font-weight: 600;
font-size: 0.95rem;
}This uses If you prefer the blue instead: .md-content h4 {
color: var(--moonlight-blue);
font-weight: 600;
font-size: 0.95rem;
}The |
FastLED Audio: only using polling
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (3)
docs/moonbase/inputoutput.md (1)
67-67:⚠️ Potential issue | 🟡 MinorRestore the
#board-detailstarget or retarget the intro link.Renaming this heading breaks the in-page link at Line 13, so “See Board details” no longer jumps to the section.
🩹 Minimal fix
+<a id="board-details"></a> ## QuinLed boards🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/moonbase/inputoutput.md` at line 67, The in-page link "See Board details" now breaks because the section heading was renamed to "## QuinLed boards" and no longer has the '#board-details' target; either restore the original anchor by adding the HTML id or explicit anchor to the "## QuinLed boards" heading (e.g. add id="board-details") or update the intro link target to match the new heading slug; locate the heading text "## QuinLed boards" and the intro link referencing '#board-details' and make them consistent.docs/gettingstarted/installer.md (1)
47-49:⚠️ Potential issue | 🟡 MinorKeep the visible board names aligned with the linked sections.
DigUnoandDigNext2still differ from the target section titles (Dig UnoandDig Next 2) indocs/moonbase/inputoutput.md, so the installer table reads inconsistently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/gettingstarted/installer.md` around lines 47 - 49, The installer table has mismatched visible board names vs. their linked section titles: update the visible labels in the table so they match the target section names in docs/moonbase/inputoutput.md — change "DigUno" to "Dig Uno" in the cell that currently reads "[Dig2Go, DigUno, Dig Quad]" and change "DigNext2" to "Dig Next 2" in the cell that currently lists "[DigNext2]"; keep the links and images the same so only the displayed text is corrected.src/MoonLight/Nodes/Drivers/D_FastLEDAudio.h (1)
185-196:⚠️ Potential issue | 🟠 MajorDrain the buffered samples instead of consuming only one per frame.
Line 194 only pulls a single sample even though
IAudioInputis fed asynchronously. At the usual render cadence, that drops most of the microphone stream and will degrade EQ / beat / BPM detection.🔧 Proposed fix
- fl::AudioSample sample = audioInput->read(); - if (sample.isValid()) { - audioProcessor.update(sample); - } + while (fl::AudioSample sample = audioInput->read()) { + if (sample.isValid()) { + audioProcessor.update(sample); + } + }🤖 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 185 - 196, Replace the single-sample read with a loop that drains all buffered samples from the asynchronous FastLED input: repeatedly call audioInput->read() and for each valid fl::AudioSample call audioProcessor.update(sample) until read() returns an invalid sample; ensure this loop exits when no more samples are available to avoid blocking the render thread (optionally add a reasonable iteration cap if needed).
🧹 Nitpick comments (1)
docs/stylesheets/moonlight.css (1)
181-185: Keeph4visually distinct fromh3.Line 182 reuses the same color token as
h3, so those two heading levels still read almost the same. A separate accent color and slightly less compressed size would preserve the hierarchy this docs fix is aiming for.🎨 Suggested tweak
.md-content h4 { - color: var(--moonlight-primary); + color: var(--moonlight-accent); font-weight: 600; - font-size: 0.85rem; + font-size: 0.95rem; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/stylesheets/moonlight.css` around lines 181 - 185, The .md-content h4 rule currently reuses var(--moonlight-primary) and too-small font-size, making h4 indistinguishable from h3; update the .md-content h4 selector to use a distinct color token (e.g., var(--moonlight-accent) or var(--moonlight-secondary)) and increase the font-size slightly (make it less compressed than 0.85rem, e.g., ~0.9–1.0rem) while keeping font-weight 600 so h4 reads as a clear, subordinate heading; adjust only the color and font-size in the .md-content h4 rule to preserve hierarchy.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/develop/standardsguidelines.md`:
- Around line 86-93: In the "Attribution" and "Documentation" sections fix
style/grammar: hyphenate "AI generated" to "AI-generated" in both occurrences,
change "Github" to "GitHub", and add the missing comma after "e.g." so the
phrase reads "e.g., GitHub repositories or other websites describing the
principles / algorithms used."; update the corresponding lines under the
"Attribution" and "Documentation" headings accordingly to keep consistent
terminology.
In `@docs/moonlight/drivers.md`:
- Around line 36-40: The table rows for "FastLED Driver", "FastLED Audio",
"Art-Net Out", "Art-Net In", and "WLED Audio" contain <img> tags (e.g.
src="../../media/moonlight/drivers/FastLED-Driver.png",
src="../../media/moonlight/drivers/FastLED-Audio.png",
src="https://github.com/user-attachments/assets/9c65921c-64e9-4558-b6ef-aed2a163fd88",
src="../../media/moonlight/drivers/ArtNetInControls.png",
src="https://github.com/user-attachments/assets/bfedf80b-6596-41e7-a563-ba7dd58cc476")
without alt attributes; add meaningful alt text to each <img> tag (for example
alt="FastLED Driver logo", alt="FastLED Audio controls screenshot", alt="Art-Net
Out icon", alt="Art-Net In controls screenshot", alt="WLED Audio icon") so
markdownlint and assistive technologies have appropriate descriptions while
preserving existing src and sizes.
In `@docs/moonlight/layouts.md`:
- Line 27: Update the alt text for the Car Lights controls image: locate the
table row containing "Car Lights" and the <img ... alt="gourds"
src="../../media/moonlight/layouts/CarLights.png"/> tag and replace the
incorrect alt "gourds" with a descriptive alt like "Car Lights controls" (or
similar concise description) so screen readers announce the correct layout.
In `@docs/moonlight/overview.md`:
- Around line 137-138: Update the user-facing copy to hyphenate the compound
adjectives: replace the phrase "FFT based" with "FFT-based" and "volume based"
with "volume-based" in the overview content (the list items containing "♫
Audio-reactive FFT based" and "♪ Audio-reactive volume based") so the compounds
are grammatically correct.
- Line 26: The Discord link uses nested Markdown
"[Discord]([Discord](https://...))" which breaks rendering; edit the line
containing that snippet (the MoonLight overview sentence) and replace the nested
form with a single Markdown link like
"[Discord](https://discord.com/channels/700041398778331156/1203994211301728296)"
so the link renders correctly.
In `@misc/instructions/MoonLight.md`:
- Line 22: Fix the typo in the MoonLight documentation sentence: replace the
misspelled word "indepenent" with the correct spelling "independent" in the
paragraph that begins "MoonLight currently supports audio-reactivity..." so the
phrase reads "Audio processing independent of another device running WLED."
In `@src/MoonLight/Nodes/Drivers/D_FastLEDAudio.h`:
- Around line 202-204: The code divides audioProcessor.getEqVolume() by norm
then casts to int16_t without clamping, which can overflow; change the
assignment for sharedData.volume to compute the scaled value then clamp it to
the representable range before converting (e.g., clamp scaledVolume to a max of
1.0f or to (INT16_MAX / 32767.0f)), and then set sharedData.volumeRaw using a
safe static_cast<int16_t> from the clamped value; update the logic around norm,
sharedData.volume and sharedData.volumeRaw to enforce this clamp.
---
Duplicate comments:
In `@docs/gettingstarted/installer.md`:
- Around line 47-49: The installer table has mismatched visible board names vs.
their linked section titles: update the visible labels in the table so they
match the target section names in docs/moonbase/inputoutput.md — change "DigUno"
to "Dig Uno" in the cell that currently reads "[Dig2Go, DigUno, Dig Quad]" and
change "DigNext2" to "Dig Next 2" in the cell that currently lists "[DigNext2]";
keep the links and images the same so only the displayed text is corrected.
In `@docs/moonbase/inputoutput.md`:
- Line 67: The in-page link "See Board details" now breaks because the section
heading was renamed to "## QuinLed boards" and no longer has the
'#board-details' target; either restore the original anchor by adding the HTML
id or explicit anchor to the "## QuinLed boards" heading (e.g. add
id="board-details") or update the intro link target to match the new heading
slug; locate the heading text "## QuinLed boards" and the intro link referencing
'#board-details' and make them consistent.
In `@src/MoonLight/Nodes/Drivers/D_FastLEDAudio.h`:
- Around line 185-196: Replace the single-sample read with a loop that drains
all buffered samples from the asynchronous FastLED input: repeatedly call
audioInput->read() and for each valid fl::AudioSample call
audioProcessor.update(sample) until read() returns an invalid sample; ensure
this loop exits when no more samples are available to avoid blocking the render
thread (optionally add a reasonable iteration cap if needed).
---
Nitpick comments:
In `@docs/stylesheets/moonlight.css`:
- Around line 181-185: The .md-content h4 rule currently reuses
var(--moonlight-primary) and too-small font-size, making h4 indistinguishable
from h3; update the .md-content h4 selector to use a distinct color token (e.g.,
var(--moonlight-accent) or var(--moonlight-secondary)) and increase the
font-size slightly (make it less compressed than 0.85rem, e.g., ~0.9–1.0rem)
while keeping font-weight 600 so h4 reads as a clear, subordinate heading;
adjust only the color and font-size in the .md-content h4 rule to preserve
hierarchy.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2a9b9e3b-231f-4bc4-b768-98ed5a524d36
⛔ Files ignored due to path filters (5)
docs/media/moonlight/layouts/BarGuords.gifis excluded by!**/*.gifdocs/media/moonlight/layouts/BarGuords.pngis excluded by!**/*.pngdocs/media/moonlight/layouts/CarLights.gifis excluded by!**/*.gifdocs/media/moonlight/layouts/CarLights.pngis excluded by!**/*.pngdocs/moonbase/MHC-ESP32-P4-shield.pngis excluded by!**/*.png
📒 Files selected for processing (14)
README.mddocs/develop/standardsguidelines.mddocs/gettingstarted/installer.mddocs/index.mddocs/moonbase/inputoutput.mddocs/moonlight/drivers.mddocs/moonlight/layouts.mddocs/moonlight/lightscontrol.mddocs/moonlight/overview.mddocs/stylesheets/moonlight.cssmisc/instructions/MoonLight.mdplatformio.inisrc/MoonBase/Modules/ModuleIO.hsrc/MoonLight/Nodes/Drivers/D_FastLEDAudio.h
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/index.md
- platformio.ini
| | FastLED Driver | <img width="100" src="https://avatars.githubusercontent.com/u/5899270?s=48&v=4"/> | <img width="320" src="../../media/moonlight/drivers/FastLED-Driver.png" /> | Based on the FastLED Channels API to set Pins, Color order, Engine and other settings at runtime! Based on upcoming FastLED v4.0 ! See [Channels API](https://github.com/FastLED/FastLED/blob/master/src/fl/channels/README.md) | | ||
| | FastLED Audio | <img width="100" src="https://avatars.githubusercontent.com/u/5899270?s=48&v=4"/> | <img width="320" src="../../media/moonlight/drivers/FastLED-Audio.png" /> | On-board microphone audio processing, allows audio-reactive effects (♪ & ♫) to use audio data (volume and bands (FFT)) and much more. Based on upcoming FastLED v4.0 ! see [FastLED Audio](https://github.com/FastLED/FastLED/blob/master/src/fl/audio/README.md)<br>Connect a digital microphone (e.g. INMP441) to an ESP32 and setup the I2S pins in the [IO module](../../moonbase/inputoutput)| | ||
| | Art-Net Out| <img width="100" src="https://github.com/user-attachments/assets/9c65921c-64e9-4558-b6ef-aed2a163fd88"> | <img width="320" src="../../media/moonlight/drivers/ArtNetOutControls.png" /> | Send Art-Net to Drive LEDS and DMX lights over the network. See [below](#art-net-out) | | ||
| | Art-Net In | <img width="100" src="../../media/moonlight/drivers/Art-Net-In.png"> | <img width="320" src="../../media/moonlight/drivers/ArtNetInControls.png" /> | Receive Art-Net (or DDP) packages e.g. from [Resolume](https://resolume.com/) or Touch Designer. See [below](#art-net-in) | | ||
| | Audio Sync | <img width="100" src="https://github.com/user-attachments/assets/bfedf80b-6596-41e7-a563-ba7dd58cc476"/> | No controls | Listens to audio sent over the local network by WLED-AC or WLED-MM and allows audio reactive effects (♪ & ♫) to use audio data (volume and bands (FFT)) | | ||
| | WLED Audio | <img width="100" src="https://github.com/user-attachments/assets/bfedf80b-6596-41e7-a563-ba7dd58cc476"/> | No controls | Listens to audio sent over the local network by WLED or WLED-MM and allows audio-reactive effects (♪ & ♫) to use audio data (volume and bands (FFT)) | |
There was a problem hiding this comment.
Add alt text to the new driver images.
These new <img> tags do not provide alt text, so markdownlint flags them and assistive technologies get no useful description.
✏️ Proposed fix
-| FastLED Driver | <img width="100" src="https://avatars.githubusercontent.com/u/5899270?s=48&v=4"/> | <img width="320" src="../../media/moonlight/drivers/FastLED-Driver.png" /> | Based on the FastLED Channels API to set Pins, Color order, Engine and other settings at runtime! Based on upcoming FastLED v4.0 ! See [Channels API](https://github.com/FastLED/FastLED/blob/master/src/fl/channels/README.md) |
-| FastLED Audio | <img width="100" src="https://avatars.githubusercontent.com/u/5899270?s=48&v=4"/> | <img width="320" src="../../media/moonlight/drivers/FastLED-Audio.png" /> | On-board microphone audio processing, allows audio-reactive effects (♪ & ♫) to use audio data (volume and bands (FFT)) and much more. Based on upcoming FastLED v4.0 ! see [FastLED Audio](https://github.com/FastLED/FastLED/blob/master/src/fl/audio/README.md)<br>Connect a digital microphone (e.g. INMP441) to an ESP32 and setup the I2S pins in the [IO module](../../moonbase/inputoutput)|
+| FastLED Driver | <img width="100" alt="FastLED logo" src="https://avatars.githubusercontent.com/u/5899270?s=48&v=4"/> | <img width="320" alt="FastLED Driver controls" src="../../media/moonlight/drivers/FastLED-Driver.png" /> | Based on the FastLED Channels API to set Pins, Color order, Engine and other settings at runtime! Based on upcoming FastLED v4.0 ! See [Channels API](https://github.com/FastLED/FastLED/blob/master/src/fl/channels/README.md) |
+| FastLED Audio | <img width="100" alt="FastLED logo" src="https://avatars.githubusercontent.com/u/5899270?s=48&v=4"/> | <img width="320" alt="FastLED Audio controls" src="../../media/moonlight/drivers/FastLED-Audio.png" /> | On-board microphone audio processing, allows audio-reactive effects (♪ & ♫) to use audio data (volume and bands (FFT)) and much more. Based on upcoming FastLED v4.0 ! see [FastLED Audio](https://github.com/FastLED/FastLED/blob/master/src/fl/audio/README.md)<br>Connect a digital microphone (e.g. INMP441) to an ESP32 and setup the I2S pins in the [IO module](../../moonbase/inputoutput)|-| WLED Audio | <img width="100" src="https://github.com/user-attachments/assets/bfedf80b-6596-41e7-a563-ba7dd58cc476"/> | No controls | Listens to audio sent over the local network by WLED or WLED-MM and allows audio-reactive effects (♪ & ♫) to use audio data (volume and bands (FFT)) |
+| WLED Audio | <img width="100" alt="WLED Audio logo" src="https://github.com/user-attachments/assets/bfedf80b-6596-41e7-a563-ba7dd58cc476"/> | No controls | Listens to audio sent over the local network by WLED or WLED-MM and allows audio-reactive effects (♪ & ♫) to use audio data (volume and bands (FFT)) |🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 36-36: Images should have alternate text (alt text)
(MD045, no-alt-text)
[warning] 36-36: Images should have alternate text (alt text)
(MD045, no-alt-text)
[warning] 36-36: Table pipe style
Expected: leading_only; Actual: leading_and_trailing; Unexpected trailing pipe
(MD055, table-pipe-style)
[warning] 37-37: Images should have alternate text (alt text)
(MD045, no-alt-text)
[warning] 37-37: Images should have alternate text (alt text)
(MD045, no-alt-text)
[warning] 37-37: Table pipe style
Expected: leading_only; Actual: leading_and_trailing; Unexpected trailing pipe
(MD055, table-pipe-style)
[warning] 38-38: Images should have alternate text (alt text)
(MD045, no-alt-text)
[warning] 38-38: Images should have alternate text (alt text)
(MD045, no-alt-text)
[warning] 38-38: Link fragments should be valid
(MD051, link-fragments)
[warning] 38-38: Table pipe style
Expected: leading_only; Actual: leading_and_trailing; Unexpected trailing pipe
(MD055, table-pipe-style)
[warning] 39-39: Images should have alternate text (alt text)
(MD045, no-alt-text)
[warning] 39-39: Images should have alternate text (alt text)
(MD045, no-alt-text)
[warning] 39-39: Link fragments should be valid
(MD051, link-fragments)
[warning] 39-39: Table pipe style
Expected: leading_only; Actual: leading_and_trailing; Unexpected trailing pipe
(MD055, table-pipe-style)
[warning] 40-40: Images should have alternate text (alt text)
(MD045, no-alt-text)
[warning] 40-40: Table pipe style
Expected: leading_only; Actual: leading_and_trailing; Unexpected trailing pipe
(MD055, table-pipe-style)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/moonlight/drivers.md` around lines 36 - 40, The table rows for "FastLED
Driver", "FastLED Audio", "Art-Net Out", "Art-Net In", and "WLED Audio" contain
<img> tags (e.g. src="../../media/moonlight/drivers/FastLED-Driver.png",
src="../../media/moonlight/drivers/FastLED-Audio.png",
src="https://github.com/user-attachments/assets/9c65921c-64e9-4558-b6ef-aed2a163fd88",
src="../../media/moonlight/drivers/ArtNetInControls.png",
src="https://github.com/user-attachments/assets/bfedf80b-6596-41e7-a563-ba7dd58cc476")
without alt attributes; add meaningful alt text to each <img> tag (for example
alt="FastLED Driver logo", alt="FastLED Audio controls screenshot", alt="Art-Net
Out icon", alt="Art-Net In controls screenshot", alt="WLED Audio icon") so
markdownlint and assistive technologies have appropriate descriptions while
preserving existing src and sizes.
| | Human Sized Cube |  | <img width="320" alt="HCS" src="https://github.com/user-attachments/assets/de1eb424-6838-4af4-8652-89a54929bb03" /> | | | ||
| | Toronto Bar Gourds |  | <img width="320" alt="gourds" src="../../media/moonlight/layouts/CeilingCubesControls.png"/> | | | ||
| | Toronto Bar Gourds |  | <img width="320" alt="gourds" src="../../media/moonlight/layouts/BarGuords.png"/> | | | ||
| | Car Lights |  | <img width="320" alt="gourds" src="../../media/moonlight/layouts/CarLights.png"/> | | |
There was a problem hiding this comment.
Fix the Car Lights image alt text.
Line 27 still labels the controls image as "gourds", so screen readers will announce the wrong layout.
✏️ Proposed fix
-| Car Lights |  | <img width="320" alt="gourds" src="../../media/moonlight/layouts/CarLights.png"/> | |
+| Car Lights |  | <img width="320" alt="Car Lights" src="../../media/moonlight/layouts/CarLights.png"/> | |📝 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.
| | Car Lights |  | <img width="320" alt="gourds" src="../../media/moonlight/layouts/CarLights.png"/> | | | |
| | Car Lights |  | <img width="320" alt="Car Lights" src="../../media/moonlight/layouts/CarLights.png"/> | | |
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 27-27: Table pipe style
Expected: leading_only; Actual: leading_and_trailing; Unexpected trailing pipe
(MD055, table-pipe-style)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/moonlight/layouts.md` at line 27, Update the alt text for the Car Lights
controls image: locate the table row containing "Car Lights" and the <img ...
alt="gourds" src="../../media/moonlight/layouts/CarLights.png"/> tag and replace
the incorrect alt "gourds" with a descriptive alt like "Car Lights controls" (or
similar concise description) so screen readers announce the correct layout.
| const float norm = 0.1; //(audioProcessor.getEqVolumeNormFactor() > 0.000001f) ? audioProcessor.getEqVolumeNormFactor() : 1.0f; | ||
| sharedData.volume = audioProcessor.getEqVolume() / norm; | ||
| sharedData.volumeRaw = static_cast<int16_t>(sharedData.volume * 32767.0f); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "D_FastLEDAudio.h" 2>/dev/nullRepository: MoonModules/MoonLight
Length of output: 111
🏁 Script executed:
head -n 210 ./src/MoonLight/Nodes/Drivers/D_FastLEDAudio.h | tail -n 15Repository: MoonModules/MoonLight
Length of output: 757
🏁 Script executed:
# Search for sharedData structure definition
rg "struct.*sharedData|class.*sharedData" ./src/MoonLight/Nodes/Drivers/D_FastLEDAudio.h -A 20 -B 2Repository: MoonModules/MoonLight
Length of output: 47
🏁 Script executed:
# Look at the top of the file to find includes and sharedData type
head -n 50 ./src/MoonLight/Nodes/Drivers/D_FastLEDAudio.hRepository: MoonModules/MoonLight
Length of output: 1754
🏁 Script executed:
# Search for sharedData declaration and its type
rg "sharedData\." ./src/MoonLight/Nodes/Drivers/D_FastLEDAudio.h -B 5 | head -n 40Repository: MoonModules/MoonLight
Length of output: 1246
🏁 Script executed:
# Look for SharedAudioData or similar struct definition
rg "struct|class" ./src/MoonLight/Nodes/Drivers/D_FastLEDAudio.h | head -n 20Repository: MoonModules/MoonLight
Length of output: 151
🏁 Script executed:
# Search for where sharedData is declared
rg "sharedData\s*=" ./src/MoonLight/Nodes/Drivers/D_FastLEDAudio.h -B 3 -A 1Repository: MoonModules/MoonLight
Length of output: 47
🏁 Script executed:
# Get the entire file to understand the structure
wc -l ./src/MoonLight/Nodes/Drivers/D_FastLEDAudio.hRepository: MoonModules/MoonLight
Length of output: 115
🏁 Script executed:
cat -n ./src/MoonLight/Nodes/Drivers/D_FastLEDAudio.hRepository: MoonModules/MoonLight
Length of output: 10735
🏁 Script executed:
# Search for sharedData struct definition across the codebase
find ./src -type f \( -name "*.h" -o -name "*.cpp" \) -exec rg -l "sharedData" {} \;Repository: MoonModules/MoonLight
Length of output: 584
🏁 Script executed:
# Search for sharedData struct definition in Nodes.h
rg "struct.*sharedData|volumeRaw" ./src/MoonBase/Nodes.h -B 5 -A 15 | head -n 80Repository: MoonModules/MoonLight
Length of output: 724
🏁 Script executed:
# Look for getEqVolume and getEqVolumeNormFactor definitions in FastLED
rg "getEqVolume|getEqBass|getEqMid" ./src -A 3 | head -n 50Repository: MoonModules/MoonLight
Length of output: 1011
🏁 Script executed:
# Search for fl::Equalizer struct documentation and volume comments
rg "volume.*0.0.*1.0|eq.volume" ./src -B 3 -A 3Repository: MoonModules/MoonLight
Length of output: 1325
Clamp the amplified volume to prevent int16_t overflow when converting to volumeRaw.
The equalizer documentation confirms volume ranges 0.0–1.0. Dividing by 0.1 scales sharedData.volume up to 10.0, and casting 10.0 * 32767.0 (327670) to int16_t overflows beyond the maximum value of 32767.
🔧 Proposed fix
- const float norm = 0.1; //(audioProcessor.getEqVolumeNormFactor() > 0.000001f) ? audioProcessor.getEqVolumeNormFactor() : 1.0f;
- sharedData.volume = audioProcessor.getEqVolume() / norm;
- sharedData.volumeRaw = static_cast<int16_t>(sharedData.volume * 32767.0f);
+ const float norm = 0.1f; //(audioProcessor.getEqVolumeNormFactor() > 0.000001f) ? audioProcessor.getEqVolumeNormFactor() : 1.0f;
+ const float amplifiedVolume = audioProcessor.getEqVolume() / norm;
+ sharedData.volume = amplifiedVolume > 1.0f ? 1.0f : amplifiedVolume;
+ sharedData.volumeRaw = static_cast<int16_t>(sharedData.volume * 32767.0f);🤖 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 202 - 204, The
code divides audioProcessor.getEqVolume() by norm then casts to int16_t without
clamping, which can overflow; change the assignment for sharedData.volume to
compute the scaled value then clamp it to the representable range before
converting (e.g., clamp scaledVolume to a max of 1.0f or to (INT16_MAX /
32767.0f)), and then set sharedData.volumeRaw using a safe static_cast<int16_t>
from the clamped value; update the logic around norm, sharedData.volume and
sharedData.volumeRaw to enforce this clamp.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
docs/moonlight/overview.md (1)
26-26:⚠️ Potential issue | 🟡 MinorFix the plural form of "artist".
The phrase "collective of light artist" should use the plural form "light artists" since it refers to multiple people.
✏️ Proposed fix
-MoonLight is created by [MoonModules](https://moonmodules.org/). MoonModules is a collective of light artist, hardware- and software developers. We love creating DIY lighting solutions based on microcontrollers, mainly ESP32. Our work can be found on GitHub and we use mainly [Discord](https://discord.com/channels/700041398778331156/1203994211301728296) for communications. We develop our products for a large community of light enthusiasts and make products we show to the world, e.g. on art-exhibitions, in venues, or at festivals. See also [about](https://moonmodules.org/about/). +MoonLight is created by [MoonModules](https://moonmodules.org/). MoonModules is a collective of light artists, hardware- and software developers. We love creating DIY lighting solutions based on microcontrollers, mainly ESP32. Our work can be found on GitHub and we use mainly [Discord](https://discord.com/channels/700041398778331156/1203994211301728296) for communications. We develop our products for a large community of light enthusiasts and make products we show to the world, e.g. on art-exhibitions, in venues, or at festivals. See also [about](https://moonmodules.org/about/).Note: The Discord link formatting issue mentioned in a previous review has been correctly fixed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/moonlight/overview.md` at line 26, Replace the incorrect singular "light artist" with the plural "light artists" in the MoonLight overview sentence ("MoonLight is created by MoonModules..."); locate the sentence containing "collective of light artist" and update it to "collective of light artists" so the phrase correctly refers to multiple people.
🧹 Nitpick comments (1)
src/MoonBase/Nodes.h (1)
193-204: Clean refactoring of FastLED Audio fields.The transition from the old fields to
fl_-prefixed counterparts is well-organized, and expanding percussion detection into individual boolean flags (fl_hihat,fl_kick,fl_snare,fl_tom) is more expressive than the previouspercussionTypeenum.Two minor nits:
- Line 198:
float fl_bpm = 0;could be0.0ffor consistency with other float initializers.- Line 193: Consider removing the commented-out
fl_vocalsActiveif it's definitively not needed, to keep the struct clean.💅 Optional consistency fix
- float fl_bpm = 0; + float fl_bpm = 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 193 - 204, Replace the inconsistent float initializer by changing fl_bpm's default from 0 to 0.0f and remove the commented-out variable fl_vocalsActive to clean up the struct; update the declarations around fl_bpm and fl_vocalsActive in Nodes.h so all FastLED Audio fields use consistent float initializers (e.g., fl_bpm -> 0.0f) and delete the commented-out line for fl_vocalsActive if it is not needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/moonbase/inputoutput.md`:
- Around line 71-91: The new board subsections use H4 headings (e.g., "Dig-2-Go,
Dig-Uno, Dig-Quad", "Dig-Next-2", "Dig-Octa") but docs/stylesheets/moonlight.css
only styles h1–h3, so H4 renders with default Material styles; add CSS rules for
h4 (and any .page content h4 variants) to match the visual treatment of h1–h3
(font-family, size/weight, color, margins) used across the docs, ensuring
spacing and image/header alignment are consistent with existing sections.
In `@src/MoonLight/Nodes/Effects/E_MoonLight.h`:
- Around line 76-83: The palette mapping uses layer->size.x / layer->size.y as
the map max while the loops run 0..size-1, which prevents the final palette
color from ever being selected; update the map call in the E_MoonLight code path
(where colorMode == 3 || colorMode == 4) to use (layer->size.x - 1) or
(layer->size.y - 1) as the input max (keeping the conditional that chooses x vs
y) so that ColorFromPalette receives the full 0..255 range when calling
layer->setRGB(Coord3D(...), ColorFromPalette(..., map(...), brightness)).
---
Duplicate comments:
In `@docs/moonlight/overview.md`:
- Line 26: Replace the incorrect singular "light artist" with the plural "light
artists" in the MoonLight overview sentence ("MoonLight is created by
MoonModules..."); locate the sentence containing "collective of light artist"
and update it to "collective of light artists" so the phrase correctly refers to
multiple people.
---
Nitpick comments:
In `@src/MoonBase/Nodes.h`:
- Around line 193-204: Replace the inconsistent float initializer by changing
fl_bpm's default from 0 to 0.0f and remove the commented-out variable
fl_vocalsActive to clean up the struct; update the declarations around fl_bpm
and fl_vocalsActive in Nodes.h so all FastLED Audio fields use consistent float
initializers (e.g., fl_bpm -> 0.0f) and delete the commented-out line for
fl_vocalsActive if it is not needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 94de4fc1-d4cd-4319-b269-faace1f39fdf
📒 Files selected for processing (78)
docs/develop/installation.mddocs/develop/standardsguidelines.mddocs/develop/sveltekit.mddocs/gettingstarted/hardware.mddocs/gettingstarted/installer.mddocs/moonbase/inputoutput.mddocs/moonlight/overview.mdinterface/src/lib/components/moonbase/EditRowWidget.svelteinterface/src/lib/components/moonbase/FieldRenderer.svelteinterface/src/lib/components/moonbase/FileEditWidget.svelteinterface/src/lib/components/moonbase/RowRenderer.svelteinterface/src/routes/moonbase/filemanager/FileManager.sveltemisc/instructions/MoonLight.mdsrc/MoonBase/DriverNode.cppsrc/MoonBase/DriverNode.hsrc/MoonBase/LiveScriptNode.cppsrc/MoonBase/LiveScriptNode.hsrc/MoonBase/Module.cppsrc/MoonBase/Module.hsrc/MoonBase/Modules/FileManager.cppsrc/MoonBase/Modules/FileManager.hsrc/MoonBase/Modules/ModuleDevices.hsrc/MoonBase/Modules/ModuleIO.hsrc/MoonBase/Modules/ModuleTasks.hsrc/MoonBase/NodeManager.cppsrc/MoonBase/NodeManager.hsrc/MoonBase/Nodes.cppsrc/MoonBase/Nodes.hsrc/MoonBase/SharedEventEndpoint.hsrc/MoonBase/SharedFSPersistence.hsrc/MoonBase/SharedHttpEndpoint.hsrc/MoonBase/SharedWebSocketServer.hsrc/MoonBase/utilities/Char.hsrc/MoonBase/utilities/Coord3D.hsrc/MoonBase/utilities/MemAlloc.hsrc/MoonBase/utilities/PureFunctions.hsrc/MoonBase/utilities/Utilities.cppsrc/MoonBase/utilities/Utilities.hsrc/MoonBase/utilities/pal.hsrc/MoonBase/utilities/pal_espidf.cppsrc/MoonLight/Layers/PhysicalLayer.cppsrc/MoonLight/Layers/PhysicalLayer.hsrc/MoonLight/Layers/VirtualLayer.cppsrc/MoonLight/Layers/VirtualLayer.hsrc/MoonLight/Modules/ModuleChannels.hsrc/MoonLight/Modules/ModuleDrivers.hsrc/MoonLight/Modules/ModuleEffects.hsrc/MoonLight/Modules/ModuleLightsControl.hsrc/MoonLight/Modules/ModuleLiveScripts.hsrc/MoonLight/Modules/ModuleMoonLightInfo.hsrc/MoonLight/Modules/palettes.hsrc/MoonLight/Nodes/Drivers/D_ArtnetIn.hsrc/MoonLight/Nodes/Drivers/D_ArtnetOut.hsrc/MoonLight/Nodes/Drivers/D_AudioSync.hsrc/MoonLight/Nodes/Drivers/D_FastLEDAudio.hsrc/MoonLight/Nodes/Drivers/D_FastLEDDriver.hsrc/MoonLight/Nodes/Drivers/D_Hub75.hsrc/MoonLight/Nodes/Drivers/D_IMU.hsrc/MoonLight/Nodes/Drivers/D_Infrared.hsrc/MoonLight/Nodes/Drivers/D_ParallelLEDDriver.hsrc/MoonLight/Nodes/Drivers/D__Sandbox.hsrc/MoonLight/Nodes/Drivers/parlio.cppsrc/MoonLight/Nodes/Drivers/parlio.hsrc/MoonLight/Nodes/Effects/E_FastLED.hsrc/MoonLight/Nodes/Effects/E_MoonLight.hsrc/MoonLight/Nodes/Effects/E_MoonModules.hsrc/MoonLight/Nodes/Effects/E_MovingHeads.hsrc/MoonLight/Nodes/Effects/E_SoulmateLights.hsrc/MoonLight/Nodes/Effects/E_WLED.hsrc/MoonLight/Nodes/Effects/E__Sandbox.hsrc/MoonLight/Nodes/Layouts/L_MoonLight.hsrc/MoonLight/Nodes/Layouts/L_SE16.hsrc/MoonLight/Nodes/Layouts/L__Sandbox.hsrc/MoonLight/Nodes/Modifiers/M_MoonLight.hsrc/MoonLight/Nodes/Modifiers/M__Sandbox.htest/test_native/test_module.cpptest/test_native/test_nodes.cpptest/test_native/test_utilities.cpp
✅ Files skipped from review due to trivial changes (59)
- src/MoonLight/Layers/VirtualLayer.cpp
- src/MoonLight/Nodes/Drivers/D_ArtnetIn.h
- src/MoonBase/DriverNode.cpp
- src/MoonLight/Nodes/Drivers/D_AudioSync.h
- src/MoonLight/Nodes/Layouts/L_MoonLight.h
- src/MoonBase/SharedHttpEndpoint.h
- src/MoonLight/Nodes/Modifiers/M__Sandbox.h
- src/MoonBase/Modules/FileManager.h
- src/MoonBase/utilities/Coord3D.h
- src/MoonLight/Modules/ModuleEffects.h
- src/MoonBase/LiveScriptNode.cpp
- src/MoonLight/Nodes/Drivers/D_FastLEDDriver.h
- src/MoonLight/Modules/ModuleLiveScripts.h
- src/MoonBase/utilities/pal.h
- src/MoonLight/Modules/palettes.h
- src/MoonLight/Nodes/Drivers/parlio.h
- src/MoonBase/Module.cpp
- docs/develop/installation.md
- test/test_native/test_module.cpp
- src/MoonBase/utilities/Char.h
- interface/src/lib/components/moonbase/FieldRenderer.svelte
- test/test_native/test_nodes.cpp
- src/MoonLight/Nodes/Drivers/D_ArtnetOut.h
- src/MoonLight/Nodes/Effects/E__Sandbox.h
- src/MoonLight/Nodes/Effects/E_MoonModules.h
- docs/develop/sveltekit.md
- src/MoonBase/Modules/ModuleTasks.h
- src/MoonLight/Nodes/Drivers/parlio.cpp
- src/MoonBase/Modules/FileManager.cpp
- test/test_native/test_utilities.cpp
- interface/src/lib/components/moonbase/FileEditWidget.svelte
- src/MoonLight/Nodes/Effects/E_WLED.h
- interface/src/lib/components/moonbase/RowRenderer.svelte
- src/MoonBase/utilities/pal_espidf.cpp
- src/MoonLight/Nodes/Effects/E_SoulmateLights.h
- src/MoonBase/utilities/Utilities.h
- src/MoonLight/Nodes/Drivers/D_IMU.h
- src/MoonBase/utilities/Utilities.cpp
- src/MoonLight/Nodes/Drivers/D__Sandbox.h
- src/MoonBase/SharedEventEndpoint.h
- src/MoonLight/Nodes/Modifiers/M_MoonLight.h
- src/MoonLight/Layers/PhysicalLayer.h
- src/MoonLight/Nodes/Drivers/D_ParallelLEDDriver.h
- src/MoonBase/DriverNode.h
- src/MoonBase/Nodes.cpp
- interface/src/lib/components/moonbase/EditRowWidget.svelte
- src/MoonBase/NodeManager.h
- src/MoonLight/Modules/ModuleMoonLightInfo.h
- src/MoonLight/Nodes/Drivers/D_Hub75.h
- src/MoonLight/Modules/ModuleDrivers.h
- src/MoonLight/Nodes/Layouts/L_SE16.h
- src/MoonBase/utilities/PureFunctions.h
- src/MoonBase/SharedWebSocketServer.h
- src/MoonBase/SharedFSPersistence.h
- src/MoonBase/utilities/MemAlloc.h
- interface/src/routes/moonbase/filemanager/FileManager.svelte
- src/MoonLight/Modules/ModuleLightsControl.h
- src/MoonBase/NodeManager.cpp
- src/MoonBase/Module.h
🚧 Files skipped from review as they are similar to previous changes (1)
- misc/instructions/MoonLight.md
| #### Dig-2-Go, Dig-Uno, Dig-Quad | ||
|
|
||
| {: style="width: 200px"} | ||
| {: style="width: 200px"} | ||
| {: style="width: 200px"} | ||
|
|
||
| * [Dig-2-Go](https://quinled.info/quinled-dig2go/), [Dig-Uno](https://quinled.info/pre-assembled-quinled-dig-uno/), [Dig-Quad](https://quinled.info/pre-assembled-quinled-dig-quad/): Choose the esp32-d0 (4MB) board in the [MoonLight Installer](../../gettingstarted/installer/) | ||
| * Dig-2-Go: Shipped with a 300 LED, GRBW led strip: Choose layout with 300 lights (e.g. Single Column for 1D, Panel 15x20 for 2D). Select Light preset GRBW in the LED Driver. | ||
| * Currently no OTA support on ESP32-D0 boards: Dig-2-Go, Uno, Quad. | ||
|
|
||
| #### Dig-Next-2 | ||
|
|
||
| {: style="width: 200px"} | ||
|
|
||
| * [Dig-Next-2](https://quinled.info/dig-next-2): Choose the esp32-d0-pico2 board in the [MoonLight Installer](../../gettingstarted/installer/) | ||
|
|
||
| #### Dig-Octa | ||
|
|
||
| {: style="width: 200px"} | ||
|
|
||
| * [Dig-Octa](https://quinled.info/quinled-dig-octa/): Choose the esp32-d0-16mb board in the [MoonLight Installer](../../gettingstarted/installer/) |
There was a problem hiding this comment.
Style the new #### sections before merging.
These new board subsections rely on H4 headings, but docs/stylesheets/moonlight.css still only customizes h1-h3. They will render as the default Material h4, which makes this page visually inconsistent with the rest of the docs.
🎨 Suggested follow-up in `docs/stylesheets/moonlight.css`
+.md-content h4 {
+ color: var(--moonlight-accent);
+ font-weight: 600;
+ font-size: 0.95rem;
+}Also applies to: 100-135
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/moonbase/inputoutput.md` around lines 71 - 91, The new board subsections
use H4 headings (e.g., "Dig-2-Go, Dig-Uno, Dig-Quad", "Dig-Next-2", "Dig-Octa")
but docs/stylesheets/moonlight.css only styles h1–h3, so H4 renders with default
Material styles; add CSS rules for h4 (and any .page content h4 variants) to
match the visual treatment of h1–h3 (font-family, size/weight, color, margins)
used across the docs, ensuring spacing and image/header alignment are consistent
with existing sections.
|
@coderabbitai, in D_FastLEDDriver.h there is addControl(temperature, "temperature", "select"); The other "select" values are shown as dropdowns. Do you know why? Please look at the implementation of addControl |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me inspect the relevant files to understand the issue. 🏁 Script executed: Length of output: 210777 I have the FieldRenderer and addControl logic — let me dig into 🏁 Script executed: Length of output: 22842
Root Cause:
|
| Step | Call | Last element |
|---|---|---|
| 1 | DriverNode::setup() → finds lightPreset in-place |
status |
| 2 | addControl(affinity, ...) → finds affinity in-place |
status |
| 3 | addControlValue("Auto") |
→ appended to status ❌ |
| 4 | addControl(engine, ...) → finds engine in-place |
status |
| 5 | addControl(temperature, ...) → finds temperature in-place |
status |
| 6 | addControlValue("Uncorrected") |
→ appended to status ❌ |
Temperature ends up with no values array. The same applies to affinity and correction.
Why <span>0</span> instead of an empty <select>
The <span>0</span> is only rendered when property.ro == true in FieldRenderer.svelte. Notice that engine is defined with ro=true:
addControl(engine, "engine", "text", 0, 32, true); // ro=truesetupControl in Nodes.cpp does clean up ro when ro=false:
else if (!control["ro"].isNull())
control.remove("ro");However, if setupControl is only called for new controls (the full addControl template was cut off in the source — I couldn't confirm), then an existing temperature control coming from saved state that somehow has a stale "ro": true would never get it cleared.
Quick diagnostic: check the saved node state file (e.g. .config/nodes.json) — look at the temperature control object. Does it have "ro": true?
The fix
The fragile pattern in Node::addControlValue (implicit "last element" targeting) is the underlying problem. The Module class already uses the explicit form:
// Module.h — explicit, safe version
void addControlValue(const JsonObject& control, const T& value) { ... }Option A — store the last addControl result in a member variable and use it in addControlValue:
// In Nodes.h, add a member:
JsonObject _lastControl;
// In addControl (after findOrCreateControl):
_lastControl = control;
// In addControlValue:
template <typename T>
void addControlValue(const T& value) {
if (_lastControl.isNull()) return;
if (_lastControl["values"].isNull()) _lastControl["values"].to<JsonArray>();
_lastControl["values"].as<JsonArray>().add(value);
}Option B — clear the values array at the start of each addControl for "select" type, so re-setup always rebuilds them:
In setupControl, add:
if (strcmp(type, "select") == 0)
control.remove("values"); // will be re-added by addControlValueThis ensures both first-setup and re-setup produce the same result.
Option A is safer because it doesn't depend on ordering assumptions at all, and matches the design intent of the API.
🧠 Learnings used
Learnt from: CR
Repo: MoonModules/MoonLight PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-03T20:55:04.582Z
Learning: Applies to {E_*.h,D_*.h,L_*.h,M_*.h} : Nodes define their own UI controls via `addControl(variable, name, type, ...)` which are stored as `JsonArray` and rendered generically in the UI
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 136
File: src/MoonBase/Modules/ModuleDevices.h:148-148
Timestamp: 2026-03-04T10:07:41.583Z
Learning: In MoonModules/MoonLight, the `// 🌙` (MoonBase addition) and `// 💫` (MoonLight addition) comment markers are ONLY required when modifying upstream SvelteKit files (i.e., files that originate from the upstream ESP32-sveltekit project). Do NOT require or suggest these markers for MoonBase-specific or MoonLight-specific files (such as `src/MoonBase/Modules/ModuleDevices.h` or other files in `src/MoonBase/` or `src/MoonLight/`). The markers indicate where upstream files have been patched.
|
@coderabbitai, regarding if I look at the json saved for the FastLED driver node, it looks okay: {"name":"FastLED Driver ☸️","on":true,"controls":[{"name":"lightPreset","value":2,"default":2,"p":1070216889,"type":"select","valid":true,"size":8,"values":["RGB","RBG","GRB","GBR","BRG","BGR","RGBW","GRBW","WRGB","Curtain GRB6","Curtain RGB2040","Lightbar RGBWYP","MH BeeEyes 150W-15","MH BeTopper 19x15W-32","MH 19x15W-24"]},{"name":"affinity","value":0,"default":0,"p":1009251389,"type":"select","valid":true,"size":8,"values":["Auto","RMT","I2S","SPI"]},{"name":"engine","value":"RMT","default":"Auto","p":1009251357,"type":"text","valid":true,"ro":true,"max":32,"size":32},{"name":"temperature","value":0,"default":0,"p":1009251390,"type":"select","valid":true,"size":8,"values":["Uncorrected","Candle","Tungsten100W","Halogen"]},{"name":"correction","value":0,"default":0,"p":1009251391,"type":"select","valid":true,"size":8,"values":["Uncorrected","Typical LED","Typical SMD5050"]},{"name":"dither","value":false,"default":false,"p":1009251392,"type":"checkbox","valid":true,"size":1},{"name":"version","value":"4.0 pre release! 20260307","default":"20260307","p":1009251293,"type":"text","valid":true,"ro":true,"max":20,"size":32},{"name":"status","value":"#1: 16#256","default":"NoInit","p":1009251325,"type":"text","valid":true,"ro":true,"max":32,"size":32}]} Could it be something in the FieldRenderer.svelte which is not handling the select, after the read only engine control? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainGood question — since the JSON is correct, the bug must be in the frontend rendering. Let me inspect 🏁 Script executed: Length of output: 14406 The 🏁 Script executed: Length of output: 12187
The BugIn the <select bind:value onchange={onChange} class="select">
{`#each` property.values as value, index (index)} <!-- ⚠️ shadows outer `value` prop! -->
<option value={property.type == 'selectFile' ? value : index}>
{value}
</option>
{/each}
</select>The The FixRename the {:else if property.type == 'select' || property.type == 'selectFile'}
<select bind:value onchange={onChange} class="select">
{`#each` property.values as optionLabel, index (index)}
<option value={property.type == 'selectFile' ? optionLabel : index}>
{optionLabel}
</option>
{/each}
</select>The same rename pattern applies to the Why
|
|
@coderabbitai, in FieldRenderer.svelte, when I add {console.log('select render', property.name, property.type, property.ro, typeof property.ro)} [Log] select render – "on" – "checkbox" – undefined – "undefined" (chunk-VJI7A3TX.js, line 3853) which says temperature read only is true. If I then say {console.log('select render', property)}; It shows in the snapshot that property.ro is true, but in the non-snapshot there is no ro value (which is right). Why in the snapshot ro = true? 
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 17021 🏁 Script executed: Length of output: 3251 🏁 Script executed: Length of output: 3428 🏁 Script executed: Length of output: 151 🏁 Script executed: Length of output: 25255 🏁 Script executed: Length of output: 8594
The BugThe image shows two critical clues:
The different How
|
| WS cycle | oldArr[3] object |
newArr[3] data |
Result |
|---|---|---|---|
| Before re-setup | temperature |
engine (ro:true, max:32) |
ro:true written onto temperature object ✓ (wrong!) |
| After re-setup | stale temperature w/ ro:true |
temperature (no ro) |
ro NOT removed (pruneMissing=false) → persists ✗ |
The Fix
In updateRecursive's array-item merge, detect when p (or name) has changed — meaning a completely different control now occupies this slot — and replace the entire object instead of merging:
// Module.svelte — updateRecursive, inside the array loop
if (bothObjects) {
const oldObj = oldItem as Record<string, unknown>;
const newObj = newItem as Record<string, unknown>;
// If 'p' or 'name' changed, this is a different control in the same slot → replace entirely
if (
(newObj.p !== undefined && oldObj.p !== newObj.p) ||
(newObj.name !== undefined && oldObj.name !== newObj.name)
) {
oldArr[i] = newItem; // Full replacement — no stale key bleed
} else {
updateRecursive(oldObj, newObj, pruneMissing);
}
}This is the minimal, targeted fix. It prevents cross-contamination when a control slot changes identity (different p = different C++ variable address = re-setup or different node) while preserving the existing partial-patch behavior for controls that are genuinely being updated in place.
Note: The
{#each}key inEditRowWidget.svelte(control.p ?? control.name) is correct for Svelte DOM reconciliation, but it doesn't help with the upstream store merge —updateRecursiveruns before Svelte sees the data. The fix must be inupdateRecursive.
✏️ Learnings added
Learnt from: ewowi
URL: https://github.com/MoonModules/MoonLight/pull/140
Timestamp: 2026-03-08T10:02:02.358Z
Learning: In MoonModules/MoonLight (interface/src/routes/moonbase/module/Module.svelte), the `updateRecursive` function merges incoming WebSocket/HTTP state into the Svelte `$state` store by array INDEX (not by control `name` or `p`). With `pruneMissing=false` (the default for WS partial updates), keys absent in new data are never deleted from old objects. This causes stale properties (like `ro: true`) from one control to persist on a different control when the controls array changes length/order between node setup cycles. The fix is to replace array items entirely when their `p` (pointer) or `name` changes, rather than merging.
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: 136
File: src/MoonBase/Modules/ModuleDevices.h:148-148
Timestamp: 2026-03-04T10:07:41.583Z
Learning: In MoonModules/MoonLight, the `// 🌙` (MoonBase addition) and `// 💫` (MoonLight addition) comment markers are ONLY required when modifying upstream SvelteKit files (i.e., files that originate from the upstream ESP32-sveltekit project). Do NOT require or suggest these markers for MoonBase-specific or MoonLight-specific files (such as `src/MoonBase/Modules/ModuleDevices.h` or other files in `src/MoonBase/` or `src/MoonLight/`). The markers indicate where upstream files have been patched.
|
@coderabbitai, when MoonLight is in AP mode, it opens a captive portal. On my Macbook, the web sockets are not active in the captive portal. If I open the UI in a normal browser, using 4.3.2.1 web sockets do work, what could be the problem? Sidely related. If I use the Parallel LED Driver , the web socket response is very fast (noticeable in the Monitor which uses the web socket to display LEDs in a preview). When I select the FastLED driver, the Monitor becomes unresponsive, about 1 fps. It varied a bit in the last weeks ,depending on which commit of FastLED master I took (it is under heavy development). It varied from full responsive web sockets a few weeks ago, to complete freeze a week ago, now it is about 1fps. This looks like an indirect effect, e.g. FastLED driver using more resources, or maybe more dma / interrupts, interfering with web sockets. Could you analyse both problems and tell if related |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain
Problem 1: WebSockets not working in the macOS captive portalThis is expected Apple behavior, not a MoonLight bug. The Captive Network Assistant (CNA) "mini-browser" is not a real Safari process — it's a very stripped-down web view with no developer hooks. For ESP32-based web UIs, the specific known limitations in the CNA are: JavaScript is limited, and WebSocket is disabled. The suggested solution is to detect when the page is loaded via So the user experience in AP mode should be:
Actionable fix in MoonLight: Detect the CNA context and show a redirect message. The URL check to detect CNA on macOS is whether the page was requested from Problem 2: FastLED driver causing ~1fps WebSocket MonitorRoot cause: RMT interrupt contention with the WiFi/lwIP stack. The WiFi stack's interrupt handling disturbs the signal timing of RMT-based LED drivers. This is a well-known ESP32 problem — all LED tasks need to be pinned to Core 1 due to interference from BLE and WiFi on Core 0. The specific mechanism: FastLED's RMT5 driver (used in FastLED v4 pre-release) fires frequent ISR callbacks to stream LED data. These ISRs compete with the WiFi/lwIP interrupt budget. When FastLED is pushing many LEDs at high frame rates, the WiFi AMPDU/TCP ACK processing gets delayed, which backs up the httpd task's ability to send WebSocket frames — degrading your Monitor to ~1fps. Your observation that the degree varies by FastLED commit is explained by ongoing RMT5 implementation changes in FastLED master (it's "under heavy development" as you noted). From the existing learning recorded in this project, the commit Mitigation strategies to investigate (in order of invasiveness):
Are the two problems related?No, they have separate causes:
The symptom (WebSockets not working) is superficially the same, but the root causes and fixes are completely independent. 🏁 Script executed: Length of output: 31758 🏁 Script executed: Length of output: 23289 🌐 Web query: 💡 Result: macOS’s captive-portal pop-up (“Captive Network Assistant”, CNA) is not a full Safari session—it’s a stripped/restricted mini-browser intended only to complete network login. In practice it has notable web-platform limitations (vendors document missing/disabled JS behaviors, and academic/industry analyses describe “limited JS features” and other restrictions compared to regular browsers). (allenchou.cc) Because of those restrictions, you should not depend on WebSockets in the CNA window for critical portal functionality (many real-world captive-portal flows already break in CNA even without WebSockets—e.g., complex federated login/redirect flows like SAML are reported as incompatible with the native macOS captive-portal window). (sonicwall.com) What to do instead (recommended captive-portal design):
How to detect CNA (commonly used):
Citations:
Problem 1: WebSockets not working in the macOS captive portalThis is expected Apple OS behavior — not a MoonLight bug. macOS's Captive Network Assistant (CNA) is a stripped/restricted mini-browser intended only to complete network login, not a full Safari session. WebSockets are not supported in it by design. This is why opening Actionable fix: Detect the CNA context on the server side and serve a redirect/info page instead of the full SPA. CNA requests can be identified via a User-Agent containing <meta http-equiv="refresh" content="0; url=http://4.3.2.1">
<p>Please open <a href="http://4.3.2.1">http://4.3.2.1</a> in your browser.</p>Apple's CNA also probes Problem 2: FastLED driver causing ~1fps WebSocket MonitorRoot cause: Looking at void loop() override {
if (FastLED.count()) {
// ... brightness/power checks ...
FastLED.show(); // ← called every single driver loop iteration, no rate limit
}
}
Your observation that it varies by FastLED commit is consistent with ongoing RMT5 changes in FastLED master — different commits have different ISR call patterns. Mitigations (in order of invasiveness)1. Try 2. Add a frame rate cap in void loop() override {
if (FastLED.count()) {
static uint32_t lastShow = 0;
uint32_t now = millis();
if (now - lastShow < 16) return; // cap at ~60fps
lastShow = now;
// ... brightness/power checks ...
FastLED.show();
}
}3. Are the two problems related?No — separate root causes:
🧠 Learnings used |
firmware ======= - pio.ini: todays FastLED , yesterdays I2SClocklessLedDriver frontend ======= - FieldRenderer: fix in shadowing value in select - Module: make sure previous controls do not bleed in into the next backend ======= - DriverNode: setGamma arguments in RGB order - Nodes: addControlValue to .cpp - SolidEffect: tweak palette rows/cols
|
@coderabbitai, regarding
I want Leds burning at the max frame rate. This is not an issue when using the parallel led driver, why is it with FastLED? I tried switching to I2S but same behavior. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me look at both driver implementations in detail and check for FastLED v4 WiFi coexistence issues.
Why D_ParallelLEDDriver is fast and FastLED is notD_ParallelLEDDriver uses MoonLight's own FastLED (both RMT and I2S) fires ISR callbacks for every completed LED data segment. The critical difference is which flavour of RMT is active: FastLED's custom RMT4 driver is specifically optimized for LED control and significantly outperforms Espressif's generic RMT5 wrapper in terms of: lower interrupt overhead and better timing precision; WiFi resistance via a dual-buffer design that prevents flickering under WiFi load; and direct hardware control vs. abstraction layer overhead. Since MoonLight uses IDF ≥ 5, on IDF ≥ 5, RMT5 is enabled by default. RMT5 is Espressif's generic For I2S, there's an additional fundamental issue: the combination of I2S clockless parallel driver with WiFi running are incompatible. The guess is that the WiFi stack generates a ton of interrupts and this disturbs the clock for the WS2812 protocol output on I2S. Why switching to I2S didn't helpIt's the inverse of the LED glitch problem — WiFi interrupts disturb I2S LED output, and I2S's constant DMA refill ISRs disturb WiFi throughput. For improved resilience under interrupt load (e.g., WiFi activity), you can increase the number of I2S DMA buffers by defining The fix: force RMT4Using RMT4 is strongly recommended if you experience flickering with WiFi enabled or need maximum LED performance. Add build_flags =
...
-D FASTLED_RMT5=0 ; Force RMT4 for WiFi coexistenceNote: The RMT4 and RMT5 drivers cannot co-exist in the same binary. Mixing both (e.g., using IDF v5 led_strip alongside FastLED's RMT4) will trigger an Espressif runtime abort at startup. So ensure no other library in your dependency chain pulls in the IDF 🏁 Script executed: Length of output: 43896 🏁 Script executed: Length of output: 43101 🌐 Web query: 💡 Result:
Sources: [1] FastLED repo documentation on ESP32 RMT4 vs RMT5, performance/WiFi notes, and build flag guidance. (github.com) [2] FastLED Issue Citations:
Why PARLIO doesn't degrade WebSockets but FastLED doesThe difference is architectural — it's not about frame rate, it's about who drives the transmission. PARLIO: pure fire-and-forget DMAIn
After step 3, the CPU is completely free. The PARLIO peripheral streams the entire frame autonomously via hardware DMA — no ISR, no callback, no CPU involvement until the next frame. WiFi interrupts run unimpeded. FastLED RMT5/I2S: ISR-driven with a hard priority ceilingRMT5 is FastLED's wrapper around Espressif's I2S has the same fundamental problem — it uses a DMA refill ISR model that fires continuously during transmission, at similarly low priority on IDF5. The fix: force RMT4RMT4 is FastLED's own legacy/optimized RMT driver with direct peripheral control, lower interrupt overhead, and explicit WiFi resistance — it is generally better for cases where WiFi is busy and LEDs must not glitch. Add to build_flags =
-D FASTLED_RMT5=0 ; Force FastLED RMT4 (WiFi-resistant dual-buffer design)Critical constraint: RMT4 and RMT5 cannot coexist in the same binary — linking both causes a startup abort/crash, so you must pick one at compile time. Ensure no other dependency in your build pulls in Espressif's Note: The ✏️ Learnings added
🧠 Learnings used |
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)
platformio.ini (1)
220-227:⚠️ Potential issue | 🟠 MajorAdd
-D FASTLED_RMT5=0to mitigate WebSocket/WiFi throughput degradation.The PR comments explicitly mention "FastLED driver causing slow/laggy WebSocket Monitor: high-frequency RMT ISRs from FastLED compete with WiFi/lwIP." Based on learnings, FastLED RMT5 (default on IDF5+) lacks the dual-buffer WiFi-resistance design of RMT4, causing this exact issue. The fix is to add
-D FASTLED_RMT5=0to the[moonlight]build_flagsto force RMT4's WiFi-resistant design.🔧 Proposed fix to add FASTLED_RMT5=0 flag
[moonlight] build_flags = -D FT_MOONLIGHT=1 -D FT_MONITOR=1 + -D FASTLED_RMT5=0 -D EFFECTS_STACK_SIZE=3072 ; psramFound() ? 4 * 1024 : 3 * 1024 -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)Based on learnings: In MoonModules/MoonLight D_FastLEDDriver: FastLED RMT5 (default on IDF >= 5) lacks the dual-buffer WiFi-resistance design of FastLED RMT4, causing WebSocket/WiFi throughput degradation at high LED frame rates. Fix: add
-D FASTLED_RMT5=0to platformio.ini for IDF5-based boards to force RMT4's dual-buffer design. This flag may need to be re-enabled even when a specific FastLED commit appeared stable, as RMT5 behavior varies across FastLED master commits.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platformio.ini` around lines 220 - 227, Add the FASTLED_RMT5=0 build flag to the build_flags block to force FastLED to use RMT4 (WiFi-resistant dual-buffer design) on IDF5-based boards; update the existing build_flags entries (the block containing FT_MOONLIGHT, FT_MONITOR, EFFECTS_STACK_SIZE, DRIVERS_STACK_SIZE, FASTLED_BUILD) by inserting -D FASTLED_RMT5=0 so the compiler defines FASTLED_RMT5 as 0 during builds and mitigates RMT5-related WiFi/WebSocket throughput degradation.
♻️ Duplicate comments (1)
src/MoonLight/Nodes/Drivers/D_FastLEDAudio.h (1)
185-196:⚠️ Potential issue | 🟠 MajorDrain the full audio buffer each
loop().Line 194 processes at most one
AudioSampleper pass even though this driver is configured for a 44.1 kHz input stream. That leaves the buffer mostly unprocessed and skews the EQ/beat/BPM state under normal render cadences.Proposed fix
- fl::AudioSample sample = audioInput->read(); - if (sample.isValid()) { - audioProcessor.update(sample); - } + while (fl::AudioSample sample = audioInput->read()) { + if (sample.isValid()) { + audioProcessor.update(sample); + } + }Run this to confirm the producer/consumer mismatch:
#!/bin/bash set -euo pipefail echo "=== Producer configuration ===" rg -n -C2 'AudioConfigI2S|44100' src/MoonLight/Nodes/Drivers/D_FastLEDAudio.h || true echo echo "=== Current consumer logic ===" rg -n -C2 'audioInput->read\(|audioProcessor\.update\(' src/MoonLight/Nodes/Drivers/D_FastLEDAudio.h || true echo echo "=== Loop orchestration context ===" rg -n -C2 --max-count 40 'loopDrivers|->loop\(|virtual void loop\(\)|vTaskDelay|getFps' src/MoonLight src/MoonBase || trueExpected: the file shows a 44.1 kHz producer but only a single
read()per driver loop.🤖 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 185 - 196, The driver currently calls audioInput->read() once per loop and processes at most one fl::AudioSample via audioProcessor.update(), causing buffered samples to be dropped; change the logic in D_FastLEDAudio (where audioInput->read() and audioProcessor.update() are used) to repeatedly read and process samples until the input indicates no more valid samples (i.e., loop on read() while samples are valid) so the full FastLED audio buffer is drained each frame; ensure the loop remains non-blocking and preserves existing error/timeout behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/moonlight/overview.md`:
- Line 26: Replace the Discord channel deep-link URL
"/channels/700041398778331156/1203994211301728296" with the public invite URL
used elsewhere in the doc (the invite referenced on Line 121) so new readers can
join; locate the Discord link in the MoonLight overview paragraph (the anchor
text "Discord") and update its href to the invite URL while keeping the visible
link text unchanged.
In `@src/MoonBase/Nodes.cpp`:
- Around line 68-75: addControlValue() currently appends values to
controls.back() instead of the JsonObject returned by the preceding
addControl(), which causes rebuilt non-last controls to get their options pushed
into the last control (especially after control.remove("values") was
introduced). Fix by using the same JsonObject handle produced/returned by
addControl() (or cache that JsonObject reference when rebuilding) and append to
that object's "values" JsonArray (using the existing JsonObject/JsonArray
methods) rather than re-fetching controls[controls.size()-1]; ensure
addControlValue() operates on the exact JsonObject instance you rebuilt so each
control gets its own values.
---
Outside diff comments:
In `@platformio.ini`:
- Around line 220-227: Add the FASTLED_RMT5=0 build flag to the build_flags
block to force FastLED to use RMT4 (WiFi-resistant dual-buffer design) on
IDF5-based boards; update the existing build_flags entries (the block containing
FT_MOONLIGHT, FT_MONITOR, EFFECTS_STACK_SIZE, DRIVERS_STACK_SIZE, FASTLED_BUILD)
by inserting -D FASTLED_RMT5=0 so the compiler defines FASTLED_RMT5 as 0 during
builds and mitigates RMT5-related WiFi/WebSocket throughput degradation.
---
Duplicate comments:
In `@src/MoonLight/Nodes/Drivers/D_FastLEDAudio.h`:
- Around line 185-196: The driver currently calls audioInput->read() once per
loop and processes at most one fl::AudioSample via audioProcessor.update(),
causing buffered samples to be dropped; change the logic in D_FastLEDAudio
(where audioInput->read() and audioProcessor.update() are used) to repeatedly
read and process samples until the input indicates no more valid samples (i.e.,
loop on read() while samples are valid) so the full FastLED audio buffer is
drained each frame; ensure the loop remains non-blocking and preserves existing
error/timeout behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8e37cd72-5c5e-4482-a5f9-0fc9ce9ea292
📒 Files selected for processing (11)
docs/moonlight/overview.mdinterface/src/lib/components/moonbase/FieldRenderer.svelteinterface/src/routes/moonbase/module/Module.sveltelib/framework/WWWData.hplatformio.inisrc/MoonBase/DriverNode.cppsrc/MoonBase/Nodes.cppsrc/MoonBase/Nodes.hsrc/MoonLight/Nodes/Drivers/D_FastLEDAudio.hsrc/MoonLight/Nodes/Drivers/D_FastLEDDriver.hsrc/MoonLight/Nodes/Effects/E_MoonLight.h
🚧 Files skipped from review as they are similar to previous changes (1)
- src/MoonBase/DriverNode.cpp
| void Node::addControlValue(const char* value) { | ||
| if (controls.size() == 0) return; // guard against empty controls | ||
| JsonObject control = controls[controls.size() - 1]; // last control | ||
| if (control["values"].isNull()) control["values"].to<JsonArray>(); // add array of values | ||
| JsonArray values = control["values"]; | ||
| values.add(value); | ||
| EXT_LOGD(ML_TAG, "%s, %d", value, control["values"].size()); | ||
| } |
There was a problem hiding this comment.
addControlValue() still targets the wrong control on re-setup.
Line 70 appends to controls.back(), not to the control returned by the preceding addControl(). Combined with the control.remove("values") change in src/MoonBase/Nodes.h, a rebuilt non-last select will keep its own values empty and push the options into whichever control happens to be last. Please append to the JsonObject being rebuilt (or cache that handle) instead of looking up the last array element.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/MoonBase/Nodes.cpp` around lines 68 - 75, addControlValue() currently
appends values to controls.back() instead of the JsonObject returned by the
preceding addControl(), which causes rebuilt non-last controls to get their
options pushed into the last control (especially after control.remove("values")
was introduced). Fix by using the same JsonObject handle produced/returned by
addControl() (or cache that JsonObject reference when rebuilding) and append to
that object's "values" JsonArray (using the existing JsonObject/JsonArray
methods) rather than re-fetching controls[controls.size()-1]; ensure
addControlValue() operates on the exact JsonObject instance you rebuilt so each
control gets its own values.
|
@coderabbitai, I would like to change the text on the welcome screen in +page.svelte: Welcome to MoonLight I was thinking to base it on "MoonLight is open-source software that lets you control a wide range of DMX and LED lights using ESP32 microcontrollers, for home, for artists, and for stages." And came up with Control a wide range of DMX and LED lights, for home, for artists, and for stages. But still not happy, can you make something better. Should be short and cover what MoonLight is |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 2181
Option A — Identity-first (recommended):
Covers: open-source (credibility), LED+DMX (scope), audience — in one clean sentence. Option B — Feature-forward:
Highlights the audio-reactive magic but gets a bit long for the card width. Option C — Evocative / poetic:
Shortest, emotional. Works well with the logo but says nothing technical. Option D — Closest to your draft, tightened:
The repeated "for" creates a nice rhythm and the parallelism reads well on three short lines if it wraps. My recommendation is Option A — it's factually precise, fits the narrow card, and "By MoonModules" can be dropped entirely since the logo and all the community links already establish the brand. The diff would be: <p class="py-6 text-center">
- <b>Effect controller for direct and networked LEDs and lights. By MoonModules.</b>
+ <b>Open-source LED & DMX controller for home, artists, and stages.</b>
</p>🧠 Learnings used |
FastLED Audio - signalConditioning default false - add drain buffer, default false - remove autoGain (default on in FA)
|
@coderabbitai, I am about to merge this PR, please do an in-depth review of all the commits made, make a summary, a recommendation to merge and a list of possible future actions. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 12484
SummaryThis PR spans 6 commits, 60 files changed across firmware, frontend, backend, and documentation. It resolves several bugs discovered during this review cycle and prepares the codebase for v0.9.0. 🐛 Bug Fixes (Functional)
🔊 FastLED Audio Refactor (
|
| Feature | File |
|---|---|
| SolidEffect "Palette rows" and "Palette cols" controls | E_MoonLight.h |
| colorMode 3/4 for palette-mapped 3D layer iteration | E_MoonLight.h |
| PARLIO affinity option extended to ESP32-P4 and ESP32-C6 | D_FastLEDDriver.h |
| Car Lights layout added; Bar Gourds assets updated | docs/moonlight/layouts.md |
| FastLED Audio Driver entry in drivers docs | docs/moonlight/drivers.md |
📦 Dependencies
- FastLED pinned to
47f0a2c(2026-03-08) — latest in a tracked chain of commits - I2SClocklessLedDriver updated to
4548eee(2026-03-07) — includessetGammaargument order fix and semaphore wait log improvements (complements theDriverNode.cppgamma fix) DRIVERS_STACK_SIZEincreased to 6144 to accommodate FastLED Audio in the driver task
📝 Documentation & Housekeeping
h4CSS rule added tomoonlight.cssusingvar(--moonlight-primary)— resolves the inconsistent heading rendering- QuinLED board names hyphenated throughout (
Dig-2-Go,Dig-Next-2,Dig-Uno v3,Dig-Quad v3,Dig-Octa v2) LightCrafter16capitalization correctedGitHubcapitalization fixed across ~40 files- AI Principles in
standardsguidelines.mdexpanded with Attribution and Documentation mkdocs.ymlsite description,README.md, anddocs/index.mdupdated with consistent open-source / home-artists-stages messaging
✅ Recommendation: Merge
All functional bugs discussed during this PR's review cycle are addressed and verified. The FieldRenderer shadowing fix, the updateRecursive identity-replace fix, the gamma argument order fix, and the addControlValue re-setup fix are all in place and correct. The FastLED Audio refactor is architecturally sound (callback path preserved as comments). Dependencies are updated and tracked. Documentation is consistent. The PR is in a mergeable state.
🔭 Future Actions
| Priority | Action |
|---|---|
| 🔴 High | WebSocket slowdown with FastLED Driver: add -D FASTLED_RMT5=0 to platformio.ini for IDF5 boards to force FastLED RMT4's dual-buffer WiFi-resistant design. Current commit (20260308) still uses RMT5 — the 1fps Monitor issue is unresolved for FastLED Driver users |
| 🔴 High | +page.svelte welcome text: update from "Effect controller for direct and networked LEDs and lights. By MoonModules." to the agreed text (e.g. "Open-source LED & DMX controller for home, artists, and stages.") — discussed in this PR's comments but not yet committed |
| 🟡 Medium | Captive portal CNA redirect: add a handler that intercepts requests with CaptiveNetworkSupport User-Agent and responds with a redirect page to http://4.3.2.1, so macOS users in AP mode are guided to open a real browser |
| 🟡 Medium | addControlValue robustness (Option A): the current Option B approach (clearing values on re-setup) is functional but fragile if addControl / addControlValue call order is non-sequential. Tracking the last addControl result in a member (_lastControl) would be more explicit |
| 🟡 Medium | FastLED Audio callback stability: monitor whether FastLED v4 audio callbacks stabilise — if they do, consider re-enabling onBeat, onBass etc. and removing the per-frame polling workaround for lower latency audio response |
| 🟢 Low | Frame rate cap in D_FastLEDDriver::loop(): even with RMT4, a configurable cap (e.g. exposed as a control, default 60fps) would help users with many LEDs avoid unnecessary CPU/interrupt pressure |
| 🟢 Low | h4 color review: current h4 uses var(--moonlight-primary) (same purple as h3). Consider switching to var(--moonlight-accent) (pink) or var(--moonlight-blue) to create a clearer visual hierarchy between h3 and h4 |
🧠 Learnings used
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2026-03-08T10:02:02.358Z
Learning: In MoonModules/MoonLight (interface/src/routes/moonbase/module/Module.svelte), the `updateRecursive` function merges incoming WebSocket/HTTP state into the Svelte `$state` store by array INDEX (not by control `name` or `p`). With `pruneMissing=false` (the default for WS partial updates), keys absent in new data are never deleted from old objects. This causes stale properties (like `ro: true`) from one control to persist on a different control when the controls array changes length/order between node setup cycles. The fix is to replace array items entirely when their `p` (pointer) or `name` changes, rather than merging.
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2026-01-28T12:08:02.954Z
Learning: MoonLight NodeManager (src/MoonBase/NodeManager.h): After switching to direct onUpdate in httpd, compareRecursive continues with its captured newData and can emit controls updates immediately after a node rename. To avoid spurious control updates, NodeManager::onUpdate must (1) guard controls array/index, and (2) skip when the current control property already equals updatedItem.value. Only "value" and "default" should drive nodeClass->updateControl/onUpdate; other control fields are UI metadata and can be ignored or just trigger requestUIUpdate.
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2026-01-28T12:18:44.193Z
Learning: MoonLight NodeManager controls updates: After rename, compareRecursive continues with captured newData and will emit controls.* changes. Ensure NodeManager::onUpdate:
- Applies only controls.value/default to the node (updateControl + onUpdate), guarded by equality against current control state to avoid re-applying after rename.
- Propagates all other controls metadata (name/min/max/type/valid/size/…) to state/UI and triggers requestUIUpdate, but does not call nodeClass->updateControl for these metadata fields.
This prevents spurious back-application while keeping the UI in sync with the new control schema.
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2026-02-27T10:36:05.408Z
Learning: In MoonModules/MoonLight Module/Node onUpdate: oldValue != "" and originId.toInt() serve different, non-overlapping purposes: (1) oldValue != "" detects first-time initialization—oldValue is JsonVariant() (null/unset, converts to "") only when a control is brand new; for numbers, oldValue==0 converts to "0" (not ""), so the check correctly identifies boot/new controls across all types. (2) originId.toInt() distinguishes UI updates (numeric client IDs like "1", "2") from backend/system updates (module names like "module", "ModuleIO" that return 0). Use oldValue != "" to skip boot but allow backend updates (e.g., D_FastLEDAudio channel recreation, ModuleLiveScripts). Use if (!originId.toInt()) return; to process only UI updates and block all backend/boot updates (e.g., ModuleIO, ModuleDevices to prevent recursion/broadcast loops). Both checks are necessary and not redundant.
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2026-02-27T13:03:49.950Z
Learning: In MoonModules/MoonLight ModuleIO.onUpdate: During file load, compareRecursive processes JSON keys in order they appear. If boardPreset appears before modded in the file, checking _state.data["modded"] in the boardPreset handler reads the old code default (false), not the value being restored from file (true), causing custom pins to be overwritten. Fix: Skip boardPreset handling during backend/file updates (originId.toInt()==false). Instead, handle boardPreset in loop20ms() after full file load completes, where final modded state is available. Check: if modded==false, apply setBoardPresetDefaults(); if modded==true, skip and preserve custom pins from file.
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2026-01-28T12:31:17.304Z
Learning: In MoonModules/MoonLight compareRecursive (src/MoonBase/Module.cpp): Treat an object’s "name" change as an identity change. When oldData["name"] != newData["name"], emit only the "name" UpdatedItem for that parent and short‑circuit (do not recurse into the subtree like controls). This prevents spurious controls.* updates after renames while keeping the update flow generic and not tied to specific child keys.
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2026-02-15T13:23:26.063Z
Learning: In MoonModules/MoonLight Module::update() + compareRecursive: passing a partial object acts as a patch — only the provided keys are visited/updated; missing siblings are preserved. Arrays are replaced only when a new array is provided. Therefore updating only preset.select and preset.selected via a small patch will not remove preset.list.
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2026-02-27T12:33:23.675Z
Learning: In MoonModules/MoonLight ModuleIO.onUpdate: The boardPreset handler must be placed BEFORE the if (!originId.toInt()) return; check to handle both UI updates (originId="1") and boot/file-load updates (originId="module"). When modded=false and boardPreset changes on boot, setBoardPresetDefaults() must be called to apply current code-defined defaults (not just pins from file), ensuring board preset definitions can evolve across code updates. Only UI-specific handlers (modded, switch1/2, maxPower, usage, index, i2cFreq) should come after the originId check.
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2026-01-26T14:59:22.091Z
Learning: In MoonModules/MoonLight PR `#114`: ModuleDevices deadlock caused by: (1) StatefulService.h line 204-205 bug - after 500ms mutex timeout, continues WITHOUT acquiring mutex, causing race conditions and JSON corruption. (2) receiveUDP() while-loop processes own UDP broadcasts via loopback, creating circular call chain: sendUDP→broadcast→receiveUDP→updateDevices→update→handlers→sendUDP. (3) updateDevices() holds _accessMutex during expensive sort/JSON operations with 15 devices, can exceed 500ms with nested calls. Solution: Fix StatefulService to wait forever or reboot on timeout; skip own broadcasts by checking message.name == ownHostname; optimize updateDevices to sort outside mutex.
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2026-03-05T07:10:15.113Z
Learning: In MoonModules/MoonLight (lib/PsychicHttp/src/PsychicWebSocket.cpp): `PsychicWebSocketClient::sendMessage()` must guard with `httpd_ws_get_fd_info()` before calling `httpd_ws_send_frame_async()`. Without the guard, when a client disconnects and reconnects (reusing the same socket fd) while a browser tab is hidden, repeated failed sends degrade the lwIP netconn state until `lwip_netconn_do_write` hard-asserts `(state!)` instead of returning ESP_FAIL, causing a reboot. Fix: check `httpd_ws_get_fd_info(server, socket) == HTTPD_WS_CLIENT_WEBSOCKET` first and return ESP_FAIL early if not. Also, in EventSocket.cpp, erase stale clients from subscriptions on ESP_FAIL instead of keeping them.
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2026-01-08T11:39:52.878Z
Learning: In MoonLight WebSocket monitor: When browser tabs are hidden, browsers throttle JavaScript (including WebSocket onmessage handlers) but keep TCP connections open. Monitor data (48KB frames every 20-300ms) continues arriving at TCP level but isn't processed. This fills ESP32's httpd_ws_send_frame_async() buffers, causing ESP_FAIL. When tab becomes visible, hundreds of queued frames overwhelm the JavaScript main thread, freezing the UI. Solution: use Page Visibility API (client_visibility event) to stop emitting monitor data to hidden clients, preventing buffer overflow and backlog buildup.
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-02-04T09:52:08.096Z
Learning: MoonLight ModuleDevices: To reduce missed packets when many devices send at once, add a 0–20 ms non-blocking jitter only to non-control (status) UDP broadcasts (post-control apply and optionally the 10s heartbeat). Implement by scheduling the send in loop20ms instead of delaying inside addUpdateHandler/httpd context; keep control packets immediate.
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: 0
File: :0-0
Timestamp: 2026-03-08T10:36:33.782Z
Learning: In MoonModules/MoonLight D_FastLEDDriver vs D_ParallelLEDDriver: The PARLIO driver (parlio.cpp) uses `queue_nonblocking = 1` in `parlio_transmit_config_t`, making LED transmission fully fire-and-forget via hardware DMA — zero ISR callbacks during TX, CPU is free for WiFi. FastLED RMT5 (default on IDF5+) uses ISR-driven segment streaming with ISR priority capped at 1-3, which causes constant WiFi interrupt contention and WebSocket throughput degradation at high frame rates. I2S has the same ISR-driven problem. Fix: add `-D FASTLED_RMT5=0` to platformio.ini to force FastLED RMT4, which has an explicit dual-buffer WiFi-resistant design. WARNING: RMT4 and RMT5 cannot coexist in the same binary (startup abort). The affinity="PARLIO" option in FastLED Driver uses FastLED's own PARLIO wrapper, not MoonLight's custom parlio.cpp.
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2026-01-15T12:38:33.598Z
Learning: In MoonLight GameOfLifeEffect (src/MoonLight/Nodes/Effects/E_MoonModules.h): The effect's nested loops (especially lines 257-323) iterate through all cells (e.g., 128×128 = 16,384) with nested neighbor checks (18 per cell = 294,912 operations) without yielding. This starves effectTask (priority 3) when higher-priority tasks like HTTP server (priority 5) run, causing the effect to disappear. UI actions (save, refresh) temporarily show one frame because HTTP completes and briefly releases CPU back to effectTask. Solution: Add taskYIELD() inside outer X loop to prevent task starvation. This is the same root cause as issue `#102` (Starsky glitches with many driver nodes).
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2026-03-08T10:36:33.782Z
Learning: In MoonModules/MoonLight D_FastLEDDriver: FastLED RMT5 (default on IDF >= 5) lacks the dual-buffer WiFi-resistance design of FastLED RMT4, causing WebSocket/WiFi throughput degradation at high LED frame rates. FastLED I2S is also incompatible with WiFi on ESP32-S3. MoonLight's D_ParallelLEDDriver (parlio.cpp) avoids this entirely by using its own DMA-streaming PARLIO implementation with zero per-frame ISR callbacks. Fix: add `-D FASTLED_RMT5=0` to platformio.ini for IDF5-based boards to force RMT4's dual-buffer design. This flag may need to be re-enabled even when a specific FastLED commit appeared stable, as RMT5 behavior varies across FastLED master commits.
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: 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: 136
File: src/MoonBase/Modules/ModuleDevices.h:148-148
Timestamp: 2026-03-04T10:07:41.583Z
Learning: In MoonModules/MoonLight, the `// 🌙` (MoonBase addition) and `// 💫` (MoonLight addition) comment markers are ONLY required when modifying upstream SvelteKit files (i.e., files that originate from the upstream ESP32-sveltekit project). Do NOT require or suggest these markers for MoonBase-specific or MoonLight-specific files (such as `src/MoonBase/Modules/ModuleDevices.h` or other files in `src/MoonBase/` or `src/MoonLight/`). The markers indicate where upstream files have been patched.
1 participant
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Documentation
Chores