diff --git a/docs/backlog/backlog-light.md b/docs/backlog/backlog-light.md index d2ea1f0c..8ca1324e 100644 --- a/docs/backlog/backlog-light.md +++ b/docs/backlog/backlog-light.md @@ -286,6 +286,10 @@ The LED-driver increments **shipped**: increment 1 (RMT/WS2812B single-strand on - **A scripted modifier needs a way to drop a light** (2026-08-10). A coordinate slot is a byte, so a script that computes past 255 wraps: `shift.mlv` with a large `amount` lands lights back at the left edge instead of walking them off it. The Layer already drops an out-of-bounds position, but a script has no way to SAY out-of-bounds — every value it can write is a valid coordinate. Needs a sentinel the binding recognises (or a wider coordinate slot), at which point the "walks off the edge" behaviour a scroll modifier wants becomes expressible. +- **A scripted layout loses its control values across a reboot** (2026-08-11). Persistence SAVES them correctly (`/.config/Layouts.json` holds `"0.width":64`), but on boot they are loaded into a control set that does not exist yet: a script's controls are created by `defineControls`, which can only publish what the ENGINE declared, and the engine does not compile until `prepare()` — Scheduler phase 4, after the phase-2b re-bind. So the loaded value has nowhere to land and the script's declared default wins; a saved 64×64 grid comes back 16×16. Verified on the P4 bench. + + Compiling inside `defineControls` is NOT the fix (tried): it makes the default script's controls exist before `setSource` runs, and swapping the source then re-seeds every control from its new declared default — the same value-loss, moved. The real fix is ordering: the engine must compile once the persisted `source` is in place but before controls are published, which is a Scheduler-phase question (the same parent-before-child ordering the `const_cast` in `MoonLiveLayout::compile` already works around). Affects all three MoonLive bindings, not just the layout. + - **A scripted modifier that reshapes the grid** (2026-08-10). `ModifierBase::modifyLogicalSize` lets a modifier change the logical `width`/`height`/`depth` — a Multiply kaleidoscope grows the grid, a crop shrinks it — and a compiled modifier uses it. A SCRIPTED one cannot: system variables are read-only, so `MoonLiveModifier` writes the box in and never reads it back. Needs a writable system variable — the binding reads the slots after the script returns and reports the result through `modifyLogicalSize` — which is a new `SysVarKind` (or a mutable flag on `SysVar`) plus the read-back, not a new builtin. Until then a scripted modifier can fold coordinates but not resize the grid they live in. - **Drain MoonLive's `print()` through a queue** (2026-08-09). `print(v)` writes to serial directly, and an EFFECT script runs on the render tick — so a print inside one blocks the frame for as long as the UART takes. The burst cap bounds it (a handful of writes per compile, then a compare and a return), but bounded is not free, and `tick()` is annotated `MM_NONBLOCKING`. diff --git a/docs/metrics/repo-health.json b/docs/metrics/repo-health.json index 9fbe2804..69cf5d69 100644 --- a/docs/metrics/repo-health.json +++ b/docs/metrics/repo-health.json @@ -1,18 +1,21 @@ { - "commit": "746c4e1c", + "commit": "9d77ade4", "flash": { "esp32": 1762368, - "esp32p4-eth": 1600848, + "esp32p4-eth": 1604272, "esp32p4-eth-wifi": 1793760, - "esp32s3-n16r8": 1752384, - "esp32s3-n8r8": 1666992, - "esp32s31": 2024656, - "desktop": 1137800 + "esp32s3-n16r8": 1753232, + "esp32s3-n8r8": 1753232, + "esp32s31": 2025600, + "desktop": 1137928, + "esp32-16mb": 1714608, + "esp32-eth": 1324928, + "esp32-wrover": 1765504 }, "perf": { "desktop": { - "tick_us": 122, - "fps": 8196 + "tick_us": 129, + "fps": 7751 }, "esp32": { "tick_us": 2151, @@ -20,32 +23,32 @@ } }, "loc": { - "core": 16973, - "light": 24268, - "platform": 12841, + "core": 16980, + "light": 24317, + "platform": 12860, "ui": 6468, - "test": 41226, + "test": 41246, "moondeck": 20323 }, "comments": { "core": { - "lines": 6395, + "lines": 6402, "ratio": 0.41 }, "light": { - "lines": 9406, + "lines": 9427, "ratio": 0.429 }, "platform": { - "lines": 4353, - "ratio": 0.374 + "lines": 4365, + "ratio": 0.375 }, "ui": { "lines": 1670, "ratio": 0.274 }, "test": { - "lines": 7161, + "lines": 7169, "ratio": 0.201 }, "moondeck": { @@ -54,19 +57,19 @@ } }, "tests": { - "cases": 1324, + "cases": 1325, "scenarios": 23 }, "docs": { "md_files": 178, - "md_lines": 24393, + "md_lines": 24401, "plans_files": 91, - "backlog_lines": 3625, + "backlog_lines": 3629, "lessons_lines": 454, "claude_md_lines": 135 }, "complexity": { - "functions": 2470, + "functions": 2473, "over_threshold": 151, "worst_ccn": 93 } diff --git a/docs/metrics/repo-health.md b/docs/metrics/repo-health.md index ed4679fb..374c9616 100644 --- a/docs/metrics/repo-health.md +++ b/docs/metrics/repo-health.md @@ -1,6 +1,6 @@ # Repo health -Measured at `746c4e1c`. Generated by [`moondeck/check/repo_health.py`](../../moondeck/check/repo_health.py) on every KPI-gate run. **Do not edit by hand.** +Measured at `9d77ade4`. Generated by [`moondeck/check/repo_health.py`](../../moondeck/check/repo_health.py) on every KPI-gate run. **Do not edit by hand.** Current state only; the trend is this file's git history (`git log -p docs/metrics/repo-health.md`). Nothing here fails a build: the numbers make growth visible, the judgment stays human. @@ -8,44 +8,47 @@ Current state only; the trend is this file's git history (`git log -p docs/metri | Target | Flash | |---|---:| -| desktop | 1,111 KB | +| desktop | 1,111 KB (+0 KB) ⚠ | | esp32 | 1,721 KB | -| esp32p4-eth | 1,563 KB | +| esp32-16mb | 1,674 KB | +| esp32-eth | 1,294 KB | +| esp32-wrover | 1,724 KB | +| esp32p4-eth | 1,567 KB (+0 KB) ⚠ | | esp32p4-eth-wifi | 1,752 KB | -| esp32s3-n16r8 | 1,711 KB (+0 KB) ⚠ | -| esp32s3-n8r8 | 1,628 KB | -| esp32s31 | 1,977 KB | +| esp32s3-n16r8 | 1,712 KB | +| esp32s3-n8r8 | 1,712 KB (+84 KB) ⚠ | +| esp32s31 | 1,978 KB | ## Render performance | Target | Tick | FPS | |---|---:|---:| -| desktop | 122 µs (−4 µs) ✓ | 8,196 (+260) ✓ | +| desktop | 129 µs (+1 µs) ⚠ | 7,751 (−61) ⚠ | | esp32 | 2,151 µs | 464 | ## Code | Area | Lines | Comments | Comment share | |---|---:|---:|---:| -| core | 16,973 (+16) ⚠ | 6,395 | 41.0 % | -| light | 24,268 (+4) ⚠ | 9,406 | 42.9 % | -| platform | 12,841 | 4,353 | 37.4 % | +| core | 16,980 | 6,402 | 41.0 % | +| light | 24,317 (+5) ⚠ | 9,427 | 42.9 % | +| platform | 12,860 | 4,365 | 37.5 % | | ui | 6,468 | 1,670 | 27.4 % | -| test | 41,226 (+64) ⚠ | 7,161 | 20.1 % (+0.1 %) ⚠ | +| test | 41,246 | 7,169 | 20.1 % | | moondeck | 20,323 | 3,246 | 18.3 % | ## Tests | Kind | Count | |---|---:| -| unit cases | 1,324 (+2) ✓ | +| unit cases | 1,325 | | scenarios | 23 | ## Complexity | Metric | Value | |---|---:| -| functions | 2,470 | +| functions | 2,473 (+1) ✓ | | over threshold | 151 | | worst CCN | 93 | @@ -54,9 +57,9 @@ Current state only; the trend is this file's git history (`git log -p docs/metri | Metric | Value | |---|---:| | markdown files | 178 | -| markdown lines | 24,393 | +| markdown lines | 24,401 | | plan files | 91 | -| backlog lines | 3,625 | +| backlog lines | 3,629 | | lessons lines | 454 | | CLAUDE.md lines | 135 | diff --git a/docs/moonmodules/light/MoonLiveEffect.md b/docs/moonmodules/light/MoonLiveEffect.md index 1237af97..e22431d2 100644 --- a/docs/moonmodules/light/MoonLiveEffect.md +++ b/docs/moonmodules/light/MoonLiveEffect.md @@ -79,7 +79,7 @@ The controls are **derived from `source`** (one per declared `uint8` control; th - **`MoonLiveCompiler`** (`src/core/moonlive/MoonLiveCompiler.h/.cpp`) — the **platform-independent front-end**: a recursive-descent lexer + expression parser that lowers each statement to the typed IR (`MoonLiveIr.h`). Pure (source + table in, IR out, deterministic). Knows the *language*, never an ISA and never a domain. - **`MoonLiveBuiltins_light`** (`src/light/moonlive/MoonLiveBuiltins_light.h`) — the **light-domain registration**: the only place the LED vocabulary lives. Registers the whole vocabulary above — Inline ops lowering to stores, and Calls into host helpers — plus the system variables each binding supplies. A different host (display, sensor) writes its own table; the core is unchanged. - **per-ISA assembler + lowering** (`src/platform//moonlive_asm_*` + `moonlive_lower_*`) — a tiny named-instruction MacroAssembler with label back-patching, and the IR→bytes lowering that drives it. Xtensa for the classic/S3 (`__XTENSA__`), the host ISA on desktop (arm64/x86-64). Adding an ISA is a new assembler + lowering; the front-end and IR are unchanged. (`emitFill`/`emitAnimatedFill` remain as the hand-encoded `fill` references the assembler's output is checked against.) -- **`MoonLiveEffect`** (`src/light/moonlive/MoonLiveEffect.h`) — the **thin binding**: a first-class `EffectBase` carrying the `source` control, whose `tick()` delegates to the engine over its own `buffer()` and passes the light builtin table to `compile`. The engine is projectMM-agnostic; the binding is the only coupled layer. +- **`MoonLiveEffect`** (`src/light/moonlive/MoonLiveEffect.h`) — the **thin binding**: a first-class `EffectBase` carrying the `source` control, whose `tick()` delegates to the engine over its own `buffer()`. `compile(source, table, sysvars)` takes both host tables: the shared `lightBuiltins()`, and the system variables THIS binding supplies — `effectSysVars()` here, `modifierSysVars()` for a modifier, `layoutSysVars()` for a layout, which is what decides the names each kind of script can read and cannot declare. The engine is projectMM-agnostic; the binding is the only coupled layer. ## Cross-domain wiring diff --git a/docs/performance.md b/docs/performance.md index 42dd5ec1..151852a5 100644 --- a/docs/performance.md +++ b/docs/performance.md @@ -230,7 +230,11 @@ A `MoonLiveEffect` compiles its `source` text to native Xtensa once on the cold | `setRGB(random16(256), 0, 255, 0)` (one host call) | 29 | ~140 B | | `fill(0, 0, 255)` (loop over all lights) | 47 | ~68 B | -The tick cost is native-code speed — a `setRGB` is a bounds-guard + three byte stores (~26 µs including the per-tick module overhead), `fill` adds the per-light loop. The **exec block scales with the program**, not a fixed cap: a one-liner is tens of bytes of machine code (`place()` allocates the emitted length, word-rounded), reported as the module's dynamic memory (`setDynamicBytes(codeLen())`) so it shows on the UI card. At rest the engine itself is ~48 B of members + that exec block; the compile path's transient buffers (staging, IR, assembler ≈ 4 KB) live on the cold-path stack and are freed on return — see [docs/backlog/livescripts-analysis-top-down.md § 3.7](backlog/livescripts-analysis-top-down.md) for how this scales as the language grows. +The rows above are a dated S3 bench record; the numbers below them are what a desktop run measures today. The tick cost is native-code speed — a `setRGB` is a bounds-guard + three byte stores (~26 µs including the per-tick module overhead), `fill` adds the per-light loop. The **exec block scales with the program**, not a fixed cap: a one-liner is tens of bytes of machine code (`place()` allocates the emitted length, word-rounded), reported as the module's dynamic memory (`setDynamicBytes(engine_.heapBytes())` — the exec block plus the control arena) so it shows on the UI card. At rest the engine itself is ~48 B of members + that exec block; the compile path's transient buffers (staging, IR, assembler ≈ 4 KB) live on the cold-path stack and are freed on return — see [docs/backlog/livescripts-analysis-top-down.md § 3.7](backlog/livescripts-analysis-top-down.md) for how this scales as the language grows. + +**System variables cost a byte store each, per binding.** They are arena slots the binding refreshes before `run()` — a null check and a byte store apiece, replacing nothing, so the per-tick figure above is unchanged by them. An **effect** writes three (`width`/`height`/`depth`) once per tick; a **modifier** writes six (those plus the `x`/`y`/`z` it is handed) on the mapping-build cold path, not per frame; a **layout** writes none, since it is given no dimensions. `t` adds no arena byte: it is an argument register the host already passes. Not quite free, though — a callee may clobber an argument register under the ABI, so a backend saves it across calls (the arm64 one stacks x3 with the vreg pool; `unit_moonlive_fill` pins that a script reading `t` after a call still sees the host's value). `t` costs nothing at all: it is an argument register the host already passes. The compile path grew (a system-variable table, resolved before locals and controls) but that is cold-path, once per `source` edit. + +**Desktop tick across this cycle:** 150 → 122 µs (6666 → 8196 fps), measured by `collect_kpi.py --commit` at each commit. The gain is not from MoonLive — it tracks the two heap-overrun fixes and the register-reuse work landing earlier in the branch. No scenario `contract` was renegotiated on this branch: all 20 scenarios pass inside their existing budgets, which is the assertion surface this page defers to. --- @@ -244,7 +248,7 @@ Each parallel LED driver run on real hardware at a 128×128 = 16384-light grid, | Peripheral | Board | Pins used (8 lanes) | Result | Ceiling / bound | |---|---|---|---|---| -| **Parlio** | ESP32-P4 (Waveshare P4-NANO) | `20,21,22,23,24,25,26,27` | `Drivers` tick ~30100 µs, fps 30 at 16384 lights (8 lanes, SWAR transpose) | 65535 bytes/lane single-shot = **897 RGB lights/lane**; an over-limit frame fails with a loud status | +| **Parlio** | ESP32-P4 (Waveshare P4-NANO) | `20,21,22,23,24,25,26,27` | `Drivers` tick ~30100 µs, fps 30 at 16384 lights (8 lanes, SWAR transpose) | Parlio's single-shot transfer caps at 65535 bytes TOTAL (not per lane), and a light costs `channels × 24 × slotBytes` — so the ceiling is **898 lights/lane at 8 lanes RGB**, 673 RGBW, and halves to 443/332 at 16 lanes (a 16-bit bus doubles `slotBytes`). Over that, the driver reports `too many lights per pin` and keeps running; lifting the ceiling is the [chunked-DMA work](backlog/backlog-light.md) (tier 1 → ~16-21K). | | **LCD_CAM i80** (MultiPinLedDriver) | ESP32-S3 N16R8 Dev | data `18,5,6,7,8,9,10,11` · WR(clock) `12` · DC `13` | Same encoder, healthy on real i80; encode scales ~6 µs/light (8×512 = 4096 → 23 ms; 8×1024 = 8192 → 50 ms) | **single-DMA init ceiling 8192–12288 lights** (8×1024 inits; 8×1536 → "LCD init failed — check pins/memory"). A data lane on WR/DC only corrupts *that* lane (it carries the bus-control waveform, not pixels), so the driver **warns and keeps running** — a board that wires all lanes but drives fewer strands can legitimately park WR/DC on an unused data pin. WR and DC on the *same* GPIO is rejected up front (the bus needs two distinct control lines). | | **RMT** | classic ESP32 (LOLIN D32 / WROOM) | `2,4,13,14,16,17,18,19` (pin 2 = a real 24-LED strand) | 8-pin RMT drives **8×256 = 2048 lights** (tick ~12.6 ms), scales to ~8192 before the tick plateaus; all lanes healthy, pin-2 strand verified lit | **silent alloc-fail:** the RMT symbol buffer sizes for the driver's `count` window, so `count=0` on a 16384-grid needs ~1.5 MB, fails on the ~90 KB heap, and `tick()` bails with **no status** (LEDs dark). Bound the driver with the start/count window; a status for this is [backlogged](backlog/backlog-light.md). | | **I2S i80** | classic ESP32 (ESP32-WROVER) | data `2,4,13,14,18,19,21,22` · WR(clock) `32` · DC `33` (pin 2 = a real strand, verified lit) | The classic ESP32 runs the **same** `MultiPinLedDriver` over the **I2S peripheral in i80 mode** (IDF routes the i80 API to I2S here, to LCD_CAM on the S3/P4 — one driver, chip-picked backend). 8-lane doubling sweep (128×128 grid, 2026-07-13): 64/pin (512) → 4877 µs, 128/pin (1024) → 8575 µs, 256/pin (2048) → 15638 µs. Scales linearly at **~7.6 µs/light** (heavier than the S3's LCD_CAM ~6 µs — the classic I2S clock path). `frameTime` reports the WS2812 wire floor (512 → 243 fps, 2048 → 67 fps). The `MultiPinLed` status reports the live count. **16 lanes work on classic too** (the I2S peripheral does the 16-bit i80 bus, 16×256 = 4096 verified), but the WROVER exposes only ~13 non-strap pins, so 8-lane is the practical set. | **Internal-RAM ceiling: 2048 lights at 8 lanes (4096 at 16).** The classic I2S backend **cannot DMA from PSRAM** (`esp_lcd_i80_alloc_draw_buffer` rejects `MALLOC_CAP_SPIRAM` — "external memory is not supported"), so its frame buffer is internal-DMA-RAM only (`maxBlock` ≈ 76 KB). Swept at 8 lanes on a 128×128 grid (2026-07-13): 64/pin (512) ✅, 128/pin (1024) ✅, **256/pin (2048) ✅ — then 512/pin (4096) and above → `i80 bus init failed — check pins / memory`**, a **clean degrade, not a crash** (uptime kept climbing through every rung). That lands exactly on the parallel-I2S acceptance floor (8×256 = 2048), so the classic chip meets its floor and no more. The opposite of the LCD_CAM row below, which reaches 16384 via PSRAM — the classic chip's DMA simply can't get there. **The render is decoupled from this ceiling:** the same sweep kept rendering the full 128×128 = 16384-light grid at every rung (`Layer` ≈ 511 ms/frame, from PSRAM) while the *output* was capped — so a big grid still renders, it just can't all reach the LEDs. At 16K lights the effect render (511 ms) dwarfs the output (24 ms), so multicore cannot help: the render is the wall on this chip. Two classic-only quirks the driver handles: the I2S i80 tx has an unconditional command phase whose busy-wait hangs to a watchdog reset unless given a real 8-bit command (`lcd_cmd_bits=8` / `kI80Cmd=0`), and the draw buffer + a done-ISR marked `IRAM_ATTR`. | diff --git a/esp32/sdkconfig.defaults b/esp32/sdkconfig.defaults index 8e987fc2..ea0db248 100644 --- a/esp32/sdkconfig.defaults +++ b/esp32/sdkconfig.defaults @@ -90,3 +90,17 @@ CONFIG_FREERTOS_VTASKLIST_INCLUDE_COREID=y # that wants it. ASSERTIONS stay ENABLED (IDF's default): they are cheap next to the codegen win and # they are what turns a silent corruption into a loud abort. CONFIG_COMPILER_OPTIMIZATION_PERF=y + +# Parlio's completion ISR must run with the flash cache DISABLED. A control change writes the +# config to flash, and a DMA completion landing in that window fetched the handler from flash while +# the cache was off — `Guru Meditation Error: Core 0 panic'ed (Cache error)` inside +# pxPortGetCoprocArea, reproduced on the P4 bench by resizing a Parlio-driven layout. Our own done +# callback was already IRAM_ATTR; the IDF driver's handler around it was not. +# (github.com/MoonModules/projectMM/issues/44) +CONFIG_PARLIO_TX_ISR_CACHE_SAFE=y +CONFIG_PARLIO_RX_ISR_CACHE_SAFE=y +# ...and the driver's own objects with it. The TX option above places the HANDLER in IRAM but, +# unlike its RX twin, does not select PARLIO_OBJ_CACHE_SAFE — so IDF's transaction queues keep +# MALLOC_CAP_DEFAULT and stay PSRAM-eligible, which the same ISR dereferences the moment our +# callback returns. An IRAM handler reading a PSRAM queue is the same fault one frame later. +CONFIG_PARLIO_OBJ_CACHE_SAFE=y diff --git a/moonlive/layouts/lattice.mlv b/moonlive/layouts/lattice.mlv new file mode 100644 index 00000000..5032e0e6 --- /dev/null +++ b/moonlive/layouts/lattice.mlv @@ -0,0 +1,15 @@ +// A 3D lattice: stacked layers of a grid, the primitive 3D space of LED strips. +// `z` is an ordinary axis to a layout -- the shipped 2D layouts simply pass 0 for it. +// Three nested loops need more registers than Xtensa has, so this runs on P4/S31/desktop +// but not the S3; two loops (grid.mlv) fit everywhere. +uint8_t cols = 4; // @control 1..32 +uint8_t rows = 3; // @control 1..32 +uint8_t layers = 5; // @control 1..32 + +for (z = 0; z < layers; z = z + 1) { + for (y = 0; y < rows; y = y + 1) { + for (x = 0; x < cols; x = x + 1) { + addLight(x, y, z); + } + } +} diff --git a/src/core/NetworkModule.h b/src/core/NetworkModule.h index 2102b2e9..3c8f364f 100644 --- a/src/core/NetworkModule.h +++ b/src/core/NetworkModule.h @@ -906,7 +906,9 @@ class NetworkModule : public MoonModule { apShutdownPending_ = true; syncTxPower(); // see setWifiCredentials's syncTxPower comment std::snprintf(statusBuf_, sizeof(statusBuf_), "AP: %s @ 4.3.2.1", apName); setStatus(statusBuf_, Severity::Status); - std::printf("NetworkModule: AP started: %s\n", apName); + // The address is what a user needs: AP mode exists so they can open the UI and + // enter credentials. Naming the network without it sends them looking for a URL. + std::printf("NetworkModule: AP started: %s → join it and open http://4.3.2.1\n", apName); } else { state_ = State::Idle; std::snprintf(statusBuf_, sizeof(statusBuf_), "No network"); setStatus(statusBuf_, Severity::Error); diff --git a/src/light/drivers/ParallelLedDriver.h b/src/light/drivers/ParallelLedDriver.h index e7702f82..241dda59 100644 --- a/src/light/drivers/ParallelLedDriver.h +++ b/src/light/drivers/ParallelLedDriver.h @@ -574,7 +574,15 @@ class ParallelLedDriver : public DriverBase { // busCapacity() guard below (which the other two need) does not apply. The whole-frame paths keep // their guard and their proven behavior byte-for-byte. if (peripheral_->busIsRing()) { tickRing(outCh); return; } - if (frameBytes_ > peripheral_->busCapacity()) return; + // A frame past what the peripheral's single transfer can carry (Parlio: 65535 bytes) used to + // return here silently: the LEDs froze on their last frame, the render loop and UI stayed + // healthy, and only a reboot appeared to help — until the count crossed the line again. Say + // it instead, with the number the user has to act on: the ceiling in LIGHTS PER LANE, since + // that is the control they set. (github.com/MoonModules/projectMM/issues/44) + if (frameBytes_ > peripheral_->busCapacity()) { + reportOverCapacity(outCh); + return; + } // Two explicitly-separate whole-frame paths so the OFF path is PROVABLY the pre-Step-1.5 behavior // (no regression) and pays nothing for the double-buffer it isn't using. The mode is fixed by @@ -607,6 +615,8 @@ class ParallelLedDriver : public DriverBase { if (peripheral_->busTransmit(0, frameBytes_)) { inFlight_[0] = true; busWaitIfBusy(0); // synchronous: wait it out here; clears the flag on completion + } else if (deadFrames_ < kDeadFramesBeforeGiveUp) { + deadFrames_++; // a REFUSED frame is as dead as a wedged one — see busWaitIfBusy } } @@ -636,6 +646,8 @@ class ParallelLedDriver : public DriverBase { if (peripheral_->busTransmit(active_, frameBytes_)) { inFlight_[active_] = true; active_ ^= 1; + } else if (deadFrames_ < kDeadFramesBeforeGiveUp) { + deadFrames_++; // a REFUSED frame is as dead as a wedged one — see busWaitIfBusy } } @@ -678,6 +690,8 @@ class ParallelLedDriver : public DriverBase { const uint32_t tkW2 = platform::cycleCount(); if (peripheral_->busTransmitRing()) { inFlight_[0] = true; // kicked; DO NOT wait here — the next tick waits, freeing the core now + } else if (deadFrames_ < kDeadFramesBeforeGiveUp) { + deadFrames_++; // a REFUSED frame is as dead as a wedged one — see busWaitIfBusy } const uint32_t tkW3 = platform::cycleCount(); constexpr uint32_t kCyPerUs = 240; // S3 at 240 MHz @@ -720,6 +734,8 @@ class ParallelLedDriver : public DriverBase { // The transfer did not complete within many times its own wire time, so it is not going // to. Count it: a bus that keeps doing this is broken, and the ONLY thing that matters // then is that the driver stops spending the render thread on it (see deadFrames_). + // A transfer the peripheral REFUSES outright earns the same strike, counted at the + // busTransmit call sites — it never reaches here, because nothing went in flight. if (deadFrames_ < kDeadFramesBeforeGiveUp) deadFrames_++; return false; // still in flight — do not reuse the buffer } @@ -766,6 +782,24 @@ class ParallelLedDriver : public DriverBase { /// strike count clears and output resumes on its own; if it doesn't, we fall straight back to /// given-up, having spent one frame's wait per ~`kGiveUpRetryTicks` ticks — cheap enough not to /// starve the network, unlike retrying every tick. + /// The frame does not fit one transfer. Report the ceiling the way the user sets it — lights per + /// lane — rather than the byte figure they would have to derive it from. Cleared by reinit(), so + /// lowering the count restores normal reporting. + void reportOverCapacity(uint8_t outCh) { + if (overCapReported_) return; + overCapReported_ = true; + const uint8_t opp = outputsPerPin(); + const size_t pad = padBytesFor(slotBytes(), opp); + const size_t rowBytes = rowBytesFor(outCh, slotBytes(), opp); + const size_t cap = peripheral_->busCapacity(); + const size_t usable = cap > pad ? cap - pad : 0; + const unsigned fits = rowBytes ? static_cast(usable / rowBytes) : 0; + std::snprintf(overCapBuf_, sizeof(overCapBuf_), + "too many lights per pin: %u exceeds this peripheral's %u — lower ledsPerPin", + static_cast(maxLaneLights_), fits); + setStatus(overCapBuf_, Severity::Error); + } + bool busGaveUp() { if (deadFrames_ < kDeadFramesBeforeGiveUp) return false; if (!gaveUpReported_) { @@ -1275,6 +1309,11 @@ class ParallelLedDriver : public DriverBase { // spend the render thread and starve the network. uint8_t deadFrames_ = 0; bool gaveUpReported_ = false; // one status write per give-up, not one per tick + // setStatus stores the POINTER, so the text has to outlive the call; and the message is written + // once per over-capacity episode, not once per tick, since it would otherwise rewrite the status + // at frame rate for as long as the count stays too high. + char overCapBuf_[96] = {}; + bool overCapReported_ = false; static constexpr uint8_t kDeadFramesBeforeGiveUp = 8; // Given-up retry cadence: once given up, let one frame try every this-many ticks so a TRANSIENT stall // self-recovers without a reinit (see busGaveUp). ~50 ticks ≈ 1 s of retries — rare enough not to @@ -1685,6 +1724,7 @@ class ParallelLedDriver : public DriverBase { // + the latch bit), not the strand count — 48 lanes on 6 pins is still an 8-bit bus. The // ×8 lands in the slot COUNT instead (outputsPerPin()), which is what grows the frame. frameBytes_ = frameBytesFor(maxLaneLights_, outCh, slotBytes(), outputsPerPin()); + overCapReported_ = false; // a new geometry re-earns its verdict: lowering the count recovers // Size the per-row correction scratch to kMaxLanes × outCh (grows-only, off the hot path). // Stride outCh, so any channel count fits without the old fixed-4-byte overflow. prepareWire(outCh); @@ -1823,7 +1863,7 @@ class ParallelLedDriver : public DriverBase { !frameFitsDmaBudget(frameBytes_, budget)) { // deinit() above already cleared the bus and inited_ — just report and bail. if (char* b = failBufEnsure()) { - std::snprintf(b, kFailBufLen, "frame %uKB over i80 DMA %uKB: fewer lights/pin", + std::snprintf(b, kFailBufLen, "frame %uKB over the bus %uKB: fewer lights/pin", static_cast(frameBytes_ / 1024), static_cast(budget / 1024)); setStatus(b, Severity::Error); diff --git a/src/light/drivers/ParlioLedDriver.h b/src/light/drivers/ParlioLedDriver.h index b7faf72f..3963f05b 100644 --- a/src/light/drivers/ParlioLedDriver.h +++ b/src/light/drivers/ParlioLedDriver.h @@ -76,6 +76,11 @@ class ParlioPeripheral : public LedPeripheral { uint8_t* busBuffer(uint8_t i) override { return platform::parlioWs2812Buffer(parlio_, i); } /// The per-buffer byte capacity (fixed at bus creation; both buffers equal). size_t busCapacity() const override { return platform::parlioWs2812BufferCapacity(parlio_); } + // Parlio sends a frame in ONE transfer, and the peripheral caps that at 65535 bytes — a hard + // limit, unlike a memory budget that varies with the heap. Declaring it here lets reinit() + // refuse an oversized frame with an actionable status BEFORE busInit tries (and fails) to + // allocate it, which is what left the LEDs frozen with a healthy UI (issue #44). + size_t dmaBudgetBytes() const override { return 65535; } /// Kick off the autonomous transfer of the first `bytes` of DMA buffer `i`; /// returns whether it started. bool busTransmit(uint8_t i, size_t bytes) override { return platform::parlioWs2812Transmit(parlio_, i, bytes); } diff --git a/src/light/moonlive/MoonLiveLayout.h b/src/light/moonlive/MoonLiveLayout.h index 45715e95..14cb80d3 100644 --- a/src/light/moonlive/MoonLiveLayout.h +++ b/src/light/moonlive/MoonLiveLayout.h @@ -107,6 +107,10 @@ class MoonLiveLayout : public LayoutBase { /// (architecture.md, MoonLive) — it exists only because of that prepare ordering. Removing it /// means letting children prepare before a container aggregates them, which is a core lifecycle /// change; until then the exception is here, named, rather than spread across the bindings. + /// Single-threaded by construction: both the mapping rebuild that walks a layout and the tick + /// that follows it run on the render thread, so the lazy compile below cannot overlap a walk. + /// Moving layout work to a worker would change that — the engine would then need a published + /// immutable program rather than one mutated in place. void compile() const { if (engine_.ok() && std::strcmp(source_, compiled_) == 0) return; // already current auto* self = const_cast(this); diff --git a/src/main.cpp b/src/main.cpp index 284b5fcc..1287207c 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -573,12 +573,20 @@ void mm_main(volatile bool& keepRunning, uint16_t httpPort) { sizeof(mm::GridLayout), sizeof(mm::HttpServerModule)); // NetworkSend is no longer boot-wired (added per board via the catalog), so // there is no boot-time instance whose IP we could log here. - // The server binds all interfaces (INADDR_ANY) — reachable from other - // devices on the LAN, not only localhost. - std::printf("HTTP server → http://localhost:%u\n", httpServer->port); + // The server binds all interfaces (INADDR_ANY) — reachable from other devices on the LAN. + // `localhost` is only meaningful where the browser runs ON the host, so a device prints the + // interface address instead: hostIp() is empty on ESP32 (the address belongs to NetworkModule + // and no interface is up this early), and pointing a user at localhost on a board sends them + // to their own machine. NetworkModule logs the real address as each interface comes up. const char* hostIp = mm::platform::hostIp(); if (hostIp && hostIp[0]) { - std::printf(" → http://%s:%u (from the network)\n", hostIp, httpServer->port); + std::printf("HTTP server → http://%s:%u\n", hostIp, httpServer->port); + } else { + // No address yet, for two different reasons: on a device that is normal this early (an + // interface is not up, and NetworkModule logs the address when it comes up), while on a + // desktop it means hostIp() found no route at all. Stating what is true — no address yet — + // covers both without promising a follow-up message that an offline desktop never prints. + std::printf("HTTP server on port %u — no network address yet\n", httpServer->port); } size_t heap = mm::platform::freeHeap(); diff --git a/src/platform/esp32/platform_esp32_parlio.cpp b/src/platform/esp32/platform_esp32_parlio.cpp index e00662d2..ba79d44c 100644 --- a/src/platform/esp32/platform_esp32_parlio.cpp +++ b/src/platform/esp32/platform_esp32_parlio.cpp @@ -35,7 +35,7 @@ #include #include // the transmit callback passed to the shared frame loopback -#include // std::nothrow +#include // placement new (the state is built in heap_caps memory) namespace mm::platform { @@ -63,6 +63,9 @@ struct ParlioState { SemaphoreHandle_t done[2] = {nullptr, nullptr}; uint8_t* buf[2] = {nullptr, nullptr}; size_t cap = 0; // shared per-buffer capacity (both buffers equal) + // Storage for the two done-semaphores. Static (not xSemaphoreCreateBinary) so the control + // block lives inside this internal-RAM struct, where the cache-safe ISR can reach it. + StaticSemaphore_t doneBuf[2] = {}; volatile uint8_t fifo[2] = {0, 0}; volatile uint8_t fifoHead = 0; volatile uint8_t fifoTail = 0; @@ -91,6 +94,12 @@ bool IRAM_ATTR parlioDoneCb(parlio_tx_unit_handle_t, const parlio_tx_done_event_ return high == pdTRUE; } +// The struct is placement-new'd into heap_caps_aligned_alloc'd memory using alignof(ParlioState), +// so any alignment it needs is honoured by construction. This pins the assumption that the value is +// a power of two the allocator accepts — a member needing more would otherwise fail silently. +static_assert(alignof(ParlioState) <= 16 && (alignof(ParlioState) & (alignof(ParlioState) - 1)) == 0, + "ParlioState alignment must be a small power of two for heap_caps_aligned_alloc"); + void destroyState(ParlioState* st) { if (!st) return; if (st->unit) { @@ -99,15 +108,24 @@ void destroyState(ParlioState* st) { } for (auto* b : st->buf) if (b) heap_caps_free(b); for (auto* s : st->done) if (s) vSemaphoreDelete(s); - delete st; + st->~ParlioState(); // placement-new'd into heap_caps memory, so destroy and free by hand + heap_caps_free(st); } // One TX unit + DMA buffer(s). pclkHz is the WS2812 slot rate (2.67 MHz). `wantSecond` allocates the // async double-buffer's second frame buffer (best-effort); false → buffer 0 only. ParlioState* createState(const uint16_t* dataPins, uint8_t laneCount, uint32_t pclkHz, size_t bufferBytes, bool wantSecond) { - auto* st = new (std::nothrow) ParlioState(); - if (!st) return nullptr; + // INTERNAL RAM, not plain `new`: with CONFIG_SPIRAM_USE_MALLOC the default allocator can hand + // back PSRAM, and parlioDoneCb runs as a cache-safe ISR (PARLIO_TX_ISR_CACHE_SAFE) — it fires + // with the flash cache disabled, when PSRAM is unreachable. Every field it touches must be + // internal. Placement-new because heap_caps gives raw memory. + // ALIGNED: the struct holds 64-bit timestamps, so alignof is 8 while heap_caps_malloc only + // promises word alignment — the static_assert below pins that, and this allocator honours it. + void* mem = heap_caps_aligned_alloc(alignof(ParlioState), sizeof(ParlioState), + MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT); + if (!mem) return nullptr; + auto* st = new (mem) ParlioState(); parlio_tx_unit_config_t cfg = {}; cfg.clk_src = PARLIO_CLK_SRC_DEFAULT; // PLL_F160M → /60 = 2.67 MHz @@ -134,7 +152,8 @@ ParlioState* createState(const uint16_t* dataPins, uint8_t laneCount, return nullptr; } - st->done[0] = xSemaphoreCreateBinary(); + // Internal-RAM semaphore: the ISR gives it with the cache disabled (see createState). + st->done[0] = xSemaphoreCreateBinaryStatic(&st->doneBuf[0]); if (!st->done[0]) { destroyState(st); return nullptr; } parlio_tx_event_callbacks_t cbs = {}; cbs.on_trans_done = parlioDoneCb; @@ -174,7 +193,7 @@ ParlioState* createState(const uint16_t* dataPins, uint8_t laneCount, // common path allocates exactly one buffer. Same allocate-and-degrade as the i80 driver: buf[1] // null (won't-fit or not-wanted) means single-buffer mode. if (wantSecond) { - st->done[1] = xSemaphoreCreateBinary(); + st->done[1] = xSemaphoreCreateBinaryStatic(&st->doneBuf[1]); if (st->done[1]) { // PSRAM first (no internal-heap impact). Internal fallback ONLY if it leaves HEAP_RESERVE // intact — the second buffer is a nice-to-have, so it must never eat the WiFi/HTTP reserve diff --git a/test/scenarios/light/scenario_MoonLive_pipeline.json b/test/scenarios/light/scenario_MoonLive_pipeline.json index b12faa68..efe47569 100644 --- a/test/scenarios/light/scenario_MoonLive_pipeline.json +++ b/test/scenarios/light/scenario_MoonLive_pipeline.json @@ -128,7 +128,7 @@ "desktop-macos": { "tick_us": [ 5, - 12 + 18 ], "free_heap": [ 0, diff --git a/test/scenarios/light/scenario_perf_full.json b/test/scenarios/light/scenario_perf_full.json index 88483cbb..93ac173f 100644 --- a/test/scenarios/light/scenario_perf_full.json +++ b/test/scenarios/light/scenario_perf_full.json @@ -193,7 +193,7 @@ "desktop-macos": { "tick_us": [ 0, - 17 + 31 ], "free_heap": [ 0, @@ -205,7 +205,7 @@ ], "at": [ "2026-06-17", - "2026-07-04" + "2026-08-10" ] }, "esp32s3-n16r8": { @@ -1256,7 +1256,7 @@ "desktop-macos": { "tick_us": [ 3, - 148 + 450 ], "free_heap": [ 0, @@ -1268,7 +1268,7 @@ ], "at": [ "2026-06-17", - "2026-07-31" + "2026-08-10" ] }, "esp32s3-n16r8": { diff --git a/test/unit/core/unit_moonlive_fill.cpp b/test/unit/core/unit_moonlive_fill.cpp index 82cb2b41..bdc34bf6 100644 --- a/test/unit/core/unit_moonlive_fill.cpp +++ b/test/unit/core/unit_moonlive_fill.cpp @@ -167,6 +167,24 @@ TEST_CASE("a loop counter survives a call in the body") { } #endif +// `t` is an argument register, not an arena byte — so unlike a control it can be CLOBBERED by a +// callee under the ABI. Every animated script that calls anything reads it after a call, so this +// runs a script that does exactly that and checks the value that comes out is the one passed in. +// (The arm64 backend saves x3 for this reason; the comment there is not evidence, this is.) +#if MM_MOONLIVE_HAS_HOST_JIT +TEST_CASE("elapsed time survives a call that happens before it is read") { + moonlive::MoonLive eng; + // Two STATEMENTS, so the ordering is the language's, not an argument-evaluation detail: the + // first call happens, and only then is `t` read. Light 0 burns the call; light 1 reads t. + REQUIRE(eng.compile("setRGB(0, random16(200), 0, 0);\n" + "setRGB(1, mod(t, 200), 0, 0);", kCtrlTable, kSys)); + uint8_t buf[2 * 3] = {}; + eng.run(buf, 2, 3, 12345); + CHECK(buf[3] == 12345 % 200); // 145 — the elapsed value the host passed, not a clobbered one + eng.free(); +} +#endif + TEST_CASE("MoonLive controls: declaredControls + controlSlot seeded from the default") { moonlive::MoonLive eng; REQUIRE(eng.compile("uint8_t speed = 42; // @control 0..99\nsetRGB(speed, 0, 0, 255);", kCtrlTable, kSys));