From 82e1c19d3a92e6591a3ce3fb32791a7fdb30a7dd Mon Sep 17 00:00:00 2001 From: Stan Grams Date: Wed, 8 Apr 2026 21:51:44 +0200 Subject: [PATCH] [docs](trx-rs): add FIX_PLAN.md with codebase weak spot analysis Co-Authored-By: Claude Opus 4.6 Signed-off-by: Stan Grams --- FIX_PLAN.md | 143 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 143 insertions(+) create mode 100644 FIX_PLAN.md diff --git a/FIX_PLAN.md b/FIX_PLAN.md new file mode 100644 index 0000000..d840181 --- /dev/null +++ b/FIX_PLAN.md @@ -0,0 +1,143 @@ +# Fix Plan + +Current state analysis of trx-rs as of 2026-04-08. + +## Overall Assessment + +The codebase is in good shape. Clippy is clean, no `unsafe` code, no TODO/FIXME markers, +robust error handling throughout. One broken test and several untested crates are the +main weak spots. + +--- + +## P0 — Broken Test + +### 1. `test_toggle_ft8_decode` returns 500 instead of 200 + +**Location:** `src/trx-client/trx-frontend/trx-frontend-http/src/api/mod.rs:1016` + +**Root cause:** The handler `toggle_ft8_decode` (decoder.rs:353) requires +`context: web::Data>` for multi-rig state resolution. +The test registers `state_rx` and `rig_tx` but not `context`, so actix-web returns 500 +(missing app data). A `make_context()` helper already exists at line 757 but is unused +by this test. + +**Fix:** Add `.app_data(web::Data::new(make_context()))` to the test's `App` builder +(line 1036-1041). ~1 line change. + +**Impact:** This is the only failing test in the entire suite (50 pass, 1 fail). + +--- + +## P1 — Test Coverage Gaps + +### 2. trx-aprs decoder — 0 tests (596 LOC) + +**Location:** `src/decoders/trx-aprs/src/lib.rs` + +Bell 202 AFSK demodulator + AX.25 HDLC frame parser + CRC-16 validation. +No `#[cfg(test)]` module at all. + +**Suggested tests:** +- CRC-16 computation on known frames +- HDLC flag detection and bit-unstuffing +- Full frame decode from synthetic AFSK audio (1200 baud sine pairs) +- Rejection of corrupted frames (bad CRC, truncated) + +### 3. trx-decode-log — 0 tests (226 LOC) + +**Location:** `src/decoders/trx-decode-log/src/lib.rs` + +JSON Lines file writer with date-based rotation. Pure I/O wrapper. + +**Suggested tests:** +- Write + read-back round-trip in a tempdir +- Date rotation triggers new file creation +- Flush error logging (mock writer) + +### 4. trx-reporting — partial tests (1,065 LOC across 2 files) + +**Location:** `src/trx-reporting/src/pskreporter.rs` (582 LOC), +`src/trx-reporting/src/aprsfi.rs` (483 LOC) + +Both files have `#[cfg(test)]` modules but coverage is limited to serialization. +Network behavior (reconnect, rate-limit, batching) is untested. + +**Suggested tests:** +- PSKReporter UDP datagram encoding round-trip +- APRS-IS login line formatting +- Spot batching and dedup logic (unit-testable without network) + +--- + +## P2 — Code Quality + +### 5. `audio.rs` is 4,000 LOC + +**Location:** `src/trx-server/src/audio.rs` + +Houses all decoder task launchers (FT8, FT4, FT2, APRS, AIS, VDES, CW, WSPR, LRPT, +WEFAX). Each launcher follows the same pattern. The file is coherent but large. + +**Suggested improvement:** Extract decoder launchers into a `decoders/` submodule +within trx-server, one file per decoder family (e.g., `ftx.rs`, `aprs.rs`, `wefax.rs`). +Keep the audio pipeline and capture logic in `audio.rs`. + +### 6. `scheduler.rs` is 1,585 LOC + +**Location:** `src/trx-client/trx-frontend/trx-frontend-http/src/scheduler.rs` + +Mixes grayline computation, timespan matching, satellite pass prediction, and the +scheduler state machine. Well-tested but dense. + +**Suggested improvement:** Extract grayline and satellite pass logic into separate +modules (these are pure functions with no HTTP dependencies). + +--- + +## P3 — Minor + +### 7. `#[allow(dead_code)]` in soapysdr backend (4 annotations) + +**Locations:** +- `vchan_impl.rs:66,87` — `fixed_slot_count`, `process_pair` +- `real_iq_source.rs:20` — `device` +- `demod.rs:113` — lifetime anchor + +All documented as intentional (lifetime anchors / reserved capacity). No action needed +unless the fields can be converted to `PhantomData` or `_`-prefixed without breaking +semantics. + +### 8. FrontendRuntimeContext test helper duplication risk + +**Location:** `src/trx-client/trx-frontend/trx-frontend-http/src/api/mod.rs:757` + +`make_context()` and `spawn_rig_responder()` are good helpers but only used by some +tests. As new endpoint tests are added, ensure they consistently use these helpers to +avoid repeating the `test_toggle_ft8_decode` bug. + +--- + +## Implementation Order + +```mermaid +gantt + title Fix Plan + dateFormat X + axisFormat %s + + section P0 + Fix test_toggle_ft8_decode :p0, 0, 1 + + section P1 + Add trx-aprs tests :p1a, 1, 3 + Add trx-decode-log tests :p1b, 1, 2 + Expand trx-reporting tests :p1c, 1, 3 + + section P2 + Split audio.rs decoder launchers :p2a, 3, 5 + Extract scheduler pure functions :p2b, 3, 5 +``` + +P0 is a one-line fix. P1 items are independent and can be parallelized. P2 items are +refactors that should wait until P1 tests provide regression safety.