refactor(band-plan): share contiguous-regions walker for ATU + SWR sweep#3018
Merged
Conversation
Extract the segment-collect + sort-and-merge logic from AtuPreTuneDialog's private computeCentersForBand() into a public BandPlanManager::contiguousRegionsForBand(low, high) so the same discrete-channel awareness can serve MainWindow::startSwrSweep. The new helper returns the band's segments merged into contiguous [low, high] regions (1 Hz adjacency tolerance), sorted by low edge. Continuous bands collapse to one region; discrete-channel bands like US 60 m return one region per legal channel. Call sites: - AtuPreTuneDialog::recomputeBands: now takes BandPlanManager& and delegates the region math to the helper, preserving the per-region center walk and narrow-region midpoint fallback for sub-segment channels (US 60 m: 2.8 kHz channel vs 9 kHz segment). - MainWindow::startSwrSweep: replaces the ad-hoc min/max walk with contiguousRegionsForBand() + first.low/last.high. Behavior identical (sorted-by-low regions give the same envelope as min/max across segments). The 60 m channelized hard-block stays intact — a future enhancement can walk each region individually, but that needs the on-air safety review the issue called out. No behavior change on either call site. 18/18 tests pass. Closes #2822. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Thanks @ten9876 — clean mechanical refactor. Behavior preservation reads correct:
AtuPreTuneDialog::computeCentersForBandkeeps the narrow-region midpoint fallback verbatim and just delegates the collect+sort+merge to the new helper. Null check onm_bandPlanstill happens at line 347 before dereference, so*m_bandPlanis safe.MainWindow::startSwrSweep'sregions.first().lowMhz/regions.last().highMhzis equivalent to the oldmin/maxwalk because the helper's docstring guarantees ascending-by-low-edge order after the merge. The60 mhard-block above still short-circuits before this code runs, so the discrete-channel case stays out of scope.- The 1 Hz adjacency epsilon, midpoint-in-window matching, and segment ordering are preserved verbatim across the move.
Two small suggestions, both optional:
- Unit test for
BandPlanManager::contiguousRegionsForBand. This is now public API with two callers and no direct coverage (notests/band_plan*exists today). A handful of cases — single continuous segment, two overlapping segments, two adjacent-within-1Hz segments, US 60 m discrete channels, empty window — would lock down the merge invariant and make future region-aware features (the SWR sweep stretch goal you mention) safer to build on. - Doc comment nit on the
Regionstruct (BandPlanManager.h:30-37). "Discrete-channel bands ... collapse to one Region per legal channel" is true only because the band-plan JSON encodes each US 60 m channel as a separate segment with regulatory gaps between them — i.e. it's a property of the input data, not the merge algorithm. Maybe reword as "non-adjacent segments produce one Region each (e.g. US 60 m's 5 channels)" so the contract is about adjacency, not band semantics.
Also noticed BandPlanManager.h comment now correctly says "5 channels" for US 60 m where the old AtuPreTuneDialog comment said "6" — nice incidental fix.
Not blocking on either.
🤖 aethersdr-agent · cost: $5.6483 · model: claude-opus-4-7
G6PWY-Chris
pushed a commit
to G6PWY-Chris/AetherSDR
that referenced
this pull request
Jun 22, 2026
…eep (aethersdr#3018) ## Summary Closes aethersdr#2822. Extracts the segment-collect + sort-and-merge contiguous-region computation from `AtuPreTuneDialog`'s private `computeCentersForBand()` into a public `BandPlanManager::contiguousRegionsForBand(low, high)` so the same discrete-channel awareness is available to other band-plan-aware features. ## What the helper does For a `[searchLow, searchHigh]` window, returns the active plan's matching segments merged into contiguous `[low, high]` regions (1 Hz adjacency tolerance), sorted by ascending low edge. - **Continuous bands** (40 m, 20 m, etc.) → one region spanning the band's regulatory edges. - **Discrete-channel bands** (US 60 m: 5 USB channels at 2.8 kHz each separated by tens of kHz of gap) → one region per legal channel. ## Call sites - `AtuPreTuneDialog::recomputeBands` now delegates region math to the helper. Per-region center walk + narrow-region midpoint fallback (US 60 m's 2.8 kHz channel vs 9 kHz segment) preserved verbatim. - `MainWindow::startSwrSweep` replaces its ad-hoc `min(lowMhz)` / `max(highMhz)` walk with `contiguousRegionsForBand() + first.lowMhz / last.highMhz`. Envelope is identical because regions are sorted by low edge after merge — the first region's low IS the global min, the last region's high IS the global max. ## What this does NOT change The 60 m channelized hard-block in `startSwrSweep` stays intact. The issue's stretch goal (walk each region individually for true discrete-channel SWR sweeping) needs on-air safety review and was explicitly out of scope for this mechanical refactor. ## Stats - 4 files, **+79 / -52** - Drops a private 50-line helper, adds a 20-line public method. - 18/18 tests pass (no test changes — the math hasn't changed, only its location). ## Test plan - [x] Clean build, 0 warnings - [x] `atu_pretune_centers_test` still green - [ ] Manual: ATU Pre-Tune sweep on US 60 m — still emits 5 tune centers, one per channel - [ ] Manual: ATU Pre-Tune sweep on IARU R1 40 m — still narrows to 7.000–7.200 MHz envelope - [ ] Manual: SWR sweep on IARU R1 40 m — still narrows to plan's edges, completes successfully - [ ] Manual: SWR sweep on US 60 m — still blocked with the channelized-band warning 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #2822. Extracts the segment-collect + sort-and-merge contiguous-region computation from
AtuPreTuneDialog's privatecomputeCentersForBand()into a publicBandPlanManager::contiguousRegionsForBand(low, high)so the same discrete-channel awareness is available to other band-plan-aware features.What the helper does
For a
[searchLow, searchHigh]window, returns the active plan's matching segments merged into contiguous[low, high]regions (1 Hz adjacency tolerance), sorted by ascending low edge.Call sites
AtuPreTuneDialog::recomputeBandsnow delegates region math to the helper. Per-region center walk + narrow-region midpoint fallback (US 60 m's 2.8 kHz channel vs 9 kHz segment) preserved verbatim.MainWindow::startSwrSweepreplaces its ad-hocmin(lowMhz)/max(highMhz)walk withcontiguousRegionsForBand() + first.lowMhz / last.highMhz. Envelope is identical because regions are sorted by low edge after merge — the first region's low IS the global min, the last region's high IS the global max.What this does NOT change
The 60 m channelized hard-block in
startSwrSweepstays intact. The issue's stretch goal (walk each region individually for true discrete-channel SWR sweeping) needs on-air safety review and was explicitly out of scope for this mechanical refactor.Stats
Test plan
atu_pretune_centers_teststill green🤖 Generated with Claude Code