Repository navigation
fix(display): waterfall rate has a declared owner. Principle VIII. - #6402
Conversation
The waterfall rate was the one client display setting whose persistence
was inferred from "a backend exists and it is not a Flex"
(RadioModel::shapesDisplayRatesLocally) in both the save and the
restore. It now has a declaration of its own, in the shape FFT FPS has:
- PanWaterfallRateShaping { clientPersistsWaterfallRate }, an optional
record on RadioCapabilities with the accessor
clientPersistsPanWaterfallRate().
- MainWindow::clientPersistsWaterfallRate(), the outer local-shaping
term AND that accessor, read by scheduleClientWaterfallRateSave and
by the restore in wirePanDisplayStatus.
Behaviour is unchanged. HL2, ANAN, Icom, RTL and the demo radio each
engage the record with true, because none of them holds a line
duration; Flex leaves it absent. The same radios store and restore the
rate as before. A family stops storing it with one edit in its own
capabilities().
A separate record, not a second field in PanFrameRateShaping: engaging
that one on four more backends would turn their undeclared FFT FPS into
a declared false.
Not changed: the identity predicate (scope.hasRadioIdentity()) and the
shapedLocally parameter name in ClientDisplaySettings; the call sites
pass the declared answer into it.
Tests: client_display_pan_settings_test reads each real backend's
declaration through the accessor and runs it against the settings
store (stored and restored only by an owner; an engaged false and an
absent record store and restore nothing; a held document is left byte
for byte), and pins that the save and the restore both go through the
one predicate.
Refs aethersdr#6185
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Reviewed at 40398b89 against the head checkout. I went looking for a backend left behind, a behaviour change hiding in a "behaviour-preserving" claim, and a test that would pass against the unfixed code. I did not find any of the three. Findings below are reasoned from source — nothing was built or run beyond the static checks and the literal verification noted at the end.
1. Issue fit
Partially, by design, and the body is straight about which part. #6185 asks for one thing — ownership of the waterfall rate declared rather than inferred from "not a Flex" — and that is delivered: PanWaterfallRateShaping::clientPersistsWaterfallRate, one accessor, read by the save and the restore. The triage's two requested rulings split cleanly: (a) wide vs narrow is implemented as the triage recommended (wide); (b) hasRadioIdentity() vs radioId().isEmpty() is explicitly untouched. That is why this is Refs and not Fixes, and #6185 correctly stays open. I confirmed (b) is genuinely untouched — ClientDisplaySettings::waterfallRate/saveWaterfallRate still gate on scope.hasRadioIdentity() (src/core/ClientDisplaySettings.h:93, :114), so a serial-less RTL dongle still writes the family-wide rtl row while FFT averaging refuses it. Unchanged, and disclosed.
Test layer is right. Capability declarations are the socket-free-CTest rung per AGENTS.md, and that is where the coverage went — no new target, client_display_pan_settings_test extended, zero socket primitives (I grepped for QTcpServer/QUdpSocket/QLocalServer/QWebSocketServer/bind(/listen(/connectToHost/QProcess/Fake*: 0 hits). No socket test added or removed by this PR.
2. Scope
13 files, every one explained by the issue. No CHANGELOG.md entry (correct — it must not have one).
| File / group | What it changes | Claimed? | Verdict |
|---|---|---|---|
src/core/backends/RadioCapabilities.h |
New PanWaterfallRateShaping record, std::optional field, clientPersistsPanWaterfallRate() accessor |
Yes | In scope |
anan/, hl2/, icom/, rtl/, sim/ *Backend.cpp |
Declare the record true (one line each + comment) |
Yes | In scope |
flex/FlexBackend.cpp |
Declare std::nullopt explicitly |
Yes | In scope |
src/gui/MainWindow.h |
clientPersistsWaterfallRate() declaration |
Yes | In scope |
src/gui/MainWindow_Wiring.cpp |
The predicate; save and restore read it | Yes | In scope |
tests/client_display_pan_settings_test.cpp |
Per-backend ownership rows, engaged-false control, source-text pins | Yes | In scope |
docs/HERMES.md, radio-capabilities-map.md, aetherd-icom-civ-backend-design.md |
The new rule | Yes | In scope |
New public surface, but the benign kind. A capability field is surface third parties can bind to, so I checked the ratchet: it is a bool nested in an std::optional record, which does not move FROZEN_BOOL_COUNT. I ran the check on the head — capability-records: 70 boolean(s), at the frozen 70 — ok (shrink only). gen_touchpoint_manifest.py --check reports manifest up to date; check_test_registration.py --strict reports OK. No new UI touchpoint header, because the record rides an existing one.
Doc inventory is complete, not partial. grep -rl for the sibling (PanFrameRateShaping) and for the new record over docs/ and tools/ returns the same three files. docs/agents/settings.md — the policy source the triage cited — carries prose, not a per-capability table, so it needs no row.
Nothing is out of scope.
3. Blockers
None.
The two things I most expected to be blockers both held up:
The "behaviour-preserving" claim is true, and I checked it by enumeration rather than by trusting the table. I grepped every production IRadioBackend subclass in the tree: flex, hl2, icom, anan, rtl, sim — exactly six, no HAL-plugin or Kiwi backend among them (MainWindow_KiwiSdr.cpp / KiwiSdrManager.cpp are a separate manager path, not an IRadioBackend). shapesDisplayRatesLocally() is m_backend != nullptr && m_flexBackend == nullptr (src/models/RadioModel.h:1375-1377), so the old gate was "non-Flex". All five non-Flex backends declare true; Flex declares std::nullopt and was already excluded. The new conjunct therefore evaluates identically on every backend that exists. "Wide" here is complete coverage, not a partial migration with four families silently dropped.
The new tests are not implementation-echo. The non-owner rows depend on the store actually refusing, so I read the bodies rather than the grep hits: ClientDisplaySettings.h:93 (if (!shapedLocally || ...) return std::nullopt;) and :114 (same shape, early return). Those are real gates, so check((CDS::waterfallRate(fresh, 0, ownsRate) == 63) == expectRate, ...) is a genuine behavioural assertion, as is the held-document byte-comparison. The rate-declined / rate-owned pair isolates "engaged with false" from "absent" — the distinction a has_value()-only accessor would collapse. Against the unfixed code the new rows do not merely fail, they do not compile, since clientPersistsPanWaterfallRate() does not exist on main.
4. Nits (all non-blocking, none worth a round trip on their own)
-
src/gui/MainWindow_Wiring.cpp:3369-3370— the restore still hands the store a literal, while the save (:3189) hands it the predicate's answer:if (const auto savedRate = ClientDisplaySettings::waterfallRate( m_radioModel.settingsScope(), sw->panIndex(), true)) {
Asymmetric with
saveWaterfallRate(scope, panIndex, clientOwns, rate)two hunks up. It is harmless — the enclosingif (clientPersistsWaterfallRate())makes the literal true whenever it is reached — and it matches theclientPersistsFftFps()sibling exactly, which is the stronger argument for leaving it. Worth noting only because the same asymmetry meansclientOwnsin the save is provably alwaystrueat the point it is passed (the earlyreturnguarantees it), so the test check "and hands the store that answer" pins a distinction with no behavioural consequence. Fine as a drift guard; just not evidence of anything. TheshapedLocally→clientOwnsrename deferred past #6139 is the right call. -
tests/client_display_pan_settings_test.cpp:437-452— the source-text gap margins are tight. I measured them against the real file: predicate signature →returnis 59 againstmaxGap 70, andif (clientPersistsWaterfallRate()) {→sw->setWfLineDuration(*savedRate);is 205 againstmaxGap 220. One added comment line inside the restore block turns the second red. That is the known cost of this file's declared convention (the FFT FPS pin uses the same 220), so it is consistency rather than a new problem — but 15 characters of headroom is thin enough to mention. -
docs/HERMES.md:2219-2223— the replacement sentence drops the fact that the outer term survives. The old text named the gate (RadioModel::shapesDisplayRatesLocally()); the new text names only the declared owner. The predicate is a conjunction of both, and the triage specifically asked for the outer term to stay, so HERMES now under-describes it. The Icom design doc hunk gets this right ("is true and the backend declares..."), so it is just the one paragraph. -
PR CI compiles this test but never runs it.
.github/workflows/ci.ymlhas only fourctest -Rfilters (DV waveform,cross_needle_meter_test,mac_nr_filter_test,asr_gpu_probe_test);client_display_pan_settings_testis in none of them. All five checks are green, and the Linuxbuildandcheck-macosjobs build the whole tree, so green proves the new rows compile on three platforms — not that they pass. The execution evidence is the body's local run (771 tests, 770 pass,peripheral_auth_dialog_interaction_testfailing on cleanmaintoo per #6008) and the mutation table. Both are specific and self-consistent; neither is reproducible from here. Not a request for anything — just so the merge decision rests on the right evidence.
5. Security audit
Required by the harness (209 CodeGuard findings across 12 rule classes). The security-audit subagent ran over the flagged file list in the head checkout and found nothing introduced by this PR, which I confirmed against the diff: the change adds no string literal outside comments, no parsing, no I/O, no crypto, and no attacker-influenced data. The only value that flows is an int rate, range-validated independently at both ends (ClientDisplaySettings.h:104-106 on read, :114-116 on write, plus WaterfallRate::clampRate at every consumer).
All 12 classes are false positives, and the flagged file list is not this PR's:
- CS-VLN-CMDI-BACKTICK (the bulk) — Markdown-quoted Flex/CI-V protocol verbs inside C++
//comments, e.g.RadioCapabilities.h:245. No shell in any of those files. - CS-VLN-CMDI-SEMI / -AMP — C++ statement terminators and the
&&operator, including this PR's own:3176. - CS-VLN-SSRF-HEX — hex colour literals in Qt stylesheets (
TitleBar.cpp:1529), not hosts. - CS-INJ-homoglyph — em dashes,
§,…,²in comments and UI strings. A[^\x00-\x7F]scan of every line this PR adds returns zero hits; the diff is pure ASCII. - CS-VLN-CMDI-SUBSHELL / -PIPE / -ENVVAR, CS-SEC-PASSWORD, CS-PII-IP / -BANK / -MED — the workflow's own scripts under
pull_request(notpull_request_target), a runtimeQRandomGenerator::system()token stored in the OS keychain, and WSJT-X multicast defaults.
The list itself is the "whole-file scan of a different commit" pattern: tests/mixer_control_availability_test.cpp and tests/rx_applet_squelch_reconciliation_test.cpp are flagged but contain no waterfall reference and are not in the diff, while tests/client_display_pan_settings_test.cpp — the one test this PR actually changes — is absent from the list.
Two pre-existing, out-of-scope notes from the audit, raised only because the harness put those files in front of me. Neither is this PR's to fix and neither should hold it up:
src/gui/TitleBar.cpp:1450-1455— the GitHub releasetag_nameis interpolated unescaped into rich text.VersionNumber::parsestrips non-numeric suffixes, so a tag likev99.0.0<img src="…">can still pass the newer-version gate with the raw string reachingFramelessMessageBox::warning. Low: Qt rich text runs no script,QLabelloads no remote image, and the source is TLSapi.github.com. If that file is ever touched,latest.toHtmlEscaped()or renderinglatestVer.toString()closes it..github/workflows/static-checks.yml:488—python tools/check_a11y.py $FILESexpands PR-derived paths unquoted. Word splitting only, no shell eval, so not command injection; a path likesrc/gui/--flag.cppcould inject an argument. An array orxargs -0would close it.
6. What I tried to break
- Hunted for the backend left behind. Enumerated every
IRadioBackendsubclass in production (six) and confirmed all five non-Flex ones declare the record. No HAL-plugin or Kiwi backend exists to fall through the new gate. The "wide" declaration is exhaustive — this is the check that would have made it a regression, and it passes. - Walked all four call sites, not just the two in the diff. Three savers — Display slider (
:4906), Clone to all Pans (:3560), Reset to Defaults (:5071) — all funnel throughscheduleClientWaterfallRateSave, which self-gates, so none bypasses the predicate. The Kiwi arm ofapplyWaterfallLineDurationreturns before the save and is untouched.grepover all ofsrc/finds exactly oneClientDisplaySettings::waterfallRate(read and onesaveWaterfallRate(write, matching what the test asserts. - Checked the restore was not previously ungated. It was already inside
if (m_radioModel.shapesDisplayRatesLocally())at:3355, so the old literaltruenever reached a Flex scope and no stored Flex document could exist to restore. The diff adds the capability conjunct and nothing else — no latent Flex behaviour change. - Byte-verified every source-text literal the test asserts, rather than trusting the diff: all six present in
MainWindow_Wiring.cppand the RTL declaration present inRtlSdrBackend.cpp, with the gap margins reported as a nit above. These checks will actually fire. - Attacked the build, since the test gained two includes.
AETHER_BACKEND_RTListarget_compile_definitions(aethercore PUBLIC ...)(CMakeLists.txt:2810) and the RTL sources go intoaethercorein the same block, so the#ifdefarm both compiles and links when RTL is present and vanishes cleanly when it is not — with the always-run source-text pin as the compensating coverage.SimBackend.cppis inaethercore(:900);using namespace AetherSDRresolves bothSimBackendandrtl::RtlSdrBackend. The test is already inAETHER_SETTINGS_CONSUMERSand isolates viaTestSettingsProfile.h, so the CMake settings contract needs no change. - Suspected the new
SimBackend simBackend;of a teardown hazard — its constructor spawns a High-QoSQThread.~SimBackend(SimBackend.cpp:154-173) does aBlockingQueuedConnectionstop,deleteLater,quit(),wait(3000), the same#502order asFlexBackend, which is safe with only aQCoreApplicationand no main event loop. AndSimSignalSource's constructor only parents and configures its timer —startSession()starts it, and the test never calls it — so the test gains an idle thread, not a ~200 Hz emitter. Not a finding. - Checked
MainWindow.h:866for a moc surprise. The diff's hunk header saysprivate slots:, which would have made the new predicate a slot; the actual enclosing specifier is plainprivate:(line 412), and the declaration sits immediately besideclientPersistsFftFps(). Stale git context, not a real placement problem. - Verified the caps-map row against the table's own columns (
| Field | Flex | HL2 | Sim |):absent | engaged true | engaged trueis correct, and consistent with thepanFrameRateShapingrow above it showing Sim absent. - Could not check: anything at runtime. No build, no test execution, no GUI, no radio — and per the nit above, PR CI does not execute this test either, so the behavioural evidence is the author's local run and mutation table, which I read for coherence but cannot reproduce.
7. Recommendation
Approve with nits — with one sentence for the maintainer to notice before merging.
The engineering is clean and the body is unusually honest about its own limits (the Refs/Fixes distinction, the deferred rename, the two named gaps in the mutation evidence, "no radio was used"). A separate record rather than a sixth field on PanFrameRateShaping is the right structural call, and the reason given is correct: engaging that record for four backends would convert their honest "not declared" for FFT FPS into a declared false.
The sentence: #6185 is still labelled maintainer-review, its triage asked for two rulings before implementation, and no human has ruled on the thread. This PR implements the triage bot's own recommendation for (a) and defers (b), so merging is ruling (a) — wide. That is the low-risk direction (it is the status quo on all five families, the policy's own answer since none of them holds a line duration, and reversible with one edit per backend plus a test row), which is why this is not a blocker and not "Needs maintainer decision". But it should be a deliberate merge, not an incidental one.
Next step: confirm ruling (a), and keep #6185 open for ruling (b) — the hasRadioIdentity() / radioId().isEmpty() split that still lets a serial-less RTL dongle write the family-wide row.
Thanks for the care here — the per-family before/after table and the mutation table with its own two limits named made this genuinely fast to verify, and the completeness check (every non-Flex backend declaring) is the part that turns "behaviour-preserving" from a claim into something a reviewer can confirm.
🤖 aethersdr-agent · cost: $18.6497 · model: claude-opus-5
|
Cisco CodeGuard — static analysis of this PR (209 finding(s))
Automated static scan by Cisco DefenseClaw CodeGuard on the changed files. Advisory — some may be false positives; the review above verifies them. 🤖 aethersdr-agent · cost: $18.8501 · model: claude-opus-5 |
Review of aethersdr#6402: the paragraph named only the declared owner. The predicate is a conjunction with RadioModel::shapesDisplayRatesLocally(). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks for the review. One fix, in
Left as they are:
For the maintainer, as you put it: merging this is ruling (a), wide: the five non-Flex families keep storing the waterfall rate, as they do today. Narrowing is one edit per backend. Ruling (b), the identity predicate, is not touched, so #6185 should stay open for it; the PR says Full ctest at 73, ON8ST |
rfoust
left a comment
There was a problem hiding this comment.
I approve the wide ownership declaration. PR #6402 addresses the declared-owner part of #6185 while preserving waterfall-rate persistence for HL2, ANAN, Icom, RTL, and Sim; Flex remains radio-owned. The serial-less RTL identity rule is explicitly unchanged, so #6185 should remain open for that separate decision.
Scope
| Files | Change | Verdict |
|---|---|---|
RadioCapabilities.h and six backends |
Declare each family's waterfall-rate owner | In scope |
MainWindow.h and MainWindow_Wiring.cpp |
Apply one predicate to save and restore | In scope |
| Settings test and three architecture docs | Cover and document the rule | In scope |
No blockers. I reviewed merge result 25c40900 against main bbded449 and head 15f706c8. The ARM64 app build and client_display_pan_settings_test passed. Flipping Sim's declaration to false made five named assertions fail; restoring it returned the test to green. Current build, Windows, macOS, static, and sanitizer-configuration checks pass. I also checked the absent/false ownership paths, Flex exclusion, unchanged anonymous RTL predicate, and save/restore symmetry. The isolated offscreen app did not publish an automation socket, so I did not claim runtime demo verification; no radio was connected.
…at to do, and Stable keeps #5996. Principle VIII. windows.md said a failed external request is logged once per session. NetworkDiagnostics::watch logs once per service, host, error and HTTP status, as support-and-logging.md already says. The ANAN-G2 upgrade note now says to lower AGC-T on an older bookmark and save it again, not only that it comes back louder. The IC-9700 waterfall-rate issue (#5996) was fixed by #6402, after the 26.10.2 tag, so Stable, which describes 26.10.2, keeps the entry. Next drops it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… and re-take the Stable snapshot. Principle VIII. (#6396) ## Summary This brings the user guide (docs.aethersdr.com) up to date with everything in v26.10.2 and re-takes the Stable snapshot so the default site shows it. Docs only, apart from the TCI page prose in `tools/docs/gen_reference.py`; no application source, test or CI changes. Every page was audited against the 106 PRs in the release. Every fact that was added or kept was checked against the source at `f753dbfc4`, the v26.10.2 release commit, not taken from the PR bodies alone. ## What changed - **Menu paths.** - AetherModem, Callsign Lookup, AetherMap and FreeDV Reporter are on **Tools**. They are created on View and then moved to Tools (`MainWindow_Menus.cpp:1519-1520`, `:1569-1575`); the style guide's example says **Tools → AetherMap...**. - **Show live CW contacts** is on **View**. - Waveforms is on **Tools**, not File. - The VUDU applet button was called TXDSP. - Labels now match the source's `...` vs `…`. - **Gaps from this cycle.** - **Windows and Network Diagnostics:** Windows Firewall rules and the Network Diagnostics Windows Firewall page (#6289); the `aether.network` and `aether.nvafx` log categories (#6289, #6308). - **Remote access:** Tailscale shim revocation semantics (#6325). - **Sleep, App Nap and power throttling (#6195):** per-OS notes. - **Hermes-Lite 2:** state restore, RX audio at unity, CW at TUNE level, multi-interface discovery and a discovery troubleshooting entry (#6259, #6254, #5975, #6009, #6293). - **ANAN-G2:** AGC-T range, a "What is remembered" section, and supply voltage and link status (#6282, #6129, #6137, #6211). - **NB2** on the HL2 and ANAN-G2 (#5966). - **DVK:** WAV import requirements, the slot menu without Delete, status text and licensing (#6247, #6262, #6251). - **Copy Assist** from unprocessed audio (#6216). - **Live CW contacts** replace the decoder's contact card (#6268). - **Status-bar indicators** work from the keyboard (#6267). - **Volume:** controllers, hotkeys and TCI `volume` follow the active output (#6129, #6260). - **Peripherals:** the KPA1500 (#5905), CTR2 link extensions and the AetherKnob (#6338), TMate 2 udev (#6250), MIDI RADE Modem and FDV modes (#6031, #6038). - **RTL-SDR:** live PPM (#6082). - **Icom:** COMP and the MK2 squelch scale (#6174, #6184). - **Known issues.** Entries whose GitHub issues are closed are removed (each checked with `gh issue view`). As decided, this includes three closed today without a demonstrated fix: #5107, #4824 and #5665. - **US spelling.** `docs-style-guide.md` asked for British spelling; it now asks for US, by the maintainer's decision, and every page is converted. The generated TCI pages are converted at their source, the prose strings in `tools/docs/gen_reference.py`. Code spans, links and URLs were left alone. UI labels were checked against the source strings, and none of them are British. Two in-page anchors and two Log Analyzer rule anchors (`#bnr-is-grayed-out-…`) follow the renamed headings. - **Stable snapshot.** `snapshot_stable.py 26.10.2` was re-run on the release tree with these corrections, before `main` was merged in, so Stable carries no post-release docs. `docs/` also has `main`'s post-release pages, in US spelling. Stable alone keeps the IC-9700 waterfall-rate entry (#5996): #6402 fixed it after the 26.10.2 tag, so 26.10.2 still has it. The PDF manual already attached to the v26.10.2 release keeps the tag's docs. ## Validation - `npm ci && npm run build` in `docs/user`: success. This is the broken-link and broken-anchor check. - `node tools/docs/test_log_rules.mjs`: 707 checks passed for 45 rules. - `python3 tools/docs/gen_reference.py --check`: up to date (8 pages). - `git diff --check`: clean. - Menu paths on a build of this branch (offscreen, isolated settings, no radio), `menu list`: `Tools → AetherModem...`, `Tools → Callsign Lookup...`, `Tools → AetherMap...`, `Tools → FreeDV Reporter...`, `Tools → Waveforms...`, `View → Show live CW contacts`. ## Screenshots now out of date (not recaptured) These need a recapture pass on the radio: - **Old panadapter title strip (#6386):** `panadapter`, `main-window`, `first-session-*`, `multi-slice-two-slices`, `split-pair`, `tnf-markers`, `offscreen-vfo-indicator`, `pan-3d-stacked-trace`, `default-light-theme`. - **Pre-canon windows (#6334, #6339, #6341, #6372):** `aetherrx*`, `aethertx*`, `aethermodem-aprs`, `bnr-panel`, `connect-dialog`, `connect-by-ip-card`, `connect-icom-rows`, `connect-smartlink-card`, `rx-agc-calibration`. - **Other:** - `title-bar` (lowercase tab states, #6281); - `menu-help` (#6294); - `display-panel` (#6371); - `support-diagnostics-dialog`; - `waveforms-dialog` and `tailnet-remote-access` (show shim 0.4.0); - `about-window` (v26.10.1); - `radio-setup-qrz`. ## Found during the audit, not fixed here - **App string bug:** `TailnetShimDialog.cpp:628` tells the user to "Connect remotely with Connect → Manual"; the real path is **File → Connect to Radio... → Connect by IP**. - **ANAN AGC-T tooltips:** the RX applet and VFO tooltips show the slider position as dB, which is wrong on ANAN. #6282 lists this as a follow-up. - **MIDI Master Volume on non-Flex radios:** it is refused while PC Audio is off, whereas hotkeys, FlexControl and HID drive the line out. The docs describe the code as it is. - **RTL mode names:** `rtl-sdr.md` writes "FM-N" and "CW-R", but the source strings are `FMN` and `CWR`. Left as is until someone confirms what the mode menu displays. - **IC-705 COMP readback:** inferred from the MK2 (#6174) and not verified on an IC-705. - **Not yet documented:** the #6230 Radio Health row "Forward responses skipped", and the Store/MSIX package. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Refs #6185.
Limits, first
maintoday (HL2, ANAN, Icom, RTL, the demo radio) declares the client its owner. Flex declares none. The same radios store and restore the same value as before; only the shape of the gate changes.capabilities()(truetofalse, orstd::nullopt), and its row inclient_display_pan_settings_testflips with it.scope.hasRadioIdentity(), so a serial-less RTL dongle still writes the family-widertlrow, where FFT averaging refuses. That is why this isRefsand notFixes.shapedLocallyparameter ofClientDisplaySettings::waterfallRate/saveWaterfallRatekeeps its name. The triage suggests renaming it to match the averaging pair. fix(tx): Show TX in Waterfall works with no radio echo. Principle VIII. #6139 inserts code one line from it, so here only the call sites change what they pass. The rename is a follow-up once fix(tx): Show TX in Waterfall works with no radio echo. Principle VIII. #6139 has landed.cf0006c5) has no conflict (git merge-tree --write-tree). That merged tree was not built. fix(gui): no drop notice for a pan control that acted. Principle VIII. #6255, which touched the same files, is inmainand under this branch.mainalready has for Icom keep running.What was wrong
Read on
mainat16cfa80b, before this change; the branch now sits onaa0dd0e4:MainWindow::scheduleClientWaterfallRateSave,src/gui/MainWindow_Wiring.cpp:3145-3157. The only owner gate ism_radioModel.shapesDisplayRatesLocally()(:3148).src/gui/MainWindow_Wiring.cpp:3333-3336, under theif (m_radioModel.shapesDisplayRatesLocally())at:3321, with a literaltruepassed to the store.src/models/RadioModel.h:1366-1368, ism_backend != nullptr && m_flexBackend == nullptr: "a backend exists and it is not a Flex".So ownership of the waterfall rate was inferred from the family, where
docs/agents/settings.mdasks for a declared capability. FFT averaging, FFT FPS and the dBm range each already have a declared owner.The rule now
PanWaterfallRateShaping { bool clientPersistsWaterfallRate; }inRadioCapabilities.h, an optional recordpanWaterfallRateShaping, and the accessorRadioCapabilities::clientPersistsPanWaterfallRate(). Absent means not declared, and an undeclared backend owns nothing.MainWindow::clientPersistsWaterfallRate()isshapesDisplayRatesLocally() && clientPersistsPanWaterfallRate(), the same shape asclientPersistsFftFps(). The outer term stays, as the triage asked.A separate record, not a second field in
PanFrameRateShaping: four backends leave that recordstd::nulloptfor FFT FPS, and engaging it to carry the waterfall rate would turn their "not declared" into "declared false".No new top-level bool in
RadioCapabilities:tools/check_capability_records.py --strictstays at 70.Per family
panWaterfallRateShapingtruetruetruetruetrueTests
client_display_pan_settings_testis extended; no new test target.Hl2Backend,IcomCivBackend,AnanBackend,SimBackend,RtlSdrBackendwhere the build has it,FlexBackend) is constructed, and its declaration is read throughclientPersistsPanWaterfallRate()and run against the real settings store: an owner stores 63 and reads it back, and reads and rewrites a rate already in the document; a non-owner does neither and leaves a held document byte for byte.falsestores and restores nothing. An absent record does not either. A record engaged withtrueon otherwise empty capabilities does (control).MainWindowlinks into no test: the predicate reads the outer term and the accessor; the save asks the predicate and hands its answer to the store; the restore asks the predicate; there is exactly one stored read and one stored write of the rate in the wiring. The RTL declaration is also pinned as text, for builds without librtlsdr.Mutation evidence. One scratch commit with these mutations was built and run through the full suite.
client_display_pan_settings_testreported 29 failed checks, and every check added here is among them:shapesDisplayRatesLocally()aloneshapesDisplayRatesLocally()alonetrueto the storeClientDisplaySettings::waterfallRate(read in the wiringhas_value()only)rate-declined:rowsicom:rows, and "the waterfall rate owner does not engage the FFT FPS record"PanFrameRateShaping{false}rtl:rows, and "the RTL backend declares the client the waterfall rate owner"trueflex:rowsTwo limits of that evidence. The "does not engage the FFT FPS record" check is one conjunction and two of the mutations each break one half of it, so the run does not separate them. And one mutation was not run: an accessor that treats an absent record as an owner, which is what the
undeclared:rows guard.Results
macOS, Qt 6.12.0, full
ctest:40398b89: 771 tests, 770 pass, 1 fails, at-j8and again on a serial rerun. It also fails on cleanmainon this machine:peripheral_auth_dialog_interaction_test: "Clear code or Status is clipped at the default dialog size" (feat(peripherals): authenticate direct TGXL, PGXL, and Antenna Genius connections #6008, a long USB serial-port name on this host widens the page). No dialog file is touched.client_display_pan_settings_test,client_display_settings_testandpan_dimension_routing_testpass. The RTL backend is built in this configuration, so thertl:rows ran.Static checks, all
--strict:check_capability_records(70 of the frozen 70),check_command_plane,check_engine_boundary,check_ci_test_gate,check_test_registration,check_wdsp_test_links,check_qsettings,check_network_timeouts,check_build_options,check_qt_pin,check_shader_dialects,check_theme_seedpass.gen_touchpoint_manifest.py --checkreports the manifest up to date.Docs updated to the new rule:
docs/HERMES.md§15.2.2,docs/architecture/radio-capabilities-map.md(apanWaterfallRateShapingrow),docs/architecture/aetherd-icom-civ-backend-design.md.73, ON8ST
🤖 Generated with Claude Code