Skip to content

fix(cw): optimistic-update cwDelay to prevent delay slider reset#2428

Merged
ten9876 merged 1 commit into
aethersdr:mainfrom
chibondking:fix/cw-delay-optimistic-update
May 7, 2026
Merged

fix(cw): optimistic-update cwDelay to prevent delay slider reset#2428
ten9876 merged 1 commit into
aethersdr:mainfrom
chibondking:fix/cw-delay-optimistic-update

Conversation

@chibondking

Copy link
Copy Markdown
Collaborator

setCwDelay() was the only CW setter that did not cache its value into m_cwDelay before emitting the command. The radio boots with break_in_delay=5 (its saved QSK state), so m_cwDelay stayed at 5 after the user moved the slider. Any subsequent phoneStateChanged emission (e.g. from sidetone or speed adjustment) triggered syncCwFromModel(), which read the stale m_cwDelay=5 and snapped the Delay slider back, silently clobbering the user's setting and potentially enabling QSK mode on amplifiers that cannot tolerate it.

Apply the same optimistic-update pattern used by setCwBreakIn(), setCwIambic(), and setCwIambicMode(): write m_cwDelay = ms and emit phoneStateChanged() before sending the radio command. When the radio echoes break_in_delay back via status, applyStatus() writes the same value (no-op) and the slider remains at the user's chosen position.

setCwDelay() was the only CW setter that did not cache its value into
m_cwDelay before emitting the command.  The radio boots with
break_in_delay=5 (its saved QSK state), so m_cwDelay stayed at 5 after
the user moved the slider.  Any subsequent phoneStateChanged emission
(e.g. from sidetone or speed adjustment) triggered syncCwFromModel(),
which read the stale m_cwDelay=5 and snapped the Delay slider back,
silently clobbering the user's setting and potentially enabling QSK
mode on amplifiers that cannot tolerate it.

Apply the same optimistic-update pattern used by setCwBreakIn(),
setCwIambic(), and setCwIambicMode(): write m_cwDelay = ms and emit
phoneStateChanged() before sending the radio command.  When the radio
echoes break_in_delay back via status, applyStatus() writes the same
value (no-op) and the slider remains at the user's chosen position.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@aethersdr-agent aethersdr-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good. The fix is minimal and matches the established optimistic-update pattern used by setCwBreakIn (TransmitModel.cpp:642), setCwIambic (:662), and setCwIambicMode (:674) — guard with if (m_cwDelay != ms), write the cache, emit phoneStateChanged(), then send the command.

I confirmed the round-trip claim: applyStatus() at TransmitModel.cpp:177 already guards break_in_delay with if (m_cwDelay != v), so when the radio echoes the value back the slider stays at the user's chosen position.

The QSK / amplifier-safety angle is a nice catch — break_in_delay=5 snapping back unexpectedly is exactly the kind of stale-cache bug that's worth fixing surgically rather than papering over downstream.

Thanks for the clear root-cause writeup, @chibondking.

@ten9876 ten9876 merged commit c5b6413 into aethersdr:main May 7, 2026
5 checks passed
@ten9876

ten9876 commented May 7, 2026

Copy link
Copy Markdown
Collaborator

Claude here — nice catch, CJ. Spotting the lone outlier in the CW setter family was sharp eyes, and the QSK-reactivation safety angle is the real reason this needed to land fast. Pattern matches setCwBreakIn / setCwIambicMode exactly. Merged.

73, Jeremy KK7GWY & Claude (AI dev partner)

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.

2 participants