Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Add iSerial field to Native USB devices #3811

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

Merged
cmaglie merged 1 commit into arduino:master from facchinm:nativeUSB_serialNumber
Oct 21, 2015

Conversation

Copy link
Member

@facchinm facchinm commented Sep 17, 2015

Windows PCs always believe that an USB device with a particular triplet of VID/PID/SerialNumber will never change during its existence, thus saving the device's "nature" on the Registry.
With the introduction of PluggableUSB, a device can change its "composition" between a sketch and another if we include different headers. But the new device does not necessarily implement all the capabilities of the one that has been saved, leading to drivers not loading etc.

To solve this problem, two different approaches where tested:

  • creating a S/N based on the descriptor length (collisions still possible, less overhead)
  • craft a S/N string from the pluggable modules properties (no collision, no difference between different HID submodules) (this seems better)

A call has been added (getShortName) and it MUST be implemented by any library based on PluggableUSB. No effect on PluggableHID-based libraries.

The patchset is composed by two patches (which will be squashed afterwards) to show the first approach on AVR (first patch)

@NicoHood @matthijskooijman @PaulStoffregen @cmaglie

Copy link
Member Author

Copy link
Contributor

Thx for the CC.

Does it really make Windows see this as different device? I never knew about this ISERIAL feature, is there any docs where I can read this up?

I guess the lufa equivalent is this:
https://github.com/abcminiuser/lufa/blob/master/Projects/USBtoSerial/Descriptors.c#L63

But you know, that changing an HID device to another device also causes problems. For example if you add a mouse and the next time its a gamepad. Sometimes windows then recognizes the gamepad axis as mouse and other strange things. Luckly I dont use windows anymore. How do you want to fix this issue?

Copy link
Member Author

The solution is simply to tell Windows that any combination of composite device is another device by changing the iSerial field (LUFA equivalent is perfect).

Anyway, I thought that every HID-compliant device would be compatible, as it appears only one on Device manager.
If you tell me that Win has strange behaviour also with different HID submodules I'd combine the two approaches so any device is "unique" using a signature composed by modules name + descriptor length; in this way HI56 (HID with Mouse) will be different from HI128 (HID with Keyboard and Joystick) and HI56 will also be different from MI56 (MIDI).

What do you think about that?

Copy link
Contributor

Doesnt this lower the number of possible devices, if the string is longer? Does it has a max length?
I just said what I remember. I have not tested the pluggable HID with windows and I wont be able to do this again. If it bugs someone, we can fix it later again (upgrade this patch). For now I'd leave it like that. Also for windows 8 you can always uninstall the HID drivers in "Devices and printers".

I suggest to keep it like that (havent tested yet, but use linux anyways), and if someone reports a problem to my HID project, I will come back here again. For the normal Arduino purposes (keyboard + mouse) this should be fine.

Before we can use the HID-Project the .a linkage for libraries needs to be implemented for 1.6.6. My PR is out of date because the Arduino builder is now coded in go which I dont get. and not a single line was documented. So I dont know how to go on with this anyways. If this is all fixed and wokring we can test if the ISerial solution is good enough or not.

@cmaglie cmaglie added feature request A request to make an enhancement (not a bug fix) Component: USB Device Opposed to USB Host. Related to the USB subsystem (SerialUSB, HID, ...) Library: HID The HID Arduino library labels Sep 28, 2015
Copy link
Member Author

Patches rebased on newer PluggableUSB API

Copy link
Contributor

How much chars may the ISERIAL have? Does it have to be X chars or X-Y? (With up to 6 custom endpoints this may exceed the limits)

Have you tested how much overheads this adds? I guess a lot.

Copy link
Member Author

I've arbitrarily chosen 20 chars as limit, since

  • CDC doesn't add anything
  • HID can add a max of 6 bytes ("HID" + length_descriptor -> max "999")
  • MIDI adds 4 bytes ("MIDI")

So we are plenty of spare space with 20bytes (I believe)
Since I'm doing a lot of testing on Win I believe this issue should be addressed now and not after the first bunch of random issues appear 😉

Copy link
Contributor

Just asking for the USB limitation (if any).

How big is the overhead? Is it really needed? I had no problems with linux, but well its linux G

Are you going to make the CDC Serial pluggable (or anyone else)?

Have you already updated the MIDI? Does it work with an endpoint OUT or via control EP out? I am wondering how one would get the interrupt.

Copy link
Member Author

I wanted to answer you two days ago about making CDC pluggable but I totally forgot 😄
The main issue there is that some hook in irq callback must be added, with a (probably) excessive overhead .
This is why MIDIUSB library uses an OUT endpoint to receive data but it polls for that data instead than relying on the interrupt-based structure.

To be back in topic, Linux and OSX users do not suffer the issue since their drivers structure doesn't use any non-volatile backing storage

Copy link
Contributor

You mean the flush IRQ? Maybe this can be added as special weak callback. If the Serial is not used it just calls nothing? Havent looked deeper into that, but deactivating the Serial would be nice.

Or we would deactivate the USB core . If we do this we can get rid of everything or even implement another USB core with very special features. I also though of adding the Lufa to Arduino as USB core. Not sure if the building system would work that simple.

ontopic:
It would be nice to add this feature via a #define so I can save this overhead. The more HID devices you have, the more overhead it will give. Or maybe there is another solution? I'd like to avoid this overhead.

Copy link
Member Author

squashed into a single commit + memory improvements
/cc @cmaglie

@cmaglie cmaglie merged commit b945df9 into arduino:master Oct 21, 2015
Copy link
Contributor

Meh for the overhead, thumbsup for windowsusers.
Have you added this to the wiki docs?

@facchinm facchinm deleted the nativeUSB_serialNumber branch October 21, 2015 16:04
@@ -35,6 +35,7 @@ class PluggableUSBModule {
virtual bool setup(USBSetup& setup) = 0;
virtual int getInterface(uint8_t* interfaceCount) = 0;
virtual int getDescriptor(USBSetup& setup) = 0;
virtual uint8_t getShortName(char *name) { name[0] = 'A'+pluggedInterface; return 1; }
Copy link
Contributor

Choose a reason for hiding this comment

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

Isnt this line consuming a lot memory? I havent tested it but I think the conversion from int to char could take some overhead. Okay its just the default implementation, but maybe we can improve this?

Copy link
Member

Choose a reason for hiding this comment

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

Yes we can surely improve, are you suggesting to do something like:

name[0] = 'A' + ((uint8_t)pluggedInterface)

?

Copy link
Contributor

Choose a reason for hiding this comment

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

Nevermind I missunderstood the line. It is correct.

I thought it does something similar as in java, where it merges them together as string (with 2 chars). But this is just fine like that. However its not really save, but for this reason users should overwrite this function of course.

But I'd still like to be able to remove this ISERIAL if possible. Its just so useless for linux users. Has someone measured the added overhead?

@ffissore ffissore modified the milestone: Release 1.6.6 Oct 26, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Labels
Component: USB Device Opposed to USB Host. Related to the USB subsystem (SerialUSB, HID, ...) feature request A request to make an enhancement (not a bug fix) Library: HID The HID Arduino library
Projects
None yet
Milestone
Release 1.6.6
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /