Usage Page and Usage on Linux with hidraw #139
Labels
No labels
3rdparty
Android
API
Author does not respond
bsd
bug
build system/CI
ci-virtual-device
CMake
Contributions Welcome
Core
dependencies
device-specific
documentation
don't_merge
duplicate
enhancement
Example
Haiku
hidraw
hotplug
info required
invalid
libusb
macOS
Misc
Not an issue
question
Release
Test
Test GUI
Windows
wontfix
No milestone
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
gh-libusb/hidapi!139
Loading…
Reference in a new issue
No description provided.
Delete branch "usage_linux"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Adapted from ApeironTuska's PR to PastaJ36/hidapi (https://github.com/PastaJ36/hidapi/pull/1)
Which was adapted from djpnewton's PR to signal11/hidapi (https://github.com/signal11/hidapi/pull/6)
Also addresses some of the issues mentioned in (https://github.com/signal11/hidapi/pull/6)
as the user may call hid_open_path directly without using hid_enumerate
NOTE: This commit does not handle composite HID descriptors
I am interested in adding support for composite descriptors though I still need to find a
device with a composite descriptor to test it correctly
Device Found
type: 308f 0015
path: /dev/hidraw10
serial_number: 53373100323943353230353139363032 - sam4s2
Manufacturer: Kiibohd
Product: Keyboard - None PartialMap USBxUART
Release: 4ac
Interface: 0
Usage (page): 0x6 (0x1)
Device Found
type: 308f 0015
path: /dev/hidraw11
serial_number: 53373100323943353230353139363032 - sam4s2
Manufacturer: Kiibohd
Product: Keyboard - None PartialMap USBxUART
Release: 4ac
Interface: 1
Usage (page): 0x6 (0x1)
Device Found
type: 308f 0015
path: /dev/hidraw12
serial_number: 53373100323943353230353139363032 - sam4s2
Manufacturer: Kiibohd
Product: Keyboard - None PartialMap USBxUART
Release: 4ac
Interface: 2
Usage (page): 0x1 (0xc)
Device Found
type: 308f 0015
path: /dev/hidraw13
serial_number: 53373100323943353230353139363032 - sam4s2
Manufacturer: Kiibohd
Product: Keyboard - None PartialMap USBxUART
Release: 4ac
Interface: 3
Usage (page): 0x2 (0x1)
Device Found
type: 308f 0015
path: /dev/hidraw14
serial_number: 53373100323943353230353139363032 - sam4s2
Manufacturer: Kiibohd
Product: Keyboard - None PartialMap USBxUART
Release: 4ac
Interface: 4
Usage (page): 0x1100 (0xff1c)
Thanks you very much for doing this PR.
We're looking forward to accept the changes.
Before I go deep into the changes, a few general comments:
Please remove all white-space changes: they are irrelevant to the change itself and makes harder to look thru history of changes, e.g. it took me some time to figure that only 2 files had been changed, not 4;
get_bytesis extremely unfriendly name for a function, that actually interpret array of bytes as big-endian number;Sounds good, I'll update the commit shortly.
Updated the commit.
get_bytestoget_hid_report_bytesand added_hidto getget_hid_item_size,get_hid_usageandget_hid_report_descriptorto make it more consistent. I've leftuses_numbered_reportsas it's a very hid specific thing (and would require a bunch more renaming to make consistent).Also, for reference it seems that in order for the Linux kernel to separate composite descriptors into multiple devices as multiple udev nodes a kernel patch (per device) is required to add
HID_QUIRK_MULTI_INPUTto https://github.com/torvalds/linux/blob/master/drivers/hid/hid-quirks.chttps://electronics.stackexchange.com/a/400268
Given that there are only a handful of devices that support this (and there doesn't seem to be any additional udev flags to help identify the order in the descriptor), I don't think any additional code for Composite HID descriptors will be useful in Linux without a bunch of kernel work (and any hidraw code will have to know how to deal with older and newer kernels).
Possibly one change that could help would be retrieving a list of all of the usages and usage pages from the descriptor and providing a list in Linux for situations where matching is more important.
I was able to add support for Bluetooth Usage Page and Usage.
I believe I have per-usage page/usage pair working with hidraw. The implementation is a bit cleaner.
The device listing should match to what macOS currently does with hidapi.
I'm getting a different behavior as root and as non-root:
normal user:
With sudo:
The second to last and third to last entries seem to be identical though - at least from the information printed by hidtest. With one of those removed the output seems to match the macOS version.
You must have device permissions in order to read from hidraw. Either by setting up udev or using root.
Because you can't read the hid descriptors without permissions, it's not possible to scan all of the different composite descriptors and will just return 0s in the Usage Page.
@z3ntu Can you include the full hid descriptor here? I can try to debug the issue.
@haata Can you give me a command to get this descriptor please? :)
I've used this utility in the past, not sure if it's available on your distro.
https://github.com/DIGImend/usbhid-dump
The last example will convert the hid descriptor to human readable format.
Is this good?
This seems a bit silly. Why are we duplicating the code to parse the report descriptor in every backend? There is nothing platform specific about it.
Also, is creating a new device per usage part of the API?
Don't get me wrong, but this seems awful, IMO it is a really bad way to handle the issue. Why can't we just provide an array with the usages?
A quick test on my desktop:
yikes...
Not to mention this is a bad API for users, let's say I want to read the raw HID events from mice.
I have to look the list of devices and search for the devices with all the usages I want with the same path.
This is the default and only behavior on Windows. Best approach to make the API consistent - implement the wider behavior on all platforms
This is a big API change. HIDAPI has a very stable API and many applications that rely on it. Developer can't just tell to all library users, that they need to change their implementation
On Window each usage is reported as a separate device
Well, that's unfortunate. If you can tie the those HID devices in the Windows API down to a physical device then you can wrap around the behavior. This could be introduced in our API without breaking changes, just ABI changes.
Looks like it can be done with
GetPhysicalDescriptor, so I don't see any blocker. Let me know if I am wrong.I'm not sure there's a way to get the full HID descriptor on macOS (though I would be happy to be proven wrong).
Have you tried it? Does it work in all cases for all possible users of this library in production?
Replacing a property field with an array of values - is a very definition of breaking change
That's a breaking change too
When you're the only developer and the user of a project, it may sound like something that can be refactored, improved, to make your self feel better.
When hundreds of users rely on the behavior they got used to in years - introducing a new source file with some common implementation is a big deal. In ideal world, agree that it would be much better that way. In practice - I personally know a few users, who rely of the fact, that this library consist of a single
hid.csource file.Secondly, this project is maintained by an open-source community purely on their enthusiasm.
If you wan't to help - PRs are welcome (no breaking changes!).
Good luck with that. macOS and Windows already prohibit that as a security concern (because you shouldn't be allowed to sniff some passwords, user may want to type in). My opinion - it is a question of "when" not "if" Linux would prohibit it too.
Is there anything pending for this PR that I can help with?
I think this is active pending actions
can you check is
O_RDONLYenough for this?0- is a valid file descriptor and may be returned byopenO_RDONLY seems to be enough from my testing.
Pushed the change (and rebased on top of master)
@z3ntu
what about after https://github.com/libusb/hidapi/pull/139/commits/ab2516fcbae280c42911f6704537ff2fe4afd409 ?
I did a quick test removing my udev rules and it didn't work. We have to read directly from the hidraw interface
/dev/hidraw*so unless udev has given some read access the default looks like this (at least on Arch Linux)in such case I'd rather suggest using a whole different approach:
instead of sending ioctl, read the descriptor from sysfs, e.g.: /sys/class/hidraw/hidraw0/device/report_descriptor
Minimum supported kernel 2.6.39 has the implementation for this sysfs file, so there will be no backward compatibility issues.
Some thoughts out loud:
In such way, we could also enumerate usage/usage_page for libusb backend on linux.
Yeah, I think this might work well.
Let me put something together for hidraw.
libusb is possible if it's just for Linux (might take some work to map libusb to hidraw; hidraw doesn't make it easy to go in reverse, I've tried)
Something else might have to be done for freebsd (I haven't used BSD that much). But if the hid descriptors are exposed the parsing code would be the same.
A little bit more string manipulation than I would have liked, but I think it works well.
The only part I'm a bit concerned about is during the open where I have to locate the report_descriptor. I'm using the basename of the filepath as the udev sysname to do the sysfs path lookup.
This will break if a symlink is used, so I may also need to resolve any symlinks.
I am open to a better solution though.
Also, elevated permissions no longer required 😄
hidtest output is now idential as root and non-root with my current devices (and it wasn't on commit
ab2516for before)I didn't realize we had a
sysfs_pathalready available from udev, during enumeration.This makes the implementation even simpler. I do like how it looks.
But still, have a few comments below.
@ -220,6 +256,157 @@ static int uses_numbered_reports(__u8 *report_descriptor, __u32 size) {return 0;}let's make it more meaningful with
register_global_error_formatditto
snprintfinsteadhave it only once, outside the loop, and loose
usage_countvariablewcsdupfrom prev dev?Hmm, I agree that a bit more string manipulations than than one would wan't to see.
And yes, based on my experience, using
realpathis a good call.Since at this point we already have a device handle, I suggest to keep the
ioctlapproach for this scenario: it looks a bit cleaner, and less error prone (as in case with25, in my other comment).Hmm, I'll have to think about this a bit.
The while loop needs to go through at least 2 iterations in order to find the first usage/usage page pair (sometimes more) as it chunks through the report descriptor.
I use the count to determine whether or not more memory needs to be allocated.
Actually I think I have solution.
so do I, in case if you want to compare :)
Agreed.
Nicely done!
I'd like to check it myself with some of my devices (over the weekend).
I'll do some additional checking myself too. Bluetooth devices tend to have more corner cases as they tend to favour multiple usage/usage page pairs instead of multiple devices.
Should I squash all of the commits together?
Not necessary, I'd do a squash-merge anyway.
it's "its"
https://github.com/libusb/hidapi/pull/368