[WIP] Parameter sinks#219
Draft
sastraxi wants to merge 6 commits into
Draft
Conversation
Defects in the WebSocket BPM path, isolated in one commit so it can be reverted cleanly when John's fixes land upstream. - The encoder emit guard keyed on "is transport", but only :bpm has a high-precision channel; the guard suppressed CC for :bpb and :rolling too, which have no other way to mod-ui. Key on the symbol so they keep emitting CC. - set_mod_tap_tempo ignored send_bpm's return, so every detent also fired a blocking POST on the 10ms loop. The POST is now the backpressure fallback its test always claimed it was. - _last_bpm_change_time was written and never read. - Restore the early return in parameter_value_commit's audio arm; without it an audio parameter that gains a binding would emit a stray CC. Records why timeInfo's `available` mask is deliberately discarded, and why param_set never carries the transport designations (mod-ui rejects them). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The PR wired :bpm's tempo send through a Parameter observer, which made a plain `param.value = x` write emit a WebSocket packet. That coupling forced `_suppress_bpm_event` — a re-entrancy flag guarding the one call site (TransportMessage) where a *remote* write must not echo back at the sender. Every future write to :bpm was a trap someone had to remember to suppress. Split the two reasons a value moves into two verbs: - `param.value = x` / `set_param_value` — a remote reconcile. mod-ui is the single writer; adopt the value, notify observers to repaint, publish nothing. - `param.edit(x)` — a local command (knob turn, dialog commit). Set the value and publish it upstream through the parameter's `sink`. A reconcile now has no send to suppress: the distinction is which method the caller reaches for, not a flag on a shared setter. `_suppress_bpm_event`, the subscribe/unsub bookkeeping, and `_on_bpm_param_changed` are gone. `ParamSink` (common/param_source.py) is the outbound dual of the existing `ParamSource` protocol. :bpm's sink routes to set_mod_tap_tempo; it is attached at bind time. The encoder and dialog-commit paths for :bpm call edit(); the CC emit and param_set arms are unchanged for every other parameter. This is the transport slice. Extending sinks to CC / param_set / audio so parameter_value_commit's dispatch collapses entirely is a natural follow-up. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Privatize Parameter's raw setter and route every write through a verb that
names its provenance:
reconcile(v) adopt the single writer's value — repaint, mark confirmed, send
nothing. Also the optimistic-preview channel the dialog scrubs on.
preview(v) a local move not yet committed — repaint, leave unconfirmed.
commit(v) a finished edit — repaint, publish through the sink.
_confirmed tracks the last value mod-ui echoed back; the gap from _value is
`pending`. With that ledger, commit can fix a real bug: if the sink's send never
leaves (WS backpressure and POST both fail) it rolls the value back to
_confirmed, so the LCD can no longer show a BPM mod-ui never accepted.
ParamSink.publish and set_mod_tap_tempo now return that success flag.
The language no longer has a word for "just set it," so every call site declares
which motion it is. preview and reconcile share mechanics today; the ledger is
the seam where they'd diverge.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The keycap sync moves off Plugin.set_param_value's hand-rolled controller loop onto a reactive channel. A parameter now carries two observer lists: the live one (subscribe) that repaints panels on any change including a preview, and a settled one (subscribe_settled) fired only by reconcile and commit — never a bare preview — and fired unconditionally, even at an unchanged value. That split is the keycap's existing invariant made explicit: a local footswitch press previews the value and refreshes its own toggle/LED, waiting for the echo; a menu/dialog commit and the mod-ui echo both settle it and refresh the keycap now. StatefulController.bind_to_parameter subscribes to the settled channel through the _unsub_param lifecycle already scaffolded for it, so set_param_value collapses to a bare reconcile and neither write path reaches for controllers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
parameter_value_commit re-derived a param's upstream on every commit — a five-arm if-chain over transport/plugin/audio/external. Give each parameter its sink at bind time instead: _sink_for maps provenance to a channel (_PluginParamSink, _ExternalCcSink, _AudioParamSink, _TransportBpmSink), and the commit collapses to param.commit(value). A param edited before bind, or added to a live board, gets its sink on first commit. The plugin path no longer reconciles on a local commit: it stays pending until mod-ui echoes it back, matching the single-writer discipline the other sinks already follow. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…aram _handle_encoder forked bpm (commit through its WebSocket sink) from every other bound param (preview + a separate CC emit). Drop the fork: a bound turn is c.parameter.commit(new_value), and the param's sink picks the transport — _MidiCcSink for a mapped encoder, _TransportBpmSink for :bpm (its range won't survive 7 bits). The unbound turn keeps its fallback CC for MIDI-learn. _MidiCcSink absorbs the old _ExternalCcSink: a mapped plugin param and an external route are the same send now. The param carries the value; the controller carries the MIDI mechanics — bar_midi_value splits into a pure to_midi(value) so the sink reads the value off the param and asks the controller only to convert and route. The parameter stays MIDI-agnostic. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There are a few different ways of doing parameter reactivity... recent changes in this area made me think we're not going far enough in the subscription model. This is an attempt to fix that. I'm not sure yet if it's the right approach.