Skip to content

fix(hl2): refuse a mode the radio cannot demodulate. Principle VIII. - #6007

Merged
ten9876 merged 4 commits into
aethersdr:mainfrom
on8st:hl2/refuse-unknown-mode
Oct 2, 2026
Merged

ten9876 merged 4 commits into
aethersdr:mainfrom
on8st:hl2/refuse-unknown-mode

Conversation

@on8st

@on8st on8st commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

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::setSliceMode stored any mode string it did not recognise verbatim. The slice then read "RTTY" (or DFM, DSTR, or any other string) while modeFromString() 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::setMode forwards whatever it is given:

  • rigctl set_mode / set_split_mode
  • SmartCAT ZZMD / ZZME
  • TCI modulation
  • memory recall
  • the Mode keyboard shortcuts and the bridge's shortcut mode_*
  • MIDI and the Ulanzi dial
  • the VFO quick-mode buttons
  • the net scheduler
  • a Kiwi spot click

The setter now applies the same check that applyRestoredState() already applies:

  • An unknown mode is refused, with a warning naming the modes the radio does demodulate.
  • The slice is re-published unchanged, which sets SliceModel's mode back to what the DSP is actually doing.

One change in the backend covers every route above. knownModeStrings() is exactly the set modeFromString() 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() on main (#5907) announces m_mode after the backend has answered, not the requested string, so the last modeChanged carries the corrected mode, and VfoWidget'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 real SliceModel in the loop.

No shared code is touched. #5580's proposed guard in SliceModel::setMode would 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:

  • rigctl still answers RPRT 0 to set_mode RTTY; TCI still echoes the requested modulation before the corrected one arrives.
  • set_mode ? and TCI's modulations_list still advertise RTTY.
  • The operator is told by a qCWarning and by the mode tab staying where it was; there is no status-bar notice.
  • Memory recall applies the stored filter after the refused mode. 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

  • Local build passes (cmake --build build).
  • Behavior verified on a real radio: not done. Every check here is socket-free.
  • Reproduction: hl2_mode_vocabulary_test gains a run-time block.
    • It puts the slice in LSB, then calls setSliceMode with RTTY, DFM, DSTR and a nonsense string.
    • For each, it requires the published mode and passband to be unchanged.
    • With the guard removed, each comes back as itself with the USB passband 150..3000.
    • Positive control: every spelling modeFromString() maps is still applied, in its offered form, so the refusal is not wider than the vocabulary.
  • With a SliceModel in the loop. A second block wires a real SliceModel to the backend as RadioModel does and asserts, after refusing RTTY, DFM, DSTR and FDV, that the LAST modeChanged is not the refused mode and the LSB passband is untouched. It goes red if setMode() announces the requested string instead of m_mode.
  • One existing assertion changes. 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: RxApplet and VfoWidget return before setMode() for it, so it never reaches a backend in production.

Checklist

  • Commits are signed
  • No new flat-key AppSettings calls
  • Code is clean-room
  • No meter UI touched
  • No docs/ change: the new behaviour is the documented intent of Hl2ModeVocabulary.h
  • Not security-sensitive

ON8ST

🤖 Generated with Claude Code

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
@on8st
on8st requested a review from a team as a code owner September 28, 2026 16:55

@aethersdr-agent aethersdr-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 shown

So 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 after setMode() finishes emitting — smallest change, keeps the fix inside the backend as intended.
  • Or make VfoWidget's handler read m_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 RTTY still gets RPRT 0; the only signal is a qCWarning and a later get_mode returning something else. That is strictly better than silently demodulating USB, and plumbing a refusal back through SliceModel::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 — recallMemory does not exist in the current tree (recallCachedMemory, RadioModel.cpp:9317, goes through applyMemoryRecallDetails), 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 says MemoryFieldValues::modes() does), recalling one onto an HL2 is worth a manual check.
  • The positive-control loop walks knownModeStrings(), which includes WBFM and DRM — modes Hl2ModeVocabulary.h itself documents as accepted-but-undisplayable. Asserting lastMode == 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 of modeFromString() (Hl2Backend.cpp:189-212) — the sets are identical, including the CW/CWU, FM/NFM, WBFM/WFM alias pairs. Nothing that previously demodulated correctly is now refused. The CWL concern the #5580 triage raised (TCI cwr, CAT MD03) does not bite: CWL is both known and published.
  • Null/ordering safety of the early return. ddc and r are both resolved above the guard (Hl2Backend.cpp:3583-3586), and emitSliceState re-null-checks rx(ddc) and m_ids.byDdc(ddc) (:9485-9488), so the early return cannot publish against a dead receiver.
  • Does the delta actually carry a mode? Yes — emitSliceState sets d.mode = r->mode unconditionally (: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, so lastMode == baseMode fails; the flipped control_receive_test assertion fails the same way. Both are genuine regression tests for the backend half.
  • Digital-voice registry leak on refusal. setMode claims the slice in DigitalVoiceModeRegistry before emitting the request. I walked applyChanges' incoming-mode block (SliceModel.cpp:1236-1278) — the corrective delta hits the else if (previousMode.has_value()) branch and calls releaseSlice, so a refused FDV-family request does not strand a claim.
  • The test comment's RADE claim. Verified: VfoWidget.cpp:2629-2632 returns before m_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

Comment thread src/core/backends/hl2/Hl2Backend.cpp
Comment thread tests/hl2_mode_vocabulary_test.cpp
…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
@on8st

on8st commented Sep 28, 2026

Copy link
Copy Markdown
Contributor Author

Blocker 1 was right, and the body said the opposite. I have corrected it: after the first commit the model was put back, but setMode()'s trailing emit modeChanged(mode) still repainted the mode tab with the refused mode.

Reproduced first, then fixed. A new block in hl2_mode_vocabulary_test wires a real SliceModel to the backend the way RadioModel does, then asserts two things after refusing RTTY, DFM, DSTR and FDV:

  • the last modeChanged is not the refused mode;
  • the LSB passband is untouched.

On 2d8395f8 it fails exactly as you traced it: the last announcement is RTTY / DFM / FDV. DSTR announces nothing, because SliceModel refuses it itself in a build without D-STAR.

Why I did not take the queued re-publish. setMode() normalises the filter polarity against m_mode after its emits. With the correction deferred, m_mode would still hold the refused mode at that point. FDV is refused by the HL2 but is USB-family in filterPolarityUsbFamily(), so an LSB passband would be flipped to USB polarity and sent to the radio. This trade-off is reasoned from normalizeFilterPolarity(), not run.

What I did instead, in the second commit:

  • SliceModel::setMode() announces the requested mode only if m_mode still holds it after the request.
  • A backend that does not answer synchronously leaves m_mode == mode, so nothing changes for any other family.
  • This is the one shared-code touch. The body now explains it, as the localization rule asks.

Nits:

  • The RPRT 0 boundary is named in the body under "What this does not change".
  • Memory recall stays listed there as unverified, as you flagged it.

Full ctest on the new head 2aeb69b4: 552/552.

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>
@ten9876 ten9876 self-assigned this Oct 2, 2026
…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 ten9876 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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(), the control_receive_test RADE leg and both hl2_mode_vocabulary_test blocks are condensed to the current invariant, and their history is removed (0ee3fab7c). The receiveOnlyModes alias-pair invariant is kept. No code changes.
  • PR body: it is the squash message, and it described a SliceModel.cpp change that the merge 09ac5c7b9 dropped. It now says that #5907 already announces m_mode after 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_test and control_receive_test pass.
  • Removing the guard turns both red: the raw delta comes back "RTTY" with 150..3000, the SliceModel holds "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 set modeFromString() 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 requested modulation, and the operator gets a qCWarning and a mode tab that stays put, with no status-bar notice. All of these are shared code.
  • Not verified on a live HL2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HL2: mode combo offers Flex's fallback set, and RTTY/DFM/D-STAR silently demodulate as plain USB

2 participants