Files
trx-rs/docs/Improvement-Areas.md
T
Claude d512268526 [feat](trx-vdes): implement Turbo FEC, CRC-16, and link-layer parsing
Add the three missing VDES decoder components per ITU-R M.2092-1:

- turbo.rs: Turbo FEC decoder with dual 8-state RSC constituent
  encoders, BCJR/MAP iterative decoding (8 iterations), QPP
  interleaver, and rate-1/2 depuncturing
- crc.rs: CRC-16-CCITT validation (poly 0x1021, init 0xFFFF) for
  decoded link-layer frames
- link_layer.rs: Structured parsing of M.2092-1 link-layer frames
  (Messages 0-6) including station addressing, ASM identification,
  geographic bounding boxes, and ACK/NACK reporting

The main decode pipeline now attempts turbo decoding first with CRC
validation, falls back to Viterbi when turbo fails, and reports
crc_ok=true when either path validates. 27 tests covering all new
modules.

https://claude.ai/code/session_01SJSN7cv3zoL1xNcb8ex2zY
Signed-off-by: Claude <noreply@anthropic.com>
2026-03-29 14:50:42 +02:00

6.3 KiB
Raw Blame History

Improvement Areas

A comprehensive audit of the trx-rs codebase covering code quality, architecture, security, testing, and performance. Each item includes the affected location and a suggested fix.

Last updated: 2026-03-29


Critical (P0)

Plugin signing and cross-platform validation — DROPPED

Plugin system has been removed from the codebase. No longer applicable.


High Priority (P1)

Session store mutex poisoning (auth.rs) — RESOLVED

Location: src/trx-client/trx-frontend/trx-frontend-http/src/auth.rs

Resolution: All 6 .write().unwrap() / .lock().unwrap() calls replaced with .unwrap_or_else(|e| { warn!(...); e.into_inner() }) pattern. Lock poisoning now logs a warning and recovers the inner data instead of crashing.

No rate limiting on TCP listener — RESOLVED

Location: src/trx-server/src/listener.rs

Resolution: Added ConnectionTracker with per-IP connection limiting (default: 10 concurrent connections per IP). Connections exceeding the limit are rejected with a log warning. Slots are released when clients disconnect.

RigState is a 33-field flat struct — RESOLVED

Location: src/trx-core/src/rig/state.rs

Resolution: Decoder fields grouped into two sub-structs:

  • DecoderConfig: 8 *_decode_enabled bool fields
  • DecoderResetSeqs: 8 *_decode_reset_seq u64 counters

Both use #[serde(flatten)] to maintain backward-compatible JSON wire format. Updated across all consumers: rig_task.rs, audio.rs, api.rs, remote_client.rs, server.rs (rigctl, http-json), codec.rs.

No spawn_blocking timeout — RESOLVED

Location: src/trx-server/src/listener.rs

Resolution: Satellite pass computation wrapped in tokio::time::timeout(30s, ...) with graceful fallback to empty results on timeout or panic.


Medium Priority (P2)

Command handler boilerplate — RESOLVED

Location: src/trx-core/src/rig/controller/handlers.rs

Resolution: Created rig_command! declarative macro that generates unit-struct command implementations from a concise table of (name, preconditions, execute body). 7 unit commands (PowerOn, PowerOff, ToggleVfo, Lock, Unlock, GetTxLimit, GetSnapshot) now use the macro. Commands with custom fields/validation (SetFreq, SetMode, SetPtt, SetTxLimit) remain as explicit impls.

No command execution timeouts at CommandExecutor level — ALREADY RESOLVED

Location: src/trx-server/src/rig_task.rs

tokio::time::timeout(command_exec_timeout, process_command(...)) already wraps all command execution (lines 370425). Default timeout: 10s. No further changes needed.

No forward compatibility in protocol — RESOLVED

Location: src/trx-protocol/src/types.rs, src/trx-protocol/src/codec.rs

Resolution:

  • Added optional protocol_version: Option<u32> to both ClientEnvelope and ClientResponse (current version: 1, defined as PROTOCOL_VERSION constant).
  • parse_envelope() now distinguishes between truly malformed JSON and valid JSON with an unrecognised cmd value, enabling clearer error messages.

unsafe string construction in spectrum encoding — RESOLVED

Location: src/trx-client/trx-frontend/trx-frontend-http/src/api.rs:63

Resolution: Replaced unsafe { String::from_utf8_unchecked(out) } with String::from_utf8(out).expect("base64 output is always valid ASCII").

6 #[allow(dead_code)] annotations — RESOLVED

Resolution:

  • is_tx_endpoint in auth.rs: made pub and removed annotation (used in tests, available for TX access control integration).
  • session_ttl() in config.rs: removed annotation (public API method).
  • device in real_iq_source.rs: annotation kept (lifetime anchor for stream).
  • iq_tx in vchan_impl.rs: annotation kept (broadcast sender kept alive).
  • fixed_slot_count in vchan_impl.rs: annotation kept (documents reserved slots).
  • process_pair in demod.rs: annotation kept (stereo AGC variant for future use).

Low Priority (P3)

Missing tests for critical modules — PARTIALLY RESOLVED

  • history_store.rs: Added 4 unit tests covering timestamp generation, serde round-trip, save/load round-trip, and expiry filtering.
  • audio.rs, api.rs, main.rs: Remain without tests (require ALSA/hardware mocking infrastructure that is beyond the scope of this pass).
  • rig_task.rs: Existing 4 tests adequate; integration tests deferred.

FT-817 VFO state inference is fragile — IMPROVED

Location: src/trx-server/trx-backend/trx-backend-ft817/src/lib.rs

Resolution: Improved update_vfo_freq() to handle the ambiguous case where both VFOs share the same frequency. When VFO B has a cached frequency that differs from the current reading, inference correctly assigns to VFO A. When frequencies match (ambiguous), defaults to VFO A — resolved after VFO toggle primes both sides. Added detailed comments explaining the inference logic.

VDES decoder has incomplete FEC — RESOLVED

Location: src/decoders/trx-vdes/src/

Turbo FEC decoder (dual 8-state RSC with BCJR/MAP iterative decoding, QPP interleaver), CRC-16-CCITT validation, and M.2092-1 link-layer frame parsing (Messages 06) have been implemented. The decode pipeline now attempts turbo decoding first, falls back to Viterbi, and validates CRC on both paths.

Plugin system lacks versioning and lifecycle — DROPPED

Plugin system has been removed from the codebase. No longer applicable.

Configurator serial detection is stubbed — RESOLVED

Location: src/trx-configurator/src/detect.rs

Resolution: Implemented detect_serial_ports() using tokio_serial::available_ports(). Returns (port_name, description) pairs with USB vendor/product info, Bluetooth, PCI, and Unknown port type descriptions.

Inconsistent frequency/rig naming across crates

Field naming varies across the codebase (freq_hz vs center_hz, rig_id vs id, model vs rig_model). Analysis shows these reflect distinct semantic contexts rather than true inconsistencies:

  • freq_hz: dial frequency; center_hz: SDR capture center; cw_center_hz: CW tone
  • rig_id: stable config key; id: runtime UUID
  • model: hardware model string; rig_model: config parameter

Decision: Documented as intentional. Renaming would break the wire protocol and provide minimal benefit. The _hz suffix convention is consistently applied.