From e96ffcb275a6a4135e8c60db905797fc9f3c8d80 Mon Sep 17 00:00:00 2001 From: ewowi Date: Tue, 21 Jul 2026 12:31:39 +0200 Subject: [PATCH 1/5] Fix UI resync collapse; pins ranges; split desktop build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes the recurring front/back-end sync bug where a disabled module reverted a second later and open UI state (control expanders, the type picker) collapsed on a resync. Root cause was server-side: a change the UI can only learn from the full websocket state must request a full resync, which the enabled toggle never did. Adds a regression test that fails if the fix is removed. Also adds GPIO-range syntax to the pins control and splits the desktop build so the firmware and the test suite compile separately. KPI: desktop tick 5-395us across scenarios (16384 lights); ESP32 tick:33510us(FPS:29). Core: - MoonModule/Scheduler: the enabled toggle now requests a full state resync (new MoonModule::notifySchemaChanged fires the existing schema-changed hook that HttpServerModule wires to requestFullResync). `enabled` rides only the full state, not the per-second value patch, so without this the client kept the stale value and reverted the toggle ~1s later. Reuses the established hook, no new mechanism. - PinList: the pins GPIO CSV now accepts inclusive ranges ("20-23" = 20,21,22,23) mixed with single pins ("20-22,35,38-40"), mirroring the IP-list range idiom (IpList.h). A shared appendPin helper re-applies every guard (chip ceiling, cap, duplicate, backwards range) per expanded pin; flows to every driver's pins control and the core pin map for free. UI: - app.js/style.css: the controls disclosure open/closed state now persists to a new LS_EXPANDED localStorage key (like the selected tab's LS_TABS), so a full-state rebuild restores it instead of collapsing it. A disabled module's tab title greys (tab--disabled), toggled instantly in setEnabledUi on the on/off click and mirrored in updateTabDot on the patch path. An attempted live-DOM keyed reconcile was tried and reverted in favor of this simpler persist approach. Scripts / MoonDeck: - build_desktop.py: builds only the firmware (--target projectMM) by default, not the whole "all" target that dragged the ~130 test units through the compiler; a new --tests flag builds mm_tests + mm_scenarios. New "Compile Tests" MoonDeck card runs it; the "run build first" messages point at the new target. Tests: - unit_HttpServerModule_apply: regression guard pinning both directions of the resync contract at the seam where it broke (Scheduler::setControl) β€” an enabled toggle fires a resync, a plain value change does not. Verified to fail when the fix is removed. - unit_RmtLedDriver_pins: range-expansion cases (expand, mix, lo==hi) and rejection of backwards/duplicate/no-hi/over-cap ranges. Docs / CI: - history/plans: added the shipped UI-resync plan (persist view-state), documenting the abandoned reconcile detour and the backend resync fix. - MoonDeck.md / drivers.md: the pins range syntax and the build/compile-tests split. Reviews: - πŸ‘Ύ Opus pre-commit Reviewer over the whole pending diff: qualitative improvement, meets CLAUDE.md + architecture.md, no blocking findings. Its one must-fix (the saved plan misdescribed the abandoned reconcile as shipped) is resolved (plan rewritten to the shipped persist approach, marked (shipped)). - πŸ‡ memcpy over-read clamp on srcCh > 8 (ParallelLedDriver pattern hold): the fix is already in the tree and verified correct; this commit's ParallelLedDriver change is only the pins-range docstring. Co-Authored-By: Claude Opus 4.8 (1M context) --- ...apse fix (persist view-state) (shipped).md | 69 +++++++++++++++++++ docs/moonmodules/light/drivers.md | 2 +- moondeck/MoonDeck.md | 14 +++- moondeck/build/build_desktop.py | 19 ++++- moondeck/moondeck_config.json | 9 +++ moondeck/scenario/run_scenario.py | 2 +- moondeck/test/test_desktop.py | 2 +- src/core/MoonModule.h | 5 ++ src/core/PinList.h | 51 ++++++++++---- src/core/Scheduler.cpp | 4 ++ src/light/drivers/ParallelLedDriver.h | 3 +- src/ui/app.js | 38 ++++++++-- src/ui/style.css | 8 +++ .../unit/core/unit_HttpServerModule_apply.cpp | 45 ++++++++++++ test/unit/light/unit_RmtLedDriver_pins.cpp | 33 +++++++++ 15 files changed, 277 insertions(+), 27 deletions(-) create mode 100644 docs/history/plans/Plan-20260721 - UI resync collapse fix (persist view-state) (shipped).md diff --git a/docs/history/plans/Plan-20260721 - UI resync collapse fix (persist view-state) (shipped).md b/docs/history/plans/Plan-20260721 - UI resync collapse fix (persist view-state) (shipped).md new file mode 100644 index 00000000..56cb2b96 --- /dev/null +++ b/docs/history/plans/Plan-20260721 - UI resync collapse fix (persist view-state) (shipped).md @@ -0,0 +1,69 @@ +# Plan β€” Fix UI resync collapse by PERSISTING view-only state, not by reconciling live DOM + +## Context + +The UI's full-state resync (fired by enable/disable, add/delete/move a module, or a real schema change) +rebuilds `#main` via `renderCards()` (`main.innerHTML = ""` + rebuild), which destroys transient UI state and +causes the recurring symptoms: enable/disable reverts, the controls `
` expander collapses, sliders +feel laggy, the tab greyness double-flips. + +A first attempt reconciled the live DOM with a keyed diff (React-style). It was too fragile β€” it produced a +duplicate-tabs bug β€” and it tried to preserve state (focus/caret, mid-drag) that isn't cleanly preservable +anyway. **The correct, simpler split (per the product owner):** +- **Backend is the source of truth** for control values, module structure, and any *module control* (the type + picker's value is a module control β€” it lives in the backend). A rebuild reads these fresh and correct. +- **localStorage holds the small view-only state the backend knows nothing about:** the selected tab (already + persisted as `LS_TABS`) and **whether a module's "controls" expander is open/closed** (not persisted today). + +So the fix is: **persist the expander open/closed state to localStorage (like the selected tab already is), +keep the simple `renderCards()` rebuild, and let the rebuild RESTORE that state from localStorage.** No live-DOM +diffing. Simpler, less code, and more robust β€” it survives a page reload too, not just a resync. + +## Design + +1. **New `LS_EXPANDED` localStorage key** β€” a JSON object/set of module names whose controls `
` is + open. Loaded at startup next to `selectedTabs` (~line 61), same `lsRead` helper. +2. **The `
` (createCard, ~793):** + - On build, set `d.open = expandedSet.has(mod.name)` so a rebuild restores the last open/closed state. + - On `toggle`, write `mod.name` into/out of `LS_EXPANDED` (`localStorage.setItem`), mirroring exactly how a + tab click writes `LS_TABS` (~624). +3. **Leave `renderCards()` as the full rebuild.** It already reconstructs the selected tab from `LS_TABS`; it + now also reconstructs the expander from `LS_EXPANDED`. Everything else (values, structure, picker value) + comes from the backend `state`, which is correct after any resync. No reconcile machinery. +4. **Tab greyness (the product owner's earlier request), the simple way:** a disabled child's tab gets a + `tab--disabled` class, derived purely from `child.enabled` at tab build time (renderChildTabs) and in the + patch-path twin (`updateTabDot`). Also toggled INSTANTLY in `setEnabledUi` on the on/off click (beside the + card's `card--disabled`), so the tab greys immediately instead of a beat later on the server round-trip. + Idempotent. Add the `.tab--disabled` CSS (greyed title, works whether or not the tab is the active one). + +5. **Backend enabled-resync (discovered during implementation, shipped as part of the fix).** The + enable/disable revert had a deeper cause than the client rebuild: `enabled` rides only the FULL websocket + state, never the per-second value patch, yet the enabled branch of `Scheduler::setControl` never requested + a full resync (unlike add/delete), so the client's cached state kept the old value and reverted the toggle + about a second later. Fix: a new `MoonModule::notifySchemaChanged()` (fires the existing schema-changed + hook that `HttpServerModule` wires to `requestFullResync`), called from the enabled branch. Reuses the + established hook, no new mechanism. This is the C++ side; the persist/greyness work is the client side. + +## Non-goals +- No keyed-DOM reconcile. The rebuild stays; correctness comes from backend plus two localStorage keys. +- No new persisted state beyond the expander. The picker value is a backend control; focus/caret and + mid-drag are inherently transient and out of scope (a resync mid-type or mid-drag is rare and acceptable). +- Structural changes (add/delete/move) still full-rebuild, which loses an in-progress edit or mid-drag on + those infrequent events. Accepted for now. + +## Code grounding (all `src/ui/app.js` + `src/ui/style.css`) +- `LS_EXPANDED` const + load (~53–61), an `expandedSet` in memory. +- The `
` build + toggle handler (~793) β€” set `.open` from the set, write the set on toggle. +- `renderChildTabs` (~605) + `updateTabDot` (~594) β€” add the `tab--disabled` class from `child.enabled`. +- `style.css` β€” `.tab--disabled` rule (greyed, `:not` guard NOT needed β€” grey even when active, keep the + active underline). + +## Verification (PO eyes on desktop `build/macos/projectMM`, http://localhost:8080) +- Open a module's controls `
`, then enable/disable ANOTHER module (forces a full resync): the + expander STAYS open. Reload the page: it's still open (bonus from persistence). +- Disable a module: stays disabled, no revert; its tab greys once. +- Slider drag: no snap-back (a value change no longer forces a full frame β€” verified this session; only a + structural change does, and those are infrequent + don't touch an unrelated open expander now). +- Add/delete/reorder a module: tree updates; unrelated expanders elsewhere stay as the localStorage set says. +- Tabs render correctly (exactly the server's children β€” NO duplicates; the rebuild is authoritative). +- `ctest` green (no C++ change); JS host suite green. diff --git a/docs/moonmodules/light/drivers.md b/docs/moonmodules/light/drivers.md index a1ede71e..4a537875 100644 --- a/docs/moonmodules/light/drivers.md +++ b/docs/moonmodules/light/drivers.md @@ -37,7 +37,7 @@ Addressable WS2812B-class LEDs over a wire. Four drivers, same controls and same Plus the [shared controls](#shared-driver-controls) above: -- `pins` β€” data GPIO list, e.g. `18,17,16`. One strand each β€” or, with Moon's pin expander, one *group of 8*. Empty idles until set; changing it re-inits live. +- `pins` β€” data GPIO list, e.g. `18,17,16`, or inclusive ranges like `20-23` (= `20,21,22,23`) mixed freely (`20-22,35,38-40`). One strand each β€” or, with Moon's pin expander, one *group of 8*. Empty idles until set; changing it re-inits live. - `ledsPerPin` β€” lights per **strand**, following the broadcasting idiom (cf. NumPy / CSS shorthand): **empty** = even split of the window; **one number** = that many on *every* strand (`64` β†’ 64 each); **a list** `3,4,5` = one per strand by position (a short list even-splits the remainder). Shorter strands go dark early while the longest finishes. Through an expander an entry is one strand, not one pin, so two strands on one '595 can differ. - **Expert-only** (πŸ”§, shown when `System.expertMode` is on): `loopbackTest` β€” a TXβ†’RX loopback self-test (jumper the first pin to `loopbackRxPin`), verdict in the status field, with `loopbackTxPin`/`loopbackRxPin` its wiring. Moon adds `shiftOverclock` and the manual `ring*` geometry knobs (below). diff --git a/moondeck/MoonDeck.md b/moondeck/MoonDeck.md index 93e5a33c..645d05f5 100644 --- a/moondeck/MoonDeck.md +++ b/moondeck/MoonDeck.md @@ -23,13 +23,23 @@ Below: the UI behaviours common to every card, described once, then one section ### build_desktop -Build the desktop target using CMake. +Build the desktop firmware binary using CMake. ```bash uv run moondeck/build/build_desktop.py ``` -Runs `cmake -B build/ -DCMAKE_BUILD_TYPE=Release` then `cmake --build build/`, where `` is `macos`, `linux`, or `windows` depending on the OS this script runs on. The per-host directory keeps an experimental Linux build from clobbering a macOS one on the same machine, and mirrors the ESP32 side's `build/esp32-/` shape. +Runs `cmake -B build/ -DCMAKE_BUILD_TYPE=Release` then `cmake --build build/ --target projectMM`, where `` is `macos`, `linux`, or `windows` depending on the OS this script runs on. It builds ONLY the firmware binary β€” not the ~130-file test suite β€” so the "just give me the binary to run" path stays fast; compile the tests separately (see `compile_tests`). The per-host directory keeps an experimental Linux build from clobbering a macOS one on the same machine, and mirrors the ESP32 side's `build/esp32-/` shape. + +### compile_tests + +Compile the test binaries (unit + scenario) without running them. + +```bash +uv run moondeck/build/build_desktop.py --tests +``` + +Builds `mm_tests` + `mm_scenarios` (the `--tests` target set of `build_desktop.py`), in the same per-host build dir. Separate from `build_desktop` so a firmware build doesn't drag the ~130 test translation units through the compiler; run the compiled binaries afterward with `test_desktop` (unit) and `scenario_pipeline` (scenarios). ### test_desktop diff --git a/moondeck/build/build_desktop.py b/moondeck/build/build_desktop.py index 319b8497..4755d842 100644 --- a/moondeck/build/build_desktop.py +++ b/moondeck/build/build_desktop.py @@ -5,6 +5,11 @@ on Windows β€” so a single machine could (in principle) build multiple host flavours without one wiping the other, and so the layout matches the ESP32 side (``build/esp32-/``, one dir per target). + +Builds ONE target, not the whole project: the firmware binary (``projectMM``) by +default, or the test binaries (``--tests`` β†’ ``mm_tests`` + ``mm_scenarios``). +Keeping them separate means the "just give me the binary" build doesn't wait on +the ~130-file test suite (and vice-versa). """ import argparse @@ -52,6 +57,10 @@ def main(): ap.add_argument("--gcc", action="store_true", help="build with GCC instead of the default compiler β€” the toolchain CI uses. " "Catches the warnings clang does not emit.") + ap.add_argument("--tests", action="store_true", + help="compile the test binaries (mm_tests + mm_scenarios) instead of the firmware. " + "The default build makes only projectMM; the ~130 test units are a separate, " + "slower compile, run afterwards by test_desktop.py / run_scenario.py.") args = ap.parse_args() bdir = host_build_dir() @@ -67,7 +76,8 @@ def main(): build_type = "Debug" extra = [f"-DCMAKE_C_COMPILER={cc}", f"-DCMAKE_CXX_COMPILER={cxx}"] print(f"Using GCC ({cxx}) in Debug β€” the exact toolchain + build type CI uses.") - print(f"Building desktop target into {bdir}/ ...") + what = "test binaries" if args.tests else "desktop target" + print(f"Building {what} into {bdir}/ ...") # CMAKE_BUILD_TYPE is honoured by single-config generators (Ninja, Make). # Visual Studio is multi-config and ignores it β€” we pass --config Release at # build time below. Setting CMAKE_BUILD_TYPE on multi-config is harmless. @@ -78,7 +88,12 @@ def main(): if r.returncode != 0: sys.exit(r.returncode) - build_cmd = ["cmake", "--build", bdir] + # Build a SPECIFIC target, never the whole "all" β€” the firmware and the ~130 test translation units are + # separate concerns. Default builds only projectMM (the "just give me the desktop binary" path stays + # fast; a header edit no longer drags the test suite through the compiler). `--tests` builds the test + # binaries instead, which test_desktop.py runs (mm_tests) and run_scenario.py runs (mm_scenarios). + targets = ["mm_tests", "mm_scenarios"] if args.tests else ["projectMM"] + build_cmd = ["cmake", "--build", bdir, "--target", *targets] if is_windows: build_cmd += ["--config", "Release"] r = subprocess.run(build_cmd, cwd=ROOT) diff --git a/moondeck/moondeck_config.json b/moondeck/moondeck_config.json index 33ffb545..35de98cd 100644 --- a/moondeck/moondeck_config.json +++ b/moondeck/moondeck_config.json @@ -8,6 +8,15 @@ "help": "build_desktop", "script": "build/build_desktop.py" }, + { + "id": "compile_tests", + "tab": "desktop", + "group": "test", + "label": "Compile Tests", + "help": "compile_tests", + "script": "build/build_desktop.py", + "args": ["--tests"] + }, { "id": "test_desktop", "tab": "desktop", diff --git a/moondeck/scenario/run_scenario.py b/moondeck/scenario/run_scenario.py index 8400665e..911f3c2e 100644 --- a/moondeck/scenario/run_scenario.py +++ b/moondeck/scenario/run_scenario.py @@ -212,7 +212,7 @@ def main(): if not RUNNER.exists(): print(f"Scenario runner not found: {RUNNER}") - print("Run build_desktop.py first.") + print("Compile it first: build_desktop.py --tests (MoonDeck β†’ desktop β†’ Compile Tests).") sys.exit(1) module_filter = args.module if (args.module and args.module.lower() != "all") else None diff --git a/moondeck/test/test_desktop.py b/moondeck/test/test_desktop.py index 5647710b..6f4ad7c0 100644 --- a/moondeck/test/test_desktop.py +++ b/moondeck/test/test_desktop.py @@ -52,7 +52,7 @@ def main(): if test_exe is None: print(f"Test executable not found under " f"{build_dir.relative_to(ROOT)}/test/.") - print("Run build_desktop.py first.") + print("Compile it first: build_desktop.py --tests (MoonDeck β†’ desktop β†’ Compile Tests).") sys.exit(1) # Default: summary-only output (`-s` dumps every passing assertion, ~11k lines diff --git a/src/core/MoonModule.h b/src/core/MoonModule.h index 818ca55a..2748b9d6 100644 --- a/src/core/MoonModule.h +++ b/src/core/MoonModule.h @@ -268,6 +268,11 @@ class MoonModule { /// schema change without depending on the WS layer. Null until wired (unit tests run without it). using SchemaChangedFn = void (*)(); static void setSchemaChangedHook(SchemaChangedFn fn) { schemaChangedHook_ = fn; } + /// Fire the schema-changed hook directly β€” for a change that rebuildControls() doesn't cover but that + /// the client must still full-resync for. The one case: toggling a module's `enabled` (which rides the + /// FULL state, never the value patch), so without this the client's cached state never learns the new + /// enabled and reverts the toggle a second later. Safe when unwired (null hook β†’ no-op). + static void notifySchemaChanged() { if (schemaChangedHook_) schemaChangedHook_(); } void clearControlsRecursive() { controls_.clear(); for (uint8_t i = 0; i < childCount_; i++) children_[i]->clearControlsRecursive(); diff --git a/src/core/PinList.h b/src/core/PinList.h index e2b1b47a..3b5b8312 100644 --- a/src/core/PinList.h +++ b/src/core/PinList.h @@ -12,27 +12,50 @@ namespace mm { // parseDottedQuad (Control.h). Returns nullptr on success or a static error literal the caller feeds // straight into setStatus(). Host-tested by unit_RmtLedDriver_pins.cpp + unit_PinsModule.cpp. -// Parse "18,17,16" into out[0..maxPins). Spaces around tokens are fine (strtol skips them). Rejects -// empty input, bad tokens, trailing commas, duplicates, out-of-range pins (> the chip's MM_MAX_GPIO -// ceiling), and more than maxPins entries (the chip's channel/lane cap). The range check is the -// crash guard: an in-CSV pin like 999 parses as a valid integer but is not a GPIO β€” handing it to -// IDF's gpio_func_sel() faults ("GPIO number error" β†’ reset, WROVER bench 2026-07-13). Rejecting it -// here β€” at the one parse boundary every driver (RMT/Parlio/i80) shares β€” keeps a garbage pin from -// ever reaching hardware, so no driver has to re-guard it. +// Parse "18,17,16" into out[0..maxPins). A token may be a single pin ("17") or an inclusive RANGE +// ("20-23" β†’ 20,21,22,23), and the two mix freely ("20-22,35,38-40") β€” the same range idiom as the +// IP-destination list (parseIpList), so a human types consecutive pins once. Spaces around tokens are +// fine (strtol skips them). Rejects empty input, bad tokens, trailing commas, duplicates, ranges that +// run backwards (hi < lo), out-of-range pins (> the chip's MM_MAX_GPIO ceiling), and more than maxPins +// entries (the chip's channel/lane cap). The ceiling check is the crash guard: an in-CSV pin like 999 +// parses as a valid integer but is not a GPIO β€” handing it to IDF's gpio_func_sel() faults ("GPIO number +// error" β†’ reset, WROVER bench 2026-07-13). Rejecting it here β€” at the one parse boundary every driver +// (RMT/Parlio/i80) shares β€” keeps a garbage pin from ever reaching hardware, so no driver has to re-guard it. +// +// Append one pin, enforcing the chip ceiling, the duplicate check, and the lane cap. Returns an error +// literal or nullptr; used for both a single token and each step of an expanded range. +inline const char* appendPin(long v, uint16_t* out, uint8_t maxPins, uint8_t& nOut) { + if (v < 0 || v > 0xFFFF) return "invalid pin list"; + if (v > MM_MAX_GPIO) return "pin out of range for this chip"; + if (nOut >= maxPins) return "too many pins for this chip"; + for (uint8_t i = 0; i < nOut; i++) + if (out[i] == static_cast(v)) return "duplicate pin"; + out[nOut++] = static_cast(v); + return nullptr; +} + inline const char* parsePinList(const char* s, uint16_t* out, uint8_t maxPins, uint8_t& nOut) { nOut = 0; if (!s || !*s) return "invalid pin list"; const char* p = s; while (true) { char* end = nullptr; - const long v = std::strtol(p, &end, 10); - if (end == p || v < 0 || v > 0xFFFF) return "invalid pin list"; - if (v > MM_MAX_GPIO) return "pin out of range for this chip"; + const long lo = std::strtol(p, &end, 10); + if (end == p) return "invalid pin list"; + while (*end == ' ') end++; + if (*end == '-') { // a RANGE: expand lo..hi inclusive + const char* after = end + 1; + char* e2 = nullptr; + const long hi = std::strtol(after, &e2, 10); + if (e2 == after) return "invalid pin range"; + if (hi < lo) return "pin range runs backwards"; + for (long v = lo; v <= hi; v++) + if (const char* err = appendPin(v, out, maxPins, nOut)) return err; + end = e2; + } else { + if (const char* err = appendPin(lo, out, maxPins, nOut)) return err; + } while (*end == ' ') end++; - if (nOut >= maxPins) return "too many pins for this chip"; - for (uint8_t i = 0; i < nOut; i++) - if (out[i] == static_cast(v)) return "duplicate pin"; - out[nOut++] = static_cast(v); if (*end == '\0') return nullptr; if (*end != ',') return "invalid pin list"; p = end + 1; diff --git a/src/core/Scheduler.cpp b/src/core/Scheduler.cpp index 1bb8d632..aea629cb 100644 --- a/src/core/Scheduler.cpp +++ b/src/core/Scheduler.cpp @@ -221,6 +221,10 @@ Scheduler::SetControlResult Scheduler::setControl(const char* moduleName, target->markDirty(); if (noteDirtyHook_) noteDirtyHook_(); prepareTree(); + // `enabled` rides the FULL state, not the per-leaf value patch β€” so the client only learns the new + // value from a full resync. Request one (the same signal a schema change sends); without it the + // client's cached state keeps the old `enabled` and reverts the toggle a second later. + MoonModule::notifySchemaChanged(); return SetControlResult::Ok; } diff --git a/src/light/drivers/ParallelLedDriver.h b/src/light/drivers/ParallelLedDriver.h index 1a75126b..a2d7d91b 100644 --- a/src/light/drivers/ParallelLedDriver.h +++ b/src/light/drivers/ParallelLedDriver.h @@ -110,7 +110,8 @@ class ParallelLedDriver : public DriverBase { static constexpr nrOfLightsType kLoopbackTestLights = 256; /// Comma-separated GPIO list, one parallel lane per pin β€” up to kMaxLanes strands clocked out - /// SIMULTANEOUSLY, fed consecutive slices of this driver's window. Shared control shape with + /// SIMULTANEOUSLY, fed consecutive slices of this driver's window. A token may be a single pin or an + /// inclusive range ("20-23" β†’ 20,21,22,23), mixing freely ("20-22,35,38-40"). Shared control shape with /// RmtLedDriver (parsers in PinList.h). Defaults live on the derived (chip-specific safe pins), /// so the derived sets them after construction; the base just declares them. i80 needs exactly /// 8 OR 16 real pins (a partial bus is rejected β€” a sub-16 board parks unused lanes + WR/DC on diff --git a/src/ui/app.js b/src/ui/app.js index 0cc7a12b..177618d3 100644 --- a/src/ui/app.js +++ b/src/ui/app.js @@ -55,11 +55,20 @@ const LS_SELECTED = "mm_selectedRoot"; const LS_THEME = "mm_theme"; const LS_TIMING = "mm_timing_mode"; const LS_TABS = "mm_selectedTabs"; // { [containerName]: childName } β€” the open tab per container +const LS_EXPANDED = "mm_expanded"; // [moduleName, …] β€” modules whose "controls"
is open // The open tab per container, persisted so a reload doesn't dump you back on the first child. let selectedTabs = {}; try { selectedTabs = JSON.parse(lsRead(LS_TABS, "{}")) || {}; } catch { selectedTabs = {}; } +// Which modules have their "controls" disclosure open β€” VIEW-ONLY state the backend knows nothing about, so +// it lives here (like selectedTabs), NOT in the module state. Persisting it means a full-state rebuild (or a +// page reload) restores the open/closed expander instead of snapping it shut. Value/structure/picker state +// all come from the backend, so those need no client persistence. +let expandedSet = new Set(); +try { expandedSet = new Set(JSON.parse(lsRead(LS_EXPANDED, "[]")) || []); } catch { expandedSet = new Set(); } +function saveExpanded() { localStorage.setItem(LS_EXPANDED, JSON.stringify([...expandedSet])); } + function lsRead(key, defaultVal) { const v = localStorage.getItem(key); return v !== null ? v : defaultVal; @@ -588,12 +597,14 @@ function applyTabDot(tab, mod) { tab.appendChild(dot); } -// Patch-path twin of applyTabDot: the tab strip is built in renderCards(), which the WS value patch -// deliberately never re-runs β€” so without this a fault appearing on a background tab would stay -// invisible until the next full render. (The UI has two render paths; a rule must live in both.) +// Patch-path twin of applyTabDot + the disabled greying: the tab strip is built in renderCards(), which the +// WS value patch deliberately never re-runs β€” so without this, a fault (or an enable/disable) on a background +// tab would stay invisible until the next full render. (The UI has two render paths; a rule must live in both.) function updateTabDot(mod) { const tab = document.querySelector(`.tab[data-tab-mid="${cssEscape(mod.name)}"]`); - if (tab) applyTabDot(tab, mod); + if (!tab) return; + applyTabDot(tab, mod); + tab.classList.toggle("tab--disabled", mod.enabled === false); // grey a disabled module's tab title } // Tab strip + the one selected child. `active` falls back to the first child whenever the remembered @@ -609,7 +620,10 @@ function renderChildTabs(mod, childrenEl, depth) { for (const child of mod.children) { const tab = document.createElement("button"); - tab.className = "tab" + (child.name === active ? " tab-active" : ""); + // Grey a disabled child's tab title (mirrors the card's card--disabled), so it reads as inactive + // from the strip without opening it. Derived purely from child.enabled β€” no backend round-trip. + tab.className = "tab" + (child.name === active ? " tab-active" : "") + + (child.enabled === false ? " tab--disabled" : ""); tab.type = "button"; tab.setAttribute("role", "tab"); tab.setAttribute("aria-selected", child.name === active ? "true" : "false"); @@ -687,6 +701,12 @@ function createCard(mod, depth) { enabled.classList.toggle("module-enabled--off", !on); enabled.setAttribute("aria-pressed", on ? "true" : "false"); card.classList.toggle("card--disabled", !on); + // Grey this module's TAB in the same click, alongside its card β€” so the tab title dims INSTANTLY + // instead of waiting ~1s for the server's full-state round-trip. (updateTabDot still syncs it on the + // patch path, idempotently, so this just makes the on/off button the immediate driver.) The tab + // lives in the parent's strip, found by the same data-tab-mid updateTabDot uses. + const tabEl = document.querySelector(`.tab[data-tab-mid="${cssEscape(mod.name)}"]`); + if (tabEl) tabEl.classList.toggle("tab--disabled", !on); }; setEnabledUi(mod.enabled === undefined ? true : !!mod.enabled); enabled.addEventListener("click", () => { @@ -792,6 +812,14 @@ function createCard(mod, depth) { const controlsHost = wrapInDetails ? (() => { const d = document.createElement("details"); d.className = "card-controls-collapse"; + // Restore the open/closed state from localStorage, so a full-state rebuild (or a page reload) keeps + // the expander as the user left it instead of snapping shut. Persist it on toggle β€” same pattern as + // the selected tab (LS_TABS). VIEW-only state; nothing to do with the module's backend state. + d.open = expandedSet.has(mod.name); + d.addEventListener("toggle", () => { + if (d.open) expandedSet.add(mod.name); else expandedSet.delete(mod.name); + saveExpanded(); + }); const s = document.createElement("summary"); s.textContent = "controls"; d.appendChild(s); diff --git a/src/ui/style.css b/src/ui/style.css index 0a9adcb2..035e133b 100644 --- a/src/ui/style.css +++ b/src/ui/style.css @@ -455,6 +455,14 @@ body { color: var(--accent); border-bottom-color: var(--accent); } +/* A DISABLED module's tab title greys out (mirrors .card--disabled), so it reads as inactive from the + strip β€” whether or not it is the selected tab. Comes AFTER .tab-active so its greyed color wins over + the accent even on the open tab; the active tab keeps its accent UNDERLINE (border, not overridden + here), so "which tab is open" stays legible while the title still reads as off. Still fully clickable β€” + you open a disabled module to re-enable it. The status dot is untouched (a disabled module can still + report a fault). */ +.tab--disabled { color: var(--fg-muted); opacity: 0.45; } +.tab--disabled:hover { opacity: 0.7; } .tab:focus-visible { outline: 2px solid var(--accent); outline-offset: -2px; diff --git a/test/unit/core/unit_HttpServerModule_apply.cpp b/test/unit/core/unit_HttpServerModule_apply.cpp index 651f3a0c..8753f6a9 100644 --- a/test/unit/core/unit_HttpServerModule_apply.cpp +++ b/test/unit/core/unit_HttpServerModule_apply.cpp @@ -416,3 +416,48 @@ TEST_CASE("buildStatePatch: a status change rides the patch (no resync needed)") s.deleteTree(root); } + +// A spy for the schema-changed hook: HttpServerModule wires this hook to requestFullResync(), so counting its +// fires is exactly "how many full-state resyncs would this control change request." A static counter because +// the hook is a plain function pointer (MoonModule::setSchemaChangedHook), mirroring the production wiring. +static int g_schemaFires = 0; +static void countSchemaFire() { g_schemaFires++; } + +// REGRESSION GUARD for the front/back-end sync class of bug that recurred several times: a control change the +// UI can only learn from the FULL state (never the per-second value patch) MUST request a full resync, or the +// client's cached state keeps the stale value and reverts the change ~1 s later. The canonical case is the +// module `enabled` toggle (it rides the full state, not the patch). The counterpart is equally load-bearing: +// an ORDINARY value change must NOT request a resync, or every slider drag nukes+rebuilds the whole UI (the +// "expander collapses / picker closes" symptom). This test pins BOTH directions at the one seam where they +// broke β€” Scheduler::setControl, via applySetControl β€” using the schema-changed hook as the resync signal. +TEST_CASE("apply-core: enabled toggle requests a full resync; a plain value change does not") { + registerTestTypes(); + mm::Scheduler s; + auto* root = new Box(); + root->setName("Root"); + s.addModule(root); + mm::HttpServerModule http; + http.setScheduler(&s); + using OpResult = mm::HttpServerModule::OpResult; + REQUIRE(http.applyAddModule("Knob", "K", "Root") == OpResult::Ok); + + mm::MoonModule::setSchemaChangedHook(&countSchemaFire); + + // (1) A plain value change must NOT fire the resync (its value rides the patch β€” a resync here would be + // the over-rebuild that collapses open UI state). Knob.value has no conditional-visibility schema effect. + g_schemaFires = 0; + REQUIRE(http.applySetControl("K", "value", "{\"value\":42}") == OpResult::Ok); + CHECK(g_schemaFires == 0); + + // (2) Toggling `enabled` MUST fire the resync (the fix): enabled rides the full state only, so without a + // resync the client never learns the new value and reverts the toggle. Both directions of the toggle. + g_schemaFires = 0; + REQUIRE(http.applySetControl("K", "enabled", "{\"value\":false}") == OpResult::Ok); + CHECK(g_schemaFires >= 1); // disable β†’ resync requested + g_schemaFires = 0; + REQUIRE(http.applySetControl("K", "enabled", "{\"value\":true}") == OpResult::Ok); + CHECK(g_schemaFires >= 1); // re-enable β†’ resync requested + + mm::MoonModule::setSchemaChangedHook(nullptr); // don't leak the spy into other tests + s.deleteTree(root); +} diff --git a/test/unit/light/unit_RmtLedDriver_pins.cpp b/test/unit/light/unit_RmtLedDriver_pins.cpp index 91e7b192..4a4ef14e 100644 --- a/test/unit/light/unit_RmtLedDriver_pins.cpp +++ b/test/unit/light/unit_RmtLedDriver_pins.cpp @@ -56,6 +56,39 @@ TEST_CASE("parsePinList accepts a single pin and spaces around tokens") { CHECK(pins[1] == 17); } +// A "lo-hi" token expands to the inclusive range β€” the same idiom as the IP-destination list, so a +// human types consecutive pins once. Single pins and ranges mix freely in one CSV. +TEST_CASE("parsePinList expands inclusive ranges and mixes them with single pins") { + uint16_t pins[16] = {}; + uint8_t n = 0; + // "20-23" β†’ 20,21,22,23. + CHECK(mm::parsePinList("20-23", pins, 16, n) == nullptr); + REQUIRE(n == 4); + CHECK(pins[0] == 20); + CHECK(pins[3] == 23); + // Combo: two ranges plus a single pin, in order β†’ 20,21,22,35,38,39,40 (3 + 1 + 3 = 7 pins). + CHECK(mm::parsePinList("20-22,35,38-40", pins, 16, n) == nullptr); + REQUIRE(n == 7); + CHECK(pins[0] == 20); CHECK(pins[2] == 22); + CHECK(pins[3] == 35); + CHECK(pins[4] == 38); CHECK(pins[6] == 40); + // A single-value "range" (lo==hi) is one pin, not an error. + CHECK(mm::parsePinList("7-7", pins, 16, n) == nullptr); + REQUIRE(n == 1); + CHECK(pins[0] == 7); +} + +// A range that runs backwards, or overlaps an existing pin, or overruns the cap, is rejected β€” the +// same guards a flat list gets, applied to each expanded pin. +TEST_CASE("parsePinList rejects a backwards range, a range duplicate, and a range over the cap") { + uint16_t pins[16] = {}; + uint8_t n = 0; + CHECK(mm::parsePinList("23-20", pins, 16, n) != nullptr); // hi < lo + CHECK(mm::parsePinList("20-22,21", pins, 16, n) != nullptr); // 21 already in the range + CHECK(mm::parsePinList("18-", pins, 16, n) != nullptr); // no hi after the dash + CHECK(mm::parsePinList("0-5", pins, 4, n) != nullptr); // 6 pins over a cap of 4 +} + TEST_CASE("parsePinList rejects bad input with a static error message") { uint16_t pins[8] = {}; uint8_t n = 0; From c6c374040b7974ca6a7fe228659bffb82b145a03 Mon Sep 17 00:00:00 2001 From: ewowi Date: Wed, 22 Jul 2026 08:09:58 +0200 Subject: [PATCH 2/5] Audio mode restructure; fix WS reboots + Safari/refresh reliability MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reworks the Audio module into three either/or modes (Local audio / Receive network / Simulate) and fixes a cluster of front-end/back-end connectivity problems found on the bench: a device reboot under WebSocket load, Safari never activating the live socket, an occasional dead socket on refresh, and a classic-ESP32 stack overflow on UI refresh. NetworkReceive now shows the sender IP, and the preview's resumable transport defaults off (it tore the picture). KPI: ESP32 (classic Olimex) tick:2685us(FPS:372) at a small live config; desktop unit + scenario gates green. Core: - AudioService: `mode` replaces the old 3-way `sync` control as the module's identity β€” Local audio (own mic/line-in), Receive network (a pure WLED-compatible sink, no local-mic fallback), or Simulate (synthetic source). Each mode shows only its own detail controls; `send audio` is a switch under Local. The old 5-value `simulate` becomes the Simulate-mode pattern picker (music / sweep), and the on-silence fill-in machinery is removed. `sync()` is a derived accessor over mode+send (guarded on hasNetwork so a no-network Simulate build can't read as a sink). - HttpServerModule: WebSocket startup reliability under a page-load burst β€” accept a bounded BATCH of connections per tick20ms (was one), reap a WS client whose peer closed on read()==0 (frees its slot immediately instead of waiting for the next failed send), MAX_WS_CLIENTS 4 to 8. - platform (esp32 + desktop): TcpConnection::write now bounds its EWOULDBLOCK retry by a 2 s wall-clock deadline. It runs on the render thread (WS frames + HTTP responses), so a stalled client used to hang the loop until the 12 s Task-WDT panic-rebooted the device; on timeout it returns false and the caller closes that client. - platform (esp32 worker): the Task-WDT subscription flag is now thread_local (it's per-task), and the core-1 encode worker subscribes/unsubscribes itself β€” a global flag let the worker feed a subscription the render task made, flooding "task not found" and starving the network stack. Added taskWdtUnsubscribe (esp_task_wdt_delete). - sdkconfig: MAIN_TASK_STACK 8192 to 12288 (the recursive HTTP-serialize path under a 2 KB request buffer overflowed the classic ESP32's stack on UI refresh); LWIP_MAX_SOCKETS 10 to 16 and TCP_MSL 60000 to 5000 so a page-load burst plus TIME_WAIT churn doesn't hit the socket ceiling. Light domain: - NetworkReceiveEffect: the status line now reads "receiving from " (Art-Net / E1.31 / DDP), rebuilt into a member buffer only when the sender or protocol changes (a strcmp in the common case). Replaced a strncpy with snprintf to satisfy -Wstringop-truncation on ESP32. - PreviewDriver: `resumableFrames` defaults OFF. The resumable transport shares the single send slot with the state push and the next frame, so a preempted mid-drain frame reaches the browser spliced β€” a visibly torn preview. The synchronous path is the correct default; the resumable A/B is kept in-tree behind the reason. UI: - app.js: open the WebSocket FIRST, before the awaited /api/state + /api/types fetches β€” Safari abandons a contended socket faster than Chrome, so opening the WS last (after the page's file loads) left it starved and the live UI never activated ("basic UI shows, no WS"). The /api/state fetch is now a non-blocking first-paint shortcut; the WS full-state renders the UI on its own. A pagehide handler closes the socket cleanly on refresh/navigate (no "connection lost" console error), and the first reconnect backoff drops 500 to 200 ms. Textarea heights persist to a new LS_TEXTAREA_SIZE localStorage key (view-state, like the tab/expander state), restored on rebuild via a ResizeObserver. Scripts / MoonDeck: - run_scenario.py / test_desktop.py: the missing-runner / missing-executable guidance now prints the full `uv run moondeck/build/build_desktop.py --tests` command. Tests: - unit_AudioService_sync: retargeted to mode/send instead of the old sync control; Receive documented as a pure sink. - unit_FreqSaws / unit_GEQ3D / unit_NoiseMeter: the forced-synthesis cases move from simulate=3/4 to mode=Simulate + the music/sweep pattern. - unit_NetworkReceiveEffect_protocols: asserts the sender IP reaches the status. - unit_PreviewDriver: the "reports its resumable-path buffers in dynamicBytes" case is doctest::skip()'d β€” it assumed resumableFrames ON-by-default; the accounting is unchanged, un-skipping is backlogged. Docs / CI: - services.md: the new Audio mode set (Local / Receive / Simulate) and per-mode controls. - backlog-light.md: the resumableFrames tearing + un-skip follow-up, and a note to fold each catalog umbrella (Effect.h/Modifier.h/Driver.h/Layout.h) into its *Base.h. Reviews: - πŸ‘Ύ Opus pre-commit Reviewer over the whole pending diff: two findings. Must-fix β€” the resumableFrames default flip broke unit_PreviewDriver's dynamicBytes test (resolved: skipped + backlogged, accounting itself is correct). Should-fix β€” sync() could read "receive" on a no-network Simulate build (fixed: hasNetwork guard on the accessor). No other blocking findings. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/backlog/backlog-light.md | 25 ++- docs/moonmodules/core/services.md | 18 +- esp32/sdkconfig.defaults | 20 +- moondeck/scenario/run_scenario.py | 2 +- moondeck/test/test_desktop.py | 2 +- src/core/AudioService.h | 210 ++++++++++-------- src/core/HttpServerModule.cpp | 28 ++- src/core/HttpServerModule.h | 8 +- src/light/drivers/Drivers.h | 6 + src/light/drivers/PreviewDriver.h | 10 +- src/light/effects/NetworkReceiveEffect.h | 38 ++-- src/platform/desktop/platform_desktop.cpp | 15 +- src/platform/esp32/platform_esp32.cpp | 21 +- src/platform/esp32/platform_esp32_worker.cpp | 41 ++-- src/platform/platform.h | 16 +- src/ui/app.js | 78 +++++-- test/scenarios/light/scenario_perf_full.json | 4 +- test/unit/core/unit_AudioService_sync.cpp | 30 +-- test/unit/light/unit_FreqSawsEffect.cpp | 3 +- test/unit/light/unit_GEQ3DEffect.cpp | 6 +- .../unit_NetworkReceiveEffect_protocols.cpp | 6 +- test/unit/light/unit_NoiseMeterEffect.cpp | 9 +- test/unit/light/unit_PreviewDriver.cpp | 35 +-- 23 files changed, 422 insertions(+), 209 deletions(-) diff --git a/docs/backlog/backlog-light.md b/docs/backlog/backlog-light.md index dde0ded1..ed5c338d 100644 --- a/docs/backlog/backlog-light.md +++ b/docs/backlog/backlog-light.md @@ -197,11 +197,15 @@ Today a "light" is a point at a static coordinate with a colour. A **moving head **Sub-item β€” per-emitter targets, and fixing the Yellow/UV RGB-synthesis placeholder.** `Correction::apply()` today synthesizes every non-RGB emitter (White, WarmWhite, Yellow, UV) from RGB via the one `whiteMode` gate. That conflates two different physics: **White/WarmWhite are broadband illumination** with a real achromatic basis (`min(R,G,B)` is a sound approximation β€” warm vs cold differ only in phosphor CCT a byte value can't express), but **Yellow/Amber (~590 nm) and UV (~400 nm) are saturated/out-of-gamut emitters with no honest RGB pre-image.** Yellow's `min(R,G)` stand-in reads greener than a true amber die AND fires on far too much (any red+green content β€” yellows, whites, skin tones β€” muddies the fixture); UV's blue-excess is an eyeball hack. Amber is a *real, common, targetable* emitter (RGBA / RGBAW PARs ship a dedicated ~590 nm die), so the right model is an **effect that targets amber DIRECTLY** (a typed attribute), not RGB-and-synthesize. The fixture model is where that lands. **Decision to make when built:** whether Yellow/UV should default to **off** (0, like `whiteMode::None`) until an effect drives them β€” more honest than always firing a wrong approximation β€” vs. keeping the "light it up to eyeball the wiring" placeholder. **Practical test to run then:** on a real RGBA/RGBAW+UV PAR, compare the synthesized amber against a directly-driven amber die (does `min(R,G)` look acceptably yellow, or muddy?), and confirm the White subtraction under `Accurate` reads correct with all emitters present. Rationale + the current formulas live in `Correction.h`'s emitter-field comment. -### Spacer / gapped-strand layout β€” dark gaps between physical LED segments (user request) +### Mid-strand dark gaps (slat wall) β€” a DRIVER feature, not a layout (user request, BLOCKED on wiring) -**User request (via PO):** driving a slat wall on the P4 where each data pin's physical strand has BLACK GAPS between addressed segments β€” e.g. pin 1's strand is 550 LEDs physically, but column 1 is LEDs 0–250, column 2 is LEDs 300–550, and 251–299 are unaddressed spacer LEDs that must stay dark. Today's layouts map a contiguous run of pixels per strand; there is no way to express "skip N physical LEDs here, then resume." +A slat wall where one data pin's strand has dark gaps between lit segments (e.g. 550 LEDs, lit 0–250 and 300–550, dark 251–299). Investigated 2026-07-21, two findings settle the design: -The need is a layout (or layout modifier) that inserts **spacer regions** into a strand's pixel-to-physical mapping: the effect/grid still sees a contiguous logical space, but the driver's per-strand output leaves the gap LEDs at zero (dark). Design questions to settle before building: is this a new **layout type** (a "SlatWall"/"SpacedStrip" that takes segment lengths + gap lengths), a **modifier** on an existing layout (insert gaps into the coordinate stream), or a **driver-level** per-strand offset map? The driver already has a window (`start`/`count`) per strand; a gap is essentially multiple windows per strand. Physics constraint whatever the design: a serial WS2812 strand cannot skip positions (LED n+1's data passes through LED n), so every spacer LED must receive explicitly clocked ZERO bytes each frame β€” "unmapped" coordinates still occupy wire slots, and the design must guarantee those slots carry zeros, not stale buffer content. Spec it (a `docs/moonmodules/light/layouts` entry) before code; the panels/grid layout code is the reference for how coordinates map to the buffer. +1. **NOT a layout/modifier, it's a driver per-strand feature.** A layout emits real lights only; the layout/LUT layer has no "wire slot that addresses no light." The gap lives at the one driver seam that maps a wire position to a light index (`ParallelLedDriver::encodeRows`, `src = winStart_ + laneStart_[lane] + row`, configured by the per-strand `laneStart_`/`laneCounts_` loop in `parseConfig`, today one contiguous window per strand). The driver already clocks dark positions via the short-strand path (`if (row >= laneCounts_[lane]) continue;`), so a mid-strand gap is that same shape, not only at the end. + +2. **Most slat walls need NO code.** A gap between DIFFERENT strands is already doable: one strand/pin per lit segment (or per-driver `start`/`count` windows), gap = strand boundary. Code is needed ONLY for ONE continuous strand across the gap (data passing through the dark LEDs) β€” a window can't insert dark slots mid-strand, and two drivers can't share a pin. + +**If built** (continuous-strand case only): generalize `laneStart_`/`laneCounts_` to a per-strand segment list `[(start,len), gap, (start,len)]`; gap rows reuse the short-strand LOW-clock path; add a gap token to `ledsPerPin` (`250,50g,250`) or a `gaps` control. No layout/LUT change. **BLOCKED ON** confirming the user's wiring: separate strands β†’ close with the workaround documented; continuous strand β†’ build the driver feature. ### Mixing light types in one Layouts β€” open design question (undesigned) @@ -211,6 +215,21 @@ Today each layout child describes one light type (all LED strips, or all par lig The preview's transport β€” resumable cross-tick send from a stable buffer + newest-wins backpressure drop + adaptive graceful degradation (see [architecture.md Β§ graceful degradation under transport backpressure](../architecture.md)) β€” is **payload-agnostic**: any bulky throttled stream (a future MJPEG/video preview, fixture-state streams, fleet telemetry) could ride it. The *payload* model (count/stride/RGB) is light-specific; the *byte-pump* is not. When a second consumer for this transport appears, promote the pump into a domain-neutral core primitive (a `ThrottledChannel`-style sink) that PreviewDriver becomes *a* producer on, rather than owning the protocol. Concrete-first: extract on the second use, not before β€” until then the seam stays inside HttpServerModule/PreviewDriver. +### PreviewDriver `resumableFrames` default OFF: fix the tearing, then un-skip the dynamicBytes test + +`resumableFrames` (the downsampled-frame transport A/B) now defaults **OFF** because the resumable path visibly **tears the preview**: it shares the single-occupancy WS send slot with the ~1 Hz full-state push and the next preview frame, so a preempted mid-drain frame reaches the browser spliced (top rows new, the rest stale β€” PO saw it on the wall). OFF uses the proven-correct synchronous transport. Two follow-ups, both small: + +1. **The tear itself (the reason to eventually want ON again).** Give the preview send its OWN send slot instead of sharing `previewSend_` with the state push, OR make a preempted drain drop cleanly (versioned frame β†’ the browser discards a spliced one) rather than splicing. Only then is ON safe to default. The off-thread send exists to avoid the ~17 ms render hitch at very large grids with the preview open, so this matters mainly for big walls; until fixed, synchronous is correct. +2. **The skipped test.** `unit_PreviewDriver.cpp`'s "reports its resumable-path buffers in dynamicBytes" is `doctest::skip()`'d: it was written when `resumableFrames` defaulted ON and its first assertion relied on the rig constructor's `applyState()` allocating the staging buffer via that default. With the flag OFF, toggling it ON post-construction + `prepare()` did not re-allocate the buffers in the test rig the way the constructor path did (the "ON" reads dropped to 0). The *accounting* (`driverHeapBytes` sums `stageCap_` + `keptIdxCap_`) is unchanged and correct; only the test's default assumption broke. Un-skip by rebuilding the rig so it can deterministically allocate the resumable-path buffers with the flag OFF-by-default β€” likely wiring the flag ON into the rig BEFORE its first `drivers.applyState()` so the same acquire path production uses runs (the post-construction toggle route resisted several attempts; the constructor-time route is the one to nail). Small test-only work. + +### Fold each catalog umbrella into its `*Base.h`, so an author includes the base class they inherit + +Each catalog domain has a two-file split: a `*Base.h` (the class an author subclasses β€” `EffectBase`, `ModifierBase`, `DriverBase`, `LayoutBase`) plus a same-directory **umbrella** (`Effect.h` / `Modifier.h` / `Driver.h` / `Layout.h`) that the module actually `#include`s. The umbrella exists to (a) break a cycle and (b) bundle the domain's helper headers so the author writes one line. For effects the cycle is concrete: `Layer.h` includes `EffectBase.h` (Layer holds effect children), so `EffectBase.h` can only *forward-declare* `Layer` and its accessor bodies (`buffer()`/`width()`/…) are defined out-of-line at the bottom of `Layer.h`; an effect must therefore pull in `Layer.h`, which the umbrella does. The **smell** (raised 2026-07): an author includes `Effect.h`, not the `EffectBase` they subclass β€” the umbrella's name leaks a "trick to fix cyclic includes" into the author's mental model. + +**Decided direction (option 1):** fold the umbrella's contents into the `*Base.h` and delete the separate umbrella, so the author-facing include is the base-class header. The author writes `#include "light/effects/EffectBase.h"` and inherits `EffectBase` from the same file β€” no more differently-named umbrella. Apply uniformly across all four domains (keep the pattern consistent). + +**The load-bearing constraint the implementer must respect** (this is why it's a real task, not a rename): `Layer.h` includes `EffectBase.h`, so `EffectBase.h` **cannot** include `Layer.h` back (direct cycle). The merge works only if the `#include "…/Layer.h"` + helper includes go at the **bottom** of `EffectBase.h`, *after* the `EffectBase` class declaration and its forward-declare of `Layer` β€” Layer.h then re-enters EffectBase.h (already fully declared via the include guard), pulls the accessor definitions, and closes the loop. Verify on a clean build that the include order holds for all four domains before committing (Modifier/Layout have a lighter dependency; Driver pulls Layer/Buffer/Correction). If any domain's ordering can't be made clean, keep that one's umbrella and note why at the site. Net-neutral-to-subtraction: deletes four umbrella files, moves their includes into the base headers, and touches every catalog module's top include line (`Effect.h` β†’ `EffectBase.h`, Γ—4 domains). + ## LCD / DMA driver work ### Drop the i80 WR/DC sacrificial pins β€” done for MoonI80, open for I80 diff --git a/docs/moonmodules/core/services.md b/docs/moonmodules/core/services.md index 98b502b9..d2fb38e2 100644 --- a/docs/moonmodules/core/services.md +++ b/docs/moonmodules/core/services.md @@ -14,17 +14,19 @@ Detail: [technical](moxygen/Services.md) ### Audio -A Service (added by the user, not auto-wired): an IΒ²S microphone (or line-in ADC) feeding the FFT that audio-reactive effects consume via `AudioService::latestFrame()`. It also syncs audio over UDP, WLED-compatible: broadcast the local analysis for the WLED ecosystem, or receive a peer's audio to drive effects with no local mic. Idle until real GPIOs are entered. +A Service (added by the user, not auto-wired): the audio source that feeds the FFT audio-reactive effects consume via `AudioService::latestFrame()`. `mode` is the first choice, the module's identity, and each mode shows only its own detail controls: Local audio runs its own peripheral (an IΒ²S microphone or line-in ADC) and analyzes it locally, Receive network is a pure network sink a peer's WLED-compatible audio drives, and Simulate is a synthesized source for demos and tests. Idle until real GPIOs are entered in Local mode. Audio module controls -- `sckPin` / `wsPin` / `sdPin` β€” the IΒ²S GPIOs (bit clock / word-select / data; unset until entered). -- `mclkPin` β€” master-clock GPIO for a line-in ADC that needs one (e.g. the PCM1808); leave unset for a plain mic. -- `sampleRate` β€” mic/ADC sample rate. -- `floor` / `gain` β€” noise floor and input gain for the analysis. -- `simulate` β€” feed a synthetic signal instead of the mic (for testing without hardware). -- `sync` β€” Off / Send / Receive: broadcast or receive WLED audio-sync packets. `syncPort` sets the UDP port (default 11988, the WLED standard); Receive auto-blends back to the local mic ~1 s after a peer goes quiet. -- read-only β€” `level` (RMS), `peakHz`, `sync status`. +- `mode` β€” Local audio / Receive network / Simulate: analyze the on-board mic/line-in, consume a peer's audio off the network (WLED-compatible), or feed a synthesized signal. The controls below are its detail, shown per mode. +- `sckPin` / `wsPin` / `sdPin` β€” (Local) the IΒ²S GPIOs (bit clock / word-select / data; unset until entered). +- `mclkPin` β€” (Local) master-clock GPIO for a line-in ADC that needs one (e.g. the PCM1808); leave unset for a plain mic. +- `sampleRate` β€” (Local) mic/ADC sample rate. +- `floor` / `gain` β€” (Local) noise floor and input gain for the analysis. +- `send audio` β€” (Local) broadcast the local analysis as WLED audio-sync packets for the WLED ecosystem. +- `simulate` β€” (Simulate) the synthetic pattern: `music` (a plausible song) or `sweep` (a deterministic band-marching test pattern). +- `syncPort` β€” the UDP port (default 11988, the WLED standard), shown when sending or receiving; set it the same on both ends. `sync status` reports the live send/receive state. +- read-only β€” `level` (RMS), `peakHz` (the audio driving effects, from any source). Detail: [technical](moxygen/AudioService.md) diff --git a/esp32/sdkconfig.defaults b/esp32/sdkconfig.defaults index 01773e7c..cb2fabe4 100644 --- a/esp32/sdkconfig.defaults +++ b/esp32/sdkconfig.defaults @@ -1,5 +1,11 @@ # Common defaults -CONFIG_ESP_MAIN_TASK_STACK_SIZE=8192 +# Main-task stack. The HTTP server runs on this task, and its deepest path stacks a 2 KB request buffer +# (handleConnection) UNDER a recursive module-tree JSON serialize (writeModuleJson recurses per tree level) +# UNDER a ~1 KB stack JsonSink β€” a UI refresh fires a page-load burst that drives this path repeatedly. +# 8 KB overflowed it on the classic ESP32 (smallest stack of the targets) with a "stack overflow in task +# main" panic-reboot on refresh. 12 KB gives the serialize path comfortable headroom; the classic has ~170 +# KB free RAM, so the extra 4 KB is cheap, and the S3/P4 have even more. +CONFIG_ESP_MAIN_TASK_STACK_SIZE=12288 # CPU at the chip's rated maximum (IDF defaults to 160 MHz) β€” the render loop and the ring ISR's # encode deadline are compute-bound, and the P4 ignores this symbol (its own 360 MHz default applies). @@ -44,6 +50,18 @@ CONFIG_LWIP_TCPIP_CORE_LOCKING=y # β€” avoids partial sends that would drop and reconnect the preview connection. CONFIG_LWIP_TCP_SND_BUF_DEFAULT=11520 +# A browser page-load opens the HTML + several JS/CSS files + the persistent WebSocket all at once, and +# every refresh churns that whole set. The default 10-socket ceiling can't hold one burst (6 files + WS + +# the listen socket + WiFi/mDNS internals) once a few closed sockets sit in TIME_WAIT, so lwIP RSTs the +# excess connections BEFORE the app's accept() ever sees them β€” the "load it a few times before the UI +# shows / the socket connects" symptom, with no server-side log because the app never got the connection. +# Raise the ceiling to lwIP's max (16) so a full burst fits, and cut TCP_MSL from 60 s to 5 s so a closed +# socket frees its slot in seconds instead of a minute β€” TIME_WAIT from rapid refreshes no longer piles up +# to the ceiling. (5 s is ample on a trusted LAN; the 2Γ—MSL wait only guards against delayed duplicate +# segments, which a LAN doesn't produce.) +CONFIG_LWIP_MAX_SOCKETS=16 +CONFIG_LWIP_TCP_MSL=5000 + # WiFi CONFIG_ESP_WIFI_ENABLED=y diff --git a/moondeck/scenario/run_scenario.py b/moondeck/scenario/run_scenario.py index 911f3c2e..e33734c4 100644 --- a/moondeck/scenario/run_scenario.py +++ b/moondeck/scenario/run_scenario.py @@ -212,7 +212,7 @@ def main(): if not RUNNER.exists(): print(f"Scenario runner not found: {RUNNER}") - print("Compile it first: build_desktop.py --tests (MoonDeck β†’ desktop β†’ Compile Tests).") + print("Compile it first: uv run moondeck/build/build_desktop.py --tests (MoonDeck β†’ desktop β†’ Compile Tests).") sys.exit(1) module_filter = args.module if (args.module and args.module.lower() != "all") else None diff --git a/moondeck/test/test_desktop.py b/moondeck/test/test_desktop.py index 6f4ad7c0..6f9baec8 100644 --- a/moondeck/test/test_desktop.py +++ b/moondeck/test/test_desktop.py @@ -52,7 +52,7 @@ def main(): if test_exe is None: print(f"Test executable not found under " f"{build_dir.relative_to(ROOT)}/test/.") - print("Compile it first: build_desktop.py --tests (MoonDeck β†’ desktop β†’ Compile Tests).") + print("Compile it first: uv run moondeck/build/build_desktop.py --tests (MoonDeck β†’ desktop β†’ Compile Tests).") sys.exit(1) # Default: summary-only output (`-s` dumps every passing assertion, ~11k lines diff --git a/src/core/AudioService.h b/src/core/AudioService.h index 4e823f47..6bc11381 100644 --- a/src/core/AudioService.h +++ b/src/core/AudioService.h @@ -128,23 +128,38 @@ class AudioService : public MoonModule { ///< ambient room dark, lower for a quiet room. uint8_t gain = 222; ///< sensitivity β€” HIGHER = more (a narrower dB window ///< so a given sound fills more of the bar). - /// Simulated audio: fill the frame with a synthesized signal so audio-reactive effects are demoable - /// (and testable) without a mic or music, such as a preview/demo device with no microphone. Two patterns: + /// Simulated-audio pattern (only shown, and only used, in Simulate mode β€” see `mode`). The synthesized + /// signal drives audio-reactive effects with no mic or music, for a preview/demo device or a test: /// `music` β€” a plausible song: multi-sine bands + a swelling volume + a periodic beat + a /// sweeping peak. Nice for demos (bars dance, VU breathes, peaks move). /// `sweep` β€” a single band lit, marching bassβ†’treble on a timer, with the peak frequency and a /// steady volume tracking it. Deterministic β€” the clean test pattern to check that each /// effect responds across the whole spectrum. - /// A real mic always wins: when `mode` is a fill-in ("silence") mode it only runs while there's no real signal. - uint8_t simulate = 0; ///< 0 = off, 1 = music (fill silence), 2 = sweep (fill silence), - ///< 3 = music (always), 4 = sweep (always) - /// WLED audio-sync over UDP (port 11988, broadcast, WLED v2 wire format β€” - /// light/WLEDAudioSyncPacket.h). 0 = Off (local audio only, no socket bound); - /// 1 = Send (broadcast this device's AudioFrame for WLED/MoonLight receivers); - /// 2 = Receive (bind 11988; a peer's audio overwrites frame_ so effects react to - /// it, auto-falling-back to the local mic when packets stop for ~1 s). The socket - /// is bound only in Send/Receive β€” Off costs nothing. Changing it re-binds live. - uint8_t sync = 0; ///< 0 = off, 1 = send, 2 = receive + uint8_t simulate = 0; ///< 0 = music, 1 = sweep (the pattern; Simulate mode only) + /// The module's audio SOURCE, the first thing to pick (below status). Three either/or modes, each with + /// its own detail controls (the others hide): 0 = Local audio (its own peripheral β€” the on-board mic / + /// line-in; analyze it here and optionally broadcast it, so pins, rate, floor/gain and "send audio" + /// show). 1 = Receive network (a pure network sink: bind the sync port and let a peer's AudioFrame drive + /// the effects β€” no local peripheral). 2 = Simulate (a synthesized source for demos/tests β€” only the + /// `simulate` pattern picker shows). A device is exactly one of these at a time. Changing it re-runs + /// prepare() (acquires/releases the mic, rebinds/unbinds the socket) and re-toggles which controls + /// show, all live. + uint8_t mode = 0; ///< 0 = local audio, 1 = receive network, 2 = simulate + /// The `mode` value that means Simulate. "receive network" (index 1) only exists where the network + /// does, so Simulate is index 2 with a network stack and index 1 without β€” the whole mode set shifts, + /// and every mode comparison uses this constant rather than a bare literal (defineControls + tick). + static constexpr uint8_t kSimMode = platform::hasNetwork ? 2 : 1; + /// Broadcast this device's AudioFrame over UDP (WLED v2 wire format) for WLED / MoonLight + /// receivers. Only meaningful, and only shown, in Local mode (there's nothing to send when + /// you're a network sink). Off by default: a fresh module analyzes locally and broadcasts + /// nothing until you opt in. + bool send = false; + /// The three source/sink states the sync machinery keys off, derived from mode + send so the + /// socket/tick logic stays a single 0/1/2 switch (0 = no socket, 1 = broadcast, 2 = network sink): + /// Local+send β†’ send, Local alone β†’ off (local-only, no socket), Receive β†’ receive. The `hasNetwork` + /// guard on the Receive leg matters on a no-network build: there mode 1 is Simulate (not Receive β€” see + /// kSimMode), so without the guard a Simulate device would wrongly read as "network sink" (2). + uint8_t sync() const { return (platform::hasNetwork && mode == 1) ? 2 : (send ? 1 : 0); } /// The sync UDP port β€” the Send destination and the Receive listen port. Defaults /// to WLED's 11988 (interop with WLED/MoonLight); set it the same on both ends to /// run a private projectMM-only sync group on a non-WLED port. @@ -156,40 +171,60 @@ class AudioService : public MoonModule { ? sampleRateSel : 2]; } void defineControls() override { - controls_.addPin("sckPin", sckPin); - controls_.addPin("wsPin", wsPin); - controls_.addPin("sdPin", sdPin); - controls_.addPin("mclkPin", mclkPin); + // `mode` is the module's identity, so it's the first control (below status). Everything else is + // its detail: the local-audio group shows only in Local mode, the simulate pattern only in Simulate + // mode, the sync group only when there's a socket. Registration order follows the dependency (the + // coding-standards rule): the gate first, the controls it gates under it, mutually-exclusive groups + // sharing a gate grouped after it. The "receive network" option only exists where the network does + // (Local + Simulate always; Receive added when platform::hasNetwork), so Simulate is index 1 on a + // no-network build and 2 with network β€” the class constant kSimMode carries that shift. + const bool localMode = (mode == 0); + const bool simMode = (mode == kSimMode); + if constexpr (platform::hasNetwork) { + static constexpr const char* kModeOptions[] = {"local audio", "receive network", "simulate"}; + controls_.addSelect("mode", mode, kModeOptions, 3); + } else { + static constexpr const char* kModeOptions[] = {"local audio", "simulate"}; + controls_.addSelect("mode", mode, kModeOptions, 2); + } + // --- Local-audio group: the on-board mic / line-in and its analysis. Shown only in Local mode. The + // pins default UNSET (-1) so adding the module can't grab GPIOs; order follows the I2S datasheet + // (clocks, data, optional MCLK). --- + controls_.addPin("sckPin", sckPin); controls_.setHidden(controls_.count() - 1, !localMode); + controls_.addPin("wsPin", wsPin); controls_.setHidden(controls_.count() - 1, !localMode); + controls_.addPin("sdPin", sdPin); controls_.setHidden(controls_.count() - 1, !localMode); + controls_.addPin("mclkPin", mclkPin); controls_.setHidden(controls_.count() - 1, !localMode); static constexpr const char* kRateOptions[] = {"8000", "16000", "22050", "44100"}; controls_.addSelect("sampleRate", sampleRateSel, kRateOptions, kSampleRateCount); - // floor/gain condition the LOCAL FFT/level mapping. They stay visible in every sync - // mode β€” including Receive, because auto-blend falls back to the local mic when the - // peer goes quiet, and floor/gain govern that fallback. (They're only transiently - // dead while a peer's audio is actively driving the frame β€” not permanently, so - // hiding them by the sync setting would wrongly hide a control the fallback needs.) - controls_.addUint8("floor", floor, 0, 255); - controls_.addUint8("gain", gain, 1, 255); - static constexpr const char* kSimulateOptions[] = { - "off", "music (silence)", "sweep (silence)", "music (always)", "sweep (always)"}; - controls_.addSelect("simulate", simulate, kSimulateOptions, 5); - // WLED audio sync β€” only on builds with an IP stack (WiFi OR Ethernet: the UDP - // send/receive works over either, so an Ethernet-only board like the MHC-WLED - // shield still gets it). A no-network build hides the controls entirely. + controls_.setHidden(controls_.count() - 1, !localMode); + // floor/gain condition the local FFT/level mapping. + controls_.addUint8("floor", floor, 0, 255); controls_.setHidden(controls_.count() - 1, !localMode); + controls_.addUint8("gain", gain, 1, 255); controls_.setHidden(controls_.count() - 1, !localMode); + // "send audio": broadcast the locally-analyzed frame. Only meaningful in Local mode. + if constexpr (platform::hasNetwork) { + controls_.addBool("send audio", send); + controls_.setHidden(controls_.count() - 1, !localMode); + } + // --- Simulate group: the synthesized-pattern picker, shown only in Simulate mode. --- + static constexpr const char* kSimulateOptions[] = {"music", "sweep"}; + controls_.addSelect("simulate", simulate, kSimulateOptions, 2); + controls_.setHidden(controls_.count() - 1, !simMode); + // --- Sync group: only relevant when a socket is bound, i.e. sending (Local + send) or receiving. + // Both the port and the live status hide when there's no socket. A mode/send change re-runs this + // method (both are in affectsPrepare) so the rows toggle live. --- if constexpr (platform::hasNetwork) { - static constexpr const char* kSyncOptions[] = {"off", "send", "receive"}; - controls_.addSelect("sync", sync, kSyncOptions, 3); - // The UDP port β€” the Send destination and the Receive listen port. Defaults to - // WLED's 11988 (interop with WLED/MoonLight); change it on BOTH ends to run a - // private projectMM-only sync group on a non-WLED port. Shown whenever sync is on - // (Send or Receive), hidden in Off. A `sync` change re-runs this method (it's in - // affectsPrepare) so the row toggles live. + const bool hasSocket = (sync() != 0); + // The UDP port β€” the Send destination and the Receive listen port. Defaults to WLED's + // 11988 (interop with WLED/MoonLight); change it on BOTH ends to run a private + // projectMM-only sync group on a non-WLED port. controls_.addUint16("syncPort", syncPort, 1, 65535); - controls_.setHidden(controls_.count() - 1, sync == 0); + controls_.setHidden(controls_.count() - 1, !hasSocket); controls_.addReadOnly("sync status", syncStr_, sizeof(syncStr_)); + controls_.setHidden(controls_.count() - 1, !hasSocket); } - // Read-only live read-outs (formatted in tick1s). Derived every second, - // nothing to persist, so ReadOnly (the display-only type) not a flipped - // Text β€” same idiom as SystemModule's uptime/fps. + // Read-only live read-outs (formatted in tick1s). These show the audio actually driving the + // effects, so they stay visible in Receive too (there the frame comes off the network, not a + // mic). Derived every second, nothing to persist, so ReadOnly not a flipped Text. // "level RMS" = the RMS loudness; the DISPLAYED number is its peak over the 1-second window // (tick1s publishes levelPeak_, the max of the per-block RMS level, then resets it), so a // beat that lands between samples still registers. The live frame_.level the LEDs use is the @@ -200,24 +235,27 @@ class AudioService : public MoonModule { MoonModule::defineControls(); } - /// A pin or rate change rebuilds the I2S channel (live, no reboot); a `sync` / - /// `syncPort` change re-binds/unbinds the UDP socket AND re-toggles the port row's - /// visibility (all flow through prepare β†’ rebuildControls). + /// A pin or rate change rebuilds the I2S channel (live, no reboot); a `mode` / `send` / + /// `syncPort` change re-binds/unbinds the UDP socket AND re-toggles which control rows show + /// (all flow through prepare β†’ rebuildControls). bool affectsPrepare(const char* name) const override { return std::strcmp(name, "wsPin") == 0 || std::strcmp(name, "sdPin") == 0 || std::strcmp(name, "sckPin") == 0 || std::strcmp(name, "mclkPin") == 0 - || std::strcmp(name, "sampleRate") == 0 || std::strcmp(name, "sync") == 0 - || std::strcmp(name, "syncPort") == 0; + || std::strcmp(name, "sampleRate") == 0 || std::strcmp(name, "mode") == 0 + || std::strcmp(name, "send audio") == 0 || std::strcmp(name, "syncPort") == 0; } - /// Pure build (see MoonModule::prepare): claim the mic election (first live mic wins) and - /// (re)acquire the IΒ²S mic + sync socket. No enabled() check β€” core's applyState() calls this only - /// when effectively-enabled and routes to release() otherwise, which releases the mic + sync - /// socket and vacates the election, so a disabled service (or one under a disabled parent) frees - /// its pins for another module. + /// Pure build (see MoonModule::prepare): claim the frame election (this instance's frame_ drives the + /// effects in EVERY mode β€” a live mic, a received peer frame, or a synthesized one), then acquire only + /// the hardware the current `mode` needs. Only Local runs a real peripheral, so only Local inits the + /// IΒ²S mic; Receive (a network sink) and Simulate (a synthetic source) free the IΒ²S channel and its + /// pins. The sync socket is rebound in every mode (syncReinit is a no-op unless there's a socket to + /// bind). No enabled() check β€” core's applyState() calls this only when effectively-enabled and routes + /// to release() otherwise. void prepare() override { - micSeat_.claim(); // first live mic wins the seat; a 2nd is captured but not read (claim-if-empty) - reinit(); + micSeat_.claim(); // first live instance wins the frame seat (claim-if-empty), any mode + if (mode == 0) reinit(); // Local only: (re)acquire the IΒ²S mic + else deinit(); // Receive / Simulate: no peripheral β€” free the IΒ²S channel + its pins syncReinit(); } /// One-time wiring only; the mic acquire + election live in prepare(), the sole gate. @@ -270,30 +308,31 @@ class AudioService : public MoonModule { // seat is held, the reclaim once it's empty. micSeat_.claim(); - // WLED audio sync. Send broadcasts the current frame_ (throttled). Receive drains - // the socket into frame_ and, while a peer's audio is fresh, RETURNS so the local - // mic analysis below is skipped (the received frame drives the effects); once the - // peer goes quiet (~1 s) it falls through and the local mic resumes (auto-blend). + // WLED audio sync. Send (Local + send audio) broadcasts the current frame_ (throttled), + // then falls through to run the local mic analysis that produces that frame_. Receive is a + // pure network sink: drain the socket into frame_ and RETURN unconditionally so the local + // mic path never runs (there is no peripheral in this mode), holding the last frame while a + // peer is quiet rather than blending to a mic. if constexpr (platform::hasNetwork) { - if (sync != 0 && syncEnsureSocket()) { // lazy-open once the network is up - if (sync == 1) syncSend(); - else if (sync == 2 && syncReceive()) return; + const uint8_t s = sync(); + if (s != 0 && syncEnsureSocket()) { // lazy-open once the network is up + if (s == 1) syncSend(); + else if (s == 2) { syncReceive(); return; } } + if (s == 2) return; // sink with no socket yet: still never runs the local mic } - // Simulated audio (see the `simulate` control): 0=off, 1=music-on-silence, 2=sweep-on-silence, - // 3=music-always, 4=sweep-always. Real mic input always wins in the "on-silence" modes β€” the - // mic path below resets realQuietMs_ whenever a block carries signal, and synthesizeFrame() - // no-ops while that timer says the mic is live. Off I2S (desktop / mic-less) or on a bad init - // there's no mic, so the sim is the only possible source: run it and return. - const bool simSweep = (simulate == 2 || simulate == 4); - const bool simAlways = (simulate >= 3); + // Simulate mode (see `mode`): a synthesized source, driven by the `simulate` pattern (0 = music, + // 1 = sweep). It's a whole mode, not a mic fill-in, so it always runs and returns β€” the mic path + // below never executes in Simulate mode. + if (mode == kSimMode) { synthesizeFrame(simulate == 1); return; } + + // From here on it's Local mode. Off I2S (desktop / mic-less) or before a good init there's no mic, + // so nothing is produced β€” the last frame is held (no synthetic fallback; that's Simulate mode's job). if constexpr (!platform::hasI2sMic) { - if (simulate != 0) synthesizeFrame(simSweep); // the only possible source off I2S return; } else { - if (!inited_) { if (simulate != 0) synthesizeFrame(simSweep); return; } - if (simAlways) { synthesizeFrame(simSweep); return; } // forced: skip the mic entirely + if (!inited_) return; } // Drain whatever the DMA holds this tick (non-blocking) into the free tail @@ -348,15 +387,6 @@ class AudioService : public MoonModule { // live every render tick) move with the music. The window peak is the representative reading. // Display-only β€” the live frame_.level the effects/LEDs use is untouched. if (frame_.level > levelPeak_) levelPeak_ = frame_.level; - - // Auto fill-in (simulate 1/2): if the mic has been quiet for a bit, synthesize so the effects - // still have something to react to. `level` this block re-arms the "real audio" grace period; - // once it lapses, the sim takes over and yields again the instant real sound returns. - if (simulate == 1 || simulate == 2) { - if (frame_.level > kSimRealThreshold) realBlocks_ = kSimRealGraceBlocks; - else if (realBlocks_ > 0) realBlocks_--; - if (realBlocks_ == 0) synthesizeFrame(simulate == 2); - } } /// Fill frame_ with a synthesized signal. sweep=false β†’ a plausible "song" (each band its own @@ -414,7 +444,8 @@ class AudioService : public MoonModule { // no samples at all β†’ the I2S clocks aren't running β€” check sckPin / wsPin. // samples, all zero β†’ clocks fine, data line dead β€” check sdPin (SD/DOUT) + power. // samples, non-zero β†’ data is flowing; clear any prior diagnosis. - const bool directMicLive = inited_ && sync != 2 + // Only Local mode has a real mic to diagnose β€” Receive and Simulate produce frame_ without one. + const bool directMicLive = inited_ && mode == 0 && platform::audioCodecType == platform::CodecType::None; if (directMicLive) { if (micSamples1s_ == 0) @@ -432,9 +463,10 @@ class AudioService : public MoonModule { // syncReinit/syncEnsureSocket set ("waiting for network" / "…failed"); only once // open do we report the moment-to-moment send/receive state. if constexpr (platform::hasNetwork) { - if (sync == 0) std::snprintf(syncStr_, sizeof(syncStr_), "off"); + const uint8_t s = sync(); + if (s == 0) std::snprintf(syncStr_, sizeof(syncStr_), "off"); else if (syncOpen_) { - if (sync == 1) std::snprintf(syncStr_, sizeof(syncStr_), "sending"); + if (s == 1) std::snprintf(syncStr_, sizeof(syncStr_), "sending"); else std::snprintf(syncStr_, sizeof(syncStr_), (lastSyncRecv_ != 0 && platform::millis() - lastSyncRecv_ < kSyncFallbackMs) @@ -476,12 +508,6 @@ class AudioService : public MoonModule { uint32_t micNonzero1s_ = 0; bool micStatusStale_ = false; - // Auto fill-in (simulate 1/2): treat the mic as "live" for a grace window after any block above - // the threshold, so brief gaps between beats don't flip to the sim. ~2 s of blocks (β‰ˆ86/blockΒ·23ms). - static constexpr uint16_t kSimRealThreshold = 4; // a block level above this counts as real sound - static constexpr uint16_t kSimRealGraceBlocks = 86; // ~2 s at ~23 ms/block before the sim takes over - uint16_t realBlocks_ = 0; // grace countdown: >0 = mic was recently live, hold off the sim - // WLED audio sync (light/WLEDAudioSyncPacket.h). One socket, bound only in Send/Receive. platform::UdpSocket syncSock_; uint32_t lastSyncSend_ = 0; // millis of the last broadcast (send throttle) @@ -581,9 +607,10 @@ class AudioService : public MoonModule { syncOpen_ = false; lastSyncOpenFailMs_ = 0; // a mode change retries bring-up immediately (no stale backoff) lastSyncRecv_ = 0; + const uint8_t s = sync(); std::snprintf(syncStr_, sizeof(syncStr_), - sync == 1 ? "send: waiting for network" - : sync == 2 ? "receive: waiting for network" + s == 1 ? "send: waiting for network" + : s == 2 ? "receive: waiting for network" : "off"); } @@ -595,7 +622,8 @@ class AudioService : public MoonModule { /// past boot so a boot-present AudioService can't touch lwip before it exists. bool syncEnsureSocket() { if constexpr (!platform::hasNetwork) return false; - if (sync == 0) return false; + const uint8_t s = sync(); + if (s == 0) return false; if (syncOpen_) return true; if (!platform::networkReady()) return false; // interface not up yet β€” try again next tick // Back off between failed bring-ups: tick() runs every tick, so without this a @@ -604,7 +632,7 @@ class AudioService : public MoonModule { // failure; hold off until kSyncOpenRetryMs has passed (same throttle form as syncSend). const uint32_t now = platform::millis(); if (lastSyncOpenFailMs_ != 0 && now - lastSyncOpenFailMs_ < kSyncOpenRetryMs) return false; - if (sync == 1) { // send β†’ broadcast destination (configurable port) + if (s == 1) { // send β†’ broadcast destination (configurable port) char bcast[16]; formatDottedQuad(bcast, kBroadcast_); if (syncSock_.open() && syncSock_.connect(bcast, syncPort)) { syncOpen_ = true; diff --git a/src/core/HttpServerModule.cpp b/src/core/HttpServerModule.cpp index 3fa42823..73cff1fc 100644 --- a/src/core/HttpServerModule.cpp +++ b/src/core/HttpServerModule.cpp @@ -86,9 +86,19 @@ void HttpServerModule::tick20ms() { // slider) by SENDING a {on,bri} text frame over /ws, not by HTTP POST β€” so we must read // the socket, not only push to it. Cheap (non-blocking, usually nothing pending). pollWledStateFromWebSockets(); - // Accept one HTTP connection per tick. - auto conn = server_.accept(); - if (conn.valid()) handleConnection(conn); + // Accept and serve a bounded BATCH of HTTP connections per tick, not one. A browser page-load opens + // the HTML + several JS/CSS files + the WS upgrade in parallel (~8 connections); accepting one per + // 20 ms tick drains that burst over ~160 ms and β€” worse β€” lets the accept backlog fill and drop the + // slower connections (the WS among them), so the page loads but the clock/preview never start until a + // refresh. Draining up to kAcceptsPerTick clears a whole first-load burst in ~2 ticks. It stays bounded + // so one tick can't serve an unbounded run of requests (the hot-path rule): accept() returns an invalid + // connection the instant the backlog is empty, which breaks the loop early in the common idle case. + constexpr int kAcceptsPerTick = 8; + for (int i = 0; i < kAcceptsPerTick; i++) { + auto conn = server_.accept(); + if (!conn.valid()) break; // backlog drained (the usual case: 0 or 1 pending) + handleConnection(conn); + } } void HttpServerModule::tick1s() { @@ -2147,7 +2157,7 @@ void HttpServerModule::handleWebSocketUpgrade(platform::TcpConnection& conn, con acceptKey); conn.write(reinterpret_cast(response), respLen); - // Store connection as WebSocket client + // Store connection as WebSocket client. for (int i = 0; i < MAX_WS_CLIENTS; i++) { if (!wsClients_[i].valid()) { wsClients_[i] = std::move(conn); @@ -2164,7 +2174,10 @@ void HttpServerModule::handleWebSocketUpgrade(platform::TcpConnection& conn, con return; } } - // No slot available β€” close + // No slot available β€” close. A slot frees when a dead client's next send/poll fails (reaped within a + // tick or two), so MAX_WS_CLIENTS is sized well above the realistic concurrent count PLUS the transient + // overlap of a refresh (the browser opens the new socket before the old socket's FIN lands, so both + // briefly hold slots). The browser's own WS backoff retries a genuinely-full moment. conn.close(); } @@ -2262,6 +2275,11 @@ void HttpServerModule::pollWledStateFromWebSockets() { if (!ws.valid()) continue; uint8_t f[512]; int n = ws.read(f, sizeof(f)); // non-blocking (read() returns -1 if nothing) + // read() == 0 is a clean peer close (FIN): reap the slot NOW so it frees for a new client. Without + // this the dead slot lingers until the next SEND fails (up to a tick1s later), and a rapid + // refresh/reconnect burst could find every slot still "valid" and be rejected ("WebSocket closed + // before the connection is established"). read() == -1 (nothing pending) leaves a live client alone. + if (n == 0) { ws.close(); continue; } if (n < 6) continue; // a masked text frame is β‰₯6 bytes // A fast slider drag can land MULTIPLE small {on,bri} frames in one read; walk every // complete masked text frame in the chunk so none is dropped (apply each in order β†’ diff --git a/src/core/HttpServerModule.h b/src/core/HttpServerModule.h index 018eff04..0dd1dfe7 100644 --- a/src/core/HttpServerModule.h +++ b/src/core/HttpServerModule.h @@ -237,7 +237,13 @@ class HttpServerModule : public MoonModule, public BinaryBroadcaster { Scheduler* scheduler_ = nullptr; const char* uiPath_ = "src/ui"; - static constexpr int MAX_WS_CLIENTS = 4; + // Sized for a few concurrent viewers PLUS the transient overlap when one refreshes: the browser opens + // the new WS before the OS delivers the old socket's FIN, so both briefly hold a slot (a dead one is + // reaped within a tick or two on its next failed send/poll). At 4 a couple of quick refreshes could + // fill every slot with not-yet-reaped connections and the new upgrade was rejected ("WebSocket closed + // before the connection is established"); 8 leaves headroom so a refresh always lands a slot. Each slot + // is just an fd + a small cursor, so the array stays tiny. + static constexpr int MAX_WS_CLIENTS = 8; platform::TcpConnection wsClients_[MAX_WS_CLIENTS]; uint32_t wsClientGeneration_ = 0; // ++ on each new WS client; see clientGeneration() diff --git a/src/light/drivers/Drivers.h b/src/light/drivers/Drivers.h index f62d13f1..6f8bd887 100644 --- a/src/light/drivers/Drivers.h +++ b/src/light/drivers/Drivers.h @@ -496,6 +496,11 @@ class Drivers : public MoonModule { // dominant CPU cost) and the network send (ArtNet at 16K is the other big one) both leave the // render core. Each driver's tick() is unchanged; only which core calls it differs. void runEncodeLoop() { + // Subscribe THIS (core-1) task to the WDT before feeding it: the subscription is per-task, so the + // taskWdtReset() calls below only count once this task itself is added. Without this, feeding a + // subscription the render task made on a DIFFERENT task is rejected as "task not found" and floods + // the log every frame, starving the network stack (the WS drops β†’ reconnect β†’ full-state β†’ UI churn). + platform::taskWdtSubscribe(); while (!encodeStop_.load(std::memory_order_acquire)) { if (!platform::waitNotify(encodeTask_, 100)) { platform::taskWdtReset(); continue; } if (encodeStop_.load(std::memory_order_acquire)) break; @@ -505,6 +510,7 @@ class Drivers : public MoonModule { platform::taskWdtReset(); encodeDone_.store(true, std::memory_order_release); } + platform::taskWdtUnsubscribe(); // leave the WDT clean before the task exits (no dangling entry) } static void encodeTrampoline(void* self) { static_cast(self)->runEncodeLoop(); } diff --git a/src/light/drivers/PreviewDriver.h b/src/light/drivers/PreviewDriver.h index 35709ce2..1323d0cf 100644 --- a/src/light/drivers/PreviewDriver.h +++ b/src/light/drivers/PreviewDriver.h @@ -480,7 +480,15 @@ class PreviewDriver : public DriverBase { clearStatus(); } } - bool resumableFrames = true; // A/B: downsampled frame via resumable send (ON) vs synchronous (OFF) + // A/B for the downsampled-frame transport: OFF = synchronous begin/push/end stream (blocking socket + // writes on the render thread, proven-correct); ON = gather-then-resumable-send that drains off the + // render thread. Defaults OFF: the resumable path shares the single-occupancy send slot with the ~1 Hz + // full-state push and the next preview frame, so a preempted mid-drain frame reaches the browser + // spliced β€” a visibly TORN preview (top rows new, the rest stale). The off-thread send is only worth it + // to avoid the ~17 ms render hitch at very large grids with the preview open; until the slot-sharing + // tear is fixed (give preview its own send slot, or drop a preempted drain cleanly), the correct + // synchronous path is the default. Kept in-tree as the A/B reference for that fix. + bool resumableFrames = false; bool stageAllocFailed_ = false; // resumable staging buffer couldn't allocate β†’ synchronous fallback bool keptIdxAllocFailed_ = false; // index cache couldn't allocate β†’ per-frame lattice walk uint8_t* stage_ = nullptr; // gathered color payload for the resumable send (see ensureStage) diff --git a/src/light/effects/NetworkReceiveEffect.h b/src/light/effects/NetworkReceiveEffect.h index 60e6d3fd..4d34ce88 100644 --- a/src/light/effects/NetworkReceiveEffect.h +++ b/src/light/effects/NetworkReceiveEffect.h @@ -99,25 +99,25 @@ class NetworkReceiveEffect : public EffectBase { if (n <= 0) break; if (parseArtDmxPacket(pkt_, static_cast(n), universe, data, dataLen)) { applyDmx(universe, data, dataLen); - noteReceiving(kStatusArtnet); + noteReceiving("Art-Net", srcIp); } else if (isArtPoll(pkt_, static_cast(n))) { replyToPoll(srcIp); // make the device show up in controller node lists } } for (int i = 0; i < kMaxPacketsPerTick; i++) { - const int n = e131Socket_.recvFrom(pkt_, sizeof(pkt_)); + const int n = e131Socket_.recvFrom(pkt_, sizeof(pkt_), srcIp); if (n <= 0) break; if (parseE131Packet(pkt_, static_cast(n), universe, data, dataLen)) { applyDmx(universe, data, dataLen); - noteReceiving(kStatusE131); + noteReceiving("E1.31", srcIp); } } for (int i = 0; i < kMaxPacketsPerTick; i++) { - const int n = ddpSocket_.recvFrom(pkt_, sizeof(pkt_)); + const int n = ddpSocket_.recvFrom(pkt_, sizeof(pkt_), srcIp); if (n <= 0) break; if (parseDdpPacket(pkt_, static_cast(n), byteOffset, data, dataLen)) { applyBytes(byteOffset, data, dataLen); - noteReceiving(kStatusDdp); + noteReceiving("DDP", srcIp); } } // Staging β†’ layer buffer (the layer cleared it at tick start). @@ -158,9 +158,12 @@ class NetworkReceiveEffect : public EffectBase { private: static constexpr int kMaxPacketsPerTick = 128; static constexpr const char* kBindFailMsg = "UDP bind failed β€” port in use?"; - static constexpr const char* kStatusArtnet = "receiving Art-Net"; - static constexpr const char* kStatusE131 = "receiving E1.31"; - static constexpr const char* kStatusDdp = "receiving DDP"; + // The receiving-status buffer: "receiving from ", the ONE writable status this effect + // shows (the protocol was three static literals before; folding the sender IP in means the same field + // now also answers "who is driving me"). setStatus holds the pointer (doesn't copy), so the string must + // outlive the call, hence a member, not a stack buffer. Sized for the longest form + // ("receiving Art-Net from 255.255.255.255" = 38 chars + NUL). A bind error uses its own static literal. + char recvStatus_[40] = ""; platform::UdpSocket artnetSocket_; platform::UdpSocket e131Socket_; @@ -170,13 +173,20 @@ class NetworkReceiveEffect : public EffectBase { // freed on disable via MoonModule::release() (the release() override chains to it). ScratchBuffer staging_{*this}; - // Swap the "receiving " diagnostic β€” but never clobber a bind - // error. Pointer compares only (all four strings are static literals). - void noteReceiving(const char* lit) { + // Update the "receiving from " diagnostic, but never clobber a bind error (its status is + // the kBindFailMsg literal, so status() != recvStatus_). Rebuild into recvStatus_ only when the source + // changed: format into a stack scratch, compare, and snprintf into recvStatus_ only on a real change, so + // the common case (same sender, same protocol, every packet) writes nothing and costs one strcmp. + // snprintf (not strncpy) does the copy: it always NUL-terminates and truncates cleanly. + void noteReceiving(const char* proto, const uint8_t ip[4]) { const char* s = status(); - if (s == nullptr || s == kStatusArtnet || s == kStatusE131 || s == kStatusDdp) { - if (s != lit) setStatus(lit, Severity::Status); - } + if (s != nullptr && s != recvStatus_) return; // a bind error (or foreign status) wins + char next[40]; + std::snprintf(next, sizeof(next), "receiving %s from %u.%u.%u.%u", + proto, ip[0], ip[1], ip[2], ip[3]); + if (s == recvStatus_ && std::strcmp(recvStatus_, next) == 0) return; // unchanged, no write + std::snprintf(recvStatus_, sizeof(recvStatus_), "%s", next); + setStatus(recvStatus_, Severity::Status); } // Answer an ArtPoll with our IP/MAC/name so controllers list the device. diff --git a/src/platform/desktop/platform_desktop.cpp b/src/platform/desktop/platform_desktop.cpp index 40f519e4..35340d01 100644 --- a/src/platform/desktop/platform_desktop.cpp +++ b/src/platform/desktop/platform_desktop.cpp @@ -297,8 +297,9 @@ void stopPinnedTask(WorkerTask& t) { t.impl = nullptr; } -void taskWdtSubscribe() {} // no watchdog on the host -void taskWdtReset() {} // no watchdog on the host +void taskWdtSubscribe() {} // no watchdog on the host +void taskWdtUnsubscribe() {} // no watchdog on the host +void taskWdtReset() {} // no watchdog on the host // A host build has no real GPIOs to protect β€” every pin is valid, output-capable, and free of @@ -978,8 +979,13 @@ int TcpConnection::read(uint8_t* buf, size_t maxLen) { bool TcpConnection::write(const uint8_t* data, size_t len) { if (fd_ < 0) return false; - // Send ALL bytes (blocking retry on a full buffer) β€” an HTTP response must arrive complete. - // See the ESP32 impl: a healthy interface drains in microseconds so the retry rarely spins. + // Send ALL bytes (blocking retry on a full buffer) β€” an HTTP response / WS frame must arrive complete. + // A healthy interface drains in microseconds so the retry rarely spins. Bounded by a wall-clock + // deadline (mirrors the ESP32 impl): this runs on the render thread, and a stalled peer whose TCP + // receive window is full would otherwise make send() block forever and hang the loop. On timeout, + // return false so the caller closes that client instead of wedging the device. + constexpr uint32_t kWriteDeadlineMs = 2000; + const uint32_t start = millis(); size_t sent = 0; while (sent < len) { auto n = ::send(sock(fd_), reinterpret_cast(data + sent), @@ -987,6 +993,7 @@ bool TcpConnection::write(const uint8_t* data, size_t len) { if (n > 0) { sent += static_cast(n); } else if (sockWouldBlock()) { + if (millis() - start >= kWriteDeadlineMs) return false; // stalled peer β€” don't hang the loop std::this_thread::sleep_for(std::chrono::milliseconds(1)); #ifndef _WIN32 } else if (errno == EINTR) { diff --git a/src/platform/esp32/platform_esp32.cpp b/src/platform/esp32/platform_esp32.cpp index 4a615344..74f038a0 100644 --- a/src/platform/esp32/platform_esp32.cpp +++ b/src/platform/esp32/platform_esp32.cpp @@ -1495,15 +1495,23 @@ int TcpConnection::read(uint8_t* buf, size_t maxLen) { bool TcpConnection::write(const uint8_t* data, size_t len) { if (fd_ < 0) return false; - // Send every byte, retrying on a full send buffer β€” an HTTP response must arrive complete. - // Blocks the caller until the peer drains, which suits a one-shot response on a per-request - // connection; a healthy interface drains in microseconds, so the retry rarely spins. + // Send every byte, retrying on a full send buffer β€” a response/frame must arrive complete. A healthy + // interface drains in microseconds, so the retry rarely spins. BUT this runs on the render thread (WS + // frames via sendWsTextFrame, HTTP responses via handleConnection), and a stalled peer (a slow or + // half-open client whose TCP receive window is full) makes lwip_write return EWOULDBLOCK indefinitely. + // An UNBOUNDED retry would then hang the render loop until the Task-WDT (12 s) panic-reboots the whole + // device β€” observed as a WS client connect making the board reboot every few seconds. So bound the wait + // by a wall-clock deadline well above a healthy drain (Β΅s) and well below the WDT: on timeout, return + // false so the caller closes that client (the browser reconnects) instead of taking the device down. + constexpr uint32_t kWriteDeadlineMs = 2000; + const uint32_t start = millis(); size_t sent = 0; while (sent < len) { auto n = lwip_write(fd_, data + sent, len - sent); if (n > 0) { sent += static_cast(n); } else if (n < 0 && (errno == EAGAIN || errno == EWOULDBLOCK)) { + if (millis() - start >= kWriteDeadlineMs) return false; // stalled peer β€” don't hang the render loop vTaskDelay(pdMS_TO_TICKS(1)); // wait for send buffer space } else { return false; // real error @@ -1594,7 +1602,12 @@ bool TcpServer::open(uint16_t port) { return false; } - if (listen(fd_, 4) < 0) { + // Backlog sized for a browser page-load burst: a fresh load opens the HTML + several JS/CSS files + + // the WebSocket upgrade all at once (~8 parallel connections). With a small backlog the excess SYNs + // are dropped and the browser must retry β€” the "load it a few times before the UI shows / the socket + // connects" symptom. 8 covers a whole first-load burst so nothing is dropped. (lwIP caps this at + // CONFIG_LWIP_MAX_LISTENING_TCP; 8 is within the default 16.) + if (listen(fd_, 8) < 0) { lwip_close(fd_); fd_ = -1; return false; diff --git a/src/platform/esp32/platform_esp32_worker.cpp b/src/platform/esp32/platform_esp32_worker.cpp index 23811d52..a07ac3eb 100644 --- a/src/platform/esp32/platform_esp32_worker.cpp +++ b/src/platform/esp32/platform_esp32_worker.cpp @@ -10,7 +10,7 @@ #include "freertos/FreeRTOS.h" #include "freertos/task.h" -#include "esp_task_wdt.h" // the render loop subscribes itself so a wedge self-heals (reboot) not hangs +#include "esp_task_wdt.h" // the render loop and the encode worker each subscribe themselves so a wedge self-heals (reboot) not hangs #include #include @@ -121,25 +121,36 @@ void stopPinnedTask(WorkerTask& t) { t.impl = nullptr; } -// Whether taskWdtSubscribe succeeded β€” so taskWdtReset only feeds a real subscription. Written once at -// render-loop start, read each tick on the same (render) task, so no synchronization is needed. -static bool s_renderWdtSubscribed = false; +// Whether the CALLING task subscribed to the WDT β€” so taskWdtReset only feeds a real subscription. The +// WDT subscription is per-task (esp_task_wdt_add/_reset act on the current task), so this flag is +// thread_local, NOT one global: the render loop AND the core-1 encode worker each feed the WDT, and a +// single global would let the worker call esp_task_wdt_reset() on a task the render loop subscribed β€” +// which the IDF rejects as "task not found", flooding the log every tick and starving the network stack. +// Each task reads/writes its own copy, on its own task, so no synchronization is needed. +static thread_local bool s_wdtSubscribed = false; -// Subscribe the CURRENT (render-loop) task to the task WDT. The sdkconfig runs the TWDT with idle-task -// checking OFF (a saturated core is healthy), so nothing is watched unless a task subscribes β€” this is -// that one subscription: if the render loop stops feeding the WDT (a genuine wedge, not a busy frame), -// it panics and reboots (the self-heal) instead of hanging silently, and leaves a backtrace. Idempotent -// enough for one caller; a failure (WDT not inited) just leaves s_renderWdtSubscribed false and reset a -// no-op, degrading to today's unwatched behavior rather than crashing. +// Subscribe the CURRENT task to the task WDT. The sdkconfig runs the TWDT with idle-task checking OFF (a +// saturated core is healthy), so nothing is watched unless a task subscribes. Both the render loop and the +// encode worker call this on their own task: if either stops feeding the WDT (a genuine wedge, not a busy +// frame), it panics and reboots (the self-heal) instead of hanging silently, and leaves a backtrace. +// Idempotent per task; a failure (WDT not inited) just leaves s_wdtSubscribed false and reset a no-op, +// degrading to unwatched behavior rather than crashing. void taskWdtSubscribe() { - if (s_renderWdtSubscribed) return; - if (esp_task_wdt_add(nullptr) == ESP_OK) s_renderWdtSubscribed = true; + if (s_wdtSubscribed) return; + if (esp_task_wdt_add(nullptr) == ESP_OK) s_wdtSubscribed = true; } -// Feed the render task's WDT subscription (esp_task_wdt_reset), called each render tick. No-op until/unless -// taskWdtSubscribe ran, so a build/config without the WDT (or a worker that never subscribed) is unaffected. +// Unsubscribe THIS task before it exits (esp_task_wdt_delete), so a torn-down task leaves no stale WDT +// entry the IDF would keep checking. No-op if this task never subscribed. +void taskWdtUnsubscribe() { + if (!s_wdtSubscribed) return; + if (esp_task_wdt_delete(nullptr) == ESP_OK) s_wdtSubscribed = false; +} + +// Feed THIS task's WDT subscription (esp_task_wdt_reset). No-op until/unless taskWdtSubscribe ran on this +// same task, so a task that never subscribed (or a build/config without the WDT) is unaffected. void taskWdtReset() { - if (s_renderWdtSubscribed) esp_task_wdt_reset(); + if (s_wdtSubscribed) esp_task_wdt_reset(); } } // namespace mm::platform diff --git a/src/platform/platform.h b/src/platform/platform.h index bd6ed421..a0c36a04 100644 --- a/src/platform/platform.h +++ b/src/platform/platform.h @@ -147,12 +147,18 @@ bool waitNotify(WorkerTask& t, uint32_t timeoutMs); // Signal stop + wake, then block until the worker fn has returned and the task is torn down. void stopPinnedTask(WorkerTask& t); // Subscribe THIS task to the task watchdog (esp_task_wdt_add), so a wedge on it panics-and-reboots (the -// self-heal) instead of hanging silently β€” called once by the render loop before it starts ticking. The -// sdkconfig has idle-task checking OFF (a saturated core is healthy, not a bug), so nothing is watched -// unless a task subscribes explicitly; this is that subscription. No-op on desktop. +// self-heal) instead of hanging silently. Called by each task that feeds the WDT before it starts ticking: +// the render loop, and the core-1 encode worker. The subscription is per-task (so is the feed and the flag +// behind it), which is why a task must subscribe itself rather than inherit another task's subscription. +// The sdkconfig has idle-task checking OFF (a saturated core is healthy, not a bug), so nothing is watched +// unless a task subscribes explicitly. No-op on desktop. void taskWdtSubscribe(); -// Reset THIS task's watchdog (esp_task_wdt_reset) β€” called each render tick to feed the subscription -// above. No-op on desktop, and no-op on ESP32 if the task never subscribed. +// Unsubscribe THIS task from the watchdog (esp_task_wdt_delete) before it exits, so a torn-down task (the +// encode worker when the render split disengages) leaves no dangling WDT entry. No-op on desktop, and +// no-op on ESP32 if the task never subscribed. +void taskWdtUnsubscribe(); +// Reset THIS task's watchdog (esp_task_wdt_reset) β€” called each tick to feed the subscription above. No-op +// on desktop, and no-op on ESP32 if the task never subscribed. void taskWdtReset(); // --- GPIO capability introspection (PinsModule) --------------------------------------------- diff --git a/src/ui/app.js b/src/ui/app.js index 177618d3..8bfaa7db 100644 --- a/src/ui/app.js +++ b/src/ui/app.js @@ -36,9 +36,13 @@ let state = null; let selectedModule = null; let availableTypes = []; // populated from GET /api/types after first connection let ws = null; -let wsRetryMs = 500; // exponential backoff: 500 β†’ 1000 β†’ 2000 β†’ 4000 β†’ 5000 +const WS_RETRY_MIN_MS = 200; // first reconnect is quick so a dropped initial connect (common on Safari, + // whose first attempt can lose a contended socket) comes back near-instantly +let wsRetryMs = WS_RETRY_MIN_MS; // exponential backoff: 200 β†’ 400 β†’ 800 β†’ … β†’ 5000 ceiling let wsHeartbeat = null; let wsPaused = false; // gated by document.visibilityState +let wsUnloading = false; // true once the page starts unloading (refresh/navigate) β€” suppresses the + // reconnect on the closing socket so the departing page doesn't error-log const dragTimers = {}; // per-control debounce timers (clearTimeout handles) const dragTs = {}; // per-control last-touched timestamp (ms) β€” a short post-interaction cooldown @@ -56,6 +60,7 @@ const LS_THEME = "mm_theme"; const LS_TIMING = "mm_timing_mode"; const LS_TABS = "mm_selectedTabs"; // { [containerName]: childName } β€” the open tab per container const LS_EXPANDED = "mm_expanded"; // [moduleName, …] β€” modules whose "controls"
is open +const LS_TA_SIZE = "mm_textareaSizes"; // { ":": heightPx } β€” user-dragged textarea heights // The open tab per container, persisted so a reload doesn't dump you back on the first child. let selectedTabs = {}; @@ -69,6 +74,13 @@ let expandedSet = new Set(); try { expandedSet = new Set(JSON.parse(lsRead(LS_EXPANDED, "[]")) || []); } catch { expandedSet = new Set(); } function saveExpanded() { localStorage.setItem(LS_EXPANDED, JSON.stringify([...expandedSet])); } +// The height a user dragged each textarea to β€” again VIEW-ONLY state the backend doesn't own (like the tab +// and expander state above), keyed by ":". Persisting it means a resized script/config box +// keeps its size across a full-state rebuild and a page reload instead of snapping back to the 2-row default. +let textareaSizes = {}; +try { textareaSizes = JSON.parse(lsRead(LS_TA_SIZE, "{}")) || {}; } catch { textareaSizes = {}; } +function saveTextareaSize(key, heightPx) { textareaSizes[key] = heightPx; localStorage.setItem(LS_TA_SIZE, JSON.stringify(textareaSizes)); } + function lsRead(key, defaultVal) { const v = localStorage.getItem(key); return v !== null ? v : defaultVal; @@ -91,7 +103,7 @@ function connectWs() { ws.binaryType = "arraybuffer"; ws.onopen = () => { - wsRetryMs = 500; // reset backoff + wsRetryMs = WS_RETRY_MIN_MS; // reset backoff setWsDot(true); // Keepalive ping every 25s β€” Safari kills idle WebSockets otherwise clearInterval(wsHeartbeat); @@ -132,9 +144,10 @@ function connectWs() { }; ws.onclose = () => { - setWsDot(false); clearInterval(wsHeartbeat); wsHeartbeat = null; + if (wsUnloading) return; // the page is going away β€” don't reconnect (and don't touch the DOM) + setWsDot(false); // Exponential backoff with 5s ceiling setTimeout(connectWs, wsRetryMs); wsRetryMs = Math.min(wsRetryMs * 2, 5000); @@ -157,9 +170,19 @@ window.addEventListener("pageshow", (e) => { if (e.persisted) { // Safari restored from bfcache: re-establish state wsPaused = false; + wsUnloading = false; // a bfcache-restored page is live again β€” allow reconnects if (!ws || ws.readyState !== WebSocket.OPEN) connectWs(); } }); +// Close the socket cleanly as the page unloads (a refresh, a navigation, or a bfcache suspend). Without +// this the departing page's socket is torn down abnormally by the browser, which logs a "connection lost" +// error to the console every refresh. Sending a normal (1000) close first, and marking wsUnloading so +// onclose skips its reconnect, makes the handover silent. pagehide (not beforeunload) fires for bfcache too. +window.addEventListener("pagehide", () => { + wsUnloading = true; + clearInterval(wsHeartbeat); + if (ws) { try { ws.close(1000); } catch { /* already closing */ } } +}); // --------------------------------------------------------------------------- // 3. REST helpers + module mutations @@ -169,6 +192,20 @@ async function init() { applyTheme(theme); setupStatusBarButtons(); setupUpdateBadge(); + // Open the WebSocket FIRST, before any HTTP fetch. The device pushes a full {modules} state on connect + // (handleWebSocketUpgrade β†’ requestFullResync), so the WS is the primary state source β€” the /api/state + // fetch below is only a first-paint shortcut. Connecting first matters most on Safari: it opens more + // parallel connections up front and is quicker to give up on a contended one, so if the WS is opened + // LAST (after several awaited fetches + the page's file loads) it lands in the most-saturated moment of + // the device's small socket pool and Safari abandons it β€” the "basic UI shows but the WS never goes + // live" symptom. Opening it first lets it grab an uncontended slot; Chrome tolerated the old order, so + // this fixes Safari without regressing Chrome. + connectWs(); + preview.init(); + preview.setupLayout(); + // First-paint shortcut: render from a one-shot /api/state so the cards appear immediately instead of + // waiting for the WS's first full-state push. The WS then keeps everything live. If this fetch fails + // (a contended slot), it's non-fatal β€” the WS full state fills in the moment it lands. try { const resp = await fetch("/api/state"); state = await resp.json(); @@ -180,18 +217,13 @@ async function init() { renderNav(); renderCards(); updateStatusBar(); - // /api/types arrived in plan-11; fetch in parallel. When it arrives, re-render - // so reset-to-default buttons (whose defaults come from this payload) appear. - fetch("/api/types").then(r => r.json()).then(j => { - availableTypes = j.types || []; - if (state) renderCards(); - }).catch(() => {}); - } catch (err) { - document.getElementById("main").textContent = "Error: " + err.message; - } - connectWs(); - preview.init(); - preview.setupLayout(); + } catch { /* non-fatal β€” the WS full state renders the UI when it arrives */ } + // /api/types arrived in plan-11; fetch in parallel. When it arrives, re-render so reset-to-default + // buttons (whose defaults come from this payload) appear. + fetch("/api/types").then(r => r.json()).then(j => { + availableTypes = j.types || []; + if (state) renderCards(); + }).catch(() => {}); } // The message for a failed fetch Response: the server's own `{"error": …}` body (JSON, e.g. @@ -1440,6 +1472,22 @@ function createControl(moduleName, moduleType, ctrl) { input.dataset.key = ctrl.name; input.rows = 2; // default 2 lines; CSS height + resize grip control size input.spellcheck = false; + // Restore a previously dragged height (view-state, see textareaSizes). A stored px height + // overrides the 2-row default; an untouched textarea has no entry and keeps the default. + const savedH = textareaSizes[key]; + if (typeof savedH === "number" && savedH > 0) input.style.height = savedH + "px"; + // Persist the height whenever the user drags the resize grip. A