-
Notifications
You must be signed in to change notification settings - Fork 203
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
Add files via upload #101
Add files via upload #101
Conversation
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.
Thanks for the changes, see below for individual comments.
Also, the changelog text here needs a lot of work, read up on how to properly describe a commit. ANd we need a signed-off-by as is documented in the Linux kernel documentation
|
||
} else { | ||
// For text output | ||
char *serial = get_dev_string(udev, desc->iSerialNumber); |
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.
This is odd, the serial number should work to be retrieved this way as well.
I think your device is broken and does not follow the specification, as this is required to be a string and not "hex values", correct? Have you asked if the device passes the USB certification process?
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.
I read about and agree that the ST.-Link/V2 device does abuse serial
number field. String should be a UTF-16 LE text.
But in reality it contains e.g. 0x06 and 0x02 as a character code.
The higher bytes are always zero.
Here my 2 devices with the modified lsusb:
5400ff006d00060072006600530053003000400002006700
4900ff007100060049007000530055004400460006008700
The device itself is working as it should with exception of the serial number.
I'm not the only one with this problem:
@@ -3829,6 +3937,13 @@ int main(int argc, char *argv[]) | |||
" -d vendor:[product]\n" | |||
" Show only devices with the specified vendor and\n" | |||
" product ID numbers (in hexadecimal)\n" | |||
" -f FORMAT\n" |
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.
You have two different things being added in one change. One is the format operation, which is nice to have. The second is your new ability to handle serial numbers in hex format, which is different.
Please break this up into at least 2 different changes and resubmit them as a patch series.
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, right. I thought better to add some also useful functions instead of making a quick and dirty modification.
You really need to split? I hoped to show you that It's a useful feature even if normally only needed with devices, which breaks the rules of the serial number descriptor.
To be honest: If you make a release without the %x output it's useless for me :-(
It was the main reason why I made this changes.
And there are definitely devices with this bug :-(
If you really need it in two steps, I can split. Please send a note.
Hello Greg, You asked me about a "signed-of-by". Read about what it is but don't find a place to add it somewhere in github and I think to put it somewhere into the source is a bad idea. Burkhard |
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.
This whole commit is not needed.
Please rebase your changes such that you have unique commits with different changelog texts that do not all have the same subject, and you remove the ones that are not needed. As for the signed-off-by, that goes in the changelog text, look at all of the other commits in this tree for examples of how to do that (do a 'git log' on the repo). If you clean that all up, and make a set of patches that only do one logical thing per commit, I'll be glad to review them. |
Closing due to slow response. Please feel free to fix up and resubmit and I will gladly review. thanks! |
Hello Greg,
thank you for your description how to send patches ;-)
I added a new option -f to lsusb to be havea more flexible list format and to be able to print the serial number in hexadecimal.
Reason is, that I've several ST-Link/V2 programmer, which use binary data for serial number string :-(
Now I can switch to the hex dump and can grep the device I'm looking for ;-)