New feature: Add driver name #1404
Conversation
libusb/os/windows_winusb.c
Outdated
| @@ -2293,6 +2293,7 @@ const struct windows_backend winusb_backend = { | |||
| winusb_cancel_transfer, | |||
| winusb_clear_transfer_priv, | |||
| winusb_copy_transfer_data, | |||
| "WinUSB", | |||
There was a problem hiding this comment.
Thank you for the PR!
I think here I am not agreeing with you that this is the driver name. As far as I understand, UsbDK and WinUSB are 2 different Windows backenda. The "driver" as per Windows meaning is something else.
For WinUSB backed it can be WinUSB, but also libusbK or libusb0 (or something new in the future).
When the feature was presented in issue #1225 it clearly stated that for Windows the new API shall return one of those 3 for now, see: #1225 (comment).
Therefore I think that here we should have a function instead of a fixed string, in order to return actually loaded driver. Otherwise the feature makes less sense to me.
Thanks for considering this.
There was a problem hiding this comment.
As far as I understand, UsbDK and WinUSB are 2 different Windows backend
Agreed.
For WinUSB backed it can be WinUSB, but also libusbK or libusb0 (or something new in the future).
Agreed but there are actually three types, resulting in eights APIs which can be tapped into:
- Composite devices:
dg_ssudbus,USBCCGP, - WinUsbX:
libusbK,libusb0,WinUSB - HID:
HIDUSB,MOUHID,KBDHID
However, these are per device APIs. So we can't return either of these names when driver name is requested for a backend because it could be any/many of them.
for Windows the new API shall return one of those 3 for now, see: #1225 (comment).
I don't think this list is accurate.
- I think
libusbKis a typo. @mcuee probably meantlibusbdK. - There is no
libusb-win32support. We have pieces of code fromlibusb-win32but that is neither a backend nor a supported API. - As for
WinUSB, as you pointed, out it is a bad name. I took it from the translation unit but I shouldn't have.
My proposal for Windows is to keep libusbdK for the windows_backend defined in windows_usbdk.c. And use Per-Device-API (or Composite-WinUsbX-HID) for the windows_backend defined in windows_winusb.c
There was a problem hiding this comment.
Hello @fabiensanglard , thank you for your prompt reply. Yes, you're right, I wrote my comment too quickly: given the function prototype you chose (and correctly advertised in #1225 before doing your development), you cannot really return a driver name, since this only makes sens on a per device basis, and there is no device argument in your API.
However, given this "detail" that I didn't realize earlier, I am now in doubt about what additional information this work brings. It is more like getting the backend name (which is useful per se) and / or some hardcoded static driver name that is 1:1 with backend name.
So, I would suggest to either rename the function to "get_backend_name" or add a "libusb_device_handle *" parameter to the current "get_driver_name" function and implement it for returning the driver name under Windows.
But we need @mcuee opinion here as he's the one who first requested the feature.
Regarding libusbK: it really is libusK (not libusbdK): https://github.com/mcuee/libusbk
Regarding libusb-win32 aka libusb0.sys: it is supported via libusk.dll (not a driver but an user space lib) that interfaces in between libusb-1.0 and libusb0.sys (it provides a "WinUBS-like" API over libusb0.sys .
So well, @mcuee : do we want backend name or driver name (or both) ?
There was a problem hiding this comment.
My original intention: just the driver name and not the backend, as there is no extra info given by the backend.
Windows WinUSBx backend -- return WinUSB (WinUSB.dll --> WinUSB.sys), libusbK (libusbK.dll --> libusbK.sys) or libusb-win32 (libusbK.dll --> libusb0.sys) accordingly.
Windows USB HID backend -- return hidusb. As for USB HID Mouse and Keyboard, I am not so sure. libusb Windows can not do much with USB HID Mouse/Keyboard.
Windows USBDK backend -- return usbdk
Linux -- return usbfs
macOS -- return IOKit
BSDs -- return ugen
USB Composite Device is a tricky part. It was not really my intention to return USBCCGP (or dg_ssudbus as a special case for Samsung devices).
USB HUB is probably another tricky part.
There was a problem hiding this comment.
It was more meaningful to get the backend name last time, at least for Windows.
Last time, it was possible to access devices using WinUSB driver through two ways.
- No libusbK.dll present
libusb-1.0.dll --> WinUSB.dll --> WinUSB.sys - With libusbK.dll present
libusb-1.0.dll --> libusbK.dll --> WinUSB.dll --> WinUSB.sys
Now there is only one way, no matter the presence of libusbK.dll.
libusb-1.0.dll --> WinUSB.dll --> WinUSB.sys
There was a problem hiding this comment.
But after reading the info, maybe it is worth getting the backend name as well in the end to have more flexibilities. In the future, maybe there will be cases that there will be two ways to access a USB device (same device driver) using different backends.
Maybe it is good to provide two functions, one for the backend (os/backend) and the others for the driver name. Or return a structure with different members (os, backend, driver name).
There was a problem hiding this comment.
Let's hear from @hjelmn and @tormodvolden as well for their opinions.
There was a problem hiding this comment.
Based on both your feedback (@mcuee and @somatique) it seems what is needed is the driver name per device. For that I need to change the proposed API to:
const char* libusb_get_driver_name(struct libusb_device *device);
It would make sense to leverage the names in composite_driver_names, winusbx_driver_names, and hid_driver_names since they are already in the windows_usb_api_backend. If you don't like the values, maybe we can change them?
If you are both ok, I can update the CL.
There was a problem hiding this comment.
const char* libusb_get_driver_name(struct libusb_device *device);
In my opinion this is the best way to go.
It would make sense to leverage the names in
composite_driver_names,winusbx_driver_names, andhid_driver_namessince they are already in thewindows_usb_api_backend. If you don't like the values, maybe we can change them?
I think current values are fine.
Thanks!
There was a problem hiding this comment.
If you are both ok, I can update the CL.
Okay, please proceed. Thanks.
Hopefully @tormodvolden or @hjelmn would have some time later to review the proposal as well.
|
Example USB Composite Device: So it seems to me the more useful is to get the driver used for an individual interface. Another USB Device: only Interface 0 is accessible by libusb Windows (USB HID interface). |
|
When building using VS2019, the build failed. Appveryor MSVC build also failed. |
|
Warnings under MSYS2 mingw64: |
|
Strange that the example testlibusb caused seg-fault. |
fabiensanglard
force-pushed
the
master
branch
4 times, most recently
from
d92c932
to
0517391
Compare
|
PTAL (now the API retrieval is on a per device basis) 1/ @mcuee Do we have the instructions to setup MINGW64 so I can investigate the segfault? |
|
MinGW development environment --> recommendation is to use MSYS2. There are multiple toolchains. The aim of libusb project is to support all of them (except msys). Currently libusb CI tests MSYS2 mingw64, clang32 and clang64 toolchain. I usually use only mingw64 and clang64 environment to carry out the tests.
|
|
As for review of this PR, I think @tormodvolden and @hjelmn will need to take a look, probably after libusb-1.0.27 release. |
|
Your latest commit is good -- no more compiler warnings and no more crash using MSYS2 mingw64 toolchain and test under Windows 11. |
|
Idealy it would be even better to zoom into the interfaces of USB Composite Device like PICKit 4. Example output from Linux usbutils ( |
|
If we want to get the driver name for each interface, we will return values like |
I understand your point but I feel it will be more useful, at least for USB Composite Devices under Windows and Linux. I think you may want to pause and let's hear from @tormodvolden and @hjelmn before continue the efforts. |
fabiensanglard
force-pushed
the
master
branch
2 times, most recently
from
6a733cb
to
e6010cf
Compare
examples/testlibusb.c
Outdated
| printf("Dev (bus %u, device %u): %04X - %04X speed: %s driver: %s\n", | ||
| libusb_get_bus_number(dev), libusb_get_device_address(dev), | ||
| desc.idVendor, desc.idProduct, speed, libusb_get_driver_name(dev)); |
There was a problem hiding this comment.
regarding tabs vs spaces - here is a very interesting case:
the file uses tabs for aligning, but the consequent alignment of multiline statement/expression should use spaces, since the first line "refix" is a fixed-width printf(:
| printf("Dev (bus %u, device %u): %04X - %04X speed: %s driver: %s\n", | |
| libusb_get_bus_number(dev), libusb_get_device_address(dev), | |
| desc.idVendor, desc.idProduct, speed, libusb_get_driver_name(dev)); | |
| printf("Dev (bus %u, device %u): %04X - %04X speed: %s driver: %s\n", | |
| libusb_get_bus_number(dev), libusb_get_device_address(dev), | |
| desc.idVendor, desc.idProduct, speed, libusb_get_driver_name(dev)); |
and the original author of these suggested that consequent lines should be aligned regardless of the "tab size" configuration in the viewer/IDE
|
Latest git commit is still good under Windows, if we just use the original proposal, to return the relevant driver name of a device (not down to an interface). |
|
Ok, I am pausing the feature until @tormodvolden and @hjelmn can take a look. I will just summarize for them the latest status. The requirement of the feature evolved over the review. It seems what is the most useful according to @mcuee is to have two functions. One to retrieve backend info. And one to retrieve the driver name for a given (device, interface). That means I can split the PR into two. One would add to the APi: The other would add the following API: |
|
@tormodvolden and @hjelmn Just wondering if you can review the proposal here. Thanks. |
|
Tested under macOS. I know we say macOS should return @hjelmn and @tormodvolden |
|
@fabiensanglard @mcuee @tormodvolden : I recently made some Windows-only libusb-1.0 dev on my side in order to get per device driver information using The way I did is that I added call to Of course this is quite Windows specific but I wonder whether there might be an interested to complement this PR with some version (or other, provider maybe) information got from |
|
Looks like an interesting idea to me. Windows is anyway one of the main focus of this PR. |
|
Thanks @sonatique this is precious. When I was researching it, I found getting the Linux driver name to be trivial but the Window side of things very complicated. Your Windows expertise is a relief! It looks like |
|
@fabiensanglard Thanks for your reply. As far as I know (not being an expert myself) we can use You're right, there is no "driver name" with the meaning of giving you the driver filename, like libusb0.sys or libusbK.sys because, I guess, this is not what is considered to be relevant by Windows (after all you can always rename this file and repackage it). The more relevant data is what is to be found in (signed) INF file. Moreover in recent version of Windows driver files are stored into "DriverStore" folder upon installation and their name is modified to be unique per INF file / device type. Before this, a single libsub0.sys (for instance) would be installed in System32/Driver for all devices to use. If someone would update this file, all device would automatically use it, even if their related application would not support some aspects of it. So, well, instead of "driver name" with I think that when developing an application for a device, if you face the problem of needing to known the driver version, it is because you may be compatible with some versions and not with others. In this case you'll refer to then "Provider" then check the "Version" (you may support multiple Provider / Version pairs) and you'll have to know the Provider in advance. This is what is closer to "driver name", as far as I understand. |
|
Did you mean to rebase here? There are no commits left. |
Fixes: #1225