Skip to content

[cw] drift-correct local CWX sidetone keyer#3202

Open
jensenpat wants to merge 1 commit into
aethersdr:mainfrom
jensenpat:aether/cwx-local-sidetone-drift
Open

[cw] drift-correct local CWX sidetone keyer#3202
jensenpat wants to merge 1 commit into
aethersdr:mainfrom
jensenpat:aether/cwx-local-sidetone-drift

Conversation

@jensenpat
Copy link
Copy Markdown
Collaborator

Summary

This is a clean replacement PR for the CWX local sidetone timing fix from #3109, rebased onto current upstream/main.

  • Replaces relative QTimer::start(durationMs) scheduling in CwxLocalKeyer with absolute, drift-corrected edge scheduling.
  • Tracks a QElapsedTimer epoch plus accumulated m_nextEdgeMs target so late GUI-thread timer dispatch is recovered on the following wait instead of accumulating.
  • Resets the timing epoch when CWX drains or is explicitly stopped, so each new run starts from a fresh clock.
  • Keeps the public API and keyStateChanged(bool) signal shape unchanged.

Root Cause

CWX local sidetone is driven from CwxLocalKeyer, which runs on the GUI thread and schedules each Morse element with m_timer.start(durationMs) relative to the time the previous timeout handler finally ran.

On macOS, panadapter/waterfall paint, VITA-49 burst handling, status processing, and general Cocoa run-loop coalescing can delay QTimer delivery even with Qt::PreciseTimer. With relative scheduling, every delayed timeout pushes every later edge farther behind. Over a long CWX macro that slip becomes audible as local sidetone stutter and can stretch letter/word spacing.

The radio's own CWX keyer remains authoritative for the on-air signal. This change only corrects AetherSDR's local sidetone cadence so it stays aligned with the intended CW timing under GUI load.

Implementation Notes

The scheduler now works like this:

  1. Start an elapsed-time epoch on the first element of a run.
  2. Accumulate the absolute target time for the next Morse edge in m_nextEdgeMs.
  3. Start the timer for target - elapsed, clamped to at least 1 ms.
  4. If a timeout arrives late, the next wait is shortened so the following edge returns to the intended timeline.
  5. Invalidate the elapsed timer and zero the target when the run drains or stop() cancels it.

Validation

Built and tested from current upstream/main plus this patch on macOS.

Automated checks run:

  • cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=RelWithDebInfo -DCMAKE_PREFIX_PATH=/opt/homebrew/opt/qt -DCMAKE_OSX_SYSROOT=/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk
  • cmake --build build --parallel 22
  • build/cw_sidetone_test passed
  • build/iambic_keyer_test passed
  • QT_QPA_PLATFORM=offscreen build/cwx_panel_test passed
  • ctest --test-dir build --output-on-failure -j 22 reached 21/22 passing; theme_manager_test is failing on current latest-main local preference/theme-import state and is unrelated to the two-file CwxLocalKeyer change.

Manual radio test performed by @jensenpat:

  • Created a CWX macro containing 30 S characters.
  • Watched each character's timing on the WAVE scope.
  • Aggressively dragged the panadapter during playback to create GUI-thread pressure.
  • Observed that character timing stayed lined up and the sidetone cadence sounded consistent throughout.

The corrected build was also deployed to the remote macOS test machine at patj@mac.jensencloud.net:/Users/patj/Desktop/AetherSDR.app and quarantine was cleared.

Reviewer Test Plan

  1. Build AetherSDR on macOS with the pinned CommandLineTools SDK.
  2. Connect to a radio in CW or CWL.
  3. Enable CW sidetone / PC audio and set CW speed around 20-30 WPM.
  4. Create a CWX macro with a repetitive pattern such as 30 S characters, or a long contest macro.
  5. Send the macro and watch timing on WAVE scope or another audio/timing view.
  6. While it sends, aggressively drag or resize the panadapter/waterfall to create GUI load.
  7. Expected result: local sidetone remains even and character timing does not progressively slip.
  8. Press Esc while CWX is active; expected result: CWX sends cwx clear and local sidetone stops.

Follow-Up Note

