Repository navigation
fix(hl2): refuse a mode the radio cannot demodulate. Principle VIII. - #6007
Conversation
Hl2Backend::setSliceMode stored any string it did not know verbatim, so a slice could READ "RTTY" (or DFM, DSTR, any string) while modeFromString() fell through to USB: indicator and detector disagreeing, nothing logged. aethersdr#5755 closed the combo; every other route still ended at this setter -- rigctl set_mode, SmartCAT ZZMD, TCI modulation, memory recall, the Mode shortcuts, MIDI, the VFO quick-mode buttons, the net scheduler, a spot click -- because SliceModel::setMode forwards whatever it is given. The setter now holds the boundary applyRestoredState() already holds: an unknown mode is refused with a warning naming the modes the radio does demodulate, and the slice is re-published unchanged, which moves the optimistic SliceModel mode back to what the DSP is doing. Kept inside the HL2 backend. A SliceModel-level guard (aethersdr#5580's bullet 4) would change every family's behaviour and is a maintainer's call. Tests: hl2_mode_vocabulary_test drives setSliceMode with RTTY, DFM, DSTR and a nonsense string and requires the mode and passband unchanged (red on origin/main: each read back as itself with the USB passband 150..3000), plus a positive control that every accepted spelling is still applied. control_receive_test's assertion that an unknown string ("RADE") comes back verbatim pinned the old behaviour; it now requires the string to be refused and the slice to stay in its previous mode. "RADE" is client-side only and never reaches a backend in production. Refs: aethersdr#5580, aethersdr#5678
There was a problem hiding this comment.
1. Issue fit
Partially. The refusal closes the residual @on8st recorded on #5580 (2026-09-20): "rigctl set_mode RTTY against an HL2 today leaves the slice reading RTTY with the DSP in USB." I verified the premise holds on current main — knownModeStrings() (Hl2ModeVocabulary.h:113-121) is exactly the set modeFromString() maps (Hl2Backend.cpp:189-212), so !isKnownModeString() is precisely the USB-fallthrough set. The guard is neither wider nor narrower than the hole. Keeping it in the backend instead of SliceModel correctly sidesteps the cross-backend blast radius the triage bot flagged as needing a maintainer ruling (RTL-SDR and Icom also publish modeList).
What it does not close: the refusal reverts the model, but the SliceModel::setMode() emission that follows it still announces the refused mode to every modeChanged subscriber — see Blocker 1. That is the same indicator-lies shape, relocated from the model to the signal.
2. Scope
| File | What it changes | Claimed by title/body? | Verdict |
|---|---|---|---|
src/core/backends/hl2/Hl2Backend.cpp |
Refuses an unmapped mode string in setSliceMode(); re-publishes slice state; qCWarning |
Yes | In scope |
tests/control_receive_test.cpp |
Flips the "RADE passes through verbatim" assertion to "refused, stays FM" | Yes — direct consequence | In scope |
tests/hl2_mode_vocabulary_test.cpp |
New run-time refusal block + positive control | Yes | In scope |
Everything in the diff is explained by the issue. No new public/protocol surface, no settings keys, no CHANGELOG.md edit, no deleted guard. Both touched tests are already registered (tests/tests.cmake:390-393, :6138-6141) and both appear in the aggregate lists at :6687 / :6693. No sockets: Hl2Backend's constructor does m_rx.assign(1, Receiver{}) and binds nothing, so the new block is socket-free — nothing to surface under the socket-test reporting rule.
3. Blockers
1. The refused mode is still broadcast to every modeChanged subscriber, after the correction — and VfoWidget paints it. (inline on Hl2Backend.cpp:3636)
Hl2Backend is constructed in-process and never moved to a thread (RadioModel.cpp:778), and both relevant connections are default AutoConnection (RadioModel.cpp:1820-1823, :1766). So emitSliceState(ddc) runs synchronously, re-entrantly, inside SliceModel::setMode():
SliceModel::setMode("RTTY")
SliceModel.cpp:187 m_mode = "RTTY"
SliceModel.cpp:190 emit modeChangeRequested("RTTY")
└─ Hl2Backend::setSliceMode → refuse → emitSliceState(ddc)
└─ RadioModel.cpp:1944 s->applyChanges(mapped) // mode = "LSB"
└─ SliceModel.cpp:1276 m_mode = "LSB"; modeChanged_ = true
└─ SliceModel.cpp:1750 emit modeChanged("LSB") ← the correction
SliceModel.cpp:191 emit modeChanged("RTTY") ← fires LAST
VfoWidget's handler reads the signal argument, not m_slice->mode():
// VfoWidget.cpp:4602-4607
connect(m_slice, &SliceModel::modeChanged, this, [this](const QString& mode) {
m_tabBtns[2]->setText(mode); // ← "RTTY"
updateModeTab();
bool isRtty = (mode == "RTTY"); // ← true; RTTY DSP controls shownSo after rigctl set_mode RTTY on an HL2 the mode tab reads "RTTY" with the RTTY control row visible, while m_mode, the combo (findText misses, stays put) and the DSP are all LSB. The PR's own body says the delta "moves every indicator back to what the DSP is doing" — it moves them back and then moves one of them forward again.
This does not fire on the accepted path: an accepted mode comes back unchanged, applyChanges sees m_mode == m, modeChanged_ stays false, and no nested emit occurs. It is new behaviour introduced by the refusal.
Two shapes of fix, both a maintainer's call on where the seam goes:
- Defer the re-publish (
QMetaObject::invokeMethod(this, "emitSliceState", Qt::QueuedConnection, Q_ARG(int, ddc))) so the correction lands aftersetMode()finishes emitting — smallest change, keeps the fix inside the backend as intended. - Or make
VfoWidget's handler readm_slice->mode()rather than the argument — but that touches a shared widget and leaves the ordering wrong for any future subscriber.
1b. Neither new test can see this (inline on hl2_mode_vocabulary_test.cpp:271). Both observe the raw SliceDelta off IRadioBackend::sliceChanged with no SliceModel in the loop, so they assert the backend half and are structurally blind to the half that actually reaches the operator. A case that drives SliceModel::setMode() and asserts the final modeChanged argument equals mode() would pin it — socket-free, deterministic, and the right layer per the AGENTS.md test-layer boundary ("refusals … non-events" → state-machine injection).
4. Nits (non-blocking)
- The refusal is invisible to the requesting client.
rigctl set_mode RTTYstill getsRPRT 0; the only signal is aqCWarningand a laterget_modereturning something else. That is strictly better than silently demodulating USB, and plumbing a refusal back throughSliceModel::setMode→ rigctl is exactly the cross-backend change this PR deliberately avoids. Worth one line in the body naming it as a known boundary rather than leaving a reader to infer it. - Memory recall still applies the stored filter over the unchanged mode. The #5580 triage raised this as item (c). I could not confirm the call site —
recallMemorydoes not exist in the current tree (recallCachedMemory,RadioModel.cpp:9317, goes throughapplyMemoryRecallDetails), so the specific mismatch the bot described may already be gone. Flagging it as unverified rather than clean: if the memory editor still offers RTTY/DFM/DSTR (the comment saysMemoryFieldValues::modes()does), recalling one onto an HL2 is worth a manual check. - The positive-control loop walks
knownModeStrings(), which includesWBFMandDRM— modesHl2ModeVocabulary.hitself documents as accepted-but-undisplayable. AssertinglastMode == canonicalOfferedMode(m)for those is the implementation restating itself, but it is the documented contract and it does pin that the refusal didn't widen. Fine as-is.
5. What I tried to break
- Is the refusal wider than the hole? No. I diffed
knownModeStrings()(Hl2ModeVocabulary.h:113-121) against every branch ofmodeFromString()(Hl2Backend.cpp:189-212) — the sets are identical, including theCW/CWU,FM/NFM,WBFM/WFMalias pairs. Nothing that previously demodulated correctly is now refused. TheCWLconcern the #5580 triage raised (TCIcwr, CATMD03) does not bite:CWLis both known and published. - Null/ordering safety of the early return.
ddcandrare both resolved above the guard (Hl2Backend.cpp:3583-3586), andemitSliceStatere-null-checksrx(ddc)andm_ids.byDdc(ddc)(:9485-9488), so the early return cannot publish against a dead receiver. - Does the delta actually carry a mode? Yes —
emitSliceStatesetsd.mode = r->modeunconditionally (:9508), so the re-publish is never a no-op delta the model would ignore. - Would the tests pass against unfixed code? No. Unfixed,
setSliceMode(0,"RTTY")stores "RTTY" and the delta carries it, solastMode == baseModefails; the flippedcontrol_receive_testassertion fails the same way. Both are genuine regression tests for the backend half. - Digital-voice registry leak on refusal.
setModeclaims the slice inDigitalVoiceModeRegistrybefore emitting the request. I walkedapplyChanges' incoming-mode block (SliceModel.cpp:1236-1278) — the corrective delta hits theelse if (previousMode.has_value())branch and callsreleaseSlice, so a refused FDV-family request does not strand a claim. - The test comment's RADE claim. Verified:
VfoWidget.cpp:2629-2632returns beforem_slice->setMode(mode)for "RADE". Accurate as written. - CI. All five checks green on
2d8395f(build, check-macos, check-windows, Static checks, Sanitizer configures). Green here means the two touched tests compile and pass their backend-level assertions — it says nothing about Blocker 1, which no registered test covers.
Everything above is reasoned from reading the code, not reproduced at runtime — I have no build and no way to drive the GUI. The re-entrancy chain in Blocker 1 is traced through four files by call graph and connection type; I could not observe the emission order empirically. The `` head checkout was empty and pull/6007/head is not fetchable from the remotes I have, so I read the base tree plus the full patch rather than the merged result — meaning I cannot rule out unmerged changes elsewhere on the branch, though `list_pr_files` shows only these three.
6. Recommendation
Request changes — on Blocker 1 only. The framing is right, the seam choice is right (backend-local, maintainer's ruling respected), and the guard is exactly as wide as the defect. But the fix as written substitutes one indicator/DSP disagreement for another: the operator ends up looking at a mode tab that says RTTY with RTTY controls on screen while the radio is in LSB, which is the fault #5580 exists to remove. Concrete next step: defer emitSliceState(ddc) to a queued invocation so the correction lands after SliceModel::setMode() has finished emitting, and add a SliceModel-level case asserting the final modeChanged argument matches mode() after a refused request. Nice work tracing the vocabulary equivalence — the containment argument in the header made this one genuinely fast to verify.
🤖 aethersdr-agent · cost: $10.3777 · model: claude-opus-5
…VIII. Review on aethersdr#6007: Hl2Backend's refusal runs synchronously inside SliceModel::setMode() -- modeChangeRequested and sliceChanged are both direct connections -- so applyChanges() had already set m_mode back and announced the real mode, and then setMode() emitted modeChanged(mode) with the REFUSED mode. VfoWidget paints the mode tab from that argument: after rigctl set_mode RTTY the tab read RTTY with the DSP in LSB. setMode() now announces the requested mode only if m_mode still holds it after the request. Family-neutral: a backend that does not answer synchronously leaves m_mode == mode and nothing changes. Why not defer the HL2 re-publish instead (the review's first shape): setMode() normalises the filter polarity against m_mode after its emits. Deferred, m_mode would still read the refused mode there, and FDV -- refused by the HL2, USB-family in filterPolarityUsbFamily() -- would flip an LSB passband to USB polarity and send it to the radio. Test: hl2_mode_vocabulary_test wires a SliceModel to the backend as RadioModel does and asserts the LAST modeChanged after refusing RTTY, DFM, DSTR and FDV is not the refused mode, and the LSB passband is untouched. Red before this change (last announcement RTTY / DFM / FDV). Refs: aethersdr#6007
|
Blocker 1 was right, and the body said the opposite. I have corrected it: after the first commit the model was put back, but Reproduced first, then fixed. A new block in
On Why I did not take the queued re-publish. What I did instead, in the second commit:
Nits:
Full ON8ST |
Two conflicts. Hl2Backend::setSliceMode: main condensed the comment (aethersdr#6072) and its last sentence said unknown strings pass through. This branch refuses them, so main's text is kept and that sentence is replaced by what the code now does. The refusal itself is unchanged. SliceModel::setMode: main (aethersdr#5907) now announces m_mode, not the requested string, after the backend has answered. That already keeps a synchronously refused mode from being announced last, which is what this branch's guard was for. main's lines are taken as they are, and this branch no longer changes SliceModel. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…III. AGENTS.md Comments: state the current invariant, keep history in the commit and PR body. The setSliceMode() comment, the control_receive_test RADE leg and both hl2_mode_vocabulary_test blocks drop their narrative of what the code used to do and which review found it. The receiveOnlyModes alias-pair invariant stays. No code or assertion changes. Refs: aethersdr#5580 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
ten9876
left a comment
There was a problem hiding this comment.
Approving at 0ee3fab7c80edf9341365ce431d4de6e58af485e. The net diff is Hl2Backend.cpp plus two tests; no shared code is touched.
Fixed in this pass
- Comment policy (AGENTS.md § Comments): the comments in
setSliceMode(), thecontrol_receive_testRADE leg and bothhl2_mode_vocabulary_testblocks are condensed to the current invariant, and their history is removed (0ee3fab7c). ThereceiveOnlyModesalias-pair invariant is kept. No code changes. - PR body: it is the squash message, and it described a
SliceModel.cppchange that the merge09ac5c7b9dropped. It now says that #5907 already announcesm_modeafter the request, and that this PR touches no shared code.
Bot blocker (refused mode announced last)
Closed by #5907 on main (SliceModel.cpp:193-205). It is pinned by the new SliceModel-in-loop block, which passes as pushed and goes red for RTTY, DFM and FDV when emit modeChanged(mode) is restored.
Verified empirically, Linux RelWithDebInfo build of 0ee3fab7c:
hl2_mode_vocabulary_testandcontrol_receive_testpass.- Removing the guard turns both red: the raw delta comes back
"RTTY"with 150..3000, theSliceModelholds"RTTY", and the RADE leg fails. - Socket-free ctest: 546/547. The one failure is
connection_panel_size_test, which fails locally on this machine regardless of the change (#5811). 48 socket-owning tests were excluded and not run. - Static gates (
check_engine_boundary --strict,gen_touchpoint_manifest --check,check_capability_records --strict,check_command_plane --strict) pass. - Memory-recall probe (scratch only, not committed): refused RTTY followed by a stored 150..3000 filter leaves the slice at
mode=LSB filter=-3000..-150.setFilterWidth()normalises polarity against the mode the slice actually holds.
Reasoned, not run
knownModeStrings()is exactly the setmodeFromString()maps, so the refusal is no wider than the USB fall-through.- The aetherd control path already refuses modes outside
receiveModeControl(request.out_of_range) before reaching the backend. - These remain unchanged and are disclosed in the body: rigctl still answers
RPRT 0, TCI still echoes the requestedmodulation, and the operator gets aqCWarningand a mode tab that stays put, with no status-bar notice. All of these are shared code. - Not verified on a live HL2.
Summary
Refs #5580 and #5678 (row 1.3). This does not close #5580: the shared-code half of it is left as #5580 describes (see below).
Hl2Backend::setSliceModestored any mode string it did not recognise verbatim. The slice then read "RTTY" (or DFM, DSTR, or any other string) whilemodeFromString()fell through to USB. The indicator and the detector disagreed, and nothing was logged.#5755 closed this for the mode combo. Every other route still ends at this setter, because
SliceModel::setModeforwards whatever it is given:set_mode/set_split_modeZZMD/ZZMEmodulationshortcut mode_*The setter now applies the same check that
applyRestoredState()already applies:SliceModel's mode back to what the DSP is actually doing.One change in the backend covers every route above.
knownModeStrings()is exactly the setmodeFromString()maps, so the refusal is neither wider nor narrower than the USB fall-through.What the operator sees. The re-publish lands synchronously inside
SliceModel::setMode()(both connections are direct).setMode()onmain(#5907) announcesm_modeafter the backend has answered, not the requested string, so the lastmodeChangedcarries the corrected mode, andVfoWidget's mode tab, which paints from that argument, shows what the DSP is doing. The filter-polarity normalisation that follows also runs against the corrected mode. A new test block pins this with a realSliceModelin the loop.No shared code is touched. #5580's proposed guard in
SliceModel::setModewould change every family's behaviour, which is why I said on #5580 on 09-20 that I would not add it. This PR does not add it either.What this does not change, all of it shared code:
RPRT 0toset_mode RTTY; TCI still echoes the requestedmodulationbefore the corrected one arrives.set_mode ?and TCI'smodulations_liststill advertise RTTY.qCWarningand by the mode tab staying where it was; there is no status-bar notice.setFilterWidth()normalises its polarity against the mode the slice actually holds.Constitution principle honored
Principle VIII (Evidence Over Assertion). The fault is shown red by a test without the guard, and green with it.
Test plan
cmake --build build).hl2_mode_vocabulary_testgains a run-time block.setSliceModewith RTTY, DFM, DSTR and a nonsense string.modeFromString()maps is still applied, in its offered form, so the refusal is not wider than the vocabulary.SliceModelin the loop. A second block wires a realSliceModelto the backend asRadioModeldoes and asserts, after refusing RTTY, DFM, DSTR and FDV, that the LASTmodeChangedis not the refused mode and the LSB passband is untouched. It goes red ifsetMode()announces the requested string instead ofm_mode.control_receive_test(from fix(hl2): withdraw the FM repeater-duplex and CTCSS-encode controls the HL2 has no verb for, and declare the DSB/CWU/CWL its demodulator already serves #5879) asserted that an unknown string, "RADE", comes back verbatim. That pinned exactly the behaviour this fixes. It now requires the string to be refused and the slice to stay in FM. "RADE" is a fair unknown:RxAppletandVfoWidgetreturn beforesetMode()for it, so it never reaches a backend in production.Checklist
AppSettingscallsdocs/change: the new behaviour is the documented intent ofHl2ModeVocabulary.hON8ST
🤖 Generated with Claude Code