set bpm from encoder via ws#216
Conversation
…pass MIDI CC emission for bound transport parameters
rreichenbach
left a comment
rreichenbach
left a comment
There was a problem hiding this comment.
Since this is more his domain, I'll defer to Cam on the code, but I did test this on hardware and it works just as I would expect. The control maps fine, and changing the setting tracks whether from Encoder, Footswitch or MOD-UI.
| # Emission is hardware-level, below the table (see input/README.md). | ||
| self._emit_midi(c, emit_value) | ||
| # 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: |
There was a problem hiding this comment.
We need to still send MIDI for beats-per-bar and rolling; this should look at the symol
| return | ||
|
|
||
| if not self._is_pedalboard_loading: | ||
| if not self._is_pedalboard_loading and param.instance_id is not None and param.instance_id != Pedalboard.TRANSPORT_INSTANCE_ID: |
There was a problem hiding this comment.
Same as above (branch on symbol not instance ID)
| # Audio parameter (volume, EQ, etc.) - handled locally, no remote update needed | ||
| if param.instance_id is None: | ||
| self.audio_parameter_commit(param.symbol, value) | ||
| return |
There was a problem hiding this comment.
We need this early return otherwise the audio parameters will emit CCs below.
| def set_mod_tap_tempo(self, bpm): | ||
| def set_mod_tap_tempo(self, bpm: float | None) -> None: | ||
| if bpm is not None: | ||
| self._last_bpm_change_time = time.time() |
There was a problem hiding this comment.
I think maybe left over from an earlier design?
| if self.ws_bridge is not None: | ||
| self.ws_bridge.send_bpm(bpm) |
There was a problem hiding this comment.
Why do we have to send this over WS? Was the REST laggy? Regardless we need to put _rest_post in the else clause to avoid a double-send. But I'd prefer to just do it one way or the other.
| def test_encoder_bpm_turn_without_websocket_bridge_falls_back_to_rest_post( | ||
| v3_system: SystemFixture, make_plugin | ||
| ): |
There was a problem hiding this comment.
This test could be deleted once we decide REST vs WS
sastraxi
left a comment
sastraxi
left a comment
There was a problem hiding this comment.
This is close! A couple real bugs here worth taking a look at.
It would be helpful if you could test on device. Personally, I like to use ./deploy.sh to just copy local files over to test, but you might prefer @rreichenbach 's approach which is to edit files directly on the device. There's a helper you can use to turn the deployed source code back into a git repository if it helps:
…MIDI CC bypass, audio param early return, and test enhancements)
4 participants
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.