Show LOCKED feedback for blocked slice tuning#2905
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds unified UI feedback when a user (or control surface) attempts to tune a slice while that slice is locked, by introducing a slice-level “tune blocked” signal and wiring multiple tuning entry points to it.
Changes:
- Add
SliceModel::tuneBlockedByLock()signal (+notifyTuneBlockedByLock()helper) and emit it whensetFrequency()/tuneAndRecenter()are blocked by lock. - Display a temporary
LOCKEDindicator in the VFO and RX applet frequency readouts when blocked tuning is attempted (with a short timer-based clear). - Update several MainWindow tuning paths (FlexControl, HID, MIDI, keyboard/band shortcuts, memory recall, etc.) to trigger the same blocked-tune feedback mechanism.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/models/SliceModel.h | Adds the new blocked-tune notification API (method + signal). |
| src/models/SliceModel.cpp | Emits blocked-tune feedback when tuning methods are prevented by slice lock. |
| src/gui/VfoWidget.h | Adds timer/state needed to show and clear LOCKED feedback. |
| src/gui/VfoWidget.cpp | Routes blocked wheel/direct-entry tuning to slice notification and displays temporary LOCKED. |
| src/gui/RxApplet.h | Adds timer/state and helpers for LOCKED feedback in the RX applet frequency display. |
| src/gui/RxApplet.cpp | Displays temporary LOCKED on blocked tuning attempts and routes relevant UI events through the notifier. |
| src/gui/MainWindow.cpp | Ensures multiple tuning/control-surface paths trigger the same blocked-tune feedback behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Thanks @rfoust — nice clean approach to routing all the blocked tuning paths through a single tuneBlockedByLock() signal. The notifyTuneBlockedByLock() helper guarding the emit with if (m_locked) is a nice touch (safe to call from anywhere). The 500 ms feedback + cancel-direct-entry-on-lock interaction read well.
Blocking — accidental reversion of three merged PRs
The diff includes changes to three files that aren't in the stated scope and that revert commits already on main:
src/gui/AetherialAudioStrip.cpp— reverts #2891 (#2902) "clarify BYPASS tooltip — Final Output Stage stays active". The diff rewinds the tooltip back to the old wording.src/gui/DxClusterDialog.cpp— reverts #2903 (which merged #2890) "raise DX Spots Levels slider cap 10 → 15". The diff puts the slider range back to1..10.src/gui/WaveApplet.cpp— reverts #2827 "persist WaveApplet drawer collapsed state across restarts". The diff removes theWaveApplet_DrawerExpandedread/write and hardcodessetSettingsExpanded(true).
Looks like the branch is based on a main from before those three merges landed (all merged 2026-05-19/20). Could you rebase onto current main and force-push? That should drop these unrelated changes and leave only the 7 files listed in the PR description.
Minor observations on the intended scope (non-blocking)
RxApplet::connectSlicelock handler (RxApplet.cpp:1670): on lock, it stuffs the current freq intom_freqEditbefore switching the stack back to index 0. Since the edit isn't visible anymore, thesetTextis just bookkeeping for the next entry — fine, just worth a one-line comment if you respin.MainWindow::registerShortcutActionsband-shortcut path: the de-nesting (early-return on!s) is a nice cleanup but slightly outside the lock-feedback scope. No issue, just noting.
Everything else in the intentional scope looks good. Ping me once it's rebased.
There was a problem hiding this comment.
Thanks @rfoust — nice, well-scoped change. The single tuneBlockedByLock signal on SliceModel with a notifyTuneBlockedByLock() helper that guards on m_locked is a clean design, and routing every tune entry point (FlexControl, MIDI, HID, keyboard step, band shortcut, memory recall, direct entry, wheel, applyTuneRequest, setFrequency/tuneAndRecenter) through it gives consistent UX. The 500ms single-shot timer + clearLockedFrequencyFeedback() on setSlice() and unlock both look correct, and updateFreqLabel() properly suppresses frequency redraws while LOCKED is on screen so a fresh frequencyChanged won't repaint over the feedback.
A couple of small things worth a look — non-blocking:
-
MainWindow.cpp— FlexControl frequency-mode init path (aroundif (s->isLocked())near line ~3510): the new branch resetsm_flexTargetMhz = -1.0on lock, whereas the originalif (!s || s->isLocked()) return;left it alone. Probably intentional (the coalesce timer would have cleared it on the next idle anyway), but if a user spins, locks mid-spin, then unlocks, the in-flight tune target is now thrown away rather than continuing. Confirm that's the behavior you want. -
MainWindow.cpp— MIDI absolute fallback (around line ~14548): the lock check happens beforesteps = v - 64; if (steps == 0) return;. So a centered absolute controller value (steps=0, would have been a no-op) on a locked slice will still flash "LOCKED" briefly. Minor — moving thesteps == 0early-return ahead of the lock check would avoid the spurious feedback for absolute MIDI knobs that pass through center. -
RxApplet.cpp—connectSlice()lock handler: the innerif (m_slice)guard inside the lambda is redundant (we're already inconnectSlice(s)and the lambda capturesthis;m_slice == shere). Cosmetic.
Everything else looks good — RAII via QTimer member, no new persistence to worry about (so no AppSettings/QSettings concern), no obvious null-pointer or leak risks, and the file set matches the stated scope.
281ce55 to
676e8e7
Compare
676e8e7 to
88851ff
Compare
|
Thanks for the review. I pushed an updated single-commit branch that addresses the feedback:
Validation:
The branch remains one signed commit, and GitHub shows it as Verified. |
Summary
LOCKEDin the VFO and RX applet frequency displays when locked tuning is attempted.Testing
git diff --checkcmake --build build --clean-first -j8