Blocking reads() from usb.core.Device

[todbot]

CircuitPython version

Adafruit CircuitPython 9.1.0-beta.1-17-g9ab8831d38 on 2024-05-02; Adafruit Feather ESP32-S2 TFT with ESP32S2

Code/REPL

import time
import board
import usb
import max3421e

import adafruit_usb_host_midi

spi = board.SPI()
cs = board.D10
irq = board.D9

host_chip = max3421e.Max3421E(spi, chip_select=cs, irq=irq)

while True:
   midi_usb_device = None
   for device in usb.core.find(find_all=True):
      try:
         midi_usb_device = adafruit_usb_host_midi.MIDI(device)
         print("Found vid/pid %04x/%04x" % (device.idVendor, device.idProduct),
               device.manufacturer, device.product)
      except ValueError:
         print("bad device:", device)

  while True:
     print("this should print 10 times a second")
     time.sleep(0.1)
     b = midi_usb_device.read(1)
     if b:
        print("should only print when data received:", b)

Behavior

Code blocks on read(), only printing "this should print 10 times a second" when a byte is read.

Description

The code demonstrates how device.read() blocks infinitely, which is different from established behavior.

The expected semantics of device.read() when using either busio.UART or usb_midi.PortIn is that the read() does not noticably block if a small timeout is given.
(a timeout given explicitly in the constructor of busio.UART, a common practice, and implicitly in how usb_midi works)

But for the usb.core.Device that is returned and used in adafruit_usb_host_midi, the device.read() blocks forever. Using asyncio or select does not help with this.

There is a non-standard timeout argument to the usb.core.Device.read()
and usb.core.Device.write() methods. However, when specifying a timeout on .read() (by modifying adafruit_usb_host_midi.MIDI.read()), the .read() throws
an unattributed usb.core.USBError, instead of the expected return value of None or 0.
Since the exception is unattributed, there's no wayt to distinguish a read timeout
from a device unplug or other USB error.

Additional information

Tested on USB Host Feather Wing and ESP32-S2 TFT Feather and ESP32-S3 Reverse TFT Feather.
Discovered by @jedgarpark so tagging him here too.

[jedgarpark]

thanks for submitting this, @todbot I'm trying to add button presses for the ESP32-S3 reverse TFT Feather's D0, D1, D2 buttons for things like UI and to send MIDI panic, while using the USB MIDI Host function on the attached MAX3421e Feather Wing

[dhalbert]

@jedgarpark Are you using keypad for the buttons? That will work if the MAX3241e reads don't suppress interrupts: I would assume they don't.

[todbot]

The problem is that code flow entirely blocks on .read(). If there’s no incoming data, the call to keys.events.get() never gets called.

[jedgarpark]

@dhalbert I was using debouncer, but didn't try keypad. I tried doing it with asyncio as well, but still was being blocked until a MIDI message came through.

[dhalbert]

Debouncer will not try to read the switch until its async task runs. keypad will read the switch transition immediately and save it in the queue, but to read the queue, you still have to wait and not be blocked.

[jedgarpark]

@dhalbert ok, i've set it to use keypad. it now can show me a queued button press occurred the next time i play a MIDI note. not useable for how i'm planning to use buttons in this MIDI monitor project, but good to know keypad can queue up presses

[todbot]

The usb.core.Device.read() also blocks on RP2040 PIO USB host.

Here's an example that should print out "doing UI things" every 0.1 seconds but blocks on read().

import time
import board
import usb.core
import usb_host
import adafruit_usb_host_midi

usb_host.Port(board.MOSI, board.MISO)  # GP3,4 on QTPy RP2040; GP16,17 on RP2040 USB Host Feather

print("Looking for midi device")
raw_midi = None
while raw_midi is None:
    for device in usb.core.find(find_all=True):
        try:
            raw_midi = adafruit_usb_host_midi.MIDI(device)
            print("Found vid/pid %04x/%04x" % (device.idVendor, device.idProduct),
                  device.manufacturer, device.product)
        except ValueError:
            continue
    time.sleep(0.1)

last_time = 0
while True:
    midi_bytes = raw_midi.read(1)  # this blocks instead of returning 0 or None
    
    if midi_bytes:
        print(time.monotonic(), "midi:",["0x%02x" % b for b in midi_bytes])
    
    if time.monotonic() - last_time > 0.1:
        last_time = time.monotonic()
        print(last_time, "doing UI things...")

[jepler]

The read method is documented as having a timeout argument in milliseconds. Did y'all give that a try? https://docs.circuitpython.org/en/latest/shared-bindings/usb/core/index.html#usb.core.Device.read

[jepler]

oh I see that the midi library is an intermediate here and probably can't provide the timeout argument. well, that complicates things.

[dhalbert]

Also from the first post:

There is a non-standard timeout argument to the usb.core.Device.read()
and usb.core.Device.write() methods. However, when specifying a timeout on .read() (by modifying adafruit_usb_host_midi.MIDI.read()), the .read() throws
an unattributed usb.core.USBError, instead of the expected return value of None or 0.
Since the exception is unattributed, there's no wayt to distinguish a read timeout
from a device unplug or other USB error.

