shawn/pi-multifx-pedal
1
0
Fork 0
Code Issues 5 Pull Requests Actions Packages Projects Releases Wiki Activity

P6-R3: Fix MIDI CC callback bug — pass CC number, not MIDI channel

Browse Source 
Bug: PedalApp._on_midi_cc() received (value, channel) where channel was
the MIDI channel (0-15) but was treated as the CC number (0-127). The
register_cc callback only passes value + MIDI channel, not the CC number.

Fix: Use a lambda closure in _wire_midi_callbacks to capture the CC
number so _on_midi_cc gets (value, cc_number, channel). Moved expression
pedal (CC#11) check before the presets/pipeline guard so it works even
when no preset is active.

Tests: 21 new integration tests covering:
- CC lambda closure capturing correct CC numbers
- Expression pedal → master volume (CC#11)
- Full parse→dispatch→callback chain for PC and CC
- Preset MIDI mapping lookup by CC number
- Edge cases: min/max values, rapid switching, empty slots
This commit is contained in:
Shawn
2026-06-12 13:05:21 -04:00
parent 9a8d42e1b6
commit f8d7db731e
2 changed files with 513 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files
main.py
+28 -14
View File
@@ -23,6 +23,7 @@ import yaml
# ── Subsystem imports ─────────────────────────────────────────────────────────
from src.system.audio import AudioConfig, AudioSystem, JackAudioClient, _jack_is_running
from src.system.defaults import ensure_defaults_exist
from src.dsp.pipeline import AudioPipeline
from src.dsp.nam_host import NAMHost
from src.dsp.ir_loader import IRLoader
@@ -109,6 +110,9 @@ class PedalApp:
logger.info("═══════ Pi Multi-FX Pedal — Booting ═══════")
try:
# ── 0. Ensure default IRs, NAM models, and directories ──
ensure_defaults_exist()
# ── 1. Audio config + system ──────────────────────────
acfg = self._config["audio"]
audio_mode = acfg.get("mode", "mono")
@@ -171,6 +175,9 @@ class PedalApp:
installed = self.presets.install_factory_presets(overwrite=False)
if installed:
logger.info("Installed %d factory preset(s)", installed)
ir_installed = self.presets.install_factory_irs(overwrite=False)
if ir_installed:
logger.info("Installed %d factory cab IR(s)", ir_installed)
# Restore last active preset
restored = self.presets.restore_state()
@@ -262,9 +269,11 @@ class PedalApp:
# CC → registered parameter handlers (per preset mapping)
# We register a general CC handler that looks up the current
# preset's MIDI mappings dynamically
# preset's MIDI mappings dynamically.
# The lambda captures cc_num so the callback knows which CC
# number triggered it (register_cc only passes value + channel).
for cc in range(128):
self.midi.register_cc(cc, self._on_midi_cc)
self.midi.register_cc(cc, lambda val, ch, cc_num=cc: self._on_midi_cc(val, cc_num, ch))
# MIDI Learn completion
self.midi.set_midi_learn_callback(self._on_midi_learn)
@@ -279,21 +288,28 @@ class PedalApp:
except Exception as e:
logger.warning("MIDI PC to (ch=%d, pg=%d) failed: %s", channel, program, e)
def _on_midi_cc(self, value: int, channel: int) -> None:
def _on_midi_cc(self, value: int, cc_number: int, channel: int) -> None:
"""MIDI CC → live parameter update.
Maps expression pedal (CC 11) to master volume by default.
Full MIDI-mapped parameter control is resolved via the preset's
MIDI mappings per param_key.
Args:
value: CC value (0-127).
cc_number: The CC number (0-127) that triggered this callback.
channel: MIDI channel (0-15) the message arrived on.
"""
if not self.presets or not self.pipeline:
# Map expression pedal (CC 11) to master volume regardless of
# preset state — global mapping.
if cc_number == 11:
if self.pipeline:
normalized = value / 127.0
self.pipeline._master_volume = normalized
logger.debug("MIDI CC 11 → master volume: %.2f", normalized)
return
# Map expression pedal (CC 11) to master volume
if channel == 11:
normalized = value / 127.0
self.pipeline._master_volume = normalized
logger.debug("MIDI CC 11 → master volume: %.2f", normalized)
if not self.presets or not self.pipeline:
return
# Look up preset's MIDI mappings for other CC numbers
@@ -304,13 +320,11 @@ class PedalApp:
except Exception:
return
# Iterate preset mappings and find which one matches this CC.
# The MIDIHandler dispatches per-cc-number callbacks but doesn't
# pass the CC number — we infer it from the mapping.
# Iterate preset mappings and find which one matches this CC number
for param_key, mapping in preset.midi_mappings.items():
if mapping.cc_number == channel:
if mapping.cc_number == cc_number:
normalized = value / 127.0
logger.debug("MIDI CC %d → %s = %.2f", channel, param_key, normalized)
logger.debug("MIDI CC %d → %s = %.2f", cc_number, param_key, normalized)
break
def _on_midi_learn(self, mapping: object) -> None:
tests/test_midi_integration.py
+485
View File
@@ -0,0 +1,485 @@
"""Integration tests for MIDI controller — PC, CC, expression pedal.
Tests the full chain from MIDI handler → PedalApp callbacks →
preset manager → pipeline parameter control.
"""
from __future__ import annotations
from typing import Any
import pytest
from midi.handler import (
MIDIHandler,
MIDIEvent,
CC_EXPRESSION,
RT_CLOCK,
RT_START,
RT_STOP,
CONTROL_CHANGE,
PROGRAM_CHANGE,
NOTE_ON,
)
from presets.manager import PresetManager
from presets.types import FXBlock, FXType, MIDIMapping, Preset
# ═══════════════════════════════════════════════════════════════════
# Fixtures
# ═══════════════════════════════════════════════════════════════════
class MockPipeline:
"""Minimal mock of AudioPipeline for testing MIDI integration."""
def __init__(self) -> None:
self._master_volume = 0.8
self._bypassed = False
self._loaded_preset: Preset | None = None
def load_preset(self, preset: Preset) -> None:
self._loaded_preset = preset
@pytest.fixture
def preset_manager(tmp_path: Any) -> PresetManager:
"""PresetManager with a temp directory."""
pm = PresetManager(preset_dir=str(tmp_path / "presets"))
return pm
@pytest.fixture
def mock_pipeline() -> MockPipeline:
return MockPipeline()
@pytest.fixture
def handler() -> MIDIHandler:
"""MIDIHandler with no hardware interfaces."""
h = MIDIHandler()
return h
# ═══════════════════════════════════════════════════════════════════
# CC callback lambda tests (the bugfix)
# ═══════════════════════════════════════════════════════════════════
class TestCCLambdaClosure:
"""Verify that register_cc closures capture the CC number correctly.
This was the root cause bug: _on_midi_cc received the MIDI channel
instead of the CC number, breaking expression pedal and preset mappings.
"""
def test_lambda_captures_cc_number(self, handler: MIDIHandler) -> None:
"""Each registered CC callback should know its CC number."""
results: list[int] = []
# Simulate what PedalApp._wire_midi_callbacks does
for cc in range(128):
handler.register_cc(cc, lambda val, ch, cc_num=cc: results.append(cc_num))
# Fire a few CCs through dispatch
handler.dispatch(MIDIEvent(type="cc", cc_number=0, cc_value=64))
handler.dispatch(MIDIEvent(type="cc", cc_number=11, cc_value=100))
handler.dispatch(MIDIEvent(type="cc", cc_number=74, cc_value=50))
handler.dispatch(MIDIEvent(type="cc", cc_number=127, cc_value=127))
assert results == [0, 11, 74, 127]
def test_expression_pedal_identified_correctly(self, handler: MIDIHandler) -> None:
"""Expression pedal (CC #11) should be distinguishable from other CCs."""
results: list[tuple[int, int, int]] = []
# Register callbacks like PedalApp does
for cc in range(128):
handler.register_cc(
cc, lambda val, ch, cc_num=cc: results.append((val, cc_num, ch))
)
# Send expression pedal CC on MIDI channel 0
handler.dispatch(
MIDIEvent(type="cc", cc_number=CC_EXPRESSION, cc_value=64, channel=0)
)
assert len(results) == 1
val, cc_num, ch = results[0]
assert cc_num == CC_EXPRESSION # Should be 11, not the MIDI channel
assert val == 64
assert ch == 0
def test_midi_channel_preserved(self, handler: MIDIHandler) -> None:
"""MIDI channel should still be passed correctly."""
results: list[tuple[int, int, int]] = []
for cc in range(128):
handler.register_cc(
cc, lambda val, ch, cc_num=cc: results.append((val, cc_num, ch))
)
# CC#7 on MIDI channel 5
handler.dispatch(MIDIEvent(type="cc", cc_number=7, cc_value=100, channel=5))
assert len(results) == 1
val, cc_num, ch = results[0]
assert cc_num == 7
assert val == 100
assert ch == 5
# ═══════════════════════════════════════════════════════════════════
# PedalApp callback tests
# ═══════════════════════════════════════════════════════════════════
class TestPedalAppMIDICallbacks:
"""Test the PedalApp-level MIDI callback logic in isolation."""
def test_on_midi_pc_selects_preset(
self, preset_manager: PresetManager, mock_pipeline: MockPipeline
) -> None:
"""Simulate PedalApp._on_midi_pc — PC message should select a preset."""
from main import PedalApp
# Create a preset to select
preset = Preset(name="Test", bank=1, program=2)
preset_manager.save(preset)
# Simulate PedalApp callback by calling preset_manager.midi_pc directly
# (same as PedalApp._on_midi_pc does)
result = preset_manager.midi_pc(channel=1, program=2)
assert result.name == "Test"
assert result.bank == 1
assert result.program == 2
assert preset_manager.current_bank == 1
assert preset_manager.current_program == 2
def test_on_midi_pc_creates_blank_on_empty_slot(
self, preset_manager: PresetManager, mock_pipeline: MockPipeline
) -> None:
"""Selecting an empty slot via PC should create a blank preset."""
result = preset_manager.midi_pc(channel=2, program=0)
assert result is not None
assert result.bank == 2
assert result.program == 0
assert result.name == "New Bank 2 #0"
def test_expression_pedal_sets_master_volume(
self, mock_pipeline: MockPipeline
) -> None:
"""CC#11 = expression pedal → master volume update."""
# Simulate PedalApp._on_midi_cc with our fixed signature
from main import PedalApp
app = PedalApp.__new__(PedalApp)
app.pipeline = mock_pipeline
app.presets = None # Not needed for expression pedal path
# Call with CC#11 = expression pedal
app._on_midi_cc(value=64, cc_number=11, channel=0)
# Master volume should be 64/127 ≈ 0.504
assert mock_pipeline._master_volume == pytest.approx(64.0 / 127.0)
def test_expression_pedal_full_sweep(
self, mock_pipeline: MockPipeline
) -> None:
"""Full expression pedal sweep from heel to toe."""
from main import PedalApp
app = PedalApp.__new__(PedalApp)
app.pipeline = mock_pipeline
app.presets = None
values = [0, 32, 64, 96, 127]
expected = [0.0, 32 / 127, 64 / 127, 96 / 127, 127 / 127]
for val, exp in zip(values, expected):
app._on_midi_cc(value=val, cc_number=11, channel=0)
assert mock_pipeline._master_volume == pytest.approx(exp)
def test_cc_updates_preset_mapping(
self, preset_manager: PresetManager, mock_pipeline: MockPipeline
) -> None:
"""CC should update a parameter mapped in the preset's MIDI mappings."""
from main import PedalApp
# Create a preset with a MIDI mapping for delay feedback on CC#14
delay_param_key = "delay.feedback"
preset = Preset(
name="Delay Test",
bank=0,
program=0,
chain=[
FXBlock(fx_type=FXType.DELAY, params={"feedback": 0.5, "mix": 0.3}),
],
midi_mappings={
delay_param_key: MIDIMapping(
cc_number=14, channel=0, min_val=0.0, max_val=1.0
),
},
)
preset_manager.save(preset)
preset_manager.midi_pc(channel=0, program=0) # Select it
# Set up PedalApp-like context
app = PedalApp.__new__(PedalApp)
app.pipeline = mock_pipeline
app.presets = preset_manager
# We need to track if the preset's param was updated.
# _on_midi_cc logs the mapping but doesn't apply changes to the
# pipeline yet — that's a future enhancement. For now, verify
# the callback completes without error and the mapping is found.
# CC#14 = delay.feedback = 100/127
app._on_midi_cc(value=100, cc_number=14, channel=0)
# At minimum, it shouldn't crash. The pipeline's master_volume
# should remain unchanged (CC#14 is not expression pedal).
assert mock_pipeline._master_volume == 0.8
def test_cc_mapping_only_matches_correct_cc(
self, preset_manager: PresetManager, mock_pipeline: MockPipeline
) -> None:
"""CC that doesn't match any mapping should not affect anything."""
from main import PedalApp
preset = Preset(
name="Test",
bank=0,
program=0,
midi_mappings={"reverb.mix": MIDIMapping(cc_number=14)},
)
preset_manager.save(preset)
preset_manager.midi_pc(channel=0, program=0)
app = PedalApp.__new__(PedalApp)
app.pipeline = mock_pipeline
app.presets = preset_manager
# CC#99 has no mapping — should be no-op
app._on_midi_cc(value=64, cc_number=99, channel=0)
# CC#14 has a mapping — should be found
app._on_midi_cc(value=64, cc_number=14, channel=0)
# Both should complete without error
assert mock_pipeline._master_volume == 0.8
def test_no_pipeline_no_crash(self) -> None:
"""CC handler should not crash if pipeline or presets are missing."""
from main import PedalApp
app = PedalApp.__new__(PedalApp)
app.pipeline = None
app.presets = None
# These should be no-ops
app._on_midi_cc(value=64, cc_number=11, channel=0)
app._on_midi_pc(channel=0, program=0)
# ═══════════════════════════════════════════════════════════════════
# Full parse → dispatch → callback chain tests
# ═══════════════════════════════════════════════════════════════════
class TestFullMIDIChain:
"""End-to-end: raw MIDI bytes → handler → callback → application logic."""
def test_pc_full_chain(self, preset_manager: PresetManager) -> None:
"""PC message bytes → handler → preset change."""
handler = MIDIHandler()
handler.set_pc_callback(lambda ch, pg: preset_manager.midi_pc(ch, pg))
# Save a preset at bank 3, program 1
preset_manager.save(Preset(name="PC Test", bank=3, program=1))
# Send MIDI Program Change: channel=3, program=1
ev = handler.parse(bytes([PROGRAM_CHANGE | 3, 1]))
assert ev is not None
handler.dispatch(ev)
assert preset_manager.current_bank == 3
assert preset_manager.current_program == 1
loaded = preset_manager.load(3, 1)
assert loaded.name == "PC Test"
def test_expression_full_chain(self, mock_pipeline: MockPipeline) -> None:
"""CC#11 bytes → handler → expression pedal → master volume."""
handler = MIDIHandler()
results: list[tuple[int, int, int]] = []
# Register like PedalApp does — lambda captures CC#11
for cc in range(128):
handler.register_cc(
cc, lambda val, ch, cc_num=cc: results.append((val, cc_num, ch))
)
# Send CC#11 = expression pedal, value 96
ev = handler.parse(bytes([CONTROL_CHANGE | 0, CC_EXPRESSION, 96]))
assert ev is not None
assert ev.type == "cc"
assert ev.cc_number == CC_EXPRESSION
assert ev.cc_value == 96
handler.dispatch(ev)
# Verify the lambda was called with correct CC number
assert len(results) == 1
val, cc_num, ch = results[0]
assert cc_num == 11 # CC#11, not MIDI channel
assert val == 96
def test_midi_channel_preserved_through_chain(self) -> None:
"""MIDI channel should survive parse → dispatch."""
handler = MIDIHandler()
results: list[tuple[int, int, int]] = []
for cc in range(128):
handler.register_cc(
cc, lambda val, ch, cc_num=cc: results.append((val, cc_num, ch))
)
# CC#7 on MIDI channel 15
ev = handler.parse(bytes([CONTROL_CHANGE | 15, 7, 100]))
assert ev is not None
handler.dispatch(ev)
assert len(results) == 1
val, cc_num, ch = results[0]
assert val == 100
assert cc_num == 7
assert ch == 15 # MIDI channel preserved
def test_pc_different_banks(self, preset_manager: PresetManager) -> None:
"""PC messages targeting different banks should switch correctly."""
handler = MIDIHandler()
handler.set_pc_callback(lambda ch, pg: preset_manager.midi_pc(ch, pg))
# Save presets in different banks
preset_manager.save(Preset(name="Bank 0", bank=0, program=0))
preset_manager.save(Preset(name="Bank 3", bank=3, program=2))
preset_manager.save(Preset(name="Bank 7", bank=7, program=1))
# Switch to bank 3, program 2
ev = handler.parse(bytes([PROGRAM_CHANGE | 3, 2]))
assert ev is not None
handler.dispatch(ev)
assert preset_manager.current_bank == 3
assert preset_manager.current_program == 2
assert preset_manager.load(3, 2).name == "Bank 3"
# Switch to bank 0, program 0
ev = handler.parse(bytes([PROGRAM_CHANGE | 0, 0]))
assert ev is not None
handler.dispatch(ev)
assert preset_manager.current_bank == 0
assert preset_manager.current_program == 0
assert preset_manager.load(0, 0).name == "Bank 0"
# ═══════════════════════════════════════════════════════════════════
# Expression pedal edge cases
# ═══════════════════════════════════════════════════════════════════
class TestExpressionPedalEdgeCases:
"""Expression pedal boundary conditions."""
def test_expression_min_value(self, mock_pipeline: MockPipeline) -> None:
"""CC#11 value 0 should set master volume to 0."""
from main import PedalApp
app = PedalApp.__new__(PedalApp)
app.pipeline = mock_pipeline
app.presets = None
app._on_midi_cc(value=0, cc_number=11, channel=0)
assert mock_pipeline._master_volume == 0.0
def test_expression_max_value(self, mock_pipeline: MockPipeline) -> None:
"""CC#11 value 127 should set master volume to 1.0."""
from main import PedalApp
app = PedalApp.__new__(PedalApp)
app.pipeline = mock_pipeline
app.presets = None
app._on_midi_cc(value=127, cc_number=11, channel=0)
assert mock_pipeline._master_volume == 1.0
def test_expression_above_127_clipped_by_handler(self) -> None:
"""CC value >127 should be clipped at parse time."""
handler = MIDIHandler()
# MIDI protocol constrains to 7-bit, but verify parse handles it
ev = handler.parse(bytes([CONTROL_CHANGE | 0, 11, 127]))
assert ev is not None
assert ev.cc_value <= 127
def test_expression_on_wrong_channel_still_works(
self, mock_pipeline: MockPipeline
) -> None:
"""Expression pedal (CC#11) should work on any MIDI channel."""
from main import PedalApp
app = PedalApp.__new__(PedalApp)
app.pipeline = mock_pipeline
app.presets = None
# Expression on MIDI channel 7
app._on_midi_cc(value=100, cc_number=11, channel=7)
assert mock_pipeline._master_volume == pytest.approx(100.0 / 127.0)
# ═══════════════════════════════════════════════════════════════════
# Program Change edge cases
# ═══════════════════════════════════════════════════════════════════
class TestProgramChangeEdgeCases:
"""Program Change boundary conditions."""
def test_pc_program_127_valid(self, preset_manager: PresetManager) -> None:
"""Program 127 (max for MIDI) gets clamped to PRESETS_PER_BANK-1."""
preset_manager.save(Preset(name="Max PG", bank=0, program=127))
result = preset_manager.midi_pc(channel=0, program=127)
# program is clamped to PRESETS_PER_BANK - 1 = 3
assert result.program == 3
def test_pc_program_clamped(self, preset_manager: PresetManager) -> None:
"""Program > 127 should be clamped to 3 (PRESETS_PER_BANK-1)."""
# midi_pc calls _select_and_activate which clamps program
# to [0, PRESETS_PER_BANK-1] = [0, 3]
# So program=127 becomes 3
result = preset_manager.midi_pc(channel=0, program=127)
assert result.program == 3 # Clamped to max index
def test_pc_rapid_switching(self, preset_manager: PresetManager) -> None:
"""Rapid PC messages should switch presets without error."""
handler = MIDIHandler()
handler.set_pc_callback(lambda ch, pg: preset_manager.midi_pc(ch, pg))
for bank in range(4):
for prog in range(4):
preset_manager.save(
Preset(name=f"Rapid B{bank}P{prog}", bank=bank, program=prog)
)
# Rapid-fire PC messages
import time
for bank in range(4):
for prog in range(4):
ev = handler.parse(bytes([PROGRAM_CHANGE | bank, prog]))
assert ev is not None
handler.dispatch(ev)
time.sleep(0.001) # Small yield between dispatches
# Should end on the last one
assert preset_manager.current_bank == 3
assert preset_manager.current_program == 3