Skip to content

fix(hl2): bookmark recall keeps DIGU/DIGL AGC off. Principle VIII. - #6150

Merged
jensenpat merged 4 commits into
aethersdr:mainfrom
on8st:hl2/digital-bookmark-agc
Oct 7, 2026
Merged

jensenpat merged 4 commits into
aethersdr:mainfrom
on8st:hl2/digital-bookmark-agc

Conversation

@on8st

@on8st on8st commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #6142

On a Hermes-Lite 2, recalling a band-stack bookmark into DIGU or DIGL no longer puts the bookmark's stored AGC mode over the data-mode AGC off (#6119).

Limits first.

  1. To rule on: an AGC chosen on purpose in a data mode does not come back from a bookmark. An operator who selects fast in DIGU and bookmarks it gets AGC off on recall. A BandStackEntry holds only the mode string, so a stale med and a chosen med cannot be told apart. The alternative, yours to choose: a BandStackEntry field recording whose choice the AGC was. That is a schema change, and older entries still read as "not chosen".
  2. Only the band-stack recall is covered, and I found no other live one. MainWindow::restoreBandState() also replays a stored AGC mode through SliceModel::setAgcMode(), but it has no caller in src or tests at this head (nor has captureCurrentBandState() or BandSettings::loadBandState()), so it is not reachable. It is left untouched.
  3. A recall never changes the AGC mode while the receiver is in DIGU/DIGL, also when it is already there: an AGC the operator runs in the data mode stays.
  4. Ignore, not migrate (shape 1, as the triage proposes). No stored entry is rewritten; a stale one ages out when saved again, because the save reads the slice's AGC, which is off.
  5. Not run on a radio or in the app. The test drives the recall in the order of MainWindow's lambda (mode, then recallBandStackReceiveDsp); MainWindow is not constructed.

What changed

  • SliceAgcRequest (src/core/backends/ReceiveCommand.h) carries Origin { Operator, Recall }, default Operator, as SliceFilterRequest already does for the filter.
  • SliceModel::recallAgcMode() is setAgcMode() with the Recall origin.
  • RadioModel::recallBandStackReceiveDsp() is the one caller that uses it.
  • Hl2Backend::requestSliceAgc() drops a Recall mode request while the receiver is in DIGU/DIGL and publishes the receiver's state, so the slice shows off again. The held AGC is untouched; the threshold recall is unchanged.

No family check above the seam, no new capability boolean, no settings change. Only Hl2Backend reads the origin: Flex sends the same agc_mode= text.

On top of #5919

Rebased onto f4e463c2. #5919 added typed adapters for DSP, audio, squelch, RX antenna and lock; the AGC route is the one it already had and is unchanged: SliceModel::receiveAgcRequested → RadioModel::dispatchSliceAgc → IRadioBackend::requestSliceAgc, one direct connection made in RadioModel::wireSliceReceiveIntentsToBackend. SliceModel has one emitter of a Field::Mode request (applyAgcMode), and nothing above the seam calls setSliceAgc directly, so a recalled mode has no second way to the HL2. The HL2 keeps the default Optimistic receive-control policy, so setMode() and the AGC echo still run synchronously, which is the order the fix relies on. The rebase was textually clean; the only edit after it is a shorter header comment in the test.

Tests

hl2_bandstack_digital_agc_test (new, 24 checks, socket-free): a real, unconnected Hl2Backend behind RadioModel.

case result
LSB with slow, recall a DIGU bookmark storing med AGC off; the slice reads off (what a re-save stores); leaving DIGU returns slow
a DIGU bookmark storing med, threshold 40 threshold 40 applied, AGC off
a DIGU bookmark storing off off, and the held AGC returns on leaving
an LSB bookmark storing fast, threshold 70, from USB and from DIGU fast, 70
the operator selects fast in DIGU, then recalls a DIGU bookmark fast stays, and is kept on leaving
limit 1: a DIGU bookmark storing fast, recalled from LSB off; selecting fast afterwards works
a Flex-shaped backend in DIGU, bookmark storing med slice set 0 agc_mode=med, agc_threshold=70, byte for byte; the base adapter gets med

