Skip to content

HOLD FOR TESTING - fix(cwx): drift-correct local sidetone keyer to stop macOS stutter (#2980). Principle I.#3109

Closed
aethersdr-agent[bot] wants to merge 1 commit into
mainfrom
aetherclaude/issue-2980
Closed

HOLD FOR TESTING - fix(cwx): drift-correct local sidetone keyer to stop macOS stutter (#2980). Principle I.#3109
aethersdr-agent[bot] wants to merge 1 commit into
mainfrom
aetherclaude/issue-2980

Conversation

@aethersdr-agent

Copy link
Copy Markdown
Contributor

Summary

Fixes #2980

What was changed

fix(cwx): drift-correct local sidetone keyer to stop macOS stutter (#2980). Principle I.

Files modified

  • src/core/CwxLocalKeyer.cpp
  • src/core/CwxLocalKeyer.h
 src/core/CwxLocalKeyer.cpp | 18 +++++++++++++++++-
 src/core/CwxLocalKeyer.h   |  8 ++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)

Generated by AetherClaude (automated agent for AetherSDR)


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

…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).
@aethersdr-agent aethersdr-agent Bot requested a review from a team as a code owner May 25, 2026 04:57
@aethersdr-agent aethersdr-agent Bot enabled auto-merge (squash) May 25, 2026 04:57
@jensenpat jensenpat changed the title fix(cwx): drift-correct local sidetone keyer to stop macOS stutter (#2980). Principle I. HOLD FOR TESTING - fix(cwx): drift-correct local sidetone keyer to stop macOS stutter (#2980). Principle I. May 25, 2026

@aethersdr-agent aethersdr-agent Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reviewed. The fix is correct and minimal — exactly the right shape for this class of timing bug.

Looks good:

  • Absolute-target scheduling against QElapsedTimer is the textbook fix for event-loop coalescing drift; qMax<qint64>(1, m_nextEdgeMs - m_elapsed.elapsed()) correctly clamps a late wakeup to an immediate catch-up tick instead of producing a negative or zero wait.
  • State lifecycle is handled cleanly: m_elapsed is invalidated and m_nextEdgeMs zeroed in both stop() and the natural-completion branch of scheduleNext(), so a subsequent start() re-anchors the clock via the !m_elapsed.isValid() check.
  • Continuity is preserved when start() is called mid-transmission (new Pending enqueued while m_running): the elapsed reference is intentionally not reset, so inter-segment timing stays aligned.
  • Comments explain the why (panadapter paint / VITA-49 bursts coalescing on macOS GUI thread) rather than restating the what. Nice.
  • static_cast<int>(wait) is safe here because wait is bounded by at most one element duration (≤ 4 × m_unitMs) once steady-state is reached.

Conventions: RAII-clean, no QSettings/AppSettings touch (none needed for this scope), header include for QElapsedTimer added correctly, scope limited to the two stated files.

No blockers from me. The "HOLD FOR TESTING" tag is the right call — drift behavior under sustained load only really shows up on a real macOS GUI loop, so the manual side of validation is what matters here. Thanks for the careful comment trail tying this back to #2980.


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

@ten9876 ten9876 marked this pull request as draft May 25, 2026 07:28
auto-merge was automatically disabled May 25, 2026 07:28

Pull request was converted to draft

@jensenpat jensenpat self-assigned this May 26, 2026
@jensenpat jensenpat closed this May 27, 2026
ten9876 pushed a commit that referenced this pull request May 27, 2026
## 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

Co-authored-by: aethersdr-agent[bot] <273844287+aethersdr-agent[bot]@users.noreply.github.com>
Co-authored-by: Codex <noreply@openai.com>
G6PWY-Chris pushed a commit to G6PWY-Chris/AetherSDR that referenced this pull request Jun 22, 2026
## Summary

This is a clean replacement PR for the CWX local sidetone timing fix
from aethersdr#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

Co-authored-by: aethersdr-agent[bot] <273844287+aethersdr-agent[bot]@users.noreply.github.com>
Co-authored-by: Codex <noreply@openai.com>
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