Skip to content

ci: trigger check-windows on MainWindow.cpp/.h changes (#2671)#3028

Merged
ten9876 merged 1 commit into
mainfrom
auto/fix-2671
May 24, 2026
Merged

ci: trigger check-windows on MainWindow.cpp/.h changes (#2671)#3028
ten9876 merged 1 commit into
mainfrom
auto/fix-2671

Conversation

@ten9876

@ten9876 ten9876 commented May 24, 2026

Copy link
Copy Markdown
Collaborator

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:

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 #2671.

🤖 Generated with Claude Code

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>
@ten9876 ten9876 enabled auto-merge (squash) May 24, 2026 02:43

@aethersdr-agent aethersdr-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Narrow, conservative CI tweak with clear rationale.

Spot-checks:

  • Both paths (src/gui/MainWindow.cpp, src/gui/MainWindow.h) exist; .cpp has ~25 Q_OS_* / HAVE_PIPEWIRE guards, so the "Linux-passing edit can still break MSVC" premise is real.
  • Patterns are added under the build filter group, which is exactly what gates check-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 run check-windows end-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

@ten9876 ten9876 merged commit 1fdd401 into main May 24, 2026
3 checks passed
@ten9876 ten9876 deleted the auto/fix-2671 branch May 24, 2026 03:05
ten9876 added a commit that referenced this pull request May 24, 2026
## 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>
K5PTB added a commit to K5PTB/AetherSDR that referenced this pull request May 25, 2026
…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>
G6PWY-Chris pushed a commit to G6PWY-Chris/AetherSDR that referenced this pull request Jun 22, 2026
… (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#2633aethersdr#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>
G6PWY-Chris pushed a commit to G6PWY-Chris/AetherSDR that referenced this pull request Jun 22, 2026
## 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI: tighten check-paths filter so Windows-affecting MainWindow changes trigger check-windows

1 participant