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

use IAD Descriptor for device descriptor to follow the specification #5495

Merged

Conversation

cuitoldfish
Copy link
Contributor

issue description

My host device based on Nucleus usb stack failed to enumerate Arduino in below step:

  1. Nucleus send GetDescritpor(device) request with specified 8 byte length to see if bcdUSB and bMaxPacketSize0 used by the device is acceptable. But Arduino send out the DeviceDescriptor with class type 0, subclass type 0, protocol type 0. Nucleus only write down these information at this step.
  2. since Arudino's usb version and packet size is acceptable by Nucleus, Nucleus then send full GetDescritpor(device) request to get full device descriptor. this time Arduino will send out DeviceDescriptorB with class type 0xEF, subclass type 0x02, protocol type 0x01.
  3. then Nucleus send subsequent GetDescriptor(configuration)
  4. Nucleus send subsequent GetDescriptor(interface). Arduino send out interface configuration with IAD. Then Nucleus think this is a wrong behavior because in step 1, the device type doesn't follow IAD specification.

specification

per "USB 2.0 ECN Interface Association Descriptor" and "USB Interface Association Descriptor Device Class Code and Use Model", if one device use IAD, then it must use class type 0xEF, subclass type 0x02, protocol type 0x01

conclusion

Currently Arduino doesn't follow the specification strictly. I guess it might had been implemented before the specification was published, at that time, the host may have own different behavior for IAD. But now, as the specification has been accepted and published, Arduino should update this.

I have tested in Win7 system, and Nucleus, this update works.

….0 ECN Interface Association Descriptor" and "USB Interface Association Descriptor Device Class Code and Use Model"
@NicoHood
Copy link
Contributor

I think I've also noticed this at the time I helped working at the pluggable USB. This always sounded wrong but I never knew why this was implemented. I think arduino should correct this now.

@cuitoldfish
Copy link
Contributor Author

I think it was implemented as a workaround since at that time the standard specification hadn't define the IAD related usage.

@facchinm
Copy link
Member

I'm ok with this one, will merge after a complete test of both CDC and CDC+HID functionality on

  • Win XP pre SP2 (this might be difficult)
  • Win XP SP3
  • Win 7
  • Win 10
  • OSX 10.6
  • OSX 10.10
  • Linux 2.6
  • Linux 4

If anyone is interested in taking part in one of these tests and owns a Micro/Leonardo (or clones), apply here and we'll set up a (common) reduced test suite 😄

@facchinm facchinm self-assigned this Oct 20, 2016
@dlabun
Copy link

dlabun commented Oct 21, 2016

I have a MSDN subscription so I can easily spin up any flavor of WinXP needed but I'll just need a few days to order a Micro... Let me know.

@cuitoldfish
Copy link
Contributor Author

@facchinm is there any plan for arduino to create one automation test environment to do the on target test? and some regression test suite on the target board can be integrated in the CI.

@facchinm
Copy link
Member

@cuitoldfish we already have a test suite but the OS "flavour" choice is not so broad (mainly because most tests are platforms independent). I'll keep you posted as soon as the reduced version is available 🙂
@dlabun it would be great, thank you so so much if you are willing to do this!

@dlabun
Copy link

dlabun commented Oct 24, 2016

@facchinm I have a Micro on order from Adafruit, should have it in my hands tomorrow. Can you send me a test case to run? I'll spin up Win XP Pro 32-bit with no service packs and with SP3 unless you have different or more flavors you'd like tested.

@cuitoldfish
Copy link
Contributor Author

@facchinm it seems there is no stop bits transmitted by the Serial library? No body use Serial::stopbits_

@PaulStoffregen
Copy link
Contributor

Just to save you guys a little time testing, CDC+HID was first supported on XP by SP3 and on Vista by SP1. On Macintosh, 10.7 (Lion) was the first to support it. CDC+HID definitely will not work on XP SP2 and Mac OSX 10.6.

@facchinm
Copy link
Member

