fix(rit): carry the slice id through the RIT/XIT seam; HL2 holds them per receiver. Principle I.

[ten9876]

Fixes #6105

Decision

Jeremy's ruling on #6012 (approving review, 2026-10-02): "RIT/XIT stay radio-wide and follow transmit, as implemented … Per-slice RIT/XIT through the seam is filed as #6105." For #6105 he chose option C, a per-slice seam modelled on FlexLib. FlexLib is the protocol authority (Principle I). Its Slice holds RITOn / RITFreq / XITOn / XITFreq per slice and sends them as slice set <n> rit_on= / rit_freq= / xit_on= / xit_freq= (reference/FlexLib_API_v4.1.5.39794/FlexLib/Slice.cs:1487-1546). Each slice stores its own XIT, and the radio applies it when that slice transmits.

Root cause

RadioModel.cpp forwarded SliceModel::ritCommandIssued and xitCommandIssued to four IRadioBackend verbs that took no slice id. SliceModel keeps RIT/XIT per slice, but Hl2Backend held a single radio-wide value:

• RIT was applied only to the transmit receiver (rxTunedHz(), gated on ownsTx).
• XIT was added in setTxFrequency().

On a multi-receiver HL2 or Radioberry this caused two bugs:

1. Overwrite. RIT or XIT changed on any slice overwrote the value for every slice.
2. XIT on handoff. setTxSlice() re-wrote the TX register with the previous slice's XIT, while the new transmit slice showed XIT off.

Fix

• Seam: setSliceRitEnabled, setSliceRitOffset, setSliceXitEnabled and setSliceXitOffset now take (int sliceId, …). The XIT-offset default still forwards to the RIT offset, for Icom's single 0x21 register. The RadioModel lambdas pass s->sliceId(). They are connected only for non-Flex backends, so Flex keeps its existing per-slice slice set path; the command text is unchanged and pinned by a test.
• HL2: RIT and XIT now live on each Receiver, resolved via m_ids.byUi.
  • rxTunedHz() no longer has the ownsTx gate. Each receiver applies its own RIT on its own NCO.
  • setTxFrequency() reads the XIT of rx(m_txDdc). A handoff, or closing the transmit receiver, therefore picks up the new owner's XIT with no re-send.
  • The RIT re-tune in setTxSlice() and logRitScope() are gone; they only existed because the value was radio-wide. Each setter instead logs a qCInfo line naming the DDC, the slice, and whether that receiver transmits. It checks the receiver exists before clamping.
  • buildReceivers(): a receiver that a (re)connect adds is seeded from receiver 0, as before, but with RIT/XIT off and no RIT-moved-NCO flag, as createPanadapter()'s seed already is. Receivers that existed keep their own RIT/XIT across a reconnect, and emitSliceState() publishes them.
• Icom: it ignores the id (one slice, always the TX slice). The three ControlSpec seam-verb names are renamed to match.
• HL2 readback (the optional item) is done. emitSliceState() now publishes ritOn / ritFreq / xitOn / xitFreq, and each setter ends by publishing them. The readout therefore shows what the receiver holds, including a clamped offset and the zero state after a fresh connect. To make this clean:
  • SliceModel::applyChanges() guards RIT/XIT the way it already guards the AGC pair, so the echo that comes with every HL2 tune emits nothing.
  • setRit() / setXit() announce once, with the members rather than the arguments. While the command is being emitted, a synchronous backend's per-verb publishes are adopted without announcing them. So an enable-then-offset pair never shows the old offset in between, and a correction (HL2's ±9999 Hz clamp) is the value announced. This has no effect on Flex, where nothing is echoed synchronously.

Validation

