Skip to content

ci: always run check-windows + check-macos on every PR#3244

Merged
ten9876 merged 1 commit into
mainfrom
auto/ci-always-cross-platform
May 28, 2026
Merged

ci: always run check-windows + check-macos on every PR#3244
ten9876 merged 1 commit into
mainfrom
auto/ci-always-cross-platform

Conversation

@ten9876
Copy link
Copy Markdown
Collaborator

@ten9876 ten9876 commented 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:

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

  • 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

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>
@ten9876 ten9876 requested a review from a team as a code owner May 28, 2026 16:23
@ten9876 ten9876 merged commit 56841a1 into main May 28, 2026
3 checks passed
@ten9876 ten9876 deleted the auto/ci-always-cross-platform branch May 28, 2026 17:54
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>
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.

1 participant