@PaulStoffregen , that was one of the points 😄 The tests should check that "advertising" as IAD does not confuse older OS while keeping CDC serial working

@dlabun
Copy link

dlabun commented Oct 25, 2016

I tried running IDE 1.6.12 on WinXP without any service service packs and it will not launch... Runs fine on SP3.

@facchinm
Copy link
Member

@dlabun that's interesting; can you paste here the output of arduino_debug.exe (it's another issue, I know, but any insight would be interesting).
Anyway, the test package is here. It compiles and launches a series of sketches declaring different USB descriptors. The second test will make your mouse move so don't worry 😄
To run it, open a terminal, cd into the extracted folder and launch with ./tester_youros -idepath $your/ide/installation/path
for example:

./tester_linux -idepath /home/martino/arduino-1.6.12/
./tester_osx -idepath ~/Downloads/Arduino.app/Contents/Java/
./tester.exe -idepath "C:\Program Files (x86)\Arduino\"

Copy and paste the output here for validation
Thanks a lot!

@NicoHood
Copy link
Contributor

XP users without SPE3 can use an older version of the library. Seriously, that make absolutely no sense to write wrong outdated code for unsupported microsoft OS. Those people will possibly use IDE 1.0.5 anyways.

@PaulStoffregen
Copy link
Contributor

The tests should check that "advertising" as IAD does not confuse older OS while keeping CDC serial working

Many years ago I tried to do this.

@facchinm
Copy link
Member

Many years ago I tried to do this.

Can you expand on this 🙂 ? Is it difficult/impossible/useless? Do you think we should adopt the IAD descriptor and drop the "GENERIC" one (as per PR request)?

@facchinm
Copy link
Member

Output on Linux 4.7.6

