[cw] Add CwxLocalKeyer drift-correction regression test#3271
Conversation
Co-authored-by: Codex <noreply@openai.com>
There was a problem hiding this comment.
Thanks for filling in the #2980 drift-correction coverage gap, @jensenpat — this is exactly the kind of focused regression test the keyer needed.
Production refactor is a no-op. The four extracted helpers (resetElapsed, startElapsed, elapsedMs, armTimer) preserve the existing semantics:
elapsedMs()'sisValid ? elapsed() : 0fallback is dead-but-harmless in production sincescheduleNext()always callsstartElapsed()before reading.onTick()moving from private to protected doesn't affect the in-classQObject::connect(&m_timer, &QTimer::timeout, this, &CwxLocalKeyer::onTick)(access check happens at the connect site, not the slot).- No other production code subclasses
CwxLocalKeyer, so adding thevirtualto the two seams has no observable effect outside the test.
Test design is solid. Driving onTick() synchronously while a fake armTimer records waits and a controllable elapsedMs substitutes the clock is the right call — much better than trying to chase Qt event-loop timing. The six scenarios (exact cadence, late-tick slip, sustained overload clamp, queued macro continuity, stop() reset, natural drain reset) cover the absolute-vs-relative scheduling concern well, and asserting the cumulative nextEdgeMs targets (60/120/180/240/300, then 480/540 across the queued boundary) is the key thing — it's exactly what would regress if someone "simplified" back to relative durations.
Nothing blocking from me. Build wiring in CMakeLists.txt matches the pattern of the neighboring cw_sidetone_test target. Nice work.
🤖 aethersdr-agent · cost: $4.3797 · model: claude-opus-4-7
…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 - Add a focused `cwx_local_keyer_drift_test` that drives `CwxLocalKeyer::onTick()` synchronously with a simulated elapsed clock. - Cover cumulative absolute edge targets, late-tick wait shortening, sustained-overload 1 ms catch-up clamping, queued macro continuity, and both `stop()` and natural-drain epoch reset paths. - Add a small protected seam around elapsed-clock reads and timer arming so production behavior stays the same while tests can avoid Qt event-loop timing races. ## Why Issue aethersdr#3219 tracks the missing automated coverage for the drift-corrected scheduling introduced for the macOS CWX sidetone stutter fix. Before this change, the keyer's `m_elapsed` / `m_nextEdgeMs` behavior was manually verified but not directly asserted by unit tests, leaving subtle absolute-vs-relative scheduling regressions easy to miss. Closes aethersdr#3219. ## Validation ```bash cmake -S . -B build-issue-3219 -G Ninja -DCMAKE_BUILD_TYPE=Debug -DCMAKE_OSX_SYSROOT=/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk cmake --build build-issue-3219 --target cwx_local_keyer_drift_test --parallel 22 ctest --test-dir build-issue-3219 -R cwx_local_keyer_drift_test --output-on-failure git diff --check ``` Note: this was a focused unit-test build; no full `AetherSDR.app` binary was produced. 👨🏼💻 Generated with OpenAI Codex (GPT-5.5 Pro 4/23) and tested by @jensenpat Co-authored-by: Codex <noreply@openai.com>
Summary
cwx_local_keyer_drift_testthat drivesCwxLocalKeyer::onTick()synchronously with a simulated elapsed clock.stop()and natural-drain epoch reset paths.Why
Issue #3219 tracks the missing automated coverage for the drift-corrected scheduling introduced for the macOS CWX sidetone stutter fix. Before this change, the keyer's
m_elapsed/m_nextEdgeMsbehavior was manually verified but not directly asserted by unit tests, leaving subtle absolute-vs-relative scheduling regressions easy to miss.Closes #3219.
Validation
cmake -S . -B build-issue-3219 -G Ninja -DCMAKE_BUILD_TYPE=Debug -DCMAKE_OSX_SYSROOT=/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk cmake --build build-issue-3219 --target cwx_local_keyer_drift_test --parallel 22 ctest --test-dir build-issue-3219 -R cwx_local_keyer_drift_test --output-on-failure git diff --checkNote: this was a focused unit-test build; no full
AetherSDR.appbinary was produced.👨🏼💻 Generated with OpenAI Codex (GPT-5.5 Pro 4/23) and tested by @jensenpat