Skip to content

Make PTT (Hold) honor its reassigned key, not hardcoded Space (#3879). Principle VI.#3884

Merged
aethersdr-agent[bot] merged 1 commit into
mainfrom
aetherclaude/issue-3879
Jun 28, 2026
Merged

Make PTT (Hold) honor its reassigned key, not hardcoded Space (#3879). Principle VI.#3884
aethersdr-agent[bot] merged 1 commit into
mainfrom
aetherclaude/issue-3879

Conversation

@aethersdr-agent

Copy link
Copy Markdown
Contributor

Summary

Fixes #3879

What was changed

Make PTT (Hold) honor its reassigned key, not hardcoded Space (#3879). Principle VI.

Files modified

  • src/gui/MainWindow.cpp
  • src/gui/MainWindow.h
  • src/gui/MainWindowHelpers.h
  • src/gui/MainWindow_Shortcuts.cpp
 src/gui/MainWindow.cpp           |  2 +-
 src/gui/MainWindow.h             |  6 ++-
 src/gui/MainWindowHelpers.h      |  6 +++
 src/gui/MainWindow_Shortcuts.cpp | 81 ++++++++++++++++++++++++++--------------
 4 files changed, 66 insertions(+), 29 deletions(-)

Generated by AetherClaude (automated agent for AetherSDR)


🤖 aethersdr-agent · cost: $7.9097 · model: claude-opus-4-8

…. 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>
@aethersdr-agent aethersdr-agent Bot requested a review from a team as a code owner June 28, 2026 15:18
@aethersdr-agent aethersdr-agent Bot enabled auto-merge (squash) June 28, 2026 15:18
@aethersdr-agent aethersdr-agent Bot requested a review from a team as a code owner June 28, 2026 15:18

@ten9876 ten9876 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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; the cancelTransmitFromIndicator() manual-reset path updated to m_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 aethersdr-agent Bot merged commit b5f8e87 into main Jun 28, 2026
6 checks passed
@aethersdr-agent aethersdr-agent Bot deleted the aetherclaude/issue-3879 branch June 28, 2026 18:20
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>
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.

key reassignment on shortcut keys is not working after first use.

1 participant