fix(icom): a refused tune must not read as a successful one

[nigelfenton]

What this fixes

FA is CI-V's NG — the radio's "no". Nothing in the backend consumed it. IcomCivScheduler::observe() treats FB and FA identically (both merely retire the transaction and carry no state), and isNg() existed in CivCodec.h:77 with no caller in the backend at all. So a refused frequency write left the optimistic frequency standing in the model, and the operator was looking at a number the radio never entered.

Why it is reachable in ordinary use

The IC-9700 has three bands and two receivers, and only two bands can be live at once — so a receiver cannot be tuned to a band the other receiver already holds. The radio answers cmd 05 with FA and stays put.

Measured on hardware 2026-08-29: six cross-band sets, six FAs, and the display followed all six. The radio never moved. See #4840.

The fix is correct on every model, not just that one: FA on a frequency write means the write did not take, whatever the reason.

What it does

On FA completing an outstanding "frequency" transaction, re-assert the radio's real VFO from m_frequencyHz (which is radio-authoritative) and tell the operator:

The radio refused the tune. It is still on 145.030000 MHz — on this model a receiver cannot move to a band the other receiver already holds.

The correction is deferred one event-loop turn via QTimer::singleShot(0, ...), exactly as the out-of-band gate in setSliceFrequency() and the refused mode in setSliceMode() already do — SliceModel has accepted and announced the operator's request by then, so a direct emit would be overwritten by that announcement and the indicator would keep lying.

Deliberately narrow. Only a frequency write is corrected, because that is the case with hardware evidence and a known-good restoration value. Other refused writes are a separate question and are left alone rather than guessed at.

Test

tests/icom_incident_telemetry_test.cpp — drives a genuinely in-flight frequency write (enqueued and dispatched, since observe() only retires a dispatched transaction), delivers an FA, and asserts three things:

1. the FA retires the outstanding frequency write
2. the operator is told the radio refused
3. the load-bearing one: exactly one frequency is republished, and it is the radio's held VFO — not the frequency the radio rejected

Assertion 3 is what fails on main: a backend that ignores FA publishes nothing there, and the display keeps the rejected frequency.

What this does NOT fix

rigctl still answers RPRT 0 for a refused tune. RigctlProtocol::cmdSetFreq replies synchronously and then marshals the real work through a QueuedConnection, so the NAK is structurally unobservable at the point the return code is written. That is a second bug with the same root symptom, and it is untouched here — this PR corrects the display, not rigctl's return code. Flagging it so a reviewer does not go looking for a rigctl change and fail to find one.

Verification limits

• The FA path is proven by the unit test above and by the six-refusal hardware session on 2026-08-29 (IC-9700 over RS-BA1).
• The warning string is not exercised against a live radio in this PR — the hardware session predates the user-facing message, which was written afterwards from what that session measured.
• No transmitter was keyed in producing this change.

[aethersdr-agent]

Issue fit

Partially. #4840 §2.5 step 1 asks for exactly this: "On isNg() for an outstanding tune, do NOT advance the model — re-assert the radio's actual frequency instead." The diff does the re-assertion, sources it from m_frequencyHz (confirmed radio-authoritative — the only assignment is IcomCivBackend.cpp:2021, from a decoded 03/00 reply), and defers it one event-loop turn with the same ordering contract setSliceFrequency()'s out-of-band gate (:3676) and setSliceMode() (:3711) already use. Steps 2 and 3 of that RFC section are correctly out of scope.

What it does not do is the "for an outstanding tune" half — the predicate never tests that this FA completed an outstanding tune. See Blocker 1.

Scope

File	What it changes	Claimed by title?	Verdict
src/core/backends/icom/IcomCivBackend.cpp	One block in onCivFrame(): consume FA, restore VFO, warn	Yes	In scope
tests/icom_incident_telemetry_test.cpp	Test-access helper + one new case	Yes	In scope