• hl2_rit_xit_test, 58/58. The 6 checks that asserted "RIT follows transmit" now assert per-receiver behaviour:

  • after a TX move, receiver 1 does not pick up receiver 0's RIT, and receiver 0 keeps its own;
  • RIT on the non-transmit receiver moves its own NCO;
  • after the transmit receiver closes, DDC 0 keeps its RIT.

  New cases:

  • RIT on a non-transmit receiver shifts only that receiver;
  • turning it off leaves the transmit receiver's RIT alone;
  • XIT on a non-transmit receiver leaves the TX register alone;
  • handoff regression: TX moves to a receiver with XIT off and the TX register is exactly that receiver's dial; it then moves back and the old XIT applies again;
  • published readback of the offset and of the clamp;
  • closing receiver 0 renumbers receiver 1 to DDC 0, and it keeps its own RIT;
  • a receiver that buildReceivers() adds starts with RIT/XIT off and without receiver 0's RIT-moved-NCO flag, while receiver 0 keeps its own.

• New slice_rit_xit_seam_test, 16/16 (socket-free, with a recording backend):

  • RIT on slice 1 reaches the seam as slice 1, and XIT on slice 0 as slice 0;
  • the Flex slice set <n> rit_on=… rit_freq=… and xit_* text is unchanged;
  • an unchanged echo emits nothing;
  • a corrected echo is adopted;
  • setRit() ends on the backend's clamped value;
  • a backend that publishes from the enable verb (the old offset), then from the offset verb, yields exactly one ritChanged(true, 500) / xitChanged(true, -250);
  • Flex shape, on a backend-less SliceModel (not RadioModel + FlexBackend): setRit sends the slice set text and announces once, a matching status echo emits nothing, and a status carrying only rit_freq keeps rit_on.

• hl2_tx_gate_test, the case added with feat(tx): apply TUNE power live while TUNE is keyed (HL2, Icom). Principle VI. #6104's transmit tail. XIT composes with TUNE and the tail:

  • An XIT change while TUNE is keyed moves only the TX register: still keyed, no drive bank written.
  • Inside the tail, XIT moves the TX register, and the tail stays pending with the RF drive still owed. When it ends, the drive is restored.
  • Mutation: an XIT offset that ends the tail turns 2 checks red.

