fix(cwx): insert CharGap between live-mode Pending entries in CwxLocalKeyer#2754
Conversation
…lKeyer In live mode each keystroke calls CwxModel::sendChar() which enqueues a separate Pending entry via CwxLocalKeyer::start(). encode() always begins with firstChar=true, so no CharGap is prepended for the transition between entries. The last element of any encoded character is always Dit or Dah, leaving m_currentlyDown=true at the boundary; without an explicit gap the key stays continuously down and adjacent characters merge into undecodable noise in the local sidetone (aethersdr#2473). Fix: in scheduleNext(), when m_elements is exhausted and the next Pending entry is about to be dequeued, enqueue a CharGap first if m_currentlyDown is true. This keys the tone down, observes the standard 3-unit inter-character silence, then begins the next character's elements. No effect on buffer mode: a full-string transmission is a single Pending entry, so the queue boundary is never crossed and encode() handles all internal CharGaps via its own firstChar logic. Closes aethersdr#2473. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Nice fix — the diagnosis is spot-on and the code change is exactly the minimal correction needed. Thanks @NF0T.
A few things I traced and confirmed:
Ordering is correct. The CharGap is enqueued before encode() appends, so when execution falls through to the m_elements.dequeue() below, the gap plays first (3u of silence with keyStateChanged(false)), then the new character's first Dit/Dah fires keyStateChanged(true) cleanly. The keyDownNext != m_currentlyDown guard correctly fires on both transitions.
No double-gap risk. encode() always starts with firstChar=true and emits no leading CharGap, so the externally-inserted one isn't duplicated.
Slow-typing path unchanged. When the keyer fully drains before the next start(), the no-pending-no-elements branch at the top of scheduleNext() resets m_currentlyDown=false, so the new check is correctly skipped on the next entry.
Space-as-its-own-Pending still totals 7u. Traced K, , 7 typed as three separate live entries: K drains → boundary inserts CharGap (3u) → encode(" ") appends WordGap (4u) → 7 arrives with m_currentlyDown=false so no extra gap → 7 starts. Total inter-character = 3u + 4u = 7u, matches standard CW word spacing. Buffer mode unaffected as you noted.
stop() cleanup is fine — clears m_elements (so any pending CharGap is dropped) and resets m_currentlyDown, so ESC/clearBuffer mid-typing won't leave a stale gap.
Minor observation, not blocking: if a Pending entry contains only unknown characters, encode() adds nothing, and the externally-inserted CharGap will play as a lone 3u silence. Edge case, inaudible, not worth fixing on this PR.
LGTM modulo manual testing of the four cases in your test plan.
|
Claude here — landed at 22:36 UTC. Fourth contribution today, Ryan! 🎉 The diagnostic correction is the part that elevates this PR — second-receiver verification that over-the-air CW was already correct, and that the issue lived entirely client-side in CwxLocalKeyer, would have saved hours of barking up the wrong tree (the radio's command stream). The Pending-boundary analysis with the K→7 worked example is exactly the kind of writeup that makes future maintenance trivial. Surgical 9-line fix at the actual root cause — Four PRs today (#2745, #2747, #2752, #2754), four different code paths (RADE UI → RADE model → CWX UI → CWX core), four clean diffs. Genuinely outstanding day of contributions. 73, Jeremy KK7GWY & Claude (AI dev partner) |
…3644) ## Summary Fixes #3623 — the CWX local sidetone drops/clips individual CW elements under panadapter/VITA-49 load on Windows. ## Root cause `CwxLocalKeyer` scheduled every element edge with a `QTimer` parented to `MainWindow`, so the timer ran on the GUI event loop and `keyStateChanged` was delivered through a GUI-thread slot. Under panadapter paint + VITA-49 burst handling the loop coalesces both, the key gate lands a block or two late, and an element gets gapped/clipped. #3202's drift-correction fixes *cumulative* slip (the message stays in sync) but not this *per-edge* jitter, and reasons specifically about macOS — this is the residual Windows case. ## Fix Re-architect `CwxLocalKeyer` to mirror **`IambicKeyer`**, which already abandoned `QTimer` for exactly this reason (*"QTimer's jitter is too high for CW"*): - Run the element schedule on a dedicated worker thread. - Time each edge to an absolute `std::chrono::steady_clock` deadline waited on an interruptible `condition_variable` — no QTimer, no event loop. - Drive the sidetone gate via a lock-free `onKeyDownChange` callback; `CwSidetoneGenerator::setKeyDown` is `std::atomic`, the same gate path the iambic keyer uses. `encode()` and the drift-correction logic are unchanged, so message timing and the live-mode CharGap behaviour (#2473/#2754) are preserved. `MainWindow` now owns the keyer as a `unique_ptr` and stops it (joining the worker) before `AudioEngine` is torn down, since the gate callback touches `m_audio->cwSidetone()`. This is the approach recommended in the issue triage. ## Testing - **`cwx_local_keyer_drift_test` (#3271)** retargeted to the new clock seam (worker-disabled synchronous subclass + callback) — **passes**; all drift-correction assertions (absolute edge targets, slip-shortened waits, sustained-overload clamping, queued-macro epoch continuity, stop/drain resets) still hold, so the scheduling math is unchanged. - Clean build (Windows MSVC, Qt 6.10.3). - Ran the app against a synthetic FlexRadio source under a heavy spectrum load and sent long CWX macros: **CW sends cleanly at both 20 and 40 wpm — the element drops/stutter are gone**, including at 40 wpm where the tighter element timing is the more demanding case. Clean startup / connect / shutdown (worker joins without hanging). ## Known residual Verified clean at **both 20 and 40 wpm** under heavy synthetic load. A single faint, low-level click heard at 20 wpm did not recur and isn't speed-correlated (40 wpm — tighter timing — is completely clean), consistent with an incidental audio under-run under the deliberately brutal synthetic load rather than the keyer. `CwSidetoneGenerator` is unchanged by this PR and its raised-cosine envelope ramps cleanly across block-quantized edges. The "fullest fix" floated in the issue (advancing the gate by sample count inside `CwSidetoneGenerator::process()`) remains available as a follow-up if sub-block edge precision is ever wanted. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Summary
Fixes garbled local sidetone when typing characters quickly in CWX live mode. Closes #2473.
Confirmed via second receiver: over-the-air CW is correct. The radio's CWX buffer queues and times characters properly. The bug is client-side only — the local sidetone keyer was merging adjacent characters into undecodable noise.
Root cause
In live mode, each keystroke calls
CwxModel::sendChar()→CwxLocalKeyer::start(ch, wpm), enqueuing a separatePendingentry per character. WhenscheduleNext()exhausts the current character's elements and dequeues the nextPending, it callsencode()withfirstChar = true— so noCharGapis prepended for the transition.The last element of any encoded character is always Dit or Dah, leaving
m_currentlyDown = trueat the boundary. The guardif (keyDownNext != m_currentlyDown)never fires, so the key stays continuously down and adjacent characters merge. Example —K→7:CharGap← missing3 unitsTyping slowly is unaffected because the keyer fully drains between characters (
m_running = falseat the nextstart()call, no boundary transition occurs).Fix
In
scheduleNext(), before dequeuing the nextPendingentry, enqueue aCharGapifm_currentlyDownis true:This keys the tone down, waits the standard 3-unit inter-character silence, then begins the next character's elements normally.
Buffer mode is unaffected: a full-string transmission is a single
Pendingentry — the queue boundary is never crossed andencode()handles all internalCharGaps via its ownfirstCharlogic.Test plan
🤖 Generated with Claude Code