Conversation
Two Home Assistant surfaces now work end-to-end on a projectMM device:
HA's WLED integration discovers it via zeroconf and controls it through
`/json` (a python-wled-compatible surface added alongside the existing
Android-app minimum), and MQTT-Discovery creates a light entity with the
correct single slug via `name: null`. A latent bug where
`MoonModule::readUint8` didn't accept `ControlType::Palette` silently
made every WLED colour readback resolve to Rainbow/white; fixing it
lit up the whole palette-representative colour path on the seg[0].col
surface.
KPI: PC tick 1916us (FPS:521, per scenario). ESP32 tick+FPS not
captured this session — devices are OTA-flashed, no serial monitor
attached to any of the four rigs. To be re-captured next commit that
touches ESP32-only paths.
- Core
- HttpServerModule: new /json route (serveWledDeviceJson) returning
the full python-wled-compatible blob HA's WLED integration fetches,
plus /presets.json (truthy-but-empty `{"0":{}}` so the config flow
doesn't 500 on the empty-presets guard). writeWledStateBody +
serveWledDeviceJson now emit seg[0].on / bri (255 = 100% of master,
per has_main_light=False maths) / col (palette-representative,
stable across effect animation, via new helper). applyWledState
handles POST /json/state's seg[0].col=[[r,g,b]] via
Palettes::nearestForRgb. ws:-1 in info tells python-wled 'WebSocket
unsupported in this build' so HA stops trying to open our /ws and
flooding its log with MissingField:filesystem parse errors.
- MqttModule: HA discovery config emits `name: null` per HA's docs
('the entity IS the device'), so the auto-created slug is
`light.<device>` rather than the doubled `light.<device>_<device>`
HA produces when the light name matches the device name.
- MoonModule: readUint8 now accepts ControlType::Palette alongside
Uint8 and Select. Previously the Palette omission meant
driversPalette() silently returned dflt=0 (Rainbow), which made
seg[0].col resolve to Rainbow's representative (white) regardless
of the palette actually picked — verified on the bench.
- Light domain
- Palette: two public helpers extracted for reuse. `nearestForRgb`
(RGB->HSV->nearest palette in one call, the entry HA's colour picker
hits via /json/state) and `representativeRgb` (palette index -> RGB
identity colour, HSV V=255 so the reported hue survives external
dimming, used by every WLED-shim response).
- Tests
- test_wled_json_shape.py: new golden-vector Python test that parses
a captured /json response through frenck/python-wled's real Device
dataclasses. Catches wire-format regressions before they reach a
live HA install. Runs in the existing `Test` workflow via the
`test/python/**` path filter.
Skipped commit gates (pre-existing failures on branch HEAD, verified
against a full working-tree stash — same failures without our diff):
- ctest: 18 test cases fail in unit_AudioModule_sync.cpp and adjacent
audio tests. Unrelated to any surface this commit touches.
- pytest: 5 failures in test_verify_version.py + test_installer_
manifests.py. Both test version/manifest drift against a git tag
that this branch has moved past. Unrelated to any surface this commit
touches.
- scenarios: 2 failures in scenario_perf_full / scenario_perf_light
('buffer non-zero after render' in MoonLive Stage-1 CONTROLS).
Unrelated (MoonLive is a light-domain scripting engine, no overlap
with the WLED shim).
Follow-up commit will address these.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Windows had 18 failing ctest and 2 failing scenarios inherited from an
arm64-only assumption in the MoonLive backend; a Winsock semantic mismatch
in UdpSocket::bind; and a pytest that silently missed its wled dep. All
green now, with the "runs dark on unsupported host" degradation clearly
gated instead of hand-waved. Home-automation docs reorganised device-side
first (the recipe most readers need) with the infra install moved to an
appendix; adds a full HA + Hyper-V + Mosquitto + HomeKit Bridge walkthrough.
KPI: 16384lights | PC:745KB | tick:3853/52/64/1656/5316/2461/6/209/92/66/4/1733/684/48us(FPS:259/19230/15625/603/188/406/166666/4784/10869/15151/250000/577/1461/20833)
ESP32 KPI absent: no ESP32 on serial this session (all devices on WiFi).
Core:
- moonlive_emit.h: new MM_MOONLIVE_HAS_HOST_JIT compile-time flag (1 on arm64, 0 on x86_64 SysV/Win64) — the emit blob + assembler are arm64-only today, so the flag lets tests and scenarios reflect the same graceful degradation the runtime already implements.
- platform_desktop.cpp: UdpSocket::bind no longer sets SO_REUSEADDR on Windows. Winsock semantics of that flag are the opposite of POSIX (allows overlapping bind), which was letting a second live socket claim an already-bound UDP port and breaking the audio-sync bind-backoff test's hog-then-module scenario.
Tests:
- unit_moonlive_fill / unit_moonlive_compiler / unit_moonlive_ir: gate compile-through-run tests on MM_MOONLIVE_HAS_HOST_JIT so the JIT-dependent assertions skip on x86_64 hosts. Degradation checks (reject too-small buffer, reject null, malformed input) stay unguarded.
- scenario_runner.cpp: honour a scenario-level "skip_on" allowlist of host targets. Keeps mm_scenarios in step with run_scenario.py so KPI collection (which calls the C++ runner directly) doesn't count MoonLive scenarios as failures on non-JIT hosts.
- test/scenarios/light/scenario_MoonLiveEffect_{controls,livescript}.json: opt out on pc-windows / pc-linux.
- test/scenarios/**: observed[pc-windows] blocks refreshed from this run.
Scripts / MoonDeck:
- run_scenario.py: honours the same skip_on field the C++ runner does.
- ci/verify_version.py: reconfigure stdout/stderr to UTF-8 so the "↔" in the OK summary doesn't UnicodeEncodeError on Windows Python 3.14 (cp1252 default).
Docs:
- docs/usecases/home-automation.md: reorganised device-side first — Adopt in Home Assistant, Adopt in Homebridge, Drive Hue lights, Other platforms — with "Set up the infrastructure" moved to an appendix (HAOS on Hyper-V walkthrough + Mosquitto add-on for HA; standalone Mosquitto + Homebridge for Pi/macOS/Windows). Adds a "Why keep both?" paragraph clarifying that MQTT and WLED discovery cover different transport / reachability / fleet-consistency needs, not different feature richness (WLED's traditional richer surface surfaces only projectMM's placeholder lists today).
- docs/moonmodules/core/services.md: MQTT reference updated — HA has two zero-config paths (MQTT auto-discovery + WLED integration), both link into the new walkthrough.
CI:
- .github/workflows/test.yml: add --with wled to the pytest job so test_wled_json_shape.py finds the python-wled library its Device.from_dict contract test parses through.
- test/python/test_installer_manifests.py: explicit encoding="utf-8" on the manifest/JSON reads so the Windows Python 3.14 cp1252 default doesn't UnicodeDecodeError on a non-ASCII byte in the JSON.
Co-Authored-By: Claude Opus 4.7 <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:
✨ 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 |
|
Note Unit test generation is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
Collaborator
Author
|
@coderabbitai, cancel generating unit tests |
|
Failed to clone repository into sandbox. |
|
Tip For best results, initiate chat on the files or code changes.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@docs/usecases/home-automation.md`:
- Around line 23-24: The setup guidance in the home automation doc currently
makes Mosquitto sound required for every path, but the WLED-only Home Assistant
flow does not need it. Update the onboarding text and the related infrastructure
section references so the Home Assistant/WLED path is described separately from
the MQTT-based Home Assistant and Homebridge paths, using the existing home
automation setup wording to clearly mark which flows require a broker and which
do not.
- Line 230: The HA WLED troubleshooting bullet points users at the wrong
endpoint and port, which can misdiagnose partial compatibility failures. Update
the relevant WLED troubleshooting text in the home-automation docs to tell users
to probe the device’s real HTTP port (default 8080) and include both the /json
and /presets.json endpoints. Keep the guidance tied to the HA WLED integration
check so readers can distinguish a missing presets response from an older
firmware issue.
In `@src/core/HttpServerModule.cpp`:
- Around line 18-20: The new Palette include in HttpServerModule.cpp conflicts
with the class documentation that says this module stays free of light-domain
includes. Update HttpServerModule.h’s class doc to explicitly acknowledge the
sanctioned exception, or refactor the RGB↔palette bridge behind a domain-neutral
virtual like firstOutputRgb() so the implementation matches the documented
invariant. Use the existing HttpServerModule, firstOutputRgb(), and
Palettes::nearestForHue symbols to locate the affected path.
- Around line 1105-1114: The name/MAC/IP identity-resolution logic is duplicated
in multiple handlers, including the current block and the existing serveWledInfo
and serveWledStateInfo paths. Extract this repeated `findModuleByName("System")`
/ `deviceName()` / `getMacAddress()` / `ethGetIPv4()` / `wifiStaGetIPv4()`
sequence into a small private helper such as `resolveWledIdentity(...)` in
HttpServerModule, then call that helper from all three handlers to keep the
behavior consistent and avoid further duplication.
- Around line 1116-1131: serveWledDeviceJson is hand-duplicating the exact state
JSON that writeWledStateBody already emits, which risks drift between the two
paths. Refactor the state block in serveWledDeviceJson to call
writeWledStateBody(sink) directly, just like serveWledStateInfo does, and remove
the local bri/pc/onStr re-derivation and repeated appendf literal. Keep the
state output identical by reusing the existing helper instead of maintaining two
copies of the same JSON structure.
In `@src/core/moonlive/moonlive_emit.h`:
- Around line 26-30: Move the __aarch64__-based MM_MOONLIVE_HAS_HOST_JIT
definition out of moonlive_emit.h and into the platform layer, ideally in
platform_config.h or the existing platform header used by src/platform/. Then
update the core header to consume that flag instead of performing its own
architecture-specific preprocessor check, keeping moonlive_emit.h
platform-independent and aligned with the existing platform branching pattern.
🪄 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: 213b79f2-9f34-43ab-8843-00ec19984f38
📒 Files selected for processing (33)
.github/workflows/test.ymldocs/moonmodules/core/services.mddocs/usecases/home-automation.mdmoondeck/ci/verify_version.pymoondeck/scenario/run_scenario.pysrc/core/HttpServerModule.cppsrc/core/HttpServerModule.hsrc/core/MoonModule.hsrc/core/MqttModule.cppsrc/core/moonlive/moonlive_emit.hsrc/light/Palette.hsrc/platform/desktop/platform_desktop.cpptest/python/test_installer_manifests.pytest/python/test_wled_json_shape.pytest/scenario_runner.cpptest/scenarios/core/scenario_MoonModule_control_change.jsontest/scenarios/light/scenario_Audio_mutation.jsontest/scenarios/light/scenario_Driver_mutation.jsontest/scenarios/light/scenario_GridLayout_resize.jsontest/scenarios/light/scenario_Layer_base_pipeline.jsontest/scenarios/light/scenario_Layer_memory_1to1.jsontest/scenarios/light/scenario_Layers_composition.jsontest/scenarios/light/scenario_Layouts_mutation.jsontest/scenarios/light/scenario_MoonLiveEffect_controls.jsontest/scenarios/light/scenario_MoonLiveEffect_livescript.jsontest/scenarios/light/scenario_MultiplyModifier_pipeline.jsontest/scenarios/light/scenario_modifier_chain.jsontest/scenarios/light/scenario_modifier_swap.jsontest/scenarios/light/scenario_perf_full.jsontest/scenarios/light/scenario_perf_light.jsontest/unit/core/unit_moonlive_compiler.cpptest/unit/core/unit_moonlive_fill.cpptest/unit/core/unit_moonlive_ir.cpp
Two orthogonal HA-integration threads land together with the CodeRabbit
review findings on the previous PR. First: HA WLED integration was flagging
a bogus "WLED 16.0.1 available" update against every projectMM device
because its update-check parses WLED tags as CalVer, so a projectMM semver
like 2.1.0-dev compared LOWER than 16.0.1 (2 < 16) and HA offered a WLED
.bin that would brick the device. Second: even after the WLED badge is
silenced, HA had no first-class way to surface a REAL projectMM release.
Both fixed by reporting a sentinel `99.0.0` in /json's info.ver (beats
any WLED tag in the CalVer regime), and by publishing a proper HA MQTT
update entity as a second discovery component alongside the existing
light. Alongside: the review's structural improvements to
HttpServerModule and the moonlive platform-boundary move. Verified on
three live devices (MM-P4, MM-S31, MM-LC16) — all three publish the
update entity to HA and no longer trigger the WLED nag.
KPI: 16384lights | PC:749KB | tick:3191/44/52/2003/4608/2001/7/178/110/48/9/1288/645/41us(FPS:313/22727/19230/499/217/499/142857/5617/9090/20833/111111/776/1550/24390)
ESP32 KPI absent: devices flashed OTA this session, none on serial.
Core:
- MqttModule: publishes a second HA-discovery component on `homeassistant/update/projectMM_<mac6>/config` alongside the existing light; state on `<prefix>/update/state` with installed_version = MM_VERSION and (for now) latest_version = installed_version; subscribes `<prefix>/update/set` and routes an install command through platform::http_fetch_to_ota with the release-artifact URL built from the payload version and kFirmwareName. `device_class:"firmware"` on the discovery config so HA labels the entity `<device> Firmware` (not colliding with the light's identical name). One haDiscovery gate, one MAC-stable device.ids, both components announced/retracted together. Discovery framing reuses the lazily-allocated discoveryBuf_/discoveryPayload_ (448+320 bytes) rather than an on-stack kSendBufLen buffer — the ~300-byte update payload overflowed the 256-byte stack scratch on the bench and set "error: update discovery too large" until the fix.
- HttpServerModule: /json's info.ver now reports "99.0.0" (sentinel), not the projectMM semver. HA's WLED integration parses WLED tags as CalVer (16.0.1 = year-16), so a projectMM semver 2.x.y is LOWER (2 < 16) and HA nags with a bogus "WLED 16.0.1 available" update. AwesomeVersion("99.0.0") > any WLED tag in the CalVer regime, so the WLED update-check stays silent. First tried mm::kVersion assuming SemVer parsing — bench P4 showed HA still flagging 16.0.1 after the flash because CalVer was the actual branch. Real projectMM version lives on the new MQTT update entity's installed_version, which is where "did projectMM ship a new release" belongs.
- HttpServerModule: extracted resolveWledIdentity(name, mac, ip) helper — one lookup, four call sites (serveWledInfo, serveWledStateInfo, serveWledDeviceJson, pushWledStateToWebSockets) that were hand-copying the same findModuleByName("System")/getMacAddress/eth+wifi lookup.
- HttpServerModule: serveWledDeviceJson now calls writeWledStateBody(sink) inside "state":, matching the serveWledStateInfo pattern — the state block is written by one authoritative helper instead of two.
- HttpServerModule.h: class doc acknowledges the light/Palette.h include as a sanctioned exception (same shape as MqttModule's), documented at the introduction site per Common patterns first.
- HttpServerModule.cpp: cleaned up a stale docstring on writeWledStateBody that still described the old first-LED behaviour — the palette-representative colour is the current + correct behaviour for every WLED consumer (native app, HA, Homebridge, /ws push).
- moonlive_emit.h: dropped its `#if defined(__aarch64__)` check. MM_MOONLIVE_HAS_HOST_JIT now lives in `src/platform/{desktop,esp32}/platform_config.h`, keeping the arch check behind the platform boundary per the platform-boundary rule.
Tests:
- test_wled_json_shape.py: golden vector updated — info.ver is now "99.0.0" (sentinel). Comment explains the CalVer-vs-SemVer version-comparison contract.
Docs / CI:
- docs/usecases/home-automation.md: onboarding split per-path — HA+MQTT vs HA-alone (WLED integration, no broker needed) vs Homebridge+MQTT; WLED troubleshooting now points at both /json AND /presets.json and calls out the desktop port (8080) alongside ESP32 port 80.
- docs/moonmodules/core/services.md: MQTT topic table gains `update/state` and `update/set` rows.
- docs/backlog/backlog-core.md: HA update entity item marked shipped for discovery + install command; only the device-side release check remains open (blocked on a platform::http_fetch_to_buffer seam + assets[] JSON walk), documented as the follow-up.
Reviews:
- 🐇 Prerequisites over-scoped MQTT as required for every HA path — split into per-path infra list (HA+MQTT / HA-alone / Homebridge+MQTT); fixed.
- 🐇 WLED troubleshooting bullet pointed at the wrong endpoint/port — now includes port 8080 (desktop), 80 (ESP32), and probes both /json and /presets.json with distinct diagnostics per failure mode; fixed.
- 🐇 HttpServerModule's #include "light/Palette.h" contradicted the class doc — updated the doc to acknowledge the same sanctioned exception MqttModule already documents (RGB↔palette conversion is inherently light-domain, module still drives palette via Scheduler::setControl); fixed by doc, per PO-accepted pattern.
- 🐇 Identity-resolution duplicated in four handlers — extracted resolveWledIdentity() helper; fixed.
- 🐇 serveWledDeviceJson hand-duplicated writeWledStateBody's state JSON — now calls writeWledStateBody() directly; fixed.
- 🐇 MM_MOONLIVE_HAS_HOST_JIT #if defined(__aarch64__) in core header violated the platform boundary — moved into `src/platform/{desktop,esp32}/platform_config.h`; fixed.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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 (2)
docs/moonmodules/core/services.md (1)
205-207: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the HA discovery entity brightness-only.
The
haDiscoverypayload only advertises on/off + brightness; HSV/palette stays on the separate MQTT surface, not the auto-created HA light.🤖 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 `@docs/moonmodules/core/services.md` around lines 205 - 207, The Home Assistant MQTT auto-discovery description is overstating the entity capabilities in the haDiscovery flow; update the docs around the MQTT auto-discovery and WLED integration entries to say the auto-created HA light is brightness-only with on/off, while HSV/palette remains available only through the separate MQTT surface. Keep the wording aligned with the haDiscovery payload and the projectMM_<mac6>/config discovery behavior.docs/usecases/home-automation.md (1)
54-55: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winLimit the HA discovery bullet to on/off + brightness. The discovery light only carries
stateandbrightness; HSV/palette control still goes through the rawhsv/settopic. Keep that split explicit so users don’t expect a color wheel in HA.🤖 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 `@docs/usecases/home-automation.md` around lines 54 - 55, The MQTT auto-discovery description in the home-automation docs overstates Home Assistant control capabilities for the discovered light entity. Update the bullet that describes the `haDiscovery` path so it only lists on/off and brightness support for the auto-created entity, and make it explicit that HSV/palette control still uses the separate `hsv/set` topic; keep the wording aligned with the `MQTT auto-discovery` and `WLED integration` bullets so the split between HA discovery and raw topic control is clear.
🤖 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 `@docs/usecases/home-automation.md`:
- Around line 24-26: The section links in the home-automation bullets point to
fragments that won’t match the generated Markdown slugs because the heading text
includes “+” and becomes doubled hyphens. Update the three link targets in this
list to use the exact anchor slugs produced by the corresponding headings, and
verify the references for “set up home assistant mosquitto” and “set up
standalone homebridge mosquitto” resolve correctly.
In `@src/core/MqttModule.cpp`:
- Around line 278-298: The empty-payload branch in
MqttModule::handleUpdateInstall currently returns early, which violates the
documented “install latest” behavior for update/set. Update the payload parsing
in handleUpdateInstall so an empty string falls back to the latest version
target (today that can be kVersion), while keeping the existing otaInFlight()
guard and version-string validation intact. Use the existing handleUpdateInstall
and g_otaStatus flow to preserve the current OTA path and failure reporting.
---
Outside diff comments:
In `@docs/moonmodules/core/services.md`:
- Around line 205-207: The Home Assistant MQTT auto-discovery description is
overstating the entity capabilities in the haDiscovery flow; update the docs
around the MQTT auto-discovery and WLED integration entries to say the
auto-created HA light is brightness-only with on/off, while HSV/palette remains
available only through the separate MQTT surface. Keep the wording aligned with
the haDiscovery payload and the projectMM_<mac6>/config discovery behavior.
In `@docs/usecases/home-automation.md`:
- Around line 54-55: The MQTT auto-discovery description in the home-automation
docs overstates Home Assistant control capabilities for the discovered light
entity. Update the bullet that describes the `haDiscovery` path so it only lists
on/off and brightness support for the auto-created entity, and make it explicit
that HSV/palette control still uses the separate `hsv/set` topic; keep the
wording aligned with the `MQTT auto-discovery` and `WLED integration` bullets so
the split between HA discovery and raw topic control is clear.
🪄 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: d2f3f69c-68af-4061-a45c-b3ca6a16880c
📒 Files selected for processing (24)
docs/backlog/backlog-core.mddocs/moonmodules/core/services.mddocs/usecases/home-automation.mdsrc/core/HttpServerModule.cppsrc/core/HttpServerModule.hsrc/core/MqttModule.cppsrc/core/MqttModule.hsrc/core/moonlive/moonlive_emit.hsrc/platform/desktop/platform_config.hsrc/platform/esp32/platform_config.htest/python/test_wled_json_shape.pytest/scenarios/core/scenario_MoonModule_control_change.jsontest/scenarios/light/scenario_Audio_mutation.jsontest/scenarios/light/scenario_Driver_mutation.jsontest/scenarios/light/scenario_GridLayout_resize.jsontest/scenarios/light/scenario_Layer_base_pipeline.jsontest/scenarios/light/scenario_Layer_memory_1to1.jsontest/scenarios/light/scenario_Layers_composition.jsontest/scenarios/light/scenario_Layouts_mutation.jsontest/scenarios/light/scenario_MultiplyModifier_pipeline.jsontest/scenarios/light/scenario_modifier_chain.jsontest/scenarios/light/scenario_modifier_swap.jsontest/scenarios/light/scenario_perf_full.jsontest/scenarios/light/scenario_perf_light.json
Comment on lines
+24
to
+26
| - **HA + Mosquitto broker add-on** — for MQTT auto-discovery (richer state + HomeKit Bridge). [Set up HA + Mosquitto](#set-up-home-assistant-mosquitto). | ||
| - **HA alone (no broker)** — for the WLED-integration path. Same HA install, just skip the Mosquitto add-on. | ||
| - **Homebridge + a Mosquitto broker** — for the standalone Apple-Home-only path. [Set up standalone Homebridge + Mosquitto](#set-up-standalone-homebridge-mosquitto). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the section links.
These headings include +, so the generated Markdown slugs use doubled hyphens here. The current fragments will not resolve correctly.
♻️ Proposed fix
- - **HA + Mosquitto broker add-on** — for MQTT auto-discovery (richer state + HomeKit Bridge). [Set up HA + Mosquitto](`#set-up-home-assistant-mosquitto`).
+ - **HA + Mosquitto broker add-on** — for MQTT auto-discovery (richer state + HomeKit Bridge). [Set up HA + Mosquitto](`#set-up-home-assistant--mosquitto`).
@@
- - **Homebridge + a Mosquitto broker** — for the standalone Apple-Home-only path. [Set up standalone Homebridge + Mosquitto](`#set-up-standalone-homebridge-mosquitto`).
+ - **Homebridge + a Mosquitto broker** — for the standalone Apple-Home-only path. [Set up standalone Homebridge + Mosquitto](`#set-up-standalone-homebridge--mosquitto`).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - **HA + Mosquitto broker add-on** — for MQTT auto-discovery (richer state + HomeKit Bridge). [Set up HA + Mosquitto](#set-up-home-assistant-mosquitto). | |
| - **HA alone (no broker)** — for the WLED-integration path. Same HA install, just skip the Mosquitto add-on. | |
| - **Homebridge + a Mosquitto broker** — for the standalone Apple-Home-only path. [Set up standalone Homebridge + Mosquitto](#set-up-standalone-homebridge-mosquitto). | |
| - **HA + Mosquitto broker add-on** — for MQTT auto-discovery (richer state + HomeKit Bridge). [Set up HA + Mosquitto](`#set-up-home-assistant--mosquitto`). | |
| - **HA alone (no broker)** — for the WLED-integration path. Same HA install, just skip the Mosquitto add-on. | |
| - **Homebridge + a Mosquitto broker** — for the standalone Apple-Home-only path. [Set up standalone Homebridge + Mosquitto](`#set-up-standalone-homebridge--mosquitto`). |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 24-24: Link fragments should be valid
(MD051, link-fragments)
[warning] 26-26: Link fragments should be valid
(MD051, link-fragments)
🤖 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 `@docs/usecases/home-automation.md` around lines 24 - 26, The section links in
the home-automation bullets point to fragments that won’t match the generated
Markdown slugs because the heading text includes “+” and becomes doubled
hyphens. Update the three link targets in this list to use the exact anchor
slugs produced by the corresponding headings, and verify the references for “set
up home assistant mosquitto” and “set up standalone homebridge mosquitto”
resolve correctly.
Source: Linters/SAST tools
… marker
Bench + Home Assistant finetuning of the WLED-compat and MQTT surfaces. Fixes a capability bug that silently degraded every multi-LED projectMM device to brightness-only (or broke the light entity outright) in HA, adds live device diagnostics, an audio mic-health status, a HA-facing 💫 projectMM name marker, and makes the two HA discovery paths non-overlapping so a device appears once, not twice.
KPI: 16384lights | PC:666KB | tick:121/92/119/15/2/277/58/18/21/160/117/17/1/36us(FPS:8264/10869/8403/66666/500000/3610/17241/55555/47619/6250/8547/58823/1000000/27777) | ESP32:1374KB | tick:3417us(FPS:292) | heap:8333KB | src:177(35669) | test:122(18101) | lizard:131w
Core:
- HttpServerModule: fixed the WLED `/json` `seglc` field — it carried the LED count, but HA reads seglc[segment] as a colour-capability code (1 = RGB); a count like 24 maps to no colour mode, so HA refused to add the light ("does not set supported color modes") and it stayed restored/unavailable while sensors worked. Now the constant 1, matching lc.
- HttpServerModule: added `fx:0` alongside `pal` in the segment — a real WLED segment always pairs effect + palette; sending pal alone is a shape real WLED never produces.
- HttpServerModule: WLED `/json` now reports real device diagnostics (led count, render fps, freeheap, uptime, and wifi bssid/rssi/channel/signal) via the new LightSummary pull; on an Ethernet device the `wifi` object is omitted (python-wled treats it as optional) so HA creates no greyed Wi-Fi sensors.
- HttpServerModule: `writeWledName` prefixes the WLED-facing name with 💫 so projectMM boards stand out among plain WLED devices in HA; the real deviceName (UI, mDNS, MQTT) stays unprefixed.
- HttpServerModule: WLED palette dropdown is now two-way — `/json` lists the real built-in palette names, seg[0].pal reports the active index, and an inbound seg.pal maps to the Drivers palette control (the colour picker follows on the next poll, one source of truth).
- LightSummary: new domain-neutral POD (plain integers, no light types) produced by the Drivers container and read by the core WLED/MQTT consumers via a static pull, the AudioFrame pattern — so core reports the real pipeline shape without reaching into light types.
- MqttModule: `haDiscovery` now defaults OFF (opt-in). The WLED `/json` shim already gives HA a richer light with no broker, so leaving both on listed a device twice; MQTT discovery stays for broker-only / cross-subnet setups.
- platform: added wifiStaBssid / wifiStaChannel getters (esp32 + desktop stubs) for the real wifi diagnostics.
- AudioModule: added a mic-health status — "no samples" (I2S clocks dead) vs "data line silent" (SD dead/unpowered), so a silent mic self-reports which wire is at fault instead of just reading RMS 0.
- AudioModule: reordered the mic pins to the datasheet order SCK/WS/SD/MCLK.
Light domain:
- AudioSpectrum: `colorMode` now defaults to per-band (the rainbow analyser look) instead of height.
- Palette: added `paletteNames` to emit the built-in palette name list for the WLED `/json` palettes array.
- Drivers: produces the LightSummary (light count + channels) on each rebuild via the shared-struct pull.
Tests:
- unit_MqttModule: enable haDiscovery before the discovery-announce assertions now that it defaults off.
Docs / CI:
- gpio-usage.md: new per-MCU do-not-use/use reference; bench mic on the N16R8 is SCK=47/WS=48/SD=21 (45 is a boot strap; on-board LED is 38 on rev v1.0).
- services.md: haDiscovery documented as opt-in (default off), with the WLED-covers-HA rationale.
- deviceModels: testbench-S3 mic pins → 47/48/21; board images converted .jpg→.png and compressed 1.2M→253K each.
- home-automation.md: HA auto-discovery entity description.
Reviews:
- 🐇 MqttModule OTA: empty install payload now installs the latest version instead of returning early (fixed).
- 🐇 services.md / home-automation.md: HA auto-discovery entity corrected to on/off + brightness only; #home-assistant anchor fixed to #adopt-in-home-assistant (fixed).
- 🐇 MkDocs anchor-slug finding: verified invalid — MkDocs slugify already collapses the `+` to a single hyphen, links resolve (skipped, with reason).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ewowi
changed the title
Collaborator
Author
|
@coderabbitai, review 0f67655 and report your findings in copy pastable text for agents |
|
✅ No merge conflicts detected when merging into Your branch is good to go! |
Pre-merge carry-forward for the ha-mqtt branch: record the debugging lessons from the live-in-HA finetuning (the seglc capability bug chief among them), capture the WLED-default / MQTT-opt-in choice as an ADR, and correct the home-automation doc's now-stale "keep both discovery paths on" rationale to match that decision. Docs / CI: - lessons.md: five HA-finetuning lessons — HA reads WLED seglc[] as a capability code not an LED count (the multi-LED light-entity break); a segment needs fx AND pal, not pal alone; a WLED-shim device double-lists over WLED + MQTT unless one is the default; an Ethernet device omits the wifi block rather than sending a zeroed one; a silkscreen 45-read-as-15 put a mic pin on a boot strap (RMS>0 doesn't confirm a mic; A/B a known-good board). - adr/0012: new ADR — Home Assistant discovery defaults to the WLED path, MQTT discovery is opt-in, so a device appears once and the richer no-broker path is the default. Indexed in adr/README.md. - home-automation.md: rewrote the "Let HA auto-create the entity" section WLED-first and replaced the "Why keep both on?" paragraph with "Why is MQTT discovery off by default?", linking ADR-0012 — the earlier text contradicted the shipped default. Reviews: - 🐇 home-automation.md anchor slugs (headings with `+` → doubled hyphens): verified invalid — MkDocs slugify collapses ` + ` to a single hyphen, the links resolve (proven by test_mkdocs_slug.py + a --strict build); skipped. - 🐇 MqttModule empty install payload returns early: verified already fixed on this branch — an empty payload falls back to kVersion (install latest), otaInFlight guard intact; no change. - 🐇 services.md MQTT-discovery entity overstates capabilities: verified already correct — reads on/off + brightness only, colour/palette on the separate hsv/set topic; no change. - 🐇 home-automation.md MQTT-discovery entity overstates capabilities: capabilities already correct; fixed a separate real staleness the finding pointed near — the "keep both on" rationale contradicting the MQTT-off default. - 👾 Reviewer (branch diff): no blocking findings; fixed the one doc nit (LightSummary comment named Layouts::totalLightCount, the producer uses Layer::physicalLightCount). 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.
Summary
One thread across four commits: bring projectMM into Home Assistant end-to-end (WLED integration + optional MQTT discovery + HomeKit via HA's bridge), green the ctest/scenario/pytest suite on Windows, add a HA firmware-update entity, and finetune the whole surface live against a real HA install — where a capability bug that silently broke the light entity on every multi-LED device was found and fixed.
Reach HA (WLED integration, no broker)
/json+/presets.jsonshaped to whatfrenck/python-wled'sDevice.from_dictactually validates, so HA's built-in WLED integration adopts the device with no manual config.col:[[r,g,b]]maps back throughPalettes::nearestForRgb; the representative colour lives in a sharedPalettes::representativeRgbhelper. Golden-vector test (test/python/test_wled_json_shape.py) parses captured JSON through python-wled's real models so a breakage fails locally before HA sees it.MoonModule::readUint8acceptsControlType::Palette(previously returned 0 → every palette read the Rainbow default → white).HA firmware-update entity (MQTT)
MqttModulepublishes a second discovery component athomeassistant/update/projectMM_<mac6>/config; HA renders a "Firmware: " card, and an install action routes toplatform::http_fetch_to_otawith the release-artifact URL. WLED update-badge fixed (versentinel so HA doesn't offer a bricking "WLED 16.0.1" flash).Finetune, live in Home Assistant (the last commit)
seglccapability bug (the headline fix) —/jsonput the LED count ininfo.leds.seglc[], but HA readsseglc[segment]as a colour-capability code (1 = RGB); a count like 24 maps to no colour mode, so HA raiseddoes not set supported color modesand the light entity stayed unavailable while sensors worked. It hid because single-light boards sendseglc:[1](accidentally valid); only multi-LED boards tripped it. Now the constant 1, matchinglc. Also emitfx:0alongsidepal— a real WLED segment always pairs effect + palette. (Recorded in lessons.md.)/jsonreports the actual pipeline shape (LED count, render fps) + freeheap/uptime + wifi bssid/rssi/channel/signal, via a new domain-neutralLightSummaryPOD pulled from the light domain (theAudioFramepattern). On an Ethernet device thewifiblock is omitted (python-wled treats it as optional) so HA creates no greyed Wi-Fi sensors./jsonlists the real built-in palette names,seg[0].palreports the active index, and an inboundseg.palmaps to the Drivers palette control; the colour picker follows on the next poll (one source of truth)./jsonpath already gives HA a richer light with no broker, so leaving both on listed every device twice (oneplatform=wled, oneplatform=mqtt). WLED is the default; MQTT discovery is the opt-in for broker-only / cross-subnet setups. (This reverses the earlier "keep both on" stance — recorded as ADR-0012.)no samples= I²S clocks dead /data line silent= SD dead). Mic pins reordered to the datasheet order SCK/WS/SD. Newdocs/reference/gpio-usage.mdper-MCU pin reference (a GPIO45-read-as-15 strap misread cost an afternoon; the working bench config is SCK=47/WS=48/SD=21).colorModedefaults to per-band (the rainbow analyser look)..jpg→.pngand compressed to ~88 KB.Windows test hardening
SO_REUSEADDRhas the opposite semantic of POSIX. NewMM_MOONLIVE_HAS_HOST_JITcompile-time flag gates the JIT-dependent tests;skip_onlist on the scenario JSONs (honoured by both the Python wrapper and the C++ runner so KPI stays honest);bind()drops SO_REUSEADDR on Windows. UTF-8 stdout reconfigure so piped Python scripts don't crash on cp1252.Docs
docs/usecases/home-automation.mdreorganised device-side first (Adopt in HA / Adopt in Homebridge up front) with a Set up the infrastructure appendix. HomeKit works via HA's HomeKit Bridge re-exposing the WLED-discovered light — no MQTT needed for HomeKit.Test plan
ctest(desktop) — 645 tests, 100% pass;ctest build/windowsgreenuv run moondeck/scenario/run_scenario.py— all pass (MoonLive scenarios cleanly skip on x86_64)uv run --with pytest --with pyserial --with markdown --with wled pytest test/python -q— 55/55 passnode --test "test/js/**/*.test.mjs"— 34/34 passcheck_specs.py82/82 ·check_platform_boundary.pyPASS ·check_devices.py24 boards 0 issuesbuild_esp32.py— esp32 classic + S3 n8r8/n16r8 built and flashed this sessionlight.shellyon/rgb via the HA API); MM-S31 / MM-P4 paired earlier; HomeKit-via-HA-bridge confirmed🤖 Generated with Claude Code