Commit 03ddf48
fix(hl2): a declined auto-gain arm explains itself where the operator is, not only in the log (#5825)
Closes #5817. The shape is the one proposed in that issue's triage,
widened in review to every route an arm request can take.
## What the operator sees today
Tick **Auto** with no stored gain for the current band and the
constructed baseline sits above `kAutoRfGainMaxBaselineDb`.
`setAutoRfGain` declines, and the checkbox **springs back to unticked
with no explanation**.
The wiring already reads `isArmed()` back, and that is correct — it is
what stops the UI reporting a state the radio is not in (#5395). But a
readback can only say *that* the request failed, and only after the
operator's own click. The account of *why* — a well-formed sentence
naming the baseline, the ceiling, and the remedy — went only to a
`qWarning`. And two routes to an arm never pass through that click at
all: the backend's own connect-time restore of a stored "on", and a
bridge `pan autorfgain on`. On both, the checkbox reported the wrong
state and a refusal was silent.
On a fresh install that is the **first** tick of Auto:
`kLnaDefaultGainDb` is 20 dB and the ceiling is 19 dB, so the shipped
default is always refused. A bench whose stored gains sit at −12 never
reproduces it, which is exactly why it went unnoticed here.
## The change
The sentence was already being composed. It is now kept, every outcome
of an arm request is announced on the seam, and one handler turns that
into the checkbox state, a card where the operator is looking, and a
channel a screen reader reads — **and takes it all back off again the
moment the control arms.**
| | |
|---|---|
| `src/core/backends/AutoRfGainControl.h` | `IAutoRfGainControl` gains
`lastArmRefusalReason()`, **defaulted to empty** so no other backend
changes; the contract now requires the backend to emit
`autoRfGainArmSettled` after every outcome |
| `src/core/backends/IRadioBackend.h` | new signal
`autoRfGainArmSettled(bool armed)`: fired after a refusal, an arm and a
disarm; not for a request that changed nothing |
| `src/core/backends/hl2/Hl2Backend.{h,cpp}` | stores the sentence it
already built — `tr()`'d, without the issue citation — **clears it on a
successful arm and on the radio-swap reset**, and emits the settled
signal on every outcome |
| `src/models/RadioModel.{h,cpp}` | relays `autoRfGainArmSettled` to the
GUI, caching nothing |
| `src/gui/MainWindow.{h,cpp}` | `onAutoRfGainArmSettled`: reflects the
outcome on every pan's copy of the checkbox; on a refusal, sets the
description on all of them and raises the card and the spoken
announcement once, on the active pan. The per-pan capability push also
reads a standing reason, for a pan built after the connect-time restore
settled |
| `src/gui/MainWindow_Wiring.cpp` | the click lambda now only commands
and reads back; the settled handler does the explaining |
| `src/gui/SpectrumWidget.{h,cpp}` | `showNoticeCard(detail, id, ms)`: a
transient card that takes the **caller's** id; the three card wrappers
share one private builder |
| `src/gui/SpectrumOverlayMenu.{h,cpp}` |
`setAutoRfGainRefusalDescription()` puts the reason on the Auto
checkbox's **accessible description**; arming, or hiding the control for
a family without the loop, clears it and restores the help tooltip.
`announceAutoRfGainRefusal()` speaks it once via
`QAccessibleAnnouncementEvent` |
| `src/core/AutomationServer.cpp` | `pan autorfgain on\|off` and its
status form now report `armed` and `refusal` alongside `requested` |
| `tests/spectrum_overlay_auto_rf_gain_refusal_test.cpp` | **new**,
registered; the widget half |
| `tests/hl2_gain_split_test.cpp` | the backend half: the reason, its
clears, and the settled signal |
### On the panadapter, not the status bar, and not a dialog
Not a dialog, because #4227 is stacked unclosable message boxes and a
refusal the operator asked for by clicking does not warrant one.
Not the status bar, because `MainWindow_Wiring`'s own `messageChanged`
handler hides `m_statusBarContainer` wholesale on any non-empty message
— so a timed notice there takes the TX indicator, PA temperature and
supply voltage with it for its whole duration, to explain a checkbox.
`MainWindow.cpp` and `MainWindow_Controllers.cpp` both route away from
it citing #4649.
### Its own card id
`showInterlockNotification` pins every card it raises to
`interlock.active` on a latest-wins rule — deliberately, so the radio's
authoritative denial supersedes a local preflight of the same refusal in
place (#3999). A gain refusal is not the same thing: routed through it,
the card would evict a live **Transmit disabled** card and be evicted by
the next one. `showNoticeCard` takes the caller's id, which is the
argument `showTxFilterNotification` already made for taking one of its
own. The refusal's id is `autorfgain.refused`.
### One handler for every route
`RadioModel::autoRfGainArmSettled` →
`MainWindow::onAutoRfGainArmSettled`. Three routes reach an arm outcome
and only one of them is a click:
- **the operator's click** — the signal lands synchronously inside
`setArmed()`; the lambda's readback afterwards is the #5395 guard for a
backend that settled silently;
- **the connect-time restore** in `Hl2Backend`, which settles inside its
link-up handler after it has already emitted `connected()` — so the pans
that would hear the signal may not exist yet, and the per-pan capability
push reads the standing reason onto a late pan's description (no card:
nothing just happened, that pan is catching up);
- **a bridge `pan autorfgain on`**, which arms without going near a
widget.
### And on a channel a screen reader actually reads
The checkbox carries the sentence as its accessible **description**
(what just happened to it) rather than its name (what it is), on every
pan's copy. And it is **spoken once, at the moment it happens**: a
description is read on arrival at a control, and on a refused tick the
operator is already on it, so without the `QAccessibleAnnouncementEvent`
the sentence would sit unread until they left and came back. The
clipping indicator beside the box raises the same event for the same
reason. One announcement, on the active pan, not one per copy.
### The clear lives in the widget, before the early return
`SpectrumOverlayMenu::setAutoRfGainEnabled(true)` clears the
description, and clears it **before** the function's "already there"
early return: on the operator's retry the box is *checked* before the
backend is asked, so when the arm succeeds `isChecked() == on` and the
early return fires. A clear written after that return never runs on the
one path it exists for.
The clear is a **restore, not a blanking**: the tooltip goes back to the
standing help text, hoisted into `autoRfGainHelpToolTip()` so the
constructor and the clear share one literal.
The backend clears its own copy too — on a successful arm, and on the
radio-swap reset in `applyRestoredState()`, so radio B's control never
reports a sentence composed about radio A's baseline.
### The sentence is no longer a log line
It is shown on the panadapter and read out by a screen reader, so it is
`tr()`'d, and the `#5354` citation moved to the `qWarning` — provenance
for us, noise to an operator.
## No constant moves
`kAutoRfGainMaxBaselineDb`, `kLnaDefaultGainDb` and the AD9866 fold
question stay exactly where #5752 left them. Each of the three candidate
"real" fixes settles the fold question by the back door, and #5752
merged explicitly **declining** to settle it. A visible refusal is the
one change that does not take a position.
## Tests
Two socket-free targets, both registered.
**`hl2_gain_split_test`** — the backend half. The sentence names both
numbers the operator needs and cites no issue number; a backend never
asked has no reason to give; a successful arm clears it (asserted, not
skipped); a radio swap clears it; and `autoRfGainArmSettled` fires on a
refusal, an arm and a disarm, and not on a no-op.
**`spectrum_overlay_auto_rf_gain_refusal_test`** — the widget half,
offscreen, a plain `QWidget` parent. Walks refuse → retry → arm and
asserts the description is *gone* afterwards and the help tooltip is
*back*, with the help text captured from the widget rather than retyped;
and that hiding the control for a family without the loop clears it.
Each guard has been broken on purpose and the assertion that catches it
named; the review threads carry the runs.
## What this does not cover
- **No hardware.** Socket-free throughout; nothing keyed. That the card
renders on a real refusal follows from the wiring and has not been
watched on screen — the demo `SimBackend` returns no
`IAutoRfGainControl`.
- **`showNoticeCard` and `onAutoRfGainArmSettled` have no test of their
own.** The overlay machinery underneath is covered by
`panadapter_message_overlay_test`; `MainWindow` is not cheaply
instantiable.
- **Whether the pans exist by the time the connect-time restore
settles** is not established; the capability-push catch-up is there for
the case where they do not.
- **The refusal itself is unchanged.** Whether 19 dB is the right
ceiling, or 20 the right default, is the fold question and stays open.
- **#5828** (an untick after a refusal is never recorded) is
pre-existing, filed with a repro, and deliberately not folded in here.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Jeremy [KK7GWY] <kk7gwy@aethersdr.com>1 parent 1360434 commit 03ddf48
17 files changed
Lines changed: 579 additions & 37 deletions
File tree
- src
- core
- backends
- hl2
- gui
- models
- tests
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
11486 | 11486 | | |
11487 | 11487 | | |
11488 | 11488 | | |
11489 | | - | |
| 11489 | + | |
| 11490 | + | |
11490 | 11491 | | |
11491 | | - | |
| 11492 | + | |
11492 | 11493 | | |
| 11494 | + | |
| 11495 | + | |
| 11496 | + | |
| 11497 | + | |
| 11498 | + | |
11493 | 11499 | | |
11494 | 11500 | | |
11495 | 11501 | | |
| |||
11505 | 11511 | | |
11506 | 11512 | | |
11507 | 11513 | | |
11508 | | - | |
11509 | | - | |
11510 | | - | |
11511 | | - | |
| 11514 | + | |
| 11515 | + | |
| 11516 | + | |
| 11517 | + | |
| 11518 | + | |
11512 | 11519 | | |
11513 | 11520 | | |
11514 | | - | |
| 11521 | + | |
| 11522 | + | |
| 11523 | + | |
11515 | 11524 | | |
11516 | 11525 | | |
11517 | 11526 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
56 | 56 | | |
57 | 57 | | |
58 | 58 | | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
59 | 65 | | |
60 | 66 | | |
61 | 67 | | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
62 | 82 | | |
63 | 83 | | |
64 | 84 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1130 | 1130 | | |
1131 | 1131 | | |
1132 | 1132 | | |
| 1133 | + | |
| 1134 | + | |
| 1135 | + | |
| 1136 | + | |
| 1137 | + | |
| 1138 | + | |
| 1139 | + | |
| 1140 | + | |
| 1141 | + | |
| 1142 | + | |
1133 | 1143 | | |
1134 | 1144 | | |
1135 | 1145 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6458 | 6458 | | |
6459 | 6459 | | |
6460 | 6460 | | |
| 6461 | + | |
| 6462 | + | |
| 6463 | + | |
| 6464 | + | |
| 6465 | + | |
6461 | 6466 | | |
6462 | 6467 | | |
6463 | 6468 | | |
| |||
7006 | 7011 | | |
7007 | 7012 | | |
7008 | 7013 | | |
7009 | | - | |
7010 | | - | |
7011 | | - | |
7012 | | - | |
7013 | | - | |
7014 | | - | |
| 7014 | + | |
| 7015 | + | |
| 7016 | + | |
| 7017 | + | |
| 7018 | + | |
| 7019 | + | |
| 7020 | + | |
| 7021 | + | |
| 7022 | + | |
| 7023 | + | |
| 7024 | + | |
| 7025 | + | |
| 7026 | + | |
| 7027 | + | |
| 7028 | + | |
| 7029 | + | |
| 7030 | + | |
| 7031 | + | |
7015 | 7032 | | |
7016 | 7033 | | |
| 7034 | + | |
| 7035 | + | |
| 7036 | + | |
| 7037 | + | |
| 7038 | + | |
| 7039 | + | |
| 7040 | + | |
7017 | 7041 | | |
7018 | 7042 | | |
| 7043 | + | |
| 7044 | + | |
| 7045 | + | |
7019 | 7046 | | |
7020 | 7047 | | |
7021 | 7048 | | |
| |||
7031 | 7058 | | |
7032 | 7059 | | |
7033 | 7060 | | |
| 7061 | + | |
7034 | 7062 | | |
7035 | 7063 | | |
7036 | 7064 | | |
| |||
7046 | 7074 | | |
7047 | 7075 | | |
7048 | 7076 | | |
| 7077 | + | |
7049 | 7078 | | |
7050 | 7079 | | |
7051 | 7080 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
250 | 250 | | |
251 | 251 | | |
252 | 252 | | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
253 | 257 | | |
254 | 258 | | |
255 | 259 | | |
| |||
1037 | 1041 | | |
1038 | 1042 | | |
1039 | 1043 | | |
| 1044 | + | |
| 1045 | + | |
| 1046 | + | |
| 1047 | + | |
1040 | 1048 | | |
1041 | 1049 | | |
1042 | 1050 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1429 | 1429 | | |
1430 | 1430 | | |
1431 | 1431 | | |
| 1432 | + | |
| 1433 | + | |
| 1434 | + | |
| 1435 | + | |
| 1436 | + | |
| 1437 | + | |
1432 | 1438 | | |
1433 | 1439 | | |
1434 | 1440 | | |
| |||
8007 | 8013 | | |
8008 | 8014 | | |
8009 | 8015 | | |
| 8016 | + | |
8010 | 8017 | | |
8011 | | - | |
| 8018 | + | |
| 8019 | + | |
| 8020 | + | |
| 8021 | + | |
| 8022 | + | |
| 8023 | + | |
| 8024 | + | |
| 8025 | + | |
| 8026 | + | |
| 8027 | + | |
8012 | 8028 | | |
8013 | 8029 | | |
8014 | 8030 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
763 | 763 | | |
764 | 764 | | |
765 | 765 | | |
| 766 | + | |
| 767 | + | |
| 768 | + | |
766 | 769 | | |
767 | 770 | | |
768 | 771 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
5481 | 5481 | | |
5482 | 5482 | | |
5483 | 5483 | | |
| 5484 | + | |
| 5485 | + | |
| 5486 | + | |
| 5487 | + | |
| 5488 | + | |
| 5489 | + | |
5484 | 5490 | | |
5485 | 5491 | | |
5486 | | - | |
5487 | | - | |
5488 | | - | |
5489 | | - | |
5490 | | - | |
5491 | | - | |
| 5492 | + | |
| 5493 | + | |
| 5494 | + | |
| 5495 | + | |
| 5496 | + | |
| 5497 | + | |
| 5498 | + | |
5492 | 5499 | | |
5493 | 5500 | | |
5494 | 5501 | | |
| |||
5533 | 5540 | | |
5534 | 5541 | | |
5535 | 5542 | | |
| 5543 | + | |
| 5544 | + | |
| 5545 | + | |
| 5546 | + | |
| 5547 | + | |
| 5548 | + | |
| 5549 | + | |
| 5550 | + | |
| 5551 | + | |
| 5552 | + | |
| 5553 | + | |
| 5554 | + | |
| 5555 | + | |
| 5556 | + | |
| 5557 | + | |
| 5558 | + | |
| 5559 | + | |
| 5560 | + | |
| 5561 | + | |
| 5562 | + | |
| 5563 | + | |
| 5564 | + | |
| 5565 | + | |
| 5566 | + | |
| 5567 | + | |
| 5568 | + | |
| 5569 | + | |
| 5570 | + | |
| 5571 | + | |
| 5572 | + | |
| 5573 | + | |
| 5574 | + | |
| 5575 | + | |
| 5576 | + | |
| 5577 | + | |
| 5578 | + | |
| 5579 | + | |
| 5580 | + | |
| 5581 | + | |
| 5582 | + | |
| 5583 | + | |
| 5584 | + | |
| 5585 | + | |
| 5586 | + | |
| 5587 | + | |
| 5588 | + | |
| 5589 | + | |
| 5590 | + | |
| 5591 | + | |
| 5592 | + | |
| 5593 | + | |
| 5594 | + | |
| 5595 | + | |
| 5596 | + | |
| 5597 | + | |
| 5598 | + | |
| 5599 | + | |
| 5600 | + | |
| 5601 | + | |
| 5602 | + | |
| 5603 | + | |
| 5604 | + | |
| 5605 | + | |
| 5606 | + | |
5536 | 5607 | | |
5537 | 5608 | | |
5538 | 5609 | | |
| |||
0 commit comments