2016/10/25 18:48:54 Scanning for device "2341"...
Protocol: (Defined at Interface level)
Config 01:
--------------
Interface 00 Setup 00
Communications (Abstract (modem)) None
Endpoint 1 IN  interrupt - unsynchronized data [16 0]
--------------
Interface 01 Setup 00
CDC Data (Unused)
Endpoint 2 OUT bulk - unsynchronized data [64 0]
Endpoint 3 IN  bulk - unsynchronized data [64 0]
--------------
2016/10/25 18:49:00 Scanning for device "2341"...
Protocol: (Defined at Interface level)
Config 01:
--------------
Interface 00 Setup 00
Communications (Abstract (modem)) None
Endpoint 1 IN  interrupt - unsynchronized data [16 0]
--------------
Interface 01 Setup 00
CDC Data (Unused)
Endpoint 2 OUT bulk - unsynchronized data [64 0]
Endpoint 3 IN  bulk - unsynchronized data [64 0]
--------------
Interface 02 Setup 00
Human Interface Device (No Subclass) None
Endpoint 0 IN  interrupt - unsynchronized data [64 0]
--------------
2016/10/25 18:49:06 Scanning for device "2341"...
Protocol: (Defined at Interface level)
Config 01:
--------------
Interface 00 Setup 00
Communications (Abstract (modem)) None
Endpoint 1 IN  interrupt - unsynchronized data [16 0]
--------------
Interface 01 Setup 00
CDC Data (Unused)
Endpoint 2 OUT bulk - unsynchronized data [64 0]
Endpoint 3 IN  bulk - unsynchronized data [64 0]
--------------
Interface 02 Setup 00
Human Interface Device (No Subclass) None
Endpoint 0 IN  interrupt - unsynchronized data [64 0]
--------------
Interface 03 Setup 00
Audio (Control Device)
--------------
Interface 04 Setup 00
Audio (MIDI Streaming)
Endpoint 1 OUT bulk - unsynchronized data [64 0]
Endpoint 2 IN  bulk - unsynchronized data [64 0]
--------------
2016/10/25 18:49:12 Scanning for device "2341"...
Protocol: (Defined at Interface level)
Config 01:
--------------
Interface 00 Setup 00
Communications (Abstract (modem)) None
Endpoint 1 IN  interrupt - unsynchronized data [16 0]
--------------
Interface 01 Setup 00
CDC Data (Unused)
Endpoint 2 OUT bulk - unsynchronized data [64 0]
Endpoint 3 IN  bulk - unsynchronized data [64 0]
--------------
Interface 02 Setup 00
Audio (Control Device)
--------------
Interface 03 Setup 00
Audio (MIDI Streaming)
Endpoint 0 OUT bulk - unsynchronized data [64 0]
Endpoint 1 IN  bulk - unsynchronized data [64 0]
--------------
==========================
starting tests on new core
==========================
2016/10/25 18:49:18 Scanning for device "2341"...
Protocol: Miscellaneous Device (?) Interface Association
Config 01:
--------------
Interface 00 Setup 00
Communications (Abstract (modem)) None
Endpoint 1 IN  interrupt - unsynchronized data [16 0]
--------------
Interface 01 Setup 00
CDC Data (Unused)
Endpoint 2 OUT bulk - unsynchronized data [64 0]
Endpoint 3 IN  bulk - unsynchronized data [64 0]
--------------
2016/10/25 18:49:24 Scanning for device "2341"...
Protocol: Miscellaneous Device (?) Interface Association
Config 01:
--------------
Interface 00 Setup 00
Communications (Abstract (modem)) None
Endpoint 1 IN  interrupt - unsynchronized data [16 0]
--------------
Interface 01 Setup 00
CDC Data (Unused)
Endpoint 2 OUT bulk - unsynchronized data [64 0]
Endpoint 3 IN  bulk - unsynchronized data [64 0]
--------------
Interface 02 Setup 00
Human Interface Device (No Subclass) None
Endpoint 0 IN  interrupt - unsynchronized data [64 0]
--------------
2016/10/25 18:49:30 Scanning for device "2341"...
Protocol: Miscellaneous Device (?) Interface Association
Config 01:
--------------
Interface 00 Setup 00
Communications (Abstract (modem)) None
Endpoint 1 IN  interrupt - unsynchronized data [16 0]
--------------
Interface 01 Setup 00
CDC Data (Unused)
Endpoint 2 OUT bulk - unsynchronized data [64 0]
Endpoint 3 IN  bulk - unsynchronized data [64 0]
--------------
Interface 02 Setup 00
Human Interface Device (No Subclass) None
Endpoint 0 IN  interrupt - unsynchronized data [64 0]
--------------
Interface 03 Setup 00
Audio (Control Device)
--------------
Interface 04 Setup 00
Audio (MIDI Streaming)
Endpoint 1 OUT bulk - unsynchronized data [64 0]
Endpoint 2 IN  bulk - unsynchronized data [64 0]
--------------
2016/10/25 18:49:36 Scanning for device "2341"...
Protocol: Miscellaneous Device (?) Interface Association
Config 01:
--------------
Interface 00 Setup 00
Communications (Abstract (modem)) None
Endpoint 1 IN  interrupt - unsynchronized data [16 0]
--------------
Interface 01 Setup 00
CDC Data (Unused)
Endpoint 2 OUT bulk - unsynchronized data [64 0]
Endpoint 3 IN  bulk - unsynchronized data [64 0]
--------------
Interface 02 Setup 00
Audio (Control Device)
--------------
Interface 03 Setup 00
Audio (MIDI Streaming)
Endpoint 0 OUT bulk - unsynchronized data [64 0]
Endpoint 1 IN  bulk - unsynchronized data [64 0]
--------------

@PaulStoffregen
Copy link
Contributor

PaulStoffregen commented Oct 25, 2016

Can you expand on this 🙂 ? Is it difficult/impossible/useless?

I never found a way to make CDC+HID backwards comparability work on both Mac and Windows. On each, I discovered incorrect descriptors that allowed it to work, but the solution for XP SP2 didn't work at all on Macs, and the solution for OSX 10.5 & 10.6 wasn't recognized by any version of Windows. Both required horrible hacks that violate the standards. Not recommended.

