Add `port_number`, `port_chain` for all platform by CCnut · Pull Request #58 · kevinmehall/nusb

CCnut · 2024-06-04T08:11:19Z

Add location paths for USB devices on Windows

src/enumeration.rs

kevinmehall · 2024-06-11T14:13:21Z

I'm wondering what location paths are used for -- what's your use case? This isn't necessarily a problem to add, but I don't want to go so far as caching every device property so wondering where to draw the line. For properties not supported, you can always look them up via the instance ID.

CCnut · 2024-06-12T07:32:53Z

If there are two devices with the same SN/PID_VID/NAME.
When two devices are reconnected, the instance ID cannot distinguish between devices on Windows.

(If the SN is split using the instance ID, the SN of the second device will get the wrong value)

kevinmehall · 2024-06-20T05:57:39Z

Does the existing parent_instance_id (which is the hub) and port_number work to distinguish identical devices?

What would be ideal is a cross-platform way to identify the port, like libusb's libusb_get_port_numbers().

Besides being Windows-specific and not portable, location_paths has the disadvantage of returning multiple strings that you have to deal with somehow. If you really need a particular property that is not supported, the instance ID is exposed so that you can make your own calls to SetupAPI or cfgmgr32. I don't think nusb wants to be an API for all possible Windows device info, though its cfgmgr32 module could be a starting point for writing one.

CCnut · 2024-06-21T04:21:22Z

parent_instance_id has the same problem as instance_id.

Getting location_paths is to get the location of the USB device in the USB device tree, it returns two types of strings, one in the device tree and one in the ACPI tree.

Try enumerating the complete usb device tree to locate the position of the usb device, or use libusb_get_bus_number and libusb_get_port_numbers for similar results.

I will try to implement libusb_get_bus_number and libusb_get_port_numbers first.

CCnut · 2024-06-27T14:28:24Z

The right is the output from libusb.

// like listdevs output
fn main() {
    env_logger::init();
    for dev in nusb::list_devices().unwrap() {
        let chain: Vec<String> = dev.port_chain().map(|v| v.to_string()).collect();
        println!(
            "{:04x}:{:04x} (bus {}, device {}) chain: {}",
            dev.vendor_id(),
            dev.product_id(),
            dev.bus_number(),
            dev.device_address(),
            chain.join(".")
        );
    }
}

kevinmehall

Haven't had a chance to test this or give it a full review, but here are some initial questions.

kevinmehall · 2024-07-10T03:47:54Z

src/platform/windows_winusb/enumeration.rs

+    // Find Port_#xxxx
+    // Port_#0002.Hub_#000D
+    if let Some(location_info) = devinst.get_property::<OsString>(DEVPKEY_Device_LocationInfo) {
+        let s = location_info.to_string_lossy();
+        if &s[0..6] == "Port_#" {
+            if let Ok(n) = s[6..10].parse::<u32>() {
+                return Some(n);
+            }
+        }
+    }
+    // Find last #USB(x)
+    // PCIROOT(B2)#PCI(0300)#PCI(0000)#USBROOT(0)#USB(1)#USB(2)#USBMI(3)
+    if let Some(location_paths) =
+        devinst.get_property::<Vec<OsString>>(DEVPKEY_Device_LocationPaths)
+    {
+        for location_path in location_paths {
+            let s = location_path.to_string_lossy();
+            for b in s.split('#').rev() {
+                if b.contains("USB(") {
+                    if let Ok(n) = b[5..b.len()].parse::<u32>() {
+                        return Some(n);
+                    }
+                    break;
+                }
+            }
+        }
+    }
+    devinst.get_property::<u32>(DEVPKEY_Device_Address)


Is there a reason all three of these techniques are used? Would be worth a comment explaining under what circumstances we'd hit each of these.

This function references the libusb code.
The DEVPKEY_Device_LocationInfo and DEVPKEY_Device_LocationPaths parameters may not return a matching format.

kevinmehall · 2024-07-10T03:48:46Z

src/platform/windows_winusb/enumeration.rs

+        }
+    }
+    // Find last #USB(x)
+    // PCIROOT(B2)#PCI(0300)#PCI(0000)#USBROOT(0)#USB(1)#USB(2)#USBMI(3)


Or, do these #USB(n) elements directly correspond to the port chain that we could just parse from the location path?

This seems to be the case

kevinmehall · 2024-07-10T03:53:54Z

src/platform/windows_winusb/enumeration.rs

 }

