Add IrModule (learn-any-remote) + WLED audio sync + Scheduler::setControl#35
Merged
Conversation
Adds WLED-compatible audio sync over UDP (broadcast/receive on port 11988) as an AudioModule mode, so a projectMM device can send its analysed audio to the WLED ecosystem or drive its own effects from a peer's audio with automatic fallback to the local mic. Also brings up line-in on the MHC-WLED ESP32-P4 shield (PCM1808 ADC) and fixes a boot crash where a sync-enabled board opened a socket before the network stack was up.
KPI: 16384lights | PC:465KB | tick:150/116/159/20/2/367/79/23/28/216/155/22/1/47us(FPS:6666/8620/6289/50000/500000/2724/12658/43478/35714/4629/6451/45454/1000000/21276) | ESP32(P4):tick:1870us(FPS:536) | src:167(32018) | test:116(16553) | lizard:113w
Core:
- AudioModule: added WLED audio sync — a `sync` control (Off / Send / Receive). Send broadcasts the analysed frame as a WLED v2 packet (~40/s, throttled); Receive binds the port and drives effects from a peer's audio, auto-blending back to the local mic ~1s after the peer goes quiet. `syncPort` is configurable and shown in Send + Receive; floor/gain stay visible (they govern the fallback mic). Added line-in support via a configurable `mclkPin` (the PCM1808 needs a master clock).
- AudioModule: fixed a boot crash-loop — setup() opened the sync socket before NetworkModule brought an interface up, so the lwip call asserted on a null core mutex and rebooted forever. Socket opening is now deferred to the tick path via syncEnsureSocket(), gated on platform::networkReady(); syncReinit() only closes + records the mode. Mirrors the existing DevicesModule lazy-open pattern.
- platform: added networkReady() — true once an interface has an IP (ESP32: eth/sta/ap connected; desktop: always). The runtime "safe to open a socket" predicate, the counterpart to the compile-time hasNetwork.
- platform: added hasNetwork = hasWiFi || hasEthernet (both configs) — "needs an IP stack" (audio sync / UDP), distinct from hasWiFi ("needs the radio": SSID / AP / TX-power / RSSI).
- NetworkModule: hide txPowerSetting unless the radio is on (Ethernet-only mode has no TX power to set).
Light domain:
- WLEDAudioSyncPacket.h: new — the netmindz/WLED-sync v2 wire format in one place (44-byte "00002" packet, port 11988), with build/parse helpers. Parse rejects v1 / short / foreign packets so a receiver ignores them rather than crashing.
Tests:
- unit_WledAudioSyncPacket: golden-vector + round-trip + rejection tests pinning the 44-byte layout, "00002" header, and exact field offsets (the WLED compatibility contract).
Docs / CI:
- README: credited wladi (myhome-control) for the MHC-WLED ESP32-P4 shield and the PCM1808 line-in pinout / FMT-pin details.
- deviceModels.json: MHC-WLED ESP32-P4 shield (renamed from ABC-WLED) — line-in AudioModule child (ws 26 / sd 33 / sck 32 / mclk 36, gain 100), planned RS-485/DMX, supported LEDs/Ethernet/Audio. Board image + pin/DIP diagram.
- Plan-20260703 - WLED audio sync.md: saved approved plan.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds two independent features: a WLED-compatible v2 audio-sync UDP protocol integrated into AudioModule with a new platform networkReady() API, and an IrModule peripheral decoding NEC IR codes via a new platform::irRead() seam, routed through a new Scheduler::setControl entry point that HttpServerModule now delegates to. Supporting docs, device metadata, and unit tests are included. ChangesWLED Audio Sync
IR Remote Module and Scheduler Refactor
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant AudioModule
participant Platform
participant UDPSocket
participant PeerDevice
AudioModule->>Platform: networkReady()
Platform-->>AudioModule: true
AudioModule->>UDPSocket: syncEnsureSocket()
alt Send mode
AudioModule->>AudioModule: buildWledAudioSync(frame_)
AudioModule->>UDPSocket: send(packet)
UDPSocket->>PeerDevice: broadcast packet
else Receive mode
PeerDevice->>UDPSocket: incoming packet
AudioModule->>UDPSocket: syncReceive()
UDPSocket->>AudioModule: parseWledAudioSync(packet)
AudioModule->>AudioModule: overwrite frame_
end
sequenceDiagram
participant IrHardware
participant IrModule
participant Scheduler
participant TargetModule
IrHardware->>IrModule: loop() polls platform::irRead
IrModule->>IrModule: processCode(code)
alt Learn mode
IrModule->>IrModule: bind code to action, persist hex
else Match binding
IrModule->>TargetModule: read current control value
IrModule->>Scheduler: setControl(module, control, delta value)
Scheduler-->>IrModule: SetControlResult
end
Possibly related PRs
Suggested labels: enhancement, audio, ir, scheduler Suggested reviewers: MoonModules maintainers familiar with AudioModule and Scheduler internals 🐰 A hop through UDP, a beam through the air, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/scenarios/light/scenario_perf_full.json (1)
87-104: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winKeep the benchmark timestamps realistic.
The updated
atvalues now point to2026-07-04, which is after the review date for this change set. That makes the capture history impossible to reproduce and weakens any freshness checks that trust this metadata.Also applies to: 176-193, 265-282, 352-369, 1217-1234, 1318-1335, 1411-1428, 1597-1614, 1902-1919, 1995-2012
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/scenarios/light/scenario_perf_full.json` around lines 87 - 104, The benchmark metadata in the scenario JSON has unrealistic future timestamps, which makes the capture history non-reproducible and breaks freshness checks. Update the `at` arrays in the affected benchmark entries (including `pc-macos` and the other listed sections) to dates that are not after the review date and are consistent with realistic capture history. Keep the values in the same structure, and make sure all referenced benchmark blocks use valid past timestamps.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/AudioModule.h`:
- Around line 528-595: Add module-level tests for the sync socket lifecycle in
AudioModule, since only buildWledAudioSync and parseWledAudioSync are currently
covered. Verify syncEnsureSocket opens once per mode, retries until
platform::networkReady() is true, and updates syncOpen_/syncStr_ for send and
receive paths. Add coverage for syncSend throttling and packet emission via
syncSock_.sendTo, and for syncReceive draining, frame_ replacement,
lastSyncRecv_ stamping, and the fresh-vs-stale fallback behavior. Use the
syncEnsureSocket, syncSend, syncReceive, and syncSock_ symbols to locate the
logic.
- Around line 528-559: syncEnsureSocket() in AudioModule is retrying a failed
open/connect-or-bind on every loop tick with no cooldown, so add a
backoff/timing gate for repeated bring-up failures. Use the existing sync state
in AudioModule (and the related syncReinit()/syncSend retry patterns) to track
the next allowed retry time, and return false without reopening until the
backoff expires after a failed syncSock_.open(), syncSock_.connect(), or
syncSock_.bind() attempt.
In `@src/light/WLEDAudioSyncPacket.h`:
- Around line 72-82: In parseWledAudioSync, the values read with wledGetFloatLE
are cast directly to uint16_t, so clamp or otherwise validate each parsed float
before conversion to prevent NaN or out-of-range input from producing undefined
results. Update the assignments for out.level, out.levelSmoothed, out.peakMag,
and out.peakHz to use a safe bounded conversion, and revise the nearby comment
to avoid saying “rounded back” since static_cast truncates.
In `@web-installer/deviceModels.json`:
- Around line 850-855: The board entry for MHC-WLED ESP32-P4 shield still points
its url field to the old ABC-WLED product page, so update that url to the new
product listing to match the renamed shield and the new image. Locate the
deviceModels.json entry for MHC-WLED ESP32-P4 shield and change only the url
value so the catalog stays consistent with the renamed product.
---
Outside diff comments:
In `@test/scenarios/light/scenario_perf_full.json`:
- Around line 87-104: The benchmark metadata in the scenario JSON has
unrealistic future timestamps, which makes the capture history non-reproducible
and breaks freshness checks. Update the `at` arrays in the affected benchmark
entries (including `pc-macos` and the other listed sections) to dates that are
not after the review date and are consistent with realistic capture history.
Keep the values in the same structure, and make sure all referenced benchmark
blocks use valid past timestamps.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8cc2aa0a-cf1e-46d3-a6c9-b1fdc0dd555a
⛔ Files ignored due to path filters (2)
docs/assets/boards/abc-wled-esp32-p4-shield.jpgis excluded by!**/*.jpgdocs/assets/boards/mhc-wled-esp32-p4-shield.jpgis excluded by!**/*.jpg
📒 Files selected for processing (14)
README.mddocs/history/plans/Plan-20260703 - WLED audio sync.mdsrc/core/AudioModule.hsrc/core/NetworkModule.hsrc/light/WLEDAudioSyncPacket.hsrc/platform/desktop/platform_config.hsrc/platform/desktop/platform_desktop.cppsrc/platform/esp32/platform_config.hsrc/platform/esp32/platform_esp32.cppsrc/platform/platform.htest/CMakeLists.txttest/scenarios/light/scenario_perf_full.jsontest/unit/light/unit_WledAudioSyncPacket.cppweb-installer/deviceModels.json
Comment on lines
+850
to
+855
| "name": "MHC-WLED ESP32-P4 shield", | ||
| "chip": "ESP32-P4", | ||
| "firmwares": [ | ||
| "esp32p4-eth" | ||
| ], | ||
| "image": "assets/boards/abc-wled-esp32-p4-shield.jpg", | ||
| "image": "assets/boards/mhc-wled-esp32-p4-shield.jpg", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the product URL to match the renamed shield.
The entry now says MHC-WLED and uses the new board image, but the url immediately below still points to the ABC-WLED product page. That leaves the catalog inconsistent and sends users to the wrong listing.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web-installer/deviceModels.json` around lines 850 - 855, The board entry for
MHC-WLED ESP32-P4 shield still points its url field to the old ABC-WLED product
page, so update that url to the new product listing to match the renamed shield
and the new image. Locate the deviceModels.json entry for MHC-WLED ESP32-P4
shield and change only the url value so the catalog stays consistent with the
renamed product.
…tCrafter Ethernet Adds an IR-receiver peripheral that learns any remote: decode a button's NEC code, bind it to an action, and thereafter that button drives the device (brightness, palette) — the physical-remote counterpart of the WLED-app bridge. To do it cleanly, the generic "set any control by name" logic moves out of HttpServer into a Scheduler primitive that IR, HTTP, Improv, and the WLED bridge all share. Also brings up W5500 Ethernet on the LightCrafter (it needs the WIZ850IO reset pin released) and folds in the earlier CodeRabbit findings on the audio-sync packet. KPI: 16384lights | PC:465KB | tick:121/94/120/15/2/299/61/17/20/172/120/17/1/36us(FPS:8264/10638/8333/66666/500000/3344/16393/58823/50000/5813/8333/58823/1000000/27777) | ESP32-S3:tick:1073us(FPS:931) | src:169(32480) | test:118(16934) | lizard:114w Core: - Scheduler: extracted setControl(module, control, value) — the generic control-set primitive (find module, validate + apply, rebuild controls, onUpdate, persist, conditional buildState). It was private to HttpServerModule; now it lives where the module tree does, so any module can drive another's control by composing against one primitive. HttpServer's applySetControl became a thin result→HTTP-status mapper (−54 lines net). Added Scheduler::instance() (the FilesystemModule singleton pattern) so a factory-created module reaches it, and a noteDirty hook so a control set from anywhere schedules the debounced save. - IrModule: new IR-receiver peripheral (factory-registered, catalog-wired, not auto-wired). Decodes an IR remote on `pin` and learns it: a `learn` select arms an action, the next received code binds to it (persisted per action in a readonly Text control, rebuilt into a fast lookup on load), and a bound code runs its action — brightness ±16 (clamped) / palette prev-next — through Scheduler::setControl. Status reports setup state, learning, a binding, a fired action, or an unassigned code. latestCode() is the seam a future LightsControl consumes. - AudioModule: 🐇 CodeRabbit — no functional change here beyond the packet-parse hardening below (kept for the branch). - platform: added irRead(pin, code) — real NEC-over-RMT decode on ESP32 (platform_esp32_ir.cpp: a persistent RX channel, an ISR that only signals a queue, decode + re-arm on the render task — never in interrupt context), a false stub on desktop. NetworkModule W5500 path unchanged from the earlier ISR/poll fix. Light domain: - WLEDAudioSyncPacket: 🐇 CodeRabbit — parse now clamps each wire float into its uint16 field (wledFloatToU16) so a foreign packet carrying NaN or an out-of-range value can't produce an undefined cast; comment corrected from "rounded" to "truncated". UI: - ui.md: IR entry rewritten for the learn workflow (learn select + per-action code read-outs; no action buttons, no last-code control — the status line carries the received code). Embeds the remote photo effects.md-style. Scripts / MoonDeck: - check_devices.py: added "IR" to the supported-capability vocabulary now that IrModule backs it. Tests: - unit_IrModule: learn→bind→fire, clamping, independent multi-action bindings, unassigned-code and missing-target robustness (9 cases, injectCodeForTest seam since irRead stubs on desktop). - unit_AudioModule_sync, unit_WledAudioSyncPacket: 🐇 CodeRabbit — added the sync socket-lifecycle tests and the float-clamp cases. Docs / catalog: - deviceModels.json: SE 16 V1 + LightCrafter 16 — IR moved planned→supported (decode works on hardware); IrModule child (SE16 pin 5, LightCrafter pin 4, parent System); LightCrafter Ethernet gains ethRstGpio 3 (the WIZ850IO nRST — without it the W5500 reads 0x00 "version mismatched"). - IRDriver.jpeg moved from the MoonLight image pool to assets/core/IrModule.jpeg; the stale MoonLight controls screenshot deleted. - Plan-20260704 - IrModule brightness + palette: saved plan. Reviews: - 🐇 WLEDAudioSyncPacket: parse clamps NaN/out-of-range floats before the uint16 cast — fixed. - 🐇 AudioModule: sync socket-lifecycle unit tests added — fixed. - 🐇 deviceModels.json MHC-WLED url still ABC-WLED — accepted as-is: the shop's product path is literally .../ABC-WLED-ESP32-P4-shield/HW10027 (verified live); the rename is internal only, changing it would break the link. - 🐇 scenario_perf_full.json "future timestamps" — accepted: the `at` dates are ≤ the capture date; the finding reflects the reviewer's training-cutoff clock, not real future dates. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ewowi
changed the title
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/core/AudioModule.h (1)
600-617: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win"level RMS" read-out freezes while receiving peer audio.
syncReceive()replacesframe_with the peer's frame and stampslastSyncRecv_, but never updateslevelPeak_. Sinceloop()returns early on a fresh receive (line 268), the mic-path statement that normally feedslevelPeak_(if (frame_.level > levelPeak_) levelPeak_ = frame_.level;) is skipped entirely.loop1s()still publisheslevelStr_fromlevelPeak_, so the "level RMS" control stays frozen at its pre-Receive value even whileframe_.peakHz/peakStr_correctly track the peer.🔧 Proposed fix
AudioFrame rf; if (parseWledAudioSync(pkt, static_cast<size_t>(n), rf)) { frame_ = rf; // received audio drives the effects lastSyncRecv_ = platform::millis(); + if (frame_.level > levelPeak_) levelPeak_ = static_cast<uint8_t>(frame_.level); }Also applies to: 261-271, 387-406
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/AudioModule.h` around lines 600 - 617, The “level RMS” value is not being refreshed when peer audio is received because `syncReceive()` updates `frame_` and `lastSyncRecv_` but leaves `levelPeak_` unchanged, and `loop()` returns early on fresh sync input. Update `syncReceive()` (or the early-return path in `loop()`) so the received `AudioFrame` also drives `levelPeak_` the same way the local mic path does, keeping `loop1s()` and `levelStr_` in sync with peer audio.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/IrModule.h`:
- Around line 128-150: The learn path in processCode() updates learnedCode_ and
codeStr_ but only marks the module dirty, so the new binding may not be
persisted immediately. Update the learn branch to also trigger the debounced
save via FilesystemModule::noteDirty() alongside markDirty(), using the
processCode() learn handling and existing setStatus()/learn_ flow as the place
to hook it in.
In `@src/main.cpp`:
- Line 214: The IrModule help-link target is pointing at the wrong docs
location, so the registered page path needs to be updated. In main.cpp, adjust
the mm::ModuleFactory::registerType<mm::IrModule> call for "IrModule" to use the
generated core/moxygen/IrModule.md path instead of core/IrModule.md so the UI
help link resolves correctly.
In `@src/platform/esp32/platform_esp32_ir.cpp`:
- Around line 117-129: The irRead flow in platform_esp32_ir.cpp decodes a frame
and then blindly calls arm(), which can leave the receiver unusable if re-arming
fails. Update irRead so it checks the result of arm() after decodeNec(), and if
re-arming fails, clear or reset the channel state used by ensureChannel() (such
as rxChan_ and currentPin_) and return false so the pin can be reinitialized
cleanly on the next read.
In `@test/unit/core/unit_IrModule.cpp`:
- Around line 165-174: The IrModule test only covers the unassigned-code no-op
path and never exercises the missing-target behavior described in the comment.
Update the unit test in the IrModule test case to explicitly trigger a binding
that resolves to a missing target and assert the typed not-found result from the
control path, while still verifying no crash and unchanged driver state. Use the
existing Rig, r.fire, latestCode, and setControl/missing-target flow referenced
by the current test to locate where to add the additional assertion.
---
Outside diff comments:
In `@src/core/AudioModule.h`:
- Around line 600-617: The “level RMS” value is not being refreshed when peer
audio is received because `syncReceive()` updates `frame_` and `lastSyncRecv_`
but leaves `levelPeak_` unchanged, and `loop()` returns early on fresh sync
input. Update `syncReceive()` (or the early-return path in `loop()`) so the
received `AudioFrame` also drives `levelPeak_` the same way the local mic path
does, keeping `loop1s()` and `levelStr_` in sync with peer audio.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 13ac43dc-e450-44a6-a366-144d893e062e
⛔ Files ignored due to path filters (2)
docs/assets/core/IrModule.jpegis excluded by!**/*.jpegdocs/backlog/moonlight_images/moonlight/drivers/irdrivercontrols.pngis excluded by!**/*.png
📒 Files selected for processing (22)
docs/history/plans/Plan-20260704 - IrModule brightness.mddocs/moonmodules/core/ui/ui.mdesp32/main/CMakeLists.txtscripts/check/check_devices.pysrc/core/AudioModule.hsrc/core/FilesystemModule.cppsrc/core/HttpServerModule.cppsrc/core/IrModule.hsrc/core/Scheduler.cppsrc/core/Scheduler.hsrc/light/WLEDAudioSyncPacket.hsrc/main.cppsrc/platform/desktop/platform_desktop.cppsrc/platform/esp32/platform_esp32.cppsrc/platform/esp32/platform_esp32_ir.cppsrc/platform/platform.htest/CMakeLists.txttest/scenarios/light/scenario_perf_full.jsontest/unit/core/unit_AudioModule_sync.cpptest/unit/core/unit_IrModule.cpptest/unit/light/unit_WledAudioSyncPacket.cppweb-installer/deviceModels.json
| mm::ModuleFactory::registerType<mm::SystemModule>("SystemModule", "core/SystemModule.md"); | ||
| mm::ModuleFactory::registerType<mm::AudioModule>("AudioModule", "core/AudioModule.md"); | ||
| mm::ModuleFactory::registerType<mm::I2cScanModule>("I2cScanModule", "core/I2cScanModule.md"); | ||
| mm::ModuleFactory::registerType<mm::IrModule>("IrModule", "core/IrModule.md"); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -HI 'IrModule.md' docsRepository: MoonModules/projectMM
Length of output: 159
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== repo files matching IrModule.md =='
git ls-files | rg '(^|/)IrModule\.md$' || true
echo
echo '== text references to IrModule.md or core/IrModule.md =='
rg -n --hidden --glob '!.git' 'IrModule\.md|core/IrModule\.md' .
echo
echo '== registration context =='
rg -n --hidden --glob '!.git' 'registerType<mm::IrModule>|IrModule' src docs .Repository: MoonModules/projectMM
Length of output: 8380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== ModuleFactory doc-path handling =='
rg -n --hidden --glob '!.git' 'registerType<|doc|help|technical|moxygen|moonmodules' src test docs
echo
echo '== likely factory implementation =='
fd -HI 'ModuleFactory*' src testRepository: MoonModules/projectMM
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== ModuleFactory definitions =='
fd -HI 'ModuleFactory*' src | sed -n '1,20p'
echo
echo '== registerType call sites with doc paths =='
rg -n 'registerType<[^>]+>\("([^"]+)", "([^"]+)"\)' src -r '$0' --only-matching
echo
echo '== docPath field / API types plumbing =='
rg -n --hidden --glob '!.git' 'docPath|/api/types|types' src/core src/ui src/platform testRepository: MoonModules/projectMM
Length of output: 22190
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== docs/moonmodules/core tree =='
fd -HI . docs/moonmodules/core | sed -n '1,200p'
echo
echo '== docs/coding-standards module-doc sections =='
sed -n '111,133p' docs/coding-standards.md
echo
echo '== src/core/ModuleFactory.h docPath comments =='
sed -n '1,90p' src/core/ModuleFactory.hRepository: MoonModules/projectMM
Length of output: 10167
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== representative core archive docs =='
for f in docs/moonmodules/core/archive/HttpServerModule.md docs/moonmodules/core/archive/AudioModule.md docs/moonmodules/core/ui/ui.md; do
echo
echo "-- $f --"
sed -n '1,120p' "$f"
done
echo
echo '== links to archive or moxygen paths under docs/moonmodules/core =='
rg -n 'archive/|moxygen/' docs/moonmodules/coreRepository: MoonModules/projectMM
Length of output: 44673
Point IrModule at the generated docs page src/main.cpp:214 — core/IrModule.md doesn’t line up with the core docs layout here, so the UI help link lands on a dead page. Use the generated core/moxygen/IrModule.md path instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main.cpp` at line 214, The IrModule help-link target is pointing at the
wrong docs location, so the registered page path needs to be updated. In
main.cpp, adjust the mm::ModuleFactory::registerType<mm::IrModule> call for
"IrModule" to use the generated core/moxygen/IrModule.md path instead of
core/IrModule.md so the UI help link resolves correctly.
Fixes four CodeRabbit findings on the IR / audio-sync branch (a lost learned-IR binding, a silently-dead IR receiver on re-arm failure, a frozen level read-out under audio sync, and an untested code path) and carries the branch's hard-won lessons into decisions.md before the merge. Core: - IrModule: 🐇 a learned IR binding now schedules the debounced save (FilesystemModule::noteDirty alongside markDirty) — a bound code is written straight to the persistent Text control, not via setControl, so it must trigger persistence itself or the binding could be lost before an unrelated save runs. Dropped a stray markDirty on the transient unassigned-code status path (nothing persistent changes there). - AudioModule: 🐇 syncReceive now feeds the received peer level into the same 1 s peak window the local mic uses, so the "level RMS" read-out reflects peer audio instead of freezing at the last local value while a peer drives the effects. - platform_esp32_ir: 🐇 irRead checks the re-arm result after a decode; on failure it tears the RX channel down (closeChannel) and returns false, so ensureChannel reopens a fresh channel next call rather than leaving the receiver enabled-but-not-armed (silently dead) forever. Tests: - unit_IrModule: 🐇 added a missing-target case — learn a code, rename the target module away, fire the code, and assert the "no Drivers module" status + no crash + unchanged state (the runAction not-found branch was untested; the prior case only covered an unassigned code). Docs / CI: - decisions.md: carried forward this branch's lessons — W5500 SPI bring-up (the ISR service is required for interrupt mode, IDF v6's poll-period pairing, a failed eth init masking the bug behind a WiFi fallback, and a 0x00 "version mismatched" read meaning the chip is held in reset → a per-board reset-release pin); the Scheduler::setControl extraction (put the generic control-set primitive where the tree owner is, not per-input seams); and the RMT/IR ISR-safety rule (signal only in the ISR, decode + re-arm on the task). - backlog-core.md: noted the two module-by-name tree-walkers (HttpServer::findModuleByName vs Scheduler::firstByName) as a consolidation candidate now that firstByName is public (Reviewer finding, deferred). - ui.md: AudioModule entry updated with the sync (Off/Send/Receive), syncPort, mclkPin (line-in), floor/gain controls that the audio-sync feature added (doc-sync gate). Reviews: - 🐇 IrModule learn-path persistence — fixed (noteDirty added). - 🐇 IrModule irRead re-arm failure — fixed (closeChannel + return false on arm() failure). - 🐇 AudioModule level read-out under sync — fixed (levelPeak_ fed from the peer frame). - 🐇 IrModule missing-target test coverage — fixed (new test case). - 🐇 main.cpp IrModule doc path → core/moxygen/IrModule.md — accepted as-is: every core module registers "core/<Name>.md" (not the moxygen subpath), and the UI resolves that to the built .html; changing only IrModule would make it the odd one out and break the help link. - 👾 Reviewer (whole-branch, pre-merge): architecturally clean, merge-ready, no must-fix; the one nice-to-have (tree-walker consolidation) recorded in backlog-core.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two commits of new capability, all bench-proven on hardware:
1 — WLED audio sync + MHC-WLED P4 shield line-in (
2f8387a)Off / Send / Receivemode. Send broadcasts the analysed audio as a WLED v2 packet (port 11988, throttled); Receive drives effects from a peer's audio and auto-blends back to the local mic ~1 s after the peer goes quiet. Interoperates with the WLED ecosystem (the format the netmindz/WLED-sync library and MoonLight'sD_WLEDAudiouse). NewWLEDAudioSyncPacket.hholds the wire format in one place.setup()before NetworkModule brought an interface up → lwip asserted on a null core mutex. Socket opening is now deferred to the tick path (syncEnsureSocket()), gated on the newplatform::networkReady().mclkPin); catalog entry + wladi credit.2 — IrModule (learn-any-remote) + Scheduler::setControl extraction; LightCrafter Ethernet (
6f51373)platform_esp32_ir.cpp), alearnselect that binds the next received code to an action, and a bound code drives the device (brightness ±16 clamped, palette prev/next). Learned codes persist. Status feedback throughout. Live-proven on the SE16 (GPIO 5, IR/Eth switch) and LightCrafter (GPIO 4, IR + Ethernet simultaneously).Scheduler::setControl— extracted the generic "set any control by name" primitive (was private to HttpServerModule) up to the Scheduler, where the module tree lives. IR, HTTP, Improv, and the WLED bridge now compose against one control-agnostic primitive — adding an input never adds plumbing. HttpServer'sapplySetControlbecame a thin result→HTTP-status mapper (net −54 lines).ethRstGpio: 3) added to the catalog; without releasing it the W5500 reads0x00("version mismatched"). Builds on the earlier W5500 ISR-service / poll fixes.supportedon both Limpkin boards (SE16, LightCrafter) now that decode works on hardware.parseWledAudioSyncnow clamps each wire float into its uint16 field (NaN/out-of-range guard); sync socket-lifecycle unit tests added.Verification
-Werror); unit + scenario tests pass; all 8 ships=True ESP32 variants build clean (the platform + core changes compile across every WiFi/Eth build-guard combination).unit_IrModule(9 cases: learn/bind/fire, clamping, independent bindings, robustness),unit_AudioModule_sync,unit_WledAudioSyncPacket(golden vector + clamp).🤖 Generated with Claude Code