Repository navigation
feat(gui): Radio Setup is a CanonWindow workspace, with a UX pass over every page. Principle VIII. - #6413
Conversation
… sheet. Principle VIII. Part of #6239. Radio Setup (RadioSetupDialog) moves from PersistentDialog to a CanonWindow workspace, following SpotHub and AetherGPS: - No title bar: the ambient ground, the canon header and the corner close button; it resizes from every edge and closes with Esc or Ctrl+W. The footer Close button is gone. It no longer follows View -> Frameless Window, so openRadioSetupPage() creates it directly. Every close path keeps the firmware-upload and pending-removal guards (the corner button reaches closeEvent(), Esc and Ctrl+W reach done()). - Geometry persists under its old key (RadioSetupDialogGeometry); the minimum grows from 960x680 to 960x720 for the header. One sheet on the body (kRadioSetupStyle, canon/radioSetup scope) styles every page, modeled on Network Diagnostics' search / navigation / page sheet. About 290 per-widget sheets are gone; widgets carry a role instead (RadioSetupDialogCommon.h): - label tones (rsTone: caption, value, hint, heading, title, accent for a status in progress, ok, warn, error) and a status chip (rsChip); - button roles (rsButton): a plain checkable button is an on/off toggle, filled with the toggle success tokens when on; Select is one of a set, filled canon cyan; Mute reads danger when engaged; Danger and Success mark reboot / remove and firmware upload; - a disabled control takes color.control.unavailable, checked or not, so an engaged Mute on a radio that cannot mute shows no engaged red. Status lines that changed colour by string (firmware upload, calibration, oscillator lock, KiwiSDR and peripheral status) set a tone, and setTone re-polishes a live widget so the change paints at once. The remaining hex literals (copy icon, USB cable list, slice swatch borders) are tokens. The groups' caption sweeps skip a label that already has a tone. Tests: the three tests that read per-widget sheet strings now read the polished palette or the rendered pixels across a Dark -> Light -> Dark switch (radio_setup_label_theme_token_test, radio_setup_region_field_test, radio_setup_headphone_output_gate_test), and the token test adds a tone change on a shown label. Each goes red when the sheet is frozen, when a caption keeps its own sheet, when the unavailable rules are dropped, or when setTone stops re-polishing. peripheral_auth_dialog_test pins the new default size. Test targets that compile RadioSetupDialog.cpp now compile CanonWindow.cpp. Docs: the Radio Setup page describes the window and the toggle colours; docs/style lists Radio Setup among the canon windows and documents the role convention. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…heir labels, and the peripheral source dot is green, yellow or red. Principle VIII. Buttons and layout. Qt's default horizontal policy for a push button (Minimum) let every Radio Setup button grow to fill its grid cell or stretched row. Every page now goes through ButtonsFitContent (installed by wrapTabInScrollArea, re-run on each layout request so buttons added later are covered): - a button left at the default policy becomes Fixed and starts at its cell's left edge; a button given any other policy or a fixed width is left alone; - a grid or a row with nothing left that really grows (an expanding widget not held to one width, a container whose contents grow, or an expanding spacer) gets a trailing expanding spacer, so its contents pack to the left instead of spreading the spare width between columns. A layout that grows or has its own stretch is left as it was; - makeInfoField() keeps a button or status chip at its own width. The CW group puts each row's related buttons in one cell (Iambic Enabled / A / B, CWU / CWL, RX / TX decode) so they stay together, and the Peripherals Add / Remove pair sits together. Source dot. PeripheralConnectionSource::colorToken() gives the indicator dot's colour: DIRECT color.accent.success, RADIO color.accent.warning, OFFLINE color.accent.danger. - The Setup Peripherals list paints only the dot in that colour (a delegate; the item carries its source), the word keeps the list's text colour, and Connecting... has no dot. - The Tuner, Amplifier and Antenna Genius applets colour their indicator from the same helper, so OFFLINE reads red there instead of the disabled grey. - peripheral_auth_dialog_test reads the painted list: OFFLINE red, RADIO yellow, DIRECT green, each with the other two absent. It fails when the delegate is removed. Docs: the Peripherals and TGXL pages give each indicator's colour. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…. Principle VIII. - RadioSetupDialog: setFixedWidth(960) and a 720 minimum height replace the 960x720 minimum, so a wider saved geometry restores at 960. - FramelessResizer::withoutLockedAxes(): an axis whose minimum equals its maximum is not a resize zone, so a fixed-width window shows no horizontal cursor at its sides and its corners resize vertically. - radio_setup_window_size_test pins the fixed width, a resize keeping 960 and a wider saved geometry restoring at 960; frameless_resizer_test pins withoutLockedAxes(). Both go red with the change reverted. - radio-setup.md: the window's height resizes, its width is fixed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…s the window. Principle VIII. A grid of two or more caption: value pairs side by side (GPS, Transmit Timings and Interlocks, Serial Port Configuration, Phone & CW's CW) packed against the left edge, leaving the right of the page empty. Its value columns now share the spare width equally, so each pair starts its own column across the page, and each value keeps its own width at the left of its cell, beside its caption. A grid is a pair grid when it has an even number of columns, at least four, and every even column starts with a caption, nothing, or a value spanning the caption's place. Serial's Refresh button sits beside the port combo it rescans rather than in the second pair column. radio_setup_pair_grid_test pins GPS and Transmit Timings: the second pair starts in the page's right half, the pair columns line up, and a caption sits within 24 px of its value. It goes red with the spreading disabled. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Every page: - Spin boxes draw their steppers (theme-colored triangles, regenerated per theme like the combo arrow); they rendered as two specks. - Controls that sat loose between groups are in one: Audio's packet-loss and sleep switches (Connection), Transmit's Max Power, Show TX, Slice/TX Follow, TX Profile and TX Band Settings (Options), Receive's mute and binaural toggles (Audio), Serial's Open / Close / Auto-open (Connection). - US English: "behavior", "colors"; group titles in title case (Single-Click Delay, Mouse Wheel). Radio: Callsign, Nickname and Station Name end at the copy column instead of running under it, and read from their start; the FlexControl chip is the size of the buttons beside it; FW Version lines up with the fields above, the empty status line takes no room, and the CAUTION sits inside the Firmware Update group under its buttons. Network: Network Name's copy button is in the left column with the others; Network MTU is its own width; IP Configuration captions its mode and sizes the address fields for an address. Transmit: the TX profile has a caption and is no longer in Timings. Phone & CW: Microphone captions align left like every other page. Receive: "Freq Offset:" with its ppb unit beside the field (was the unit twice, one of them a column away). Filters: Low Latency and Sharp Filters sit over the slider's two ends; the digital-modes checkbox is in Filter Options, not an untitled box. Antennas: the names list is as tall as its rows, not the page. Appearance: the slack is under the last group, not a gap between two; the swatches lose the letter row that repeated their own letters. SmartLink: table headings align left over their columns. QRZ: the callsign and password fields are 260 px and Test Login sits under them. Serial: TMate 2 Display's RGB and timing fields are their own width with caption-toned labels. Peripherals: a serial port combo is no longer as wide as the longest port description, which pushed the page past the window with a long-named device plugged in (peripheral_auth_dialog_interaction_test). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…boxes. Principle VIII. - A group's title is a heading on its own line above the box, in line with its left edge and clear of it, as Add Receiver sits above its box; it was a label cut into the box's top border. - Title Case for every heading, column heading and button, matching the group titles: Configured Receivers, Add Receiver, Name / Server / Password / Type, Custom Name, SHA-256 Fingerprint, Add Receiver, Browse Public…, Forget Selected / Forget All, Clear Code, Calibrate from Current VFO. - Serial's Pin / Function / Polarity column headings take the heading tone, as the Antennas page's do. - The user docs that name those controls follow. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Accessibility check findingsThis PR touches
See Findings are warnings only — the build is not blocked. |
![aethersdr-agent[bot]](https://avatars.githubusercontent.com/in/3286087?s=60&v=4){kind=small}
There was a problem hiding this comment.
Issue fit
Partially — the implementation is good; the authorisation is the open question. There is no issue for this change: the body says "Part of #6239", and #6239's own body is explicit:
Each item below is its own issue and its own PR. Screens move to the canon only when an issue asks them to; nothing here authorises restyling a screen as a side effect of other work. Items under "Candidates" are not approved scope until the maintainer picks them.
Radio Setup is not in the Picked list and is not even listed under Candidates. And docs/style/dialog-patterns.md:263-265 — the sentence #6239's own triage pass cited when it called SystemInfoDialog and HelpDialog weak candidates — says tool and workspace dialogs keep the persistent pattern. Radio Setup is the largest workspace dialog in the app. This PR resolves that tension by editing the canon to include itself: docs/style/aethersdr-style-guide.md:116 and dialog-patterns.md:238/256/275 gain "Radio Setup" in the list of windows "the maintainer has moved to it".
That is a maintainer call, not a blocker — SpotHub (#6401) was also unpicked and merged — so it is filed below as such rather than as a violation. The engineering against the PR's own stated intent is complete and matches the AetherGPS/SpotHub recipe.
Scope
| Area | What it changes | Claimed in body | Verdict |
|---|---|---|---|
RadioSetupDialog.{h,cpp} → CanonWindow, MainWindow.cpp:3592-3605 |
The named change: canon chrome, Kind::Workspace, one sheet, geometry key reused |
Yes | In scope (pending the #6239 pick) |
| Fixed 960 width, min height 680 → 720, footer Close removed | Removes horizontal resize; a wider saved geometry restores at 960 | Yes | Needs maintainer decision — disclosed, but no issue calls the resizable width a defect |
| Title Case sweep (headings, buttons, column headings) + 9 user-doc files | ~15 user-visible labels renamed | Yes | Needs maintainer decision — grep -rniE "title case|sentence case" docs/ returns nothing, so there is no canon to cite either way; preference by default (§6) |
ButtonsFitContent + pair-grid engine (RadioSetupDialog.cpp:290-470) |
New global layout behaviour on every page, re-run on each LayoutRequest |
Yes | In scope as a UX pass; see Nit 1 |
AmpApplet.cpp:968, TunerApplet.cpp:704, AntennaGeniusApplet.cpp:305 |
OFFLINE dot color.text.disabled → color.accent.danger; AG disconnected goes grey → red |
Yes | Needs maintainer decision — this changes meaning outside the Radio Setup window. Red-as-danger for a device that is merely absent is a semantic choice, and no issue calls grey-OFFLINE a defect |
PeripheralConnectionSource.h:56-66 colorToken() |
New shared accessor, three call sites de-duplicated | Implied | In scope, good factoring |
FramelessResizer::withoutLockedAxes() + frameless_resizer_test |
Global to every FramelessResizer user; correct for any fixed-axis window |
Yes | In scope, tested |
kInfoRightLabelWidth 160 → 128, copy-column spacing, per-page fixes |
Layout polish | Yes | In scope |
dialog-patterns.md:228 removes DxClusterDialog from the persistent list |
Cleanup of a line #6401 should have removed | No | Not a blocker — correct, but it belongs to #6401; worth a line in the body |
docs/style/* adding Radio Setup to the canon window lists |
Amends canon to cover this window | Partly | See Issue fit |
Nothing in the diff is unexplained, and no CHANGELOG.md entry was added — correct.
Blockers
None. The window conversion itself holds up under the checks in the last section.
Nits (non-blocking)
1. ButtonsFitContent::packLayout permanently packs any layout whose only growable widget is explicitly hidden at build time — src/gui/RadioSetupDialog.cpp:361-371 + 404-412.
if (w->isHidden() || w->maximumWidth() <= w->minimumWidth()) {
continue;
}fit() runs from the ButtonsFitContent constructor, i.e. inside wrapTabInScrollArea() (:472, and buildDeferredTab() at :9122) before the page is in the visible tree — so "hidden" means explicitly hidden. Two things are explicitly hidden at that moment: serialCustomEdit (RadioSetupDialog_Peripherals.cpp:981, setVisible(...currentData() == "__custom__")) and every non-current page of a QStackedWidget (ui.addressStack, ui.portStack), which QStackedLayout hides.
I probed the policies with a standalone Qt 6 program rather than assuming them: QLineEdit is Expanding (expands horizontally), QComboBox is Preferred (does not). So in the serial row, the combo cannot grow and the line edit is skipped for being hidden → canGrowHorizontally() returns false → addStretch(1) plus setProperty("rsPacked", true), which is one-shot, so the later LayoutRequest when the widget is shown returns early. In a QBoxLayout, once any item has stretch > 0 the stretch-0 items get no extra width whatever their policy.
Failure scenario (reasoned from code, not run): Peripherals → a serial device → set Connection Type: Serial → Custom…. The custom-port field comes up at its sizeHint instead of filling the row. Same shape for netIpEdit when the mode stack starts on Serial and the operator switches to Network. Cheapest fix is to not treat an explicitly-hidden growable widget as non-growable:
if (QWidget* w = item->widget()) {
if (w->maximumWidth() <= w->minimumWidth()) {
continue;
}(dropping w->isHidden() ||), or else clear rsPacked and re-evaluate on a child's QEvent::Show/Hide.
2. PeripheralListDelegate assumes U+2007 has the same advance as ● — src/gui/RadioSetupDialog_Peripherals.cpp:216-219.
// The item as usual with the dot's glyph blanked to a space of the
// same advance, then the dot drawn in its colour at that position.
QString blanked = text;
blanked[lineStart] = QChar(0x2007); // figure spaceFIGURE SPACE is a digit-width space; ● (U+25CF) is typically wider. Where they differ, the word after the dot shifts left relative to the painted dot and can crowd or overlap it. pixelsOfToken() in the new test only proves the right-coloured pixels exist, so it would not catch this. Comparing fm.horizontalAdvance(QStringLiteral("●")) with the chosen blank — or dropping the dot from opt.text and reserving its advance explicitly — would make the comment true.
3. The Direct/Radio arms of the new dot test bypass the production wiring — tests/peripheral_auth_dialog_test.cpp:1872-1874. list->item(0)->setData(Qt::UserRole + 1, ...) pokes the role directly (and re-hardcodes kSourceRole's value), so only the OFFLINE arm exercises RadioSetupDialog_Peripherals.cpp:2277, where the role is actually derived. A wrong ui.state.source → role mapping would stay green on two of three states.
4. aWiderSavedGeometryRestoresAt960 seeds the new "x,y,w,h" form only — tests/radio_setup_window_size_test.cpp:56. A real pre-PR RadioSetupDialogGeometry is a base64 saveGeometry() blob; the legacy-form path exists in CanonWindow (Network Diagnostics went through it) but the clamp is not asserted against it. Not this PR's to add, just noting the gap in the migration claim.
5. Esc still skips m_peripheralRowSavers. closeEvent (RadioSetupDialog.cpp:1872-1883) runs the savers for the uncommitted "user cleared IP" edits; Esc / reject() goes through done() (:1862-1869), which does not. Pre-existing — both overrides are on main at RadioSetupDialog.cpp:1281 and :1291 — but the footer Close button that used to be a saver-running path is gone, and the docs now name Esc as a primary close (docs/user/docs/radio-setup.md:28).
Socket tests
None. No added or modified test in this PR opens, binds, listens or connects; the new radio_setup_window_size_test and radio_setup_pair_grid_test are offscreen widget tests over a bare RadioModel, correctly registered in tests/tests.cmake and added to AETHER_SETTINGS_CONSUMERS.
Other tools' findings
CodeGuard: every finding I was handed is outside this PR's diff. The 287 are in .github/workflows/*.yml and src/core/backends/SquelchLevelScale.h; git diff --stat main...HEAD shows this PR touches neither — this is a whole-file scan of main's tip (consistent with CodeGuard's known behaviour on these reviews). The CS-VLN-CMDI-BACKTICK hits inside RadioSetupDialog.cpp are the lexical match on Markdown-quoted text in // comments.
The required security-audit (CodeGuard) pass returned no findings in the PR's own changes, and I confirmed its three load-bearing conclusions against the tree: the auth-code field keeps QLineEdit::Password + the aetherSensitiveValue grab/bridge mask and its reveal-generation re-conceals on endpoint edit, page change and finished() (which still fires — CanonWindow is a QDialog); the new delegate reads only opt.text and kSourceRole, and item text is label + address + stateWord, never codeEdit; and WA_DeleteOnClose is not new — MainWindow::showOrRaisePersistent (MainWindow.h:2267) already set it, so the lifetime model is unchanged.
What I tried to break
- A use-after-free from the
PersistentDialog→CanonWindowswap. Didn't find one.WA_DeleteOnCloseis unchanged fromshowOrRaisePersistent; the slot isQPointer<RadioSetupDialog>(MainWindow.h:1671) and all five external uses null-check;wireRadioSetupDialogSignalsusesdlgas the context object for the one MainWindow→dialog connection (MainWindow.cpp:3654); every lambda connect inRadioSetupDialog{,_Peripherals}.cpphas a context object that dies with the dialog; andcloseEvent/doneboth end with the base call as the last statement. - An orphaned behaviour from leaving
m_persistentDialogs. The tracked set's only consumer is the frameless-propagation loop insetFramelessWindow(MainWindow.cpp:10247), whichCanonWindowdeliberately ignores — and the docs now say so. - Both close guards on every path.
confirmFirmwareClose()andm_peripheralRemovalPendingare checked in bothdone()andcloseEvent, and the new ✕ goes throughQDialog::close(CanonWindow.cpp:74), the same entry the old footer Close used. - Missing or misresolving tokens. Parsed both theme files:
color.canon.*(incl.muted,cyan,ink,raised,line,lineHi,onAccent),color.accent.{danger,warning,success},color.control.unavailableandcolor.toggle.success.*all resolve in Default Dark and Default Light. Light'scanon.mutedis#506070on a near-white ground, so the hint tone is readable there. - Compile breaks from the
colorToken()refactor. All three applets includePeripheralConnectionSource.h;AntennaGeniusApplet.cpp:299has theusing PeripheralConnectionSource::Source;the unqualifiedSource::Directneeds; the"{{%1}}"+.arg(token).arg(size)placeholder order is right in both applets. - A static-check regression. On the PR head:
check_test_registration --strictOK,check_ci_test_gate --strictOK,check_a11y.pyonRadioSetupDialog.cpp9 warnings — identical tomain's 9, same lines. - Weakened tests. The three rewritten assertions got stronger, not weaker: the region/label tests now read the polished palette across a Dark → Light → Dark round trip instead of matching a sheet string, and the headphone gate test reads rendered pixels of
color.control.unavailableinstead ofstyleSheet().contains(...). - Could not check: anything at runtime. I am headless — no build, no test run, no GUI. Every finding above is reasoned from the code at
/tmp/aetherclaude/pr-6413, with two exceptions I did verify empirically: the Qt default size policies in Nit 1 (standalone Qt 6 program) and the static-check runs. Worth saying plainly: CI green here is compile-only.check_ci_test_gatereports 7 tests selected by-Rout of 763 registered, and none of them is a Radio Setup test — so the two new tests and three rewrites have run only on the author's machine.
Recommendation
Needs maintainer decision. The conversion is careful work — the guards survive every close path, the one-sheet migration is token-clean in both themes, the new tests are real discriminators rather than assertions of the implementation back to itself, and withoutLockedAxes() is a genuinely reusable fix. Three things want your ruling before merge: (1) Radio Setup is not a picked item on #6239 and dialog-patterns.md currently says workspace dialogs keep the persistent pattern — this PR edits that sentence to include itself; (2) the fixed 960 px width and the Title Case sweep are preferences with no issue behind them; (3) the OFFLINE-dot colour change reaches three applets outside this window. If you want the bundle split, the natural seam is the canon move + withoutLockedAxes in this PR and the UX pass plus the applet dot in their own. Nit 1 is worth fixing either way.
Thanks — this is a large surface handled with unusual care, and the body's disclosure of what was and wasn't verified live made the review much faster.
🤖 aethersdr-agent · cost: $20.4486 · model: claude-opus-5
|
Cisco CodeGuard — static analysis of this PR (295 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: $20.6223 · model: claude-opus-5 |
ten9876
left a comment
ten9876
left a comment
There was a problem hiding this comment.
PR #6413 — Approve with nits; no blockers. Reviewed head 2f04ce19357055a1b0772a53ad033f581e6ff488 against tracking #6239 and the approved direction in RFC #6226. The CanonWindow migration, preserved geometry key, height-only resizing, page layout changes and removal/firmware close guards meet the stated intent. This is a disclosed visual/UX migration, not a bug-fix PR carrying an undisclosed redesign.
| File/group | Change | Claimed? | Scope verdict |
|---|---|---|---|
| RadioSetupDialog, Common, Peripherals | Canon workspace, shared role sheet, page layout and labels | Yes | In scope |
| MainWindow | Create/reopen the canon workspace directly | Yes | Necessary integration |
| FramelessResizer | Suppress edge cursors on locked axes | Yes | Necessary sizing support |
| PeripheralConnectionSource; Tuner/Amp/AntennaGenius applets | Shared DIRECT/RADIO/OFFLINE colors | Explicitly | Disclosed companion change |
| Tests/tests.cmake | Regression coverage and CanonWindow linkage | Yes | In scope |
| Style docs and ten user-guide pages | Window behavior, labels and source colors | Yes | In scope |
The six commits all concern this migration; no unrelated commits, new wire verbs, settings keys, dependencies or changelog entries were found.
Blockers: None.
Non-blocking nits:
-
USB cable item colors do not follow a live theme change —
src/gui/RadioSetupDialog.cpp:7746.refreshListresolves tokens intoQListWidgetItemforeground brushes, but it only reruns for cable events and initial population. A socket-free injected disabled CAT cable remained#8598b4after Dark → Light, whilecolor.canon.mutedbecame#506070. This leaves part of the stated live-theme migration incomplete; the old literal colors were also static, so this is not a new functional regression. Refresh the item brushes on theme changes, preserving the selection as the existing lambda already does. After definingrefreshList, the connection can be:connect(&ThemeManager::instance(), &ThemeManager::themeChanged, cableList, refreshList);
-
“Radio Side” uses the on/off role instead of the selection role —
src/gui/RadioSetupDialog.cpp:5644. Only “Client Side” receivesButtonRole::Select. Clicking “Radio Side” in the isolated demo rendered a green selection in both the source rule and the Light-theme screenshot; selecting its peer renders cyan. This contradicts the new explanation of these mutually exclusive buttons indocs/user/docs/radio-setup.md:30. Apply the same role immediately afterradioSideBtn->setCheckable(true):setButtonRole(radioSideBtn, ButtonRole::Select);
Verification and attacks: Built the PR head on Linux/Qt 6.12 with the review skill's shared-core, -g1 configuration. All 16 selected CTest registrations passed after restoring the source. The focused coverage included firmware close paths, pending peripheral removal, theme switching, dimmed audio controls, geometry, pair grids, locked axes and all nine peripheral-auth UI cases. Deliberately breaking fixed width, pair spreading, locked-axis filtering, source-dot color, caption/value colors and unavailable colors made the corresponding tests fail. A separate temporary socket-free probe confirmed actual legacy base64 geometry restores to 960×750. Every temporary source edit was restored.
Drove only the review-owned offscreen pr6413-review instance, with fresh isolated settings, an explicit socket and TX disabled. Bridge identity returned txAllowed:false; the radio remained DEMO-0001, model AetherSDR Demo. A resize requesting 1300×880 returned actual:{w:960,h:880}. Closing/reopening and restarting the whole process retained 960×880. Visited twelve demo-reachable settings pages, switched Dark/Light, injected a demo disconnect, and reproduced nit 2. Both review-owned processes were stopped; the operator's instance was untouched.
Engine-boundary strict checking and the touchpoint manifest passed. The bot's nine accessibility warnings also exist on the merge base. A separate code-review skill was unavailable. Hardware-only pages, real firmware uploads/device convergence and native Windows/macOS interaction were not exercised; the close/state guards were tested with socket-free injection.
CI checks succeeded, but the pull-request runs were created at 17:36:47 UTC, before the observed main tip 2e1217f7 at 18:00:00 UTC. With non-strict protection, that green does not verify the merge with current main. Local results above are for the exact PR head, not a fresh merge result.
Recommendation: Approve with nits. The migration survived the lifecycle, persistence and regression-test attacks. Correct the two color details when convenient; obtain the maintainer's approval and normal current-main validation before landing.
…e. Principle VIII. From the #6413 reviews: - OFFLINE takes a new color.accent.offline token (orange: #ff7a1a dark, #c4560a light) in the Radio Setup device list and the Tuner, Amplifier and Antenna Genius applets, so it is not read as transmit red. - Escape keeps the uncommitted Peripherals address clears, as the corner button does: done() runs the row savers too. - The USB cable list re-resolves its item colors on a theme switch, leaving the list and the cable being edited as they are. - Radio Side takes the Select role like Client Side. - The device list's source dot is drawn in its own cell with the item clipped around it, instead of over a figure space whose width differs. - US spellings in comments and docs (color, center). Tests: - peripheral_auth_dialog_test drives the dot through the link probes (connected, relayed, neither), not by setting the item's role. - external-ag closes with Escape as well. - radio_setup_label_theme_token_test pins the cable colors across a switch and the record-mode roles. - radio_setup_window_size_test restores a wider legacy saveGeometry() blob at 960. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Both reviews' findings are answered in Maintainer decisions (the bot's "Needs maintainer decision" items):
@aethersdr-agent nits
@ten9876 nits
The branch-added comments and docs now use US spellings (color, center). |
There was a problem hiding this comment.
Reviewed at the PR head 3b5186c against the checkout at /tmp/aetherclaude/pr-6413. Everything below is reasoned from code — I have no build, no test run and no way to drive the GUI, so nothing here was reproduced at runtime, and I say so where it matters.
Issue fit
There is no linked issue; the PR is its own write-up and says only "Part of #6239". That tracker is the problem rather than the authority: Radio Setup is not in #6239's "Picked: tool windows on CanonWindow" list (AetherRX/TX, AetherModem, the Settings libraries, AetherMap, Connect to Radio, AetherGPS, AGC-T), and the body of #6239 says, verbatim:
Each item below is its own issue and its own PR. Screens move to the canon only when an issue asks them to; nothing here authorises restyling a screen as a side effect of other work. Items under "Candidates" are not approved scope until the maintainer picks them.
I read the whole #6239 thread (list_issue_comments): the only ruling in it is the #6273 one that moved Waveforms and Network Diagnostics "by maintainer direction, outside this issue's one-PR-per-surface plan" — so a ruling of that kind is how this would be authorised, and none is visible for Radio Setup. The engineering is strong and the body is unusually honest (it even names the five pages it could not check live); what it is missing is the pick. Details under Blockers.
Scope
| File / group | What it changes | Claimed? | Verdict |
|---|---|---|---|
src/gui/RadioSetupDialog.{h,cpp}, RadioSetupDialogCommon.h, RadioSetupDialog_Peripherals.cpp |
PersistentDialog → CanonWindow::Kind::Workspace; one canon sheet + role properties replacing ~290 per-widget sheets; plus a UX pass (Title Case renames, fixed 960 width, footer Close removed, button packing, pair grids, per-page layout fixes) |
Yes | Two changes in one PR, on a surface #6239 has not picked → needs maintainer decision (blockers 1 and 2) |
src/gui/MainWindow.cpp:3592-3605 |
Direct creation + WA_DeleteOnClose instead of showOrRaisePersistent() |
Yes | In scope, and byte-for-byte the pattern NetworkDiagnostics (MainWindow.cpp:4664) and AetherGPS (MainWindow_Wiring.cpp:221) already use |
src/gui/FramelessResizer.{h,cpp} + tests/frameless_resizer_test.cpp |
withoutLockedAxes() |
Yes | In scope — the fixed width needs it |
src/gui/AmpApplet.cpp, TunerApplet.cpp, AntennaGeniusApplet.cpp, PeripheralConnectionSource.h |
Applet source-dot colours now share colorToken(); OFFLINE goes from color.text.disabled to orange |
Yes (body + docs) | Outside the Radio Setup surface. "Applet panel and applets" is an unpicked candidate in #6239 → maintainer call, bundled |
resources/themes/default-{dark,light}.json, src/core/ThemeSeedGenerated.cpp |
New color.accent.offline (+ color.orange.500) |
Yes | New public theme surface → maintainer decision (small). Seed is consistent with the JSON and alphabetically placed, so tools/check_theme_seed.py should hold |
docs/style/dialog-patterns.md, aethersdr-style-guide.md, theme-style-guide.md |
Adds Radio Setup to every canon list; documents the role vocabulary and the new token; removes the stale DxClusterDialog (SpotHub) line |
Partly ("Docs: the user docs follow…") | The DxCluster removal is a correct stale-line cleanup. The rest amends the canon in the same PR that takes the exception — see blocker 1 |
docs/user/docs/*.md (10 files) |
Labels, colours, window behaviour | Yes | Accurate against the code I read. versioned_docs/ correctly untouched |
tests/ (3 new, 4 updated) + tests/tests.cmake |
New targets registered, added to AETHER_SETTINGS_CONSUMERS, CanonWindow.cpp added to every RadioSetup-linking target |
Yes | In scope and complete |
No dead additions: every new target is registered and every new symbol has call sites. No deleted guard: the firmware-upload confirmation, the pending-removal gate and the address-clear savers all survive on every close path.
Socket tests: none added, none removed. The new seam RadioSetupDialog::swapPeripheralLinkForTest() (src/gui/RadioSetupDialog.h:127-131) is probe injection into the existing in-process device struct — no socket, no peer, no fake firmware. Recording it here as the notification channel.
Blockers
1. src/gui/RadioSetupDialog.h:46 — Radio Setup is not a picked canon surface, and the PR edits the canon that says so.
class RadioSetupDialog : public CanonWindow {Three canon sentences point the other way, and all three are in the tree you can read today:
- #6239 (quoted above): candidates "are not approved scope until the maintainer picks them", and nothing there authorises restyling as a side effect. Radio Setup is not even in the candidate list; the nearest row is the unpicked "Persistent tool dialogs (one PR per dialog)".
docs/agents/gui.md:85-87: "Apply it to UI your issue already changes; never restyle other screens on your own, and never change a base token's value in a migration PR." (The base-token half is clean — no base token's value moves here.)docs/style/dialog-patterns.mdon main: "UseCanonWindowonly for windows the style guide covers: short-lived windows, and the tool windows the maintainer has moved to it" — anddialog-patterns.md:238/:256/:275in this PR insert "Radio Setup" into exactly those sentences. A PR that satisfies a rule by rewriting the rule is the thing that needs a human to sign, not a reviewer.
This is not a judgement on the work — if the maintainer picks Radio Setup, the implementation looks ready. It is a request for the pick to exist and be linked before merge, as #6273's ruling was.
2. The "UX pass over every page" is a second change, and part of it is preference-class.
#6239 asks for one surface per issue per PR. Beyond the chrome move, this PR also: renames visible labels to Title Case ("Clear code" → "Clear Code" at src/gui/RadioSetupDialog_Peripherals.cpp:909, "Single-click delay" → "Single-Click Delay" at src/gui/RadioSetupDialog.cpp:9504, "Browse public…", "Add receiver", "Forget selected/all", "Calibrate from current VFO", "Mouse wheel"); pins the window to a fixed 960 px width; removes the footer Close button; moves group titles above their boxes; and repacks every button row. None of these points at an issue calling the old behaviour a defect, so by §6 they are preferences needing their own issue and ruling.
Two items in the pass genuinely do point at a defect and would survive on their own, with the right framing:
- the serial-port combo sizing to the longest port description and pushing the Peripherals page past the window (
RadioSetupDialog_Peripherals.cpp:1000-1004) — a real clipping bug; - the Callsign/Nickname/Station Name fields running under the copy-button column (
RadioSetupDialog.cpp:713-716).
Suggested split: the chrome move (once picked) in one PR; the two clipping fixes in a second, each citing its symptom; the Title Case / layout / footer-Close preferences in a third behind a maintainer ruling.
Nits (non-blocking)
docs/user/screens.json:1683and:1751— the PR hand-edited the<img ... alt="...">text indocs/user/docs/slice-colors.md:13andkiwisdr-and-web-888.md:26, butscreens.jsonstill carries the old wording ("Single-click delay", "Browse public… and Add receiver").tools/docs/embed_screenshots.pywrites the page alt fromscreens.json, so--checkreports those pages out of step and the next run reverts the edits. Update the twoaltstrings inscreens.jsoninstead of (or as well as) the pages. Related: AGENTS.md:185-189 asks a UI change to re-shoot stale shots or name them in the PR body; both of these shots show the old title bar, the footer Close button and the old labels, and the body names none as stale. (embed_screenshots.py --checkis not wired into any workflow, so CI will not catch this.)src/gui/MainWindow.cpp:10237-10239— now false:// RadioSetupDialog frameless propagation flows through the // m_persistentDialogs loop below (#2781) — all four entry points use // showOrRaisePersistent so the dialog is always tracked there.
openRadioSetupPage()no longer goes throughshowOrRaisePersistent(), so the dialog is never inm_persistentDialogs. The behaviour is right (a canon window must not follow View → Frameless Window, and the docs now say so) — the comment andMainWindow.h:998's "persistentRadioSetupDialog" wording should say that instead.src/gui/RadioSetupDialog.cpp:1870and:1884— on the ✕ path the savers run twice:closeEvent()runs them, thenCanonWindow::closeEvent→QDialog::closeEvent→reject()→done()runs them again. Reading the two savers (RadioSetupDialog_Peripherals.cpp:1246,:1309), each returns early when the saved IP is already gone, so the second pass is a no-op anddisconnectNow()cannot fire twice. Reasoned from code, not observed — worth a one-line comment saying the double-run is known and idempotent, since a future saver that is not idempotent would break silently here.src/gui/RadioSetupDialog_Peripherals.cpp:233-247— the delegate's dot cell is computed fromopt.fontmetrics andtextRect.left() + margin, then the style's own paint is clipped out of exactly that rectangle. If the style ever lays the text out in a different font thanopt.font, or the view is right-to-left (text starts at the right edge while the dot is drawn at the left), the clipped-out cell and the drawn dot part company — a blank gap plus a dot in the wrong place. Low risk and the pixel test covers LTR with the sheet's font; mentioning it because the failure mode is silent.docs/user/docs/peripherals.md:111-112— "is concealed again when you change page" does not hold for the top-level Setup page switch: the conceal pass is wired only to the inner peripheraldetailStack'scurrentChanged, so a revealed keychain code stays in the field (EchoMode Normal) while the Peripherals page is off-screen. Pre-existing on main, exposure is bounded (field not visible, grabs masked, bridge returns<hidden>, every close path conceals) — credited to the security-audit pass below. Since this PR is already editing that bullet list, it is the cheap moment to either wire the conceal to the top-level page change or soften the sentence.src/gui/RadioSetupDialogCommon.h:62and:74— thekNames[]arrays are indexed by the enum with no size check. I diffed everyrs{Tone,Button,Chip}="…"selector in the sheet against both arrays and all 15 match, so nothing is broken today; astatic_assert(std::size(kNames) == static_cast<int>(Tone::Accent) + 1)would keep a future enum value from indexing off the end.- Light-theme contrast, FYI only:
color.accent.offlinelight#c4560ameasures ≈4.5:1 on white, against ≈3.1:1 for the existingaccent.warningamber it sits beside — the new token is the better of the two, so no action. (Hand-computed WCAG, not measured by a tool.) - Prose spelling:
docs/style/theme-style-guide.mdchanges "centred" → "centered" andradio-setup.md"colours" → "colors" in body text. I could not find a spelling rule in AGENTS.md/CONTRIBUTING.md, so this is convention only — the surrounding docs are consistently British in prose ("minimises", "colours") while UI labels are US.
What I tried to break
- Lifetime, the one thing the
WA_DeleteOnCloseswitch really changes. The dialog is now destroyed on close where it used to live for the process. I chased the dangling references:MainWindow.h:1671is aQPointer, the four other call sites (MainWindow.cpp:7305,MainWindow_Controllers.cpp:282/301/340/2769) all null-check it,ThemeManager's tracked-widget map removes entries onQObject::destroyed(ThemeManager.h:372-373),ControlAvailabilityRegistryholdsQPointers, andPeripheralAuthStoreguards every async keychain callback withQPointer<QObject> context(PeripheralAuthStore.cpp:34/113/135/189), so an in-flight reveal or clear cannot land on a dead dialog.wireRadioSetupDialogSignals()connects only dlg→this or this→dlg-as-context, so every connection dies with the sender or the context. The one hole I could not close by reading: a programmatic reopen in the same event-loop turn as the ✕ would find theQPointerstill set (the delete is a posteddeleteLater) andshow()a window about to vanish. No such call site exists — every entry point is a user action — so I am not filing it. - Closing while a firmware upload runs.
confirmFirmwareClose()(RadioSetupDialog.cpp:1799) latches onm_firmwareClosePromptOpen, so the closeEvent→reject→done chain does not stack two modals, and it callsuploader->cancel()before returning true — so destroying the dialog afterwards does not abandon a live transfer. Unchanged by this PR; I checked because the new deletion made it newly reachable. - Geometry migration. The PR deliberately reuses the old
RadioSetupDialogGeometrykey, which held PersistentDialog's base64saveGeometry()blob.CanonWindow::restoreGeometryFromSettings()(CanonWindow.cpp:153-183) handles both forms,setFixedWidth(960)runs in the constructor so either path is clamped, andradio_setup_window_size_testpins both forms at 960 with a CONTROL assertion that the restore actually ran. I tried to find a saved value that would strand the window (zero/negative width, a position on an unplugged monitor) — both are handled. - The new tests, read as an attacker. They are not self-agreeing:
radio_setup_label_theme_token_testreads the polished palette across a Dark→Light→Dark round trip (a freshly built dialog is identical either way, so construction-time assertions could not see the defect), and — the thing I specifically went looking for — it keeps a floor at lines 149-161 asserting every eager-page caption and line edit has an emptystyleSheet(), which is what stops the old "untracked literal sheet" regression class from becoming invisible now that collection is by role.peripheral_auth_dialog_testcounts pixels of exactly the token colour and asserts zero pixels of the other two tokens, driven through the injected link probes. The window-size test assertsminimumWidth == maximumWidth == 960with the height left free. All of these go red if the change is reverted. - Role plumbing. Every
rsTone/rsButton/rsChipvalue used in the sheet matches thekNames[]arrays (15/15) — a typo'd role would style nothing and fail no test, so I checked by hand.withoutLockedAxes()was checked against the one window it changes (minimumSize()==maximumSize()on the width only, so the sides stop resizing and the corners go vertical) and against the siblings that installFramelessResizerwith no explicit min/max, where it is a no-op. Thecanon/radioSetupscope resolves through the generic"canon"scope block in the theme JSON (default-dark.json:394) — there is no per-window entry forspotHuboraetherGpseither, so the new scope needs no JSON addition. - Label renames breaking a consumer. I grepped every renamed string across
src,tests,toolsanddocs: no test, no automation-bridge selector and no tool matches on them. The only stale copies aredocs/user/screens.json(nit above) anddocs/user/versioned_docs/(correctly frozen). - Couldn't check: anything visual — the 960 px layout actually fitting every page, the group-title placement, the spin-box arrows, the dot alignment inside a real list row. The body says 16 pages were verified live on DEMO-0001 and that Calibration, RTL Receiver, HL2 Hardware, Droop Correction and APD were not; I have no way to confirm either half. CI at
3b5186cwas queued / in_progress on every lane except "Sanitizer option configures" (success) and a skipped release job — so there is no green here to lean on, and theButtonsFitContentevent filter plus the delegate are both exactly the kind of code that compiles everywhere and misbehaves on one platform's style.
Security audit
A CodeGuard audit ran over the flagged file list (security-audit subagent, repo root /tmp/aetherclaude/pr-6413). No findings introduced by this PR. The auth-code block is logic-identical to main's — the only deltas are the removed applyEditStyle/kButtonStyle calls and the "Clear code" → "Clear Code" label; reveal generation-counting, endpoint re-check, PeripheralAuthStore::clear result handling and the "a revealed vault value is never replayed" rule are untouched, there is no logging in the file, and no path routes a code into status text, tooltips, accessible descriptions or list items.
One needs-human, pre-existing on main, credited to security-audit (CodeGuard): the top-level page-switch conceal gap at src/gui/RadioSetupDialog_Peripherals.cpp:1793 vs the navigation handler at src/gui/RadioSetupDialog.cpp:1496 — see the peripherals.md:111 nit above.
Refuted and dropped: every CS-VLN-CMDI-* and CG-PATH-001 hit is in .github/workflows/*.yml, which this PR does not touch — the files in the checkout are main as of the branch point, and the only real interpolations into run: are github.base_ref, a numeric PR number and a grep -c integer, with no pull_request_target and no head_ref. CS-PII-IP / CS-VLN-SSRF-PRIV-192 are the "e.g. 192.168.1.100" placeholder and RFC 5737 TEST-NET addresses in tests; CS-SEC-PASSWORD is QLineEdit::Password on keychain-backed fields; CS-INJ-homoglyph is the ●/⚙/… UI glyphs; CS-VLN-SSRF-HEX is the request-ID base constants. The C++ CS-VLN-CMDI-BACKTICK hits are the known lexical match on Markdown-quoted text inside // comments.
Recommendation
Needs maintainer decision. The code is good — the lifetime switch follows the established canon pattern exactly, the geometry migration is handled in both saved forms, and the three new tests are genuinely discriminating with explicit controls, which is better than most PRs of this size manage. What is missing is authority, not quality: #6239 says a screen moves to the canon only when an issue asks it to and that candidates are not approved scope until picked, Radio Setup is not picked, and this PR adds itself to the canon docs that say so. On top of that it bundles a page-by-page UX pass whose Title Case renames, fixed width and removed footer Close are preference-class under §6.
Concrete next step: ask the maintainer for a pick for Radio Setup (a comment in #6239, or a ruling in this thread as PR #6273 got), and in the meantime split the two clipping fixes — the serial-port combo and the copy-column overrun — into their own PR, since both point at a real defect and need no ruling. Thanks for the unusually candid body, including naming the five pages you could not verify live; that is what let this review focus on the parts I could actually check.
🤖 aethersdr-agent · cost: $19.8893 · model: claude-opus-5
|
Cisco CodeGuard — static analysis of this PR (475 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: $20.0783 · model: claude-opus-5 |
Accessibility check findingsThis PR touches
See Findings are warnings only — the build is not blocked. |
Resolves RadioSetupDialog.cpp against main's tone/shown-text API (aethersdr#6413) and regenerates the touchpoint manifest.
…ts. Principle XI. (#6421) ## Summary #6199 moved the remaining slice and transmit controls onto typed `IRadioBackend` requests. Those requests are dispatched only to a connected radio with an open TX session. Three tests outside every focused set still described the retired route, and they fail on `main`'s unfiltered Full Suite at `d3ab349f3` (run 38302350478): - **`fm_tone_presentation_contract`** grepped `RadioModel.cpp` for the removed `&SliceModel::fmDtcsCommandIssued` forward and `m_backend->setSliceFmDtcs(`. It now checks the current route: `SliceModel` publishes the normalized `DtcsSetting` request, and the Icom adapter's `Field::FmDtcs` case applies it with `setSliceFmDtcs(`. - **`icom_family_test`**: the operator's mic edit ran against a backend whose connect attempt to TEST-NET never completes, so TX admission stays closed exactly as it does in production until `onConnected()`. The edit now runs on a backend rebuilt through the existing socket-free `RadioModel::rebuildBackendForTest()`, with its link marked up and an unstarted `IcomSession` (no streams or sockets). - **`hl2_tci_signaling_test`**: the HL2 link is marked up before the TCI client promotes a slice, as it is when a TCI client talks to a live radio. No Metis client is started. Every assertion message and expected value is unchanged; only the preconditions moved. This does not touch `radio_setup_headphone_output_gate_test`, which has been failing since #6413 (#6418). Closes #6420. ## Constitution principle honored **Principle XI**: the fix is demonstrated by CI re-running on `main`. These tests are the Full Suite's evidence for the #6199 route, and each one now fails when the backend dispatch it pins is removed. ## Test plan - [x] Linux x86-64, Qt 6.12: all three pass, plus the 23 focused #6199 tests (26/26). - [x] Negative controls: Icom `Field::MicGain` returning `Unsupported` fails `icom_family_test`'s mic assertion; HL2 `Field::TxSlice` returning `Unsupported` fails both `hl2_tci_signaling_test` promote assertions; removing the Icom `setSliceFmDtcs(` dispatch fails the contract. Each restored passes. - [x] `check_test_registration.py`, whitespace. - [ ] `main` Full Suite on the squash commit (expected: only the pre-existing #6418 failure remains). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…g. Principle XI. (#6424) Full Suite has been red on main since #6413 (37fbdbc). Every run since then fails one test: ``` FAIL! : RadioSetupHeadphoneOutputGateTest::dimmedRowsLookUnavailableInBothThemes() Default Dark | Headphone: label: not the unavailable token #3a4a5a Default Light | Headphone: label: not the unavailable token #b0bcc8 ``` ## Cause The app is correct: the dimmed Headphone label does change color (460 of 2160 pixels differ from the live one). The test, though, also checks that pixels of the *exact* `color.control.unavailable` value appear in the render. That only happens when a glyph pixel is fully covered. #6413 moved captions to the canon 12px style. The CI image (Ubuntu 24.04, DejaVu) renders with subpixel antialiasing, and under it no pixel of "Headphone:" is fully covered. "Line Out:" happens to keep one. Reproduced locally with a DejaVu-only fontconfig set to `rgba=rgb`, `lcdfilter=lcddefault`. It gives 461/303 differing pixels, against CI's 460/303, and the same failure. With `hintnone`, the Mute check breaks the same way. ## Fix Test-only. `main()` sets `QFont::NoAntialias` on the application font, so every glyph pixel is the label's color under any fontconfig. ## Verification - Passes under three font setups: the LCD (CI-like) config, LCD + `hintnone`, and native Arch fonts. - Mutation: pointing `QLabel[rsTone="caption"]:disabled` at `color.canon.muted` still fails the test with "not the unavailable token". 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…6435 B, C). Principle VIII. (#6652) ## Summary Implements items B and C of RFC #6435. Item A, the transmit-path acknowledgement, is being reworked in the RFC and stays out of this PR. B and C are in separate commits. 1. **Red-main attribution** (Principle VIII). The `full-suite.yml` tracking issue now gives: - The commit's source PR and its author. A direct push says so instead of naming an unmerged PR. - How the commit compares with its parent's full-suite run: - A green parent makes this commit the first red one. - A red parent is compared test by test against the failed list filed for the parent run the verdict links, matched by run id. Tests failing here but not there are named as starting at this commit. If nothing fails here that passed there, it says "look earlier". - If either side has no complete test list, the verdict states both possibilities instead of guessing. That covers a missing parent entry, two lists for one run, a build failure, "unknown" and a list capped at 25 names. - The failed list keeps every test in CTest's FAILED summary, whatever its status. A crash (`ILLEGAL`, `NUMERICAL`) can no longer make a partial list look complete. - A cancelled parent run is skipped. - A pointer to the #4703 flake umbrella, and a reminder that reverting is a human decision. This adds `actions: read` and `pull-requests: read` to the workflow's permissions. A new Static checks step, `node tools/test_full_suite_attribution.mjs`, runs the issue step's script against stubbed GitHub data, plus the failed-list producer against a CTest summary. 2. **Intent-first review** (Principle VII). - The PR template gets an **Intent** section, and `/pr-review` checks it against the linked issue before reading the code. - Asking for validation inside trusted code, or for abstraction the stated intent doesn't need, is a nit, not a blocker. Part of #6435. Supersedes the B and C commits of #6436. ## Intent After this merges: - A red main names its likely PR, and which of its failing tests started at that commit. - Reviews start from what a PR is for. Out of scope: - The transmit-path gate (RFC #6435 item A). - Any automatic revert. ## Constitution principle honored - **Principle VII:** the review rule cites "so the interior can treat parsed values as sound". - **Principle VIII:** a red main carries its own attribution. ## Test plan - [x] `node tools/test_full_suite_attribution.mjs`: 18/18. - Against the previous commit it fails 6 of 18. - Breaking the run-id match fails the two-run case. - Accepting the first of several lists fails the re-run case. - [x] Ran the issue step's script against real main commits, with `github` stubbed by read-only `gh api` calls: - `37fbdbc` (#6413): green parent, so it is the first red commit. - `d3ab349` (#6199): names `fm_tone_presentation_contract`, `icom_family_test` and `hl2_tci_signaling_test` as starting there. #6436's version said "look earlier" here. - `68ccf459f` (#6619): names `show_tx_in_waterfall_client_flag_test` and `transmit_seam_drop_notice_test`. - `933f2fdee` (#6621): names `hl2_tci_signaling_test`. - [x] `full-suite.yml` and `static-checks.yml` parse as YAML. - [ ] The step only runs on the next red main, so the live issue write is unexercised until then. ## Checklist - [x] Commits are signed (`docs/COMMIT-SIGNING.md`) - [x] No `src/` changes - [x] Documentation updated: PR template, `/pr-review`. No `CHANGELOG.md` entry. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_012U8wmr3AQGVrmQLp2zNBwa --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Part of #6239. Radio Setup moves from a
PersistentDialogto aCanonWindowworkspace, following SpotHub and AetherGPS. This PR also includes a UX pass over every page.Window
FramelessResizer::withoutLockedAxes()turns off resizing along a locked axis, so the sides show no horizontal resize cursor.RadioSetupDialogGeometry.Styling
canon/radioSetup) styles every page.rsTone), a status chip (rsChip) or a button role (rsButton).setTonerepolishes a live widget, so a theme switch or status change paints at once. The USB cable list re-resolves its item colors on a switch.Layout
ButtonsFitContent).color.accent.offlinetoken (#ff7a1adark,#c4560alight), so OFFLINE isn't read as transmit red. The Tuner, Amplifier and Antenna Genius applets use the same colors.Docs
Tests
New:
radio_setup_window_size_test: the fixed width, a resize that keeps 960, and a wider saved geometry restoring at 960.radio_setup_pair_grid_test: GPS and Transmit Timings spread across the page and keep each pair together.frameless_resizer_test::testWithoutLockedAxes.radio_setup_window_size_testalso restores a wider legacysaveGeometry()blob at 960.radio_setup_label_theme_token_test: USB cable colors across a theme switch; Radio Side and Client Side share the Select role.Updated:
peripheral_auth_dialog_testreads the painted source dots, driven through the link probes (connected, relayed, neither). The external-ag case also closes with Escape.New tests go red with their change reverted. Locally, 25 Radio Setup, peripheral and theme tests pass;
tools/check_a11y.pyreports the same 9 Radio Setup warnings as main.Verified live on the demo radio (DEMO-0001) for Radio, Network, GPS, Audio, Transmit, Phone & CW, Receive, Filters, Antennas, Transverters, USB Cables, Peripherals, Appearance, SmartLink, QRZ and Serial.
These pages need radio-specific hardware and were not reviewed live: Calibration, RTL Receiver, HL2 Hardware, Droop Correction and APD.
🤖 Generated with Claude Code