diff --git a/.gitmodules b/.gitmodules index 30e1c13..9b9c110 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,3 +1,10 @@ [submodule "src/memllib"] path = src/memllib - url = https://github.com/monkey-w1n5t0n/memllib.git + # Upstream (the lab's shared library), not the monkey-w1n5t0n fork. + # Phase 0 pointed this at the fork because the pin b37fc53 existed on no + # remote. Those three fork commits touch only `examples/`, which the + # firmware never compiles (it is not in the sketch symlink forest) and whose + # content is already ported into nisps/ml/{jolt,ou_noise,feedback,geo_push}. + # With the pin moved to an upstream commit, upstream is the correct source. + # The fork's feat/nisps-core-swap branch is still pushed; nothing is lost. + url = https://github.com/MusicallyEmbodiedML/memllib.git diff --git a/ALIGNMENT.md b/ALIGNMENT.md index 342cd6f..f5ee140 100644 --- a/ALIGNMENT.md +++ b/ALIGNMENT.md @@ -84,9 +84,17 @@ Schemas declare per-mode dims and since P5.3 both targets honour them. Is the mi Legacy a-immersive was mobile-first; Manifold is desktop-first. Defer until user data exists. -### Q4: Who owns memllib? (2026-07-21) +### Q4: Who owns memllib? — DECIDED, half-executed (2026-07-21) -Fork-pin (PIO `lib_deps` on `monkey-w1n5t0n/memllib`), vendor the actually-used subset, or upstream the nisps-swap to MusicallyEmbodiedML? Requires the load-bearing-surface inventory (plan §5). Phase 0 pushes the branch either way. +Operator decision: **vendor**, self-contained in this repo. The inventory +(`docs/specs/recon/memllib-usage-inventory.md`) settled the shape: there is no small load-bearing +subset — it is all of memllib bar `examples/` (~1.8 MB, 24/24 compiled TUs link). The fork is +dissolved: its three commits touch only `examples/`, which the firmware never compiles and whose +content already lives in `nisps/ml/{jolt,ou_noise,feedback,geo_push}.hpp`, so the submodule now +points at upstream and is pinned to current `main` (verified: all three variants build, +316 bytes +flash, and it brings the `l r input swap` hardware fix plus the `NavigateToView` the SelfTest +variant was already written against). **Remaining: the vendoring copy itself**, which lands with the +PlatformIO cut (plan §5). Delete this entry when it does. ### Q5: Legacy feedback modes — delete or keep for A/B? (2026-07-21) diff --git a/docs/specs/plans/simplification-plan.md b/docs/specs/plans/simplification-plan.md index 2062f4a..e76b458 100644 --- a/docs/specs/plans/simplification-plan.md +++ b/docs/specs/plans/simplification-plan.md @@ -100,15 +100,17 @@ DESCRIPTIVE — it documents that engine voice spaces already square the value i One cut, no dual path. `firmware/useq-celium/` already proves the PIO + arduino-pico pattern in-repo. - `platformio.ini` with one `[env]` per firmware variant passing `-DMEMLNAUT_MODE_TYPE=`; selftest becomes a plain `-DNISPS_SELFTEST=1` env. Deletes: the sed/python machinery in `firmware-common.sh` that **mutates the committed .ino**, the `.ino` comment-registry, the entire `NISPS_ST_*` token-paste table (already silently missing the currently-active SLPWorkshop variant — L12), the sketch-tree symlink forest, and the global TFT_eSPI library mutation (handled via PIO lib config/build flags instead). -- **memllib consumption decision** (§7.5): the prerequisite inventory is DONE — - `../recon/memllib-usage-inventory.md`, built from the firmware linker map rather than grep. Result: - all 24 compiled TUs are linked, so "the load-bearing subset" is ~1.8 MB / 84 files — essentially - all of memllib minus `examples/`. There is no small subset to lift. It also surfaced a - consequence that was not visible when §7.5 was decided: memllib is the LAB's shared library and - our fork is 3 commits ahead but **31 behind** (+2034/−153 across the files we link, including - `new staticmlp`, `jolts`, `verb`). Vendoring freezes that gap permanently. Recommended: rebase the - three NISPS commits onto upstream `main` first, THEN vendor — it is the only moment that merge is - cheap. Operator call before the cut. +- **memllib consumption decision** (§7.5): **SETTLED — the submodule bump landed; the vendoring copy + is what remains.** See `../recon/memllib-usage-inventory.md`. Result: all 24 compiled TUs link, so + the vendoring surface is ~1.8 MB / 84 files — all of memllib bar `examples/`; there is no small + subset to lift. The operator chose "rebase then vendor", but on inspection **there was no rebase to + do**: all three fork commits touch only `examples/`, which is not in the sketch symlink forest and + is never compiled, and whose content is already ported into `nisps/ml/`. So the fork is dissolved + and the submodule is repointed at upstream, pinned to current `main` — verified by building all + three variants (+316 bytes flash, one `constexpr`→`const` fix in the `.ino` because upstream made + `kSampleRate` runtime-settable). That bump brings the `l r input swap` hardware fix and the + `NavigateToView` the SelfTest variant had already been written against. **Vendor from this + snapshot**, recording the upstream commit so a re-sync stays a documented diff. - Then **S9**: a CI job compiling 2–3 representative envs with cached toolchain — firmware enters an automated gate for the first time. ## §6 Phase 5 — Vision-facing architecture (each item spec-first, own session) diff --git a/docs/specs/recon/memllib-usage-inventory.md b/docs/specs/recon/memllib-usage-inventory.md index faea68d..3fdc2b3 100644 --- a/docs/specs/recon/memllib-usage-inventory.md +++ b/docs/specs/recon/memllib-usage-inventory.md @@ -46,39 +46,67 @@ Firmware's direct include surface is just six entry points — `audio/AudioDrive of memllib minus `examples/`.** There is no small subset to lift. `synth/` alone is 1.3 MB (mostly `maximilian.cpp`) and all three of its TUs link. -## The consequence that was not visible when §7.5 was decided +## The fork divergence — and the correction to this document's first version `src/memllib` is not a third-party dependency. It is **the lab's shared library** -(`MusicallyEmbodiedML/memllib`), and our fork is: +(`MusicallyEmbodiedML/memllib`), and our fork was 3 commits ahead / 31 behind. -- **3 commits ahead** — `bf9691c` (swap memlp includes for nisps/core, inline RL utilities), - `32cc831` (newlib RNG fix), `b37fc53` (seed-helper rename). These are the NISPS-specific changes. -- **31 commits behind** — including `770a990 new staticmlp`, `9fcd459 jolts`, `d0d8a72 noise`, - `616b8e7 cc select`, `a8bf255 verb`, `671abbe screen ordering`, `e291192 l r input swap`. - Diffed against the five subdirs we link: **30 files changed, +2034 / −153**. +**The first version of this document (commit `7a30da9`) drew the wrong conclusion from that, and +recommended a rebase-then-vendor on the strength of it. Two facts checked afterwards overturn it:** -Vendoring freezes that gap permanently and converts "we are behind the lab" into "we have our own -divergent copy". That may be exactly what is wanted — self-containment is a real goal, and the lab's -`main` is not obviously a branch we track — but it is a fork of a colleague-shared library, not a -snapshot of a vendor drop, and the 31 commits contain work (`staticmlp`, `jolts`) that sounds -directly relevant to this project. +1. **All three of our commits touch only `examples/`** — `bf9691c` (swap memlp includes for + nisps/core, inline RL utilities), `32cc831` (newlib RNG fix), `b37fc53` (seed-helper rename). + `examples/` is **not in the sketch symlink forest** (`firmware/MEMLNaut-NISPS/src/` symlinks + exactly `audio hardware interface synth utils PicoDefs.hpp`), so it is never compiled. Those + commits existed to let the RL code build against `nisps/core` **while it was being ported**, and + that port is finished: `nisps/ml/{jolt,ou_noise,feedback,geo_push}.hpp` cite the upstream sources + directly. So there is nothing of ours to carry forward, and **no rebase to perform** — vendoring + the five linked subdirs drops `examples/` and our three commits with it. +2. **Two of the "31 commits of work we are missing" were already absorbed.** `nisps/ml/jolt.hpp:3` + cites `9fcd459 "jolts"` and `nisps/ml/ou_noise.hpp:3` cites `d0d8a72 "noise"` as their sources. + The alarm in the first version — "the 31 commits contain work that sounds directly relevant" — + was therefore overstated for exactly the two commits it named. -## Options, with what each costs +So the real question was never "rebase or not" but **"which snapshot do we vendor: the pinned +`188496d` we build today, or current upstream?"** -1. **Vendor as decided.** Copy the five subdirs into the repo, drop the submodule and the symlink - forest, delete `examples/`. Repo grows ~1.8 MB. Self-contained, no submodule init, PlatformIO - gets a plain `lib/` — this materially simplifies the §5 migration. Cost: the 31 upstream commits - become a manual merge, forever. -2. **Vendor, but first rebase our 3 commits onto upstream `main`.** Same end state, except the - snapshot is current rather than 31 commits stale. Costs one merge now (the three commits are - small and mechanical), and it is the only moment when that merge is cheap. -3. **Fork-pin (status quo + Phase 0's fix).** Already works: the pin is reachable, CI is green, - fresh clones build. Not self-contained, and keeps the submodule friction PlatformIO would rather - not have. +## The staleness was already costing us -**Recommendation: option 2.** The operator's goal (self-contained) is satisfied identically by 1 and -2, but 2 does not silently discard `staticmlp`/`jolts`/`verb`. Doing the rebase after vendoring -means doing it against a copy that no longer has upstream history — i.e. never. +Upstream `main` **has `DisplayDriver::NavigateToView`** (`display/DisplayDriver.hpp:54`); the pinned +commit does not. The SelfTest firmware variant called it and had been failing to compile — fixed in +`b953681` by routing around the missing method. That variant was not written against broken code; it +was written against a **newer memllib than the pin**. The gap is not theoretical. -This decision gates the PlatformIO migration (plan §5), which otherwise has to keep the submodule -and its symlink workaround. +Upstream also carries `e291192 "l r input swap"`, a **hardware bug fix**: the physical L/R input +sockets are wired to the opposite codec ADC channels, so `AudioDriver.cpp` now swaps them at the +lowest level. Every mode on the pinned commit sees its stereo input backwards. + +## Verified: current upstream builds, and costs almost nothing + +Submodule moved to `e291192` (upstream `main`), all three variants built with `arduino-cli`: + +| variant | flash | Δ vs pin | RAM | Δ | +|---|---|---|---|---| +| SLPWorkshop | 145348 | +320 | 87388 | +4 | +| PAFSynth | 145300 | +312 | 107060 | +4 | +| SelfTest | 141840 | +320 | 12028 | +4 | + +Exactly **one** compile error had to be fixed: `MEMLNaut-NISPS.ino:169` used `kSampleRate` in a +`constexpr`, and upstream `1997699 "mode sample rate"` made it a runtime `extern size_t` so a mode +can choose its own rate. `constexpr` → `const`; it is a once-per-second diagnostic print. + +The +316-byte uniform delta is the AudioDriver/DisplayDriver changes. The bulky new upstream code +(`GrainDelayI16`, `ReverbI16`, `ModFXI16`, `CCSelectView`, `NameInputView`, `RLView`, `VUMeterView`, +`PSRAMManager`) is header-only and unreferenced, so the linker drops all of it. + +## Where this leaves the vendoring + +Vendor **from current upstream**, not from the old pin. Take the five linked subdirs + +`PicoDefs.hpp`; drop `examples/` (~1.8 MB → the vendored surface). Record the exact upstream commit +so a future re-sync is a documented diff rather than an archaeology exercise. + +The submodule now points at **upstream** rather than the fork: with the pin moved to an upstream +commit, the fork has nothing the firmware compiles. The fork's `feat/nisps-core-swap` branch remains +pushed, so nothing is destroyed. + +This unblocks the PlatformIO migration (plan §5). diff --git a/firmware/MEMLNaut-NISPS/MEMLNaut-NISPS.ino b/firmware/MEMLNaut-NISPS/MEMLNaut-NISPS.ino index 042f444..42fcf58 100644 --- a/firmware/MEMLNaut-NISPS/MEMLNaut-NISPS.ino +++ b/firmware/MEMLNaut-NISPS/MEMLNaut-NISPS.ino @@ -166,7 +166,10 @@ void loop() { blip_counter = 0; Serial.println("."); digitalWrite(33, HIGH); - constexpr float audioHeadroomMul = 1.0f / (1000000.f * 48.0f / kSampleRate); + // `const`, not `constexpr`: memllib made kSampleRate a runtime + // `extern size_t` so a mode can pick its own rate. This is a + // once-per-second diagnostic print, so the divide is free. + const float audioHeadroomMul = 1.0f / (1000000.f * 48.0f / kSampleRate); Serial.printf("ml: %d, aud: %d, q: %f\n", PERF_GET_MEAN(MLSTATS), AUDIOLOOP_MEAN, diff --git a/src/memllib b/src/memllib index b37fc53..e291192 160000 --- a/src/memllib +++ b/src/memllib @@ -1 +1 @@ -Subproject commit b37fc53864545f7acca3ce2fba734e00ee38b58d +Subproject commit e291192d8e4f2fca7b79670c4df9c2ec8bdf03cd