MIDIUSB
MIDIUSB copied to clipboard
arduino-libraries
Add attachInterrupt to MIDIUSB.h
Add attachInterrupt method to MIDIUSB.h so that sketch can define a callback function.
Example sketch included.
Is this SAMD-specific code in the example meant to be part of the public API?
USB->DEVICE.INTENCLR.reg = 0xFF; //SAMD interrupts continuously without this.
That part is probably SAMD specific but it addresses a bug in issue #65.
It can be removed from the example. The attached handler is not getting called repeatedly, the ISRHandler in USBCore.cpp is. The call stops that unrelated problem from occurring.
Perhaps there is a device agnostic way to address issue #65.
The proposed solution is indeed samd-only since the handleEndpoint callback is not part of PluggableUSB generic APIs (beside the line to disable continuous interrupts which should be fixed at core level here ).
I like the idea but the library is intended to run on a few platforms and should behave the same on all of them, or at least limitations should be documented.
This PR is not about issue #65. It is about adding attachInterrupt to MIDIUSB.h for its own sake, using the fact that handleEndpoint is in the the public section of PluggableUSB.h. The example sketch is just showing how to use the proposed attachInterrupt method, it's not about the content of the callback.
I think you are saying that handleEndpoint in PluggableUSB.h is only supported for SAMD. If that's the case, it is my mistake, and this PR can be disregarded.
My main concern is for any public API to not require CPU specific hacks, especially not writing to specific peripheral registers.
A secondary concern is exposing more use of interrupt context via public APIs. Arduino already has plenty of precedent for this with functions like attachInterrupt(). But use of interrupt context is error prone and difficult to do correctly, even for experts.
Sadly, Arduino lacks any sort of API where events can be triggered from interrupt context and then later cause user code to run in main program context. @facchinm, @cmaglie and I have talked about this before, but it's a daunting project to add such a large and consequential API to Arduino.
I know it is easy to dismiss concerns about the safety of interrupt context. But consider that even this example makes several calls to Serial1.print() from its interrupt function. Is that necessarily safe? What happens if any other part of the program also uses Serial1? What happens if events occur so rapidly that Serial1's transmit buffer fills up? Even if this is always safe on SAMD, will Serial1 use work on other platforms? Calling other libraries from a different interrupt than they use is a recipe for future bugs.
Arduino as a platform really does need a safe API for libraries like MIDIUSB to have users a way to respond to events....
Agreed!
I was concerned that the parameters of attachInterrupt in this proposal do not match those of existing implementations in other libraries. Also, the callback param of "ep" is probably asking for trouble since it is offered without explanation.
While it is pretty cool that this works, and the interrupt only fires when a message arrives, I don't think it offers an advantage to polling available(), unless there is a case when you want your loop code to always give up control to the incoming messages.
I'll leave this up to your judgement.
