fx3_cmd → diagnostics tool: plan + Phase 1 (extract fx3_core)#162
fx3_cmd → diagnostics tool: plan + Phase 1 (extract fx3_core)#162ringof wants to merge 6 commits into
Conversation
…rness Draft plan documenting how to package fx3_cmd for quick diagnostics by extracting a shared FX3 host core and diagnostics CLI into rx888-tools while keeping the fuzz/soak/regression harness in this repo. Documentation only; no code or build changes. https://claude.ai/code/session_01YcMhE8u6WjetJosAT5dLPa
Behavior-preserving split of the fx3_cmd monolith. Move the operator
-diagnostics primitives into a new shared translation unit so both the
firmware test harness (this repo) and the rx888-tools diagnostics CLI
can link the same core.
New tests/fx3_core.{c,h}: firmware upload (find_rx888_stream,
upload_firmware), the vendor-command primitives (do_test/gpio/adc/att/
vga/wdg_max/start/stop/i2cr/i2cw/reset/usbreset/reload/raw), and the
read-only status reads (do_test_stack_check/stats_pll/stats_shdn).
Functions gain external linkage and prototypes in fx3_core.h.
tests/fx3_cmd.c: include fx3_core.h, drop the moved bodies and their
redundant forward declarations; keep the regression/fuzz/soak harness,
the interactive debug console, and main()/dispatch. do_stats was
already shared in fx3_stats.c.
tests/Makefile: add fx3_core.o/fx3_core.h to the build.
Verified: clean -Wall -Wextra build; fx3_cmd usage and dispatch tokens
identical to pre-refactor; make check-health passes.
https://claude.ai/code/session_01YcMhE8u6WjetJosAT5dLPa
ringof
changed the title
Keep the shared diagnostics core convergent with rx888-tools (PR #26), which evolved the same fx3_core/fx3_usb/fx3_stats files after the Phase 1 extraction. Ports the two robustness improvements; leaves out that repo's rx888.h migration and command_lookup refactor (unrelated to these). --no-claim read-only mode (fx3_usb.{c,h}, fx3_cmd.c): g_no_claim makes open_rx888 skip the interface claim / kernel-driver detach / EP1 ring restart, and close_rx888 skip the release. A new --no-claim CLI flag, restricted to the read-only EP0 commands (test/stats/stats_pll/stack_check), lets a host read counters without disturbing another process streaming with the interface claimed. GETSTATS 26-byte tolerance (fx3_stats.{c,h}, fx3_core.c): read_stats accepts payloads >= GETSTATS_MIN_LEN (26) and records payload_len/has_gpio_state, reading gpio_state[26..29] only when the 30-byte payload is present. do_stats prints gpio=n/a on legacy firmware; stats_shdn fails gracefully when gpio_state is absent. On current 30-byte firmware has_gpio_state is always 1, so output is unchanged. Verified: clean -Wall -Wextra build; --no-claim rejects writes and allows reads; make check-health passes. https://claude.ai/code/session_01YcMhE8u6WjetJosAT5dLPa
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 11eb732097
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| strcmp(cmd, "test") != 0 && | ||
| strcmp(cmd, "stats") != 0 && | ||
| strcmp(cmd, "stats_pll") != 0 && | ||
| strcmp(cmd, "stack_check") != 0) { |
There was a problem hiding this comment.
Allowing stack_check here breaks the new --no-claim safety contract for running alongside an active streamer: do_test_stack_check() is not a read-only status read, because it enables firmware debug mode with TESTFX3 wValue=1 and sends the stack command via READINFODEBUG before polling the response. In the fx3_cmd --no-claim stack_check scenario this can still mutate/debug-drive the firmware while another process owns the streaming interface, so it should be rejected like the other perturbing commands or documented/implemented as a claimed operation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch on the classification — confirmed: do_test_stack_check() (tests/fx3_core.c:445) enables debug mode via TESTFX3 wValue=1 and drives the stack command over READINFODEBUG, so it is not a passive read like test/stats/stats_pll.
Decision: keeping stack_check in the --no-claim allow-list, but fixing the inaccurate "read-only" framing it was justified under. The actual contract --no-claim enforces is EP0 diagnostics that neither reconfigure the radio nor need the streaming interface — all four stay on EP0 and never claim interface 0 / restart the EP1 ring. stack_check toggles an internal firmware debug flag and reads RTOS stack info; it does not touch the RF/sample path (gain/atten/ADC-clock) that a concurrent streamer depends on, which is the perturbation the writes (gpio/att/vga/adc) are rejected to avoid.
Updated in 2af19d6: reworded the guard comment and --no-claim usage text to state this accurately (debug-mode toggle called out explicitly), and added a hardware-free CLI smoke test (make -C tests check-cli, wired into CI) pinning the guard's exit codes. The residual we accept is debug-console CPU contention on the FX3, not a stream-config mutation.
Generated by Claude Code
Pin the --no-claim allow-list (test/stats/stats_pll/stack_check) and the usage exit codes via exit-code assertions that resolve before libusb touches the device, so they run in CI without an RX888. Wire as `make -C tests check-cli` and run it in the host-tools workflow job. Also reword the guard comment and --no-claim usage text: they described the allow-list as "read-only," but stack_check toggles firmware debug mode (TESTFX3 wValue=1) and drives the debug console. The accurate contract is EP0 diagnostics that neither reconfigure the radio nor need the streaming interface; stack_check stays on EP0 and leaves the RF/sample path untouched. No behavior change. https://claude.ai/code/session_01YcMhE8u6WjetJosAT5dLPa
…submodule) Mark Phases 0-2 done: rx888-tools post-#26 rx888.h is reconciled to the firmware protocol, and the firmware/tools cores are byte-identical except the include line. Replace Phase 3 with the verified firmware-side steps and the evidence-settled seam (share core/usb/stats from the submodule; keep bulk/fuzz/lifecycle/cmd; carve harness-only codes into fx3_test_proto.h). Add a single-source anti-drift rule as Phase 4. https://claude.ai/code/session_01YcMhE8u6WjetJosAT5dLPa
…le (Phase 3) Delete the duplicated fx3_core/usb/stats + fx3_proto.h and compile the core straight from the rx888_tools submodule (bumped to the post-#26 rev fd01e67), so the firmware harness and rx888-tools no longer carry parallel copies. Harness-only protocol codes (HANG*, EP1_IN, FX3_MAX_EP0LEN) move to a new firmware-local fx3_test_proto.h; production constants come from the submodule's rx888.h. Net -1,056 lines, no behavior change. https://claude.ai/code/session_01YcMhE8u6WjetJosAT5dLPa
2 participants
Summary
Splits
fx3_cmdinto a reusable diagnostics core + the firmware regression/fuzz/soak harness, and re-points the harness at the shared core in therx888_toolssubmodule so the two repos stop carrying parallel copies.Commits
docs: plan …—docs/fx3_cmd-split-plan.md: the split plan (now updated to the verified, as-built state).tests: extract diagnostics core … (Phase 1)— behavior-preserving in-repo refactor intofx3_core.tests: backport --no-claim + GETSTATS length tolerance— keeps this repo's core convergent with rx888-tools (PR #26, merged) ahead of the cutover.tests: add hardware-free CLI smoke test …—cli_smoke.sh(make check-cli, wired into CI) pins the--no-claimguard's exit codes; also corrects the guard/usage "read-only" wording (stack_checktoggles debug mode but stays on EP0).docs: update fx3_cmd split plan …— re-baseline the plan to verified reality.tests: build harness against shared FX3 core from rx888_tools submodule (Phase 3)— the de-duplication.Phase 3 — single shared core (this PR)
tests/rx888_tools6191883 → fd01e67(merged PR Enhancement: add debug-over-USB console subcommand to fx3_cmd #26 addssrc/fx3_cmd/{fx3_core,fx3_usb,fx3_stats}+ reconciledinclude/rx888.h).fx3_core.{c,h},fx3_usb.{c,h},fx3_stats.{c,h},fx3_proto.h(−1,056 lines net).tests/fx3_test_proto.hcarries the only harness-only protocol bits (HANG*,EP1_IN,FX3_MAX_EP0LEN), kept out of the publicrx888.h.tests/Makefilecompiles the core straight from the submodule (-Irx888_tools/src/fx3_cmd -Irx888_tools/include); local harness modules re-point"fx3_proto.h"→"rx888.h".fx3_cmdis the same size;make,check-health,check-cliall green; dispatch/usage identical.Verification gate (Phase 0)
Post-#26
rx888.hmatches the firmware source of truth (SDDC_FX3/protocol.h) on every shared symbol:AD8370_VGA,LED_BLUEbit 11,GETSTATS=0xB3,WDG_MAX_RECOV=14, USB IDs; theHANG*test codes are correctly absent from the public header.Anti-drift rule (going forward)
Core edits land in rx888-tools first, then the firmware bumps the submodule pin — the firmware repo never re-forks the core or re-creates
fx3_proto.h. (fx3_bulkstays firmware-local by design: rx888-tools streams vialibrx888/rx888_stream.)Bench follow-up (needs hardware, not blocking)
Run
tests/fw_test.sh/validate.shonce on a real RX888mk2 to confirm the harness scenarios pass post-cutover.https://claude.ai/code/session_01YcMhE8u6WjetJosAT5dLPa