Everything in the diff is explained by the issue. No build config, no CHANGELOG.md, no settings keys, no new public/protocol surface (sliceChanged and configurationWarning already exist on IRadioBackend), and no - lines outside the include block — nothing deleted, so no previously-fixed symptom can recur. icom_incident_telemetry_test is a registered target (tests/tests.cmake:459).

Socket tests: none added or removed. The new case greps clean for QTcpServer/QUdpSocket/QLocalServer/bind/listen/connectToHost/Fake*; it injects CivFrame values into onCivFrame() via a friend accessor. Correct layer per the AGENTS.md test-layer boundary (refusals/non-events → socket-free state-machine injection), and no synthetic radio peer.

Blockers

1. The predicate is a sticky global, not a correlation to this frame — inline at IcomCivBackend.cpp:1921.

stats().lastCompletedKey is "the key of the last transaction that finished," updated in five places (IcomCivScheduler.cpp:40, 172, 237, 247) and never cleared. Two consequences I could not talk myself out of:

• An unmatched FA trips it. When no transaction is in flight, observe() returns Unmatched (IcomCivScheduler.cpp:322-324) without touching lastCompletedKey, and onCivFrame only early-returns on Stale. So the block runs against whatever key happened to finish last.
• "frequency" is almost always that key. semanticKey() (:3243) maps kReadFreq, kSetFreq and kSetFreqTrx to the same "frequency" string, and queueControl(cmdReadFrequency(addr)) runs on every even poll phase (:6107). So the guard sits latched "on" through normal operation on every Icom, not just after a tune.

