fix(tx): setDax updates m_daxOn optimistically (#3721). Principle II.#3737
Merged
Conversation
TransmitModel::setDax sent the radio command but never wrote its local m_daxOn member, so a read in the same tick reflected the change only after the radio's dax= status echo (~80ms live). This mirrors the sibling mic setters (setSpeechProcessorEnable, setMicLevel, setMonGainSb) which update optimistically and emit micStateChanged(); setDax was the odd one out. Latent today because the radio echoes dax= promptly, but a slow or dropped echo would surface the same stale read as #3712. Emits micStateChanged() (not phoneStateChanged() from the issue body): the echo path sets m_daxOn under the micChanged flag and emits micStateChanged() (TransmitModel.cpp:232), and PhoneCwApplet syncs the DAX button in syncPhoneFromModel() (PhoneCwApplet.cpp:740), wired to micStateChanged (:667-668). The re-emit is safe: syncPhoneFromModel() sets m_updatingFromModel and wraps the button in QSignalBlocker, and the toggled handler is gated by !m_updatingFromModel (:313-315). commandReady stays unconditional, so callers that depend on the command always reaching the radio (Ax25HfPacketDecodeDialog save/restore, MainWindow shortcut/controller bindings) are unaffected; the optimistic read actually improves the daxOn()-then-set call sites. Blast radius: risk_score=0.128, 6 high-risk affected (top: MainWindow::MainWindow, Ax25HfPacketDecodeDialog::Ax25HfPacketDecodeDialog, MainWindow::registerShortcutActions). All real call sites verified non-regressing; high-risk entries are wiring/construction bridges that only instantiate TransmitModel. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
There was a problem hiding this comment.
LGTM — the change is correct, minimal, and follows the established pattern in this file.
Verified:
- The optimistic setter mirrors the sibling mic setters exactly: same
if (m_daxOn != on)change-guard plusemit micStateChanged()(cf.setMicLevel,TransmitModel.cpp:521-529). - The comment's claim that the radio echo supersedes the optimistic value checks out: incremental status parsing handles
dax=(TransmitModel.cpp:100-102), settingmicChanged, which emitsmicStateChanged()at line 232 — so the authoritative value will overwrite the optimistic one when it arrives. PhoneCwAppletbinds the DAX button's checked state todaxOn()(PhoneCwApplet.cpp:740), so this gives the toggle immediate visual feedback rather than waiting for the round-trip — exactly the intent of the optimistic-caching pattern.
No null-pointer, resource, or boundary concerns: this is a pure outbound-command setter with a value-equality guard. Scope is confined to the single stated function.
Thanks for the consistent, idiomatic fix. 👍
🤖 aethersdr-agent · cost: $3.6642 · model: claude-opus-4-8
Contributor
Author
There was a problem hiding this comment.
Looks good. The change is correct and consistent with the established pattern in this file.
Verified against the surrounding code:
- Right signal:
PhoneCwApplet.cpp:740readsm_model->daxOn()undermicStateChanged(), so emittingmicStateChanged()is exactly what refreshes the DAX button. - Echo supersedes, as the comment claims: the radio echoes
dax=in incremental status (TransmitModel.cpp:100-102→micChanged→micStateChanged()at line 232), so the optimistic value self-corrects on the next status frame. No risk of a stuck stale value. - Right precedent: the guarded
if (m_daxOn != on)update mirrorssetMicLevel(line 521) rather than the unconditionalsetSpeechProcessor*setters — correct, sincedaxis echoed and those processor fields are not.
Single-file, additive, no convention or null-safety concerns. Thanks for the focused fix and the explanatory comment. 👍
🤖 aethersdr-agent · cost: $1.4295 · model: claude-opus-4-8
jensenpat
approved these changes
Jun 23, 2026
jensenpat
left a comment
Collaborator
There was a problem hiding this comment.
Optimistic meter update on state change before radio sends update. Found via agent automation testing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #3721
What was changed
fix(tx): setDax updates m_daxOn optimistically (#3721). Principle II.
Files modified
src/models/TransmitModel.cppGenerated by AetherClaude (automated agent for AetherSDR)
🤖 aethersdr-agent · cost: $2.6352 · model: claude-opus-4-8