fix(audio): persist BYPASS state via active flag (#2892)#2978
Conversation
isTxBypassed() / isRxBypassed() returned !m_txBypassSnapshot.isEmpty(), which collapses two distinct states into one whenever BYPASS is engaged with no chain stages enabled: the snapshot stays empty even though setTxBypassed(true) was just called, so subsequent reads return false. AetherialAudioStrip.cpp:257-258 reads isTxBypassed()/isRxBypassed() on every TX/RX side-toggle to re-sync the BYPASS button checked state - the false return unchecks the button and the engine's idea of bypass silently desynchronises from the GUI. Add bool m_txBypassActive / m_rxBypassActive members and treat them as the source of truth for currently-bypassed. The snapshot retains its restore role unchanged: it still records which stages were enabled at the moment of bypass so we can re-enable exactly that set on release. Reproduces deterministically on Linux per the issue body; verified on nobara-dell against a FLEX-6700 in both directions (baseline binary unchecks BYPASS on TX/RX/TX cycle; fix binary keeps it checked). Co-authored-by: Don @ cloaked.agency <don@cloaked.agency> & K6OZY
There was a problem hiding this comment.
Reviewed the diff against AudioEngine.{cpp,h} and confirmed the root cause analysis. The empty-snapshot edge case is real: when BYPASS is clicked with all chain stages already disabled, m_txBypassSnapshot stays empty and the old isTxBypassed() returns false, which then unchecks the strip's BYPASS button on the next TX/RX side-toggle (AetherialAudioStrip.cpp:257-258 re-reads the getter). Adding m_txBypassActive/m_rxBypassActive as the source of truth — with the snapshot vector retained purely for its restore role — is the right factoring.
A few things I verified:
- The flag is assigned before
emit txBypassChanged(on)in both setters, so any slot that calls back intoisTxBypassed()(e.g.ClientChainApplet.cpp:404) sees the new state. Correct ordering. - All external readers in
AetherialAudioStrip.cppandClientChainApplet.cppgo through the getter, not the snapshot directly — no stragglers using!snapshot.isEmpty()semantics. - The restart-variant case the PR body mentions is covered: on a fresh session both flags default to
false, so the first BYPASS click takes theon != isTxBypassed()branch and sets the flag even when the snapshot ends up empty. - The early-return guard
if (on == isTxBypassed()) return;correctly routes through the new flag. +14 / -8matches the stated scope; nothing leaks outsideAudioEngine.
The header comments are also updated to call out that the snapshot is now restore-only and the flag is the source of truth, which will save the next reader the same chase you did.
Looks good. Thanks for the careful root-cause write-up @Ozy311 — the diagnostic detail in the PR body (including the flush() parallel from #2968 you didn't even need here) makes this very easy to review. 73 de AetherClaude.
🤖 aethersdr-agent · cost: $10.4173 · model: claude-opus-4-7
|
Merged — thanks @Ozy311! The flag-before-emit ordering matters for the slot graph (anyone re-reading Also nice attribution to @TnxQSO-Admin's root-cause writeup in the issue body — the suggested-fix-by-reporter → implemented-by-fellow-community-contributor handoff is the exact pattern we love seeing. Shipping in the next release. 73, |
|
As it seems, as implemented the bypass button now affects all channel strip presets instead of the active. |
…thersdr#2978) ## Summary `AudioEngine::isTxBypassed()` and `isRxBypassed()` returned `!m_txBypassSnapshot.isEmpty()`, conflating "no bypass engaged" with "bypass engaged while every chain stage was already disabled." When BYPASS is clicked in the latter state, the snapshot stays empty, so the getter returns `false` moments after `setTxBypassed(true)` was called. `AetherialAudioStrip.cpp:257-258` re-reads `isTxBypassed()` / `isRxBypassed()` on every TX/RX side-toggle to resync the BYPASS button's checked state. The stale `false` unchecks the button and the engine's view of bypass silently desynchronises from the GUI. ## Why This is exactly the failure reported in aethersdr#2892 by @TnxQSO-Admin: BYPASS engages, user switches the strip TX→RX→TX (or the inverse), and the BYPASS button spontaneously unchecks. The reporter's root-cause analysis pinpointed the empty-snapshot edge case and recommended dedicated bool members as the source of truth — this PR implements that recommendation. The fix also covers the restart variant the issue body mentions: when a prior session ended with bypass engaged, all stages were persisted as disabled. The next BYPASS click in a fresh session hits the same empty-snapshot case; with the flag, the button now stays checked. ## Scope - `src/core/AudioEngine.h` — declare `bool m_txBypassActive{false}` and `bool m_rxBypassActive{false}` alongside the existing snapshot vectors; comments updated to clarify the new division of responsibility. - `src/core/AudioEngine.cpp` — getters return the flag; setters assign the flag before `emit`. The snapshot retains its restore role unchanged — it still records which stages were enabled at the moment of bypass so we can re-enable exactly that set on release. +14 / -8 lines. No protocol, persistence, or UX change; no signal-emission change beyond timing the flag write before the emit. ## Test plan - [x] Local incremental `ninja -C build` clean on Linux (Nobara 43, Qt 6.10.3, gcc 15.2.1) - [x] Bug reproduced on baseline `upstream/main` (`b9df0b3`) against a FLEX-6700 — TX side, all stages off, click BYPASS, switch RX, switch back to TX → BYPASS unchecks itself - [x] Fix verified against the same FLEX-6700, same repro steps — BYPASS stays checked across the TX/RX/TX cycle - [ ] Mac smoke — open to running this on macOS if requested Closes aethersdr#2892 --- 73, Ozy **K6OZY** AI compute partnership: [cloaked.agency](https://cloaked.agency) — drafted with Don, our VP of Engineering agent, on Linux dev stack (`nobara-dell`).
Summary
AudioEngine::isTxBypassed()andisRxBypassed()returned!m_txBypassSnapshot.isEmpty(), conflating "no bypass engaged" with "bypassengaged while every chain stage was already disabled." When BYPASS is clicked
in the latter state, the snapshot stays empty, so the getter returns
falsemoments after
setTxBypassed(true)was called.AetherialAudioStrip.cpp:257-258re-readsisTxBypassed()/isRxBypassed()on every TX/RX side-toggle to resync the BYPASS button's checked state. The
stale
falseunchecks the button and the engine's view of bypass silentlydesynchronises from the GUI.
Why
This is exactly the failure reported in #2892 by @TnxQSO-Admin: BYPASS engages,
user switches the strip TX→RX→TX (or the inverse), and the BYPASS button
spontaneously unchecks. The reporter's root-cause analysis pinpointed the
empty-snapshot edge case and recommended dedicated bool members as the source
of truth — this PR implements that recommendation.
The fix also covers the restart variant the issue body mentions: when a prior
session ended with bypass engaged, all stages were persisted as disabled. The
next BYPASS click in a fresh session hits the same empty-snapshot case; with
the flag, the button now stays checked.
Scope
src/core/AudioEngine.h— declarebool m_txBypassActive{false}andbool m_rxBypassActive{false}alongside the existing snapshot vectors;comments updated to clarify the new division of responsibility.
src/core/AudioEngine.cpp— getters return the flag; setters assign theflag before
emit. The snapshot retains its restore role unchanged — itstill records which stages were enabled at the moment of bypass so we can
re-enable exactly that set on release.
+14 / -8 lines. No protocol, persistence, or UX change; no signal-emission
change beyond timing the flag write before the emit.
Test plan
ninja -C buildclean on Linux (Nobara 43, Qt 6.10.3, gcc 15.2.1)upstream/main(b9df0b3) against a FLEX-6700 — TX side, all stages off, click BYPASS, switch RX, switch back to TX → BYPASS unchecks itselfCloses #2892
73, Ozy K6OZY
AI compute partnership: cloaked.agency — drafted with Don, our VP of Engineering agent, on Linux dev stack (
nobara-dell).