Skip to content
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

USB enumeration timeouts due to missing ZLPs #553

Open
tlyu opened this issue Feb 1, 2024 · 4 comments
Open

USB enumeration timeouts due to missing ZLPs #553

tlyu opened this issue Feb 1, 2024 · 4 comments
Labels

Comments

@tlyu
Copy link

tlyu commented Feb 1, 2024

USBCore doesn't send required ZLPs for device-to-host transfers. This can cause timeouts during enumeration, among other problems.

Test case outline: create a HID interface with a report descriptor that's exactly 64 bytes long. Attach the device to Windows while monitoring the USB bus. Observe multiple control transfer timeouts and bus resets as Windows tries to get the device to enumerate correctly.

For whatever reason, Windows often (always?) requests 64 bytes more than were advertised for a HID report descriptor. This means that if the descriptor is an exact multiple of wMaxPacketSize, and the device sends fewer bytes than requested by the host, the device must send a ZLP to terminate the transfer. (see USB 2.0 §5.5.3) The missing ZLP causes Windows to time out.

It's also possible to replicate the timeout on other platforms (tested on macOS) by explicitly requesting a control read for more than 64 bytes to retrieve a descriptor that is exactly 64 bytes long.

I don't have a minimized test case yet, but the patch in keyboardio/Kaleidoscope-Bundle-Keyboardio#53 appears to resolve the problem for us.

It's a bit surprising that nobody has (knowingly?) encountered this before. It could also cause problems for configuration descriptor sets or string descriptors.

I can submit the patch as a PR against this repository directly, but I don't currently have a lot of time to create a minimized test case or to do exhaustive testing of the patch.

@per1234 per1234 added the bug label Feb 2, 2024
@per1234
Copy link
Contributor

per1234 commented Feb 2, 2024

Hi @tlyu. Thanks for your report! Is this the same bug that was previously reported here: #112 ?

@tlyu
Copy link
Author

tlyu commented Feb 2, 2024

No. #112 is a related issue with receiving ZLPs in non-control transactions, very likely fixed by the combination of arduino/Arduino#6886 and keyboardio/Kaleidoscope-Bundle-Keyboardio#51. Note that arduino/Arduino#6886 seems to have been stalled for years due to lack of a signed CLA.

@tlyu
Copy link
Author

tlyu commented Feb 3, 2024

Minimal test case: With an Arduino Micro, edit the USB product string in boards.txt to be

Arduino Micro f0123456789abcdef

which is 31 ASCII characters long, and produces a string descriptor exactly 64 bytes long. Compile and upload any sketch that enables the CDC interface (it's enabled by default, I think). That will demonstrate the bug.

I've tested the above case via manual control read on macOS of more bytes than are in the descriptor. On Windows, there is a 5-second timeout on an initial attempt to fetch the string descriptor by requesting 255 bytes. Subsequent requests for the string descriptor do provide correct lengths, and succeed.

(For a HID report descriptor, a timeout will cause Windows to ignore that HID interface, but that's harder to produce a minimal test case for.)

@tlyu
Copy link
Author

tlyu commented Feb 3, 2024

Demo script, using the Python usb1 module (requires libusb):

import usb1

with usb1.USBContext() as uc:
    # uc.setDebug(usb1.LOG_LEVEL_DEBUG)
    dh = uc.openByVendorIDAndProductID(0x2341, 0x8037)
    b1 = dh.controlRead(0x80, 6, 0x0302, 0, 64, 100)
    print(f"requested 64 bytes, got {len(b1)}")
    b2 = dh.controlRead(0x80, 6, 0x0302, 0, 255, 100)
    print(f"requested 255 bytes, got {len(b2)}")

Failure output:

requested 64 bytes, got 64
Traceback (most recent call last):
  File "/Users/tlyu/src/py-hid-utils/avrdesc-demo.py", line 8, in <module>
    b2 = dh.controlRead(0x80, 6, 0x0302, 0, 255, 100)
  File "/Users/tlyu/src/keyboardio/.venv/lib/python3.10/site-packages/usb1/__init__.py", line 1350, in controlRead
    transferred = self._controlTransfer(
  File "/Users/tlyu/src/keyboardio/.venv/lib/python3.10/site-packages/usb1/__init__.py", line 1307, in _controlTransfer
    mayRaiseUSBError(result)
  File "/Users/tlyu/src/keyboardio/.venv/lib/python3.10/site-packages/usb1/__init__.py", line 127, in mayRaiseUSBError
    __raiseUSBError(value)
  File "/Users/tlyu/src/keyboardio/.venv/lib/python3.10/site-packages/usb1/__init__.py", line 119, in raiseUSBError
    raise __STATUS_TO_EXCEPTION_DICT.get(value, __USBError)(value)
usb1.USBErrorTimeout: LIBUSB_ERROR_TIMEOUT [-7]

Verbose fail log (debug logging): avrdesc-fail-long.txt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants