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.
| # 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 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
There was a problem hiding this comment.
@jpremill would still suggest removing this. The service is no longer considered to be healthy if the WS bridge is not up.
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)
sastraxi
left a comment
sastraxi
left a comment
There was a problem hiding this comment.
Good to merge after the final comments are addressed!
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.