[midi] Fix center-64 MIDI VFO tuning jumps (CTR2-Midi)#4271
Conversation
There was a problem hiding this comment.
Solid, well-scoped fix. The root cause is diagnosed correctly: MidiControlManager.cpp:328 force-sets binding.relative = true for learned VFO knobs, so the explicit-relative path always decoded two's-complement and the Tier-2 center-64 fallback (line 416) was unreachable — matching the reporter's 63-step jump signature. Routing VFO relative CC through the new auto-detecting decoder with per-binding encoding lock is the right shape, and the extracted header + focused unit test is a nice touch. Encoding state is correctly cleared in rebuildIndex()/clearBindings(), so re-learns reset without stale locks, and the automation injection uses BlockingQueuedConnection correctly for the cross-thread hop to the MIDI manager. All CI is green and the commit is signed.
No blocking issues.
Non-blocking notes
- Encoding auto-detection keys off the first directional value being exactly
63/65; a fast initial turn (or a stray value) can lock the wrong encoding for the session. Inline comment with detail — worth a doc line at least. - The automation
midi ccpath decodes under its own{ch0,CC,0}encoding slot that persists Center64 once detected within a session, so a proof that sends65then later1would decode1as center-64, not two's-complement. Fine for the documented 65/63 use, just a testing gotcha to keep in mind.
🤖 aethersdr-agent · cost: $4.8266 · model: claude-opus-4-8
| if (value == 63 || value == 65) { | ||
| encoding = MidiRelativeCcEncoding::Center64; | ||
| } else if (value != 64) { |
There was a problem hiding this comment.
The encoding is locked from the first directional value, and only 63/65 select center-64 — any other non-64 first value falls to two's-complement and stays there. Two realistic mis-locks:
- A center-64 encoder whose first movement is a fast/multi-step detent (e.g.
66or62) is classified two's-complement, then66decodes as66-128 = -62(big backward jump) and stays wrong. - A two's-complement encoder whose first value happens to be
63/65(a 63-step spin) locks to center-64.
Both are unlikely as a first event and the common single-detent case is handled, so this is fine to ship — but please note the heuristic limitation in a comment or the bridge doc so it's not surprising later. No code change required.
There was a problem hiding this comment.
Clean, well-targeted fix for the right root cause. Learned VFO CC bindings are forced relative=true (MidiControlManager.cpp:328), so #2994's center-64 tier — guarded by !binding.relative — was dead code for exactly these bindings; routing the relative VFO path through the new auto-detecting decoder is the correct place to fix it. Per-binding encoding lock keeps existing two's-complement encoders from having their 1/127 pulses reinterpreted, the BlockingQueuedConnection from the main thread to m_midiControl (which lives on m_extCtrlThread) is the correct cross-thread pattern with no deadlock, and the decoder is nicely isolated behind a header-only pure function with focused tests. All 7 checks green, commit signed.
Non-blocking notes
MidiRelativeCcDecoder.h: the center-64 signature only matches a ±1 first detent (63/65). A center-64 controller whose first movement is ≥2 steps (e.g.66/62) locksTwosComplementfor the binding's lifetime and decodes as a large wrong-direction jump. Fine for CTR2-MIDI (sends ±1 per detent), but see the inline suggestion for a more robust detection band.
Nice work — thanks for the isolated decoder + regression test rather than inlining the logic.
🤖 aethersdr-agent · cost: $3.3624 · model: claude-opus-4-8
| if (value == 63 || value == 65) { | ||
| encoding = MidiRelativeCcEncoding::Center64; | ||
| } else if (value != 64) { |
There was a problem hiding this comment.
The auto-detect only recognizes the ±1 center-64 signature. If a center-64 controller's first directional event is ≥2 steps (value 66/62/67/61…), this falls into the else if (value != 64) branch and permanently locks TwosComplement — then 66 decodes as 66-128 = -62, a large jump in the wrong direction, for the rest of the binding's life.
Since two's-complement small moves cluster near 0/128 and center-64 small moves cluster near 64, you can disambiguate on a band around center without ambiguity (a genuine two's-complement value in 60..68 would be a ~60-step first flick — implausible as an initial event):
| if (value == 63 || value == 65) { | |
| encoding = MidiRelativeCcEncoding::Center64; | |
| } else if (value != 64) { | |
| if (value >= 60 && value <= 68 && value != 64) { | |
| encoding = MidiRelativeCcEncoding::Center64; | |
| } else if (value != 64) { |
Judgment call on the exact band width — not required for the CTR2-MIDI case this PR targets, but it makes first-movement detection robust to a fast initial detent.
There was a problem hiding this comment.
@jensenpat Saw the help wanted - not sure if you are looking for hardware testing? I was already in the process of testing my CTR2-MIDI this week, when I came across this PR. I've got a CTR2-MIDI on the bench and ran it through your midi cc bridge path: the fix works — slow first click locks center-64, and both slow and fast detents tune the right direction and amount (65→+100 Hz, 72→+800 Hz, and back).
I confirmed the auto-detect edge case the bot flagged is real (a fast first value like 72 does lock two's-complement), but on this controller it's hard to hit in practice — the firmware's speed-tuning always sends ±1 first from rest, so a real first touch lands on 63/65 every time.
One extra thing to note: if I change the knob's MIDI Type (WheelA→WheelB) without re-Learning, it tunes in an unexpected way — the binding stays locked to the encoding detected at the previous Learn, so a re-Learn is needed after any MIDI Type change for it to decode correctly. As I understand it, this PR decodes the two relative wheel encodings — WheelA (center-64) and WheelB (two's-complement) — so the user needs the knob set to WheelA or WheelB for VFO tuning. The CTR2 offers other MIDI Types too (sliders, a button, WheelB-r) that I haven't tested.
Happy to share the capture logs and/or test further, if that's the help you are looking for?
— authored by agent (Claude Code) on behalf of @skerker
Summary
65and63produce one clockwise/counter-clockwise step.1/127behavior for existing relative encoders by locking the detected encoding per binding.midi ccautomation verb for repeatable controller-path proof.Root cause
MIDI Learn marks every VFO Control Change binding as relative. The explicit-relative path then unconditionally decoded values as two's complement, so CTR2-MIDI's center-64 detents (
65/63) became-63/+63. The center-64 fallback added in #2994 was guarded by!binding.relative, making it unreachable for learned VFO bindings.The reporter's follow-up log shows the exact signature: 14.300 MHz moved to 14.363 MHz and then 14.237 MHz in 63-step increments.
Proof
Built the macOS app and drove the same learned VFO relative decoder through the agent automation bridge on a connected FLEX-8400M (firmware 4.2.18.41174), with the configured 100 Hz tuning step:
midi cc 65midi cc 63Both detents moved exactly one 100 Hz step in opposite directions, the starting frequency was restored, and the final radio snapshot reported
transmitting:false.Tests
cmake --build build --target AetherSDR -j8midi_relative_cc_decoder_testmidi_settings_testbridge_docs_checktools/check_engine_boundary.py --strict(zero blocking findings)tools/check_a11y.py(existing warning-only findings)Fixes #4096
👨🏼💻 Generated with OpenAI Codex (GPT-5.6 Sol) and tested by @jensenpat