test: drop two host-dependent assumptions that fail on macOS. Principle VIII. by ten9876 · Pull Request #6134 · aethersdr/AetherSDR

ten9876 · 2026-10-03T22:24:45Z

Refs #6113, #6008. on8st reported both failures on #6115 and #6113.

on8st sees 614 of 616 tests pass on macOS (Apple silicon, Qt 6.12.0, clean main at d9de044c). The two failures are hl2_pan_create_async_test and peripheral_auth_dialog_test. Linux CI passes both.

I have not verified this on macOS; there is no Mac here. I reproduced both failures on Linux by forcing the macOS conditions, as described below.

1. `hl2_pan_create_async_test`: "the reuser has its own chain" (test bug; the product guard is correct)

Cause. The check compared the reopened receiver's DSP with the retired one by raw address: newDsp != oldDsp. In the "deleted" run, the retired chain is freed before createPanadapter() builds the new one. The macOS allocator then returns the same 536-byte block. on8st printed the addresses and saw old == new in 30 of 30 runs. In the "still alive" run the old chain is still allocated, so the addresses differ.

Linux reproduction. I wrote an LD_PRELOAD shim (not committed) that keeps the most recently freed block of a given size and returns it to the next request of that size, whatever thread makes it. That is what a size-class allocator does. With REUSE_LO=REUSE_HI=536 (sizeof(Hl2RxDsp)), unpatched main fails 3 of 3 runs with exactly on8st's result: only [FAIL] the reuser has its own chain, in the "deleted" run only.

The product guard holds when an address is reused. In those same runs the stale frame and stale meter checks pass, as on8st saw in 60 of 60 runs. The forwards capture producer = QPointer<Hl2RxDsp>(dsp). A QPointer follows the QObject's shared reference-count block, not its address. When the old DSP is destroyed, producer becomes null. producer.data() then returns nullptr, not the old address, so a new object at that address cannot pass receiver->dsp != producer.data().

Fix (test only). Compare with oldGuard.data(), the test's existing QPointer, instead of the raw address. The retired chain has been deleted in both runs by this point; run 2 deletes it in the KeepAlive block. The check therefore means: a chain exists behind UI 1, and the retired one is gone. Its label now says that. Run 2 still exercises the identity term through the stale frame and stale meter checks, which are unchanged.

Validation (Linux, GCC, Qt 6.12). Plain build: 0 failures in 5 of 5 runs. With the reuse shim: 0 failures in 3 of 3 runs, where unpatched main failed 3 of 3.

