feat(shortcuts): add Master Volume Up/Down configurable keyboard shortcuts (#3791)#3820
Conversation
…tcuts (aethersdr#3791) The keyboard-shortcut action table (registerShortcutActions, Audio category) exposed per-slice AF Gain Up/Down, mute toggles, master mute, and squelch — but no master volume up/down, even though the identical nudge logic already shipped for external controllers (Aether Control / StreamDeck "VolumeUp"/"VolumeDown" in MainWindow_Controllers.cpp). Register master_volume_up / master_volume_down in the Audio category, reusing that exact path: clamp MasterVolume ±5 in [0,100], update the title-bar slider via TitleBar::setMasterVolume(), and push to the audio path via applyMasterVolume() — keeping GUI, persistence, and TCI in sync. Registered with no default key (Up/Down are taken by per-slice AF Gain) and autoRepeat=true so holding the bound key ramps continuously. Fixes aethersdr#3791 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Reviewed and verified against the tree — this is clean. Thanks for the well-scoped change and the clear write-up, @jensenpat. 👍
What I checked:
- Faithful to the shipped path. The two handlers are an exact mirror of the existing
VolumeUp/VolumeDowncontroller dispatch (MainWindow_Controllers.cpp:1014–1023): samestd::clamp(... ±5, 0, 100), samem_titleBar->setMasterVolume(next), sameapplyMasterVolume(next). Good reuse rather than a parallel implementation. - State stays consistent across repeats.
applyMasterVolume()(MainWindow.cpp:5556) writesMasterVolumeback viaAppSettings+save()and clamps internally, so the next read reflects the prior nudge —autoRepeat=trueramps correctly, and GUI/persistence/TCI stay in lockstep. - No null-pointer / include risk.
m_titleBaris null-guarded (matches the controller path);<algorithm>is already included (MainWindow_Shortcuts.cpp:49,std::clampused in several existing handlers); and the addedTitleBar.his genuinely needed sinceMainWindow.honly forward-declares it. registerActionsignature supports the trailingbool autoRepeat(ShortcutManager.h:31).
Conventions (AppSettings, RAII, C++20 std::clamp) are all honored, and the change is confined to the stated scope. No blocking concerns from me.
One small optional thought (non-blocking): the read-clamp-write logic now lives in three places (here + both controller dispatch sites). If you ever touch this again, a tiny nudgeMasterVolume(int delta) helper on MainWindow would let all three call sites share it — but that's cleanup for another PR, not something to hold this one for.
🤖 aethersdr-agent · cost: $6.6801 · model: claude-opus-4-8
ten9876
left a comment
There was a problem hiding this comment.
Trivial, correct. The two new registrations reuse the exact master-volume nudge logic the controller VolumeUp/Down path already ships (clamp MasterVolume ±5 in [0,100] → m_titleBar->setMasterVolume → applyMasterVolume), so GUI/persistence/audio stay in sync with no new logic. No default key (correct — Up/Down are AF-Gain), autoRepeat=true for hold-to-ramp (consistent with rf_gain), TitleBar.h include added. No TX/safety surface. Nicely bridge-proven via the action-combo setCurrentText echo + filtered dialog grab.
Non-blocking nit: the ±5 nudge now lives in ~4 spots — a small nudgeMasterVolume(delta) helper would DRY it up, but that duplication predates this PR. Thanks @jensenpat.
Summary
Adds Master Volume Up and Master Volume Down to the configurable keyboard-shortcut list (Audio category), as requested in #3791.
Root cause
MainWindow::registerShortcutActions()(src/gui/MainWindow_Shortcuts.cpp) registers the Audio category with per-slice AF Gain Up/Down, mute/mute-all, master mute, and squelch — but no master volume up/down. The user is correct that this is simply a gap: the identical nudge logic already ships for external controllers (the Aether Control / StreamDeck"VolumeUp"/"VolumeDown"dispatch inMainWindow_Controllers.cpp).Fix
Register
master_volume_up/master_volume_downin the Audio category, reusing that exact, already-shipped path:MasterVolume±5 into[0,100],TitleBar::setMasterVolume(),applyMasterVolume()— keeping GUI, persistence, and TCI in sync.Registered with no default key (Up/Down are taken by per-slice AF Gain, and master volume is a distinct concept — the user binds keys themselves) and
autoRepeat=trueso holding the bound key ramps continuously (consistent withrf_gain_up/downand the tune actions). Single TU; one added include (TitleBar.h, needed for thesetMasterVolumecall sinceMainWindow.honly forward-declares it).How the agent automation bridge proved it
Built and driven through the bridge (
AETHER_AUTOMATION=1). The two new entries had to be shown to actually appear in the Configure Shortcuts… list.Reaching that dialog needed the bridge fidelity work in #3819 (closed-menu
QActiontrigger + per-pid sockets), so the proof was captured against a local merge of this branch withfeat/automation-bridge-fidelity. This branch itself is the single-file change above.1 — machine-readable. The dialog's Action selector is a non-editable
QComboBoxpopulated directly fromShortcutManager::actions().setCurrentTextonly takes if the item exists, so an exact echo is proof of registration:2 — visual. With the dialog's search filter set to
Master Volume, agrabofAetherSDR::ShortcutDialogshows exactly two rows — Master Volume Up and Master Volume Down, both Audio category, both with empty Current/Default Key (no default binding, as designed).MainWindow_Shortcuts.cpp)registerActionentries reusing the shipped controllerVolumeUp/Downnudge; +1 includesetCurrentTextexact-echo (both entries) + filtered dialoggrabshowing both Audio rows💻 Generated with Claude Code (Opus 4.8) with architecture by @jensenpat