Skip to content

fix(rx-applet): add matching radeActivated guard to mirror VfoWidget#2376

Merged
ten9876 merged 2 commits into
aethersdr:mainfrom
mvanhorn:mvanhorn/issue-2062-rxapplet-rade-guard
May 8, 2026
Merged

fix(rx-applet): add matching radeActivated guard to mirror VfoWidget#2376
ten9876 merged 2 commits into
aethersdr:mainfrom
mvanhorn:mvanhorn/issue-2062-rxapplet-rade-guard

Conversation

@mvanhorn

@mvanhorn mvanhorn commented May 5, 2026

Copy link
Copy Markdown
Contributor

Closes #2062

Why

VfoWidget.cpp was patched in #2026 so its mode-switch lambda only emits radeActivated(false, ...) when RADE was actually active on that widget. The matching emit at RxApplet.cpp:396 stayed unconditional - the same shape as the multi-pan regression #2026 fixed. The MainWindow handler else if (sliceId == m_radeSliceId) deactivateRADE() makes today's behavior correct, but the asymmetry is a tripwire: any future softening of the MainWindow guard puts the original bug right back.

What changes

Defense-in-depth guard on RxApplet, mirroring VfoWidget:

  • RxApplet.h: new bool m_radeActive{false}; private member, with a comment pointing at the VfoWidget parallel.
  • RxApplet.cpp (HAVE_RADE block in the mode-combo lambda):
    • When user picks RADE, set m_radeActive = true after emitting activate.
    • When user picks any other mode, only emit radeActivated(false, ...) if m_radeActive is true; clear it on the way out.
  • Inline comment in RxApplet.cpp cites fix: RADE deactivated by mode change on unrelated slice in multi-pan setup #2026 so future readers see the pattern.

Net diff: +14 / -1 across the two files.

Behavior

  • Steady state: identical. MainWindow keeps the authoritative m_radeSliceId check; this just stops shouting false from a slice that never said true.
  • The case the guard now protects: user is on slice 0 in RADE, flips slice 1 from USB to LSB. Before this PR, slice 1's RxApplet emitted radeActivated(false, 1); the MainWindow guard ignored it because 1 != m_radeSliceId. After this PR, the spurious emit doesn't fire at all. If MainWindow ever softens its check, RADE on slice 0 stays up.

Verification

  • Builds in the existing ghcr.io/ten9876/aethersdr-ci:latest image (no new find_package, no new dependencies, no Dockerfile change needed).
  • No new behavior, signals, or public API; the m_radeActive field is private and only mutated inside the existing mode-combo lambda.
  • Rebased on upstream/main at the latest tip before push, signed-off and SSH-signed per branch protection.

Closes aethersdr#2062

PR aethersdr#2026 patched VfoWidget so its mode-switch lambda only emits
`radeActivated(false, ...)` when RADE was actually active on that
widget. The MainWindow handler `(else if (sliceId == m_radeSliceId)
deactivateRADE())` makes the matching unguarded emit at
`RxApplet.cpp:396` harmless today, but the asymmetry is exactly the
shape of the multi-pan regression aethersdr#2026 fixed: it would re-tear-down
RADE on a different slice if the MainWindow guard ever softened.

This applies the same defense-in-depth guard:

- Add `bool m_radeActive{false};` to `RxApplet.h` (private), mirroring
  `VfoWidget`.
- In the mode-combo lambda, set `m_radeActive = true` after emitting
  the activate signal.
- Wrap the deactivate emit in `if (m_radeActive) { ... m_radeActive =
  false; }` so a USB->LSB swap on a non-RADE slice no longer fires a
  spurious `radeActivated(false)`.

Comment cites aethersdr#2026 inline so future readers see the parallel.
No behavior change in the steady state: MainWindow still owns the
authoritative `m_radeSliceId` check; this just stops shouting `false`
from a slice that never said `true`.

@ten9876 ten9876 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Claude here on Jeremy's behalf — thanks for the careful write-up @mvanhorn, and the link back to #2026 is exactly the kind of context that makes review easy. The intent (close the asymmetry, defend against future softening of the MainWindow guard) is sound. Unfortunately the implementation as written introduces two correctness regressions because RxApplet has a different lifecycle than VfoWidget, and the local flag pattern doesn't transfer cleanly.

Why VfoWidget's pattern doesn't carry over directly

VfoWidget RxApplet
Instance count One per slice (per-spectrum overlay) Single instance, rebound across slices via setSlice()
m_radeActive source External setter setRadeActive(bool) driven by MainWindow's authoritative state (VfoWidget.cpp:3669; MainWindow calls at lines 12090 / 12122) Local-only in this PR — set when the user picks RADE in this combo, cleared on the next non-RADE pick

In VfoWidget, m_radeActive tracks "is RADE active on this slice" because each instance is bound to one slice and synchronized externally. In RxApplet, the same flag would need to track "is the currently bound slice in RADE" — but the PR doesn't sync it on setSlice() and there's no external setter feeding it from MainWindow's authoritative state.

Two regressions this introduces

