-
Notifications
You must be signed in to change notification settings - Fork 398
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
Add hid_get_device_info #432
Conversation
lets have it against a feature branch for now: https://github.com/libusb/hidapi/tree/hid_get_device_info |
This could replace #164, as it will be possible to get the path from the device_info struct. |
9b09429
to
94aea4f
Compare
Yes, lazy initialization of the device info should be beneficial, specially for those who never use it. |
The calls to get the serial number and other strings are now all copying off the device_info, so I would guess it to be rare that it won't get loaded, but I have no problem making it lazy and caching it |
Rebase to master, when you have a chance - got some conflicting changes in there. |
Co-authored-by: Ihor Dutchak <ihor.youw@gmail.com>
Co-authored-by: Ihor Dutchak <ihor.youw@gmail.com>
53a1c91
to
5497de1
Compare
Mac Mini M1 (macOS 12.4) output. The output seems to be fine.
|
Windows MSYS2 mingw64 output. Seems to be fine.
|
@Julusian do you consider this as still a draft? |
Sorry, I got distracted on other things @mcuee you haven't really tested my changes much there. You have for the refactoring, but the |
I see. Unfortunately the one used in hidtest is not a common device but rather a modified Microchip generic HID FW example from Alan Ott. So maybe I need to use other codes to test this pull request. Will hidapitester be good enought? |
I have been changing the ids to be for a device I have, then stopping the application when it starts on the read/write tests |
This is the output from my Microchip Custom Demo (original Microchip FW). It is probably good enough for this testing.
|
Another libusb update: sorted out the "get usage_page/usage". It has to be handled differently during the enumeration and when the device is fully opened. |
Now I'm happy with libusb implementation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated hidraw implementation too.
Dependency on libudev has been a major blocker for me, from using hidraw backend on Android, for a while. Today I made a tiny step to using sysfs directly instead of libudev (hid_enumerate/filter over VID/PID).
Second major improvement - hid_get_*_string
functions: wcsncpy
crashes of the source string is NULL. Add explicit check.
If anyone can additionally test libusb and hidraw backends - I'd highly appreciate it.
Since no one complains about my latest changes - either everything works as expecter or no one had a chance to look :) |
@Youw No issue with the libusb backend under Ubuntu 20.04.
But there is no way to open the device using the hidraw backend. It can not even find the device after the above. Maybe it does not work with hidraw driver.
|
@Youw
|
Results of the Logitech USB Receiver under Linux.
|
That is actually unexpected. LIBUSB backed should have re-attach the kernel (hidraw) driver back, once the device is closed. |
As far as I can tell - the driver was reattached (otherwise you wouldn't be able to enumerate it with hidraw after libusb). |
There is an interesting side-effect/improvement in libusb backed:
|
It can not enumerate with the hidraw after libusb. Therefore I think the re-attach does not work in this case, probably due to the use of CTRL-C to terminate the hidtest application. So I do not think this is a real issue. |
Ah, right.
Agree. |
Thanks for the tip. I will try this on the Teensy HID device. |
Commit 5c9f147 (libusb#432) replaced a call to strdup with an explicit memcpy to a buffer on the stack. However, it incorrectly used the buffer size, instead of the clamped uevent length, as the argument to memcpy, resulting in reads past the end of uevent: $ valgrind -q hidtest/hidtest_hidraw 1>/dev/null ==51900== Invalid read of size 8 ==51900== at 0x48571D5: parse_uevent_info (hid.c:496) ==51900== by 0x48574F2: create_device_info_for_device (hid.c:578) ==51900== by 0x4857ED7: hid_enumerate (hid.c:876) ==51900== by 0x1094CE: main (test.c:105) ==51900== Address 0x4b6c1a0 is 7 bytes after a block of size 185 alloc'd ==51900== at 0x4846CC3: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==51900== by 0x4AA0E45: UnknownInlinedFun (fileio.c:534) ==51900== by 0x4AA0E45: read_virtual_file_at.constprop.0 (fileio.c:572) ==51900== by 0x4A9D37C: UnknownInlinedFun (fileio.h:74) ==51900== by 0x4A9D37C: UnknownInlinedFun (fileio.h:77) ==51900== by 0x4A9D37C: sd_device_get_sysattr_value (sd-device.c:2318) ==51900== by 0x4A8B0D1: udev_device_get_sysattr_value (libudev-device.c:747) ==51900== by 0x48574BE: create_device_info_for_device (hid.c:578) ==51900== by 0x4857ED7: hid_enumerate (hid.c:876) ==51900== by 0x1094CE: main (test.c:105) ==51900== Fix this by using uevent_len as the argument to memcpy. Calling strndupa was considered but abandoned, as it is not standard. Fixes: 5c9f147 (libusb#432) Fixes: 4779d63
…497) Commit 5c9f147 (#432) replaced a call to strdup with an explicit memcpy to a buffer on the stack. However, it incorrectly used the buffer size, instead of the clamped uevent length, as the argument to memcpy, resulting in reads past the end of uevent: Fix this by using uevent_len as the argument to memcpy. Calling strndupa was considered but abandoned, as it is not standard. Fixes: 5c9f147 (#432) Fixes: 4779d63
Resolves: #431
Closes: #164
Closes: #163
I don't like the signature of
get_device_info_for_device
in linux, and Im unsure aboutfill_device_info_for_device
in libusb.The mac, linux and libusb implementations appear to work fine with hidtest.
I havent tested the other backends yet