Skip to content

set bpm from encoder via ws#216

Open
jpremill wants to merge 24 commits into
TreeFallSound:prereleasefrom
jpremill:feat/set-bpm-from-encoder-via-ws
Open

set bpm from encoder via ws#216
jpremill wants to merge 24 commits into
TreeFallSound:prereleasefrom
jpremill:feat/set-bpm-from-encoder-via-ws

Conversation

@jpremill

Copy link
Copy Markdown

based on feedback to address #215 based on #213 conversation. I tried to keep it more atomic than I am used to;
I have only verified via the the tests on these and have not tested on. physical the device yet.

sastraxi and others added 22 commits July 20, 2026 18:00
# Conflicts:
#	tests/v3/test_footswitch_table_dispatch.py
# Conflicts:
#	pistomp/controller_manager.py
# Conflicts:
#	pistomp/controller_manager.py
…pass MIDI CC emission for bound transport parameters
@jpremill
jpremill marked this pull request as draft July 22, 2026 03:09
@jpremill
jpremill marked this pull request as ready for review July 22, 2026 03:11
Comment thread modalapi/modhandler.py Outdated
Comment on lines +431 to +435
# Transport BPM parameters bypass 7-bit MIDI CC emission for high-precision WebSocket transport.
if c.parameter is None or not (
c.parameter.instance_id == Pedalboard.TRANSPORT_INSTANCE_ID and c.parameter.symbol == BPM_SYMBOL
):
self._emit_midi(c, emit_value)

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.

Parameters are reactive (subscription based), so I wonder if instead of detecting at action-time we can proactively change their subscription instead. I think it would be easier to maintain.

Comment thread modalapi/modhandler.py Outdated
Comment on lines +883 to +890
tp = self.current.pedalboard.transport_plugin
if tp is not None:
if ROLLING_SYMBOL in tp.parameters:
tp.set_param_value(ROLLING_SYMBOL, 1.0 if msg.rolling else 0.0)
if BPM_SYMBOL in tp.parameters:
tp.set_param_value(BPM_SYMBOL, msg.bpm)
if BPB_SYMBOL in tp.parameters:
tp.set_param_value(BPB_SYMBOL, msg.beats_per_bar)

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.

I think we can skip a lot of these nullability checks. I don't see a reason to ever not instantiate a transport plugin; every pedalboard is "playable" even if the user never uses that feature. And when would we not want to store those parameters? Basically, we should make setting tp params always safe.

I know this part of the design might be inherited... I still wonder if we can do better.

Comment thread modalapi/pedalboard.py Outdated
Comment on lines +241 to +249
def _build_transport_plugin(self, time_info: dict | None) -> Plugin.Plugin | None:
"""The /pedalboard pseudo-instance carrying :bpm/:bpb/:rolling. Built
from mod-ui's timeInfo block. None when the board reports no transport
metadata (available == 0 or absent)."""
if not time_info:
return None
available = int(time_info.get("available", 0) or 0)
if available == 0:
return None

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.

I would like to tighten the return type to Plugin.Plugin and create a stub transport plugin if the pedalboard itself doesn't have one. MOD-UI's

transport {rolling} {bpb} {bpm} {sync}

message is broadcast unconditionally, regardless of how a board is saved. It saves -1 for each unmapped param, and if it for some reason doesn't exist, we also know it's unmapped. But the parameters are still there. They'll just be unbound if the struct doesn't exist.

IMO we should a) unconditionally create the plugin and b) unconditionally create each parameter. Pretty sure the bitmask isn't useful for us. WDYT?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I think I don't have enough time in the codebase to really have an opinion so I took your suggest as gospel.

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.

3 participants