adafruit / Adafruit_TinyUSB_Arduino

Arduino library for TinyUSB
MIT License
463 stars 119 forks source link

_desc_cfg_buffer is not big enough, but something breaks if I make it bigger. #110

Closed WestfW closed 3 years ago

WestfW commented 3 years ago

Operating System

MacOS

IDE version

1.8.15

Board

Metro M0 Express

BSP version

1.7.3

TinyUSB Library version

1.3.0

Sketch

bug_mcdc.zip

What happened ?

Now that multiple CDC ports are possible, I tried to define four of them:

Adafruit_USBD_CDC USBSer1;
Adafruit_USBD_CDC USBSer2;
Adafruit_USBD_CDC USBSer3;
Adafruit_USBD_CDC USBSer4;

Adafruit_USBD_CDC *ports[CFG_TUD_CDC + 1] = { &Serial, &USBSer1, &USBSer2, &USBSer3, NULL};

void setup() {
  pinMode(LED, OUTPUT);
  Serial1.begin(115200);
  // start up all of the USB Vitual ports, and wait for them to enumerate.
  Serial1.println("Serial 0 begin");
  Serial.begin(115200);   // Do these in order, or
  USBSer1.begin(115200);  //   "bad things will happen"
  USBSer2.begin(115200);
  USBSer3.begin(115200);
}

but I only three show up on my host. Tracking this down, I find that _desc_cfg_buffer[] is only 256 bytes, and sizeof(TUD_CDC_DESCRIPTOR) is 66 bytes, so "obviously" trying to add the 4th CDC descriptor fails.

If I try to increase the size of _desc_cfg_buffer[], something else fails and I don't get any USB connectivity at all, and I'm having trouble tracking down why that would be. :-(

In Adafruit_USBD_Device.h, I see:

  // Provide user buffer for configuration descriptor, needed if total length >
  // 256
  void setConfigurationBuffer(uint8_t *buf, uint32_t buflen);

But I can't find any further documentation or examples on how this should be used, or why it would be different than just increasing the size of _desc_cfg_buffer[]

How to reproduce ?

Adjust CFG_TUD_CDC Try to create more than 3 total CDC interfaces. Observe that only 3 are created.

Debug Log

Additional debug was added:

sizeof CDC descriptor = 66
OK getInterfaceDescriptor(0 xxx 247
sizeof CDC descriptor = 66
OK getInterfaceDescriptor(2 xxx 181
sizeof CDC descriptor = 66
OK getInterfaceDescriptor(4 xxx 115
sizeof CDC descriptor = 66
Bad len from getInterfaceDescriptor(6 xxx 49
void Adafruit_USBD_CDC::begin(uint32_t): 83: Failed

Screenshots

No response

hathach commented 3 years ago

you could manually increase it or, use setConfigurationBuffer() https://github.com/adafruit/Adafruit_TinyUSB_Arduino/blob/master/src/arduino/Adafruit_USBD_Device.h#L78

WestfW commented 3 years ago

I did manually increase it. It stops working, for unknown reasons. Here's the relevant part of the debug log with three CDCs (well, 4 CDCs in the sketch, but failed to put the forth one into the composite descriptor because of the length issue):

USBD Setup Received 80 06 00 02 00 00 CF 00        (Note 00CF = 9 + 3*66)
  Get Descriptor Configuration[0]
  Queue EP 80 with 64 bytes ... OK
USBD Xfer Complete on EP 80 with 64 bytes
  Queue EP 80 with 64 bytes ... OK
USBD Xfer Complete on EP 80 with 64 bytes
  Queue EP 80 with 64 bytes ... OK
USBD Xfer Complete on EP 80 with 64 bytes
  Queue EP 80 with 15 bytes ... OK
USBD Xfer Complete on EP 80 with 15 bytes
  Queue EP 00 with 0 bytes ... OK
USBD Xfer Complete on EP 00 with 0 bytes

USBD Setup Received 00 09 01 00 00 00 00 00 
  Set Configuration
  Open EP 81 with Size = 8
  Open EP 01 with Size = 64
  Open EP 82 with Size = 64
  Queue EP 01 with 64 bytes ... OK
  CDC opened
  Open EP 83 with Size = 8
  Open EP 02 with Size = 64
  Open EP 84 with Size = 64
  Queue EP 02 with 64 bytes ... OK
  CDC opened
  Open EP 85 with Size = 8
  Open EP 03 with Size = 64
  Open EP 86 with Size = 64
  Queue EP 03 with 64 bytes ... OK
  CDC opened
  Queue EP 80 with 0 bytes ... OK
USBD Xfer Complete on EP 80 with 0 bytes

The similar debug with an increase to_desc_cfg_buffer[512] has:

USBD Setup Received 80 06 00 02 00 00 11 01         (0x111 = 9 + 4*66, so it THINKS its doing 4*CDC)
  Get Descriptor Configuration[0]
  Queue EP 80 with 64 bytes ... OK
USBD Xfer Complete on EP 80 with 64 bytes
  Queue EP 80 with 64 bytes ... OK
USBD Xfer Complete on EP 80 with 64 bytes
  Queue EP 80 with 64 bytes ... OK
USBD Xfer Complete on EP 80 with 64 bytes
  Queue EP 80 with 64 bytes ... OK
USBD Xfer Complete on EP 80 with 64 bytes
  Queue EP 80 with 17 bytes ... OK
USBD Xfer Complete on EP 80 with 17 bytes
  Queue EP 00 with 0 bytes ... OK
USBD Xfer Complete on EP 00 with 0 bytes

USBD Setup Received 00 09 01 00 00 00 00 00 
  Set Configuration
  Open EP 81 with Size = 8
  Open EP 01 with Size = 64
  Open EP 82 with Size = 64
  Queue EP 01 with 64 bytes ... OK
  CDC opened
  Open EP

At which point the board hangs :-(

hathach commented 3 years ago

samd can only support up to 8 endpoint including endpoint zero which is 0-7, 4 cdc requires more than that endpoint 8 IN. There should be a better error handling in tinyusb stack. But that is the limit of samd

WestfW commented 3 years ago

Oh! I mis-read the datasheet, somehow getting the idea that the SAMD21 supported the maximum number of endpoints (probably the ambiguity between "16 endpoints" and "16 bi-directional endpoints.")

Sigh. Never mind.

hathach commented 3 years ago

8 bi-directional endpoint is more than most other mcu could do. Going further, rp2040 is a nice choice, it is also supported (Earl Phil) by this library.