feat(keyer): split Keyer and Straight Key pages, allow both interfaces at once - #5
Open
tcpreplay-dev wants to merge 2 commits into
Open
tcpreplay-dev wants to merge 2 commits into
tcpreplay-dev wants to merge 2 commits into
Conversation
tcpreplay-dev
force-pushed
the
feature/keyer-options-ui
branch
from
August 25, 2026 15:59
bb24cb4 to
4c430b0
Compare
tcpreplay-dev
force-pushed
the
feature/keyer-options-ui
branch
from
August 25, 2026 17:01
4c430b0 to
ff4aefc
Compare
4 tasks
tcpreplay-dev
marked this pull request as ready for review
August 25, 2026 17:25
…s at once Previously one HaliKey device served both roles, with a global "straight key mode" flag deciding which. An operator with both a paddle and a straight key had to reconfigure to switch. Role is now determined by the device an edge arrives on, so both can be connected together. Settings move to per-role groups (keyer/*, straightkey/*), migrating the old halikey/* values into whichever role the retired straightKeyMode flag selected, so existing setups survive the upgrade. Options dialog: - "HaliKey" renamed to "Keyer"; new "Straight Key" page after it. Keyers are the common case, so that page stays first. - Keyer page gains a Radio Keyer section for KP: iambic A/B, paddle orientation, and keying weight as a slider with the 8 values KP accepts (090-125 in fives), shown as a 0.90-1.25 ratio rather than the raw integer. All three ride in one command, so a change to any re-sends the other two; the page follows the radio's echo rather than assuming its writes landed. Local paddle swap is kept in a separate group, since it composes on top of the radio's orientation rather than replacing it. - Straight Key page carries the timing buffer: an on/off switch plus speed bounds and dah/dit ratio, with the resulting delay shown live in ms. The operator states how they send; the buffer arithmetic stays out of the UI. - Connection controls are one shared widget, so the two pages cannot drift and the port-conflict rule has a single home. Auto-connect replaces the Refresh button. A gated poll drives both reconnect on hotplug and list freshness, so the button had nothing left to do. The port list rebuilds only when the set actually changes, otherwise an open popup or a live selection gets yanked away mid-interaction. Port conflict is enforced in software because the platforms disagree: Windows opens exclusively and fails the second attempt, but macOS and Linux both succeed silently, leaving two readers on one key. Comparison is by port string, so on macOS one interface appearing as both cu.* and tty.* is not caught — called out in the page's help text rather than papered over. Defaults: iambic B, weight 1.10, slowest 20 WPM, dah/dit 4.0 (390 ms pre-roll). Also removes HalikeyDevice::availablePorts() and the halikeyEnabled setting, both of which had no callers. Note: buttonrowdispatcher still falls back to iambic A when KP is unreported, where this page now falls back to B. The window is only until the RDY dump lands, but the two disagree in it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Paddle orientation, iambic mode and weight are mirrored into RadioState after the KP is sent. The K4 does not echo KP, so waiting for it left the local iambic keyer on its old orientation — the controls looked applied but did nothing. buttonrowdispatcher already did this for its N/R toggle. "Fastest I send" sets the contact-bounce floor at half a dit. It only tightens the fixed 10ms floor once the declared speed passes ~60 WPM, where 10ms would start clipping real elements. A straight key's speed is the operator's hand, so unlike the paddle it cannot be read from KS. The speed pair is kept ordered in the setters, after load, and in the spinboxes themselves.
Crash on quit: EXC_BAD_ACCESS at 0x8 in QObject::thread(), called from TcpClient::sendCAT — a null/dangling client, since thread() reads d_ptr at offset 8. Qt deletes MainWindow's children in construction order, so ConnectionController (built during startup) is destroyed well before the Options dialog, which is created lazily on first open. Destroying the Keyer page's radio buttons emits toggled, which reached sendKeyerPaddle() and dereferenced the freed client. This page is the first options page to send CAT at all — the previous one sent none — so the KP controls exposed a hazard that was already latent: ~ConnectionController deleted m_tcpClient without clearing the pointer, and sendCAT dereferenced it unguarded. Fixed at both levels: the pointer is nulled and the send paths check it, which covers every caller rather than just this one; and the page stops sending once its destructor runs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tcpreplay-dev
force-pushed
the
feature/keyer-options-ui
branch
from
August 25, 2026 21:34
ff4aefc to
736ef10
Compare
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.



Stacked on #4. Separated deliberately from the keying work so this UI direction can be
rejected without losing it.
Why
One HaliKey device served both roles, with a global "straight key mode" flag deciding which.
An operator owning both a paddle and a straight key had to reconfigure to switch. Role is now
determined by the device an edge arrives on, so both can be connected at once.
Options dialog
"HaliKey" → "Keyer", with a new Straight Key page after it. Keyers are the common
case, so that page stays first.
Keyer page gains a Radio Keyer section for
KPonly —KS,CW,SDandMLalreadyhave main-window controls and stay out of it:
KPaccepts (090–125 in fives),shown as a 0.90–1.25 ratio rather than the raw integer
KPcarries all three fields in one command, so changing any one re-sends the other two, andthe page follows the radio's echo rather than assuming its writes landed. The local paddle
swap sits in its own group because it composes on top of the radio's orientation rather
than replacing it — two controls that otherwise look like they do the same thing.
Straight Key page carries the timing buffer from #4: an on/off switch, speed bounds, and
dah/dit ratio, with the resulting delay shown live in ms. The operator states how they send;
the buffer arithmetic stays out of the UI.
Connection controls are one shared widget, so the two pages can't drift and the port-conflict
rule has a single home.
Auto-connect replaces Refresh
A gated poll drives both reconnect-on-hotplug and list freshness, leaving the button nothing
to do. Two details that matter: the list rebuilds only when the port set actually changes
(otherwise an open popup or a live selection gets yanked away), and polling is gated rather
than always-on, since Windows enumeration is materially more expensive than macOS IOKit.
Port conflict is enforced in software
The platforms disagree. Windows opens exclusively and fails the second attempt; macOS and
Linux both succeed silently, leaving two readers on one key. So the guard can't rely on the
OS.
Comparison is by port string, so on macOS one interface appearing as both
cu.*andtty.*isn't caught — stated in the page's help text rather than papered over.
Settings migration
Keys move to per-role groups (
keyer/*,straightkey/*), migrating the oldhalikey/*values into whichever role the retired
straightKeyModeflag selected, so existing setupssurvive the upgrade.
Defaults: iambic B, weight 1.10, slowest 20 WPM, dah/dit 4.0 → 390 ms pre-roll.
Also removes
HalikeyDevice::availablePorts()and thehalikeyEnabledsetting, both ofwhich had no callers.
Known inconsistency
buttonrowdispatcher.cpp:181still falls back to iambic A whenKPis unreported, wherethis page falls back to B. The window is only until the
RDYdump lands, but the twodisagree in it. Left alone rather than changed silently.
Testing
Built and exercised against a real K4.
IambicKeyerTestspasses. Not yet built on Windows orLinux — the layout uses minimum widths rather than fixed pixels for the new controls, but the
wider default fonts there are untested.