fix(cat): make set_split_vfo / tx_enable idempotent to stop radio TX watchdog#2568
Conversation
…watchdog
cmdSetSplitVfo unconditionally called setTxSlice(true) on every invocation,
emitting `slice set N tx=1` to the radio even when the slice was already
the TX slice. cmdTxEnable in TciProtocol had the same pattern.
Hamlib clients (VARA, VARAC, N1MM, WSJT-X, fldigi) poll set_split_vfo
every ~3 s as part of their state-sync loop. Multiplied by AetherSDR's
8 CAT channels (RigctlServer + RigctlPty) and several concurrent clients,
the radio receives bursts of redundant `slice set N tx=1` commands at a
sustained ~3 s cadence — observable in connection logs as four identical
commands at the same millisecond.
On FLEX-8600 firmware 4.1.5 this repeated tx-assignment churn appears
to disturb the radio's TX state machine: after PTT engages, the radio
issues `state=UNKEY_REQUESTED` roughly 1 s into the transmission even
though the client is still sending `xmit 1` and dax_tx VITA-49 packets
at the correct rate. Symptom for the operator is "PTT engages, carrier
appears, then radio drops back to RX after ~1 second" — making VARA HF,
VARAC, and any other digital-mode software effectively unusable.
Confirmed root cause by instrumenting feedDaxTxAudio and sendToRadio:
DAX TX packets continued to flow throughout the broken-PTT window with
correct stream ID (0x84000000), size (284 bytes), and socket sendto()
returning sent=284. The unkey was radio-side, not client-side packet
loss or audio gating. Disabling CAT entirely made PTT hold for the full
transmission duration, confirming the spam was the trigger.
Mirror the existing `!isTxSlice()` guard from cmdSetPtt (line 397):
if (auto* s = currentSlice(); s && !s->isTxSlice())
s->setTxSlice(true);
When the slice is already TX, the command is a no-op anyway — sending
it again costs a round trip and apparently provokes firmware-side
TX-state churn. The first non-redundant call (e.g. on actual split
disable, or after slice teardown / profile load) still goes through.
Tested on macOS with FLEX-8600 fw 4.1.5.39794 + VARA HF v4.9.0 + VARAC:
PTT now holds for the full transmission instead of unkeying after ~1 s.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Thanks for the deep dive — that's an exceptionally well-documented root cause. The diagnosis path (ruling out DAX, packet stream, kernel writes before landing on CAT-side state churn) and the counter-experiment confirming direction are the kind of evidence that makes this kind of "looks like a TX-path bug" fix reviewable at all. Appreciated.
Verified against main:
RigctlProtocol.cpp:443-444(split-disable branch) andTciProtocol.cpp:407-413(tx_enable enable branch) both callsetTxSlice(true)unconditionally.RigctlProtocol::cmdSetPtt(line 397) andTciProtocol's set_ptt path (line 386) already have the!isTxSlice()guard.
So the fix really is just "apply the existing pattern to the two stragglers." Minimal, on-pattern, and the non-redundant cases (!enable split disable after teardown, profile reload re-claim) still flow through.
One thing worth checking before merge — in TciProtocol::cmdTxEnable, the current code wraps setTxSlice(true) in QMetaObject::invokeMethod(s, [s]() { ... }, Qt::QueuedConnection). The cmdSetPtt precedent (RigctlProtocol.cpp:395-399 and TciProtocol.cpp:384-388) puts the !isTxSlice() check inside the queued lambda, so the check runs on the same thread as the mutation. If this PR places the guard outside the invokeMethod call, it still kills the poll storm (caller thread sees isTxSlice() == true on subsequent polls), but there's a small TOCTOU window where another protocol channel could race. Putting it inside the lambda matches the established pattern and closes that window — not a blocker, but worth mirroring cmdSetPtt exactly.
For RigctlProtocol::cmdSetSplitVfo, the existing code calls currentSlice()/setTxSlice() directly (no invokeMethod), so the inline s && !s->isTxSlice() check is the right shape.
Two follow-up thoughts (not for this PR):
- The same poll-storm hypothesis would predict similar firmware churn from any other CAT command that maps to a redundant radio-side state set. If you have logs handy showing only
slice set ... tx=1was the culprit, great; otherwise it might be worth a quick grep over the other hamlib state-sync handlers (set_freq,set_mode) for unguarded sets in the future. - The fact that v0.8.20 had the same code but didn't reproduce is interesting — agrees with your "radio TX-state tracking got more sensitive" theory, but might also be worth a one-line comment near the guard pointing future readers at this PR so the
!isTxSlice()check doesn't get "cleaned up" by a future drive-by refactor.
Minor: the PR description has a templated placeholder leak — s ${PR_DIFF}${PR_DIFF} !s->isTxSlice() — looks like a render bug, not a code issue.
Nice work, and thanks for the patience to chase this down across the full TX pipeline.
ten9876
left a comment
There was a problem hiding this comment.
Claude here, reviewing on Jeremy's behalf.
Verdict
Strong fix with exemplary diagnostic work. Merge-worthy after Jeremy verifies on his FLEX-8600.
The root-cause analysis is the kind I'd want every TX-path bug report to include — the counter-experiment (gating dax_tx packets shortens unkey to 127 ms, proving packet flow isn't the trigger) is the piece that elevates this from "guess that fixed it" to "this is the trigger, here's the evidence."
Verified
isTxSlice()reads radio-confirmed state, not a cached local prediction.SliceModel.h:34returnsm_txSlicewhich is set from radio status updates, so the idempotency check tests against the authoritative ground truth — exactly right.SliceModel::setTxSliceis not idempotent at the model layer.SliceModel.cpp:332-335sendsslice set N tx=0/1unconditionally. Moving the guard inside the model would be wrong (model would trust its own state vs. the radio's) — the PR's choice to guard at the call site is correct.cmdSetPttclaim is correct —RigctlProtocol.cpp:397-398andTciProtocol.cpp:386-387both already have the!isTxSlice()guard, matching the pattern this PR extends.- All build / check-paths / check-windows (skipped) / CodeQL / analyze (cpp) green.
Stale-code audit
setTxSlice(true) call sites across the codebase:
| Site | Pattern | Status |
|---|---|---|
RigctlProtocol.cpp:398 (cmdSetPtt) |
one-shot, polled but guarded | ✓ pre-existing guard |
RigctlProtocol.cpp:444 (cmdSetSplitVfo) |
polled every ~3s | fixed by PR |
TciProtocol.cpp:387 (one of the TCI handlers) |
guarded | ✓ pre-existing guard |
TciProtocol.cpp:411 (cmdTxEnable) |
TCI command path | fixed by PR |
MainWindow.cpp × 6 (lines 8929, 8962, 9312, 9360, 10290, 13086) |
one-shot user actions / split setup | not bugs |
The 6 MainWindow sites are user-button-press / slice-creation / split-setup handlers — they fire once per user action, not in polled loops, so they're outside the hot-loop scenario. Confirmed by quick context check that none are inside repeating timers or status-handler callbacks.
Minor notes (non-blocking)
- The PR uses C++17 init-if syntax (
if (auto* s = currentSlice(); s && !s->isTxSlice())). Matches project style; clean. - The added comment explaining the Hamlib polling cadence is good — exactly the kind of "comment non-obvious protocol decisions" that CLAUDE.md asks for. Worth adding firmware version to the comment (
fw 4.1.5.39794from your test setup) since the PR explicitly notes the symptom didn't reproduce on the same un-guarded code in v0.8.20 — that's a firmware-sensitivity signal future readers should see in the source. - No tests. Defensible — this needs a real hamlib client + radio + multi-second PTT window to reproduce. A unit test would just verify the guard exists, which the diff already does. Manual verification is the right test mode here.
What I'd want from Jeremy before merge
- Verify on the FLEX-8600 with VARA HF / VARAC on the actual rig
- Spot-check one other rigctld client (N1MM or fldigi) to make sure the fix doesn't regress them
- Ideally Windows TCI client check for the parallel
cmdTxEnablefix, though the same code path argument applies — if it works for rigctld it works for TCI
Thank you
@pepefrog1234 — the diagnostic discipline here (ruling out DAX path, packet format, network timing, firmware watchdog with targeted instrumentation, then the counter-experiment) is genuinely the right way to chase this kind of bug. The fact that v0.8.20 had the same unguarded code and didn't trigger is the honest note that keeps the analysis grounded — you're not claiming certainty beyond what you measured. Appreciated.
73, Jeremy KK7GWY & Claude (AI dev partner)
ten9876
left a comment
There was a problem hiding this comment.
Approved — diagnostic work is exceptional, root cause verified, fix is minimal and matches the existing cmdSetPtt guard pattern.
…#2931) ## Summary With two or more slices, TX would not stay on the slice the user picked — it kept jumping back to another slice every couple of seconds, and the user couldn't keep TX where they wanted it. Root cause is `cmdSetSplitVfo` reclaiming TX on every `set_split_vfo 0` **poll** rather than on a real split-state change. ## Root Cause Each rigctld / PTY CAT channel is bound to a fixed slice (`m_sliceIndex`). `cmdSetSplitVfo` moved TX onto that slice whenever a client sent `set_split_vfo 0` (split disabled). #2568 added an `!isTxSlice()` guard to stop the redundant **command spam**, but that only suppressed the already-TX case — it didn't stop TX from being **seized**. Hamlib loggers (VARA, VARAC, N1MM, fldigi) poll `set_split_vfo 0 VFOA` every few seconds. With a CAT channel bound to a non-TX slice the sequence was: ``` user clicks slice A TX badge → slice set 0 tx=1 (A is TX) ~1.7 s later logger polls split → channel bound to slice B cmdSetSplitVfo sees B != TX → slice set 1 tx=1 TX jumps back to B ``` The `!isTxSlice()` guard actually *guaranteed* the re-seize: the user moving TX away is exactly the moment the bound slice becomes non-TX, which re-satisfies the guard on the very next poll. Captured in the connection log (slice 0 = A, slice 1 = B, single client, no split): ``` 08:11:59.663 TX: C162|slice set 0 tx=1 ← user clicks A's TX badge 08:11:59.785 RX: slice 0 ... tx=1 ← A is TX (user succeeded) 08:12:01.379 TX: C167|slice set 1 tx=1 ← logger split poll, 1.7 s later 08:12:01.379 TX: C168|slice set 1 tx=1 08:12:01.379 TX: C169|slice set 1 tx=1 08:12:01.379 TX: C170|slice set 1 tx=1 08:12:01.498 RX: slice 1 ... tx=1 ← TX yanked back to B ``` ## Fix Track the last split state per `RigctlProtocol` instance (tri-state: unknown / disabled / enabled) and reclaim TX only on a genuine split→non-split **transition** (1→0 edge) — never on a steady-state poll or the first report after connect. ```cpp const bool wasEnabled = (m_lastSplitEnable == 1); const bool firstReport = (m_lastSplitEnable < 0); m_lastSplitEnable = enable ? 1 : 0; if (!enable && wasEnabled && !firstReport) { if (auto* s = currentSlice(); s && !s->isTxSlice()) s->setTxSlice(true); } ``` A logger that sits in non-split and polls repeatedly no longer fights the user's TX choice, while a real "turn split off" still moves TX back to the client's slice. ## Test plan - [x] macOS, FLEX-6600 fw 4.2.18, VARAC on rigctld - [x] Slice A = TX, add slice B → select TX back to A → TX **stays** on A (previously yanked to B within ~2 s) - [x] Genuine split toggle (split on → split off) still moves TX to the channel's slice - [ ] Reviewer: confirm contest-logger split workflows (N1MM true split on/off) still behave on Windows/Linux Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Jeremy Fielder <kk7gwy@aethersdr.com>
…aethersdr#2931) ## Summary With two or more slices, TX would not stay on the slice the user picked — it kept jumping back to another slice every couple of seconds, and the user couldn't keep TX where they wanted it. Root cause is `cmdSetSplitVfo` reclaiming TX on every `set_split_vfo 0` **poll** rather than on a real split-state change. ## Root Cause Each rigctld / PTY CAT channel is bound to a fixed slice (`m_sliceIndex`). `cmdSetSplitVfo` moved TX onto that slice whenever a client sent `set_split_vfo 0` (split disabled). aethersdr#2568 added an `!isTxSlice()` guard to stop the redundant **command spam**, but that only suppressed the already-TX case — it didn't stop TX from being **seized**. Hamlib loggers (VARA, VARAC, N1MM, fldigi) poll `set_split_vfo 0 VFOA` every few seconds. With a CAT channel bound to a non-TX slice the sequence was: ``` user clicks slice A TX badge → slice set 0 tx=1 (A is TX) ~1.7 s later logger polls split → channel bound to slice B cmdSetSplitVfo sees B != TX → slice set 1 tx=1 TX jumps back to B ``` The `!isTxSlice()` guard actually *guaranteed* the re-seize: the user moving TX away is exactly the moment the bound slice becomes non-TX, which re-satisfies the guard on the very next poll. Captured in the connection log (slice 0 = A, slice 1 = B, single client, no split): ``` 08:11:59.663 TX: C162|slice set 0 tx=1 ← user clicks A's TX badge 08:11:59.785 RX: slice 0 ... tx=1 ← A is TX (user succeeded) 08:12:01.379 TX: C167|slice set 1 tx=1 ← logger split poll, 1.7 s later 08:12:01.379 TX: C168|slice set 1 tx=1 08:12:01.379 TX: C169|slice set 1 tx=1 08:12:01.379 TX: C170|slice set 1 tx=1 08:12:01.498 RX: slice 1 ... tx=1 ← TX yanked back to B ``` ## Fix Track the last split state per `RigctlProtocol` instance (tri-state: unknown / disabled / enabled) and reclaim TX only on a genuine split→non-split **transition** (1→0 edge) — never on a steady-state poll or the first report after connect. ```cpp const bool wasEnabled = (m_lastSplitEnable == 1); const bool firstReport = (m_lastSplitEnable < 0); m_lastSplitEnable = enable ? 1 : 0; if (!enable && wasEnabled && !firstReport) { if (auto* s = currentSlice(); s && !s->isTxSlice()) s->setTxSlice(true); } ``` A logger that sits in non-split and polls repeatedly no longer fights the user's TX choice, while a real "turn split off" still moves TX back to the client's slice. ## Test plan - [x] macOS, FLEX-6600 fw 4.2.18, VARAC on rigctld - [x] Slice A = TX, add slice B → select TX back to A → TX **stays** on A (previously yanked to B within ~2 s) - [x] Genuine split toggle (split on → split off) still moves TX to the channel's slice - [ ] Reviewer: confirm contest-logger split workflows (N1MM true split on/off) still behave on Windows/Linux Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Jeremy Fielder <kk7gwy@aethersdr.com>
Summary
Digital-mode software (VARA HF / VARAC / WSJT-X / N1MM / fldigi) over rigctld is currently unusable on FLEX-8600 fw 4.1.5: PTT engages and the carrier comes up, but the radio drops back to RX after roughly 1 second even though the client is still asserting PTT and pushing dax_tx audio. This PR fixes that.
Root Cause
RigctlProtocol::cmdSetSplitVfoandTciProtocol::cmdTxEnableboth calledsetTxSlice(true)unconditionally, emittingslice set <N> tx=1to the radio every time the protocol handler ran — even when the slice was already the TX slice. The matching code incmdSetPttcorrectly guards with!isTxSlice(); only these two paths were missing the guard.Hamlib-based contest loggers and digital-mode software poll
set_split_vfoevery ~3 s as part of their state-sync loop. AetherSDR exposes 8 CAT channels (kCatChannels = 8, RigctlServer + RigctlPty), so a single polling client multiplied by all the channels it touches produces a burst of identicalslice set 0 tx=1commands. Connection logs show them stacked at the same millisecond:This sustained spam interferes with the radio's internal TX-state tracking. When PTT subsequently engages, the radio firmware decides the stream is unhealthy and forces an
UNKEY_REQUESTEDroughly 1 s into the transmission:The hamlib client (VARAC in this case) is still holding PTT — the radio gave up on its own.
Diagnosis Path
The symptom ("PTT cut after 1 s") naturally suggests DAX TX audio path, packet format, network timing, or firmware watchdog. We ruled all of those out with targeted instrumentation:
xmit 1reaches radio, radio entersstate=TRANSMITTINGm_radioTransmittingflips true viaradioTransmittingChangedm_daxTxModetrue in DIGU modefeedDaxTxAudioreceives audio throughout TX windowsendToRadioemits packets with correct stream ID (0x84000000) and size (284 B)QUdpSocket::writeDatagramreturnssent=284(no kernel drop)UNKEY_REQUESTED~1 s after PTTA counter-experiment confirmed direction: gating dax_tx packets so they only flow during
m_radioTransmitting=truemade the radio unkey in 127 ms instead of ~1 s — the radio wants a continuous packet stream, so packet flow isn't the trigger. The trigger is the CAT command spam: with the CAT service disabled entirely, PTT holds for the full transmission duration with the same DAX path, same firmware, same VARA build.Fix
Add the same
!isTxSlice()idempotency check thatcmdSetPttalready has:TciProtocol::cmdTxEnablegets the same treatment. When the slice is already the TX slice, the command is a no-op anyway; sending it again costs a TCP round-trip and apparently provokes firmware-side TX-state churn.The first non-redundant call still goes through — e.g. on actual split disable, after slice teardown, or after profile-global load (the original
#145re-claim case is preserved inMainWindow.cpp:8598).Why This Was a Deep Bug
set_split_vfo 0 VFOArequests.cmdSetPtthas the guard, so the pattern was known; only this one handler missed it.Test Plan
slice set 0 tx=1no longer spammed every 3 s in the connection log🤖 Generated with Claude Code