search

ruabmbua / hidapi-rs

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

feat: Native macOS backend #131

Open Tiwalun opened 1 year ago

Tiwalun commented 1 year ago

This is a port of the hidapi code for macOS to pure Rust, with some small modifications to make it more thread safe.

I've tested it with probe-rs, and it seems to work fine so far.

The error handling could be improved, but that might be better to do in coordination with the other native backends.

Remaining tasks

ruabmbua commented 1 year ago

Hi, thank you for your work! I will hopefully have some time to review it in the coming week.

Tiwalun commented 1 year ago

Thanks for looking at this! It's good to get some feedback from somebody who knows these APIs.

Having written something similar myself, this design is really strange. Rust's standard library has functionality which could make the implementation significantly simpler.

As mentioned in the description, this is more or less a direct port of the hidapi C library, with only minor changes to work around the problem mentioned in #127.

I'm not sure what the requirements regarding compatibilty with the existing implementation are. If this is only an optional backend, and the hidapi library is kept as an alternative, then it should probably be compatible, which limits the amount of changes that can be done. On the other hand, if compatibility is not a concern, then some of the improvements like using a mpsc::channel could be done more easily.

@ruabmbua What are your thoughts regarding compatibility with the C library?

ruabmbua commented 1 year ago

Thanks for the review and the work done! I already started with it myself, but could not motivate myself to start learning macos API`s ^^.

Regarding compatibility to the current public API of the library: I would keep it as is for now (with non breaking changes allowed). This means I agree that the new optional pure rust macos backend should try to mimic the old one as close as it can.

I might revisit the public API in the future (v3.0.0), and do mostly some fixes that make it more idomatic rust. I am not sure about async as of yet, I struggle to see the large benefit of making the library true async. Its not likely, that an application will need to handle a huge amount of hid devices in comparison to e.g. network connections. You can always spawn dedicated worker threads for devices, and integrate them with e.g. async channels + an API wrapper into e.g. tokio. Maybe I am missing a use case, where a true async implementation for hidapi would help.