feat(radio): radio-declared band capability via optional bands= discovery/status key#4027
feat(radio): radio-declared band capability via optional bands= discovery/status key#4027nigelfenton wants to merge 4 commits into
Conversation
A radio (or gateway presenting non-Flex hardware) can now declare its true band set with an optional discovery/status key, e.g. bands=2m,440,23cm (names from BandDefs). When present, the band menu is built from the declaration instead of the HF layout + ModelCapabilities flags — so an Icom IC-9700 bridged in as a FLEX-6700 offers 2m/440/23cm rather than an HF grid it cannot tune. Names are validated against BandDefs and deduplicated; unknown names are dropped. Real Flex radios never send the key: parsing yields an empty list and the band UI is unchanged, bit for bit. Accepted on both the discovery packet and the radio status line (covers direct-IP connects); mid-session declaration changes propagate through the existing infoChanged refresh.
resolveBandStackKey knows native hardware bands and configured XVTRs but not radio-declared bands, so selecting a declared 440/23cm was refused with the needs-a-transverter message. When the band is in the radio's declaration, pass the declared name through as the stack key - the declaring radio defines and honours these keys. Flex radios never declare, so their refusal path is unchanged.
Covers the wire contract both ways: a packet without the key yields an empty declaration (real Flex radios — band UI untouched), and a gateway packet's bands=2m,440,23cm is captured verbatim without disturbing the neighbouring fields. Validation/dedup against BandDefs is RadioModel's job and out of scope here.
There was a problem hiding this comment.
Nice work, @nigelfenton — this is a well-scoped, defensively-written feature. I traced the full path (discovery/status parse → RadioModel::m_declaredBands → refreshXvtr/wirePanadapter push → SpectrumOverlayMenu::setDeclaredBands → grid build → bandSelected → stack-key resolution) and it holds together. A few things I specifically verified as correct:
- Additive / no-op for real Flex.
parseDeclaredBands("")yields an empty list, and theelsebranch insetXvtrBandsis the pre-existing HF-layout + 4m/2m-capability code moved verbatim under the guard. Real radios get a bit-for-bit-identical menu. ✅ - Mid-session propagation.
handleRadioStatussetschanged = true, which reachesif (changed) emit infoChanged(), andrefreshXvtr(connected toinfoChanged) now re-pushesdeclaredBands(). So a status-linebands=change does flow through. ✅ - No leak in the rebuild. Declared buttons are parented to
m_bandPanel, which is fullydeleteLater()'d and rebuilt on eachsetXvtrBands. ✅ - Boundary validation (Principle VII). Names are validated against
kBands, deduped, and case-folded; unknowns dropped. The grid can't be driven by junk. ✅
Two things worth a look, neither a blocker:
1. The validation logic itself has no unit test. The three new checks in radio_discovery_test.cpp only assert that RadioDiscovery captures the raw bands= string verbatim — they never exercise parseDeclaredBands, which is where the actual Principle-VII guarantee lives. I'd add a small test (in the RadioModel/BandDefs test target) covering: unknown name dropped (bands=2m,junk,440 → [2m,440]), dedup (440,440 → [440]), and case-fold (2M,23CM → [2m,23cm]). That locks in the security property the PR is built around.
2. Declared bands that overlap native capability don't send the declared token. The new stackKey = bandName fallback only fires when resolveBandStackKey returns empty. For a gateway impersonating a FLEX-6700 (has2Meters/has4Meters true), declaring bands=2m,440,23cm means clicking 2m resolves natively to bare key 2 (normalizedNativeBandKey strips the m), so the command is band=2 — while 440 and 23cm fall through to band=440 / band=23cm. That mix is arguably correct-by-impersonation (a 6700's native 2m band-stack slot is legitimate), but it does mean Aether-gate must honour both the bare Flex keys (2/4/6) and the declared tokens depending on the band. Worth confirming that's what the bridge expects — a one-line comment on the ordering would save the next reader the same trace.
Minor: if firmware ever adopts a bands= status key for another purpose it'd land here, but the BandDefs allow-list makes the failure mode "drop everything unrecognised," so it degrades safely. Fine as-is.
Overall this is a clean, engine-agnostic change that keeps model truth in ModelCapabilities and only narrows the selector. Thanks for the thorough PR description and the before/after framing.
🤖 aethersdr-agent · cost: $8.7183 · model: claude-opus-4-8
There was a problem hiding this comment.
Nice, well-scoped change — thanks @nigelfenton. I traced the whole path (discovery/status parse → RadioModel::declaredBands() → setDeclaredBands → grid rebuild → band-stack key) and it holds together. A few notes below, nothing blocking.
What checks out
- Safety invariant is real.
info.bandsis only ever populated from thebands=key, which real Flex radios never send, soparseDeclaredBands("")yields an empty list and both the grid build and thestackKey = bandNamepassthrough are inert on real hardware. The "unchanged, bit for bit" claim is accurate — the guard isdeclaredBands().contains(bandName), which is always false when the list is empty. - Compiles cleanly.
parseDeclaredBandsand theSpectrumOverlayMenugrid loop both sit insidenamespace AetherSDR, so the unqualifiedkBandsreferences resolve;440/23cm/2mare all present inkBands. All 6 CI jobs (build, check-macos, check-windows, CodeQL, accessibility) are green. - Mid-session propagation works. The status-path
bandschange setschanged = true → emit infoChanged()(RadioModel.cpp:5465), which is wired torefreshXvtr(MainWindow.cpp:5013), which now re-pushessetDeclaredBands. Good. - Grid layout math (
grid->addWidget(btn, row, col % 3)thenif (++col % 3 == 0) ++row, with the trailingif (col % 3) ++row) places buttons correctly and leavesrowpointing at the next free row for the XVTR/utility blocks. Utility + XVTR rows render in both branches, matching the stated scope.
Suggestions
-
The Principle VII validation is the value of this PR, but it has no direct test. The new tests only assert that discovery captures
bands=2m,440,23cmverbatim — they exercise the transport, notparseDeclaredBands, where the actual validation/dedup/case-fold lives. I'd add a test that drives a hostile/messy declaration throughRadioModel(e.g.connectToRadioorhandleRadioStatuswithbands=2M,GARBAGE,2m,,440,70cm) and assertsdeclaredBands() == {"2m","440"}— proving unknown names drop, dupes collapse, and2M→2mcanonicalizes. That's the claim reviewers most want backed by CI, and it's reachable through the publicdeclaredBands()getter without exposing the anonymous-namespace helper. -
Design note (not a defect): declared VHF/UHF names go out as raw band-stack keys. For a band with no native/XVTR mapping, the fallback sends
display pan set <pan> band=440/band=23cm— non-standard keys that only the declaring gateway understands. That's the intended contract, but there's no signal if the gateway doesn't honor it (silent no-op on the far side rather than the visible refusal a real Flex would give). Worth a sentence in the code comment or docs making explicit that the declaring radio owns these keys end-to-end. -
Minor: overlap when the impersonated model natively has a declared band. A FLEX-6700 gateway declaring
2mwill resolve2m → "2"viaresolveBandStackKeybefore the declared-band passthrough fires, so it sendsband=2(native key), notband=2m. Correct behavior, but it means the gateway must also handle native VHF keys — fine to leave as-is, just flagging the asymmetry.
Solid contribution — the fallback-to-model-string design keeps this genuinely additive.
🤖 aethersdr-agent · cost: $7.3480 · model: claude-opus-4-8
…dering
Addresses the two non-blocking review notes on the bands= PR:
1. The Principle-VII validation had no direct test — the discovery tests only
checked that RadioModel captures the raw bands= string. Extracted
parseDeclaredBands into DeclaredBands.{h,cpp} (a light, dependency-free TU:
Qt + BandDefs.h only) and added declared_bands_test covering the security
property directly: unknown names dropped (2m,junk,440 -> [2m,440]), dedup
(440,440 -> [440]), case-fold (2M,23CM -> [2m,23cm]), whitespace, and the
empty/absent -> empty-list real-Flex path. 9/9 pass. Behaviour unchanged;
RadioModel.cpp now includes+calls the same function.
2. Added a comment at the declared-band stack-key fallback noting that
resolveBandStackKey() runs first, so a declared band that also matches the
impersonated model's native capability resolves natively (bare key, e.g.
2m -> '2' on a FLEX-6700 gateway) while model-less declared bands (440/23cm)
pass the declared token — so a bridge honours both per band.
All tests pass; Windows build clean.
|
Thanks for the careful trace — both notes addressed in c3ece31:
|
Summary
A radio may now declare the band set it can actually tune with an optional discovery/status key:
Names come from the existing
BandDefsvocabulary; they are validated, deduplicated and case-folded on parse, so a malformed declaration cannot inject junk into the band UI. When the key is present, the band selector is built from the declaration (inBandDefsorder) instead of the HF layout +ModelCapabilitiesflags. When it is absent — every real Flex radio — parsing yields an empty list and the band UI is unchanged, bit for bit. The key is accepted on both the discovery packet and the radio status line (so a direct-IP connect with no discovery packet can still declare), and a mid-session change propagates through the existinginfoChangedrefresh.Why
Gateways and emulators present non-Flex hardware to AetherSDR as a Flex model, and the model string then drives a band menu the hardware cannot honour. Two live examples from my bench, bridged through Aether-gate — a universal radio→Flex bridge I have built on top of my public flex-sim core (the Aether-gate repository goes public alongside this feature):
With
bands=, each declares its truth. Same IC-9700, stock vs this branch:Before — stock, offered the full HF grid it cannot tune:
After — exactly 2m / 440 / 23cm:
Scope / non-goals
ModelCapabilitiesremains the single source of model truth.BandDefs.RadioModel, and the onlygui/touch is the menu build.Constitution principle honored
bands=is untrusted wire input (discovery / status).parseDeclaredBandsvalidates every name againstBandDefsat the parse boundary, deduplicates, and drops anything unrecognised, so the band UI can never be driven by a malformed or hostile declaration. Absent / empty ⇒ the real-Flex path, unchanged.ModelCapabilities/ the model string. No FlexLib-sourced model truth is overridden.Test plan
cmake --build build) — Windows 11 (MSVC / Ninja) and Ubuntu 24.04 (GCC / Ninja)radio_discovery_test(with 3 new checks for the key),model_capabilities_test(212),xvtr_policy_test(38), all green on both platformsChecklist
docs/COMMIT-SIGNING.md) — ED25519, verifiedAppSettingscalls (Principle V) — N/A: no settings surface in this changeMeterSmoother— N/A: no meter UI in this changebands=key is documented in the discovery-field handling and the commit messages; happy to add a dedicated docs note if reviewers prefer🤖 Generated with Claude Code