From 234e01ec3dc4561240ff364c2755ef8646b2f866 Mon Sep 17 00:00:00 2001 From: ewowi Date: Wed, 12 Aug 2026 10:15:12 +0200 Subject: [PATCH 1/3] Keep MoonLive scripts on the filesystem, not in every module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A scripted module carried its script as a fixed 1 KB array, plus a second copy to notice edits — resident whether or not a script was loaded, so six modules held ~16 KB of a classic ESP32's 320 KB for text that was mostly empty. The script now lives in a file; the module holds its name, reads it into a right-sized buffer to compile, and frees it. Scripts are bounded by the filesystem instead of by an array nobody can grow. Performance: desktop 132 us/tick (7575 fps), esp32 2151 us/tick (464 fps). Light domain - A `script` control (~32 B) replaces the `source` textarea in all three bindings. The UI loads, edits and saves the file through the /api/file endpoints that already existed, so this needed no new backend surface. A fresh module reports "no script — set the script name" and renders nothing, rather than every new module compiling the same default. - The rebuild check is a 4-byte FNV-1a of the script text, not a second copy of it. It only ever answered "did this change". - Per-binding control-name pools are gone: the engine owns the names it publishes now, so three private copies of the same fact went with them. - /moonlive/ is created on demand — the write endpoint does not make parent directories, so a first save on a fresh device failed with nowhere obvious to look. Core - The engine copies declared control NAMES out of the source before returning. They pointed into the source text, which the caller is now free to release the moment compile() ends — and does. A control briefly appeared named "\x05" before this was found. - IrProgram's op array is heap-allocated and sized from a token count, RAII-owned (destructor frees, copy deleted). It was a ~2 KB stack member on a 12 KB main task, the same cost for a one-statement script as a full one — so growing it would have traded a compile limit for a stack overflow. SEVEN sequential statements used to fail; forty compile. kMaxIrOps 64 → 4096 is now a sanity bound, not the working limit. - Widening that count to uint16_t left four uint8_t loop counters iterating over it — three lowerers and IrProgram::hasInline — which wrapped at 256 ops and spun forever. On a device that is a watchdog reset from a script that merely got long. Bisected (60 statements fine, 80 hung); the regression test HANGS when the fix is reverted, which is how it was checked. - ParlioLedDriver asks the platform for its 65535-byte transfer cap rather than naming the number in the light domain, and an over-capacity frame reports the ceiling in lights per pin on both the reinit and tick paths — the KB figure was the one a user could not act on. Tests - A shared fixture writes each script to a file, so tests exercise the path that ships. It is thread-local: the concurrency test compiles from two threads, and a shared name buffer had them compiling each other's script. - Tests that relied on a built-in default script now name one. There is no default any more. Docs/CI - MIGRATING: `source` is gone, so a persisted script is an unknown key and ignored — the entry says where to find the text (/.config/Layouts.json as "N.source") and how to restore it. - The three module specs, and the plan's step 1 marked done with what actually shipped. Verified on the desktop: a 16x12 scripted grid layout with a scripted lines effect, both compiled from files written over the API, surviving a restart and reloading from persistence. Not yet run on hardware — the boards were unreachable; that is next. Flash: esp32 1762368, esp32s3-n16r8 1752992, esp32s31 2025600, esp32p4-eth 1603952, desktop 1138184. Tests: 1326 cases. Co-Authored-By: Claude Opus 5 (1M context) --- docs/MIGRATING.md | 17 + ...and the stack as the register overflow.md" | 312 ++++++++++++++++++ docs/metrics/repo-health.json | 42 +-- docs/metrics/repo-health.md | 30 +- docs/moonmodules/light/MoonLiveEffect.md | 14 +- docs/moonmodules/light/MoonLiveLayout.md | 2 +- docs/moonmodules/light/MoonLiveModifier.md | 2 +- moondeck/moonlive/disasm.py | 3 + src/core/moonlive/MoonLive.cpp | 11 +- src/core/moonlive/MoonLive.h | 6 + src/core/moonlive/MoonLiveBuiltins.h | 6 - src/core/moonlive/MoonLiveCompiler.cpp | 20 +- src/core/moonlive/MoonLiveIr.h | 51 ++- src/light/drivers/ParallelLedDriver.h | 22 +- src/light/drivers/ParlioLedDriver.h | 11 +- src/light/moonlive/MoonLiveEffect.h | 39 ++- src/light/moonlive/MoonLiveLayout.h | 60 ++-- src/light/moonlive/MoonLiveModifier.h | 45 +-- src/light/moonlive/MoonLiveScriptFile.h | 74 +++++ src/platform/desktop/moonlive_lower_host.cpp | 4 +- src/platform/desktop/platform_desktop.cpp | 4 + src/platform/esp32/moonlive_lower_riscv.cpp | 4 +- src/platform/esp32/moonlive_lower_xtensa.cpp | 4 +- src/platform/esp32/platform_esp32_parlio.cpp | 3 + src/platform/platform.h | 5 + test/unit/core/unit_moonlive_compiler.cpp | 23 ++ test/unit/light/MoonLiveScriptFixture.h | 33 ++ test/unit/light/unit_MoonLiveLayout.cpp | 55 +-- test/unit/light/unit_MoonLiveModifier.cpp | 40 ++- 29 files changed, 754 insertions(+), 188 deletions(-) create mode 100644 "docs/history/plans/Plan-20260809 - MoonLive scales \342\200\224 right-sized IR, and the stack as the register overflow.md" create mode 100644 src/light/moonlive/MoonLiveScriptFile.h create mode 100644 test/unit/light/MoonLiveScriptFixture.h diff --git a/docs/MIGRATING.md b/docs/MIGRATING.md index 624ae916..1a0da701 100644 --- a/docs/MIGRATING.md +++ b/docs/MIGRATING.md @@ -20,6 +20,23 @@ projectMM ships **no migration code**: the persistence layer is robust by defaul ## Unreleased (`next-iteration`) +### MoonLive scripts move to the filesystem (2026-08-11) + +A scripted module used to carry its script as a `source` textarea — a fixed 1 KB array per module, plus a second 1 KB copy to notice edits, **resident whether or not a script was loaded**. Six modules cost 13 KB of a classic ESP32's 320 KB for text that was mostly empty. The script now lives in a file under `/moonlive/`, and the module holds only its **name** (~32 bytes): it is read into a right-sized buffer to compile and freed immediately, so nothing script-sized stays in RAM. A script is bounded by the filesystem instead of by a 1 KB array. + +**Action: *re-add a module* — or, to keep your scripts, *update a file* first.** + +The `source` control no longer exists, so a persisted `"source"` value is an unknown key and is ignored (the robust-reader rule). A MoonLive module therefore boots with **no script**, reporting `no script — set the script name`, and renders nothing until one is named. + +| What | Why | What to do | +|---|---|---| +| Your script text | It was persisted under `source`, a control that is gone | **Copy it out before updating** — it is in `/.config/Layouts.json` (or `Effects.json`) as `"N.source"`. Save it as `/moonlive/.mlv` via the File Manager, then set the module's `script` control to `.mlv` | +| The module's own controls | A script's `@control` sliders exist only once it has compiled, so they are absent until a script is named | Nothing — they reappear with the script, keeping their persisted values | + +`/moonlive/` is created on demand: naming a script is enough to make the folder appear, so a fresh device needs no setup. + +**Editing today** goes through the File Manager rather than the module's own card. Wiring the card's editor to the same file is a separate change. + ### MoonLive: a script can no longer declare a name the engine supplies (2026-08-10) `t` (elapsed milliseconds), `width`/`height`/`depth` (the logical grid) and `x`/`y`/`z` (the light a modifier is transforming) are now **system variables** the engine supplies, so a script cannot declare one. Previously each binding faked them by prepending hidden declarations to the script, which meant an effect could declare its own `width` and quietly disagree with the layer it was drawing into. diff --git "a/docs/history/plans/Plan-20260809 - MoonLive scales \342\200\224 right-sized IR, and the stack as the register overflow.md" "b/docs/history/plans/Plan-20260809 - MoonLive scales \342\200\224 right-sized IR, and the stack as the register overflow.md" new file mode 100644 index 00000000..398386a8 --- /dev/null +++ "b/docs/history/plans/Plan-20260809 - MoonLive scales \342\200\224 right-sized IR, and the stack as the register overflow.md" @@ -0,0 +1,312 @@ +# Plan: MoonLive scales — right-sized IR, and the stack as the register overflow + +## Context + +MoonLive scripts hit hard walls far below what a user would call a complex script. Two separate +ceilings, both measured on the host, neither obvious from the error text (every one reports +`codegen failed (unsupported on this target, or too large)`): + +- **Seven `addLight` statements in a row fail.** `kMaxIrOps = 64` ([MoonLiveIr.h:33](src/core/moonlive/MoonLiveIr.h#L33)) + and a call costs ~9 IR ops. No nesting, no register pressure — this is the wall a user meets first. +- **Nested `for` loops are refused on Xtensa.** Measured: `LOWER BAIL: vregsUsed=11 +2 > kRegCount=12` + ([moonlive_lower_xtensa.cpp:24](src/platform/esp32/moonlive_lower_xtensa.cpp#L24)). They work on + desktop and RISC-V, which have larger register maps. The shipped default layout script `grid.mlv` + is a nested loop, so the module's own default cannot compile on the smallest target. + +The goal is that a script's complexity is bounded by memory the device actually has, not by +constants chosen when a script was one statement. That means two changes, and they are +independent: **right-size the IR** (removes the statement wall) and **spill to the stack** +(removes the register wall). Neither alone is enough — shipping only the spiller leaves the +7-statement wall, which is the one users hit first. + +Spilling is also the industry-standard answer: values that outlive the register file live in the +frame. It is what every real compiler does, and it is the mechanism that makes "how complex can a +script be" a memory question instead of a register-count question. + +## Every ceiling, and what happens to it + +Seven fixed constants bound a script. They are **not** one problem: what each costs, and where its +storage lives, decides the treatment. Measured sizes: + +| Ceiling | Value | Limits | Where it lives | Treatment | +|---|---|---|---|---| +| `kMaxIrOps` | 64 | total instructions | `IrProgram` = **2056 B stack local** ([MoonLiveCompiler.cpp:512](src/core/moonlive/MoonLiveCompiler.cpp#L512)) | **right-size on the heap** — the wall users hit first | +| `kCodeCap` | 768 B | emitted machine code | `buf_[kCap]` inside the assembler, itself a **1368 B stack local** ([moonlive_lower_xtensa.cpp:28](src/platform/esp32/moonlive_lower_xtensa.cpp#L28)) | **right-size on the heap**, same mechanism | +| `kMaxVRegs` | 16 | values a program can name | index width in `IrInst` | **raise to 32** once spilling makes >16 usable; `IrProgram::push` keeps validating | +| `kMaxFixups` | 32 | branches | assembler member | **right-size** with the code buffer (same owner, same lifetime) | +| `kIrLabels`/`kMaxLabels` | 16 | ~8 loops | IR + assembler members | **right-size** with the op array; the estimator counts `for` tokens | +| `locals[4]` | 4 | loop nesting depth | Parser, stack | **raise to 8**, 12 B — a fixed bump, not worth an allocation | +| `kMaxCtrls` | 8 | script-declared controls | 128 B in Parser + the binding's name pool | **raise to 16**; bounded by UI sanity, not by memory. Note the *binding* mirrors this in a fixed name pool, so both move together | + +**The constraint that drives this:** `CONFIG_ESP_MAIN_TASK_STACK_SIZE = 12288` +([sdkconfig.defaults:8](esp32/sdkconfig.defaults#L8)), and the compile path already burns **~3.4 KB** +of it (`IrProgram` 2056 + assembler 1368, both live at once). Naively raising `kMaxIrOps` to 256 +makes `IrProgram` alone 8 KB of *stack* — a bootloop, not a fix. This project has already lost a P4 +to a large stack frame. So the two big arrays move to the heap and are sized to the script; the small +ones are simply raised, because 12 B or 128 B does not need an allocator. + +## The end state (PO, 2026-08-11) + +MoonLive is **bounded by memory, not by registers or fixed arrays** — a fairly complete language, +large scripts, nice effects. Three goals, and every step towards them is judged against the +**classic ESP32**: 320 KB internal, no PSRAM, so anything assuming plentiful RAM fails there first. +(The classic is not structurally blocked — its Xtensa backend compiles and its exec heap is enabled; +what stopped it was a crash, parked separately.) + +1. **Spilling** — register allocation with spilling to the stack, so a script is never refused for + naming more live values than the ISA has registers. Linear-scan (Poletto & Sarkar), designed + below. +2. **Scripts on the filesystem** — the source lives on LittleFS, not in a fixed per-module array. +3. **Classic ESP32 runnability** — the yardstick for all of the above. + +### Decided: a heap buffer, not a streaming lexer + +Goal 2 could load the script into a right-sized heap buffer for the compile and free it after, or +stream it from the file so no buffer exists at all. **Heap buffer**, for three reasons: + +- **The waste is the fixed array, not the transient buffer.** `source_` + `compiled_` + names is + **2240 B per module, always resident — 13.1 KB across six modules, 4.1% of a classic's internal + RAM, held whether or not a script is loaded.** A compile-time buffer is proportional to the script + and freed immediately; the fixed arrays are permanent and mostly empty. Removing them is the win. +- **Seeking a file is not simpler than seeking RAM.** LittleFS does wear-levelling and block + caching, so `parseFor`'s backward re-lex of the step clause could hit flash mid-compile — + unpredictable latency in place of a pointer decrement. +- **Streaming needs compiler surgery first.** `parseFor` re-lexes the step from a saved source + pointer after emitting the body, and `DeclaredControl::name` points INTO the source and outlives + the compile (13 sites hold such pointers). Both are fixable, neither is a lexer swap. + +Streaming stays possible later; it is an optimisation of a transient allocation, not the thing that +makes large scripts fit. + +**Two consequences:** `kMaxScriptBytes` stops being a ceiling (a script is bounded by heap), and +`compiled_` — which only answers "did the source change" — becomes a hash rather than a second full +copy, removing another 1 KB per binding. FNV-1a is already the project's idiom for that. + +### Sequence + +Each step makes the next cheaper: + +1. ✅ **Right-size `IrProgram`** (2026-08-11). The op array is heap-allocated and sized from a + token count before parsing; `IrProgram` owns it RAII (destructor frees, copy deleted), so there + is no manual free path to miss — unlike the reverted `32026eb5`, whose four independently- + nullable tables produced the heap corruption its own comment records. `kMaxIrOps` 64 → 4096 is + now a sanity bound, not the working limit: **7 sequential statements used to fail, 40 compile**, + and ~2 KB moved off the 12 KB main-task stack. + + **Found while verifying:** widening `count` to `uint16_t` left four `uint8_t` loop counters + iterating over it — three lowerers plus `IrProgram::hasInline` — which wrapped at 256 ops and + spun forever. On a device that is a watchdog reset from a script that merely got long. Bisected + (60 statements fine, 80 hung), fixed, and pinned by a test that HANGS when the fix is reverted. + The first version of that test passed either way: repeated statements hit the code-buffer + ceiling before reaching the wrap, so it needed a long arithmetic chain instead — many cheap ops, + little emitted code. + + **Still standing:** `kCodeCap` is a separate ceiling and now the binding one (40 statements + exceed it on Xtensa), as are `kIrLabels`, `locals[4]` and `kMaxVRegs`. +2. **Scripts on the filesystem.** Independent of the compiler work — different files, different + risk — and the step that most helps the classic. +3. **Spilling.** By then it has stack headroom and no 1 KB source ceiling to fight. Landing it first + would put the hardest algorithm on the tightest stack budget, where an overrun reads as a + bootloop rather than a compiler bug. + +## Decisions taken + +- **Allocate to fit the script, not to `kMax`.** The op array and the code buffer move to a + right-sized `platform::alloc` ([platform.h:55](src/platform/platform.h#L55)), sized from a cheap + pre-pass over the source and freed when compilation ends. A one-statement script pays for one + statement instead of 3.4 KB, so this *reduces* peak memory for the common case while removing the + ceiling for the rare one. Compilation is cold-path, so an allocation there costs nothing that + matters. +- **Raise the cheap ceilings rather than allocating them.** `locals`, `kMaxCtrls` and `kMaxVRegs` are + tens of bytes. Subjecting them to an allocator would add machinery that buys nothing — the standard + construct is only worth it where the size actually varies. +- **The spill algorithm lives in core, once.** Correct spilling across a loop back-edge is the + hardest logic here, and only the arm64 backend is ever executed by tests — three copies would + leave two permanently under-tested. Backends supply their register count and consume two new IR + ops. (CLAUDE.md Principle 3: core owns the hard constructs, written once.) +- **Spill everywhere, not interval splitting.** Once a value is chosen for spilling it is spilled + for its whole lifetime: every def stores, every use reloads. Splitting halves the reload traffic + but the correctness argument across a back-edge is exactly the part that goes wrong. The simple + form is provably safe and these loops are cold-path. +- **Fix the RISC-V scratch aliasing first, on its own.** Verified bug, latent today: `kScratchFn = 16` + ([moonlive_asm_riscv.cpp:20](src/platform/esp32/moonlive_asm_riscv.cpp#L20)) is x16/a6, but + `kRvReg[12] == 16` — so it *is* vreg R12. In `call()`, `mv a6, a0` stashes the result, the restore + loop reloads x16 from frame offset 48 and destroys it, then `mv dst, a6` returns R12's stale value. + It only bites when `vregsUsed > 12`, which is precisely what this work causes. Landing it inside + the feature would make the first hardware symptom look like "the new spiller broke calls". +- **Not in scope: narrowing `call()`'s save-sets.** The spill pass computes exactly the + live-across-call mask that would shrink RISC-V's 18-register and arm64's 14-register unconditional + saves. Deliberately deferred: an over-long interval only costs an unnecessary spill (fail-safe), + whereas a register wrongly omitted from a save-set corrupts a value (fail-dangerous) from the same + analysis. Ship the safe consumer first, backlog the other by name. + +## Design + +### 1. Right-sized IR and code buffer (removes the 7-statement wall) + +`IrProgram` gains a heap op array instead of `IrInst ops[kMaxIrOps]`: + +```cpp +struct IrProgram { + IrInst* ops = nullptr; // platform::alloc'd to fit; freed in the destructor + uint16_t cap = 0; // what was allocated + uint16_t count = 0; + VReg vregsUsed = kFirstTemp; + bool reserve(uint16_t ops); // false on alloc failure — degrade, never crash +}; +``` + +The **assembler gets the same treatment**: `buf_[kCap]`, `labelPos_[]` and `fixups_[]` become one +right-sized allocation with the same lifetime. This is the other 1368 B of stack, and `kCodeCap` is +a ceiling in its own right — a long script overflows the code buffer even when its IR fits. + +**Sizing.** One cheap pre-pass over the token stream counts statements, call arguments and `for` +keywords, then multiplies by the known worst-case ops (and bytes) per construct. Over-estimating is +free — a few unused entries; under-estimating must be impossible, so the estimator is deliberately +conservative and `push()`/`emit()` still fail cleanly if it is ever wrong. The existing +`overflow_` path stays as the backstop it already is. + +`count`/`cap` widen to `uint16_t`, so the ceiling stops being a `uint8_t`. `kMaxIrOps` and `kCodeCap` +survive as upper *sanity* bounds — a runaway script fails with a diagnostic rather than exhausting +the heap — not as the working limit. + +Both allocations are freed when compilation ends: they are compile-time scratch, not part of the +running program. The only thing that outlives a compile is the exec block, which is unchanged. + +### 2. Spill to the frame (removes the register wall) + +**New IR ops** ([MoonLiveIr.h](src/core/moonlive/MoonLiveIr.h)): + +```cpp +Spill, // slot[imm] = a +Reload, // dst = slot[imm] +``` + +**New core pass** `src/core/moonlive/MoonLiveSpill.{h,cpp}`: + +```cpp +/// Rewrite `ir` so no op names a vreg the target does not have, inserting Spill/Reload against a +/// fixed slot file. False when even the spilled form does not fit (fail, never miscompile). +bool spillToBudget(IrProgram& ir, const RegBudget& budget); +``` + +**Algorithm: linear-scan register allocation** (Poletto & Sarkar) over the op array, with +**loop-extended live intervals**. Three passes, no heap beyond the interval array: + +- **Find loops.** The grammar has no `break`, `continue` or `goto`, so a loop is exactly a + `BranchNe` whose target label is bound earlier in the array. The op array is therefore already in + reverse-postorder and no CFG needs building — that is the one bespoke simplification, and it + carries a guard: a branch pattern that is *not* properly nested makes the pass refuse rather than + allocate against a wrong interval, so a future `break` fails loudly instead of miscompiling. +- **Naive intervals**, then **loop extension**: any value live at a loop header is live to the end of + that loop, applied innermost-first. This is the step naive "first def to last use" gets wrong, and + it is conservative — it can only lengthen an interval, so it may cost a needless spill but never + produces a wrong one. +- **Scan**, spilling the active interval with the furthest end when no register is free. + +**Backend surface** — identical on all three, and the algorithm appears nowhere in the platform layer: + +```cpp +void prologue(uint8_t slots); // slots == 0 emits nothing: a non-spilling script pays zero +void spillStore(Reg r, uint8_t slot); +void spillLoad(Reg r, uint8_t slot); +``` + +Each lowerer gains two switch arms and *loses* its hand-rolled budget bail (the three duplicated +`vregsUsed + N > kRegCount` checks collapse into the one core pass). + +Where the slots live differs per target and is the real per-backend work: + +| backend | frame today | spill slots | +|---|---|---| +| Xtensa | whole-routine 48 B from `entry a1, 48`; `call()` uses 16/20/24/28 | bytes 32–47 are free — 4 slots at zero cost; `entry` immediate grows for more | +| RISC-V | **none outside `call()`** (`prologue()` is empty) | needs a real 2-instruction prologue/epilogue; `encSw`/`encLw` already exist at file scope | +| arm64 | **none outside `call()`**; the `call()` frame is 100% full | needs a prologue *and* two new `str`/`ldr` encoders — the only backend with no general store/load | + +### 3. Conditional inline scratch (the cheap part of the register fix) + +All three lowerers reserve scratch vregs for `FillElems` unconditionally — `+3` on host, `+2` on +Xtensa and RISC-V — even when the program contains no such op. A layout script never emits one. This +single unconditional reservation is what makes `grid.mlv` (11 vregs, budget 12) fail on Xtensa. +Core reports which inline ops a program actually contains; each backend maps that to its own scratch +count. Nested loops compile on Xtensa from this alone, and it is the `RegBudget.reserved` field the +spill pass consumes — not throwaway. + +## Files + +- `src/core/moonlive/MoonLiveIr.h` — heap op array, `uint16_t` counts, `Spill`/`Reload`, + `inlineScratch()`, the raised `kMaxVRegs`/`kIrLabels` +- `src/core/moonlive/MoonLiveSpill.{h,cpp}` — **new**, the linear scan +- `src/core/moonlive/MoonLiveCompiler.cpp` — the sizing pre-pass, the pass call site (~line 516), the + temp allocator's failure path (lines 141-160), `locals[]` and `kMaxCtrls` +- `src/light/moonlive/MoonLive{Layout,Effect,Modifier}.h` — the bindings' `ctrlNames_` name pool + mirrors `kMaxCtrls` and must grow with it, or the extra controls compile but never appear in the UI +- `src/core/moonlive/moonlive_emit.h` — the `RegBudget` seam +- `src/platform/esp32/moonlive_asm_riscv.cpp` — the `kScratchFn` fix (line 20), prologue + spill surface +- `src/platform/desktop/moonlive_asm_host.{h,cpp}` — new `str`/`ldr` encoders, prologue; the only + backend tests execute +- `src/platform/esp32/moonlive_asm_xtensa.{h,cpp}` — promote the private `s32i`/`l32i` lambdas to the + spill surface +- the three `moonlive_lower_*.cpp` — two switch arms each, minus their budget bails +- `moondeck/moonlive/` — generalise `emit_xtensa.cpp` to an ISA flag so `disasm.py --isa riscv` works +- `docs/moonmodules/light/MoonLive*.md`, `moonlive/README.md` — the new limits +- `docs/backlog/backlog-light.md` — narrow `call()` save-sets, by name + +Add a `static_assert` per backend that no scratch register appears in its vreg map — the invariant +the RISC-V bug broke, made unbreakable rather than commented. + +## Verification + +The governing risk: **only arm64 is executed by tests**; Xtensa and RISC-V are compile-time-excluded +and validated on hardware. So arm64 carries the correctness proof, and the device backends carry +only encoding risk, which `disasm.py` retires without a flash. + +1. **The key test — a squeezed budget on the host.** A test-only budget override runs + `spillToBudget` with a register count *smaller* than the host's, forcing the spiller to run on the + one backend that executes. Same script compiled at full and squeezed budgets must produce + identical pixels. This makes the hard algorithm testable rather than hardware-only. +2. **The back-edge case specifically**: a nested loop at a squeezed budget where the counters are + guaranteed spilled — every expected light placed exactly once. +3. **Spill across a call**: value spilled, `random16()` called, value used. Proves slots survive + `call()`'s own frame — the RISC-V case to check hardest, since prologue and `call()` both move sp. +4. **One test per ceiling**, each a script that fails today and must pass after — this is what proves + the overview table was actually delivered rather than partly delivered: + - 30+ straight-line statements (`kMaxIrOps`, fails at 7 today) + - a script whose emitted code exceeds 768 bytes (`kCodeCap`) + - 6-deep loop nesting (`locals`) and 10+ loops in one script (`kIrLabels`/`kMaxLabels`/`kMaxFixups`) + - 12 declared controls (`kMaxCtrls`) — and the *binding* surfaces all 12, since it mirrors the cap + in its own name pool + - a script needing more than 16 live values (`kMaxVRegs`, only reachable once spilling works) +5. **Stack, not just heap**: assert the compile path's stack frame *shrank*. `IrProgram` and the + assembler stop being 3.4 KB of stack locals; a one-statement script must allocate proportionally + less than a hundred-statement one. Without this the change could pass every functional test while + quietly moving the bootloop somewhere else. +6. **Degrade**: a deliberately absurd script fails with a clear diagnostic and no crash; an alloc + failure in `reserve()` fails the compile cleanly rather than writing through a null pointer. +6. **Unchanged behaviour**: `unit_moonlive_ir` / `unit_moonlive_fill` (the `fill` behavioural golden, + and kArg4 surviving a call) stay green — they pin that a `FillElems` program still gets its + scratch. `unit_MoonLiveScripts.cpp:118` (bare vs commented produce equal length) is the canary for + the pass accidentally becoming source-dependent. +7. **Encodings on device backends without flashing**: `uv run moondeck/moonlive/disasm.py` on a + spilling script, reading the actual `s32i`/`l32i` offsets against the frame layout. This is the + tool that found the `Mov`→`addi 0` bug. Extend it to RISC-V, which has no equivalent today. +8. **Memory + hot path**: `collect_kpi.py --commit`. The IR allocation is cold-path, but a modifier + script runs once per light, so measure a mapping rebuild on a large grid. Confirm a non-spilling + script emits no prologue and costs nothing. +9. **Hardware, the final gate (PO)**: flash `grid.mlv` on an S3 (Xtensa — the target that fails + today) and a P4 (RISC-V — the target with the scratch bug), and look at the wall. + +## Suggested commit boundaries + +The PO decides commits and branches; this is the order that keeps each step independently +verifiable, riskiest-last: + +1. RISC-V `kScratchFn` aliasing fix + the `static_assert`s (independent bug) +2. Conditional inline scratch — nested loops compile on Xtensa +3. Right-sized IR + code buffer, and the cheap ceilings raised (`locals`, `kMaxCtrls`, `kIrLabels`, + `kMaxFixups`) — the 7-statement wall goes and the compile stack shrinks +4. Spill surface on the three assemblers (encodings verifiable in isolation, dead code until 5) +5. The core spill pass + `kMaxVRegs` raised — the register ceiling goes + +Steps 1–3 deliver the ceiling a user meets first and are independently shippable; 4–5 are the +spiller. If the branch needs splitting for review size, that is the seam. diff --git a/docs/metrics/repo-health.json b/docs/metrics/repo-health.json index 69cf5d69..83fdee96 100644 --- a/docs/metrics/repo-health.json +++ b/docs/metrics/repo-health.json @@ -1,21 +1,21 @@ { - "commit": "9d77ade4", + "commit": "38a28dc9", "flash": { "esp32": 1762368, - "esp32p4-eth": 1604272, + "esp32p4-eth": 1603952, "esp32p4-eth-wifi": 1793760, - "esp32s3-n16r8": 1753232, + "esp32s3-n16r8": 1752992, "esp32s3-n8r8": 1753232, "esp32s31": 2025600, - "desktop": 1137928, + "desktop": 1138184, "esp32-16mb": 1714608, "esp32-eth": 1324928, "esp32-wrover": 1765504 }, "perf": { "desktop": { - "tick_us": 129, - "fps": 7751 + "tick_us": 132, + "fps": 7575 }, "esp32": { "tick_us": 2151, @@ -23,24 +23,24 @@ } }, "loc": { - "core": 16980, - "light": 24317, - "platform": 12860, + "core": 17044, + "light": 24402, + "platform": 12878, "ui": 6468, - "test": 41246, - "moondeck": 20323 + "test": 41313, + "moondeck": 20326 }, "comments": { "core": { - "lines": 6402, - "ratio": 0.41 + "lines": 6433, + "ratio": 0.411 }, "light": { - "lines": 9427, + "lines": 9473, "ratio": 0.429 }, "platform": { - "lines": 4365, + "lines": 4377, "ratio": 0.375 }, "ui": { @@ -48,29 +48,29 @@ "ratio": 0.274 }, "test": { - "lines": 7169, + "lines": 7193, "ratio": 0.201 }, "moondeck": { - "lines": 3246, + "lines": 3248, "ratio": 0.183 } }, "tests": { - "cases": 1325, + "cases": 1326, "scenarios": 23 }, "docs": { "md_files": 178, - "md_lines": 24401, + "md_lines": 24421, "plans_files": 91, "backlog_lines": 3629, "lessons_lines": 454, "claude_md_lines": 135 }, "complexity": { - "functions": 2473, - "over_threshold": 151, + "functions": 2480, + "over_threshold": 153, "worst_ccn": 93 } } diff --git a/docs/metrics/repo-health.md b/docs/metrics/repo-health.md index 374c9616..935dcc5a 100644 --- a/docs/metrics/repo-health.md +++ b/docs/metrics/repo-health.md @@ -1,6 +1,6 @@ # Repo health -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.** +Measured at `38a28dc9`. 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,48 +8,48 @@ Current state only; the trend is this file's git history (`git log -p docs/metri | Target | Flash | |---|---:| -| desktop | 1,111 KB (+0 KB) ⚠ | +| desktop | 1,112 KB (+0 KB) ⚠ | | esp32 | 1,721 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 | 1,566 KB (−0 KB) ✓ | | esp32p4-eth-wifi | 1,752 KB | -| esp32s3-n16r8 | 1,712 KB | -| esp32s3-n8r8 | 1,712 KB (+84 KB) ⚠ | +| esp32s3-n16r8 | 1,712 KB (−0 KB) ✓ | +| esp32s3-n8r8 | 1,712 KB | | esp32s31 | 1,978 KB | ## Render performance | Target | Tick | FPS | |---|---:|---:| -| desktop | 129 µs (+1 µs) ⚠ | 7,751 (−61) ⚠ | +| desktop | 132 µs (+3 µs) ⚠ | 7,575 (−176) ⚠ | | esp32 | 2,151 µs | 464 | ## Code | Area | Lines | Comments | Comment share | |---|---:|---:|---:| -| core | 16,980 | 6,402 | 41.0 % | -| light | 24,317 (+5) ⚠ | 9,427 | 42.9 % | -| platform | 12,860 | 4,365 | 37.5 % | +| core | 17,044 (+64) ⚠ | 6,433 | 41.1 % (+0.1 %) ⚠ | +| light | 24,402 (+85) ⚠ | 9,473 | 42.9 % | +| platform | 12,878 (+18) ⚠ | 4,377 | 37.5 % | | ui | 6,468 | 1,670 | 27.4 % | -| test | 41,246 | 7,169 | 20.1 % | -| moondeck | 20,323 | 3,246 | 18.3 % | +| test | 41,313 (+67) ⚠ | 7,193 | 20.1 % | +| moondeck | 20,326 (+3) ⚠ | 3,248 | 18.3 % | ## Tests | Kind | Count | |---|---:| -| unit cases | 1,325 | +| unit cases | 1,326 (+1) ✓ | | scenarios | 23 | ## Complexity | Metric | Value | |---|---:| -| functions | 2,473 (+1) ✓ | -| over threshold | 151 | +| functions | 2,480 (+7) ✓ | +| over threshold | 153 (+2) ⚠ | | worst CCN | 93 | ## Documentation @@ -57,7 +57,7 @@ Current state only; the trend is this file's git history (`git log -p docs/metri | Metric | Value | |---|---:| | markdown files | 178 | -| markdown lines | 24,401 | +| markdown lines | 24,421 (+20) ⚠ | | plan files | 91 | | backlog lines | 3,629 | | lessons lines | 454 | diff --git a/docs/moonmodules/light/MoonLiveEffect.md b/docs/moonmodules/light/MoonLiveEffect.md index e22431d2..aca82f8f 100644 --- a/docs/moonmodules/light/MoonLiveEffect.md +++ b/docs/moonmodules/light/MoonLiveEffect.md @@ -4,7 +4,7 @@ MoonLive is projectMM's **live-script engine** — author an effect as text and Scripts call the same [power functions](power-functions.md) compiled effects use, reached through the builtin table — so the vocabulary is shared, in its flat scalar form. -A scripted effect carries its **script source** as an editable, persisted multi-line text control (a resizable `textarea` in the UI), and a front-end (lexer → parser → IR → per-ISA assembler) compiles it to native code on the next tick. The grammar is a sequence of **statements** — a function call, or a `for` loop over them — with **expression arguments**, so any argument may be a literal or a nested call: +A scripted effect names a **script file** under `/moonlive/`; the UI loads, edits and saves that file, and the module holds only the name (~32 bytes) — the text is read into a right-sized buffer to compile and freed immediately, so nothing script-sized stays resident. A front-end (lexer → parser → IR → per-ISA assembler) compiles it to native code on the next tick. The grammar is a sequence of **statements** — a function call, or a `for` loop over them — with **expression arguments**, so any argument may be a literal or a nested call: ``` setRGB(random16(256), 0, 0, 255); // a random pixel, blue @@ -16,7 +16,7 @@ The functions are **not built into the compiler** — `setRGB`, `fill`, `random1 ## Controls -- `source` — the script text (default: random pixels — `setRGB(random16(256), random16(256), random16(256), random16(256));`, one random light in a random color each tick). Editing it recompiles live: a valid script swaps in on the next tick; a failed compile frees the old code, shows the diagnostic in the module status, and renders dark until fixed (the script-editor loop, robust + no reboot). +- `script` — the file name under `/moonlive/`, e.g. `lines.mlv`. A fresh module has none: it reports `no script — set the script name` and renders nothing, rather than every new module compiling the same default. Naming one (or re-naming it after an edit) recompiles live: a valid script swaps in on the next tick; a failed compile frees the old code, shows the diagnostic in the module status, and renders dark until fixed (the script-editor loop, robust + no reboot). The directory is created on demand. - **Scripted controls** — a script declares a tunable variable with a range annotation, and the engine surfaces it as a real `uint8` MoonModule control (slider + UI + persistence), bound to a live value the running native code reads each tick: ```c @@ -25,7 +25,7 @@ The functions are **not built into the compiler** — `setRGB`, `fill`, `random1 setRGB(speed, hue, 0, 255); ``` - Declaring the variable is what **creates** the control: `uint8_t = ;` becomes a `` slider (default ``, range `0..255`). The trailing `// @control ..` only **adjusts that control's range**; it's optional. A declared name used in a statement reads the control's **current** value. Editing a control's slider does **not** recompile — the value lands in the engine's control-values arena and the next render tick reads it (the live-edit guarantee, the *no-reboot* principle). Editing the `source` recompiles and re-derives the control set; a control kept across the edit keeps its slider value, a removed control's saved value drops. Stage 1 is `uint8` only. + Declaring the variable is what **creates** the control: `uint8_t = ;` becomes a `` slider (default ``, range `0..255`). The trailing `// @control ..` only **adjusts that control's range**; it's optional. A declared name used in a statement reads the control's **current** value. Editing a control's slider does **not** recompile — the value lands in the engine's control-values arena and the next render tick reads it (the live-edit guarantee, the *no-reboot* principle). Saving the script file and re-naming it recompiles and re-derives the control set; a control kept across the edit keeps its slider value, a removed control's saved value drops. Stage 1 is `uint8` only. ### System variables — what the engine hands a script @@ -70,7 +70,7 @@ Registered by the light domain, not built into the compiler (the core owns only ### Wire contract — control declaration -The controls are **derived from `source`** (one per declared `uint8` control; the optional `@control` annotation only refines a control's range), then **surfaced in `/api/state`** — the device JSON view the integrator consumes — as regular `uint8` controls alongside `source`. So an integrator sees and writes them exactly like any other control — e.g. `POST /api/control` with `{"module": "ML", "control": "speed", "value": 80}`; they're fully present in the device JSON, just authored in the script rather than fixed in the module. The script's `\n` line breaks are standard JSON string escapes the device decodes, so a multi-line `source` round-trips. +The controls are **derived from the script** (one per declared `uint8` control; the optional `@control` annotation only refines a control's range), then **surfaced in `/api/state`** — the device JSON view the integrator consumes — as regular `uint8` controls alongside `script`. So an integrator sees and writes them exactly like any other control — e.g. `POST /api/control` with `{"module": "ML", "control": "speed", "value": 80}`; they're fully present in the device JSON, just authored in the script rather than fixed in the module. The script's `\n` line breaks are standard JSON string escapes the device decodes, so a multi-line script round-trips through `/api/file`. ## Pieces @@ -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()`. `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. +- **`MoonLiveEffect`** (`src/light/moonlive/MoonLiveEffect.h`) — the **thin binding**: a first-class `EffectBase` carrying the `script` 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 @@ -95,9 +95,9 @@ MoonLive's native-codegen approach — compile a small C-like language straight [unit_moonlive_fill](../../../test/unit/core/unit_moonlive_fill.cpp) runs the engine path in-process on the desktop host backend (`compile`/`run`, the animated routine, zero-lights, recompile, `free`, the `allocExec`/`writeExec`/`freeExec` round-trip, the buffer-shape guards). [unit_moonlive_ir](../../../test/unit/core/unit_moonlive_ir.cpp) pins the **behavioral golden** — a compiled `fill` and the hand-encoded reference render an identical buffer — plus setRGB's single-pixel write and the runtime bounds guard. [unit_moonlive_compiler](../../../test/unit/core/unit_moonlive_compiler.cpp) pins the expression grammar (`random16` in any/every argument slot, uint16 bounds), the parser diagnostics (no crash on malformed input), live recompile, and the **domain-neutral** property: with an empty builtin table the core knows *no* functions, and a host can register an arbitrary name against the same machinery. -The grammar + bounds guard are verified live on the S3/Olimex (Xtensa) by editing the `source` control — the device compiles the expression on-chip and renders it. +The grammar + bounds guard are verified live on the S3/Olimex (Xtensa) by saving a script file and naming it — the device compiles the expression on-chip and renders it. -[scenario_MoonLiveEffect_livescript](../../../test/scenarios/light/scenario_MoonLiveEffect_livescript.json) exercises the effect **as a wired MoonModule** — what the unit tests can't reach: add it, live-edit the `source` to recolor (recompile), push a broken script (`MoonLive::compile` fails, frees the previous code, `MoonLiveEffect` reports the parse error in the status and renders dark — no crash), recover, resize the grid to 1×1 and back while rendering (the every-grid-size hard rule), then remove and re-add (exec memory re-acquired clean). It runs in-process on the desktop backend each commit, and the same JSON runs live over REST against the device backends. The Xtensa/RISC-V backends are validated by the live S3/P4 runs (a `MoonLiveEffect` on a Layer lights the grid from its `source`), which the desktop tests can't reach. +[scenario_MoonLiveEffect_livescript](../../../test/scenarios/light/scenario_MoonLiveEffect_livescript.json) exercises the effect **as a wired MoonModule** — what the unit tests can't reach: add it, live-edit the script file to recolor (recompile), push a broken script (`MoonLive::compile` fails, frees the previous code, `MoonLiveEffect` reports the parse error in the status and renders dark — no crash), recover, resize the grid to 1×1 and back while rendering (the every-grid-size hard rule), then remove and re-add (exec memory re-acquired clean). It runs in-process on the desktop backend each commit, and the same JSON runs live over REST against the device backends. The Xtensa/RISC-V backends are validated by the live S3/P4 runs (a `MoonLiveEffect` on a Layer lights the grid from its script file), which the desktop tests can't reach. ## Source diff --git a/docs/moonmodules/light/MoonLiveLayout.md b/docs/moonmodules/light/MoonLiveLayout.md index d46d77b3..9b85dd90 100644 --- a/docs/moonmodules/light/MoonLiveLayout.md +++ b/docs/moonmodules/light/MoonLiveLayout.md @@ -77,7 +77,7 @@ So it runs twice. On the first pass `addLight` counts; on the second it emits ea | control | what it does | |---|---| -| `source` | the script; editing it recompiles and re-places the lights live | +| `script` | the file name under `/moonlive/`; naming it (or re-naming it after an edit) recompiles and re-places the lights live | Plus one control per `@control` the script declares. diff --git a/docs/moonmodules/light/MoonLiveModifier.md b/docs/moonmodules/light/MoonLiveModifier.md index 43ee9ff6..014a5384 100644 --- a/docs/moonmodules/light/MoonLiveModifier.md +++ b/docs/moonmodules/light/MoonLiveModifier.md @@ -42,7 +42,7 @@ It is for debugging and comes back out again — [what print costs](../../../moo | control | what it does | |---|---| -| `source` | the script; editing it recompiles and re-maps live | +| `script` | the file name under `/moonlive/`; naming it (or re-naming it after an edit) recompiles and re-maps live | Plus one control per `@control` the script declares — `uint8_t amount = 4; // @control 0..64` becomes a slider, and moving it rebuilds the mapping just as editing the script does. diff --git a/moondeck/moonlive/disasm.py b/moondeck/moonlive/disasm.py index 49f3a655..565a3dba 100644 --- a/moondeck/moonlive/disasm.py +++ b/moondeck/moonlive/disasm.py @@ -43,6 +43,9 @@ def main() -> int: ["c++", "-std=c++20", "-O0", "-I", os.path.join(ROOT, "src"), "-I", os.path.join(ROOT, "src", "platform", "desktop"), TOOL_SRC, os.path.join(ROOT, "src", "core", "moonlive", "MoonLiveCompiler.cpp"), + # The IR sizes its op array with platform::alloc, so the platform implementation has + # to come along — the compiler is no longer self-contained. + os.path.join(ROOT, "src", "platform", "desktop", "platform_desktop.cpp"), "-o", emitter], capture_output=True, text=True) if build.returncode != 0: diff --git a/src/core/moonlive/MoonLive.cpp b/src/core/moonlive/MoonLive.cpp index 85872b1c..00cc00bd 100644 --- a/src/core/moonlive/MoonLive.cpp +++ b/src/core/moonlive/MoonLive.cpp @@ -67,7 +67,16 @@ bool MoonLive::compile(const char* source, const BuiltinTable& table, const SysV else if (ctrlArena_[i] > hi) ctrlArena_[i] = hi; } controlCount_ = cr.controlCount; - for (uint8_t i = 0; i < cr.controlCount; i++) controls_[i] = cr.controls[i]; + for (uint8_t i = 0; i < cr.controlCount; i++) { + controls_[i] = cr.controls[i]; + // Re-point `name` at our own copy: the parser's pointer is into the source text, which the + // caller may free as soon as this returns. + const uint8_t len = cr.controls[i].nameLen < kMaxControlName - 1 + ? cr.controls[i].nameLen : static_cast(kMaxControlName - 1); + for (uint8_t j = 0; j < len; j++) ctrlNames_[i][j] = cr.controls[i].name[j]; + ctrlNames_[i][len] = '\0'; + controls_[i].name = ctrlNames_[i]; + } ctrl_ = reinterpret_cast(block); return true; } diff --git a/src/core/moonlive/MoonLive.h b/src/core/moonlive/MoonLive.h index bac22e45..c79fa407 100644 --- a/src/core/moonlive/MoonLive.h +++ b/src/core/moonlive/MoonLive.h @@ -116,6 +116,12 @@ class MoonLive { uint8_t* ctrlArena_ = nullptr; // live control + system-variable bytes (platform::alloc, kArenaBytes, fixed) uint8_t controlCount_ = 0; // controls the current program declared DeclaredControl controls_[kMaxCtrls] = {}; // the declared-control metadata for the binding + // The declared NAMES, owned. A DeclaredControl's `name` points into the SOURCE TEXT, which the + // caller is free to release the moment compile() returns — and does, now that a script is read + // from a file into a transient buffer. Copying the bytes here is what lets the engine outlive + // the text it was built from; without it a binding reads freed memory when it publishes its + // controls, which showed up as a control literally named "\x05". + char ctrlNames_[kMaxCtrls][kMaxControlName] = {}; }; } // namespace mm::moonlive diff --git a/src/core/moonlive/MoonLiveBuiltins.h b/src/core/moonlive/MoonLiveBuiltins.h index f7fd8cf3..189ed9fe 100644 --- a/src/core/moonlive/MoonLiveBuiltins.h +++ b/src/core/moonlive/MoonLiveBuiltins.h @@ -102,12 +102,6 @@ static constexpr uint8_t kMaxCtrls = 8; // a script declares a handful // the compile. Word-aligned so allocExec/writeExec's word-rounding never exceeds it. static constexpr size_t kCodeCap = 2048; -// The script text a binding holds. 1 KB, not 512 B: 512 could not hold a DOCUMENTED script — the -// shipped lines.mlv is ~490 characters with its comments — and a script that overruns is silently -// truncated mid-token, so it fails to compile with no hint that length was the reason. A binding is -// ~2 KB at this size, which the smallest board still carries. -static constexpr size_t kMaxScriptBytes = 1024; - static constexpr uint8_t kMaxSysVars = 8; static constexpr uint8_t kArenaBytes = kMaxCtrls + kMaxSysVars; diff --git a/src/core/moonlive/MoonLiveCompiler.cpp b/src/core/moonlive/MoonLiveCompiler.cpp index d6d0622e..f51aa2e3 100644 --- a/src/core/moonlive/MoonLiveCompiler.cpp +++ b/src/core/moonlive/MoonLiveCompiler.cpp @@ -560,8 +560,26 @@ CompileResult compileSource(const char* source, const BuiltinTable& table, if (!source) { r.error = "no source"; return r; } if (!out || cap == 0) { r.error = "no code buffer"; return r; } - Lexer lex(source); + // Size the op array to THIS script before parsing. The bound is per-TOKEN rather than + // per-construct: no token the lexer can produce lowers to more than a handful of ops (the + // densest is a call argument — evaluate, then the Call itself), so counting tokens and + // multiplying is an over-estimate that cannot undershoot. Over-estimating costs a few unused + // entries on a cold path; undershooting would fail a script that fits, so the direction of the + // error is the whole point. push() still refuses past `cap`, so a wrong estimate degrades with + // a diagnostic rather than corrupting memory. + uint32_t tokens = 0; + for (Lexer scan(source); scan.kind != Tok::End && scan.kind != Tok::Error; scan.advance()) { + if (++tokens > kMaxIrOps) break; // runaway source — reserve() rejects past the bound + } IrProgram ir; + // +8 covers a program's fixed overhead (the prologue/epilogue ops a tiny script still needs) + // so a one-token source cannot round down to nothing. + if (!ir.reserve(static_cast(tokens * kIrOpsPerToken + 8 > kMaxIrOps + ? kMaxIrOps : tokens * kIrOpsPerToken + 8))) { + r.error = "script too large"; + return r; + } + Lexer lex(source); Parser parser{lex, table, sysvars, ir}; if (!parser.parseProgram()) { r.error = parser.error; r.errorCol = parser.errorCol; return r; } diff --git a/src/core/moonlive/MoonLiveIr.h b/src/core/moonlive/MoonLiveIr.h index 598e0b83..75574baf 100644 --- a/src/core/moonlive/MoonLiveIr.h +++ b/src/core/moonlive/MoonLiveIr.h @@ -3,6 +3,7 @@ #include #include #include "core/moonlive/MoonLiveBuiltins.h" // InlineOp (a neutral opcode tag) +#include "platform/platform.h" // alloc/free — the op array is sized to the script // MoonLive IR — the typed intermediate representation between the front-end and the per-ISA // assembler (§3.2 of livescripts-analysis-top-down.md). The front-end lowers an AST to a flat @@ -30,7 +31,15 @@ using VReg = uint8_t; enum : VReg { kArg0 = 0, kArg1 = 1, kArg2 = 2, kArg3 = 3, kArg4 = 4, kFirstTemp = 5 }; static constexpr uint8_t kMaxVRegs = 16; // a statement uses a handful; no allocator yet -static constexpr uint8_t kMaxIrOps = 64; // a statement is a handful of ops; fixed, no heap +// An upper SANITY bound, not the working limit: the op array is sized to the script (see IrProgram), +// so a one-statement script pays for one statement. This exists only so a runaway source fails with +// a diagnostic instead of asking for an allocation that would exhaust a small device's heap. +static constexpr uint16_t kMaxIrOps = 4096; + +// Ops a single source token can lower to, worst case. The compiler sizes its op array by counting +// tokens and multiplying — an over-estimate by construction, which is the safe direction: a few +// unused entries on a cold path, versus refusing a script that would have fit. +static constexpr uint16_t kIrOpsPerToken = 4; // The op set — neutral. Three-address form: dst plus up to three source operands. (Counted // Control flow arrived with the script-level `for`, which is what the note here anticipated: the @@ -96,14 +105,42 @@ static constexpr uint8_t kMaxControlName = 24; // max control-name length (inc // rejects longer names so the binding's name pool // can't truncate distinct names into a collision -// A lowered program: a fixed list of ops plus the vreg high-water mark. +// A lowered program: the ops, sized to the script, plus the vreg high-water mark. +// +// The op array is HEAP-ALLOCATED rather than an `IrInst ops[kMaxIrOps]` member. As a member it cost +// the same ~2 KB of STACK for a one-statement script as for a full one, and this object is a local +// on the compile path of a 12 KB main task — so raising the ceiling by growing the array would have +// traded a compile limit for a stack overflow (this project has already lost a P4 to a large stack +// frame). Sizing to the script makes the small case cheaper AND the large case possible. +// Compilation is cold path, so the allocation costs nothing that matters. +// +// Ownership is RAII: one allocation, freed in the destructor, copying deleted. There is no manual +// free path to miss — the reverted 32026eb5 turned four tables into independently-nullable pointers +// and its own comment records the heap corruption that followed from missing one guard. struct IrProgram { - IrInst ops[kMaxIrOps]; - uint8_t count = 0; - VReg vregsUsed = kFirstTemp; + IrInst* ops = nullptr; + uint16_t cap = 0; // entries allocated + uint16_t count = 0; + VReg vregsUsed = kFirstTemp; + + IrProgram() = default; + ~IrProgram() { platform::free(ops); } + IrProgram(const IrProgram&) = delete; // owns a buffer; a copy would double-free + IrProgram& operator=(const IrProgram&) = delete; + + /// Size the op array to `n` entries. False when the allocation fails or `n` exceeds the sanity + /// bound, so the caller reports a diagnostic instead of writing through a null pointer. + bool reserve(uint16_t n) { + if (n == 0 || n > kMaxIrOps) return false; + platform::free(ops); + ops = static_cast(platform::alloc(sizeof(IrInst) * n)); + cap = ops ? n : 0; + count = 0; + return ops != nullptr; + } bool push(const IrInst& i) { - if (count >= kMaxIrOps) return false; + if (!ops || count >= cap) return false; // Reject any op that names a vreg outside the fixed register budget — an invalid program // is dropped at the seam rather than reaching a backend that would index past its map. if (i.dst >= kMaxVRegs || i.a >= kMaxVRegs || i.b >= kMaxVRegs || @@ -122,7 +159,7 @@ struct IrProgram { /// fold into the index vreg) and stays with each backend; WHICH ops are present is a property of /// the program, so it is answered once here. bool hasInline(InlineOp which) const { - for (uint8_t i = 0; i < count; i++) + for (uint16_t i = 0; i < count; i++) // uint16_t: `count` is, so a uint8_t never terminates if (ops[i].op == IrOp::Inline && ops[i].inlineOp == which) return true; return false; } diff --git a/src/light/drivers/ParallelLedDriver.h b/src/light/drivers/ParallelLedDriver.h index 241dda59..ae40387b 100644 --- a/src/light/drivers/ParallelLedDriver.h +++ b/src/light/drivers/ParallelLedDriver.h @@ -580,7 +580,7 @@ class ParallelLedDriver : public DriverBase { // 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); + reportOverCapacity(outCh, peripheral_->busCapacity()); return; } @@ -785,13 +785,16 @@ class ParallelLedDriver : public DriverBase { /// 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) { + /// `cap` is the byte ceiling to measure against — the peripheral's live buffer capacity on the + /// tick path, or its declared DMA budget at reinit(), where no bus exists yet and busCapacity() + /// would read 0. Passing it in keeps one message for both, instead of a KB figure on one path + /// and the actionable light count on the other. + void reportOverCapacity(uint8_t outCh, size_t cap) { 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_), @@ -1862,14 +1865,11 @@ class ParallelLedDriver : public DriverBase { if (const size_t budget = peripheral_->dmaBudgetBytes(); !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 the bus %uKB: fewer lights/pin", - static_cast(frameBytes_ / 1024), - static_cast(budget / 1024)); - setStatus(b, Severity::Error); - } else { - setStatus(peripheral_->initFailMsg(), Severity::Error); - } + // Same message the tick path gives, measured against the DECLARED budget (no bus is up + // yet, so busCapacity() would read 0): the light count the user has to lower, not a KB + // figure they would have to convert. reinit() cleared overCapReported_ above, so this + // reports once per geometry rather than once per attempt. + reportOverCapacity(correction_.outChannels, budget); return; } // Allocate the second buffer only when wanted (see wantSecond above — gated on the toggle AND the diff --git a/src/light/drivers/ParlioLedDriver.h b/src/light/drivers/ParlioLedDriver.h index 3963f05b..16159a85 100644 --- a/src/light/drivers/ParlioLedDriver.h +++ b/src/light/drivers/ParlioLedDriver.h @@ -76,11 +76,12 @@ 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; } + // Parlio sends a frame in ONE transfer, and the peripheral caps that. The cap is a HARDWARE + // fact (derived from PARLIO_LL_TX_MAX_BITS_PER_FRAME), so the platform owns the number and this + // asks for it — reinit() then refuses 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 platform::parlioMaxTransferBytes(); } /// 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/MoonLiveEffect.h b/src/light/moonlive/MoonLiveEffect.h index 837b755d..86f5432d 100644 --- a/src/light/moonlive/MoonLiveEffect.h +++ b/src/light/moonlive/MoonLiveEffect.h @@ -2,6 +2,7 @@ #include "light/effects/EffectBase.h" #include "core/moonlive/MoonLive.h" +#include "light/moonlive/MoonLiveScriptFile.h" #include "light/moonlive/MoonLiveBuiltins_light.h" #include #include @@ -31,7 +32,10 @@ class MoonLiveEffect : public EffectBase { // next render tick reads — no recompile (the live-edit guarantee). Editing the source // recompiles (the script-editor loop), which re-derives the control set. void defineControls() override { - controls_.addTextArea("source", source_, sizeof(source_)); + // The script NAME, not the script. The text lives in a file the UI loads, edits and + // saves through /api/file — so a module costs ~32 bytes here instead of a resident + // kilobyte, and a script is bounded by the filesystem rather than by this array. + controls_.addText("script", script_, sizeof(script_)); // Every control the script declared. System variables (`width`, `height`, `depth`, `t`) // are not controls and never appear here, so there is nothing to filter out. uint8_t n = 0; @@ -39,13 +43,10 @@ class MoonLiveEffect : public EffectBase { for (uint8_t i = 0; i < n; i++) { uint8_t* slot = engine_.controlSlot(decls[i].offset); if (!slot) continue; // engine not compiled yet (first sweep) — controls appear after prepare - // The declared name is a span into source_ (not NUL-terminated); copy it into a stable - // member pool so the control descriptor's borrowed name pointer stays valid. The compiler - // rejects names ≥ kMaxControlName, so the full name always fits — no truncation, no - // distinct-names-collapsing-to-the-same-prefix collision. - std::memcpy(ctrlNames_[i], decls[i].name, decls[i].nameLen); - ctrlNames_[i][decls[i].nameLen] = '\0'; - controls_.addUint8(ctrlNames_[i], *slot, decls[i].min, decls[i].max); + // The engine owns its declared names (MoonLive::compile copies them out of the + // source before the text is freed), so the descriptor can borrow that pointer + // directly — a second per-binding pool would be the same fact in two places. + controls_.addUint8(decls[i].name, *slot, decls[i].min, decls[i].max); } } @@ -68,10 +69,12 @@ class MoonLiveEffect : public EffectBase { // would be a second, disagreeing answer: set it to 16 on an 8x8 panel and the effect draws // off the edge. The compiler reserves the name, so that cannot happen. moonlive::resetPrintBudget(); - if (engine_.compile(source_, moonlive::lightBuiltins(), moonlive::effectSysVars())) { + const char* err = nullptr; + if (moonlive::compileScriptFile(engine_, script_, moonlive::lightBuiltins(), + moonlive::effectSysVars(), err)) { clearStatus(); } else { - setStatus(engine_.error(), Severity::Error); + setStatus(err, Severity::Error); } // The compile re-derives the declared-control set, so rebuild the control list to surface // it (the same rebuildControls() pattern NetworkModule uses when a state change reshapes @@ -104,9 +107,11 @@ class MoonLiveEffect : public EffectBase { /// Replace the script. The next prepare() compiles it — the same path a UI edit takes, so a /// test and a user exercise identical code. - void setSource(const char* s) { - if (!s) return; - std::snprintf(source_, sizeof(source_), "%s", s); + /// Point the module at a script in the shared script directory. The file itself is written by + /// the UI (or the File Manager); this only says WHICH one, and the next prepare() compiles it. + void setScript(const char* name) { + if (!name) return; + std::snprintf(script_, sizeof(script_), "%s", name); } private: @@ -123,15 +128,13 @@ class MoonLiveEffect : public EffectBase { } - char source_[moonlive::kMaxScriptBytes] = "setRGB(random16(256), random16(256), random16(256), random16(256));"; + // A fresh card starts with NO script: it reports "no script" and renders nothing until one + // is named. Naming a default here would make every new module compile the same effect. + char script_[32] = ""; // 512 fits a multi-line // multi-control script (a decl per control + the // statement); grow-on-demand is backlogged for the // bigger Ripples-class scripts of later stages. - // Stable NUL-terminated copies of the script-declared control names (the control descriptor - // borrows the pointer; the decl span into source_ is not NUL-terminated). Sized to the - // compiler's name limit so a name always fits without truncation. - char ctrlNames_[moonlive::kMaxCtrls][moonlive::kMaxControlName] = {}; }; } // namespace mm diff --git a/src/light/moonlive/MoonLiveLayout.h b/src/light/moonlive/MoonLiveLayout.h index 14cb80d3..048a45d7 100644 --- a/src/light/moonlive/MoonLiveLayout.h +++ b/src/light/moonlive/MoonLiveLayout.h @@ -1,6 +1,7 @@ #pragma once #include "core/moonlive/MoonLive.h" +#include "light/moonlive/MoonLiveScriptFile.h" #include "light/layouts/LayoutBase.h" #include "light/moonlive/MoonLiveBuiltins_light.h" #include @@ -39,7 +40,9 @@ class MoonLiveLayout : public LayoutBase { const char* tags() const override { return "📝"; } // scripted void defineControls() override { - controls_.addTextArea("source", source_, sizeof(source_)); + // The script NAME, not the script — the text lives in a file the UI loads and saves + // through /api/file. A module costs ~32 bytes here instead of a resident kilobyte. + controls_.addText("script", script_, sizeof(script_)); // Every control the SCRIPT declared — including any extents it loops over. A layout does not // RECEIVE a width: the pipeline derives its bounding box from the coordinates the layouts // actually place (Layouts::prepare, "max coordinate + 1 per axis"), so a width handed in @@ -50,9 +53,7 @@ class MoonLiveLayout : public LayoutBase { for (uint8_t i = 0; i < n; i++) { uint8_t* slot = engine_.controlSlot(decls[i].offset); if (!slot) continue; - std::memcpy(ctrlNames_[i], decls[i].name, decls[i].nameLen); - ctrlNames_[i][decls[i].nameLen] = '\0'; - controls_.addUint8(ctrlNames_[i], *slot, decls[i].min, decls[i].max); + controls_.addUint8(decls[i].name, *slot, decls[i].min, decls[i].max); } } @@ -89,9 +90,11 @@ class MoonLiveLayout : public LayoutBase { } /// Replace the script. The next prepare() compiles it — the path a UI edit takes. - void setSource(const char* s) { - if (!s) return; - std::snprintf(source_, sizeof(source_), "%s", s); + /// Point the layout at a script in the shared script directory; the next prepare() compiles it. + void setScript(const char* name) { + if (!name) return; + std::snprintf(script_, sizeof(script_), "%s", name); + compiledHash_ = 0; // a different file: whatever was compiled is not it } private: @@ -112,15 +115,22 @@ class MoonLiveLayout : public LayoutBase { /// 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 + if (engine_.ok() && compiledHash_ != 0) return; // already current for this script auto* self = const_cast(this); moonlive::resetPrintBudget(); // A layout is the one script with no layer to ask, so it gets the clock and nothing else: // it names its own size controls, and `x`/`y` stay free as ordinary loop counters. - if (self->engine_.compile(source_, moonlive::lightBuiltins(), moonlive::layoutSysVars())) + const char* err = nullptr; + uint32_t hash = 0; + if (moonlive::compileScriptFile(self->engine_, script_, moonlive::lightBuiltins(), + moonlive::layoutSysVars(), err, &hash)) { self->clearStatus(); - else self->setStatus(self->engine_.error(), Severity::Error); - std::snprintf(self->compiled_, sizeof(compiled_), "%s", source_); + } else { + self->setStatus(err, Severity::Error); + } + // The CONTENT hash, not a copy of the text: 4 bytes to answer "is what I compiled still what + // the file says", which is all the rebuild check ever needed. 0 means "nothing compiled". + self->compiledHash_ = hash; self->setDynamicBytes(engine_.heapBytes()); } @@ -151,24 +161,16 @@ class MoonLiveLayout : public LayoutBase { mutable moonlive::MoonLive engine_; - // Default script — a grid, the layout almost every panel is. The nested loop and the index - // arithmetic are the whole definition, which is the case for scripting a layout at all. - char source_[moonlive::kMaxScriptBytes] = - "uint8_t cols = 16; // @control 1..64\n" - "uint8_t rows = 16; // @control 1..64\n" - "for (y = 0; y < rows; y = y + 1) {\n" - " for (x = 0; x < cols; x = x + 1) {\n" - " addLight(x, y, 0);\n" - " }\n" - "}"; - - // The source the loaded program was built from, so compile() is a no-op when current. - // sizeof(source_), never a literal: a copy too small to hold source_ truncates, never - // compares equal, and the mapping rebuilds every frame — the blank-screen loop this - // comparison exists to prevent. - mutable char compiled_[sizeof(source_)] = {}; - - char ctrlNames_[moonlive::kMaxCtrls][moonlive::kMaxControlName] = {}; + // The script's FILE NAME, inside the shared script directory. Empty on a fresh card: it reports + // "no script" and places no lights until one is named, rather than every new layout compiling + // the same default grid. + char script_[32] = ""; + + // FNV-1a of the text the loaded program was built from — 4 bytes in place of a second copy of + // the source. Non-zero means "this engine holds a compiled program for that content"; 0 means + // nothing is compiled, which is what setScript() restores when the file changes. + mutable uint32_t compiledHash_ = 0; + }; } // namespace mm diff --git a/src/light/moonlive/MoonLiveModifier.h b/src/light/moonlive/MoonLiveModifier.h index a7e9628e..56c299b5 100644 --- a/src/light/moonlive/MoonLiveModifier.h +++ b/src/light/moonlive/MoonLiveModifier.h @@ -1,6 +1,7 @@ #pragma once #include "core/moonlive/MoonLive.h" +#include "light/moonlive/MoonLiveScriptFile.h" #include "light/modifiers/ModifierBase.h" #include "light/moonlive/MoonLiveBuiltins_light.h" #include @@ -43,7 +44,9 @@ class MoonLiveModifier : public ModifierBase { const char* tags() const override { return "📝"; } // scripted void defineControls() override { - controls_.addTextArea("source", source_, sizeof(source_)); + // The script NAME, not the script — the text lives in a file the UI loads and saves + // through /api/file. A module costs ~32 bytes here instead of a resident kilobyte. + controls_.addText("script", script_, sizeof(script_)); // Every control the script declared. System variables (`x`/`y`/`z`, `width`/`height`/ // `depth`, `t`) are not controls and never appear here, so there is nothing to filter out. uint8_t n = 0; @@ -51,9 +54,7 @@ class MoonLiveModifier : public ModifierBase { for (uint8_t i = 0; i < n; i++) { uint8_t* slot = engine_.controlSlot(decls[i].offset); if (!slot) continue; - std::memcpy(ctrlNames_[i], decls[i].name, decls[i].nameLen); - ctrlNames_[i][decls[i].nameLen] = '\0'; - controls_.addUint8(ctrlNames_[i], *slot, decls[i].min, decls[i].max); + controls_.addUint8(decls[i].name, *slot, decls[i].min, decls[i].max); } } @@ -68,11 +69,13 @@ class MoonLiveModifier : public ModifierBase { // defines — the binding writes their slots per call, and the compiler reserves the names so // a script cannot declare one and shadow the value it is being handed. moonlive::resetPrintBudget(); - if (engine_.compile(source_, moonlive::lightBuiltins(), - moonlive::modifierSysVars())) { + const char* err = nullptr; + uint32_t hash = 0; + if (moonlive::compileScriptFile(engine_, script_, moonlive::lightBuiltins(), + moonlive::modifierSysVars(), err, &hash)) { clearStatus(); } else { - setStatus(engine_.error(), Severity::Error); + setStatus(err, Severity::Error); } rebuildControls(); setDynamicBytes(engine_.heapBytes()); @@ -82,8 +85,8 @@ class MoonLiveModifier : public ModifierBase { // again: setting the flag unconditionally makes the two call each other forever, the // mapping is rebuilt every frame, and the fixture renders nothing at all. Comparing the // compiled source is what breaks that cycle. - if (std::strcmp(source_, compiled_) != 0) { - std::snprintf(compiled_, sizeof(compiled_), "%s", source_); + if (hash != compiledHash_) { + compiledHash_ = hash; needsRebuild_ = true; } } @@ -143,15 +146,16 @@ class MoonLiveModifier : public ModifierBase { // treated as a first compile. Keeping it made a disabled-then-re-enabled modifier inert — // the Layer folds while the engine is empty, then prepare() recompiles, sees the same // source, and never asks for the rebuild that would apply it. - compiled_[0] = '\0'; + compiledHash_ = 0; ModifierBase::release(); } /// Replace the script. The next prepare() compiles it — the same path a UI edit takes, so a /// test and a user exercise identical code. - void setSource(const char* s) { - if (!s) return; - std::snprintf(source_, sizeof(source_), "%s", s); + void setScript(const char* name) { + if (!name) return; + std::snprintf(script_, sizeof(script_), "%s", name); + compiledHash_ = 0; // a different file: whatever was compiled is not it } private: @@ -160,15 +164,16 @@ class MoonLiveModifier : public ModifierBase { // Default script — a mirror on x. Chosen because it is instantly readable on a bench strand // (the pattern runs the other way) and is a modifier people actually reach for, so a working // binding looks like something rather than like nothing. - char source_[moonlive::kMaxScriptBytes] = "setXYZ(0, width - 1 - x, y, z);"; + // The script's FILE NAME, inside the shared script directory. Empty on a fresh card: it reports + // "no script" and passes coordinates through untouched until one is named. + char script_[32] = ""; - // The source the CURRENT mapping was built from; a rebuild is needed only when it changes. - // sizeof(source_), never a literal: this is the string compared to decide whether to rebuild, - // so a copy too small to hold source_ truncates, never matches, and the mapping rebuilds every - // frame — the blank-screen loop the comparison exists to prevent. - char compiled_[sizeof(source_)] = {}; + // FNV-1a of the text the loaded program was built from — 4 bytes in place of a second copy of + // the source. It answers the one question the rebuild check ever asked ("did this change"), and + // an unconditional rebuild here would make prepare() and the Layer's rebuild call each other + // forever, which is the blank-fixture loop the comparison exists to prevent. + uint32_t compiledHash_ = 0; - char ctrlNames_[moonlive::kMaxCtrls][moonlive::kMaxControlName] = {}; bool needsRebuild_ = false; // a recompile happened; the Layer's mapping is stale Coord3D box_{0, 0, 0}; // the logical box, from modifyLogicalSize diff --git a/src/light/moonlive/MoonLiveScriptFile.h b/src/light/moonlive/MoonLiveScriptFile.h new file mode 100644 index 00000000..88014fb5 --- /dev/null +++ b/src/light/moonlive/MoonLiveScriptFile.h @@ -0,0 +1,74 @@ +#pragma once + +#include "core/moonlive/MoonLive.h" +#include "platform/platform.h" + +#include +#include + +namespace mm::moonlive { + +/// Where a scripted module's `.mlv` file lives. One fixed directory, the way `/.config` holds +/// persisted state: a module stores a NAME, not a path, so it cannot reach outside this folder and +/// the File Manager has one obvious place to look. +inline constexpr const char* kScriptDir = "/moonlive"; + +/// The largest script the loader will read into RAM at once. Not a language limit — the buffer is +/// sized to the FILE and freed the moment the compile ends — but a bound so a stray large file +/// cannot ask a 320 KB device for an allocation it will not survive. +inline constexpr long kScriptFileMax = 16384; + +/// Read `/` and compile it. The source lives in a right-sized heap buffer for the +/// duration of the compile and is freed before returning, so a module holds a filename (~32 B) and +/// the emitted code — never the script text. That is the whole point: the fixed per-module arrays +/// this replaces cost ~2 KB EACH, resident whether or not a script was loaded. +/// +/// Returns true when the script compiled. On any failure `err` names it, in the words a user needs: +/// which file, and what was wrong with it. +/// FNV-1a over the script text. A caller that must know "did this change" keeps 4 bytes rather than +/// a second copy of the source — which is the whole reason the text is not resident any more. +inline uint32_t scriptHash(const char* s, size_t len) { + uint32_t h = 2166136261u; + for (size_t i = 0; i < len; i++) { h ^= static_cast(s[i]); h *= 16777619u; } + return h; +} + +/// As compileScriptFile, and additionally reports the source's hash so a caller can tell a changed +/// script from an unchanged one without holding the text. +inline bool compileScriptFile(MoonLive& engine, const char* name, + const BuiltinTable& builtins, const SysVarTable& sysvars, + const char*& err, uint32_t* hashOut = nullptr) { + // The script directory must exist before anything can be SAVED into it, and on a fresh device + // nothing has created it yet — the write endpoint does not make parent directories, so a first + // save would fail with nowhere obvious to look. Creating it here (mkdir -p, a no-op when it is + // already there) means naming a script is enough to make the folder appear. + platform::fsMkdir(kScriptDir); + + if (!name || !name[0]) { err = "no script — set the script name"; return false; } + + char path[96]; + std::snprintf(path, sizeof(path), "%s/%s", kScriptDir, name); + + const long size = platform::fsSize(path); + if (size < 0) { err = "script not found"; return false; } + if (size == 0) { err = "script is empty"; return false; } + if (size > kScriptFileMax) { err = "script too large"; return false; } + + // +1 for the NUL the lexer reads as End. fsRead null-terminates on success, but the buffer has + // to have room for it. + char* text = static_cast(platform::alloc(static_cast(size) + 1)); + if (!text) { err = "no memory for the script"; return false; } + + const int read = platform::fsRead(path, text, static_cast(size) + 1); + if (read <= 0) { platform::free(text); err = "script could not be read"; return false; } + + if (hashOut) *hashOut = scriptHash(text, static_cast(read)); + const bool ok = engine.compile(text, builtins, sysvars); + if (!ok) err = engine.error(); + // Freed on BOTH paths, before returning: the text has done its job either way, and a failed + // compile is exactly when a device can least afford to leak. + platform::free(text); + return ok; +} + +} // namespace mm::moonlive diff --git a/src/platform/desktop/moonlive_lower_host.cpp b/src/platform/desktop/moonlive_lower_host.cpp index b3264516..3e393bfc 100644 --- a/src/platform/desktop/moonlive_lower_host.cpp +++ b/src/platform/desktop/moonlive_lower_host.cpp @@ -52,7 +52,9 @@ size_t lowerToBytes(const IrProgram& ir, uint8_t* out, size_t cap) { return labels[id]; }; - for (uint8_t i = 0; i < ir.count; i++) { + // uint16_t, matching IrProgram::count: the op array is sized to the script now, so a + // uint8_t counter wrapped at 256 ops and looped forever instead of emitting. + for (uint16_t i = 0; i < ir.count; i++) { const IrInst& op = ir.ops[i]; switch (op.op) { case IrOp::Const: a.movImm(reg(op.dst), op.imm); break; diff --git a/src/platform/desktop/platform_desktop.cpp b/src/platform/desktop/platform_desktop.cpp index 5d3fa4ed..c4126b82 100644 --- a/src/platform/desktop/platform_desktop.cpp +++ b/src/platform/desktop/platform_desktop.cpp @@ -1603,6 +1603,10 @@ uint8_t* parlioWs2812Buffer(const ParlioWs2812Handle& h, uint8_t buffer) { size_t parlioWs2812BufferCapacity(const ParlioWs2812Handle& h) { return h.impl ? static_cast(h.impl)->capacity : 0; } +// The desktop host emulates the bus in ordinary memory, so there is no single-transfer ceiling to +// declare — 0 is the "no bound" contract dmaBudgetBytes() reads, matching every other host-side +// Parlio stub here. +size_t parlioMaxTransferBytes() { return 0; } bool parlioWs2812Transmit(ParlioWs2812Handle& h, uint8_t buffer, size_t bytes) { return h.impl && static_cast(h.impl)->transmit(buffer, bytes); } diff --git a/src/platform/esp32/moonlive_lower_riscv.cpp b/src/platform/esp32/moonlive_lower_riscv.cpp index 00c8cac9..7b89fd46 100644 --- a/src/platform/esp32/moonlive_lower_riscv.cpp +++ b/src/platform/esp32/moonlive_lower_riscv.cpp @@ -46,7 +46,9 @@ size_t lowerToBytes(const IrProgram& ir, uint8_t* out, size_t cap) { return labels[id]; }; - for (uint8_t i = 0; i < ir.count; i++) { + // uint16_t, matching IrProgram::count: the op array is sized to the script now, so a + // uint8_t counter wrapped at 256 ops and looped forever instead of emitting. + for (uint16_t i = 0; i < ir.count; i++) { const IrInst& op = ir.ops[i]; switch (op.op) { case IrOp::Const: a.movImm(reg(op.dst), op.imm); break; diff --git a/src/platform/esp32/moonlive_lower_xtensa.cpp b/src/platform/esp32/moonlive_lower_xtensa.cpp index 9c4224c4..cff10d0b 100644 --- a/src/platform/esp32/moonlive_lower_xtensa.cpp +++ b/src/platform/esp32/moonlive_lower_xtensa.cpp @@ -47,7 +47,9 @@ size_t lowerToBytes(const IrProgram& ir, uint8_t* out, size_t cap) { return labels[id]; }; - for (uint8_t i = 0; i < ir.count; i++) { + // uint16_t, matching IrProgram::count: the op array is sized to the script now, so a + // uint8_t counter wrapped at 256 ops and looped forever instead of emitting. + for (uint16_t i = 0; i < ir.count; i++) { const IrInst& op = ir.ops[i]; switch (op.op) { case IrOp::Const: a.movImm(reg(op.dst), op.imm); break; diff --git a/src/platform/esp32/platform_esp32_parlio.cpp b/src/platform/esp32/platform_esp32_parlio.cpp index ba79d44c..cb53673d 100644 --- a/src/platform/esp32/platform_esp32_parlio.cpp +++ b/src/platform/esp32/platform_esp32_parlio.cpp @@ -265,6 +265,8 @@ uint8_t* parlioWs2812Buffer(const ParlioWs2812Handle& h, uint8_t buffer) { return (st && buffer < 2) ? st->buf[buffer] : nullptr; } +size_t parlioMaxTransferBytes() { return kParlioMaxTransferBytes; } + size_t parlioWs2812BufferCapacity(const ParlioWs2812Handle& h) { auto* st = static_cast(h.impl); return st ? st->cap : 0; @@ -410,6 +412,7 @@ bool parlioWs2812Init(ParlioWs2812Handle&, const uint16_t*, uint8_t, uint32_t, s } uint8_t* parlioWs2812Buffer(const ParlioWs2812Handle&, uint8_t) { return nullptr; } size_t parlioWs2812BufferCapacity(const ParlioWs2812Handle&) { return 0; } +size_t parlioMaxTransferBytes() { return 0; } // no Parlio here → no bound (the budget-0 contract) bool parlioWs2812Transmit(ParlioWs2812Handle&, uint8_t, size_t) { return false; } bool parlioWs2812Wait(ParlioWs2812Handle&, uint8_t, uint32_t) { return true; } uint32_t parlioWs2812LastTransmitUs(const ParlioWs2812Handle&) { return 0; } diff --git a/src/platform/platform.h b/src/platform/platform.h index a8d2914c..d86484d0 100644 --- a/src/platform/platform.h +++ b/src/platform/platform.h @@ -1165,6 +1165,11 @@ bool parlioWs2812Init(ParlioWs2812Handle& h, const uint16_t* dataPins, uint8_t* parlioWs2812Buffer(const ParlioWs2812Handle& h, uint8_t buffer); size_t parlioWs2812BufferCapacity(const ParlioWs2812Handle& h); +// The most bytes Parlio can send in ONE transfer — a HARDWARE ceiling, not a heap budget, so it +// needs no handle and holds before anything is allocated. A caller sizes a frame against it to +// refuse an impossible configuration up front instead of failing the bus init. +size_t parlioMaxTransferBytes(); + // Start the autonomous DMA transfer of buffer `buffer`'s first `bytes`; pair // with parlioWs2812Wait on the SAME buffer. No refill deadline once started // (single-shot, not the loop-transmission mode Parlio also offers). diff --git a/test/unit/core/unit_moonlive_compiler.cpp b/test/unit/core/unit_moonlive_compiler.cpp index 1ce3692b..62ad5c4c 100644 --- a/test/unit/core/unit_moonlive_compiler.cpp +++ b/test/unit/core/unit_moonlive_compiler.cpp @@ -310,6 +310,29 @@ TEST_CASE("a for loop's condition and step must name the loop variable") { #endif } +// The op array is sized to the script, so `count` is a uint16_t — and every loop over it has to be +// one too. A uint8_t counter wrapped at 256 ops and spun forever, which on a device is a watchdog +// reset from a script that merely got long. Found by bisecting: 60 statements fine, 80 hung. +TEST_CASE("a long script compiles or refuses, but never spins") { + uint8_t out[16384]; + // A long ARITHMETIC chain, not many statements: each `+ 1` is one cheap op, so this passes 256 + // IR ops while staying inside the code buffer. Repeated statements hit the code ceiling first + // and return before the wrap, which is why they do not pin this. + std::string many = "addLight(1"; + for (int i = 0; i < 200; i++) many += " + 1"; + many += ", 0, 0);"; + auto r = moonlive::compileSource(many.c_str(), kTable, kSys, out, sizeof(out)); + // Either answer is fine — what is NOT fine is never returning, which is what this pins. + CHECK((r.ok || std::strlen(r.error) > 0)); + + // And the sanity bound still refuses a runaway rather than trying to allocate for it. + std::string absurd; + for (int i = 0; i < 3000; i++) absurd += "addLight(1, 0, 0);"; + auto big = moonlive::compileSource(absurd.c_str(), kTable, kSys, out, sizeof(out)); + CHECK_FALSE(big.ok); + CHECK(std::string(big.error) == "script too large"); +} + TEST_CASE("compileSource: malformed control declarations fail with a diagnostic, never crash") { uint8_t out[768]; const char* bad[] = { diff --git a/test/unit/light/MoonLiveScriptFixture.h b/test/unit/light/MoonLiveScriptFixture.h new file mode 100644 index 00000000..87ee9f4d --- /dev/null +++ b/test/unit/light/MoonLiveScriptFixture.h @@ -0,0 +1,33 @@ +#pragma once + +#include "doctest.h" +#include "light/moonlive/MoonLiveScriptFile.h" +#include "platform/platform.h" + +#include +#include +#include +#include + +/// Put a script on the filesystem and return its NAME. +/// +/// A scripted module holds a file name now, not the text — so a test that wants to compile +/// something has to write the file first, which is the same path the UI takes when it saves an +/// edit. Testing through the file is the point: it exercises what actually ships, rather than a +/// text-only path production never uses. +/// +/// Each call gets a fresh name, so tests that compile several scripts do not collide. +/// Returns a name valid for the caller's thread until its next call. THREAD-LOCAL, not static: the +/// concurrency test compiles scripts from two threads at once, and a shared counter and buffer would +/// hand both threads the same name — each would then compile the other's script. +inline const char* mmWriteScript(const char* text) { + static std::atomic counter{0}; + thread_local char name[32]; + std::snprintf(name, sizeof(name), "t%d.mlv", ++counter); + + char path[96]; + std::snprintf(path, sizeof(path), "%s/%s", mm::moonlive::kScriptDir, name); + mm::platform::fsMkdir(mm::moonlive::kScriptDir); + REQUIRE(mm::platform::fsWriteAtomic(path, text, std::strlen(text))); + return name; +} diff --git a/test/unit/light/unit_MoonLiveLayout.cpp b/test/unit/light/unit_MoonLiveLayout.cpp index 907f8350..6a23e922 100644 --- a/test/unit/light/unit_MoonLiveLayout.cpp +++ b/test/unit/light/unit_MoonLiveLayout.cpp @@ -11,6 +11,7 @@ // broken script leaves an empty fixture rather than taking the pipeline down. #include "doctest.h" +#include "MoonLiveScriptFixture.h" #include "light/moonlive/MoonLiveLayout.h" #include "light/moonlive/MoonLiveBuiltins_light.h" #include "platform/platform.h" @@ -38,7 +39,7 @@ namespace { std::vector place(const char* script) { MoonLiveLayout l; l.defineControls(); - if (script) l.setSource(script); + if (script) l.setScript(mmWriteScript(script)); l.prepare(); std::vector out; @@ -71,10 +72,10 @@ TEST_CASE("the light count is known before any coordinate is asked for") { // forEachCoord. A count that came from the walk would arrive too late to be useful. MoonLiveLayout l; l.defineControls(); - l.setSource("uint8_t cols = 5; // @control 1..64\n" + l.setScript(mmWriteScript("uint8_t cols = 5; // @control 1..64\n" "uint8_t rows = 3; // @control 1..64\n" "for (yy = 0; yy < rows; yy = yy + 1) {" - " for (xx = 0; xx < cols; xx = xx + 1) { addLight(xx, yy, 0); } }"); + " for (xx = 0; xx < cols; xx = xx + 1) { addLight(xx, yy, 0); } }")); l.prepare(); CHECK(l.lightCount() == 15); // answered without anyone calling forEachCoord } @@ -83,7 +84,7 @@ TEST_CASE("the count and the coordinates always agree, because one script produc // The property SphereLayout names: count and emit run the same code, so they cannot drift. MoonLiveLayout l; l.defineControls(); - l.setSource("for (i = 0; i < 7; i = i + 1) { addLight(i, 0, 0); }"); + l.setScript(mmWriteScript("for (i = 0; i < 7; i = i + 1) { addLight(i, 0, 0); }")); l.prepare(); std::vector seen; @@ -99,7 +100,7 @@ TEST_CASE("a scripted layout allocates nothing, like every other layout") { // have. The script calls out per light instead, so the only heap here is the compiled program. MoonLiveLayout l; l.defineControls(); - l.setSource("for (i = 0; i < 4096; i = i + 1) { addLight(i, 0, 0); }"); + l.setScript(mmWriteScript("for (i = 0; i < 4096; i = i + 1) { addLight(i, 0, 0); }")); l.prepare(); CHECK(l.lightCount() == 4096); // dynamicBytes is the JIT'd program only — no coordinate storage grows with the light count. @@ -129,7 +130,7 @@ TEST_CASE("a broken script leaves an empty fixture rather than taking the pipeli // fixture reports no lights, the module carries the diagnostic, and the device keeps running. MoonLiveLayout l; l.defineControls(); - l.setSource("for (i = 0; i < 4; i = i + 1) { addLight(i, i"); // unclosed + l.setScript(mmWriteScript("for (i = 0; i < 4; i = i + 1) { addLight(i, i")); // unclosed l.prepare(); CHECK(l.lightCount() == 0); CHECK(l.severity() == MoonModule::Severity::Error); @@ -139,11 +140,11 @@ TEST_CASE("editing the script changes the fixture") { // The live-edit loop: the same module, a new script, a different physical shape. MoonLiveLayout l; l.defineControls(); - l.setSource("for (i = 0; i < 4; i = i + 1) { addLight(i, 0, 0); }"); + l.setScript(mmWriteScript("for (i = 0; i < 4; i = i + 1) { addLight(i, 0, 0); }")); l.prepare(); CHECK(l.lightCount() == 4); - l.setSource("for (i = 0; i < 2; i = i + 1) { addLight(i, 0, 0); }"); + l.setScript(mmWriteScript("for (i = 0; i < 2; i = i + 1) { addLight(i, 0, 0); }")); l.prepare(); CHECK(l.lightCount() == 2); } @@ -173,7 +174,7 @@ TEST_CASE("the scripts the documentation shows all compile") { for (const char* s : fromDocs) { MoonLiveLayout l; l.defineControls(); - l.setSource(s); + l.setScript(mmWriteScript(s)); l.prepare(); INFO("script: " << s); CHECK(l.severity() != MoonModule::Severity::Error); @@ -188,7 +189,7 @@ TEST_CASE("the scripts the documentation shows all compile") { TEST_CASE("a layout answers count and coordinates every time it is asked") { MoonLiveLayout l; l.defineControls(); - l.setSource("for (i = 0; i < 6; i = i + 1) { addLight(i, 0, 0); }"); + l.setScript(mmWriteScript("for (i = 0; i < 6; i = i + 1) { addLight(i, 0, 0); }")); l.prepare(); CHECK(l.lightCount() == 6); @@ -219,7 +220,7 @@ TEST_CASE("a subtraction feeding a loop bound produces the whole value") { MoonLiveLayout l; l.defineControls(); // 10 - 4 must be 6 lights. A widened -1 makes the bound enormous and the count is not 6. - l.setSource("for (i = 0; i < 10 - 4; i = i + 1) { addLight(i, 0, 0); }"); + l.setScript(mmWriteScript("for (i = 0; i < 10 - 4; i = i + 1) { addLight(i, 0, 0); }")); l.prepare(); CHECK(l.lightCount() == 6); @@ -243,22 +244,22 @@ TEST_CASE("a subtraction feeding a loop bound produces the whole value") { TEST_CASE("a scripted control keeps its live value when the script is edited") { MoonLiveLayout l; l.defineControls(); - l.setSource("uint8_t cols = 16; // @control 1..64\n" - "for (i = 0; i < cols; i = i + 1) { addLight(i, 0, 0); }"); + l.setScript(mmWriteScript("uint8_t cols = 16; // @control 1..64\n" + "for (i = 0; i < cols; i = i + 1) { addLight(i, 0, 0); }")); l.prepare(); CHECK(l.lightCount() == 16); // A second script declaring cols at the same offset inherits the live 16, not its own 8. - l.setSource("uint8_t cols = 8; // @control 1..64\n" - "for (i = 0; i < cols; i = i + 1) { addLight(i, 1, 0); }"); + l.setScript(mmWriteScript("uint8_t cols = 8; // @control 1..64\n" + "for (i = 0; i < cols; i = i + 1) { addLight(i, 1, 0); }")); l.prepare(); CHECK(l.lightCount() == 16); // A script whose first control is a NEW slot gets its own initialiser: nothing to inherit. - l.setSource("uint8_t cols = 16; // @control 1..64\n" + l.setScript(mmWriteScript("uint8_t cols = 16; // @control 1..64\n" "uint8_t rows = 3; // @control 1..64\n" "for (yy = 0; yy < rows; yy = yy + 1) {" - " for (xx = 0; xx < cols; xx = xx + 1) { addLight(xx, yy, 0); } }"); + " for (xx = 0; xx < cols; xx = xx + 1) { addLight(xx, yy, 0); } }")); l.prepare(); CHECK(l.lightCount() == 48); // 16 inherited, rows 3 its own } @@ -293,7 +294,7 @@ TEST_CASE("a loop counter survives the body that uses it") { SUBCASE("through a call — addLight") { MoonLiveLayout l; l.defineControls(); - l.setSource("for (i = 0; i < 6; i = i + 1) { addLight(i, i, 0); }"); + l.setScript(mmWriteScript("for (i = 0; i < 6; i = i + 1) { addLight(i, i, 0); }")); l.prepare(); CHECK(l.lightCount() == 6); // a clobbered counter gives some other number } @@ -327,7 +328,7 @@ TEST_CASE("a loop counter survives the body that uses it") { TEST_CASE("a stray character in a for header is rejected, not spun on") { MoonLiveLayout l; l.defineControls(); - l.setSource("for (i = 0; i < 4; i = i @ 1) { addLight(i, 0, 0); }"); + l.setScript(mmWriteScript("for (i = 0; i < 4; i = i @ 1) { addLight(i, 0, 0); }")); l.prepare(); // must return — a hang fails by timeout CHECK(l.severity() == MoonModule::Severity::Error); CHECK(l.lightCount() == 0); @@ -346,7 +347,7 @@ TEST_CASE("two threads can run scripts at once without stealing each other's sin l.defineControls(); char src[128]; std::snprintf(src, sizeof(src), "for (i = 0; i < %d; i = i + 1) { addLight(i, 0, 0); }", cols); - l.setSource(src); + l.setScript(mmWriteScript(src)); l.prepare(); for (int r = 0; r < reps; r++) if (l.lightCount() != static_cast(cols)) return false; @@ -369,15 +370,15 @@ TEST_CASE("two threads can run scripts at once without stealing each other's sin TEST_CASE("a scripted layout reports every heap byte it holds, compiled or not") { MoonLiveLayout l; l.defineControls(); - l.setSource("uint8_t cols = 4; // @control 1..64\n" - "for (i = 0; i < cols; i = i + 1) { addLight(i, 0, 0); }"); + l.setScript(mmWriteScript("uint8_t cols = 4; // @control 1..64\n" + "for (i = 0; i < cols; i = i + 1) { addLight(i, 0, 0); }")); l.prepare(); const size_t compiled = l.dynamicBytes(); CHECK(compiled > 0); CHECK(l.lightCount() == 4); // A broken script frees the code but keeps the arena, so the figure drops without reaching zero. - l.setSource("for (i = 0; i < 4; i = i + 1) { addLight(i, i"); // unclosed + l.setScript(mmWriteScript("for (i = 0; i < 4; i = i + 1) { addLight(i, i")); // unclosed l.prepare(); CHECK(l.severity() == MoonModule::Severity::Error); CHECK(l.dynamicBytes() < compiled); // the code block is gone @@ -395,9 +396,12 @@ TEST_CASE("a scripted layout reports every heap byte it holds, compiled or not") TEST_CASE("a layout that changes size mid-build cannot overrun the mapping") { MoonLiveLayout layout; layout.defineControls(); - layout.setSource("uint8_t cols = 4; // @control 1..64\n" - "for (i = 0; i < cols; i = i + 1) { addLight(i, 0, 0); }"); + layout.setScript(mmWriteScript("uint8_t cols = 4; // @control 1..64\n" + "for (i = 0; i < cols; i = i + 1) { addLight(i, 0, 0); }")); layout.prepare(); + // The script's own controls (`cols`) exist only once it has COMPILED, and a module starts with + // no script now — so the control list has to be rebuilt after prepare() for setWidth to find it. + layout.rebuildControls(); mm::Layouts group; group.addChild(&layout); @@ -428,3 +432,4 @@ TEST_CASE("a layout that changes size mid-build cannot overrun the mapping") { } #endif // MM_MOONLIVE_HAS_HOST_JIT + diff --git a/test/unit/light/unit_MoonLiveModifier.cpp b/test/unit/light/unit_MoonLiveModifier.cpp index 263a2761..b9eb3297 100644 --- a/test/unit/light/unit_MoonLiveModifier.cpp +++ b/test/unit/light/unit_MoonLiveModifier.cpp @@ -8,6 +8,7 @@ // broken script degrades to a pass-through instead of taking the pipeline down. #include "doctest.h" +#include "MoonLiveScriptFixture.h" #include "light/moonlive/MoonLiveModifier.h" #include "light/moonlive/MoonLiveEffect.h" #include "platform/platform.h" @@ -34,7 +35,7 @@ Coord3D transform(const char* script, lengthType x, lengthType y, lengthType z, lengthType w = 255, lengthType h = 255, lengthType d = 1) { MoonLiveModifier m; m.defineControls(); - if (script) m.setSource(script); + if (script) m.setScript(mmWriteScript(script)); m.prepare(); Coord3D box{w, h, d}; m.modifyLogicalSize(box); // the Layer always does this before folding @@ -47,7 +48,7 @@ Coord3D transform(const char* script, lengthType x, lengthType y, lengthType z, TEST_CASE("a scripted modifier mirrors the pattern, the way a hand-written one would") { // The default script. A mirror is the shape that makes a working binding obvious on a bench // strand — the pattern simply runs the other way. - const Coord3D p = transform(nullptr, 10, 20, 0); + const Coord3D p = transform("setXYZ(0, width - 1 - x, y, z);", 10, 20, 0); CHECK(p.x == 244); // width(255) - 1 - 10 CHECK(p.y == 20); // untouched axes stay put CHECK(p.z == 0); @@ -84,7 +85,7 @@ TEST_CASE("a broken script leaves the pattern alone rather than taking the layer // the pipeline keeps rendering. MoonLiveModifier m; m.defineControls(); - m.setSource("setXYZ(0, x, y"); // no closing paren, no semicolon + m.setScript(mmWriteScript("setXYZ(0, x, y")); // no closing paren, no semicolon m.prepare(); Coord3D box{255, 255, 1}; m.modifyLogicalSize(box); @@ -109,15 +110,16 @@ TEST_CASE("editing the script changes the transform without a rebuild of the fir // The live-edit loop: the same module, a new script, a different mapping. MoonLiveModifier m; m.defineControls(); + m.setScript(mmWriteScript("setXYZ(0, width - 1 - x, y, z);")); m.prepare(); Coord3D box{255, 255, 1}; m.modifyLogicalSize(box); // the Layer hands every modifier its box before folding Coord3D a{10, 20, 0}; m.modifyLogical(a); - CHECK(a.x == 244); // the default mirror + CHECK(a.x == 244); // the mirror - m.setSource("setXYZ(0, x, y, z);"); + m.setScript(mmWriteScript("setXYZ(0, x, y, z);")); m.prepare(); Coord3D b{10, 20, 0}; @@ -163,7 +165,7 @@ TEST_CASE("folding a wall's worth of lights compiles the script once, not once p // an unchanged value across the whole fold proves no compile happened inside it. MoonLiveModifier m; m.defineControls(); - m.setSource("setXYZ(0, width - 1 - x, y, z);"); + m.setScript(mmWriteScript("setXYZ(0, width - 1 - x, y, z);")); m.prepare(); Coord3D box{255, 255, 1}; m.modifyLogicalSize(box); @@ -186,12 +188,12 @@ TEST_CASE("folding a wall's worth of lights compiles the script once, not once p TEST_CASE("editing a script asks the layer to rebuild its mapping") { MoonLiveModifier m; m.defineControls(); - m.setSource("setXYZ(0, x, y, z);"); + m.setScript(mmWriteScript("setXYZ(0, x, y, z);")); m.prepare(); CHECK(m.consumeNeedsRebuild() == true); // the first compile needs one too CHECK(m.consumeNeedsRebuild() == false); // and it is consumed, not sticky - m.setSource("setXYZ(0, 7 - x, y, z);"); + m.setScript(mmWriteScript("setXYZ(0, 7 - x, y, z);")); m.prepare(); CHECK(m.consumeNeedsRebuild() == true); // an edit asks again @@ -207,13 +209,13 @@ TEST_CASE("editing a script asks the layer to rebuild its mapping") { // be able to read the EXTENT it is folding within, and the default has to use it. TEST_CASE("the default script mirrors within the grid it is given, not a fixed 255") { // A 16-wide grid: x=0 must land on the far end of THAT grid, 15 — not 245. - const Coord3D p = transform(nullptr, 0, 0, 0, /*w=*/16, /*h=*/16, /*d=*/1); + const Coord3D p = transform("setXYZ(0, width - 1 - x, y, z);", 0, 0, 0, /*w=*/16, /*h=*/16, /*d=*/1); CHECK(p.x == 15); CHECK(p.y == 0); // Every coordinate has to stay inside the box, or the Layer discards it. for (lengthType i = 0; i < 16; i++) { - const Coord3D q = transform(nullptr, i, 0, 0, 16, 16, 1); + const Coord3D q = transform("setXYZ(0, width - 1 - x, y, z);", i, 0, 0, 16, 16, 1); CAPTURE(i); CHECK(q.x >= 0); CHECK(q.x < 16); @@ -240,7 +242,7 @@ TEST_CASE("a script that computes a position outside the grid leaves lights mapp // bytes cannot fail: draw::fill writes every byte itself, whatever the fold decided. MoonLiveModifier m; m.defineControls(); - m.setSource("setXYZ(0, x + 200, y, z);"); // deliberately off the end of a 16-wide grid + m.setScript(mmWriteScript("setXYZ(0, x + 200, y, z);")); // deliberately off the end of a 16-wide grid m.prepare(); Coord3D box{16, 16, 1}; m.modifyLogicalSize(box); @@ -273,6 +275,10 @@ TEST_CASE("a script that computes a position outside the grid leaves lights mapp TEST_CASE("re-preparing with an unchanged script does not ask for another rebuild") { MoonLiveModifier m; m.defineControls(); + // A module with no script compiles nothing and therefore asks for nothing — the rebuild request + // exists to APPLY a new transform, and there is none. Name one, so the first prepare has + // something to compile and the "unchanged" case below is the real question. + m.setScript(mmWriteScript("setXYZ(0, width - 1 - x, y, z);")); m.prepare(); CHECK(m.consumeNeedsRebuild() == true); // the first compile needs one @@ -284,7 +290,7 @@ TEST_CASE("re-preparing with an unchanged script does not ask for another rebuil CHECK(m.consumeNeedsRebuild() == false); // A real edit still asks. - m.setSource("setXYZ(0, y, x, z);"); + m.setScript(mmWriteScript("setXYZ(0, y, x, z);")); m.prepare(); CHECK(m.consumeNeedsRebuild() == true); } @@ -329,7 +335,7 @@ TEST_CASE("a subtraction produces the whole value, not just its low byte") { TEST_CASE("a for loop runs its body once per step") { MoonLiveModifier m; m.defineControls(); - m.setSource("for (i = 0; i < 4; i = i + 1) { print(i); } setXYZ(0, x, y, z);"); + m.setScript(mmWriteScript("for (i = 0; i < 4; i = i + 1) { print(i); } setXYZ(0, x, y, z);")); m.prepare(); CHECK(m.severity() != MoonModule::Severity::Error); // it compiles at all Coord3D box{16, 16, 1}; m.modifyLogicalSize(box); @@ -342,7 +348,7 @@ TEST_CASE("a loop over an empty range runs its body no times") { // The entry guard: `i < 0` must skip the body entirely rather than wrap and run forever. MoonLiveModifier m; m.defineControls(); - m.setSource("for (i = 0; i < 0; i = i + 1) { print(99); } setXYZ(0, x, y, z);"); + m.setScript(mmWriteScript("for (i = 0; i < 0; i = i + 1) { print(99); } setXYZ(0, x, y, z);")); m.prepare(); CHECK(m.severity() != MoonModule::Severity::Error); Coord3D box{16, 16, 1}; m.modifyLogicalSize(box); @@ -354,8 +360,8 @@ TEST_CASE("a loop over an empty range runs its body no times") { TEST_CASE("loops nest, which is what placing a grid of lights needs") { MoonLiveModifier m; m.defineControls(); - m.setSource("for (a = 0; a < 2; a = a + 1) { for (b = 0; b < 2; b = b + 1) { print(a); } }" - " setXYZ(0, x, y, z);"); + m.setScript(mmWriteScript("for (a = 0; a < 2; a = a + 1) { for (b = 0; b < 2; b = b + 1) { print(a); } }" + " setXYZ(0, x, y, z);")); m.prepare(); CHECK(m.severity() != MoonModule::Severity::Error); Coord3D box{16, 16, 1}; m.modifyLogicalSize(box); @@ -379,7 +385,7 @@ TEST_CASE("a loop in an effect script paints every light it walks") { layer.setChannelsPerLight(3); auto* fx = new MoonLiveEffect(); fx->defineControls(); - fx->setSource("for (i = 0; i < 8; i = i + 1) { setRGB(i, i, 0, 0); }"); + fx->setScript(mmWriteScript("for (i = 0; i < 8; i = i + 1) { setRGB(i, i, 0, 0); }")); layer.addChild(fx); layouts.applyState(); layer.applyState(); From 97d004ff8a7db262712f5ae49af022e932a8acc8 Mon Sep 17 00:00:00 2001 From: ewowi Date: Wed, 12 Aug 2026 13:58:20 +0200 Subject: [PATCH 2/3] Stop a renamed script from running the old program MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hardware found what 1228 tests did not: naming a script never recompiled anything. The effect still asked whether the "source" control had changed - a control renamed to "script" - and the layout cached its compiled program behind a hash that a control write never cleared. Both held a new filename while running the previous script. Performance: desktop 127 us/tick (7874 fps), esp32 2151 us/tick (464 fps). Light domain - MoonLiveEffect::affectsPrepare tests "script". Found on a P4: the effect showed the new name and dyn=0, having compiled nothing. The unit tests call prepare() directly, so the control-change path had no coverage at all — which is why they passed. - MoonLiveLayout invalidates its compiled hash when the script control is written. addText binds the buffer directly, so a control write never reached setScript() and compile()'s early-return kept the old program. Pinned by a test that fails without it. - A script name is a BASENAME ending in .mlv, rejected otherwise. It was pasted straight into the path, so `../.config/NetworkModule.json` would have read the device's saved credentials as a script. The fixed directory is the boundary; now it holds. - reportOverCapacity counts down through frameBytesFor instead of dividing. The frame is 64-byte rounded, so the division overshot by one: it reported 898 lights per lane, whose frame rounds to 65536 against a 65535 cap. A limit that still fails is worse than none. Core - MoonLive::compile's staging buffer and each assembler's buf_ are heap-allocated, RAII owned, with every write and both branch patchers guarded against a failed allocation. That is ~4.1 KB off a compile chain sharing a 12 KB task — the plan named this ("buf_ inside the assembler, itself a stack local") and step 1 had only done IrProgram, while raising kCodeCap 768 → 2048 grew what remained. Scripts/MoonDeck - The monitor opens its serial port before probing the network. raised_log_level contacts every device in moondeck.json at a 3 s timeout each; with a dozen registered and most powered off, that was half a minute before the first byte — losing the boot output it was pointed at. Docs/CI - MIGRATING no longer tells a layout user to edit the `source` control it just removed. - The Parlio ceilings are the corrected 897/673/442/332, and platform.h says plainly that a 0 transfer cap means NO bound rather than zero bytes. - Backlog: MoonLive compiling watchdogs a classic ESP32 — `rst:0x8 (TG1WDT_SYS_RESET)`, captured on serial while adding one layout. Not a panic and not the stack overflow I first chased: the compile simply takes longer than the 12 s task watchdog allows while the render task waits. The stack work above did not change it. The entry records the measured signature, the ruled-out theories, and to measure before assuming which part is slow. Verified on the P4: layout 256 lights 16x16 (268 B) and effect (988 B), both compiled from files written over the API. The classic still resets, now with the watchdog signature. Flash: esp32 1715008, esp32s3-n16r8 1753792, esp32s31 2025600, esp32p4-eth 1603920, desktop 1138376. Tests: 1328 cases. Co-Authored-By: Claude Opus 5 (1M context) --- docs/MIGRATING.md | 2 +- docs/backlog/backlog-light.md | 6 +++ docs/metrics/repo-health.json | 48 +++++++++---------- docs/metrics/repo-health.md | 30 ++++++------ docs/performance.md | 2 +- moondeck/run/monitor_esp32.py | 16 ++++--- src/core/moonlive/MoonLive.cpp | 38 +++++++++++---- src/light/drivers/ParallelLedDriver.h | 13 +++-- src/light/moonlive/MoonLiveEffect.h | 4 +- src/light/moonlive/MoonLiveLayout.h | 7 +++ src/light/moonlive/MoonLiveScriptFile.h | 20 ++++++++ src/platform/desktop/moonlive_asm_host.cpp | 7 ++- src/platform/desktop/moonlive_asm_host.h | 16 ++++++- src/platform/esp32/moonlive_asm_riscv.cpp | 5 +- src/platform/esp32/moonlive_asm_riscv.h | 16 ++++++- src/platform/esp32/moonlive_asm_xtensa.cpp | 7 ++- src/platform/esp32/moonlive_asm_xtensa.h | 16 ++++++- src/platform/platform.h | 3 ++ .../light/scenario_MoonLive_pipeline.json | 4 +- .../light/scenario_peripheral_grid_sweep.json | 8 ++-- test/unit/light/unit_MoonLiveLayout.cpp | 36 ++++++++++++++ 21 files changed, 230 insertions(+), 74 deletions(-) diff --git a/docs/MIGRATING.md b/docs/MIGRATING.md index 1a0da701..9e34049e 100644 --- a/docs/MIGRATING.md +++ b/docs/MIGRATING.md @@ -49,7 +49,7 @@ A **layout** is the one script that legitimately used those names for its own co | What | Why | What to do | |---|---|---| -| A scripted layout declaring `width`/`height` | The name is what the layout is defining, so the declaration is a compile error and no lights are placed | Edit the script's `source` control, renaming its own controls (the shipped `grid.mlv` uses `cols`/`rows`) | +| A scripted layout declaring `width`/`height` | The name is what the layout is defining, so the declaration is a compile error and no lights are placed | Edit the `.mlv` file in the File Manager, renaming its own controls (the shipped `grid.mlv` uses `cols`/`rows`), then set the module's `script` control to that file | | A scripted **modifier** using `x`, `y` or `z` as a loop variable | A modifier IS handed a coordinate under those names, so they cannot also be counters there | Rename the loop variable to something the modifier is not handed (`i`, `n`) | Effects and modifiers need no change: they were already being handed these values, just through a preamble instead of by name. The error names the clash, and the module shows it on its card, so a broken script says why rather than failing silently. diff --git a/docs/backlog/backlog-light.md b/docs/backlog/backlog-light.md index 8ca1324e..7f368fb7 100644 --- a/docs/backlog/backlog-light.md +++ b/docs/backlog/backlog-light.md @@ -290,6 +290,12 @@ The LED-driver increments **shipped**: increment 1 (RMT/WS2812B single-strand on 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. +- **MoonLive compiling watchdogs a classic ESP32** (2026-08-12). Naming a script on an Olimex Gateway resets the board with `rst:0x8 (TG1WDT_SYS_RESET)` — the TASK watchdog at 12 s, not a panic: there is no `Guru Meditation`, no backtrace, and the last serial lines are ordinary ticks. So the compile is not crashing, it is taking longer than twelve seconds with the render task waiting on it, and the watchdog does its job. Bench-captured on serial while adding one `MoonLiveLayout` with `grid.mlv`; the P4 compiles the same script in well under a second. + + **Not a stack overflow** — that was the earlier theory and it was wrong. ~4.1 KB was moved off the compile chain (`MoonLive::compile`'s staging buffer and each assembler's `buf_`, both now heap, RAII-owned) which was worth doing on its own merits (the plan named it) but did not change this: the board still resets, now with the watchdog signature rather than `Double exception`. The earlier `Double exception` runs came from a board carrying persisted WiFi credentials, a separate issue. + + **Where to look:** the classic's ticks already read ~9 ms with `renderWait` ~8 ms BEFORE any compile, so the render loop has almost no slack. Either the compile is genuinely that slow on a 240 MHz single-issue Xtensa with no PSRAM, or something in the path blocks (the LittleFS read, `platform::alloc` under a fragmented heap). Measure first — instrument `compileScriptFile` with timings and run it on the classic — before assuming which. Moving the compile off the render task is the likely fix, but it is a scheduling change and wants its own cycle. + - **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 83fdee96..04811dc5 100644 --- a/docs/metrics/repo-health.json +++ b/docs/metrics/repo-health.json @@ -1,21 +1,21 @@ { - "commit": "38a28dc9", + "commit": "234e01ec", "flash": { - "esp32": 1762368, - "esp32p4-eth": 1603952, + "esp32": 1715008, + "esp32p4-eth": 1603920, "esp32p4-eth-wifi": 1793760, - "esp32s3-n16r8": 1752992, + "esp32s3-n16r8": 1753792, "esp32s3-n8r8": 1753232, "esp32s31": 2025600, - "desktop": 1138184, + "desktop": 1138376, "esp32-16mb": 1714608, - "esp32-eth": 1324928, + "esp32-eth": 1324816, "esp32-wrover": 1765504 }, "perf": { "desktop": { - "tick_us": 132, - "fps": 7575 + "tick_us": 127, + "fps": 7874 }, "esp32": { "tick_us": 2151, @@ -23,54 +23,54 @@ } }, "loc": { - "core": 17044, - "light": 24402, - "platform": 12878, + "core": 17064, + "light": 24436, + "platform": 12934, "ui": 6468, - "test": 41313, - "moondeck": 20326 + "test": 41349, + "moondeck": 20330 }, "comments": { "core": { - "lines": 6433, + "lines": 6442, "ratio": 0.411 }, "light": { - "lines": 9473, + "lines": 9487, "ratio": 0.429 }, "platform": { - "lines": 4377, - "ratio": 0.375 + "lines": 4409, + "ratio": 0.377 }, "ui": { "lines": 1670, "ratio": 0.274 }, "test": { - "lines": 7193, + "lines": 7200, "ratio": 0.201 }, "moondeck": { - "lines": 3248, + "lines": 3252, "ratio": 0.183 } }, "tests": { - "cases": 1326, + "cases": 1328, "scenarios": 23 }, "docs": { "md_files": 178, - "md_lines": 24421, + "md_lines": 24427, "plans_files": 91, - "backlog_lines": 3629, + "backlog_lines": 3635, "lessons_lines": 454, "claude_md_lines": 135 }, "complexity": { - "functions": 2480, - "over_threshold": 153, + "functions": 2486, + "over_threshold": 154, "worst_ccn": 93 } } diff --git a/docs/metrics/repo-health.md b/docs/metrics/repo-health.md index 935dcc5a..74960f90 100644 --- a/docs/metrics/repo-health.md +++ b/docs/metrics/repo-health.md @@ -1,6 +1,6 @@ # Repo health -Measured at `38a28dc9`. Generated by [`moondeck/check/repo_health.py`](../../moondeck/check/repo_health.py) on every KPI-gate run. **Do not edit by hand.** +Measured at `234e01ec`. 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. @@ -9,13 +9,13 @@ Current state only; the trend is this file's git history (`git log -p docs/metri | Target | Flash | |---|---:| | desktop | 1,112 KB (+0 KB) ⚠ | -| esp32 | 1,721 KB | +| esp32 | 1,675 KB (−46 KB) ✓ | | esp32-16mb | 1,674 KB | -| esp32-eth | 1,294 KB | +| esp32-eth | 1,294 KB (−0 KB) ✓ | | esp32-wrover | 1,724 KB | | esp32p4-eth | 1,566 KB (−0 KB) ✓ | | esp32p4-eth-wifi | 1,752 KB | -| esp32s3-n16r8 | 1,712 KB (−0 KB) ✓ | +| esp32s3-n16r8 | 1,713 KB (+1 KB) ⚠ | | esp32s3-n8r8 | 1,712 KB | | esp32s31 | 1,978 KB | @@ -23,33 +23,33 @@ Current state only; the trend is this file's git history (`git log -p docs/metri | Target | Tick | FPS | |---|---:|---:| -| desktop | 132 µs (+3 µs) ⚠ | 7,575 (−176) ⚠ | +| desktop | 127 µs (−5 µs) ✓ | 7,874 (+299) ✓ | | esp32 | 2,151 µs | 464 | ## Code | Area | Lines | Comments | Comment share | |---|---:|---:|---:| -| core | 17,044 (+64) ⚠ | 6,433 | 41.1 % (+0.1 %) ⚠ | -| light | 24,402 (+85) ⚠ | 9,473 | 42.9 % | -| platform | 12,878 (+18) ⚠ | 4,377 | 37.5 % | +| core | 17,064 (+20) ⚠ | 6,442 | 41.1 % | +| light | 24,436 (+34) ⚠ | 9,487 | 42.9 % | +| platform | 12,934 (+56) ⚠ | 4,409 | 37.7 % (+0.2 %) ⚠ | | ui | 6,468 | 1,670 | 27.4 % | -| test | 41,313 (+67) ⚠ | 7,193 | 20.1 % | -| moondeck | 20,326 (+3) ⚠ | 3,248 | 18.3 % | +| test | 41,349 (+36) ⚠ | 7,200 | 20.1 % | +| moondeck | 20,330 (+4) ⚠ | 3,252 | 18.3 % | ## Tests | Kind | Count | |---|---:| -| unit cases | 1,326 (+1) ✓ | +| unit cases | 1,328 (+2) ✓ | | scenarios | 23 | ## Complexity | Metric | Value | |---|---:| -| functions | 2,480 (+7) ✓ | -| over threshold | 153 (+2) ⚠ | +| functions | 2,486 (+6) ✓ | +| over threshold | 154 (+1) ⚠ | | worst CCN | 93 | ## Documentation @@ -57,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,421 (+20) ⚠ | +| markdown lines | 24,427 (+6) ⚠ | | plan files | 91 | -| backlog lines | 3,629 | +| backlog lines | 3,635 (+6) ⚠ | | lessons lines | 454 | | CLAUDE.md lines | 135 | diff --git a/docs/performance.md b/docs/performance.md index 151852a5..d65d358c 100644 --- a/docs/performance.md +++ b/docs/performance.md @@ -248,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) | 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). | +| **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 **897 lights/lane at 8 lanes RGB**, 673 RGBW, and halves to 442/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/moondeck/run/monitor_esp32.py b/moondeck/run/monitor_esp32.py index 8b83ecb4..34e6d81e 100644 --- a/moondeck/run/monitor_esp32.py +++ b/moondeck/run/monitor_esp32.py @@ -100,13 +100,17 @@ def main(): print("Press Ctrl+C (or Stop in MoonDeck) to stop.\n") sys.stdout.flush() - with raised_log_level(active_device_ips(), LOG_INFO): - try: - ser = serial.Serial(args.port, args.baud, timeout=1) - except serial.SerialException as e: - print(f"Cannot open {args.port}: {e}") - sys.exit(1) + # OPEN THE PORT FIRST. raised_log_level contacts every device in moondeck.json over HTTP at a + # 3 s timeout each — with a dozen registered and most powered off, that is half a minute of + # blocking before a single byte is read, and the boot output you were monitoring FOR is already + # gone. The log level is a nicety; the serial stream is the point. + try: + ser = serial.Serial(args.port, args.baud, timeout=1) + except serial.SerialException as e: + print(f"Cannot open {args.port}: {e}") + sys.exit(1) + with raised_log_level(active_device_ips(), LOG_INFO): with open(LOG_FILE, "w") as log: try: while True: diff --git a/src/core/moonlive/MoonLive.cpp b/src/core/moonlive/MoonLive.cpp index 00cc00bd..39b291a5 100644 --- a/src/core/moonlive/MoonLive.cpp +++ b/src/core/moonlive/MoonLive.cpp @@ -39,25 +39,44 @@ void* MoonLive::place(const uint8_t* staged, size_t len) { return block; } +// The emitted-code staging buffer, on the HEAP. +// +// It was `uint8_t staging[kCodeCap]` — 2 KB of stack, in a call chain that also holds the +// assembler's own 2 KB buffer and its tables: ~4.7 KB in one go. On a classic ESP32 that overflowed +// the task the compile runs on, and the fault surfaced as `Double exception` inside +// _xt_context_save (the handler faulting while saving context) with LBEG pointing back into +// MoonLive::compile — a crash on the HTTP task from naming a script. Compilation is cold path, so +// the allocation costs nothing that matters, and this is the same reasoning that moved IrProgram's +// op array off the stack. +namespace { +struct Staging { + uint8_t* p = static_cast(platform::alloc(kCodeCap)); + ~Staging() { platform::free(p); } + explicit operator bool() const { return p != nullptr; } +}; +} // namespace + bool MoonLive::compile(uint8_t r, uint8_t g, uint8_t b) { - uint8_t staging[kCodeCap]; - size_t len = emitFill(staging, kCodeCap, r, g, b); - void* block = place(staging, len); + Staging staging; + if (!staging) { error_ = "no memory to compile"; return false; } + size_t len = emitFill(staging.p, kCodeCap, r, g, b); + void* block = place(staging.p, len); if (!block) return false; fn_ = reinterpret_cast(block); return true; } bool MoonLive::compile(const char* source, const BuiltinTable& table, const SysVarTable& sysvars) { - uint8_t staging[kCodeCap]; - CompileResult cr = compileSource(source, table, sysvars, staging, kCodeCap); + Staging staging; + if (!staging) { freeCode(); error_ = "no memory to compile"; return false; } + CompileResult cr = compileSource(source, table, sysvars, staging.p, kCodeCap); if (!cr.ok) { freeCode(); error_ = cr.error; return false; } // surface the parse diagnostic // Allocate the control arena (fixed address) and seed new slots, BEFORE publishing the control // set — ensureArena reads the previous controlCount_ to know which slots are new. if (!ensureArena(cr.controls, cr.controlCount)) { freeCode(); error_ = "no control memory"; return false; } // Place the code. Only after it succeeds do we publish the new control set — a failed place() // must not leave declaredControls() advertising controls for code that isn't running. - void* block = place(staging, cr.len); + void* block = place(staging.p, cr.len); if (!block) return false; // controlCount_/controls_ unchanged // Clamp any kept slot whose range shrank (e.g. @control 0..99 edited to 0..10) so a stale live // value can't fall outside the new bounds before the native code reads it. @@ -100,9 +119,10 @@ bool MoonLive::ensureArena(const DeclaredControl* decls, uint8_t count) { } bool MoonLive::compileAnimated() { - uint8_t staging[kCodeCap]; - size_t len = emitAnimatedFill(staging, kCodeCap); - void* block = place(staging, len); + Staging staging; + if (!staging) { error_ = "no memory to compile"; return false; } + size_t len = emitAnimatedFill(staging.p, kCodeCap); + void* block = place(staging.p, len); if (!block) return false; anim_ = reinterpret_cast(block); return true; diff --git a/src/light/drivers/ParallelLedDriver.h b/src/light/drivers/ParallelLedDriver.h index ae40387b..642bdab6 100644 --- a/src/light/drivers/ParallelLedDriver.h +++ b/src/light/drivers/ParallelLedDriver.h @@ -793,10 +793,17 @@ class ParallelLedDriver : public DriverBase { 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 usable = cap > pad ? cap - pad : 0; - const unsigned fits = rowBytes ? static_cast(usable / rowBytes) : 0; + const size_t pad = padBytesFor(slotBytes(), opp); + // Count DOWN through frameBytesFor, not up through a division: the frame is 64-byte ROUNDED, + // so `(cap - pad) / rowBytes` overshoots by one — it reported 898 lights, whose frame rounds + // to 65536 against a 65535 cap. A limit that still fails is worse than no limit. + unsigned fits = 0; + if (rowBytes && cap > pad) { + fits = static_cast((cap - pad) / rowBytes); + while (fits > 0 && frameBytesFor(static_cast(fits), outCh, + slotBytes(), opp) > cap) fits--; + } std::snprintf(overCapBuf_, sizeof(overCapBuf_), "too many lights per pin: %u exceeds this peripheral's %u — lower ledsPerPin", static_cast(maxLaneLights_), fits); diff --git a/src/light/moonlive/MoonLiveEffect.h b/src/light/moonlive/MoonLiveEffect.h index 86f5432d..e37e2a62 100644 --- a/src/light/moonlive/MoonLiveEffect.h +++ b/src/light/moonlive/MoonLiveEffect.h @@ -53,9 +53,9 @@ class MoonLiveEffect : public EffectBase { // A `source` edit must recompile — route it through the prepare rebuild sweep so a new // script swaps in live (the script-editor loop). A SCRIPTED CONTROL's value change must NOT // recompile: it just updates an arena byte the running native code reads next tick. So only - // "source" triggers a rebuild; every scripted control returns false (the live-edit path). + // "script" triggers a rebuild; every scripted control returns false (the live-edit path). bool affectsPrepare(const char* controlName) const override { - return std::strcmp(controlName, "source") == 0; + return std::strcmp(controlName, "script") == 0; } // Compile the source on the cold rebuild path. A failed compile (parse error or no exec diff --git a/src/light/moonlive/MoonLiveLayout.h b/src/light/moonlive/MoonLiveLayout.h index 048a45d7..d88111a6 100644 --- a/src/light/moonlive/MoonLiveLayout.h +++ b/src/light/moonlive/MoonLiveLayout.h @@ -90,6 +90,13 @@ class MoonLiveLayout : public LayoutBase { } /// Replace the script. The next prepare() compiles it — the path a UI edit takes. + /// A control write lands DIRECTLY in script_ (addText binds the buffer), so setScript() is not + /// called and nothing would clear the compiled-hash — compile() would early-return and keep + /// running the previous script under a new name. Clearing it here covers both paths. + void onControlChanged(const char* name) override { + if (name && std::strcmp(name, "script") == 0) compiledHash_ = 0; + } + /// Point the layout at a script in the shared script directory; the next prepare() compiles it. void setScript(const char* name) { if (!name) return; diff --git a/src/light/moonlive/MoonLiveScriptFile.h b/src/light/moonlive/MoonLiveScriptFile.h index 88014fb5..f363a34b 100644 --- a/src/light/moonlive/MoonLiveScriptFile.h +++ b/src/light/moonlive/MoonLiveScriptFile.h @@ -18,6 +18,10 @@ inline constexpr const char* kScriptDir = "/moonlive"; /// cannot ask a 320 KB device for an allocation it will not survive. inline constexpr long kScriptFileMax = 16384; +/// Longest script name accepted. Bounds the path buffer below — a module's `script` control is 32 +/// bytes, so this is the same limit stated where the path is built. +inline constexpr size_t kMaxScriptName = 40; + /// Read `/` and compile it. The source lives in a right-sized heap buffer for the /// duration of the compile and is freed before returning, so a module holds a filename (~32 B) and /// the emitted code — never the script text. That is the whole point: the fixed per-module arrays @@ -46,6 +50,22 @@ inline bool compileScriptFile(MoonLive& engine, const char* name, if (!name || !name[0]) { err = "no script — set the script name"; return false; } + // A BASENAME only. The fixed directory is the point — a module names a script, it does not + // address the filesystem — so a separator or a `..` would let a control value reach outside + // kScriptDir (`../.config/NetworkModule.json` reads the device's saved credentials). Rejected + // rather than sanitised: a name that needs rewriting to be safe is a name a user mistyped. + for (const char* c = name; *c; c++) + if (*c == '/' || *c == '\\') { err = "script name is a file in the script folder, not a path"; return false; } + if (std::strcmp(name, "..") == 0 || std::strncmp(name, "../", 3) == 0) { + err = "script name is a file in the script folder, not a path"; return false; + } + // .mlv, so a stray name cannot pull in an unrelated file that happens to sit alongside. The + // upper bound also lets the compiler see that the snprintf below cannot truncate. + const size_t len = std::strlen(name); + if (len < 5 || len > kMaxScriptName || std::strcmp(name + len - 4, ".mlv") != 0) { + err = "script name must end in .mlv"; return false; + } + char path[96]; std::snprintf(path, sizeof(path), "%s/%s", kScriptDir, name); diff --git a/src/platform/desktop/moonlive_asm_host.cpp b/src/platform/desktop/moonlive_asm_host.cpp index 08d6d2e9..bc478528 100644 --- a/src/platform/desktop/moonlive_asm_host.cpp +++ b/src/platform/desktop/moonlive_asm_host.cpp @@ -47,12 +47,12 @@ void HostAssembler::addFixup(size_t at, Label label, uint8_t kind) { } void HostAssembler::emit32(uint32_t w) { - if (len_ + 4 > kCap) { overflow_ = true; return; } + if (!buf_ || len_ + 4 > kCap) { overflow_ = true; return; } buf_[len_++] = uint8_t(w); buf_[len_++] = uint8_t(w >> 8); buf_[len_++] = uint8_t(w >> 16); buf_[len_++] = uint8_t(w >> 24); } void HostAssembler::emitBytes(const uint8_t* p, size_t n) { - if (len_ + n > kCap) { overflow_ = true; return; } + if (!buf_ || len_ + n > kCap) { overflow_ = true; return; } std::memcpy(buf_ + len_, p, n); len_ += n; } @@ -160,6 +160,9 @@ void HostAssembler::call(Reg d, Reg a, Reg b, Reg c, const void* fn) { void HostAssembler::ret() { emit32(0xd65f03c0u); } void HostAssembler::patchBranches() { + // Nothing was emitted if the buffer never allocated, so there is nothing to patch — + // stated rather than left to the reader to derive from fixupCount_ being 0. + if (!buf_) return; for (uint8_t i = 0; i < fixupCount_; i++) { const Fixup& f = fixups_[i]; int32_t target = labelPos_[f.label]; diff --git a/src/platform/desktop/moonlive_asm_host.h b/src/platform/desktop/moonlive_asm_host.h index 31dafe21..db845fef 100644 --- a/src/platform/desktop/moonlive_asm_host.h +++ b/src/platform/desktop/moonlive_asm_host.h @@ -1,5 +1,7 @@ #pragma once +#include "platform/platform.h" // alloc/free — the emit buffer is heap, not stack + #include "core/moonlive/MoonLiveIr.h" // kCodeCap — one cap for the staging buffer and every backend #include @@ -33,6 +35,13 @@ enum class Cond : uint8_t { Lo /* unsigned < */, Hs /* unsigned >= */, Ne /* != class HostAssembler { public: + // Owns buf_ (see below). Freed here, copying deleted — an emitter that was copied + // would double-free the buffer it emits into. + ~HostAssembler() { platform::free(buf_); } + HostAssembler() = default; + HostAssembler(const HostAssembler&) = delete; + HostAssembler& operator=(const HostAssembler&) = delete; + // --- buffer --- // Resolve all branch fixups against bound labels, then expose the finished bytes. Call // once after the last instruction; bytes()/size() are valid only after finalize(). @@ -75,7 +84,12 @@ class HostAssembler { void emitBytes(const uint8_t* p, size_t n); void addFixup(size_t at, Label label, uint8_t kind); // enqueue a branch fixup (bounds-checked) - uint8_t buf_[kCap] = {}; + // HEAP, not a member array: the assembler is a stack local in lowerToBytes, so a kCap-sized + // member put 2 KB on the compile chain's stack — on top of the staging buffer and the parser + // frames. On a classic ESP32 that overflowed the task and faulted inside _xt_context_save + // (the plan named this: "buf_[kCap] inside the assembler, itself a stack local"). The buffer is + // scratch that ends in a memcpy to the caller's output, so nothing outlives the object. + uint8_t* buf_ = static_cast(platform::alloc(kCap)); size_t len_ = 0; bool overflow_ = false; diff --git a/src/platform/esp32/moonlive_asm_riscv.cpp b/src/platform/esp32/moonlive_asm_riscv.cpp index 37ec23d9..4afb3954 100644 --- a/src/platform/esp32/moonlive_asm_riscv.cpp +++ b/src/platform/esp32/moonlive_asm_riscv.cpp @@ -35,7 +35,7 @@ constexpr bool rvScratchOutsideMap() { static_assert(rvScratchOutsideMap(), "a scratch register is also a vreg — calls will corrupt it"); void RiscvAssembler::emit32(uint32_t w) { - if (len_ + 4 > kCap) { overflow_ = true; return; } + if (!buf_ || len_ + 4 > kCap) { overflow_ = true; return; } buf_[len_++] = uint8_t(w); buf_[len_++] = uint8_t(w >> 8); buf_[len_++] = uint8_t(w >> 16); buf_[len_++] = uint8_t(w >> 24); } @@ -169,6 +169,9 @@ void RiscvAssembler::call(Reg d, Reg a, Reg b, Reg c, const void* fn) { void RiscvAssembler::ret() { emit32(0x00008067u); } // ret = jalr x0, ra, 0 void RiscvAssembler::patchBranches() { + // Nothing was emitted if the buffer never allocated, so there is nothing to patch — + // stated rather than left to the reader to derive from fixupCount_ being 0. + if (!buf_) return; for (uint8_t i = 0; i < fixupCount_; i++) { const Fixup& f = fixups_[i]; if (labelPos_[f.label] < 0) continue; // unbound label — leave as-is (overflow_ already failed the compile) diff --git a/src/platform/esp32/moonlive_asm_riscv.h b/src/platform/esp32/moonlive_asm_riscv.h index fd340fcf..69c7d5b0 100644 --- a/src/platform/esp32/moonlive_asm_riscv.h +++ b/src/platform/esp32/moonlive_asm_riscv.h @@ -1,5 +1,7 @@ #pragma once +#include "platform/platform.h" // alloc/free — the emit buffer is heap, not stack + #include "core/moonlive/MoonLiveIr.h" // kCodeCap — one cap for the staging buffer and every backend #include @@ -32,6 +34,13 @@ enum class Cond : uint8_t { Lo /* unsigned < */, Hs /* unsigned >= */ }; class RiscvAssembler { public: + // Owns buf_ (see below). Freed here, copying deleted — an emitter that was copied + // would double-free the buffer it emits into. + ~RiscvAssembler() { platform::free(buf_); } + RiscvAssembler() = default; + RiscvAssembler(const RiscvAssembler&) = delete; + RiscvAssembler& operator=(const RiscvAssembler&) = delete; + void finalize() { patchBranches(); } const uint8_t* bytes() const { return buf_; } size_t size() const { return len_; } @@ -64,7 +73,12 @@ class RiscvAssembler { void emit32(uint32_t w); void addFixup(size_t at, Label label); // enqueue a branch fixup (bounds-checked) - uint8_t buf_[kCap] = {}; + // HEAP, not a member array: the assembler is a stack local in lowerToBytes, so a kCap-sized + // member put 2 KB on the compile chain's stack — on top of the staging buffer and the parser + // frames. On a classic ESP32 that overflowed the task and faulted inside _xt_context_save + // (the plan named this: "buf_[kCap] inside the assembler, itself a stack local"). The buffer is + // scratch that ends in a memcpy to the caller's output, so nothing outlives the object. + uint8_t* buf_ = static_cast(platform::alloc(kCap)); size_t len_ = 0; bool overflow_ = false; diff --git a/src/platform/esp32/moonlive_asm_xtensa.cpp b/src/platform/esp32/moonlive_asm_xtensa.cpp index d8871edb..74bbd6a7 100644 --- a/src/platform/esp32/moonlive_asm_xtensa.cpp +++ b/src/platform/esp32/moonlive_asm_xtensa.cpp @@ -32,7 +32,9 @@ static_assert(xtScratchOutsideMap(), "a scratch register is also a vreg — call void XtensaAssembler::emit(const uint8_t* p, size_t n) { - if (len_ + n > kCap) { overflow_ = true; return; } + // !buf_ covers a failed allocation: the compile then fails cleanly at overflowed() instead + // of writing through a null pointer. + if (!buf_ || len_ + n > kCap) { overflow_ = true; return; } std::memcpy(buf_ + len_, p, n); len_ += n; } void XtensaAssembler::emit2(uint16_t w) { @@ -206,6 +208,9 @@ void XtensaAssembler::call(Reg d, Reg a, Reg b, Reg c, const void* fn) { } void XtensaAssembler::patchBranches() { + // Nothing was emitted if the buffer never allocated, so there is nothing to patch — + // stated rather than left to the reader to derive from fixupCount_ being 0. + if (!buf_) return; for (uint8_t i = 0; i < fixupCount_; i++) { const Fixup& f = fixups_[i]; if (labelPos_[f.label] < 0) continue; // unbound label — leave as-is (overflow_ already failed the compile) diff --git a/src/platform/esp32/moonlive_asm_xtensa.h b/src/platform/esp32/moonlive_asm_xtensa.h index 5b79f125..5e08eff8 100644 --- a/src/platform/esp32/moonlive_asm_xtensa.h +++ b/src/platform/esp32/moonlive_asm_xtensa.h @@ -1,5 +1,7 @@ #pragma once +#include "platform/platform.h" // alloc/free — the emit buffer is heap, not stack + #include "core/moonlive/MoonLiveIr.h" // kCodeCap — one cap for the staging buffer and every backend #include @@ -22,6 +24,13 @@ enum class Cond : uint8_t { Lo /* unsigned < */, Hs /* unsigned >= */ }; class XtensaAssembler { public: + // Owns buf_ (see below). Freed here, copying deleted — an emitter that was copied + // would double-free the buffer it emits into. + ~XtensaAssembler() { platform::free(buf_); } + XtensaAssembler() = default; + XtensaAssembler(const XtensaAssembler&) = delete; + XtensaAssembler& operator=(const XtensaAssembler&) = delete; + void finalize() { patchBranches(); } const uint8_t* bytes() const { return buf_; } size_t size() const { return len_; } @@ -55,7 +64,12 @@ class XtensaAssembler { void emit3(uint32_t w); // wide (24-bit) instruction void addFixup(size_t at, Label label); // enqueue a branch fixup (bounds-checked) - uint8_t buf_[kCap] = {}; + // HEAP, not a member array: the assembler is a stack local in lowerToBytes, so a kCap-sized + // member put 2 KB on the compile chain's stack — on top of the staging buffer and the parser + // frames. On a classic ESP32 that overflowed the task and faulted inside _xt_context_save + // (the plan named this: "buf_[kCap] inside the assembler, itself a stack local"). The buffer is + // scratch that ends in a memcpy to the caller's output, so nothing outlives the object. + uint8_t* buf_ = static_cast(platform::alloc(kCap)); size_t len_ = 0; bool overflow_ = false; diff --git a/src/platform/platform.h b/src/platform/platform.h index d86484d0..4e0d50fb 100644 --- a/src/platform/platform.h +++ b/src/platform/platform.h @@ -1168,6 +1168,9 @@ size_t parlioWs2812BufferCapacity(const ParlioWs2812Handle& h); // The most bytes Parlio can send in ONE transfer — a HARDWARE ceiling, not a heap budget, so it // needs no handle and holds before anything is allocated. A caller sizes a frame against it to // refuse an impossible configuration up front instead of failing the bus init. +// +// 0 means NO BOUND (the dmaBudgetBytes contract), not "zero bytes usable" — it is what a host +// without Parlio returns, and what a caller reads as "nothing to check against". size_t parlioMaxTransferBytes(); // Start the autonomous DMA transfer of buffer `buffer`'s first `bytes`; pair diff --git a/test/scenarios/light/scenario_MoonLive_pipeline.json b/test/scenarios/light/scenario_MoonLive_pipeline.json index efe47569..05ce0e41 100644 --- a/test/scenarios/light/scenario_MoonLive_pipeline.json +++ b/test/scenarios/light/scenario_MoonLive_pipeline.json @@ -58,7 +58,7 @@ "observed": { "desktop-macos": { "tick_us": [ - 5, + 4, 16 ], "free_heap": [ @@ -71,7 +71,7 @@ ], "at": [ "2026-08-09", - "2026-08-10" + "2026-08-12" ] } } diff --git a/test/scenarios/light/scenario_peripheral_grid_sweep.json b/test/scenarios/light/scenario_peripheral_grid_sweep.json index d31cfb64..d74b01e2 100644 --- a/test/scenarios/light/scenario_peripheral_grid_sweep.json +++ b/test/scenarios/light/scenario_peripheral_grid_sweep.json @@ -489,7 +489,7 @@ "desktop-macos": { "tick_us": [ 4, - 21 + 30 ], "free_heap": [ 0, @@ -501,7 +501,7 @@ ], "at": [ "2026-07-26", - "2026-07-31" + "2026-08-12" ] } } @@ -567,7 +567,7 @@ "desktop-macos": { "tick_us": [ 17, - 90 + 121 ], "free_heap": [ 0, @@ -579,7 +579,7 @@ ], "at": [ "2026-07-26", - "2026-07-31" + "2026-08-12" ] } } diff --git a/test/unit/light/unit_MoonLiveLayout.cpp b/test/unit/light/unit_MoonLiveLayout.cpp index 6a23e922..9bcf2e11 100644 --- a/test/unit/light/unit_MoonLiveLayout.cpp +++ b/test/unit/light/unit_MoonLiveLayout.cpp @@ -433,3 +433,39 @@ TEST_CASE("a layout that changes size mid-build cannot overrun the mapping") { #endif // MM_MOONLIVE_HAS_HOST_JIT +// A control write lands directly in the module's buffer — addText binds it — so setScript() is NOT +// called. Nothing then cleared the compiled-hash, and compile()'s early-return kept the OLD program +// running under the new name. Found by review; the same class of bug hardware found in the effect. +TEST_CASE("naming a different script through the control actually swaps the program") { + MoonLiveLayout l; + l.defineControls(); + const char* four = mmWriteScript("for (i = 0; i < 4; i = i + 1) { addLight(i, 0, 0); }"); + l.setScript(four); + l.prepare(); + REQUIRE(l.lightCount() == 4); + + // Write the OTHER script the way the API does: straight into the bound control buffer. + const char* nine = mmWriteScript("for (i = 0; i < 9; i = i + 1) { addLight(i, 0, 0); }"); + const auto& cs = l.controls(); + for (uint8_t i = 0; i < cs.count(); i++) + if (cs[i].name && std::strcmp(cs[i].name, "script") == 0) + std::snprintf(static_cast(cs[i].ptr), 32, "%s", nine); + l.onControlChanged("script"); + l.prepare(); + CHECK(l.lightCount() == 9); // the new file, not the cached program +} + +// The fixed script directory is a boundary: a module names a file inside it, and cannot address the +// filesystem. Without this, a control value of "../.config/NetworkModule.json" reads the device's +// saved WiFi credentials as if they were a script. +TEST_CASE("a script name cannot escape the script folder") { + MoonLiveLayout l; + l.defineControls(); + for (const char* bad : {"../.config/NetworkModule.json", "..", "sub/dir.mlv", "grid.txt"}) { + INFO(bad); + l.setScript(bad); + l.prepare(); + CHECK(l.severity() == MoonModule::Severity::Error); + CHECK(l.lightCount() == 0); + } +} From 6623dcced69df11cbb71047d133b2c540cccb77d Mon Sep 17 00:00:00 2001 From: ewowi Date: Thu, 13 Aug 2026 23:23:55 +0200 Subject: [PATCH 3/3] Give MoonLive a stack machine: the frame is where values live MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A script's variables and call arguments now live in the call frame instead of registers, so how complex a script can be is a memory question rather than a register-count one. Scripted layouts and effects run on desktop and on RISC-V (an S31 held layout + effect + modifier for over an hour); on Xtensa a script that stores a pixel still fails, for a windowed-ABI reason documented below. KPI: 16384lights | Desktop:1094KB | tick:124/100/3/6/124/281/20/4/272/70/17/22/5/124/22/7/243/45/4us(FPS:8064/10000/333333/166666/8064/3558/50000/250000/3676/14285/58823/45454/200000/8064/45454/142857/4115/22222/250000) | ESP32:1589KB | src:220(56124) | test:163(32995) | lizard:157w Core - Script variables get frame slots: a `for`'s counter and limit each take one, a read is a Reload into a temp that dies immediately. The guard that protected a local's register is gone — every vreg reaching freeTemp is now a temp. - Call arguments are staged through the frame: each is parked as soon as it is computed and all are reloaded for the one instruction that reads them, so only one argument occupies a register at a time. Measured on Xtensa: grid.mlv 212 -> 186 bytes, three-deep nesting compiling for the first time, and looped effects, four-deep layouts and plasma compiling at all. - spillToBudget numbers its slots above the front end's and refuses a compile when either exceeds what the backend's frame can address — checked before the "already fits" early return, which used to skip it entirely. - register-and-slot-contract.md writes down who owns which register index and which frame slot, because four places derive numbers from each other. Light domain - A failed script load is latched against the NAME that failed, not as a bare flag. As a bool it latched on the empty script every device boots with and then skipped every later compile, so a card sat at "no script" forever. - Layout rebuilds run on the render thread: HTTP marks the tree dirty and tick() does the work at a frame boundary. A scripted layout's compiled code has its frame on the calling task's stack, so an HTTP handler ran it on the web server's stack rather than the one the pipeline is budgeted against. Platform - Xtensa: a14/a15 removed from the vreg map — they carry retw.n's return linkage, and using them corrupted the return path (IllegalInstruction on every scripted layout). static_assert now covers scratch and window registers. - Xtensa: branch relaxation. Conditional branches carry a signed byte of displacement; a loop body past ~127 bytes was silently truncated into the middle of the program. Emitted as inverted-condition-over-`j` (18-bit), with a range check that refuses rather than miscompiles. - Xtensa: the call RESULT is parked in the frame, not in a12. call8 rotates the window, so the callee's a4 IS our a12 and it overwrote the stash. - All three backends bounds-check their register-map lookup: the inline ops address scratch as vregsUsed+n, and an out-of-range index read past the array and named a register chosen by accident. - currentThreadId(): C++ thread_local is unusable on ESP32 — the compiler reaches TLS through THREADPTR, which is 0 on a FreeRTOS task created without it, so the access faults at 0xfffffff0 and dies as a Double exception. Tests - The device backends now run on the development machine: two per-ISA TUs share one body, driven by a `lower` seam on compileSource. Golden length + byte hash per backend catch an emission change without flashing a board; a call-bearing script is length-only, because it embeds a host address that ASLR moves. - Regression tests for the give-up latch, loop-extended live intervals (all tests passed with extension disabled before this), and the frame-capacity guard. The fixture no longer leaves 79 t*.mlv files behind per run. Docs/CI - Plan-20260813 supersedes 20260809 from step 4: what a windowed register ABI is, why Xtensa has one and arm64/RISC-V do not, and how to treat it as flat (restrict the map to a2..a7, which needs the host arguments in the frame — step 3b, not yet done). Steps 1-3 marked done. - backlog-light.md records the Xtensa root cause with the ESP-IDF citation: "a8..a15 clobbered (if window_spill8)" against a map of a2..a11. - disasm.py did not link MoonLiveSpill.cpp and compiled every script against modifierSysVars, so it had never once read the shipped grid.mlv. Reviews - 👾 pre-commit gates: 10 passed, 0 failed, 3 skipped (conditional triggers not matched). GCC caught three issues clang did not: -Wshadow in the Xtensa call encoders, and std::memcpy/ssize_t resolving inside the test's wrapper namespace. Co-Authored-By: Claude Opus 5 (1M context) --- CMakeLists.txt | 1 + docs/backlog/backlog-light.md | 30 +- ...and the stack as the register overflow.md" | 14 + ...200\224 the frame is where values live.md" | 352 +++++++++++++++++ docs/metrics/repo-health.json | 54 +-- docs/metrics/repo-health.md | 38 +- esp32/main/CMakeLists.txt | 1 + moondeck/moonlive/disasm.py | 13 +- moondeck/moonlive/emit_xtensa.cpp | 15 +- moonlive/effects/plasma.mlv | 18 + src/core/HttpServerModule.cpp | 10 +- src/core/NetworkModule.h | 5 + src/core/Scheduler.cpp | 15 +- src/core/Scheduler.h | 13 + src/core/moonlive/MoonLive.h | 7 +- src/core/moonlive/MoonLiveCompiler.cpp | 137 +++++-- src/core/moonlive/MoonLiveCompiler.h | 13 +- src/core/moonlive/MoonLiveIr.h | 37 +- src/core/moonlive/MoonLiveSpill.cpp | 360 ++++++++++++++++++ src/core/moonlive/MoonLiveSpill.h | 34 ++ src/core/moonlive/moonlive_emit.h | 29 +- .../moonlive/register-and-slot-contract.md | 74 ++++ src/light/moonlive/MoonLiveBuiltins_light.h | 48 ++- src/light/moonlive/MoonLiveLayout.h | 23 +- src/platform/desktop/moonlive_asm_host.cpp | 49 ++- src/platform/desktop/moonlive_asm_host.h | 16 + src/platform/desktop/moonlive_lower_host.cpp | 22 +- src/platform/desktop/platform_desktop.cpp | 6 + src/platform/esp32/moonlive_asm_riscv.cpp | 47 ++- src/platform/esp32/moonlive_asm_riscv.h | 21 +- src/platform/esp32/moonlive_asm_xtensa.cpp | 148 +++++-- src/platform/esp32/moonlive_asm_xtensa.h | 29 +- src/platform/esp32/moonlive_lower_riscv.cpp | 18 +- src/platform/esp32/moonlive_lower_xtensa.cpp | 19 +- src/platform/esp32/platform_esp32.cpp | 7 + src/platform/platform.h | 10 + test/CMakeLists.txt | 3 + .../light/scenario_MoonLive_pipeline.json | 12 +- .../light/scenario_peripheral_grid_sweep.json | 16 +- test/unit/core/moonlive_device_codegen.inc | 151 ++++++++ .../unit/core/unit_moonlive_codegen_riscv.cpp | 53 +++ .../core/unit_moonlive_codegen_xtensa.cpp | 79 ++++ test/unit/core/unit_moonlive_spill.cpp | 264 +++++++++++++ test/unit/light/MoonLiveScriptFixture.h | 29 ++ test/unit/light/unit_MoonLiveLayout.cpp | 66 ++++ 45 files changed, 2229 insertions(+), 177 deletions(-) create mode 100644 "docs/history/plans/Plan-20260813 - MoonLive on a stack machine \342\200\224 the frame is where values live.md" create mode 100644 moonlive/effects/plasma.mlv create mode 100644 src/core/moonlive/MoonLiveSpill.cpp create mode 100644 src/core/moonlive/MoonLiveSpill.h create mode 100644 src/core/moonlive/register-and-slot-contract.md create mode 100644 test/unit/core/moonlive_device_codegen.inc create mode 100644 test/unit/core/unit_moonlive_codegen_riscv.cpp create mode 100644 test/unit/core/unit_moonlive_codegen_xtensa.cpp create mode 100644 test/unit/core/unit_moonlive_spill.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index c7cda635..b08bc3fc 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -94,6 +94,7 @@ add_library(mm_core STATIC src/core/Scheduler.cpp src/core/moonlive/MoonLive.cpp src/core/moonlive/MoonLiveCompiler.cpp + src/core/moonlive/MoonLiveSpill.cpp ) target_include_directories(mm_core PUBLIC src/) target_link_libraries(mm_core PUBLIC mm_platform) diff --git a/docs/backlog/backlog-light.md b/docs/backlog/backlog-light.md index 7f368fb7..16896faf 100644 --- a/docs/backlog/backlog-light.md +++ b/docs/backlog/backlog-light.md @@ -290,11 +290,35 @@ The LED-driver increments **shipped**: increment 1 (RMT/WS2812B single-strand on 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. -- **MoonLive compiling watchdogs a classic ESP32** (2026-08-12). Naming a script on an Olimex Gateway resets the board with `rst:0x8 (TG1WDT_SYS_RESET)` — the TASK watchdog at 12 s, not a panic: there is no `Guru Meditation`, no backtrace, and the last serial lines are ordinary ticks. So the compile is not crashing, it is taking longer than twelve seconds with the render task waiting on it, and the watchdog does its job. Bench-captured on serial while adding one `MoonLiveLayout` with `grid.mlv`; the P4 compiles the same script in well under a second. +- **Xtensa's vreg map violates the windowed ABI** (2026-08-13). ROOT CAUSE, found by comparing a + working ESP32-S31 (RISC-V) against a crashing S3 on identical firmware and scripts. - **Not a stack overflow** — that was the earlier theory and it was wrong. ~4.1 KB was moved off the compile chain (`MoonLive::compile`'s staging buffer and each assembler's `buf_`, both now heap, RAII-owned) which was worth doing on its own merits (the plan named it) but did not change this: the board still resets, now with the watchdog signature rather than `Double exception`. The earlier `Double exception` runs came from a board carrying persisted WiFi credentials, a separate issue. + `call8` rotates the register window by eight: the callee's `a0..a7` ARE the caller's `a8..a15`, so + every host call clobbers `a8..a15`. ESP-IDF states it plainly — `a8..a15 clobbered (if + window_spill8)` in `components/xtensa/include/xtensa/coreasm.h`. Our map is `a2..a11`, so `a8..a11` + hold script values inside the rotation window. Measured crash: `A0 = 0x00000100` — that is + `nLights` (256), a script value sitting in the return-address register. Intermittent, because + whether it is fatal depends on which vreg held what when the window turned; that intermittency is + what made it survive a day of plausible theories. - **Where to look:** the classic's ticks already read ~9 ms with `renderWait` ~8 ms BEFORE any compile, so the render loop has almost no slack. Either the compile is genuinely that slow on a 240 MHz single-issue Xtensa with no PSRAM, or something in the path blocks (the LittleFS read, `platform::alloc` under a fragmented heap). Measure first — instrument `compileScriptFile` with timings and run it on the classic — before assuming which. Moving the compile off the render task is the likely fix, but it is a scheduling change and wants its own cycle. + Only `a0`(return address), `a1`(sp) and `a2..a7` survive a call, so the safe map is **six** vregs, + not ten. Six does not fit today: five are the fixed ABI vregs (`buf`, `nLights`, `cpl`, `t`, + `ctrls`), leaving one. Closing this needs those five OFF permanent registers and into the frame — + the register-promotion question [Plan-20260813](../history/plans/Plan-20260813%20-%20MoonLive%20on%20a%20stack%20machine%20%E2%80%94%20the%20frame%20is%20where%20values%20live.md) + defers on purpose. RISC-V is unaffected: no window, and its `call()` saves 14 registers into an + explicit 80-byte frame, which is why the S31 runs layout + plasma + modifier stably. + + Until then a scripted layout, and any effect whose script CALLS a builtin, is unreliable on Xtensa. + Straight-line and inline-only scripts (`setRGB`, `fill`) are fine — they emit no call. + +- **A scripted LAYOUT crashes on Xtensa: the emitted code is wrong** (2026-08-12). Naming any script on a `MoonLiveLayout` resets an S3 — `addLight(0, 0, 0);` alone is enough. A scripted EFFECT on the same board is fine, which is the whole clue: an effect writes pixels through `setRGB`, a layout calls `addLight`, and only the layout path faults. + + **Measured, so the earlier theories are retired.** Per-stage timings on the S3 read `stat 2.0 ms, alloc 8 us, read 2.7 ms, compile 0.5 ms` — the compile COMPLETES, in about five milliseconds, and the crash comes after it returns. So this is not a slow compile and not the task watchdog running out: tracing either side of `runScript` shows the fault lands *inside the JIT'd code*, as `Guru Meditation Error: Double exception` with a corrupted backtrace, or as `LoadProhibited` at a nonsense address. (Filesystem access IS expensive — ~90% of the load — but that is a cost, not the bug.) + + **The defect is visible in the disassembly.** `uv run moondeck/moonlive/disasm.py '