@dlabun
Copy link

dlabun commented Oct 25, 2016

The WinXP issue appears to be related to Java being incompatible without SP3 being install. Java itself is throwing an error of "The procedure entry point DecodePointer could not be located in dynamic link library KERNEL32.dll".

@dlabun
Copy link

dlabun commented Oct 26, 2016

@facchinm How long did it take your computer to run the test case? The test case "locked up" on my Win7 machine... This is all I got for information after waiting for 5 minutes:

C:\Users\Douglas\Downloads\arduinoCompositeUSBTest\arduinoCompositeUSBTest>tester.exe -idepath "C:\Program Files (x86)\Arduino"
2016/10/26 13:38:21 Scanning for device "2341"...
Protocol: (Defined at Interface level)
Config 01:
--------------
Interface 00 Setup 00
Communications (Abstract (modem)) None
Endpoint 1 IN  interrupt - unsynchronized data [16 0]
--------------
Interface 01 Setup 00
CDC Data (Unused)
Endpoint 2 OUT bulk - unsynchronized data [64 0]
Endpoint 3 IN  bulk - unsynchronized data [64 0]
--------------
run as superuser
2016/10/26 13:38:32 Scanning for device "2341"...
Protocol: (Defined at Interface level)
Config 01:
--------------
Interface 00 Setup 00
Communications (Abstract (modem)) None
Endpoint 1 IN  interrupt - unsynchronized data [16 0]
--------------
Interface 01 Setup 00
CDC Data (Unused)
Endpoint 2 OUT bulk - unsynchronized data [64 0]
Endpoint 3 IN  bulk - unsynchronized data [64 0]
--------------
Interface 02 Setup 00
Human Interface Device (No Subclass) None
Endpoint 0 IN  interrupt - unsynchronized data [64 0]
--------------
run as superuser

@dlabun
Copy link

dlabun commented Oct 26, 2016

Also, No success at all on WinXP SP3... The test case won't execute as it dies while linking.

Linking everything together...
"C:\Program Files\Arduino\hardware\tools\avr/bin/avr-gcc" -Wall -Wextra -Os -flt
o -fuse-linker-plugin -Wl,--gc-sections -mmcu=atmega32u4  -o "C:\DOCUME~1\ADMINI
~1\LOCALS~1\Temp\testCDC174327871/CDC.ino.elf" "C:\DOCUME~1\ADMINI~1\LOCALS~1\Te
mp\testCDC174327871\sketch\CDC.ino.cpp.o" "C:\DOCUME~1\ADMINI~1\LOCALS~1\Temp\te
stCDC174327871/core\core.a" "-LC:\DOCUME~1\ADMINI~1\LOCALS~1\Temp\testCDC1743278
71" -lm


C:\arduinoCompositeUSBTest>

@facchinm
Copy link
Member

Oh, the first one looks like a problem with the board not being recognized early enough for the following test to start... You are not on a VM right? I'm modifying the go executable to make the timeout longer (and attaching it here of course).
The XP SP3 issue could be related to #2989 . We could try adding some random code or include until it compiles correctly (or use this test case, if reproducible, to solve the problem once for all 😄 )

@dlabun
Copy link

dlabun commented Oct 27, 2016

I was on my physical Win7 PC when the test case wouldn't finish executing... Glad you warned me about the mouse jumping because I freaked me out for a moment.

@cuitoldfish
Copy link
Contributor Author

@facchinm , what's the current progress for this verification?

@facchinm
Copy link
Member

Hi @cuitoldfish , verification is ongoing (slowly) and no problem has been found yet, I believe we can merge it before next AVR core release

@facchinm facchinm added this to the Release 1.8.2 milestone Feb 8, 2017
@cmaglie cmaglie merged commit 40e7b1d into arduino:master Mar 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants