Usage Page and Usage on Linux with hidraw #139
Conversation
|
Thanks you very much for doing this PR. Before I go deep into the changes, a few general comments:
|
|
Sounds good, I'll update the commit shortly. |
haata
changed the title
|
Updated the commit.
|
|
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 https://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. The last example will convert the hid descriptor to human readable format. |
|
Is this good? |
There was a problem hiding this comment.
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:
$ sudo hidtest/hidtest-hidraw | grep 'Device Found' | wc -l
59
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.
$ sudo hidtest/hidtest-hidraw | grep 4079 -A 5 -B 1
Device Found
type: 046d 4079
path: /dev/hidraw4
serial_number: 4079-df-7a-84-07
Manufacturer: Logitech
Product: USB Receiver
Release: 4000
Interface: 2
Usage (page): 0x6 (0x1)
--
Device Found
type: 046d 4079
path: /dev/hidraw4
serial_number: 4079-df-7a-84-07
Manufacturer: Logitech
Product: USB Receiver
Release: 4000
Interface: 2
Usage (page): 0x2 (0x1)
--
Device Found
type: 046d 4079
path: /dev/hidraw4
serial_number: 4079-df-7a-84-07
Manufacturer: Logitech
Product: USB Receiver
Release: 4000
Interface: 2
Usage (page): 0x1 (0x1)
--
Device Found
type: 046d 4079
path: /dev/hidraw4
serial_number: 4079-df-7a-84-07
Manufacturer: Logitech
Product: USB Receiver
Release: 4000
Interface: 2
Usage (page): 0x1 (0xff00)
--
Device Found
type: 046d 4079
path: /dev/hidraw4
serial_number: 4079-df-7a-84-07
Manufacturer: Logitech
Product: USB Receiver
Release: 4000
Interface: 2
Usage (page): 0x2 (0xff00)
--
Device Found
type: 046d 4079
path: /dev/hidraw4
serial_number: 4079-df-7a-84-07
Manufacturer: Logitech
Product: USB Receiver
Release: 4000
Interface: 2
Usage (page): 0x4 (0xff00)
This is the default and only behavior on Windows. Best approach to make the API consistent - implement the wider behavior on all platforms
|
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 |
|
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 Secondly, this project is maintained by an open-source community purely on their enthusiasm.
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 did a quick test removing my udev rules and it didn't work. We have to read directly from the hidraw interface |
|
in such case I'd rather suggest using a whole different approach: 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: |
|
Yeah, I think this might work well. 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) |
- Elevated permissions are no longer required to query usage and usage pages - Some additional work is required when opening a /dev/hidraw device in order to locate the sysfs path * Needed to determine if we're using numbered reports or not
|
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. Also, elevated permissions no longer required 😄 |
- In most cases enumeration will be used to get the absolute path to /dev/hidraw* However it is possible to use a symlink to open the hidraw interface
|
hidtest output is now idential as root and non-root with my current devices (and it wasn't on commit ab2516f or before) |
- Code review fixes - Simplify get_next_hid_usage loops
Co-authored-by: Ihor Dutchak <ihor.youw@gmail.com>
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. |
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. The implementation idea is similar, as in #125 for macOS.
| break; | ||
| } | ||
|
|
||
| /* Skip over this key and it's associated data */ |
Adapted from ApeironTuska's PR to PastaJ36/hidapi (PastaJ36/hidapi#1)
Which was adapted from djpnewton's PR to signal11/hidapi (signal11/hidapi#6)
Also addresses some of the issues mentioned in (signal11/hidapi#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)