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

Fix serial number reporting on Windows for some devices #131

Merged
merged 4 commits into from
Nov 13, 2023

Conversation

xobs
Copy link
Contributor

@xobs xobs commented Oct 29, 2023

Some devices have their serial numbers misreported. For example, the BMP device USB\VID_1D50&PID_6018&MI_02\6&A694CA9&0&0000 gets a reported serial number of 6 rather than A694CA9.

Fix this and update the testcase.

The existing program incorrectly decoded the first character as being
the serial port. If there is an `&` in the serial number, then we want
the 2nd field.

Signed-off-by: Sean Cross <[email protected]>
@xobs
Copy link
Contributor Author

xobs commented Oct 29, 2023

Additionally, several clippy lints were fixed as a result of looking over this code.

Copy link
Contributor

@eldruin eldruin left a comment

Choose a reason for hiding this comment

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

Nice, thank you!
Could you add a short entry to the changelog as well?

@xobs
Copy link
Contributor Author

xobs commented Oct 30, 2023

I'm actually not sure about the validity of this approach.

image

For example, on my system, the serial number shows up as 3aab607b. That's because my hwid is 8&3aab607b&0&0000. However, my actual serial number is 97B6DC14.

It's possible to get the serial number by trimming &MI_00 from the port, then looking for a matching ParentIdPrefix value that matches the hwid.

Is there still interest in this patch if it returns "a" serial number rather than "the" serial number?

@eldruin
Copy link
Contributor

eldruin commented Nov 1, 2023

I see. I am not sure myself. Maybe somebody else can weigh in?

@danielstuart14
Copy link
Contributor

danielstuart14 commented Nov 10, 2023

Yes! Even if it doesn't return "the" serial number, it still is a lot better than before.
With this PR, I can distinguish between two identical devices (each with two CDC endpoints), which I couldn't do before.

Before:
Screenshot 2023-11-10 164936

After:
Screenshot 2023-11-10 164919

@xobs
Copy link
Contributor Author

xobs commented Nov 12, 2023

Sounds good. I've updated CHANGELOG.md as requested.

Copy link
Contributor

@eldruin eldruin left a comment

Choose a reason for hiding this comment

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

Great, thank you!

@eldruin eldruin merged commit 919f9df into serialport:main Nov 13, 2023
21 checks passed
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.

3 participants