• Mutation checks (each mutation was reverted, and the file's sha256 matched afterwards):

  Mutation	Checks that went red
  Re-gate RIT to the TX receiver	3
  TX XIT read from DDC 0 instead of m_txDdc	5, including the handoff regression
  Drop the XIT readback	2
  Lambda passes slice 0 instead of s->sliceId()	1
  Unguarded applyChanges	2
  setRit() announces its arguments	1
  RIT in-flight guard removed from applyChanges	1
  XIT in-flight guard removed from applyChanges	1
  buildReceivers() seed keeps receiver 0's RIT/XIT	1
  buildReceivers() seed keeps the RIT-moved-NCO flag	1

• ctest: 522/522 passed with --no-tests=error. This is the full set of 621 minus 99 excluded tests: those whose sources use socket APIs, the socket-owning families, and connection_panel_size_test (Four tests fail deterministically on clean main locally: the a11y announcement fixture never receives focus (offscreen) #5811). The two RIT tests matched that grep only on the word connectRadio and were run directly, 58/58 and 16/16.

• Static gates: check_engine_boundary --strict (0 blocking), gen_touchpoint_manifest --check (up to date), check_capability_records --strict (71, at the frozen 71), check_command_plane --strict (401 = baseline). No ratchet grew.

• Agent automation bridge, built-in demo only. Offscreen, isolated settings, AETHER_AUTOMATION_NO_TX=1, never keyed. get radio confirmed the radio was AetherSDR Demo / DEMO-0001.

  • Used invoke on the RX applet's RIT toggle / RIT increase ×2 / XIT toggle / XIT decrease.
  • get slice 0 went from ritOn=false ritFreq=0 xitOn=false xitFreq=0 to ritOn=true ritFreq=20 xitOn=true xitFreq=-10.
  • After zero + off it returned to all off/zero, with transmitting=false throughout.

• Not verified:

  • Only a sanity pass on the demo. It runs SimBackend, which is a seam backend, not Flex; the Flex command text is covered by the unit test.
  • No HL2 or Icom hardware run. HL2 multi-receiver behaviour is proven only by hl2_rit_xit_test against the constructed backend.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AADGw3HVn5FMMVrx2RQZ9g

[aethersdr-agent]

Issue fit

Yes, on the code. #6105's structural defect — RadioModel capturing s but forwarding [this] and dropping the slice id — is closed: both lambdas now capture s and pass s->sliceId(), and all four IRadioBackend verbs carry it. Consequence 1 (cross-receiver overwrite) is closed by moving ritOn/ritHz/xitOn/xitHz onto Receiver and dropping the ownsTx gate in rxTunedHz(). Consequence 2 (XIT readout vs TX register after a handoff) is closed by setTxFrequency() reading rx(m_txDdc)->xitOn/xitHz instead of a radio-wide member — every caller (tune 2842, handoff 3162, calibration 4513, pushInitialState 7029) and removePanadapter's retuneReceiver(m_txDdc) therefore pick up the new owner's XIT with no re-send. The optional readback item is done too.

The FlexLib citation holds: Slice.cs:1482-1553 does keep RITOn/RITFreq/XITOn/XITFreq per slice and sends slice set <n> rit_on=/rit_freq=/xit_on=/xit_freq= per slice. Principle I framing is accurate.

My one reservation is governance, not code — see "Needs maintainer decision".

Scope

File(s)	What it changes	Claimed?	Verdict
IRadioBackend.h	Four verbs gain int sliceId; XIT-offset default still forwards to the RIT offset	Yes	In scope
hl2/Hl2Backend.{h,cpp}	RIT/XIT move to Receiver; ownsTx gate dropped; setTxFrequency reads the TX owner's XIT; setTxSlice RIT block and logRitScope() removed; clampRitXit() extracted; emitSliceState publishes the four fields	Yes	In scope
icom/IcomCivBackend.{h,cpp}	Renamed overrides, Q_UNUSED(sliceId), scrubDrive passes sliceId()	Yes	In scope
icom/IcomControls.cpp	Three seamVerb strings renamed	Yes	In scope — report-only metadata (ControlSpec::seamVerb, IcomControls.h:77-80); no gate or doc pins these
models/RadioModel.cpp	Lambdas capture s, pass s->sliceId()	Yes	In scope
models/SliceModel.cpp	setRit/setXit emit the members; applyChanges guards the RIT/XIT pairs	Yes — body's readback section	In scope
tests/hl2_rit_xit_test.cpp, tests/slice_rit_xit_seam_test.cpp, tests/tests.cmake	Rewritten/new coverage, registration	Yes	In scope

No unrelated files, no new public surface beyond the seam the issue asked for (verb signatures change; no new protocol verb, settings key or capability field), no CHANGELOG.md entry, no socket test added or removed (slice_rit_xit_seam_test injects a RecordingBackend through setBackendForTest — no QTcpServer/QUdpSocket/bind/listen/connectToHost, and no fake radio peer).

On the - lines specifically: the deleted setTxSlice() RIT re-tune block and logRitScope() are the #6012 "RIT follows transmit" behaviour this change supersedes, not a lost guard — nothing re-asserts a value that no longer exists. The deleted comment "RIT belongs to the transmit receiver, so it leaves one and joins the other" names behaviour, not a symptom, and the six hl2_rit_xit_test checks that asserted it are correctly inverted rather than dropped. The ncoMovedForRit comment losing "or transmit moved away" is right for the same reason; the flag itself is per-Receiver already.

No RFC gap: GOVERNANCE.md:111-112 exempts "Bug fixes with a clear root cause" and "Protocol compliance fixes matching SmartSDR behavior".

Blockers

None. I could not construct a case where the final state is wrong.

Needs maintainer decision

1. The fix shape was #6105's open question, and I can't find the ruling that settled it on C. #6105 is still state: open with the maintainer-review label, and its only thread comment is the agent triage, which recommends option B, floats a third shape (slice id on the two RIT verbs only, XIT TX-gated, "a non-TX slice's XIT is meaningless everywhere"), and closes with "Not eligible for implementation until the fix shape is chosen and aetherclaude-eligible is added." The body's quoted #6012 ruling ("RIT/XIT stay radio-wide and follow transmit … Per-slice RIT/XIT through the seam is filed as #6105") is not in #6012's comment thread either — it may well be in a PR review body, which list_issue_comments doesn't return, so I'm reporting this as unlocated, not as absent. The second half — that option C was chosen for #6105 — I could not find anywhere.

   Three reasons this is worth a deliberate ruling rather than a shrug: the triage argued B was cheaper because RadioModel.cpp sits under #5554 §2.6 / #5262 M4 and C's signature change is "itself M4-shaped work"; GOVERNANCE.md:62-63 is explicit that "Changes that touch src/gui/, src/core/, or src/models/ always require maintainer review regardless of domain"; and C has a real advantage over B the triage itself identified, which this PR should get credit for — under B, CAT/TCI/rigctl writes to a non-TX slice (TciProtocol.cpp, SmartCatProtocol.cpp:924/937, RigctlProtocol.cpp) would still update SliceModel and light the readout while nothing reached the radio. C closes that; B leaves it. So I'd rule for C on the merits. It just needs to be the maintainer's recorded call, and ideally a comment on #6105 plus the label.

   Worth noting in C's favour against the triage's third shape: this PR stores XIT per receiver but only ever lets the TX owner's reach the single TX register, so it already honours "one transmitter" without making a non-TX slice's XIT do anything.

Nits (non-blocking)

• Enable-then-offset publishes a stale offset mid-flight (RadioModel.cpp:1650). Final state is correct on every path I walked, but the intermediate ritChanged is new and uncovered. Detail inline.
• clampRitXit() runs before the !r null check (Hl2Backend.cpp:4416, :4447) — an offset aimed at a slice that isn't running logs a clamp warning for a receiver that doesn't exist.
• kRitXitMaxHz = 9999 vs FlexLib's ±99999 (Hl2Backend.h:588). Pre-existing from #6012, but this PR makes it operator-visible for the first time via the readback, and the PR's own authority disagrees with it. Detail inline.
• logRitScope()'s removal leaves no RIT trail in a support bundle (Hl2Backend.h:584). It was added deliberately in response to #6012 review item 3. Detail inline.
• One test check doesn't prove what its message says (hl2_rit_xit_test.cpp:203) — the trailing setTxSlice(1) is the already-ours early-return branch. The real handoff regression at :209-211 is sound.

What I tried to break, and failed

• Stale verb names and un-overridden siblings. Grepped the whole head checkout (not just src/): setRitEnabled|setRitOffset|setXitEnabled|setXitOffset appear in exactly zero files. No docs/, tools/, scripts/ or resources/ reference; docs/automation-bridge.md:4149's rit.offset row carries no seamVerb, so it isn't stale; icom_meters_test.cpp:568 pins a different row (setSliceFmDtcs); hal-plugin/ has no IRadioBackend subclass. Because all four are override, a missed sibling would be a compile error, and check-macos/check-windows/build/Static checks are all green (7/7 on 2ded146).
• The second receiver and DDC compaction — the case I expected to find a bug in. removePanadapter does m_rx.erase(m_rx.begin() + ddc) and m_ids.remove(ddc) renumbers DDC indices while UI numbers stay fixed. Because the new fields live inside Receiver, they shift with the vector element, so closing receiver 0 does not hand receiver 1's RIT to the wrong slice. Putting them on Receiver rather than in a side map keyed by DDC is what makes that fall out for free — it's strictly better than the old radio-wide members.
• A readback feedback loop. Every ritChanged/xitChanged consumer is display-only and already signal-blocked: VfoWidget.cpp:4935/4942 and RxApplet.cpp:2768/2780 set labels under QSignalBlocker, MainWindow.cpp:8510-8536 refreshes Stream Deck / RC-28 / TMATE2 labels, MainWindow_Wiring.cpp:2184 repaints the overlay, TciServer.cpp:2985-2986 broadcasts a flag read back from the model. Nothing calls setRit/setXit from them, and applyChanges emits only the status signals — so the new per-tune publish cannot loop. The applyChanges guard strictly reduces emissions versus main, including for Flex.
• emitSliceState zeroing a restored RIT on connect. No path to zero: grep -i rit|xit over RestoredRadioState.h is empty and there is no RIT/XIT settings key, so there's no persisted value for the fresh-connect (false, 0) publish to clobber.
• The clamp early-return swallowing the correction. if (!r || (clamped == r->ritHz && clamped == hz)) looked like a candidate — ask for 20000 twice and the second call finds clamped == r->ritHz. The second conjunct saves it: when the clamp bites, clamped != hz, so it never returns and always republishes 9999. Correct, and deliberately so.
• retuneReceiver() not publishing. Verified the chain: retuneReceiver → setSliceFrequency(ids->uiNumber, ...) → emitSliceState(ddc) at the tail. The "retuneReceiver() publishes as part of the re-tune" comment is accurate.
• The body's "connected only for non-Flex backends". wireBackendReceiverState() does run for every backend and FlexBackend does emit sliceChanged, so I expected this to be wrong — but the slice-creation block the lambdas live in is behind if (!s && !m_flexBackend) at RadioModel.cpp:1541. Claim holds.
• Test-layer policy and the CMake contract. Both tests are socket-free model/backend-table tests, the correct layer. slice_rit_xit_seam_test links aethercore Qt6::Core matching the atu_seam_gate_test/control_receive_test precedent, uses TestSettingsProfile, is registered with add_test, and is in AETHER_SETTINGS_CONSUMERS. Neither target compiles AppSettings.cpp, so ${AETHER_SETTINGS_SOURCES} is correctly absent.

Could not check: nothing was built or run — every finding above is reasoned from the head checkout at /tmp/aetherclaude/pr-6109, not reproduced at runtime. I cannot confirm the body's 587/587 ctest run, the four static gates, the mutation table, or the automation-bridge demo pass. The HL2 and Icom hardware gap the body already declares stands. And I cannot read PR review bodies, only issue comments, which is the limit on the ruling check above.

Recommendation

Needs maintainer decision — on the fix shape only, not the code. The implementation is the most careful thing I've reviewed on this seam: the Receiver-struct placement makes DDC compaction correct for free, the clamp republish is right in the one case it would have been easy to get wrong, the six inverted test checks are inverted rather than deleted, and the mutation table names the exact checks I'd have asked for. If the option-C ruling exists, this is Approve-with-nits and the nits are all small. Concrete next step: point at the ruling (or get one recorded on #6105 with the label), then address the five nits — the one worth a line of code is the mid-flight stale-offset publish.

Thanks for the unusually honest "Not verified" section; it's what let me spend the review on the parts that actually needed attacking.

---

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

[aethersdr-agent]

Antares Detector — candidate vulnerable file(s)

• src/core/KiwiSdrManager.cpp

The function assignedProfileForSlice(int sliceId) const directly returns m_sliceAssignments.value(sliceId), which blindly returns a value from a map without any validation. This is a direct use of untrusted sliceId as an index into a map, which can lead to path traversal or out-of-bounds access if the map is manipulated by external input.

_{Localized by Cisco Foundation AI Antares-1B running locally in the AetherClaude sandbox, seeded by the Cartographer security map. Advisory only — please verify before acting.}

---

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

[ten9876]

Re the Antares detector note: src/core/KiwiSdrManager.cpp isn't in this diff, which touches only the RIT/XIT seam, the HL2/Icom backends, RadioModel/SliceModel and two tests. Nothing in this PR needs to change for it. Thread sweep: all 6 review threads were answered and resolved at 8199c1d97. The option-C ruling is recorded on #6105.

[ten9876]

Admin-merged on the maintainer's approval (self-authored PR). Head e59b8ddab:

• Per-slice RIT/XIT through the seam (option C, modelled on FlexLib Slice.cs).
• 6 of 6 review threads resolved.
• Socket-free ctest on the merged tree passes 541/541, including hl2_rit_xit_test (58), slice_rit_xit_seam_test, hl2_tx_gate_test (now with XIT × TUNE × the 70 ms transmit tail) and hl2_backend_seam_test.
• CI green. Drift since the run: test: drop two host-dependent assumptions that fail on macOS. Principle VIII. #6134 and fix(amp): size the rail's fan pull-down to its widest caption. Principle XI. #6133, which touch none of this PR's files and merge cleanly.