Add CW keyboard USB HID and MIDI keying controls#2361
Add CW keyboard USB HID and MIDI keying controls#2361jensenpat wants to merge 3 commits intoten9876:mainfrom
Conversation
Add shared CW straight-key, dit, and dah actions for keyboard shortcuts and MIDI mappings; persist the XML-safe shortcut IDs; normalize legacy dotted CW MIDI IDs; and make the TX status badge an emergency transmit cancel control. Add focused shortcut and MIDI settings regression tests. Co-authored-by: Codex <[email protected]>
There was a problem hiding this comment.
Thanks @jensenpat — this is a thorough, well-tested rollup. Persisting CW shortcuts via XML-safe IDs (cwkey/cwdit/cwdah) plus the bidirectional MIDI migration is the right call (avoids touching AppSettings's element-name validator), and pulling the initial syncLocalKeyerToRadio() after the iambic callbacks are installed fixes a real ordering bug. The new shortcut_manager_test and the expanded midi_settings_test (plus the missing add_test(NAME midi_settings_test ...) registration) close the regression window neatly.
A few notes:
1. TCP backstop is re-enabled — please double-check on a connected radio. RadioModel::sendNetCwCommand now ends with sendCmd(fullCmd) after the four UDP copies. The previous comment that this code is replacing said the opposite — that fw 4.1.5 rejected every CW command over TCP with 0x50001000 ("command syntax error") and the TCP send was producing log noise. The new comment says the 16-bit timestamp format makes it dedupable. Worth confirming aethersdr.log doesn't show a 0x50001000 per keying event; if it does, this will spam logs even if keying still works via UDP. If you have a fresh on-radio capture confirming clean TCP responses with the new timestamp format, calling that out in the PR description would be helpful for the next person to revisit this.
2. m_currentMidiTrace stashing pattern. In MainWindow.cpp the lambda for paramActionTrace writes m_currentMidiTrace = {paramId, traceId, midiCallbackMs, midiDispatchMs}, calls the setter (which reads it via setCwStraightKeyState etc.), and then clears it. This works because the call is synchronous on the main thread, but it's brittle — if any future MIDI setter ever defers via QMetaObject::invokeMethod or a QTimer, the trace will be empty by the time it lands. A small comment on the MidiActionTrace struct in MainWindow.h noting "valid only during synchronous setter dispatch" would help.
3. shortcutSequenceFromKeyEvent Qt6 deprecation. QKeySequence(static_cast<int>(modifiers) | ev->key()) works but is deprecated in Qt6 in favor of QKeyCombination. Not a blocker — most of the codebase still uses the int form — but if you're touching it anyway, QKeySequence(QKeyCombination(modifiers, static_cast<Qt::Key>(ev->key()))) is the modern form.
4. m_lastCwPaddleTraceId semantics. The atomic holds the most recent paddle-input trace, which iambic-generated dit/dah edges then attribute to. That's fine for tracing but means a single squeeze produces N edges all tagged with the original press's trace ID — readers of aether.cw should know that's expected. Minor.
Scope review: all 15 files map to the stated PR scope (CW keyboard/MIDI, persistence, TX cancel badge, netCW diagnostics rolled up from #2336). Nothing outside scope. Conventions look good — AppSettings not QSettings, atomics for cross-thread state, RAII throughout.
Use event COMMENT only — leaving final approval to maintainers since the TCP backstop change has practical impact worth a maintainer's eyes.
|
Claude here on Jeremy's behalf — really nice work, and the persistence-bug catch (dotted shortcut IDs being silently dropped because AppSettings won't write XML-invalid element names) is a great find. Hardware validation on both keyboard + MIDI paths is exactly the rigor we want. Two requests before merge: 1. Rebase onto current 2. Resolve the atomic-naming overlap. This is the one piece that needs explicit attention:
You have two clean options on rebase:
I lean toward option 2 since the new names describe their actual role better, but either is fine. The other small overlap (paramAction signal in MidiControlManager + stale comment in MainWindow.h:327) should auto-resolve cleanly because main deleted those lines and your branch didn't touch them — standard 3-way merge drops them. Once rebased + atomic-naming resolved, this is a clean admin-merge. 73, Jeremy KK7GWY & Claude (AI dev partner) |
- Add CW keyboard and MIDI controls (#2361) - KeyboardMapWidget: fix J tile mapping to Qt::Key_I - MainWindow: don't block CW/Space PTT shortcuts during slider lease - Honor cw break_in setting for keyboard CW keying
|
Claude here on Jeremy's behalf — thanks @jensenpat, this was solid work and exactly the kind of operator-facing CW polish AetherSDR needed. Merged into main as b4283a9 via #2391. You're preserved as Co-Author on the squashed commit, alongside Codex. A few things changed on the way in:
Hardware-validated against FLEX-8600 fw 4.1.5 on the way through. Closing this PR in favor of #2391. Really nice contribution — looking forward to more. 73, Jeremy KK7GWY & Claude (AI dev partner) |
Summary
This PR rolls up the CW/netCW work that started from PR #2336 and adds operator-facing keyboard and MIDI controls for manual CW keying.
It includes:
aether.cwlogs.Trigger straight key,Trigger CW Left Paddle, andTrigger CW Right Paddle.cwkey,cwdit, andcwdah.cw.key,cw.dit,cw.dah) so older test bindings still load and save back ascwkey,cwdit, andcwdah.TXstatus badge that acts as an emergency TX cancel control.CW Keying Behavior
Trigger straight key
Trigger straight keyis implemented as a true momentary control. Keyboard press and MIDI gate-on assert the netCW key path; key release and MIDI gate-off release it.On press, AetherSDR sends:
cw ptt 1cw key 1On release, AetherSDR sends:
cw key 0cw ptt 0This preserves the required ordering for break-in-off setups where the radio needs explicit netCW PTT around key state.
Trigger CW Left Paddle / Right Paddle
The left and right paddle controls feed the local iambic keyer when it is running. That keeps sidetone and on-air timing aligned with the configured CW speed, and it emits netCW key edges for generated dits/dahs.
If the local keyer is not running, the paddle path falls back to a direct held-key path so the radio still receives key-down/key-up state.
MIDI Changes
The MIDI controls now reuse the same CW action IDs and display names as keyboard shortcuts:
cwkey/Trigger straight keycwdit/Trigger CW Left Paddlecwdah/Trigger CW Right PaddleMidiSettingsnormalizes legacy dotted CW param IDs on read and write:cw.key->cwkeycw.dit->cwditcw.dah->cwdahThis keeps older mappings from being stranded while avoiding dots in saved CW action IDs going forward.
Keyboard Shortcut Persistence
The original CW shortcut IDs used dots, which produced settings keys like
Shortcut_cw.key.AppSettingsintentionally skips invalid XML element names, so those shortcuts were not persisted.This PR avoids changing the XML/settings handler and instead renames the CW shortcut IDs to XML-safe names:
Shortcut_cwkeyShortcut_cwditShortcut_cwdahA new
shortcut_manager_testverifies that the CW shortcuts save and reload from the database, and that no dotted CW shortcut keys are written.TX Cancel Badge
The large red status-bar
TXindicator is now clickable. Left-clicking it sends a conservative emergency cancel sequence intended for a stuck transmit condition:cw key 0cw ptt 0transmit tune 0xmit 0The handler checks Multi-Flex ownership before acting. If another station owns TX and this client is not locally transmitting, it reports that TX is owned elsewhere instead of pretending this client can cancel another station's transmit.
netCW Diagnostics
The CW/netCW log path records each stage of MIDI/keyboard input and netCW scheduling, including trace IDs, source labels, timing deltas, VITA stream/index details, UDP copy timing, and TCP fallback/backstop information.
During local on-air testing, the reviewed netCW traces showed:
cw ptt 1 -> cw key 1cw key 0 -> cw ptt 0Validation
Built with the project-required 22 parallel jobs:
Ran focused tests:
ctest --test-dir build-ninja --output-on-failure -R "(shortcut_manager_test|midi_settings_test|radio_status_ownership_test)"Result: 3/3 tests passed.
Also validated manually by @jensenpat:
Reviewer Notes
The actual netCW UDP datagrams are still sent on the
PanadapterStreamnetwork thread, while timing/scheduling is initiated fromRadioModel. This PR does not move netCW scheduling to a dedicated high-priority timer thread; it focuses on functional CW controls, persistence, diagnostics, and a practical stuck-TX escape hatch.👨🏼💻 Generated with OpenAI Codex (GPT-5.5 Pro 4/23) and tested by @jensenpat