fix(rc28): sync wheel accumulator to snapped frequency after auto-snap fires#3940
Conversation
…p fires (aethersdr#3939) After the 600 ms auto-snap timer rounds the slice frequency to the nearest 1 kHz, m_flexTargetMhz was left pointing at the pre-snap value. The re-base guard in applyFlexControlWheelAction only fires when the delta exceeds 1 kHz, so a sub-kHz snap (e.g. 200 Hz) was invisible to it — the next knob tick would resume from the stale pre-snap target, effectively undoing the snap. Setting m_flexTargetMhz = snapped immediately after applyTuneRequest keeps the accumulator in sync with the displayed flag. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Thanks for the fix, @wa2n-code — clean, well-scoped, and the root-cause writeup is spot on.
I traced the full path to confirm the diagnosis:
- The RC-28 wheel drives
applyFlexControlWheelAction("WheelFrequency", …)(MainWindow_Controllers.cpp:2488), which updates them_flexTargetMhzaccumulator and also armsm_hidSnapTimer(:2486). - The snap timer fires 600 ms after the last tick, by which point the 20 ms
m_flexCoalesceTimeris long inactive — so the re-base guard's live branch is thestd::abs(m_flexTargetMhz - s->frequency()) > 0.001(1 kHz) test at:1344. - A sub-kHz snap (e.g. 200 Hz) leaves
m_flexTargetMhzunder that 1 kHz threshold, so the guard doesn't re-base, and the nextm_flexTargetMhz += steps*stepHzresumes from the stale off-grid target — undoing the snap. Exactly as described.
Setting m_flexTargetMhz = snapped right after applyTuneRequest realigns the accumulator with the displayed frequency, so the next guard comparison sees ~0 delta and continues from the snapped value. Units are consistent (both MHz; std::round(freq*1000)/1000 snaps to 1 kHz), and all accesses to m_flexTargetMhz here — wheel action, coalesce timer, and this snap callback — run on the main thread, so no new races are introduced.
A couple of notes, neither blocking:
- Non-snap path unchanged: when
snapped == frequency(already on-grid,> 1e-9false) the accumulator is untouched, which is correct — in that casem_flexTargetMhzalready tracks the on-grid frequency from the prior tune. - Test plan is manual-only. That's understandable for an RC-28 HID timing interaction, but if the automation bridge can drive an encoder tick + snap-timer window, a regression check for "sub-kHz snap sticks across the next tick" would be worth adding later given how easily this silently regresses.
LGTM.
🤖 aethersdr-agent · cost: $4.1974 · model: claude-opus-4-8
|
One file, 4 lines. Precise root-cause writeup and a minimal fix. The diagnosis is exactly right. The snap timer rounds the slice to 1 kHz, but the re-base guard only fires when Three correctness points confirmed: units are consistent (both MHz; CI: All 6 checks pass including For future reference: a bridge-driven regression test for "sub-kHz snap sticks across the next tick" would be worth adding when the automation bridge gains encoder-tick synthesis — this pattern silently regresses otherwise. Not a merge blocker. Tier 3 only ( |
NF0T
left a comment
There was a problem hiding this comment.
Root cause confirmed through the code path: sub-kHz snap falls below the 1 kHz re-base guard, leaving m_flexTargetMhz stale. Fix is correct — units consistent, non-snap path unaffected, main-thread-only access (no new races). All 6 CI checks pass. Tier 3 only.
Squash subject: fix(rc28): sync wheel accumulator to snapped frequency after auto-snap fires — Principle XI. (#3940)
ten9876
left a comment
There was a problem hiding this comment.
Reviewed at high effort (multi-angle + verify). The fix is correct and well-targeted:
- Units consistent (all MHz; snapped = frequency rounded to 1 kHz, matching the 0.001 re-base threshold).
- GUI-thread only; ordering is right (m_flexTargetMhz set after applyTuneRequest, and IncrementalTune doesn't reset it — only CommandedTargetCenter/AbsoluteJump do).
- Genuinely fixes #3939: a sub-kHz snap no longer leaves a stale pre-snap target that undoes itself on the next tick.
Refuted two speculative concerns: 're-activation after a typed tune' (guard is false when already aligned, and it'd resync to the correct current freq regardless) and '1 kHz vs stepHz grid mismatch' (syncing the accumulator to the displayed frequency is the correct behavior; standard steps all divide 1 kHz).
One optional, non-blocking follow-up: the resync sits inside the >1e-9 guard, so a pre-existing mixed-controller edge (RC-28 + MIDI moving the slice onto the grid) can still leave m_flexTargetMhz stale. Moving the assignment outside the 'if' would close it for free — but that's pre-existing and out of scope here.
LGTM. Thanks @wa2n-code.
#3961) ## Release documentation prep for **v26.7.1** (2026-07-02) 29 commits since v26.6.5. Docs-only + version bump — no code behavior change. ### Version bump 26.6.5 → 26.7.1 The three canonical locations (per `AGENTS.md`): - `CMakeLists.txt:2` — `project(AetherSDR VERSION 26.7.1)` → `AETHERSDR_VERSION` (the app's version string) - `README.md` — Current version line - `AGENTS.md` — Current version line ### `CHANGELOG.md` Promoted `[Unreleased]` → **`## [v26.7.1] — 2026-07-02`** in house style (v-prefix, em-dash, "N commits since…" opener + headline), authored from all merged PRs and categorized: - **Added:** 3D stacked-trace spectrum (#3899), 3D dBm scale (#3937), in-process NVIDIA BNR + AFX download-on-demand (#3902), TX meter mouse-over readouts (#3936), SWR manual sweep range (#3885), CW sidetone capture (#3895), KiwiSDR metadata scaffolding (#3898), bridge verbs (#3920) - **Changed:** BNR container/NIM backend removed, 60 fps + per-pixel GPU FFT trace (#3958), FlexLib-sourced model capabilities (#3954/#2177), RX speaker-latency cut (#3897), Display pane sections (#3935) - **Fixed:** #3922, #3926, #3941, #3940, #3942, #3921, #3903, #3892, #3924, #3891, #3890, #3900, #3947 - Fresh empty `[Unreleased]` restored above the release block. Internal/CI/docs/self-fix PRs (#3931/#3916/#3934/#3929) omitted per house style. ### `ROADMAP.md` - Cycle header → "post-v26.7.1" - NVIDIA BNR removed from **In flight** (shipped this cycle) - Five v26.7.1 highlights prepended to **Recently shipped** ### `README.md` - Highlights: GPU spectrum bullet now notes the **per-pixel FFT trace at up to 60 fps** and the **3D stacked-trace** spectrum mode ### Reviewer notes - Touches protected paths (`*.md` + `AGENTS.md` Tier-1 + `CMakeLists.txt`) — needs `@aethersdr/infrastructure` sign-off. - Tagging `v26.7.1` and cutting the release build are **not** part of this PR (docs prep only). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Summary
m_flexTargetMhzwas left pointing at the pre-snap value.applyFlexControlWheelActiononly fires when the delta exceeds 1 kHz, so a sub-kHz snap (e.g. 200 Hz) was invisible to it — the next knob tick would resume from the stale pre-snap target, effectively undoing the snap.m_flexTargetMhz = snappedimmediately afterapplyTuneRequestkeeps the accumulator in sync with the displayed frequency.Fixes #3939.
Test plan