Skip to content

[midi] Fix center-64 MIDI VFO tuning jumps (CTR2-Midi)#4271

Open
jensenpat wants to merge 1 commit into
aethersdr:mainfrom
jensenpat:aether/fix-4096-midi-cc
Open

[midi] Fix center-64 MIDI VFO tuning jumps (CTR2-Midi)#4271
jensenpat wants to merge 1 commit into
aethersdr:mainfrom
jensenpat:aether/fix-4096-midi-cc

Conversation

@jensenpat

Copy link
Copy Markdown
Collaborator

Summary

  • Detect center-64 relative MIDI CC encoding for learned VFO Tune Knob bindings, so CTR2-MIDI values 65 and 63 produce one clockwise/counter-clockwise step.
  • Preserve the established two's-complement 1/127 behavior for existing relative encoders by locking the detected encoding per binding.
  • Add a focused decoder regression test and an RX-only midi cc automation 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:

Action Active-slice frequency
Before 14.100000 MHz
midi cc 65 14.100100 MHz
midi cc 63 14.100000 MHz

Both detents moved exactly one 100 Hz step in opposite directions, the starting frequency was restored, and the final radio snapshot reported transmitting:false.

VFO at 14.100100 MHz after one CTR2 clockwise detent

Tests

  • cmake --build build --target AetherSDR -j8
  • midi_relative_cc_decoder_test
  • midi_settings_test
  • bridge_docs_check
  • tools/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

@jensenpat jensenpat self-assigned this Jul 16, 2026
@jensenpat jensenpat changed the title Fix center-64 MIDI VFO tuning jumps [midi] Fix center-64 MIDI VFO tuning jumps Jul 16, 2026
@jensenpat
jensenpat marked this pull request as ready for review July 16, 2026 19:07
@jensenpat
jensenpat requested review from a team as code owners July 16, 2026 19:07
@jensenpat jensenpat changed the title [midi] Fix center-64 MIDI VFO tuning jumps [midi] Fix center-64 MIDI VFO tuning jumps (CTR2-Midi) Jul 16, 2026

@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.

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 cc path decodes under its own {ch0,CC,0} encoding slot that persists Center64 once detected within a session, so a proof that sends 65 then later 1 would decode 1 as 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

Comment on lines +24 to +26
if (value == 63 || value == 65) {
encoding = MidiRelativeCcEncoding::Center64;
} else if (value != 64) {

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.

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. 66 or 62) is classified two's-complement, then 66 decodes as 66-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.

@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.

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) locks TwosComplement for 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

Comment on lines +24 to +26
if (value == 63 || value == 65) {
encoding = MidiRelativeCcEncoding::Center64;
} else if (value != 64) {

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.

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):

Suggested change
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.

@skerker skerker Jul 17, 2026

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.

@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

@jensenpat jensenpat added help wanted Extra attention needed priority: medium Medium priority labels Jul 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

help wanted Extra attention needed priority: medium Medium priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

VFO tuning (just one bump) results in a jump greater than the step of 100 using CTR2-MIDI controller

2 participants