Plan-17: release 1.0 distribution scaffolding#5
Merged
Merged
Conversation
KPI: 16384lights | PC:304KB | tick:71/56/127/108us(FPS:14084/17857/7874/9259) | ESP32:882KB | tick:52377us(FPS:19) | heap:192KB | src:52(7685) | test:30(3716) | lizard:24w
Stand up the path from "git tag v1.0.0" to "end user flashes ESP32 from a
browser." Ships the CI scaffolding, build-script reshape, install page, and
docs; the actual v1.0.0 tag is a separate step the product owner drives.
Four ESP32 board variants replace the previous --profile split:
esp32 - classic ESP32, WiFi only
esp32-eth - classic ESP32, Ethernet only (Olimex pin map baked in,
WiFi components excluded -> smaller image, more free RAM)
esp32-eth-wifi - classic ESP32, Eth + WiFi both compiled in
esp32s3-n16r8 - ESP32-S3 DevKitC-1 with the N16R8 module
(16 MB flash, 8 MB octal PSRAM)
Each S3 SKU gets its own board key because the sdkconfig fragment encodes
flash size + partition table + PSRAM mode; flashing an n16r8 binary onto a
different module misaligns or fails PSRAM init. New SKUs become new keys.
Build script:
- scripts/build/build_esp32.py: BOARDS catalogue + --board flag.
board_cmake_args() composes -DSDKCONFIG_DEFAULTS, -DMM_BOARD_NAME,
conditional -DMM_ETH_ONLY and -DMM_NO_ETH from the board's spec. --profile
kept as a deprecated alias one release. Board-change marker .mm_board
(legacy .mm_profile honoured for migration). build_esp32_ethonly.py
forwards --board esp32-eth.
- scripts/build/verify_version.py: CI guard that fails when tag and
library.json version disagree.
- scripts/build/package_desktop.py: host-aware build + package (macOS arm64
tarball / Windows x64 zip, version from library.json).
- scripts/build/generate_manifest.py: ESP Web Tools manifest per board,
offsets sourced from ESP-IDF's flasher_args.json (ESP32 bootloader at
0x1000, ESP32-S3 at 0x0 - wrong offset bricks visibly).
- scripts/build/generate_build_info.py (renamed from generate_version.py):
emits src/core/build_info.h carrying MM_VERSION + MM_BOARD_NAME +
MM_BUILD_DATE in one header. Generated, gitignored.
Platform layer:
- src/platform/esp32/platform_config.h: hasEthernet constexpr flag gated on
MM_NO_ETH, parallel to the existing hasWiFi/MM_NO_WIFI pair.
- src/platform/esp32/platform_esp32.cpp: ethInit() / ethLinkUp / ethConnected
/ ethGetIP wrapped in #ifndef MM_NO_ETH with stub implementations in the
#else branch matching the desktop platform. Boards without on-chip EMAC
(esp32 classic without the Eth fragment, esp32s3) no longer try to
reference eth_esp32_emac_config_t headers that aren't in scope.
sdkconfig:
- esp32/sdkconfig.defaults: removed the Olimex-specific Eth lines (RMII PHY
config, DMA buffers); the base is now genuinely WiFi-only / board-neutral.
- esp32/sdkconfig.defaults.eth (renamed from .olimex_gw): self-sufficient
Eth fragment, carries the full Olimex pin set + product-page link.
- esp32/sdkconfig.defaults.esp32s3-n16r8 (renamed from .esp32s3_n16r8):
hyphen instead of underscore, matches the board key.
SystemModule:
- New 'board' read-only control (between 'build' and 'chip') showing
MM_BOARD_NAME. Surfaced for support diagnostics today; read by the future
OTA path to pick a matching release asset.
- Reads constants (kVersion, kBuildDate, kBoardName) from the unified
build_info.h rather than the bare MM_VERSION/MM_BUILD_DATE macros.
CI workflow (.github/workflows/release.yml):
- Tag-triggered (push tags v*) + branch-triggered on main / next-iteration
with a paths filter, so branch pushes exercise the 4 ESP32 builds + 2
desktop builds without publishing. verify-version, release, and the three
Pages-related steps gate themselves on startsWith(github.ref, 'refs/tags/')
so branch CI never tries to verify against a tag that doesn't exist or
publish a release that shouldn't exist.
- RC tag handling (vX.Y.Z-rcN):
* gh-release flips prerelease: true on -rc tags
* Pages staging + upload + deploy all skipped on -rc tags
Lets you iterate on RC tags without disturbing the live installer URL.
End users only land on a release the day a stable tag flips Pages.
- Concurrency keyed on github.ref so branch + tag builds don't fight for
the same lock.
- esp-idf-ci-action v1 with v5.4, cache for ~/.espressif + ~/esp/esp-idf.
Install page:
- docs/install/index.html: self-contained ESP Web Tools page. Board dropdown
+ <esp-web-install-button> recreated on each change (mitigates the
manifest-cache risk across ESP Web Tools versions). Inlined CSS echoes the
device-UI palette. Browser-warning banner for Web Serial absence.
- docs/install/README.md: three local-test recipes (page-render only,
end-to-end with downloaded CI artifacts, RC dry-run pointer) + the
GitHub Pages manual-setup instruction.
Docs:
- README.md: "From a release" rewritten - installer URL for ESP32, Releases
page for macOS / Windows desktop. Teensy / RPi / Linux bullets dropped
(not packaged in 1.0; one trailing sentence directs the curious to source
builds + the 2.0 roadmap).
- docs/building.md: "Build profiles" -> "Boards" table with the four
variants, S3-per-SKU rationale, Olimex link, --profile deprecation note.
- docs/plan.md: "## 13 README" + "## Release 1.0" replaced by "## Release
2.0 - distribution catches up to the source tree" (P4 board, OTA, Linux
desktop, Teensy, RPi binaries, nightly CI, Improv WiFi, runtime PHY/pin
config, macOS code-signing). "WiFi runtime disable" updated to reference
the new board keys.
- docs/moonmodules/core/SystemModule.md: documents the new 'board' control
with the full key catalogue.
- docs/moonmodules/core/NetworkModule.md: updated --profile reference.
Pre-merge plan reconciliation (carried from previous merge):
- CLAUDE.md: gate 3 wording specifies "do this on the branch before the
merge commit" so plan reconciliation lands with the merge train, not as a
tail commit on main. New Event-3 conditional gate 7 (principles audit)
with concrete forward-looking phrases to grep for. Reviewer agent moved
from Event 1 to Event 2 mandatory (gate 5), on-demand pre-commit only.
- docs/history/decisions.md: new lessons block covering this branch
(c.min/c.max bound-width trap, per-tick integer division round-to-zero,
early-return on degenerate inputs leaking state, HTML5 dragstart e.target
reality, severity-as-real-axis, lifecycle gates on commit+merge, reviewer
triage, docs hierarchy, util/modules folder rejection, .claude/*.lock
gitignore).
- docs/history/plan-13..16: archived under docs/history/archive/.
Tests:
- test/test_system_module.cpp: control count 11 -> 12 (new 'board' row).
Verification:
- All commit gates passed:
Desktop build: clean, zero warnings (MSVC-gated /W4 /WX flags pass on
gcc/clang via the else branch)
ctest: 1/1 passed
mm_scenarios: 8/8 passed
check_platform_boundary.py: pass
check_specs.py: 21/21 ok
All 4 ESP32 boards build (esp32 1.00 MB, esp32-eth 602 KB,
esp32-eth-wifi 1.05 MB, esp32s3-n16r8 1.01 MB)
collect_kpi.py --commit: live ESP32 capture, monitor.log fresh
- Build-break found and fixed mid-gates: the step-1 sdkconfig restructure
removed CONFIG_ETH_USE_ESP32_EMAC=y from the base file, which broke the
esp32 + esp32s3-n16r8 builds because platform_esp32.cpp's ethInit()
references EMAC-only headers. Resolution: mirror the existing
hasWiFi/MM_NO_WIFI pattern with hasEthernet/MM_NO_ETH (see Platform
layer section above). Exactly the kind of cross-board issue branch CI
catches earlier - confirming why we added the branch trigger.
KPI Details:
Desktop:
Lights: 16,384
Binary: 304 KB
[doctest] test cases: 153 | 153 passed | 0 failed | 0 skipped
tick: 71us, 56us, 127us, 108us (FPS: 14084, 17857, 7874, 9259) (per scenario)
8 scenario(s), 8 passed, 0 failed
Platform boundary: PASS
Specs: 21 modules, 21 ok, 0 missing, 0 outdated
ESP32 (last live monitor, esp32-eth-wifi flashed):
Image: ~1 MB (varies per board, see verification)
Flash (code+data): 882 KB
tick: 52379us (FPS: 19) heap free: ~192 KB
Code:
52 source files (7685 lines)
30 test files (3716 lines)
33 specs, 8 scenarios
Lizard: 24 warnings (pre-existing, mostly in HttpServerModule.cpp +
FilesystemModule.cpp - accepted complexity for the wire
contract handling, see docs/coding-standards.md)
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 7
🤖 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 @.github/workflows/release.yml:
- Around line 14-46: Add a top-level permissions block to the workflow to
declare minimal defaults (e.g., set permissions: contents: read) so non-release
jobs don’t inherit broad rights; then keep or add the elevated permissions only
inside the release job definition (the job named "release" or the job that
performs the tag-driven publication) to grant whatever write/token scopes it
needs. Update the file surrounding the existing top-level keys (on:,
concurrency:) to include this global permissions block and ensure the release
job's own permissions section overrides it with the higher privileges.
- Line 56: Replace all mutable action version tags (e.g., actions/checkout@v4,
actions/upload-artifact@v4, actions/download-artifact@v4,
actions/upload-pages-artifact@v3) with their corresponding immutable commit
SHAs: locate each occurrence of the `uses:` entries in the release workflow
(instances of `actions/checkout@v4`, `actions/upload-artifact@v4`,
`actions/download-artifact@v4`, `actions/upload-pages-artifact@v3`) and replace
the tag with the GitHub Actions repository commit SHA (found on the action's
GitHub releases or commit history) so each `uses:` reads like
`repo@<commit-sha>`; ensure you pick the SHA that matches the intended
tag/release and update all listed occurrences consistently, then run a CI
dry-run or workflow lint to verify syntax.
- Around line 53-65: The job-level if on the verify-version job causes
downstream jobs (like build-esp32 which has needs: verify-version) to be skipped
when the job is skipped; remove the job-level if for the verify-version job and
instead apply the conditional to the step named "Verify tag matches library.json
version" (use the same expression: startsWith(github.ref, 'refs/tags/') ||
github.event_name == 'workflow_dispatch') so the verify-version job always runs
but only executes the verification step for tags or workflow_dispatch, keeping
the GITHUB_REF_NAME env and the existing run: python
scripts/build/verify_version.py intact.
In `@docs/install/index.html`:
- Around line 12-14: The script tag loading "install-button.js" in
docs/install/index.html is pinned loosely to "`@10`" and lacks SRI/crossorigin
protection; update the src to an exact immutable artifact URL (include the full
exact version and file path, e.g., the specific release file instead of "`@10`"),
and either add a proper integrity attribute plus crossorigin="anonymous" for
Subresource Integrity or replace the src with a same-origin self-hosted path
(ensuring the hosted file matches the pinned artifact). Target the <script ...
src=".../install-button.js?module"> element when making this change.
In `@docs/moonmodules/core/SystemModule.md`:
- Line 21: Update the `board` description to use present-tense wording: replace
the forward-looking phrase "Read by the future OTA path to pick a matching
release asset" with a present-tense statement such as "Used by OTA asset
selection to match release artifacts by board key." Keep the rest of the line
intact (the `board` read-only description and the example board keys) and ensure
the `board` symbol is described in present tense per the documentation
guidelines.
In `@README.md`:
- Around line 24-26: Update the README section titled "Desktop — download and
run." to use present tense and remove future/roadmap phrasing: replace
"Code-signing is on the roadmap", "Teensy, Raspberry Pi, and Linux desktop
builds aren't packaged in 1.0", and "distribution catches up in 2.0" with
present-tense facts about current behavior (e.g., state that macOS Gatekeeper
prompts on first run and provide the current distribution status for
Teensy/Raspberry Pi/Linux) so the content describes the system as it is now;
edit the sentences in that paragraph only to remove forward-looking language and
ensure concise present-tense statements.
In `@test/test_system_module.cpp`:
- Around line 17-23: The test currently only checks the total number of controls
via sys.controls().count() == 12, which can miss a missing or empty "board"
control; update the test to explicitly lookup and assert the "board" control
exists and is non-empty by calling sys.controls().find("board") (or equivalent
map/index access) and asserting the returned control is present and its
value/string is not empty (or matches expected format), in addition to keeping
the existing count check.
🪄 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: dedf114d-6619-4d75-a23c-a09ab4cf7409
⛔ Files ignored due to path filters (7)
scripts/build/build_esp32.pyis excluded by!**/build/**scripts/build/build_esp32_ethonly.pyis excluded by!**/build/**scripts/build/generate_build_info.pyis excluded by!**/build/**scripts/build/generate_manifest.pyis excluded by!**/build/**scripts/build/generate_version.pyis excluded by!**/build/**scripts/build/package_desktop.pyis excluded by!**/build/**scripts/build/verify_version.pyis excluded by!**/build/**
📒 Files selected for processing (29)
.github/workflows/release.yml.gitignoreCLAUDE.mdCMakeLists.txtREADME.mddocs/building.mddocs/history/archive/plan-13.mddocs/history/archive/plan-14.mddocs/history/archive/plan-15.mddocs/history/archive/plan-16.mddocs/history/decisions.mddocs/history/plan-17.mddocs/install/README.mddocs/install/index.htmldocs/moonmodules/core/NetworkModule.mddocs/moonmodules/core/SystemModule.mddocs/plan.mdesp32/main/CMakeLists.txtesp32/sdkconfig.defaultsesp32/sdkconfig.defaults.esp32s3-n16r8esp32/sdkconfig.defaults.ethesp32/sdkconfig.defaults.olimex_gwscripts/MoonDeck.mdscripts/moondeck.pyscripts/moondeck_config.jsonsrc/core/SystemModule.hsrc/platform/esp32/platform_config.hsrc/platform/esp32/platform_esp32.cpptest/test_system_module.cpp
💤 Files with no reviewable changes (1)
- esp32/sdkconfig.defaults.olimex_gw
| if: startsWith(github.ref, 'refs/tags/') || github.event_name == 'workflow_dispatch' | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Report action refs that are not pinned to full commit SHAs.
rg -n '^\s*-\s*uses:\s*[^@]+@' .github/workflows/release.yml | \
rg -n -v '@[0-9a-fA-F]{40}$'Repository: ewowi/projectMM
Length of output: 501
Pin third-party GitHub Actions in .github/workflows/release.yml to commit SHAs
Mutable tags like @v4 weaken supply-chain guarantees in a release pipeline; pin each affected uses: to an immutable commit SHA.
- Line 56 (
actions/checkout@v4) - Line 71 (
actions/checkout@v4) - Line 109 (
actions/upload-artifact@v4) - Line 118 (
actions/checkout@v4) - Line 121 (
actions/upload-artifact@v4) - Line 130 (
actions/checkout@v4) - Line 133 (
actions/upload-artifact@v4) - Line 152 (
actions/checkout@v4) - Line 154 (
actions/download-artifact@v4) - Line 196 (
actions/upload-pages-artifact@v3)
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 56-56: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 56-56: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 @.github/workflows/release.yml at line 56, Replace all mutable action version
tags (e.g., actions/checkout@v4, actions/upload-artifact@v4,
actions/download-artifact@v4, actions/upload-pages-artifact@v3) with their
corresponding immutable commit SHAs: locate each occurrence of the `uses:`
entries in the release workflow (instances of `actions/checkout@v4`,
`actions/upload-artifact@v4`, `actions/download-artifact@v4`,
`actions/upload-pages-artifact@v3`) and replace the tag with the GitHub Actions
repository commit SHA (found on the action's GitHub releases or commit history)
so each `uses:` reads like `repo@<commit-sha>`; ensure you pick the SHA that
matches the intended tag/release and update all listed occurrences consistently,
then run a CI dry-run or workflow lint to verify syntax.
KPI: 16384lights | PC:304KB | tick:71/57/128/109us(FPS:14084/17543/7812/9174) | ESP32:919KB | tick:52462us(FPS:19) | heap:192KB | src:52(7701) | test:30(3739) | lizard:24w
Two streams of fixes from the same review cycle:
Branch CI was broken - the plan-17 push fired the workflow and all five
jobs reported `skipped`. Root cause: verify-version had a tag-only job-
level `if:`, and the build jobs declared `needs: verify-version`. GitHub's
default `needs:` operator treats a `skipped` dependency as a blocking
state, so every build inherited the skip. The repo reviewer caught the
same issue independently and suggested the cleaner shape adopted here.
Reviewer findings (in the same review pass) - applied or skipped each
against current code per CLAUDE.md "verify each finding" rule.
Branch CI fix:
- Move the if: condition from the verify-version *job* to the *step* inside
it. The job now always runs (so `needs: verify-version` is satisfied for
every trigger), and the verification step only executes when there's
actually a tag to verify (tag push or workflow_dispatch).
- This is cleaner than the per-build `if: needs.verify-version.result !=
'failure'` guards I'd drafted locally - adopted the reviewer's shape.
Reviewer findings APPLIED:
- release.yml: add a top-level `permissions: contents: read` default. The
release job keeps its own write+id-token block. Verify and build jobs
inherit read-only, which is all they need.
- docs/install/index.html: ESP Web Tools pinned to exact 10.2.1, integrity
hash + crossorigin="anonymous" on the entry. Note in the comment that the
entry-file SRI does not cover the chained module imports - full self-host
is a 2.0 hardening item.
- docs/moonmodules/core/SystemModule.md: 'board' description rewritten in
present tense ("Identifies which release asset matches the device" rather
than "Read by the future OTA path").
- README.md: present-tense rewrite of the desktop block. macOS Gatekeeper
is described as current behaviour with the quarantine-clear command;
Teensy/RPi/Linux distribution status now points at docs/plan.md instead
of "2.0 roadmap" phrasing.
- test/test_system_module.cpp: explicit 'board' control check (exists +
ReadOnly + non-empty), mirroring the existing bootReason pattern. The
count == 12 check is kept as a structural guard.
Reviewer findings SKIPPED:
- SHA-pin first-party actions (actions/checkout etc.): tag-pinning is the
established WLED / ESPHome / Home Assistant pattern for first-party
Espressif/GitHub actions; SHA-pinning without dependabot rewrites adds
friction. 2.0 hardening if it ever becomes a concern.
- ControlType::Uint8/Uint16 bounds check via c.min/c.max: already addressed
in an earlier branch. The reviewer's suggested re-fix would re-introduce
the c.min/c.max width trap documented in decisions.md (the fields are
uint8_t and can't bound a uint16_t range).
Reviewer findings APPLIED (pre-existing bugs found by the rabbit, fixed
in this commit per the "fix when found" policy):
- src/core/Base64.h: base64Encode signature migrated to std::span<const
uint8_t> input + std::span<char> output. The size contract is now
enforced by span bounds. Both call sites in HttpServerModule.cpp updated
(WS handshake + password obfuscation).
- src/core/HttpServerModule.cpp handleDeleteModule: was leaking descendant
modules. `mod->teardown(); delete mod;` only freed the children-pointer
array (MoonModule's destructor calls `delete[] children_`); each child
module the array pointed to was leaked. Replaced with the
`teardown() + Scheduler::deleteTree(mod)` pair that handleReplaceModule
already uses. Real memory leak on a tree delete from the UI.
- src/core/HttpServerModule.cpp handleAddModule: empty parent_id caused
an orphan - the module was constructed, set up, allocated memory, but
never registered with any tree or the scheduler. Rejected with 400 now
(top-level modules are policy-fixed in main.cpp and never added via the
HTTP surface). Matches handleDeleteModule and handleReplaceModule's
top-level policy. The parent lookup also moved before the factory create
so a 404 on a bad parent_id no longer constructs-then-discards.
Verification: all commit gates passed:
- Desktop build: clean, zero warnings
- ctest: 1/1 (includes the new board-control check)
- mm_scenarios: 8/8
- check_platform_boundary.py: pass
- check_specs.py: 21/21
- All 4 ESP32 boards build clean (esp32 1.00 MB, esp32-eth 602 KB,
esp32-eth-wifi 1.05 MB, esp32s3-n16r8 1.06 MB)
- collect_kpi.py --commit: live ESP32 capture
The real test for the CI fix is the next branch push: build jobs should
report `in_progress` / `success` while verify-version still skips its
internal step + release stays `skipped`.
KPI Details:
Desktop:
Lights: 16,384
Binary: 304 KB
[doctest] test cases: 154 | 154 passed | 0 failed | 0 skipped
tick: 71us, 57us, 128us, 109us (FPS: 14084, 17543, 7812, 9174)
8 scenario(s), 8 passed, 0 failed
Platform boundary: PASS
Specs: 21 modules, 21 ok
ESP32 (live monitor, esp32-eth-wifi flashed):
Flash (code+data): 919 KB
tick: 52462us (FPS: 19), heap free: ~192 KB
Code:
52 source files (7701 lines, +16 vs prev)
30 test files (3739 lines, +23 - the new board-control test)
33 specs, 8 scenarios
Lizard: 24 warnings (pre-existing)
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The first branch CI run after the plan-17 push surfaced three Windows-
specific failures, all stacked on the same push:
1. generate_build_info.py crashed on `OUT_FILE.write_text(content)`.
Python's Path.write_text() defaults to the platform's preferred codec
(cp1252 on Windows), which can't encode the em-dashes + arrow I'd put
in the docstring comments. Fix: (a) replaced the non-ASCII characters
with ASCII (->, --, plain colon) so the generated file is portable,
(b) forced encoding="utf-8" on both read and write so a future edit
slipping a non-ASCII character back into the comments doesn't reopen
the wound.
2. platform_esp32.cpp won't compile under ESP-IDF v5.4 - the v5.x EMAC
config has `emac_rmii_clock_gpio_t clock_gpio` (strongly-typed enum),
v6 has `int clock_gpio`. `static_cast<int>(GPIO_NUM_17)` only
compiles against v6. This is the plan-17 risk-1 firing - the original
workflow pinned `esp_idf_version: v5.4` despite the project being on
v6.1-dev locally.
Fix: bump CI's IDF to `v6.1-dev` (the rolling Docker tag on
espressif/idf, which tracks the same v6.1-dev line the local project
pins to via SHA d1b91b79b5). Source-side fix would have been wrong -
the project deliberately uses v6 APIs (esp_eth_phy_new_generic, new
mDNS component manager). Cache key bumped to esp-idf-v6.1-dev-* to
invalidate the stale v5.4 tooling cache.
Risk going forward: `v6.1-dev` is a rolling tag - CI may pick up a
newer dev snapshot than the local SHA. Acceptable for now; pin to an
exact SHA via manual IDF checkout if drift bites.
3. platform_desktop.cpp won't compile under MSVC - it includes POSIX
socket headers (sys/socket.h, netinet/in.h, arpa/inet.h, unistd.h,
fcntl.h) and calls POSIX-only APIs (sendmsg + MSG_DONTWAIT, fcntl
F_GETFL/F_SETFL with O_NONBLOCK, ::read/::write on socket fds, errno
after socket calls). Windows uses winsock2.h + ws2tcpip.h, WSASend
with WSABUF, ioctlsocket FIONBIO, recv/send, closesocket,
WSAGetLastError, plus WSAStartup/Cleanup bookkeeping.
This is a real platform-layer port (2-3h of careful translation +
manual Windows-side testing), and it's scope creep for plan-17 which
was supposed to ship distribution infrastructure, not new platform
support. Removed the build-windows job + the dist/projectMM-*.zip
upload from release.yml; documented as the top item in docs/plan.md
("Windows desktop port - blocker for 1.0 Windows binary").
Trade picture: with build-windows removed, v1.0.0 can ship the 4
ESP32 firmwares + macOS arm64 desktop without being blocked on
Windows. Adding it back is a single-job restore + glob restore once
the port lands; the surrounding scaffolding stays identical.
README.md updated to say macOS-only desktop binary today, with a link
to docs/plan.md for the Windows status. Plan-17.md gets a 2-item post-
implementation note at the top recording both divergences (no Windows
binary, IDF version bumped to v6.1-dev in CI to match local).
No source changes - desktop build and tests still clean (unchanged
from the previous commit's gate run). The real verification is the
next branch CI push:
- verify-version + release should report `skipped` (branch trigger)
- build-esp32 x 4 should now go GREEN (no more emac type mismatch)
- build-macos should go green (no change)
- No build-windows job at all
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The previous shape had two #-prefixed reminder lines inside the multi-line files: input. softprops/action-gh-release treats every line as a literal glob pattern (no comment stripping), so fail_on_unmatched_files: true tripped on '# dist/projectMM-*.zip - re-enable when ...' with: Error: Pattern '# dist/projectMM-*.zip ...' does not match any files. Surfaced during the v1.0.0-rc1 dry-run on next-iteration: build matrix all green, gh-release step failed before publishing, no release artifacts attached. Cleanup: delete the orphan tag (no release exists to delete), retry under the same rc1 number (clean since nothing shipped). Fix: drop the #-prefix lines from inside files:. The Windows-restoration reminder is now a real YAML comment immediately above the block. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
ewowi
added a commit
that referenced
this pull request
Jun 3, 2026
Moondeck.json restructures from a flat device list + single port into named
networks, each holding its own devices, last-used serial port, and WiFi
credentials. Header network bar auto-selects from host's current subnet on
load; manual override pins. Improv WiFi reads the active network's creds —
wifi_credentials.json retired. Existing moondeck.json migrates in place.
KPI: n/a (no functional src/ changes — only comments in scenario_runner.cpp)
Tests
- scenario_runner.cpp: KEEP IN SYNC comment pointing at run_live_scenario.py tolerance defaults.
- unit_ArtNetSendDriver_no_alloc_in_loop.cpp: present-tense ("would need" → "needs", "grow happens" → "grow runs"). [CodeRabbit]
- unit_FilesystemModule_persistence.cpp: present-tense ("would have caught" → "catches"). [CodeRabbit]
- scenario_PreviewDriver_detail.json: present-tense descriptions ("must still hit" → "still hits", "must not affect" → "does not affect"). [CodeRabbit]
- All scenario JSONs: routine observed-range widenings from gate sweeps.
Scripts / MoonDeck
- moondeck.py: load_state() detects legacy flat shape and runs _migrate_to_networks (buckets by /24 subnet, largest bucket "Home"); save_state() strips volatile per-device fields including conditionally board when it equals the deduced value; new _active_network / _auto_select_network / _deduce_board helpers; /api/state GET auto-selects on host subnet, /api/discover attributes found devices to matching network, /api/refresh refreshes single network's devices; flash-event breadcrumb still links last_port. NAMING COLLISION breadcrumb added to _deduce_board docstring pointing at docs/plan.md. KEEP IN SYNC note between _deduce_board and the JS board picker list. contextlib.suppress for three inline try/except sites. [CodeRabbit #5]
- moondeck_ui/app.js: getActiveNetwork() helper; renderNetworkBar() + setupNetworkBar() build the dropdown, Rename/Add buttons, WiFi panel; applyNetworkBarVisibility() hides the bar on PC tab; every state.devices / state.port site re-routed through the active network; cross-ref comment at the hardware-board picker pointing at moondeck.py::_deduce_board.
- moondeck_ui/index.html: network-bar div above the per-tab content in the sidebar.
- moondeck_ui/style.css: .network-bar + .network-wifi styling; .device-board picker layout fix.
- build/host_wifi.py: primary credentials source is the active network's wifi block in moondeck.json; OS auto-detect fallback stays.
- check/collect_kpi.py: 18 FPS banner → derived from MIN_ESP32_FPS_LED_PRODUCT (currently 10). [CodeRabbit #3]
- scenario/run_live_scenario.py: max_alloc_block 0 fails when contract demands >0 (was silently passing). [CodeRabbit #6]; KEEP IN SYNC comment about tolerance defaults mirroring scenario_runner.cpp.
- scenario/_observed.py: shared widen-only range update for observed.<target> blocks (factored from both runners).
- docs/screenshot_modules.py: --extras-only flag + _ExtrasOnlyDone sentinel; lets MoonDeck UI re-shoots run without projectMM.
- build/flash_esp32.py: writes scripts/.last_flash.json breadcrumb on success; MoonDeck consumes it to attribute last_port.
Deleted
- scripts/build/wifi_credentials.example.json — the live source moved into moondeck.json's network records.
- .gitignore entry for scripts/build/wifi_credentials.json.
Docs
- testing.md: new Live tab screenshot; observed-range model wording.
- building.md: MoonDeck PC tab + ESP32 tab screenshots placed in their sections.
- MoonDeck.md: Network bar UI Features bullet; improv_provision section updated for the new active-network credentials flow.
- docs/plan.md: "Board vs firmware separation, runtime board presets (multi-commit, started)" roadmap.
- docs/tests/unit-tests.md + scenario-tests.md: regenerated (esp32 row now included in PreviewDriver tables from stale-doc fix). [CodeRabbit #1]
- assets/screenshots/moondeck_{pc,esp32,live}.png: re-captured with the network bar.
Reviews
- 🐰 CodeRabbit: 6 fixed (#1 esp32 row from stale doc → regen; #2 unit-test tense; #3 18 FPS → 10 FPS; #5 contextlib.suppress; #6 max_alloc_block 0 fails; #7 scenario descriptions present-tense). 1 skipped: #4 (show tolerated contract values in perf table) — conflates contract (promise) with tolerance (measurement noise); reviewer agent agreed.
- 👾 Reviewer (Opus, branch diff): ship with changes; 3 architectural calls accepted (sync comments for tolerance defaults, board naming-collision breadcrumb, hardware-board catalog cross-ref) + 1 minor refactor (FPS helper docstring extended to name shared core).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Jun 3, 2026
ewowi
added a commit
that referenced
this pull request
Jun 4, 2026
Adds the LOLIN S3 N16R8 board entry to the catalog and the full chain
of fixes it needed: a writable WiFi-TX-power cap to prevent the
on-module LDO brown-out, USB-Serial-JTAG support in the Improv
listener so the device responds on native-USB S3 boards, a CORS
preflight handler so cross-origin POSTs from the web installer
actually reach `/api/control`, and a generic per-board controls
fan-out across MoonDeck and the installer paths so future per-board
fields land without code changes. Web installer also gains a
Monitor button + retry affordance, and `maxBlock` now reports
internal-RAM block (useful) instead of PSRAM block (always ~8MB).
KPI: tick:153831us(FPS:6) on esp32s3-n16r8 with txPower capped to
8 dBm (LOLIN brown-out fix). Slower than full-power ArtNet because
8 dBm cuts radio TX margin — known hardware trade-off, not a code
regression.
Core:
- NetworkModule: added writable `txPowerSetting` (0..21 dBm, 0=no
override) re-applied on change via syncTxPower(). Setting back to 0
actively pushes 80 quarter-dBm to undo any prior cap (no IDF
"reset to default" API exists; sticky-cap was the previous behavior).
- BoardModule: ASCII-printable validation rationale documented at
setBoard() (JSON encoding, UI rendering, C-string round-trip).
- HttpServerModule: added OPTIONS preflight handler returning 204 +
CORS headers. Root cause of every "cross-origin POST silently
blocked" symptom — the orchestrator's HTTP inject from preview now
lands properly. Also documented path-agnostic preflight choice.
- SystemModule: maxBlock display switched to maxInternalAllocBlock;
added comment explaining how PSRAM is detected (heap-size derivation,
not a flag).
Platform:
- platform.h / platform_esp32.cpp / platform_desktop.cpp: new
`maxInternalAllocBlock()` API alongside existing `maxAllocBlock`.
Internal-only block is the memory-pressure KPI; the all-memory
variant reports ~8 MB on PSRAM boards and tells you nothing.
Layer::canAllocate keeps the all-memory call (light buffers may
live in PSRAM); every diagnostic site (SystemModule, main.cpp tick
log, HttpServerModule /api/state, scenario_runner) now uses the
internal-only variant.
- platform.h / platform_esp32.cpp / platform_desktop.cpp: new
`wifiSetTxPower(int8_t quarterDbm)` setter wrapping
esp_wifi_set_max_tx_power. Clamps into ESP-IDF's 8..84 range.
- platform_esp32_improv.cpp: USB-Serial-JTAG read+write path added
alongside UART0 (guarded by SOC_USB_SERIAL_JTAG_SUPPORTED). LOLIN
S3 N16R8 and other native-USB S3 boards expose USB-C through
USB-Serial-JTAG, not UART0 — without this the Improv task was deaf
to host writes on those boards. Both transports drained
symmetrically each loop with a single 10 ms yield when both are
empty; ESP_LOGW for driver-install warnings + compound
`"listening (uart unavailable)"` status so partial-transport
failures stay visible.
UI:
- src/ui/install-picker.js: added `installRowExtras` slot so the
installer page can slot the Erase-chip checkbox between firmware
dropdown and Install button without the on-device OTA UI inheriting
installer-specific affordances.
- src/ui/app.js: sendControl() now logs non-ok HTTP + network errors
to console.warn (no retry — design intent for the single-shot
`?board=` consume path).
Scripts / MoonDeck:
- MoonDeck `_push_board_to_device` now fans out the full
`controls.<Module>.<control>` block from boards.json on every push
(was Board.board only). Falls back to bare-board for catalog-
missing names; no-op on empty board. Generic — any future per-board
control in boards.json lands via MoonDeck without code changes.
- docs/install/boards.json: new "LOLIN S3 N16R8" entry injecting
`Network.txPowerSetting: 8` (8 dBm — well below the ~13 dBm
threshold ESPHome documents for the brown-out symptom).
- docs/install/index.html: rich installer-UI changes — Monitor
button + serial-monitor modal (Reset/Clear/Close, autoscroll,
UTF-8 decode, RTS-pulse reset), Retry button on the "Improv not
detected" dialog with stale-port probe + 250 ms post-close wait
for SDK lock release, lazy-render WiFi-creds form (avoids macOS
iCloud Passwords prompt on page load), Skip-creds path exits
cleanly via onSuccess({url:""}) rather than failing in provision(),
HTTP injection runs on the Improv-success path too (was needsIp-
only — fixes the "txPower not applied after Improv install"
symptom), 2 s → 3 s post-flash port-reopen wait (boot race against
Improv task init on S3), monitor button auto-disabled during
install (Web Serial port-mutex enforcement), pendingBoard fallback
now covers Improv-success HTTP fail.
- docs/install/install-orchestrator.js: stale-prePickedPort probe
with fallback to requestPort() in both start() and eraseOnly();
isTransientImprovError helper extracted next to isImprovNotDetected
so SDK error-string drift has one update site; uiShowNeedsIpRetrying
contract reconciled with actual behavior.
- docs/install/devices.js: pendingBoard explicitly cleared on
successful inject (was leaking across re-installs); erase confirm
text rewritten to reflect the orchestrator-driven flow (EWT
branding removed).
Docs / CI:
- docs/install/README.md, scripts/MoonDeck.md: replaced "ESP Web
Tools installer page" branding with the orchestrator-driven
description (present tense, no backward-looking change-log talk).
- docs/moonmodules/core/BoardModule.md: MoonDeck section updated to
reflect full controls fan-out; "Pre-WiFi limitation" paragraph
trimmed to a one-line pointer; full timing-constraint rationale
moved to docs/history/decisions.md as the lesson worth carrying
forward.
- docs/moonmodules/core/NetworkModule.md: txPowerSetting bullet
added (trimmed to wire contract + intended use).
- docs/history/decisions.md: new "Board-injection pipeline timing
constraint" entry — names the controls that wouldn't tolerate the
HTTP-after-WiFi fan-out (country code, antenna selector, pre-
association TX-power) and the two escape hatches (new vendor RPC,
sdkconfig bake). Warns against extending SET_BOARD's wire format.
Tests:
- test/scenario_runner.cpp: switched to maxInternalAllocBlock for
the regression KPI; updated rationale to explain why (PSRAM
boards previously masked internal-heap fragmentation).
- test/unit/core/unit_BoardModule.cpp: added c.readonly assertion
on the board control — regression guard against accidentally
making the BoardModule.board field user-editable.
Reviews:
- 👾 Reviewer (on-demand pre-commit): 0 blockers, 10 should-fix, 12
nits — all 22 addressed in this commit. Highlights: #1
syncTxPower=0 now actively lifts the cap (was sticky until reboot);
#2 pendingBoard fallback covers the Improv-success HTTP-fail path
the LOLIN feature most needs to defend; #4 Monitor button now
actually disabled during install (matched the documented mutex);
#5 Improv driver-install warnings now logged via ESP_LOGW and
carried in compound status (were silently overwritten by
"listening"); #7 JTAG TX timeout 50 ms → 0 (truly non-blocking);
#8 UART/JTAG polling made symmetric. The remaining items are
comment trims, sentinel-vs-tagged-union docstring clarifications,
and the 32-vs-34-quarter-dBm example fix in platform.h.
- 🐇 CodeRabbit (prior round): 10 findings processed earlier in the
branch; 2 rejected with reason (HttpServerModule single-line `if
(c.readonly)` brace fix — no clang-tidy config in the repo, and
the codebase has many single-line ifs; consumePendingBoardParam
retry-token — contradicts the documented "no retry" single-shot
design).
KPI Details:
Desktop:
Lights: 16,384
Binary: 375 KB
[doctest] test cases: 207 | 207 passed | 0 failed | 0 skipped
tick: 352us, 99us, 304us, 100us, 99us, 100us, 45us, 35us, 35us
(FPS: 2840, 10101, 3289, 10000, 10101, 10000, 22222, 28571, 28571)
=== 10 scenario(s), 10 passed, 0 failed ===
Platform boundary: PASS
Specs: 26 modules, 26 ok, 0 missing, 0 outdated
ESP32 (esp32s3-n16r8 on LOLIN S3 N16R8 at txPower=8 dBm):
Image: 1,307,517 bytes (69% partition free)
Flash (code+data): 1171 KB
tick: 153831us (FPS: 6)
free heap: 8354995 maxBlock (internal): 163840 (160 KB)
ArtNetSend: ~105 ms/tick (LOLIN at 8 dBm — known trade-off)
Code:
62 source files (10491 lines)
38 test files (5410 lines)
39 specs, 10 scenarios
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ewowi
added a commit
that referenced
this pull request
Jun 4, 2026
PR-merge review pass turned up 12 items; 5 fixed pre-merge per the Reviewer's "quick wins worth landing before merge" recommendation, 2 deferred to docs/plan.md as concrete follow-ups, 5 deferred as nits. Plus the gate-3 (carry-forward lessons) and gate-7 (live perf snapshot to docs/performance.md) outputs that the PR-merge checklist mandates whenever the branch touches tick-path code. KPI: tick:138506us(FPS:7) on esp32s3-n16r8 — unchanged from commit 6212fc0 (this round's only src/ change is a desktop stub return- value tweak, no ESP32 behavioral delta). Core: - platform_desktop.cpp: wifiSetTxPower(0) now returns true to match the documented "0 = no-override = success" contract — same shape the MM_NO_WIFI ESP32 stub already uses. Non-zero still returns false (no radio to set). Without this, NetworkModule's syncTxPower on desktop never updated appliedTxPowerSetting_ and retried on every loop1s tick — harmless but inconsistent. Docs / CI: - docs/install/index.html: user-visible browser-warning text "ESP Web Tools needs Web Serial" → "Web Serial is required". EWT is gone; the warning lives on past it. - docs/install/README.md: same string at the bottom of the local- preview docs. - docs/moonmodules/core/BoardModule.md: rewrote the past-tense "This commit replaced ESP Web Tools' install button..." paragraph as a present-tense statement about what the orchestrator does today. Per § Principles, module spec pages describe the system as-is; changelog-style narrative lives in history. - docs/moonmodules/core/NetworkModule.md: "Board-specific GPIO config ... will be handled by a future IO module" → present-tense ("is hardcoded per board via compile-time defines in sdkconfig.defaults.<board>"). Forward-looking promises belong in plan.md. - docs/history/decisions.md: three new "Lessons from the LOLIN S3 N16R8 enablement branch" entries — USB-Serial-JTAG vs UART0 on S3 (the dual-stdio mirror that made the wrong inference cheap), CORS preflight silent-failure (the root cause behind every "inject silently fails" symptom), and cache-staleness on hardware-event invalidation (the appliedTxPowerSetting_ pattern). Plus the ESP-Web-Tools replacement rationale moved out of BoardModule.md. - docs/plan.md: two new open-follow-up entries under § Board injection — per-control validator hook on ControlDescriptor (the right fix for the HTTP-vs-Improv board-name validation asymmetry), and shared JS helpers across device-UI and web-installer (defer until a second cross-context helper earns the 5-file build-glue cost). - docs/performance.md: new "ESP32-S3 — LOLIN S3 N16R8" section covering tick budget at txPower=8 dBm (~138 ms / FPS 7), the ArtNet-slow-at-low-TX explanation, memory layout (Layer in PSRAM, maxBlock as internal-RAM-only KPI), and the use-case framing (LOLIN fits "lots of PSRAM, accept WiFi compromise"; Ethernet boards win on FPS). Reviews: - 👾 PR-merge Reviewer (whole branch diff, 7 commits): 0 blockers, 7 should-fix, 5 nits. (a) #1-3 (stale EWT copy, past-tense spec, forward-looking spec) → fixed above. (b) #10 (desktop stub contract) → fixed above. (c) #4 (validation hook) + #6 (shared helpers) → documented as follow-ups in plan.md. (d) #5, #7-9, #11-12 → deferred per the Reviewer's own "future-work, not merge blocker / low priority / tiny / minor / nit" framing. KPI Details: Desktop: Lights: 16,384 Binary: 375 KB [doctest] test cases: 207 | 207 passed | 0 failed | 0 skipped tick: 354us, 100us, 303us, 99us, 99us, 98us, 45us, 35us, 34us (FPS: 2824, 10000, 3300, 10101, 10101, 10204, 22222, 28571, 29411) === 10 scenario(s), 10 passed, 0 failed === Platform boundary: PASS Specs: 26 modules, 26 ok, 0 missing, 0 outdated ESP32 (esp32s3-n16r8 on LOLIN S3 N16R8 at txPower=8 dBm): tick: 138506us (FPS: 7) [unchanged from 6212fc0 — desktop- only src/ change this round, no ESP32 behavioral delta] Code: 62 source files (10591 lines) 38 test files (5410 lines) 39 specs, 10 scenarios Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ewowi
added a commit
that referenced
this pull request
Jun 12, 2026
…bit review Renames the audio module from MicModule to AudioModule (it does audio acquisition plus level/FFT analysis, and will gain line-in / USB sources beyond the I²S mic, so the name now reflects the module, not one source). Makes the primary-source references in the docs clickable (datasheets, Espressif API pages, vendor sites) per the spec-and-test principle. Processes the CodeRabbit review on the prior commit: six findings fixed, six skipped with a stated reason. KPI: 16384lights | PC:340KB | tick:121/89/119/122/56/2/37/21/117/342us(FPS:8264/11235/8403/8196/17857/500000/27027/47619/8547/2923) | tick:21177us(FPS:47) | heap:33066KB | src:87(16753) | test:53(8518) | lizard:68w Core: - MicModule.h → AudioModule.h (git mv, history preserved): class MicModule → AudioModule, registration string "MicModule" → "AudioModule", the latestFrame() static, the main.cpp wiring. The mic-source platform seam (hasI2sMic, audioMicRead, AudioMicHandle) is deliberately NOT renamed: it correctly names today's actual source; it broadens when line-in lands (concrete-first). Verified live on the S3: AudioModule registers and runs (AudioModule:523us in the tick breakdown), no boot-loop. Light domain: - AudioLevel.h: clamp the DcBlocker IIR output to the int32 range before narrowing (a settling transient could push the float past int32 bounds, which is UB on the cast). 🐇 #1. - AudioSpectrumEffect.h: fix a height-gradient off-by-one — the gradient divided by (h-1) but the spectrum spans specH rows; when the bottom row is the level meter (specH == h-1) the top spectrum row never reached full red. Now divides by (specH-1). 🐇 #4. - AudioVolumeEffect.h: defensively zero any per-light channels beyond RGB (buffers are RGB/cpl=3 today; this keeps the write correct for any cpl and leaves no stale bytes). 🐇 #3. Scripts / build: - idf_component.yml: pin esp_wifi_remote ~1.6.1 and esp_hosted ~2.12.9 (the P4-NANO-validated versions) instead of "*", so the P4 build can't silently drift to a new minor; also corrected the stale "esp_hosted bring-up prelude" comment (that prelude was removed). 🐇 #2. Verified the pins resolve and the P4 builds + boots. Docs / CI: - AudioModule.md (was MicModule.md): retitled; opening reframed as "acquires an audio source ... named for what it does, not one source" (present-tense, no claim line-in exists yet). Added primary-source links: INMP441 datasheet, esp-dsp, ESP-IDF I2S API, Hann window. First fully em-dash-free module spec. - RmtLedDriver.md / LcdLedDriver.md / ParlioLedDriver.md: linked WS2812B datasheet, ESP-IDF RMT v2 / esp_lcd / Parlio API pages, the v6.0 migration guide (legacy-RMT removal), ESP32-P4 product page. (Caught + fixed an agent-guessed Parlio URL that 404'd; all links verified live.) - SystemModule.md / building.md: linked ESP32-C6, esp_hosted, esp_wifi_remote, Waveshare ESP32-P4-NANO. - decisions.md: reworded the "designed fresh" audio lesson so it no longer reads as "don't look at prior art at all" (which fought the study-with-respect principle); now "reference proven behaviour, don't trace structure," with the flat-mic / no-correction-table call as the example. 🐇 #6. - AudioVolumeEffect.md: em-dashes removed (touched alongside the effect). 🐇 #7 (partial). - AudioSpectrumEffect.md / AudioFrame.h / test files: AudioModule rename propagated (@module annotations, references). Reviews — 🐇 CodeRabbit, prior commit (b79d54f): - Fixed #1 (DcBlocker int32 clamp), #2 (component version pins), #3 (AudioVolumeEffect extra-channel clear), #4 (AudioSpectrumEffect specH off-by-one), #6 (decisions.md prior-art wording), #7 (AudioVolumeEffect.md em-dashes). - Skipped #5 (architecture.md Buffer*/Correction* "pin at wiring time"): describes the identity-mapping fast path where the pointer is genuinely stable; the suggestion proposes a different architecture, not a doc fix; paragraph untouched this branch. - Skipped #7 (README.md / backlog.md bulk em-dash sweep): the rule is "as files are touched, not a single sweep"; a ~176-dash reformat is its own task. - Skipped #8 (AudioModule stale frame): already handled — latestFrame() returns the static silent frame when active_ is null (teardown nulls it); reinit keeps the same active mic and refreshes within one loop. - Skipped #9 (AudioModule platform boundary): not a violation — the boundary check passes and the neutral platform.h facade is included by ~10 core modules; the rule forbids platform-specific code outside src/platform/, not the abstract interface. An IAudioSource indirection is exactly the bespoke abstraction the architecture avoids. - Skipped #10 (desktop improv signature): decl and definition match exactly; the desktop build passes, which proves it. - Skipped #11/#12 (platform_esp32.cpp fcntl error-handling / include): pre-existing code not changed this branch; expanding into untouched code violates minimal-change scope. Hardware verified this commit: S3 boots clean at 225 FPS with AudioModule live; P4 boots clean at 60 FPS on Ethernet with the pinned components resolved and wifiCoproc reporting "not detected" (the known C6-slave-firmware blocker, unchanged). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ewowi
added a commit
that referenced
this pull request
Jun 12, 2026
… fixes Processes the Event-2 (pre-merge) Reviewer pass over the branch. The headline is a structural deduplication of the three LED drivers: the parallel WS2812 drivers (S3 LCD_CAM i80, P4 Parlio) were ~250 of ~370 lines byte-for-byte identical, now one shared CRTP base. Also fixes four stale doc/comment items, makes the AudioModule enabled-toggle actually stop its FFT cost, and removes two dead platform flags. No control names or output behaviour change; the LED hot path is unchanged (measured zero overhead, see below). KPI: 16384lights | PC:340KB | tick:116/95/118/115/56/1/37/20/113/332us(FPS:8620/10526/8474/8695/17857/1000000/27027/50000/8849/3012) | ESP32:1178KB | tick:17341us(FPS:57) | heap:33068KB | src:88(16543) | test:53(8518) | lizard:65w Light domain (the dedup, per the No-duplication rule): - ParallelLedDriver.h (new): a CRTP base ParallelLedDriver<Derived> holding the shared body of the parallel drivers — the pins/ledsPerPin/loopback controls, parseConfig, the per-ROW fused correct+encode loop(), reinit/deinit, the loopback self-test. Binding is CRTP (static polymorphism), NOT a virtual second hierarchy: the base calls derived()->busX() resolved at compile time, no vtable, no per-light indirection — so it stays inside the data-over-objects / hot-path rules and "the one deliberate class hierarchy is the module tree" rule. - LcdLedDriver.h: 378 → 92 lines, now a derived shell supplying only the i80-specific pieces (clockPin/dcPin, the exactly-8-pins rule via kExactLaneCount, the platform::lcdWs2812* calls). - ParlioLedDriver.h: 362 → 82 lines, the simpler shell (no clock/dc, kExactLaneCount=false, kClockHz). - Drivers.h (DriverBase): absorbs the status-string lifecycle (configErr_/failBuf_, setConfigErr/clearConfigErr/failBufEnsure/clearFailBuf) that was triplicated verbatim across all three drivers (RMT too). - RmtLedDriver.h: loses its status-lifecycle copy (now inherited); symbol-per-light model stays standalone (genuinely different from the parallel drivers — concrete-first boundary). Core: - AudioModule.h: removed respectsEnabled()=false so the Scheduler honours `enabled` — disabling the module now skips loop() entirely, stopping the FFT (the real per-tick cost). Verified on hardware: 524us enabled -> 0us disabled. The FFT is never gated while enabled (it is the capability audio effects consume, not an optional cost). - platform_config.h (esp32 + desktop): removed the dead isEsp32/isEsp32S3 family flags (no users anywhere); kept isEsp32P4 (drives ethPins + hasWifiCoprocessor). Desktop now mirrors with isEsp32P4=false. Docs / CI: - architecture.md: § Firmware vs board now lists the two P4 firmwares (esp32p4-eth, esp32p4-eth-wifi); § Drivers updated for the LCD_CAM/Parlio drivers and the four-driver shared Correction pointer. (👾 MUST-FIX 1, 2) - sdkconfig.defaults.esp32p4-eth-wifi: corrected the comment that described an esp_hosted bring-up prelude the code deliberately does NOT do (it was removed; esp_hosted self-inits at boot). Following the stale comment would reintroduce the bench-fixed SDIO teardown. (👾 MUST-FIX 3) - platform_esp32_lcd.cpp: the header + #else/#endif comments now name the correct SOC_LCDCAM_I80_LCD_SUPPORTED macro (they said SOC_LCD_I80_SUPPORTED — the exact macro confusion that caused the classic-ESP32 boot loop). (👾 MUST-FIX 4) - check_specs.py: skip CRTP template bases (template<...> class X : public DriverBase) — shared infrastructure, not a registered module; the concrete derived classes still carry the docs. - backlog.md: the LED-driver duplication item updated (driver-logic dedup landed via the CRTP base; the platform loopback capture+verify extraction remains as a follow-up). Reviews — 👾 Reviewer (Fable 5, over git diff main...HEAD; verdict "merge with noted fixes"): - Fixed all 4 MUST-FIX (stale docs/comments, above). - SHOULD-CONSIDER #1/#2 (Lcd/Parlio driver-logic + status duplication): FIXED via the CRTP base + DriverBase status lifecycle (chose to do it now per the No-duplication rule rather than backlog). The remaining platform loopback capture+verify duplication is the tracked follow-up. - SHOULD-CONSIDER #4 (AudioModule enabled toggle couldn't stop the FFT): FIXED (above); the FFT itself is must-have, only the disabled-still-runs gap was the bug. - SHOULD-CONSIDER #5 (unused isEsp32/isEsp32S3 flags): FIXED (removed). - SHOULD-CONSIDER #3 (audio headers in src/light/ giving a core->light arrow): accepted as-is — documented producer/consumer placement, boundary check passes. Verification: 56 driver/encoder unit tests pass, desktop build zero-warning, spec + platform-boundary clean. All three ESP32 targets (esp32, esp32s3-n16r8, esp32p4-eth-wifi) build clean under -Werror, including both CRTP instantiations. On hardware: S3 LcdLedDriver loopback bit-perfect over a 13->12 jumper (1536/1536 symbols captured, 0-bits 15..15 / 1-bits 30..30 ticks, zero jitter); P4 ParlioLedDriver renders the S3's audio spectrum live over the DDP path. Hot-path A/B (same S3, same config): pre-dedup ~3421-3442us/frame vs CRTP ~3425-3436us/frame — statistically identical, zero overhead (CRTP is static dispatch; the per-light loop is unchanged). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ewowi
added a commit
that referenced
this pull request
Jun 21, 2026
Follow-up to the "Improv = REST over serial" work on this branch: processes the CodeRabbit review of the prior commit and adds the host-side + device-side test coverage that makes config-push over serial provable without hardware. The Improv frame format (built three times — device C++, Python, installer JS) and the device's APPLY_OP chunk reassembly + sequence guard now have automated tests; the installer's frame builders were extracted into a shared, dependency-free module so the JS is testable in node at all. Also restores 18 historical plans under the corrected Plan-YYYYMMDD (ISO-8601) naming. KPI: 16384lights | PC:365KB | tick:111/87/110/9/1/312/37/15/19/111/11us(FPS:9009/11494/9090/111111/1000000/3205/27027/66666/52631/9009/90909) | ESP32:1249KB | tick:5155us(FPS:193) | heap:8335KB | src:97(19754) | test:68(10118) | lizard:77w (ESP32 tick 5155us is a fresh live capture from the S3 running the full testbench config — Grid + AudioSpectrum + RandomMap + RmtLed + NetworkSend broadcasting Art-Net. Higher than the old idle-ish 3.6ms because NetworkSend is active; this change touches no render-path code, so the tick is unchanged for a like-for-like config.) Core: - ImprovOpReassembler (new): extracted the APPLY_OP chunk-reassembly + out-of-order/duplicate sequence guard out of the ESP32 handler into a pure, header-only core state machine (the ImprovFrame.h precedent — algorithm in core, UART in platform). Returns a typed Continue/Ready/Error. This makes the heart of "REST over serial" desktop-unit-testable and shrinks the platform handler (removed two g_improv fields). - HttpServerModule: split OpResult::NotFound into ModuleNotFound + ControlNotFound so the HTTP /api/control handler reports the two distinct 404 bodies again (CodeRabbit #1); added OpResult::AlreadyExists so an idempotent add reports {"ok":true,"note":"already exists"} again (#5); documented the serial "parent" vs HTTP "parent_id" key difference at the applyOp site (#6). - ImprovProvisioningModule: a non-Ok APPLY_OP result is now surfaced — logged over serial and parked in provision_status — so a silently-misconfigured device is visible on a monitor and via /api/state instead of looking like a clean install (#2). Ok/AlreadyExists are both treated as success. Platform: - platform_esp32_improv.cpp: improvHandleApplyOp now delegates chunk merge + seq guard to mm::ImprovOpReassembler, keeping only the serial I/O (busy guard, ack/error sends). No behaviour change; the logic is now the unit-tested core path. UI: - improv-frame.js (new): the pure Improv frame builders (buildImprovFrame, encodeApplyOpFrames, the command IDs) extracted from install-orchestrator.js so node:test can import them without the orchestrator's top-level unpkg imports. The orchestrator imports them back — no behaviour change. - install-orchestrator.js: APPLY_OP inter-op pacing 30ms -> 120ms to cover the worst-case single-buffer consume window with headroom (CodeRabbit #3; the closed-loop read-back-ack upgrade is backlogged, ops are idempotent). Tests: - unit_ImprovOpReassembler (new, 12 cases): in-order multi-chunk reassembly + NUL-termination, duplicate-chunk rejection, out-of-order/skipped-seq rejection, overflow at the buffer-minus-NUL boundary, exact-fit boundary, mid-stream seq-0 reset, empty final chunk, recovery after every error. - test/js/improv-frame.test.mjs + test/python/test_improv_frame.py (new): assert the SAME golden frame (buildImprovFrame(0x03,[0x01]) == 49 4d 50 52 4f 56 01 03 01 01 e3) so the device C++, Python, and JS frame builders provably agree; the JS suite also pins APPLY_OP chunking (seq/last, the 125-byte boundary). Encode (JS) + reassemble (C++) prove APPLY_OP end to end without hardware. - unit_HttpServerModule_apply: updated for the ModuleNotFound/ControlNotFound/AlreadyExists split. Scripts / CI: - .github/workflows/test.yml (new): PR-triggered pytest (test/python) + node --test (test/js) — the first PR-level test gate (none existed). Python tests carry deps in a PEP-723 inline block (repo convention, no central pyproject). Docs / CI: - testing.md: new "Host-side tests (Python + JS)" tier with a per-test inventory of exactly what the Python/JS frame suites pin, plus the C++ reassembler coverage. test/python + test/js added to the file layout. - CLAUDE.md: added commit gate 10 (host-side Python/JS unit tests); corrected the plan naming convention to Plan-YYYYMMDD (ISO-8601, sorts chronologically) and clarified plans are product-owner reference (agents write, don't auto-read). - ImprovProvisioningModule.md: documented the serial-vs-HTTP parent key difference. - docs/history/plans: restored 18 historical plans (plan-01..18, deleted in an old merge) under Plan-YYYYMMDD names; renamed the Improv-as-REST plan to the corrected ISO date. - backlog: removed the shipped "board injection / general data injector" item (it IS APPLY_OP now); added the closed-loop-pacing follow-up; renamed stale boards.json -> deviceModels.json references. Reviews: - 🐇 #1 module-vs-control 404 specificity — fixed (OpResult split). - 🐇 #2 dropped APPLY_OP result — fixed (logged + provision_status). - 🐇 #3 open-loop 30ms pacing — fixed (bumped to 120ms; ack-loop backlogged). - 🐇 #4 perf-bound "regression" — accepted: the cited tick_us are observed.* telemetry (0 bounds, 21 observed), which only widens on a slow run; no render-path change, not a regression. - 🐇 #5 dropped "already exists" note — fixed (AlreadyExists OpResult). - 🐇 #6 parent vs parent_id — fixed (documented at code site + spec). - 🐇 #7 NUL-guard off-by-one — no action (CodeRabbit confirmed correct). - 🐇 #8 plan date format — fixed (YYYYDDMM -> ISO YYYYMMDD, all plans renamed). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jul 2, 2026
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.
Plan-17 — release 1.0 distribution scaffolding. Stands up the path from
git tag v1.0.0to "end user flashes ESP32 from a browser." The tag itself is a separate step the product owner drives.What's in here
.github/workflows/release.yml) — tag-triggered release pipeline, branch-triggered builds onmain/next-iterationfor pre-release sanity. RC tags (vX.Y.Z-rcN) ship as pre-releases and skip the Pages publish so the live installer URL only flips on a stable tag.esp32,esp32-eth,esp32-eth-wifi,esp32s3-n16r8. Replaces the previous--profilesplit. Each S3 SKU gets its own key since the sdkconfig fragment encodes flash size + partition table + PSRAM mode.hasEthernet/MM_NO_ETHmirroring the existinghasWiFi/MM_NO_WIFIpair. Boards without on-chip EMAC no longer reference EMAC-only headers (build break found and fixed mid-gates — exactly the cross-board issue branch CI catches earlier).boardcontrol — shows the compile-time board name (from the new unifiedsrc/core/build_info.h). Read by the future OTA path to pick the matching release asset.docs/install/index.html+README.md) — self-contained ESP Web Tools page, board dropdown, browser-warning banner. README documents three local-test recipes (page-render only, end-to-end with downloaded CI artifacts viagh run download, RC dry-run pointer).package_desktop.py) — macOS arm64 tarball + Windows x64 zip, MSVC-gated warning flags inCMakeLists.txt.verify_version.py) — CI fails fast ifgit tagandlibrary.json["version"]disagree.CLAUDE.mdgate-3 wording (do reconciliation on the branch before merge), Event-3 principles audit gate, reviewer-agent placement;docs/history/decisions.mdwith this branch's lessons;plan-13..16→docs/history/archive/.Detailed commit message: see a24f073.
How to test before merging
The CI on this branch already runs 4 ESP32 builds + 2 desktop builds on every push (no release, no Pages deploy). Three escalating ways to validate the installer end-to-end:
docs/install/README.md.gh run downloadthe latest CI run, serve locally, flash the real binary per board. ~5 min loop. Seedocs/install/README.md§ "End-to-end with CI-built firmware".v1.0.0-rcNtag — full release pipeline minus Pages. IteratercN → rcN+1until clean, then tagv1.0.0.library.jsondeliberately stays at0.1.0in this PR. Bumping to1.0.0-rc1is a separate commit on the same branch when ready for path 3.Pre-merge gates
All Event 1 commit gates passed locally:
Event 2 PR-merge gates — CodeRabbit + Reviewer agent run on this PR; gates 3 (plan reconciliation), 4 (docs sync), 8 (README refresh) are already in this commit train per the gate-3-on-branch rule.
🤖 Generated with Claude Code
Summary by CodeRabbit
Release 1.0 Distribution
New Features
Documentation
Bug Fixes
Merge-time gate notes
7214a47); pre-existing bugs the rabbit caught (Base64 → std::span, handleAddModule orphan, handleDeleteModule subtree leak) were fixed in the same commit per "fix when found" policy. Branch CI green across 4 ESP32 + macOS builds. RC dry-run validated end-to-end (v1.0.0-rc1published as pre-release with 21 assets, Pages correctly skipped). Product owner reviewed the diff closely throughout implementation.