The comment says "Only a frequency write is corrected here" — the key cannot express that distinction. And onCivFrame never filters frame.to, so an FA the radio addresses to a different CI-V controller (the aether-gate coexistence case #4840 §4 Q1 raises, and the same bus this PR's hardware run was on) reaches the block too.

Failure scenario: operator is spinning the VFO on an IC-705. A 05 write is on the wire; m_frequencyHz still holds the previous readback. An FA arrives that observe() reports Unmatched — the poll's frequency read completed a moment earlier, so lastCompletedKey == "frequency". The block fires: status-bar message claiming a refused tune that never happened, and a deferred sliceChanged that lands after the operator's optimistic value and drags the readout backwards. That is the same failure mode this PR exists to prevent, arriving from the other direction.

Fix shape: capture m_civScheduler.stats().inFlightKey before the observe() call at :1878, require observation == Observation::Accepted, and carry a backend flag for "a frequency write is outstanding" (set on the cmdSetFrequency path in setSliceFrequency(), cleared on completion), since the scheduler key alone cannot separate the write from the poll read or the transceive push.

2. The warning asserts an IC-9700 fact on every model — inline at IcomCivBackend.cpp:1937.

The text hardcodes "on this model a receiver cannot move to a band the other receiver already holds" while the comment four lines above correctly says the mechanism is model-independent. On an IC-705 or IC-7300MK2 — IcomModels.h:35 declares receivers = 1 for both — an FA on a frequency write (out-of-band, memory/scan mode, radio busy) tells the operator about a second receiver that does not exist. The new test asserts this message against *ic705, which is the tell. Suggestion inline gates the clause on m_model->receivers > 1.

Nits (non-blocking)

• The new test would pass unchanged against both defects above: it only exercises the matched-FA-completes-a-genuine-write path. Cases for (a) an FA with nothing in flight and lastCompletedKey left at "frequency" from a poll, and (b) an FA completing a frequency read, would both fail today and are cheap at this layer — prepareOutstandingFrequencyWrite() already builds most of the fixture.
• qCWarning says "radio refused the frequency write" unconditionally; once the predicate is tightened that will be true, but today it is the same claim as the operator-facing message.

What I tried to break, without success

• Is m_frequencyHz optimistic? No. Grepped every occurrence: one assignment, :2021, inside the kReadFreq/kSetFreqTrx decode. Restoring it cannot re-assert a client-invented value, so Constitution II holds — this is the client putting the display back where the radio is, not asserting state it owns.
• Does the deferred lambda outlive the backend? No. QTimer::singleShot(0, this, ...) uses the context overload, so a disconnect or destruction between the FA and the next event-loop turn drops it.
• Does the block break the frame dispatch below it? No. An FA carries cmd == 0xFA, which falls through noteControlSeen() and lands in the switch's default: exactly as before; placement above the switch changes nothing for other commands.
• Does the test fixture reach null state? No. prepareOutstandingFrequencyWrite() leaves m_session null, and pumpCiv() (:5730) guards on !m_session || !m_connected before dispatching. The takeNext() slot guard also passes because m_lastDispatchMs starts at 0.
• Is the test load-bearing? Yes for what it covers — delete the new block and published.size() == 1 fails, since nothing else emits sliceChanged on an FA.
• Sibling refusals left unfixed? setPanCenter() and setSliceMode() already carry their own re-assertion; CivCodec.h:77's isNg() still has no other backend caller, which matches the diff's stated narrowness.

Everything above is reasoned from the head checkout at /tmp/aetherclaude/pr-5373 — I have no build or test run here, so no claim is a runtime observation. CI is green on all four checks for 5df5585, but ctest in ci.yml is -R-filtered, so that does not tell us icom_incident_telemetry_test ran.

Recommendation

Request changes. The direction is right and the RFC backs it; the seam, the deferral and the restoration value all check out. Blocker 1 is the one that matters: the correlation is to "whatever finished last" rather than to this frame, and because the frequency poller keeps that latch set, the false-positive path is ordinary operation rather than a corner. Tightening it to inFlightKey + Accepted + an explicit write flag, then adding the two negative test cases, would make this mergeable — and would let the hardware evidence from the IC-9700 stand for what it actually proves.

Thanks for the unusually clear comments and for citing the measurement rather than asserting it.

---

_{🤖 aethersdr-agent · cost: $6.6617 · model: claude-opus-5}

[aethersdr-agent]

Blocker 1 — this tests "what finished last", not "this FA completed my tune".

lastCompletedKey is set in five places in IcomCivScheduler.cpp (:40, :172, :237, :247) and never cleared, so it is a latch rather than a property of frame. Two ways that breaks here:

1. Unmatched FA. With nothing in flight, observe() returns Unmatched (IcomCivScheduler.cpp:322-324) without touching the key, and onCivFrame only early-returns on Stale. The block still runs.
2. "frequency" is not a write. semanticKey() (:3243) collapses kReadFreq, kSetFreq and kSetFreqTrx onto one string, and queueControl(cmdReadFrequency(addr)) runs every even poll phase (:6107) — so the latch sits on "frequency" through normal operation. The comment above says "Only a frequency write is corrected here", but the key cannot carry that distinction.

onCivFrame also never filters frame.to, so an FA the radio addresses to another CI-V controller (the aether-gate coexistence case in #4840 §4 Q1) reaches this too.

Concrete failure: operator spinning the VFO on an IC-705, a 05 write on the wire, m_frequencyHz still the previous readback. An unmatched FA arrives shortly after a frequency poll completed → status bar claims a refusal that never happened, and the deferred sliceChanged lands after the operator's optimistic value and drags the readout backwards.

Suggested shape: capture m_civScheduler.stats().inFlightKey before the observe() call at :1878, require observation == Observation::Accepted, and add a backend flag set on the cmdSetFrequency path in setSliceFrequency() so a write is distinguishable from the poll read and the transceive push. No inline suggestion, because the fix spans code outside this hunk.

[aethersdr-agent]

Blocker 2 — this sentence is an IC-9700 fact stated to every model.

The comment at :1914 gets it right ("Correct on every model... whatever the reason"), but the operator-facing text hardcodes the dual-receiver explanation. IcomModels.h:35 declares receivers = 1 for the IC-705 and IC-7300MK2, where an FA on a frequency write (out-of-band, memory/scan mode, radio busy) would explain itself with a second receiver the radio does not have. The new test asserts exactly this message against *ic705.

Suggested change

	emit configurationWarning(
	tr("The radio refused the tune. It is still on %1 MHz — on this "
	"model a receiver cannot move to a band the other receiver "
	"already holds.")
	.arg(actualMhz, 0, 'f', 6));
	emit configurationWarning(
	m_model && m_model->receivers > 1
	? tr("The radio refused the tune. It is still on %1 MHz — on this "
	"model a receiver cannot move to a band the other receiver "
	"already holds.")
	.arg(actualMhz, 0, 'f', 6)
	: tr("The radio refused the tune. It is still on %1 MHz.")
	.arg(actualMhz, 0, 'f', 6));

[aethersdr-agent]

Non-blocking: this case only covers the matched-FA-completes-a-genuine-write path, so it passes unchanged against both blockers. Two cheap additions at this same layer would fail today:

• deliver the FA with nothing in flight after a frequency poll read has completed, and assert nothing is published and no warning is raised;
• let the outstanding request be cmdReadFrequency rather than cmdSetFrequency and assert the same.

prepareOutstandingFrequencyWrite() already builds most of the fixture — the second only needs the frame swapped, and the first only needs the takeNext() line dropped.

[ten9876]

Issue fit

No linked issue (references #4840); a clear-root-cause bug fix, which GOVERNANCE.md exempts from needing an RFC. The diagnosis is correct and well-evidenced: FA is CI-V's NG, observe() retired it identically to FB, isNg() had no backend caller, so a refused frequency write left the optimistic value standing — measured on hardware, six cross-band FAs the display followed. Re-asserting the radio-authoritative m_frequencyHz one event-loop turn later (matching the existing setSliceFrequency/setSliceMode deferral, for the same SliceModel-announces-first reason) is the right shape, and the test pins the load-bearing assertion (exactly one republish, and it's the held VFO). The deliberately-narrow scope (frequency only, where there's evidence and a known-good value) is the right call.

One correctness gap: the trigger reads the wrong signal for "a frequency write was just refused."

Scope

Two files, on-issue. CHANGELOG.md untouched. Preflight: no sockets, no fake peers.

Blockers

1. The FA-handler gates on the stale lastCompletedKey, not on whether this FA completed a frequency write (inline). observe() sets stats().lastCompletedKey only when the frame matches the in-flight transaction; an unmatched FA returns Observation::Unmatched and leaves lastCompletedKey at its prior value. Since frequency writes are the most common transaction, lastCompletedKey == "frequency" is usually true from the last real tune — so a later unmatched FA (a stray/duplicate NG, or an NG for a transaction that already expired) fires this block spuriously: a false "The radio refused the tune" toast plus a redundant re-assert, with no frequency write actually refused. That's the lying-indicator failure this PR exists to remove, inverted. The correct signal is already captured 55 lines up — const IcomCivScheduler::Observation observation = m_civScheduler.observe(...). Gate on observation == Observation::Accepted (this FA completed the in-flight tx) and lastCompletedKey == "frequency". The test drives an Accepted FA so it passes either way; add an unmatched-FA-with-stale-key row so the false-positive stays fixed.

Nits (non-blocking)

• The warning text hardcodes the IC-9700 cause ("on this model a receiver cannot move to a band the other receiver already holds"), but the block fires on every Icom model — an IC-705 (single receiver) refusing a frequency write for any other reason shows a dual-receiver explanation that's simply wrong for it. Either scope the model-specific sentence to the IC-9700 profile, or state the refusal generically and append the dual-receiver note only when the profile has two receivers.
• The singleShot(0) captures actualMhz at FA time; if the operator lands a successful tune in the one-turn gap, this deferred re-assert stomps it back. The existing setSliceFrequency deferral has the same shape, so this is pre-existing risk rather than new — but a generation/most-recent-intent guard on the deferred emit would close it for both.

What was verified vs read

• Verified by me in the PR head: that observe() only updates lastCompletedKey on a matched frame (scheduler lines 320-323 return Unmatched without touching it) while the FA-handler reads it unconditionally; that the Observation return is already captured at line 1865 and unused by the new block; that the warning text is model-agnostic in firing but IC-9700-specific in wording.
• Read, not re-run: the test's three assertions and the deferral rationale (CI is green, and the test covers the Accepted path).
• No bridge session — the demo can't emit CI-V FA; the fix's own socket-free scheduler test is the right vehicle, and the missing unmatched-FA row is where the blocker would be pinned.

[ten9876]

Blocker — this reads the wrong signal. observe() updates lastCompletedKey only on a matched frame; an unmatched FA returns Observation::Unmatched and leaves the key at its prior value — usually "frequency", since tunes dominate the transaction stream. So a stray/duplicate/expired-then-arriving FA fires a false "radio refused the tune" toast and a redundant re-assert with nothing actually refused. The right signal is already in hand at line 1865:

Suggested change

	if (frame.isNg() && m_civScheduler.stats().lastCompletedKey == "frequency"
	if (frame.isNg()
	&& observation == IcomCivScheduler::Observation::Accepted
	&& m_civScheduler.stats().lastCompletedKey == "frequency"
	&& m_frequencyHz != 0) {

Add an unmatched-FA-with-stale-lastCompletedKey test row so the false positive stays closed.

[nigelfenton]

Independent reproduction from a different entry point, plus two things the PR body doesn't currently mention. Both strengthen the case for the fix rather than changing its shape — the blocker @ten9876 raised stands on its own and I'm not arguing with it.

Found via the band panel, not a typed tune. Pressing 430 on an IC-9700 sitting on 144.200 USB changes the mode to FM and leaves the frequency at 144.200. That is a nastier symptom than a tune that simply doesn't move: selectBand()'s non-Flex branch sets mode first and frequency second (MainWindow_Wiring.cpp:3383), so the mode write succeeds while the frequency write is NAKed and swallowed. You end up on the old band's frequency in the new band's mode — 144.200, the 2 m SSB calling frequency, in FM with a ±5 kHz filter. The give-away was the audio changing, not the display.

The automation bridge reports success too. automation_probe tune 432.100 returns ok: true, tune: 432.1 with the radio unmoved, and rigctl F 432100000 returns RPRT 0. So scripted callers get the same false success as the display. One root cause, three surfaces.

Captured on hardware today, 2026-09-01, IC-9700 at 10.0.0.8:

100ms tx  fe fe a2 e0 05 00 00 10 32 04 fd    set 432.100
 83ms rx  fa                                   refused

(trace is newest-first; the fa at 83 ms answers the 05 at 100 ms)

Control: in-band tunes work (144.200 → 144.500 ✅), and 23 cm tunes work — only 430 is refused, because Sub was holding it. Exactly the mechanism in the PR body.

A measurement note for anyone else trying to reproduce this. civ trace's ring holds ~116 frames spanning about 3.2 seconds, continuously refilled by routine polling (16 ×23, 14 ×8 per sweep). Any capture taken after a sleep has already rotated the 05 and its FA out, and the trace then looks like no frequency command was ever sent. I drew exactly that wrong conclusion before checking the ring's span. Capture immediately after the tune, in the same script, or you will measure the absence of your own evidence.

On the blocker: agreed, and the unmatched-FA-with-stale-key row is the right test to pin it. The nit about the warning text hardcoding the dual-receiver explanation also matters more than it might look — the band-panel path above would show that sentence on an IC-705, where it is simply untrue.

[nigelfenton]

Both addressed in 30bb4fa7. Thank you — the blocker is real and the nit turned out to matter more than it looked.

Blocker: gated on the observation, not the stale key.

You were right that lastCompletedKey is the wrong signal. I verified it in the scheduler rather than taking it: all three return Observation::Unmatched sites (IcomCivScheduler.cpp:323, :349, :352) return without touching m_stats.lastCompletedKey, so an unmatched FA inherits whatever the last matched transaction left there — and since frequency writes dominate, that is usually "frequency". A stray or duplicate NG then fired a refusal toast and a redundant re-assert with nothing actually refused.

The predicate now requires observation == Observation::Accepted and the key. As you said, the Observation was already captured 55 lines up and unused.

Nit: the warning text, which is reachable in ordinary use.

Worth more than a nit. That sentence fired on every Icom model, and I found the path where it bites: #4840's band panel reaches this same warning, so an IC-705 operator pressing a band button gets a dual-receiver explanation for a radio that has one receiver. The refusal is now stated generically, with the cause appended only when m_model->receivers > 1.

Test: the unmatched-FA row you asked for.

Delivers two NGs to the same backend. The first matches the in-flight write and must still correct exactly once; the second arrives with nothing in flight and must publish nothing and warn nobody. It asserts the stale key really does still read "frequency" in between, so it pins the actual mechanism rather than a proxy for it.

Verified by mutation rather than a green re-run: removing the Accepted gate fails both new assertions and passes every pre-existing one — which is exactly why it needed its own row, since the Accepted path passes with or without the fix.

FAIL  an unmatched FA republishes NOTHING (no redundant re-assert)
FAIL  an unmatched FA does not tell the operator a tune was refused

Test is 14/14 with the fix in place. check_engine_boundary --strict and check_test_registration --strict both clean, and all four CI jobs are green on 30bb4fa7.

Your second nit — the singleShot(0) generation guard — I have deliberately not done. You noted it is pre-existing shape shared with setSliceFrequency's deferral rather than new risk here. Fixing it in this PR would mean changing that path too, which is scope this PR should not take. Happy to do it as a follow-up if you want it; it seems like the kind of thing that wants doing once for both sites rather than half here.

[nigelfenton]

Independent reproduction, 2026-09-04 — different entry point, and a wire capture of the rigctl half

Reproduced on the IC-9700 over RS-BA1 tonight while settling an unrelated question (whether the radio supports full duplex for satellite work). Worth adding because it arrived from a different direction than the 2026-08-29 session in the PR body: not a deliberate cross-band probe, but an ordinary satellite band change — Main sitting on 2 m, tune the downlink to 435.640.

The wire, captured via civ trace in the same script as the action (the ring is only ~3.2 s, so a later grab reads as "no command sent"):

tx  fe fe a2 e0 05 00 00 64 35 04 fd     <- set 435.640000 MHz
rx  fa                                    <- radio REFUSES

and the client was told:

rigctl: F 435640000  ->  RPRT 0
rigctl: f            ->  145918304       (unmoved)

So: the radio said no, the return code said yes, and the frequency never moved. Baseline before and after was identical — 145.918304 MHz, FM, VFOA, split off.

What this adds to what's already in the body

The PR already documents the six-refusal session and already flags the rigctl return code as a known second bug. Two things are new here:

1. A second, independent entry point. The 08-29 session set out to provoke FA across bands. This one hit it incidentally, doing something an operator would plausibly do — retuning to a satellite downlink on a band the other receiver holds. It is reachable in ordinary use, not just under deliberate probing.

2. RPRT 0 is now captured on the wire rather than asserted in prose. The "What this does NOT fix" section states that RigctlProtocol::cmdSetFreq answers synchronously and cannot observe the NAK. The capture above is that claim measured end to end: the FA and the RPRT 0 in the same event, with a readback proving the radio stayed put.

This is evidence for the existing diagnosis, not a request for scope change. The rigctl return code remains out of scope for this PR, exactly as the body says.

Limits

• This exercised the refusal path only. The user-facing warning string this PR adds was not observed — I was running main's build (AetherSDR.exe from build/), not this branch, so the correction and message are still proven by the unit test rather than by me on hardware.
• Nothing was keyed. rigctl t read 0 (RX) throughout and again immediately before shutdown; the whole session was receive-only.

🤖 Generated with Claude Code