fix: route native 4m/2m band selection#2831
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes FlexRadio FLEX-6500/6700 native 4m (70 MHz) and 2m (144 MHz) band routing by making band-stack key resolution capability-aware, preventing user-configured XVTRs named “2m/4m” from being shadowed, and ensuring high-VHF direct frequency entries preselect the correct band stack before tuning.
Changes:
- Extend
XvtrPolicy::resolveBandStackKey()to optionally considerModelCapabilitiesso native keys"4"/"2"are only used when the connected model reports native 4m/2m support; add regression tests. - Ensure configured XVTR buttons carry an explicit
X<n>stack-key hint throughSpectrumOverlayMenu→MainWindowso native VHF doesn’t override same-named XVTRs. - Route RX applet direct-entry through
MainWindow’s tune policy (with band-stack preselect) and treat display-style entries like144.200.000as explicit MHz for >54 MHz tuning.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/xvtr_policy_test.cpp | Adds coverage for capability-gated native 2m/4m key resolution. |
| src/models/XvtrPolicy.h | Extends band-stack key resolver API to accept ModelCapabilities. |
| src/models/XvtrPolicy.cpp | Implements capability-aware native band key normalization for 2m/4m. |
| src/gui/VfoWidget.cpp | Treats explicit “MHz-style” inputs as MHz above 54 MHz for direct entry. |
| src/gui/SpectrumOverlayMenu.h | Adds per-button stackKey and extends bandSelected to carry a hint. |
| src/gui/SpectrumOverlayMenu.cpp | Emits bandSelected with stackKey when known (configured XVTR buttons). |
| src/gui/RxApplet.h | Adds directEntryCommitted signal to route tuning via MainWindow policy. |
| src/gui/RxApplet.cpp | Uses explicit-MHz detection and emits directEntryCommitted instead of tuning directly. |
| src/gui/MainWindow.h | Adds band-stack preselect result type and helper declaration. |
| src/gui/MainWindow.cpp | Introduces band-stack preselect-before-tune for commanded direct entries; threads stackKey hints through band selection path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| static bool isExplicitMhzEntry(const QString& rawText, const QString& normalizedText) | ||
| { | ||
| const int dot = normalizedText.indexOf(QLatin1Char('.')); | ||
| if (dot < 0) { | ||
| return false; | ||
| } | ||
|
|
||
| // `14.225.000`, `144.200.000`, and `440.100.000` are MHz.kHz.Hz | ||
| // display-style entries. Keep them as MHz even before the slice is | ||
| // already on an XVTR/high-frequency band. | ||
| if (rawText.count(QLatin1Char('.')) >= 2) { | ||
| return true; | ||
| } | ||
|
|
||
| // Single-dot entries with a normal MHz field are also explicit MHz | ||
| // (`14.225`, `144.200`, `440.100`). Preserve the historic HF shortcut | ||
| // where `14225.0` means 14.225 MHz by treating five-plus leading digits | ||
| // as kHz-style input. | ||
| return dot <= 4; | ||
| } |
| static bool isExplicitMhzEntry(const QString& rawText, const QString& normalizedText) | ||
| { | ||
| const int dot = normalizedText.indexOf(QLatin1Char('.')); | ||
| if (dot < 0) { | ||
| return false; | ||
| } | ||
|
|
||
| if (rawText.count(QLatin1Char('.')) >= 2) { | ||
| return true; | ||
| } | ||
|
|
||
| return dot <= 4; | ||
| } |
![aethersdr-agent[bot]](https://avatars.githubusercontent.com/in/3286087?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thanks for tackling this, @rfoust — the XvtrPolicy capability-gating and the X<n> stack-key plumbing through SpectrumOverlayMenu are a clean design and the new test coverage in xvtr_policy_test.cpp exercises the right cases (native wins on capable radio; same-named XVTR still resolves on non-capable). The BandStackPreselectResult flow with the 250 ms re-tune timer is a sensible way to let the band-stack settle before the final retune.
A few things to flag before merge:
1. Branch is behind main — would revert PR #2826 (FFT Line slider, #2722)
git merge-base main HEAD for this branch is 26008b2c, which predates 4d5b4d7a ("Honor FFT Line slider on QPainter (software) render path"). The diff against current main includes a src/gui/SpectrumWidget.cpp hunk that:
- removes the
drawLine/ "width 0 = Off" guard from both the heat-map and solid-fill branches ofdrawSpectrum - removes the cosmetic
QPenwithsetWidthF(m_fftLineWidth)and hardcodesQPen(..., 1.5)back in - drops the
update()call fromsetFftLineWidth
These changes aren't in your PR description or stated file list, and they undo the freshly-landed software-render path fix. Please rebase on main so the merge keeps #2722's behavior. (The file isn't even listed in the PR's "Files changed" overview in the description, so I assume this is purely a rebase artifact rather than intentional.)
2. Duplicated isExplicitMhzEntry() — Copilot's flag is valid
Confirmed: the helper is byte-identical in src/gui/VfoWidget.cpp:146 and src/gui/RxApplet.cpp:108, both as file-static. The parsing rule ("≥2 dots → MHz; single dot with ≤4 leading digits → MHz") is non-obvious and exactly the sort of thing that will silently diverge the next time someone tweaks one side. Suggest hoisting into a small shared utility (e.g. src/util/FrequencyEntry.{h,cpp} or alongside the existing parsing in BandSettings/XvtrPolicy) and having both widgets call it. Bonus: that's a natural home for a unit test of the heuristic itself.
3. resolveRadioBandStackKey thin wrapper
The new free function in MainWindow.cpp:382 just forwards to XvtrPolicy::resolveBandStackKey with m_radioModel.capabilities(). Two call sites is borderline; given the wrapper has no logic, calling XvtrPolicy::resolveBandStackKey(bandName, xvtrs, m_radioModel.capabilities()) directly would be clearer and matches how the rest of MainWindow uses the policy module. Not a blocker.
4. PR description mentions TX antenna behavior, but no antenna code is in the diff
Keep TX antenna behavior radio-authoritative by reflecting
txant/tx_ant_list; no local antenna forcing.
I don't see any txant / tx_ant_list changes in the diff. If that was meant to be in this PR, it got lost; if it landed elsewhere, please drop the bullet from the description so reviewers and the changelog aren't misled.
Capability-gating logic and the direct-entry routing through MainWindow::applyTuneRequest look correct to me — once the rebase is sorted and the helper is shared, this should be in good shape. 73
rfoust
force-pushed
the
codex/fix-vhf-uhf-band-select
branch
from
ba739a9 to
b1bfc6c
Compare
|
Addressed the review feedback in
Verified locally with:
|
rfoust
added
priority: medium
rfoust
force-pushed
the
codex/fix-vhf-uhf-band-select
branch
from
b1bfc6c to
8775a5f
Compare
ten9876
force-pushed
the
codex/fix-vhf-uhf-band-select
branch
from
8775a5f to
848fd4a
Compare
|
@rfoust I've rebased this branch onto current Audit: per-file line counts on the rebased diff match the original PR within 2 lines (the resolved STYLEGUIDE comment difference). No silent reverts of recent CI should re-run on the force-push (new head 73, Jeremy KK7GWY & Claude (AI dev partner) |
|
Claude here — merged as 909cd06, thanks @rfoust! This is exactly the kind of PR we love to land: real-world bug The Ships in the next release (target v26.5.3). 73, |
Summary
Fixes FLEX-6500/6700 built-in 4m/2m band routing and direct frequency entry above 54 MHz.
XvtrPolicyto resolve4m -> band=4and2m -> band=2only whenModelCapabilitiesreports native 4m/2m support.2mor4m.X<n>stack key so native 4m does not shadow a user XVTR named4m.FrequencyEntryParserso the VFO widget and RX applet keep the same parsing rules.144.200.000and440.100.000as MHz and preselect the proper band stack before tuning above 54 MHz.MainWindowtune policy as the VFO widget.Notes
The FLEX-6700 SmartSDR wire capture in #695 confirms native keys
band=4for 4m andband=2for 2m. I did not add a native 440/70cm key; direct 440 MHz still requires a radio-reported configured XVTR.Verification
cmake --build build --parallel./build/xvtr_policy_test./build/frequency_entry_parser_testctest --test-dir build -R "model_capabilities_test|frequency_entry_parser_test" --output-on-failuregit diff --check