From dc1d4bc9519f7447342b58dbf136606438831f05 Mon Sep 17 00:00:00 2001 From: Ion Reguera Date: Thu, 11 Jun 2026 12:30:58 +0200 Subject: [PATCH 01/14] chore(mtcreceiver): bump to 24h-continuity-rc1 (7e9b81d) + clear wrap on start() >24h MTC continuity baseline. The fade engine consumes the QF tick callback (MtcTickSource -> callbackMs); the unified receiver makes callbackMs continuous past 24h so fades keep their phase across the rollover. Add MtcReceiver::resetWrapOffset() in MtcTickSource::start() before constructing the receiver, so a tick source restarted in-process after a >24h run starts the new session at 0 (the wire-driven reset can't catch a graceful restart's small backward delta). 9/9 unit tests pass incl. test_mtc_tick_source. Plan 3d. --- mtcreceiver | 2 +- src/time/MtcTickSource.cpp | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/mtcreceiver b/mtcreceiver index 59fc76e..7e9b81d 160000 --- a/mtcreceiver +++ b/mtcreceiver @@ -1 +1 @@ -Subproject commit 59fc76edb4ded90296c1aa904a9540c1aabf2ab5 +Subproject commit 7e9b81d4eeda26e86ddba3ae4305927501144d84 diff --git a/src/time/MtcTickSource.cpp b/src/time/MtcTickSource.cpp index 35bb586..caf7263 100644 --- a/src/time/MtcTickSource.cpp +++ b/src/time/MtcTickSource.cpp @@ -74,6 +74,13 @@ MtcStartError MtcTickSource::start(const std::string& midiPort) { return MtcStartError::kPortNotFound; } + // Clear the >24h wrap accumulator before (re)starting. MtcReceiver keeps it + // in a process-global static, so a tick source restarted in-process after a + // >24h run would otherwise inherit a stale +86_400_000 ms offset and put the + // new session 24h ahead. The wire-driven reset can't catch a graceful + // restart (small backward delta), so clear it explicitly here. (Plan 3d) + MtcReceiver::resetWrapOffset(); + // Construct MtcReceiver — opens the port and starts the checker thread. // portIndex is the last constructor parameter (preserves positional compat). receiver_ = std::make_unique( From 069f951ce0e1d525d3df3f2361efaad71a770d76 Mon Sep 17 00:00:00 2001 From: Ion Reguera Date: Thu, 11 Jun 2026 17:49:49 +0200 Subject: [PATCH 02/14] chore(mtcreceiver): re-pin to 24h-continuity-rc2 (8a30d05) rc1 + Plan 5 Phase A hardening (strict-< resync tolerance, test-state timeout reset). API-identical to rc1 (no signature change); the soak ships this commit. --- mtcreceiver | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mtcreceiver b/mtcreceiver index 7e9b81d..8a30d05 160000 --- a/mtcreceiver +++ b/mtcreceiver @@ -1 +1 @@ -Subproject commit 7e9b81d4eeda26e86ddba3ae4305927501144d84 +Subproject commit 8a30d056e5fbe27ef19f0485c5ef142b7f7b4ef0 From e121fd941c87d33ccc25e0ccc6f5bd5751c824de Mon Sep 17 00:00:00 2001 From: Ion Reguera Date: Tue, 7 Jul 2026 20:35:24 +0200 Subject: [PATCH 03/14] =?UTF-8?q?docs(claude):=20restructure=20CLAUDE.md?= =?UTF-8?q?=20=E2=80=94=20component=20detail=20extracted=20from=20cuems-RE?= =?UTF-8?q?LATIONS=20index?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CLAUDE.md | 48 ++++++++++++++---------------------------------- 1 file changed, 14 insertions(+), 34 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 6292635..355d92d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,44 +1,24 @@ -# gradient-motion-engine Development Guidelines +# gradient-motion-engine -Auto-generated from all feature plans. Last updated: 2026-04-23 +Part of the **CUEMS** ecosystem — see the [`cuems-RELATIONS`](https://github.com/stagesoft/cuems-RELATIONS) repo for the system index, architecture diagram, and protocol/port map. -## Active Technologies -- C++17 (GCC, `-Wall -O3 -pthread`) + None (C++ standard library only — ``, ``, ``, ``, ``) (002-gradient-curves) -- C++17 (GCC, `-Wall -O3 -pthread`) + mtcreceiver v2.0.0 (submodule, pinned at `59fc76e`), (004-adapt-mtc-tick-v2) -- N/A (in-memory adapter; no persistence) (004-adapt-mtc-tick-v2) -- C++17 (GCC, `-Wall -O3 -pthread`) + NNG 1.10.1 (`libnng-dev`, C API — `nng_bus0_open`, (005-nng-bus-client) -- N/A — all state is in-memory. The queue is a fixed-size array; (005-nng-bus-client) -- C++17 (GCC, `-Wall -O3 -pthread`) + liblo (OSC sending), NNG 1.10.1 (already linked), nlohmann-json (already linked), RtMidi via mtcreceiver submodule (already linked) (006-fade-registry-tick-loop) -- N/A — all state in-memory (`std::unordered_map` inside `FadeRegistry`, fixed SPSC queue for status) (006-fade-registry-tick-loop) +## Role -- C++17 (GCC, `-Wall -O3 -pthread`) (001-phase0-scaffold) +Timecode-driven motion and gradient evaluation engine with OSC output. C++17 (GCC, `-Wall -O3 -pthread`). MTC-synced via the `mtcreceiver` submodule; talks to the CUEMS engine over the NNG bus (`libnng`) and sends OSC via `liblo`. On CUEMS hosts it listens on the `gradient_osc_port` (7100 in `settings.xml`). Also uses nlohmann-json. -## Project Structure +## Active technologies (per feature history) -```text -src/ -tests/ -``` +- C++17 (GCC, `-Wall -O3 -pthread`), C++ standard library only for the core (``, ``, ``, ``, ``) — `001-phase0-scaffold`, `002-gradient-curves`. +- `mtcreceiver` v2.0.0 (submodule) — `004-adapt-mtc-tick-v2`. +- NNG 1.10.1 (`libnng-dev`, C API `nng_bus0_open`) — `005-nng-bus-client`. +- liblo (OSC), nlohmann-json, RtMidi via mtcreceiver — `006-fade-registry-tick-loop`. All state in-memory (`FadeRegistry` map + fixed SPSC status queue). -## Commands +## Build & release -# Add commands for C++17 (GCC, `-Wall -O3 -pthread`) +Standard C++ submodule build (`git submodule update --init`, cmake/make). Release lineage: `rc_1` carries the fleet-wide MTC >24h work + `24h_extended_support` tag (tip `069f951`, 24h `mtcreceiver` `8a30d05`); `main` is the development line. -## Code Style +For additional per-feature context (project structure, shell commands), read the current plan at [specs/007-osc-input-transport/plan.md](specs/007-osc-input-transport/plan.md). Non-code artifacts follow the same `specs/planning/` convention as cuems-utils. -C++17 (GCC, `-Wall -O3 -pthread`): Follow standard conventions +## Field notes -## Recent Changes -- 006-fade-registry-tick-loop: Added C++17 (GCC, `-Wall -O3 -pthread`) + liblo (OSC sending), NNG 1.10.1 (already linked), nlohmann-json (already linked), RtMidi via mtcreceiver submodule (already linked) -- 005-nng-bus-client: Added C++17 (GCC, `-Wall -O3 -pthread`) + NNG 1.10.1 (`libnng-dev`, C API — `nng_bus0_open`, -- 004-adapt-mtc-tick-v2: Added C++17 (GCC, `-Wall -O3 -pthread`) + mtcreceiver v2.0.0 (submodule, pinned at `59fc76e`), - - - - - - -For additional context about technologies to be used, project structure, -shell commands, and other important information, read the current plan: -[specs/007-osc-input-transport/plan.md](specs/007-osc-input-transport/plan.md) - +- Uses the shared `mtcreceiver` submodule — the 2s-resync-skip fix (`aa44894`) and the raw-wire-MTC timebase apply here too; see the mtcreceiver CLAUDE.md. From 2454c2f17485a22e6cb4db2c3069d280dd70fed2 Mon Sep 17 00:00:00 2001 From: Ion Reguera Date: Tue, 7 Jul 2026 21:05:23 +0200 Subject: [PATCH 04/14] docs(claude): document gradient-motiond daemon + engine GradientClient OSC contract --- CLAUDE.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 355d92d..9f9ae30 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -4,7 +4,9 @@ Part of the **CUEMS** ecosystem — see the [`cuems-RELATIONS`](https://github.c ## Role -Timecode-driven motion and gradient evaluation engine with OSC output. C++17 (GCC, `-Wall -O3 -pthread`). MTC-synced via the `mtcreceiver` submodule; talks to the CUEMS engine over the NNG bus (`libnng`) and sends OSC via `liblo`. On CUEMS hosts it listens on the `gradient_osc_port` (7100 in `settings.xml`). Also uses nlohmann-json. +Timecode-driven motion and gradient evaluation engine with OSC output — runs as the daemon **`gradient-motiond`** (unit wired by cuems-common). C++17 (GCC, `-Wall -O3 -pthread`). MTC-synced via the `mtcreceiver` submodule; talks to the CUEMS engine over the NNG bus (`libnng`) and sends OSC via `liblo`. Also uses nlohmann-json. + +**Engine-side client:** `cuems-engine`'s `GradientClient` (`players/GradientClient.py`) is a fire-and-forget UDP OSC client targeting `gradient_osc_port` (7100 in `settings.xml`). Commands: `/gradient/start_fade`, `/gradient/cancel_motion `, `/gradient/cancel_all`. The engine delegates cue fades here (loop-cue fades, ActionCue fades via `ActionHandler`) — the engine's loop only supervises; the fade curve itself is evaluated by this daemon. ## Active technologies (per feature history) From bc3fb0d0b8a870f77ba8d2b38899222b5fbb1466 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Masip?= Date: Wed, 29 Jul 2026 18:56:48 +0200 Subject: [PATCH 05/14] chore(mtcreceiver): re-pin to 59fc76e (mtcframe-conversions-and-fullframe-reset) Working tree submodule was already checked out at 59fc76e (per CLAUDE.md's documented pin) but the last commit on this branch had re-pinned the recorded pointer to 8a30d05, leaving the two out of sync. --- mtcreceiver | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mtcreceiver b/mtcreceiver index 8a30d05..59fc76e 160000 --- a/mtcreceiver +++ b/mtcreceiver @@ -1 +1 @@ -Subproject commit 8a30d056e5fbe27ef19f0485c5ef142b7f7b4ef0 +Subproject commit 59fc76edb4ded90296c1aa904a9540c1aabf2ab5 From 1d4b23511dcf6ce45689d400e4dd65b7d7b4f9be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Masip?= Date: Wed, 29 Jul 2026 18:56:59 +0200 Subject: [PATCH 06/14] docs(planning): add T069 plan for OSC send-failure misclassification Debugging session on the cuems-engine side traced silently-dying fades back to FadeMotion::evalAndSend treating lo_send()'s bytes-sent return value as a failure indicator (ret != 0) instead of checking for < 0. --- ...-fix-osc-send-failure-misclassification.md | 288 ++++++++++++++++++ 1 file changed, 288 insertions(+) create mode 100644 specs/planning/T069-fix-osc-send-failure-misclassification.md diff --git a/specs/planning/T069-fix-osc-send-failure-misclassification.md b/specs/planning/T069-fix-osc-send-failure-misclassification.md new file mode 100644 index 0000000..486074c --- /dev/null +++ b/specs/planning/T069-fix-osc-send-failure-misclassification.md @@ -0,0 +1,288 @@ + + +# T069 — Fix OSC Send-Failure Misclassification in `FadeMotion::evalAndSend` + +**Issued by**: cuems-engine debugging session (`fix/gradient-engine-action` branch) +**Target repo**: `gradient-motion-engine` +**Primary file**: `src/motion/FadeMotion.cpp` +**Context version**: v0.3.0 (`debian/changelog` top entry `cuems-gradient-motiond (0.3.0-1)`) +**Date**: 2026-07-29 +**Status**: plan — not yet applied. To be validated per the checklist below, then folded into the package (changelog entry + version bump) once green. + +--- + +## Background + +While debugging a separate, already-fixed `node_name`-filter bug in cuems-engine +(`GradientClient` was sending the node's UUID instead of the daemon's +`--node-name`), we confirmed the daemon now correctly *receives* +`/gradient/start_fade`. But the downstream OSC tick to the target player +(`osc_host:osc_port:osc_path`) never arrives, or arrives for only a handful of +ticks before the fade silently dies. + +### Root cause (verified, not hypothesized) + +`FadeMotion::evalAndSend` (`src/motion/FadeMotion.cpp:81-89`): + +```cpp +const int ret = oscSend_(osc_target_, osc_path.c_str(), value); // -> lo_send() +... +r.failed = (ret != 0); +``` + +`oscSend_` defaults to `gme::osc::sendFloat` (`src/osc/OscSender.cpp:20-24`), +which forwards directly to liblo's `lo_send()`. Per `/usr/include/lo/lo.h:128`, +`lo_send` returns **`-1` on failure**; on success it returns the number of +bytes sent — **never `0`**. This was verified empirically, not just read from +the header comment: + +```c +// compiled against the installed liblo.so.7 (0.32-2) +lo_address addr = lo_address_new("127.0.0.1", "9999"); +int ret = lo_send(addr, "/test/path", "f", 0.5f); +printf("lo_send returned: %d\n", ret); // -> 20 +``` + +So `ret != 0` is **true on every successful send**, not just on failures. +`MotionRegistry::tick` (`src/motion/MotionRegistry.cpp:173-186`) increments +`m.consecutive_osc_failures` whenever `r.failed`, and at +`kOscFailureThreshold = 5` (`src/motion/MotionRegistry.h:74-81`, "at 200 Hz +this equals 25 ms") emits `MotionError:"osc_send_failed"` and removes the +motion. Net effect: **every fade with `duration_ms` greater than ~25 ms is +killed almost immediately after starting**, having sent only a handful of +ticks near `start_value`. The daemon logs this at INFO level — confirmed live +against `cuems-gradient-motiond` on the test box via +`journalctl -u cuems-gradient-motiond`, which showed +`GradientEngine: MotionError motion_id=... reason=osc_send_failed` within +~25 ms of the corresponding `start_fade`. + +### Why the existing test suite didn't catch it + +Every `OscSendFn` mock in `tests/test_fade_motion.cpp` and +`tests/test_motion_registry.cpp` uses the **opposite, POSIX-style** +convention — `return 0;` for success, `return -1;` for injected failure +(e.g. `tests/test_fade_motion.cpp:150,167,189,213,228,321,361,380,398,496,510,524` +and the explicit failure-injection tests at `:443-446,470-473`). No existing +test ever passes a positive "success" value through the send function, so +nothing exercises the real liblo return convention. `MotionRegistry.h:93` / +`FadeMotion.h:65` declare `OscSendFn = std::function` with no documented return-value contract — that omission is +the actual gap that let the mismatch through code review. + +### Fix + +`FadeMotion.cpp:88`: `r.failed = (ret != 0);` → `r.failed = (ret < 0);` + +--- + +## Execution checklist + +### 1. Checkout + +```bash +cd /disk/Projects/StageLab/gradient-motion-engine +git status # confirm clean tree before starting +git checkout -b fix/osc-send-failure-threshold-t069 +``` + +### 2. Regression test FIRST (TDD — confirm it fails before the fix) + +Add to `tests/test_fade_motion.cpp`, in the US4 section (after +`test_us4_transient_failure_recovery`, ~line 487), following the existing +`TestCtx`/`ASSERT_TRUE` idiom: + +```cpp +static bool test_us4_real_lo_send_success_not_misclassified() { + // Regression for T069: liblo's lo_send() returns bytes-sent (a positive, + // non-zero int) on success, never 0. A mock that mirrors this — instead + // of the POSIX-style "0 = success" convention every other test in this + // file uses — must NOT trip the failure counter. + TestCtx ctx; + auto send = [](lo_address, const char*, float) -> int { return 20; }; + auto reg = ctx.makeRegWithSend(send); + + auto cmd = TestCtx::makeCmd("freal", "linear", 0.0f, 1.0f, 10000.0f, 0); + reg->apply(cmd); + + for (int i = 0; i < gme::motion::MotionRegistry::kOscFailureThreshold; ++i) { + reg->tick(i * 10); + } + + ASSERT_TRUE(reg->size() == 1, + "us4_real_lo_send: fade must survive N ticks of a real " + "liblo-style positive return value"); + + for (auto& r : ctx.emitted) + ASSERT_TRUE(r.reason != "osc_send_failed", + "us4_real_lo_send: no spurious osc_send_failed"); + return true; +} +``` + +Register it in `main()`'s `tests[]` array (~line 468): +```cpp +{"us4_real_lo_send_not_misclassified", test_us4_real_lo_send_success_not_misclassified}, +``` + +Build and run — **this test must FAIL on the current code** (proves the +regression test actually exercises the bug before touching production code): + +```bash +cmake -B build -DCMAKE_BUILD_TYPE=Debug +cmake --build build -j"$(nproc)" +ctest --test-dir build --output-on-failure -R test_fade_motion +``` +Expected: `FAIL [us4_real_lo_send: ...]` and the ctest case for +`test_fade_motion` reports failure. If it passes before the fix, stop — +something about the reproduction is wrong and the fix must not proceed on a +false premise. + +### 3. Apply the fix + +`src/motion/FadeMotion.cpp:88`: +```diff +- r.failed = (ret != 0); ++ r.failed = (ret < 0); +``` + +Re-run the same build/test command. All of `test_fade_motion` (including the +new case and the two pre-existing US4 tests, which still use the POSIX-style +mock and must keep passing since `-1 < 0` is still `true`) must pass: + +```bash +cmake --build build -j"$(nproc)" +ctest --test-dir build --output-on-failure +``` + +Run the **full** suite, not just `test_fade_motion` — confirm no other test +implicitly depended on the old (buggy) comparison: +```bash +ctest --test-dir build --output-on-failure +``` + +### 4. Document the return-value contract (prevents recurrence) + +Add a one-line contract note to the `OscSendFn` declarations so the next +person wiring a mock doesn't repeat the mismatch: + +- `src/motion/FadeMotion.h:65` (near the `OscSendFn` typedef) +- `src/motion/MotionRegistry.h:93` (same) + +```cpp +// Return convention matches liblo's lo_send(): >= 0 (bytes sent) on +// success, < 0 on failure. NEVER assume 0 == success — lo_send never +// returns exactly 0 for a non-empty message. +using OscSendFn = std::function; +``` + +### 5. Additional logging + +The current failure path logs only the terminal `MotionError:"osc_send_failed"` +at INFO once the threshold is hit — there is no visibility into individual +tick-level send failures leading up to it, which made this bug hard to +distinguish from "daemon never received the command at all" during the +original investigation. Add: + +- **`FadeMotion::evalAndSend`** (`FadeMotion.cpp`, right after the `oscSend_` + call): on `ret < 0`, emit a `GME_LOG_WARNING` (via a callback or a direct + include of `logging.h` — check whether `FadeMotion` currently has any + logging dependency; if not, prefer threading a lightweight log callback + through `MotionRegistry::tick` rather than adding a hard dependency from + `src/motion/` on `daemon/`'s logging, to keep the library/daemon split + intact per `GradientEngine.cpp`'s comment about `libgradient_motion` vs the + daemon binary) including `motion_id`, `osc_path`, and the raw `ret` value: + ``` + WARNING FadeMotion: osc send failed motion_id= path= lo_send_ret= + ``` + Rate-limit or dedupe if this proves noisy in practice (a real dead target + will log this every tick for `kOscFailureThreshold` ticks before removal — + 5 lines is fine, no rate-limiting needed at that volume). +- **`MotionRegistry::tick`** (`MotionRegistry.cpp:173-186`): when a motion + recovers from a nonzero `consecutive_osc_failures` count back to 0 (i.e. + transient failure that self-healed), log at DEBUG: + `DEBUG MotionRegistry: motion_id= osc send recovered after failures`. + This distinguishes "always worked" from "flaked and recovered" in the + journal without adding INFO-level noise to the common case. + +### 6. Version bump + +Source of truth: `CMakeLists.txt:28` → `project(gradient-motion-engine VERSION 0.3.0 ...)`. +This is a bug fix, not a feature — bump the patch component per the existing +convention (`git log` shows prior bumps as a single commit touching +`CMakeLists.txt` + `debian/changelog` + `CHANGELOG.md` together): + +- `CMakeLists.txt:28`: `VERSION 0.3.0` → `VERSION 0.3.1` +- `debian/changelog`: new top entry: + ``` + cuems-gradient-motiond (0.3.1-1) unstable; urgency=medium + + * Fix FadeMotion::evalAndSend misclassifying every successful lo_send() + as a failure (ret != 0 instead of ret < 0), which silently killed + every fade with duration_ms > ~25ms via the 5-consecutive-failure + threshold. Add OscSendFn return-value contract docs and warning-level + logging on individual send failures. + + -- + ``` +- `CHANGELOG.md`: new `## [0.3.1] - 2026-07-29` entry under a `### Fixed` + heading, referencing this plan file (`specs/planning/T069-...md`). + +### 7. Build the package + +Per `debian/rules` (standard `dh $@`, configured via +`override_dh_auto_configure` with `-DBUILD_TESTS=OFF` for the release build): + +```bash +cd /disk/Projects/StageLab/gradient-motion-engine +dpkg-buildpackage -us -uc -b +``` + +This produces `../cuems-gradient-motiond_0.3.1-1_amd64.deb` (and the +`-dbgsym` companion) in the parent directory, matching the naming pattern of +the currently-installed `cuems-gradient-motiond_0.3.0-1_amd64.deb`. + +### 8. Pre-deploy validation (before this file is "passed into the package") + +- [ ] `ctest --test-dir build --output-on-failure` — full suite green, + including the new regression test. +- [ ] New regression test fails on a clean revert of the one-line fix + (sanity-check the test isn't vacuously passing). +- [ ] `dpkg-buildpackage` completes without error; `dpkg -c` on the resulting + `.deb` shows the updated `/usr/bin/gradient-motiond`. +- [ ] Manual smoke test on the dev/test box (`10.16.10.3`, per prior session): + install the new `.deb`, restart `cuems-gradient-motiond`, replay the + same raw `/gradient/start_fade` probe used earlier in this + investigation (correct `node_name`, `duration_ms` well above 25ms — + e.g. 5000), and confirm via `journalctl -u cuems-gradient-motiond + --since ` that `MotionComplete` appears **after** roughly + `duration_ms` has elapsed (not ~25ms later), with **no** + `MotionError:"osc_send_failed"` in between. + - [ ] Independently confirm the downstream player actually received the + ticks (e.g. a throwaway UDP listener on the probe's `osc_port`, or + observing the real player's parameter change) — the journal check + alone only proves the daemon *thinks* it succeeded now, which is + exactly the class of assumption that caused this bug in the first + place; corroborate with an actual received packet. + +### 9. Rollback plan + +If the fix regresses something unexpected: `dpkg -i` the previously-built +`cuems-gradient-motiond_0.3.0-1_amd64.deb` (already present in +`/disk/Projects/StageLab/`) and restart the service. No schema/data +migration is involved — this is a pure logic fix with no persistent state, +so rollback is a straight binary swap. + +--- + +## Related artifacts + +| Artifact | Location | +|---|---| +| Daemon source of the bug | `src/motion/FadeMotion.cpp:81-89` | +| Threshold logic | `src/motion/MotionRegistry.cpp:173-186`, `MotionRegistry.h:74-81` | +| liblo contract | `/usr/include/lo/lo.h:122-130` | +| Existing test harness pattern | `tests/test_fade_motion.cpp` (`TestCtx`, `test_us4_osc_failure_threshold`) | +| cuems-engine node_name fix (separate, already applied) | cuems-engine `src/cuemsengine/players/GradientClient.py`, `NodeEngine.py::_resolve_gradient_node_name` | From d9e470d5b0fa3f2df461aef0f41ddb0f3176467c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Masip?= Date: Wed, 29 Jul 2026 18:58:08 +0200 Subject: [PATCH 07/14] Revert "chore(mtcreceiver): re-pin to 59fc76e (mtcframe-conversions-and-fullframe-reset)" This reverts commit bc3fb0d0b8a870f77ba8d2b38899222b5fbb1466. --- mtcreceiver | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mtcreceiver b/mtcreceiver index 59fc76e..8a30d05 160000 --- a/mtcreceiver +++ b/mtcreceiver @@ -1 +1 @@ -Subproject commit 59fc76edb4ded90296c1aa904a9540c1aabf2ab5 +Subproject commit 8a30d056e5fbe27ef19f0485c5ef142b7f7b4ef0 From bedcc3f4555f988ae357649851449b73ecf6cea6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Masip?= Date: Wed, 29 Jul 2026 19:03:23 +0200 Subject: [PATCH 08/14] fix: correct OSC send-failure classification in FadeMotion::evalAndSend (v0.3.1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit T069 — liblo's lo_send() returns bytes-sent (positive, non-zero) on success and -1 on failure, never 0. FadeMotion::evalAndSend checked `ret != 0`, which misclassified every successful send as a failure and killed any fade with duration_ms > ~25ms once kOscFailureThreshold (5 consecutive ticks) was reached — sending only a handful of ticks near start_value before removal. The existing test suite didn't catch this because every OscSendFn mock used the opposite POSIX-style "0 = success" convention; nothing exercised a real liblo-style positive return. - src/motion/FadeMotion.cpp: `ret != 0` -> `ret < 0`. - src/motion/FadeMotion.h, MotionRegistry.h: document the OscSendFn return-value contract to prevent recurrence. - src/motion/FadeMotion.cpp: WARNING log on individual send failures. - src/motion/MotionRegistry.cpp: DEBUG log when a motion recovers from a transient failure streak. - tests/test_fade_motion.cpp: regression test with a liblo-style mock (returns 20) asserting the fade survives kOscFailureThreshold ticks. - CMakeLists.txt, debian/changelog, CHANGELOG.md: bump 0.3.0 -> 0.3.1. See specs/planning/T069-fix-osc-send-failure-misclassification.md. Co-Authored-By: Claude Sonnet 5 --- CHANGELOG.md | 18 ++++++++++++++++++ CMakeLists.txt | 2 +- debian/changelog | 10 ++++++++++ src/motion/FadeMotion.cpp | 9 ++++++++- src/motion/FadeMotion.h | 10 ++++++++-- src/motion/MotionRegistry.cpp | 6 ++++++ src/motion/MotionRegistry.h | 4 ++++ tests/test_fade_motion.cpp | 27 +++++++++++++++++++++++++++ 8 files changed, 82 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 55b95c1..cb5703f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,24 @@ All notable changes to this project are documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.3.1] - 2026-07-29 — Fix OSC send-failure misclassification + +See [specs/planning/T069-fix-osc-send-failure-misclassification.md](specs/planning/T069-fix-osc-send-failure-misclassification.md). + +### Fixed + +* **`FadeMotion::evalAndSend`** (`src/motion/FadeMotion.cpp`): `lo_send()` returns the + number of bytes sent (a positive, non-zero int) on success and `-1` on failure — never + `0`. The failure check used `ret != 0`, which misclassified every successful send as a + failure and killed any fade with `duration_ms` greater than ~25ms once + `kOscFailureThreshold` (5 consecutive ticks) was reached. Changed to `ret < 0`. +* Documented the `OscSendFn` return-value contract on both declarations + (`src/motion/FadeMotion.h`, `src/motion/MotionRegistry.h`) to prevent test mocks (or + future callers) from reintroducing the POSIX-style "0 = success" assumption. +* Added a `WARNING` log on individual OSC send failures in `FadeMotion::evalAndSend`, and + a `DEBUG` log in `MotionRegistry::tick` when a motion recovers from a transient failure + streak — prior visibility was limited to the terminal `MotionError:"osc_send_failed"`. + ## [0.3.0] - 2026-05-13 — Phase H: OSC Input Transport Replaces the NNG bus-client inbound command transport with a localhost UDP OSC listener. diff --git a/CMakeLists.txt b/CMakeLists.txt index 8dc76f9..e0a07cb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -25,7 +25,7 @@ cmake_minimum_required(VERSION 3.10) # Allow target_link_libraries() on targets defined in other directories # (needed to link cuemslogger into mtcreceiver from the root CMakeLists) cmake_policy(SET CMP0079 NEW) -project(gradient-motion-engine VERSION 0.3.0 LANGUAGES CXX) +project(gradient-motion-engine VERSION 0.3.1 LANGUAGES CXX) set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD_REQUIRED ON) diff --git a/debian/changelog b/debian/changelog index 4c5a3d1..cc17494 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,13 @@ +cuems-gradient-motiond (0.3.1-1) unstable; urgency=medium + + * Fix FadeMotion::evalAndSend misclassifying every successful lo_send() + as a failure (ret != 0 instead of ret < 0), which silently killed + every fade with duration_ms > ~25ms via the 5-consecutive-failure + threshold. Add OscSendFn return-value contract docs and warning-level + logging on individual send failures. + + -- Adrià Masip Wed, 29 Jul 2026 19:00:53 +0200 + cuems-gradient-motiond (0.3.0-1) unstable; urgency=medium * Phase H — OSC Input Transport (spec 007-osc-input-transport). diff --git a/src/motion/FadeMotion.cpp b/src/motion/FadeMotion.cpp index a636685..3d5e7ad 100644 --- a/src/motion/FadeMotion.cpp +++ b/src/motion/FadeMotion.cpp @@ -15,6 +15,7 @@ #include #include +#include namespace gme { namespace motion { @@ -79,13 +80,19 @@ EvalResult FadeMotion::evalAndSend(long mtc_ms) { // Send OSC const int ret = oscSend_(osc_target_, osc_path.c_str(), value); + if (ret < 0) { + std::fprintf(stderr, + "WARNING FadeMotion: osc send failed motion_id=%s path=%s " + "lo_send_ret=%d\n", + motion_id.c_str(), osc_path.c_str(), ret); + } // Update last_sent_value unconditionally (registry removes on failure anyway) last_sent_value = value; EvalResult r; r.completed = (t >= 1.0f); - r.failed = (ret != 0); + r.failed = (ret < 0); r.failure_reason = r.failed ? "osc_send_failed" : nullptr; return r; } diff --git a/src/motion/FadeMotion.h b/src/motion/FadeMotion.h index e4c4ce7..ad58223 100644 --- a/src/motion/FadeMotion.h +++ b/src/motion/FadeMotion.h @@ -61,7 +61,13 @@ namespace motion { */ class FadeMotion final : public IMotion { public: - /** @brief Function type matching `gme::osc::sendFloat`. */ + /** + * @brief Function type matching `gme::osc::sendFloat`. + * + * Return convention matches liblo's `lo_send()`: `>= 0` (bytes sent) on + * success, `< 0` on failure. NEVER assume `0` == success — `lo_send` + * never returns exactly `0` for a non-empty message. + */ using OscSendFn = std::function; /** @@ -124,7 +130,7 @@ class FadeMotion final : public IMotion { * `consecutive_osc_failures` — that is managed by `MotionRegistry::tick`. * * @param mtc_ms Current MTC head position in milliseconds. - * @return `EvalResult { completed=(t>=1.0), failed=(ret!=0) }`. + * @return `EvalResult { completed=(t>=1.0), failed=(ret<0) }`. * * @throws Never. */ diff --git a/src/motion/MotionRegistry.cpp b/src/motion/MotionRegistry.cpp index 88dd8fb..7d0de93 100644 --- a/src/motion/MotionRegistry.cpp +++ b/src/motion/MotionRegistry.cpp @@ -182,6 +182,12 @@ void MotionRegistry::tick(long mtc_ms) { continue; } } else { + if (m.consecutive_osc_failures > 0) { + std::fprintf(stderr, + "DEBUG MotionRegistry: motion_id=%s osc send " + "recovered after %d failures\n", + m.motion_id.c_str(), m.consecutive_osc_failures); + } m.consecutive_osc_failures = 0; } diff --git a/src/motion/MotionRegistry.h b/src/motion/MotionRegistry.h index 543eaea..dc442d7 100644 --- a/src/motion/MotionRegistry.h +++ b/src/motion/MotionRegistry.h @@ -89,6 +89,10 @@ class MotionRegistry { * * Stored and forwarded to `MotionFactory::fromCommand` when a `START_FADE` * command is applied. Not called directly by the registry. + * + * Return convention matches liblo's `lo_send()`: `>= 0` (bytes sent) on + * success, `< 0` on failure. NEVER assume `0` == success — `lo_send` + * never returns exactly `0` for a non-empty message. */ using OscSendFn = std::function; diff --git a/tests/test_fade_motion.cpp b/tests/test_fade_motion.cpp index 8249a9d..0ef562c 100644 --- a/tests/test_fade_motion.cpp +++ b/tests/test_fade_motion.cpp @@ -486,6 +486,32 @@ static bool test_us4_transient_failure_recovery() { return true; } +static bool test_us4_real_lo_send_success_not_misclassified() { + // Regression for T069: liblo's lo_send() returns bytes-sent (a positive, + // non-zero int) on success, never 0. A mock that mirrors this — instead + // of the POSIX-style "0 = success" convention every other test in this + // file uses — must NOT trip the failure counter. + TestCtx ctx; + auto send = [](lo_address, const char*, float) -> int { return 20; }; + auto reg = ctx.makeRegWithSend(send); + + auto cmd = TestCtx::makeCmd("freal", "linear", 0.0f, 1.0f, 10000.0f, 0); + reg->apply(cmd); + + for (int i = 0; i < gme::motion::MotionRegistry::kOscFailureThreshold; ++i) { + reg->tick(i * 10); + } + + ASSERT_TRUE(reg->size() == 1, + "us4_real_lo_send: fade must survive N ticks of a real " + "liblo-style positive return value"); + + for (auto& r : ctx.emitted) + ASSERT_TRUE(r.reason != "osc_send_failed", + "us4_real_lo_send: no spurious osc_send_failed"); + return true; +} + // --------------------------------------------------------------------------- // US5 — MTC Pause, Resume, Rewind Behavior // --------------------------------------------------------------------------- @@ -557,6 +583,7 @@ int main() { {"us3_cancel_all_timing", test_us3_cancel_all_timing}, {"us4_failure_threshold", test_us4_osc_failure_threshold}, {"us4_transient_recovery", test_us4_transient_failure_recovery}, + {"us4_real_lo_send_not_misclassified", test_us4_real_lo_send_success_not_misclassified}, {"us5_pause_no_ticks", test_us5_pause_no_ticks}, {"us5_resume_correct_value", test_us5_resume_correct_value}, {"us5_rewind_before_start", test_us5_rewind_before_start}, From ec7e812b80140512552b973009b1f5061f7a7ab2 Mon Sep 17 00:00:00 2001 From: Ion Reguera Date: Wed, 29 Jul 2026 19:43:23 +0200 Subject: [PATCH 09/14] fix(log): arm CuemsLogger on the gradient_motion library; route bypasses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit NOT MERGED, NOT DEPLOYED — authored on a branch for review at your convenience (active work is ongoing in this repo). gradient-motiond's daemon-side logging already works on the fleet (syslog, real priorities); this fixes the LIBRARY half. Root cause: HAVE_CUEMS_LOGGER was defined on mtcreceiver and the gradient-motiond executable but NOT on the gradient_motion static library, so every GME_LOG_* compiled into the library fell back to logging.h's std::cerr emitter — stamped PRIORITY=6 under systemd, invisible to `cuems-logs -e` — and GradientEngine.cpp, listed in BOTH targets, compiled twice with two different logging backends. That unavailable macro layer is also why src/motion/* hand-rolled fprintf(stderr, "WARNING ...") lines. * src/CMakeLists.txt: define + link PUBLIC on gradient_motion, guarded by ENABLE_CUEMS_LOGGER AND TARGET cuemslogger (covers BUILD_DAEMON=OFF). PUBLIC kills the dual-backend hazard: daemon and the test binaries now share one backend. Side effect to be aware of: ctest output goes to syslog when cuemslogger is enabled. * Root CMakeLists: drop the now-redundant PRIVATE arming on the daemon (it inherits via the library's usage requirements). * Hand-rolled fprintf/cerr sites -> GME_LOG_*: MotionFactory (unknown curve, lo_address_new failure -> WARNING; START_CROSSFADE stub -> INFO), MotionRegistry (crossfade drop -> INFO, cancelMotion miss -> WARNING), CurveFactory (unknown curve -> WARNING). * OscServer: liblo errorHandler + callback exceptions -> GME_LOG_ERROR; the two startup-aborting failures (UDP bind, lo_server_thread_start) -> GME_LOG_CRITICAL. These bypassed the GME_LOG_* layer the same file uses everywhere else. * GradientEngine::onTick (MIDI thread, 100 Hz) deliberately untouched — already level-gated and queue-pop-only. Verified: full build (daemon + tests), ctest 9/9 passed, libgradient_motion.a now resolves CuemsLogger symbols. --- CMakeLists.txt | 7 +++---- daemon/comms/OscServer.cpp | 17 ++++++++--------- src/CMakeLists.txt | 14 ++++++++++++++ src/gradient/CurveFactory.cpp | 5 +++-- src/motion/MotionFactory.cpp | 15 ++++++++------- src/motion/MotionRegistry.cpp | 10 +++++----- 6 files changed, 41 insertions(+), 27 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 8dc76f9..2a07810 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -176,10 +176,9 @@ if(BUILD_DAEMON) target_include_directories(gradient-motiond PRIVATE ${LIBLO_INCLUDE_DIRS}) endif() - if(ENABLE_CUEMS_LOGGER) - target_compile_definitions(gradient-motiond PRIVATE HAVE_CUEMS_LOGGER) - target_link_libraries(gradient-motiond PRIVATE cuemslogger) - endif() + # HAVE_CUEMS_LOGGER + cuemslogger now arrive via gradient_motion's + # PUBLIC usage requirements (src/CMakeLists.txt) — armed on the library + # so all its objects and every consumer share one logging backend. # Install rules — places the daemon at /bin/gradient-motiond. # debian/rules sets CMAKE_INSTALL_PREFIX=/usr so the .deb ships diff --git a/daemon/comms/OscServer.cpp b/daemon/comms/OscServer.cpp index 8687ce2..8522107 100644 --- a/daemon/comms/OscServer.cpp +++ b/daemon/comms/OscServer.cpp @@ -40,8 +40,8 @@ struct OscServer::Impl { bool started = false; static void errorHandler(int num, const char* msg, const char* where) { - std::fprintf(stderr, "ERROR OscServer: liblo error %d — %s (path: %s)\n", - num, msg ? msg : "", where ? where : ""); + GME_LOG_ERROR("OscServer: liblo error " + std::to_string(num) + " — " + + (msg ? msg : "") + " (path: " + (where ? where : "") + ")"); } // Per-address callback entry point (called on the liblo network thread). @@ -84,11 +84,11 @@ struct OscServer::Impl { break; } } catch (const std::exception& e) { - std::fprintf(stderr, "ERROR OscServer: exception in callback for %s: %s\n", - path, e.what()); + GME_LOG_ERROR("OscServer: exception in callback for " + std::string(path) + + ": " + e.what()); } catch (...) { - std::fprintf(stderr, "ERROR OscServer: unknown exception in callback for %s\n", - path); + GME_LOG_ERROR("OscServer: unknown exception in callback for " + + std::string(path)); } return 0; // 0 = handled; do not try further methods } @@ -125,8 +125,7 @@ bool OscServer::start() { impl_->server_thread = lo_server_thread_new(port_str.c_str(), Impl::errorHandler); if (!impl_->server_thread) { - std::fprintf(stderr, "FATAL OscServer: failed to bind UDP port %s\n", - port_str.c_str()); + GME_LOG_CRITICAL("OscServer: failed to bind UDP port " + port_str); return false; } @@ -143,7 +142,7 @@ bool OscServer::start() { Impl::onMessage, impl_.get()); if (lo_server_thread_start(impl_->server_thread) != 0) { - std::fprintf(stderr, "FATAL OscServer: lo_server_thread_start failed\n"); + GME_LOG_CRITICAL("OscServer: lo_server_thread_start failed"); lo_server_thread_free(impl_->server_thread); impl_->server_thread = nullptr; return false; diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 2f434e3..d43a66c 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -24,6 +24,20 @@ target_include_directories(gradient_motion PUBLIC ${CMAKE_SOURCE_DIR} ) +# Arm the CuemsLogger backend on the LIBRARY, not just the daemon executable. +# Previously only mtcreceiver and gradient-motiond carried HAVE_CUEMS_LOGGER, +# so every GME_LOG_* compiled into this library fell back to the std::cerr +# emitter — under systemd those lines are stamped PRIORITY=6 and invisible to +# `cuems-logs -e` — and GradientEngine.cpp, listed in BOTH targets, was +# compiled twice with two different logging backends. PUBLIC on purpose: the +# daemon and the test binaries then share one backend (side effect: ctest +# output for the 5 test binaries goes to syslog when cuemslogger is enabled). +# The TARGET guard covers BUILD_DAEMON=OFF, where cuemslogger is never added. +if(ENABLE_CUEMS_LOGGER AND TARGET cuemslogger) + target_compile_definitions(gradient_motion PUBLIC HAVE_CUEMS_LOGGER) + target_link_libraries(gradient_motion PUBLIC cuemslogger) +endif() + target_link_libraries(gradient_motion PUBLIC nlohmann_json::nlohmann_json ) diff --git a/src/gradient/CurveFactory.cpp b/src/gradient/CurveFactory.cpp index a75513e..d781d0e 100644 --- a/src/gradient/CurveFactory.cpp +++ b/src/gradient/CurveFactory.cpp @@ -17,6 +17,7 @@ #include "EaseOutCurve.h" #include "SCurve.h" #include "ResampledCurve.h" +#include "daemon/logging.h" namespace gme { namespace gradient { @@ -54,8 +55,8 @@ CurveFactory::createCurve(const std::string& type, inner = std::make_unique(); } else { - std::cerr << "[CurveFactory] Unknown curve type: '" - << type << "' — returning nullopt\n"; + GME_LOG_WARNING("CurveFactory: unknown curve type '" + std::string(type) + + "' — returning nullopt"); return std::nullopt; } diff --git a/src/motion/MotionFactory.cpp b/src/motion/MotionFactory.cpp index 7e94e64..eab6d90 100644 --- a/src/motion/MotionFactory.cpp +++ b/src/motion/MotionFactory.cpp @@ -17,6 +17,7 @@ #include #include +#include "daemon/logging.h" namespace gme { namespace motion { @@ -29,8 +30,8 @@ static std::unique_ptr makeFadeMotion(const gme::signal::FadeCommand& c : cmd.curve_params; auto curveOpt = gme::gradient::CurveFactory::createCurve(cmd.curve_type, params); if (!curveOpt) { - std::fprintf(stderr, "WARNING MotionFactory: unknown curve type '%s' " - "(motion_id=%s)\n", cmd.curve_type.c_str(), cmd.motion_id.c_str()); + GME_LOG_WARNING("MotionFactory: unknown curve type '" + cmd.curve_type + + "' (motion_id=" + cmd.motion_id + ")"); ctx.emitStatus(gme::signal::StatusKind::MotionError, cmd.motion_id, "unknown_curve_type"); return nullptr; @@ -49,9 +50,9 @@ static std::unique_ptr makeFadeMotion(const gme::signal::FadeCommand& c // Build lo_address lo_address addr = gme::osc::makeAddress(cmd.osc_host, cmd.osc_port); if (!addr) { - std::fprintf(stderr, "WARNING MotionFactory: lo_address_new failed for " - "%s:%d (motion_id=%s)\n", - cmd.osc_host.c_str(), cmd.osc_port, cmd.motion_id.c_str()); + GME_LOG_WARNING("MotionFactory: lo_address_new failed for " + cmd.osc_host + + ":" + std::to_string(cmd.osc_port) + + " (motion_id=" + cmd.motion_id + ")"); ctx.emitStatus(gme::signal::StatusKind::MotionError, cmd.motion_id, "osc_address_failed"); return nullptr; @@ -82,8 +83,8 @@ std::unique_ptr MotionFactory::fromCommand(const gme::signal::FadeComma case Type::START_CROSSFADE: // TODO Phase 7: return makeCrossfadePair(cmd, ctx); - std::fprintf(stderr, "INFO MotionFactory: START_CROSSFADE not yet " - "implemented (motion_id=%s)\n", cmd.motion_id.c_str()); + GME_LOG_INFO("MotionFactory: START_CROSSFADE not yet implemented (motion_id=" + + cmd.motion_id + ")"); return nullptr; default: diff --git a/src/motion/MotionRegistry.cpp b/src/motion/MotionRegistry.cpp index 88dd8fb..2a83431 100644 --- a/src/motion/MotionRegistry.cpp +++ b/src/motion/MotionRegistry.cpp @@ -17,6 +17,7 @@ #include #include #include +#include "daemon/logging.h" namespace gme { namespace motion { @@ -89,9 +90,8 @@ void MotionRegistry::apply(gme::signal::FadeCommand& cmd) { cancelAll(); break; case Type::START_CROSSFADE: - std::fprintf(stderr, "INFO MotionRegistry: START_CROSSFADE dropped " - "(deferred to future feature, motion_id=%s)\n", - cmd.motion_id.c_str()); + GME_LOG_INFO("MotionRegistry: START_CROSSFADE dropped (deferred to " + "future feature, motion_id=" + cmd.motion_id + ")"); break; } } @@ -137,8 +137,8 @@ void MotionRegistry::addMotion(std::unique_ptr m) { void MotionRegistry::cancelMotion(const std::string& motion_id, bool snap_to_end) { auto it = motions_.find(motion_id); if (it == motions_.end()) { - std::fprintf(stderr, "WARNING MotionRegistry: cancelMotion: motion_id '%s' " - "not found\n", motion_id.c_str()); + GME_LOG_WARNING("MotionRegistry: cancelMotion: motion_id '" + motion_id + + "' not found"); return; } From adb7ad09d578e521b7329267b23dabceb7c13332 Mon Sep 17 00:00:00 2001 From: Ion Reguera Date: Wed, 29 Jul 2026 19:43:50 +0200 Subject: [PATCH 10/14] chore(submodules): converge cuemslogger + mtcreceiver on reconciled heads cuemslogger c39e2d8 -> 6463cd9: master now carries the anton closelog()-in-setNewSlug memory fix and the audioplayer header fix (ancestry-gated before any consumer bumped: git merge-base --is-ancestor 30c9cb4 6463cd9). mtcreceiver 8a30d05 -> 20ab95b: same 24h-continuity line this branch's base already tracks, plus the journald-aware fallback stub (with logDebug and a shared multi-header guard) and the MTC-RX FullFrame diagnostic at debug priority. This repo builds mtcreceiver with HAVE_CUEMS_LOGGER, so it uses the real logger path; run ctest after merging as the base branch intended. --- cuemslogger | 2 +- mtcreceiver | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cuemslogger b/cuemslogger index c39e2d8..6463cd9 160000 --- a/cuemslogger +++ b/cuemslogger @@ -1 +1 @@ -Subproject commit c39e2d82fedf82ce997e490735a85863e096ae21 +Subproject commit 6463cd90a682b2b66fd5a4d11f94987e9e43303e diff --git a/mtcreceiver b/mtcreceiver index 8a30d05..20ab95b 160000 --- a/mtcreceiver +++ b/mtcreceiver @@ -1 +1 @@ -Subproject commit 8a30d056e5fbe27ef19f0485c5ef142b7f7b4ef0 +Subproject commit 20ab95b96baa9693a6c59e786a1bbd7b27beaae0 From 9f1e0be1c97652967ea220455a7f88d13d160f87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Masip?= Date: Wed, 29 Jul 2026 18:11:35 +0000 Subject: [PATCH 11/14] build(deb): make packaging build runner-safe; add real BUILD_TESTS option The packaging BUILD stage could not run reliably on GitHub Actions or any virtualized runner: - dh_auto_test ran the full ctest suite, including the latency-budgeted test_motion_registry_bench and bench_osc_latency, which miss their p99 budgets under neighbour load on shared runners. The MTC tests also want an ALSA sequencer a container may not expose, and bench_osc_latency writes its results back into tests/bench_results/, dirtying the source tree mid-package-build. - -DBUILD_TESTS=OFF in debian/rules was a dead flag: no such option existed in CMakeLists.txt, so every packaging build silently compiled the entire test suite. - A checkout without submodules failed late with CMake's FATAL_ERROR rather than a legible message. Changes: - debian/rules: override_dh_auto_test skips the suite in the BUILD stage (TEST/COVERAGE are separate upstream pipeline stages). Escape hatch RUN_DEB_TESTS=1 forces it locally; DEB_BUILD_OPTIONS=nocheck still wins. override_dh_auto_configure guards for the mtcreceiver/cuemslogger submodules and fails up front with the remedy. - CMakeLists.txt: add a real BUILD_TESTS option, default ON so plain `cmake ..` and the CI TEST/COVERAGE stages are unaffected, guarding enable_testing() and add_subdirectory(tests). Verified: default configure still yields 9 tests; -DBUILD_TESTS=OFF yields 0. Full dpkg-buildpackage -b -us -uc exits 0, compiles no test binaries, and the extracted binary reports gradient-motiond 0.3.1. Co-Authored-By: Claude Opus 5 (1M context) --- CMakeLists.txt | 13 +++++++++++-- debian/rules | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e0a07cb..4a2aee3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -39,6 +39,11 @@ add_compile_options(-Wall -O3 -pthread) # =========================================================== option(ENABLE_CUEMS_LOGGER "Build with CuemsLogger syslog backend" ON) option(BUILD_DAEMON "Build the gradient-motiond daemon (requires rtmidi)" ON) +# Default ON so plain `cmake ..` and the CI TEST/COVERAGE stages keep building +# the suite. The packaging BUILD stage (debian/rules) turns it OFF: a .deb only +# ships the daemon, and the latency-budgeted benchmarks are unfit for shared +# runners. +option(BUILD_TESTS "Build the unit/integration test suite" ON) # =========================================================== # External dependencies via pkg-config @@ -134,8 +139,12 @@ add_subdirectory(src) # =========================================================== # Tests # =========================================================== -enable_testing() -add_subdirectory(tests) +if(BUILD_TESTS) + enable_testing() + add_subdirectory(tests) +else() + message(STATUS "BUILD_TESTS=OFF — test suite not configured") +endif() # =========================================================== # Version header (generated from project VERSION above) diff --git a/debian/rules b/debian/rules index aacbcd5..5978129 100755 --- a/debian/rules +++ b/debian/rules @@ -6,9 +6,44 @@ export DEB_BUILD_MAINT_OPTIONS = hardening=+all %: dh $@ +# --- Submodule guard ------------------------------------------------------- +# The daemon links mtcreceiver and cuemslogger, which live in git submodules. +# On a runner that checked out without `submodules: recursive` CMake aborts +# midway with a FATAL_ERROR; fail up front with the actual remedy instead. override_dh_auto_configure: + @for m in mtcreceiver cuemslogger; do \ + test -f "$$m/CMakeLists.txt" || { \ + echo "ERROR: submodule '$$m' is missing from the source tree."; \ + echo " Run 'git submodule update --init --recursive', or"; \ + echo " check out with actions/checkout submodules: recursive."; \ + exit 1; }; \ + done dh_auto_configure -- \ -DCMAKE_BUILD_TYPE=Release \ -DCMAKE_INSTALL_PREFIX=/usr \ -DBUILD_DAEMON=ON \ -DBUILD_TESTS=OFF + +# --- Packaging BUILD stage: no tests here --------------------------------- +# In the CI/CD pipeline the suite is exercised in dedicated, earlier stages +# (TEST -> COVERAGE) against the same tree; this BUILD stage only has to emit +# a deterministic .deb. Running dh_auto_test here would both duplicate that +# work and fail non-deterministically on shared/virtualized runners (GitHub +# Actions et al.): `test_motion_registry_bench` and `bench_osc_latency` are +# latency-budgeted, the MTC tests want an ALSA sequencer that a container may +# not expose, and `bench_osc_latency` writes its results back into the source +# tree (tests/bench_results/), which dirties a packaging build. +# +# Note this override is what actually skips the run: -DBUILD_TESTS=OFF above +# keeps the suite from being *compiled*, and this keeps it from being *run*, +# so the stage stays correct even if the suite is re-enabled locally. +# +# Escape hatch: `RUN_DEB_TESTS=1 dpkg-buildpackage ...` forces the suite for a +# local package build. An explicit `DEB_BUILD_OPTIONS=nocheck` still wins, +# because dh_auto_test honours it internally. +override_dh_auto_test: +ifeq ($(RUN_DEB_TESTS),1) + dh_auto_test +else + @echo "dh_auto_test: skipped in packaging BUILD stage — tests run in the CI TEST/COVERAGE stages (set RUN_DEB_TESTS=1 to force)" +endif From c1ebd0369fbfcbd998cd383017063aeafee4178b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Masip?= Date: Wed, 29 Jul 2026 18:16:02 +0000 Subject: [PATCH 12/14] docs(planning): add T070 plan for layered CI/CD GitHub Actions suite Plans the TEST -> COVERAGE -> DOCUMENT -> BUILD -> DEPLOY pipeline, replacing the two disconnected workflows (tests.yml, docs.yml) with a gated suite that also covers the currently-absent BUILD and DEPLOY stages. Records the repo-specific constraints the implementation must respect, so they are not re-derived: the two latency-budgeted benchmarks must be collected rather than gated on; test_motion_registry_bench is mislabelled "integration" so label-based gating silently does nothing; bench_osc_latency writes into the source tree; the .deb must be built in a debian:bookworm container because ${shlibs:Depends} otherwise stamps Ubuntu-flavoured dependencies into a package destined for Debian nodes. Leaves three decisions open for confirmation: the DEPLOY target (no APT repo infrastructure exists in the ecosystem, so GitHub Releases is the assumed artifact of record), the workflow filename (README badges are pinned to tests.yml), and Pages deploy ownership. Co-Authored-By: Claude Opus 5 (1M context) --- .../T070-cicd-github-actions-suite.md | 336 ++++++++++++++++++ 1 file changed, 336 insertions(+) create mode 100644 specs/planning/T070-cicd-github-actions-suite.md diff --git a/specs/planning/T070-cicd-github-actions-suite.md b/specs/planning/T070-cicd-github-actions-suite.md new file mode 100644 index 0000000..c4ff3fb --- /dev/null +++ b/specs/planning/T070-cicd-github-actions-suite.md @@ -0,0 +1,336 @@ + + +# T070 — Layered CI/CD Suite (GitHub Actions): TEST → COVERAGE → DOCUMENT → BUILD → DEPLOY + +**Issued by**: packaging session following [T069](T069-fix-osc-send-failure-misclassification.md) +**Target repo**: `gradient-motion-engine` +**Primary files**: `.github/workflows/*.yml`, `tests/CMakeLists.txt`, `README.md` +**Context version**: v0.3.1 (`debian/changelog` top entry `cuems-gradient-motiond (0.3.1-1)`) +**Date**: 2026-07-29 +**Status**: plan — not yet applied. +**Prerequisite (already landed)**: commit `9f1e0be` *"build(deb): make packaging build runner-safe; add real BUILD_TESTS option"* — the BUILD stage below depends on it. + +--- + +## Background + +The repo currently has two disconnected workflows: + +| File | Does | Gap | +|---|---|---| +| `.github/workflows/tests.yml` | Build (Debug, `--coverage`) → modprobe ALSA → `ctest` (all 9 tests) → lcov → Codecov | Conflates TEST and COVERAGE; gates on the latency benchmarks, which are unfit for shared runners | +| `.github/workflows/docs.yml` | mkdocs + Doxygen → GitHub Pages | Runs independently of test outcome; can publish docs for a tree whose tests failed | + +There is **no BUILD stage and no DEPLOY stage** — `.deb` packages are today built by hand on a workstation (that is how `cuems-gradient-motiond_0.3.1-1_amd64.deb` was produced) and moved to the fleet by hand. Nothing verifies that the version in `CMakeLists.txt`, `debian/changelog`, and `CHANGELOG.md` agree, which is exactly the kind of three-file bump T069 §6 performed manually. + +The goal is a single layered pipeline where each stage gates the next. + +--- + +## Constraints discovered in this repo (do not re-derive these) + +1. **Two tests are latency-budgeted and will flake on shared runners.** + `bench_osc_latency` (label `bench`) and `test_motion_registry_bench` (label + `integration`) assert p99 budgets that a noisy-neighbour GitHub runner + misses. They must be collected, not gated on. +2. **`test_motion_registry_bench` is mislabelled.** It carries + `LABELS "integration"` (`tests/CMakeLists.txt:179`) despite being a + benchmark, so `ctest -LE bench` does *not* exclude it. Relabel it to + `bench`; that makes label-based gating actually work and leaves + `integration` meaning only `test_osc_server_integration`, which is a + legitimate gate. +3. **`bench_osc_latency` writes into the source tree.** + `BENCH_RESULTS_FILE="${CMAKE_SOURCE_DIR}/tests/bench_results/osc_latency.txt"` + (`tests/CMakeLists.txt:154`) — running it leaves the checkout dirty. That + file is dirty in the working tree right now for exactly this reason. +4. **Submodules are mandatory.** `mtcreceiver` and `cuemslogger` are needed by + the daemon; CMake hard-fails without them (`CMakeLists.txt:70-84`). Every + job needs `submodules: recursive`. +5. **`DOXYGEN_WARN_AS_ERROR YES`** (`CMakeLists.txt:114`) — the docs target is + already a strict gate; use it as one. +6. **Badge URLs are pinned to workflow filenames.** `README.md:15` points at + `actions/workflows/tests.yml/badge.svg` and `:17` at `docs.yml`. Renaming + or deleting either file silently breaks the badge. See §Decisions. +7. **The fleet is Debian, the default runner is Ubuntu.** `${shlibs:Depends}` + resolves against the *build* distro, so an `ubuntu-latest` build stamps + Ubuntu-flavoured dependencies (`liblo7`, `librtmidi6` version ranges) into + a package destined for Debian nodes. The BUILD stage must run in a + `debian:bookworm` container. +8. **`DEB_BUILD_MAINT_OPTIONS = hardening=+all`** is already set in + `debian/rules`; no runner-side hardening flags are needed. + +--- + +## Target pipeline + +``` + ┌────────────────────────────────────┐ + push / PR ──────▶│ 1. TEST (gating, fast) │ + └───────────────┬────────────────────┘ + │ needs + ┌──────────────────────┼──────────────────────┐ + ▼ ▼ ▼ + ┌──────────────────┐ ┌───────────────────┐ ┌────────────────────┐ + │ 2. COVERAGE │ │ 3. DOCUMENT │ │ 2b. BENCH │ + │ lcov → Codecov │ │ Doxygen + mkdocs │ │ non-gating, upload │ + │ + coverage badge │ │ + badges │ │ results as artifact│ + └────────┬─────────┘ └─────────┬─────────┘ └────────────────────┘ + └──────────┬───────────┘ + ▼ + ┌──────────────────────────────┐ + │ 4. BUILD (debian:bookworm) │ + │ dpkg-buildpackage → .deb │ + │ + version-consistency guard │ + └──────────────┬───────────────┘ + ▼ (tag v* / release published only) + ┌──────────────────────────────┐ + │ 5. DEPLOY │ + │ attach .deb to GH Release │ + └──────────────────────────────┘ +``` + +Stages 2, 2b and 3 run in parallel once TEST is green. DEPLOY is the only +stage restricted by trigger. + +--- + +## Execution checklist + +### 0. Branch + +```bash +git checkout main && git pull +git checkout -b feat/cicd-pipeline-t070 +``` + +### 1. Make label-based gating meaningful (prerequisite edit) + +`tests/CMakeLists.txt:179`: + +```diff +- set_tests_properties(test_motion_registry_bench PROPERTIES LABELS "integration") ++ # Latency-budgeted: grouped with the other benchmark so CI can gate on ++ # `ctest -LE bench` and collect these separately (see T070). ++ set_tests_properties(test_motion_registry_bench PROPERTIES LABELS "bench") +``` + +Verify the split is what the pipeline expects: + +```bash +cmake -B build && ctest --test-dir build -N -LE bench # expect 7 tests +cmake -B build && ctest --test-dir build -N -L bench # expect 2 tests +``` + +### 2. Make the benchmark output path overridable (constraint 3) + +So the bench job does not dirty the checkout. `tests/CMakeLists.txt:153-155`: + +```diff ++set(BENCH_RESULTS_DIR "${CMAKE_SOURCE_DIR}/tests/bench_results" ++ CACHE PATH "Directory bench_osc_latency writes its results to") + target_compile_definitions(bench_osc_latency PRIVATE +- BENCH_RESULTS_FILE="${CMAKE_SOURCE_DIR}/tests/bench_results/osc_latency.txt" ++ BENCH_RESULTS_FILE="${BENCH_RESULTS_DIR}/osc_latency.txt" + ) +``` + +CI then passes `-DBENCH_RESULTS_DIR=${{ runner.temp }}/bench` and uploads that +directory as an artifact. Default behaviour for local developers is unchanged. + +### 3. `.github/workflows/ci.yml` — stages 1–4 + +Replaces `tests.yml`. **Keep the filename decision from §Decisions in mind +before deleting `tests.yml`.** Structure (SPDX header as in the existing +workflows): + +```yaml +name: CI +on: + push: + branches: [main, rc_1] + pull_request: + types: [opened, synchronize, reopened, ready_for_review] + workflow_dispatch: + +concurrency: + group: ci-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read +``` + +**Job `test`** (gating) — `ubuntu-latest`: +- `actions/checkout@v4` with `submodules: recursive` +- apt: `build-essential cmake pkg-config librtmidi-dev liblo-dev nlohmann-json3-dev libtinyxml2-dev` +- `ccache` via `hendrikmuhs/ccache-action@v1` keyed on `${{ runner.os }}-ci` +- `sudo modprobe snd-seq snd-seq-dummy || true` (keep the `|| true` from + `tests.yml:52` — Azure kernels lack the modules; commit `0c2af7b` fixed + precisely this) +- configure `-DCMAKE_BUILD_TYPE=Debug`, build, then + `ctest --test-dir build --output-on-failure -LE bench` + +**Job `bench`** (non-gating) — `needs: test`, `continue-on-error: true`: +- same setup, `-DBENCH_RESULTS_DIR=${{ runner.temp }}/bench` +- `ctest --test-dir build --output-on-failure -L bench` +- `actions/upload-artifact@v4` for the results dir, `if: always()` +- Rationale: a p99 miss on a shared runner is not a defect in this daemon, but + the trend data is worth keeping. Do **not** let this job block the pipeline. + +**Job `coverage`** — `needs: test`: +- lift the lcov + Codecov steps verbatim from `tests.yml:56-77` (the + `--ignore-errors inconsistent` flags and the `/usr/*`, `*/tests/*`, + `*/mtcreceiver/*`, `*/cuemslogger/*` exclusions are already tuned — do not + re-tune them here) +- run coverage over `-LE bench` only, so the flaky benches cannot fail the job +- keep `fail_ci_if_error: false` and the existing `CODECOV_TOKEN` secret + +**Job `docs`** — `needs: test`: +- apt `doxygen`, `pip install mkdocs==1.6.1 mkdocs-material mkdoxy` (pin as + `docs.yml:44` already does) +- `mkdocs build` — with `DOXYGEN_WARN_AS_ERROR YES` this fails on any + undocumented symbol, which is the intended DOCUMENT gate +- upload the site as an artifact; **do not deploy to Pages here** — Pages + deployment stays in `docs.yml` (see §Decisions) + +**Job `package`** — `needs: [coverage, docs]`, `container: debian:bookworm` +(constraint 7): +- `apt-get install -y --no-install-recommends build-essential devscripts + debhelper cmake pkg-config librtmidi-dev liblo-dev nlohmann-json3-dev + libtinyxml2-dev git ca-certificates` +- checkout with `submodules: recursive` (inside a container, `git` must be + installed *before* checkout for submodules to work — install it in a + pre-step or use `actions/checkout@v4`'s bundled fallback) +- **version-consistency guard** (see §4 below) — run this *before* building +- `dpkg-buildpackage -b -us -uc` +- move `../*.deb ../*.changes ../*.buildinfo` into `artifacts/` and upload + +Because commit `9f1e0be` landed, this job needs no test-related workarounds: +`override_dh_auto_test` already skips the suite, `-DBUILD_TESTS=OFF` is now a +real option so the suite is not even compiled, and the submodule guard in +`override_dh_auto_configure` produces a legible error if checkout misbehaves. + +### 4. Version-consistency guard + +A shell step in `package` (and reused by DEPLOY) asserting the three sources of +truth agree — this is the drift T069 §6 had to reconcile by hand: + +```bash +cmake_ver=$(grep -oP 'project\(gradient-motion-engine VERSION \K[0-9.]+' CMakeLists.txt) +deb_ver=$(dpkg-parsechangelog -S Version | cut -d- -f1) +chg_ver=$(grep -oP '^## \[\K[0-9.]+' CHANGELOG.md | head -1) +test "$cmake_ver" = "$deb_ver" && test "$cmake_ver" = "$chg_ver" || { + echo "Version drift: CMakeLists=$cmake_ver debian/changelog=$deb_ver CHANGELOG=$chg_ver"; exit 1; } +``` + +On a tag build, additionally assert `"v$cmake_ver" = "$GITHUB_REF_NAME"`. + +### 5. `.github/workflows/release.yml` — stage 5 (DEPLOY) + +```yaml +on: + push: + tags: ['v*'] +permissions: + contents: write # required to create/attach to the Release +``` + +- Reuse the `package` job (extract it into a reusable workflow called with + `workflow_call` from both `ci.yml` and `release.yml`, so the release artifact + is built by identical steps to the ones CI exercises on every push). +- `softprops/action-gh-release@v2` attaching `*.deb`, `*.changes`, + `*.buildinfo`, with the body generated from the matching `CHANGELOG.md` + section. +- Guard: refuse to publish if the version guard (§4) fails, and if the tag is + not on `main` or `rc_1`. + +### 6. Badges (the DOCUMENT deliverable) + +`README.md:14-17` currently carries License, Tests, codecov, and Docs badges. +Update/extend to match the new suite: + +| Badge | Source | +|---|---| +| License GPLv3 | unchanged (`README.md:14`) | +| CI | `actions/workflows/ci.yml/badge.svg` — **replaces** the `tests.yml` badge | +| codecov | unchanged — already token-pinned (`README.md:16`) | +| Docs | unchanged (`docs.yml`) | +| Release | `https://img.shields.io/github/v/release/stagesoft/gradient-motion-engine` | +| Debian package | `https://img.shields.io/github/v/tag/...?label=deb` or a shields endpoint badge fed by the release asset name | + +Note `cuems-engine` renders its coverage badge via `genbadge` pushed to +`gh-pages/badges` (`ci.yml:112-125` there) because it has no Codecov token; +this repo *does* have one (`README.md:16`), so use the Codecov badge and do +**not** replicate the genbadge/gh-pages mechanism. + +--- + +## Decisions required before implementation + +1. **DEPLOY target.** No APT repository infrastructure exists anywhere in the + ecosystem checkout (searched for `reprepro`/`aptly`/`packagecloud`/ + `cloudsmith`/`deb.cuems` — no hits). This plan therefore assumes **GitHub + Releases as the artifact of record**, with fleet nodes installing via + `dpkg -i` of a downloaded asset, mirroring how `cuems-engine` publishes to + PyPI on `release: published`. If a fleet APT repo is intended instead + (`reprepro` on a Stagelab host, signed with a repo key), stage 5 changes + shape: it needs a GPG signing secret, `dput`/`reprepro includedeb`, and the + `-us -uc` flags in `debian/rules` invocations become real signing. **Confirm + which before implementing stage 5**; stages 1–4 are unaffected either way. +2. **Workflow filename.** `README.md:15` badge is pinned to `tests.yml`. + Either (a) name the new pipeline `tests.yml` to preserve the badge and the + branch-protection rule that likely references the "Tests" check, or + (b) name it `ci.yml` (matching `cuems-engine`) and update both the README + badge *and* any required-status-check config in repo settings in the same + change. Recommendation: **(b)**, for ecosystem consistency — but it requires + a settings touch that CI cannot do for itself. +3. **Pages ownership.** `docs.yml` owns the `pages` concurrency group and the + `github-pages` environment. Keep it as the sole deployer (this plan's `docs` + job only validates and uploads an artifact) to avoid two workflows racing + for the same environment. + +--- + +## Verification checklist + +- [ ] `ctest -N -LE bench` reports 7 tests; `-L bench` reports 2 (§1). +- [ ] `cmake -B build -DBENCH_RESULTS_DIR=/tmp/b && ctest -L bench` leaves + `git status` clean (§2). +- [ ] Open a throwaway PR: `test` gates, `bench` runs red-but-non-blocking, + `coverage` and `docs` run in parallel, `package` produces a `.deb` + artifact downloadable from the run summary. +- [ ] `dpkg-deb -I` on the CI-produced `.deb` shows `Version: 0.3.1-1` and + Debian-flavoured `Depends:` — compare against the locally built package + to confirm constraint 7 is actually addressed. +- [ ] Deliberately bump only `CMakeLists.txt` and confirm the version guard + fails the `package` job (§4). Revert. +- [ ] Push tag `v0.3.1-test` on a scratch branch → release workflow attaches + assets → delete the test release and tag. +- [ ] All README badges render green after merge (§6). + +## Rollback + +Purely additive to `.github/workflows/` plus two `tests/CMakeLists.txt` label +and path edits. Revert the merge commit; nothing in the daemon, the library, or +the package contents changes. The only externally visible state is any GitHub +Release created by a real tag push — delete the release and tag if a bad one +ships. + +--- + +## Related artifacts + +| Artifact | Location | +|---|---| +| Runner-safe packaging (prerequisite) | commit `9f1e0be`, `debian/rules`, `CMakeLists.txt:41-46` | +| Existing test workflow | `.github/workflows/tests.yml` | +| Existing docs workflow | `.github/workflows/docs.yml` | +| Ecosystem CI reference (lint→test→badge) | `cuems-engine/.github/workflows/ci.yml` | +| Ecosystem publish reference (release-triggered) | `cuems-engine/.github/workflows/pypi-publish.yml` | +| Test labels / bench paths | `tests/CMakeLists.txt:17,36,51,68,91,112,133,154,158,179` | +| Package metadata | `debian/control`, `debian/changelog` | +| Badges | `README.md:14-17` | +| Prior task | [T069](T069-fix-osc-send-failure-misclassification.md) | From ff44b5eba1da15e98c3be57675c6a01c364cce5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Masip?= Date: Thu, 30 Jul 2026 10:13:44 +0000 Subject: [PATCH 13/14] docs: drop stale NNG references; generalize inbound-transport comments An audit of the libnng dependency found it is already fully gone: no nng_* calls or includes, no find_package(nng) in any of the four CMakeLists, no libnng-dev in debian/control Build-Depends or the CI apt lists, and `ldd` on the built daemon shows only liblo. It was removed in 538d992 (v0.3.0, spec 007-osc-input-transport), which replaced the NNG bus client with a localhost UDP OSC listener. libnng is not even installed on the dev machine and the daemon builds and links fine. What remained were stale mentions that misdescribe the current design: - CLAUDE.md still claimed the daemon "talks to the CUEMS engine over the NNG bus (libnng)" and listed NNG 1.10.1 as an active technology. This is the file seeding every session's context, so it was the highest-value fix. Replaced with the OSC transport description and a 007 entry that records the supersession explicitly -- a future reader hitting the NNG references still in specs/005-* needs to know they are historical. - CLAUDE.md also said "FadeRegistry map + fixed SPSC status queue". The type is MotionRegistry, and the status queue was the NNG-side one that was removed; the surviving SPSC queue carries inbound commands. Comments were generalized rather than merely de-NNG'd, so they survive a future transport swap: - src/signal/LockFreeQueue.h: producer is "the inbound command thread" (parenthetically, the liblo OSC server thread today), not "the NNG receive thread". - daemon/comms/OscServer.h: "matches NngBusClient convention" referenced a deleted file; now "the convention for daemon-specific inbound-transport clients". - src/engine/GradientEngine.cpp: "no NNG" -> "no outbound channel". Deliberately left as historical record: specs/** (including the 006 quickstart's libnng-dev apt line), .cursor/plans/, CHANGELOG.md, CONTRIBUTORS.md, debian/changelog, debian/copyright, and the README release-notes entries. The two test-file comments explaining the local StatusEvent struct also stay -- that rationale is still live. Comment-only in the code; the daemon rebuilds and links clean. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 6 +++--- daemon/comms/OscServer.h | 3 ++- src/engine/GradientEngine.cpp | 2 +- src/signal/LockFreeQueue.h | 9 +++++---- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9f9ae30..c4879c9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -4,7 +4,7 @@ Part of the **CUEMS** ecosystem — see the [`cuems-RELATIONS`](https://github.c ## Role -Timecode-driven motion and gradient evaluation engine with OSC output — runs as the daemon **`gradient-motiond`** (unit wired by cuems-common). C++17 (GCC, `-Wall -O3 -pthread`). MTC-synced via the `mtcreceiver` submodule; talks to the CUEMS engine over the NNG bus (`libnng`) and sends OSC via `liblo`. Also uses nlohmann-json. +Timecode-driven motion and gradient evaluation engine with OSC output — runs as the daemon **`gradient-motiond`** (unit wired by cuems-common). C++17 (GCC, `-Wall -O3 -pthread`). MTC-synced via the `mtcreceiver` submodule; receives commands from the CUEMS engine over localhost UDP OSC and sends OSC out, both via `liblo`. Also uses nlohmann-json. **Engine-side client:** `cuems-engine`'s `GradientClient` (`players/GradientClient.py`) is a fire-and-forget UDP OSC client targeting `gradient_osc_port` (7100 in `settings.xml`). Commands: `/gradient/start_fade`, `/gradient/cancel_motion `, `/gradient/cancel_all`. The engine delegates cue fades here (loop-cue fades, ActionCue fades via `ActionHandler`) — the engine's loop only supervises; the fade curve itself is evaluated by this daemon. @@ -12,8 +12,8 @@ Timecode-driven motion and gradient evaluation engine with OSC output — runs a - C++17 (GCC, `-Wall -O3 -pthread`), C++ standard library only for the core (``, ``, ``, ``, ``) — `001-phase0-scaffold`, `002-gradient-curves`. - `mtcreceiver` v2.0.0 (submodule) — `004-adapt-mtc-tick-v2`. -- NNG 1.10.1 (`libnng-dev`, C API `nng_bus0_open`) — `005-nng-bus-client`. -- liblo (OSC), nlohmann-json, RtMidi via mtcreceiver — `006-fade-registry-tick-loop`. All state in-memory (`FadeRegistry` map + fixed SPSC status queue). +- liblo (OSC), nlohmann-json, RtMidi via mtcreceiver — `006-fade-registry-tick-loop`. All state in-memory (`MotionRegistry` map + fixed SPSC command queue). +- liblo UDP OSC listener (`OscServer`, `127.0.0.1:`) as the inbound transport — `007-osc-input-transport`. Superseded the NNG bus client of `005-nng-bus-client`; `libnng` is **no longer a build or runtime dependency** (removed in v0.3.0, commit `538d992`), and the outbound NNG status channel is gone — motion status events are logged only. ## Build & release diff --git a/daemon/comms/OscServer.h b/daemon/comms/OscServer.h index c59215b..8abf8ae 100644 --- a/daemon/comms/OscServer.h +++ b/daemon/comms/OscServer.h @@ -26,7 +26,8 @@ namespace comms { * wire contract, parses them via `parseFadeOscCommand`, and on `ParseResult::Ok` * pushes to the `LockFreeQueue` supplied at construction. * - * Namespace: `gme::daemon::comms` (matches NngBusClient convention). + * Namespace: `gme::daemon::comms` — the convention for daemon-specific + * inbound-transport clients. * * ## Threading * diff --git a/src/engine/GradientEngine.cpp b/src/engine/GradientEngine.cpp index dfb486c..7bf10dc 100644 --- a/src/engine/GradientEngine.cpp +++ b/src/engine/GradientEngine.cpp @@ -41,7 +41,7 @@ GradientEngine::~GradientEngine() { bool GradientEngine::initialize(const GradientEngineConfig& config) { if (initialized_) return true; - // --- Build MotionRegistry (status events → logged, no NNG) --- + // --- Build MotionRegistry (status events → logged, no outbound channel) --- registry_ = std::make_unique( tickSource_, [](gme::signal::StatusKind k, diff --git a/src/signal/LockFreeQueue.h b/src/signal/LockFreeQueue.h index e721059..ed34af2 100644 --- a/src/signal/LockFreeQueue.h +++ b/src/signal/LockFreeQueue.h @@ -9,8 +9,9 @@ * @file LockFreeQueue.h * @brief Fixed-capacity single-producer / single-consumer ring buffer. * - * `LockFreeQueue` is the hand-off path from the NNG receive - * thread (producer) to the MTC tick thread (consumer). It satisfies + * `LockFreeQueue` is the hand-off path from the inbound command + * thread (producer — whichever transport is in use; currently the liblo + * OSC server thread) to the MTC tick thread (consumer). It satisfies * Principle IV (Real-Time Safety) of the project constitution: * * - **Zero heap allocation** after construction. Storage is @@ -22,7 +23,7 @@ * * ## SPSC contract * - * - **Exactly one thread calls `push`** (the NNG receive thread). + * - **Exactly one thread calls `push`** (the inbound command thread). * - **Exactly one site at a time calls `pop`** — either the MTC tick * callback or the 100 ms fallback drain timer, serialised via an * external `std::atomic_flag` owned by the queue's owner. @@ -46,7 +47,7 @@ * @code * gme::signal::LockFreeQueue q; * - * // Producer (NNG recv thread): + * // Producer (inbound command thread): * FadeCommand cmd = ...; * if (!q.push(std::move(cmd))) { * GME_LOG_WARNING("fade command queue overflow — oldest dropped"); From 0285507efd898f7944eeb854f8f872e147dcc97d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Masip?= Date: Thu, 30 Jul 2026 10:13:44 +0000 Subject: [PATCH 14/14] docs: drop stale NNG references; generalize inbound-transport comments An audit of the libnng dependency found it is already fully gone: no nng_* calls or includes, no find_package(nng) in any of the four CMakeLists, no libnng-dev in debian/control Build-Depends or the CI apt lists, and `ldd` on the built daemon shows only liblo. It was removed in 538d992 (v0.3.0, spec 007-osc-input-transport), which replaced the NNG bus client with a localhost UDP OSC listener. libnng is not even installed on the dev machine and the daemon builds and links fine. What remained were stale mentions that misdescribe the current design: - CLAUDE.md still claimed the daemon "talks to the CUEMS engine over the NNG bus (libnng)" and listed NNG 1.10.1 as an active technology. This is the file seeding every session's context, so it was the highest-value fix. Replaced with the OSC transport description and a 007 entry that records the supersession explicitly -- a future reader hitting the NNG references still in specs/005-* needs to know they are historical. - CLAUDE.md also said "FadeRegistry map + fixed SPSC status queue". The type is MotionRegistry, and the status queue was the NNG-side one that was removed; the surviving SPSC queue carries inbound commands. Comments were generalized rather than merely de-NNG'd, so they survive a future transport swap: - src/signal/LockFreeQueue.h: producer is "the inbound command thread" (parenthetically, the liblo OSC server thread today), not "the NNG receive thread". - daemon/comms/OscServer.h: "matches NngBusClient convention" referenced a deleted file; now "the convention for daemon-specific inbound-transport clients". - src/engine/GradientEngine.cpp: "no NNG" -> "no outbound channel". Deliberately left as historical record: specs/** (including the 006 quickstart's libnng-dev apt line), .cursor/plans/, CHANGELOG.md, CONTRIBUTORS.md, debian/changelog, debian/copyright, and the README release-notes entries. The two test-file comments explaining the local StatusEvent struct also stay -- that rationale is still live. Comment-only in the code; the daemon rebuilds and links clean. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 6 +++--- daemon/comms/OscServer.h | 3 ++- src/engine/GradientEngine.cpp | 2 +- src/signal/LockFreeQueue.h | 9 +++++---- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9f9ae30..c4879c9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -4,7 +4,7 @@ Part of the **CUEMS** ecosystem — see the [`cuems-RELATIONS`](https://github.c ## Role -Timecode-driven motion and gradient evaluation engine with OSC output — runs as the daemon **`gradient-motiond`** (unit wired by cuems-common). C++17 (GCC, `-Wall -O3 -pthread`). MTC-synced via the `mtcreceiver` submodule; talks to the CUEMS engine over the NNG bus (`libnng`) and sends OSC via `liblo`. Also uses nlohmann-json. +Timecode-driven motion and gradient evaluation engine with OSC output — runs as the daemon **`gradient-motiond`** (unit wired by cuems-common). C++17 (GCC, `-Wall -O3 -pthread`). MTC-synced via the `mtcreceiver` submodule; receives commands from the CUEMS engine over localhost UDP OSC and sends OSC out, both via `liblo`. Also uses nlohmann-json. **Engine-side client:** `cuems-engine`'s `GradientClient` (`players/GradientClient.py`) is a fire-and-forget UDP OSC client targeting `gradient_osc_port` (7100 in `settings.xml`). Commands: `/gradient/start_fade`, `/gradient/cancel_motion `, `/gradient/cancel_all`. The engine delegates cue fades here (loop-cue fades, ActionCue fades via `ActionHandler`) — the engine's loop only supervises; the fade curve itself is evaluated by this daemon. @@ -12,8 +12,8 @@ Timecode-driven motion and gradient evaluation engine with OSC output — runs a - C++17 (GCC, `-Wall -O3 -pthread`), C++ standard library only for the core (``, ``, ``, ``, ``) — `001-phase0-scaffold`, `002-gradient-curves`. - `mtcreceiver` v2.0.0 (submodule) — `004-adapt-mtc-tick-v2`. -- NNG 1.10.1 (`libnng-dev`, C API `nng_bus0_open`) — `005-nng-bus-client`. -- liblo (OSC), nlohmann-json, RtMidi via mtcreceiver — `006-fade-registry-tick-loop`. All state in-memory (`FadeRegistry` map + fixed SPSC status queue). +- liblo (OSC), nlohmann-json, RtMidi via mtcreceiver — `006-fade-registry-tick-loop`. All state in-memory (`MotionRegistry` map + fixed SPSC command queue). +- liblo UDP OSC listener (`OscServer`, `127.0.0.1:`) as the inbound transport — `007-osc-input-transport`. Superseded the NNG bus client of `005-nng-bus-client`; `libnng` is **no longer a build or runtime dependency** (removed in v0.3.0, commit `538d992`), and the outbound NNG status channel is gone — motion status events are logged only. ## Build & release diff --git a/daemon/comms/OscServer.h b/daemon/comms/OscServer.h index c59215b..8abf8ae 100644 --- a/daemon/comms/OscServer.h +++ b/daemon/comms/OscServer.h @@ -26,7 +26,8 @@ namespace comms { * wire contract, parses them via `parseFadeOscCommand`, and on `ParseResult::Ok` * pushes to the `LockFreeQueue` supplied at construction. * - * Namespace: `gme::daemon::comms` (matches NngBusClient convention). + * Namespace: `gme::daemon::comms` — the convention for daemon-specific + * inbound-transport clients. * * ## Threading * diff --git a/src/engine/GradientEngine.cpp b/src/engine/GradientEngine.cpp index dfb486c..7bf10dc 100644 --- a/src/engine/GradientEngine.cpp +++ b/src/engine/GradientEngine.cpp @@ -41,7 +41,7 @@ GradientEngine::~GradientEngine() { bool GradientEngine::initialize(const GradientEngineConfig& config) { if (initialized_) return true; - // --- Build MotionRegistry (status events → logged, no NNG) --- + // --- Build MotionRegistry (status events → logged, no outbound channel) --- registry_ = std::make_unique( tickSource_, [](gme::signal::StatusKind k, diff --git a/src/signal/LockFreeQueue.h b/src/signal/LockFreeQueue.h index e721059..ed34af2 100644 --- a/src/signal/LockFreeQueue.h +++ b/src/signal/LockFreeQueue.h @@ -9,8 +9,9 @@ * @file LockFreeQueue.h * @brief Fixed-capacity single-producer / single-consumer ring buffer. * - * `LockFreeQueue` is the hand-off path from the NNG receive - * thread (producer) to the MTC tick thread (consumer). It satisfies + * `LockFreeQueue` is the hand-off path from the inbound command + * thread (producer — whichever transport is in use; currently the liblo + * OSC server thread) to the MTC tick thread (consumer). It satisfies * Principle IV (Real-Time Safety) of the project constitution: * * - **Zero heap allocation** after construction. Storage is @@ -22,7 +23,7 @@ * * ## SPSC contract * - * - **Exactly one thread calls `push`** (the NNG receive thread). + * - **Exactly one thread calls `push`** (the inbound command thread). * - **Exactly one site at a time calls `pop`** — either the MTC tick * callback or the 100 ms fallback drain timer, serialised via an * external `std::atomic_flag` owned by the queue's owner. @@ -46,7 +47,7 @@ * @code * gme::signal::LockFreeQueue q; * - * // Producer (NNG recv thread): + * // Producer (inbound command thread): * FadeCommand cmd = ...; * if (!q.push(std::move(cmd))) { * GME_LOG_WARNING("fade command queue overflow — oldest dropped");