-
Notifications
You must be signed in to change notification settings - Fork 86
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Initial implementation of USB MIDI SysEx message handling. #634
Conversation
Build available for early initial testing here: Kevin |
Thanks Kevin!!! Tested using USBGadget=1 on RPIZero2. I can confirm Dexed can now modify all TG voice parameters with your changes!!!! Huzzah! |
Great work @diyelectromusic. Do you think this is ready for merging, or would you like to collect more testing/feedback? |
Not sure... I think its ok and it obviously works in some cases, but I'd hardly say it's had a rigorous test :) I guess it can't do much worse than the existing implementation that ignores SysEx completely for USB, but there is a risk I've messed up normal USB MIDI, but I don't think so. I'd like someone to try it with several USB devices though... I might have a go myself at some point, but haven't yet. I'd need to dig several devices out, and none of them really do SysEx. I've never really used more than a single USB device at a time myself. As an aside, I've wondered for a while now if we ought to have a "dev" branch for anyone interesting in living more "on the edge"... or if not, just make it clear that the main branch is essentially a dev branch so if something breaks then we're sorry, but to let us know and pop back a bit later for an update!! :) |
Yes, I think the latter. Otherwise it'd be work, not a hobby ;) |
I think the issue is more what do people coming to the site expect? I think they'd expect a release of MiniDexed to largely work. It isn't their hobby to be fighting with config options and debugging our experimental code - their hobby is probably playing music and they'd like to use MiniDexed to do it. So I don't know - but either way, as I say, we should be setting appropriate expectations. But the pressure for significant testing of PRs prior to integration is quite high when they go live straight away out to non-technical users, not to mention it isn't clear what to download to test every change individually. It would be nice to be able to have them integrated and closed more quickly but then still allow others with a more technical bent to give them a good shake down without pushing that out to those who just want to install MiniDexed and use it. And it would be nice for this to be a regular thing - a known build of all changes so far, so anything willing to keep updating their system will have a known place to go for the latest to help us build confidence in it. I'm not a git person, so don't know what a typical workflow ought to be, but ideally PRs would be closed pretty quickly, so they don't get out of date, once accepted into a dev branch and then when we feel the dev branch has had a reasonable shake down we can move everything forward. A good time to do that would be to track the major steps in circle in my view... But I am very aware that, especially with some of the more recent changes and given the complexity of MiniDexed these days, it feels like the chances of breaking something increase with every PR, especially when we wait such a long time to update circle. Kevin |
I suspect we can probably merge this in before it gets too out of date, following the "its probably working, tell us if you find something that isn't" approach. Kevin |
Thanks @diyelectromusic. Anything we should change in the docs? |
I don't think any docs need to change - I'd certainly always assumed it was already doing this - hence the surprise when it was reported it wasn't! |
Initial attempt to fix the problem described in #633
Has had very little testing so far besides "kind of works for me". Tried on a Pi 3 in USB Gadget Mode with Dexed and MIDIOx.
Not tested:
Please let me know if you get a chance to try any of the above.
Note: I've also removed the additional "noisy" serial MIDI debug line that messes up MIDI debug parsing. That is only required if we're debugging the serial handler itself, so I've commented it out.