ci: always run check-windows + check-macos on every PR#3244
Merged
Conversation
The check-paths allow-list approach has been a chronic leaker — every time a new file gained an `#ifdef Q_OS_WIN` or `Q_OS_MAC` branch, the gap surfaced as a release-time regression we then patched with a follow-up path-filter addition. Pattern receipts: #796 (origin), #2671 (MainWindow added), #3052 (AudioEngine added), #3210 (re-added after relapse), the v26.5.3 ClientPhaseRotator.cpp M_PI MSVC break, and the CwSidetonePortAudioSink WASAPI fix in #3241 (Windows-only logic that never compiled in CI). Trade ~15-20 min of runner time per PR for a category of regression we kept paying for at release time. Remove the check-paths job entirely and run check-windows + check-macos unconditionally. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
ten9876
added a commit
that referenced
this pull request
May 28, 2026
…#3250) Closes #3248. Implements the three small robustness fixes flagged during PR #3236's review and tracked in #3248. Each is independent and scoped to one file. ## 1. \`StreamDeckPlusParser::parse\` — don't drop simultaneous events Both the encoder-push and LCD-key paths overwrote \`m_prevEncBtns\` / \`m_prevKeys\` with \`newState\` *before* the loop that finds the first changed bit, so any second-changed-bit got silently dropped (the next \`poll()\` saw no remaining change). Switch to one-bit-at-a-time commit: \`\`\`cpp m_prevKeys ^= (1u << i); // consume just this bit return {.button = i + 1, ...}; \`\`\` Now if two keys change in the same HID report, the first poll iteration emits one and the next poll iteration emits the other. Verified by hand-trace: press both → emit press 1 → emit press 2 → release both → emit release 1 → emit release 2. ✅ ## 2. \`HidEncoderManager\` — check \`hid_write\` return on image writes \`setKeyImage\` and \`setTouchscreenImage\` looped through 1024-byte feature reports ignoring the return. If the device unplugs mid-stream the loop spins through the rest writing into a dead handle with no log signal. Now: \`\`\`cpp const int written = hid_write(m_device, pkt, PACKET_SIZE); if (written < 0) { qCWarning(lcDevices) << \"HidEncoderManager::setKeyImage: hid_write failed\" << \"key=\" << key << \"page=\" << pageNumber << \"— device disconnected? Will retry on hotplug.\"; return; } \`\`\` The next \`poll()\`'s \`hid_read() < 0\` triggers \`close()\` + hotplug reopen, which is now correlatable in logs with the user-visible \"deck went blank\". ## 3. \`HidEncoderManager\` — atomic gate-check members \`m_device\` / \`m_openVid\` / \`m_openPid\` are read from the main thread (refreshStreamDeckLabels at MainWindow.cpp:6778, status snapshot at :7061+) and written from \`m_extCtrlThread\` (open / close / hotplugCheck). Wrap in \`std::atomic\` so cross-thread reads are well-defined. Relaxed memory order is sufficient because callers treat the result as a hint and the real gate is re-checked inside the queued slot. \`std::atomic<hid_device*>\` and \`std::atomic<uint16_t>\` have implicit conversion / assignment operators, so no .cpp call-site needs explicit \`.load()\` / \`.store()\` — the source diff is purely the header change plus the existing reads continuing to compile under the new types. ## Test plan - [x] Build clean on Linux (627/627 targets, no new warnings) - [ ] Confirm check-windows and check-macos pass (now always run per #3244) - [ ] Hand-trace the lost-events fix once more - [ ] Optional: hotplug a StreamDeck+ during a label refresh and watch for the new \`hid_write failed\` warning Diff stats: +61 / -25 across 3 files. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ten9876
added a commit
to rfoust/AetherSDR
that referenced
this pull request
May 29, 2026
Branch predates the always-on CI gate from aethersdr#3244. Add a trailing newline so the matrix (build / check-windows / check-macos) runs before merge. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
ten9876
added a commit
that referenced
this pull request
May 29, 2026
Branch predates the always-on CI gate from #3244. Add a trailing newline so the matrix (build / check-windows / check-macos) runs before merge. Co-Authored-By: Claude Opus 4.7 <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
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
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:
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
🤖 Generated with Claude Code