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

query windows registry for all COM ports + Modem class devices #84

Merged
merged 9 commits into from
May 12, 2024

Conversation

Crzyrndm
Copy link
Contributor

@Crzyrndm Crzyrndm commented Mar 17, 2023

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 list

The 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 manager

EDIT

This now resolves #81
The additional commits (9e7791a14df3193494ccbd82f25de872f9d7dbd2..c3bf38bba9b4166aeeb7bab29872d6c916bd880d) refactor get_ports_guids to also get the guids for Modem class devices

Filtering of devices returned by iterating has been inverted (!name.starts_with("COM") instead of the previous name.starts_with("LPT")) because there is potentially more than LPT devices to filter out

COM 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

@Crzyrndm
Copy link
Contributor Author

Crzyrndm commented Mar 25, 2023

See update. With this branch all COM ports are now found with full information
image
(the last two are not found on main)

@Crzyrndm Crzyrndm changed the title query windows registry for all COM ports query windows registry for all COM ports + Modem class devices Mar 25, 2023
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(
Copy link
Contributor

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

Copy link
Contributor Author

@Crzyrndm Crzyrndm Mar 26, 2023

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)

Copy link
Contributor

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!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See #89

Copy link
Contributor

@mlsvrts mlsvrts left a 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!

@sirhcel
Copy link
Contributor

sirhcel commented May 12, 2024

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.

Filtering of devices returned by iterating has been inverted (!name.starts_with("COM") instead of the previous name.starts_with("LPT")) because there is potentially more than LPT devices to filter out

COM 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

I changed the filtering back to just exclude ports starting with LPT for two reasons: First, there hasn't been any report on getting the wrong ports reported on Windows so far. Second, just selecting ports starting with COM would exclude virtual serial ports like the virtual null-modem ports I learned from in #187.

@sirhcel sirhcel merged commit f47c1ed into serialport:main May 12, 2024
30 checks passed
@Crzyrndm Crzyrndm deleted the #81-missing-modem-ports branch May 12, 2024 20:54
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.

Missing COM ports in Windows
3 participants