gdsports / USB_Host_Library_SAMD

USB host library 2.0 for Zero/M0/SAMD
125 stars 39 forks source link

USB MIDI message loss, tips on how to diagnose? #8

Open todbot opened 4 years ago

todbot commented 4 years ago

Hi, This library is perfect for my needs, but I'm finding it is losing MIDI messages. Has anyone else seen this?

To demonstrate the issue, I've create a lightly modified version of the USBH_MIDI_dump.ino file at: https://gist.github.com/todbot/7f05268ed3c5201c163a3e269d2c2d64 It parses out the received buffer into a collection of 4-byte USB-MIDI messages and logs the number of Note-On vs Note-Off messages.

If hooking up a USB MIDI keyboard (I've tried several) and mashing on the keys, you'll notice a mismatch between Note-On and Note-Off messages. You an also hear this if hooking up a serial MIDI synth to USB_MIDI_converter.ino.

I have enabled DEBUG_USB_HOST and can see the startup messages but no errors. I've tested this on a Trinket M0, ItsyBitsy M0 and ItsyBitsy M4, all with the same result.

I'd like to diagnose this issue. I have some experience with USB but not host stacks. Any tips on where to start?

miotislucifugis commented 4 years ago

Yeah, i am experiencing this too. It seems like too much data clogs the pipes. Aftertouch and pitch bend especially.

studiohsoftware commented 3 years ago

SAMD Host library does a good job of enumerating devices, even through a hub (at least through the hub that I have).

The enumeration is carried out by repeated calls to UsbH.Task() within loop() in the ino.

Once the enumeration is done, however, the only way to get data from the devices is by polling them, and each poll reconfigures the device pipe. It's unnecessary, and since SAMD USB host hardware will poll for you, there is no need to do it this way. In any case, the polling drops messages on occasion, probably due to race condition, or delays in polling.

Not only this, but repeated calls to Task() after enumeration result in periodic polling of the hub (if present), which is unnecessary unless USB hot swap is needed.

So once enumeration is done using UsbH.Task(), you can stop calling UsbH.Task(), and configure the device pipe one time, and enable interrupts on it.

The upshot is that your code now does nothing unless a USB event occurs. The downside is that you lose USB hot swap, and also you make assumptions about the endpoint config. In the case of supporting MIDI only, those assumptions are okay.

I plan to publish an interrupt example using a pull request this week. There is only one small addition to usbh_midi.h.

studiohsoftware commented 3 years ago

This example should work with a SINGLE midi device by itself without a hub. https://github.com/studiohsoftware/USB_Host_Library_SAMD/tree/master/examples/USBH_MIDI/USB_MIDI_Using_Interrupts

Note there is one change to the USBHost libarary here (inline function GetEpAddress added at the bottom): https://github.com/studiohsoftware/USB_Host_Library_SAMD/blob/master/src/usbh_midi.h

The interrupt approach requires a fixed EP address on the host side, and that EP must match the EP on the device side. Further, the pipes are a pairing of device address and EP address, so you can't have two pipes using the same EP. So this means that interrupts can not be used to monitor more than one thing (hub or device) if the devices share EP addresses (determined in the devices, not the host).

The current library gets around this because it does not require a fixed EP on the host side. Each time it polls, it configures the pipe for the device address and EP, and then polls the device. Pretty sure this overhead is leading to the dropped messages. If the host re-configures the pipe at the wrong moment, it seems a pending message from the device would be lost.

Feedback is encouraged if it seems anything I have written here is false.

I have added a pull request here: https://github.com/gdsports/USB_Host_Library_SAMD/pull/15

diyelectromusic commented 3 years ago

I've been experiencing MIDI message losses too and looking into it. Curiously there is this comment in Usb.cpp in InTransfer (line 321) related to reading in more data than was expected: // This can happen. Use of assert on Arduino locks up the Arduino. // So I will trim the value, and hope for the best.

Does anyone know which circumstances might trigger this? I've been following through the buffer handling, the use of MIDI_EVENT_PACKET_SIZE (64) to set up buffers, how maxPktSize is set from the device descriptor endpoints (which in the case of my "minikeys" seems to be 64 again, from examining the device descriptors), and so on, but I haven't spotted anything where this might occur unless the underlying SAMD21 hardware can return more data in response to a "dispatch packet"?

But I also haven't found anywhere else in the code where there is a hint that the data handling might stall or data get lost unless Release is called somehow whilst in use (which reset everything). But I don't know ultimately what determines the host transfer rates. If it doesn't ask for packets quick enough could data actually be lost at the minikeys end?

I'm pretty new to USB hosts, so apologies for the confused understanding, but I'm trying to learn how some of this works :)

Kevin

studiohsoftware commented 3 years ago

Hi it seems to me you must use the interrupt approach to avoid dropped messages. If you are doing that and you still see data loss, it could be packet size. There is discussion of that in the comments on the pull request. https://github.com/gdsports/USB_Host_Library_SAMD/pull/15