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

fix(host/cdc): Add case for CDC class in device descriptor (IEC-236) #89

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,11 @@ static const char *TAG = "cdc_acm_parsing";
static bool cdc_parse_is_cdc_compliant(const usb_device_desc_t *device_desc, const usb_config_desc_t *config_desc, uint8_t intf_idx)
{
int desc_offset = 0;
if (device_desc->bDeviceClass == USB_CLASS_PER_INTERFACE) {
if (device_desc->bDeviceClass == USB_CLASS_COMM) {
// 0. This is a Communication Device Class: Class defined in Device descriptor
return true;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of our host tests for descriptors parsing are not passing, because:

Here, device_desc->bDeviceClass == USB_CLASS_COMM we made sure, that this whole device is CLASS_COMM, but we also have to check, whether it's underlying interfaces are USB_CLASS_COMM as well.

There is a CDC device failing our tests, with a following descriptors (SimCom 7070G):

*** Device descriptor ***
bLength 18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass 0x2
bDeviceSubClass 0x0
bDeviceProtocol 0x0
bMaxPacketSize0 64
idVendor 0x1e0e
idProduct 0x9206
bcdDevice 0.00
iManufacturer 3
iProduct 2
iSerialNumber 4
bNumConfigurations 1
*** Configuration descriptor ***
bLength 9
bDescriptorType 2
wTotalLength 154
bNumInterfaces 6
bConfigurationValue 1
iConfiguration 1
bmAttributes 0xe0
bMaxPower 500mA
	*** Interface descriptor ***
	bLength 9
	bDescriptorType 4
	bInterfaceNumber 0
	bAlternateSetting 0
	bNumEndpoints 2
	bInterfaceClass 0xff
	bInterfaceSubClass 0xff
	bInterfaceProtocol 0xff
	iInterface 0
		*** Endpoint descriptor ***
		bLength 7
		bDescriptorType 5
		bEndpointAddress 0x81	EP 1 IN
		bmAttributes 0x2	BULK
		wMaxPacketSize 64
		bInterval 0
		*** Endpoint descriptor ***
		bLength 7
		bDescriptorType 5
		bEndpointAddress 0x1	EP 1 OUT
		bmAttributes 0x2	BULK
		wMaxPacketSize 64
		bInterval 0

According to your changes we marked this interface as CDC compliant (because we marked the whole device CDC compliant) But, in fact, this interface is not CDC compliant bInterfaceClass 0xff if I am not mistaken.

Suggested change
return true;
const usb_intf_desc_t *intf_desc = usb_parse_interface_descriptor(config_desc, intf_idx, 0, NULL);
if (intf_desc->bInterfaceClass == USB_CLASS_COMM) {
return true;
}

Or this suggestion... the idea is more clear here..

    if (device_desc->bDeviceClass == USB_CLASS_PER_INTERFACE || device_desc->bDeviceClass == USB_CLASS_COMM) {
        const usb_intf_desc_t *intf_desc = usb_parse_interface_descriptor(config_desc, intf_idx, 0, NULL);
        if (intf_desc->bInterfaceClass == USB_CLASS_COMM) {
            // 1. This is a Communication Device Class: Class defined in Interface, or Device descriptor
            return true;
        }

With this changes, the host tests are passing correctly. We will update the tests case for this scenario

}
else if (device_desc->bDeviceClass == USB_CLASS_PER_INTERFACE) {
const usb_intf_desc_t *intf_desc = usb_parse_interface_descriptor(config_desc, intf_idx, 0, NULL);
if (intf_desc->bInterfaceClass == USB_CLASS_COMM) {
// 1. This is a Communication Device Class: Class defined in Interface descriptor
Expand Down
Loading