Device rewrite

[IARI]

Added the concept of MidiActions.
It comprises not only the action to be executed, when the MidiAction is triggered, but also the storing/saving of the binding of midi-messages in the device.
A MidiAction is a decorator that can be applied to Methods anywhere in the Program.
The method which is decorated contains the actual program-code that should be executed, when the MidiAction is triggered. In the most simple case it would be a method that is also a handler for a gui-element of the program.
the decorator registers the MidiAction in a global place. every time a midi action is used from some place in the code, it will tell the midi action which device should be used. the devace - just as before - is merely a container for the data that can be de/serialized easily

There are three types of midi actions:

• MidiAction - regular single-trigger midi action
• MidiRowAction - a midi action that stores a one-dimensional list of bindings
• MidiGridAction - a midi action that stores a two-dimensional list of bindings

Each of them comes in two variants:

• normal: messages with velocity 0 will be ignored
• continuous: all messages are interpreted

When midi-messages are learned, not the complete message, but only a key is stored in the device.
the key is the original midi messages without the velocity.
when the action is triggered, however, the velocity is simply routed through to the method.

This restriction makes it impossible to bind midi messages with the same signature that differ in velocity only to different midi actions. For instance a button that would send the same midi message with velocity 0 and 127 cannot trigger different things on push and release. However, this should not be encouraged anyways.
If this should ever become an issue in the future - say someone wants to add actions that have a relevant "on" and "off" part to them, it is recommended to subclass MidiAction and make a new model that reflects this behavior (store two midi messages).

[Vampouille]

The purpose of this pull request is to simplify code and also remove duplicate code. There is no issue with current device format, so there is no need to change device format and break compatibility. Please keep device format as is.

[Vampouille]

I think midi message with more than 3 byte are not interesting for superboucle. It is mainly bpm information or power up message from some device. So you can safely just drop thoses messages.

[IARI]

The code simplification does not work with the original midi device as it depends on knowing which mapping in the device is an input.

[IARI]

About velocity, currently superboucle support midi keyboard or midi drum that send velocity based on >real user velocity. If velocity is not 0 or 127 then I consider that midi device send variable velocity.

I assume we're only talking about the "single trigger" events right now (not Volume where velocity is routed through).
It was not easy for me to understand what the code was supposed to be doing, so please correct me if I got it wrong - from my understanding, the relevant code is this:

Learn mode:
If velocity is 0 or 127 remember that in the device-mapping. Otherwise remember -1.
the code looks like this: self.<midi_action_name> = btn_id

performance:
You check both cases with self.device.<midi_action_name> in [btn_id, btn_id_vel]

I had a long discussion with @sonejostudios and opposed the idea at first, but in the end implemented it: I just dont't save the velocity in the mapping at all.
The result should behave exactly like your code from how i understand it except for one disadvantage:
Velocity 0 and 127 can no longer be distinguished.

Whether this is relevant boils down to the following:
Are there Midi devices are, where you would want to distinguish - i.e. that send messages that are identical except for velocity which could have distinct meanings ?

clearly there are buttons that would send push- and release messages with velocity 0 and 127 respectively. But so far the program does not need to distinguish them: there are no uses for button-release messages. If this would become relevant in the future an elegant workaround is possible by extending the MidiAction class.

Can you see a case might have missed? Or do you have any other reason to believe the velocity would have to be saved in the mapping?

[Vampouille]

If you don't use velocity with device that send 0 and 127 for push and release, then device will trigger 2 events when you push and release button. So this will start and then stop clip with one push and releaase.
-1 is a special value for variable velocity devices.

[IARI]

To avoid this, I am simply ignoring velocity 0 messages.

[IARI]

@Vampouille
What is the current status?
Can you think of any cases, where the behavior implemented would break an existing setup?
Is there anything you think that needs to be changed?
Generally, what do you think about this PR?

@sonejostudios has tested the branch a bit so far and didn't run into problems.

[Vampouille]

This PR have too many modifications. You cannot change file extension, file format... Other people are using superboucle, I don't want to break compatibility.
About zero velocity, you cannot ignore those midi messages.
For me, the purpose of this PR is to remove duplicate codes in device.py. Adding some generic getter setter in device class is a good idea, can you make a little PR with this modification.

[IARI]

Apologies, it seems I haven't made the motivation clear enough.

While the original motivation to start these changes was indeed to remove code duplicates:

The motivation for this PR is far more, namely to improve flexibility with respect to midi actions - I felt it would take any contributor that wanted to add a new feature whith midi bindings hours of unnecessary work, because

• the code was very hard to read/understand (we know code is read 90% and written 10% of the time)
• changes would have to be made in a lot of places (learn.py, manager, manager ui, the device and probably the gui.py too)
• the duplications you mentioned are an additional danger

To me it seemed impossible to add a new midi action with the old code while trying to maintain readability.

With this PR I have tried to solve all 3 issues to the best of what I could do, while trying to stay close to how the program handled midi learning as much as I could.

I absolutely agree, that changing a file format and thereby breaking compatibility comes with a big cost and shouldn't be done without a good reason.

If you believe that my attempt to solve them is not the right way, I'd be glad if we could talk about it.
If you think the problem is not that bad and doesn't need to be handled, then that is fine too, otherwise I gladly like to help with it, if I can.

[IARI]

If you want to add a new Midi-triggerable Action with this version, it is in my opinion a lot easier.

Assume there is a method somewhere which implements an action performed by the program, usually via gui (does not have to be in gui.py, but so far all are) that you want to be midi-learnable (and -triggerable)
The procedure would be as follows:

• add the @MidiAction decorator to the method
  
  this will register it as a midiaction and make it learnable - no change in the device or device format is needed. The Methodname however should be unique, because it is used to identify the midi action and safe it to the device
• add some widgets to the learn-dialog that are supposed to control the learning-process in the device.
  
  If they are similar to the midi actions that already exist nothing more should be required - the learning works via the naming scheme of the gui elements.
  If they are very different in structure, it should be fairly easy to add code that follows the fashion.

It has been a month since I implemented all of this, but if i have not forgotten anything, that should be it.
Sidenote: I think It would theoretically be possible to add special learnable widgets, if someone really wanted to do that.

I was planning to add a sample midi action (load next or previous sond from the playlist) in an additional commit to make the exact code changes visible in the diff.

We've put a lot of thought into the mechanics, but I do not at all claim that it is perfect, and I'd be glad to change things, that don't seem proper, if you were interested in this PR - like we can absolutely talk about the learning of vel-0 messages (I believe it is possible with single-actions, but not with row-actions. with continuous actions also it is no problem.).