1. Externally activated RADE. When RADE is turned on outside the RxApplet mode combo — VfoWidget combo, profile load on startup, or the explicit activation path at MainWindow.cpp:9287m_radeActive in RxApplet stays false. The user then changes the mode on the same slice via RxApplet → guard suppresses the deactivate emit → MainWindow never hears about it → RADE engine stays running on a slice no longer in RADE mode.

2. Slice switch corrupts the flag. Slice 0 in RADE (set via RxApplet) → m_radeActive=true. Active slice changes to slice 1 (USB); setSlice() rebinds the same RxApplet but doesn't reset m_radeActive (RxApplet.cpp:1112-1117). User picks LSB on slice 1 → emits radeActivated(false, 1) → MainWindow correctly ignores (1 != m_radeSliceId), but the lambda still clears m_radeActive to false. User switches back to slice 0 (still RADE) and picks LSB → guard now incorrectly suppresses → RADE engine orphaned.

Both are reachable in normal multi-pan workflows — same class of bug docs/multi-pan-pitfalls.md calls out.

The simpler fix

Use what's already authoritative — the slice's pre-change mode. By the time the combo lambda fires, m_slice->setMode(mode) hasn't run yet (it's the last line in the lambda), so m_slice->mode() still holds the prior mode:

#ifdef HAVE_RADE
    if (mode == "RADE") {
        emit radeActivated(true, m_slice ? m_slice->sliceId() : -1);
        return;
    }
    // Defense-in-depth (matches the spirit of #2026): only emit deactivate
    // if THIS slice was actually in RADE.  Avoids spurious teardown on a
    // different pan when MainWindow's m_radeSliceId guard is bypassed.
    if (m_slice && m_slice->mode() == "RADE")
        emit radeActivated(false, m_slice->sliceId());
#endif
    if (m_slice) m_slice->setMode(mode);

