Skip to content

fix(rade): fully restore slice state when RADE engine fails to start#2898

Open
NF0T wants to merge 1 commit into
aethersdr:mainfrom
NF0T:fix/rade-activation-failure-full-rollback
Open

fix(rade): fully restore slice state when RADE engine fails to start#2898
NF0T wants to merge 1 commit into
aethersdr:mainfrom
NF0T:fix/rade-activation-failure-full-rollback

Conversation

@NF0T
Copy link
Copy Markdown
Collaborator

@NF0T NF0T commented May 20, 2026

Summary

When RADEEngine::start() fails, activateRADE() left the slice in a
partially mutated state. Three radio-visible items were set before start()
but never rolled back on failure:

  1. Mode stuck at DIGU/DIGL instead of reverting to the pre-RADE mode
  2. Filter width left at RADE's ±3500 Hz edges, visually wrong even after
    mode reverts (e.g. USB slice with 3.5 kHz edges)
  3. TX badge potentially moved to the RADE slice if it wasn't the TX slice
    before activation — left on the wrong slice after failure

These gaps were identified in review of #2861, which fixes the stranded
audio_mute/engine state. This PR closes the remaining two items flagged
there. It can land before or after #2861; if both are open it supersedes
#2861 entirely.

Changes — src/gui/MainWindow.cpp only

Capture four values before any mutations, all after the !s null guard:

  • prevTxSliceId — the slice ID that currently owns TX (or -1 if none).
    Captured by iterating m_radioModel.slices() before setTxSlice(true).
  • prevMode — the slice's current mode string, before setMode(DIGU/DIGL).
  • prevFilterLow / prevFilterHigh — current filter edges (Hz), before
    setFilterWidth(±3500, 0/0, 3500).

In the !ok failure block:

  1. Call deactivateRADE() — handles m_radeSliceId, m_radePrevMute
    audioMute, engine thread teardown, and the modeChanged disconnect.
    This is already correct and idempotent for fields not yet set at this
    point (m_digitalVoiceTxSlice, DAX/audio connections).
  2. Restore setMode(prevMode).
  3. Restore setFilterWidth(prevFilterLow, prevFilterHigh).
  4. Restore TX badge:
    • prevTxSliceId == sliceId: RADE slice already owned TX — nothing to undo.
    • prevTxSliceId >= 0 && != sliceId: call setTxSlice(true) on the previous
      owner; the radio echoes tx=0 on the RADE slice automatically.
    • prevTxSliceId == -1: no TX slice before activation — call setTxSlice(false)
      on the RADE slice to remove the badge.

Also promotes qWarning()qCWarning(lcRade) so the failure appears in the
aether.rade support-bundle category.

Test plan

  • Happy path: activate RADE normally — no behavior change, slice stays in
    DIGU/DIGL, TX badge stays on RADE slice, filter correct
  • Failure simulation: temporarily return false from RADEEngine::start(),
    with the test slice not the current TX slice:
    - Slice mode reverts to pre-RADE mode
    - Filter width reverts to pre-RADE values
    - TX badge returns to original slice
    - aether.rade log shows "RADE engine failed to start — restoring slice state"
  • Re-entry: activating RADE a second time immediately after failure succeeds
    cleanly with no leftover state

Closes the two residual gaps flagged in reviews of #2861.

When RADEEngine::start() fails, activateRADE() left the slice in a
partially mutated state: mode stuck at DIGU/DIGL with RADE-shaped filter
edges, and the TX badge potentially moved to the wrong slice.

Capture prevMode, prevFilterLow/High, and prevTxSliceId before any
mutations. On failure: call deactivateRADE() (handles mute, engine
teardown, m_radeSliceId), then restore mode, filter width, and TX badge.

TX badge restore: if prevTxSliceId differs from sliceId, either call
setTxSlice(true) on the original owner (radio echoes tx=0 on the RADE
slice automatically), or setTxSlice(false) if no slice owned TX before.

Also promotes the failure log from qWarning to qCWarning(lcRade) so it
appears in the aether.rade support-bundle category.

Closes the two residual gaps flagged in review of aethersdr#2861.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@aethersdr-agent aethersdr-agent Bot left a comment

Choose a reason for hiding this comment

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

Reviewed end-to-end against MainWindow.cpp around activateRADE() / deactivateRADE() and SliceModel's API — the rollback is well-formed.

Correctness checks

  • prevTxSliceId, prevMode, prevFilterLow, prevFilterHigh are all captured after the !s null guard and before any mutation. ✔
  • TX-badge restore tri-state matches the radio's invariant that at most one slice owns TX:
    • prevTxSliceId == sliceId → no-op (RADE slice already owned TX before activation), ✔
    • prevTxSliceId >= 0 && != sliceId → previous owner restored via setTxSlice(true); radio will echo tx=0 on the RADE slice, ✔
    • prevTxSliceId == -1setTxSlice(false) on the RADE slice to clear the badge, ✔
  • The deactivateRADE() call inside the !ok branch is safe at this point:
    • m_radeSliceId and m_radePrevMute are already set (lines 13802–13803), so the audio-mute rollback works.
    • m_digitalVoiceTxSlice is not yet set; setDigitalVoiceTxSlice(-1) from its current -1 is a no-op.
    • The DAX/audio connect() calls all happen after the !ok block, so the corresponding disconnect()s in deactivateRADE() are harmless no-ops (Qt returns false, no warning).
    • RADEEngine::stop() invoked on a never-started engine is safe per RADEEngine.cpp.
  • Re-fetching m_radioModel.slice(sliceId) after deactivateRADE() is defensive but appropriate — deactivateRADE doesn't delete slices, but the pattern is consistent with the rest of the file. ✔
  • qWarning()qCWarning(lcRade) matches the category declared in LogManager.h/.cpp. ✔

Scope — single file, ~28 lines, all in the one function. No drive-by changes.

No issues found. The rollback closes the three gaps cleanly (mode, filter, TX badge) and the re-entry path will see a clean slice state. Nice incremental on top of #2861.

Thanks for the careful capture-before-mutate ordering and the explicit tri-state for the TX badge — that's the part that would have been easy to get subtly wrong.

73, AetherClaude

@NF0T NF0T mentioned this pull request May 21, 2026
2 tasks
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