2. `peripheral_auth_dialog_test`: "Clear code or Status is clipped at the default dialog size" (depends on the host's serial ports)

on8st saw a layout that needs about 680 px in a 640 px viewport. On Linux the same page needs 502 px in a 644 px viewport.

Likely cause. The Peripherals page has three serial-port combos: ACOM, SPE and LP-100A. Each is filled from QSerialPortInfo::availablePorts() before it is first shown, with items labelled "<portName> — <description>". The combos keep Qt's default AdjustToContentsOnFirstShow, so each one's minimum width is its longest label. That puts the host's port names into the page's minimum width.

CI runners only have short names like ttyS0 — . A Mac has cu.* and tty.* ports such as cu.Bluetooth-Incoming-Port or cu.debug-console, plus any USB-serial radio with a long description.

Linux reproduction. I injected one label into the serial combos and measured the page (probe code, not committed). Production does not need the stylesheet nudge my probe used: it fills the combos before the first size hint is computed.

widest label	combo minimum	page minimum	viewport 644
`ttyACM0 — ESP32S3 DEV` (this machine)	176	502	fits
`cu.Bluetooth-Incoming-Port —`	202	528	fits
`cu.usbserial-A10KC1QZ — FT232R USB UART`	288	614	fits
`cu.usbmodem1101 — IC-7300 USB Serial Port Interface`	340	666	fails: horizontal scroll max 22, test exits on this check

What this PR changes (test only). When the fit check fails, the test now prints:

the dialog size, the viewport width, the page minimum and the horizontal scroll maximum;
each serial combo's minimum width and its widest item.

The test still fails in that case. I have not seen on8st's port list, so the next macOS run should confirm or rule out the cause.

Not in this PR: a product fix, which needs a decision. If the diagnostic confirms the cause, the real app does the same on that Mac: the Peripherals tab scrolls sideways at the default size, and Clear and Status move off screen. The likely fix is three lines on those combos:

setSizeAdjustPolicy(QComboBox::AdjustToMinimumContentsLengthWithIcon)
setMinimumContentsLength(~18)

The closed combo would elide long labels instead of widening the grid. That is a UX choice, so it waits for the maintainer.

Request

@on8st, could you run both tests on this PR's head?

ctest -R '^(hl2_pan_create_async_test|peripheral_auth_dialog_test)$' --output-on-failure

If peripheral_auth_dialog_test still fails, please paste the lines under "clipped at the default dialog size".

🤖 Generated with Claude Code

https://claude.ai/code/session_01AADGw3HVn5FMMVrx2RQZ9g

…le VIII. hl2_pan_create_async_test compared the reopened receiver's DSP with the retired one by raw address. The retired chain is freed before the new one is allocated, and macOS hands the same block back, so the check failed on every run there while the producer guard it sits beside held. Compare with the QPointer instead. peripheral_auth_dialog_test fails on macOS at the 960x680 fit check. Each serial-port combo on the Peripherals page is as wide as its longest "port — description" label, so the page's width depends on the host's ports. The failure branch now prints the viewport, the page minimum and each serial combo's minimum and widest label, so the next macOS run names the cause. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AADGw3HVn5FMMVrx2RQZ9g

ten9876 · 2026-10-03T22:45:10Z

Admin-merged on the maintainer's approval (merge once CI is green). Head 4ee8c7466. Test-only change:

hl2_pan_create_async_test no longer assumes freed memory isn't reused. It's proven on Linux under a forced-reuse allocator shim (3/3), and the product's QPointer guard is unaffected.
peripheral_auth_dialog_test gains layout diagnostics, and still fails on a too-wide page.

@on8st, could you run both tests on main on your Mac? Please post the diagnostics' longest port label. If it confirms that a long serial-port label is widening the Peripherals tab, the dropdowns will get an ellipsis fix in a small follow-up.

aethersdr-agent

Issue fit

No fixes/closes line — the body refs #6113 and #6008 and is written against two test failures on8st reported (614/616 on Apple silicon, Qt 6.12.0, main at d9de044c). Reviewed against the stated intent.

Hunk 1: yes. I verified the load-bearing claim — that this is a test bug and the product guard is sound — from code rather than taking the 60/60 empirical result on faith. All three forwards in src/core/backends/hl2/Hl2Backend.cpp capture producer = QPointer<Hl2RxDsp>(dsp) and gate on if (!producer || !receiver || receiver->dsp != producer.data()) (lines 846–851 spectrum, 868–873 audio, 891–895 meter). The !producer term short-circuits first, so a recycled allocation at the retired chain's address cannot pass the guard: producer.data() is nullptr, not the old address. The comment at 840–845 says exactly this. So the raw-address compare in the test was the only thing that could fail under allocator reuse, and removing it loses no product coverage. The framing is correct.

Hunk 2: diagnosis only, and the body says so plainly ("The test still fails in that case"). The product fix is correctly deferred — a setSizeAdjustPolicy / setMinimumContentsLength change to three shipping combos is a UX decision, so routing it to a maintainer instead of smuggling it into a test-fix PR is the right call. It should get its own issue.

Scope

File	What it changes	Claimed by the body?	Verdict
`tests/hl2_pan_create_async_test.cpp`	One `check()` predicate + 3 comment lines; raw-address compare → `QPointer`	Yes, §1	In scope
`tests/peripheral_auth_dialog_test.cpp`	`<QComboBox>` include + a diagnostic block on the existing failure branch	Yes, §2	In scope

Test-only, two files, no product code, no new public surface, no settings/capability/protocol surface, no CHANGELOG.md entry. Nothing removed except the one predicate term, and nothing in the diff is unexplained by the body.

Socket scan (PR sources, not the working tree): neither hunk adds bind/listen/connectToHost, a peer process, or a Fake* radio/amp/tuner. Both tests pre-exist and are socket-free by assertion — hl2_pan_create_async_test keeps check(Access::transportIdle(backend), "setup opens no transport socket"), and the peripheral test's invalid-code path still asserts rejection "before opening a socket" against the in-memory PeripheralAuthStoreFake. Nothing to record beyond that.

CI. All 7 checks green on 4ee8c74, but neither test is in any ctest -R set in ci.yml, so green here means builds on three platforms and nothing more. The only run evidence is the author's own: Linux plain 5/5, Linux + LD_PRELOAD reuse shim 3/3 where unpatched main failed 3/3. The author states outright that macOS is unverified, which is the honest disclosure Principle VIII wants rather than something to hold against the PR.

Blockers

None.

Nits (non-blocking)

The new check's second conjunct is vacuous, and its label claims more than it tests. See inline on hl2_pan_create_async_test.cpp:766-770. By that line oldGuard is null in both runs, so newDsp != oldGuard.data() reduces to newDsp != nullptr — the term already next to it. "the retired one is gone" is proven by the earlier check(oldGuard.isNull() != retiredChainAlive, …), not by this one. Two ways to fix, both one edit.
Title overclaims for hunk 2. "drop two host-dependent assumptions" — hunk 2 drops nothing; the fit assertion is unchanged and still fails on that Mac. "drop one host-dependent assumption and instrument another" (or similar) would match the body, which is scrupulous about this.
The "likely cause" is a hypothesis, and these fit tests are font-sensitive as well as port-sensitive — CI's offscreen runs get DejaVu Sans with no theme resources, and macOS does not, so a combo minimum can move without any port list changing. The diagnostic handles this fine as written: printing the combo minimum and its widest label next to the page minimum discriminates between "long port name" and "wider font," so no change needed — worth stating in the follow-up issue so the next reader does not treat the port list as the only variable.

What I tried to break

"Test bug, not a product bug." The attack: if the forwards compared raw pointers, this PR would be papering over a real stale-frame leak that only manifests under allocator reuse. Checked all three connect lambdas in Hl2Backend.cpp — QPointer with a !producer short-circuit. Claim holds.
Coverage loss from dropping the identity compare. Walked the regression the old check would have caught: a backend that handed the same live Hl2RxDsp back for the reused UI. That now surfaces through check(framesOnUi1 == 1, …) / metersOnUi1.size() == 1 after the reopen, which in the retiredChainAlive run run with oldGuard non-null, plus the unblended-meter check (expectedDbm, tolerance 1e-6) at the end. The only case no longer covered is free-then-reallocate-at-the-same-address, which is the untestable one. Not a finding — but the pre-PR check was a genuine identity test in run 2, which is what nit 1's option (b) would restore.
Ordering around the KeepAlive deletion. Confirmed delete oldGuard.data() runs via Qt::BlockingQueuedConnection, so it is synchronous and the outer QPointer copy is cleared (shared refcount) before the spinUntil below it — i.e. the author's "freed by now in both runs" is right, which is precisely what makes the conjunct dead.
Null deref in the new diagnostic. scroll->widget()->minimumSizeHint() would crash if any ancestor QScrollArea had no widget. Grepped RadioSetupDialog.cpp — all three construction sites (:349/:353, :5686/:5690, :5764/:5780) call setWidget. Safe. Also checked clearButton is non-null by the guard above, widest.toUtf8() outlives the fprintf full-expression, and every %d argument is an int.
Diagnostic scope creep. dialog.findChildren<QComboBox*>() sweeps every tab, not just Peripherals, and __custom__ is also used by ConnectionPanel combos — but ConnectionPanel is not in this dialog, and the (shown)/(hidden) tag discloses off-page combos anyway. The block only runs on the failure branch and still return 1s; the pass path is byte-identical.
Could not check: anything requiring a run. I have no build and no Mac, so every statement above is reasoned from the code in the head checkout, not reproduced at runtime. The macOS behaviour itself rests entirely on on8st's report and the author's LD_PRELOAD emulation of it.

Noting for the record that this already merged as afda19ff, so the nits are follow-up material rather than merge conditions.

Recommendation

Approve with nits. Hunk 1 fixes a real test bug with the right mechanism and I confirmed the product guard it sits beside is genuinely sound. Hunk 2 is honest instrumentation of an unreproducible-here failure, with the product fix correctly held for a maintainer. Concrete next step: the two follow-ups are (a) tighten the check() label per nit 1 so the test does not assert something it cannot see, and (b) open the UX issue for the three serial combos' size-adjust policy once on8st's run on this head confirms the port-label cause. Thanks for the LD_PRELOAD reproduction and the measurement table — being explicit that there is no Mac on the bench made this much faster to review.

_{🤖 aethersdr-agent · cost: $3.9331 · model: claude-opus-5}

aethersdr-agent · 2026-10-03T22:50:15Z

+    // Compare with the QPointer, not oldDsp's raw address: the retired chain
+    // is freed by now in both runs, and an allocator may hand its block to the
+    // reuser's chain (macOS does every time), so equal addresses prove nothing.
+    check(newDsp != nullptr && newDsp != oldGuard.data(),
+          "the reuser has its own chain, and the retired one is gone");


The mechanism is right — the raw-address compare was unfalsifiable under allocator reuse, and I confirmed the product guard holds without it (Hl2Backend.cpp:846-851, :868-873, :891-895 all gate on !producer || before touching producer.data(), so a recycled address can never pass).

But as written the second conjunct is dead, and the label claims a property this check cannot establish:

Run 1 (retiredChainAlive == false): oldGuard was already asserted null ~20 lines up, at check(oldGuard.isNull() != retiredChainAlive, …) after flushDeferredDeletes.

Run 2 (retiredChainAlive == true): the if (keepAlive) block just above does delete oldGuard.data() over a BlockingQueuedConnection, so by the time control reaches here oldGuard is null too — your own comment says "freed by now in both runs."

So oldGuard.data() is nullptr in both runs and newDsp != oldGuard.data() is newDsp != nullptr, the term right beside it. And "the retired one is gone" is proven by that earlier isNull() assertion, not by this line: were oldGuard non-null and merely a different object, this check would still pass.

Two ways out, either is fine:

(a) Say only what it tests. Keeps the current coverage and drops the misleading half of the label — identity is already carried by the stale-frame and stale-meter checks above:

Suggested change

// Compare with the QPointer, not oldDsp's raw address: the retired chain

// is freed by now in both runs, and an allocator may hand its block to the

// reuser's chain (macOS does every time), so equal addresses prove nothing.

check(newDsp != nullptr && newDsp != oldGuard.data(),

"the reuser has its own chain, and the retired one is gone");

// Identity cannot be tested by address here: the retired chain is freed by

// now in both runs, and an allocator may hand its block to the reuser's

// chain (macOS does every time). The stale frame and stale meter checks

// above carry the producer-identity term instead.

check(newDsp != nullptr, "a chain is built behind the reused UI 1");

(b) Make it a real identity test again in run 2. Move this spinUntil / receiverDsp / check trio above the if (keepAlive) block. With the old chain still allocated there, newDsp != oldGuard.data() compares two live objects and is a genuine distinctness assertion; run 1 degrades to != nullptr, which the comment can acknowledge. This is the stronger option but it is a reorder reaching outside these lines, so no suggestion fence for it.

aethersdr-agent · 2026-10-03T22:50:15Z

+        // The page's width depends on the host: each serial-port combo is as
+        // wide as its longest "port — description" label. Name the widest one.


The diagnostic itself is good and I could not break it — all three QScrollAreas in RadioSetupDialog.cpp call setWidget (:353, :5690, :5780), so scroll->widget()->minimumSizeHint() is safe; clearButton is non-null by the guard above; the pass path is untouched.

Two notes, neither blocking:

The commit subject says "drop two host-dependent assumptions," but this hunk drops none — the fitsViewport assertion is unchanged and the body correctly states "The test still fails in that case." Worth retitling so the subject matches the body's own honesty.

Port labels are the likely cause but not the only host variable: these fit checks are also font-sensitive (CI runs offscreen with DejaVu Sans and no theme resources; macOS does not), so a combo minimum can move with no change to the port list. Your diagnostic already discriminates between the two — printing the combo minimum and its widest label alongside the page minimum is exactly what separates "long port name" from "wider font" — so nothing to change here. Just worth carrying into the follow-up issue so the next reader does not fixate on the port list.

on8st · 2026-10-04T05:16:36Z

The run you asked for, on clean main at 22507580 (contains this PR), macOS on Apple silicon, Qt 6.12.0: 620 of 621 pass.

hl2_pan_create_async_test passes now.
peripheral_auth_dialog_test still fails, at -j8 and alone, and the diagnostic confirms your cause:

Clear code or Status is clipped at the default dialog size
  dialog 960x680, viewport 658, page minimum 784, horizontal scroll max 126
  serial combo (hidden): minimum 344, widest item "tty.usbmodem<13 characters> — LG Monitor Controls"
  serial combo (hidden): minimum 344, widest item "tty.usbmodem<13 characters> — LG Monitor Controls"
  serial combo (hidden): minimum 344, widest item "tty.usbmodem<13 characters> — LG Monitor Controls"

I replaced the device serial in the port name by its length; nothing else is edited. The port is a monitor's USB control channel, so any Mac with such a display has it. Not checked: the app itself (I did not open the Peripherals tab), and main after 22507580.

73, ON8ST

…ite 2. Principle III. (#6140) **Update, `d60f4660` (2026-10-05):** `main` `12df6943` merged in; one conflict, in `docs/architecture/radio-capabilities-map.md`, both sides kept. No code of this PR changed. **Update, `dabf83e3` (2026-10-05):** one fix on top of the reviewed head: the 3D floor resync after a zoom is no longer adopted or stored as the operator's range. Maintainer ruling of 2026-10-05: this PR owns `dbmRanges[pan]`, its helpers and its accepted range; #6129 rebases onto it. Line references below are at `7a5ca748`; `MainWindow_Wiring.cpp` gains four lines after `:4269` since. **Update after review, `7a5ca748` (2026-10-05).** The change request is applied, and it narrows this PR: - **Only the Hermes-Lite 2 is covered now.** Storing and restoring FFT FPS and the dBm range is gated on an owner the backend declares, not on backend identity. The HL2 declares both. Icom, ANAN, RTL-SDR and the demo backend declare none and behave as on `main`. A demo run that showed FFT FPS restored at `295bff6e` will, by the code, not reproduce at this head; that was not run. - **#6129 (open) adds the same `dbmRanges[pan] = {min, max}` table** with a different accept-set (`min < max`, -250..100), a different gate (`!radioOwnsDbmScale`: ANAN, Icom) and a restore into the widget instead of the pan model. With the HL2-only opt-in no radio is claimed by both PRs any more. The two accept-sets still differ and this PR adopts whichever the maintainer rules. A trial merge of `7a5ca748` with #6129 at `04762258` conflicts in `MainWindow_Wiring.cpp` and the two touchpoint files; `ClientDisplaySettings.h` merges as text and then holds two definitions of `DbmRange`, `dbmRange()` and `saveDbmRange()`. The comparison is in the review reply. - **A stored dBm value is validated before it is narrowed to a float** (NaN, the infinities and ±1e308 are refused through the real store and reader). - The source-text pins match whole call statements. Refs #5678 (HL2 parity; rows P16, the dBm range, and the FPS half of P7 in our status table). No issue exists for this. #5111 (the Flex side) is not touched. ## Limits, first - **Where it is stored, since you ruled on that for #6119:** in the radio's settings document, not a flat key. Two optional tables, `fftFps[pan]` and `dbmRanges[pan] = {min, max}`, in the existing per-radio `ClientDisplay` feature document that already holds `waterfallRates`. No schema bump. - **Who may store and restore is declared per feature, by the backend.** FFT FPS: a new optional record, `PanFrameRateShaping`, field `clientPersistsFrameRate`, read through `RadioCapabilities::clientPersistsPanFrameRate()`. The dBm range: a new field in the existing `PanAmplitudeModel`, `clientPersistsDbmRange`, read through `RadioCapabilities::clientPersistsDbmRange()`. Same shape as `BackendPanAveraging::clientPersistsAveraging` (#5924). No new top-level capability bool: the count stays at the frozen 70. - **Two second conditions remain, and you may want them changed.** FFT FPS also needs `RadioModel::shapesDisplayRatesLocally()`, as the averaging path does: the restore sits where the local shaper is seeded. The range also needs absolute bins, inside its accessor: the client writes the range into the pan model, which is wrong on a radio whose bins are scaled by it or whose backend publishes its own range. Neither condition grants anything without the declaration. - **The waterfall-rate gate on `main` is not touched.** It still reads `shapesDisplayRatesLocally()` alone. - **Half of what was observed.** FFT AVG and Wt Avg are not fixed on the HL2. #5924 gave them a table in this document (`fftAverages`), kept only for a backend that declares `clientPersistsAveraging`; the HL2 declares it false. - **Not seen in a running app at this head, on any radio.** The defect was (HL2, 2026-10-01, 2 of 2 restarts: FPS 12 came back 25; a 120 dB scale came back -40 / 90 dB). - **It changes what the pan model holds, on an HL2 only:** an operator's range is now written into `PanadapterModel`. That range is absolute dB (dBFS under a dBm label). - **Families:** the Hermes-Lite 2 only, for both settings. It is the radio the defect was observed on, and it stores nothing itself. Flex, Icom, ANAN, RTL-SDR and the demo backend declare no owner: nothing is stored or restored for them, as on `main`. Widening to another family is one declaration in that family's backend, for its owner or the maintainer to make; #6129 claims the ANAN side of the range. - **Yours to move:** the FPS bounds 5..60 (the slider's own) and the plausibility bounds, moved unchanged from `MainWindow_Wiring.cpp:168` on `main` to `core/DbmRangePlausibility.h`. ## What was wrong on `main` A Flex stores both and reports them on connect. A radio with no display engine reports nothing (observed on the HL2, and fixed here for the HL2 only; line references are at `7a5ca748`): - **FFT FPS** stays at the widget default (`SpectrumWidget.h:1666`, 25) and pan wiring seeds the shaper from the widget (`MainWindow_Wiring.cpp:3320`). - **The dBm range:** pan wiring primes the widget from the model (`MainWindow_Wiring.cpp:4074`, `MainWindow_Session.cpp:1855`), the model starts at -130 / -40 (`PanadapterModel.h:279`), and on a radio that does not own the scale nothing ever moves it: on an HL2 the range command is attempted and dropped at the command plane (the HL2 leaves `radioOwnsDbmScale` at its default `true`, `RadioCapabilities.h:439`, so the gate at `MainWindow_Wiring.cpp:3779` does not stop it), and nothing echoes. - Of a pan's display settings only the waterfall rate (`core/ClientDisplaySettings.h:90-132`) and, since #5924, FFT averaging for a backend that declares it (`:48-88`) have a client store. ## The change - **FFT FPS**, where the backend declares the client its owner, is saved by the Display slider (before the adaptive-throttle return), Clone to all Pans and Reset to Defaults, and restored into the widget before the shaper is seeded from it. - **The dBm range**, where the backend declares the client its owner, is saved on a scale request or a finished drag, and restored into the pan model before the widget is primed from it, at both prime sites. FFT Floor Auto's own moves are not stored. - The pending-write key gains the field. It was (radio, pan), so two fields of one pan edited inside the 250 ms window replaced each other. - `dbm_range_plausibility_test` now includes the real predicate instead of a hand copy. One row added to the touchpoint manifest (240 with `main` at `12df6943` merged in). - A Flex is unchanged, and so are Icom, ANAN, RTL-SDR and the demo backend: none declares an owner, so every new reader returns nothing and every new writer returns before the store. Save and restore of each setting ask one predicate (`MainWindow::clientPersistsFftFps()`, `MainWindow::clientOwnsPanDbmRange()`), each reading one `RadioCapabilities` accessor. - `docs/architecture/radio-capabilities-map.md` has the new record and field with each family's value. ## Tests `client_display_pan_settings_test` (new, socket-free, real store): round trip per radio and pan; the Flex-shaped scope; ownership of both settings read through the two accessors off real HL2, Icom, ANAN and Flex backends (the HL2 stores and restores; the others and an undeclared backend write nothing, read nothing and leave a held document byte for byte); refusals on write and read; a newer schema; coexistence with an `fftAverages` table in #5924's shape; two fields in one deferral window. `spectrum_overlay_fft_fps_bounds_test` (new, real widget, in a `TestSettingsProfile`): the slider runs the stored bounds. **One source-text pin block, and its claim:** the order and presence of the save and restore calls inside `MainWindow` methods, which link into no test, and that the save and the restore of each setting ask the one predicate. Fail-before, one build and one run per row. The two ownership rows are at `7a5ca748`; the other rows were measured at `295bff6e` and not run again: | mutation | failing checks | |---|---| | this branch | 0 | | `main`'s wiring, and a store that neither reads nor writes the new tables | 17 | | pan wiring does not restore FFT FPS | 1 (source text) | | the store ignores who owns the value on read | 1 | | the pending-write key has no field | 2 | | both ownership accessors return true for every backend | 33 (8 each for Icom, ANAN, Flex and an undeclared backend, and the relative-bins check) | | both ownership accessors return false for every backend | 8 (7 HL2, and the absolute-bins control) | | a stored range is not validated on read | 1 | | a scale request does not adopt the range | 1 (source text) | | the writer replaces the whole document | 6, `fftAverages` and the waterfall rate among them | | the stored FPS maximum is 61 | 1 (the slider bounds test) | Full ctest, macOS, Qt 6.12.0, at `d60f4660` (`main` `12df6943` merged in, no code of this PR changed): 645 of 646 pass at `-j8`; the one failure is `peripheral_auth_dialog_test` (#6008), which also fails alone. The run before it, at `7a5ca748` (`main` `85f2a124` merged in): 644 of 646 pass at `-j8`. Two failures, neither from this PR: `peripheral_auth_dialog_test` (#6008; diagnostic on #6134), which also fails alone, and `vkamp_connection_test` (#5254), which passed when rerun alone. `hl2_receiver_count_restart_paced_test` and `libre_radar_test` passed at `-j8` in this run. Static checks (`static-checks.yml`, stdin from `/dev/null`): the script steps of the `Static checks` job pass, including the touchpoint manifest (240, up to date) and `check_capability_records.py --strict` (70 booleans, at the frozen 70); colour ratchet +0. `check_a11y` on the three changed GUI files: no findings. The `Sanitizer option configures` job was not run locally. ## With the open PRs - **#5924** (merged, and merged in here at `295bff6e`): it conflicted in `ClientDisplaySettings.h`, `MainWindow.h` and `MainWindow_Wiring.cpp`, all resolved by keeping both sides. The tables do not overlap and its pending-write key has its own prefix. - **#5919** (merged) is in this branch through `main`. - #6117 and #6115 (both merged) are in this branch. - **#6129** (open, head `04762258`): see the update block at the top. - **#6139** (open, ours): both PRs add to `RadioCapabilities.h` and to each backend's `capabilities()`, in different places. A trial merge of `7a5ca748` with its head `98be49a1` is clean; the merged tree was not compiled. ## Not verified - The restore in a running app, on any radio, at this head. - The older rows of the fail-before table at this head (see Tests). - Linux and Windows: built and tested on macOS only. - The 3D view in a running app. Its floor resync after a zoom is excluded from the adopt and the store by a guard (`dabf83e3`) that is pinned as source text only; the path was not driven with frames. Reset to Defaults does not reset the dBm range; it did not before. **Known red on `main`:** `peripheral_auth_dialog_test` (#6008) fails on this Mac at this head, as it did on clean `main` at `22507580` ("Clear code or Status is clipped at the default dialog size": a serial-port name on this host widens the page; diagnostic on #6134). Clean `main` at `85f2a124` was not built here. `hl2_pan_create_async_test` passed; `vkamp_connection_test` (#5254) failed at `-j8` and passed alone. 73, ON8ST 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

@jcmerg

#6388) Refs #5482, #6353 This is the follow-up to the two forward notes in the approving review of #6353. ## Limits, first - **macOS, offscreen platform only** (Qt 6.12.0). The pixels are from `QWidget::grab()` in a test. Nobody looked at the dialog on a screen, and there is no screenshot. - **Two themes rendered: Default Dark and Default Light.** No user theme. - **The Line Out row's styling is touched; its logic is not.** That row is @jcmerg's (#6260). His predicate, his reason text and his `applyLineoutAvailability` lambda are unchanged. Only the three stylesheets of the row gain a `:disabled` rule, the same one the Headphone row gets, so the two rows keep agreeing. - **The Front Speaker row is not touched.** It is never disabled. - **A dimmed Mute looks the same engaged or not** (from this PR's review, `c0bfd883`). An engaged Mute on an unavailable row took the unavailable text on the engaged red background. It now takes the row's own unchecked background (`color.background.1`), so the engaged state is not shown while the row is unavailable. §4a names no look for a control that is unavailable and also has state, so whether the state should stay visible is the maintainer's call. The checked state is still on the widget for accessibility clients. - **Contrast is what the token gives.** `color.control.unavailable` is 2.09:1 (dark) and 1.77:1 (light) by the style guide's own table (§4a). This PR uses the token and does not change it. - **The Line Out reason sentence is not hoisted.** It exists in one place only (`src/gui/RadioSetupDialog.cpp:4774-4775` at `9fabcbf8`); no other source file and no test repeats it. - `Refs`, not `Fixes`: closing #5482 stays your call. ## What was wrong At `9fabcbf8`, in Radio Setup → Audio → "Radio Audio Outputs", a disabled row shows it on the slider only. The other three widgets of each row carry their own stylesheet, and none has a `:disabled` rule: | widget | stylesheet | applied at (Line Out / Headphone) | |---|---|---| | caption label | `kLabelStyleTemplate`, `QLabel { color: {{color.text.primary}}; … }` (`src/gui/RadioSetupDialogCommon.h:31-32`) | `src/gui/RadioSetupDialog.cpp:4740` / `:4794` | | value label | `kValueStyleTemplate`, `QLabel { color: {{color.accent.bright}}; … }` (`RadioSetupDialog.cpp:129-130`) | `:4745` / `:4799` | | Mute button | inline `QPushButton { … color: {{color.text.primary}}; … }` plus `:checked` | `:4751-4753` / `:4805-4807` | | slider | none of its own | `:4742` / `:4796` | An explicit `color` with no `:disabled` rule also applies when the widget is disabled, so the label, the value and the Mute button render the same in both states. The review of #6353 measured 0 differing pixels on them; the test here reproduces that (below). The two headphone reason sentences were written out three times: `src/gui/TitleBar.cpp:1297-1302`, `src/gui/RadioSetupDialog.cpp:4826-4834`, and `tests/radio_setup_headphone_output_gate_test.cpp:134-139`. ## The change **The look.** A disabled label, value or Mute button of the two rows takes `color.control.unavailable`, the token §4a of `docs/style/theme-style-guide.md` names for the unavailable state: ``` QLabel:disabled { color: {{color.control.unavailable}}; } QPushButton:disabled { color: {{color.control.unavailable}}; } ``` This is the pattern already in the tree for the TX timing fields (#6166, `RadioSetupDialog.cpp:2565-2567`, `QLineEdit:disabled`) and for `WfmApplet.cpp:58`. The rules go through `ThemeManager::applyStyleSheet`, so they follow a theme switch like their neighbours. No colour literal is added. The enabled look is unchanged: the rule is appended to the existing template, and only for these six widgets. `makeValueLabel` takes an optional extra rule so `kValueStyleTemplate` keeps its single reference. The shared `applyLabelStyle` is not changed, so no other page of the dialog moves. **The wording.** `src/gui/MixerControlAvailability.h` now holds the two sentences next to the predicate, as `kHeadphoneLevelUnavailableReason` and `kHeadphoneMuteUnavailableReason`. They are `constexpr char[]`, so the header stays free of Qt. `TitleBar::setHeadphoneAvailable`, the Radio Setup lambda and both tests read them from there. The text itself is unchanged, character for character. No predicate, no capability field, no default and no user-visible string changes. `docs/user/docs/audio-settings.md` already says the controls "are dimmed with the reason in the tooltip", which stays true. ## Tests `tests/radio_setup_headphone_output_gate_test.cpp` gains `dimmedRowsLookUnavailableInBothThemes` (offscreen, no socket). For each of Default Dark and Default Light it builds the dialog on the HL2 capabilities (both rows dimmed) and on the Demo (both rows live), and for the label, value and Mute button of each row: - renders the widget's own region with `grab()` and counts differing pixels, exactly, no tolerance. It requires more than 0 between dimmed and live; - first asserts the two renders are comparable: same size, same value text, same mute state. So the enabled state is the only difference; - control 1: the same widget rendered twice differs in 0 pixels; - control 2: after a disconnect the same HL2 widgets are live again and render pixel-identical to the Demo's, and different from their own dimmed render; - requires the resolved stylesheet to carry the colour `color.control.unavailable` has in the active theme. With the change, differing pixels (measured at `6c83ac5a`, the same diff before it was rebased onto current `main`; the test that asserts them passes at the head), dimmed vs live, identical in both themes: | region | Headphone | Line Out | |---|---|---| | label | 1329 / 6840 | 852 / 6840 | | value | 410 / 2280 | 410 / 2280 | | Mute | 512 / 3800 | 512 / 3800 | **Fail-before.** Named mutation: this commit with both `:disabled` rule strings emptied, which is main's styling. Full build and ctest. All twelve regions read 0 differing pixels, as in the review: ``` PIXELS Default Dark | Headphone: label: dimmed vs live 0 / 6840 (same widget twice: 0) PIXELS Default Dark | Headphone: value: dimmed vs live 0 / 2280 (same widget twice: 0) PIXELS Default Dark | Headphone: mute: dimmed vs live 0 / 3800 (same widget twice: 0) PIXELS Default Dark | Line Out: label: dimmed vs live 0 / 6840 (same widget twice: 0) PIXELS Default Dark | Line Out: value: dimmed vs live 0 / 2280 (same widget twice: 0) PIXELS Default Dark | Line Out: mute: dimmed vs live 0 / 3800 (same widget twice: 0) (the same six lines for Default Light) FAIL! : RadioSetupHeadphoneOutputGateTest::dimmedRowsLookUnavailableInBothThemes() 'failures.isEmpty()' returned FALSE. (Default Dark | Headphone: label: dimmed renders as live Default Dark | Headphone: label: not the unavailable token #3a4a5a … Totals: 5 passed, 1 failed, 0 skipped, 0 blacklisted ``` The test could not be run on unmodified main as it stands, because it reads the sentences from the header. That is why the fail-before is a mutation and not "the test alone". **The wording.** `radio_setup_headphone_output_gate_test` compares the row against the header's two constants. `tests/mixer_control_availability_test.cpp` gains two rows: the title bar's mute and slider descriptions equal the header's sentences. Its existing rows still check the content independently ("Unavailable:", "no headphone output", "speaker button", "master volume"). ## ctest macOS (Apple Silicon), Qt 6.12.0, `ENABLE_ASR=OFF`, full suite at `-j8`, failures rerun serially, on `c0bfd883` (this PR's head): - **765 tests, 763 passed, 2 failed**, both also on the serial rerun: the dialog test below, and `vkamp_connection_test` with only its two known checks (#5254). - `peripheral_auth_dialog_interaction_test` fails, at `-j8` and serially: "Clear code or Status is clipped at the default dialog size", `dialog 960x680, viewport 658, page minimum 784`. This is the host-dependent failure of #6008 (diagnostic in #6134): the Peripherals page is as wide as the longest serial-port name on this Mac. The page minimum is 784, the same figure as on clean main here. This PR does not touch the Peripherals page. - `radio_setup_headphone_output_gate_test` (6 passed), `mixer_control_availability_test`, `titlebar_headphone_mute_test`, `unified_title_bar_test` and `radio_setup_label_theme_token_test` pass. Static checks, run locally: `tools/check_a11y.py` on `RadioSetupDialog.cpp` 9 findings before and 9 after, `TitleBar.cpp` 0; `tools/audit_colours.py` summary identical before and after; `gen_touchpoint_manifest.py --check` and `docs/gen_reference.py --check` up to date; the `tools/check_*.py --strict` set passes (`check_codeql_coverage.py` needs a build directory and was not run). 73, ON8ST 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

…I. (#6139) **Update after review, `ea2cf144` (2026-10-05):** ownership of the flag is now declared, not inferred. Only the Hermes-Lite 2 declares the client its owner (`RadioCapabilities::txWaterfallClientFlag`, `clientPersistsFlag` true). The demo, ANAN, Icom and RTL-SDR declare nothing and keep `main`'s behaviour: the toggle sends the wire text, the model stays false, nothing is stored or restored. That is narrower than this PR was. On the demo, `txwaterfall on` answers with the radio-echo note again. Full suite at this head: 644 of 645; the one failure is `peripheral_auth_dialog_test` (#6008), which fails on clean `main` too. Source line numbers below are `main` at `85f2a124`. Refs #5678 (HL2 parity; "Show TX in Waterfall", row P30 of our status table). No issue exists for this. ## Limits, first - **Nothing was keyed.** On an HL2 the toggle now sets the flag. What the waterfall draws during an over on an HL2 has not been seen, and what that radio's receive path reads while it transmits has not been measured. A `true` model flag shows neither the colour law nor real receive IQ while keyed; the demo cannot key at all. - **The fix has not been seen in a running app.** The dead toggle was (2026-10-01, bridge verb, 6 of 6 polls false). The fix is tested socket-free on the real models and dialog. - **It reaches the Hermes-Lite 2 only.** The request, the store and the restore are gated on one accessor, `RadioCapabilities::clientPersistsShowTxInWaterfall()`, which reads a declared per-feature record. Flex, Icom, ANAN, RTL-SDR and the demo leave the record absent. On Icom, ANAN, RTL-SDR and the demo the toggle is as dead as on `main`. Engaging another family is one declaration, for that family's owner or the maintainer to make. - **Yours to move:** the keyed rows take the colour law a Flex's keyed rows use (`dbmToWaterfallLevel`). On an HL2 they are absolute dB (dBFS under a dBm label); whether the law suits them is unknown until someone keys one. - **Persistence is in the radio's settings document**, not a flat key: one optional bool, `showTxInWaterfall`, in the existing per-radio `ClientDisplay` feature document. No schema bump. Written and read only where the backend declares the client its owner. ## What was wrong on `main` On a Flex the flag is the radio's: `transmit set show_tx_in_waterfall=` tells it, and its transmit status echoes it (`FlexBackend.cpp:1193`). That echo is the only writer of `TransmitModel::showTxInWaterfall()`. - The Radio Setup button sends the wire text unconditionally (`RadioSetupDialog.cpp:2717`), and so does the bridge verb (`AutomationServer.cpp:3599`). - A radio with no command plane drops it (`RadioModel.cpp:9764`) and nothing echoes, so the flag can never become true. The demo has a command plane and no handler for the flag, so nothing echoes there either. - While keyed, the pane holding the TX passband draws rows only when the flag is true (`SpectrumWidget.cpp:8488`). With it false that pane's waterfall stands still for the whole over. There is something to draw: the HL2 mutes its demodulator while keyed and the spectrum keeps running on real IQ (`Hl2RxDsp::setAudioMuted`). ## The change - `RadioModel::requestLocalShowTxInWaterfall(bool)`, the shape of the existing `requestLocalPanWeightedAverage`: where the backend declares the client the owner of the flag it applies the flag and returns true; everywhere else it returns false and the caller sends the wire text, which only a Flex answers. No raw command is added above the seam (`check_command_plane`: 400, baseline 400). - It writes the model through `TransmitModel::applyChanges`, the path an echo takes. - It stores the value per radio, on the same declaration. A disconnect clears the transmit model (`TransmitModel.cpp:43`) and no status puts the flag back, so `RadioModel::onConnected()` restores it, on the same declaration. - The Radio Setup button and the `txwaterfall` verb ask the model first. - `RadioCapabilities::txWaterfallClientFlag`: a new optional per-feature record with one field, `clientPersistsFlag` (no default). Engaged by the HL2 only. No top-level capability boolean is added (`check_capability_records`: 70 of 70). `SpectrumWidget` is not changed. Default unchanged: off. ## Tests `show_tx_in_waterfall_client_flag_test` (new, socket-free, real backends): on the real `Hl2Backend`, request, store per radio, restore on connect, and the bridge verb; on the real Icom, ANAN, RTL-SDR and demo backends, each with a `ClientDisplay` document that already holds the flag, nothing is restored, the request declines and the document is untouched; the same on a real `FlexBackend` (the echo still works) and on a backend that engages the record with the field false; the bridge verb's radio-echo note on Flex and on the demo; the store's refusals. `radio_setup_show_tx_waterfall_test` (new, production dialog, real backends): on an HL2 a click sets the model and sends no wire text; on an Icom, which declares no owner, and on a Flex the click sends the wire text and the model is not set. No source-text pin. Fail-before, one build and one run per row, failing checks in the two tests: | mutation | client flag test | dialog test | |---|---|---| | this branch | 0 | 0 | | `main`'s behaviour (request declines, connect restores nothing) | 11 | 1 slot | | the request does not write the model | 5 | 1 slot | | the connect edge does not restore | 1 | 0 | | the request is taken on a Flex too | 4 | 1 slot (Flex) | | the request does not store | 3 | 0 | | the button sends the wire text unguarded | 0 | 1 slot | | the bridge verb skips the local route | 2 | 0 | | the store ignores who owns the flag on read | 1 | 0 | | the ownership accessor is true for every backend | 40 | 2 slots | | the ownership accessor is false for every backend | 15 | 1 slot | The two accessor rows were measured at `ea2cf144`, one build and one full run each. The other rows were measured at earlier heads, before the opt-out checks existed, and were not re-run. Full ctest, macOS, Qt 6.12.0, at `ea2cf144`: 644 of 645 pass. The one failure is not ours; see below. Static checks (`static-checks.yml`, stdin from `/dev/null`): clean. `check_a11y` on `RadioSetupDialog.cpp`: 9 warnings, the same 9 as `main`, none on lines this PR touches. `check_capability_records`: 70 booleans, frozen at 70. The steps were run from this branch's `static-checks.yml`; `main`'s newer `check_build_options` steps are not on this branch and were not run. ## With the open PRs - #5924, #6117 and #6115 are merged and in this branch. #5919 is merged into `main` and is not in this branch. - #6140 (open, `7a5ca748`) adds its own records to `RadioCapabilities.h`; a trial merge with this head is clean and the merged header still counts 70 booleans. A trial merge with `main` `85f2a124` is clean. Neither merge result was built. ## Not verified - Anything keyed; see Limits. - Linux and Windows at this head. ANAN, Icom and RTL-SDR on a radio: they keep `main`'s behaviour by declaration, checked on their real backends, never connected. - A running app at this head, the demo included. - That the main window hands a restored flag to the spectrum widget. **Known red on `main`:** `peripheral_auth_dialog_test` (#6008) fails on this Mac at `ea2cf144` with the diagnostic it prints on clean `main` at `22507580` (pasted on #6134): "Clear code or Status is clipped at the default dialog size", `dialog 960x680, viewport 658, page minimum 784, horizontal scroll max 126` (a serial-port name on this host widens the page). Clean `main` was not re-run at `85f2a124`. `hl2_pan_create_async_test` and `vkamp_connection_test` passed here. 73, ON8ST 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Robbie Foust <rfoust@gmail.com>

ten9876 requested a review from a team as a code owner October 3, 2026 22:24

ten9876 merged commit afda19f into main Oct 3, 2026
7 checks passed

ten9876 deleted the fix/macos-test-assumptions branch October 3, 2026 22:44

ten9876 mentioned this pull request Oct 3, 2026

fix(rit): carry the slice id through the RIT/XIT seam; HL2 holds them per receiver. Principle I. #6109

Merged

aethersdr-agent Bot reviewed Oct 3, 2026

View reviewed changes

ten9876 mentioned this pull request Oct 4, 2026

fix(hl2): open DIGU/DIGL with AGC off, correct §18. Principle VIII. #6119

Merged

This was referenced Oct 4, 2026

fix(tx): Show TX in Waterfall works with no radio echo. Principle VIII. #6139

Merged

fix(gui): FFT FPS and the dBm range survive a restart. Principle VIII. #6140

Merged

This was referenced Oct 10, 2026

fix(gui): dim the Radio Setup headphone row. Principle VIII. #6353

Merged

fix(gui): dimmed Radio Audio Outputs rows look dimmed. Principle VIII. #6388

Merged

Navigation Menu

Repository navigation

test: drop two host-dependent assumptions that fail on macOS. Principle VIII. - #6134

test: drop two host-dependent assumptions that fail on macOS. Principle VIII.#6134
ten9876 merged 1 commit into
mainfrom
fix/macos-test-assumptions

ten9876 commented Oct 3, 2026

Uh oh!

Uh oh!

ten9876 commented Oct 3, 2026

Uh oh!

aethersdr-agent Bot left a comment

Uh oh!

aethersdr-agent Bot Oct 3, 2026

Uh oh!

aethersdr-agent Bot Oct 3, 2026

Uh oh!

on8st commented Oct 4, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		// The page's width depends on the host: each serial-port combo is as
		// wide as its longest "port — description" label. Name the widest one.

Conversation

ten9876 commented Oct 3, 2026

1. hl2_pan_create_async_test: "the reuser has its own chain" (test bug; the product guard is correct)

2. peripheral_auth_dialog_test: "Clear code or Status is clipped at the default dialog size" (depends on the host's serial ports)

Request

Uh oh!

Uh oh!

ten9876 commented Oct 3, 2026

Uh oh!

aethersdr-agent Bot left a comment

Choose a reason for hiding this comment

Issue fit

Scope

Blockers

Nits (non-blocking)

What I tried to break

Recommendation

Uh oh!

aethersdr-agent Bot Oct 3, 2026

Choose a reason for hiding this comment

Uh oh!

aethersdr-agent Bot Oct 3, 2026

Choose a reason for hiding this comment

Uh oh!

on8st commented Oct 4, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

1. `hl2_pan_create_async_test`: "the reuser has its own chain" (test bug; the product guard is correct)

2. `peripheral_auth_dialog_test`: "Clear code or Status is clipped at the default dialog size" (depends on the host's serial ports)