That's a smaller diff than the current PR, no new member needed, correct across:

  • single-instance rebind (always reads from the bound slice)
  • externally activated RADE (slice's mode is "RADE" regardless of how it got there)
  • the multi-pan case the PR is targeting (slice 1's mode is USB, never matches "RADE", so deactivate is suppressed exactly when intended)

The fuller version of the VfoWidget pattern (add RxApplet::setRadeActive(bool), call it from MainWindow::activateRADE/deactivateRADE, refresh in setSlice()) gets you the same correctness with more code and more wiring — fine if you'd prefer it for symmetry, but the slice-mode check above is genuinely sufficient.

Happy to land this once it's reworked along those lines. Thanks again for the contribution.

73, Jeremy KK7GWY & Claude (AI dev partner)

…ve flag

The local-flag pattern from VfoWidget doesn't transfer cleanly to
RxApplet because the lifecycle is different:

- VfoWidget is one instance per slice; m_radeActive is set externally
  by MainWindow::setRadeActive() so it tracks the authoritative state.
- RxApplet is a single instance rebound across slices via setSlice();
  the local flag isn't synced from MainWindow and isn't reset on
  setSlice(), so it gets out of sync in two real scenarios:

    1. Externally activated RADE (VfoWidget combo, profile load on
       startup, MainWindow::activateRADE direct path) leaves
       m_radeActive=false in RxApplet; subsequent mode change in
       RxApplet would suppress the deactivate emit and orphan the
       RADE engine.

    2. Slice 0 in RADE → slice switch to slice 1 (USB) doesn't reset
       the flag → user picks LSB on slice 1 → flag clears (because
       the lambda runs even when MainWindow's authoritative guard
       silently filters the spurious emit) → user switches back to
       slice 0 (still RADE) → guard now incorrectly suppresses.

Both reachable in normal multi-pan workflows, same class of bug as
docs/multi-pan-pitfalls.md catalogues.

The simpler fix uses what's already authoritative: the slice's
pre-change mode.  m_slice->setMode(mode) runs at the END of the
lambda, so m_slice->mode() inside the lambda still holds the prior
mode.  Read it directly; emit deactivate only when the slice was
actually in RADE.  No new state, no sync hazard, correct across:

  - single-instance setSlice() rebind
  - externally activated RADE
  - the multi-pan case the original PR was targeting

Co-Authored-By: Matt Van Horn <mvanhorn@nibbcorp.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ten9876

ten9876 commented May 8, 2026

Copy link
Copy Markdown
Collaborator

Claude here — pushed the rework on top of your branch (4e75a36) along the lines of the earlier review. The local m_radeActive flag is removed; the guard now reads m_slice->mode() inline (which still holds the pre-change mode because m_slice->setMode(mode) runs at the END of the lambda).

The rationale is in the commit body: this approach is correct across single-instance setSlice() rebind, externally activated RADE (VfoWidget combo / profile load / MainWindow::activateRADE direct path), and the multi-pan case your original PR was targeting. No new state, no sync hazard.

Build clean locally. CI re-running. Marking the PR ready (out of draft) once you've had a chance to look at the rework — or you can mark ready yourself if it looks right.

You're a co-author on the rework commit so credit on the squash-merge will be shared.

73, Jeremy KK7GWY & Claude (AI dev partner)

@ten9876 ten9876 marked this pull request as ready for review May 8, 2026 23:32
@ten9876 ten9876 requested a review from jensenpat as a code owner May 8, 2026 23:32
@ten9876 ten9876 merged commit 465407b into aethersdr:main May 8, 2026
5 checks passed
@ten9876

ten9876 commented May 8, 2026

Copy link
Copy Markdown
Collaborator

Claude here — thanks for the patience while we worked through the lifecycle differences, Matt. Final shape (slice mode read pre-change) is small, stateless, and correct across all three failure modes (single-instance setSlice rebind, externally-activated RADE, multi-pan slice swap). The defense-in-depth intent of your original PR is preserved — just hung off the slice's authoritative state instead of a parallel local flag. Merged.

73, Jeremy KK7GWY & Claude (AI dev partner)

@mvanhorn

mvanhorn commented May 9, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the rework and the co-author credit, Jeremy. Reading m_slice->mode() pre-change is cleaner than my parallel flag, and it sidesteps the lifecycle traps you walked through. Glad the defense-in-depth shape made it across.

73, Matt

ten9876 pushed a commit that referenced this pull request May 16, 2026
Closes #2734

## Why

`RxApplet.cpp` was patched in #2376 so its mode-combo lambda only emits
`radeActivated(false, ...)` when `m_slice->mode() == "RADE"`. That guard
is never true in steady state.

"RADE" is a client-side-only concept. When the user selects RADE,
`setMode("RADE")` sends `slice set N mode=RADE` to the radio, which
ignores the unknown mode string and echoes back the real mode (`DIGL`
for 40m, `DIGU` for 60m/WSPR). `SliceModel::handleSliceStatus`
immediately overwrites `m_mode` from that echo. By the time the user
switches away from RADE, `m_slice->mode()` is already `"DIGL"` — the
guard always evaluates false, `radeActivated(false)` is never emitted,
`deactivateRADE()` never runs, and `audio_mute=1` is stranded on the
slice.

The radio's per-band stack saves that muted state. Returning to the band
(via the band-stack, on reconnect, or after a restart) reloads
`audio_mute=1`, making RX audio silently dead on that band until
manually toggled. All other bands are unaffected because they were saved
before RADE muted the slice.

## What changes

**`src/gui/RxApplet.cpp`** — remove the `mode() == "RADE"` guard; emit
`radeActivated(false, ...)` unconditionally, matching pre-#2376
behavior:

```cpp
// "RADE" is client-side only — the radio echoes back the real mode
// (DIGL/DIGU) immediately, so mode() == "RADE" is never true in
// steady state. Emit unconditionally; MainWindow's
// sliceId == m_radeSliceId check is the authoritative filter that
// prevents spurious deactivations from non-RADE slices (#2026).
emit radeActivated(false, m_slice ? m_slice->sliceId() : -1);
```

Net diff: +6 / −14 in one file.

## Why the unconditional emit is safe

`MainWindow`'s `radeActivated` handler (wired in `MainWindow.cpp`)
already guards:
```cpp
else if (sliceId == m_radeSliceId) deactivateRADE();
```
A spurious `radeActivated(false, sliceId)` from a non-RADE slice is
silently dropped there. That check is the safety #2026 actually relied
on for the multi-pan case — not the mode string.

`RxApplet` is also architecturally different from `VfoWidget`: a single
`RxApplet` instance cycles through whichever slice the user selects via
the tab row (N:1), whereas each `VfoWidget` is permanently bound to one
slice (1:1). A mode-string or boolean flag check in `RxApplet` is
inherently unreliable across slice rebinds; `MainWindow::m_radeSliceId`
is the right authority.

## Behavior

- **Steady state / multi-pan**: identical to today. A USB→LSB swap on a
non-RADE slice fires `radeActivated(false, sliceB)`; MainWindow drops it
because `sliceB != m_radeSliceId`. RADE on slice A is unaffected.
- **The fixed case**: user enables RADE on 40m, then switches to USB via
the RxApplet combo. `radeActivated(false, 0)` now fires; MainWindow
calls `deactivateRADE()`; `audio_mute` is restored; band-stack saves the
clean state.

## Verification

Tested on **FLEX-8400, firmware v4.1.5.39794**:
- Enable RADE on 40m via RxApplet combo → switch to USB → slice unmutes,
RADE engine stops ✓
- Enable RADE on 40m via VfoWidget combo → switch to USB via RxApplet
combo → same result ✓
- Multi-pan: RADE on slice 0 (Pan A), swap slice 1 (Pan B) USB→LSB via
RxApplet → RADE on Pan A remains active ✓
- Close + reopen AetherSDR, tune to 40m → audio present ✓

## Follow-on

A separate PR will add a `modeChanged` listener in
`MainWindow::activateRADE` to auto-deactivate RADE when the slice mode
changes via external paths (TCI client, profile load, remote SmartSDR
client). That closes the same class of bug regardless of which code path
drives the mode change.

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

Co-authored-by: Claude Sonnet 4.6 <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.

Defense-in-depth: add matching radeActivated guard to RxApplet

2 participants