diff --git a/GUIDE.md b/GUIDE.md index 342f91dbf..7ee8093fb 100644 --- a/GUIDE.md +++ b/GUIDE.md @@ -342,9 +342,12 @@ never authoritatively sets its own copy. A persistent daemon-thread bridge (`poll_ws_messages`, ~10ms). `output_set` meter/scope spam is dropped at the bridge. Inbound (`parse_message` → typed messages → each handler's `_handle_ws_message`): -- `param_set …/:bypass v` — live bypass delta → set bypass + redraw. -- `param_set …/{sym} v` — control value → refresh cached `Parameter.value` (a later - edit opens at current); no live redraw. +- `param_set …/:bypass v` — live bypass delta → set bypass + redraw. Routed through + `Plugin.set_param_value`, which also reconciles any bound footswitch's indicators. +- `param_set …/{sym} v` — control value → `Plugin.set_param_value` refreshes the cached + `Parameter.value` (a later edit opens at current) and mirrors it onto any control + bound to that param: a footswitch redraws its LED/keycap; a knob/encoder updates its + cached position. Params bound to nothing do no work. - `add {inst} {uri} … {bypassed} …` — appears **only** in the (re)connect/load dump; bypass rides in field 4 — its sole arrival point on connect. Same bypass dispatch. - `loading_start` / `loading_end {snapshot}` — bracket a dump; `loading_end` stashes @@ -360,8 +363,11 @@ Outbound behaviour depends on the initiator: - **Footswitch press** → MIDI CC (absolute `toggled` intent) → mod-host processes internally → mod-host emits `param_set` feedback on port 5556 → `msg_callback` to - ALL clients (including us). Emit-only is correct here; the feedback echo drives the - LCD/LED update. + ALL clients (including us). piStomp updates its **LED optimistically on press**; + the feedback echo drives the LCD/plugin state update and reconciles if it differs. + Waiting for the echo would lag the switch whenever mod-host's feedback stream is + gated on a slow client — the `data_finish`/`output_data_ready` handshake stalls when + a mod-ui browser tab is backgrounded (see `../mod-ui/docs/output-data-flow.md`). - **Non-footswitch UI tap** → WS `send_parameter` → mod-ui calls `host.bypass()` → `msg_callback_broadcast` **skips the origin socket** (us), and mod-host does NOT generate `param_set` feedback for `bypass` commands it received from mod-ui. No echo @@ -457,6 +463,7 @@ changes (not for commands mod-ui itself issued). This determines who sees what: ``` Path A — Footswitch (MIDI CC): poll_controls() → Footswitch.pressed() + → flip toggled, _set_led() # LED-only optimistic update → midiout.send_message([CC, midi_CC, 127/0]) # direct to ALSA → JACK → mod-host:midi_in # bypasses mod-ui WS entirely → mod-host applies change @@ -464,8 +471,10 @@ Path A — Footswitch (MIDI CC): → mod-ui process_read_message_body → msg_callback("param_set /graph/X :bypass V") # ALL clients, no skip → pi-stomp poll_ws_messages() receives it - → plugin.set_bypass() + lcd.refresh_plugins() - Emit-only is correct: pi-stomp waits for the feedback echo to update LCD/LED. + → plugin.set_bypass() + lcd.refresh_plugins() # reconciles + Optimistic update keeps the switch responsive even when the echo is delayed by + mod-host's data_finish/output_data_ready handshake (stalls on a backgrounded + mod-ui browser tab); the later echo is authoritative and corrects any divergence. Path B — Non-footswitch UI tap (LCD plugin widget click): toggle_plugin_bypass(widget, plugin) diff --git a/modalapi/mod.py b/modalapi/mod.py index 72a73b923..379f88fd0 100755 --- a/modalapi/mod.py +++ b/modalapi/mod.py @@ -516,14 +516,14 @@ def _handle_ws_message(self, msg: WebSocketMessage): self.update_lcd_fs(footswitch=fs) elif isinstance(msg, ParamSetMessage): - # Keep the cached value fresh so a later edit opens at the current - # value. Not drawn anywhere live, so no LCD refresh. + # Mirror mod-ui's live value: refresh the cache (so a later edit opens + # at the current value) and sync any bound control. The connect-dump + # delivers the real mod-ui state here — :bypass aside, nothing else + # repaints a non-bypass footswitch. if self.current is not None: for plugin in self.current.pedalboard.plugins: if plugin.instance_id == msg.instance: - param = plugin.parameters.get(msg.symbol) - if param is not None: - param.value = msg.value + plugin.set_param_value(msg.symbol, msg.value) break elif isinstance(msg, MidiMapMessage): diff --git a/modalapi/modhandler.py b/modalapi/modhandler.py index 510f89ebb..9e755faa0 100755 --- a/modalapi/modhandler.py +++ b/modalapi/modhandler.py @@ -390,14 +390,14 @@ def _handle_ws_message(self, msg: WebSocketMessage): self.update_lcd_fs(footswitch=fs) elif isinstance(msg, ParamSetMessage): - # Keep the cached value fresh so a later long-press edit opens at the - # current value. Not drawn anywhere live, so no LCD refresh. + # Mirror mod-ui's live value: refresh the cache (so a later edit opens + # at the current value) and sync any bound control. The connect-dump + # delivers the real mod-ui state here — :bypass aside, nothing else + # repaints a non-bypass footswitch. if self.current is not None: for plugin in self.current.pedalboard.plugins: if plugin.instance_id == msg.instance: - param = plugin.parameters.get(msg.symbol) - if param is not None: - param.value = msg.value + plugin.set_param_value(msg.symbol, msg.value) break elif isinstance(msg, MidiMapMessage): diff --git a/modalapi/plugin.py b/modalapi/plugin.py index f13bf3c9d..88155ddbb 100755 --- a/modalapi/plugin.py +++ b/modalapi/plugin.py @@ -18,7 +18,7 @@ import json from common.parameter import Parameter -from pistomp.footswitch import Footswitch +from pistomp.controller import Controller Point = tuple[int, int] @@ -38,7 +38,7 @@ def __init__( self.parameters: dict[str, Parameter] = parameters self.bypass_indicator_xy: tuple[Point, Point] = ((0, 0), (0, 0)) self.lcd_xyz: LcdPosition | None = None - self.controllers: list[Footswitch] = [] + self.controllers: list[Controller] = [] self.has_footswitch: bool = False self.category: str | None = category @@ -56,15 +56,20 @@ def toggle_bypass(self) -> float: param.value = new_value return new_value - def set_bypass(self, bypass: bool) -> None: - param = self.parameters.get(":bypass") + def set_param_value(self, symbol: str, value: float) -> None: + """Cache a param's value and mirror it onto any control bound to it, so + a footswitch's LED/keycap (or a knob/encoder's cached position) tracks + mod-ui's live value. set_value is polymorphic per control type.""" + param = self.parameters.get(symbol) if param is None: return - param.value = 1.0 if bypass else 0.0 - if self.has_footswitch: - for c in self.controllers: - if isinstance(c, Footswitch): - c.set_value(param.value) + param.value = value + for c in self.controllers: + if c.parameter is param: + c.set_value(value) + + def set_bypass(self, bypass: bool) -> None: + self.set_param_value(":bypass", 1.0 if bypass else 0.0) def to_json(self) -> str: return json.dumps(self, default=lambda o: o.__dict__, sort_keys=True, indent=4) diff --git a/pistomp/controller.py b/pistomp/controller.py index 9061613a7..f1b0c6505 100755 --- a/pistomp/controller.py +++ b/pistomp/controller.py @@ -16,6 +16,9 @@ from enum import Enum import json import logging +from typing import Optional + +from common.parameter import Parameter class Controller: @@ -25,7 +28,7 @@ def __init__(self, midi_channel, midi_CC): self.midi_CC = midi_CC self.minimum = None self.maximum = None - self.parameter = None + self.parameter: Optional[Parameter] = None self.hardware_name = None #self.type = None # this will conflict with encoder.type for EncoderMidiControl self.midi_min = 0 @@ -34,7 +37,7 @@ def __init__(self, midi_channel, midi_CC): def to_json(self): return json.dumps(self, default=lambda o: o.__dict__, sort_keys=True, indent=4) - def set_value(self, bypass_value: float): + def set_value(self, value: float): logging.error("Controller subclass hasn't overriden the set_value method") diff --git a/pistomp/footswitch.py b/pistomp/footswitch.py index fab0e9caa..caa67e233 100755 --- a/pistomp/footswitch.py +++ b/pistomp/footswitch.py @@ -16,6 +16,7 @@ import logging import time import sys +from typing import Any, Callable from rtmidi.midiconstants import CONTROL_CHANGE import common.token as Token @@ -91,8 +92,8 @@ def __init__(self, id, led_pin, pixel, midi_CC, midi_channel, midiout, refresh_c self.display_label = None self.toggled = False self.led = None - self.midiout = midiout - self.refresh_callback = refresh_callback + self.midiout: Any = midiout + self.refresh_callback: Callable[..., Any] = refresh_callback self.relay_list = [] self.preset_callback = None self.preset_callback_arg = None @@ -139,14 +140,18 @@ def set_midi_CC(self, midi_CC): def set_midi_channel(self, midi_channel): self.midi_channel = midi_channel - @property - def drives_display(self) -> bool: - """True when unbound: no inbound echo will arrive, so the press updates - indicators itself. When bound to a plugin :bypass, the WS broadcast does.""" - return self.parameter is None - - def set_value(self, bypass_value: float): - self.toggled = (bypass_value < 1) + def set_value(self, value: float): + param = self.parameter + if param is not None and param.symbol != Token.COLON_BYPASS: + # Non-:bypass binding: "on" is the max end (the value an on-press + # sends), so compare against the range midpoint. The bypass + # inversion below would light the LED for an OFF param. + lo = param.minimum if param.minimum is not None else 0 + hi = param.maximum if param.maximum is not None else 1 + self.toggled = value >= (lo + hi) / 2 + else: + # :bypass (or relay, param is None): engaged when not bypassed. + self.toggled = (value < 1) self._set_led(self.toggled) self.refresh_callback(footswitch=self) @@ -207,11 +212,7 @@ def _log_longpress_events(self): info.timestamps.update({self.id: now}) def pressed(self, state): - # If a footswitch can be mapped to control a relay, preset, MIDI or all 3 - # - # The footswitch will only "toggle" if it's associated with a relay - # (in which case it will toggle with the relay) or with a Midi message - # + """Handle a footswitch press: route to relay, preset, or MIDI CC as configured.""" new_toggled = not self.toggled # First handle Longpress Events @@ -254,18 +255,19 @@ def pressed(self, state): cc = [self.midi_channel | CONTROL_CHANGE, self.midi_CC, 127 if self.toggled else 0] logging.debug("Sending CC event: %d" % self.midi_CC) self.midiout.send_message(cc) - if self.drives_display: - self._set_led(self.toggled) - - if self.drives_display: - self.refresh_callback(footswitch=self) + self._set_led(self.toggled) + # LCD / plugin state is updated only when the echo arrives, so unbound + # footswitches (tap-tempo / relay / preset) still refresh immediately. + if self.parameter is None: + self.refresh_callback(footswitch=self) + return def set_display_label(self, label): self.display_label = label def add_relay(self, relay): self.relay_list.append(relay) - self.set_value(not relay.init_state()) + self.set_value(0.0 if relay.init_state() else 1.0) def clear_relays(self): self.relay_list.clear() @@ -278,6 +280,7 @@ def clear_pedalboard_info(self): self.toggled = False self.disabled = False self.display_label = None + self.parameter = None self.set_category(None) self.preset_callback = None self.clear_relays() diff --git a/pistomp/lcd320x240.py b/pistomp/lcd320x240.py index ae9d898e7..0c6c23a40 100644 --- a/pistomp/lcd320x240.py +++ b/pistomp/lcd320x240.py @@ -483,22 +483,19 @@ def footswitch_label(self, footswitch): return self.shorten_name(param.instance_id, self.footswitch_width) def draw_footswitch(self, plugin): + color = self.get_plugin_color(plugin) for c in plugin.controllers: if isinstance(c, Footswitch): fs_id = c.id - #fss[fs_id] = None label = self.footswitch_label(c) c.set_display_label(label) - y = 0 x = self.get_footswitch_pitch() * fs_id self.footswitch_slots[fs_id] = label - color = self.get_plugin_color(plugin) p = FootswitchWidget(Box.xywh(x, y, self.plugin_width, self.footswitch_height), self.small_font, - label, color, plugin.is_bypassed(), parent=self.footswitch_panel, object=c) + label, color, not c.toggled, parent=self.footswitch_panel, object=c) self.w_footswitches.append(p) self.footswitch_panel.add_widget(p) - break def draw_unbound_footswitches(self): for fs in self.footswitches: @@ -510,7 +507,7 @@ def draw_unbound_footswitches(self): y = 0 x = self.get_footswitch_pitch() * slot p = FootswitchWidget(Box.xywh(x, y, self.plugin_width, self.footswitch_height), self.small_font, - label, None, True, parent=self.footswitch_panel, object=fs) + label, None, not fs.toggled, parent=self.footswitch_panel, object=fs) self.w_footswitches.append(p) self.footswitch_panel.add_widget(p) self.footswitch_panel.refresh() diff --git a/tests/snapshots/v3/test_plugins/test_v3_footswitch_states_snapshot/initial.png b/tests/snapshots/v3/test_plugins/test_v3_footswitch_states_snapshot/initial.png new file mode 100644 index 000000000..28804e612 Binary files /dev/null and b/tests/snapshots/v3/test_plugins/test_v3_footswitch_states_snapshot/initial.png differ diff --git a/tests/snapshots/v3/test_plugins/test_v3_footswitch_states_snapshot/toggled.png b/tests/snapshots/v3/test_plugins/test_v3_footswitch_states_snapshot/toggled.png new file mode 100644 index 000000000..b9f91a8ff Binary files /dev/null and b/tests/snapshots/v3/test_plugins/test_v3_footswitch_states_snapshot/toggled.png differ diff --git a/tests/snapshots/v3/test_plugins/test_v3_pedalboard_switch_multi_fs_same_plugin_show_bound_off_color/beths.png b/tests/snapshots/v3/test_plugins/test_v3_pedalboard_switch_multi_fs_same_plugin_show_bound_off_color/beths.png new file mode 100644 index 000000000..908b1b341 Binary files /dev/null and b/tests/snapshots/v3/test_plugins/test_v3_pedalboard_switch_multi_fs_same_plugin_show_bound_off_color/beths.png differ diff --git a/tests/snapshots/v3/test_plugins/test_v3_pedalboard_switch_multi_fs_same_plugin_show_bound_off_color/doom_bass.png b/tests/snapshots/v3/test_plugins/test_v3_pedalboard_switch_multi_fs_same_plugin_show_bound_off_color/doom_bass.png new file mode 100644 index 000000000..37d5dbcb9 Binary files /dev/null and b/tests/snapshots/v3/test_plugins/test_v3_pedalboard_switch_multi_fs_same_plugin_show_bound_off_color/doom_bass.png differ diff --git a/tests/test_footswitch.py b/tests/test_footswitch.py index c504d5085..6900535bc 100644 --- a/tests/test_footswitch.py +++ b/tests/test_footswitch.py @@ -4,8 +4,10 @@ from contextlib import contextmanager import time -from unittest.mock import MagicMock +from typing import Optional, cast +from unittest.mock import MagicMock, patch +from common.parameter import Parameter from pistomp.footswitch import Footswitch import pistomp.switchstate as switchstate @@ -31,7 +33,9 @@ def _make_footswitch(**kwargs): refresh_callback=MagicMock(), ) defaults.update(kwargs) - yield Footswitch(**defaults) + fs = Footswitch(**defaults) + fs._set_led = MagicMock() # type: ignore[method-assign] + yield fs Footswitch.callbacks = {} Footswitch.all_longpress_groups = {} @@ -69,6 +73,12 @@ def test_set_longpress_groups_ignores_unknown_group(self): fs.set_longpress_groups(["not_a_real_group"]) assert fs.longpress_groups == ["not_a_real_group"] + def test_set_longpress_groups_none_clears_groups(self): + with _make_footswitch() as fs: + fs.set_longpress_groups(["next_snapshot"]) + fs.set_longpress_groups(None) + assert fs.longpress_groups == [] + class TestPressed: def test_short_press_toggles_enabled_and_sends_midi(self): @@ -103,6 +113,40 @@ def test_preset_callback_called_on_press(self): cb.assert_called_once_with("bank_a") assert fs.toggled is False # preset press does not toggle enabled + def test_preset_callback_without_arg(self): + with _make_footswitch(midi_CC=None) as fs: + cb = MagicMock() + fs.add_preset(cb) + + fs.pressed(switchstate.Value.RELEASED) + + cb.assert_called_once_with() + + def test_taptempo_short_press_does_not_toggle(self): + with _make_footswitch(midi_CC=42) as fs: + taptempo = MagicMock() + taptempo.is_enabled.return_value = True + fs.taptempo = taptempo + + fs.pressed(switchstate.Value.RELEASED) + + taptempo.is_enabled.assert_called_once() + assert fs.toggled is False + fs.midiout.send_message.assert_not_called() + assert isinstance(fs.refresh_callback, MagicMock) + fs.refresh_callback.assert_not_called() + assert isinstance(fs._set_led, MagicMock) + fs._set_led.assert_not_called() + + def test_unbound_footswitch_refresh_callback_on_press(self): + """A footswitch with MIDI but no bound parameter still refreshes the LCD immediately.""" + with _make_footswitch(midi_CC=42) as fs: + fs.pressed(switchstate.Value.RELEASED) + + assert fs.toggled is True + assert isinstance(fs.refresh_callback, MagicMock) + fs.refresh_callback.assert_called_once_with(footswitch=fs) + def test_longpress_logs_timestamp(self): with _make_footswitch() as fs: fs.set_longpress_groups(["toggle_bypass"]) @@ -124,6 +168,101 @@ def test_relay_toggle_on_longpress(self): relay.enable.assert_called_once() + def test_relay_disable_on_second_longpress(self): + with _make_footswitch(midi_CC=None) as fs: + relay = MagicMock() + relay.init_state.return_value = False + fs.add_relay(relay) + + fs.pressed(switchstate.Value.LONGPRESSED) + relay.enable.assert_called_once() + relay.reset_mock() + + fs.pressed(switchstate.Value.LONGPRESSED) + relay.disable.assert_called_once() + + +class TestSetValue: + @staticmethod + def _param(symbol: str, value: float, minimum: Optional[int] = 0, maximum: Optional[int] = 1) -> Parameter: + return cast(Parameter, MagicMock(symbol=symbol, value=value, minimum=minimum, maximum=maximum)) + + def test_bypass_engaged_when_not_bypassed(self): + with _make_footswitch() as fs: + fs.parameter = self._param(":bypass", 0) + fs.set_value(0) # bypass off → effect active + assert fs.toggled is True + + def test_bypass_off_when_bypassed(self): + with _make_footswitch() as fs: + fs.parameter = self._param(":bypass", 1) + fs.set_value(1) # bypassed → effect inactive + assert fs.toggled is False + + def test_non_bypass_off_value_is_off(self): + # Regression: an OFF toggle param (value 0) must not light the switch. + with _make_footswitch() as fs: + fs.parameter = self._param("solo", 0) + fs.set_value(0) + assert fs.toggled is False + + def test_non_bypass_on_value_is_on(self): + with _make_footswitch() as fs: + fs.parameter = self._param("solo", 1) + fs.set_value(1) + assert fs.toggled is True + + def test_non_bypass_handles_missing_range(self): + with _make_footswitch() as fs: + fs.parameter = self._param("gain", 1, minimum=None, maximum=None) + fs.set_value(1) + assert fs.toggled is True + + def test_set_value_drives_led_and_refresh(self): + with _make_footswitch() as fs: + fs.parameter = self._param(":bypass", 0) + fs.set_value(0) + assert isinstance(fs._set_led, MagicMock) + fs._set_led.assert_any_call(True) + assert isinstance(fs.refresh_callback, MagicMock) + fs.refresh_callback.assert_called_once_with(footswitch=fs) + + +class TestLongpressEventPolling: + def test_simultaneous_longpress_calls_group_callback(self): + Footswitch.init({"toggle_bypass": MagicMock()}) + Footswitch.all_longpress_groups["toggle_bypass"].number_in_group = 2 + try: + with _make_footswitch(id=1, midi_CC=None) as fs1, _make_footswitch(id=2, midi_CC=None) as fs2: + fs1.set_longpress_groups(["toggle_bypass"]) + fs2.set_longpress_groups(["toggle_bypass"]) + fs1.pressed(switchstate.Value.LONGPRESSED) + fs2.pressed(switchstate.Value.LONGPRESSED) + + Footswitch.check_longpress_events() + + Footswitch.callbacks["toggle_bypass"].assert_called_once() + assert len(Footswitch.all_longpress_groups["toggle_bypass"].timestamps) == 0 + finally: + Footswitch.callbacks = {} + Footswitch.all_longpress_groups = {} + + def test_single_longpress_expires_and_calls_callback(self): + Footswitch.init({"toggle_bypass": MagicMock()}) + Footswitch.all_longpress_groups["toggle_bypass"].number_in_group = 1 + try: + with _make_footswitch(id=1, midi_CC=None) as fs: + fs.set_longpress_groups(["toggle_bypass"]) + fs.pressed(switchstate.Value.LONGPRESSED) + + with patch("pistomp.footswitch.time.monotonic", return_value=time.monotonic() + 1.0): + Footswitch.check_longpress_events() + + Footswitch.callbacks["toggle_bypass"].assert_called_once() + finally: + Footswitch.callbacks = {} + Footswitch.all_longpress_groups = {} + class TestClearPedalboardInfo: def test_clears_state(self): @@ -132,9 +271,18 @@ def test_clears_state(self): fs.display_label = "Reverb" pixel = MagicMock() fs.pixel = pixel + fs.set_category("Reverb") + fs.set_longpress_groups(["next_snapshot"]) + fs.add_relay(MagicMock()) + fs.add_preset(MagicMock()) + fs.parameter = MagicMock() # set last: add_relay needs parameter=None fs.clear_pedalboard_info() assert fs.toggled is False + assert fs.disabled is False assert fs.display_label is None + assert fs.category is None assert fs.preset_callback is None + assert len(fs.relay_list) == 0 + assert fs.parameter is None diff --git a/tests/v1/test_midi_learn.py b/tests/v1/test_midi_learn.py index a361de37c..6e3284d24 100644 --- a/tests/v1/test_midi_learn.py +++ b/tests/v1/test_midi_learn.py @@ -30,6 +30,35 @@ def test_v1_midi_learn_binds_footswitch_live(v1_system: SystemFixtureLegacy, mak snapshot("bound") +def test_v1_param_set_syncs_bound_footswitch(v1_system: SystemFixtureLegacy, make_plugin, make_parameter): + """A non-:bypass param_set syncs the footswitch bound to that param — its + toggled state mirrors mod-ui's value (no :bypass inversion).""" + handler = v1_system.handler + hw = v1_system.hw + ws_bridge = v1_system.ws_bridge + + assert handler.current + + binding, fs0 = next((k, v) for k, v in hw.controllers.items() if isinstance(v, Footswitch)) + channel, cc = binding.split(":") + + solo = make_parameter("Solo", "mixer", value=0.0) + plugin = make_plugin("mixer", bypassed=False, has_footswitch=False, + parameters={"solo": solo}) + handler.current.pedalboard.plugins = [plugin] + handler.update_lcd() + + ws_bridge.inject(f"midi_map /graph/mixer solo {channel} {cc} 0.0 1.0") + handler.poll_ws_messages() + assert fs0.parameter is solo + assert fs0.toggled is False # value 0.0 → off + + ws_bridge.inject("param_set /graph/mixer solo 1.0") + handler.poll_ws_messages() + assert solo.value == 1.0 + assert fs0.toggled is True # synced on + + def test_v1_midi_learn_replay_is_idempotent(v1_system: SystemFixtureLegacy, make_plugin): """A replayed midi_map matching the current binding is a no-op (no duplicate controllers).""" handler = v1_system.handler diff --git a/tests/v3/test_midi_learn.py b/tests/v3/test_midi_learn.py index 673474fe8..a21cad2e7 100644 --- a/tests/v3/test_midi_learn.py +++ b/tests/v3/test_midi_learn.py @@ -60,6 +60,36 @@ def test_v3_midi_learn_replay_is_idempotent(v3_system: SystemFixture, make_plugi assert plugin.controllers.count(fs0) == 1 +def test_v3_param_set_syncs_bound_footswitch(v3_system: SystemFixture, make_plugin, make_parameter): + """A non-:bypass param_set (e.g. connect-dump or external change) syncs the + footswitch bound to that param — its toggled state mirrors mod-ui's value.""" + handler = v3_system.handler + hw = v3_system.hw + ws_bridge = v3_system.ws_bridge + + assert handler.current + + fs0 = hw.footswitches[0] + channel, cc = _binding_for(hw, fs0).split(":") + + solo = make_parameter("Solo", "mixer", value=0.0) + plugin = make_plugin("mixer", bypassed=False, has_footswitch=False, + parameters={"solo": solo}) + handler.current.pedalboard.plugins = [plugin] + handler.lcd.link_data(handler.pedalboard_list, handler.current, hw.footswitches) + handler.lcd.draw_main_panel() + + ws_bridge.inject(f"midi_map /graph/mixer solo {channel} {cc} 0.0 1.0") + handler.poll_ws_messages() + assert fs0.parameter is solo + assert fs0.toggled is False # value 0.0 → off + + ws_bridge.inject("param_set /graph/mixer solo 1.0") + handler.poll_ws_messages() + assert solo.value == 1.0 + assert fs0.toggled is True # synced on → LED/keycap on + + def test_v3_midi_learn_unknown_instance_is_ignored(v3_system: SystemFixture, make_plugin): """A midi_map for an instance we don't have is a safe no-op.""" handler = v3_system.handler diff --git a/tests/v3/test_plugins.py b/tests/v3/test_plugins.py index 63dbb9b39..32cc90f21 100644 --- a/tests/v3/test_plugins.py +++ b/tests/v3/test_plugins.py @@ -137,44 +137,66 @@ def test_v3_toggle_plugin_bypass_no_footswitch_sends_websocket(v3_system: System def test_v3_toggle_plugin_bypass_via_footswitch(v3_system: SystemFixture, make_plugin, get_urls): - """Plugin with has_footswitch: toggle_plugin_bypass() routes through footswitch.pressed().""" + """Plugin with has_footswitch: toggle_plugin_bypass() sends MIDI and waits + for the WS echo to update state and display.""" handler = v3_system.handler hw = v3_system.hw + ws_bridge = v3_system.ws_bridge mock_post = v3_system.mock_post assert handler.current - plugin = make_plugin("fuzz", has_footswitch=True) - plugin.controllers = [hw.footswitches[0]] + plugin = make_plugin("fuzz") + handler._bind_controller_to_param(plugin, plugin.parameters[":bypass"], hw.footswitches[0]) handler.current.pedalboard.plugins = [plugin] + handler.lcd.link_data(handler.pedalboard_list, handler.current, hw.footswitches) + handler.lcd.draw_main_panel() handler.toggle_plugin_bypass(None, plugin) assert not any("pi_stomp_set" in u for u in get_urls(mock_post)) - assert hw.footswitches[0].toggled is True + assert hw.footswitches[0].toggled is False # bypass intent, echo not yet received + assert plugin.is_bypassed() is False # plugin state unchanged until echo + + # Simulate mod-host broadcasting the bypass change back. + ws_bridge.inject("param_set /graph/fuzz :bypass 0.0") + handler.poll_ws_messages() + assert plugin.is_bypassed() is False + assert hw.footswitches[0].toggled is True # echo confirmed: plugin active -def test_v3_bound_footswitch_emits_absolute_values_without_display(v3_system: SystemFixture, make_plugin): +def test_v3_bound_footswitch_emits_absolute_values_and_updates_on_echo(v3_system: SystemFixture, make_plugin): """A bound :bypass footswitch sends alternating absolute CC values from local - intent — so rapid presses that outrun the echo stay correct — and leaves its - indicators and parameter for the inbound echo to update.""" + intent — so rapid presses that outrun the echo stay correct — and only updates + its indicators when the mod-ui echo arrives.""" + handler = v3_system.handler hw = v3_system.hw + ws_bridge = v3_system.ws_bridge + assert handler.current + fs = hw.footswitches[0] fs.refresh_callback = MagicMock() fs.midiout.send_message.reset_mock() plugin = make_plugin("fuzz") fs.parameter = plugin.parameters[":bypass"] - assert not fs.drives_display + plugin.controllers.append(fs) + handler.current.pedalboard.plugins = [plugin] + handler.lcd.link_data(handler.pedalboard_list, handler.current, hw.footswitches) + handler.lcd.draw_main_panel() - bypass_before = fs.parameter.value for _ in range(3): fs.pressed(switchstate.Value.RELEASED) sent = [c.args[0][2] for c in fs.midiout.send_message.call_args_list] assert sent == [127, 0, 127] - fs.refresh_callback.assert_not_called() - assert fs.parameter.value == bypass_before + assert fs.refresh_callback.call_count == 0 # no display update before echo + + # Echo the final state back; display updates once. + ws_bridge.inject("param_set /graph/fuzz :bypass 0.0") + handler.poll_ws_messages() + assert fs.refresh_callback.call_count == 1 + assert fs.toggled is True def test_v3_preset_change_leans_on_ws_drain_not_rest(v3_system: SystemFixture, make_plugin, get_urls): @@ -603,6 +625,135 @@ def test_v3_websocket_bypass_event_matches_canonical_id(v3_system): assert plugin.is_bypassed() +# --------------------------------------------------------------------------- +# Footswitch strip visual states +# --------------------------------------------------------------------------- + + +def test_v3_footswitch_states_snapshot(v3_system: SystemFixture, make_plugin, snapshot): + """Full main-panel snapshot covering bound and unbound footswitches before and after toggles.""" + import pistomp.switchstate as switchstate + + handler = v3_system.handler + hw = v3_system.hw + ws_bridge = v3_system.ws_bridge + + assert handler.current + assert handler.lcd + + on_plugin = make_plugin("fuzz", category="Distortion", bypassed=False, has_footswitch=True) + off_plugin = make_plugin("delay", category="Delay", bypassed=True, has_footswitch=True) + + fs0 = hw.footswitches[0] + fs1 = hw.footswitches[1] + fs2 = hw.footswitches[2] + fs3 = hw.footswitches[3] + binding0 = next(k for k, v in hw.controllers.items() if v is fs0) + binding1 = next(k for k, v in hw.controllers.items() if v is fs1) + on_plugin.parameters[":bypass"].binding = binding0 + off_plugin.parameters[":bypass"].binding = binding1 + + # fs2 unbound but already toggled on (e.g. tap-tempo enabled); fs3 unbound off. + fs2.toggled = True + fs3.toggled = False + + handler.current.pedalboard.plugins = [on_plugin, off_plugin] + handler.bind_current_pedalboard() + handler.lcd.link_data(handler.pedalboard_list, handler.current, hw.footswitches) + handler.lcd.draw_main_panel() + snapshot("initial") + + # Toggle one bound and one unbound footswitch. + fs1.pressed(switchstate.Value.RELEASED) # bound delay: off -> on (MIDI sent) + fs3.pressed(switchstate.Value.RELEASED) # unbound: off -> on (immediate) + + # Simulate mod-host echoing the bypass change for the bound footswitch. + ws_bridge.inject("param_set /graph/delay :bypass 0.0") + handler.poll_ws_messages() + snapshot("toggled") + + +# --------------------------------------------------------------------------- +# Pedalboard switch: non-bypass footswitch colour regression +# --------------------------------------------------------------------------- + + +def test_v3_pedalboard_switch_multi_fs_same_plugin_show_bound_off_color( + v3_system: SystemFixture, make_plugin, make_parameter, snapshot +): + """After a pedalboard switch, all footswitches bound to params of the same plugin + must show BOUND_OFF_BG — not UNBOUND_BG — immediately, without a WS echo. + + Regression: draw_footswitch broke after the first footswitch per plugin due to + an unconditional `break`, leaving subsequent ones as unbound (color=None). + Additionally, the initial is_bypassed state was taken from plugin.is_bypassed() + which is wrong for non-bypass params; mod-ui only broadcasts param_set on change + so at-default (OFF) values never arrive via WS on a pedalboard switch. + """ + handler = v3_system.handler + hw = v3_system.hw + assert handler.current + + fs0 = hw.footswitches[0] + fs1 = hw.footswitches[1] + fs2 = hw.footswitches[2] + binding0 = next(k for k, v in hw.controllers.items() if v is fs0) + binding1 = next(k for k, v in hw.controllers.items() if v is fs1) + binding2 = next(k for k, v in hw.controllers.items() if v is fs2) + + # --- "Beths": only fs0 bound to :bypass --- + beths = make_plugin("beths", category="Distortion", bypassed=False) + beths.parameters[":bypass"].binding = binding0 + + handler.current.pedalboard.plugins = [beths] + handler.bind_current_pedalboard() + handler.lcd.link_data(handler.pedalboard_list, handler.current, hw.footswitches) + handler.lcd.draw_main_panel() + snapshot("beths") + + # --- "Doom Bass": fs0/1/2 bound to non-bypass params of the SAME plugin --- + solo1 = make_parameter("Solo1", "doom", value=0.0, minimum=0.0, maximum=1.0) + solo2 = make_parameter("Solo2", "doom", value=0.0, minimum=0.0, maximum=1.0) + solo3 = make_parameter("Solo3", "doom", value=0.0, minimum=0.0, maximum=1.0) + solo1.binding = binding0 + solo2.binding = binding1 + solo3.binding = binding2 + + doom = make_plugin( + "doom", + category="Delay", + parameters={ + "solo1": solo1, + "solo2": solo2, + "solo3": solo3, + }, + ) + + handler.current.pedalboard.plugins = [doom] + hw.reinit(None) + handler.bind_current_pedalboard() + handler.lcd.link_data(handler.pedalboard_list, handler.current, hw.footswitches) + handler.lcd.draw_main_panel() + snapshot("doom_bass") + + # All three should have a non-None color (bound-but-off, not unbound). + for fs in (fs0, fs1, fs2): + wfs = next((w for w in handler.lcd.w_footswitches if w.object is fs), None) + assert wfs is not None, f"No widget for footswitch {fs.id}" + assert wfs.color is not None, ( + f"Footswitch {fs.id} shows UNBOUND_BG — expected BOUND_OFF_BG after pedalboard switch" + ) + # All three solos are OFF (value=0.0 < midpoint); widget must reflect that. + assert wfs.is_bypassed is True, ( + f"Footswitch {fs.id} is_bypassed should be True (solo is off) but is {wfs.is_bypassed}" + ) + + +# --------------------------------------------------------------------------- +# Instance ID normalization +# --------------------------------------------------------------------------- + + def test_v3_websocket_bypass_event_with_multiword_id(v3_system): """WS parser correctly strips /graph/ prefix from multi-word instance IDs.""" handler = v3_system.handler