fx3_cmd → diagnostics tool: plan + Phase 1 (extract fx3_core)

.github/workflows/build.yml

@@ -69,3 +69,6 @@ jobs:
 
       - name: Build test tools
         run: make -C tests
+
+      - name: CLI smoke test (fx3_cmd --no-claim guard + usage)
+        run: make -C tests check-cli

docs/fx3_cmd-split-plan.md

@@ -0,0 +1,141 @@
+# Plan: Package `fx3_cmd` for diagnostics & split it from the firmware test harness
+
+Status: **Phases 0–2 complete; Phase 3 verified and ready to implement on
+branch `claude/fx3-cmd-to-tool`.** See "Progress" below for the as-built state.
+
+## 1. Goal
+
+Phil wants `fx3_cmd` packaged as a quick **diagnostics** tool. Today it is a
+single ~5,800-line monolith (`tests/fx3_cmd.c`) that mixes two unrelated jobs:
+
+1. **Operator diagnostics** — probe, poke, and recover a device on the bench.
+2. **Firmware regression / fuzz / soak harness** — ~50 issue-numbered
+   scenarios driven by CI and the `*.sh` orchestration scripts.
+
+A clean diagnostics package requires separating (1) from (2), not shipping the
+whole monolith. The end-state is a **single shared FX3 host core** owned by
+`rx888-tools`, consumed by the firmware harness via the existing submodule, so
+the two repos stop carrying parallel copies.
+
+## 2. Findings that drive the design (verified, not assumed)
+
+- `fx3_cmd.c` has **no compile-time dependency on the firmware tree**. It
+  includes only libusb + its own sibling modules. The only firmware coupling is
+  a *default path string* (`../SDDC_FX3/SDDC_FX3.img`, overridable with `-F`).
+- The destination repo **`rx888-tools`** (`github.com/ringof/rx888_tools`,
+  already wired here as the `tests/rx888_tools` submodule) is the Linux
+  host-tools home: `librx888` + `rx888_stream` + `rx888_dsp` + `iqrecord`.
+- `SDDC_FX3/protocol.h` is the **single source of truth** for the wire protocol.
+  `tests/fx3_proto.h` mirrors it for the host tools; `rx888-tools/include/
+  rx888.h` is the tools-side copy. Consolidation onto `rx888.h` removes a drift
+  hazard.
+- The firmware test scripts invoke the **diagnostic subcommands as primitives**
+  (`load`, `test`, `start`, `stop`, `adc`, `gpio`, `reload`), so the harness
+  cannot lose those commands — it needs the same core the diagnostics CLI uses.
+
+## 3. Progress (as built — updated 2026-06-08)
+
+**Phase 0 — reconciliation: DONE.** rx888-tools `main` (post-PR #26, rev
+`fd01e67`) ships a reconciled `include/rx888.h`. Verified against the firmware
+source of truth — all previously-divergent symbols now agree:
+
+| Symbol            | firmware (`protocol.h`/`fx3_proto.h`) | rx888-tools `rx888.h` (post-#26) |
+|-------------------|---------------------------------------|----------------------------------|
+| VGA SETARG id 11  | `AD8370_VGA`                          | `AD8370_VGA` ✓ (was `AD8340`)    |
+| `LED_BLUE`        | bit 11                                | bit 11 ✓ (was bit 12)            |
+| `GETSTATS`        | `0xB3`                                | `0xB3` ✓ (was absent)            |
+| `WDG_MAX_RECOV`   | SETARG 14                             | `14` ✓ (was absent)              |
+| `RX888_VID/PID_*` | present                               | present ✓                        |
+| `HANG*` test codes| present (test-only)                   | **absent** ✓ (correctly omitted) |
+
+**Phase 1 — extract shared core: DONE (in rx888-firmware).** `tests/fx3_core.
+{c,h}` carry the 19 diagnostics primitives; the harness, debug console, and
+`main`/dispatch stay in `fx3_cmd.c`.
+
+**Phase 2 — land diagnostics in rx888-tools: DONE (PR #26, merged).**
+`src/fx3_cmd/{fx3_cmd.c, fx3_core.{c,h}, fx3_usb.{c,h}, fx3_stats.{c,h}}` +
+reconciled `include/rx888.h`, with its own `tests/{fx3_cmd_smoke,hw_fx3_cmd}.sh`.
+
+**Convergence backport — DONE (this branch).** `--no-claim` read-only mode +
+26-byte GETSTATS tolerance backported into the firmware core so it stays
+identical to the tools core ahead of the submodule cutover.
+
+**Key verification result:** the two cores are now **byte-identical except the
+`#include` line.** Diff of firmware `tests/{fx3_core,fx3_usb,fx3_stats}.{c,h}`
+vs rx888-tools `src/fx3_cmd/*` shows the *only* differences are
+`#include "fx3_proto.h"` ↔ `#include "rx888.h"`, the location of
+`CTRL_TIMEOUT_MS` (header vs proto), and one header comment. `fx3_stats.h` is
+identical. **There is zero logic drift** — Phase 3 is a build-seam change, not a
+code merge.
+
+## 4. Target architecture (end-state)
+
+Single source of host-side FX3 protocol + transport, owned by rx888-tools:
+
+```
+rx888-tools/
+  include/rx888.h                 ← canonical protocol enums + USB IDs (DONE)
+  src/fx3_cmd/fx3_core.{c,h}       ← USB open/claim/reset, EP0 senders, primitives
+  src/fx3_cmd/fx3_usb.{c,h}        ← open/claim, --no-claim, CTRL_TIMEOUT_MS
+  src/fx3_cmd/fx3_stats.{c,h}      ← GETSTATS decode (26/30-byte tolerant)
+  src/fx3_cmd/fx3_cmd.c            ← diagnostics CLI
+
+rx888-firmware/  (harness links the submodule's shared core)
+  tests/fx3_cmd.c                  ← harness main + ~50 scenarios (KEEPS)
+  tests/fx3_bulk.c                 ← bulk EP1 reads (KEEPS — no tools equivalent)
+  tests/fx3_fuzz.c, fx3_lifecycle.c ← harness-only (KEEP)
+  tests/fx3_test_proto.h           ← NEW: harness-only wire bits not in rx888.h
+  tests/Makefile                   ← compile core from tests/rx888_tools/src/fx3_cmd
+```
+
+**Seam (settled by evidence):**
+- **Shared, from the submodule:** `fx3_core`, `fx3_usb`, `fx3_stats`.
+- **Firmware-only:** `fx3_bulk` (rx888-tools has no equivalent), `fx3_fuzz`,
+  `fx3_lifecycle`, `fx3_cmd.c` (harness main + scenarios).
+- **`tests/fx3_proto.h` is deleted.** Its production symbols come from the
+  submodule's `rx888.h` + `fx3_usb.h`. Its harness-only symbols —
+  `HANGFX3`/`HANGMAIN`/`HANGCOLDSTART` (test-fault injectors), `EP1_IN`,
+  `FX3_MAX_EP0LEN` — move to a new firmware-local `tests/fx3_test_proto.h`,
+  included only by `fx3_bulk/fuzz/cmd`. They are deliberately kept out of the
+  public `rx888.h`.
+
+## 5. Phase 3 — re-point the harness at the shared core (this branch)
+
+1. **Bump the submodule pin** `tests/rx888_tools` `6191883 → fd01e67` (the PR #26
+   merge that adds `src/fx3_cmd/`). CI already checks out `submodules: recursive`.
+2. **Add `tests/fx3_test_proto.h`** carrying the harness-only bits:
+   `HANGFX3`/`HANGMAIN`/`HANGCOLDSTART`, `EP1_IN`, `FX3_MAX_EP0LEN`.
+3. **Re-point firmware-local module includes:** `fx3_bulk.c`, `fx3_fuzz.c`,
+   `fx3_lifecycle.c`, `fx3_cmd.c` swap `"fx3_proto.h"` → `"rx888.h"` +
+   `"fx3_test_proto.h"` (the latter only where the harness-only symbols are used).
+4. **Rewire `tests/Makefile`:** compile
+   `tests/rx888_tools/src/fx3_cmd/{fx3_core,fx3_usb,fx3_stats}.c` with
+   `-Itests/rx888_tools/src/fx3_cmd -Itests/rx888_tools/include`; **delete** the
+   local `fx3_core.{c,h}`, `fx3_usb.{c,h}`, `fx3_stats.{c,h}`, and `fx3_proto.h`.
+5. **Validate:** `make -C tests` + `check-health` + `check-cli`, then the
+   hardware harness (`fw_test.sh`/`validate.sh`); confirm `fx3_cmd` usage and
+   dispatch are byte-identical pre/post.
+
+**Risk: low.** No logic moves; the core is proven identical and the protocol is
+reconciled. The only realistic failure is a missing production symbol after
+deleting `fx3_proto.h`, which the compiler flags immediately in step 5.
+
+## 6. Phase 4 — packaging polish & anti-drift
+
+- Versioning for `fx3_cmd` (independent of firmware), man page / trimmed `--help`.
+- **Single-source rule (enforced going forward):** core edits land in
+  rx888-tools first, then bump the firmware submodule pin. The firmware repo
+  never re-forks `fx3_core/usb/stats` or re-creates a `fx3_proto.h`. Document
+  this so no fourth protocol mirror reappears.
+
+## 7. Risks / open questions
+
+- **Cross-repo session scope:** `add_repo`/`list_repos` are unavailable in this
+  session, so rx888-tools is read-only here (via the submodule). Phase 3 is
+  entirely firmware-side, so this is not a blocker; any future rx888-tools edit
+  (e.g. moving `EP1_IN` into a shared header) must go through that repo separately.
+- **Submodule pin couples builds:** the firmware-test build now tracks a
+  rx888-tools revision. Acceptable and intended; the pin is explicit and bumped
+  deliberately.
+- **`fx3_bulk` stays forked:** rx888-tools streams via `librx888`/`rx888_stream`,
+  so it has no `fx3_bulk`. The harness keeps its own; this is by design, not drift.

tests/Makefile

@@ -19,24 +19,44 @@ CFLAGS  ?= -O2 -Wall -Wextra -Wno-unused-parameter
 # host-only targets such as check-health remain usable without libusb-dev.
 LIBUSB_CFLAGS = $(shell pkg-config --cflags libusb-1.0)
 LIBUSB_LDLIBS = $(shell pkg-config --libs libusb-1.0)
-FX3_CFLAGS    = $(CFLAGS) $(LIBUSB_CFLAGS)
+FX3_CFLAGS    = $(CFLAGS) $(LIBUSB_CFLAGS) -I$(FX3_CORE_DIR) -I$(RX888_TOOLS)/include
 
 # rx888_tools submodule paths
 RX888_TOOLS := rx888_tools
 RX888_STREAM_BIN := $(RX888_TOOLS)/rx888_stream
+# Shared FX3 host core (single source of truth — see docs/fx3_cmd-split-plan.md)
+FX3_CORE_DIR := $(RX888_TOOLS)/src/fx3_cmd
 
 all: fx3_cmd rx888_stream
 
-# fx3_cmd is built from several modules (issue #139 modularization start).
-FX3_OBJS := fx3_cmd.o fx3_usb.o fx3_stats.o fx3_bulk.o fx3_fuzz.o fx3_lifecycle.o
-FX3_HDRS := fx3_proto.h fx3_usb.h fx3_stats.h fx3_bulk.h fx3_fuzz.h fx3_lifecycle.h
+# fx3_cmd links the firmware harness (local) against the shared FX3 host core
+# that lives in the rx888_tools submodule. Local objects are the harness main +
+# scenarios, the bulk reader, the fuzzers, and the lifecycle tests; the core
+# objects (fx3_core/usb/stats) are compiled straight from the submodule so the
+# two repos no longer carry parallel copies (docs/fx3_cmd-split-plan.md, Phase 3).
+FX3_LOCAL_OBJS := fx3_cmd.o fx3_bulk.o fx3_fuzz.o fx3_lifecycle.o
+FX3_CORE_OBJS  := fx3_core.o fx3_usb.o fx3_stats.o
+FX3_OBJS       := $(FX3_LOCAL_OBJS) $(FX3_CORE_OBJS)
+
+FX3_HDRS := fx3_bulk.h fx3_fuzz.h fx3_lifecycle.h fx3_test_proto.h \
+            $(FX3_CORE_DIR)/fx3_core.h $(FX3_CORE_DIR)/fx3_usb.h \
+            $(FX3_CORE_DIR)/fx3_stats.h $(RX888_TOOLS)/include/rx888.h
 
 fx3_cmd: $(FX3_OBJS)
 	$(CC) $(FX3_CFLAGS) -o $@ $(FX3_OBJS) $(LIBUSB_LDLIBS) $(LDLIBS)
 
+# Local harness modules (sources in tests/).
 %.o: %.c $(FX3_HDRS)
 	$(CC) $(FX3_CFLAGS) -c -o $@ $<
 
+# Shared core, compiled from the submodule into tests/ object files.
+fx3_core.o: $(FX3_CORE_DIR)/fx3_core.c $(FX3_HDRS)
+	$(CC) $(FX3_CFLAGS) -c -o $@ $<
+fx3_usb.o: $(FX3_CORE_DIR)/fx3_usb.c $(FX3_HDRS)
+	$(CC) $(FX3_CFLAGS) -c -o $@ $<
+fx3_stats.o: $(FX3_CORE_DIR)/fx3_stats.c $(FX3_HDRS)
+	$(CC) $(FX3_CFLAGS) -c -o $@ $<
+
 rx888_stream: $(RX888_STREAM_BIN)
 	@# Symlink into tests/ so fw_test.sh finds it alongside fx3_cmd
 	ln -sf $(RX888_STREAM_BIN) $@
@@ -56,8 +76,14 @@ $(HOST_HEALTH_BIN): $(HOST_HEALTH_DEPS)
 check-health: $(HOST_HEALTH_BIN)
 	./$(HOST_HEALTH_BIN)
 
+# --- Host CLI smoke test (needs fx3_cmd built; no hardware) ---------------
+# Pins the --no-claim guard allow-list and usage exit codes — all resolved
+# before libusb touches the device, so it runs in CI without an RX888.
+check-cli: fx3_cmd
+	./cli_smoke.sh
+
 clean:
 	rm -f fx3_cmd $(FX3_OBJS) rx888_stream $(HOST_HEALTH_BIN)
 	$(MAKE) -C $(RX888_TOOLS) clean 2>/dev/null || true
 
-.PHONY: all clean rx888_stream check-health
+.PHONY: all clean rx888_stream check-health check-cli

tests/README.md

@@ -542,15 +542,24 @@ exclude them.
 
 ## File map
 
+The shared FX3 host core (`fx3_core`, `fx3_usb`, `fx3_stats`, and the canonical
+protocol header `rx888.h`) lives in the **`rx888_tools` submodule** and is the
+single source of truth — the harness compiles it straight from there rather than
+keeping its own copy (see `docs/fx3_cmd-split-plan.md`). Only the harness-specific
+modules live in `tests/`.
+
 | File | Purpose |
 |------|---------|
-| `fx3_cmd.c` | Vendor command exerciser, scenarios, soak test, `usbreset`/`reload`, `main()` |
-| `fx3_proto.h` | Shared protocol constants (IDs, command codes, GPIO/arg masks) |
-| `fx3_usb.{c,h}` | USB transport + device open/close/upload helpers |
-| `fx3_stats.{c,h}` | `GETSTATS` decoding (`struct fx3_stats`, `read_stats`) |
+| `fx3_cmd.c` | Harness main + scenarios, soak test, `usbreset`/`reload`, `main()`/dispatch |
+| `fx3_test_proto.h` | Harness-only protocol bits kept out of the public header (`HANG*`, `EP1_IN`, `FX3_MAX_EP0LEN`) |
 | `fx3_bulk.{c,h}` | Bulk (EP1-IN) read helpers (primed async start-and-read) |
 | `fx3_fuzz.{c,h}` | Seeded `protocol_fuzz` / `stream_fuzz` + coverage/failure log (#139) |
 | `fx3_lifecycle.{c,h}` | Host enumeration-race tests: `reopen_race_storm`, `two_actor_open` (#143) |
+| `cli_smoke.sh` | Hardware-free `fx3_cmd` arg/`--no-claim`-guard exit-code checks (`make check-cli`) |
+| `rx888_tools/src/fx3_cmd/fx3_core.{c,h}` | **Shared core (submodule):** EP0 command senders + diagnostics primitives |
+| `rx888_tools/src/fx3_cmd/fx3_usb.{c,h}` | **Shared core (submodule):** USB transport, open/close/upload, `--no-claim` |
+| `rx888_tools/src/fx3_cmd/fx3_stats.{c,h}` | **Shared core (submodule):** `GETSTATS` decode (`struct fx3_stats`, `read_stats`) |
+| `rx888_tools/include/rx888.h` | **Shared core (submodule):** canonical FX3 protocol constants (IDs, codes, GPIO/arg) |
 | `fw_test.sh` | TAP test suite wrapper (single-pass; parks ADC in SHDN on exit) |
 | `soak_test.sh` | Soak test wrapper (firmware upload + `fx3_cmd soak`) |
 | `usb_trace.sh` | Host-side USB lifecycle tracer (no device claim) |

tests/cli_smoke.sh

@@ -0,0 +1,89 @@
+#!/usr/bin/env bash
+#
+# cli_smoke.sh — hardware-free argument/guard smoke test for fx3_cmd.
+#
+# Exercises only the exit paths that fx3_cmd resolves BEFORE touching the USB
+# device, so it runs anywhere fx3_cmd links (CI included) — no RX888, no
+# privileges. It pins two behaviours:
+#
+#   1. The --no-claim guard rejects every command that is not a read-only EP0
+#      read (test/stats/stats_pll/stack_check) with exit 2, BEFORE libusb_init.
+#      This is the new safety interlock that keeps --no-claim from perturbing a
+#      running stream; a regression here is silent and dangerous, hence a test.
+#   2. The no-argument invocation prints usage and exits 2.
+#   3. The allow-listed EP0 diagnostics PASS the guard: under --no-claim they
+#      exit != 2 (device-absent -> 1 in CI, 0 on the bench), proving the
+#      allow-list wires those four through and nothing else. (test/stats/
+#      stats_pll are passive reads; stack_check also toggles firmware debug
+#      mode but stays on EP0 and is permitted by design.)
+#
+# Exit 0 iff every case matches. Intended for `make check-cli` and CI.
+
+set -u
+
+DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
+FX3_CMD="${FX3_CMD:-$DIR/fx3_cmd}"
+
+if [ ! -x "$FX3_CMD" ]; then
+    echo "cli_smoke: $FX3_CMD not built — run 'make -C tests fx3_cmd' first" >&2
+    exit 1
+fi
+
+pass=0
+fail=0
+
+# expect_exit <expected> <description> <fx3_cmd args...>
+expect_exit() {
+    local expected="$1" desc="$2"; shift 2
+    "$FX3_CMD" "$@" >/dev/null 2>&1
+    local rc=$?
+    if [ "$rc" = "$expected" ]; then
+        printf 'ok   %-52s (exit %s)\n' "$desc" "$rc"
+        pass=$((pass + 1))
+    else
+        printf 'FAIL %-52s (exit %s, want %s)\n' "$desc" "$rc" "$expected"
+        fail=$((fail + 1))
+    fi
+}
+
+# expect_not_exit <forbidden> <description> <fx3_cmd args...>
+expect_not_exit() {
+    local forbidden="$1" desc="$2"; shift 2
+    "$FX3_CMD" "$@" >/dev/null 2>&1
+    local rc=$?
+    if [ "$rc" != "$forbidden" ]; then
+        printf 'ok   %-52s (exit %s, not %s)\n' "$desc" "$rc" "$forbidden"
+        pass=$((pass + 1))
+    else
+        printf 'FAIL %-52s (exit %s, must not be %s)\n' "$desc" "$rc" "$forbidden"
+        fail=$((fail + 1))
+    fi
+}
+
+echo "cli_smoke: $FX3_CMD"
+
+# --- usage ---------------------------------------------------------------
+expect_exit 2 "no arguments -> usage"                            # argc < 2
+
+# --- --no-claim guard rejects writes / recovery / harness (exit 2) -------
+# These resolve before libusb_init, so they are hermetic (no USB needed).
+expect_exit 2 "--no-claim gpio (write rejected)"   --no-claim gpio 0x1
+expect_exit 2 "--no-claim att (write rejected)"    --no-claim att 10
+expect_exit 2 "--no-claim vga (write rejected)"    --no-claim vga 128
+expect_exit 2 "--no-claim adc (write rejected)"    --no-claim adc 32000000
+expect_exit 2 "--no-claim i2cw (write rejected)"   --no-claim i2cw 0x12 0x00 0xff
+expect_exit 2 "--no-claim reset (recovery rejected)"   --no-claim reset
+expect_exit 2 "--no-claim reload (upload rejected)"    --no-claim reload
+expect_exit 2 "--no-claim start (harness rejected)"    --no-claim start
+
+# --- --no-claim allow-list passes the guard (exit != 2) ------------------
+# Past the guard these reach open_rx888(); with no device attached they exit 1
+# in CI (0 on the bench). Either way, exit 2 would mean the guard wrongly
+# rejected a read command, which is the regression we are pinning against.
+expect_not_exit 2 "--no-claim test (allowed)"        --no-claim test
+expect_not_exit 2 "--no-claim stats (allowed)"       --no-claim stats
+expect_not_exit 2 "--no-claim stats_pll (allowed)"   --no-claim stats_pll
+expect_not_exit 2 "--no-claim stack_check (allowed)" --no-claim stack_check
+
+echo "cli_smoke: $pass passed, $fail failed"
+[ "$fail" -eq 0 ]

tests/fx3_bulk.c

@@ -4,7 +4,8 @@
  */
 #include "fx3_bulk.h"
 #include "fx3_usb.h"
-#include "fx3_proto.h"
+#include "rx888.h"
+#include "fx3_test_proto.h"
 
 #include <stdlib.h>
 #include <unistd.h>