ruabmbua / hidapi-rs

Rust bindings for the hidapi C library
MIT License
173 stars 82 forks source link

Add a native Rust Linux hidraw backend to replace hidapi #107

Closed carlosmn closed 1 year ago

carlosmn commented 1 year ago

I've been meaning to get around to this and just had a few free days so here we are. My ultimate goal is being able to have something on which we can do sensible async. At worst maybe we can add a AsRawFd but we can probably do better.

I've been away from the device I normally use so I haven't been able to test the IO portion itself, but I have checked the enumeration shows the same via the lshid example including the page and usage page values. The actual IO should be trivially correct as we're just asking the kernel to do the IO (though the report get situation with the buffer is a bit suspicious). I've tried to keep the behaviour the same as the C hidapi by using nix to capture the system calls more accurately instead of letting the Rust stdlib make it nicer for us.

I've added this as a new feature/backend in case there are particularities of this implementation that means you might want to go back to hidapi. It also made it easier to check both backends build and enumerate the same. The name is probably not the best, but I started by dealing with libudev so that's what I named it. It probably would make sense to rename it if/when this gets merged. Eventually hopefully this can be turned on by default, but I'm not sure how this project would want to deal with setting the defaults.

I see there were a couple of attempts at doing some generic-ish backend work and bypassing hidapi. I looked at a branch or two, but ended up building the interaction in a way that felt like it made sense to me. Hopefully it's sensible for others too.

The Box<dyn ...> does mean an extra allocation but I think the alternative of struct HidDevice<T: HidDeviceBackend> would break API. An alternative would also be to embed the backend HidDevice and change the type of the field depending on the backend at compile time. That wouldn't really use the trait to make sure you've implemented everything though, but that's not the end of the world as it's all internal types anyway (and there are workarounds you can do in tests to enforce that, I think).

I added a couple of test files from a wireless mouse that I happen to have nearby to test that the usage and usage_page values are the same as hidapi.

carlosmn commented 1 year ago

I've renamed the backend and options to simply linux as ideally that would be the only/default one there.

carlosmn commented 1 year ago

It looks like naming an option linux makes clippy think we're confused about what we mean, so now it's named linux-native and linux_native. This is probably for the best as it makes space in the naming for things like macos_native.

I've also edited some earlier commits so we open the right file so things do work.

Unfortunately by having the path be the dev node, i.e. /dev/hidrawN, the udev library doesn't seem to have a way to open based off of that, as it only has Device::from_syspath as a way to create the device directly, so we don't actually have enough information, so get_device_info() is currently simply broken.

carlosmn commented 1 year ago

Oh I also made it so the check_error methods are only available when using C hidapi.

ruabmbua commented 1 year ago

Regarding udev problem with the devnode, can you maybe elaborate more? I think I had no issues with that when I was working on this myself. It is also poissible to map from /dev file to /sys path with udev (although I do not remember the exact API for that).

carlosmn commented 1 year ago

In order to match the C hidapi does, and so you can pass the DeviceInfo::path into HidApi::open_path and have it work then we need to open via the dev node, and that's what we can store in our struct. I don't know off hand if the files under /sys can be opened to use as devices.

The way C hidapi then can get the info is via udev_device_new_from_devnum and then filling is as usual, but that constructor is not available in the Rust udev library. I don't see a way to go from one to the other without going through and listing them all, but I'm not that familiar with the layouts here.

carlosmn commented 1 year ago

It looks like it should be easy yeah https://github.com/systemd/systemd/blob/5015b5014bcff93371aef2c78b92efcfc2e38a40/src/libsystemd/sd-device/sd-device.c#L304 though it would be better if the udev library supported it.

carlosmn commented 1 year ago

It turns out this completely broke macOS (and probably Windows) because they want to add methods so I guess I'll see if I can tackle that.

carlosmn commented 1 year ago

It's not super clean but I've made it so each os can implements its own extras.

carlosmn commented 1 year ago

Heh, it looks a bit too much for the PR view :) I'll go through the comments. A lot looks like the result of porting the C code to Rust and keeping some of its behaviour.

ruabmbua commented 1 year ago

Thanks for the work, I will merge this back now and do some small fixups. I will then release it to crates.io after I have some more time verifying it does not break anything (hopefully I have time soon).

carlosmn commented 1 year ago

Awesome, glad we could get to something we're happy with.