-
Notifications
You must be signed in to change notification settings - Fork 898
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
hid_enumerate doesn't return a null pointer when disconnecting two devices #102
Comments
Please give more detail of what it does, what you did, and what you expect it to do. |
When I remove two devices one by one, the function returns a null pointer (there is no more devices connected), which is normal. But when I remove two devices at the same time and there is no more devices connected, the function returns a pointer on unsetted structures (path, serial_number... are null). It should return a null pointer as well. |
you're going to need to provide more detail than this. What version of HIDAPI, did you use the latest from the git repo? What platform? Can you make a simple test case that demonstrates the problem? What does it do in the test GUI? When do you call hid_enumerate() with respect to when you remove the devices? |
The version of hidapi is the 0.7.0 available at https://github.com/signal11/hidapi/downloads. In the tests GUI, it says no devices connected as expected. The function “hid_enumerate” is called in a loop similar to: The devices are removed during the call of the function Sleep. “current_devs” is not null then. |
Use the latest from git (ie: git clone ...) You have to press "re-scan devices" in the GUI. |
I have tried the latest version. I still have the same bug. In fact with the GUI, I have the same problem, if I do the re-scan just after I remove the two devices. The information “No device connected” is not displayed, but data without the device name: "1b67:0015 (usage:ff00:0001)", instead of "1b67:0015 Toradex Orientation Sensor (usage:ff00:0001)" But with the latest version, more curious things happen. When I connect a sensor device, the hid_enumerate recognizes my usb keyboard and usb mouse. But when I disconnect the sensor device, they disappear from the list as well. They are not recognized. |
If you re-scan again, is the problem cleared? Is the problem that it just takes a few seconds for devices to show disconnected? I do not have Vista, but don't have this problem. The keyboard and mouse should not ever show up in there on Windows, at least according to the documentation. Can you debug into the hid_enumerate() function and find out if the devices are indeed showing up from Windows and see if anything jumps out at you? |
If I re-scan again, the problem is cleared. Well yes, it takes a few seconds to show disconnected, but it shouldn’t go through a device half initialized. I can deal with it in my program by checking that even if I have a device detected, the structure is fully initialized . |
I agree, but this is the data Windows is giving us. Is there a way you can determine in the hidapi code whether the data is valid or not, so that we can filter out the invalid ones?
I remember now that someone sent a patch to make them show up. Apparently it's valid to send feature reports to KB's and mice. This should be in the git version, but not in 0.7.0.
That makes me wonder whether you have the right version of the testgui. The testgui uses hid_enumerate() under the hood so it should be the same. Did you build the testgui from source (including downloading the externals package)?
Thanks, I appreciate your looking into this. Alan. |
This is strange: the pointer cur_dev, which should be null, is not null. So a way to be sure that he is valid is to check one of his parameter as well (line 389 in hid.c):
Instead of :
Well this is maybe not the nicest solution, but that’s the first one coming to my mind.
Yes, I hadn’t recompiled the exe. The keyboard and the mouse actually appear in the GUI as well, even if I cannot connect to them. They disappear when I disconnect the toradex device.
I’ve found the origin of the problem. Line 331 to 352, it checks if it’s a HIDClass, and the mouse and keyboard are not. So the loop won’t be ended by the “break”, but by the “goto cont;”. That’s why they won’t appear unless there is a HIDClass device. |
Most devices don't have a serial number, so you can't use the absence of one to say that a device handle is invalid.
Keyboards and mice are HID devices. The loop at 331-352 is executed per_device, looking to see if any of the interfaces of that device have HIDClass bound to them. Having different devices connected in_addition to a KB and mouse shouldn't cause the KB and mouse to show or not show. If this is happening, There's something wrong with the logic. Are the KB and mouse only getting into HIDClass if there is a non-kb/mouse attached? |
Can you do the test on the manufacturer_string or product_string? They are null as well.
Well, when I execute the loop, KB and mouse are not considered as HID devices: the driver_name is not "HID_Class". I know that the loop is executed per device. But as you say, the logic is wrong, because the loop will go to the break instruction if there is a HIDclass device and won’t test the others. As my KB and mouse are not considered as HIDclass device, they won’t be considered unless there is a HIDclass device connected as well. |
No. Some devices may not have these either (although less likely). You'll probably be unable to test based on anything in the hid_device struct. We'll have to test based on something that comes from Windows which is probably not in the struct.
What is the name of the driver (actually, it's the Setup Class)? (there may be more than one driver per device, that's what the loop on 331-252 is doing, checking each SetupClass for the device).
Let's get the terminology right so we can understand each other better. There's a HID Device (Which is Windows terminology for what is actually a USB Interface on some device which reports to be of the HID class). There can be multiple HID interfaces on a single USB device, but we don't have to worry about that because Windows handles that and gives us a separate HID Device for each. 282 starts the loop for the HID Devices. Each HID Device can have multiple Setup Classes. 331-252 iterates over the setup classes for the device (i is the member index). If one of the SetupClasses is "HIDClass", it breaks out of the loop and uses that device. When it gets to the last setup class (when SetupDiEnumDeviceInfo() returns false), or if it's not able to look up the class name with SetupDiGetDeviceRegistryProperty(... SPDRP_CLASS), it skips that device (goto cont), and continues with the next HID Device. When you say "if there is a HIDClass device [it] won't test the others," what you mean is that for each device, once if finds a "HIDClass" SetupClass, it won't test the other SetupClasses for that device. This is right and is the proper logic. I'm wondering whether the "goto cont" on 343 should maybe be a continue. Just because getting the SPDRP_CLASS fails for one member index, doesn't mean we shouldn't check the other member indexes for that device. Try that and see what happens different. I know that when Windows starts unbinding drivers, it's not an atomic operation. Some functions will succeed for a while while the driver is unbinding itself, and others will fail. We need to find what fails immediately so we can test for it. This is what the test for "HIDClass" is for, because some version of Windows will give you a HID device before the HIDClass driver is bound on attachment. Nice, eh? |
Any updates? |
When I disconnect two devices at a time and if there is no more devices connected, the function hid_enumerate doesn't return a pointer which is set at NULL.
The text was updated successfully, but these errors were encountered: