217 midi unlearning tweak12 bug / address footswitch complete teardown#218
217 midi unlearning tweak12 bug / address footswitch complete teardown#218jpremill wants to merge 4 commits into
Conversation
|
@sastraxi I still haven't setup the deployment to the hardware device,so again this is all based on the reasonable assumption of the tests passing. The test snapshots were very helpful to identify issues, but I wasn't able to manually test using the emulator. I suspect it has something to do with the MIDI registration on mac silicon. |
|
@jpremill I gave some tips on how to deploy in the other PR! Will be reviewing this one shortly. By the way, I hope you don't mind, I had to change the upstream branch due to some shuffling around on my end. There's a small merge conflict now. |
| controller = self.hardware.controllers.get(binding) | ||
| old_controller = self.hardware.controllers.get(old_binding) if old_binding is not None else None | ||
|
|
||
| if old_controller is not None and old_binding != binding: |
There was a problem hiding this comment.
This should be fine after the addition of binding_range to fix a bug with the "Advanced" binding features of MOD-UI (we were ignoring the parameter range before), but it would be good to have a test that explicitly makes sure a different range bound on the same parameter still works as we expect it to. Sorry for the churn.
|
@sastraxi I saw the guidance; its just a matter of making the time to do it. I just have been finding a few minutes in the evening to make some progress; I think after this changeset, i'll get the env setup before pushing changes. |
da82043 to
19ea5aa
Compare
|
@jpremill awesome! No rush. I'm going to be away for a week starting Sunday, so there may be some delays from my end in review. |
3 participants
fixes #217
I addressed the early return in _apply_midi_binding that allowed unmapping to reach old_controller.unbind_from_parameter(), which exposed that footswitch unmapping was incomplete — Footswitch was missing an unbind_from_parameter() override to clear its parameter label/category, and update_footswitch() was missing an else: branch to repaint unmapped switches back to grey.I noticed this after the first commit. I was hesitant to continue the fix just based on needing to update the footswitch too, but I suppose this is the whole point of the testing.