-pub fn probe_device(devinst: DevInst) -> Option<DeviceInfo> {
+pub fn enum_root_hub(rootinst: DevInst) -> impl Iterator<Item = (DevInst, Vec<u32>)> {


Would there be a way to do this starting from a device and looking up its parent rather than switching to start at a root hub? Besides the increased complexity, one argument against this is that with hotplug attach events, we're given a single DevInst and would like to be able to find all of its DeviceInfo properties without walking the whole tree.

kevinmehall · 2024-07-10T04:06:13Z

src/platform/macos_iokit/enumeration.rs

+
+fn get_port_number(device: &IoService) -> Option<u32> {
+    get_integer_property::<u32>(device, "PortNum").or_else(|| {
+        if let Ok(parent) = get_parent(device) {


Is the parent something other than a hub here? How is the line below getting the port for a specific child device when the property is on the parent?

This function also references the libusb code.

kevinmehall · 2024-07-10T04:06:22Z

src/platform/macos_iokit/enumeration.rs

+            };
+
+            // Ignore the same sessionID
+            if session_id


Did you observe it reporting a device as its own parent or find documentation indicating that, or is this just a precaution to prevent infinite-looping?

The parent device may return the same sessionID, which needs to be ignored until a new sessionID is obtained or the obtaining fails.

kevinmehall

This is far too complicated for the functionality added. Libusb does a lot of crazy stuff that is no longer relevant; please don't copy what libusb does without understanding why it does it. Since much of this code is practically unreachable, it's also untested.

For example, this adds an alternative way to look up port numbers on macOS if portNum is missing -- it turns out that libusb had this because macOS versions 10.11.0 - 10.11.2 lacked the portNum property, but macOS 10.11.2 is 9 years old. Meanwhile, the locationId is just a bitwise encoding of the port path we're looking for, no iokit tree traversal needed.

Similarly on Windows, this adds two additional methods of finding the port number. The Windows backend already relies heavily on the port number, because it's what's passed to the hub ioctls to get descriptors. We've been using DEVPKEY_Device_Address exclusively for that with no problems so far.

I've gone ahead and implemented a simpler version as #71. Need to figure out what to do with the bus number though, and may end up using some of this Windows code after all.

kevinmehall · 2024-08-03T22:21:43Z

src/platform/windows_winusb/enumeration.rs

+    let bus_devs = cfgmgr32::list_interfaces(GUID_DEVINTERFACE_USB_HOST_CONTROLLER, None)
+        .iter()
+        .flat_map(|i| get_device_interface_property::<WCString>(i, DEVPKEY_Device_InstanceId))
+        .flat_map(|d| DevInst::from_instance_id(&d))
+        .flat_map(|d| d.children())
+        .map(|d| d.instance_id().to_string())
+        .enumerate()
+        .map(|v| (v.1, (v.0 + 1) as u8))
+        .collect::<HashMap<String, u8>>();


The existing bus number using DEVPKEY_Device_BusNumber is indeed wrong (#70) but as mentioned in that issue I'm considering just exposing the bus ID as a string rather than trying to map it to a small integer.

kevinmehall · 2024-08-03T22:21:56Z

src/platform/windows_winusb/enumeration.rs

+        }
+    }
+    // Find last #USB(x)
+    // PCIROOT(B2)#PCI(0300)#PCI(0000)#USBROOT(0)#USB(1)#USB(2)#USBMI(3)


This seems to be the case

CCnut · 2024-08-04T15:23:58Z

The usb bus number on Windows is related to GUID_DEVINTERFACE_USB_HOST_CONTROLLER.

Here's what I've tried so far to get the bus number.

Use a prefix of DEVPKEY_Device_LocationPaths such as PCIROOT(B2)#PCI(0300)#PCI(0000) to compare with the device returned by GUID_DEVINTERFACE_USB_HOST_CONTROLLER.
Alternatively, use the root hub's DEVPKEY_Device_PhysicalDeviceLocation, which are in reverse order of the bus number.

kevinmehall · 2024-08-12T05:16:34Z

Superseded by #71, but thanks for your work on this in helping figure out what was needed here.

windows: add location_paths

27016ec

kevinmehall reviewed Jun 11, 2024

View reviewed changes

src/enumeration.rs Outdated Show resolved Hide resolved

windows: Fix location_paths return type

0c5e11d

kevinmehall mentioned this pull request Jun 20, 2024

The serial_number returned by the list_devices function is in uppercase, while libusb returns it in normal case (lowercase). #22

Closed

CCnut added 2 commits June 27, 2024 11:50

Add DeviceInfo::port_chain (macos in progress)

dcf4e38

Add DeviceInfo::{port_number, port_chain} for all platform

8e1539d

CCnut changed the title ~~windows: add location_paths~~ Add port_number, port_chain for all platform Jun 27, 2024

Merge branch 'kevinmehall:main' into main

4e811e8

kevinmehall reviewed Jul 10, 2024

View reviewed changes

CCnut added 5 commits July 24, 2024 01:11

Merge branch 'kevinmehall:main' into main

dde203d

windows: get port_chain starting from a device

cc2949b

Merge branch 'main' into main

721d3af

macos: fix fmt

28b6762

windows: fix probe_device

980cbc1

kevinmehall mentioned this pull request Aug 3, 2024

Add DeviceInfo::port_chain #71

Merged

kevinmehall reviewed Aug 3, 2024

View reviewed changes

kevinmehall closed this Aug 12, 2024

Conversation

CCnut commented Jun 4, 2024

Uh oh!

Uh oh!

kevinmehall commented Jun 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CCnut commented Jun 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kevinmehall commented Jun 20, 2024

Uh oh!

CCnut commented Jun 21, 2024

Uh oh!

CCnut commented Jun 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kevinmehall left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kevinmehall left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CCnut commented Aug 4, 2024

Uh oh!

kevinmehall commented Aug 12, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kevinmehall commented Jun 11, 2024 •

edited

Loading

CCnut commented Jun 12, 2024 •

edited

Loading

CCnut commented Jun 27, 2024 •

edited

Loading