Make PTT (Hold) honor its reassigned key, not hardcoded Space (#3879). Principle VI.#3884
Merged
Merged
Conversation
…. Principle VI. PTT (Hold) has no QShortcut (QShortcut has no key-released signal), so it runs in the app-level event filter — which was hardcoded to Qt::Key_Space and ignored the rebindable ShortcutManager binding. Reassigning "PTT (Hold)" to another key (e.g. Up arrow) showed as moved in the dialog (ShortcutManager persists it) but never keyed the radio; Space kept transmitting. The reported "reverts to defaults" symptom is a perception side-effect: the rebind had no effect, so it read as not-saved. Persistence itself is correct (clearBinding saves empty, loadBindings preserves it). Fix mirrors the CW momentary path (handleCwMomentaryShortcut): resolve the key via shortcutSequenceFromKeyEvent() + actionForKey() and only PTT when the matched action id is ptt_hold. New handlePttHoldShortcut() helper invoked beside handleCwMomentaryShortcut in eventFilter. Adds shared kPttHoldActionId constant and renames m_spacePttActive -> m_pttHoldActive (no longer Space-specific). Cross-platform; the hardcode affected all platforms. Keeps the transmit path fail-closed — keys only on the operator's deliberately-bound key, only when shortcuts are enabled and the radio is connected (Principle VI). Blast radius: seed MainWindow::eventFilter risk_score=0.516 but upstream_count=0 (Qt-invoked override, no callers); the high_risk list is downstream callees spread across eventFilter's unrelated branches (AppSettings/ThemeManager via the applet-geometry and theme branches), none of which the PTT branch touches. No cross-TU symbol motion: handlePttHoldShortcut lives in the same TU that already uses actionForKey/shortcutSequenceFromKeyEvent, and kPttHoldActionId is a header-only inline constexpr — test-target link surface unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2 tasks
ten9876
approved these changes
Jun 28, 2026
ten9876
left a comment
Collaborator
There was a problem hiding this comment.
Approved. Full Principle VI review:
- Correct fix — PTT (Hold) now resolves its bound key via ShortcutManager (
kPttHoldActionId) instead of hardcoded Space, so a reassigned key actually keys TX (#3879). - Faithful + safe — press→
requestPttOn/release→requestPttOff, the connected/not-text-input gates (false-don't-consume when gated), and auto-repeat suppression are unchanged from the old Space path; keying still routes through the PTT coordinator →xmit→ interlock (Quindar preserved). Default Space still registered; thecancelTransmitFromIndicator()manual-reset path updated tom_pttHoldActive. - Consistent — uses the identical pattern to the existing CW momentary keys (
handleCwMomentaryShortcut), a proven momentary-keying path.
Residual stuck-TX risks are pre-existing and shared with the CW momentary keys, not introduced here: a modifier-combo binding released modifier-first won't match the unkey sequence, and there's no focus-loss auto-unkey (cancelTransmitFromIndicator() is the manual stop). Default plain-key bindings are immune. Filing a follow-up to add a focus-loss/deactivation auto-unkey safety net covering the whole momentary-keying family. CI green on all 6.
aethersdr-agent Bot
added a commit
that referenced
this pull request
Jun 28, 2026
…#3888). Principle VI. Closes two pre-existing stuck-TX windows in the momentary keyboard-keying family (PTT (Hold), CW straight key / paddles), surfaced by #3884's rebindable PTT-hold key. 1. Focus loss / deactivation while held — the KeyRelease that would un-key goes to whatever app gains focus and never reaches our event filter, so the flag stayed set and TX stayed keyed. Add failSafeMomentaryKeyingToRx(): on QEvent::ActivationChange (window deactivated) and, as a macOS belt-and-suspenders, QEvent::ApplicationStateChange (app backgrounded), clear every momentary flag and issue the matching un-key. It reuses the existing requestPttOff / setCw*State paths so the interlock gating and Quindar outro still run, and no-ops when nothing is keyed. 2. Modifier-combo binding released modifier-first — a binding like Ctrl+T released Ctrl-first delivers a KeyRelease whose QKeySequence(modifiers|key) is now plain T and no longer matches Ctrl+T, so the un-key never fired. While a momentary action is active, also accept a release matched by the bound base key alone (keyEventMatchesActionBaseKey), so releasing any part of the combo fails safe to RX. Client-side only; the un-key terminates in the existing xmit / sendCwKey paths with their interlock gating intact. A held transmit key must never strand the transmitter — fail-safe-to-RX is the Principle VI expectation. Blast radius: risk_score=0.044 on handlePttHoldShortcut, 3 high-risk affected (top: MainWindow::eventFilter, RadioModel::transmitModel, RadioModel::isConnected). All are the existing caller/un-key path this change reuses; no signatures change and the new changeEvent branch is a separate ActivationChange early-path that never touches the minimal-mode AppSettings logic. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #3879
What was changed
Make PTT (Hold) honor its reassigned key, not hardcoded Space (#3879). Principle VI.
Files modified
src/gui/MainWindow.cppsrc/gui/MainWindow.hsrc/gui/MainWindowHelpers.hsrc/gui/MainWindow_Shortcuts.cppGenerated by AetherClaude (automated agent for AetherSDR)
🤖 aethersdr-agent · cost: $7.9097 · model: claude-opus-4-8