The title-bar TX cancel path forces MOX/CW/PTT down but does not currently clear the CWX local sidetone queue. Esc is the correct CWX abort path today. That is a separate UX/cancel-path follow-up from this timing drift fix.

👨🏼‍💻 Generated with OpenAI Codex (GPT-5.5 Pro 4/23) and tested by @jensenpat

…ethersdr#2980). Principle I.

CwxLocalKeyer schedules each element by calling m_timer.start(durationMs)
relative to "now" after every tick. On macOS the Cocoa run loop coalesces
GUI-thread sources (panadapter paint, VITA-49 burst handling, status
processing) and defers QTimer dispatch even with Qt::PreciseTimer set.
Each deferral pushes the next edge later; the slip accumulates over
the rest of the macro. At 20 WPM a dit is 60 ms, so 10–15 ms of jitter
per tick rots the dit/dah cadence within seconds — audible as stutter
and structurally wrong letter/word spacing.

JS69135 confirmed via WebSDR that the radio's on-air signal sounds
perfect: the air-side keyer (FlexLib's domain — Principle I, the radio
is authoritative for protocol behavior) is fine, and the symptom is
entirely AetherSDR's local sidetone. QSK-on exposes it because the
operator's ear has the radio's fast-keyed monitor as a reference; QSK-off
masks it because semi-break-in holds T/R continuously and small offsets
become inaudible.

Switch to drift-corrected absolute scheduling: track an QElapsedTimer
from the start of a run, accumulate target edge times in m_nextEdgeMs,
and set each m_timer.start() to the difference between target and
elapsed. A late tick is followed by a correspondingly shorter wait, so
the next edge lands on its absolute target instead of drifting later.

When the run drains (both queue and elements empty) or stop() is called,
m_elapsed is invalidated so the next start() begins a fresh epoch.

Public API and signal shape unchanged — only private state added.

Blast radius: risk_score=0.139, 1 high-risk affected
(MainWindow::MainWindow constructs CwxLocalKeyer; recompile-only, no
behavioral coupling — public surface and keyStateChanged signature
are unchanged).
Co-authored-by: Codex <noreply@openai.com>
@jensenpat jensenpat changed the title fix(cwx): drift-correct local sidetone keyer [cw] drift-correct local CWX sidetone keyer May 27, 2026
@jensenpat jensenpat marked this pull request as ready for review May 27, 2026 02:05
@jensenpat jensenpat requested a review from a team as a code owner May 27, 2026 02:05
Copy link
Copy Markdown
Contributor

@aethersdr-agent aethersdr-agent Bot left a comment

Choose a reason for hiding this comment

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

Reviewed the diff against main. The change is well-scoped to the two CwxLocalKeyer files and the implementation matches the description.

A few things I checked and was happy with:

  • Reset paths are symmetric. Both stop() and the final-drain branch in scheduleNext() invalidate m_elapsed and zero m_nextEdgeMs, so the next run starts from a fresh epoch. The partial-drain path (next Pending already queued) intentionally keeps the epoch alive — that's correct, since CWX text segments arriving back-to-back should stay drift-corrected against the same clock, and it also plays nicely with the m_currentlyDown CharGap injection just above it.
  • Catch-up after a large stall. When a timeout is delivered very late, the next wait clamps to 1 ms and elements fire back-to-back until the absolute timeline is caught up. That's the right behaviour for sidetone (the radio is the authority on what's on air) and matches the "long contest macro" scenario in the PR body.
  • Overflow. m_nextEdgeMs is qint64, wait is clamped [1, m_nextEdgeMs] before the int cast — safe for any realistic CW run length.
  • No AppSettings / RAII / lifetime concerns for this change.

Nothing else from me — clean drift-correction patch and the inline comment at CwxLocalKeyer.cpp explaining why the absolute clock is in use is exactly the kind of context that's worth keeping in the source.

Thanks for the careful write-up of the manual-test methodology, and for noting the title-bar TX cancel-path follow-up rather than scope-creeping it into this PR.

73 and thanks @jensenpat 👍


🤖 aethersdr-agent · cost: $5.2470 · model: claude-opus-4-7

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.

CWX: Saved Messages stuttering and timing spacing incorrect on macOS (CW timing issues)

1 participant