-
-
Notifications
You must be signed in to change notification settings - Fork 7k
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
Conversation
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: 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? |
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 What do you think about that? |
Doesnt this lower the number of possible devices, if the string is longer? Does it has a max length? 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. |
b9c3fcd
to
8d01a44
Compare
Patches rebased on newer PluggableUSB API |
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. |
I've arbitrarily chosen 20 chars as limit, since
So we are plenty of spare space with 20bytes (I believe) |
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. |
I wanted to answer you two days ago about making CDC pluggable but I totally forgot 😄 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 |
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: |
fff8541
to
b945df9
Compare
squashed into a single commit + memory improvements |
Meh for the overhead, thumbsup for windowsusers. |
@@ -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; } |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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)
?
There was a problem hiding this comment.
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?
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:
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