ruabmbua / hidapi-rs

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

Merge ruabmbua/master fork to Osspial/master #12

Closed ruabmbua closed 6 years ago

ruabmbua commented 6 years ago

Edit: this PR contains many more changes, and is here to trick github.

Hi,

I recently hit a API limitation of the library, which turned out to be a real pain in the ***.

To be more specific I came into the situation, where I absolutely had to capture an open hid device by a closure with static lifetime bounds (handler of a gtk-rs button click event).

I could not figure out any way to handle this, and so I propose this patch to the library, which should be backwards compatible (except by one thing which I will go through later). The advantage of it is the much greater flexibility with handling devices and the api, without having to worry about the lifetime constraints.

For example you now can:

It is implemented, by replacing the PhantomData marker inside the devices, which ensured the lifetime bound to the API, by a new mechanism, which introduces a refcounter to the global hidapi ffi context.

It is just a new HidApiLock struct with a special constructor, and a Drop impl, which releases and acquires the global API lock. This struct is injected inside devices and the HidApi context with a Rc smartpointer. To make sure droporder is guaranteed under any condition, devices use a ManuallyDrop smart pointer, which makes sure the device itself gets dropped before the global API context lock.

And yes, I also did a little bit of dependency updating in my fork, I hope that is ok for you.

Edit: The "one thing", which I forgot is, that HidDevice<'a> is now just HidDevice, which has to be adapted in cases, where the struct was used inside another struct from the library user. This should anyways be trivial.

mberndt123 commented 6 years ago

i ran into a similar problem, +1 for this PR!

mberndt123 commented 6 years ago

By the way, what do you think about using Arc instead? I'd say the atomic reference counting on the API is unlikely to lead to performance problems in any real program, while I do need to be able to pass a HidDevice between threads (the design of the program I'm trying to extend requires this).

ruabmbua commented 6 years ago

I considered it, but I was not sure, if hidapi is threadsafe on all platforms (and I did not have any time to confirm this).

If we change it, we also have to change the global api lock to an AtomicBool.

ruabmbua commented 6 years ago

You can workaround the limitation by using hid IO thread with rust mpsc::channel()