usb-midi-device: Fix handling of incoming MIDI messages under load. - #1163
Conversation
Prevents situation where a high MIDI RX load fills up the schedule() queue with redundant calls to _on_rx() and causes a RuntimeError. Also refactor the callback handler so that a RuntimeError when scheduling doesn't stop RX endpoint transfers from continuing. This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <angus@redyak.com.au>
If the _rx buffer is full then no OUT transfer is submitted to TinyUSB. This meant that completely filling the RX buffer could permanently stall the OUT direction endpoint, as the scheduled callback handler wouldn't re-queue it. Signed-off-by: Angus Gratton <angus@redyak.com.au>
dpgeorge
left a comment
dpgeorge
left a comment
There was a problem hiding this comment.
Thanks, this looks like a good fix.
Maybe (separately) it's worth improving micropython.schedule() so you can ask it to only queue the given function at most once? Or even better, preallocate the queue slot from Python so it's guaranteed to always be able to schedule.
|
Tested this fix too on the Pico 2 [RP2350], and it seems fine. See here. |
|
Thanks @dpgeorge and @psitech!
Yes, I think something like this would be useful to avoid this kind of pattern when the incoming event rate spikes up. |
3 participants
Summary
Fixes #1158 (RuntimeError raised in the USB transfer callback under high load of incoming MIDI messages), in three parts:
micropython.schedule()queue by not scheduling redundant callbacks.micropython.schedule()does fail due to a full queue then the ongoing USB OUT endpoint transfer is still re-queued, to avoid stalling the device permanently.Testing
Note there is still an issue when USB-MIDI data is sent at max line rate to ESP32-S3. That issue is tracked in #1162.
Generative AI
I did not use generative AI tools when creating this PR.