Skip to content
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

A selectable interface for waiting on reads being available #202

Open
carlosmn opened this issue Oct 24, 2020 · 6 comments
Open

A selectable interface for waiting on reads being available #202

carlosmn opened this issue Oct 24, 2020 · 6 comments
Labels
documentation Improvements or additions to documentation enhancement New feature or request

Comments

@carlosmn
Copy link

Right now, you can use hid_read to wait for something to be available or hid_read_timeout to limit how long you want to read. While sticking to single-threaded code when interacting with the library (as you might want to do in certain languages or due to thread-safety not being explicitly defined for the multiple implementations behind the library), this forces you to choose the resolution at which you want to be able to react to requests to write due to e.g. user requests as a balance between that reaction time and constantly waking up from waiting on the device needlessly.

Ideally I think there would be some descriptor I can pass to select or poll so it can sleep until one of several things happen so the code can react to

If the library however is meant to be thread-safe and I should be able to read and write on different threads concurrently, then that's something that really should be made explicit in the documenation. I haven't found anything about threads in the README or the header save for a note in hid_init().

@Youw
Copy link
Member

Youw commented Oct 24, 2020

Yeap, README better be updated (e.g. #45).

Async interface is is generally a good idea, but something that is not simple to implement, not for current implementation of hidapi.

As for multithreaded usage of hidapi - many projects successfully using a separate thread for hid_read/_timeout, and another thread for read/everything else.

@carlosmn
Copy link
Author

A full-featured async interface is indeed quite a bit of work. I was looking at what libusb provides and to get it to really work, it looks like you'd unfortunately need to get libusb, hidapi and the caller to coordinate so libusb has its async API called at the appropriate times. It's a shame as this doesn't seem very workable for a library that's trying to work around the differences in the different platforms.

I'm using rust so the thread safety is a property of the type system, and whether reading and writing in different threads is safe, and how that would work is part of the type, so it's not just a matter of using it in multiple threads but the library wrapping this in rust needs to be confident in how this would work. I can use my own fork to do some messing around myself, but something explicit that upstream can use to get the types to line up would definitely be good to have.

This might be a naïve approach, but given that this library already has a background thread that wakes up when there is something to read in order to add it to a queue (on libusb and macOS), when that is done, it could also notify a file descriptor via eventfd/kqueue which the user of the library will be listening in. This does make it a bit cumbersome to use as you'd need to consume this message in addition to the actual data, but it should make it possible to provide this interface without too many changes.

There is of course the issue of hidraw not behaving this way and a user of the library could instead wait on the same descriptor that the library is waiting on, but then you don't want to consume data from it. And I guess Windows has its own ways of doing things.

@mcuee mcuee added documentation Improvements or additions to documentation enhancement New feature or request labels Dec 4, 2020
@mcuee
Copy link
Member

mcuee commented May 10, 2023

Reference discussion here at hidapi-rs: interesting the suggestion there is to remove HIDAPI depedancy.

@Julusian
Copy link
Contributor

Reference discussion here at hidapi-rs: interesting the suggestion there is to remove HIDAPI depedancy.

I suspect that is partly influenced by the rust ecosystem, where it is preferred to avoid c libraries when possible due to soundness concerns. And also there will be a cost to doing the type translation and things, which could be avoided by having more of it in rust.
The soundness guarantees of rust is only really true if the libraries used (in this case hidapi) are also sound, and that can be hard to verify.

@Youw
Copy link
Member

Youw commented May 10, 2023

remove HIDAPI depedancy

Notably, it says "Remove sigal11 hidapi dependency", even though it looks like they are using libusb/hidapi.

@carlosmn
Copy link
Author

Removing non-memory-safe code is generally nice, but as the author of that library's most recent work on (optional) bypassing of hidapi, for me it's mostly not about avoiding the C language.

It is still about what I wrote above. In order to use libusb/hidapi and use async (or some other form of notification) you would first have to come up with something that makes sense in C across the platforms and backends and get it into this library. Then you would have to figure out how to make that work in a different language, which may have its own assumptions.

For Linux, if you're willing to live with hidraw, it is basically trivial to bypass hidapi since the kernel is the one doing the bulk of the work. For the other backends/OSs, it is more work, but not having to consume hidapi's API and instead being able to go one level deeper, well that does allow you to make choices that make async in other languages easier.

It's just unfortunate how all of this interacts (I'm not even sure how you could try to read and write concurrently if you're in a language that doesn't support threading very well).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
Status: To do
Development

No branches or pull requests

4 participants