Skip to content

Don't drop on back-pressure; add logging + de-duplication instead - #252

Merged
sastraxi merged 12 commits into
mainfrom
feat/remove-backpressure
Sep 9, 2026
Merged

sastraxi merged 12 commits into
mainfrom
feat/remove-backpressure

Conversation

@sastraxi

@sastraxi sastraxi commented Sep 6, 2026 •

Copy link
Copy Markdown
Collaborator

There's this back-pressure detector that aimed to figure out if MOD-UI was too slow to respond to our requests, mainly introduced for Blend Mode as there can be a ton of param_set interpolations depending on how many parameters you're driving with that feature.

This feature was implemented by dropping new outbound WS messages if the queue was full.

Why we remove it

The number does not measure MOD-UI. ws.transport.get_write_buffer_size() counts bytes in our own transport that we have not given to the kernel. MOD-UI can read all of them and still be slow.

The threshold is never reached. We set it to 8KB. The websockets library pauses only at the 64KB high-water mark. Between 8KB and 64KB nothing is wrong, but the detector reported a problem.

The flag could stay set for the rest of the session. Only the send loop set it and cleared it. A producer that saw the flag refused the message and did not queue it, so the queue could empty while the flag was true. The send loop then waited on _wakeup, and nothing could wake it. This is the bug in #251: the parameter dialog opens, but the value does not change.

It guarded the wrong failure. A reconnect empties the queue. A value sent while the socket was down returned true, commit painted it, and the reconnect deleted it. The LCD kept a value that MOD-UI never received.

What replaces it

send_parameter and send_bpm refuse for one reason: there is no connection. The connect scope sets self.ws and clears it in a finally. The send path never writes it.

Host.load in MOD-UI is not a coroutine, so Tornado runs it on the ioloop and reads no socket while it runs. That is the only condition that fills our buffer, and MOD-UI already brackets it with loading_start and loading_end. Tornado also answers a protocol PING on that ioloop, so ws.latency measures the delay directly. The worker pings every 5 seconds and logs peak latency each minute. There is no ping timeout: it would close the socket during a long board load, and the reconnect would empty the queue.

Inbound, coalesce_param_sets (ws_protocol.py) collapses each drain's param_set burst to the last per (instance, symbol), keeping the survivor at its original position — a fast scrub repaints once per tick instead of once per echo. Safe because the port is level-sampled and the feed is in-order; :bypass arrives as PluginBypassMessage, so bypass echoes are untouched.

Commit / confirm semantics

A send that leaves confirms itself. commit now advances _confirmed on success; only an inbound echo could before. On the no-echo paths — a footswitch-less UI bypass (MOD-UI skips the origin socket; mod-host emits no param_set for a bypass it got from mod-ui), or any WS send, which mod-ui never echoes — _confirmed stayed frozen at the pre-edit value forever, so a later failed commit rolled the screen back to a value neither MOD-UI nor the player held. Rollback now targets the last confirmed value: echoed, or self-committed, whichever came last.

A MIDI-learn sub-range change reclamps _confirmed. set_binding_range/clear_binding_range clamp both the value and _confirmed into the new extents (one _reclamp helper replacing the twin bodies), and fire committed observers — a keycap can no longer sit at a value the new extents exclude, and a failed commit cannot roll back outside them.

MIDI CC sinks refuse the load window. _publish_cc and _publish_switch_cc return False while _is_pedalboard_loading, matching _publish_plugin_param: a scrub mid-load no longer reaches mod-host while the equivalent WS edit is refused — and, with self-confirming commits, no longer poisons _confirmed either. The audio card still always lands.

The volume encoder commits. It wrote the card directly and only previewed the parameter — two stores that could drift, and it bypassed audio_parameter_commit's input-gain VU recalibration. It is one commit through _sink_for → _publish_audio now.

A bound analog control commits its parameter. _handle_analog only emitted CC; the parameter tracked the echo alone, and the LCD bar projected raw ADC while the parameter tracked mod-ui — drifting through every echo-latency window and any WS outage. A bound (non-external) pedal now commits: the CC-space value snaps onto the parameter's own ParameterSteps grid, then commit publishes through _sink_for's new AnalogMidiControl → _publish_cc arm (external pedals keep the raw-CC-only path; their port owns the value). to_midi/bar_midi_value moved to the base Controller so encoder and pedal share the CC lattice, and the LCD bar projects the parameter for bound controls, the ADC for everything else.

Rename. subscribe_settled → on_commit, _notify_settled → _notify_committed: the vocabulary now says what it does — values are previewed or committed, and an echo is just MOD-UI's commit.

Other fixes

Panels did not use _sink_for. PluginPanel._send_param called ws_bridge directly, so a footswitch- or encoder-bound parameter left as a param_set from a panel and as a MIDI CC everywhere else. Panels now call parameter_value_commit.

Two rollbacks could not run. set_param called reconcile, which writes _confirmed — the value commit reverts to. It now calls preview. And modhandler.py:414 passed new_value to display_parameter_value, which previewed the refused value again one line after the rollback. It now passes param.value.

A refused send reverts everywhere. The audio did not change, so a screen that keeps the new value disagrees with what the player hears. The panel no longer retries. _publish_plugin_param and BPM refuse on a dead bridge or a board load; _publish_cc refuses the board load (MIDI is fire-and-forget); the audio card always succeeds.

BPM ignored the load window. set_mod_tap_tempo sent during a pedalboard load, and MOD-UI overwrote the value. The guard is there, not in _publish_bpm, because the tap-tempo footswitch calls it from the callbacks map.

Deleted

The retry in _flush_param_queue. The REST set_bpm fallback, which sent to the same server and port the WebSocket could not reach, with no timeout, on the 10ms loop. get_stats(), which had no callers. docs/backpressure-removal-plan.md, a plan for completed work; the facts worth keeping moved to architecture.md.

What to check

The finally in websocket_bridge.py that clears self.ws. Everything depends on it, and tests/test_websocket_bridge.py covers it. In plugins/base.py, set_param uses preview and _flush_param_queue clears the queue. _sink_for gains an AnalogMidiControl arm below the external bail. The commit/confirm fixes are pinned in tests/v3/test_reactive_parameter.py (test_good_commit_advances_confirmed_without_echo, test_rollback_targets_the_last_confirmed_value), tests/v3/test_sink_routing.py (load-window CC refusal), tests/v3/test_encoder_dispatch.py (volume commits), tests/v3/test_analog_commit.py (bound pedal commits + bar projection; external pedal stays raw-CC), and tests/test_ws_protocol.py (coalesce_param_sets).

@rreichenbach rreichenbach left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine. I didn't test blend mode, but other functions seem right.

@sastraxi
sastraxi marked this pull request as ready for review September 6, 2026 21:12
@sastraxi
sastraxi merged commit 5497941 into main Sep 9, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants