fix(cat): resolve SmartSDR Flex/TS-2000 split behavior gaps and fix TS-2000 satellite-mode TX block#3739
Conversation
Behavior corrections to the SmartCAT (Flex/TS-2000) split path, independent of slice-lifecycle management (deferred to the slice-management consolidation): - FB/ZZFB no longer falls back to VFO A when there is no VFO B — it returns "?;" (matches ZZME). The old fallback reported VFO A's frequency for FB while ZZME returned "?;", and that freq-yes/mode-no contradiction broke controllers' VFO sync (aethersdr#3633). - FR (TS-2000) is now a real RX-VFO selector: FR0 = VFO A, FR1 = VFO B accepted only when a real VFO B exists (configured + present) else "?;"; FA always reports VFO A (no A/B swap). FR; reports the selector. - ZZFR is unsupported → "?;". Removed its invented std::swap(m_vfoA,m_vfoB), which has no SmartSDR equivalent and corrupted the VFO mapping. - Split (ZZSW/FT/ZZFT) now actually engages a mechanism instead of just toggling a flag: reuse the operator-configured VFO B slice if present; NOT_ENABLED ("?;") if a VFO B is configured but its slice is absent; else an XIT-offset fallback on slice A. XIT VFO-B set rejects an out-of-range offset (|Δ|>9999 Hz) instead of silently clamping to a wrong TX frequency. - Destructor tears the split down on client disconnect (restore TX to slice A, clear the XIT offset) so it isn't left applied on the radio. No slice is created or removed — split uses reuse/XIT only. Dedicated-TX-slice creation on a genuine single-VFO port is deferred to the slice-management work (single-VFO split falls back to XIT in the interim). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Aligns the CAT regression tests with the corrected SmartCAT behavior: - Flex §13.34: ZZFR is now unsupported → "?" (the invented A/B swap was removed). Asserts "?" for read/FR1/FR0 and uses query() so the "?;" replies are consumed (the old send()-based set assumed a silent no-op and desynced the read stream). - TS-2000 §3.1: FB; → "FB"+11-digit Hz when a VFO B is mapped, else "?" (no more VFO-A fallback — aethersdr#3633). §3.2 already skips when no VFO B. - TS-2000 §8: FR is now a real gated RX-VFO selector — FR1 accepted only when a real VFO B exists (else "?"), FA still reports VFO A (no swap). query() FR1 so a "?;" rejection reply is consumed. Verified on a running FLEX-6500: Flex 123/126 (the 3 remaining are the PTY round-trip tests, which need --pty), TS-2000 §3/§8 pass (the §10 "ZZ-blocked" failures are from running the TS-2000 suite against a FlexCAT-dialect port). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ethersdr#3702) Per aethersdr#3702 triage: single-VFO split-enable now returns NOT_ENABLED (matches SmartSDR-for-Mac) instead of an XIT-offset fallback. Removes the m_xitSplit state and its FB/ZZME/teardown branches. Create-on-demand (SmartSDR-for-Windows behavior) remains deferred to slice-lifecycle work. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…2000 split
WSJT-X in TS-2000 (Kenwood) mode drives split through Hamlib's TS-2000 set_vfo,
which queries `SA;` (satellite mode) before VFO operations. SmartCatProtocol had
no SA handler, so it fell through to the generic unknown-command `?;`; Hamlib's
Kenwood backend treats `?;` as a fatal -9 ("Command rejected by the rig"), which
aborts set_split_freq_mode and blocks transmit. We are never in satellite mode,
so report it OFF (`SA0;`) and accept any SA set as a no-op, letting Hamlib proceed.
Pre-existing TS-2000-dialect completeness gap (SA was never implemented) — NOT a
regression from this PR — but found while doing split-VFO compliance testing with
WSJT-X TS-2000 mode, and it lives in this same file, so it's folded in here rather
than churning a separate PR. Confirmed on-air: WSJT-X TS-2000 split transmit works
after this change.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The split sections assumed a usable VFO B and issued enable commands with send(); on a single-VFO port (or one whose VFO B slice isn't open) those return "?" (NOT_ENABLED) instead of a silent ack, and the unconsumed "?" desynced the read stream — cascading into dozens of bogus failures. Gate the split sections on a VFO-B probe and, when absent, verify the NOT_ENABLED behavior with query() (consuming the "?"): - CAT_Flex_test section 8 + the 12.7/12.8 FT<->ZZSW cross-check - CAT_TS-2000_test section 7 (FT) Also accept "?" in Flex 3.1 (ZZFB) on a single-VFO port (aethersdr#3633: no VFO-A fallback). Result: single-VFO runs clean (Flex 5002 103/106 — only the 3 --pty tests; TS-2000 5004 93/93); dual paths unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- aethersdr#1: split teardown on disconnect undoes a split ONLY if WE engaged it (new m_weEngagedSplit) — no longer stomps an operator's/another client's split that this client merely adopted via FT1/ZZSW1. - aethersdr#2: cmdOI returns "?;" with no VFO B instead of falling back to VFO A, consistent with FB/ZZME (aethersdr#3633). - aethersdr#4/aethersdr#8: FT/ZZSW/ZZFT share one splitCommand() helper; a malformed arg returns "?;" rather than silently disabling split (was: any non-"1" arg → disable). - aethersdr#5: clear the m_rxVfoB RX-VFO selector on split teardown. - aethersdr#7: reported split state is derived from the model (splitActive(): a non-RX slice is TX) instead of a stored m_splitEnabled flag that could drift when split is changed via the GUI / rigctld / another CAT client. - aethersdr#9: drop the stale "…else XIT" remnant from the split-mechanism comment. - aethersdr#3: document that the FR selector (m_rxVfoB) intentionally does not swap VFOs (SmartSDR-Mac parity) — echo-only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…dr#10) Replace the duplicated `c.query("ZZFB"/"FB").startsWith(...)` VFO-B probe in the split-section gates with one hasVfoB(CatClient&) helper per suite, so the gates have a single definition and can't drift apart. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- aethersdr#7: deriving split state from the model (a non-RX slice is TX) reads STALE state — setTxSlice() updates the slice's TX flag only on the radio's async status echo, so FT;/ZZSW;/IF reported split OFF in the window right after enableSplit (Flex 8.2/8.3/8.4/8.8/12.7, TS-2000 7.2/7.3). Restore the synchronous m_splitEnabled flag for the reported state; keep m_weEngagedSplit for the aethersdr#1 teardown-ownership fix. Cross-consumer reconciliation = RFC aethersdr#3715. - aethersdr#2: cmdOI returning "?;" with no VFO B broke the "OI always returns a status body" contract (CAT test 15.15) and real-radio parity. OI is a composite status block (like IF), not a value read — restore the VFO-A fallback. The aethersdr#3633 "?;" rule stays on FB/ZZME. Validated: Flex 5001 123/126 (3 = --pty) / 5002 103/106; TS-2000 5003 96/96 / 5004 93/93. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…meter) cmdZZFR ignores its arg (ZZFR is unsupported → "?;"). Drop the parameter name so gcc on Linux (RPi 5) doesn't warn; clang on macOS didn't flag it. Clean build on both toolchains. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The --pty option carried a default value, so the PTY round-trip section always
ran and timed out (3 spurious "failures") when no PTY device was wired up. Gate
it on parser.isSet("pty"): when --pty isn't given, skip §15 (Flex) / §14
(TS-2000) cleanly via section15skip()/section14ptySkip(), mirroring the existing
--ptt/--cw skip pattern.
Suites are now fully green without --pty (Flex 103/103, TS-2000 90/90).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Thorough, well-documented work — thank you, @K5PTB. The splitCommand() consolidation removes the toggle-on-poll defect across all three split verbs, the m_weEngagedSplit ownership flag is exactly the right guard for teardown-on-disconnect, the kRitMaxHz hoist is clean (no duplicate definition left behind), and the test changes correctly switch to query() on the now-?; paths so the read stream stays in sync. CI is green across build / macOS / Windows / CodeQL. The inline comments explaining why (the async setTxSlice vs. synchronous m_splitEnabled, the OI composite-block exception to the #3633 rule) are genuinely helpful for the next maintainer.
A couple of things to confirm rather than blockers — both stem from the deliberate "manager-free" scope:
1. disableSplit() tears down a split this client only adopted. enableSplit() is careful: it sets m_weEngagedSplit only when it actually moves TX onto B, so it never claims an operator's/another client's pre-existing split. But an explicit FT0/ZZSW0/ZZFT0 calls teardownSplit() unconditionally — which does sliceA()->setTxSlice(true) and seizes TX back to A regardless of m_weEngagedSplit. In a MultiFlex session, client X's FT0 would then stomp a split that the operator (or client Y) established. The destructor correctly gates on m_weEngagedSplit; the explicit-disable path does not. Is letting an explicit FT0 override an un-owned split the intended asymmetry, or should disableSplit() also respect ownership (e.g. only reclaim when m_weEngagedSplit)? Worth a one-line comment either way.
2. FR stores m_rxVfoB but nothing routes RX to it. The PR/commit describes FR as "a real gated RX-VFO selector (was a no-op)," but sliceA()/sliceB() map purely off m_vfoA/m_vfoB and never consult m_rxVfoB, and FA "always reports VFO A" per the comment. So FR1 gates on a present VFO B and records intent, but RX reception/reporting doesn't actually move — functionally it's still an accepted-and-echoed flag, just gated now. If that's the intended SmartSDR-for-Mac parity (RX stays on A; only the echo changes), great — but the "real selector" framing may mislead a future reader into assuming RX routing changed. Consider softening the wording, or confirm whether FR1 is meant to be observable beyond the FR; echo.
Neither blocks merge; the split/SA/#3633 behavior changes themselves look correct and the deferral of create-on-demand to RFC #3715 is reasonable. Nice attention to stream-sync in the tests.
🤖 aethersdr-agent · cost: $6.3649 · model: claude-opus-4-8
|
@AetherClaude and reviewers, Thanks for the careful read. Both points are intended behavior under the manager-free scope — confirming the reasoning rather than changing anything here: 1.
So: a disconnect is conservative, an explicit command is authoritative. I'll add a one-line comment making that distinction explicit when this comes off draft. Full cross-client split-state reconciliation (so every client reports radio truth rather than its own intent) is the larger item deferred to RFC #3715. 2. Neither needs a code change to land — appreciate the thorough pass and the note on stream-sync in the tests. 🤖 Generated with Claude Code |
There was a problem hiding this comment.
Thorough, well-reasoned work — thanks @K5PTB. The inline comments documenting why each behavior was chosen (synchronous m_splitEnabled vs. the async setTxSlice() echo, the OI-must-return-a-body vs. FB-returns-?; distinction, the ZZFR removal rationale) make this easy to follow, and the test updates are notably careful about the read-stream sync issue (using query() to consume ?; replies rather than send()). All six CI checks pass.
A couple of things to confirm — neither blocking:
1. New destructor reaches into RadioModel via the non-owning m_model pointer.
~SmartCatProtocol() → teardownSplit() → sliceA() → m_model->slices(). This is safe in practice because CatPort tears down sessions before the model is destroyed, and SliceModel::setTxSlice() only emits commandReady (harmless even if the radio link is already gone — no crash, no blocking I/O). The one assumption worth keeping in mind: sliceA()'s if (!m_model) guard catches a null pointer but not a dangling one, so this relies on RadioModel always outliving the CAT sessions. That holds with the current CatPort ownership, so just flagging it as a lifetime invariant to preserve, not a defect.
2. Confirm the intended asymmetry in split teardown ownership.
The disconnect path is guarded by m_weEngagedSplit (so a disconnect never stomps a split this client only adopted) — good. But the explicit-command path (FT0/ZZSW0/ZZFT0 → disableSplit() → teardownSplit()) is unguarded: it always hands TX back to slice A, even when this client merely adopted an operator's/another client's pre-existing split. That's defensible — an explicit "split off" is a deliberate user action and arguably should win — but it's a slightly different contract than the disconnect path, and the PR description's "never stomps an operator's split this client merely adopted" framing could read as applying to both. Worth a one-line confirmation that explicit-disable-stomps-adopted-split is intended (and ideally a test asserting it).
The SA0; minimal-form caveat and the cross-consumer reconciliation deferral to RFC #3715 are both clearly called out — that scoping looks right. Nice fix on the WSJT-X set_split_freq_mode (-9) abort; that's a real integration pain point.
🤖 aethersdr-agent · cost: $2.3218 · model: claude-opus-4-8
ten9876
left a comment
There was a problem hiding this comment.
Approving — companion to the just-merged #3724, same high quality. The splitCommand() consolidation removes the toggle-on-poll defect across FT/ZZSW/ZZFT; m_weEngagedSplit correctly scopes disconnect-teardown to a split this client engaged; the #3633 freq-yes/mode-no rule, FR/ZZFR, and NOT_ENABLED handling are SmartSDR-for-Mac-aligned; and the TS-2000 SA0; fix unblocks WSJT-X's set_split_freq_mode (-9) abort. The two bot notes are confirmed-intended behavior (authoritative explicit-FT0 vs conservative disconnect; FR echo-only RX-stays-on-A parity) — clarifying comments to follow, no code change needed. Well-tested, no conflict, CI green. Create-on-demand/cross-client reconciliation deferred to RFC #3715. Landing per maintainer decision (lifting the author's hold-for-next-cycle, as with #3724). Thanks @K5PTB.
Summary
This started as the Flex/TS-2000-vs-SmartSDR behavior-difference fixes from the #3702 triage. Integration testing (WSJT-X, fldigi, MacLoggerDX) surfaced one more real fix (TS-2000
SA), and a high-effort code review plus cross-platform testing produced lifecycle/robustness hardening.1. SmartCAT behavior alignment (#3702)
FB/ZZFBandZZMEwith no VFO B →?;(no VFO-A fallback) — the CAT/TCP TS-2000: VFO-1/VFO-2 frequency and mode not passed through to control software #3633 freq-yes/mode-no contradiction.FRis now a real gated RX-VFO selector (was a no-op);ZZFR→?;(was an invented A/Bstd::swap).FT/ZZSW/ZZFTengage the operator's VFO B as the TX slice (was a status-flag-only toggle).?;) when VFO B is configured-but-absent or on a single-VFO port (matches SmartSDR-for-Mac).2. WSJT-X TS-2000 integration fix
SA(satellite mode) implemented (SA0;). WSJT-X's Hamlib TS-2000 backend queriesSA;duringset_vfo/split setup; the prior unknown-command?;made Hamlib abortset_split_freq_mode(-9) and block transmit. Pre-existing TS-2000 gap, folded in here since it lives in this file and blocked TS-2000 split.3. Lifecycle & safety (code review)
m_weEngagedSplit) — never stomps an operator's/another client's split this client merely adopted.FT/ZZSW/ZZFTshare onesplitCommand()helper; a malformed arg returns?;instead of silently disabling split.m_rxVfoBselector reset on teardown; stale "…else XIT" comment removed;cmdZZFRunused-parameter warning fixed (clean on gcc + clang).4. Tests
hasVfoB()helper.--ptyis passed (no spurious timeouts).Scope / deferred
This grew beyond the original #3702 triage. Create-on-demand split on a single-VFO port (the SmartSDR-for-Windows path) and cross-consumer split-state reconciliation are deferred to the slice-lifecycle RFC #3715. (Two review suggestions — deriving split state from the model, and
OI→?;— were tried and reverted: the slice TX flag updates asynchronously, andOIis a composite status block that must always return a body. See commit33c3d6e7.)Addresses #3702. Refs #3715.
Built and regression tested on MacOS (M2), Linux (RPi 5) and Windows 11. Integration testing (WSJT-X 3.0.0, fldigi-4.2, MacLoggerDX) on MacOS (M2).
🤖 Generated with Claude Code