Mutation, at this head on f4e463c2: with the Recall condition in Hl2Backend::requestSliceAgc disabled (a scratch commit, not pushed), the full suite fails this test with 10 checks red, the bookmark cases above among them; with it restored the test passes 24 of 24. The fail-before without the fix was run before the rebase only (base 22507580, the same 10 red).

Full suite at 693fa675 (the review nits, comments only): 643 of 644, the same single failure.

Full suite at the first head (macOS, Qt 6.12.0, -j8): 643 of 644. The one failure is peripheral_auth_dialog_test (added by #6008), also on the serial rerun: it depends on the host's serial-port names. Clean main at f4e463c2 was not built here for comparison; the same test fails the same way on our other branches on main at a325fb99 and earlier.

Static checks: every step of the "Static checks" job run locally at this head against f4e463c2 (Python 3.14, CI uses 3.12): all pass; touchpoint manifest regenerated, no change. One src/gui file is touched since the review nits, MainWindow.cpp, by a comment only; the accessibility step finds nothing new there. The "Sanitizer option configures" job needs the CI image and was not run.

Not verified

  • On a radio, and through the real MainWindow recall.
  • The fail-before without the fix at this head; the mutation above stands in for it.
  • Linux and Windows locally; CI only.
  • ANAN-G2, Icom, RTL-SDR and Sim were not driven; the base adapter they use is covered through a stub.

73, ON8ST

🤖 Generated with Claude Code

Fixes aethersdr#6142

A band-stack bookmark saved in DIGU/DIGL before aethersdr#6119 stores the AGC of
that time. Recalling it set the mode (AGC off), then replayed the stored
mode through the operator path, which Hl2Backend took as the operator's
choice: the held AGC was dropped and the data mode ran on the stored AGC.

SliceAgcRequest carries an Origin (Operator, Recall), as
SliceFilterRequest does. RadioModel::recallBandStackReceiveDsp asks for
Recall through SliceModel::recallAgcMode; Hl2Backend::requestSliceAgc
drops a recalled mode while the receiver is in DIGU/DIGL and echoes the
receiver's state, so the slice shows what the receiver runs. The
threshold recall is unchanged. No other backend reads the origin.

Limit: a bookmark stores only the AGC mode string, so an AGC the
operator chose inside DIGU and then bookmarked is not told apart from a
stale one and is not brought back by the recall.

Test: hl2_bandstack_digital_agc_test, a real unconnected Hl2Backend
behind RadioModel, driven in MainWindow's recall order. Ten checks red
on main at 2250758 and again with the rule disabled.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@on8st
on8st requested a review from a team as a code owner October 4, 2026 13:18

@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.

Issue fit

Yes, for the case #6142 describes. I traced the whole recall order against the head checkout and it holds: MainWindow.cpp:5337 applies the bookmark's mode first, Hl2Backend::setSliceMode → followModeAgc (Hl2Backend.cpp:2924-2935) holds the AGC and sets off, the echo is synchronous so slice->agcMode() already reads off, and RadioModel.cpp:4229's entry.agcMode != slice->agcMode() therefore fires every time. The new Origin::Recall gate at Hl2Backend.cpp:3171-3178 is the first point on that chain where a replay is distinguishable from an operator click, which is the right seam.

One correction to the framing carried over from the triage: #6119 is merged (e0c68e36 on main), so isDigitalDataMode/followModeAgc/agcModeBeforeDigital are live, this PR is not stacked on anything open, and the defect is a live regression rather than a latent one. The PR body already treats it that way.

The self-heal claim checks out: the bookmark-save lambda reads entry.agcMode = slice->agcMode() (MainWindow.cpp:5313), which is off in DIGU after the fix, so a stale entry ages out on re-save without a migration.

Scope

File(s) What it changes Claimed? Verdict
src/core/backends/ReceiveCommand.h SliceAgcRequest::Origin { Operator, Recall } + defaulted field yes in scope; mirrors SliceFilterRequest::Origin four structs up
src/models/SliceModel.{h,cpp} recallAgcMode(), private applyAgcMode(mode, origin), three emit sites updated yes in scope
src/models/RadioModel.cpp the one recall call site yes in scope
src/core/backends/hl2/Hl2Backend.{h,cpp} the drop gate + header comment yes in scope
docs/HERMES.md §18 records the recall behaviour yes in scope
tests/hl2_bandstack_digital_agc_test.cpp, tests/tests.cmake new socket-free target, registered and added to AETHER_SETTINGS_CONSUMERS yes in scope

Everything in the diff is explained by the issue. No new public/protocol surface (Origin is an internal struct field, not a capability bool — per the frozen-count rule a record/enum field needs no ratchet argument), no settings key, no default changed, no CHANGELOG.md entry. No guard or early return removed: the only - lines of substance are the two hoisted locals in requestSliceAgc and the three emit sites gaining a field.

Socket tests: none. tests/hl2_bandstack_digital_agc_test.cpp includes only QCoreApplication/QString/QStringList; no QTcpSocket/QUdpSocket/bind/listen/connectToHost, no fake peer, and the Hl2Backend is never connected so there is no Metis transport. Links Qt6::Core Qt6::Network only because aethercore needs it.

Blockers

None.

Needs maintainer decision

Limit 1 is a real behaviour regression for post-#6119 bookmarks, and the thread left it open. src/core/backends/hl2/Hl2Backend.cpp:3171-3178:

        if (r && request.field == SliceAgcRequest::Field::Mode
            && request.origin == SliceAgcRequest::Origin::Recall
            && isDigitalDataMode(r->mode)) {

On main today, an operator who selects fast in DIGU, bookmarks it, and later recalls that bookmark from LSB gets fast back. After this PR they get off. The PR tests that case explicitly and labels it ("limit: a DIGU bookmark saved with a chosen AGC recalls to off, not to it"), and the triage reached the same conclusion — a BandStackEntry holds four characters that are identical whether med came from a stock pre-#6119 profile or a deliberate override. The trade (fix every stale entry, lose the deliberate round-trip) is defensible and self-healing, but it is the open question #6142 carries maintainer-review for, so it should be ruled on rather than merged through. The alternative named in the body — an "whose choice" flag on BandStackEntry — is a schema change and is correctly not attempted here.

Nits (non-blocking)

  • src/core/backends/hl2/Hl2Backend.cpp:3176 — the threshold half of the recall is correct only because the drop's echo is synchronous. After the mode is dropped, emitSliceState(ddc) → sliceChanged → SliceModel::applyDelta (SliceModel.cpp:1768-1771) resets m_agcMode to off before RadioModel.cpp:4232 calls setAgcThreshold, so the threshold request carries mode="off", m == r->agcMode in setSliceAgc, and agcModeBeforeDigital survives. I confirmed the synchrony: Hl2Backend itself is never moved to a worker thread (only m_metis, m_txDsp, m_dspBuildContext and the per-receiver Hl2RxDsp at Hl2Backend.cpp:323-334, 848), and the sliceChanged connect at RadioModel.cpp:1539 uses the default AutoConnection. If that ever becomes queued, m_agcMode would still read med at the threshold emit and setSliceAgc(id, "med", 40) would clear the hold — the bug back through the side door. The comment at :3172-3174 says "the echo puts back what the receiver holds"; worth one more clause saying the threshold step depends on that happening before it returns, since nothing else records it.
  • tests/hl2_bandstack_digital_agc_test.cpp:56-63 — recall() reimplements MainWindow's lambda (mode, then recallBandStackReceiveDsp) rather than driving it. That is honest and disclosed, but the order is load-bearing for the fix, so nothing pins it against MainWindow drifting. Not worth a GUI test; a one-line comment at MainWindow.cpp:5337 pointing at this test would do.
  • docs/HERMES.md:3006 — the inserted sentence leaves ...its threshold still applies. With AGC off the gain is the AGC-off as a ~105-column line in a block otherwise wrapped at ~78. Cosmetic.

What I tried to break

  • A second route to the HL2 that bypasses the gate. SliceModel has exactly one emitter of a Field::Mode request (applyAgcMode); agcCommandIssued has no connect() anywhere in src or tests (only the declaration and a comment), so it is not an alternate path; nothing above the seam calls setSliceAgc directly (RadioModel.cpp:9888 is the sole dispatch). The MainWindow recall's raw slice set N agc_mode= wire text is inside the externalReceiveReplacementActive() branch only, and would be dropped by the HL2's absent command plane regardless.
  • Sibling recall sites left unfixed. MainWindow::restoreBandState (MainWindow.cpp:10946) does replay a stored AGC through setAgcMode(), and I confirmed it is genuinely dead: restoreBandState, captureCurrentBandState and BandSettings::loadBandState have only their declarations/definitions, no callers in src or tests. The other setAgcMode() callers (rigctl, TCI, SmartCAT, automation, the applets, the shortcut cyclers) are all operator-equivalent edits and correctly keep Operator.
  • Over-broad gating. Only Hl2Backend and FlexBackend override requestSliceAgc; Flex ignores the origin and still sends agc_mode=, which the test pins byte-for-byte, and the base IRadioBackend::requestSliceAgc still forwards the stored mode for ANAN/Icom/RTL/Sim. Adding a defaulted field at the end of the aggregate does not disturb the two existing SliceAgcRequest constructions (hl2_tci_signaling_test.cpp:661, hl2_state_restore_test.cpp:155), which default-init then assign.
  • Null/unmapped receiver. ddcForSlice returns -1 via m_ids.byUi when the slice is unmapped, so r is null, the gate's r && fails, and the call falls to the base which returns on its own !r. r non-null implies the id record exists, so the drop path's emitSliceState cannot hit its !ids early return and strand the slice on an optimistic med.
  • Does the test discriminate? Reasoning only — with the guard removed, the base setSliceAgc(id, "med", …) would clear agcModeBeforeDigital and set med, and Hl2Fixture::runs() asserts both the published delta and slice->agcMode(), so the stale-bookmark and threshold cases go red. It also catches a missing emitSliceState on the drop path, since slice->agcMode() would stay med even though the cached agc still reads off from the earlier setMode publish.
  • Transient flap. applyAgcMode writes m_agcMode = "med" and emits agcModeChanged("med") before the drop, so the five consumers (RxApplet, VfoWidget, AgcCalibrationDialog, MainWindow_Wiring, RadioResourceAdapter) see med then off inside one call stack. None of them persists, and the backend-side capture (notifyOperatingStateChanged) is never reached on the drop path, so nothing stores med. This is the documented Optimistic receive-control policy, not a defect.
  • Couldn't check: anything at runtime. CI is green on cfa3c7ab (build, check-macos, check-windows, Static checks, Sanitizer option configures), but the per-PR ctest is -R-filtered and does not run this new target, so the 24-check result and the mutation run are the author's, not CI's. Every finding above is reasoned from the head checkout, not reproduced.

Recommendation

Needs maintainer decision. The implementation is clean and I could not break it: the gate is at the only seam where a replay is distinguishable, the single-caller blast radius is real, no other backend changes behaviour, the dead sibling recall is correctly identified as dead, and the test asserts both halves of the contract with a limit case that documents the trade rather than hiding it. What needs a ruling is limit 1 — this knowingly costs a deliberate in-DIGU AGC its round-trip through a bookmark, which is a live regression for anyone who has bookmarked since #6119 merged. That question was left open in #6142's thread and in the triage; once it is answered either way, this merges as-is (the nits are all optional). Thanks for the unusually legible limits section and for naming the fail-before gap instead of papering over it.


🤖 aethersdr-agent · cost: $10.0431 · model: claude-opus-5

Review nits on the bookmark recall fix, no behaviour change:
- Hl2Backend::requestSliceAgc says the drop's echo must stay synchronous,
  because the threshold recall that follows reads the slice's mode.
- MainWindow's recall lambda names the test that replays its order.
- HERMES section 18: one over-long line wrapped.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@on8st

on8st commented Oct 4, 2026

Copy link
Copy Markdown
Contributor Author

The three nits are in 494d4205, comments and docs only, as a new commit:

  • Hl2Backend::requestSliceAgc: the comment now says the echo must stay synchronous, because the threshold recall that follows reads the slice's mode.
  • MainWindow's recall lambda names the test that replays its order.
  • docs/HERMES.md §18: the long line is wrapped.

Full suite at 494d4205 (macOS, Qt 6.12.0): 643 of 644; the one failure is peripheral_auth_dialog_test, as before. Limit 1 is unchanged and waits for your ruling.

73, ON8ST

@ten9876 ten9876 self-assigned this Oct 4, 2026

@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.

Reviewed head 494d4205cf51c5defc4689d577822341b7b2bb0d.

Issue fit: Yes for #6142's stale DIGU/DIGL bookmark AGC replay. The recall origin reaches the HL2-specific gate and the synchronous echo restores the receiver's actual AGC before threshold recall. The disclosed trade remains unresolved: a deliberately saved data-mode AGC choice is also ignored when recalling from a voice mode.

Scope

File/group Change Claimed? Verdict
ReceiveCommand.h, SliceModel.{h,cpp} Typed Operator/Recall origin, recall method Yes In scope; additive internal command metadata
RadioModel.cpp Bookmark recall selects Recall origin Yes Direct issue fit
Hl2Backend.{h,cpp} Ignore recalled mode in DIGU/DIGL and publish real state Yes Correct location for family policy
MainWindow.cpp Document required mode-before-DSP ordering Body says no GUI file touched Comment only; correct description
docs/HERMES.md Document recall policy Yes In scope
New regression test, tests.cmake Socket-free HL2/base/Flex-shaped coverage Yes In scope, registered

No unrelated authored commits, schema migration, new capability bool, family checks above the seam, or changelog change. The added public C++ request origin is explicitly motivated by the issue; no wire command is added.

Blockers

No implementation defect demonstrated. Maintainer decision remains required at src/core/backends/hl2/Hl2Backend.cpp:3176–3180: Jeremy, accept ignoring deliberately bookmarked DIGU/DIGL AGC choices, or require provenance in the bookmark schema. #6142 explicitly leaves this undecided; the PR correctly discloses it. GOVERNANCE.md reserves “UX behavior — how controls work” to the maintainer. This is not a request to invent a regression claim against an intentional, disclosed choice.

Nits / limits (non-blocking)

  1. src/gui/MainWindow.cpp:5336: correct the body's “No src/gui file touched” sentence; this file does change, although only its ordering comment.
  2. src/models/RadioModel.cpp:4226: the adjacent comment still says the caller sends AGC as wire text. At this head the registered receiveAgcRequested dispatcher is the route. Update that stale explanation so future fixes do not create a second send path.
  3. tests/hl2_bandstack_digital_agc_test.cpp:54: the new test reconstructs the MainWindow recall order rather than driving the real GUI; retain that limit. I did not independently run the entire 24-check backend fixture or author's full-suite mutation.

Verification and attacks

  • Read all new test code and registration. It creates an unconnected real backend and injects slice state; no listener, bound socket, synthetic firmware, or TX. No socket preflight exception needed.
  • Built a focused C++20/Qt6 harness against the head's actual SliceModel and dependencies. recallAgcMode("slow") emitted field=0 origin=1 mode=slow threshold=65; operator setAgcMode("fast") emitted origin=0; threshold 40 retained mode=fast with field=1 origin=0.
  • Mutated the recall helper to use Operator: the same harness emitted origin=0 for the recall and exited 1. Restored the scratch source afterward. This independently pins the new model-intent distinction, not the complete backend recall behavior.
  • Read the direct/unique dispatcher wiring, backend owner-thread contract, synchronous slice-delta return, threshold ordering, external-receive bypass, voice recall, already-digital operator choice and the intentionally lost bookmark choice. No second AGC wire route remains above the seam. restoreBandState() has only declaration/definition and no caller, as claimed.
  • Ran all four boundary/manifest/capability/command-plane gates at merge base and head: all pass, byte-identical output. No #5554 pattern growth. Automated review skill unavailable; independent manual pass and focused mutation used.
  • No app/bridge or hardware session: the simulator lacks HL2's data-mode AGC policy. Full GUI ordering, hardware behavior and non-HL2 family sessions remain unverified here; the body honestly distinguishes them.

CI and recommendation

Needs maintainer decision on deliberate data-mode AGC bookmark recall. The five displayed checks pass, but earliest creation at this head (2026-10-04T15:50:48Z) precedes current main (2026-10-04T18:26:40Z); their merge tree is older than today's, with non-strict protection not forcing refresh. They also do not mean every registered test ran. Once Jeremy rules on the trade, retain the documented limits and verify the required checks on the integration to land.

— Jeremy KK7GWY + Codex

@ten9876 ten9876 removed their assignment Oct 4, 2026
Review nit on the bookmark recall fix, comment only: the note in
RadioModel::recallBandStackReceiveDsp said the caller sends the AGC as
wire text. That is true only under KiwiSDR external receive; otherwise
the slice setters are the single route, through the receive-intent
dispatcher.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@on8st

on8st commented Oct 4, 2026

Copy link
Copy Markdown
Contributor Author

Nits 1 and 2 are done; nit 3 stays as a stated limit.

  • Body: it said no src/gui file is touched. Since the first nit commit, MainWindow.cpp carries a comment; the sentence is corrected.
  • RadioModel::recallBandStackReceiveDsp: the comment now says the caller sends wire text only under KiwiSDR external receive, and that otherwise the slice setters are the one route, through the receive-intent dispatcher. Comment only, 693fa675.
  • Test order: the test still replays MainWindow's recall order instead of driving the GUI; that limit stays in the body.

Full suite at 693fa675 (macOS, Qt 6.12.0): 643 of 644; the one failure is peripheral_auth_dialog_test, as before. I have not merged main in to refresh the checks; say if you want that before you rule on limit 1.

73, ON8ST

@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.

Issue fit: yes for #6142. At 693fa675, a band-stack recall into DIGU/DIGL on an HL2 no longer replaces the data-mode AGC off with the bookmark's stored AGC.

  • Origin: the recalled AGC mode now carries SliceAgcRequest::Origin::Recall.
  • Drop: Hl2Backend::requestSliceAgc drops it while the receiver is in a data mode.
  • Echo: it then publishes the receiver's real state synchronously, so the threshold recall that follows reads off and does not clear the held AGC.

Scope

File Change Claimed? Verdict
src/core/backends/ReceiveCommand.h:30-37 Origin { Operator, Recall }, default Operator (mirrors SliceFilterRequest::Origin) Yes In scope; internal seam metadata, no wire text
src/models/SliceModel.{h,cpp} recallAgcMode() / private applyAgcMode(); threshold and off-level emits name Operator Yes In scope
src/models/RadioModel.cpp:4231 the one recall call site uses recallAgcMode Yes In scope
src/core/backends/hl2/Hl2Backend.{h,cpp}:3167-3183 the drop plus synchronous echo Yes In scope; family policy below the seam
src/gui/MainWindow.cpp:5336, docs/HERMES.md comments and docs Yes (body corrected) In scope
tests/hl2_bandstack_digital_agc_test.cpp, tests/tests.cmake new socket-free target, registered Yes In scope

Everything in the diff is explained by #6142. The three authored commits are all from 10-04, and there is no CHANGELOG.md, settings key, capability bool, or removed guard. Socket preflight: the test builds a real but unconnected Hl2Backend, so no Metis transport is ever opened. Merge is clean against current main (85f2a124).

Blockers

None.

Needs maintainer decision (@ten9876)

  1. Limit 1: an AGC deliberately chosen in DIGU/DIGL and bookmarked no longer comes back from that bookmark (Hl2Backend.cpp:3176-3180). On main today, fast in DIGU → bookmark → recall from LSB gives fast; after this PR it gives off.

    • Why it happens: a BandStackEntry stores only the mode string, so a stale pre-#6119 med and a chosen med look identical.
    • Option A — accept: take the trade. Stale entries heal on re-save, since the save reads the slice's AGC, which is now off.
    • Option B — schema field: add a provenance field to BandStackEntry (a schema change; old entries still read as "not chosen").

    The test pins the trade explicitly ("limit: a DIGU bookmark saved with a chosen AGC recalls to off"), so whichever way you rule, it is visible.

Nits (non-blocking)

None outstanding. The earlier rounds' nits (echo-synchrony comment, MainWindow order pointer, stale RadioModel comment, body's src/gui sentence) are all in 494d4205 / 693fa675.

What I tried to break / verified

  • Built and ran this head's hl2_bandstack_digital_agc_test (Linux, Qt 6.12.0): all checks pass.

  • Three mutations, each built and run, then restored (sha256 matches git show HEAD: for both files, touched before the rebuild):

    Mutation Result
    drop path returns without emitSliceState(ddc) exit 1, 8 FAIL (e.g. "a DIGU bookmark storing med leaves the data-mode AGC off", "saving the bookmark again stores off")
    RadioModel::recallBandStackReceiveDsp calls setAgcMode (operator origin) instead of recallAgcMode exit 1, 10 FAIL
    the Origin::Recall condition in the gate replaced by false exit 1, 10 FAIL, matching the body's figure

    So the echo, the origin plumbing and the gate are each pinned on their own; none rides on another.

  • Second route to the HL2: SliceModel has one Field::Mode emitter (applyAgcMode). The other setAgcMode() callers (rigctl, TCI, SmartCAT, automation, applets, shortcut cyclers, AetherClockEngine's WWVB preset) are genuine operator or feature edits and correctly stay Operator. MainWindow::restoreBandState() replays AGC but has no caller.

  • Other families: only Hl2Backend and FlexBackend override requestSliceAgc. Flex ignores the origin, and the test pins its agc_mode= text byte for byte. ANAN, Icom, RTL and Sim go through the base adapter, and no other backend has a data-mode AGC follow.

  • The full-state echo on the drop path: the recall lambda's earlier setters (mode, tune, filter, audio gain) all land synchronously on the HL2 under the Optimistic policy. The echo therefore publishes state that already includes them, and cannot roll any of them back.

  • App not driven: the demo uses the base AGC adapter, not the HL2's data-mode policy, so this path can't be reached there. Hardware and the real MainWindow recall order remain unverified, as the body says.

Recommendation: no code blocker. It is ready once you rule on Limit 1.

— Jeremy KK7GWY + Claude

@on8st

on8st commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Merged current main (ca4e7cb0) to clear the conflict, as a new merge commit 1e4f9fff; nothing rebased. The one conflict was in tests/tests.cmake, where this PR and #6129 each added a test to AETHER_SETTINGS_CONSUMERS at the same line; both are kept. The touchpoint manifest is current.

Full suite on the merged head, macOS, Qt 6.12.0: 658 tests, 4 fail, and clean main ca4e7cb0 fails the same 4 on this machine. One of them is new on main: control_receive_test, "RTL adjustable filter qualification excludes legacy WFM", which is #6248. The other three are #5254, #6008 and #6224. This PR's own hl2_bandstack_digital_agc_test passes.

73, ON8ST

@jensenpat jensenpat self-assigned this Oct 7, 2026

@jensenpat jensenpat 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.

Issue fit: yes for #6142, at 1e4f9fff9. A band-stack recall into DIGU/DIGL on an HL2 no longer puts the bookmark's stored AGC over the data-mode AGC off. The stored threshold still applies. Voice bookmarks and every non-HL2 backend still receive the stored mode.

Scope

File Change Claimed? Verdict
src/core/backends/ReceiveCommand.h SliceAgcRequest::Origin { Operator, Recall }, default Operator Yes In scope. Internal seam metadata, no wire text
src/models/SliceModel.{h,cpp} recallAgcMode() / applyAgcMode(); threshold and off-level stay Operator Yes In scope
src/models/RadioModel.cpp the one band-stack recall call uses recallAgcMode Yes In scope
src/core/backends/hl2/Hl2Backend.{h,cpp} drop a Recall mode while the receiver is DIGU/DIGL, then emitSliceState Yes In scope. Family policy stays below the seam
src/gui/MainWindow.cpp, docs/HERMES.md order comment and the data-mode AGC note Yes In scope
tests/hl2_bandstack_digital_agc_test.cpp, tests/tests.cmake new socket-free target, registered Yes In scope

No CHANGELOG entry, settings key, capability bool, or removed guard. Only Hl2Backend and FlexBackend override requestSliceAgc. Flex ignores the origin. ANAN, Icom, RTL-SDR and Sim use the base adapter, which still calls setSliceAgc. MainWindow::restoreBandState() still uses setAgcMode and still has no caller.

The KiwiSDR external-receive path is unchanged: the recall lambda sends the bookmark as wire text, and recallBandStackReceiveDsp skips the slice AGC setters while that replacement is active.

Blockers

None.

Limit 1 is the behavior this PR ships, and it matches the issue's ignore shape. A BandStackEntry stores only the mode string, so a stale pre-#6119 med and an AGC the operator chose inside DIGU and then bookmarked cannot be told apart. Recall of either leaves DIGU/DIGL at AGC off. The operator can still select fast afterward, and that live choice is not cleared by a later recall while the receiver stays in the data mode. The test pins the trade (limit: a DIGU bookmark saved with a chosen AGC recalls to off). A provenance field on BandStackEntry would be a separate schema change.

Verification

  • Read the recall lambda, followModeAgc, setSliceAgc, dispatchSliceAgc (Qt::DirectConnection), and emitSliceState (always publishes agcMode). The echo is synchronous, so the threshold recall that follows reads off and does not clear agcModeBeforeDigital.
  • hl2_bandstack_digital_agc_test constructs a real unconnected Hl2Backend. It does not bind, listen, or open Metis. Registered in tests.cmake, not on the frozen per-PR gate.
  • Built and ran that test at this SHA (Qt 6.12.0, macOS): 24/24.
  • Mutation: the Origin::Recall condition in Hl2Backend::requestSliceAgc replaced with false. The same test exited 1 with 10 failures, including the stale DIGU/DIGL cases and the held-AGC return. Source restored; the test passed again.
  • git merge-tree --write-tree of this head with origin/main 43e2633e is clean. The recall gate and recallAgcMode call survive. Those five main commits are not in this CI run.

Required checks on 1e4f9fff9 are green (build, check-macos, check-windows, Static checks, plus Sanitizer option configures). The per-PR jobs do not run this new test; the local run above does.

Not run: a radio, or the real MainWindow recall (tune, filter, and audio gain sit between setMode and recallBandStackReceiveDsp; none of them set AGC).

@jensenpat
jensenpat merged commit 5b41f53 into aethersdr:main Oct 7, 2026
5 checks passed
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: an old DIGU/DIGL band-stack bookmark re-applies medium AGC over the data-mode AGC off

3 participants