ci: trigger check-windows on MainWindow.cpp/.h changes (#2671)#3028
Conversation
The dorny/paths-filter `build` group only watched CMakeLists.txt, third_party/, and .github/workflows/. That misses the most common source of MSVC-only breakage: MainWindow.cpp/.h. The file is densely guarded with `#ifdef Q_OS_WIN`, `Q_OS_MAC`, and `HAVE_PIPEWIRE` branches, so an edit that compiles on Linux can still break Windows. Recent post-mortems of this exact class: - #2633 → #2662: unconditional m_daxBridge reference inside a macOS-only guard, caught only after release. - #2670: Windows behaviour change not exercised by CI. - v26.5.3 (#3024): ClientPhaseRotator.cpp's bare M_PI broke MSVC (needs `_USE_MATH_DEFINES` on Windows). That one wasn't a MainWindow file, but it's the same shape of failure — a path not on the filter, so check-windows didn't run, so we landed a broken Windows build and had to patch it post-merge. Add MainWindow.cpp and MainWindow.h to the `build` filter so any change to them forces a Windows CI run. Keeps the cost low (most PRs still skip check-windows) while closing the gap on the highest-leverage hot file in the tree. Closes #2671. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
LGTM. Narrow, conservative CI tweak with clear rationale.
Spot-checks:
- Both paths (
src/gui/MainWindow.cpp,src/gui/MainWindow.h) exist; .cpp has ~25Q_OS_*/HAVE_PIPEWIREguards, so the "Linux-passing edit can still break MSVC" premise is real. - Patterns are added under the
buildfilter group, which is exactly what gatescheck-windows(if: needs.check-paths.outputs.build-changed == 'true'), so the trigger wiring is correct. - Test-plan bullet #1 is automatically satisfied here because
.github/workflows/**is already a watched path — this PR itself will runcheck-windowsend-to-end before merge, which is the right validation.
One small nit, not blocking: the PR explicitly defers other hot platform-guarded files (e.g. the ClientPhaseRotator.cpp M_PI case cited as motivation). That's fine as a scope choice, but it does mean the same class of incident can still slip through any other unfiltered file. A small followup that either (a) broadens the filter to src/dsp/** for the DSP-heavy files prone to MSVC math-macro / intrinsic divergence, or (b) just inverts the filter to run Windows on any C++ source change with an explicit skip-list, would close the gap more durably. Worth a separate ticket if the breakage rate keeps justifying it — no need to do it in this PR.
Thanks @ten9876.
🤖 aethersdr-agent · cost: $2.8091 · model: claude-opus-4-7
## Summary Fix TX waterfall scrolling visibly slower than RX (~3x slower in practice) after PR #3019 landed. ## Root cause `pushWaterfallRow` (FFT-derived row path, sole TX waterfall source post-#3019) was gating row emission to one row per `m_wfLineDuration` (100 ms by default) on the premise that RX native tiles also arrive at line_duration cadence. They don't. Native tiles arrive at the radio's FFT rate (~30 Hz on this fleet) and `updateWaterfallRow` pushes one row per tile with no rate gate. So RX = ~30 rows/sec and TX = ~10 rows/sec → 3x perceptual slowdown. Pre-#3019 this was masked: with show-tx-in-waterfall on, native tiles *also* ran during TX, filling the cadence regardless of whether the FFT gate let a row through. #3019 correctly stopped the double-advance — and that exposed the underlying pacing-assumption bug from #2666. ## Fix Emit one row per FFT frame from `pushWaterfallRow`, matching the native-tile behaviour exactly. The intermediate-frame linear-power accumulator becomes unnecessary (no intermediates to combine when each frame is its own row); intra-frame bin interpolation is now in dBm space, consistent with `updateWaterfallRow`. ## What I left alone State members (`m_fftRowDebtMs`, `m_lastFftFrameMs`, `m_fftAccumPower`, `m_fftAccumCount`) and `resetFftWaterfallAccumulator()` are now unused but left in place to keep this diff focused on the behavioural fix. Cleanup PR can remove them. ## Diagnostic evidence Probe instrumentation in `pushWaterfallRow` and `updateSpectrum` confirmed before the fix: - FFT frames arrive at ~30 Hz during both RX and TX (~33 ms intervals, no radio-side throttle) - `pushWaterfallRow` was called ~270 times in a 9-second TX window, but only emitted 92 rows (~10 rows/sec, matching the gated cadence) After the fix, every FFT frame produces a row → ~30 rows/sec, parity with RX. ## Stats - 1 file, +22 / -55 - No new flat-key AppSettings (Principle V N/A) - MeterSmoother N/A (no meter code) ## Test plan - [x] Local build clean - [x] Visual confirmation by Jeremy on FLEX-8600 USB at 14.265 MHz: TX waterfall scroll now matches RX speed - [ ] CI: build + check-paths green; check-windows skipped (no build paths changed) — this PR doesn't touch MainWindow.cpp so #3028's new filter doesn't fire ## Checklist - [x] No new flat-key `AppSettings` - [x] All meter UI uses `MeterSmoother` (N/A) - [x] Documentation updated if user-visible behavior changed (release notes will mention TX waterfall rate fix) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ethersdr#2962) CAT protocol coverage ───────────────────── Full Kenwood TS-2000 command set, referencing the manual appendix, except commands not applicable to a FlexRadio. FlexCAT (ZZFA/ZZFB/ZZMD/ZZFI/…) extensions on a second port. Commands implemented across both dialects: • Frequency: FA/FB, ZZFA/ZZFB, IF/ZZIF, OI • Mode: MD/ZZMD, ZZME (extended mode IDs) • Filter: SL/SH, FW, ZZFI/ZZFJ • AGC: GT/ZZGT, ZZAR/ZZAS (per-slice fast/slow threshold) • Audio: AG/ZZAG, ZZLE/ZZLB/ZZLF (level/balance/AF), ZZMA/ZZAB (mute) • RF/Mic: PC/ZZPC, MG/ZZMC, RA/PA (attenuator/preamp) • Squelch: SQ • RIT: RC/ZZRC, RD/ZZRD, RU/ZZRU, RG/ZZRG, RT/ZZRT, ZZRW/ZZRY • XIT: XT/ZZXG, ZZXC/ZZXS • PTT / TX: TX (TX0 = PTT off per spec), RX, FT, ZZFT, FR, ZZSW • CW: KS/KY, PT (sidetone) • Noise: NB/NL, NR/NT, RL, ZZNL/ZZNR • Meter: SM/ZZSM • VFO step: UP/DN • Status: PS, ID, TY, BY, LK, RM • AI (async push): AI/ZZAI — frequency and mode push on change • FlexCAT-only: ZZBI, ZZDE, ZZFR Why CAT in addition to rigctld ─────────────────────────────── Some external apps do not support rigctld, but expect to send TS-2000 CAT commands, with extensions by FlexRadio. setups that currently use SmartCAT can work directly with AetherSDR without change. TCI is preferred, but this supports both legacy apps and legacy hams. CAT applet redesign ──────────────────── The space-consuming configuration app only appears when popped out, avoiding clutter of the right panel. The right-panel applet shows a compact per-port status table (enabled, dialect, port, PTY path, client count). The pop-out window adds full controls: enable toggle, dialect selector, port number, symlink path. Right-click on the PTY path label → "Copy PTY Name". Two TCP ports by default: 4532 (rigctld) and 5001 (Flex CAT). Up to 6 more ports can be configured/enabled. Each port gets a dedicated PTY symlink in the per-user runtime directory (cat-A … cat-H). Theme: all widget styles use ThemeManager token templates so the applet tracks live theme changes (phase-5 switcher not yet built, but the plumbing is in place per aethersdr#3028). Test suites ──────────── Three C++ integration test executables (cmake targets): • rigctld_test — 16 sections, TCP + PTY round-trip • CAT_TS-2000_test — 14 sections (TS-2000 dialect), TCP + PTY • CAT_Flex_test — 15 sections (FlexCAT dialect), TCP + PTY PTT (--ptt) and CW keyer (--cw) tests are disabled by default; enable only with a dummy load connected. PTY defaults use QStandardPaths to resolve the per-user symlink directory, matching CatPort::defaultSymlinkPath. Integration testing ──────────────────── Tested end-to-end against: | App | Dialect | Transport | |----------------|----------|-----------| | N1MM Logger+ | TS-2000 | TCP | | Not1MM | rigctld | TCP | | WSJT-X | rigctld | TCP / PTY | | fldigi | TS-2000 | TCP / PTY | | flrig | TS-2000 | TCP | | MacLoggerDX | TS-2000 | PTY | Safety and security ──────────────────── • TX safety: SmartCatProtocol tracks whether it asserted PTT (m_pttAssertedByMe); SmartCatSession::onDisconnected and ~dtor both call releasePtt() so an abrupt client drop cannot leave the radio keyed. • PTY symlinks: per-user directory (QStandardPaths::RuntimeLocation / CacheLocation), chmod 0700, atomic rename — no /tmp cross-user collisions, no TOCTOU window (GHSA-qxhr-cwrc-pvrm). • Dead code removed: SmartCatServer.{h,cpp} were unreachable (not in CMakeLists, broken 6-arg ctor); deleted. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… (aethersdr#3028) ## Summary - Adds `src/gui/MainWindow.cpp` and `src/gui/MainWindow.h` to the `dorny/paths-filter` `build` group so the Windows CI job runs whenever the hot file changes. - The file is densely platform-guarded (`#ifdef Q_OS_WIN`, `Q_OS_MAC`, `HAVE_PIPEWIRE`), so Linux-passing edits can still break MSVC. ## Motivation Same class of failure has bitten us repeatedly: - **aethersdr#2633 → aethersdr#2662** — unconditional `m_daxBridge` reference inside a macOS-only guard, caught post-release. - **aethersdr#2670** — Windows behaviour change not exercised by CI. - **v26.5.3 (aethersdr#3024)** — `ClientPhaseRotator.cpp` bare `M_PI` broke MSVC (needs `_USE_MATH_DEFINES`). Different file, same shape: path not on the filter → `check-windows` didn't run → broken Windows build landed and required a follow-up patch. This PR closes the gap for the highest-leverage file. Other hot platform-guarded files can be added later if their breakage rate justifies the runner time. ## Cost Most PRs continue to skip `check-windows` (the filter still excludes most source paths). Cost increase is bounded to the subset of PRs that touch MainWindow.cpp/.h, which is exactly the subset that needs the coverage. ## Test plan - [ ] CI: `check-paths` job runs and reports `build-changed=true` on this PR (because `.github/workflows/**` is itself a watched path). - [ ] CI: `check-windows` job runs and passes (no source changed, so it should be a clean cache hit). - [ ] After merge: next PR that touches `src/gui/MainWindow.cpp` triggers `check-windows` automatically. Closes aethersdr#2671. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Summary Fix TX waterfall scrolling visibly slower than RX (~3x slower in practice) after PR aethersdr#3019 landed. ## Root cause `pushWaterfallRow` (FFT-derived row path, sole TX waterfall source post-aethersdr#3019) was gating row emission to one row per `m_wfLineDuration` (100 ms by default) on the premise that RX native tiles also arrive at line_duration cadence. They don't. Native tiles arrive at the radio's FFT rate (~30 Hz on this fleet) and `updateWaterfallRow` pushes one row per tile with no rate gate. So RX = ~30 rows/sec and TX = ~10 rows/sec → 3x perceptual slowdown. Pre-aethersdr#3019 this was masked: with show-tx-in-waterfall on, native tiles *also* ran during TX, filling the cadence regardless of whether the FFT gate let a row through. aethersdr#3019 correctly stopped the double-advance — and that exposed the underlying pacing-assumption bug from aethersdr#2666. ## Fix Emit one row per FFT frame from `pushWaterfallRow`, matching the native-tile behaviour exactly. The intermediate-frame linear-power accumulator becomes unnecessary (no intermediates to combine when each frame is its own row); intra-frame bin interpolation is now in dBm space, consistent with `updateWaterfallRow`. ## What I left alone State members (`m_fftRowDebtMs`, `m_lastFftFrameMs`, `m_fftAccumPower`, `m_fftAccumCount`) and `resetFftWaterfallAccumulator()` are now unused but left in place to keep this diff focused on the behavioural fix. Cleanup PR can remove them. ## Diagnostic evidence Probe instrumentation in `pushWaterfallRow` and `updateSpectrum` confirmed before the fix: - FFT frames arrive at ~30 Hz during both RX and TX (~33 ms intervals, no radio-side throttle) - `pushWaterfallRow` was called ~270 times in a 9-second TX window, but only emitted 92 rows (~10 rows/sec, matching the gated cadence) After the fix, every FFT frame produces a row → ~30 rows/sec, parity with RX. ## Stats - 1 file, +22 / -55 - No new flat-key AppSettings (Principle V N/A) - MeterSmoother N/A (no meter code) ## Test plan - [x] Local build clean - [x] Visual confirmation by Jeremy on FLEX-8600 USB at 14.265 MHz: TX waterfall scroll now matches RX speed - [ ] CI: build + check-paths green; check-windows skipped (no build paths changed) — this PR doesn't touch MainWindow.cpp so aethersdr#3028's new filter doesn't fire ## Checklist - [x] No new flat-key `AppSettings` - [x] All meter UI uses `MeterSmoother` (N/A) - [x] Documentation updated if user-visible behavior changed (release notes will mention TX waterfall rate fix) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
src/gui/MainWindow.cppandsrc/gui/MainWindow.hto thedorny/paths-filterbuildgroup so the Windows CI job runs whenever the hot file changes.#ifdef Q_OS_WIN,Q_OS_MAC,HAVE_PIPEWIRE), so Linux-passing edits can still break MSVC.Motivation
Same class of failure has bitten us repeatedly:
m_daxBridgereference inside a macOS-only guard, caught post-release.ClientPhaseRotator.cppbareM_PIbroke MSVC (needs_USE_MATH_DEFINES). Different file, same shape: path not on the filter →check-windowsdidn't run → broken Windows build landed and required a follow-up patch.This PR closes the gap for the highest-leverage file. Other hot platform-guarded files can be added later if their breakage rate justifies the runner time.
Cost
Most PRs continue to skip
check-windows(the filter still excludes most source paths). Cost increase is bounded to the subset of PRs that touch MainWindow.cpp/.h, which is exactly the subset that needs the coverage.Test plan
check-pathsjob runs and reportsbuild-changed=trueon this PR (because.github/workflows/**is itself a watched path).check-windowsjob runs and passes (no source changed, so it should be a clean cache hit).src/gui/MainWindow.cpptriggerscheck-windowsautomatically.Closes #2671.
🤖 Generated with Claude Code