Skip to content

ci: add AudioEngine to check-windows path filter (#3052)#3210

Merged
ten9876 merged 1 commit into
aethersdr:mainfrom
M7HNF-Ian:fix/3052-ci-audioengine-path-filter
May 27, 2026
Merged

ci: add AudioEngine to check-windows path filter (#3052)#3210
ten9876 merged 1 commit into
aethersdr:mainfrom
M7HNF-Ian:fix/3052-ci-audioengine-path-filter

Conversation

@M7HNF-Ian

Copy link
Copy Markdown
Contributor

Problem

AudioEngine.cpp and AudioEngine.h are as dense with #ifdef Q_OS_WIN / Q_OS_MAC / HAVE_PIPEWIRE branches as MainWindow, but were absent from the dorny/paths-filter build group that gates the check-windows CI job.

A Linux-passing change inside a #ifdef Q_OS_WIN block can therefore land on main without triggering a Windows build check. This is the same class of issue that motivated adding MainWindow to the filter in #2671.

Post-mortem: PR #2929 (WASAPI mono-mic recovery) slipped past check-windows for exactly this reason — AudioEngine.cpp carried the platform-guarded change but wasn't on the filter.

Fix

Add src/core/AudioEngine.cpp and src/core/AudioEngine.h to the build paths group, with a comment referencing the post-mortem. Pattern mirrors the existing MainWindow entry.

Fixes #3052

AudioEngine.cpp / .h are as dense with platform-guarded code as
MainWindow (WASAPI, CoreAudio, ALSA/PipeWire branches), but were absent
from the dorny/paths-filter build group. A Linux-passing change inside a
Q_OS_WIN block could therefore land on main without triggering a Windows
CI run.

Post-mortem: aethersdr#2929 (WASAPI mono-mic recovery) slipped past check-windows
for exactly this reason. Adding both files mirrors the pattern already
established for MainWindow (aethersdr#2671).

Fixes aethersdr#3052
@M7HNF-Ian M7HNF-Ian requested a review from a team as a code owner May 27, 2026 08:27
@ten9876 ten9876 merged commit af10f43 into aethersdr:main May 27, 2026
3 checks passed
@ten9876

ten9876 commented May 27, 2026

Copy link
Copy Markdown
Collaborator

Claude here — merged, thanks @M7HNF-Ian. Closes #3052. Same-class protection the MainWindow entry has had since #2671, and the comment captures the #2929/#3036 post-mortem clearly for anyone auditing the filter list later.

73, Jeremy KK7GWY & Claude (AI dev partner)

ten9876 added a commit that referenced this pull request May 28, 2026
…per (#3239)

Completes the platform matrix promised in #3232.  Linux (evdev) landed
in #3238; this PR adds the Windows (hidapi) and macOS (IOKit) sides.
The mapper dialog drops its Linux guard and now builds everywhere.

Architecture
------------
All three backends expose the same Qt signal contract:
    tuneSteps(int)               - rotary delta
    buttonEvent(sig, action)     - canonical Linux-keycode-name strings
    connectionChanged(bool, name)

A new typedef `UlanziDialBackend` (src/core/UlanziDialBackend.h)
resolves to the right concrete class per platform.  Mapper dialog and
MainWindow dispatcher use only the alias and have no platform switches
of their own.

Windows (UlanziDialWindowsManager)
----------------------------------
- hidapi (already a project dep) enumerates HID devices, matches by
  product_string substring "Ulanzi Dial"
- Opens every matching interface (the dial enumerates as multiple
  HID interfaces on Windows: Keyboard + Consumer Control, possibly
  Mouse) and polls each via QTimer at 5ms intervals
- HID report parsing: standard 8-byte keyboard report (modifier mask
  + up-to-6-keys array, diffed across frames for transitions) and
  Consumer Control reports (2-byte usage code)
- HID usage codes mapped to canonical Linux keycode integers so the
  signature strings (`KEY_PLAYPAUSE`, `Ctrl+V`, ...) match Linux
  exactly and `kPillSpecs` is unchanged
- Chord assembly state machine identical to the Linux backend
- Hot-plug via 3s rescan timer
- LIMITATION: no exclusive-claim equivalent yet.  The dial's keystrokes
  still reach the focused window because hidapi doesn't suppress OS
  input routing.  RawInput + RIDEV_NOLEGACY is the proper fix; tracked
  for a follow-up

macOS (UlanziDialMacOSManager)
------------------------------
- IOKit HID Manager with device-matching dictionary
  ({ kIOHIDProductKey: "Ulanzi Dial" })
- `IOHIDManagerOpen` with `kIOHIDOptionsTypeSeizeDevice` -- the
  documented macOS exclusive-claim mechanism.  Equivalent to Linux
  EVIOCGRAB: when seized, the dial's events stop reaching the OS
  keyboard stack, so its media keys don't leak to the focused window
- Event-driven via `IOHIDManagerRegisterInputValueCallback`; no polling
- Same usage-page-to-Linux-keycode mapping and chord assembly as
  Windows backend
- Scheduled on `CFRunLoopGetMain()` (IOKit requires a CFRunLoop, and
  only the main thread has one integrated with Qt's event loop on
  macOS).  MainWindow conditionally skips `moveToThread` on macOS to
  match this constraint
- Hot-plug native via IOHIDManager device-matching/removal callbacks

Build wiring (CMakeLists.txt)
-----------------------------
- UlanziDialBackend.h + UlanziDialMapperDialog.{cpp,h}: unconditional
- EvdevEncoderManager.{cpp,h}: UNIX AND NOT APPLE (Linux)
- UlanziDialWindowsManager.{cpp,h}: WIN32
- UlanziDialMacOSManager.{cpp,h}: APPLE
- macOS links -framework IOKit and -framework CoreFoundation

Tested
------
- Linux: compiles + runs as before (Linux behaviour unchanged; same
  EvdevEncoderManager underneath the alias)
- Windows + macOS: compile validation gated by CI (check-windows from
  #3210 and check-macos from #3223).  Live testing on hardware happens
  in the user's environment

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ten9876 added a commit that referenced this pull request May 28, 2026
## Summary

Drop the \`check-paths\` allow-list job that gated \`check-windows\` and
\`check-macos\` based on a hand-maintained set of files. Both
cross-platform CI builds now run on every PR.

## Why

The path-filter approach has been a chronic leaker. Every time a new
file gained a platform-guarded branch (\`#ifdef Q_OS_WIN\`, \`#ifdef
Q_OS_MAC\`, \`HAVE_PIPEWIRE\`, etc.) that wasn't on the allow-list, CI
silently skipped the relevant cross-platform check and the regression
surfaced at release tag time. The follow-up was always to add another
path to the filter — and then the same class of issue would resurface
elsewhere a few PRs later.

Pattern receipts:
- #796 — original \"MSVC issue slipped past CI\" lesson
- #2633#2662 — macOS \`m_daxBridge\` reference inside macOS-only
guard, missed
- #2670 — Windows behavior change not exercised by CI
- #2671 — added MainWindow.cpp/.h to filter after a miss
- #2929#3052 — WASAPI mono-mic recovery landed unverified; added
AudioEngine.cpp/.h
- #3210 — re-added AudioEngine after a relapse
- v26.5.3 — ClientPhaseRotator.cpp \`M_PI\` MSVC break slipped through
- #3241 (yesterday) — CwSidetonePortAudioSink WASAPI fix; entire
\`#ifdef Q_OS_WIN\` block never compiled by CI because the file wasn't
on the filter

## Trade-off

Cost: ~15-20 min of GitHub Actions runner time per PR (Windows + macOS
in parallel, both cached aggressively — cold ~5 min, warm ~2-3 min).

Benefit: a whole class of release-time regression we've been paying for
repeatedly.

## Test plan

- [x] YAML valid (\`python3 -c 'import yaml; yaml.safe_load(...)'\`)
- [ ] Confirm \`check-windows\` and \`check-macos\` actually run on this
PR (no longer marked \`skipping\`)
- [ ] Both pass

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@M7HNF-Ian M7HNF-Ian deleted the fix/3052-ci-audioengine-path-filter branch June 7, 2026 14:58
G6PWY-Chris pushed a commit to G6PWY-Chris/AetherSDR that referenced this pull request Jun 22, 2026
…thersdr#3210)

## Problem

`AudioEngine.cpp` and `AudioEngine.h` are as dense with `#ifdef
Q_OS_WIN` / `Q_OS_MAC` / `HAVE_PIPEWIRE` branches as `MainWindow`, but
were absent from the dorny/paths-filter `build` group that gates the
`check-windows` CI job.

A Linux-passing change inside a `#ifdef Q_OS_WIN` block can therefore
land on `main` without triggering a Windows build check. This is the
same class of issue that motivated adding `MainWindow` to the filter in
aethersdr#2671.

**Post-mortem:** PR aethersdr#2929 (WASAPI mono-mic recovery) slipped past
`check-windows` for exactly this reason — `AudioEngine.cpp` carried the
platform-guarded change but wasn't on the filter.

## Fix

Add `src/core/AudioEngine.cpp` and `src/core/AudioEngine.h` to the
`build` paths group, with a comment referencing the post-mortem. Pattern
mirrors the existing `MainWindow` entry.

Fixes aethersdr#3052
G6PWY-Chris pushed a commit to G6PWY-Chris/AetherSDR that referenced this pull request Jun 22, 2026
…per (aethersdr#3239)

Completes the platform matrix promised in aethersdr#3232.  Linux (evdev) landed
in aethersdr#3238; this PR adds the Windows (hidapi) and macOS (IOKit) sides.
The mapper dialog drops its Linux guard and now builds everywhere.

Architecture
------------
All three backends expose the same Qt signal contract:
    tuneSteps(int)               - rotary delta
    buttonEvent(sig, action)     - canonical Linux-keycode-name strings
    connectionChanged(bool, name)

A new typedef `UlanziDialBackend` (src/core/UlanziDialBackend.h)
resolves to the right concrete class per platform.  Mapper dialog and
MainWindow dispatcher use only the alias and have no platform switches
of their own.

Windows (UlanziDialWindowsManager)
----------------------------------
- hidapi (already a project dep) enumerates HID devices, matches by
  product_string substring "Ulanzi Dial"
- Opens every matching interface (the dial enumerates as multiple
  HID interfaces on Windows: Keyboard + Consumer Control, possibly
  Mouse) and polls each via QTimer at 5ms intervals
- HID report parsing: standard 8-byte keyboard report (modifier mask
  + up-to-6-keys array, diffed across frames for transitions) and
  Consumer Control reports (2-byte usage code)
- HID usage codes mapped to canonical Linux keycode integers so the
  signature strings (`KEY_PLAYPAUSE`, `Ctrl+V`, ...) match Linux
  exactly and `kPillSpecs` is unchanged
- Chord assembly state machine identical to the Linux backend
- Hot-plug via 3s rescan timer
- LIMITATION: no exclusive-claim equivalent yet.  The dial's keystrokes
  still reach the focused window because hidapi doesn't suppress OS
  input routing.  RawInput + RIDEV_NOLEGACY is the proper fix; tracked
  for a follow-up

macOS (UlanziDialMacOSManager)
------------------------------
- IOKit HID Manager with device-matching dictionary
  ({ kIOHIDProductKey: "Ulanzi Dial" })
- `IOHIDManagerOpen` with `kIOHIDOptionsTypeSeizeDevice` -- the
  documented macOS exclusive-claim mechanism.  Equivalent to Linux
  EVIOCGRAB: when seized, the dial's events stop reaching the OS
  keyboard stack, so its media keys don't leak to the focused window
- Event-driven via `IOHIDManagerRegisterInputValueCallback`; no polling
- Same usage-page-to-Linux-keycode mapping and chord assembly as
  Windows backend
- Scheduled on `CFRunLoopGetMain()` (IOKit requires a CFRunLoop, and
  only the main thread has one integrated with Qt's event loop on
  macOS).  MainWindow conditionally skips `moveToThread` on macOS to
  match this constraint
- Hot-plug native via IOHIDManager device-matching/removal callbacks

Build wiring (CMakeLists.txt)
-----------------------------
- UlanziDialBackend.h + UlanziDialMapperDialog.{cpp,h}: unconditional
- EvdevEncoderManager.{cpp,h}: UNIX AND NOT APPLE (Linux)
- UlanziDialWindowsManager.{cpp,h}: WIN32
- UlanziDialMacOSManager.{cpp,h}: APPLE
- macOS links -framework IOKit and -framework CoreFoundation

Tested
------
- Linux: compiles + runs as before (Linux behaviour unchanged; same
  EvdevEncoderManager underneath the alias)
- Windows + macOS: compile validation gated by CI (check-windows from
  aethersdr#3210 and check-macos from aethersdr#3223).  Live testing on hardware happens
  in the user's environment

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

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

Drop the \`check-paths\` allow-list job that gated \`check-windows\` and
\`check-macos\` based on a hand-maintained set of files. Both
cross-platform CI builds now run on every PR.

## Why

The path-filter approach has been a chronic leaker. Every time a new
file gained a platform-guarded branch (\`#ifdef Q_OS_WIN\`, \`#ifdef
Q_OS_MAC\`, \`HAVE_PIPEWIRE\`, etc.) that wasn't on the allow-list, CI
silently skipped the relevant cross-platform check and the regression
surfaced at release tag time. The follow-up was always to add another
path to the filter — and then the same class of issue would resurface
elsewhere a few PRs later.

Pattern receipts:
- aethersdr#796 — original \"MSVC issue slipped past CI\" lesson
- aethersdr#2633aethersdr#2662 — macOS \`m_daxBridge\` reference inside macOS-only
guard, missed
- aethersdr#2670 — Windows behavior change not exercised by CI
- aethersdr#2671 — added MainWindow.cpp/.h to filter after a miss
- aethersdr#2929aethersdr#3052 — WASAPI mono-mic recovery landed unverified; added
AudioEngine.cpp/.h
- aethersdr#3210 — re-added AudioEngine after a relapse
- v26.5.3 — ClientPhaseRotator.cpp \`M_PI\` MSVC break slipped through
- aethersdr#3241 (yesterday) — CwSidetonePortAudioSink WASAPI fix; entire
\`#ifdef Q_OS_WIN\` block never compiled by CI because the file wasn't
on the filter

## Trade-off

Cost: ~15-20 min of GitHub Actions runner time per PR (Windows + macOS
in parallel, both cached aggressively — cold ~5 min, warm ~2-3 min).

Benefit: a whole class of release-time regression we've been paying for
repeatedly.

## Test plan

- [x] YAML valid (\`python3 -c 'import yaml; yaml.safe_load(...)'\`)
- [ ] Confirm \`check-windows\` and \`check-macos\` actually run on this
PR (no longer marked \`skipping\`)
- [ ] Both pass

🤖 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: expand check-windows path filter to hot platform-guarded files

2 participants