-
Notifications
You must be signed in to change notification settings - Fork 124
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
query windows registry for all COM ports + Modem
class devices
#84
Conversation
Modem
class devices
let guid_buffer = &mut guids[class_start_idx..]; | ||
// Find out how many GUIDs are associated with this class name. num_guids will tell us how many there actually are. | ||
let res = unsafe { | ||
SetupDiClassGuidsFromNameA( |
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.
We should probably be using the unicode (SetupDiClassGuidsFromNameW
) or automatic (SetupDiClassGuidsFromName
) versions of all windows functions. Otherwise devices from non-english speaking manufacturers or users in different localities may run into issues getting correct device names and information.
See: https://learn.microsoft.com/en-us/windows/win32/intl/unicode-in-the-windows-api
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.
Note that the API used hasn't changed in this PR, only the additional class being queried
https://github.com/serialport/serialport-rs/pull/84/files#diff-130c30419f44f04eb2238fe2f4482481ecc923ed216b379b214de25165b43dfeL37
I'm fine with updating to using the _W
version (and update the registry query to do the same) here, or making a separate PR (which would make more sense to me if there's more instances than these two functions to update. I haven't looked)
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.
In that case, I think updating these in another PR instead of putting additional changes here makes more sense to me!
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.
See #89
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've tested this on my Windows 11 hardware and detection is still working well, thanks for the PR!
some refactoring of `get_ports_guids` to handle multiple class names
…s could show up as)
According to issue serialport#187 null-modem ports are appearing as CNCxy and there have not been complaints about other devices showing up unintendedly so far.
a500e17
to
0720d5a
Compare
Thank you very much for your contribution @Crzyrndm and the review @mlsvrts! I finally got the time to look into it and check it out on the current main branch.
I changed the filtering back to just exclude ports starting with |
As noted in #81 the list of ports returned from
serialport::available_ports
in windows is incomplete when using v4.2.0.This PR does the bare minimum to present a complete listThe easiest way to get the full list (to my knowledge) is to query the registry, although that gives basically zero information about the additional keys
and as such this PR adds the ports with type "Unknown". This at least makes those ports visible through "available ports". To my understanding, the device driver can choose which category it comes under causing the generic "ports" category to be incomplete for certain devices.Future work would involve digging up more information about these "unknown" ports including the "human" name visible in device managerEDIT
This now resolves #81
The additional commits (9e7791a14df3193494ccbd82f25de872f9d7dbd2..c3bf38bba9b4166aeeb7bab29872d6c916bd880d) refactor
get_ports_guids
to also get the guids forModem
class devicesFiltering of devices returned by iterating has been inverted (
!name.starts_with("COM")
instead of the previousname.starts_with("LPT")
) because there is potentially more than LPT devices to filter outCOM ports that don't belong to either of these categories will still be added to the "unknown" list ensuring that the list of available ports is always complete, even if missing detailed information