fix(cw): Zero Beat tunes the slice that owns the button, never the active one (#2516)#3312
Conversation
…tive one (aethersdr#2516) The zeroBeatRequested handler operated on activeSlice() instead of the slice that owns the VfoWidget that emitted the signal. zeroBeatRequested carries no sliceId, and clicking the Zero Beat child button is consumed by the button so it does not propagate to the panel's activation hook — so pressing Zero Beat on slice A while slice B was active tuned slice B (the reported 'Slice B VFO moves on two different bands'). Capture sliceId (like the sibling autotune handlers already do) and resolve via m_radioModel.slice(sliceId). Additionally short-circuit when that slice is not the active one: the shared CW decoder is fed only by the active slice's audio and its estimatedPitch() re-routes per active slice (routeCwDecoderOutput()), so the detected pitch is meaningful only for the active slice — applying it to a non-active slice would tune on a pitch derived from a different slice's audio. The guard makes pressing Zero Beat on a non-active slice a safe no-op rather than a wrong tune. Does not touch slice 0 RX flow. UX alternatives (activate-first-then-tune, or disabling the button when the slice is inactive) were considered and left for maintainer direction; this is the minimal correctness fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Looks good — thanks @jensenpat.
The fix lines up cleanly with the sibling autotuneRequested / autotuneOnceRequested handlers right above it (same sliceId capture + m_radioModel.slice(sliceId) resolution pattern), and the diff is exactly what the description claims.
The active-slice guard is the right call: m_cwDecoder.estimatedPitch() only reflects the active slice's audio path (via routeCwDecoderOutput()), so refusing the tune when the clicked slice isn't active is strictly safer than the alternatives — and importantly it never falls back to mutating the wrong slice. Silent no-op is defensible as the minimal correctness fix; the disable/activate-then-tune UX variants are reasonable follow-ups but rightly out of scope here.
Nothing else to flag — single-file change, no conventions touched (no QSettings/RAII concerns), no resource paths, no boundary error handling needed (handler is a pure read+command issue).
🤖 aethersdr-agent · cost: $3.8052 · model: claude-opus-4-7
NF0T
left a comment
There was a problem hiding this comment.
Minimal, correct fix for a clear slice-ownership bug. The original handler capturing [this] and resolving via activeSlice() was the odd one out — both autotuneRequested and autotuneOnceRequested directly above it already capture sliceId and resolve via m_radioModel.slice(sliceId). This brings zero-beat into line with that established pattern.
The active-slice guard is the right call: m_cwDecoder.estimatedPitch() only reflects the active slice's audio (routeCwDecoderOutput() re-wires the decoder per active-slice change), so applying that pitch reading to a non-active slice would be wrong in a different way from the original bug. Safe no-op is the correct disposition. Verified the guard conditions are both necessary, and that after the guard slice and active are guaranteed to be the same object — applyTuneRequest is called correctly.
Frequency math unchanged and correct ((detected - configured) / 1.0e6 → MHz offset). applyTuneRequest routes through TuneIntent::IncrementalTune as expected. Slice 0 RX flow untouched. Fixes #2516 present in body.
✅ Approved.
…tive one (aethersdr#2516) (aethersdr#3312) Fixes aethersdr#2516 ## Problem Pressing **Zero Beat** on Slice A's VFO could tune **Slice B** instead. The `zeroBeatRequested` handler operated on `activeSlice()` rather than the slice that owns the `VfoWidget` that emitted the signal. The signal carries no `sliceId`, and clicking the Zero Beat child `QPushButton` is consumed by the button so it does **not** propagate to the panel's `mousePressEvent` activation hook — so pressing Zero Beat on Slice A does not activate Slice A first. If Slice B was the active slice (e.g. most-recently focused on another pan/band), Slice B got tuned — exactly the report ("Slice B VFO moves on two different bands"). The sibling autotune handlers right above it already do this correctly by capturing `sliceId` and resolving `m_radioModel.slice(sliceId)`. ## Fix Capture `sliceId` (matching the autotune handlers) and resolve the owning slice via `m_radioModel.slice(sliceId)`. Additionally short-circuit when that slice is **not the active one**: ```cpp connect(w, &VfoWidget::zeroBeatRequested, this, [this, sliceId]() { SliceModel* slice = m_radioModel.slice(sliceId); if (!slice) return; SliceModel* active = activeSlice(); if (!active || active->sliceId() != sliceId) return; // pitch valid only for active slice ... }); ``` ### Why the active-slice guard is required The shared `m_cwDecoder` is fed only by the **active** slice's audio, and its `estimatedPitch()` re-routes per active slice (`routeCwDecoderOutput()` / `setActiveSliceInternal`). So the detected pitch is only meaningful for the active slice — retargeting the tune to the button's slice while it is *not* active would apply a different slice's audio pitch to this slice's frequency, still wrong. The guard makes Zero Beat on a non-active slice a **safe no-op** rather than a wrong tune. The hard requirement is met two ways: it only ever resolves the tune against the button's own `sliceId`, and it bails unless that slice is also active. Does not touch slice 0 RX flow. ## UX decision This is the minimal correctness fix. Two UX alternatives — (a) activate the clicked slice first, defer one tick so the decoder re-routes, then tune; (b) disable the Zero Beat button in `VfoWidget` when its slice is inactive — were considered. The maintainer opted to keep the silent no-op for now, so they are intentionally **not** included here. ## Files - `src/gui/MainWindow.cpp` — `zeroBeatRequested` handler in `wireVfoWidget` ## Testing Builds clean (985/985, macOS). Manual: 1. Two pans; slices A and B on different bands, both CW. Make **B** active; ensure a detectable CW tone on A. 2. Press Zero Beat on **A**'s VFO → neither slice moves (safe no-op), and critically **B does not move** (previously it would have). 3. Make **A** active with a CW signal present; press Zero Beat on A → A tunes to zero-beat. Normal single-active-slice behavior is unchanged. 💻 Generated with Claude Code (Opus 4.8) with architecture by @jensenpat Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Fixes #2516
Problem
Pressing Zero Beat on Slice A's VFO could tune Slice B instead. The
zeroBeatRequestedhandler operated onactiveSlice()rather than the slice that owns theVfoWidgetthat emitted the signal. The signal carries nosliceId, and clicking the Zero Beat childQPushButtonis consumed by the button so it does not propagate to the panel'smousePressEventactivation hook — so pressing Zero Beat on Slice A does not activate Slice A first. If Slice B was the active slice (e.g. most-recently focused on another pan/band), Slice B got tuned — exactly the report ("Slice B VFO moves on two different bands").The sibling autotune handlers right above it already do this correctly by capturing
sliceIdand resolvingm_radioModel.slice(sliceId).Fix
Capture
sliceId(matching the autotune handlers) and resolve the owning slice viam_radioModel.slice(sliceId). Additionally short-circuit when that slice is not the active one:Why the active-slice guard is required
The shared
m_cwDecoderis fed only by the active slice's audio, and itsestimatedPitch()re-routes per active slice (routeCwDecoderOutput()/setActiveSliceInternal). So the detected pitch is only meaningful for the active slice — retargeting the tune to the button's slice while it is not active would apply a different slice's audio pitch to this slice's frequency, still wrong. The guard makes Zero Beat on a non-active slice a safe no-op rather than a wrong tune. The hard requirement is met two ways: it only ever resolves the tune against the button's ownsliceId, and it bails unless that slice is also active.Does not touch slice 0 RX flow.
UX decision
This is the minimal correctness fix. Two UX alternatives — (a) activate the clicked slice first, defer one tick so the decoder re-routes, then tune; (b) disable the Zero Beat button in
VfoWidgetwhen its slice is inactive — were considered. The maintainer opted to keep the silent no-op for now, so they are intentionally not included here.Files
src/gui/MainWindow.cpp—zeroBeatRequestedhandler inwireVfoWidgetTesting
Builds clean (985/985, macOS). Manual:
💻 Generated with Claude Code (Opus 4.8) with architecture by @jensenpat