mayo commented 1 year ago

Checklist

[ ] Component contains License
[ ] Component contains README.md
[ ] Component contains idf_component.yml file with url field defined
[ ] Component was added to upload job
[ ] Component was added to build job
[ ] Optional: Component contains unit tests
[ ] CI passing

Change description

I have number of USB devices that are simple CDC-ACM "serial" devices, but present themselves with bDeviceClass, bDeviceSubClass, and bDeviceProtocol set to 0x00, which according to the standard (https://www.usb.org/defined-class-codes, Base Class 00h (Device) section) indicates that class information should be determined from the interface descriptors. The implementation to detect the device with the null triple is virtually identical to the already existing IAD device detection.

This change extends the device interface detection to also recognize devices with null class triple.

CLAassistant commented 1 year ago

All committers have signed the CLA.

tore-espressif commented 1 year ago

@mayo thank you very much for the update. Could you please share full configuration descriptors of the devices that you are testing with?

mayo commented 1 year ago

@tore-espressif below is the full device configuration from lsusb for one of the devices. I'll try to get the others once I can get the devices on hand, though they are essentially the same, just different Vendor ID and Product ID.


Bus 020 Device 034: ID 04b4:0003 Cypress Semiconductor Corp. 
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               2.00
  bDeviceClass            0 (Defined at Interface level)
  bDeviceSubClass         0 
  bDeviceProtocol         0 
  bMaxPacketSize0         8
  idVendor           0x04b4 Cypress Semiconductor Corp.
  idProduct          0x0003 
  bcdDevice            0.00
  iManufacturer           1 Rainforest Automation, Inc.
  iProduct                2 RFA-Z105-2 HW2.7.3 EMU-2
  iSerial                 0 
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength           84
    bNumInterfaces          3
    bConfigurationValue     1
    iConfiguration          0 
    bmAttributes         0x80
      (Bus Powered)
    MaxPower              500mA
    Interface Association:
      bLength                 8
      bDescriptorType        11
      bFirstInterface         0
      bInterfaceCount         2
      bFunctionClass          2 Communications
      bFunctionSubClass       2 Abstract (modem)
      bFunctionProtocol       1 AT-commands (v.25ter)
      iFunction               0 
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           1
      bInterfaceClass         2 Communications
      bInterfaceSubClass      2 Abstract (modem)
      bInterfaceProtocol      1 AT-commands (v.25ter)
      iInterface              0 
      CDC Header:
        bcdCDC               1.10
      CDC ACM:
        bmCapabilities       0x02
          line coding and serial state
      CDC Union:
        bMasterInterface        0
        bSlaveInterface         1 
      CDC Call Management:
        bmCapabilities       0x00
        bDataInterface          1
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x83  EP 3 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval              10
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        1
      bAlternateSetting       0
      bNumEndpoints           2
      bInterfaceClass        10 CDC Data
      bInterfaceSubClass      0 Unused
      bInterfaceProtocol      0 
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x01  EP 1 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               0
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x82  EP 2 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               0
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        2
      bAlternateSetting       0
      bNumEndpoints           0
      bInterfaceClass       255 Vendor Specific Class
      bInterfaceSubClass      5 
      bInterfaceProtocol      0 
      iInterface              0 
Device Status:     0x0000
  (Bus Powered)

tore-espressif commented 1 year ago

@mayo Thank you for the quick response.

Now I see, so your device is implementing Interface Association Descriptor, but it doesn't set

bDeviceClass = 0xEF (Miscellaneous Device Class), bDeviceSubClass = 0x02 (Common Class), bDeviceProtocol = 0x01 (Interface Association Descriptor)

so the current implementation does not work for you right?

The changes LGTM! Could you also please bump version to 2.0.1 in https://github.com/espressif/idf-extra-components/blob/master/usb/usb_host_cdc_acm/idf_component.yml#L1 and write short description in https://github.com/espressif/idf-extra-components/blob/master/usb/usb_host_cdc_acm/CHANGELOG.md ?

EDIT: ~~There seems to be an issue with our build system, I'll check~~ Solved

mayo commented 1 year ago

Now I see, so your device is implementing Interface Association Descriptor, but it doesn't set
bDeviceClass = 0xEF (Miscellaneous Device Class), bDeviceSubClass = 0x02 (Common Class), bDeviceProtocol = 0x01 (Interface Association Descriptor)
so the current implementation does not work for you right?

That is correct.

New changes addressing your comments pushed. Thank you for the speedy turnaround!

tore-espressif commented 1 year ago

@mayo Thanks again. PRs like this are very useful because we are not able to test the driver for all possible USB CDC devices.

Will merge after CI passes

tore-espressif commented 1 year ago

Component released: https://components.espressif.com/components/espressif/usb_host_cdc_acm

espressif / idf-extra-components

Add support for null class triple (IEC-40) #228

Checklist

Change description