Add Device::claim_associated_interface, Interface::enable_raw_io, Interface::set_timeout_millisecond for Windows

[wangxiaochuTHU]

1. Add Device::claim_associated_interface for the composite device claiming an associated interface on Windows platform.
   Following your guidelines, WinUsb_GetAssociatedInterface is used for composite device claiming non-default interfaces, with the following two assumptions that work for my test device ( I'm not sure if it is the general rule)

• The interface 0 is the default interface used in WinUSB WinUsb_Initialize
• Any other non-default interface has a positive (non-zero) interface number

2. Add Interface::enable_raw_io for Windows platform, which may be useful in certain cases

3. Add Interface::set_timeout_millisecond for a transfer

4. Make several assert_eq to debug_assert_eq on some key bulk-in paths

5. Update HubHandle::get_node_connection_info, for getting a more precise connection speed (Fix the issue that super speed was mis-reported as high speed).

These additions can work but look to be ugly APIs and please just reallocate these snippets in a correct way if needed.

[kevinmehall]

Looks like a good start on several important things! Do you mind splitting the unrelated commits to separate PRs for easier review?

Initial feedback:

Add Device::claim_associated_interface

Typically this library likes to expose OS functionality without too much abstraction on top of it, but this might be a case where there is a practical need for nusb to do more of the work. Doing this as a Windows-only API means a cross platform app has to know about and deal with this Windows quirk. It can also depend on how the driver is installed -- if a composite device is bound to WinUSB as a whole, it needs this. If the device is bound to usbccgp and individual interfaces bound to WinUSB, then the interfaces can be opened separately and use WinUsb_Initialize (should already work today).

So I think what I'd like to see here for the whole-device WinUSB case, is that the first time opening any interface calls CreateFile and WinUSB_Initialize, and stores that handle in the WindowsDevice. It would then return a WindowsInterface wrapping that handle if the interface is 0, or for other interfaces, call WinUsb_GetAssociatedInterface and use that handle. Subsequent calls to claim_interface would return either the cached interface 0 handle if not already claimed, or WinUsb_GetAssociatedInterface for a different interface. It would need to keep track of which interfaces are claimed, and prevent claiming the same interface multiple times. When all interfaces are released, close the initial handle and file handle.

The interface 0 is the default interface used in WinUSB WinUsb_Initialize Any other non-default interface has a positive (non-zero) interface number

This is true only when WinUSB is bound to the whole device. If a composite device is bound to usbccgp, each interface association descriptor group or interface outside of an IAD group will be a separate WinUSB device object, and WinUsb_Intiialize returns the first interface within the group, and WinUsb_GetAssociatedInterface for subsequent interfaces in the group. The case without IADs is handled now, and handling IADs is an obscure edge case for things like CDC or UVC class interfaces with the driver replaced with WinUSB, and isn't nearly as important to support. Libusb only fixed it recently. It would require going through the descriptors to find the IADs and the interfaces under them.

Add Interface::enable_raw_io for Windows platform

There's a bunch of discussion about this on #6 and I think we decided that the right thing to do is turn RAW_IO on unconditionally for all IN endpoints when they are first used, instead of making an API for it. From the libusb discussions about RAW_IO it seems like it is not safe to call SetPipePolicy while transfers are pending, so that might be a problem with the API as you proposed here.

Add Interface::set_timeout_millisecond for a transfer

Not sure about this. It's tough because each operating system has its own way of doing timeouts:

• Windows: Timeout applies to all transfers on the endpoint
• Mac: Timeout set for each transfer
• Linux: No timeout in the kernel, expects you to do your own timing and cancel the transfer

Rust futures tend to not implement their own timeout, and instead you race the future with a timer and cancel the future if the timer completes first. For cases where you're using nusb without other futures, I think it would be nice to have a block_on with a timeout, but since that would be generally useful, that might belong in futures_lite or elsewhere instead of in nusb.

Make several assert_eq to debug_assert_eq on some key bulk-in paths

Seems fine, but any particular motivation for this?

Update HubHandle::get_node_connection_info, for getting a more precise connection speed (Fix the issue that super speed was mis-reported as high speed).

This looks good, though you can probably drop the Windows version check. I haven't tested nusb on anything lower than Windows 10, and with all prior Windows versions out of support by Microsoft and and Rust, it doesn't seem like it makes sense to go out of our way to support them.

[wangxiaochuTHU]

Looks like a good start on several important things! Do you mind splitting the unrelated commits to separate PRs for easier review?

Yes, I attempted to commit them one by one, they just showed up inside one PR. I will try learning to split (it may take some time though) and only commit the necessary parts.

Add Device::claim_associated_interface

So I think what I'd like to see here for the whole-device WinUSB case, is that the first time opening any interface calls CreateFile and WinUSB_Initialize, and stores that handle in the WindowsDevice.

This way indeed seems better in most cases. But apears that it could fail in certain cases, where e.g $n\ge2$ composite devices are connected to the host at the same time, such that we will have four devices (say WinUSB-0, WinUSB-1, usbccgp-A and usbccgp-B), then how to know if WinUSB-0 corresponds to usbccgp-A or usbccgp-B might be a problem?

The interface 0 is the default interface used in WinUSB WinUsb_Initialize Any other non-default interface has a positive (non-zero) interface number

This is true only when WinUSB is bound to the whole device.

Thanks for making me clear.

Add Interface::enable_raw_io for Windows platform

There's a bunch of discussion about this on #6 and I think we decided that the right thing to do is turn RAW_IO on unconditionally for all IN endpoints when they are first used, instead of making an API for it. From the libusb discussions about RAW_IO it seems like it is not safe to call SetPipePolicy while transfers are pending, so that might be a problem with the API as you proposed here.

It sounds great to enable RAW_IO for IN endpoints by default.

Add Interface::set_timeout_millisecond for a transfer

For cases where you're using nusb without other futures, I think it would be nice to have a block_on with a timeout, but since that would be generally useful, that might belong in futures_lite or elsewhere instead of in nusb.

Yes, I didn't realize the fact that the timeout can be implemented on futures. As a matter of fact, what my use case really needs is synchronously reading BULK-IN endpoint, since the reading thread is time crucial and I assume the execution of async fn in Rust would bring in some overhead, wouldn't it?

Make several assert_eq to debug_assert_eq on some key bulk-in paths

Seems fine, but any particular motivation for this?

This is because I assume assert_eq could be also executed in release mode, slightly bringing an extra overhead though it might be extremely small. Would it matter to the reading of high-throughput BULK-IN stream?

Update HubHandle::get_node_connection_info, for getting a more precise connection speed (Fix the issue that super speed was mis-reported as high speed).

This looks good, though you can probably drop the Windows version check. I haven't tested nusb on anything lower than Windows 10, and with all prior Windows versions out of support by Microsoft and and Rust, it doesn't seem like it makes sense to go out of our way to support them.

Yes, it makes sense that not using older Windows.

[kevinmehall]

Yes, I attempted to commit them one by one, they just showed up inside one PR.

Your commits are actually split pretty well; they should just be on separate branches so they can be submitted as separate PRs. Look at git cherry-pick to copy specific commits to a different branch.

This way indeed seems better in most cases. But apears that it could fail in certain cases, where e.g
composite devices are connected to the host at the same time, such that we will have four devices (say WinUSB-0, WinUSB-1, usbccgp-A and usbccgp-B), then how to know if WinUSB-0 corresponds to usbccgp-A or usbccgp-B might be a problem?

We find the usbccgp parent device object, which represents the whole USB device, and look only at the child device objects under that parent, which represent interfaces or interface groups of that device. See here.

As a matter of fact, what my use case really needs is synchronously reading BULK-IN endpoint, since the reading thread is time crucial and I assume the execution of async fn in Rust would bring in some overhead, wouldn't it?

There is some overhead in making separate WinUsb_ReadPipe, GetQueuedCompletionStatusEx WinUsb_GetOverlappedResult syscalls for async IO and the cross-thread notify, yes. Rust async/await overhead expected to be comparatively small. However, to maximize USB throughput and minimize latency, you really need to use async IO. With a blocking API, you can only submit one transfer at a time because your thread is blocked until it finishes. While you are handling its completion and preparing to make the next transfer, there is no transfer active, and the bus goes idle. With the Queue API you can easily submit multiple transfers to make sure the host controller always has a transfer to process.

This is because I assume assert_eq could be also executed in release mode, slightly bringing an extra overhead though it might be extremely small. Would it matter to the reading of high-throughput BULK-IN stream?

I highly doubt a predictable branch would have a measurable impact when profiling, compared to the overhead of system calls.

[wangxiaochuTHU]

Sorry, current APIs are beautify, and I found it difficult to just modify the whole-device WinUSB case while leaving current APIs unchanged and keeping compatibility with other ways of driver installation. What I finally made today looks like a mess with redundancy, so I feel I have to give up its updating.