[jepler]

At least it looks like the timeout error should be possible to distinguish, it is of type USBTimeoutError, while all other errors are of type USBError. Both derive from OSError. It's true they don't have any relevant attributes, but an except clause should be able to catch them as different things, by having the except clause for the more specific USBTimeoutError first.

[dhalbert]

I do agree it should just maybe just return instead of raising an error.

[jepler]

fbofw I think this module strives for compatibility with pyusb

[todbot]

Unfortunately, adding a try/except for usb.core.USBTimeoutError doesn't work. I've modified adafruit_usb_host_midi to accept a timeout in its constructor that it passes to usb.core.Device.read(), and the USBTimeoutError is thrown once then a USBError is thrown.

Full gist here: https://gist.github.com/todbot/07a2ef16ec9ee83eb8980f8a128d660b
Tested on a Feather TFT ESP32-S2 with MAX3421E FeatherWing.

[jepler]

USBTimeoutError is thrown once then a USBError is thrown.

Ah, I see.

[jepler]

The build in #9240 may be helpful for further diagnosis. I tried to mark each site in the source that might be throwing the USBError exception with a different string. so, what string is shown with the 2nd exception that "should have been" USBTimeoutError?

[todbot]

The build in #9240 may be helpful for further diagnosis. I tried to mark each site in the source that might be throwing the USBError exception with a different string. so, what string is shown with the 2nd exception that "should have been" USBTimeoutError?

Thanks. I tried the artifact from PR #9240 and the result is usb.core.USBError: tuh_edpt_xfer() failed after one usb.core.USBTimeoutError.

[tannewt]

I think we'll need @hathach to look into this because we try to abort the pending transfer when we reach the timeout:

circuitpython/shared-module/usb/core/Device.c

Lines 201 to 204 in ca2a24e

	if (result == 0xff) {
	tuh_edpt_abort_xfer(xfer->daddr, xfer->ep_addr);
	mp_raise_usb_core_USBTimeoutError();
	}

This should allow the subsequent call to work ok. It may be weird because our transaction is being NAKed by the device each SOF.

The USBTimeoutError is pyusb behavior it does here:
https://github.com/pyusb/pyusb/blob/4a433a5fddd6dcbc632454d53f57e020b0b5513b/usb/backend/libusb0.py#L445-L446

[hathach]

ah, abort xfer isn't implemented on max3421 just yet, though it should work with pio-usb. Let me try to reproduce and double check with the rp2040 first.

[todbot]

ah, abort xfer isn't implemented on max3421 just yet, though it should work with pio-usb. Let me try to reproduce and double check with the rp2040 first.

I believe the RP2040 PIO host does implement abort xfer. At least the above CircuitPython code works as expected on rp2040 and doesn’t throw the error.

[hathach]

@todbot sorry, I misread your above statement with rp2040 as ""it does not work with pio-usb as well". PR hathach/tinyusb#2646 implements abort xfer for max3421e. It should work, please try it out to see if that work for you

[todbot]

Hi @hathach, thanks for the patch! It fixes the issue so now max3421e USB host mode behaves the same as usb_host.Port PIO USB host mode on RP2040: timing out instead of erroring.

To test, I make a local build of CircuitPython for adafruit_feather_esp32s2_tft incorporating this update to tinyusb and @jepler's labeled USBErrors. My test code now times out in the same manner as similar code using usb_host.

I guess when CircuitPython updates TinyUSB next, this issue can be closed.

[todbot]

One thing I can notice now though with the max3421e module: the minimum timeout passed to usb.core.Device.read() seems to be 100 milliseconds. Longer timeouts work, but shorter timeouts do not. Shorter timeouts are sort of critical for doing anything useful.

[tannewt]

What happens with shorter timeouts?

[todbot]

What happens with shorter timeouts?

Any timeout less than 100ms results in a 100ms timeout.

[tannewt]

What happens with shorter timeouts?

Any timeout less than 100ms results in a 100ms timeout.

Does it work correctly on RP2-PIO USB host? The timeout code is the same (but background tasks or timekeeping could be messing us up.)

[todbot]

What happens with shorter timeouts?
Any timeout less than 100ms results in a 100ms timeout.
Does it work correctly on RP2-PIO USB host? The timeout code is the same (but background tasks or timekeeping could be messing us up.)

Ignore my reported 100ms timeout error. The error was in the todbot (me), not the code.

Both the RP2 PIO USB Host (usb_host.Port) and the MAX3421E USB Host (max3421e) now heed a timeout in usb.core.Device.read(). Yay! With this and adding timeout to adafruit_usb_host_midi to give it more typical stream read semantics, it can be used non-blocking with adafruit_midi. Thus I think this issue is resolved. (I can submit a PR onadafruit_usb_host_midi if you like)

btw, when specifying the minimum read timeout (1 millisecond), PIO USB does take 1ms, but with the max3421e it takes 2-3 milliseconds on average. Not deal-breaking, but something I wanted to note.

[tannewt]

Fixed by #9252. Thanks @todbot !

[hathach]

glad that works out, feel free to pin me whenever you need my help