From 8c68f3c2f03bf5eeee9c888a73c47fdea758a2f0 Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Sun, 19 Apr 2026 15:18:12 -0400 Subject: [PATCH 01/26] Add external MIDI device synchronization Introduces ExternalMidiManager and ExternalMidiOut for sending MIDI messages to external devices (e.g. Source Audio C4, HX Stomp) on pedalboard load. Config is per-device with glob-based port auto-detection and per-pedalboard message override via config.yml. Co-Authored-By: Claude Sonnet 4.6 --- modalapi/external_midi.py | 362 ++++++++++++++++++ modalapi/mod.py | 24 +- modalapi/modhandler.py | 28 +- pistomp/hardware.py | 16 + .../default_config_pistomptre.yml | 43 +++ 5 files changed, 470 insertions(+), 3 deletions(-) create mode 100644 modalapi/external_midi.py diff --git a/modalapi/external_midi.py b/modalapi/external_midi.py new file mode 100644 index 000000000..720536e99 --- /dev/null +++ b/modalapi/external_midi.py @@ -0,0 +1,362 @@ +# This file is part of pi-stomp. +# +# pi-stomp is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# pi-stomp is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with pi-stomp. If not, see . + +from __future__ import annotations + +import fnmatch +import logging +import time +from typing import TypedDict + +import rtmidi +from rtmidi import MidiOut as RtMidiOut + +MidiMessage = list[int] + +# Constant for identifying external parameters +EXTERNAL_INSTANCE_ID = "External" + + +class ExternalMidiOut: + """ + Wrapper around external MIDI port that implements the same interface as + the virtual port's midiout. Allows controls to send MIDI to external devices + transparently, with automatic fallback to virtual port if device unavailable. + """ + + def __init__( + self, external_midi_manager: ExternalMidiManager, port_name: str, midi_channel: int, fallback_midiout: RtMidiOut + ): + """ + Initialize external MIDI output wrapper. + + Args: + external_midi_manager: Reference to ExternalMidiManager. + port_name: Name of external MIDI port from config. + midi_channel: MIDI channel (0-15). + fallback_midiout: Virtual port midiout to use if external unavailable. + """ + self.external_midi = external_midi_manager + self.port_name = port_name + self.channel = midi_channel + self.fallback = fallback_midiout + + def send_message(self, message: MidiMessage) -> None: + """ + Send MIDI message to external port, falling back to virtual port if unavailable. + + Args: + message: MIDI message as [status_byte, data1, data2, ...] + """ + if len(message) < 3: + # Not a CC message we can route, just use fallback + self.fallback.send_message(message) + return + + # Extract CC and value from message + # Status byte format: 0xBn where n is channel (already included) + cc = message[1] + value = message[2] + + # Try to send to external port + success = self.external_midi.send_cc(self.port_name, self.channel, cc, value) + + if not success: + # External port unavailable, fallback to virtual port + logging.debug(f"External port {self.port_name} unavailable, using virtual port") + self.fallback.send_message(message) + + +class PortConfig(TypedDict, total=False): + auto_detect: list[str] + port_index: int + + +class ExternalMidiConfig(TypedDict, total=False): + enabled: bool + send_delay_ms: int + ports: dict[str, PortConfig] # configured devices + messages: dict[str, list[MidiMessage]] # port_name -> list of MIDI messages + + +class ExternalMidiManager: + """ + Manages external MIDI device synchronization. + Sends MIDI messages to external devices when pedalboards are loaded. + """ + + def __init__(self): + self.midi_ports: dict[str, rtmidi.MidiOut] = {} + self.port_configs: dict[str, PortConfig] = {} + self.messages: dict[str, list[MidiMessage]] = {} + self.enabled: bool = False + self.send_delay_ms: int = 10 + + def update_config(self, cfg: ExternalMidiConfig | None) -> None: + """ + Update configuration incrementally (can be called multiple times). + Only updates fields that are present. + """ + if cfg is None: + return + + if "enabled" in cfg: + self.enabled = cfg["enabled"] + if self.enabled: + logging.debug("External MIDI enabled") + else: + logging.debug("External MIDI disabled") + + if "send_delay_ms" in cfg: + self.send_delay_ms = cfg["send_delay_ms"] + + if "ports" in cfg: + # Merge ports (port-level granularity) + self.port_configs.update(cfg["ports"]) + + if "messages" in cfg: + # Merge messages at port level + # This allows pedalboard config to override specific ports while keeping others default + self.messages.update(cfg["messages"]) + + def _get_available_ports(self) -> list[str]: + try: + temp_out = rtmidi.MidiOut() + ports = temp_out.get_ports() + del temp_out + return ports + except Exception as e: + logging.error(f"Failed to enumerate MIDI ports: {e}") + return [] + + def _find_port_by_name(self, port_config: PortConfig) -> int | None: + """ + Find MIDI port index matching a given config, returning its index if found. + """ + if "port_index" in port_config: + return port_config["port_index"] + + # Auto-detect by name patterns + auto_detect = port_config.get("auto_detect", []) + if not auto_detect: + return None + + available_ports = self._get_available_ports() + if not available_ports: + return None + + # Search for matching ports using glob patterns + matched_ports = [] + for pattern in auto_detect: + for idx, port_name in enumerate(available_ports): + # Case-insensitive glob matching + if fnmatch.fnmatch(port_name.lower(), pattern.lower()): + matched_ports.append((idx, port_name)) + + if not matched_ports: + logging.warning(f"No MIDI ports matched patterns: {auto_detect}") + return None + + # Warn if multiple matches + if len(matched_ports) > 1: + port_names = [name for _, name in matched_ports] + logging.warning( + f"Multiple MIDI ports matched {auto_detect}: {port_names}. Using first match: {matched_ports[0][1]}" + ) + + selected_idx, selected_name = matched_ports[0] + logging.info(f"Auto-detected MIDI port: {selected_name} (index {selected_idx})") + return selected_idx + + def _init_port(self, port_name: str) -> rtmidi.MidiOut | None: + if port_name in self.midi_ports: + return self.midi_ports[port_name] + + port_config = self.port_configs.get(port_name) + if not port_config: + logging.warning(f"No configuration found for MIDI port: {port_name}") + return None + + port_idx = self._find_port_by_name(port_config) + if port_idx is None: + logging.warning(f"Could not find MIDI port for: {port_name}") + return None + + try: + midi_out = rtmidi.MidiOut() + midi_out.open_port(port_idx) + self.midi_ports[port_name] = midi_out + logging.info(f"Opened MIDI port: {port_name}") + return midi_out + except Exception as e: + logging.error(f"Failed to open MIDI port {port_name} (index {port_idx}): {e}") + self.midi_ports[port_name] = None + return None + + def _invalidate_port(self, port_name: str) -> None: + """ + Invalidate a port that has failed, closing it and removing from cache. + This forces re-discovery/re-opening on next use. + """ + if port_name in self.midi_ports: + midi_out = self.midi_ports[port_name] + try: + midi_out.close_port() + except Exception as e: + logging.debug(f"Error closing invalidated port {port_name}: {e}") + del self.midi_ports[port_name] + + def _validate_midi_message(self, message: MidiMessage) -> bool: + if not isinstance(message, list) or len(message) < 2: + logging.warning(f"Invalid MIDI message format (must be list with 2+ bytes): {message}") + return False + + status = message[0] + if not (0x80 <= status <= 0xFF): + logging.warning(f"Invalid MIDI status byte (must be 0x80-0xFF): 0x{status:02X}") + return False + + for i, byte in enumerate(message[1:], start=1): + if not (0x00 <= byte <= 0x7F): + logging.warning(f"Invalid MIDI data byte at position {i} (must be 0x00-0x7F): 0x{byte:02X}") + return False + + return True + + def _send_messages(self, port_name: str, messages: list[MidiMessage], delay_ms: int = 10): + """ + Send MIDI messages to a port. + + Args: + port_name: Name of port configuration. + messages: List of MIDI messages to send. + delay_ms: Delay between messages in milliseconds. + """ + midi_out = self._init_port(port_name) + if midi_out is None: + logging.warning(f"Skipping messages for unavailable port: {port_name}") + return + + for i, message in enumerate(messages): + if not self._validate_midi_message(message): + logging.warning(f"Skipping invalid MIDI message {i + 1}/{len(messages)}: {message}") + continue + + try: + midi_out.send_message(message) + logging.debug(f"Sent MIDI message to {port_name}: {[f'0x{b:02X}' for b in message]}") + + # Delay between messages (except after last one) + if i < len(messages) - 1 and delay_ms > 0: + time.sleep(delay_ms / 1000.0) + + except Exception as e: + logging.error(f"Failed to send MIDI message to {port_name}: {e}") + self._invalidate_port(port_name) + break # Stop sending remaining messages to this broken port + + def send_cc(self, port_name: str, channel: int, cc: int, value: int) -> bool: + """ + Send a single MIDI CC message to an external port. + Used by controls (footswitches, encoders, expression pedals) when + configured to route to external MIDI devices. + + Args: + port_name: Name of port configuration from config file. + channel: MIDI channel (0-15). + cc: MIDI CC number (0-127). + value: MIDI CC value (0-127). + + Returns: + True if message was sent successfully, False if port unavailable (caller should fallback). + + Raises: + RuntimeError: If port_name not in config (config validation failed at startup). + """ + if not self.enabled: + logging.debug(f"External MIDI disabled, falling back to virtual port") + return False + + # Port name must exist in config (should have been validated at startup) + if port_name not in self.port_configs: + raise RuntimeError( + f"Port '{port_name}' not found in external_midi config. " + f"Available ports: {list(self.port_configs.keys())}. " + f"This should have been caught during config validation." + ) + + # Validate MIDI values (programming errors) + if not (0 <= channel <= 15): + raise ValueError(f"Invalid MIDI channel {channel} (must be 0-15)") + if not (0 <= cc <= 127): + raise ValueError(f"Invalid MIDI CC {cc} (must be 0-127)") + if not (0 <= value <= 127): + raise ValueError(f"Invalid MIDI value {value} (must be 0-127)") + + # Lazy initialization of port (may fail if device not connected) + midi_out = self._init_port(port_name) + if midi_out is None: + # Port not available - caller should fallback to virtual port + logging.debug(f"Port {port_name} not available, caller will fallback to virtual port") + return False + + # Construct and send MIDI message + # Status byte: 0xB0 (CC) | channel + status = 0xB0 | channel + message = [status, cc, value] + + try: + midi_out.send_message(message) + logging.debug(f"Sent CC to {port_name}: channel={channel} cc={cc} value={value}") + return True + except Exception as e: + logging.error(f"Failed to send CC to {port_name}: {e}") + self._invalidate_port(port_name) + return False + + def send_messages_for_pedalboard(self) -> bool: + """ + Send external MIDI messages for current pedalboard configuration. + Configuration should have been set via update_config() before calling this. + + Returns: + True if messages were sent successfully, False otherwise. + """ + if not self.enabled: + return False + + if not self.messages: + return False + + for port_name, messages in self.messages.items(): + if not messages: + continue + + logging.debug(f"Sending MIDI message(s) to {port_name}: {messages}") + self._send_messages(port_name, messages, self.send_delay_ms) + + return True + + def close(self): + """Close ports and clean up.""" + for port_name, midi_out in self.midi_ports.items(): + try: + midi_out.close_port() + logging.debug(f"Closed MIDI port: {port_name}") + except Exception as e: + logging.warning(f"Error closing MIDI port {port_name}: {e}") + + self.midi_ports.clear() + logging.info("External MIDI manager closed") diff --git a/modalapi/mod.py b/modalapi/mod.py index bf7bedf40..8dad58195 100755 --- a/modalapi/mod.py +++ b/modalapi/mod.py @@ -27,6 +27,7 @@ import modalapi.pedalboard as Pedalboard import common.parameter as Parameter import modalapi.wifi as Wifi +import modalapi.external_midi as ExternalMidi from pistomp.analogmidicontrol import AnalogMidiControl from pistomp.footswitch import Footswitch @@ -136,12 +137,20 @@ def __init__(self, audiocard, homedir): self.current_menu = MenuType.MENU_NONE # This file is modified when the pedalboard is changed via MOD UI - self.pedalboard_modification_file = "/home/pistomp/data/last.json" + self.data_dir = "/home/pistomp/data" + self.pedalboard_modification_file = os.path.join(self.data_dir, "last.json") self.pedalboard_change_timestamp = os.path.getmtime(self.pedalboard_modification_file)\ if Path(self.pedalboard_modification_file).exists() else 0 self.wifi_manager = Wifi.WifiManager() + # External MIDI device synchronization + self.external_midi = None + try: + self.external_midi = ExternalMidi.ExternalMidiManager() + except Exception as e: + logging.warning(f"Failed to initialize external MIDI manager: {e}") + # Callback function map. Key is the user specified name, value is function from this handler # Used for calling handler callbacks pointed to by names which may be user set in the config file self.callbacks = {"set_mod_tap_tempo": self.set_mod_tap_tempo, @@ -153,10 +162,14 @@ def __del__(self): logging.info("Handler cleanup") if self.wifi_manager: del self.wifi_manager + if self.external_midi is not None: + self.external_midi.close() def cleanup(self): if self.lcd is not None: self.lcd.cleanup() + if self.external_midi is not None: + self.external_midi.close() # Container for dynamic data which is unique to the "current" pedalboard # The self.current pointed above will point to this object which gets @@ -183,6 +196,7 @@ def __init__(self, plugin): def add_hardware(self, hardware): self.hardware = hardware + hardware.external_midi = self.external_midi def add_lcd(self, lcd): self.lcd = lcd @@ -537,6 +551,14 @@ def set_current_pedalboard(self, pedalboard): self.load_current_presets() self.update_lcd() + # Send external MIDI messages for this pedalboard + # Config was already updated by hardware.reinit(cfg) above + if self.external_midi is not None: + try: + self.external_midi.send_messages_for_pedalboard() + except Exception as e: + logging.warning(f"Failed to send external MIDI messages: {e}") + # Selection info self.selectable_items.clear() self.selectable_items.append((SelectedType.PEDALBOARD, None)) diff --git a/modalapi/modhandler.py b/modalapi/modhandler.py index 2eb0429c6..a662fde3f 100755 --- a/modalapi/modhandler.py +++ b/modalapi/modhandler.py @@ -27,6 +27,8 @@ import common.util as util import modalapi.pedalboard as Pedalboard import modalapi.wifi as Wifi +import modalapi.external_midi as ExternalMidi +from modalapi.external_midi import EXTERNAL_INSTANCE_ID import pistomp.settings as Settings from pistomp.analogmidicontrol import AnalogMidiControl @@ -97,18 +99,31 @@ def __init__(self, audiocard, homedir): "next_snapshot": self.preset_incr_and_change, "previous_snapshot": self.preset_decr_and_change, "toggle_bypass": self.system_toggle_bypass, - "toggle_tap_tempo_enable": self.toggle_tap_tempo_enable + "toggle_tap_tempo_enable": self.toggle_tap_tempo_enable, + "universal_encoder_sw": self.universal_encoder_sw } + # External MIDI device synchronization + self.external_midi = None + try: + self.external_midi = ExternalMidi.ExternalMidiManager() + except Exception as e: + logging.warning(f"Failed to initialize external MIDI manager: {e}") + def __del__(self): logging.info("Handler cleanup") if self.wifi_manager: del self.wifi_manager + if self.external_midi is not None: + self.external_midi.close() + def cleanup(self): if self.lcd is not None: self.lcd.cleanup() if self.hardware is not None: self.hardware.cleanup() + if self.external_midi is not None: + self.external_midi.close() # Container for dynamic data which is unique to the "current" pedalboard # The self.current pointed above will point to this object which gets @@ -123,6 +138,7 @@ def __init__(self, pedalboard): def add_hardware(self, hardware): self.hardware = hardware + hardware.external_midi = self.external_midi def add_lcd(self, lcd): self.lcd = lcd @@ -229,7 +245,7 @@ def poll_modui_changes(self): self.pedalboard_change_timestamp = ts self.lcd.draw_info_message("Loading...") mod_bundle = self.get_pedalboard_bundle_from_mod() - if mod_bundle: + if mod_bundle and self.current: logging.info("Pedalboard changed via MOD from: %s to: %s" % (self.current.pedalboard.bundle, mod_bundle)) pb = self.reload_pedalboard(mod_bundle) @@ -354,6 +370,14 @@ def set_current_pedalboard(self, pedalboard): self.lcd.link_data(self.pedalboard_list, self.current, self.hardware.footswitches) self.lcd.draw_main_panel() + # Send external MIDI messages for this pedalboard + # Config was already updated by hardware.reinit(cfg) above + if self.external_midi is not None: + try: + self.external_midi.send_messages_for_pedalboard() + except Exception as e: + logging.warning(f"Failed to send external MIDI messages: {e}") + def bind_current_pedalboard(self): # "current" being the pedalboard mod-host says is current # The pedalboard data has already been loaded, but this will overlay diff --git a/pistomp/hardware.py b/pistomp/hardware.py index 4fb2e5275..ed3dc6dfe 100755 --- a/pistomp/hardware.py +++ b/pistomp/hardware.py @@ -26,6 +26,7 @@ import pistomp.taptempo as taptempo from abc import abstractmethod +from modalapi.external_midi import ExternalMidiManager class Hardware: @@ -56,6 +57,7 @@ def __init__(self, default_config, handler, midiout, refresh_callback): self.debounce_map = None self.ledstrip = None self.taptempo = taptempo.TapTempo(None) + self.external_midi: ExternalMidiManager | None = None def toggle_tap_tempo_enable(self, bpm=0): if self.taptempo: @@ -113,6 +115,9 @@ def reinit(self, cfg): # Footswitch configuration self.__init_footswitches(self.cfg) + # External MIDI configuration + self.__init_external_midi(self.cfg) + # Analog control configuration for ac in self.analog_controls: try: @@ -124,6 +129,7 @@ def reinit(self, cfg): if cfg is not None: self.__init_midi(cfg) self.__init_footswitches(cfg) + self.__init_external_midi(cfg) @abstractmethod def init_analog_controls(self): @@ -320,6 +326,16 @@ def __init_midi(self, cfg): if isinstance(ac, AnalogMidiControl.AnalogMidiControl): ac.set_midi_channel(self.midi_channel) + def __init_external_midi(self, cfg): + """Initialize/update external MIDI config (called for both default and pedalboard config).""" + if self.external_midi is None: + return + if cfg is None or Token.HARDWARE not in cfg: + return + ext_cfg = cfg[Token.HARDWARE].get("external_midi") + if ext_cfg: + self.external_midi.update_config(ext_cfg) + def __init_footswitches_default(self): for fs in self.footswitches: fs.clear_relays() diff --git a/setup/config_templates/default_config_pistomptre.yml b/setup/config_templates/default_config_pistomptre.yml index e0e836ef5..c2be313a6 100644 --- a/setup/config_templates/default_config_pistomptre.yml +++ b/setup/config_templates/default_config_pistomptre.yml @@ -81,3 +81,46 @@ hardware: - id: 3 type: VOLUME + # external_midi: + # External MIDI device synchronization - sends messages when pedalboards load + # Can be overridden per-pedalboard by creating .pedalboard/config.yml + # + # enabled: Enable/disable external MIDI (default: false) + # send_delay_ms: Delay between consecutive messages in milliseconds (default: 10) + # + # ports: Define external MIDI devices + # : + # auto_detect: [] List of glob patterns to match port names (case-insensitive) + # port_index: Manual port index (overrides auto_detect) + # + # messages: MIDI messages to send for each port on pedalboard load + # : + # - [0xSS, 0xDD, ...] List of MIDI messages (hex bytes) + # + # Example configuration: + # + # external_midi: + # enabled: true + # send_delay_ms: 10 + # ports: + # c4: + # auto_detect: ["*C4*", "*Source Audio*"] + # hx_stomp: + # auto_detect: ["*HX Stomp*"] + # messages: + # c4: + # - [0xB0, 0x66, 0x00] # CC 102 = 0 (bypass) + # hx_stomp: + # - [0xC0, 0x00] # Program Change 0 + # + # MIDI message formats: + # Program Change: [0xCn, program] where n = channel (0-F) + # Control Change: [0xBn, cc, value] where n = channel (0-F) + # + # Per-pedalboard override example (.pedalboard/config.yml): + # hardware: + # external_midi: + # messages: + # c4: + # - [0xC0, 0x05] # Program Change 5 for this pedalboard only + From 74282667b741e963af68a76061876f05adcb76f2 Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Sun, 19 Apr 2026 16:37:29 -0400 Subject: [PATCH 02/26] feat: route controller MIDI to external devices via midi_port config Any controller (footswitch, analog, encoder) can now direct its MIDI output to an external hardware device instead of the virtual port by adding a `midi_port:` field in hardware config. The port name must match one defined in the `external_midi:` section. Changes: - controller.py: add RoutingDestination/RoutingInfo dataclasses, AnalogDisplayInfo/FootswitchDisplayInfo TypedDicts, get_routing_info() and get_display_info() on Controller base class - analogmidicontrol.py: inherit Controller, add _clamp_endpoints(), _send_value(), send_current_value(), get_display_info(); refactor refresh() and initialize() to use these helpers Note: endpoint clamping logic duplicates fix/analog-endpoint-clamping which can be dropped once this merges. - hardware.py: add ExternalMidiOut wrapping in create_footswitches/ analog_controls/encoders; add __init_encoders_and_analog() for per-pedalboard routing updates; add sync_analog_controls() (respects per-control autosync flag); add __validate_midi_port(), _create_external_parameter() - pistomptre.py: pass midiout through add_encoder() signature - modhandler.py + mod.py: call sync_analog_controls() after reinit(); bind_current_pedalboard() now clears stale bindings, skips external controllers for plugin binding, and adds external controllers to display with synthetic parameters; parameter_value_commit() guards against external and audio parameters (modhandler only) - config template: document midi_port option for all control types Co-Authored-By: Claude Sonnet 4.6 --- modalapi/mod.py | 23 +++ modalapi/modhandler.py | 71 +++++++- pistomp/analogmidicontrol.py | 72 ++++---- pistomp/controller.py | 78 ++++++++- pistomp/hardware.py | 157 ++++++++++++++++-- pistomp/pistomptre.py | 5 +- .../default_config_pistomptre.yml | 3 + 7 files changed, 345 insertions(+), 64 deletions(-) diff --git a/modalapi/mod.py b/modalapi/mod.py index 8dad58195..4b0103d17 100755 --- a/modalapi/mod.py +++ b/modalapi/mod.py @@ -30,6 +30,7 @@ import modalapi.external_midi as ExternalMidi from pistomp.analogmidicontrol import AnalogMidiControl +from pistomp.controller import RoutingDestination from pistomp.footswitch import Footswitch from pistomp.handler import Handler from enum import Enum @@ -559,6 +560,9 @@ def set_current_pedalboard(self, pedalboard): except Exception as e: logging.warning(f"Failed to send external MIDI messages: {e}") + # Sync current state of analog controls (expression pedals, etc.) + self.hardware.sync_analog_controls() + # Selection info self.selectable_items.clear() self.selectable_items.append((SelectedType.PEDALBOARD, None)) @@ -577,6 +581,15 @@ def bind_current_pedalboard(self): # "current" being the pedalboard mod-host says is current # The pedalboard data has already been loaded, but this will overlay # any real time settings + + # Clear previous parameter bindings from all controllers + # TODO: modhandler.py skips volume encoders here — align these two once we understand why + for controller in self.hardware.controllers.values(): + controller.parameter = None + + # Clear analog controllers display data + self.current.analog_controllers = {} + footswitch_plugins = [] if self.current.pedalboard: #logging.debug(self.current.pedalboard.to_json()) @@ -587,6 +600,16 @@ def bind_current_pedalboard(self): if param.binding is not None: controller = self.hardware.controllers.get(param.binding) if controller is not None: + routing = controller.get_routing_info() + + # External controllers shouldn't be bound to plugin parameters + if routing.destination == RoutingDestination.EXTERNAL: + logging.warning( + f"Plugin parameter {plugin.name}:{param.name} is bound to external controller " + f"{param.binding} (routed to {routing.port_name}) - ignoring plugin binding" + ) + continue + # TODO possibly use a setter instead of accessing var directly # What if multiple params could map to the same controller? controller.parameter = param diff --git a/modalapi/modhandler.py b/modalapi/modhandler.py index a662fde3f..0080a26f0 100755 --- a/modalapi/modhandler.py +++ b/modalapi/modhandler.py @@ -32,6 +32,7 @@ import pistomp.settings as Settings from pistomp.analogmidicontrol import AnalogMidiControl +from pistomp.controller import RoutingDestination from pistomp.encodermidicontrol import EncoderMidiControl from pistomp.footswitch import Footswitch from pistomp.handler import Handler @@ -364,6 +365,9 @@ def set_current_pedalboard(self, pedalboard): cfg = yaml.load(ymlfile, Loader=yaml.SafeLoader) self.hardware.reinit(cfg) + # Sync current state of analog controls (expression pedals, etc.) + self.hardware.sync_analog_controls() + # Initialize the data and draw on LCD self.bind_current_pedalboard() self.load_current_presets() @@ -382,6 +386,16 @@ def bind_current_pedalboard(self): # "current" being the pedalboard mod-host says is current # The pedalboard data has already been loaded, but this will overlay # any real time settings + + # Clear previous parameter bindings from all controllers except volume encoder + for controller in self.hardware.controllers.values(): + is_volume = hasattr(controller, 'type') and controller.type == Token.VOLUME + if not is_volume: + controller.parameter = None + + # Clear analog controllers display data + self.current.analog_controllers = {} + footswitch_plugins = [] if self.current.pedalboard: #logging.debug(self.current.pedalboard.to_json()) @@ -392,8 +406,16 @@ def bind_current_pedalboard(self): if param.binding is not None: controller = self.hardware.controllers.get(param.binding) if controller is not None: - # TODO possibly use a setter instead of accessing var directly - # What if multiple params could map to the same controller? + routing = controller.get_routing_info() + + # External controllers shouldn't be bound to plugin parameters + if routing.destination == RoutingDestination.EXTERNAL: + logging.warning( + f"Plugin parameter {plugin.name}:{param.name} is bound to external controller " + f"{param.binding} (routed to {routing.port_name}) - ignoring plugin binding" + ) + continue + controller.parameter = param controller.set_value(param.value) plugin.controllers.append(controller) @@ -415,17 +437,41 @@ def bind_current_pedalboard(self): controller.cfg[Token.ID] = controller.id self.current.analog_controllers[key] = controller.cfg - # LAME special case for volume control - # Doesn't seem quite right to add this here, but it's where all the mapped controls are bound + # Special case for volume control for e in self.hardware.encoders: if e.type == Token.VOLUME: cfg = { - Token.CATEGORY : None, - Token.TYPE : e.type, - Token.ID : e.id + Token.CATEGORY: None, + Token.TYPE: e.type, + Token.ID: e.id } self.current.analog_controllers[Token.VOLUME] = cfg + # Handle external controllers (bind synthetic parameter + add to display) + for controller in self.hardware.controllers.values(): + routing = controller.get_routing_info() + if routing.destination == RoutingDestination.EXTERNAL: + if controller.midi_CC is not None: + controller.parameter = self.hardware._create_external_parameter( + controller, routing.port_name, controller.midi_channel, controller.midi_CC + ) + if isinstance(controller, AnalogMidiControl): + if controller.midi_CC is not None: + key = f"{controller.midi_channel}:{controller.midi_CC}" + display_info = controller.get_display_info() + display_info['category'] = 'External' + self.current.analog_controllers[key] = display_info + elif isinstance(controller, EncoderMidiControl): + if controller.midi_CC is not None: + key = f"{controller.midi_channel}:{controller.midi_CC}" + cfg = { + Token.CATEGORY: 'External', + Token.TYPE: controller.type, + Token.ID: controller.id, + **controller.get_display_info() + } + self.current.analog_controllers[key] = cfg + def pedalboard_change(self, pedalboard=None): logging.info("Pedalboard change") self.lcd.draw_info_message("Loading...") @@ -573,6 +619,17 @@ def get_num_footswitches(self): # def parameter_value_commit(self, param, value): param.value = value + + # Audio parameter (volume, EQ, etc.) - no REST update needed + if param.instance_id is None: + self.audio_parameter_commit(param.symbol, value) + return + + # External MIDI parameters are local-only (visual feedback), no REST update needed + if param.instance_id == EXTERNAL_INSTANCE_ID: + logging.debug("Skipping REST update for external parameter: %s" % param.symbol) + return + url = self.root_uri + "effect/parameter/pi_stomp_set//graph%s/%s" % (param.instance_id, param.symbol) formatted_value = ("%.1f" % param.value) self.parameter_set_send(url, formatted_value, 200) diff --git a/pistomp/analogmidicontrol.py b/pistomp/analogmidicontrol.py index b12128fc7..aa1e0dde9 100755 --- a/pistomp/analogmidicontrol.py +++ b/pistomp/analogmidicontrol.py @@ -13,8 +13,6 @@ # You should have received a copy of the GNU General Public License # along with pi-stomp. If not, see . -from typing_extensions import override - import adafruit_mcp3xxx.mcp3008 as MCP from adafruit_mcp3xxx.analog_in import AnalogIn @@ -22,8 +20,9 @@ from rtmidi.midiconstants import CONTROL_CHANGE import common.util as util -import json import pistomp.analogcontrol as analogcontrol +import pistomp.controller as controller +from pistomp.controller import AnalogDisplayInfo import logging @@ -33,12 +32,11 @@ def as_midi_value(adc_value: int): return util.renormalize(adc_value, 0, 1023, 0, 127) -class AnalogMidiControl(analogcontrol.AnalogControl): - def __init__(self, spi, adc_channel, tolerance, midi_CC, midi_channel, midiout, type, id=None, cfg={}, autosync=False): +class AnalogMidiControl(analogcontrol.AnalogControl, controller.Controller): + def __init__(self, spi, adc_channel, tolerance, midi_CC, midi_channel, midiout, type, id=None, cfg={}, autosync=False, value_change_callback=None): super(AnalogMidiControl, self).__init__(spi, adc_channel, tolerance) - self.midi_CC = midi_CC + controller.Controller.__init__(self, midi_channel, midi_CC) self.midiout = midiout - self.midi_channel = midi_channel self.autosync = autosync # Parent member overrides @@ -47,6 +45,7 @@ def __init__(self, spi, adc_channel, tolerance, midi_CC, midi_channel, midiout, self.last_read = 0 # this keeps track of the last potentiometer value self.value = None self.cfg = cfg + self.value_change_callback = value_change_callback def set_midi_channel(self, midi_channel): self.midi_channel = midi_channel @@ -54,37 +53,48 @@ def set_midi_channel(self, midi_channel): def set_value(self, value): self.value = value - @override - def initialize(self): - if not self.autosync: - return + def _clamp_endpoints(self, value: int) -> int: + """Clamp ADC values near endpoints to exact 0/1023 (deadband at extremes).""" + if value <= self.tolerance: + return 0 + if value >= 1023 - self.tolerance: + return 1023 + return value - # read the analog pin - value = self.readChannel() + def _send_value(self, value): + """Send ADC value as MIDI CC and invoke callback if set.""" set_volume = as_midi_value(value) - cc = [self.midi_channel | CONTROL_CHANGE, self.midi_CC, set_volume] - logging.debug("AnalogControl force-sending CC event %s" % cc) + logging.debug("AnalogControl Sending CC event %s" % cc) self.midiout.send_message(cc) - # save the reading to prevent duplicate sends on next poll - self.last_read = value + if self.value_change_callback: + self.value_change_callback(value, self) - @override - def refresh(self): - # read the analog pin - value = self.readChannel() + def send_current_value(self): + """Force-send the current ADC value unconditionally. Used by sync_analog_controls().""" + value = self._clamp_endpoints(self.readChannel()) + self._send_value(value) + self.last_read = value - # how much has it changed since the last read? - pot_adjust = abs(value - self.last_read) - value_changed = pot_adjust > self.tolerance + def initialize(self): + if not self.autosync: + return + self.send_current_value() - if value_changed: - set_volume = as_midi_value(value) + def refresh(self): + value = self._clamp_endpoints(self.readChannel()) + if abs(value - self.last_read) > self.tolerance: + self._send_value(value) + self.last_read = value - cc = [self.midi_channel | CONTROL_CHANGE, self.midi_CC, set_volume] - logging.debug("AnalogControl Sending CC event %s" % cc) - self.midiout.send_message(cc) + def get_normalized_value(self) -> float: + return self.last_read / 1023.0 - # save the potentiometer reading for the next loop - self.last_read = value + def get_display_info(self) -> AnalogDisplayInfo: + return { + **super(AnalogMidiControl, self).get_display_info(), + 'type': self.type, + 'id': self.id, + 'category': None, + } diff --git a/pistomp/controller.py b/pistomp/controller.py index d55859b2a..3611f6c32 100755 --- a/pistomp/controller.py +++ b/pistomp/controller.py @@ -13,23 +13,68 @@ # You should have received a copy of the GNU General Public License # along with pi-stomp. If not, see . +from __future__ import annotations + +from dataclasses import dataclass from enum import Enum import json import logging +from typing import TypedDict +from common.parameter import Parameter +from rtmidi import MidiOut + + +class RoutingDestination(Enum): + """Where MIDI messages are routed.""" + VIRTUAL = "virtual" # MIDI through virtual port + EXTERNAL = "external" # External hardware device + + +@dataclass(frozen=True) +class RoutingInfo: + """Immutable routing information for a controller.""" + destination: RoutingDestination + port_name: str | None = None # Only for EXTERNAL destination + + @classmethod + def virtual(cls) -> 'RoutingInfo': + """Factory for virtual port routing.""" + return cls(destination=RoutingDestination.VIRTUAL) + + @classmethod + def external(cls, port_name: str) -> 'RoutingInfo': + """Factory for external port routing.""" + return cls(destination=RoutingDestination.EXTERNAL, port_name=port_name) + + +class AnalogDisplayInfo(TypedDict, total=False): + """Display information for analog controls and encoders.""" + type: str # Token.KNOB, Token.EXPRESSION, Token.VOLUME + id: int # Position on screen (0-based from left) + category: str | None # Plugin category (for color coding) or None + port_name: str | None # External port name if routed externally + midi_cc: int | None # MIDI CC for external routing display + + +class FootswitchDisplayInfo(TypedDict, total=False): + """Display information for footswitches.""" + id: int + label: str | None + color: tuple[int, int, int] | None # RGB + category: str | None class Controller: - def __init__(self, midi_channel, midi_CC): - self.midi_channel = midi_channel - self.midi_CC = midi_CC - self.minimum = None - self.maximum = None - self.parameter = None + def __init__(self, midi_channel: int, midi_CC: int | None): + self.midi_channel: int = midi_channel + self.midi_CC: int | None = midi_CC + self.parameter: Parameter | None = None self.hardware_name = None - #self.type = None # this will conflict with encoder.type for EncoderMidiControl - self.midi_min = 0 - self.midi_max = 127 + #self.type = None # this will conflict with encoder.type for EncoderController + self.midi_min: int = 0 + self.midi_max: int = 127 + self.midiout: MidiOut | None = None # Set by subclasses def to_json(self): return json.dumps(self, default=lambda o: o.__dict__, sort_keys=True, indent=4) @@ -37,4 +82,19 @@ def to_json(self): def set_value(self, value): logging.error("Controller subclass hasn't overriden the set_value method") + def get_routing_info(self) -> RoutingInfo: + """Get routing information for this controller.""" + from modalapi.external_midi import ExternalMidiOut + if isinstance(self.midiout, ExternalMidiOut): + return RoutingInfo.external(self.midiout.port_name) + else: + return RoutingInfo.virtual() + def get_display_info(self) -> dict: + """Get display information. Supplement in subclasses.""" + routing = self.get_routing_info() + info: dict = {} + if routing.destination == RoutingDestination.EXTERNAL: + info['port_name'] = routing.port_name + info['midi_cc'] = self.midi_CC + return info diff --git a/pistomp/hardware.py b/pistomp/hardware.py index ed3dc6dfe..c23331405 100755 --- a/pistomp/hardware.py +++ b/pistomp/hardware.py @@ -20,13 +20,14 @@ import common.token as Token import common.util as Util -from pistomp.analogcontrol import AnalogControl +import common.parameter as Parameter +from common.parameter import TTL_PROPERTIES, TTL_INTEGER import pistomp.analogmidicontrol as AnalogMidiControl import pistomp.footswitch as Footswitch import pistomp.taptempo as taptempo from abc import abstractmethod -from modalapi.external_midi import ExternalMidiManager +from modalapi.external_midi import ExternalMidiOut, ExternalMidiManager, EXTERNAL_INSTANCE_ID class Hardware: @@ -48,7 +49,7 @@ def __init__(self, default_config, handler, midiout, refresh_callback): # Standard hardware objects (not required to exist) self.relay = None - self.analog_controls: list[AnalogControl] = [] + self.analog_controls = [] self.encoders = [] self.controllers = {} self.footswitches = [] @@ -91,6 +92,15 @@ def poll_controls(self): if s: s.check_longpress_events() + def sync_analog_controls(self): + """Send current values of analog controls with autosync enabled via MIDI.""" + for control in self.analog_controls: + if getattr(control, 'autosync', False) and hasattr(control, 'send_current_value'): + try: + control.send_current_value() + except Exception as e: + logging.warning(f"Failed to sync analog control {control.midi_CC}: {e}") + def poll_indicators(self): for i in self.indicators: i.refresh() @@ -118,18 +128,15 @@ def reinit(self, cfg): # External MIDI configuration self.__init_external_midi(self.cfg) - # Analog control configuration - for ac in self.analog_controls: - try: - ac.initialize() - except Exception as e: - logging.warning(f"Failed to initialize analog control {ac}: {e}") + # Encoder and analog controller configuration (update midiout for external routing) + self.__init_encoders_and_analog(self.cfg) # Pedalboard specific config if cfg is not None: self.__init_midi(cfg) self.__init_footswitches(cfg) self.__init_external_midi(cfg) + self.__init_encoders_and_analog(cfg) @abstractmethod def init_analog_controls(self): @@ -218,16 +225,27 @@ def create_footswitches(self, cfg): if taptempo: taptempo.set_callback(self.handler.get_callback(tap_tempo_callback)) + # Configure external MIDI routing if specified + midi_port = Util.DICT_GET(f, "midi_port") + if midi_port: + midi_port = self.__validate_midi_port(midi_port) + + if midi_port: + midiout = ExternalMidiOut(self.external_midi, midi_port, midi_channel, self.midiout) + logging.info(f"Footswitch {idx} routing MIDI CC {midi_cc} to external port '{midi_port}'") + else: + midiout = self.midiout + if adc_input is not None: fs = Footswitch.Footswitch(id if id else idx, gpio_output, pixel, midi_cc, midi_channel, - self.midiout, refresh_callback=self.refresh_callback, + midiout, refresh_callback=self.refresh_callback, adc_input=adc_input, spi=self.spi, taptempo = taptempo) logging.debug("Created Footswitch on ADC input: %d, Midi Chan: %d, CC: %s" % (adc_input, midi_channel, midi_cc)) elif gpio_input is not None: fs = Footswitch.Footswitch(id if id else idx, gpio_output, pixel, midi_cc, midi_channel, - self.midiout, refresh_callback=self.refresh_callback, + midiout, refresh_callback=self.refresh_callback, gpio_input=gpio_input, taptempo = taptempo) logging.debug("Created Footswitch on GPIO input: %d, Midi Chan: %d, CC: %s" % @@ -266,17 +284,29 @@ def create_analog_controls(self, cfg): if autosync is None: autosync = False # Default to False + # Configure external MIDI routing if specified + midi_port = Util.DICT_GET(c, "midi_port") + if midi_port: + midi_port = self.__validate_midi_port(midi_port) + + if midi_port: + midiout = ExternalMidiOut(self.external_midi, midi_port, midi_channel, self.midiout) + logging.info(f"Analog control {id} routing MIDI CC {midi_cc} to external port '{midi_port}'") + else: + midiout = self.midiout + control = AnalogMidiControl.AnalogMidiControl(self.spi, adc_input, threshold, midi_cc, midi_channel, - self.midiout, control_type, id, c, autosync) + midiout, control_type, id, c, autosync) self.analog_controls.append(control) key = format("%d:%d" % (midi_channel, midi_cc)) self.controllers[key] = control logging.debug("Created AnalogMidiControl Input: %d, Midi Chan: %d, CC: %d" % (adc_input, midi_channel, midi_cc)) - def add_encoder(self, id, type, callback, longpress_callback, midi_channel, midi_cc): + def add_encoder(self, id, type, callback, longpress_callback, midi_channel, midi_cc, shortpress_config=None, midiout=None): # This should be implemented by hardware subclasses that support tweak encoders (Tre at least) pass + def create_encoders(self, cfg): if cfg is None or (Token.HARDWARE not in cfg) or (Token.ENCODERS not in cfg[Token.HARDWARE]): return @@ -298,7 +328,18 @@ def create_encoders(self, cfg): logging.error("Config file error. Encoder specified without %s" % Token.ID) continue - control = self.add_encoder(id, type, None, longpress_callback, midi_channel, midi_cc) + # Configure external MIDI routing if specified + midi_port = Util.DICT_GET(c, "midi_port") + if midi_port: + midi_port = self.__validate_midi_port(midi_port) + + if midi_port: + midiout = ExternalMidiOut(self.external_midi, midi_port, midi_channel, self.midiout) + logging.info(f"Encoder {id} routing MIDI CC {midi_cc} to external port '{midi_port}'") + else: + midiout = self.midiout + + control = self.add_encoder(id, type, None, longpress_callback, midi_channel, midi_cc, midiout=midiout) self.encoders.append(control) if midi_cc is not None: @@ -316,6 +357,92 @@ def get_real_midi_channel(self, cfg): pass return chan + def _create_external_parameter(self, controller, port_name, midi_channel, midi_cc): + name = f"{port_name}:{midi_cc}" + info = { + Token.NAME: name, + Token.SYMBOL: f"external_{port_name}_{midi_cc}", + Token.RANGES: { + Token.MINIMUM: 0, + Token.MAXIMUM: 127 + }, + TTL_PROPERTIES: [TTL_INTEGER] + } + val = getattr(controller, 'midi_value', 0) + return Parameter.Parameter(info, val, f"{midi_channel}:{midi_cc}", EXTERNAL_INSTANCE_ID) + + def __validate_midi_port(self, port_name): + """Validate a midi_port name against external_midi config. Returns port_name or None.""" + if port_name is None: + return None + if self.external_midi is None: + logging.error( + f"Control configured with midi_port '{port_name}' but external_midi not initialized. " + f"Falling back to virtual port." + ) + return None + if port_name not in self.external_midi.port_configs: + available = list(self.external_midi.port_configs.keys()) if self.external_midi.port_configs else [] + logging.error( + f"Control configured with midi_port '{port_name}' but port not found in external_midi config. " + f"Available ports: {available}. Falling back to virtual port." + ) + return None + return port_name + + def __init_encoders_and_analog(self, cfg): + """Update midiout for encoders and analog controllers based on config.""" + if cfg is None: + return + + midi_channel = self.get_real_midi_channel(cfg) + + if Token.HARDWARE in cfg and Token.ENCODERS in cfg[Token.HARDWARE]: + cfg_encoders = cfg[Token.HARDWARE][Token.ENCODERS] + if cfg_encoders: + for enc_cfg in cfg_encoders: + enc_id = Util.DICT_GET(enc_cfg, Token.ID) + if enc_id is None: + continue + encoder = next((e for e in self.encoders if e.id == enc_id), None) + if encoder is None: + continue + midi_port = Util.DICT_GET(enc_cfg, "midi_port") + if midi_port: + midi_port = self.__validate_midi_port(midi_port) + midi_cc = Util.DICT_GET(enc_cfg, Token.MIDI_CC) + if midi_cc is not None and hasattr(encoder, 'midi_CC'): + encoder.midi_CC = midi_cc + if midi_port: + encoder.midiout = ExternalMidiOut(self.external_midi, midi_port, midi_channel, self.midiout) + logging.debug(f"Encoder {enc_id} routing CC {midi_cc} to external port '{midi_port}'") + else: + encoder.midiout = self.midiout + logging.debug(f"Encoder {enc_id} routing to virtual port") + + if Token.HARDWARE in cfg and Token.ANALOG_CONTROLLERS in cfg[Token.HARDWARE]: + cfg_analog = cfg[Token.HARDWARE][Token.ANALOG_CONTROLLERS] + if cfg_analog: + for analog_cfg in cfg_analog: + analog_id = Util.DICT_GET(analog_cfg, Token.ID) + if analog_id is None: + continue + analog = next((a for a in self.analog_controls if a.id == analog_id), None) + if analog is None: + continue + midi_port = Util.DICT_GET(analog_cfg, "midi_port") + if midi_port: + midi_port = self.__validate_midi_port(midi_port) + midi_cc = Util.DICT_GET(analog_cfg, Token.MIDI_CC) + if midi_cc is not None and hasattr(analog, 'midi_CC'): + analog.midi_CC = midi_cc + if midi_port: + analog.midiout = ExternalMidiOut(self.external_midi, midi_port, midi_channel, self.midiout) + logging.debug(f"Analog controller {analog_id} routing CC {midi_cc} to external port '{midi_port}'") + else: + analog.midiout = self.midiout + logging.debug(f"Analog controller {analog_id} routing to virtual port") + def __init_midi_default(self): self.__init_midi(self.cfg) @@ -327,7 +454,7 @@ def __init_midi(self, cfg): ac.set_midi_channel(self.midi_channel) def __init_external_midi(self, cfg): - """Initialize/update external MIDI config (called for both default and pedalboard config).""" + """Initialize/update external MIDI config (called for both default and pedalboard).""" if self.external_midi is None: return if cfg is None or Token.HARDWARE not in cfg: diff --git a/pistomp/pistomptre.py b/pistomp/pistomptre.py index 63a80aae6..f5e370915 100755 --- a/pistomp/pistomptre.py +++ b/pistomp/pistomptre.py @@ -90,7 +90,7 @@ def __init__(self, cfg, handler, midiout, refresh_callback): def init_lcd(self): self.handler.add_lcd(Lcd.Lcd(self.handler.homedir, self.handler, flip=False)) - def add_encoder(self, id, type, callback, longpress_callback, midi_channel, midi_cc): + def add_encoder(self, id, type, callback, longpress_callback, midi_channel, midi_cc, shortpress_config=None, midiout=None): enc_pins = Util.DICT_GET(ENC, id) if enc_pins is None: logging.error("Cannot create encoder object for id:", id) @@ -108,7 +108,8 @@ def add_encoder(self, id, type, callback, longpress_callback, midi_channel, midi enc = EncoderMidiControl.EncoderMidiControl(self.handler, d_pin=d_pin, clk_pin=clk_pin, callback=callback, midi_channel=midi_channel, midi_CC=midi_cc, - midiout=self.midiout, type=Token.KNOB, id=id) + midiout=midiout if midiout is not None else self.midiout, + type=Token.KNOB, id=id) if sw_pin is not None: longpress = self.handler.get_callback(longpress_callback) diff --git a/setup/config_templates/default_config_pistomptre.yml b/setup/config_templates/default_config_pistomptre.yml index c2be313a6..b6df85ab0 100644 --- a/setup/config_templates/default_config_pistomptre.yml +++ b/setup/config_templates/default_config_pistomptre.yml @@ -23,6 +23,7 @@ hardware: # adc_input: The analog input to which the switch is connected (required) # ledstrip_position: The position of the corresponding LED (optional) # midi_CC: The MIDI CC message to be sent when switch is clicked (optional) + # midi_port: Route MIDI to an external device port (optional, must match a port in external_midi) # longpress: The name of a handler method to call when switch is long-pressed (optional) # longpress can be a list enclosed with []'s # @@ -54,6 +55,7 @@ hardware: # id: The id and position on the screen (starting with 0 on the left) # type: The control type, used to represent the control on the screen (optional) # midi_CC: The MIDI CC message to be sent when the control is adjusted (optional) + # midi_port: Route MIDI to an external device port (optional, must match a port in external_midi) # autosync: Whether to send current value on pedalboard load (optional, default: false) # #analog_controllers: @@ -69,6 +71,7 @@ hardware: # type: The control type (default is KNOB, VOLUME controls output volume) # midi_CC: The MIDI CC message to be sent when the control is adjusted (optional) # cannot be used along with type=VOLUME + # midi_port: Route MIDI to an external device port (optional, must match a port in external_midi) # longpress: The name of a handler method to call when switch is long-pressed (optional) # encoders: From c18fea3315d56fc84e329c9921bae98774e36810 Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Sun, 10 May 2026 16:08:34 -0400 Subject: [PATCH 03/26] Remove unnecessary universal_encoder_sw --- GUIDE.md | 2 +- modalapi/modhandler.py | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/GUIDE.md b/GUIDE.md index 4ea52a3bf..6d6f22b28 100644 --- a/GUIDE.md +++ b/GUIDE.md @@ -110,7 +110,7 @@ encoders: - id: 1 midi_CC: 70 # Rotation longpress: previous_snapshot - shortpress: universal_encoder_sw # Default if omitted + # shortpress omitted — defaults to the built-in encoder click handler - id: 2 midi_CC: 71 diff --git a/modalapi/modhandler.py b/modalapi/modhandler.py index 3ea753ddb..715c8c40f 100755 --- a/modalapi/modhandler.py +++ b/modalapi/modhandler.py @@ -100,8 +100,7 @@ def __init__(self, audiocard, homedir): "next_snapshot": self.preset_incr_and_change, "previous_snapshot": self.preset_decr_and_change, "toggle_bypass": self.system_toggle_bypass, - "toggle_tap_tempo_enable": self.toggle_tap_tempo_enable, - "universal_encoder_sw": self.universal_encoder_sw + "toggle_tap_tempo_enable": self.toggle_tap_tempo_enable } # External MIDI device synchronization From 93f700a57dc238d87b32a85632ec5b098c5388d0 Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Mon, 11 May 2026 21:20:07 -0400 Subject: [PATCH 04/26] Import EXTERNAL_INSTANCE_ID directly in modhandler parameter_value_commit uses bare EXTERNAL_INSTANCE_ID but only the ExternalMidi namespace alias is imported. Add the direct import so tests calling parameter_midi_change don't NameError. Co-Authored-By: Claude Opus 4.7 --- modalapi/modhandler.py | 1 + 1 file changed, 1 insertion(+) diff --git a/modalapi/modhandler.py b/modalapi/modhandler.py index 9326810f4..bb6448f7d 100755 --- a/modalapi/modhandler.py +++ b/modalapi/modhandler.py @@ -31,6 +31,7 @@ import modalapi.pedalboard as Pedalboard import modalapi.wifi as Wifi import modalapi.external_midi as ExternalMidi +from modalapi.external_midi import EXTERNAL_INSTANCE_ID from pistomp.lcd320x240 import Lcd from pistomp.hardware import Controller, Hardware import pistomp.settings as Settings From da5f259d2dddef32159885f014f39b27c351474b Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Mon, 11 May 2026 22:46:34 -0400 Subject: [PATCH 05/26] MIDI tests --- tests/test_external_midi.py | 303 ++++++++++++++++++++++++++++++++++++ 1 file changed, 303 insertions(+) create mode 100644 tests/test_external_midi.py diff --git a/tests/test_external_midi.py b/tests/test_external_midi.py new file mode 100644 index 000000000..1318f486f --- /dev/null +++ b/tests/test_external_midi.py @@ -0,0 +1,303 @@ +""" +Tests for ExternalMidiManager and ExternalMidiOut. + +These exercise the UX flows added on feat/external-midi: + - configuring external MIDI ports via auto-detect / explicit index + - sending pedalboard load messages with inter-message delay + - per-pedalboard config overrides (incremental update_config) + - send_cc with graceful fallback when an external port is unavailable + - ExternalMidiOut wrapper preferring external port, falling back to virtual +""" + +from typing import cast +from unittest.mock import MagicMock + +import pytest + +from modalapi.external_midi import ExternalMidiManager, ExternalMidiOut + + +def _port(mgr: ExternalMidiManager, name: str) -> MagicMock: + # Casting at the access site lets pyright resolve + # the mock helpers (`assert_called_once_with`, `call_args_list`, etc). + return cast(MagicMock, mgr.midi_ports[name]) + + +@pytest.fixture +def fake_ports(monkeypatch): + """Patch rtmidi.MidiOut so MidiOut() returns a fresh MagicMock per call. + + Returns (available_ports_list, created_outs_list). Modify available_ports_list + in place to control what get_ports() returns. created_outs_list collects each + MagicMock instance constructed via rtmidi.MidiOut(), so tests can assert on + open_port / send_message / close_port calls. + """ + available_ports: list[str] = [] + created_outs: list[MagicMock] = [] + + def _factory(*args, **kwargs): + m = MagicMock() + m.get_ports.return_value = list(available_ports) + created_outs.append(m) + return m + + monkeypatch.setattr("modalapi.external_midi.rtmidi.MidiOut", _factory) + return available_ports, created_outs + + +class TestUpdateConfig: + def test_disabled_by_default(self): + mgr = ExternalMidiManager() + assert mgr.enabled is False + assert mgr.send_delay_ms == 10 + assert mgr.port_configs == {} + assert mgr.messages == {} + + def test_none_config_is_noop(self): + mgr = ExternalMidiManager() + mgr.update_config(None) + assert mgr.enabled is False + + def test_enables_and_sets_delay(self): + mgr = ExternalMidiManager() + mgr.update_config({"enabled": True, "send_delay_ms": 25}) + assert mgr.enabled is True + assert mgr.send_delay_ms == 25 + + def test_ports_merge_across_calls(self): + """Pedalboard config can add/override individual ports without wiping defaults.""" + mgr = ExternalMidiManager() + mgr.update_config({"ports": {"c4": {"auto_detect": ["*C4*"]}}}) + mgr.update_config({"ports": {"hx": {"auto_detect": ["*HX*"]}}}) + assert "c4" in mgr.port_configs + assert "hx" in mgr.port_configs + + def test_messages_merge_per_port(self): + """Per-pedalboard override updates only the named port, leaves others.""" + mgr = ExternalMidiManager() + mgr.update_config({"messages": {"c4": [[0xC0, 0x00]], "hx": [[0xC0, 0x01]]}}) + mgr.update_config({"messages": {"c4": [[0xC0, 0x05]]}}) + assert mgr.messages["c4"] == [[0xC0, 0x05]] + assert mgr.messages["hx"] == [[0xC0, 0x01]] + + +class TestPortDiscovery: + def test_auto_detect_glob_case_insensitive(self, fake_ports): + available, _ = fake_ports + available[:] = ["Midi Through:0", "Source Audio C4 Synth", "HX Stomp"] + mgr = ExternalMidiManager() + mgr.update_config( + { + "enabled": True, + "ports": {"c4": {"auto_detect": ["*c4*"]}}, + "messages": {"c4": [[0xC0, 0x05]]}, + } + ) + assert mgr.send_messages_for_pedalboard() is True + # Opened port 1 (the C4) + _port(mgr, "c4").open_port.assert_called_once_with(1) + + def test_explicit_port_index_wins(self, fake_ports): + available, _ = fake_ports + available[:] = ["A", "B", "C"] + mgr = ExternalMidiManager() + mgr.update_config( + { + "enabled": True, + "ports": {"manual": {"port_index": 2, "auto_detect": ["*A*"]}}, + "messages": {"manual": [[0xC0, 0x00]]}, + } + ) + mgr.send_messages_for_pedalboard() + _port(mgr, "manual").open_port.assert_called_once_with(2) + + def test_no_match_skips_port(self, fake_ports): + available, _ = fake_ports + available[:] = ["Midi Through"] + mgr = ExternalMidiManager() + mgr.update_config( + { + "enabled": True, + "ports": {"c4": {"auto_detect": ["*C4*"]}}, + "messages": {"c4": [[0xC0, 0x05]]}, + } + ) + # send_messages_for_pedalboard returns True (iteration ran), but no port opened + mgr.send_messages_for_pedalboard() + assert "c4" not in mgr.midi_ports or mgr.midi_ports.get("c4") is None + + +class TestSendMessagesForPedalboard: + def test_disabled_short_circuits(self, fake_ports): + mgr = ExternalMidiManager() + mgr.update_config({"messages": {"c4": [[0xC0, 0]]}}) + assert mgr.send_messages_for_pedalboard() is False + + def test_no_messages_returns_false(self): + mgr = ExternalMidiManager() + mgr.update_config({"enabled": True}) + assert mgr.send_messages_for_pedalboard() is False + + def test_delay_applied_between_messages(self, fake_ports, monkeypatch): + available, _ = fake_ports + available[:] = ["dev"] + sleeps: list[float] = [] + monkeypatch.setattr("modalapi.external_midi.time.sleep", lambda s: sleeps.append(s)) + + mgr = ExternalMidiManager() + mgr.update_config( + { + "enabled": True, + "send_delay_ms": 25, + "ports": {"dev": {"auto_detect": ["*dev*"]}}, + "messages": {"dev": [[0xC0, 0], [0xC0, 1], [0xC0, 2]]}, + } + ) + mgr.send_messages_for_pedalboard() + # Delays between consecutive messages, but not after the last one + assert sleeps == [0.025, 0.025] + assert _port(mgr, "dev").send_message.call_count == 3 + + def test_invalid_message_is_skipped_others_sent(self, fake_ports): + available, _ = fake_ports + available[:] = ["dev"] + mgr = ExternalMidiManager() + mgr.update_config( + { + "enabled": True, + "send_delay_ms": 0, + "ports": {"dev": {"auto_detect": ["dev"]}}, + "messages": { + "dev": [ + [0x00, 0x00], # invalid status byte (< 0x80) + [0xC0, 0x05], # valid + [0xB0, 0x80, 0x00], # invalid data byte (> 0x7F) + [0xB0, 0x10, 0x40], # valid + ] + }, + } + ) + mgr.send_messages_for_pedalboard() + sent = [c.args[0] for c in _port(mgr, "dev").send_message.call_args_list] + assert sent == [[0xC0, 0x05], [0xB0, 0x10, 0x40]] + + def test_failing_port_invalidated_and_stops(self, fake_ports): + available, _ = fake_ports + available[:] = ["dev"] + mgr = ExternalMidiManager() + mgr.update_config( + { + "enabled": True, + "send_delay_ms": 0, + "ports": {"dev": {"auto_detect": ["dev"]}}, + "messages": {"dev": [[0xC0, 1], [0xC0, 2], [0xC0, 3]]}, + } + ) + midi_out = MagicMock() + midi_out.send_message.side_effect = RuntimeError("device disconnected") + # Pre-seed the cache so _init_port returns our failing mock + mgr.midi_ports["dev"] = midi_out + + mgr.send_messages_for_pedalboard() + # Stopped after first failure; port removed from cache so next call re-opens + assert midi_out.send_message.call_count == 1 + assert "dev" not in mgr.midi_ports + midi_out.close_port.assert_called_once() + + +class TestSendCC: + def test_returns_false_when_disabled(self): + mgr = ExternalMidiManager() + mgr.update_config({"ports": {"dev": {"port_index": 0}}}) + assert mgr.send_cc("dev", 0, 10, 64) is False + + def test_unknown_port_raises(self): + mgr = ExternalMidiManager() + mgr.update_config({"enabled": True}) + with pytest.raises(RuntimeError, match="not found in external_midi config"): + mgr.send_cc("ghost", 0, 10, 64) + + @pytest.mark.parametrize( + "channel,cc,value", + [(-1, 10, 64), (16, 10, 64), (0, -1, 64), (0, 128, 64), (0, 10, -1), (0, 10, 128)], + ) + def test_invalid_midi_args_raise(self, channel, cc, value): + mgr = ExternalMidiManager() + mgr.update_config({"enabled": True, "ports": {"dev": {"port_index": 0}}}) + with pytest.raises(ValueError): + mgr.send_cc("dev", channel, cc, value) + + def test_sends_cc_and_returns_true(self, fake_ports): + available, _ = fake_ports + available[:] = ["dev"] + mgr = ExternalMidiManager() + mgr.update_config({"enabled": True, "ports": {"dev": {"port_index": 0}}}) + assert mgr.send_cc("dev", 2, 80, 100) is True + # CC status = 0xB0 | channel + _port(mgr, "dev").send_message.assert_called_once_with([0xB2, 80, 100]) + + def test_returns_false_when_port_unavailable(self, fake_ports): + mgr = ExternalMidiManager() + # auto_detect won't match → _init_port returns None + mgr.update_config({"enabled": True, "ports": {"dev": {"auto_detect": ["*nope*"]}}}) + available, _ = fake_ports + available[:] = ["something_else"] + assert mgr.send_cc("dev", 0, 10, 64) is False + + def test_send_failure_invalidates_port(self, fake_ports): + mgr = ExternalMidiManager() + mgr.update_config({"enabled": True, "ports": {"dev": {"port_index": 0}}}) + midi_out = MagicMock() + midi_out.send_message.side_effect = RuntimeError("broken") + mgr.midi_ports["dev"] = midi_out + assert mgr.send_cc("dev", 0, 10, 64) is False + assert "dev" not in mgr.midi_ports + + +class TestClose: + def test_close_closes_all_ports(self, fake_ports): + available, _ = fake_ports + available[:] = ["a", "b"] + mgr = ExternalMidiManager() + mgr.update_config( + { + "enabled": True, + "ports": {"a": {"port_index": 0}, "b": {"port_index": 1}}, + "messages": {"a": [[0xC0, 0]], "b": [[0xC0, 0]]}, + } + ) + mgr.send_messages_for_pedalboard() + outs = [_port(mgr, "a"), _port(mgr, "b")] + mgr.close() + for o in outs: + o.close_port.assert_called_once() + assert mgr.midi_ports == {} + + +class TestExternalMidiOut: + def test_short_message_uses_fallback(self): + manager = MagicMock() + fallback = MagicMock() + out = ExternalMidiOut(manager, "dev", 0, fallback) + out.send_message([0xF0, 0x7E]) # 2 bytes + fallback.send_message.assert_called_once_with([0xF0, 0x7E]) + manager.send_cc.assert_not_called() + + def test_routes_to_external_when_available(self): + manager = MagicMock() + manager.send_cc.return_value = True + fallback = MagicMock() + out = ExternalMidiOut(manager, "dev", 3, fallback) + out.send_message([0xB3, 80, 100]) + manager.send_cc.assert_called_once_with("dev", 3, 80, 100) + fallback.send_message.assert_not_called() + + def test_falls_back_when_external_unavailable(self): + manager = MagicMock() + manager.send_cc.return_value = False + fallback = MagicMock() + out = ExternalMidiOut(manager, "dev", 0, fallback) + msg = [0xB0, 10, 64] + out.send_message(msg) + manager.send_cc.assert_called_once_with("dev", 0, 10, 64) + fallback.send_message.assert_called_once_with(msg) From 2fd85ac495f08f80c18d366a72c21ed8143a4b77 Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Fri, 5 Jun 2026 01:02:25 -0400 Subject: [PATCH 06/26] Fix: update last_read before value_change_callback in AnalogMidiControl The sync resolution took the pre-fix ordering where last_read was updated after _send_value() returned, so a callback re-reading get_normalized_value() (blend mode) observed the previous pedal position. Move the assignment into _send_value() before the callback fires, matching release/current. Co-Authored-By: Claude Opus 4.8 --- pistomp/analogmidicontrol.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pistomp/analogmidicontrol.py b/pistomp/analogmidicontrol.py index 4701b181b..a34117921 100755 --- a/pistomp/analogmidicontrol.py +++ b/pistomp/analogmidicontrol.py @@ -68,6 +68,10 @@ def _send_value(self, value): logging.debug("AnalogControl Sending CC event %s" % cc) self.midiout.send_message(cc) + # Update last_read BEFORE the callback so get_normalized_value() reflects + # the new position when blend mode re-reads the control inside the callback. + self.last_read = value + if self.value_change_callback: self.value_change_callback(value, self) @@ -75,7 +79,6 @@ def send_current_value(self): """Force-send the current ADC value unconditionally. Used by sync_analog_controls().""" value = self._clamp_endpoints(self.readChannel()) self._send_value(value) - self.last_read = value def initialize(self): if not self.autosync: @@ -86,7 +89,6 @@ def refresh(self): value = self._clamp_endpoints(self.readChannel()) if abs(value - self.last_read) > self.tolerance: self._send_value(value) - self.last_read = value def get_normalized_value(self) -> float: return self.last_read / 1023.0 From 5a4858ee9ab07fcf54879c9e29543060e8e1582e Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Sat, 6 Jun 2026 14:02:44 -0400 Subject: [PATCH 07/26] Bugfixes + refators --- modalapi/external_midi.py | 119 +++++++++++++---------------------- pistomp/hardware.py | 10 +-- tests/test_external_midi.py | 121 +++++++++++++++++++++++++----------- 3 files changed, 133 insertions(+), 117 deletions(-) diff --git a/modalapi/external_midi.py b/modalapi/external_midi.py index 720536e99..9273ffbe2 100644 --- a/modalapi/external_midi.py +++ b/modalapi/external_midi.py @@ -25,7 +25,6 @@ MidiMessage = list[int] -# Constant for identifying external parameters EXTERNAL_INSTANCE_ID = "External" @@ -36,46 +35,19 @@ class ExternalMidiOut: transparently, with automatic fallback to virtual port if device unavailable. """ - def __init__( - self, external_midi_manager: ExternalMidiManager, port_name: str, midi_channel: int, fallback_midiout: RtMidiOut - ): - """ - Initialize external MIDI output wrapper. - - Args: - external_midi_manager: Reference to ExternalMidiManager. - port_name: Name of external MIDI port from config. - midi_channel: MIDI channel (0-15). - fallback_midiout: Virtual port midiout to use if external unavailable. - """ + def __init__(self, external_midi_manager: ExternalMidiManager, port_name: str, fallback_midiout: RtMidiOut): self.external_midi = external_midi_manager self.port_name = port_name - self.channel = midi_channel self.fallback = fallback_midiout def send_message(self, message: MidiMessage) -> None: - """ - Send MIDI message to external port, falling back to virtual port if unavailable. - - Args: - message: MIDI message as [status_byte, data1, data2, ...] - """ - if len(message) < 3: - # Not a CC message we can route, just use fallback + try: + success = self.external_midi.send_raw(self.port_name, message) + except Exception: + logging.warning(f"External MIDI send failed for port {self.port_name}, falling back to virtual") self.fallback.send_message(message) return - - # Extract CC and value from message - # Status byte format: 0xBn where n is channel (already included) - cc = message[1] - value = message[2] - - # Try to send to external port - success = self.external_midi.send_cc(self.port_name, self.channel, cc, value) - if not success: - # External port unavailable, fallback to virtual port - logging.debug(f"External port {self.port_name} unavailable, using virtual port") self.fallback.send_message(message) @@ -87,8 +59,8 @@ class PortConfig(TypedDict, total=False): class ExternalMidiConfig(TypedDict, total=False): enabled: bool send_delay_ms: int - ports: dict[str, PortConfig] # configured devices - messages: dict[str, list[MidiMessage]] # port_name -> list of MIDI messages + ports: dict[str, PortConfig] + messages: dict[str, list[MidiMessage]] class ExternalMidiManager: @@ -123,12 +95,15 @@ def update_config(self, cfg: ExternalMidiConfig | None) -> None: self.send_delay_ms = cfg["send_delay_ms"] if "ports" in cfg: - # Merge ports (port-level granularity) - self.port_configs.update(cfg["ports"]) + # Deep merge: overlay individual port fields without replacing entire dicts + for port_name, port_cfg in cfg["ports"].items(): + if port_name in self.port_configs: + self.port_configs[port_name] = {**self.port_configs[port_name], **port_cfg} + else: + self.port_configs[port_name] = port_cfg if "messages" in cfg: - # Merge messages at port level - # This allows pedalboard config to override specific ports while keeping others default + # Merge messages at port level (replace entire message list per port) self.messages.update(cfg["messages"]) def _get_available_ports(self) -> list[str]: @@ -202,7 +177,7 @@ def _init_port(self, port_name: str) -> rtmidi.MidiOut | None: return midi_out except Exception as e: logging.error(f"Failed to open MIDI port {port_name} (index {port_idx}): {e}") - self.midi_ports[port_name] = None + del self.midi_ports[port_name] return None def _invalidate_port(self, port_name: str) -> None: @@ -267,11 +242,35 @@ def _send_messages(self, port_name: str, messages: list[MidiMessage], delay_ms: self._invalidate_port(port_name) break # Stop sending remaining messages to this broken port + def send_raw(self, port_name: str, message: MidiMessage) -> bool: + """ + Send a raw MIDI message to an external port. + Same contract as send_cc: enabled gate, lazy port discovery, invalidation on failure. + Returns True on success, False if port unavailable (caller should fallback). + """ + if not self.enabled: + return False + + if port_name not in self.port_configs: + logging.error(f"Port '{port_name}' not found in external_midi config, falling back to virtual") + return False + + midi_out = self._init_port(port_name) + if midi_out is None: + return False + + try: + midi_out.send_message(message) + return True + except Exception as e: + logging.error(f"Failed to send MIDI message to {port_name}: {e}") + self._invalidate_port(port_name) + return False + def send_cc(self, port_name: str, channel: int, cc: int, value: int) -> bool: """ Send a single MIDI CC message to an external port. - Used by controls (footswitches, encoders, expression pedals) when - configured to route to external MIDI devices. + Convenience wrapper around send_raw that constructs the CC message. Args: port_name: Name of port configuration from config file. @@ -281,23 +280,7 @@ def send_cc(self, port_name: str, channel: int, cc: int, value: int) -> bool: Returns: True if message was sent successfully, False if port unavailable (caller should fallback). - - Raises: - RuntimeError: If port_name not in config (config validation failed at startup). """ - if not self.enabled: - logging.debug(f"External MIDI disabled, falling back to virtual port") - return False - - # Port name must exist in config (should have been validated at startup) - if port_name not in self.port_configs: - raise RuntimeError( - f"Port '{port_name}' not found in external_midi config. " - f"Available ports: {list(self.port_configs.keys())}. " - f"This should have been caught during config validation." - ) - - # Validate MIDI values (programming errors) if not (0 <= channel <= 15): raise ValueError(f"Invalid MIDI channel {channel} (must be 0-15)") if not (0 <= cc <= 127): @@ -305,26 +288,8 @@ def send_cc(self, port_name: str, channel: int, cc: int, value: int) -> bool: if not (0 <= value <= 127): raise ValueError(f"Invalid MIDI value {value} (must be 0-127)") - # Lazy initialization of port (may fail if device not connected) - midi_out = self._init_port(port_name) - if midi_out is None: - # Port not available - caller should fallback to virtual port - logging.debug(f"Port {port_name} not available, caller will fallback to virtual port") - return False - - # Construct and send MIDI message - # Status byte: 0xB0 (CC) | channel status = 0xB0 | channel - message = [status, cc, value] - - try: - midi_out.send_message(message) - logging.debug(f"Sent CC to {port_name}: channel={channel} cc={cc} value={value}") - return True - except Exception as e: - logging.error(f"Failed to send CC to {port_name}: {e}") - self._invalidate_port(port_name) - return False + return self.send_raw(port_name, [status, cc, value]) def send_messages_for_pedalboard(self) -> bool: """ diff --git a/pistomp/hardware.py b/pistomp/hardware.py index eb37daeec..57b150693 100755 --- a/pistomp/hardware.py +++ b/pistomp/hardware.py @@ -242,7 +242,7 @@ def create_footswitches(self, cfg): midi_port = self.__validate_midi_port(midi_port) if midi_port: - midiout = ExternalMidiOut(self.external_midi, midi_port, midi_channel, self.midiout) + midiout = ExternalMidiOut(self.external_midi, midi_port, self.midiout) logging.info(f"Footswitch {idx} routing MIDI CC {midi_cc} to external port '{midi_port}'") else: midiout = self.midiout @@ -303,7 +303,7 @@ def create_analog_controls(self, cfg): midi_port = self.__validate_midi_port(midi_port) if midi_port: - midiout = ExternalMidiOut(self.external_midi, midi_port, midi_channel, self.midiout) + midiout = ExternalMidiOut(self.external_midi, midi_port, self.midiout) logging.info(f"Analog control {id} routing MIDI CC {midi_cc} to external port '{midi_port}'") else: midiout = self.midiout @@ -348,7 +348,7 @@ def create_encoders(self, cfg): midi_port = self.__validate_midi_port(midi_port) if midi_port: - midiout = ExternalMidiOut(self.external_midi, midi_port, midi_channel, self.midiout) + midiout = ExternalMidiOut(self.external_midi, midi_port, self.midiout) logging.info(f"Encoder {id} routing MIDI CC {midi_cc} to external port '{midi_port}'") else: midiout = self.midiout @@ -433,7 +433,7 @@ def __init_encoders_and_analog(self, cfg): if midi_cc is not None and hasattr(encoder, 'midi_CC'): encoder.midi_CC = midi_cc if midi_port: - encoder.midiout = ExternalMidiOut(self.external_midi, midi_port, midi_channel, self.midiout) + encoder.midiout = ExternalMidiOut(self.external_midi, midi_port, self.midiout) logging.debug(f"Encoder {enc_id} routing CC {midi_cc} to external port '{midi_port}'") else: encoder.midiout = self.midiout @@ -456,7 +456,7 @@ def __init_encoders_and_analog(self, cfg): if midi_cc is not None and hasattr(analog, 'midi_CC'): analog.midi_CC = midi_cc if midi_port: - analog.midiout = ExternalMidiOut(self.external_midi, midi_port, midi_channel, self.midiout) + analog.midiout = ExternalMidiOut(self.external_midi, midi_port, self.midiout) logging.debug(f"Analog controller {analog_id} routing CC {midi_cc} to external port '{midi_port}'") else: analog.midiout = self.midiout diff --git a/tests/test_external_midi.py b/tests/test_external_midi.py index 1318f486f..c19b9eb1a 100644 --- a/tests/test_external_midi.py +++ b/tests/test_external_midi.py @@ -80,6 +80,15 @@ def test_messages_merge_per_port(self): assert mgr.messages["c4"] == [[0xC0, 0x05]] assert mgr.messages["hx"] == [[0xC0, 0x01]] + def test_ports_deep_merge_preserves_defaults(self): + """Pedalboard config overlay should merge fields, not replace the whole port dict.""" + mgr = ExternalMidiManager() + mgr.update_config({"ports": {"c4": {"auto_detect": ["*C4*"], "port_index": 0}}}) + # Overlay only changes auto_detect — port_index should be preserved + mgr.update_config({"ports": {"c4": {"auto_detect": ["*C4 Ultra*"]}}}) + assert mgr.port_configs["c4"].get("auto_detect") == ["*C4 Ultra*"] + assert mgr.port_configs["c4"].get("port_index") == 0 + class TestPortDiscovery: def test_auto_detect_glob_case_insensitive(self, fake_ports): @@ -205,44 +214,39 @@ def test_failing_port_invalidated_and_stops(self, fake_ports): midi_out.close_port.assert_called_once() -class TestSendCC: +class TestSendRaw: def test_returns_false_when_disabled(self): mgr = ExternalMidiManager() mgr.update_config({"ports": {"dev": {"port_index": 0}}}) - assert mgr.send_cc("dev", 0, 10, 64) is False + assert mgr.send_raw("dev", [0xB0, 10, 64]) is False - def test_unknown_port_raises(self): + def test_unknown_port_returns_false(self): mgr = ExternalMidiManager() mgr.update_config({"enabled": True}) - with pytest.raises(RuntimeError, match="not found in external_midi config"): - mgr.send_cc("ghost", 0, 10, 64) + assert mgr.send_raw("ghost", [0xB0, 10, 64]) is False - @pytest.mark.parametrize( - "channel,cc,value", - [(-1, 10, 64), (16, 10, 64), (0, -1, 64), (0, 128, 64), (0, 10, -1), (0, 10, 128)], - ) - def test_invalid_midi_args_raise(self, channel, cc, value): + def test_sends_message_and_returns_true(self, fake_ports): + available, _ = fake_ports + available[:] = ["dev"] mgr = ExternalMidiManager() mgr.update_config({"enabled": True, "ports": {"dev": {"port_index": 0}}}) - with pytest.raises(ValueError): - mgr.send_cc("dev", channel, cc, value) + assert mgr.send_raw("dev", [0xB0, 80, 100]) is True + _port(mgr, "dev").send_message.assert_called_once_with([0xB0, 80, 100]) - def test_sends_cc_and_returns_true(self, fake_ports): + def test_sends_non_cc_message(self, fake_ports): available, _ = fake_ports available[:] = ["dev"] mgr = ExternalMidiManager() mgr.update_config({"enabled": True, "ports": {"dev": {"port_index": 0}}}) - assert mgr.send_cc("dev", 2, 80, 100) is True - # CC status = 0xB0 | channel - _port(mgr, "dev").send_message.assert_called_once_with([0xB2, 80, 100]) + assert mgr.send_raw("dev", [0xC0, 5]) is True # Program Change + _port(mgr, "dev").send_message.assert_called_once_with([0xC0, 5]) def test_returns_false_when_port_unavailable(self, fake_ports): mgr = ExternalMidiManager() - # auto_detect won't match → _init_port returns None mgr.update_config({"enabled": True, "ports": {"dev": {"auto_detect": ["*nope*"]}}}) available, _ = fake_ports available[:] = ["something_else"] - assert mgr.send_cc("dev", 0, 10, 64) is False + assert mgr.send_raw("dev", [0xB0, 10, 64]) is False def test_send_failure_invalidates_port(self, fake_ports): mgr = ExternalMidiManager() @@ -250,10 +254,35 @@ def test_send_failure_invalidates_port(self, fake_ports): midi_out = MagicMock() midi_out.send_message.side_effect = RuntimeError("broken") mgr.midi_ports["dev"] = midi_out - assert mgr.send_cc("dev", 0, 10, 64) is False + assert mgr.send_raw("dev", [0xB0, 10, 64]) is False assert "dev" not in mgr.midi_ports +class TestSendCC: + def test_returns_false_when_disabled(self): + mgr = ExternalMidiManager() + mgr.update_config({"ports": {"dev": {"port_index": 0}}}) + assert mgr.send_cc("dev", 0, 10, 64) is False + + @pytest.mark.parametrize( + "channel,cc,value", + [(-1, 10, 64), (16, 10, 64), (0, -1, 64), (0, 128, 64), (0, 10, -1), (0, 10, 128)], + ) + def test_invalid_midi_args_raise(self, channel, cc, value): + mgr = ExternalMidiManager() + mgr.update_config({"enabled": True, "ports": {"dev": {"port_index": 0}}}) + with pytest.raises(ValueError): + mgr.send_cc("dev", channel, cc, value) + + def test_delegates_to_send_raw(self, fake_ports): + available, _ = fake_ports + available[:] = ["dev"] + mgr = ExternalMidiManager() + mgr.update_config({"enabled": True, "ports": {"dev": {"port_index": 0}}}) + assert mgr.send_cc("dev", 2, 80, 100) is True + _port(mgr, "dev").send_message.assert_called_once_with([0xB2, 80, 100]) + + class TestClose: def test_close_closes_all_ports(self, fake_ports): available, _ = fake_ports @@ -275,29 +304,51 @@ def test_close_closes_all_ports(self, fake_ports): class TestExternalMidiOut: - def test_short_message_uses_fallback(self): - manager = MagicMock() - fallback = MagicMock() - out = ExternalMidiOut(manager, "dev", 0, fallback) - out.send_message([0xF0, 0x7E]) # 2 bytes - fallback.send_message.assert_called_once_with([0xF0, 0x7E]) - manager.send_cc.assert_not_called() - - def test_routes_to_external_when_available(self): + def test_delegates_to_send_raw(self): manager = MagicMock() - manager.send_cc.return_value = True + manager.send_raw.return_value = True fallback = MagicMock() - out = ExternalMidiOut(manager, "dev", 3, fallback) + out = ExternalMidiOut(manager, "dev", fallback) out.send_message([0xB3, 80, 100]) - manager.send_cc.assert_called_once_with("dev", 3, 80, 100) + manager.send_raw.assert_called_once_with("dev", [0xB3, 80, 100]) fallback.send_message.assert_not_called() - def test_falls_back_when_external_unavailable(self): + def test_falls_back_when_send_raw_fails(self): manager = MagicMock() - manager.send_cc.return_value = False + manager.send_raw.return_value = False fallback = MagicMock() - out = ExternalMidiOut(manager, "dev", 0, fallback) + out = ExternalMidiOut(manager, "dev", fallback) msg = [0xB0, 10, 64] out.send_message(msg) - manager.send_cc.assert_called_once_with("dev", 0, 10, 64) + manager.send_raw.assert_called_once_with("dev", msg) fallback.send_message.assert_called_once_with(msg) + + def test_falls_back_when_send_raw_raises(self): + """Exceptions from send_raw must not crash the poll loop.""" + manager = MagicMock() + manager.send_raw.side_effect = RuntimeError("port not in config") + fallback = MagicMock() + out = ExternalMidiOut(manager, "dev", fallback) + out.send_message([0xB0, 10, 64]) + fallback.send_message.assert_called_once_with([0xB0, 10, 64]) + + def test_non_cc_message_routes_to_external(self): + """Non-CC messages are sent to external port via send_raw like any other.""" + manager = MagicMock() + manager.send_raw.return_value = True + fallback = MagicMock() + out = ExternalMidiOut(manager, "dev", fallback) + msg = [0x92, 64, 100] # Note On + out.send_message(msg) + manager.send_raw.assert_called_once_with("dev", msg) + fallback.send_message.assert_not_called() + + def test_program_change_routes_to_external(self): + """Even short messages go through send_raw.""" + manager = MagicMock() + manager.send_raw.return_value = True + fallback = MagicMock() + out = ExternalMidiOut(manager, "dev", fallback) + out.send_message([0xC0, 5]) # Program Change, 2 bytes + manager.send_raw.assert_called_once_with("dev", [0xC0, 5]) + fallback.send_message.assert_not_called() From 9602ecf97e47eb8b77b2dda81a4c0a6248d2c248 Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Sat, 6 Jun 2026 14:26:40 -0400 Subject: [PATCH 08/26] Remove dead code, noisy logging, over-verbosity --- modalapi/modhandler.py | 2 +- pistomp/controller.py | 61 ++++++++++++++++-------------------------- pistomp/hardware.py | 29 +++++--------------- 3 files changed, 30 insertions(+), 62 deletions(-) diff --git a/modalapi/modhandler.py b/modalapi/modhandler.py index 2285bdf8f..9b7a3d336 100755 --- a/modalapi/modhandler.py +++ b/modalapi/modhandler.py @@ -668,7 +668,7 @@ def bind_current_pedalboard(self): routing = controller.get_routing_info() if routing.destination == RoutingDestination.EXTERNAL: if controller.midi_CC is not None: - controller.parameter = self.hardware._create_external_parameter( + controller.parameter = self.hardware.create_external_parameter( controller, routing.port_name, controller.midi_channel, controller.midi_CC ) if isinstance(controller, AnalogMidiControl): diff --git a/pistomp/controller.py b/pistomp/controller.py index d3120ba14..e41867018 100755 --- a/pistomp/controller.py +++ b/pistomp/controller.py @@ -17,7 +17,6 @@ from dataclasses import dataclass from enum import Enum -import json import logging from typing import TypedDict from common.parameter import Parameter @@ -25,76 +24,62 @@ class RoutingDestination(Enum): - """Where MIDI messages are routed.""" - VIRTUAL = "virtual" # MIDI through virtual port - EXTERNAL = "external" # External hardware device + VIRTUAL = "virtual" + EXTERNAL = "external" @dataclass(frozen=True) class RoutingInfo: - """Immutable routing information for a controller.""" destination: RoutingDestination - port_name: str | None = None # Only for EXTERNAL destination + port_name: str | None = None @classmethod - def virtual(cls) -> 'RoutingInfo': - """Factory for virtual port routing.""" + def virtual(cls) -> "RoutingInfo": return cls(destination=RoutingDestination.VIRTUAL) @classmethod - def external(cls, port_name: str) -> 'RoutingInfo': - """Factory for external port routing.""" + def external(cls, port_name: str) -> "RoutingInfo": return cls(destination=RoutingDestination.EXTERNAL, port_name=port_name) class AnalogDisplayInfo(TypedDict, total=False): - """Display information for analog controls and encoders.""" - type: str # Token.KNOB, Token.EXPRESSION, Token.VOLUME - id: int # Position on screen (0-based from left) - category: str | None # Plugin category (for color coding) or None - port_name: str | None # External port name if routed externally - midi_cc: int | None # MIDI CC for external routing display - - -class FootswitchDisplayInfo(TypedDict, total=False): - """Display information for footswitches.""" - id: int - label: str | None - color: tuple[int, int, int] | None # RGB + type: str # Token.KNOB, Token.EXPRESSION, Token.VOLUME + id: int # Position on screen (0-based from left) category: str | None + port_name: str | None # External port name if routed externally + midi_cc: int | None # MIDI CC for external routing display class Controller: - def __init__(self, midi_channel: int, midi_CC: int | None): self.midi_channel: int = midi_channel self.midi_CC: int | None = midi_CC self.parameter: Parameter | None = None - self.hardware_name = None - #self.type = None # this will conflict with encoder.type for EncoderController + # type is not declared here — it conflicts with encoder.Encoder.type in EncoderController's MRO. + # Subclasses that carry type must declare it themselves. self.midi_min: int = 0 self.midi_max: int = 127 - self.midiout: MidiOut | None = None # Set by subclasses + self.midi_value: int = 0 + self.midiout: MidiOut | None = None - def to_json(self): - return json.dumps(self, default=lambda o: o.__dict__, sort_keys=True, indent=4) + def set_value(self, value: float) -> None: + logging.error(f"Controller subclass ({self.__class__.__name__}) hasn't overriden the set_value method") - def set_value(self, bypass_value: float): - logging.error("Controller subclass hasn't overriden the set_value method") + def bind_to_parameter(self, parameter: Parameter) -> None: + self.parameter = parameter + self.set_value(parameter.value) def get_routing_info(self) -> RoutingInfo: - """Get routing information for this controller.""" from modalapi.external_midi import ExternalMidiOut + if isinstance(self.midiout, ExternalMidiOut): return RoutingInfo.external(self.midiout.port_name) - else: - return RoutingInfo.virtual() + return RoutingInfo.virtual() def get_display_info(self) -> dict: - """Get display information. Supplement in subclasses.""" routing = self.get_routing_info() info: dict = {} if routing.destination == RoutingDestination.EXTERNAL: - info['port_name'] = routing.port_name - info['midi_cc'] = self.midi_CC - return info + info["port_name"] = routing.port_name + info["midi_cc"] = self.midi_CC + return info \ No newline at end of file diff --git a/pistomp/hardware.py b/pistomp/hardware.py index 57b150693..859c36b76 100755 --- a/pistomp/hardware.py +++ b/pistomp/hardware.py @@ -138,16 +138,13 @@ def reinit(self, cfg): # External MIDI configuration self.__init_external_midi(self.cfg) - # Encoder and analog controller configuration (update midiout for external routing) - self.__init_encoders_and_analog(self.cfg) - # Pedalboard specific config if cfg is not None: self.__init_midi(cfg) self.__init_footswitches(cfg) self.__init_external_midi(cfg) self.__init_encoders(cfg) - self.__init_encoders_and_analog(cfg) + self.__apply_midi_routing(cfg) @abstractmethod def init_analog_controls(self): @@ -376,7 +373,7 @@ def get_real_midi_channel(self, cfg): pass return chan - def _create_external_parameter(self, controller, port_name, midi_channel, midi_cc): + def create_external_parameter(self, controller, port_name, midi_channel, midi_cc): name = f"{port_name}:{midi_cc}" info = { Token.NAME: name, @@ -391,26 +388,16 @@ def _create_external_parameter(self, controller, port_name, midi_channel, midi_c return Parameter.Parameter(info, val, f"{midi_channel}:{midi_cc}", EXTERNAL_INSTANCE_ID) def __validate_midi_port(self, port_name): - """Validate a midi_port name against external_midi config. Returns port_name or None.""" - if port_name is None: - return None if self.external_midi is None: - logging.error( - f"Control configured with midi_port '{port_name}' but external_midi not initialized. " - f"Falling back to virtual port." - ) + logging.error(f"midi_port '{port_name}' set but external_midi not initialized, falling back to virtual") return None if port_name not in self.external_midi.port_configs: - available = list(self.external_midi.port_configs.keys()) if self.external_midi.port_configs else [] - logging.error( - f"Control configured with midi_port '{port_name}' but port not found in external_midi config. " - f"Available ports: {available}. Falling back to virtual port." - ) + logging.error(f"midi_port '{port_name}' not found in external_midi config, falling back to virtual") return None return port_name - def __init_encoders_and_analog(self, cfg): - """Update midiout for encoders and analog controllers based on config.""" + def __apply_midi_routing(self, cfg): + """Apply per-pedalboard MIDI routing overrides for encoders and analog controllers.""" if cfg is None: return @@ -434,10 +421,8 @@ def __init_encoders_and_analog(self, cfg): encoder.midi_CC = midi_cc if midi_port: encoder.midiout = ExternalMidiOut(self.external_midi, midi_port, self.midiout) - logging.debug(f"Encoder {enc_id} routing CC {midi_cc} to external port '{midi_port}'") else: encoder.midiout = self.midiout - logging.debug(f"Encoder {enc_id} routing to virtual port") if Token.HARDWARE in cfg and Token.ANALOG_CONTROLLERS in cfg[Token.HARDWARE]: cfg_analog = cfg[Token.HARDWARE][Token.ANALOG_CONTROLLERS] @@ -457,10 +442,8 @@ def __init_encoders_and_analog(self, cfg): analog.midi_CC = midi_cc if midi_port: analog.midiout = ExternalMidiOut(self.external_midi, midi_port, self.midiout) - logging.debug(f"Analog controller {analog_id} routing CC {midi_cc} to external port '{midi_port}'") else: analog.midiout = self.midiout - logging.debug(f"Analog controller {analog_id} routing to virtual port") def __init_midi_default(self): self.__init_midi(self.cfg) From 56c844f1020241511179602dff643eb6fb162bc0 Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Sat, 6 Jun 2026 14:46:55 -0400 Subject: [PATCH 09/26] A bit more type safety --- pistomp/analogmidicontrol.py | 1 - pistomp/controller.py | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/pistomp/analogmidicontrol.py b/pistomp/analogmidicontrol.py index a34117921..7623f88e4 100755 --- a/pistomp/analogmidicontrol.py +++ b/pistomp/analogmidicontrol.py @@ -13,7 +13,6 @@ # You should have received a copy of the GNU General Public License # along with pi-stomp. If not, see . -from typing_extensions import override from typing import Any diff --git a/pistomp/controller.py b/pistomp/controller.py index e41867018..7b4829d16 100755 --- a/pistomp/controller.py +++ b/pistomp/controller.py @@ -76,10 +76,10 @@ def get_routing_info(self) -> RoutingInfo: return RoutingInfo.external(self.midiout.port_name) return RoutingInfo.virtual() - def get_display_info(self) -> dict: + def get_display_info(self) -> AnalogDisplayInfo: routing = self.get_routing_info() - info: dict = {} + info: AnalogDisplayInfo = {} if routing.destination == RoutingDestination.EXTERNAL: info["port_name"] = routing.port_name info["midi_cc"] = self.midi_CC - return info \ No newline at end of file + return info From 5850be4f6ce6a393635e2093dae0641ef549399d Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Sat, 6 Jun 2026 15:38:58 -0400 Subject: [PATCH 10/26] Add v1 system fixture + integration test --- tests/integration/conftest.py | 141 +++++++++++++++++- .../test_v1_startup_snapshot/0.png | Bin 0 -> 561 bytes tests/types.py | 35 ++++- tests/v1/__init__.py | 0 tests/v1/conftest.py | 38 +++++ tests/v1/test_startup.py | 7 + 6 files changed, 209 insertions(+), 12 deletions(-) create mode 100644 tests/snapshots/v1/test_startup/test_v1_startup_snapshot/0.png create mode 100644 tests/v1/__init__.py create mode 100644 tests/v1/conftest.py create mode 100644 tests/v1/test_startup.py diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index fe411e814..cf5208f99 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -8,31 +8,65 @@ import json from collections.abc import Generator from pathlib import Path -from typing import Any from unittest.mock import patch, MagicMock import pytest import yaml +from PIL import Image from tests.conftest import FakeWebSocketBridge -from tests.types import SystemFixture -from tests.conftest import FakeWebSocketBridge +from tests.types import SystemFixture, SystemFixtureLegacy import common.token as Token PROJECT_ROOT = Path(__file__).parent.parent.parent with patch("pistomp.settings.Settings.load_settings"), patch("pistomp.settings.Settings.set_setting"): from modalapi.modhandler import Modhandler + from modalapi.mod import Mod + from pistomp.hardware import Hardware + from pistomp.pistomp import Pistomp from pistomp.pistompcore import Pistompcore from pistomp.pistomptre import Pistomptre +# --------------------------------------------------------------------------- +# FakeMonoLcd — captures the gfxhat pixel buffer that lcdgfx.Lcd pushes +# --------------------------------------------------------------------------- + + +class FakeMonoLcd: + """Stand-in for the gfxhat lcd module. lcdgfx.Lcd draws into its own PIL + images then pushes pixels here via set_pixel/show. Each show() snapshots + the current buffer into an L-mode frame.""" + + WIDTH = 128 + HEIGHT = 64 + + def __init__(self): + self._buf = Image.new("L", (self.WIDTH, self.HEIGHT)) + self.frames: list[Image.Image] = [] + + def dimensions(self): + return (self.WIDTH, self.HEIGHT) + + def set_pixel(self, x, y, value): + if 0 <= x < self.WIDTH and 0 <= y < self.HEIGHT: + self._buf.putpixel((x, y), 255 if value else 0) + + def show(self): + # lcdgfx flips coordinates for the upside-down panel; un-rotate for readable baselines. + self.frames.append(self._buf.copy().transpose(Image.Transpose.ROTATE_180)) + + def clear(self): + self._buf.paste(0, (0, 0, self.WIDTH, self.HEIGHT)) + + # --------------------------------------------------------------------------- # Shared stack builder # --------------------------------------------------------------------------- -def _build_stack(hw_class: Any, cfg_path: Path, fake_lcd, tmp_path) -> Generator[SystemFixture, None, None]: +def _build_stack(hw_class: type[Hardware], cfg_path: Path, fake_lcd, tmp_path) -> Generator[SystemFixture, None, None]: cwd = str(PROJECT_ROOT) data_dir = tmp_path / "data" @@ -129,6 +163,105 @@ def _v3_stack(fake_lcd, tmp_path) -> Generator[SystemFixture, None, None]: yield from _build_stack(Pistomptre, cfg_path, fake_lcd, tmp_path) +# --------------------------------------------------------------------------- +# v1 stack — Mod handler + Pistomp hardware + monochrome lcdgfx LCD +# --------------------------------------------------------------------------- + + +def _v1_stack(tmp_path) -> Generator[SystemFixtureLegacy, None, None]: + """Real Mod + Pistomp stack. The mono LCD's frames are captured via + FakeMonoLcd (returned as lcd, exposing .frames like FakeLcd).""" + cwd = str(PROJECT_ROOT) + cfg_path = PROJECT_ROOT / "setup" / "config_templates" / "default_config_pistomp.yml" + + data_dir = tmp_path / "data" + data_dir.mkdir() + last_json = data_dir / "last.json" + last_json.write_text(json.dumps({"pedalboard": "/path/to/rig.pedalboard"})) + + from pistomp.lcdgfx import Lcd as MonoLcd + + Mod._Mod__single = None # pyright: ignore[reportAttributeAccessIssue] + Pistomp._Pistomp__single = None # pyright: ignore[reportAttributeAccessIssue] + setattr(MonoLcd, "_Lcd__single", None) + + with open(cfg_path) as f: + cfg = yaml.safe_load(f) + + fake_bridge = FakeWebSocketBridge() + fake_dev = FakeMonoLcd() + + # Inject a real lcdgfx.Lcd backed by the capturing fake device. Patching the + # module-level Lcd would break its internal __single self-reference, so we + # override init_lcd instead. + def fake_init_lcd(hw_self): + hw_self.mod.add_lcd(MonoLcd(hw_self.mod.homedir, lcd=fake_dev, backlight=MagicMock(), touch=MagicMock())) + + with ( + patch("requests.get") as mock_get, + patch("requests.post") as mock_post, + patch("modalapi.pedalboard.Pedalboard.load_bundle"), + patch("modalapi.wifi.WifiManager") as mock_wm_cls, + patch("modalapi.mod.AsyncWebSocketBridge", return_value=fake_bridge), + patch("modalapi.mod.ExternalMidi.ExternalMidiManager"), + patch("pistomp.hardware.Hardware.init_spi"), + patch("pistomp.pistomp.Pistomp.run_test"), + patch("pistomp.pistomp.Pistomp.init_lcd", fake_init_lcd), + ): + mock_wm_cls.return_value.queue.pending_op_count.return_value = 0 + + def get_side_effect(url, **kwargs): + resp = MagicMock() + resp.status_code = 200 + if "pedalboard/list" in url: + resp.text = json.dumps( + [ + {Token.TITLE: "Integration Rig", Token.BUNDLE: "/path/to/rig.pedalboard"}, + {Token.TITLE: "New Rig", Token.BUNDLE: "/path/to/new.pedalboard"}, + ] + ) + elif "snapshot/list" in url: + resp.text = json.dumps({"0": "Clean", "1": "Lead"}) + elif "snapshot/name" in url: + resp.text = json.dumps({"name": "Clean"}) + else: + resp.text = "{}" + return resp + + mock_get.side_effect = get_side_effect + + def post_side_effect(*args, **kwargs): + resp = MagicMock() + resp.status_code = 200 + resp.text = "{}" + return resp + + mock_post.side_effect = post_side_effect + + mock_audiocard = MagicMock() + mock_audiocard.get_volume_parameter.return_value = 0.0 + handler = Mod(mock_audiocard, cwd) + handler.data_dir = str(data_dir) + handler.last_json_monitor.path = str(last_json) + + midiout = MagicMock() + hw = Pistomp(cfg, handler, midiout, handler.update_lcd_fs) + handler.add_hardware(hw) + handler.load_pedalboards() + + pb = handler.pedalboards["/path/to/rig.pedalboard"] + pb.plugins = [] + handler.set_current_pedalboard(pb) + handler.pedalboards["/path/to/new.pedalboard"].plugins = [] + + mock_get.reset_mock() + mock_get.side_effect = get_side_effect + mock_post.reset_mock() + mock_post.side_effect = post_side_effect + + yield SystemFixtureLegacy(handler, hw, fake_dev, mock_get, mock_post, fake_bridge) + + _BUILDERS = { "v2": _v2_stack, "v3": _v3_stack, diff --git a/tests/snapshots/v1/test_startup/test_v1_startup_snapshot/0.png b/tests/snapshots/v1/test_startup/test_v1_startup_snapshot/0.png new file mode 100644 index 0000000000000000000000000000000000000000..7dd55db9f4730ddc23e08de1008caf2b7afa9475 GIT binary patch literal 561 zcmV-10?z%3P)jB1jAKH2iLtA9wfh+fco|Wy&0x@jT5=gnIRmqsxer zpDc*>wi>C(*P`mllJK~2rCYYF2>(JWs`KMyl39`&e#B+GlCSMa+A67ffx6nA(&A)R zewPcnW^+*PcfFeGQJcaFR zJhWec35JCNFlMyJY=XW(5;Y-vz~HRMzJF-L!90Y&8?c{=@a?bB#RT z1ONa40000000000000000000000000n1b<>CAP5)vZP|hTG>WI*}jD5?68t%uGK!` zAv(Acq!-7>CegFmgM(`u&KfWehAM~hk-1%xEFaY<%RfIc>?N!7N#jGCPE{h@D00000NkvXXu0mjfx~C2j literal 0 HcmV?d00001 diff --git a/tests/types.py b/tests/types.py index bb1c7387a..b976285c2 100644 --- a/tests/types.py +++ b/tests/types.py @@ -1,20 +1,39 @@ from __future__ import annotations from dataclasses import dataclass -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Generic, Protocol, TypeVar from unittest.mock import MagicMock if TYPE_CHECKING: - from modalapi.modhandler import Modhandler + from PIL import Image + from modalapi.mod import Mod # noqa: F401 (forward ref in SystemFixtureLegacy) + from modalapi.modhandler import Modhandler # noqa: F401 (forward ref in SystemFixture) + from pistomp.handler import Handler from pistomp.hardware import Hardware - from tests.conftest import FakeLcd, FakeWebSocketBridge + from tests.conftest import FakeWebSocketBridge + +HandlerT = TypeVar("HandlerT", bound="Handler") + + +class CapturedLcd(Protocol): + """Test-double LCDs that record rendered frames (FakeLcd, FakeMonoLcd).""" + + frames: list[Image.Image] @dataclass -class SystemFixture: - handler: Modhandler - hw: Hardware - lcd: FakeLcd - mock_get: MagicMock +class SystemFixtureBase(Generic[HandlerT]): + handler: HandlerT + hw: Hardware + lcd: CapturedLcd + mock_get: MagicMock mock_post: MagicMock ws_bridge: FakeWebSocketBridge + + +class SystemFixture(SystemFixtureBase["Modhandler"]): + """v2/v3 stack (Modhandler).""" + + +class SystemFixtureLegacy(SystemFixtureBase["Mod"]): + """v1 stack (Mod).""" diff --git a/tests/v1/__init__.py b/tests/v1/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/v1/conftest.py b/tests/v1/conftest.py new file mode 100644 index 000000000..5d77468d6 --- /dev/null +++ b/tests/v1/conftest.py @@ -0,0 +1,38 @@ +"""v1-specific fixtures — delegates stack construction to integration/conftest. + +v1 uses the monochrome lcdgfx LCD, so it provides its own ``snapshot`` fixture +backed by the captured FakeMonoLcd frames rather than the color ``fake_lcd``. +""" +from collections.abc import Generator +from pathlib import Path + +import pytest + +from tests.conftest import assert_snapshot, _TESTS_DIR +from tests.integration.conftest import _v1_stack +from tests.types import SystemFixtureLegacy + + +@pytest.fixture +def v1_system(tmp_path) -> Generator[SystemFixtureLegacy, None, None]: + yield from _v1_stack(tmp_path) + + +@pytest.fixture +def snapshot(request, v1_system, snapshot_update): + """Assert the latest mono LCD frame matches a stored PNG snapshot. + + Mirrors the root color snapshot fixture but reads from the v1 system's + captured FakeMonoLcd frames.""" + counter = [0] + rel = Path(request.fspath).relative_to(_TESTS_DIR) + module = str(rel.with_suffix("")) + test = request.node.name + + def _assert(suffix=None): + if suffix is None: + suffix = str(counter[0]) + counter[0] += 1 + assert_snapshot(v1_system.lcd.frames[-1], f"{module}/{test}/{suffix}", update=snapshot_update) + + return _assert diff --git a/tests/v1/test_startup.py b/tests/v1/test_startup.py new file mode 100644 index 000000000..4d2b46dcc --- /dev/null +++ b/tests/v1/test_startup.py @@ -0,0 +1,7 @@ +"""Startup smoke test for the v1 (Mod + Pistomp) stack.""" + + +def test_v1_startup_snapshot(v1_system, snapshot): + lcd = v1_system.lcd + assert len(lcd.frames) > 0 + snapshot() From 35b56b93e4eeada5d82119890569e7f3f68819bb Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Sat, 6 Jun 2026 16:11:25 -0400 Subject: [PATCH 11/26] external-midi: fix _init_port KeyError, double-close, midi_port log level - C2: _init_port no longer dels a cache key set only on success, so an open_port failure returns None instead of raising KeyError. - 4C: external_midi.close() now runs once in cleanup(); removed the duplicate call in both handlers' __del__. - 4E: a midi_port config typo logs at warning, not error. Co-Authored-By: Claude Opus 4.8 --- modalapi/external_midi.py | 1 - modalapi/mod.py | 2 -- modalapi/modhandler.py | 2 -- pistomp/hardware.py | 4 +-- tests/test_external_midi.py | 14 ++++++++++ tests/test_handler_cleanup.py | 49 +++++++++++++++++++++++++++++++++++ tests/test_hardware.py | 47 +++++++++++++++++++++++++++++++++ 7 files changed, 112 insertions(+), 7 deletions(-) create mode 100644 tests/test_handler_cleanup.py create mode 100644 tests/test_hardware.py diff --git a/modalapi/external_midi.py b/modalapi/external_midi.py index 9273ffbe2..13347d6a6 100644 --- a/modalapi/external_midi.py +++ b/modalapi/external_midi.py @@ -177,7 +177,6 @@ def _init_port(self, port_name: str) -> rtmidi.MidiOut | None: return midi_out except Exception as e: logging.error(f"Failed to open MIDI port {port_name} (index {port_idx}): {e}") - del self.midi_ports[port_name] return None def _invalidate_port(self, port_name: str) -> None: diff --git a/modalapi/mod.py b/modalapi/mod.py index d18091116..ed427112f 100755 --- a/modalapi/mod.py +++ b/modalapi/mod.py @@ -180,8 +180,6 @@ def __del__(self): logging.info("Handler cleanup") if self.wifi_manager: del self.wifi_manager - if self.external_midi is not None: - self.external_midi.close() if self.ws_bridge is not None: self.ws_bridge.stop() diff --git a/modalapi/modhandler.py b/modalapi/modhandler.py index 9b7a3d336..295ac1bdb 100755 --- a/modalapi/modhandler.py +++ b/modalapi/modhandler.py @@ -143,8 +143,6 @@ def __del__(self): logging.info("Handler cleanup") if self.wifi_manager: del self.wifi_manager - if self.external_midi is not None: - self.external_midi.close() # ws_bridge.stop() lives in cleanup(), not here — join() in __del__ blows up # during interpreter shutdown on Py 3.14. Daemon thread dies with the process. diff --git a/pistomp/hardware.py b/pistomp/hardware.py index 859c36b76..d60170f0b 100755 --- a/pistomp/hardware.py +++ b/pistomp/hardware.py @@ -389,10 +389,10 @@ def create_external_parameter(self, controller, port_name, midi_channel, midi_cc def __validate_midi_port(self, port_name): if self.external_midi is None: - logging.error(f"midi_port '{port_name}' set but external_midi not initialized, falling back to virtual") + logging.warning(f"midi_port '{port_name}' set but external_midi not initialized, falling back to virtual") return None if port_name not in self.external_midi.port_configs: - logging.error(f"midi_port '{port_name}' not found in external_midi config, falling back to virtual") + logging.warning(f"midi_port '{port_name}' not found in external_midi config, falling back to virtual") return None return port_name diff --git a/tests/test_external_midi.py b/tests/test_external_midi.py index c19b9eb1a..6e31f6ae5 100644 --- a/tests/test_external_midi.py +++ b/tests/test_external_midi.py @@ -214,6 +214,20 @@ def test_failing_port_invalidated_and_stops(self, fake_ports): midi_out.close_port.assert_called_once() +class TestInitPort: + def test_open_port_failure_returns_none_without_keyerror(self, monkeypatch): + """A failing open_port must not KeyError on a cache entry that was never set.""" + failing = MagicMock() + failing.open_port.side_effect = RuntimeError("cannot open") + monkeypatch.setattr("modalapi.external_midi.rtmidi.MidiOut", lambda *a, **k: failing) + + mgr = ExternalMidiManager() + mgr.update_config({"enabled": True, "ports": {"dev": {"port_index": 0}}}) + + assert mgr._init_port("dev") is None + assert "dev" not in mgr.midi_ports + + class TestSendRaw: def test_returns_false_when_disabled(self): mgr = ExternalMidiManager() diff --git a/tests/test_handler_cleanup.py b/tests/test_handler_cleanup.py new file mode 100644 index 000000000..4667cbbaf --- /dev/null +++ b/tests/test_handler_cleanup.py @@ -0,0 +1,49 @@ +"""External MIDI must be closed exactly once, in cleanup() — not also in __del__. + +Closing in both leads to a double-close: cleanup() runs at shutdown, then __del__ +fires again at GC/interpreter teardown on the same (already-closed) manager. +""" + +from unittest.mock import MagicMock + +from modalapi.mod import Mod +from modalapi.modhandler import Modhandler + + +class TestModCleanup: + def test_del_does_not_close_external_midi(self): + h = object.__new__(Mod) + h.wifi_manager = None + h.external_midi = MagicMock() + h.ws_bridge = None + h.__del__() + h.external_midi.close.assert_not_called() + + def test_cleanup_closes_external_midi(self): + h = object.__new__(Mod) + h.wifi_manager = None + h.lcd = None + h.external_midi = MagicMock() + h.ws_bridge = None + h.cleanup() + h.external_midi.close.assert_called_once() + + +class TestModhandlerCleanup: + def test_del_does_not_close_external_midi(self): + h = object.__new__(Modhandler) + h.wifi_manager = None + h.external_midi = MagicMock() + h.__del__() + h.external_midi.close.assert_not_called() + + def test_cleanup_closes_external_midi(self): + h = object.__new__(Modhandler) + h.wifi_manager = None + h._tuner_engine = None + h._lcd = None + h._hardware = None + h.external_midi = MagicMock() + h.ws_bridge = None + h.cleanup() + h.external_midi.close.assert_called_once() diff --git a/tests/test_hardware.py b/tests/test_hardware.py new file mode 100644 index 000000000..7397cf44d --- /dev/null +++ b/tests/test_hardware.py @@ -0,0 +1,47 @@ +"""Unit tests for pistomp.hardware.Hardware helpers.""" + +import logging + +from modalapi.external_midi import ExternalMidiManager +from pistomp.hardware import Hardware + + +class _StubHardware(Hardware): + """Concrete subclass so object.__new__ works (Hardware is abstract).""" + + def init_analog_controls(self): ... + def init_encoders(self): ... + def init_footswitches(self): ... + def init_relays(self): ... + def cleanup(self): ... + def test(self): ... + def add_encoder(self, *a, **k): ... + + +def _validate(hw, port_name): + return hw._Hardware__validate_midi_port(port_name) + + +class TestValidateMidiPort: + def test_unknown_port_logs_warning_not_error(self, caplog): + """A config typo on midi_port is user error, not a system fault — warn, don't error.""" + hw = object.__new__(_StubHardware) + hw.external_midi = ExternalMidiManager() # empty port_configs + + with caplog.at_level(logging.WARNING): + assert _validate(hw, "ghost") is None + + recs = [r for r in caplog.records if "ghost" in r.getMessage()] + assert recs + assert all(r.levelno == logging.WARNING for r in recs) + + def test_uninitialized_external_midi_logs_warning_not_error(self, caplog): + hw = object.__new__(_StubHardware) + hw.external_midi = None + + with caplog.at_level(logging.WARNING): + assert _validate(hw, "dev") is None + + recs = [r for r in caplog.records if "dev" in r.getMessage()] + assert recs + assert all(r.levelno == logging.WARNING for r in recs) From e017b1047d283484c78ed7e7dcce17be536a2eae Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Sat, 6 Jun 2026 16:15:05 -0400 Subject: [PATCH 12/26] external-midi: type-safe volume guard, align mod.py with modhandler Add a class-level Controller.type default so the volume guard resolves on every controller (Footswitch -> None) without hasattr/getattr. mod.py now skips the volume control when clearing bindings, matching modhandler; drops the stale "align these two" TODO. Co-Authored-By: Claude Opus 4.8 --- modalapi/mod.py | 6 ++--- modalapi/modhandler.py | 5 ++--- pistomp/controller.py | 4 ++-- tests/test_controller_type.py | 42 +++++++++++++++++++++++++++++++++++ 4 files changed, 49 insertions(+), 8 deletions(-) create mode 100644 tests/test_controller_type.py diff --git a/modalapi/mod.py b/modalapi/mod.py index ed427112f..49e460cfb 100755 --- a/modalapi/mod.py +++ b/modalapi/mod.py @@ -696,10 +696,10 @@ def bind_current_pedalboard(self): # The pedalboard data has already been loaded, but this will overlay # any real time settings - # Clear previous parameter bindings from all controllers - # TODO: modhandler.py skips volume encoders here — align these two once we understand why + # Clear previous parameter bindings from all controllers except the volume control for controller in self.hardware.controllers.values(): - controller.parameter = None + if controller.type != Token.VOLUME: + controller.parameter = None # Clear analog controllers display data self.current.analog_controllers = {} diff --git a/modalapi/modhandler.py b/modalapi/modhandler.py index 295ac1bdb..afa30fa0a 100755 --- a/modalapi/modhandler.py +++ b/modalapi/modhandler.py @@ -599,10 +599,9 @@ def bind_current_pedalboard(self): # The pedalboard data has already been loaded, but this will overlay # any real time settings - # Clear previous parameter bindings from all controllers except volume encoder + # Clear previous parameter bindings from all controllers except the volume control for controller in self.hardware.controllers.values(): - is_volume = hasattr(controller, 'type') and controller.type == Token.VOLUME - if not is_volume: + if controller.type != Token.VOLUME: controller.parameter = None # Clear analog controllers display data diff --git a/pistomp/controller.py b/pistomp/controller.py index 7b4829d16..9aa362120 100755 --- a/pistomp/controller.py +++ b/pistomp/controller.py @@ -51,12 +51,12 @@ class AnalogDisplayInfo(TypedDict, total=False): class Controller: + type: str | None = None # class default; not in __init__ — clashes with Encoder.type in EncoderController's MRO + def __init__(self, midi_channel: int, midi_CC: int | None): self.midi_channel: int = midi_channel self.midi_CC: int | None = midi_CC self.parameter: Parameter | None = None - # type is not declared here — it conflicts with encoder.Encoder.type in EncoderController's MRO. - # Subclasses that carry type must declare it themselves. self.midi_min: int = 0 self.midi_max: int = 127 self.midi_value: int = 0 diff --git a/tests/test_controller_type.py b/tests/test_controller_type.py new file mode 100644 index 000000000..9854a0041 --- /dev/null +++ b/tests/test_controller_type.py @@ -0,0 +1,42 @@ +"""Controller.type is a class-level default so the volume guard is type-safe. + +Controller deliberately doesn't set ``type`` in __init__ (MRO clash with +encoder.Encoder.type in EncoderController). A class-level default lets +``controller.type`` resolve on every Controller — Footswitch included — +without hasattr/getattr. +""" + +from unittest.mock import MagicMock + +import common.token as Token +from modalapi.mod import Mod +from pistomp.controller import Controller + + +class _Ctl: + """Stand-in controller — v1 config supplies no VOLUME control to use directly.""" + + def __init__(self, type): + self.type = type + self.parameter = "bound" + + +def test_controller_type_defaults_to_none(): + c = Controller(midi_channel=0, midi_CC=7) + assert c.type is None + + +def test_bind_preserves_volume_binding_clears_others(): + h = object.__new__(Mod) + h.wifi_manager = None + vol = _Ctl(Token.VOLUME) + knob = _Ctl(Token.KNOB) + h.hardware = MagicMock() + h.hardware.controllers = {"0:7": vol, "0:8": knob} + h.current = MagicMock() + h.current.pedalboard.plugins = [] + + h.bind_current_pedalboard() + + assert vol.parameter == "bound" + assert knob.parameter is None From fe3e7005a3ce83f2e497cfacad36eefeffaef939 Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Sat, 6 Jun 2026 16:26:13 -0400 Subject: [PATCH 13/26] external-midi: single-pass MIDI routing for all controls (C1/C3) C1: external_midi is None during create_*, so midi_port wiring there always fell back to virtual; __apply_midi_routing also had no footswitch branch and ran only for pedalboard cfg. Pull routing out of create_* and do it in one __apply_midi_routing pass (encoders, analog, footswitches) for both default and pedalboard cfg, via a single __resolve_midiout helper. reinit now routes the default cfg too. C3: open external ports eagerly at routing time, and back off failed ports so a disconnected device doesn't re-enumerate on every 10ms poll tick. Co-Authored-By: Claude Opus 4.8 --- modalapi/external_midi.py | 16 +++++ pistomp/hardware.py | 120 ++++++++++++------------------------ tests/test_external_midi.py | 22 +++++++ tests/test_hardware.py | 91 ++++++++++++++++++++++++++- 4 files changed, 167 insertions(+), 82 deletions(-) diff --git a/modalapi/external_midi.py b/modalapi/external_midi.py index 13347d6a6..a2808072b 100644 --- a/modalapi/external_midi.py +++ b/modalapi/external_midi.py @@ -27,6 +27,8 @@ EXTERNAL_INSTANCE_ID = "External" +PORT_RETRY_BACKOFF_S = 5.0 # don't re-enumerate a failed port more often than this (C3) + class ExternalMidiOut: """ @@ -75,6 +77,7 @@ def __init__(self): self.messages: dict[str, list[MidiMessage]] = {} self.enabled: bool = False self.send_delay_ms: int = 10 + self._open_failures: dict[str, float] = {} def update_config(self, cfg: ExternalMidiConfig | None) -> None: """ @@ -155,28 +158,40 @@ def _find_port_by_name(self, port_config: PortConfig) -> int | None: logging.info(f"Auto-detected MIDI port: {selected_name} (index {selected_idx})") return selected_idx + def open_port(self, port_name: str) -> bool: + """Eagerly open a port at routing time so the first poll-loop send doesn't enumerate (C3).""" + return self._init_port(port_name) is not None + def _init_port(self, port_name: str) -> rtmidi.MidiOut | None: if port_name in self.midi_ports: return self.midi_ports[port_name] + last_fail = self._open_failures.get(port_name) + if last_fail is not None and (time.monotonic() - last_fail) < PORT_RETRY_BACKOFF_S: + return None + port_config = self.port_configs.get(port_name) if not port_config: logging.warning(f"No configuration found for MIDI port: {port_name}") + self._open_failures[port_name] = time.monotonic() return None port_idx = self._find_port_by_name(port_config) if port_idx is None: logging.warning(f"Could not find MIDI port for: {port_name}") + self._open_failures[port_name] = time.monotonic() return None try: midi_out = rtmidi.MidiOut() midi_out.open_port(port_idx) self.midi_ports[port_name] = midi_out + self._open_failures.pop(port_name, None) logging.info(f"Opened MIDI port: {port_name}") return midi_out except Exception as e: logging.error(f"Failed to open MIDI port {port_name} (index {port_idx}): {e}") + self._open_failures[port_name] = time.monotonic() return None def _invalidate_port(self, port_name: str) -> None: @@ -191,6 +206,7 @@ def _invalidate_port(self, port_name: str) -> None: except Exception as e: logging.debug(f"Error closing invalidated port {port_name}: {e}") del self.midi_ports[port_name] + self._open_failures[port_name] = time.monotonic() # back off before re-enumerating (C3) def _validate_midi_message(self, message: MidiMessage) -> bool: if not isinstance(message, list) or len(message) < 2: diff --git a/pistomp/hardware.py b/pistomp/hardware.py index d60170f0b..0a47ff34f 100755 --- a/pistomp/hardware.py +++ b/pistomp/hardware.py @@ -137,6 +137,7 @@ def reinit(self, cfg): # External MIDI configuration self.__init_external_midi(self.cfg) + self.__apply_midi_routing(self.cfg) # Pedalboard specific config if cfg is not None: @@ -233,16 +234,8 @@ def create_footswitches(self, cfg): if taptempo: taptempo.set_callback(self.handler.get_callback(tap_tempo_callback)) - # Configure external MIDI routing if specified - midi_port = Util.DICT_GET(f, "midi_port") - if midi_port: - midi_port = self.__validate_midi_port(midi_port) - - if midi_port: - midiout = ExternalMidiOut(self.external_midi, midi_port, self.midiout) - logging.info(f"Footswitch {idx} routing MIDI CC {midi_cc} to external port '{midi_port}'") - else: - midiout = self.midiout + # midi_port routing is applied later in __apply_midi_routing (external_midi is None here — C1) + midiout = self.midiout fs: Footswitch.Footswitch | None = None if adc_input is not None: @@ -294,19 +287,9 @@ def create_analog_controls(self, cfg): if autosync is None: autosync = False # Default to False - # Configure external MIDI routing if specified - midi_port = Util.DICT_GET(c, "midi_port") - if midi_port: - midi_port = self.__validate_midi_port(midi_port) - - if midi_port: - midiout = ExternalMidiOut(self.external_midi, midi_port, self.midiout) - logging.info(f"Analog control {id} routing MIDI CC {midi_cc} to external port '{midi_port}'") - else: - midiout = self.midiout - + # midi_port routing is applied later in __apply_midi_routing (external_midi is None here — C1) control = AnalogMidiControl.AnalogMidiControl(self.spi, adc_input, threshold, midi_cc, midi_channel, - midiout, control_type, id, c, autosync) + self.midiout, control_type, id, c, autosync) self.analog_controls.append(control) key = format("%d:%d" % (midi_channel, midi_cc)) self.controllers[key] = control @@ -339,19 +322,9 @@ def create_encoders(self, cfg): logging.error("Config file error. Encoder specified without %s" % Token.ID) continue - # Configure external MIDI routing if specified - midi_port = Util.DICT_GET(c, "midi_port") - if midi_port: - midi_port = self.__validate_midi_port(midi_port) - - if midi_port: - midiout = ExternalMidiOut(self.external_midi, midi_port, self.midiout) - logging.info(f"Encoder {id} routing MIDI CC {midi_cc} to external port '{midi_port}'") - else: - midiout = self.midiout - + # midi_port routing is applied later in __apply_midi_routing (external_midi is None here — C1) try: - control = self.add_encoder(id, type, None, longpress_callback, midi_channel, midi_cc, midiout=midiout) + control = self.add_encoder(id, type, None, longpress_callback, midi_channel, midi_cc, midiout=self.midiout) self.encoders.append(control) except Exception: logging.exception("Failed to create encoder with config: %s" % c) @@ -396,54 +369,41 @@ def __validate_midi_port(self, port_name): return None return port_name - def __apply_midi_routing(self, cfg): - """Apply per-pedalboard MIDI routing overrides for encoders and analog controllers.""" - if cfg is None: + def __resolve_midiout(self, cfg_entry): + """Return the midiout for a control: its external port (opened eagerly) if routed, else virtual.""" + midi_port = Util.DICT_GET(cfg_entry, "midi_port") + if midi_port: + midi_port = self.__validate_midi_port(midi_port) + if not midi_port or self.external_midi is None: + return self.midiout + self.external_midi.open_port(midi_port) # eager (C3): first poll-loop send must not enumerate + return ExternalMidiOut(self.external_midi, midi_port, self.midiout) + + def __route_section(self, cfg, section, controls, set_cc): + cfg_list = Util.DICT_GET(cfg[Token.HARDWARE], section) + if not cfg_list: return + for entry in cfg_list: + ctrl_id = Util.DICT_GET(entry, Token.ID) + if ctrl_id is None: + continue + ctrl = next((c for c in controls if getattr(c, 'id', None) == ctrl_id), None) + if ctrl is None: + continue + # Footswitch midi_CC (incl. NONE removal) is owned by __init_footswitches; only encoders/analog here. + if set_cc: + midi_cc = Util.DICT_GET(entry, Token.MIDI_CC) + if midi_cc is not None and hasattr(ctrl, 'midi_CC'): + ctrl.midi_CC = midi_cc + ctrl.midiout = self.__resolve_midiout(entry) - midi_channel = self.get_real_midi_channel(cfg) - - if Token.HARDWARE in cfg and Token.ENCODERS in cfg[Token.HARDWARE]: - cfg_encoders = cfg[Token.HARDWARE][Token.ENCODERS] - if cfg_encoders: - for enc_cfg in cfg_encoders: - enc_id = Util.DICT_GET(enc_cfg, Token.ID) - if enc_id is None: - continue - encoder = next((e for e in self.encoders if e.id == enc_id), None) - if encoder is None: - continue - midi_port = Util.DICT_GET(enc_cfg, "midi_port") - if midi_port: - midi_port = self.__validate_midi_port(midi_port) - midi_cc = Util.DICT_GET(enc_cfg, Token.MIDI_CC) - if midi_cc is not None and hasattr(encoder, 'midi_CC'): - encoder.midi_CC = midi_cc - if midi_port: - encoder.midiout = ExternalMidiOut(self.external_midi, midi_port, self.midiout) - else: - encoder.midiout = self.midiout - - if Token.HARDWARE in cfg and Token.ANALOG_CONTROLLERS in cfg[Token.HARDWARE]: - cfg_analog = cfg[Token.HARDWARE][Token.ANALOG_CONTROLLERS] - if cfg_analog: - for analog_cfg in cfg_analog: - analog_id = Util.DICT_GET(analog_cfg, Token.ID) - if analog_id is None: - continue - analog = next((a for a in self.analog_controls if a.id == analog_id), None) - if analog is None: - continue - midi_port = Util.DICT_GET(analog_cfg, "midi_port") - if midi_port: - midi_port = self.__validate_midi_port(midi_port) - midi_cc = Util.DICT_GET(analog_cfg, Token.MIDI_CC) - if midi_cc is not None and hasattr(analog, 'midi_CC'): - analog.midi_CC = midi_cc - if midi_port: - analog.midiout = ExternalMidiOut(self.external_midi, midi_port, self.midiout) - else: - analog.midiout = self.midiout + def __apply_midi_routing(self, cfg): + """Route every control to its external port or the virtual port (default + pedalboard cfg).""" + if cfg is None or Token.HARDWARE not in cfg: + return + self.__route_section(cfg, Token.ENCODERS, self.encoders, set_cc=True) + self.__route_section(cfg, Token.ANALOG_CONTROLLERS, self.analog_controls, set_cc=True) + self.__route_section(cfg, Token.FOOTSWITCHES, self.footswitches, set_cc=False) def __init_midi_default(self): self.__init_midi(self.cfg) diff --git a/tests/test_external_midi.py b/tests/test_external_midi.py index 6e31f6ae5..1bd9b5e24 100644 --- a/tests/test_external_midi.py +++ b/tests/test_external_midi.py @@ -228,6 +228,28 @@ def test_open_port_failure_returns_none_without_keyerror(self, monkeypatch): assert "dev" not in mgr.midi_ports +class TestOpenBackoff: + def test_failed_open_backs_off_no_reenumerate(self, fake_ports): + """A port whose device is absent must not re-enumerate on every poll tick (C3).""" + available, created = fake_ports + available[:] = ["something_else"] + mgr = ExternalMidiManager() + mgr.update_config({"enabled": True, "ports": {"c4": {"auto_detect": ["*c4*"]}}}) + + assert mgr._init_port("c4") is None + n = len(created) + assert mgr._init_port("c4") is None + assert len(created) == n # second attempt skipped enumeration + + def test_open_port_eager_returns_bool(self, fake_ports): + available, _ = fake_ports + available[:] = ["dev"] + mgr = ExternalMidiManager() + mgr.update_config({"enabled": True, "ports": {"dev": {"port_index": 0}}}) + assert mgr.open_port("dev") is True + assert "dev" in mgr.midi_ports + + class TestSendRaw: def test_returns_false_when_disabled(self): mgr = ExternalMidiManager() diff --git a/tests/test_hardware.py b/tests/test_hardware.py index 7397cf44d..cf0fdca42 100644 --- a/tests/test_hardware.py +++ b/tests/test_hardware.py @@ -1,8 +1,14 @@ """Unit tests for pistomp.hardware.Hardware helpers.""" import logging +from types import SimpleNamespace +from typing import cast +from unittest.mock import MagicMock -from modalapi.external_midi import ExternalMidiManager +import pytest + +import common.token as Token +from modalapi.external_midi import ExternalMidiManager, ExternalMidiOut from pistomp.hardware import Hardware @@ -15,7 +21,8 @@ def init_footswitches(self): ... def init_relays(self): ... def cleanup(self): ... def test(self): ... - def add_encoder(self, *a, **k): ... + def add_encoder(self, *a, **k): + raise NotImplementedError def _validate(hw, port_name): @@ -45,3 +52,83 @@ def test_uninitialized_external_midi_logs_warning_not_error(self, caplog): recs = [r for r in caplog.records if "dev" in r.getMessage()] assert recs assert all(r.levelno == logging.WARNING for r in recs) + + +@pytest.fixture +def routed_hw(monkeypatch): + """A Hardware with one encoder, analog control, and footswitch, and a 'c4' external port.""" + monkeypatch.setattr("modalapi.external_midi.rtmidi.MidiOut", lambda *a, **k: MagicMock()) + + hw = object.__new__(_StubHardware) + hw.midiout = MagicMock(name="virtual") + hw.external_midi = ExternalMidiManager() + hw.external_midi.update_config({"enabled": True, "ports": {"c4": {"port_index": 0}}}) + + hw.encoders = [SimpleNamespace(id=1, midi_CC=70, midiout=hw.midiout)] + hw.analog_controls = cast(list, [SimpleNamespace(id=2, midi_CC=75, midiout=hw.midiout)]) + hw.footswitches = cast(list, [SimpleNamespace(id=0, midiout=hw.midiout)]) + return hw + + +def _route(hw, cfg): + hw._Hardware__apply_midi_routing(cfg) + + +class TestApplyMidiRouting: + def test_footswitch_routed_to_external_port(self, routed_hw): + """C1: footswitches had no routing branch, so midi_port never took effect.""" + cfg = {Token.HARDWARE: {Token.FOOTSWITCHES: [{Token.ID: 0, "midi_port": "c4"}]}} + _route(routed_hw, cfg) + fs = routed_hw.footswitches[0] + assert isinstance(fs.midiout, ExternalMidiOut) + assert fs.midiout.port_name == "c4" + + def test_encoder_and_analog_routed_to_external_port(self, routed_hw): + cfg = { + Token.HARDWARE: { + Token.ENCODERS: [{Token.ID: 1, "midi_port": "c4"}], + Token.ANALOG_CONTROLLERS: [{Token.ID: 2, "midi_port": "c4"}], + } + } + _route(routed_hw, cfg) + assert isinstance(routed_hw.encoders[0].midiout, ExternalMidiOut) + assert isinstance(routed_hw.analog_controls[0].midiout, ExternalMidiOut) + + def test_encoder_midi_cc_override(self, routed_hw): + cfg = {Token.HARDWARE: {Token.ENCODERS: [{Token.ID: 1, Token.MIDI_CC: 99}]}} + _route(routed_hw, cfg) + assert routed_hw.encoders[0].midi_CC == 99 + + def test_no_midi_port_falls_back_to_virtual(self, routed_hw): + cfg = {Token.HARDWARE: {Token.FOOTSWITCHES: [{Token.ID: 0}]}} + _route(routed_hw, cfg) + assert routed_hw.footswitches[0].midiout is routed_hw.midiout + + def test_external_port_opened_eagerly(self, routed_hw): + """C3: the port is opened at routing time, not lazily inside the poll loop.""" + cfg = {Token.HARDWARE: {Token.FOOTSWITCHES: [{Token.ID: 0, "midi_port": "c4"}]}} + _route(routed_hw, cfg) + assert "c4" in routed_hw.external_midi.midi_ports + + +class TestReinitDefaultRouting: + def test_reinit_applies_routing_for_default_cfg(self, monkeypatch): + """C1: default-config routing was never applied (only pedalboard cfg was).""" + hw = object.__new__(_StubHardware) + hw.default_cfg = {Token.HARDWARE: {}} + hw.handler = MagicMock() + + for name in ( + "_Hardware__init_midi_default", + "_Hardware__init_footswitches", + "_Hardware__init_encoders", + "_Hardware__init_external_midi", + ): + setattr(hw, name, lambda *a, **k: None) + routed = [] + setattr(hw, "_Hardware__apply_midi_routing", lambda cfg: routed.append(cfg)) + monkeypatch.setattr("pistomp.footswitch.Footswitch.init", staticmethod(lambda cb: None)) + + hw.reinit(None) + + assert routed == [hw.cfg] From e6571d733707c60c36d4de225621f6bac32c8fbd Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Sun, 7 Jun 2026 00:45:00 -0400 Subject: [PATCH 14/26] external-midi: show external controllers on v1 (mod.py) (4D) External-routed controls aren't bound to any plugin parameter, so the plugin loop skipped them and they were invisible on v1. Port modhandler's external block to mod.py: bind a synthetic parameter and add an 'External' display entry. Extend the v1 mono draw_analog_assignments to render External entries as port:cc (it previously keyed only on type). Co-Authored-By: Claude Opus 4.8 --- modalapi/mod.py | 14 +++++++ pistomp/lcdgfx.py | 10 +++-- .../0.png | Bin 0 -> 588 bytes tests/test_controller_type.py | 6 ++- tests/test_mod_external_binding.py | 37 ++++++++++++++++++ tests/v1/test_external_midi.py | 15 +++++++ 6 files changed, 78 insertions(+), 4 deletions(-) create mode 100644 tests/snapshots/v1/test_external_midi/test_external_analog_assignments_render/0.png create mode 100644 tests/test_mod_external_binding.py create mode 100644 tests/v1/test_external_midi.py diff --git a/modalapi/mod.py b/modalapi/mod.py index 49e460cfb..8a61a723d 100755 --- a/modalapi/mod.py +++ b/modalapi/mod.py @@ -745,6 +745,20 @@ def bind_current_pedalboard(self): if elem.has_footswitch is False] self.current.pedalboard.plugins += footswitch_plugins + # Externally-routed controllers: bind a synthetic parameter and show under "External" + for controller in self.hardware.controllers.values(): + routing = controller.get_routing_info() + if routing.destination != RoutingDestination.EXTERNAL or controller.midi_CC is None: + continue + controller.parameter = self.hardware.create_external_parameter( + controller, routing.port_name, controller.midi_channel, controller.midi_CC + ) + if isinstance(controller, AnalogMidiControl): + key = f"{controller.midi_channel}:{controller.midi_CC}" + display_info = controller.get_display_info() # already carries type/id + display_info[Token.CATEGORY] = 'External' + self.current.analog_controllers[key] = display_info + def pedalboard_select(self, direction): # 0 means the pedalboard field is selected but a new pedalboard hasn't been scrolled to yet if direction == 0: diff --git a/pistomp/lcdgfx.py b/pistomp/lcdgfx.py index 731d69232..384b93dfd 100755 --- a/pistomp/lcdgfx.py +++ b/pistomp/lcdgfx.py @@ -325,9 +325,13 @@ def draw_analog_assignments(self, controllers): knob = Token.NONE for k, v in controllers.items(): control_type = util.DICT_GET(v, Token.TYPE) - s = k.split(":") - text = "%s:%s" % (self.shorten_name(s[0], self.plugin_width), - self.shorten_name(s[1], self.plugin_width_medium)) + if util.DICT_GET(v, Token.CATEGORY) == 'External': + port = util.DICT_GET(v, 'port_name') or '' + text = "%s:%s" % (self.shorten_name(port, self.plugin_width), util.DICT_GET(v, 'midi_cc')) + else: + s = k.split(":") + text = "%s:%s" % (self.shorten_name(s[0], self.plugin_width), + self.shorten_name(s[1], self.plugin_width_medium)) if control_type == Token.EXPRESSION: exp = text elif control_type == Token.KNOB: diff --git a/tests/snapshots/v1/test_external_midi/test_external_analog_assignments_render/0.png b/tests/snapshots/v1/test_external_midi/test_external_analog_assignments_render/0.png new file mode 100644 index 0000000000000000000000000000000000000000..a6bcb6e5af5bd04b989c2c10b1854ba581dbf88d GIT binary patch literal 588 zcmV-S0<-;zP)5-Hd?;^iP+Pt(+Ddn8~J~r$0 zP_B`$>~cxlGIw#e+T{Y>NfF8qjx^3q(;Aq>Uv|@lAayPrqX@Z2-x(AALx^EptrbUe`Iowu)vBi^>ADGMz2qq)TJ4u#dtNwTi)*R+ z{^t2*YX}2er*hz9HB;Bp?oN}~V_P5pmE7^Fmd_0v!tA{Ew(RTpxGFgj%Fb~#T(rWD zQdO;qYeUd}l=dynq)`UMqTJ|C&CTXQSLJ>~000000000000000000000000000000 zxSilnmc*u!lt(HStW|7EZS0ruQXEmz!nOLz5hEGuE`F2rs_CfmFPj2)OD6D#G4KCY zqs`A|4&pQAo#$0Glb;n3AA(xg3RpjE0yd31IP3cVa3E72yI;YbUrhui;C?uYNJoi1 aiueL6#|J{10E0yU0000 Date: Sun, 7 Jun 2026 01:07:29 -0400 Subject: [PATCH 15/26] 4B reorder --- modalapi/modhandler.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/modalapi/modhandler.py b/modalapi/modhandler.py index afa30fa0a..2193e7842 100755 --- a/modalapi/modhandler.py +++ b/modalapi/modhandler.py @@ -537,9 +537,6 @@ def set_current_pedalboard(self, pedalboard): cfg = yaml.load(ymlfile, Loader=yaml.SafeLoader) self.hardware.reinit(cfg) - # Sync current state of analog controls (expression pedals, etc.) - self.hardware.sync_analog_controls() - # Initialize the data and draw on LCD self.bind_current_pedalboard() self.load_current_presets() @@ -555,6 +552,9 @@ def set_current_pedalboard(self, pedalboard): except Exception as e: logging.warning(f"Failed to send external MIDI messages: {e}") + # Sync analog controls last: after bind + external send, matching mod.py (4B) + self.hardware.sync_analog_controls() + # Prepare blend modes if configured (snapshot-based activation) try: blend_configs = cfg.get('blend_snapshots', []) if cfg else [] From 421af88ce3fd72f4e4d389eb7a1bff8df7238ce1 Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Sun, 7 Jun 2026 01:08:10 -0400 Subject: [PATCH 16/26] Real loopback test --- .../test_external_midi_loopback.py | 132 ++++++++++++++++++ 1 file changed, 132 insertions(+) create mode 100644 tests/integration/test_external_midi_loopback.py diff --git a/tests/integration/test_external_midi_loopback.py b/tests/integration/test_external_midi_loopback.py new file mode 100644 index 000000000..86f97c642 --- /dev/null +++ b/tests/integration/test_external_midi_loopback.py @@ -0,0 +1,132 @@ +""" +Real-device loopback tests for external MIDI routing. +macOS-only for now: virtual ports need CoreMIDI. Linux/CI needs the ALSA sequencer (`sudo modprobe snd-seq-dummy snd-seq`). +""" + +import sys +import time +import uuid +from unittest.mock import MagicMock + +import pytest + +from modalapi.external_midi import ExternalMidiManager, ExternalMidiOut + +pytestmark = pytest.mark.skipif( + sys.platform != "darwin", + reason="virtual MIDI ports require CoreMIDI (macOS); Linux/CI needs ALSA seq modules", +) + + +def _wait_for(predicate, timeout=1.0): + """Poll predicate until true or timeout — MIDI delivery is asynchronous.""" + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + if predicate(): + return True + time.sleep(0.005) + return predicate() + + +@pytest.fixture +def loopback(): + """Open a real virtual MIDI-in port; yield (port_name, received_messages). + + Each test gets a uniquely-named port so async callbacks can't cross-talk. + """ + import rtmidi + + port_name = f"pistomp-loopback-{uuid.uuid4().hex[:8]}" + midi_in = rtmidi.MidiIn() + midi_in.open_virtual_port(port_name) + received: list[list[int]] = [] + + def _on_message(event, data): + message, _delta = event + received.append(message) + + midi_in.set_callback(_on_message) + time.sleep(0.05) # let the port register before MidiOut enumerates + try: + yield port_name, received + finally: + midi_in.close_port() + del midi_in + + +def _manager_for(port_name, glob=None): + """Enabled manager with one port auto-detected by name glob.""" + mgr = ExternalMidiManager() + mgr.update_config( + { + "enabled": True, + "send_delay_ms": 0, + "ports": {"dev": {"auto_detect": [glob or f"*{port_name}*"]}}, + } + ) + return mgr + + +class TestRealLoopback: + def test_send_raw_reaches_real_device(self, loopback): + port_name, received = loopback + mgr = _manager_for(port_name) + + assert mgr.send_raw("dev", [0xB0, 75, 42]) is True + assert _wait_for(lambda: received == [[0xB0, 75, 42]]) + mgr.close() + + def test_external_midi_out_prefers_real_port_over_fallback(self, loopback): + port_name, received = loopback + mgr = _manager_for(port_name) + fallback = MagicMock() + out = ExternalMidiOut(mgr, "dev", fallback) + + out.send_message([0xB0, 70, 7]) + + assert _wait_for(lambda: received == [[0xB0, 70, 7]]) + fallback.send_message.assert_not_called() + mgr.close() + + def test_external_midi_out_falls_back_when_device_absent(self, loopback): + # Glob matches nothing → real enumeration finds no port → fallback used. + _, received = loopback + mgr = _manager_for("unused", glob="*no-such-pistomp-port*") + fallback = MagicMock() + out = ExternalMidiOut(mgr, "dev", fallback) + + out.send_message([0xB0, 70, 7]) + + fallback.send_message.assert_called_once_with([0xB0, 70, 7]) + assert received == [] + mgr.close() + + def test_send_messages_for_pedalboard_delivers_sequence(self, loopback): + port_name, received = loopback + mgr = _manager_for(port_name) + mgr.messages = {"dev": [[0xC0, 5], [0xB0, 7, 100]]} + + assert mgr.send_messages_for_pedalboard() is True + + assert _wait_for(lambda: received == [[0xC0, 5], [0xB0, 7, 100]]) + mgr.close() + + def test_absent_device_backs_off_no_per_send_reenumerate(self, loopback, monkeypatch): + # C3 end-to-end: an absent device must not re-enumerate on every send. + _, _received = loopback + mgr = _manager_for("unused", glob="*no-such-pistomp-port*") + + enumerations = [] + real_enumerate = mgr._get_available_ports + + def counting_enumerate(): + enumerations.append(1) + return real_enumerate() + + monkeypatch.setattr(mgr, "_get_available_ports", counting_enumerate) + + assert mgr.send_raw("dev", [0xB0, 1, 1]) is False + assert mgr.send_raw("dev", [0xB0, 1, 2]) is False # within backoff window + + assert len(enumerations) == 1 # second send short-circuited on backoff + mgr.close() From 212ba8f7ffeba42f4d898373de2ca956f84fa847 Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Sun, 7 Jun 2026 01:36:57 -0400 Subject: [PATCH 17/26] Removed dead code; added more loopback tests --- modalapi/external_midi.py | 46 +------------ modalapi/modhandler.py | 10 ++- pistomp/analogcontrol.py | 10 +-- pistomp/analogmidicontrol.py | 5 -- pistomp/analogswitch.py | 12 +--- pistomp/controller.py | 4 +- pistomp/hardware.py | 7 +- pistomp/pistomptre.py | 4 +- .../test_external_midi_loopback.py | 65 +++++++++++++++++++ tests/test_external_midi.py | 26 -------- 10 files changed, 81 insertions(+), 108 deletions(-) diff --git a/modalapi/external_midi.py b/modalapi/external_midi.py index a2808072b..d7e9e4074 100644 --- a/modalapi/external_midi.py +++ b/modalapi/external_midi.py @@ -226,14 +226,6 @@ def _validate_midi_message(self, message: MidiMessage) -> bool: return True def _send_messages(self, port_name: str, messages: list[MidiMessage], delay_ms: int = 10): - """ - Send MIDI messages to a port. - - Args: - port_name: Name of port configuration. - messages: List of MIDI messages to send. - delay_ms: Delay between messages in milliseconds. - """ midi_out = self._init_port(port_name) if midi_out is None: logging.warning(f"Skipping messages for unavailable port: {port_name}") @@ -258,11 +250,7 @@ def _send_messages(self, port_name: str, messages: list[MidiMessage], delay_ms: break # Stop sending remaining messages to this broken port def send_raw(self, port_name: str, message: MidiMessage) -> bool: - """ - Send a raw MIDI message to an external port. - Same contract as send_cc: enabled gate, lazy port discovery, invalidation on failure. - Returns True on success, False if port unavailable (caller should fallback). - """ + """Send one raw message; True on success, False if the port is unavailable (caller falls back).""" if not self.enabled: return False @@ -282,38 +270,8 @@ def send_raw(self, port_name: str, message: MidiMessage) -> bool: self._invalidate_port(port_name) return False - def send_cc(self, port_name: str, channel: int, cc: int, value: int) -> bool: - """ - Send a single MIDI CC message to an external port. - Convenience wrapper around send_raw that constructs the CC message. - - Args: - port_name: Name of port configuration from config file. - channel: MIDI channel (0-15). - cc: MIDI CC number (0-127). - value: MIDI CC value (0-127). - - Returns: - True if message was sent successfully, False if port unavailable (caller should fallback). - """ - if not (0 <= channel <= 15): - raise ValueError(f"Invalid MIDI channel {channel} (must be 0-15)") - if not (0 <= cc <= 127): - raise ValueError(f"Invalid MIDI CC {cc} (must be 0-127)") - if not (0 <= value <= 127): - raise ValueError(f"Invalid MIDI value {value} (must be 0-127)") - - status = 0xB0 | channel - return self.send_raw(port_name, [status, cc, value]) - def send_messages_for_pedalboard(self) -> bool: - """ - Send external MIDI messages for current pedalboard configuration. - Configuration should have been set via update_config() before calling this. - - Returns: - True if messages were sent successfully, False otherwise. - """ + """Send the current pedalboard's external messages (config set earlier via update_config).""" if not self.enabled: return False diff --git a/modalapi/modhandler.py b/modalapi/modhandler.py index 2193e7842..33727ba98 100755 --- a/modalapi/modhandler.py +++ b/modalapi/modhandler.py @@ -24,8 +24,6 @@ import subprocess import sys import yaml -from typing import Any - from typing import cast, Any import common.token as Token @@ -35,7 +33,7 @@ import modalapi.external_midi as ExternalMidi from modalapi.external_midi import EXTERNAL_INSTANCE_ID from pistomp.lcd320x240 import Lcd -from pistomp.hardware import Controller, Hardware +from pistomp.hardware import Hardware import pistomp.settings as Settings from blend.snapshot import SnapshotManager from modalapi.websocket_bridge import AsyncWebSocketBridge @@ -926,7 +924,7 @@ def user_backup_data(self, arg): logging.info("Data backup...") cmd = os.path.join(self.homedir, 'util', 'data-backup.sh') try: - output = subprocess.check_output([cmd, os.path.join(self.backup_dir, self.backup_file), self.data_dir]) + subprocess.check_output([cmd, os.path.join(self.backup_dir, self.backup_file), self.data_dir]) self.lcd.draw_message_dialog("Backup complete", "Info") logging.info("Backup complete") except subprocess.CalledProcessError as e: @@ -945,8 +943,8 @@ def user_restore_data(self, arg): logging.info("Restoring data backup...") cmd = os.path.join(self.homedir, 'util', 'data-restore.sh') try: - output = subprocess.check_output(['sudo', '-u', self.username, cmd, - os.path.join(self.backup_dir, self.backup_file), self.data_dir]) + subprocess.check_output(['sudo', '-u', self.username, cmd, + os.path.join(self.backup_dir, self.backup_file), self.data_dir]) logging.info("Restore complete") self.system_menu_restart_sound(None) except subprocess.CalledProcessError as e: diff --git a/pistomp/analogcontrol.py b/pistomp/analogcontrol.py index bd4a57d02..c5f853a81 100755 --- a/pistomp/analogcontrol.py +++ b/pistomp/analogcontrol.py @@ -17,14 +17,12 @@ class AnalogControl: - def __init__(self, spi, adc_channel, tolerance): - self.spi = spi self.adc_channel = adc_channel - self.last_read = 0 # this keeps track of the last potentiometer value + self.last_read = 0 # this keeps track of the last potentiometer value self.tolerance = tolerance # to keep from being jittery we'll only change the - # value when the control has moved a significant amount + # value when the control has moved a significant amount def readChannel(self): adc = self.spi.xfer2([1, (8 + self.adc_channel) << 4, 0]) @@ -34,7 +32,3 @@ def readChannel(self): def refresh(self): """Read current value from hardware and potentially take action.""" logging.error("AnalogControl subclass hasn't overriden the refresh method") - - def initialize(self): - """Called when the pedalboard has been loaded, e.g. to sync current value.""" - logging.error("AnalogControl subclass hasn't implemented initialize method") \ No newline at end of file diff --git a/pistomp/analogmidicontrol.py b/pistomp/analogmidicontrol.py index 7623f88e4..d31f6b0b6 100755 --- a/pistomp/analogmidicontrol.py +++ b/pistomp/analogmidicontrol.py @@ -79,11 +79,6 @@ def send_current_value(self): value = self._clamp_endpoints(self.readChannel()) self._send_value(value) - def initialize(self): - if not self.autosync: - return - self.send_current_value() - def refresh(self): value = self._clamp_endpoints(self.readChannel()) if abs(value - self.last_read) > self.tolerance: diff --git a/pistomp/analogswitch.py b/pistomp/analogswitch.py index 501fd6b92..27db4d340 100755 --- a/pistomp/analogswitch.py +++ b/pistomp/analogswitch.py @@ -20,14 +20,14 @@ import pistomp.switchstate as switchstate from pistomp.taptempo import TapTempo -LONG_PRESS_TIME = 0.5 # Hold seconds which defines a long press +LONG_PRESS_TIME = 0.5 # Hold seconds which defines a long press FALLING_THRESHOLD = 800 # ASSUMES 10-bit ADC, can be changed for debounce handling -class AnalogSwitch(analogcontrol.AnalogControl): +class AnalogSwitch(analogcontrol.AnalogControl): def __init__(self, spi, adc_channel, tolerance, callback, taptempo: TapTempo | None = None): super(AnalogSwitch, self).__init__(spi, adc_channel, tolerance) - #self.value = None # this keeps track of the last value, do we still need this? + # self.value = None # this keeps track of the last value, do we still need this? self.callback = callback self.state = switchstate.Value.RELEASED self.start_time = 0 @@ -59,9 +59,3 @@ def refresh(self): self.callback(switchstate.Value.RELEASED) elif self.state is switchstate.Value.LONGPRESSED: self.state = switchstate.Value.RELEASED - - @override - def initialize(self): - # no-op for stateless switches - pass - diff --git a/pistomp/controller.py b/pistomp/controller.py index 9aa362120..843204237 100755 --- a/pistomp/controller.py +++ b/pistomp/controller.py @@ -43,8 +43,8 @@ def external(cls, port_name: str) -> "RoutingInfo": class AnalogDisplayInfo(TypedDict, total=False): - type: str # Token.KNOB, Token.EXPRESSION, Token.VOLUME - id: int # Position on screen (0-based from left) + type: str | None # Token.KNOB, Token.EXPRESSION, Token.VOLUME + id: int | None # Position on screen (0-based from left); None if unpositioned category: str | None port_name: str | None # External port name if routed externally midi_cc: int | None # MIDI CC for external routing display diff --git a/pistomp/hardware.py b/pistomp/hardware.py index 0a47ff34f..9368fcc6a 100755 --- a/pistomp/hardware.py +++ b/pistomp/hardware.py @@ -297,7 +297,7 @@ def create_analog_controls(self, cfg): (adc_input, midi_channel, midi_cc)) @abstractmethod - def add_encoder(self, id, type, callback, longpress_callback, midi_channel, midi_cc, shortpress_config=None, midiout=None) -> Encoder.Encoder | EncoderMidiControl.EncoderMidiControl: + def add_encoder(self, id, type, callback, longpress_callback, midi_channel, midi_cc, midiout=None) -> Encoder.Encoder | EncoderMidiControl.EncoderMidiControl: # This should be implemented by hardware subclasses that support tweak encoders (Tre at least) ... @@ -425,11 +425,6 @@ def __init_external_midi(self, cfg): if ext_cfg: self.external_midi.update_config(ext_cfg) - def __init_footswitches_default(self): - for fs in self.footswitches: - fs.clear_relays() - self.__init_footswitches(self.cfg) - def __init_footswitches(self, cfg): if cfg is None or (Token.HARDWARE not in cfg) or (Token.FOOTSWITCHES not in cfg[Token.HARDWARE]): return diff --git a/pistomp/pistomptre.py b/pistomp/pistomptre.py index e78b545d5..f2e0e1dd9 100755 --- a/pistomp/pistomptre.py +++ b/pistomp/pistomptre.py @@ -69,7 +69,7 @@ def __init__(self, cfg, handler, midiout, refresh_callback): try: self.ledstrip = Ledstrip.Ledstrip() - except Exception as e: + except Exception: self.ledstrip = None logging.error("Could not initialize LED Strip") @@ -90,7 +90,7 @@ def __init__(self, cfg, handler, midiout, refresh_callback): def init_lcd(self): self.handler.add_lcd(Lcd.Lcd(self.handler.homedir, self.handler, flip=False)) - def add_encoder(self, id, type, callback, longpress_callback, midi_channel, midi_cc, shortpress_config=None, midiout=None): + def add_encoder(self, id, type, callback, longpress_callback, midi_channel, midi_cc, midiout=None): enc_pins = Util.DICT_GET(ENC, id) if enc_pins is None: raise ValueError("Cannot create encoder object for id:", id) diff --git a/tests/integration/test_external_midi_loopback.py b/tests/integration/test_external_midi_loopback.py index 86f97c642..2bae19ab8 100644 --- a/tests/integration/test_external_midi_loopback.py +++ b/tests/integration/test_external_midi_loopback.py @@ -10,7 +10,12 @@ import pytest +import common.token as Token +import pistomp.switchstate as switchstate from modalapi.external_midi import ExternalMidiManager, ExternalMidiOut +from pistomp.analogmidicontrol import AnalogMidiControl +from pistomp.encodermidicontrol import EncoderMidiControl +from pistomp.footswitch import Footswitch pytestmark = pytest.mark.skipif( sys.platform != "darwin", @@ -130,3 +135,63 @@ def counting_enumerate(): assert len(enumerations) == 1 # second send short-circuited on backoff mgr.close() + + +class TestControlRoutesToRealPort: + """End-to-end: a real control routed via ExternalMidiOut emits framed CC bytes on the wire. + + Each builds the actual control with its midiout set to an external wrapper (as + __apply_midi_routing does), drives the action method directly, and asserts the + bytes land on the virtual port. Covers every externally-routable control: + footswitch press, tweak-encoder rotation, expression/knob movement. + """ + + def _routed(self, port_name): + mgr = _manager_for(port_name) + fallback = MagicMock() + return mgr, ExternalMidiOut(mgr, "dev", fallback), fallback + + def test_footswitch_press_reaches_real_port(self, loopback): + port_name, received = loopback + mgr, out, fallback = self._routed(port_name) + fs = Footswitch(1, None, None, midi_CC=61, midi_channel=0, midiout=out, refresh_callback=MagicMock()) + + fs.pressed(switchstate.Value.RELEASED) # short press → CC 127 (toggled on) + + assert _wait_for(lambda: received == [[0xB0, 61, 127]]) + fallback.send_message.assert_not_called() + mgr.close() + + def test_tweak_encoder_rotation_reaches_real_port(self, loopback): + port_name, received = loopback + mgr, out, fallback = self._routed(port_name) + enc = EncoderMidiControl( + MagicMock(), + d_pin=None, + clk_pin=None, + callback=None, + midi_CC=70, + midi_channel=0, + midiout=out, + type=Token.KNOB, + id=1, + ) + + enc.refresh(1) # one detent clockwise → midi_value 0 + per_click(8) + + assert _wait_for(lambda: received == [[0xB0, 70, 8]]) + fallback.send_message.assert_not_called() + mgr.close() + + def test_expression_movement_reaches_real_port(self, loopback): + port_name, received = loopback + mgr, out, fallback = self._routed(port_name) + exp = AnalogMidiControl( + MagicMock(), 0, 16, midi_CC=75, midi_channel=0, midiout=out, type=Token.EXPRESSION, id=4 + ) + + exp._send_value(1023) # full travel → MIDI 127 + + assert _wait_for(lambda: received == [[0xB0, 75, 127]]) + fallback.send_message.assert_not_called() + mgr.close() diff --git a/tests/test_external_midi.py b/tests/test_external_midi.py index 1bd9b5e24..eb740b081 100644 --- a/tests/test_external_midi.py +++ b/tests/test_external_midi.py @@ -5,7 +5,6 @@ - configuring external MIDI ports via auto-detect / explicit index - sending pedalboard load messages with inter-message delay - per-pedalboard config overrides (incremental update_config) - - send_cc with graceful fallback when an external port is unavailable - ExternalMidiOut wrapper preferring external port, falling back to virtual """ @@ -294,31 +293,6 @@ def test_send_failure_invalidates_port(self, fake_ports): assert "dev" not in mgr.midi_ports -class TestSendCC: - def test_returns_false_when_disabled(self): - mgr = ExternalMidiManager() - mgr.update_config({"ports": {"dev": {"port_index": 0}}}) - assert mgr.send_cc("dev", 0, 10, 64) is False - - @pytest.mark.parametrize( - "channel,cc,value", - [(-1, 10, 64), (16, 10, 64), (0, -1, 64), (0, 128, 64), (0, 10, -1), (0, 10, 128)], - ) - def test_invalid_midi_args_raise(self, channel, cc, value): - mgr = ExternalMidiManager() - mgr.update_config({"enabled": True, "ports": {"dev": {"port_index": 0}}}) - with pytest.raises(ValueError): - mgr.send_cc("dev", channel, cc, value) - - def test_delegates_to_send_raw(self, fake_ports): - available, _ = fake_ports - available[:] = ["dev"] - mgr = ExternalMidiManager() - mgr.update_config({"enabled": True, "ports": {"dev": {"port_index": 0}}}) - assert mgr.send_cc("dev", 2, 80, 100) is True - _port(mgr, "dev").send_message.assert_called_once_with([0xB2, 80, 100]) - - class TestClose: def test_close_closes_all_ports(self, fake_ports): available, _ = fake_ports From d578bd4d875442dc44be2d40421130bd9f473b75 Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Sun, 7 Jun 2026 01:42:24 -0400 Subject: [PATCH 18/26] midi_port sends to ext midi "instead of" virtual --- pistomp/config.py | 59 +++++++++++++++++++ setup/config_templates/default_config.yml | 49 +++++++++++++++ .../default_config_pistomptre.yml | 9 ++- 3 files changed, 114 insertions(+), 3 deletions(-) diff --git a/pistomp/config.py b/pistomp/config.py index c0ed1f620..7a20afc81 100644 --- a/pistomp/config.py +++ b/pistomp/config.py @@ -92,6 +92,10 @@ "midi_CC": { "type": "integer" }, + "midi_port": { + "type": "string", + "description": "Send MIDI to this external port instead of the virtual MIDI Through port; falls back to virtual if the device is unavailable (must match a port in external_midi)" + }, "preset": { "oneOf": [ { @@ -121,9 +125,16 @@ "adc_input": { "type": "integer" }, + "id": { + "type": "integer" + }, "midi_CC": { "type": "integer" }, + "midi_port": { + "type": "string", + "description": "Send MIDI to this external port instead of the virtual MIDI Through port; falls back to virtual if the device is unavailable (must match a port in external_midi)" + }, "threshold": { "type": "integer", "minimum": 0, @@ -154,6 +165,10 @@ "midi_CC": { "type": "integer" }, + "midi_port": { + "type": "string", + "description": "Send MIDI to this external port instead of the virtual MIDI Through port; falls back to virtual if the device is unavailable (must match a port in external_midi)" + }, "type": { "enum": ["KNOB", "VOLUME"] }, @@ -165,6 +180,50 @@ "id" ] } + }, + "external_midi": { + "type": "object", + "properties": { + "enabled": { + "type": "boolean" + }, + "send_delay_ms": { + "type": "integer", + "minimum": 0 + }, + "ports": { + "type": "object", + "additionalProperties": { + "type": "object", + "properties": { + "auto_detect": { + "type": "array", + "items": { + "type": "string" + } + }, + "port_index": { + "type": "integer", + "minimum": 0 + } + } + } + }, + "messages": { + "type": "object", + "additionalProperties": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "integer", + "minimum": 0, + "maximum": 255 + } + } + } + } + } } }, "required": [ diff --git a/setup/config_templates/default_config.yml b/setup/config_templates/default_config.yml index 37050956b..fc7d0ed90 100755 --- a/setup/config_templates/default_config.yml +++ b/setup/config_templates/default_config.yml @@ -23,6 +23,8 @@ hardware: # adc_input: The analog input to which the switch is connected (required) # ledstrip_position: The position of the corresponding LED (optional) # midi_CC: The MIDI CC message to be sent when switch is clicked (optional) + # midi_port: Send MIDI to this external port INSTEAD of the virtual MIDI Through port (optional) + # Falls back to the virtual port only if the device is unavailable; must match a port in external_midi # longpress: The name of a handler method to call when switch is long-pressed (optional) # longpress can be a list enclosed with []'s # @@ -55,6 +57,8 @@ hardware: # id: The id and position on the screen (starting with 0 on the left) # type: The control type, used to represent the control on the screen (optional) # midi_CC: The MIDI CC message to be sent when the control is adjusted (optional) + # midi_port: Send MIDI to this external port INSTEAD of the virtual MIDI Through port (optional) + # Falls back to the virtual port only if the device is unavailable; must match a port in external_midi # autosync: Whether to send current value on pedalboard load (optional, default: false) # #analog_controllers: @@ -70,6 +74,8 @@ hardware: # type: The control type (default is KNOB, VOLUME controls output volume) # midi_CC: The MIDI CC message to be sent when the control is adjusted (optional) # cannot be used along with type=VOLUME + # midi_port: Send MIDI to this external port INSTEAD of the virtual MIDI Through port (optional) + # Falls back to the virtual port only if the device is unavailable; must match a port in external_midi # longpress: The name of a handler method to call when switch is long-pressed (optional) # encoders: @@ -82,6 +88,49 @@ hardware: - id: 3 type: VOLUME + # external_midi: + # External MIDI device synchronization - sends messages when pedalboards load + # Can be overridden per-pedalboard by creating .pedalboard/config.yml + # + # enabled: Enable/disable external MIDI (default: false) + # send_delay_ms: Delay between consecutive messages in milliseconds (default: 10) + # + # ports: Define external MIDI devices + # : + # auto_detect: [] List of glob patterns to match port names (case-insensitive) + # port_index: Manual port index (overrides auto_detect) + # + # messages: MIDI messages to send for each port on pedalboard load + # : + # - [0xSS, 0xDD, ...] List of MIDI messages (hex bytes) + # + # Example configuration: + # + # external_midi: + # enabled: true + # send_delay_ms: 10 + # ports: + # c4: + # auto_detect: ["*C4*", "*Source Audio*"] + # hx_stomp: + # auto_detect: ["*HX Stomp*"] + # messages: + # c4: + # - [0xB0, 0x66, 0x00] # CC 102 = 0 (bypass) + # hx_stomp: + # - [0xC0, 0x00] # Program Change 0 + # + # MIDI message formats: + # Program Change: [0xCn, program] where n = channel (0-F) + # Control Change: [0xBn, cc, value] where n = channel (0-F) + # + # Per-pedalboard override example (.pedalboard/config.yml): + # hardware: + # external_midi: + # messages: + # c4: + # - [0xC0, 0x05] # Program Change 5 for this pedalboard only + # Blend Mode Configuration (Pedalboard-specific) # This feature interpolates between snapshots based on analog input position. # IMPORTANT: This should be configured per-pedalboard in .pedalboard/config.yml diff --git a/setup/config_templates/default_config_pistomptre.yml b/setup/config_templates/default_config_pistomptre.yml index ebb591f50..8721660ef 100644 --- a/setup/config_templates/default_config_pistomptre.yml +++ b/setup/config_templates/default_config_pistomptre.yml @@ -23,7 +23,8 @@ hardware: # adc_input: The analog input to which the switch is connected (required) # ledstrip_position: The position of the corresponding LED (optional) # midi_CC: The MIDI CC message to be sent when switch is clicked (optional) - # midi_port: Route MIDI to an external device port (optional, must match a port in external_midi) + # midi_port: Send MIDI to this external port INSTEAD of the virtual MIDI Through port (optional) + # Falls back to the virtual port only if the device is unavailable; must match a port in external_midi # longpress: The name of a handler method to call when switch is long-pressed (optional) # longpress can be a list enclosed with []'s # @@ -56,7 +57,8 @@ hardware: # id: The id and position on the screen (starting with 0 on the left) # type: The control type, used to represent the control on the screen (optional) # midi_CC: The MIDI CC message to be sent when the control is adjusted (optional) - # midi_port: Route MIDI to an external device port (optional, must match a port in external_midi) + # midi_port: Send MIDI to this external port INSTEAD of the virtual MIDI Through port (optional) + # Falls back to the virtual port only if the device is unavailable; must match a port in external_midi # autosync: Whether to send current value on pedalboard load (optional, default: false) # #analog_controllers: @@ -72,7 +74,8 @@ hardware: # type: The control type (default is KNOB, VOLUME controls output volume) # midi_CC: The MIDI CC message to be sent when the control is adjusted (optional) # cannot be used along with type=VOLUME - # midi_port: Route MIDI to an external device port (optional, must match a port in external_midi) + # midi_port: Send MIDI to this external port INSTEAD of the virtual MIDI Through port (optional) + # Falls back to the virtual port only if the device is unavailable; must match a port in external_midi # longpress: The name of a handler method to call when switch is long-pressed (optional) # encoders: From d51d8e85d071030106e73824a8c99f0f9a81d7d3 Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Sun, 7 Jun 2026 01:47:38 -0400 Subject: [PATCH 19/26] Config schema test + drop dead shortpress docs S1 regression net: assert all shipped templates validate and the external_midi/midi_port surface is accepted (rejects non-string midi_port). Remove the never-implemented encoder shortpress_config feature from the guide; encoder buttons are UI-nav/longpress only. Co-Authored-By: Claude Opus 4.8 --- GUIDE.md | 23 +++++---------- tests/test_config_schema.py | 58 +++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 15 deletions(-) create mode 100644 tests/test_config_schema.py diff --git a/GUIDE.md b/GUIDE.md index 4de24a917..65628397a 100644 --- a/GUIDE.md +++ b/GUIDE.md @@ -132,33 +132,26 @@ JACK (bridges via `-X seq`) - ✅ Expression Pedal (CC 75) - sends to virtual port - ✅ Footswitches (CC 60-63) - send to virtual port when pressed - ✅ Rotary Encoder Rotation (Tweak1=CC70, Tweak2=CC71) - send to virtual port -- ✅ Encoder Button Presses - Can optionally send MIDI via `shortpress` config (see below) +- ❌ Encoder Button Presses - UI navigation / handler actions only; do not send MIDI -### Encoder Button Configuration (v3 only) +### Encoder Button Behavior (v3) -Encoder buttons support configurable `shortpress` callbacks: +Encoder button presses are wired to handler callbacks, not MIDI. A short press +invokes the built-in click handler (`universal_encoder_sw`, UI navigation); a +long press invokes the named `longpress` callback from config. ```yaml encoders: - id: 1 - midi_CC: 70 # Rotation + midi_CC: 70 # rotation sends CC 70 longpress: previous_snapshot - # shortpress omitted — defaults to the built-in encoder click handler - - - id: 2 - midi_CC: 71 - shortpress: - callback: send_midi_cc - args: {cc: 72} # Button sends CC 72 to virtual port ``` -Shortpress accepts string (callback name) or object with `callback` and `args` (expanded as kwargs). - #### Implementation Details | Control | Shortpress | Longpress | |---------|------------|-----------| -| Encoder | String or `{callback, args}` via `encoderconfig.parse_shortpress_config()` | String only (no args) | +| Encoder | Built-in click handler (UI nav) - no config | String (callback name) - no args | | Footswitch | Hardcoded (toggle/MIDI) - no config | String or list (group names) - no args | | `GpioSwitch` | `callback_arg` (dict→kwargs, value→arg, None) | `longpress_callback_arg` (dict→kwargs, value→arg, None) | | `AnalogSwitch` | Single `callback(state)` - no separate longpress | Same callback, state=LONGPRESSED | @@ -364,7 +357,7 @@ GET /get_bpm # Get current BPM **Encoders** (`pistomp/encoder.py`, `pistomp/encodermidicontrol.py`): - **Base**: Quadrature decoding, GPIO interrupts, debounce - **MIDI Control**: Sends CC on rotation (v3 tweak encoders) -- **Buttons**: Configurable shortpress (callback + args) and longpress +- **Buttons**: Built-in shortpress (UI nav) and configurable longpress (callback name) - **State Machines** (v1/v2 only): `TopEncoderMode`, `BotEncoderMode`, `UniversalEncoderMode` **Analog Controls** (`pistomp/analogmidicontrol.py`): diff --git a/tests/test_config_schema.py b/tests/test_config_schema.py new file mode 100644 index 000000000..fd95b74de --- /dev/null +++ b/tests/test_config_schema.py @@ -0,0 +1,58 @@ +"""Schema regression net for pistomp/config.py (S1: external_midi / midi_port). + +Guards two things: every shipped template stays schema-valid, and the +external-MIDI routing surface (per-control `midi_port` + the `external_midi` +block) is both accepted when well-formed and rejected when malformed. +""" + +import glob + +import pytest +import yaml +from jsonschema import Draft4Validator, exceptions, validate + +from pistomp.config import schema + +TEMPLATES = sorted(glob.glob("setup/config_templates/default_config*.yml")) + + +def test_schema_is_well_formed(): + Draft4Validator.check_schema(schema) + + +@pytest.mark.parametrize("path", TEMPLATES, ids=lambda p: p.rsplit("/", 1)[-1]) +def test_shipped_template_validates(path): + with open(path) as fh: + cfg = yaml.safe_load(fh) + validate(instance=cfg, schema=schema) + + +def test_midi_port_and_external_midi_accepted(): + cfg = { + "hardware": { + "version": 3.0, + "midi": {"channel": 14}, + "footswitches": [{"id": 0, "midi_CC": 60, "midi_port": "c4"}], + "analog_controllers": [{"adc_input": 5, "id": 0, "midi_CC": 75, "midi_port": "hx", "type": "EXPRESSION"}], + "encoders": [{"id": 1, "midi_CC": 70, "midi_port": "c4"}], + "external_midi": { + "enabled": True, + "send_delay_ms": 10, + "ports": {"c4": {"auto_detect": ["*C4*"]}, "hx": {"port_index": 2}}, + "messages": {"c4": [[0xB0, 0x66, 0x00]], "hx": [[0xC0, 0x00]]}, + }, + } + } + validate(instance=cfg, schema=schema) + + +def test_non_string_midi_port_rejected(): + cfg = { + "hardware": { + "version": 3.0, + "midi": {"channel": 14}, + "encoders": [{"id": 1, "midi_port": 5}], + } + } + with pytest.raises(exceptions.ValidationError): + validate(instance=cfg, schema=schema) From 60255d49593e6a81210c81f960d84b57b036c0f0 Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Sun, 7 Jun 2026 01:51:07 -0400 Subject: [PATCH 20/26] Strip plan-marker references from comments Remove C1-3 / 4A-G / 4D markers from code and test comments; history belongs in source control, not the source. Wording kept, markers dropped. Co-Authored-By: Claude Opus 4.8 --- modalapi/external_midi.py | 6 +++--- modalapi/modhandler.py | 2 +- pistomp/hardware.py | 8 ++++---- tests/integration/test_external_midi_loopback.py | 2 +- tests/test_external_midi.py | 4 ++-- tests/test_hardware.py | 6 +++--- tests/test_mod_external_binding.py | 2 +- tests/v1/test_external_midi.py | 2 +- 8 files changed, 16 insertions(+), 16 deletions(-) diff --git a/modalapi/external_midi.py b/modalapi/external_midi.py index d7e9e4074..80688c74d 100644 --- a/modalapi/external_midi.py +++ b/modalapi/external_midi.py @@ -27,7 +27,7 @@ EXTERNAL_INSTANCE_ID = "External" -PORT_RETRY_BACKOFF_S = 5.0 # don't re-enumerate a failed port more often than this (C3) +PORT_RETRY_BACKOFF_S = 5.0 # don't re-enumerate a failed port more often than this class ExternalMidiOut: @@ -159,7 +159,7 @@ def _find_port_by_name(self, port_config: PortConfig) -> int | None: return selected_idx def open_port(self, port_name: str) -> bool: - """Eagerly open a port at routing time so the first poll-loop send doesn't enumerate (C3).""" + """Eagerly open a port at routing time so the first poll-loop send doesn't enumerate.""" return self._init_port(port_name) is not None def _init_port(self, port_name: str) -> rtmidi.MidiOut | None: @@ -206,7 +206,7 @@ def _invalidate_port(self, port_name: str) -> None: except Exception as e: logging.debug(f"Error closing invalidated port {port_name}: {e}") del self.midi_ports[port_name] - self._open_failures[port_name] = time.monotonic() # back off before re-enumerating (C3) + self._open_failures[port_name] = time.monotonic() # back off before re-enumerating def _validate_midi_message(self, message: MidiMessage) -> bool: if not isinstance(message, list) or len(message) < 2: diff --git a/modalapi/modhandler.py b/modalapi/modhandler.py index 33727ba98..3f874fc78 100755 --- a/modalapi/modhandler.py +++ b/modalapi/modhandler.py @@ -550,7 +550,7 @@ def set_current_pedalboard(self, pedalboard): except Exception as e: logging.warning(f"Failed to send external MIDI messages: {e}") - # Sync analog controls last: after bind + external send, matching mod.py (4B) + # Sync analog controls last: after bind + external send, matching mod.py self.hardware.sync_analog_controls() # Prepare blend modes if configured (snapshot-based activation) diff --git a/pistomp/hardware.py b/pistomp/hardware.py index 9368fcc6a..e3bb7fb13 100755 --- a/pistomp/hardware.py +++ b/pistomp/hardware.py @@ -234,7 +234,7 @@ def create_footswitches(self, cfg): if taptempo: taptempo.set_callback(self.handler.get_callback(tap_tempo_callback)) - # midi_port routing is applied later in __apply_midi_routing (external_midi is None here — C1) + # midi_port routing is applied later in __apply_midi_routing (external_midi is None here) midiout = self.midiout fs: Footswitch.Footswitch | None = None @@ -287,7 +287,7 @@ def create_analog_controls(self, cfg): if autosync is None: autosync = False # Default to False - # midi_port routing is applied later in __apply_midi_routing (external_midi is None here — C1) + # midi_port routing is applied later in __apply_midi_routing (external_midi is None here) control = AnalogMidiControl.AnalogMidiControl(self.spi, adc_input, threshold, midi_cc, midi_channel, self.midiout, control_type, id, c, autosync) self.analog_controls.append(control) @@ -322,7 +322,7 @@ def create_encoders(self, cfg): logging.error("Config file error. Encoder specified without %s" % Token.ID) continue - # midi_port routing is applied later in __apply_midi_routing (external_midi is None here — C1) + # midi_port routing is applied later in __apply_midi_routing (external_midi is None here) try: control = self.add_encoder(id, type, None, longpress_callback, midi_channel, midi_cc, midiout=self.midiout) self.encoders.append(control) @@ -376,7 +376,7 @@ def __resolve_midiout(self, cfg_entry): midi_port = self.__validate_midi_port(midi_port) if not midi_port or self.external_midi is None: return self.midiout - self.external_midi.open_port(midi_port) # eager (C3): first poll-loop send must not enumerate + self.external_midi.open_port(midi_port) # eager: first poll-loop send must not enumerate return ExternalMidiOut(self.external_midi, midi_port, self.midiout) def __route_section(self, cfg, section, controls, set_cc): diff --git a/tests/integration/test_external_midi_loopback.py b/tests/integration/test_external_midi_loopback.py index 2bae19ab8..72242afed 100644 --- a/tests/integration/test_external_midi_loopback.py +++ b/tests/integration/test_external_midi_loopback.py @@ -117,7 +117,7 @@ def test_send_messages_for_pedalboard_delivers_sequence(self, loopback): mgr.close() def test_absent_device_backs_off_no_per_send_reenumerate(self, loopback, monkeypatch): - # C3 end-to-end: an absent device must not re-enumerate on every send. + # End-to-end: an absent device must not re-enumerate on every send. _, _received = loopback mgr = _manager_for("unused", glob="*no-such-pistomp-port*") diff --git a/tests/test_external_midi.py b/tests/test_external_midi.py index eb740b081..9230d5f80 100644 --- a/tests/test_external_midi.py +++ b/tests/test_external_midi.py @@ -215,7 +215,7 @@ def test_failing_port_invalidated_and_stops(self, fake_ports): class TestInitPort: def test_open_port_failure_returns_none_without_keyerror(self, monkeypatch): - """A failing open_port must not KeyError on a cache entry that was never set.""" + """A failing open_port must not KeyError when the port is absent from the cache.""" failing = MagicMock() failing.open_port.side_effect = RuntimeError("cannot open") monkeypatch.setattr("modalapi.external_midi.rtmidi.MidiOut", lambda *a, **k: failing) @@ -229,7 +229,7 @@ def test_open_port_failure_returns_none_without_keyerror(self, monkeypatch): class TestOpenBackoff: def test_failed_open_backs_off_no_reenumerate(self, fake_ports): - """A port whose device is absent must not re-enumerate on every poll tick (C3).""" + """A port whose device is absent must not re-enumerate on every poll tick.""" available, created = fake_ports available[:] = ["something_else"] mgr = ExternalMidiManager() diff --git a/tests/test_hardware.py b/tests/test_hardware.py index cf0fdca42..d46d42268 100644 --- a/tests/test_hardware.py +++ b/tests/test_hardware.py @@ -76,7 +76,7 @@ def _route(hw, cfg): class TestApplyMidiRouting: def test_footswitch_routed_to_external_port(self, routed_hw): - """C1: footswitches had no routing branch, so midi_port never took effect.""" + """A footswitch with midi_port routes to its external port.""" cfg = {Token.HARDWARE: {Token.FOOTSWITCHES: [{Token.ID: 0, "midi_port": "c4"}]}} _route(routed_hw, cfg) fs = routed_hw.footswitches[0] @@ -105,7 +105,7 @@ def test_no_midi_port_falls_back_to_virtual(self, routed_hw): assert routed_hw.footswitches[0].midiout is routed_hw.midiout def test_external_port_opened_eagerly(self, routed_hw): - """C3: the port is opened at routing time, not lazily inside the poll loop.""" + """The external port is opened at routing time, not lazily inside the poll loop.""" cfg = {Token.HARDWARE: {Token.FOOTSWITCHES: [{Token.ID: 0, "midi_port": "c4"}]}} _route(routed_hw, cfg) assert "c4" in routed_hw.external_midi.midi_ports @@ -113,7 +113,7 @@ def test_external_port_opened_eagerly(self, routed_hw): class TestReinitDefaultRouting: def test_reinit_applies_routing_for_default_cfg(self, monkeypatch): - """C1: default-config routing was never applied (only pedalboard cfg was).""" + """Routing is applied for the default config, not only for pedalboard cfg.""" hw = object.__new__(_StubHardware) hw.default_cfg = {Token.HARDWARE: {}} hw.handler = MagicMock() diff --git a/tests/test_mod_external_binding.py b/tests/test_mod_external_binding.py index e33e4b285..aab5f8d68 100644 --- a/tests/test_mod_external_binding.py +++ b/tests/test_mod_external_binding.py @@ -1,4 +1,4 @@ -"""4D: external controllers must be bound + displayed on v1 (mod.py), as on v3. +"""External controllers must be bound + displayed on v1 (mod.py), as on v3. An externally-routed control isn't bound to any plugin parameter, so the plugin loop skips it. Without the dedicated external block it stays invisible: no diff --git a/tests/v1/test_external_midi.py b/tests/v1/test_external_midi.py index 89f9681ab..500d4c3a5 100644 --- a/tests/v1/test_external_midi.py +++ b/tests/v1/test_external_midi.py @@ -1,4 +1,4 @@ -"""4D: external controllers must render in the v1 mono analog-assignments zone.""" +"""External controllers must render in the v1 mono analog-assignments zone.""" import common.token as Token From 70be97cfbf7d973207f5355da115426c4e1c6a81 Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Sun, 7 Jun 2026 02:30:53 -0400 Subject: [PATCH 21/26] Characterizaiton tests for ext midi --- tests/v1/test_binding.py | 69 ++++++++++++++++++++++++++++++++++++++++ tests/v3/test_plugins.py | 27 ++++++++++++++++ 2 files changed, 96 insertions(+) create mode 100644 tests/v1/test_binding.py diff --git a/tests/v1/test_binding.py b/tests/v1/test_binding.py new file mode 100644 index 000000000..8f73de1fe --- /dev/null +++ b/tests/v1/test_binding.py @@ -0,0 +1,69 @@ +"""Characterization of Mod.bind_current_pedalboard() (v1). + +Pins behavior that differs from the v3 (modhandler) twin so a future shared +controller-manager extraction can't silently flatten the asymmetry: + + - v1 reorders the plugin chain so footswitch-controlled plugins sit last. + - v1 populates analog_controllers from an AnalogMidiControl's cfg dict, + stamping CATEGORY + TYPE (but, unlike v3, no ID). +""" + +import common.token as Token +from pistomp.analogmidicontrol import AnalogMidiControl +from pistomp.footswitch import Footswitch + + +def _key_of(hw, predicate): + return next(k for k, v in hw.controllers.items() if predicate(v)) + + +def test_v1_bind_footswitch_and_analog(v1_system, make_plugin): + handler = v1_system.handler + hw = v1_system.hw + + fs_key = _key_of(hw, lambda c: isinstance(c, Footswitch)) + knob_key = _key_of(hw, lambda c: isinstance(c, AnalogMidiControl) and c.type == Token.KNOB) + fs = hw.controllers[fs_key] + knob = hw.controllers[knob_key] + + fuzz = make_plugin("fuzz", category="Distortion") + fuzz.parameters[":bypass"].binding = fs_key + tone = make_plugin("tone", category="Filter") + tone.parameters[":bypass"].binding = knob_key + + handler.current.pedalboard.plugins = [fuzz, tone] + handler.bind_current_pedalboard() + + # Footswitch bound to its plugin's bypass param; plugin flagged. + assert fs.parameter is fuzz.parameters[":bypass"] + assert fuzz.has_footswitch is True + + # Analog control surfaced in the LCD assignment dict with category + type. + analog_entries = [ + cfg for cfg in handler.current.analog_controllers.values() + if cfg.get(Token.TYPE) == Token.KNOB + ] + assert len(analog_entries) == 1 + entry = analog_entries[0] + assert entry[Token.CATEGORY] == "Filter" + # v1 does NOT stamp an ID onto the analog cfg (v3 does) — pin the difference. + assert Token.ID not in entry + + +def test_v1_bind_reorders_footswitch_plugins_to_end(v1_system, make_plugin): + """v1 moves footswitch-controlled plugins to the tail of the chain.""" + handler = v1_system.handler + hw = v1_system.hw + + fs_key = _key_of(hw, lambda c: isinstance(c, Footswitch)) + + fuzz = make_plugin("fuzz") # footswitch-controlled + fuzz.parameters[":bypass"].binding = fs_key + reverb = make_plugin("reverb") # no controller binding + + # Footswitch plugin deliberately placed first. + handler.current.pedalboard.plugins = [fuzz, reverb] + handler.bind_current_pedalboard() + + titles = [p.instance_id for p in handler.current.pedalboard.plugins] + assert titles == ["reverb", "fuzz"], "footswitch plugin must be reordered to the end" diff --git a/tests/v3/test_plugins.py b/tests/v3/test_plugins.py index d027be87b..7c34f960d 100644 --- a/tests/v3/test_plugins.py +++ b/tests/v3/test_plugins.py @@ -5,6 +5,7 @@ import pistomp.switchstate as switchstate from pistomp.encodermidicontrol import EncoderMidiControl +from pistomp.footswitch import Footswitch from common.parameter import Parameter from modalapi.plugin import Plugin import common.token as Token @@ -72,6 +73,31 @@ def test_v3_bind_volume_encoder_populates_analog_controllers(v3_system: SystemFi assert Token.VOLUME in handler.current.analog_controllers +def test_v3_bind_does_not_reorder_footswitch_plugins(v3_system: SystemFixture, make_plugin): + """v3 (modhandler) leaves the plugin chain order untouched. + + Counterpart to v1's reorder-to-end behavior — pins the asymmetry so a shared + controller-manager extraction must preserve it rather than unify it. + """ + handler = v3_system.handler + hw = v3_system.hw + + fs_key = next(k for k, v in hw.controllers.items() if isinstance(v, Footswitch)) + + fuzz = make_plugin("fuzz") # footswitch-controlled, placed first + fuzz.parameters[":bypass"].binding = fs_key + reverb = make_plugin("reverb") # no controller binding + + assert handler.current + handler.current.pedalboard.plugins = [fuzz, reverb] + handler.bind_current_pedalboard() + + assert hw.controllers[fs_key].parameter is fuzz.parameters[":bypass"] + assert fuzz.has_footswitch is True + titles = [p.instance_id for p in handler.current.pedalboard.plugins] + assert titles == ["fuzz", "reverb"], "v3 must not reorder footswitch plugins" + + # --------------------------------------------------------------------------- # Plugin bypass # --------------------------------------------------------------------------- @@ -97,6 +123,7 @@ def test_v3_toggle_plugin_bypass_via_footswitch_sends_midi_cc(v3_system: SystemF handler.toggle_plugin_bypass(None, plugin) + assert isinstance(fs.midiout, MagicMock) fs.midiout.send_message.assert_called_once() sent_cc = fs.midiout.send_message.call_args[0][0] assert sent_cc[1] == fs.midi_CC From 10d8d49b919f8251cdb4d5fc82333cbb86882f07 Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Sun, 7 Jun 2026 11:15:31 -0400 Subject: [PATCH 22/26] Extract current and controller management out of mod/modhandler --- modalapi/mod.py | 87 ++--------------- modalapi/modhandler.py | 112 ++------------------- pistomp/controller.py | 5 + pistomp/controller_manager.py | 151 +++++++++++++++++++++++++++++ pistomp/current.py | 31 ++++++ tests/test_controller_manager.py | 70 +++++++++++++ tests/test_controller_type.py | 46 --------- tests/test_handler_cleanup.py | 6 +- tests/test_mod_external_binding.py | 37 ------- 9 files changed, 275 insertions(+), 270 deletions(-) create mode 100644 pistomp/controller_manager.py create mode 100644 pistomp/current.py create mode 100644 tests/test_controller_manager.py delete mode 100644 tests/test_controller_type.py delete mode 100644 tests/test_mod_external_binding.py diff --git a/modalapi/mod.py b/modalapi/mod.py index 8a61a723d..6d1a6b0f3 100755 --- a/modalapi/mod.py +++ b/modalapi/mod.py @@ -35,8 +35,8 @@ from modalapi.ws_protocol import parse_message, LoadingEndMessage, PedalSnapshotMessage, PluginBypassMessage, WebSocketMessage from modalapi.pedalboard_monitor import FileChangeMonitor, read_pedalboard_bundle -from pistomp.analogmidicontrol import AnalogMidiControl -from pistomp.controller import RoutingDestination +from pistomp.controller_manager import ControllerManager +from pistomp.current import Current from pistomp.footswitch import Footswitch from pistomp.handler import Handler from enum import Enum @@ -136,7 +136,7 @@ def __init__(self, audiocard, homedir): self.software_version = None self.git_describe = None - self.current = None # pointer to Current class + self.current: Current | None = None self.deep = None # pointer to current Deep class # Stores snapshot index from loading_end until pedalboard change is detected @@ -180,29 +180,19 @@ def __del__(self): logging.info("Handler cleanup") if self.wifi_manager: del self.wifi_manager - if self.ws_bridge is not None: - self.ws_bridge.stop() + self.ws_bridge.stop() def cleanup(self): if self.lcd is not None: self.lcd.cleanup() if self.external_midi is not None: self.external_midi.close() - if self.ws_bridge is not None: - self.ws_bridge.stop() - logging.info("WebSocket bridge stopped") + self.ws_bridge.stop() # Container for dynamic data which is unique to the "current" pedalboard # The self.current pointed above will point to this object which gets # replaced when a different pedalboard is made current (old Current object # gets deleted and a new one added via self.set_current_pedalboard() - class Current: - def __init__(self, pedalboard): - self.pedalboard = pedalboard - self.presets = {} - self.preset_index = 0 # Assumes pedalboard loads at snapshot 0 (default behavior) - self.analog_controllers = {} # { type: (plugin_name, param_name) } - class Deep: def __init__(self, plugin): self.plugin = plugin @@ -218,6 +208,7 @@ def __init__(self, plugin): def add_hardware(self, hardware): self.hardware = hardware hardware.external_midi = self.external_midi + self._controller_manager = ControllerManager(hardware, reorder_footswitch_plugins=True) def add_lcd(self, lcd): self.lcd = lcd @@ -601,7 +592,7 @@ def set_current_pedalboard(self, pedalboard): del self.current # Create a new "current" - self.current = self.Current(pedalboard) + self.current = Current(pedalboard) if self.next_pedalboard_preset_index is not None: self.current.preset_index = self.next_pedalboard_preset_index @@ -695,69 +686,7 @@ def bind_current_pedalboard(self): # "current" being the pedalboard mod-host says is current # The pedalboard data has already been loaded, but this will overlay # any real time settings - - # Clear previous parameter bindings from all controllers except the volume control - for controller in self.hardware.controllers.values(): - if controller.type != Token.VOLUME: - controller.parameter = None - - # Clear analog controllers display data - self.current.analog_controllers = {} - - footswitch_plugins = [] - if self.current.pedalboard: - #logging.debug(self.current.pedalboard.to_json()) - for plugin in self.current.pedalboard.plugins: - if plugin is None or plugin.parameters is None: - continue - for sym, param in plugin.parameters.items(): - if param.binding is not None: - controller = self.hardware.controllers.get(param.binding) - if controller is not None: - routing = controller.get_routing_info() - - # External controllers shouldn't be bound to plugin parameters - if routing.destination == RoutingDestination.EXTERNAL: - logging.warning( - f"Plugin parameter {plugin.name}:{param.name} is bound to external controller " - f"{param.binding} (routed to {routing.port_name}) - ignoring plugin binding" - ) - continue - - # TODO possibly use a setter instead of accessing var directly - # What if multiple params could map to the same controller? - controller.parameter = param - controller.set_value(param.value) - plugin.controllers.append(controller) - if isinstance(controller, Footswitch): - # TODO sort this list so selection orders correctly (sort on midi_CC?) - plugin.has_footswitch = True - footswitch_plugins.append(plugin) - controller.set_category(plugin.category) - elif isinstance(controller, AnalogMidiControl): - key = "%s:%s" % (plugin.instance_id, param.name) - controller.cfg[Token.CATEGORY] = plugin.category # somewhat LAME adding to cfg dict - controller.cfg[Token.TYPE] = controller.type - self.current.analog_controllers[key] = controller.cfg - - # Move Footswitch controlled plugins to the end of the list - self.current.pedalboard.plugins = [elem for elem in self.current.pedalboard.plugins - if elem.has_footswitch is False] - self.current.pedalboard.plugins += footswitch_plugins - - # Externally-routed controllers: bind a synthetic parameter and show under "External" - for controller in self.hardware.controllers.values(): - routing = controller.get_routing_info() - if routing.destination != RoutingDestination.EXTERNAL or controller.midi_CC is None: - continue - controller.parameter = self.hardware.create_external_parameter( - controller, routing.port_name, controller.midi_channel, controller.midi_CC - ) - if isinstance(controller, AnalogMidiControl): - key = f"{controller.midi_channel}:{controller.midi_CC}" - display_info = controller.get_display_info() # already carries type/id - display_info[Token.CATEGORY] = 'External' - self.current.analog_controllers[key] = display_info + self._controller_manager.bind(self.current) def pedalboard_select(self, direction): # 0 means the pedalboard field is selected but a new pedalboard hasn't been scrolled to yet diff --git a/modalapi/modhandler.py b/modalapi/modhandler.py index 3f874fc78..109822b08 100755 --- a/modalapi/modhandler.py +++ b/modalapi/modhandler.py @@ -40,9 +40,8 @@ from modalapi.ws_protocol import parse_message, LoadingEndMessage, PedalSnapshotMessage, PluginBypassMessage, WebSocketMessage from modalapi.pedalboard_monitor import FileChangeMonitor, read_pedalboard_bundle -from pistomp.analogmidicontrol import AnalogMidiControl -from pistomp.controller import RoutingDestination -from pistomp.encodermidicontrol import EncoderMidiControl +from pistomp.controller_manager import ControllerManager +from pistomp.current import Current from pistomp.footswitch import Footswitch from pistomp.tuner import TunerEngine, TunerPanel, TunerSourceFactory from pistomp.tuner.source import AudioSource, build_source @@ -80,7 +79,7 @@ def __init__(self, audiocard: Audiocard, homedir, data_dir="/home/pistomp/data") self.bypass_left = False self.bypass_right = False - self.current: Modhandler.Current | None = None + self.current: Current | None = None self._lcd: Lcd | None = None self._hardware: Hardware | None = None @@ -157,20 +156,7 @@ def cleanup(self): self._hardware.cleanup() if self.external_midi is not None: self.external_midi.close() - if self.ws_bridge is not None: - self.ws_bridge.stop() - logging.info("WebSocket bridge stopped") - - # Container for dynamic data which is unique to the "current" pedalboard - # The self.current pointed above will point to this object which gets - # replaced when a different pedalboard is made current (old Current object - # gets deleted and a new one added via self.set_current_pedalboard() - class Current: - def __init__(self, pedalboard: Pedalboard.Pedalboard): - self.pedalboard: Pedalboard.Pedalboard = pedalboard - self.presets: dict[int, str] = {} - self.preset_index: int = 0 # Assumes pedalboard loads at snapshot 0 (default behavior) - self.analog_controllers: dict[str, dict[str, Any]] = {} # { type: (plugin_name, param_name) } + self.ws_bridge.stop() def _rest_get(self, url: str) -> Response | None: try: @@ -189,6 +175,7 @@ def _rest_post(self, url: str, *, json=None, data=None) -> Response | None: def add_hardware(self, hardware): self._hardware = hardware hardware.external_midi = self.external_midi + self._controller_manager = ControllerManager(hardware, stamp_plugin_cfg_id=True) def add_lcd(self, lcd): self._lcd = lcd @@ -521,7 +508,7 @@ def set_current_pedalboard(self, pedalboard): del self.current # Create a new "current" - self.current = self.Current(pedalboard) + self.current = Current(pedalboard) if self.next_pedalboard_preset_index is not None: self.current.preset_index = self.next_pedalboard_preset_index @@ -596,92 +583,7 @@ def bind_current_pedalboard(self): # "current" being the pedalboard mod-host says is current # The pedalboard data has already been loaded, but this will overlay # any real time settings - - # Clear previous parameter bindings from all controllers except the volume control - for controller in self.hardware.controllers.values(): - if controller.type != Token.VOLUME: - controller.parameter = None - - # Clear analog controllers display data - self.current.analog_controllers = {} - - footswitch_plugins = [] - if self.current: - #logging.debug(self.current.pedalboard.to_json()) - for plugin in self.current.pedalboard.plugins: - if plugin is None or plugin.parameters is None: - continue - for sym, param in plugin.parameters.items(): - if param.binding is not None: - controller = self.hardware.controllers.get(param.binding) - if controller is not None: - routing = controller.get_routing_info() - - # External controllers shouldn't be bound to plugin parameters - if routing.destination == RoutingDestination.EXTERNAL: - logging.warning( - f"Plugin parameter {plugin.name}:{param.name} is bound to external controller " - f"{param.binding} (routed to {routing.port_name}) - ignoring plugin binding" - ) - continue - - # TODO possibly use a setter instead of accessing var directly - # What if multiple params could map to the same controller? - controller.parameter = param # pyright: ignore[reportAttributeAccessIssue] - controller.set_value(param.value) - plugin.controllers.append(controller) - if isinstance(controller, Footswitch): - # TODO sort this list so selection orders correctly (sort on midi_CC?) - plugin.has_footswitch = True - footswitch_plugins.append(plugin) - controller.set_category(plugin.category) - elif isinstance(controller, AnalogMidiControl): - key = "%s:%s" % (plugin.instance_id, param.name) - controller.cfg[Token.CATEGORY] = plugin.category # somewhat LAME adding to cfg dict - controller.cfg[Token.TYPE] = controller.type - controller.cfg[Token.ID] = controller.id - self.current.analog_controllers[key] = controller.cfg - elif isinstance(controller, EncoderMidiControl): - key = "%s:%s" % (plugin.instance_id, param.name) - controller.cfg[Token.CATEGORY] = plugin.category # somewhat LAME adding to cfg dict - controller.cfg[Token.TYPE] = controller.type - controller.cfg[Token.ID] = controller.id - self.current.analog_controllers[key] = controller.cfg - - # Special case for volume control - for e in self.hardware.encoders: - if e.type == Token.VOLUME: - cfg = { - Token.CATEGORY: None, - Token.TYPE: e.type, - Token.ID: e.id - } - self.current.analog_controllers[Token.VOLUME] = cfg - - # Handle external controllers (bind synthetic parameter + add to display) - for controller in self.hardware.controllers.values(): - routing = controller.get_routing_info() - if routing.destination == RoutingDestination.EXTERNAL: - if controller.midi_CC is not None: - controller.parameter = self.hardware.create_external_parameter( - controller, routing.port_name, controller.midi_channel, controller.midi_CC - ) - if isinstance(controller, AnalogMidiControl): - if controller.midi_CC is not None: - key = f"{controller.midi_channel}:{controller.midi_CC}" - display_info = controller.get_display_info() - display_info['category'] = 'External' - self.current.analog_controllers[key] = display_info - elif isinstance(controller, EncoderMidiControl): - if controller.midi_CC is not None: - key = f"{controller.midi_channel}:{controller.midi_CC}" - cfg = { - Token.CATEGORY: 'External', - Token.TYPE: controller.type, - Token.ID: controller.id, - **controller.get_display_info() - } - self.current.analog_controllers[key] = cfg + self._controller_manager.bind(self.current) def pedalboard_change(self, pedalboard=None): logging.info("Pedalboard change") diff --git a/pistomp/controller.py b/pistomp/controller.py index 843204237..590fe1336 100755 --- a/pistomp/controller.py +++ b/pistomp/controller.py @@ -50,6 +50,11 @@ class AnalogDisplayInfo(TypedDict, total=False): midi_cc: int | None # MIDI CC for external routing display +# Per-pedalboard analog/encoder assignment display, keyed by "instance:param" +# (plugin-bound), "channel:cc" (external), or Token.VOLUME. +AnalogControllers = dict[str, AnalogDisplayInfo] + + class Controller: type: str | None = None # class default; not in __init__ — clashes with Encoder.type in EncoderController's MRO diff --git a/pistomp/controller_manager.py b/pistomp/controller_manager.py new file mode 100644 index 000000000..43b52bea2 --- /dev/null +++ b/pistomp/controller_manager.py @@ -0,0 +1,151 @@ +# This file is part of pi-stomp. +# +# pi-stomp is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# pi-stomp is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with pi-stomp. If not, see . + +from __future__ import annotations + +import logging +from typing import cast, TYPE_CHECKING + +import common.token as Token +from pistomp.analogmidicontrol import AnalogMidiControl +from pistomp.controller import AnalogDisplayInfo, RoutingDestination +from pistomp.current import Current +from pistomp.encodermidicontrol import EncoderMidiControl +from pistomp.footswitch import Footswitch + +if TYPE_CHECKING: + from pistomp.hardware import Hardware + + +class ControllerManager: + """ + Manages controller/parameter bindings on the current pedalboard, + overlaying per-pedalboard config on top of the base. + Version differences are passed as flags rather than subclassed: + + reorder_footswitch_plugins v1 moves footswitch-controlled plugins to the + tail of the chain; v3 leaves order untouched. + stamp_plugin_cfg_id v3 stamps the controller id onto plugin-bound + analog/encoder cfg entries; v1 does not. + """ + + def __init__( + self, + hardware: "Hardware", + *, + reorder_footswitch_plugins: bool = False, + stamp_plugin_cfg_id: bool = False, + ): + self._hw = hardware + self._reorder_footswitch_plugins = reorder_footswitch_plugins + self._stamp_plugin_cfg_id = stamp_plugin_cfg_id + + def bind(self, current: Current | None) -> None: + """Rebind all controllers for the active pedalboard state.""" + if current is None: + return + + # Clear previous parameter bindings from all controllers except volume. + for controller in self._hw.controllers.values(): + if controller.type != Token.VOLUME: + controller.parameter = None + + current.analog_controllers = {} + + if current.pedalboard: + footswitch_plugins = self._bind_plugin_parameters(current) + self._bind_volume_encoders(current) + if self._reorder_footswitch_plugins: + self._move_footswitch_plugins_to_end(current, footswitch_plugins) + + self._bind_external_controllers(current) + + def _bind_plugin_parameters(self, current) -> list: + """Bind controllers referenced by plugin parameters; return the plugins + that gained a footswitch.""" + footswitch_plugins = [] + for plugin in current.pedalboard.plugins: + if plugin is None or plugin.parameters is None: + continue + for param in plugin.parameters.values(): + if param.binding is None: + continue + controller = self._hw.controllers.get(param.binding) + if controller is None: + continue + + routing = controller.get_routing_info() + # External controllers aren't bound to plugin parameters. + if routing.destination == RoutingDestination.EXTERNAL: + logging.warning( + f"Plugin parameter {plugin.name}:{param.name} is bound to external controller " + f"{param.binding} (routed to {routing.port_name}) - ignoring plugin binding" + ) + continue + + controller.parameter = param + controller.set_value(param.value) + plugin.controllers.append(controller) + + if isinstance(controller, Footswitch): + plugin.has_footswitch = True + footswitch_plugins.append(plugin) + controller.set_category(plugin.category) + elif isinstance(controller, (AnalogMidiControl, EncoderMidiControl)): + key = "%s:%s" % (plugin.instance_id, param.name) + controller.cfg[Token.CATEGORY] = plugin.category # somewhat LAME adding to cfg dict + controller.cfg[Token.TYPE] = controller.type + if self._stamp_plugin_cfg_id: + controller.cfg[Token.ID] = controller.id + current.analog_controllers[key] = cast(AnalogDisplayInfo, controller.cfg) + return footswitch_plugins + + def _bind_volume_encoders(self, current) -> None: + """Surface VOLUME-type encoders in the assignment display (v3 only in + practice — v1 has no VOLUME-typed encoder).""" + for e in self._hw.encoders: + if e.type == Token.VOLUME: + entry: AnalogDisplayInfo = {"category": None, "type": e.type, "id": e.id} + current.analog_controllers[Token.VOLUME] = entry + + @staticmethod + def _move_footswitch_plugins_to_end(current, footswitch_plugins) -> None: + plugins = current.pedalboard.plugins + current.pedalboard.plugins = [p for p in plugins if p.has_footswitch is False] + footswitch_plugins + + def _bind_external_controllers(self, current) -> None: + """Externally-routed controllers: bind a synthetic parameter and show + them under an "External" category.""" + for controller in self._hw.controllers.values(): + routing = controller.get_routing_info() + if routing.destination != RoutingDestination.EXTERNAL or controller.midi_CC is None: + continue + + controller.parameter = self._hw.create_external_parameter( + controller, routing.port_name, controller.midi_channel, controller.midi_CC + ) + + if not isinstance(controller, (AnalogMidiControl, EncoderMidiControl)): + continue + key = f"{controller.midi_channel}:{controller.midi_CC}" + # AnalogMidiControl.get_display_info() already carries type/id; the + # encoder's does not, so seed them and let the dict override. + entry: AnalogDisplayInfo = { + "type": controller.type, + "id": controller.id, + **controller.get_display_info(), + "category": "External", + } + current.analog_controllers[key] = entry diff --git a/pistomp/current.py b/pistomp/current.py new file mode 100644 index 000000000..dd448e16c --- /dev/null +++ b/pistomp/current.py @@ -0,0 +1,31 @@ +# This file is part of pi-stomp. +# +# pi-stomp is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# pi-stomp is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with pi-stomp. If not, see . + +from __future__ import annotations + +from dataclasses import dataclass, field + +from pistomp.controller import AnalogControllers +from modalapi.pedalboard import Pedalboard + + +@dataclass +class Current: + """Mutable per-pedalboard state for the active ("current") pedalboard.""" + + pedalboard: Pedalboard + presets: dict[int, str] = field(default_factory=dict) + preset_index: int = 0 # Assumes pedalboard loads at snapshot 0 (default behavior) + analog_controllers: AnalogControllers = field(default_factory=dict) diff --git a/tests/test_controller_manager.py b/tests/test_controller_manager.py new file mode 100644 index 000000000..c0e47a517 --- /dev/null +++ b/tests/test_controller_manager.py @@ -0,0 +1,70 @@ +"""ControllerManager.bind() — controller→parameter binding, version-flagged.""" + +from unittest.mock import MagicMock + +import common.token as Token +from modalapi.external_midi import ExternalMidiOut +from pistomp.analogmidicontrol import AnalogMidiControl +from pistomp.controller import RoutingInfo +from pistomp.controller_manager import ControllerManager +from pistomp.current import Current + + +def _make_current() -> Current: + """A Current with an empty (truthy) pedalboard — no plugin bindings.""" + current = Current(MagicMock()) + current.pedalboard.plugins = [] + return current + + +class _Ctl: + """Minimal virtual-routed controller — v1 config supplies no VOLUME control + to use directly.""" + + def __init__(self, type): + self.type = type + self.parameter = "bound" + self.midi_CC = None + + def get_routing_info(self): + return RoutingInfo.virtual() + + +def test_bind_preserves_volume_binding_clears_others(): + """Controller.type is a class-level default, so the volume guard is type-safe: + bind() clears every controller's parameter except the VOLUME control's.""" + vol = _Ctl(Token.VOLUME) + knob = _Ctl(Token.KNOB) + hw = MagicMock() + hw.controllers = {"0:7": vol, "0:8": knob} + hw.encoders = [] + + current = _make_current() + ControllerManager(hw).bind(current) + + assert vol.parameter == "bound" + assert knob.parameter is None + + +def _external_analog(midi_cc=75, midi_channel=0, ctrl_id=3): + ext_out = ExternalMidiOut(MagicMock(), "c4", MagicMock()) + return AnalogMidiControl(MagicMock(), 0, 16, midi_cc, midi_channel, ext_out, Token.KNOB, id=ctrl_id, cfg={}) + + +def test_external_controller_bound_and_displayed(): + """An externally-routed control isn't bound to any plugin parameter, so the + plugin loop skips it. The external block binds a synthetic parameter and adds + an "External" display entry — otherwise it'd be invisible on the LCD.""" + ctrl = _external_analog() + hw = MagicMock() + hw.controllers = {"0:75": ctrl} + hw.encoders = [] + hw.create_external_parameter.return_value = "SYNTH_PARAM" + + current = _make_current() + ControllerManager(hw).bind(current) + + assert ctrl.parameter == "SYNTH_PARAM" + entry = current.analog_controllers["0:75"] + assert entry.get("category") == "External" + assert entry.get("port_name") == "c4" diff --git a/tests/test_controller_type.py b/tests/test_controller_type.py deleted file mode 100644 index b5d989eed..000000000 --- a/tests/test_controller_type.py +++ /dev/null @@ -1,46 +0,0 @@ -"""Controller.type is a class-level default so the volume guard is type-safe. - -Controller deliberately doesn't set ``type`` in __init__ (MRO clash with -encoder.Encoder.type in EncoderController). A class-level default lets -``controller.type`` resolve on every Controller — Footswitch included — -without hasattr/getattr. -""" - -from unittest.mock import MagicMock - -import common.token as Token -from modalapi.mod import Mod -from pistomp.controller import Controller, RoutingInfo - - -class _Ctl: - """Stand-in controller — v1 config supplies no VOLUME control to use directly.""" - - def __init__(self, type): - self.type = type - self.parameter = "bound" - self.midi_CC = None - - def get_routing_info(self): - return RoutingInfo.virtual() - - -def test_controller_type_defaults_to_none(): - c = Controller(midi_channel=0, midi_CC=7) - assert c.type is None - - -def test_bind_preserves_volume_binding_clears_others(): - h = object.__new__(Mod) - h.wifi_manager = None - vol = _Ctl(Token.VOLUME) - knob = _Ctl(Token.KNOB) - h.hardware = MagicMock() - h.hardware.controllers = {"0:7": vol, "0:8": knob} - h.current = MagicMock() - h.current.pedalboard.plugins = [] - - h.bind_current_pedalboard() - - assert vol.parameter == "bound" - assert knob.parameter is None diff --git a/tests/test_handler_cleanup.py b/tests/test_handler_cleanup.py index 4667cbbaf..aaf59e98e 100644 --- a/tests/test_handler_cleanup.py +++ b/tests/test_handler_cleanup.py @@ -15,7 +15,7 @@ def test_del_does_not_close_external_midi(self): h = object.__new__(Mod) h.wifi_manager = None h.external_midi = MagicMock() - h.ws_bridge = None + h.ws_bridge = MagicMock() h.__del__() h.external_midi.close.assert_not_called() @@ -24,7 +24,7 @@ def test_cleanup_closes_external_midi(self): h.wifi_manager = None h.lcd = None h.external_midi = MagicMock() - h.ws_bridge = None + h.ws_bridge = MagicMock() h.cleanup() h.external_midi.close.assert_called_once() @@ -44,6 +44,6 @@ def test_cleanup_closes_external_midi(self): h._lcd = None h._hardware = None h.external_midi = MagicMock() - h.ws_bridge = None + h.ws_bridge = MagicMock() h.cleanup() h.external_midi.close.assert_called_once() diff --git a/tests/test_mod_external_binding.py b/tests/test_mod_external_binding.py deleted file mode 100644 index aab5f8d68..000000000 --- a/tests/test_mod_external_binding.py +++ /dev/null @@ -1,37 +0,0 @@ -"""External controllers must be bound + displayed on v1 (mod.py), as on v3. - -An externally-routed control isn't bound to any plugin parameter, so the plugin -loop skips it. Without the dedicated external block it stays invisible: no -synthetic parameter, no entry in analog_controllers for the LCD. -""" - -from unittest.mock import MagicMock - -import common.token as Token -from modalapi.external_midi import ExternalMidiOut -from modalapi.mod import Mod -from pistomp.analogmidicontrol import AnalogMidiControl - - -def _external_analog(midi_cc=75, midi_channel=0, ctrl_id=3): - ext_out = ExternalMidiOut(MagicMock(), "c4", MagicMock()) - return AnalogMidiControl(MagicMock(), 0, 16, midi_cc, midi_channel, ext_out, Token.KNOB, id=ctrl_id, cfg={}) - - -def test_external_controller_bound_and_displayed(): - h = object.__new__(Mod) - h.wifi_manager = None - h.ws_bridge = None - ctrl = _external_analog() - h.hardware = MagicMock() - h.hardware.controllers = {"0:75": ctrl} - h.hardware.create_external_parameter.return_value = "SYNTH_PARAM" - h.current = MagicMock() - h.current.pedalboard.plugins = [] - - h.bind_current_pedalboard() - - assert ctrl.parameter == "SYNTH_PARAM" - entry = h.current.analog_controllers["0:75"] - assert entry["category"] == "External" - assert entry["port_name"] == "c4" From 79d923525befc1e2a597078047b88c7c17823576 Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Sun, 7 Jun 2026 21:42:09 -0400 Subject: [PATCH 23/26] Drop unnecessary ID stamp controller manager config --- modalapi/modhandler.py | 2 +- pistomp/controller_manager.py | 7 +---- .../test_external_midi_loopback.py | 28 +++++++++++++++++-- tests/v1/test_binding.py | 5 ++-- 4 files changed, 30 insertions(+), 12 deletions(-) diff --git a/modalapi/modhandler.py b/modalapi/modhandler.py index 109822b08..745834c3e 100755 --- a/modalapi/modhandler.py +++ b/modalapi/modhandler.py @@ -175,7 +175,7 @@ def _rest_post(self, url: str, *, json=None, data=None) -> Response | None: def add_hardware(self, hardware): self._hardware = hardware hardware.external_midi = self.external_midi - self._controller_manager = ControllerManager(hardware, stamp_plugin_cfg_id=True) + self._controller_manager = ControllerManager(hardware) def add_lcd(self, lcd): self._lcd = lcd diff --git a/pistomp/controller_manager.py b/pistomp/controller_manager.py index 43b52bea2..4f730ee17 100644 --- a/pistomp/controller_manager.py +++ b/pistomp/controller_manager.py @@ -37,8 +37,6 @@ class ControllerManager: reorder_footswitch_plugins v1 moves footswitch-controlled plugins to the tail of the chain; v3 leaves order untouched. - stamp_plugin_cfg_id v3 stamps the controller id onto plugin-bound - analog/encoder cfg entries; v1 does not. """ def __init__( @@ -46,11 +44,9 @@ def __init__( hardware: "Hardware", *, reorder_footswitch_plugins: bool = False, - stamp_plugin_cfg_id: bool = False, ): self._hw = hardware self._reorder_footswitch_plugins = reorder_footswitch_plugins - self._stamp_plugin_cfg_id = stamp_plugin_cfg_id def bind(self, current: Current | None) -> None: """Rebind all controllers for the active pedalboard state.""" @@ -107,8 +103,7 @@ def _bind_plugin_parameters(self, current) -> list: key = "%s:%s" % (plugin.instance_id, param.name) controller.cfg[Token.CATEGORY] = plugin.category # somewhat LAME adding to cfg dict controller.cfg[Token.TYPE] = controller.type - if self._stamp_plugin_cfg_id: - controller.cfg[Token.ID] = controller.id + controller.cfg[Token.ID] = controller.id current.analog_controllers[key] = cast(AnalogDisplayInfo, controller.cfg) return footswitch_plugins diff --git a/tests/integration/test_external_midi_loopback.py b/tests/integration/test_external_midi_loopback.py index 72242afed..c7b4aedfc 100644 --- a/tests/integration/test_external_midi_loopback.py +++ b/tests/integration/test_external_midi_loopback.py @@ -33,11 +33,27 @@ def _wait_for(predicate, timeout=1.0): return predicate() +def _port_is_visible(port_name, timeout=1.0): + """Block until CoreMIDI publishes the virtual port to enumeration.""" + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + import rtmidi + temp = rtmidi.MidiOut() + ports = temp.get_ports() + del temp + if any(port_name in p for p in ports): + return True + time.sleep(0.01) + return False + + @pytest.fixture def loopback(): """Open a real virtual MIDI-in port; yield (port_name, received_messages). Each test gets a uniquely-named port so async callbacks can't cross-talk. + The fixture blocks until CoreMIDI has published the port to enumeration, + eliminating the race between port creation and port discovery. """ import rtmidi @@ -51,7 +67,9 @@ def _on_message(event, data): received.append(message) midi_in.set_callback(_on_message) - time.sleep(0.05) # let the port register before MidiOut enumerates + + assert _port_is_visible(port_name), f"Virtual port {port_name!r} never became visible" + try: yield port_name, received finally: @@ -76,6 +94,7 @@ class TestRealLoopback: def test_send_raw_reaches_real_device(self, loopback): port_name, received = loopback mgr = _manager_for(port_name) + mgr.open_port("dev") assert mgr.send_raw("dev", [0xB0, 75, 42]) is True assert _wait_for(lambda: received == [[0xB0, 75, 42]]) @@ -84,6 +103,7 @@ def test_send_raw_reaches_real_device(self, loopback): def test_external_midi_out_prefers_real_port_over_fallback(self, loopback): port_name, received = loopback mgr = _manager_for(port_name) + mgr.open_port("dev") fallback = MagicMock() out = ExternalMidiOut(mgr, "dev", fallback) @@ -110,6 +130,7 @@ def test_send_messages_for_pedalboard_delivers_sequence(self, loopback): port_name, received = loopback mgr = _manager_for(port_name) mgr.messages = {"dev": [[0xC0, 5], [0xB0, 7, 100]]} + mgr.open_port("dev") assert mgr.send_messages_for_pedalboard() is True @@ -149,7 +170,10 @@ class TestControlRoutesToRealPort: def _routed(self, port_name): mgr = _manager_for(port_name) fallback = MagicMock() - return mgr, ExternalMidiOut(mgr, "dev", fallback), fallback + out = ExternalMidiOut(mgr, "dev", fallback) + # Eagerly open the port so the first send doesn't race enumeration. + mgr.open_port("dev") + return mgr, out, fallback def test_footswitch_press_reaches_real_port(self, loopback): port_name, received = loopback diff --git a/tests/v1/test_binding.py b/tests/v1/test_binding.py index 8f73de1fe..0602d95b9 100644 --- a/tests/v1/test_binding.py +++ b/tests/v1/test_binding.py @@ -5,7 +5,7 @@ - v1 reorders the plugin chain so footswitch-controlled plugins sit last. - v1 populates analog_controllers from an AnalogMidiControl's cfg dict, - stamping CATEGORY + TYPE (but, unlike v3, no ID). + stamping CATEGORY + TYPE + ID. """ import common.token as Token @@ -46,8 +46,7 @@ def test_v1_bind_footswitch_and_analog(v1_system, make_plugin): assert len(analog_entries) == 1 entry = analog_entries[0] assert entry[Token.CATEGORY] == "Filter" - # v1 does NOT stamp an ID onto the analog cfg (v3 does) — pin the difference. - assert Token.ID not in entry + assert Token.ID in entry def test_v1_bind_reorders_footswitch_plugins_to_end(v1_system, make_plugin): From f5110fdbdc8ee696aa05796a31fccf1de7a04ad4 Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Sun, 7 Jun 2026 22:54:26 -0400 Subject: [PATCH 24/26] Greatly simplified device targeting --- modalapi/external_midi.py | 90 +++----------- pistomp/config.py | 26 ++-- pistomp/hardware.py | 6 +- setup/config_templates/default_config.yml | 28 ++--- .../default_config_pistomptre.yml | 30 ++--- .../test_external_midi_loopback.py | 40 +++--- tests/test_config_schema.py | 14 ++- tests/test_external_midi.py | 114 ++++++++---------- tests/test_hardware.py | 40 +++--- 9 files changed, 146 insertions(+), 242 deletions(-) diff --git a/modalapi/external_midi.py b/modalapi/external_midi.py index 80688c74d..05893c85b 100644 --- a/modalapi/external_midi.py +++ b/modalapi/external_midi.py @@ -15,7 +15,6 @@ from __future__ import annotations -import fnmatch import logging import time from typing import TypedDict @@ -53,37 +52,36 @@ def send_message(self, message: MidiMessage) -> None: self.fallback.send_message(message) -class PortConfig(TypedDict, total=False): - auto_detect: list[str] - port_index: int - - class ExternalMidiConfig(TypedDict, total=False): enabled: bool send_delay_ms: int - ports: dict[str, PortConfig] messages: dict[str, list[MidiMessage]] +def _client_name_of(port: str) -> str: + """Extract the ALSA client name from an rtmidi port string like 'Client Name:Port Name N:M'.""" + return port.split(":")[0].strip() + + class ExternalMidiManager: """ Manages external MIDI device synchronization. Sends MIDI messages to external devices when pedalboards are loaded. + + Config keys in `messages` and `midi_port` are ALSA client names exactly as + reported by the device (e.g. 'Source Audio C4 Synth'). Matching is + case-insensitive against the client-name prefix of each rtmidi port string. """ def __init__(self): self.midi_ports: dict[str, rtmidi.MidiOut] = {} - self.port_configs: dict[str, PortConfig] = {} self.messages: dict[str, list[MidiMessage]] = {} self.enabled: bool = False self.send_delay_ms: int = 10 self._open_failures: dict[str, float] = {} def update_config(self, cfg: ExternalMidiConfig | None) -> None: - """ - Update configuration incrementally (can be called multiple times). - Only updates fields that are present. - """ + """Update configuration incrementally; only fields present are updated.""" if cfg is None: return @@ -97,14 +95,6 @@ def update_config(self, cfg: ExternalMidiConfig | None) -> None: if "send_delay_ms" in cfg: self.send_delay_ms = cfg["send_delay_ms"] - if "ports" in cfg: - # Deep merge: overlay individual port fields without replacing entire dicts - for port_name, port_cfg in cfg["ports"].items(): - if port_name in self.port_configs: - self.port_configs[port_name] = {**self.port_configs[port_name], **port_cfg} - else: - self.port_configs[port_name] = port_cfg - if "messages" in cfg: # Merge messages at port level (replace entire message list per port) self.messages.update(cfg["messages"]) @@ -119,44 +109,15 @@ def _get_available_ports(self) -> list[str]: logging.error(f"Failed to enumerate MIDI ports: {e}") return [] - def _find_port_by_name(self, port_config: PortConfig) -> int | None: - """ - Find MIDI port index matching a given config, returning its index if found. - """ - if "port_index" in port_config: - return port_config["port_index"] - - # Auto-detect by name patterns - auto_detect = port_config.get("auto_detect", []) - if not auto_detect: - return None - - available_ports = self._get_available_ports() - if not available_ports: - return None - - # Search for matching ports using glob patterns - matched_ports = [] - for pattern in auto_detect: - for idx, port_name in enumerate(available_ports): - # Case-insensitive glob matching - if fnmatch.fnmatch(port_name.lower(), pattern.lower()): - matched_ports.append((idx, port_name)) - - if not matched_ports: - logging.warning(f"No MIDI ports matched patterns: {auto_detect}") - return None - - # Warn if multiple matches - if len(matched_ports) > 1: - port_names = [name for _, name in matched_ports] - logging.warning( - f"Multiple MIDI ports matched {auto_detect}: {port_names}. Using first match: {matched_ports[0][1]}" - ) - - selected_idx, selected_name = matched_ports[0] - logging.info(f"Auto-detected MIDI port: {selected_name} (index {selected_idx})") - return selected_idx + def _find_port_index(self, device_name: str) -> int | None: + """Return the index of the first port whose ALSA client name matches device_name (case-insensitive).""" + available = self._get_available_ports() + for idx, port in enumerate(available): + if _client_name_of(port).lower() == device_name.lower(): + logging.info(f"Found MIDI port: {port} (index {idx})") + return idx + logging.warning(f"No MIDI port found with device name: {device_name!r}") + return None def open_port(self, port_name: str) -> bool: """Eagerly open a port at routing time so the first poll-loop send doesn't enumerate.""" @@ -170,15 +131,8 @@ def _init_port(self, port_name: str) -> rtmidi.MidiOut | None: if last_fail is not None and (time.monotonic() - last_fail) < PORT_RETRY_BACKOFF_S: return None - port_config = self.port_configs.get(port_name) - if not port_config: - logging.warning(f"No configuration found for MIDI port: {port_name}") - self._open_failures[port_name] = time.monotonic() - return None - - port_idx = self._find_port_by_name(port_config) + port_idx = self._find_port_index(port_name) if port_idx is None: - logging.warning(f"Could not find MIDI port for: {port_name}") self._open_failures[port_name] = time.monotonic() return None @@ -254,10 +208,6 @@ def send_raw(self, port_name: str, message: MidiMessage) -> bool: if not self.enabled: return False - if port_name not in self.port_configs: - logging.error(f"Port '{port_name}' not found in external_midi config, falling back to virtual") - return False - midi_out = self._init_port(port_name) if midi_out is None: return False diff --git a/pistomp/config.py b/pistomp/config.py index 7a20afc81..e587b5fd7 100644 --- a/pistomp/config.py +++ b/pistomp/config.py @@ -167,7 +167,13 @@ }, "midi_port": { "type": "string", - "description": "Send MIDI to this external port instead of the virtual MIDI Through port; falls back to virtual if the device is unavailable (must match a port in external_midi)" + "description": "Send MIDI to this external port instead of the virtual MIDI Through port; falls back to virtual if the device is unavailable (must be the device name)" + }, + "midi_channel": { + "type": "integer", + "minimum": 0, + "maximum": 15, + "description": "Override MIDI channel for this encoder (0-15); useful when the external device is on a different channel than the hardware default" }, "type": { "enum": ["KNOB", "VOLUME"] @@ -191,24 +197,6 @@ "type": "integer", "minimum": 0 }, - "ports": { - "type": "object", - "additionalProperties": { - "type": "object", - "properties": { - "auto_detect": { - "type": "array", - "items": { - "type": "string" - } - }, - "port_index": { - "type": "integer", - "minimum": 0 - } - } - } - }, "messages": { "type": "object", "additionalProperties": { diff --git a/pistomp/hardware.py b/pistomp/hardware.py index e3bb7fb13..276d4a434 100755 --- a/pistomp/hardware.py +++ b/pistomp/hardware.py @@ -364,9 +364,6 @@ def __validate_midi_port(self, port_name): if self.external_midi is None: logging.warning(f"midi_port '{port_name}' set but external_midi not initialized, falling back to virtual") return None - if port_name not in self.external_midi.port_configs: - logging.warning(f"midi_port '{port_name}' not found in external_midi config, falling back to virtual") - return None return port_name def __resolve_midiout(self, cfg_entry): @@ -395,6 +392,9 @@ def __route_section(self, cfg, section, controls, set_cc): midi_cc = Util.DICT_GET(entry, Token.MIDI_CC) if midi_cc is not None and hasattr(ctrl, 'midi_CC'): ctrl.midi_CC = midi_cc + midi_channel = Util.DICT_GET(entry, "midi_channel") + if midi_channel is not None and hasattr(ctrl, 'midi_channel'): + ctrl.midi_channel = midi_channel ctrl.midiout = self.__resolve_midiout(entry) def __apply_midi_routing(self, cfg): diff --git a/setup/config_templates/default_config.yml b/setup/config_templates/default_config.yml index fc7d0ed90..c603c87f6 100755 --- a/setup/config_templates/default_config.yml +++ b/setup/config_templates/default_config.yml @@ -24,7 +24,7 @@ hardware: # ledstrip_position: The position of the corresponding LED (optional) # midi_CC: The MIDI CC message to be sent when switch is clicked (optional) # midi_port: Send MIDI to this external port INSTEAD of the virtual MIDI Through port (optional) - # Falls back to the virtual port only if the device is unavailable; must match a port in external_midi + # Falls back to the virtual port only if the device is unavailable; must be the device name (e.g. 'Source Audio C4 Synth') # longpress: The name of a handler method to call when switch is long-pressed (optional) # longpress can be a list enclosed with []'s # @@ -58,7 +58,7 @@ hardware: # type: The control type, used to represent the control on the screen (optional) # midi_CC: The MIDI CC message to be sent when the control is adjusted (optional) # midi_port: Send MIDI to this external port INSTEAD of the virtual MIDI Through port (optional) - # Falls back to the virtual port only if the device is unavailable; must match a port in external_midi + # Falls back to the virtual port only if the device is unavailable; must be the device name (e.g. 'Source Audio C4 Synth') # autosync: Whether to send current value on pedalboard load (optional, default: false) # #analog_controllers: @@ -75,7 +75,9 @@ hardware: # midi_CC: The MIDI CC message to be sent when the control is adjusted (optional) # cannot be used along with type=VOLUME # midi_port: Send MIDI to this external port INSTEAD of the virtual MIDI Through port (optional) - # Falls back to the virtual port only if the device is unavailable; must match a port in external_midi + # Falls back to the virtual port only if the device is unavailable; must be the device name (e.g. 'Source Audio C4 Synth') + # midi_channel: Override MIDI channel for this encoder (0-15, optional) + # Use when the external device is on a different channel than the hardware default # longpress: The name of a handler method to call when switch is long-pressed (optional) # encoders: @@ -95,13 +97,8 @@ hardware: # enabled: Enable/disable external MIDI (default: false) # send_delay_ms: Delay between consecutive messages in milliseconds (default: 10) # - # ports: Define external MIDI devices - # : - # auto_detect: [] List of glob patterns to match port names (case-insensitive) - # port_index: Manual port index (overrides auto_detect) - # - # messages: MIDI messages to send for each port on pedalboard load - # : + # messages: MIDI messages to send on pedalboard load + # : Device name exactly as reported by the hardware (run `aconnect -l` to find it) # - [0xSS, 0xDD, ...] List of MIDI messages (hex bytes) # # Example configuration: @@ -109,15 +106,10 @@ hardware: # external_midi: # enabled: true # send_delay_ms: 10 - # ports: - # c4: - # auto_detect: ["*C4*", "*Source Audio*"] - # hx_stomp: - # auto_detect: ["*HX Stomp*"] # messages: - # c4: + # Source Audio C4 Synth: # - [0xB0, 0x66, 0x00] # CC 102 = 0 (bypass) - # hx_stomp: + # HX Stomp: # - [0xC0, 0x00] # Program Change 0 # # MIDI message formats: @@ -128,7 +120,7 @@ hardware: # hardware: # external_midi: # messages: - # c4: + # Source Audio C4 Synth: # - [0xC0, 0x05] # Program Change 5 for this pedalboard only # Blend Mode Configuration (Pedalboard-specific) diff --git a/setup/config_templates/default_config_pistomptre.yml b/setup/config_templates/default_config_pistomptre.yml index 8721660ef..62645ada9 100644 --- a/setup/config_templates/default_config_pistomptre.yml +++ b/setup/config_templates/default_config_pistomptre.yml @@ -24,7 +24,9 @@ hardware: # ledstrip_position: The position of the corresponding LED (optional) # midi_CC: The MIDI CC message to be sent when switch is clicked (optional) # midi_port: Send MIDI to this external port INSTEAD of the virtual MIDI Through port (optional) - # Falls back to the virtual port only if the device is unavailable; must match a port in external_midi + # Falls back to the virtual port only if the device is unavailable; must be the device name (e.g. 'Source Audio C4 Synth') + # midi_channel: Override MIDI channel for this encoder (0-15, optional) + # Use when the external device is on a different channel than the hardware default # longpress: The name of a handler method to call when switch is long-pressed (optional) # longpress can be a list enclosed with []'s # @@ -58,7 +60,7 @@ hardware: # type: The control type, used to represent the control on the screen (optional) # midi_CC: The MIDI CC message to be sent when the control is adjusted (optional) # midi_port: Send MIDI to this external port INSTEAD of the virtual MIDI Through port (optional) - # Falls back to the virtual port only if the device is unavailable; must match a port in external_midi + # Falls back to the virtual port only if the device is unavailable; must be the device name (e.g. 'Source Audio C4 Synth') # autosync: Whether to send current value on pedalboard load (optional, default: false) # #analog_controllers: @@ -75,7 +77,9 @@ hardware: # midi_CC: The MIDI CC message to be sent when the control is adjusted (optional) # cannot be used along with type=VOLUME # midi_port: Send MIDI to this external port INSTEAD of the virtual MIDI Through port (optional) - # Falls back to the virtual port only if the device is unavailable; must match a port in external_midi + # Falls back to the virtual port only if the device is unavailable; must be the device name (e.g. 'Source Audio C4 Synth') + # midi_channel: Override MIDI channel for this encoder (0-15, optional) + # Use when the external device is on a different channel than the hardware default # longpress: The name of a handler method to call when switch is long-pressed (optional) # encoders: @@ -95,13 +99,8 @@ hardware: # enabled: Enable/disable external MIDI (default: false) # send_delay_ms: Delay between consecutive messages in milliseconds (default: 10) # - # ports: Define external MIDI devices - # : - # auto_detect: [] List of glob patterns to match port names (case-insensitive) - # port_index: Manual port index (overrides auto_detect) - # - # messages: MIDI messages to send for each port on pedalboard load - # : + # messages: MIDI messages to send on pedalboard load + # : Exact ALSA client name of the device (run `aconnect -l` to find it) # - [0xSS, 0xDD, ...] List of MIDI messages (hex bytes) # # Example configuration: @@ -109,15 +108,10 @@ hardware: # external_midi: # enabled: true # send_delay_ms: 10 - # ports: - # c4: - # auto_detect: ["*C4*", "*Source Audio*"] - # hx_stomp: - # auto_detect: ["*HX Stomp*"] # messages: - # c4: + # Source Audio C4 Synth: # - [0xB0, 0x66, 0x00] # CC 102 = 0 (bypass) - # hx_stomp: + # HX Stomp: # - [0xC0, 0x00] # Program Change 0 # # MIDI message formats: @@ -128,7 +122,7 @@ hardware: # hardware: # external_midi: # messages: - # c4: + # Source Audio C4 Synth: # - [0xC0, 0x05] # Program Change 5 for this pedalboard only diff --git a/tests/integration/test_external_midi_loopback.py b/tests/integration/test_external_midi_loopback.py index c7b4aedfc..dbc03fada 100644 --- a/tests/integration/test_external_midi_loopback.py +++ b/tests/integration/test_external_midi_loopback.py @@ -38,6 +38,7 @@ def _port_is_visible(port_name, timeout=1.0): deadline = time.monotonic() + timeout while time.monotonic() < deadline: import rtmidi + temp = rtmidi.MidiOut() ports = temp.get_ports() del temp @@ -77,16 +78,10 @@ def _on_message(event, data): del midi_in -def _manager_for(port_name, glob=None): - """Enabled manager with one port auto-detected by name glob.""" +def _manager_for(port_name): + """Enabled manager; port_name is the exact ALSA client device name used as the key.""" mgr = ExternalMidiManager() - mgr.update_config( - { - "enabled": True, - "send_delay_ms": 0, - "ports": {"dev": {"auto_detect": [glob or f"*{port_name}*"]}}, - } - ) + mgr.update_config({"enabled": True, "send_delay_ms": 0}) return mgr @@ -94,18 +89,18 @@ class TestRealLoopback: def test_send_raw_reaches_real_device(self, loopback): port_name, received = loopback mgr = _manager_for(port_name) - mgr.open_port("dev") + mgr.open_port(port_name) - assert mgr.send_raw("dev", [0xB0, 75, 42]) is True + assert mgr.send_raw(port_name, [0xB0, 75, 42]) is True assert _wait_for(lambda: received == [[0xB0, 75, 42]]) mgr.close() def test_external_midi_out_prefers_real_port_over_fallback(self, loopback): port_name, received = loopback mgr = _manager_for(port_name) - mgr.open_port("dev") + mgr.open_port(port_name) fallback = MagicMock() - out = ExternalMidiOut(mgr, "dev", fallback) + out = ExternalMidiOut(mgr, port_name, fallback) out.send_message([0xB0, 70, 7]) @@ -114,11 +109,10 @@ def test_external_midi_out_prefers_real_port_over_fallback(self, loopback): mgr.close() def test_external_midi_out_falls_back_when_device_absent(self, loopback): - # Glob matches nothing → real enumeration finds no port → fallback used. _, received = loopback - mgr = _manager_for("unused", glob="*no-such-pistomp-port*") + mgr = _manager_for("no-such-pistomp-port") fallback = MagicMock() - out = ExternalMidiOut(mgr, "dev", fallback) + out = ExternalMidiOut(mgr, "no-such-pistomp-port", fallback) out.send_message([0xB0, 70, 7]) @@ -129,8 +123,8 @@ def test_external_midi_out_falls_back_when_device_absent(self, loopback): def test_send_messages_for_pedalboard_delivers_sequence(self, loopback): port_name, received = loopback mgr = _manager_for(port_name) - mgr.messages = {"dev": [[0xC0, 5], [0xB0, 7, 100]]} - mgr.open_port("dev") + mgr.messages = {port_name: [[0xC0, 5], [0xB0, 7, 100]]} + mgr.open_port(port_name) assert mgr.send_messages_for_pedalboard() is True @@ -140,7 +134,7 @@ def test_send_messages_for_pedalboard_delivers_sequence(self, loopback): def test_absent_device_backs_off_no_per_send_reenumerate(self, loopback, monkeypatch): # End-to-end: an absent device must not re-enumerate on every send. _, _received = loopback - mgr = _manager_for("unused", glob="*no-such-pistomp-port*") + mgr = _manager_for("no-such-pistomp-port") enumerations = [] real_enumerate = mgr._get_available_ports @@ -151,8 +145,8 @@ def counting_enumerate(): monkeypatch.setattr(mgr, "_get_available_ports", counting_enumerate) - assert mgr.send_raw("dev", [0xB0, 1, 1]) is False - assert mgr.send_raw("dev", [0xB0, 1, 2]) is False # within backoff window + assert mgr.send_raw("no-such-pistomp-port", [0xB0, 1, 1]) is False + assert mgr.send_raw("no-such-pistomp-port", [0xB0, 1, 2]) is False # within backoff window assert len(enumerations) == 1 # second send short-circuited on backoff mgr.close() @@ -170,9 +164,9 @@ class TestControlRoutesToRealPort: def _routed(self, port_name): mgr = _manager_for(port_name) fallback = MagicMock() - out = ExternalMidiOut(mgr, "dev", fallback) + out = ExternalMidiOut(mgr, port_name, fallback) # Eagerly open the port so the first send doesn't race enumeration. - mgr.open_port("dev") + mgr.open_port(port_name) return mgr, out, fallback def test_footswitch_press_reaches_real_port(self, loopback): diff --git a/tests/test_config_schema.py b/tests/test_config_schema.py index fd95b74de..1223e86cb 100644 --- a/tests/test_config_schema.py +++ b/tests/test_config_schema.py @@ -32,14 +32,18 @@ def test_midi_port_and_external_midi_accepted(): "hardware": { "version": 3.0, "midi": {"channel": 14}, - "footswitches": [{"id": 0, "midi_CC": 60, "midi_port": "c4"}], - "analog_controllers": [{"adc_input": 5, "id": 0, "midi_CC": 75, "midi_port": "hx", "type": "EXPRESSION"}], - "encoders": [{"id": 1, "midi_CC": 70, "midi_port": "c4"}], + "footswitches": [{"id": 0, "midi_CC": 60, "midi_port": "Source Audio C4 Synth"}], + "analog_controllers": [ + {"adc_input": 5, "id": 0, "midi_CC": 75, "midi_port": "HX Stomp", "type": "EXPRESSION"} + ], + "encoders": [{"id": 1, "midi_CC": 70, "midi_port": "Source Audio C4 Synth"}], "external_midi": { "enabled": True, "send_delay_ms": 10, - "ports": {"c4": {"auto_detect": ["*C4*"]}, "hx": {"port_index": 2}}, - "messages": {"c4": [[0xB0, 0x66, 0x00]], "hx": [[0xC0, 0x00]]}, + "messages": { + "Source Audio C4 Synth": [[0xB0, 0x66, 0x00]], + "HX Stomp": [[0xC0, 0x00]], + }, }, } } diff --git a/tests/test_external_midi.py b/tests/test_external_midi.py index 9230d5f80..59aa4fbc7 100644 --- a/tests/test_external_midi.py +++ b/tests/test_external_midi.py @@ -1,11 +1,8 @@ """ Tests for ExternalMidiManager and ExternalMidiOut. -These exercise the UX flows added on feat/external-midi: - - configuring external MIDI ports via auto-detect / explicit index - - sending pedalboard load messages with inter-message delay - - per-pedalboard config overrides (incremental update_config) - - ExternalMidiOut wrapper preferring external port, falling back to virtual +Config keys are ALSA client device names (e.g. 'Source Audio C4 Synth'). +Matching is case-insensitive against the client-name prefix of each rtmidi port string. """ from typing import cast @@ -17,8 +14,6 @@ def _port(mgr: ExternalMidiManager, name: str) -> MagicMock: - # Casting at the access site lets pyright resolve - # the mock helpers (`assert_called_once_with`, `call_args_list`, etc). return cast(MagicMock, mgr.midi_ports[name]) @@ -49,7 +44,6 @@ def test_disabled_by_default(self): mgr = ExternalMidiManager() assert mgr.enabled is False assert mgr.send_delay_ms == 10 - assert mgr.port_configs == {} assert mgr.messages == {} def test_none_config_is_noop(self): @@ -63,82 +57,73 @@ def test_enables_and_sets_delay(self): assert mgr.enabled is True assert mgr.send_delay_ms == 25 - def test_ports_merge_across_calls(self): - """Pedalboard config can add/override individual ports without wiping defaults.""" - mgr = ExternalMidiManager() - mgr.update_config({"ports": {"c4": {"auto_detect": ["*C4*"]}}}) - mgr.update_config({"ports": {"hx": {"auto_detect": ["*HX*"]}}}) - assert "c4" in mgr.port_configs - assert "hx" in mgr.port_configs - def test_messages_merge_per_port(self): """Per-pedalboard override updates only the named port, leaves others.""" mgr = ExternalMidiManager() - mgr.update_config({"messages": {"c4": [[0xC0, 0x00]], "hx": [[0xC0, 0x01]]}}) - mgr.update_config({"messages": {"c4": [[0xC0, 0x05]]}}) - assert mgr.messages["c4"] == [[0xC0, 0x05]] - assert mgr.messages["hx"] == [[0xC0, 0x01]] - - def test_ports_deep_merge_preserves_defaults(self): - """Pedalboard config overlay should merge fields, not replace the whole port dict.""" - mgr = ExternalMidiManager() - mgr.update_config({"ports": {"c4": {"auto_detect": ["*C4*"], "port_index": 0}}}) - # Overlay only changes auto_detect — port_index should be preserved - mgr.update_config({"ports": {"c4": {"auto_detect": ["*C4 Ultra*"]}}}) - assert mgr.port_configs["c4"].get("auto_detect") == ["*C4 Ultra*"] - assert mgr.port_configs["c4"].get("port_index") == 0 + mgr.update_config({"messages": {"Source Audio C4 Synth": [[0xC0, 0x00]], "HX Stomp": [[0xC0, 0x01]]}}) + mgr.update_config({"messages": {"Source Audio C4 Synth": [[0xC0, 0x05]]}}) + assert mgr.messages["Source Audio C4 Synth"] == [[0xC0, 0x05]] + assert mgr.messages["HX Stomp"] == [[0xC0, 0x01]] class TestPortDiscovery: - def test_auto_detect_glob_case_insensitive(self, fake_ports): + def test_device_name_matches_by_client_name(self, fake_ports): available, _ = fake_ports - available[:] = ["Midi Through:0", "Source Audio C4 Synth", "HX Stomp"] + available[:] = [ + "Midi Through:Midi Through Port-0 14:0", + "Source Audio C4 Synth:Source Audio C4 Synth MIDI 1 20:0", + "HX Stomp:HX Stomp MIDI 1 21:0", + ] mgr = ExternalMidiManager() mgr.update_config( { "enabled": True, - "ports": {"c4": {"auto_detect": ["*c4*"]}}, - "messages": {"c4": [[0xC0, 0x05]]}, + "messages": {"Source Audio C4 Synth": [[0xC0, 0x05]]}, } ) assert mgr.send_messages_for_pedalboard() is True - # Opened port 1 (the C4) - _port(mgr, "c4").open_port.assert_called_once_with(1) + _port(mgr, "Source Audio C4 Synth").open_port.assert_called_once_with(1) - def test_explicit_port_index_wins(self, fake_ports): + def test_device_name_match_is_case_insensitive(self, fake_ports): available, _ = fake_ports - available[:] = ["A", "B", "C"] + available[:] = ["Source Audio C4 Synth:Source Audio C4 Synth MIDI 1 20:0"] mgr = ExternalMidiManager() mgr.update_config( { "enabled": True, - "ports": {"manual": {"port_index": 2, "auto_detect": ["*A*"]}}, - "messages": {"manual": [[0xC0, 0x00]]}, + "messages": {"source audio c4 synth": [[0xC0, 0x05]]}, } ) - mgr.send_messages_for_pedalboard() - _port(mgr, "manual").open_port.assert_called_once_with(2) + assert mgr.send_messages_for_pedalboard() is True + _port(mgr, "source audio c4 synth").open_port.assert_called_once_with(0) def test_no_match_skips_port(self, fake_ports): available, _ = fake_ports - available[:] = ["Midi Through"] + available[:] = ["Midi Through:Midi Through Port-0 14:0"] mgr = ExternalMidiManager() mgr.update_config( { "enabled": True, - "ports": {"c4": {"auto_detect": ["*C4*"]}}, - "messages": {"c4": [[0xC0, 0x05]]}, + "messages": {"Source Audio C4 Synth": [[0xC0, 0x05]]}, } ) - # send_messages_for_pedalboard returns True (iteration ran), but no port opened mgr.send_messages_for_pedalboard() - assert "c4" not in mgr.midi_ports or mgr.midi_ports.get("c4") is None + assert "Source Audio C4 Synth" not in mgr.midi_ports + + def test_port_name_without_colon_matches_exactly(self, fake_ports): + """Port strings with no colon (short names) match the key directly.""" + available, _ = fake_ports + available[:] = ["dev"] + mgr = ExternalMidiManager() + mgr.update_config({"enabled": True, "messages": {"dev": [[0xC0, 0x00]]}}) + assert mgr.send_messages_for_pedalboard() is True + _port(mgr, "dev").open_port.assert_called_once_with(0) class TestSendMessagesForPedalboard: def test_disabled_short_circuits(self, fake_ports): mgr = ExternalMidiManager() - mgr.update_config({"messages": {"c4": [[0xC0, 0]]}}) + mgr.update_config({"messages": {"dev": [[0xC0, 0]]}}) assert mgr.send_messages_for_pedalboard() is False def test_no_messages_returns_false(self): @@ -157,7 +142,6 @@ def test_delay_applied_between_messages(self, fake_ports, monkeypatch): { "enabled": True, "send_delay_ms": 25, - "ports": {"dev": {"auto_detect": ["*dev*"]}}, "messages": {"dev": [[0xC0, 0], [0xC0, 1], [0xC0, 2]]}, } ) @@ -174,7 +158,6 @@ def test_invalid_message_is_skipped_others_sent(self, fake_ports): { "enabled": True, "send_delay_ms": 0, - "ports": {"dev": {"auto_detect": ["dev"]}}, "messages": { "dev": [ [0x00, 0x00], # invalid status byte (< 0x80) @@ -197,17 +180,14 @@ def test_failing_port_invalidated_and_stops(self, fake_ports): { "enabled": True, "send_delay_ms": 0, - "ports": {"dev": {"auto_detect": ["dev"]}}, "messages": {"dev": [[0xC0, 1], [0xC0, 2], [0xC0, 3]]}, } ) midi_out = MagicMock() midi_out.send_message.side_effect = RuntimeError("device disconnected") - # Pre-seed the cache so _init_port returns our failing mock mgr.midi_ports["dev"] = midi_out mgr.send_messages_for_pedalboard() - # Stopped after first failure; port removed from cache so next call re-opens assert midi_out.send_message.call_count == 1 assert "dev" not in mgr.midi_ports midi_out.close_port.assert_called_once() @@ -221,7 +201,9 @@ def test_open_port_failure_returns_none_without_keyerror(self, monkeypatch): monkeypatch.setattr("modalapi.external_midi.rtmidi.MidiOut", lambda *a, **k: failing) mgr = ExternalMidiManager() - mgr.update_config({"enabled": True, "ports": {"dev": {"port_index": 0}}}) + mgr.update_config({"enabled": True}) + # Bypass enumeration so we reach open_port directly + monkeypatch.setattr(mgr, "_find_port_index", lambda name: 0) assert mgr._init_port("dev") is None assert "dev" not in mgr.midi_ports @@ -233,18 +215,18 @@ def test_failed_open_backs_off_no_reenumerate(self, fake_ports): available, created = fake_ports available[:] = ["something_else"] mgr = ExternalMidiManager() - mgr.update_config({"enabled": True, "ports": {"c4": {"auto_detect": ["*c4*"]}}}) + mgr.update_config({"enabled": True}) - assert mgr._init_port("c4") is None + assert mgr._init_port("missing_device") is None n = len(created) - assert mgr._init_port("c4") is None + assert mgr._init_port("missing_device") is None assert len(created) == n # second attempt skipped enumeration def test_open_port_eager_returns_bool(self, fake_ports): available, _ = fake_ports available[:] = ["dev"] mgr = ExternalMidiManager() - mgr.update_config({"enabled": True, "ports": {"dev": {"port_index": 0}}}) + mgr.update_config({"enabled": True}) assert mgr.open_port("dev") is True assert "dev" in mgr.midi_ports @@ -252,10 +234,11 @@ def test_open_port_eager_returns_bool(self, fake_ports): class TestSendRaw: def test_returns_false_when_disabled(self): mgr = ExternalMidiManager() - mgr.update_config({"ports": {"dev": {"port_index": 0}}}) assert mgr.send_raw("dev", [0xB0, 10, 64]) is False - def test_unknown_port_returns_false(self): + def test_unknown_port_returns_false(self, fake_ports): + available, _ = fake_ports + available[:] = ["something_else"] mgr = ExternalMidiManager() mgr.update_config({"enabled": True}) assert mgr.send_raw("ghost", [0xB0, 10, 64]) is False @@ -264,7 +247,7 @@ def test_sends_message_and_returns_true(self, fake_ports): available, _ = fake_ports available[:] = ["dev"] mgr = ExternalMidiManager() - mgr.update_config({"enabled": True, "ports": {"dev": {"port_index": 0}}}) + mgr.update_config({"enabled": True}) assert mgr.send_raw("dev", [0xB0, 80, 100]) is True _port(mgr, "dev").send_message.assert_called_once_with([0xB0, 80, 100]) @@ -272,20 +255,20 @@ def test_sends_non_cc_message(self, fake_ports): available, _ = fake_ports available[:] = ["dev"] mgr = ExternalMidiManager() - mgr.update_config({"enabled": True, "ports": {"dev": {"port_index": 0}}}) + mgr.update_config({"enabled": True}) assert mgr.send_raw("dev", [0xC0, 5]) is True # Program Change _port(mgr, "dev").send_message.assert_called_once_with([0xC0, 5]) def test_returns_false_when_port_unavailable(self, fake_ports): - mgr = ExternalMidiManager() - mgr.update_config({"enabled": True, "ports": {"dev": {"auto_detect": ["*nope*"]}}}) available, _ = fake_ports available[:] = ["something_else"] + mgr = ExternalMidiManager() + mgr.update_config({"enabled": True}) assert mgr.send_raw("dev", [0xB0, 10, 64]) is False def test_send_failure_invalidates_port(self, fake_ports): mgr = ExternalMidiManager() - mgr.update_config({"enabled": True, "ports": {"dev": {"port_index": 0}}}) + mgr.update_config({"enabled": True}) midi_out = MagicMock() midi_out.send_message.side_effect = RuntimeError("broken") mgr.midi_ports["dev"] = midi_out @@ -301,7 +284,6 @@ def test_close_closes_all_ports(self, fake_ports): mgr.update_config( { "enabled": True, - "ports": {"a": {"port_index": 0}, "b": {"port_index": 1}}, "messages": {"a": [[0xC0, 0]], "b": [[0xC0, 0]]}, } ) @@ -343,7 +325,6 @@ def test_falls_back_when_send_raw_raises(self): fallback.send_message.assert_called_once_with([0xB0, 10, 64]) def test_non_cc_message_routes_to_external(self): - """Non-CC messages are sent to external port via send_raw like any other.""" manager = MagicMock() manager.send_raw.return_value = True fallback = MagicMock() @@ -354,7 +335,6 @@ def test_non_cc_message_routes_to_external(self): fallback.send_message.assert_not_called() def test_program_change_routes_to_external(self): - """Even short messages go through send_raw.""" manager = MagicMock() manager.send_raw.return_value = True fallback = MagicMock() diff --git a/tests/test_hardware.py b/tests/test_hardware.py index d46d42268..253bf28da 100644 --- a/tests/test_hardware.py +++ b/tests/test_hardware.py @@ -30,17 +30,11 @@ def _validate(hw, port_name): class TestValidateMidiPort: - def test_unknown_port_logs_warning_not_error(self, caplog): - """A config typo on midi_port is user error, not a system fault — warn, don't error.""" + def test_known_port_returned(self): + """A valid device name passes through unchanged.""" hw = object.__new__(_StubHardware) - hw.external_midi = ExternalMidiManager() # empty port_configs - - with caplog.at_level(logging.WARNING): - assert _validate(hw, "ghost") is None - - recs = [r for r in caplog.records if "ghost" in r.getMessage()] - assert recs - assert all(r.levelno == logging.WARNING for r in recs) + hw.external_midi = ExternalMidiManager() + assert _validate(hw, "Source Audio C4 Synth") == "Source Audio C4 Synth" def test_uninitialized_external_midi_logs_warning_not_error(self, caplog): hw = object.__new__(_StubHardware) @@ -57,14 +51,16 @@ def test_uninitialized_external_midi_logs_warning_not_error(self, caplog): @pytest.fixture def routed_hw(monkeypatch): """A Hardware with one encoder, analog control, and footswitch, and a 'c4' external port.""" - monkeypatch.setattr("modalapi.external_midi.rtmidi.MidiOut", lambda *a, **k: MagicMock()) + mock_out = MagicMock() + mock_out.get_ports.return_value = ["My MIDI Device"] + monkeypatch.setattr("modalapi.external_midi.rtmidi.MidiOut", lambda *a, **k: mock_out) hw = object.__new__(_StubHardware) hw.midiout = MagicMock(name="virtual") hw.external_midi = ExternalMidiManager() - hw.external_midi.update_config({"enabled": True, "ports": {"c4": {"port_index": 0}}}) + hw.external_midi.update_config({"enabled": True}) - hw.encoders = [SimpleNamespace(id=1, midi_CC=70, midiout=hw.midiout)] + hw.encoders = [SimpleNamespace(id=1, midi_CC=70, midi_channel=13, midiout=hw.midiout)] hw.analog_controls = cast(list, [SimpleNamespace(id=2, midi_CC=75, midiout=hw.midiout)]) hw.footswitches = cast(list, [SimpleNamespace(id=0, midiout=hw.midiout)]) return hw @@ -77,17 +73,17 @@ def _route(hw, cfg): class TestApplyMidiRouting: def test_footswitch_routed_to_external_port(self, routed_hw): """A footswitch with midi_port routes to its external port.""" - cfg = {Token.HARDWARE: {Token.FOOTSWITCHES: [{Token.ID: 0, "midi_port": "c4"}]}} + cfg = {Token.HARDWARE: {Token.FOOTSWITCHES: [{Token.ID: 0, "midi_port": "My MIDI Device"}]}} _route(routed_hw, cfg) fs = routed_hw.footswitches[0] assert isinstance(fs.midiout, ExternalMidiOut) - assert fs.midiout.port_name == "c4" + assert fs.midiout.port_name == "My MIDI Device" def test_encoder_and_analog_routed_to_external_port(self, routed_hw): cfg = { Token.HARDWARE: { - Token.ENCODERS: [{Token.ID: 1, "midi_port": "c4"}], - Token.ANALOG_CONTROLLERS: [{Token.ID: 2, "midi_port": "c4"}], + Token.ENCODERS: [{Token.ID: 1, "midi_port": "My MIDI Device"}], + Token.ANALOG_CONTROLLERS: [{Token.ID: 2, "midi_port": "My MIDI Device"}], } } _route(routed_hw, cfg) @@ -99,6 +95,12 @@ def test_encoder_midi_cc_override(self, routed_hw): _route(routed_hw, cfg) assert routed_hw.encoders[0].midi_CC == 99 + def test_encoder_midi_channel_override(self, routed_hw): + """External device may be on a different channel than the hardware default.""" + cfg = {Token.HARDWARE: {Token.ENCODERS: [{Token.ID: 1, "midi_channel": 0}]}} + _route(routed_hw, cfg) + assert routed_hw.encoders[0].midi_channel == 0 + def test_no_midi_port_falls_back_to_virtual(self, routed_hw): cfg = {Token.HARDWARE: {Token.FOOTSWITCHES: [{Token.ID: 0}]}} _route(routed_hw, cfg) @@ -106,9 +108,9 @@ def test_no_midi_port_falls_back_to_virtual(self, routed_hw): def test_external_port_opened_eagerly(self, routed_hw): """The external port is opened at routing time, not lazily inside the poll loop.""" - cfg = {Token.HARDWARE: {Token.FOOTSWITCHES: [{Token.ID: 0, "midi_port": "c4"}]}} + cfg = {Token.HARDWARE: {Token.FOOTSWITCHES: [{Token.ID: 0, "midi_port": "My MIDI Device"}]}} _route(routed_hw, cfg) - assert "c4" in routed_hw.external_midi.midi_ports + assert "My MIDI Device" in routed_hw.external_midi.midi_ports class TestReinitDefaultRouting: From c68ba53305e2f41a00a30753d2651ccbb1cf8edd Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Tue, 9 Jun 2026 17:48:47 -0400 Subject: [PATCH 25/26] external_midi is not nullable --- modalapi/mod.py | 18 ++++++------------ modalapi/modhandler.py | 18 ++++++------------ 2 files changed, 12 insertions(+), 24 deletions(-) diff --git a/modalapi/mod.py b/modalapi/mod.py index 6d1a6b0f3..dc7d55499 100755 --- a/modalapi/mod.py +++ b/modalapi/mod.py @@ -159,11 +159,7 @@ def __init__(self, audiocard, homedir): logging.info("WebSocket bridge started") # External MIDI device synchronization - self.external_midi = None - try: - self.external_midi = ExternalMidi.ExternalMidiManager() - except Exception as e: - logging.warning(f"Failed to initialize external MIDI manager: {e}") + self.external_midi = ExternalMidi.ExternalMidiManager() # Callback function map. Key is the user specified name, value is function from this handler # Used for calling handler callbacks pointed to by names which may be user set in the config file @@ -185,8 +181,7 @@ def __del__(self): def cleanup(self): if self.lcd is not None: self.lcd.cleanup() - if self.external_midi is not None: - self.external_midi.close() + self.external_midi.close() self.ws_bridge.stop() # Container for dynamic data which is unique to the "current" pedalboard @@ -613,11 +608,10 @@ def set_current_pedalboard(self, pedalboard): # Send external MIDI messages for this pedalboard # Config was already updated by hardware.reinit(cfg) above - if self.external_midi is not None: - try: - self.external_midi.send_messages_for_pedalboard() - except Exception as e: - logging.warning(f"Failed to send external MIDI messages: {e}") + try: + self.external_midi.send_messages_for_pedalboard() + except Exception as e: + logging.warning(f"Failed to send external MIDI messages: {e}") # Sync current state of analog controls (expression pedals, etc.) self.hardware.sync_analog_controls() diff --git a/modalapi/modhandler.py b/modalapi/modhandler.py index 745834c3e..d3446c0a0 100755 --- a/modalapi/modhandler.py +++ b/modalapi/modhandler.py @@ -126,11 +126,7 @@ def __init__(self, audiocard: Audiocard, homedir, data_dir="/home/pistomp/data") } # External MIDI device synchronization - self.external_midi = None - try: - self.external_midi = ExternalMidi.ExternalMidiManager() - except Exception as e: - logging.warning(f"Failed to initialize external MIDI manager: {e}") + self.external_midi = ExternalMidi.ExternalMidiManager() # Blend mode manager - multiple blend snapshots per pedalboard self.blend_modes: dict[str, Any] = {} # {snapshot_name: BlendMode} @@ -154,8 +150,7 @@ def cleanup(self): self._lcd.cleanup() if self._hardware is not None: self._hardware.cleanup() - if self.external_midi is not None: - self.external_midi.close() + self.external_midi.close() self.ws_bridge.stop() def _rest_get(self, url: str) -> Response | None: @@ -531,11 +526,10 @@ def set_current_pedalboard(self, pedalboard): # Send external MIDI messages for this pedalboard # Config was already updated by hardware.reinit(cfg) above - if self.external_midi is not None: - try: - self.external_midi.send_messages_for_pedalboard() - except Exception as e: - logging.warning(f"Failed to send external MIDI messages: {e}") + try: + self.external_midi.send_messages_for_pedalboard() + except Exception as e: + logging.warning(f"Failed to send external MIDI messages: {e}") # Sync analog controls last: after bind + external send, matching mod.py self.hardware.sync_analog_controls() From e06d38a4f541aeaa24b85d414b8f56d2f0c13e6e Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Tue, 9 Jun 2026 19:31:20 -0400 Subject: [PATCH 26/26] Move routing ownership from Controller to Hardware registry A control is a source; it shouldn't know its destination. Routing detection piggybacked on the ExternalMidiOut wrapper the control held for sending (isinstance on self.midiout), fusing "where do I send" with "how do I send" and making the wrapper undeletable. Split them: Hardware owns external_routing (control -> RoutingInfo), the source of truth for the internal-vs-external split, rebuilt each reinit in __route_section. is_external/external_port_name read it. Controller drops get_routing_info; get_display_info returns own-presentation only. ControllerManager.bind queries the hardware registry and builds the External display entry from it. RoutingInfo/RoutingDestination survive as the registry's value vocabulary (the durable internal/external axis, future home of a v4 routing table). ExternalMidiOut is now purely a sender. Co-Authored-By: Claude Opus 4.8 --- pistomp/controller.py | 16 +++----------- pistomp/controller_manager.py | 19 ++++++++-------- pistomp/hardware.py | 27 ++++++++++++++++++----- tests/test_controller_manager.py | 17 +++++++------- tests/test_hardware.py | 38 ++++++++++++++++++++++++++++---- 5 files changed, 77 insertions(+), 40 deletions(-) diff --git a/pistomp/controller.py b/pistomp/controller.py index 590fe1336..f36804376 100755 --- a/pistomp/controller.py +++ b/pistomp/controller.py @@ -74,17 +74,7 @@ def bind_to_parameter(self, parameter: Parameter) -> None: self.parameter = parameter self.set_value(parameter.value) - def get_routing_info(self) -> RoutingInfo: - from modalapi.external_midi import ExternalMidiOut - - if isinstance(self.midiout, ExternalMidiOut): - return RoutingInfo.external(self.midiout.port_name) - return RoutingInfo.virtual() - def get_display_info(self) -> AnalogDisplayInfo: - routing = self.get_routing_info() - info: AnalogDisplayInfo = {} - if routing.destination == RoutingDestination.EXTERNAL: - info["port_name"] = routing.port_name - info["midi_cc"] = self.midi_CC - return info + """Own-presentation only; routing-derived fields are added by the + registry owner (ControllerManager._bind_external_controllers).""" + return {} diff --git a/pistomp/controller_manager.py b/pistomp/controller_manager.py index 4f730ee17..f50a168c9 100644 --- a/pistomp/controller_manager.py +++ b/pistomp/controller_manager.py @@ -20,7 +20,7 @@ import common.token as Token from pistomp.analogmidicontrol import AnalogMidiControl -from pistomp.controller import AnalogDisplayInfo, RoutingDestination +from pistomp.controller import AnalogDisplayInfo from pistomp.current import Current from pistomp.encodermidicontrol import EncoderMidiControl from pistomp.footswitch import Footswitch @@ -82,12 +82,11 @@ def _bind_plugin_parameters(self, current) -> list: if controller is None: continue - routing = controller.get_routing_info() # External controllers aren't bound to plugin parameters. - if routing.destination == RoutingDestination.EXTERNAL: + if self._hw.is_external(controller): logging.warning( f"Plugin parameter {plugin.name}:{param.name} is bound to external controller " - f"{param.binding} (routed to {routing.port_name}) - ignoring plugin binding" + f"{param.binding} (routed to {self._hw.external_port_name(controller)}) - ignoring plugin binding" ) continue @@ -124,23 +123,25 @@ def _bind_external_controllers(self, current) -> None: """Externally-routed controllers: bind a synthetic parameter and show them under an "External" category.""" for controller in self._hw.controllers.values(): - routing = controller.get_routing_info() - if routing.destination != RoutingDestination.EXTERNAL or controller.midi_CC is None: + if not self._hw.is_external(controller) or controller.midi_CC is None: continue + port_name = self._hw.external_port_name(controller) controller.parameter = self._hw.create_external_parameter( - controller, routing.port_name, controller.midi_channel, controller.midi_CC + controller, port_name, controller.midi_channel, controller.midi_CC ) if not isinstance(controller, (AnalogMidiControl, EncoderMidiControl)): continue key = f"{controller.midi_channel}:{controller.midi_CC}" - # AnalogMidiControl.get_display_info() already carries type/id; the - # encoder's does not, so seed them and let the dict override. + # Seed type/id (the encoder's get_display_info is empty); routing + # fields come from the registry, not the control. entry: AnalogDisplayInfo = { "type": controller.type, "id": controller.id, **controller.get_display_info(), + "port_name": port_name, + "midi_cc": controller.midi_CC, "category": "External", } current.analog_controllers[key] = entry diff --git a/pistomp/hardware.py b/pistomp/hardware.py index 276d4a434..bd881f57b 100755 --- a/pistomp/hardware.py +++ b/pistomp/hardware.py @@ -31,7 +31,9 @@ import pistomp.taptempo as taptempo from abc import ABC, abstractmethod +from rtmidi import MidiOut from modalapi.external_midi import ExternalMidiOut, ExternalMidiManager, EXTERNAL_INSTANCE_ID +from pistomp.controller import RoutingInfo, RoutingDestination import pistomp.relay as Relay Controller = Union[AnalogMidiControl.AnalogMidiControl, EncoderMidiControl.EncoderMidiControl, Footswitch.Footswitch] @@ -67,6 +69,9 @@ def __init__(self, default_config, handler, midiout, refresh_callback): self.ledstrip = None self.taptempo = taptempo.TapTempo(None) self.external_midi: ExternalMidiManager | None = None + # control → destination; absent means internal (virtual/mod-host). + # Rebuilt every reinit in __route_section. + self.external_routing: dict[Controller, RoutingInfo] = {} def toggle_tap_tempo_enable(self, bpm: float = 0.0): if self.taptempo: @@ -110,6 +115,13 @@ def sync_analog_controls(self): except Exception as e: logging.warning(f"Failed to sync analog control {control.midi_CC}: {e}") + def is_external(self, controller: Controller) -> bool: + return controller in self.external_routing + + def external_port_name(self, controller: Controller) -> str | None: + info = self.external_routing.get(controller) + return info.port_name if info is not None else None + def poll_indicators(self): for i in self.indicators: i.refresh() @@ -125,6 +137,7 @@ def recalibrateVU_baseline(self, baseline): def reinit(self, cfg): # reinit hardware as specified by the new cfg context (after pedalboard change, etc.) self.cfg = self.default_cfg.copy() + self.external_routing.clear() # rebuilt by __route_section for this cfg overlay self.__init_midi_default() @@ -366,15 +379,15 @@ def __validate_midi_port(self, port_name): return None return port_name - def __resolve_midiout(self, cfg_entry): - """Return the midiout for a control: its external port (opened eagerly) if routed, else virtual.""" + def __resolve_midiout(self, cfg_entry) -> tuple[MidiOut | ExternalMidiOut, RoutingInfo]: + """Return (midiout, routing): the wire to send on and its destination.""" midi_port = Util.DICT_GET(cfg_entry, "midi_port") if midi_port: midi_port = self.__validate_midi_port(midi_port) if not midi_port or self.external_midi is None: - return self.midiout + return self.midiout, RoutingInfo.virtual() self.external_midi.open_port(midi_port) # eager: first poll-loop send must not enumerate - return ExternalMidiOut(self.external_midi, midi_port, self.midiout) + return ExternalMidiOut(self.external_midi, midi_port, self.midiout), RoutingInfo.external(midi_port) def __route_section(self, cfg, section, controls, set_cc): cfg_list = Util.DICT_GET(cfg[Token.HARDWARE], section) @@ -395,7 +408,11 @@ def __route_section(self, cfg, section, controls, set_cc): midi_channel = Util.DICT_GET(entry, "midi_channel") if midi_channel is not None and hasattr(ctrl, 'midi_channel'): ctrl.midi_channel = midi_channel - ctrl.midiout = self.__resolve_midiout(entry) + ctrl.midiout, routing = self.__resolve_midiout(entry) + if routing.destination == RoutingDestination.EXTERNAL: + self.external_routing[ctrl] = routing + else: + self.external_routing.pop(ctrl, None) def __apply_midi_routing(self, cfg): """Route every control to its external port or the virtual port (default + pedalboard cfg).""" diff --git a/tests/test_controller_manager.py b/tests/test_controller_manager.py index c0e47a517..c3d5ad59a 100644 --- a/tests/test_controller_manager.py +++ b/tests/test_controller_manager.py @@ -3,9 +3,7 @@ from unittest.mock import MagicMock import common.token as Token -from modalapi.external_midi import ExternalMidiOut from pistomp.analogmidicontrol import AnalogMidiControl -from pistomp.controller import RoutingInfo from pistomp.controller_manager import ControllerManager from pistomp.current import Current @@ -18,7 +16,7 @@ def _make_current() -> Current: class _Ctl: - """Minimal virtual-routed controller — v1 config supplies no VOLUME control + """Minimal internally-routed controller — v1 config supplies no VOLUME control to use directly.""" def __init__(self, type): @@ -26,9 +24,6 @@ def __init__(self, type): self.parameter = "bound" self.midi_CC = None - def get_routing_info(self): - return RoutingInfo.virtual() - def test_bind_preserves_volume_binding_clears_others(): """Controller.type is a class-level default, so the volume guard is type-safe: @@ -38,6 +33,7 @@ def test_bind_preserves_volume_binding_clears_others(): hw = MagicMock() hw.controllers = {"0:7": vol, "0:8": knob} hw.encoders = [] + hw.is_external.return_value = False current = _make_current() ControllerManager(hw).bind(current) @@ -47,18 +43,20 @@ def test_bind_preserves_volume_binding_clears_others(): def _external_analog(midi_cc=75, midi_channel=0, ctrl_id=3): - ext_out = ExternalMidiOut(MagicMock(), "c4", MagicMock()) - return AnalogMidiControl(MagicMock(), 0, 16, midi_cc, midi_channel, ext_out, Token.KNOB, id=ctrl_id, cfg={}) + return AnalogMidiControl(MagicMock(), 0, 16, midi_cc, midi_channel, MagicMock(), Token.KNOB, id=ctrl_id, cfg={}) def test_external_controller_bound_and_displayed(): """An externally-routed control isn't bound to any plugin parameter, so the plugin loop skips it. The external block binds a synthetic parameter and adds - an "External" display entry — otherwise it'd be invisible on the LCD.""" + an "External" display entry — otherwise it'd be invisible on the LCD. Routing + is read from the hardware registry, not the control.""" ctrl = _external_analog() hw = MagicMock() hw.controllers = {"0:75": ctrl} hw.encoders = [] + hw.is_external.return_value = True + hw.external_port_name.return_value = "c4" hw.create_external_parameter.return_value = "SYNTH_PARAM" current = _make_current() @@ -68,3 +66,4 @@ def test_external_controller_bound_and_displayed(): entry = current.analog_controllers["0:75"] assert entry.get("category") == "External" assert entry.get("port_name") == "c4" + assert entry.get("midi_cc") == 75 diff --git a/tests/test_hardware.py b/tests/test_hardware.py index 253bf28da..2d9f68117 100644 --- a/tests/test_hardware.py +++ b/tests/test_hardware.py @@ -1,7 +1,6 @@ """Unit tests for pistomp.hardware.Hardware helpers.""" import logging -from types import SimpleNamespace from typing import cast from unittest.mock import MagicMock @@ -12,6 +11,13 @@ from pistomp.hardware import Hardware +class _Ctl: + """Hashable double; SimpleNamespace can't key the registry (defines __eq__).""" + + def __init__(self, **kw): + self.__dict__.update(kw) + + class _StubHardware(Hardware): """Concrete subclass so object.__new__ works (Hardware is abstract).""" @@ -60,9 +66,10 @@ def routed_hw(monkeypatch): hw.external_midi = ExternalMidiManager() hw.external_midi.update_config({"enabled": True}) - hw.encoders = [SimpleNamespace(id=1, midi_CC=70, midi_channel=13, midiout=hw.midiout)] - hw.analog_controls = cast(list, [SimpleNamespace(id=2, midi_CC=75, midiout=hw.midiout)]) - hw.footswitches = cast(list, [SimpleNamespace(id=0, midiout=hw.midiout)]) + hw.encoders = [_Ctl(id=1, midi_CC=70, midi_channel=13, midiout=hw.midiout)] + hw.analog_controls = cast(list, [_Ctl(id=2, midi_CC=75, midiout=hw.midiout)]) + hw.footswitches = cast(list, [_Ctl(id=0, midiout=hw.midiout)]) + hw.external_routing = {} # __new__ bypasses __init__; __route_section writes here return hw @@ -78,6 +85,28 @@ def test_footswitch_routed_to_external_port(self, routed_hw): fs = routed_hw.footswitches[0] assert isinstance(fs.midiout, ExternalMidiOut) assert fs.midiout.port_name == "My MIDI Device" + assert routed_hw.is_external(fs) + assert routed_hw.external_port_name(fs) == "My MIDI Device" + assert routed_hw.external_routing[fs].port_name == "My MIDI Device" + + def test_unrouted_control_is_internal(self, routed_hw): + """No midi_port → internal: absent from the registry, sends to virtual.""" + cfg = {Token.HARDWARE: {Token.FOOTSWITCHES: [{Token.ID: 0}]}} + _route(routed_hw, cfg) + fs = routed_hw.footswitches[0] + assert fs.midiout is routed_hw.midiout + assert not routed_hw.is_external(fs) + assert routed_hw.external_port_name(fs) is None + assert fs not in routed_hw.external_routing + + def test_routing_overlay_clears_external(self, routed_hw): + """A later cfg pass with no midi_port removes a prior external routing.""" + ext = {Token.HARDWARE: {Token.FOOTSWITCHES: [{Token.ID: 0, "midi_port": "My MIDI Device"}]}} + _route(routed_hw, ext) + fs = routed_hw.footswitches[0] + assert routed_hw.is_external(fs) + _route(routed_hw, {Token.HARDWARE: {Token.FOOTSWITCHES: [{Token.ID: 0}]}}) + assert not routed_hw.is_external(fs) def test_encoder_and_analog_routed_to_external_port(self, routed_hw): cfg = { @@ -119,6 +148,7 @@ def test_reinit_applies_routing_for_default_cfg(self, monkeypatch): hw = object.__new__(_StubHardware) hw.default_cfg = {Token.HARDWARE: {}} hw.handler = MagicMock() + hw.external_routing = {} # __new__ bypasses __init__; reinit clears it for name in ( "_Hardware__init_midi_default",