[codex] Stabilize MIDI jog-wheel VFO tuning#2422
Conversation
Touch-sensitive DJ controllers can emit a NoteOn touch sensor before their continuous CC movement data, and radio RF_frequency echoes can lag behind rapid tune commands. That combination made learned VFO bindings and subsequent wheel steps jump or fight stale radio state. This teaches MIDI learn to wait for continuous movement data for slider-style targets, decodes common 1/127 relative CC pulses for VFO tuning, and keeps a short-lived MIDI tune target so the next wheel step is based on local intent rather than delayed radio echo. Constraint: Hercules DJControl Starlight jog wheels emit touch NoteOn events before rotation CC events. Constraint: SliceModel accepts asynchronous RF_frequency status echoes from the radio while tune commands are in flight. Rejected: Keep MIDI-side acceleration for VFO tune | controller detents already encode movement density and extra acceleration caused overshoot. Confidence: medium Scope-risk: narrow Directive: Do not bind slider-style MIDI targets to touch NoteOn events unless the target explicitly represents a button or gate. Tested: cmake --build build --target AetherSDR -j4 Tested: ctest --test-dir build -R midi_settings_test --output-on-failure Not-tested: Physical radio command timing across high-latency SmartLink paths
There was a problem hiding this comment.
Thanks @s53zo — this is a thorough, well-scoped fix with a clear root-cause writeup and good in-line comments explaining the why (which makes the diff easy to follow). The three concerns it addresses (touch-sensor learn confusion, unit-pulse misinterpretation, and stale frequency() echo as the next base) are all real and the chosen mitigations look right. A few small notes:
Looks good
- Touch-NoteOn filter is correctly scoped to
MidiParamType::Sliderlearn, so button bindings still capture NoteOn. - The
m_midiTuneIdleTimermember + lambda is constructed on the main thread and therelativeActionconnection arrives queued fromm_extCtrlThread, so all timer mutation happens on the main thread — no affinity issues. setVfoFrequency()for immediate cursor feedback follows the same pattern already used atMainWindow.cpp:2926/8456/10280.- The lock branch correctly clears both
m_midiTuneTargetMhzand stops the idle timer.
Minor — feel free to fold in or punt
-
MainWindow.cppre-snap clause is hard to reach. In the guard:if (m_midiTuneTargetMhz < 0.0 || (!m_midiTuneIdleTimer.isActive() && std::abs(m_midiTuneTargetMhz - s->frequency()) > 0.001)) {
The single-shot timer's timeout lambda always resets
m_midiTuneTargetMhzto-1.0, so the second clause is only reachable in the brief window betweenQTimergoing inactive and the timeout slot running. It works as defensive belt-and-suspenders but a one-line comment to that effect would save the next reader a few minutes. -
accumulateRelativeStepis a private member but it's a pure transformation overm_relativeAccumandm_relativeTimer. Not a blocker, just noting it could be a free function in the anon namespace if you want to keep it symmetric withrelativeCcDelta/isUnitRelativeCcPulse. -
Pre-existing, not yours: the
static int slowDividerinsideflushRelativeAccumis shared across all relative params (so two simultaneous non-VFO relative knobs would interleave its parity). This PR doesn't make it worse — VFO tune now bypasses that branch — but worth flagging as a follow-up if anyone hits it. -
250 ms idle window is reasonable for typical jog-then-stop cadence; might want it as a named constant if it ever needs tuning per controller, but fine as a magic number for now.
No blocking issues from me. Nice work isolating the touch vs. movement events with the RtMidi probe before patching.
|
Claude here — three independent failure modes (touch-sensor learn, legacy unit-pulse decoding, radio-echo lag) cleanly separated into three layered fixes. The in-flight tune target with the 1 kHz divergence re-anchor is a nice touch — handles the mixed mouse/MIDI case without extra state machines. Backward-compat for legacy bindings is the cherry on top. Thanks for the live RtMidi capture work, s53zo. Merged. 73, Jeremy KK7GWY & Claude (AI dev partner) |
Summary
Fixes #2421.
This stabilizes MIDI
VFO Tune Knobbehavior for touch-sensitive DJ controllers such as the Hercules DJControl Starlight. The fix keeps the existing radio command path (slice tune <id> <freq> autopan=0) but corrects how MIDI learn and MIDI relative tuning compute the target before that command is issued.Root Cause
The Starlight jog wheel emits two different kinds of events:
NoteOnbefore wheel movement1one direction,127the other)AetherSDR could learn the touch sensor instead of the movement CC, which made press/release look like VFO commands. Separately, relative tuning based each new target on
SliceModel::frequency(), which can be rewritten by delayed radioRF_frequencyechoes while tune commands are still in flight.That combination made the wheel feel jumpy: the app could bind to the wrong source, interpret relative pulses as absolute endpoints, or base the next step on stale radio state.
What Changed
NoteOnevents and waits for continuous movement data (CCorPitchBend).VFO Tune KnobCC bindings default to relative mode.1/127relative pulses are handled as relative pulses for backward compatibility.User Impact
Operators using touch-sensitive MIDI controllers should be able to map jog wheels to
VFO Tune Knobwithout accidental large jumps. Existing bad touch-sensor bindings should be ignored at runtime, but users should still re-learn the VFO control by rotating the wheel so the saved binding points at the movement CC.Validation
Follow-up
A separate enhancement could combine Starlight coarse/fine CC pairs for 14-bit slider resolution, but that is intentionally out of scope for this VFO jog-wheel fix.