ruabmbua / hidapi-rs

Rust bindings for the hidapi C library
MIT License
165 stars 79 forks source link

Raw Windows backend #124

Closed sidit77 closed 11 months ago

sidit77 commented 1 year ago

Hello,

I've spend some time porting the Windows backend of hidapi to Rust. The lshid and open_first_device examples as well as my headset-controller seems to work just like before.

Progress

My main question is how I should proceed with the error handling. The current HidError enum is doesn't feel sufficient. I think it might be a good idea to turn the HidApiError case into a more general BackendError that contains a backend specific error (in my case a WinError or something like that contains the windows error code for example). This should allow for way better error handling than the current text only approach of the c library. What do you think?

ruabmbua commented 1 year ago

Hey, thanks for working on this. I will get around to review this in more detail soon!

Regarding error handling: I would like to make the library as comfortable to use as possible, meaning I`d prefer making it easy handling errors in a cross platform manner as far as possible. Of course if someone really wants to handle some platform specific error, we should still add that to be directly matchable.

The current state with the weird string based errors that are not even completely implemented for all the backends is just inherited by hidapi C library, and I am not really happy with it anyway. Maybe I `ll have some time to clean this up some more.

ruabmbua commented 1 year ago

b.t.w. cool project, one of the uses I still get out of this library is my headset! Maybe I will add support for it, as my tool is cmdline only for 1 setting I care about ^^

sidit77 commented 1 year ago

Another thing to consider is that I'm currently using the "old" win32 APIs. However Windows also has newer WinRT APIs which are much higher level, inherently async, and don't even require unsafe when used from Rust.

#[pollster::main]
async fn main() -> anyhow::Result<()> {
    let selector = HidDevice::GetDeviceSelectorVidPid(0xFFC0, 0x1, 0x1038, 0x2206)?;
    let devices = DeviceInformation::FindAllAsyncAqsFilter(&selector)?.await?;
    if let Some(device) = devices.into_iter().next() {
    println!("Found device: {}", device.Name()?);
        let device = HidDevice::FromIdAsync(&device.Id()?, FileAccessMode::ReadWrite)?.await?;
        {
            let output = device.CreateOutputReport()?;
            let len = output.Data()?.Length()?;
            let writer = DataWriter::new()?;
            writer.WriteBytes(&[0x0, 0xb0])?;
            for _ in 0..(len - 2) {
                writer.WriteByte(0)?;
            }
            output.SetData(&writer.DetachBuffer()?)?;
            device.SendOutputReportAsync(&output)?.await?;

        }
        let (sender, receiver) = flume::unbounded();
        let token = device.InputReportReceived(&TypedEventHandler::new(move |_, args: &Option<HidInputReportReceivedEventArgs>| {
            if let Some(args) = args {
                let report = args.Report()?;
                sender.send(report).unwrap();
            }
            Ok(())
        }))?;
        while let Ok(report) = receiver.recv() {
            let buffer = report.Data()?;
            let reader = DataReader::FromBuffer(&buffer)?;
            let mut bytes = [0u8; 8];
            reader.ReadBytes(&mut bytes)?;
            println!("{:?}", bytes);
        }
        device.RemoveInputReportReceived(token)?;
    }
}

This example uses nothing but the official windows crate from Microsoft and no utillity classes (except for anyhow, pollster, and flume but that's just for the sake of this example).

The only problem is that this might bump the minimal supported Windows version and that the code obviously diverges more from the original C code of hidapi so I would be less confident that they always do the exact same thing.

ruabmbua commented 1 year ago

Can you rebase this onto master, and add a CI pipeline step for this new backend? We now have CI pipelines for all the linux, windows, mac backends.

sidit77 commented 12 months ago

The master branch has a new get_report_descriptor function that I still have to implement. I'm currently study for my exams so I'll have to see when I feel like procrastinating :P.

ruabmbua commented 12 months ago

I think we can just make the get_report_descriptor optional via a cargo feature, and disable it for this backend by default. It can be picked up later, seems like for windows it will be more difficult to implement it.

sidit77 commented 12 months ago

I've implemented get_report_descriptor like this for now:

fn get_report_descriptor(&self, _buf: &mut [u8]) -> HidResult<usize> {
    Err(HidError::HidApiError { message: "get_report_descriptor is not supported on this backend".to_string() })
}
sidit77 commented 11 months ago

I have now actually ported the get_report_descriptor function. It return the same output as the c library for all hid devices on my pc.

However it would probably be a good idea to also test it against the testcases from the hidapi repo. Also some of the code still needs some refactoring to make it more ideomatic and efficient.

sidit77 commented 11 months ago

I finally found the time to port all the descriptor parser tests and fixed two bugs that caused some of them to fail.

I think it should be done now.

ruabmbua commented 11 months ago

Also seems like CI is failing (probably simple to fix, looks like a typo with launching cargo test

ruabmbua commented 11 months ago

Thank you for all the hard work! I will not yet release this to crates.io, I want to test it a bit myself before. I will probably have some time in the coming week.