Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fixed native USB serial port not appearing on Mac OS X 10.6.8.

  • Loading branch information...
commit 3e9ef444018f64f2eee76909a602dfb440d87ee5 1 parent 05a2d77
@bitron bitron authored
Showing with 4 additions and 1 deletion.
  1. +4 −1 hardware/arduino/sam/cores/arduino/USB/USBCore.cpp
View
5 hardware/arduino/sam/cores/arduino/USB/USBCore.cpp
@@ -385,11 +385,14 @@ static bool USBD_SendDescriptor(Setup& setup)
if (USB_DEVICE_DESCRIPTOR_TYPE == t)
{
TRACE_CORE(puts("=> USBD_SendDescriptor : USB_DEVICE_DESCRIPTOR_TYPE\r\n");)
- if (setup.wLength == 8)
+ if (setup.wLength >= 8)
{
_cdcComposite = 1;
}
desc_addr = _cdcComposite ? (const uint8_t*)&USB_DeviceDescriptorA : (const uint8_t*)&USB_DeviceDescriptor;
+ if( *desc_addr > setup.wLength ) {
+ desc_length = setup.wLength;
+ }
}
else if (USB_STRING_DESCRIPTOR_TYPE == t)
{

3 comments on commit 3e9ef44

@louismdavis

The change at line 388 is causing a USB enumeration problem for the composite device on Windows machines.
The Device Descriptor for a composite device needs to have a Device Class of 0.
On Windows, wLength is 18 bytes for the Get_Descriptor request. Before the change, the code would have returned USB_DeviceDescriptor, which has a Device Class of 0. After the change, the code returns USB_DeviceDescriptorA, which has a Device Class of 2.

This would work, if the device was only a CDC device. This does not work if it is a composite device with CDC and HID.

Since both CDC and HID are defined by default, this causes a problem for all DUEs on Windows systems.

@cmaglie
Owner

lousmdavis,

first thank you, I've seen the forum thread, great work.

This commit was due to the behaviour seen on older macosx (<=10.6.8).

We got the following facts:

  • macosx versions <= 10.6.8 sends a wLenght of 0x12
  • if we keep the == condition (==0x08) the CDC-Serial is not seen from macosx
  • changing the condition to >= 0x08 make it working

Note that this happens only with high-speed devices. If you attach a low-speed device macosx
requests a wLength of 0x08. This probably makes Leonardo unaffected.

I'm going to set the condition back to ==, but my question now is: there is a workaround to make it
working also on macos 10.6.8 and older?

C

@louismdavis

I am not sure why there is a need for conditional code to send a different Device Descriptor depending on the length of the request.

If I understand the USB specs properly, the device should return the same Device Descriptor, no matter what the length is requested.

The USB code looks like it is trying to handle three scenarios:
1. CDC only
2. HID only
3. Composite: CDC and HID

If the USB code is configured as CDC only, CDC_ENABLED defined and HID_ENABLED not defined, then it should have a Device Descriptor with Device Class of 2.
If the USB code is configured as HID only, CDC_ENABLED not defined and HID_ENABLED defined, then it should have a Device Descriptor with Device Class of 0.
If the USB code is configured as Composite, CDC_ENABLED defined and HID_ENABLED defined, then it should have a Device Descriptor with Device Class of 0.

I believe that you could simplify the code and accomplish this behavior by making the following changes to the code:
1. Change the preprocessor macro to set Device Class to 0 if HID is defined, this covers the HID only and Composite cases. Otherwise, set Device Class to 2, this covers the CDC only case.
2. Eliminate the alternate USB_DeviceDescriptor.
3. Remove the conditional code based on request length.

#ifdef HID_ENABLED
#define DEVICE_CLASS 0x00
#else
#define DEVICE_CLASS 0x02
#endif

//  DEVICE DESCRIPTOR
const DeviceDescriptor USB_DeviceDescriptor =
    D_DEVICE(DEVICE_CLASS,0x00,0x00,64,USB_VID,USB_PID,0x100,IMANUFACTURER,IPRODUCT,0,1);

...

if (USB_DEVICE_DESCRIPTOR_TYPE == t)
{
    TRACE_CORE(puts("=> USBD_SendDescriptor : USB_DEVICE_DESCRIPTOR_TYPE\r\n");)

    desc_addr = (const uint8_t*)&USB_DeviceDescriptor;
    if( *desc_addr > setup.wLength ) {
        desc_length = setup.wLength;
    }
}

I haven't fully tested this, but I think this will accomplish what is desired.

Please sign in to comment.
Something went wrong with that request. Please try again.