diff --git a/common/param_source.py b/common/param_source.py index 763564396..3ef2c7271 100644 --- a/common/param_source.py +++ b/common/param_source.py @@ -74,4 +74,17 @@ def is_bypassed(self) -> bool: ... def set_bypass(self, bypass: bool) -> None: ... - pedalboard_snapshot: "dict[Symbol, float]" \ No newline at end of file + pedalboard_snapshot: "dict[Symbol, float]" + + +@runtime_checkable +class ParamSink(Protocol): + """The outbound dual of `ParamSource` — where a `Parameter.commit` goes next. + + `reconcile` adopts the single writer's value and sends nothing; `commit` + publishes a finished local edit through this sink. `publish` returns whether + the send left — False lets commit roll the value back. A param with no sink + is display-only. + """ + + def publish(self, param: "Parameter") -> bool: ... \ No newline at end of file diff --git a/common/parameter.py b/common/parameter.py index 819ef8b3c..1ce2b4708 100644 --- a/common/parameter.py +++ b/common/parameter.py @@ -17,10 +17,13 @@ from collections.abc import Callable from enum import Enum -from typing import NewType, NotRequired, TypedDict +from typing import NewType, NotRequired, TypedDict, TYPE_CHECKING import json import common.util as util +if TYPE_CHECKING: + from common.param_source import ParamSink + # strings as they appear in TTL files TTL_ENUMERATION = 'enumeration' TTL_INTEGER = 'integer' @@ -114,12 +117,18 @@ def __init__(self, plugin_info: PortInfo, value: float, binding: str | None, ins # fallbacks only serve the params we synthesise (bypass, volume, VU). self.default: float = float(ranges.get("default", self.minimum)) - # Reactive value: a property setter that notifies observers. _observers - # must exist before the first assignment below, or the write fires into - # a missing list. + # Reactive value. Writes go through reconcile/preview/commit, never a raw + # setter — the verb names the provenance (see those methods). _confirmed + # is the last value the single writer (mod-ui) echoed back; the gap from + # _value is `pending`. self._observers: list[Callable[[Parameter], None]] = [] - self._value: float = 0.0 - self.value = float(value) + self._settled_observers: list[Callable[[Parameter], None]] = [] + self._value: float = float(value) + self._confirmed: float = float(value) + # Where local edits go upstream. None = display-only: reconciled from + # mod-ui, never sent back. Attached at bind time by whoever owns the + # remote channel. + self.sink: ParamSink | None = None self.binding: str | None = binding self.instance_id: str | None = instance_id.lstrip("/") if instance_id else instance_id self.type = Type.DEFAULT @@ -154,14 +163,49 @@ def __init__(self, plugin_info: PortInfo, value: float, binding: str | None, ins def value(self) -> float: return self._value - @value.setter - def value(self, v: float) -> None: - if v == self._value: + @property + def pending(self) -> bool: + """A committed edit the single writer hasn't echoed back yet.""" + return self._value != self._confirmed + + def reconcile(self, value: float) -> None: + """Adopt the single writer's value: repaint, mark confirmed, settle, + publish nothing. The settle fires even at an unchanged value — a mod-ui + echo confirming what we optimistically previewed still has to refresh a + keycap the preview left alone.""" + self._confirmed = value + self._set(value) + self._notify_settled() + + def preview(self, value: float) -> None: + """An optimistic local move not yet committed — a knob mid-turn whose CC + emit is the real send, a footswitch keycap already toggled by the press. + Repaints live observers; does not settle; publishes nothing.""" + self._set(value) + + def commit(self, value: float) -> None: + """A finished local edit: repaint, publish through the sink, then settle. + Rolls back to the last confirmed value (and does not settle) if the send + never leaves — otherwise the LCD would show a number mod-ui never took. + Publishing is unconditional; preview and reconcile share mechanics, so + there is nothing to diff against here.""" + self._set(value) + if self.sink is not None and not self.sink.publish(self): + self._set(self._confirmed) + return + self._notify_settled() + + def _set(self, value: float) -> None: + if value == self._value: return - self._value = v + self._value = value for observe in self._observers: observe(self) + def _notify_settled(self) -> None: + for observe in self._settled_observers: + observe(self) + def subscribe(self, cb: Callable[[Parameter], None]) -> Callable[[], None]: """Register *cb* to fire on every changed-value write. Returns its own unsubscriber. An unchanged write (v == current) does not notify.""" @@ -173,6 +217,19 @@ def _unsub() -> None: pass return _unsub + def subscribe_settled(self, cb: Callable[[Parameter], None]) -> Callable[[], None]: + """Register *cb* for settled values only — a reconcile or a committed + edit, never a bare preview — fired unconditionally, even when the value + is unchanged. Stateful presentation (a footswitch keycap) uses this so it + tracks confirmed state, not a mid-scrub. Returns its own unsubscriber.""" + self._settled_observers.append(cb) + def _unsub() -> None: + try: + self._settled_observers.remove(cb) + except ValueError: + pass + return _unsub + def get_enum_value_list(self) -> list[tuple[str, float]]: return [(v["label"], v["value"]) for v in self.enum_values] diff --git a/modalapi/modhandler.py b/modalapi/modhandler.py index 5e9b12d4d..ad316583a 100755 --- a/modalapi/modhandler.py +++ b/modalapi/modhandler.py @@ -57,6 +57,7 @@ TapTempoEffect, ) from common.parameter import BYPASS_SYMBOL, Parameter, PortInfo, Symbol +from common.param_source import ParamSink from common.parameter_steps import ParameterSteps, effective_multiplier from modalapi.plugin import Plugin from blend.input_controller import InputController @@ -125,6 +126,67 @@ STARTUP_REST_BACKOFF_S = (0.25, 0.25, 0.5, 1.0, 2.0) +def _is_transport_bpm(param: Parameter | None) -> bool: + return ( + param is not None + and param.instance_id == Pedalboard.TRANSPORT_INSTANCE_ID + and param.symbol == BPM_SYMBOL + ) + + +class _TransportBpmSink: + """:bpm's upstream channel. mod-ui's :bpm is a global designation, not a + plugin control port — param_set is rejected — so a commit rides the dedicated + transport-bpm command (POST fallback). set_mod_tap_tempo returns whether the + send left; commit rolls back if it didn't.""" + + def __init__(self, send: Callable[[float], bool]): + self._send = send + + def publish(self, param: Parameter) -> bool: + return self._send(param.value) + + +class _PluginParamSink: + """A plugin control port's upstream channel: param_set over the WebSocket. + Returns whether the send left so a failed commit rolls the LCD back.""" + + def __init__(self, send: Callable[[Parameter], bool]): + self._send = send + + def publish(self, param: Parameter) -> bool: + return self._send(param) + + +class _MidiCcSink: + """A MIDI-mapped param's upstream channel: the CC its encoder emits. For a + pedalboard mapping mod-ui applies its map and echoes param_set (reconciling + us); for an external route the CC drives outboard gear. Either way the CC is + the whole send and it always lands — so an encoder turn and its dialog agree + on one transport. The param carries the value; the controller carries the + MIDI mechanics (7-bit conversion, channel, routing).""" + + def __init__(self, controller: EncoderController, emit: Callable[[Controller, int], None]): + self._controller = controller + self._emit = emit + + def publish(self, param: Parameter) -> bool: + self._emit(self._controller, self._controller.to_midi(param.value)) + return True + + +class _AudioParamSink: + """An audio-card level's upstream channel: a local ALSA write, no remote echo, + so the send always lands.""" + + def __init__(self, commit: Callable[[Symbol, float], None]): + self._commit = commit + + def publish(self, param: Parameter) -> bool: + self._commit(param.symbol, param.value) + return True + + def _remove_binding_row(layer: ContextLayer, binding_id: str) -> None: # Drop any PEDALBOARD-layer row whose control.id matches a learned binding # that's being replaced. Scans all event_kind buckets since a re-learn could @@ -225,10 +287,6 @@ def __init__(self, audiocard: Audiocard, homedir, data_dir="/home/pistomp/data") # Suppress outbound WebSocket messages while a pedalboard change is in flight. self._is_pedalboard_loading = False - # Reactive BPM parameter observer state - self._bpm_unsub: Callable[[], None] | None = None - self._suppress_bpm_event: bool = False - # Tuner state self._tuner_source_factory: TunerSourceFactory | None = None self._tuner_source_spec: str = "jack" @@ -402,7 +460,7 @@ def _handle_encoder(self, event: EncoderEvent) -> bool: if c.type == Token.VOLUME and c.parameter is not None: new_value = ParameterSteps.for_parameter(c.parameter).move(delta) - c.parameter.value = new_value + c.parameter.preview(new_value) self.audiocard.set_volume_parameter(self.audiocard.MASTER, new_value) d = self.lcd.draw_audio_parameter_dialog(c.parameter, self.audio_parameter_commit) if d is not None: @@ -411,28 +469,26 @@ def _handle_encoder(self, event: EncoderEvent) -> bool: # Resolve the binding row for badge shadow_state (side effect), even # though the effect type no longer branches the encoder-turn response. - # CC is the transport for a plugin-bound (MIDI-learned) encoder; mod-ui - # applies its mapping on receipt. The local param.value write drives - # reactive observers; the CC tail below is the sole transport to mod-host. if c.midi_CC is not None: self.effective_table.resolve( ControlRef(cls=ControlClass.ANALOG, id=f"{c.midi_channel}:{c.midi_CC}"), EventKind.ROTATE, ) if c.parameter is not None: + # One transport per bound turn: the param's sink (CC to mod-host for + # a mapped encoder, the WebSocket for :bpm) owns the send. bind + # attaches it; a hand-built encoder gets it on first turn. + if c.parameter.sink is None: + c.parameter.sink = self._sink_for(c.parameter, c) new_value = ParameterSteps.for_parameter(c.parameter).move(delta) - c.parameter.value = new_value + c.parameter.commit(new_value) self.lcd.display_parameter_value(c.parameter, new_value) - emit_value = c.bar_midi_value() - else: - emit_value = self._advance_encoder_fallback(c, delta) - - # Unconditional, and must stay that way: an unbound encoder has no row, - # and this emit is the only way mod-ui sees its CC to MIDI-learn it. - # Emission is hardware-level, below the table (see input/README.md). - # Transport parameters bypass 7-bit MIDI CC emission for high-precision WebSocket transport. - if c.parameter is None or c.parameter.instance_id != Pedalboard.TRANSPORT_INSTANCE_ID: - self._emit_midi(c, emit_value) + return True + + # Unbound: no sink, no row. This fallback CC is the only way mod-ui sees + # the encoder to MIDI-learn it. Emission is hardware-level, below the + # table (see input/README.md). + self._emit_midi(c, self._advance_encoder_fallback(c, delta)) return True def encoder_fallback(self, controller: EncoderController) -> int: @@ -529,7 +585,7 @@ def _fire_row(self, decl: BindingDecl, event: ControllerEvent) -> bool: controller.set_led(controller.toggled) self._emit_midi(controller, 127 if controller.toggled else 0) if controller.parameter is not None: - controller.parameter.value = controller.value_for(controller.toggled) + controller.parameter.preview(controller.value_for(controller.toggled)) self.update_lcd_fs(footswitch=controller) case ParamEffect(): # Footswitch PRESS with a bound plugin param. "on" polarity @@ -544,7 +600,7 @@ def _fire_row(self, decl: BindingDecl, event: ControllerEvent) -> bool: if fs.midi_CC is not None: self._emit_midi(fs, 127 if new_toggled else 0) if fs.parameter is not None: - fs.parameter.value = fs.value_for(new_toggled) + fs.parameter.preview(fs.value_for(new_toggled)) self.update_lcd_fs(footswitch=fs) case RelayEffect(): if fs is not None: @@ -880,14 +936,13 @@ def _handle_ws_message(self, msg: WebSocketMessage): # labels track even when the change originates elsewhere (Link, # MIDI slave, another HMI). :rolling's enum flips Playing/Stopped. if self._current is not None: + # A remote reconcile: adopt mod-ui's values, publish nothing. + # set_param_value routes through reconcile, not commit, so :bpm's + # sink never fires back at the sender. tp = self.current.pedalboard.transport_plugin tp.set_param_value(ROLLING_SYMBOL, 1.0 if msg.rolling else 0.0) tp.set_param_value(BPB_SYMBOL, msg.beats_per_bar) - self._suppress_bpm_event = True - try: - tp.set_param_value(BPM_SYMBOL, msg.bpm) - finally: - self._suppress_bpm_event = False + tp.set_param_value(BPM_SYMBOL, msg.bpm) if self.hardware and self.hardware.taptempo: self.hardware.taptempo.set_bpm(msg.bpm) if self.hardware.taptempo.is_enabled(): @@ -1213,19 +1268,53 @@ def bind_current_pedalboard(self): # The pedalboard data has already been loaded, but this will overlay # any real time settings self._controller_manager.bind(self.current) - self._bind_transport_bpm_listener() + self._attach_sinks() - def _bind_transport_bpm_listener(self) -> None: - if self._bpm_unsub is not None: - self._bpm_unsub() - self._bpm_unsub = None - if self._current is not None: - bpm_param = self.current.pedalboard.transport_plugin.parameters[BPM_SYMBOL] - self._bpm_unsub = bpm_param.subscribe(self._on_bpm_param_changed) + def _attach_sinks(self) -> None: + """Give every editable parameter its upstream channel, keyed by + provenance (see `_sink_for`). Eager over the whole board so an encoder + turn — which commits with no dialog in between — finds a sink already + there; dialog commits fall back to a lazy attach for hand-built params.""" + if self._current is None: + return + # transport_plugin is kept out of .plugins (the effect graph must not + # paint it), so reach it explicitly for :bpm's sink. + boards = [*self.current.pedalboard.plugins, self.current.pedalboard.transport_plugin] + for plugin in boards: + for param in plugin.parameters.values(): + param.sink = self._sink_for(param) + for controller in self.hardware.controllers.values(): + if controller.parameter is not None: + controller.parameter.sink = self._sink_for(controller.parameter, controller) + if self.volume_parameter is not None: + self.volume_parameter.sink = self._sink_for(self.volume_parameter) + + def _sink_for(self, param: Parameter, controller: Controller | None = None) -> ParamSink | None: + """The upstream channel a param's commit rides, by provenance. None is + display-only: reconciled from mod-ui, never sent back — bpb/rolling when + unmapped (mod-ui rejects param_set on them) and an external footswitch + (its press path owns the CC). A param mapped to an encoder rides that + encoder's CC; :bpm is the exception — its range won't fit 7 bits, so it + keeps the WebSocket. Pass *controller* when the caller holds it (an + encoder turn); otherwise it's recovered from the binding.""" + if _is_transport_bpm(param): + return _TransportBpmSink(self.set_mod_tap_tempo) + if param.instance_id is None: + return _AudioParamSink(self.audio_parameter_commit) + enc = controller if isinstance(controller, EncoderController) else None + if enc is None and param.binding is not None: + enc = self.hardware.controllers.get(param.binding) + enc = enc if isinstance(enc, EncoderController) else None + if enc is not None and enc.midi_CC is not None: + return _MidiCcSink(enc, self._emit_midi) + if param.instance_id in (ExternalMidi.EXTERNAL_INSTANCE_ID, Pedalboard.TRANSPORT_INSTANCE_ID): + return None + return _PluginParamSink(self._publish_plugin_param) - def _on_bpm_param_changed(self, param: Parameter) -> None: - if not self._suppress_bpm_event: - self.set_mod_tap_tempo(param.value) + def _publish_plugin_param(self, param: Parameter) -> bool: + if self._is_pedalboard_loading or self.ws_bridge is None or param.instance_id is None: + return False + return self.ws_bridge.send_parameter(param.instance_id, param.symbol, param.value) def _redraw_after_binding(self, controller: Controller, is_footswitch: bool) -> None: if is_footswitch: @@ -1429,39 +1518,14 @@ def effective_table(self) -> ContextStack: # # Parameter Stuff # - def parameter_value_commit(self, param, value): - # Route plugin params through the plugin's mirror so a bound footswitch - # reconciles now, not only on the mod-host echo — the same set_value the - # ParamSetMessage arm runs. Audio/external params have no plugin mirror. - plugin = ( - next((p for p in self.current.pedalboard.plugins if p.instance_id == param.instance_id), None) - if param.instance_id is not None and self._current is not None - else None - ) - if plugin is not None: - plugin.set_param_value(param.symbol, value) - else: - param.value = value - - # Audio parameter (volume, EQ, etc.) - handled locally, no remote update needed - if param.instance_id is None: - self.audio_parameter_commit(param.symbol, value) - - # External MIDI parameters have no mod-host counterpart. The dialog's NAV - # path owns sending the CC that _handle_encoder would have sent for a turn; - # the binding table identifies which controller carries it. - if param.binding is not None: - winner = self.effective_table.resolve( - ControlRef(cls=ControlClass.ANALOG, id=param.binding), EventKind.ROTATE - ) - if winner is not None and any(isinstance(e, MidiCcEffect) for e in winner.effects): - controller = self.hardware.controllers.get(param.binding) - if controller is not None: - self._emit_midi(controller, int(value)) - return - - if not self._is_pedalboard_loading and param.instance_id is not None and param.instance_id != Pedalboard.TRANSPORT_INSTANCE_ID: - self.ws_bridge.send_parameter(param.instance_id, param.symbol, param.value) + def parameter_value_commit(self, param: Parameter, value: float) -> None: + # The sink owns the route (WebSocket param_set, transport-bpm, external + # CC, local ALSA write); commit repaints, publishes through it, settles. + # Bind attaches sinks eagerly; a param edited before then (or added to a + # live board) gets one on first commit. + if param.sink is None: + param.sink = self._sink_for(param) + param.commit(value) @property def wifi_ip(self) -> str | None: @@ -1816,12 +1880,16 @@ def audio_parameter_commit(self, symbol, value): def get_callback(self, callback_name): return util.DICT_GET(self.callbacks, callback_name) - def set_mod_tap_tempo(self, bpm: float | None) -> None: - if bpm is not None: - self._last_bpm_change_time = time.time() - if self.ws_bridge is not None: - self.ws_bridge.send_bpm(bpm) - self._rest_post(self.root_uri + "set_bpm", json={"value": bpm}) + def set_mod_tap_tempo(self, bpm: float | None) -> bool: + # WebSocket first: _rest_post blocks the 10ms loop, and an encoder spin + # calls this once per detent. POST only when backpressure refused the send. + # Returns whether the value left, so a failed send rolls the LCD back. + if bpm is None: + return False + if self.ws_bridge is not None and self.ws_bridge.send_bpm(bpm): + return True + resp = self._rest_post(self.root_uri + "set_bpm", json={"value": bpm}) + return resp is not None and resp.ok def set_sync_mode(self, mode: SyncMode) -> None: """Optimistically switch the clock source; mod-ui's transport echo diff --git a/modalapi/pedalboard.py b/modalapi/pedalboard.py index dc8127408..9b80885fd 100755 --- a/modalapi/pedalboard.py +++ b/modalapi/pedalboard.py @@ -238,7 +238,12 @@ def hydrate(self, plugin_dict) -> None: def _build_transport_plugin(self, time_info: dict | None) -> Plugin.Plugin: """The /pedalboard pseudo-instance carrying :bpm/:bpb/:rolling. Built - from mod-ui's timeInfo block (or default unbound parameters when absent).""" + from mod-ui's timeInfo block, or from mod-ui's own defaults when the + board carries none.""" + # timeInfo's `available` mask says which ports mod-ui has *addressed*, + # not which exist — transport is global and all three are always + # settable. Built unconditionally so the type stays non-Optional; a + # board that never addressed them just has unbound parameters. time_info = time_info or {} parameters: dict[Symbol, Parameter] = { diff --git a/modalapi/plugin.py b/modalapi/plugin.py index 06c1ade70..f9bc69d90 100755 --- a/modalapi/plugin.py +++ b/modalapi/plugin.py @@ -24,7 +24,7 @@ from common.color import RectBorder from common.parameter import BYPASS_SYMBOL, Parameter, Symbol, json_default from modalapi.plugin_customization import PluginCustomization, PluginExtraData -from pistomp.controller import Controller, StatefulController +from pistomp.controller import Controller if TYPE_CHECKING: from plugins.base import PluginPanel @@ -129,24 +129,16 @@ def toggle_bypass(self) -> float: if param is None: return 0.0 new_value = 0.0 if param.value else 1.0 - param.value = new_value + param.preview(new_value) return new_value def set_param_value(self, symbol: Symbol, value: float) -> None: - """Cache a param's value from mod-ui and mirror it onto any bound - footswitch. The mirror is unconditional (outside the idempotent setter) - because a MIDI-originated echo arrives at the same value we already - wrote optimistically — the setter skips it, but the footswitch keycap - still needs to update. See plan: the mod-ui MIDI echo asymmetry.""" + """Reconcile a param to mod-ui's value. Any bound stateful controller + (a footswitch keycap) resyncs through its own subscription, so this + doesn't reach for controllers.""" param = self.parameters.get(symbol) - if param is None: - return - param.value = value - for c in self.controllers: - # Only stateful controllers hold a presentation copy to sync (a - # footswitch keycap, a pot's reading). Encoders own no copy. - if c.parameter is param and isinstance(c, StatefulController): - c.set_value(value) + if param is not None: + param.reconcile(value) def set_bypass(self, bypass: bool) -> None: self.set_param_value(BYPASS_SYMBOL, 1.0 if bypass else 0.0) diff --git a/pistomp/controller.py b/pistomp/controller.py index fa117d106..05c62bb69 100755 --- a/pistomp/controller.py +++ b/pistomp/controller.py @@ -113,3 +113,8 @@ def set_value(self, value: float) -> None: def bind_to_parameter(self, parameter: Parameter) -> None: super().bind_to_parameter(parameter) self.set_value(parameter.value) + # The keycap mirrors settled values — a mod-ui echo or a menu/dialog + # commit — but not a bare preview: a local press updates its own toggle + # and LED, then waits for the echo to refresh. Neither write path needs + # to know the controller exists. + self._unsub_param = parameter.subscribe_settled(lambda p: self.set_value(p.value)) diff --git a/pistomp/encoder_controller.py b/pistomp/encoder_controller.py index c339163c0..85755c021 100644 --- a/pistomp/encoder_controller.py +++ b/pistomp/encoder_controller.py @@ -135,19 +135,22 @@ def poll(self) -> None: # ── Value ──────────────────────────────────────────────────────────── + def to_midi(self, value: float) -> int: + """Convert a bound-parameter value to this control's 7-bit CC byte. The + MIDI mechanics (range, channel, routing) are the controller's, not the + param's — the param stays MIDI-agnostic.""" + assert self.parameter is not None, "to_midi is bound-only; unbound lives on the handler" + midi_value = util.renormalize( + value, self.parameter.minimum, self.parameter.maximum, self.midi_min, self.midi_max + ) + return int(_clamp(midi_value, 0, 127)) + def bar_midi_value(self) -> int: """0-127 for the LCD bar and the MIDI-learn emit of a *bound* encoder, derived from the parameter (the owner). Unbound, the value lives on the handler — ask Modhandler.encoder_fallback.""" assert self.parameter is not None, "bar_midi_value is bound-only; unbound lives on the handler" - midi_value = util.renormalize( - self.parameter.value, - self.parameter.minimum, - self.parameter.maximum, - self.midi_min, - self.midi_max, - ) - return int(_clamp(midi_value, 0, 127)) + return self.to_midi(self.parameter.value) def _compute_multiplier(self, rotations: int) -> float: now = time.monotonic() diff --git a/plugins/audio_midi/source.py b/plugins/audio_midi/source.py index ffe073efd..45994ae03 100644 --- a/plugins/audio_midi/source.py +++ b/plugins/audio_midi/source.py @@ -106,7 +106,7 @@ def set_param_value(self, symbol: Symbol, value: float) -> None: self._hardware.recalibrateVU_gain(value) p = self.parameters.get(symbol) if p is not None: - p.value = value + p.reconcile(value) def subscribe(self, cb: Callable[[Parameter], None]) -> Callable[[], None]: unsubs = [p.subscribe(cb) for p in self.parameters.values()] diff --git a/tests/integration/test_tap_tempo.py b/tests/integration/test_tap_tempo.py index 2702864e7..e9c99d4b7 100644 --- a/tests/integration/test_tap_tempo.py +++ b/tests/integration/test_tap_tempo.py @@ -6,10 +6,35 @@ def test_set_mod_tap_tempo(modhandler_system: SystemFixture): - """set_mod_tap_tempo() POSTs to /set_bpm with the BPM value.""" + """set_mod_tap_tempo() sends BPM over the WebSocket, not the blocking POST, + and reports that the value left.""" handler = modhandler_system.handler mock_post = modhandler_system.mock_post + assert handler.set_mod_tap_tempo(120) is True + + assert "transport-bpm 120" in modhandler_system.ws_bridge.sent + mock_post.assert_not_called() + + +def test_set_mod_tap_tempo_reports_failure_when_send_never_leaves(modhandler_system: SystemFixture): + """Backpressure plus a rejected POST means the value never left — commit + relies on this False to roll the LCD back.""" + handler = modhandler_system.handler + modhandler_system.ws_bridge.send_bpm = MagicMock(return_value=False) + failed = MagicMock() + failed.ok = False + modhandler_system.mock_post.side_effect = lambda *a, **k: failed + + assert handler.set_mod_tap_tempo(120) is False + + +def test_set_mod_tap_tempo_falls_back_to_post_under_backpressure(modhandler_system: SystemFixture): + """A refused WebSocket send (backpressure) falls back to POST /set_bpm.""" + handler = modhandler_system.handler + mock_post = modhandler_system.mock_post + modhandler_system.ws_bridge.send_bpm = MagicMock(return_value=False) + handler.set_mod_tap_tempo(120) mock_post.assert_called_once() diff --git a/tests/test_parameter_steps.py b/tests/test_parameter_steps.py index fa910bfbc..eca64427a 100644 --- a/tests/test_parameter_steps.py +++ b/tests/test_parameter_steps.py @@ -59,7 +59,7 @@ def __init__(self, parameters: dict[Symbol, Parameter]) -> None: def set_param_value(self, symbol: Symbol, value: float) -> None: p = self.parameters.get(symbol) if p is not None: - p.value = value + p.reconcile(value) class _ConcretePluginPanel: diff --git a/tests/v3/test_encoder_value_resync.py b/tests/v3/test_encoder_value_resync.py index fa394212b..98e141f89 100644 --- a/tests/v3/test_encoder_value_resync.py +++ b/tests/v3/test_encoder_value_resync.py @@ -57,7 +57,7 @@ def test_value_continues_from_nav_change(v3_system: SystemFixture): assert param.value > 50.0 # Nav encoder (simulated via direct write, as parameter_value_change does). - param.value = 90.0 + param.preview(90.0) # One detent forward — continues from 90, not from the pre-nav position. enc.refresh(1) @@ -70,7 +70,7 @@ def test_value_stays_near_nav_change_on_backward_turn(v3_system: SystemFixture): enc = _bound_tweak(v3_system, param) enc.refresh(5) - param.value = 80.0 + param.preview(80.0) enc.refresh(-1) assert param.value > 60.0 diff --git a/tests/v3/test_footswitch_param_sync.py b/tests/v3/test_footswitch_param_sync.py index 66264c195..8a5e4ebb3 100644 --- a/tests/v3/test_footswitch_param_sync.py +++ b/tests/v3/test_footswitch_param_sync.py @@ -66,7 +66,7 @@ def test_press_on_writes_param_to_max(v3_system: SystemFixture, make_plugin, mak def test_press_off_writes_param_to_min(v3_system: SystemFixture, make_plugin, make_parameter): handler, fs0, solo = _bind_solo_footswitch(v3_system, make_plugin, make_parameter) fs0.toggled = True - solo.value = solo.maximum + solo.reconcile(solo.maximum) event = SwitchEvent(controller=fs0, kind=SwitchEventKind.PRESS, timestamp=1000.0) assert handler.handle(event) is True diff --git a/tests/v3/test_reactive_parameter.py b/tests/v3/test_reactive_parameter.py index 70bf73864..92e43ab50 100644 --- a/tests/v3/test_reactive_parameter.py +++ b/tests/v3/test_reactive_parameter.py @@ -108,28 +108,102 @@ def _open_fullscreen(v3_system: SystemFixture, plugin: Plugin) -> _TrackedFullsc # --------------------------------------------------------------------------- -# 1. Parameter.value setter notifies on change, skips on no-change +# 1. reconcile notifies on change, skips on no-change # --------------------------------------------------------------------------- -def test_param_value_setter_notifies_on_change_not_on_noop(): - """Writing param.value from any site notifies; an unchanged write does not.""" +def test_reconcile_notifies_on_change_not_on_noop(): + """A value write notifies observers; an unchanged write does not.""" info: PortInfo = {"shortName": "x", "symbol": "x", "ranges": {"minimum": 0, "maximum": 1}} p = Parameter(info, 0.0, None, "inst") calls: list[Parameter] = [] p.subscribe(lambda param: calls.append(param)) - p.value = 1.0 + p.reconcile(1.0) assert len(calls) == 1 assert calls[0] is p - p.value = 1.0 # unchanged — no notification + p.reconcile(1.0) # unchanged — no notification assert len(calls) == 1 - p.value = 0.5 + p.reconcile(0.5) assert len(calls) == 2 +def test_commit_publishes_and_stays_pending_until_reconciled(): + """A committed edit publishes, holds its value, and reads pending until the + single writer echoes it back.""" + info: PortInfo = {"shortName": "x", "symbol": "x", "ranges": {"minimum": 0, "maximum": 200}} + p = Parameter(info, 120.0, None, "inst") + sent: list[float] = [] + + class OkSink: + def publish(self, param: Parameter) -> bool: + sent.append(param.value) + return True + + p.sink = OkSink() + p.commit(150.0) + + assert p.value == 150.0 + assert sent == [150.0] + assert p.pending + + p.reconcile(150.0) + assert not p.pending + + +def test_commit_rolls_back_when_publish_never_leaves(): + """A send that doesn't leave reverts the value to the last confirmed one — + the LCD must never show a number the single writer didn't accept.""" + info: PortInfo = {"shortName": "x", "symbol": "x", "ranges": {"minimum": 0, "maximum": 200}} + p = Parameter(info, 120.0, None, "inst") + seen: list[float] = [] + p.subscribe(lambda param: seen.append(param.value)) + + class DeadSink: + def publish(self, param: Parameter) -> bool: + return False + + p.sink = DeadSink() + p.commit(150.0) + + assert p.value == 120.0 + assert not p.pending + assert seen == [150.0, 120.0] # painted optimistically, then reverted + + +def test_settled_fires_on_reconcile_and_commit_not_preview(): + """subscribe_settled fires for a reconcile (even unchanged) and a successful + commit, but never a bare preview — and not a rolled-back commit.""" + info: PortInfo = {"shortName": "x", "symbol": "x", "ranges": {"minimum": 0, "maximum": 200}} + p = Parameter(info, 120.0, None, "inst") + settled: list[float] = [] + p.subscribe_settled(lambda param: settled.append(param.value)) + + p.preview(130.0) + assert settled == [] # a scrub does not settle + + p.reconcile(130.0) # echo confirming the previewed value — unchanged + assert settled == [130.0] # ...still settles, unconditionally + + class OkSink: + def publish(self, param: Parameter) -> bool: + return True + + p.sink = OkSink() + p.commit(140.0) + assert settled == [130.0, 140.0] + + class DeadSink: + def publish(self, param: Parameter) -> bool: + return False + + p.sink = DeadSink() + p.commit(150.0) + assert settled == [130.0, 140.0] # rolled back — did not settle + + def test_subscribe_returns_unsubscriber(): """The returned callable tears down the subscription.""" info: PortInfo = {"shortName": "x", "symbol": "x", "ranges": {"minimum": 0, "maximum": 1}} @@ -137,11 +211,11 @@ def test_subscribe_returns_unsubscriber(): calls: list[Parameter] = [] unsub = p.subscribe(lambda param: calls.append(param)) - p.value = 1.0 + p.reconcile(1.0) assert len(calls) == 1 unsub() - p.value = 0.0 + p.reconcile(0.0) assert len(calls) == 1 # no more notifications after unsubscribe @@ -151,12 +225,12 @@ def test_plugin_subscribe_fans_out_to_all_params(make_plugin): calls: list[Parameter] = [] unsub = plugin.subscribe(lambda param: calls.append(param)) - plugin.parameters[BYPASS_SYMBOL].value = 1.0 - plugin.parameters[Symbol("gain")].value = 0.9 + plugin.parameters[BYPASS_SYMBOL].reconcile(1.0) + plugin.parameters[Symbol("gain")].reconcile(0.9) assert len(calls) == 2 unsub() - plugin.parameters[BYPASS_SYMBOL].value = 0.0 + plugin.parameters[BYPASS_SYMBOL].reconcile(0.0) assert len(calls) == 2 # unsubscribed @@ -539,5 +613,5 @@ def test_dismissed_dialog_unsubscribes(v3_system: SystemFixture, make_plugin): dialog.pop() v3_system.handler.poll_lcd_updates() - gain.value = 0.75 + gain.reconcile(0.75) assert dialog.last_param_value == 0.5 # never redrew after dismissal diff --git a/tests/v3/test_transport_bindings.py b/tests/v3/test_transport_bindings.py index 618365821..e0ecc99a0 100644 --- a/tests/v3/test_transport_bindings.py +++ b/tests/v3/test_transport_bindings.py @@ -90,9 +90,10 @@ def test_transport_plugin_unconditional_fallback(v3_system: SystemFixture): assert ROLLING_SYMBOL in tp_zero.parameters -def test_reactive_bpm_parameter_change_triggers_set_mod_tap_tempo(v3_system: SystemFixture): - """Writing to transport_plugin.parameters[BPM_SYMBOL].value reactively notifies - subscribers and triggers set_mod_tap_tempo.""" +def test_bpm_edit_publishes_but_reconcile_does_not(v3_system: SystemFixture): + """The provenance split: `edit()` is a local command and publishes upstream; + the plain setter / `set_param_value` is a remote reconcile and publishes + nothing. mod-ui stays the single writer with no flag to suppress.""" from unittest.mock import MagicMock handler = v3_system.handler @@ -101,18 +102,21 @@ def test_reactive_bpm_parameter_change_triggers_set_mod_tap_tempo(v3_system: Sys _attach_transport_plugin(handler) ws_bridge.send_bpm = MagicMock(return_value=True) + bpm = handler.current.pedalboard.transport_plugin.parameters[BPM_SYMBOL] - # Change BPM parameter value directly (e.g. via encoder or set_param_value) - tp = handler.current.pedalboard.transport_plugin - tp.set_param_value(BPM_SYMBOL, 148.0) + # Reconcile (mod-ui echoing its own state): adopt the value, send nothing. + bpm.reconcile(148.0) + handler.current.pedalboard.transport_plugin.set_param_value(BPM_SYMBOL, 149.0) + ws_bridge.send_bpm.assert_not_called() - # Verify reactive subscriber triggered send_bpm - ws_bridge.send_bpm.assert_called_once_with(148.0) + # Local edit (a knob turn): set and publish through the sink. + bpm.commit(150.0) + ws_bridge.send_bpm.assert_called_once_with(150.0) -def test_transport_message_ws_suppresses_bpm_echo(v3_system: SystemFixture): - """An incoming WebSocket TransportMessage updates transport parameters without - echo-calling send_bpm back to mod-ui.""" +def test_transport_message_reconciles_without_echo(v3_system: SystemFixture): + """An incoming TransportMessage reconciles the transport parameters through the + plain setter, so :bpm's sink never echoes send_bpm back at mod-ui.""" from unittest.mock import MagicMock handler = v3_system.handler @@ -510,6 +514,68 @@ def test_encoder_bpm_turn_without_websocket_bridge_falls_back_to_rest_post( assert mock_post.call_args[1]["json"] == {"value": 121.0} +def test_encoder_bpm_turn_does_not_post_when_websocket_accepts(v3_system: SystemFixture, make_plugin): + """The POST is a backpressure fallback, not a companion to the send — it blocks + the 10ms loop and an encoder spin calls it once per detent.""" + from pistomp.input.event import EncoderEvent + + handler = v3_system.handler + hw = v3_system.hw + mock_post = v3_system.mock_post + assert handler.current is not None + + handler.current.pedalboard.plugins = [make_plugin("noise", bypassed=False)] + enc1 = next(e for e in hw.encoders if e.id == 1) + channel, cc = _binding_for(hw, enc1).split(":") + _attach_transport_plugin(handler, bpm_cc={"channel": int(channel), "control": int(cc)}) + + mock_post.reset_mock() + handler._handle_encoder(EncoderEvent(controller=enc1, rotations=1, multiplier=1.0)) + + assert any("transport-bpm 121.0" in m for m in v3_system.ws_bridge.sent) + mock_post.assert_not_called() + + +def test_encoder_bpb_turn_still_emits_midi_cc(v3_system: SystemFixture, make_plugin): + """Only :bpm leaves by WebSocket. :bpb and :rolling have no other way out, so + their bound encoders must keep emitting CC.""" + from unittest.mock import MagicMock + from pistomp.input.event import EncoderEvent + + handler = v3_system.handler + hw = v3_system.hw + assert handler.current is not None + + handler.current.pedalboard.plugins = [make_plugin("noise", bypassed=False)] + enc1 = next(e for e in hw.encoders if e.id == 1) + channel, cc = _binding_for(hw, enc1).split(":") + + # Mock before binding: the CC sink captures its emitter when the encoder binds. + handler._emit_midi = MagicMock() + tp = _attach_transport_plugin(handler, bpb_cc={"channel": int(channel), "control": int(cc)}) + handler._emit_midi.reset_mock() + + handler._handle_encoder(EncoderEvent(controller=enc1, rotations=1, multiplier=1.0)) + + assert tp.parameters[BPB_SYMBOL].value == 5.0 + handler._emit_midi.assert_called_once() + + +def test_transport_dialog_commit_never_param_sets(v3_system: SystemFixture, make_plugin): + """mod-ui rejects param_set on the transport designations, so a :bpb dialog + commit must not emit one — it would draw a "specially designated port" error.""" + handler = v3_system.handler + assert handler.current is not None + + handler.current.pedalboard.plugins = [make_plugin("noise", bypassed=False)] + tp = _attach_transport_plugin(handler) + + v3_system.ws_bridge.sent.clear() + handler.parameter_value_commit(tp.parameters[BPB_SYMBOL], 6.0) + + assert not any("param_set" in m and ":bpb" in m for m in v3_system.ws_bridge.sent) + + def test_encoder_bpm_turn_parameter_dialog_snapshot(v3_system: SystemFixture, make_plugin, snapshot): """Turning a BPM-bound encoder 1 detent notch displays the parameter dialog on the LCD at 121 BPM.""" from pistomp.input.event import EncoderEvent diff --git a/uilib/parameterdialog.py b/uilib/parameterdialog.py index 5393569fc..ba84783cf 100644 --- a/uilib/parameterdialog.py +++ b/uilib/parameterdialog.py @@ -296,7 +296,7 @@ def tick(self): def update_value(self, new_value: float) -> None: """Update display with new value (controller already calculated it).""" self.reset_timeout() - self.parameter.value = new_value + self.parameter.preview(new_value) def parameter_value_change(self, direction, count: int = 1, multiplier: float = 1.0): self.reset_timeout() @@ -311,7 +311,7 @@ def parameter_value_change(self, direction, count: int = 1, multiplier: float = if new_value == self.parameter.value: return - self.parameter.value = new_value + self.parameter.preview(new_value) if self.action is not None: self.action(self.object, new_value)