search

sidit77 / async-hid

A async Rust library for interacting with HID devices
MIT License
3 stars 2 forks source link

Hashing DeviceInfo #5

Closed zardini123 closed 4 months ago

zardini123 commented 5 months ago

Like issue #3 (Hashing DeviceId), being able to hash DeviceInfo allows for operations of DeviceInfo in sets, especially in the case of keeping track of used/unused devices. Except most importantly, being able to actually open a device for I/O is done with DeviceInfo.

DeviceInfo contains a couple different types, including String, u16, DeviceId, and BackendPrivateData.

https://github.com/sidit77/async-hid/blob/fc7a832ec965c2441b4a477bb6cd0d103ea898e7/src/lib.rs#L14-L33

String and u16 are hashable. Once issue #3 is solved, DeviceId is hashable. Problem is, BackendPrivateData for winrt implementation contains OnceLock does not impl Hash (OnceLock docs). Therefore, putting #[derive(Hash)] on BackendPrivateData fails.

https://github.com/sidit77/async-hid/blob/fc7a832ec965c2441b4a477bb6cd0d103ea898e7/src/backend/winrt/mod.rs#L167-L170

Implementation ideas

From doing analysis on how traits of OnceLock is implemented, OnceLock's PartialEq trait simply calls the .get() operands to get the underlying value wrapped in an Option. Implementation of Hash for DeviceInfo could hash the result of the .get() call.

sidit77 commented 5 months ago

I think we should simply not include private_data in the hash (an probably also in the equality check) for two reasons: 1) It's not actually part of the core api and instead an implementation detail of the SerialNumberExt extension trait. It only exists to make the implementation of this trait more efficient. 2) I don't think it actually provides usefull information. Unless devices can dynamically change their name or ids, two DeviceInfos are equal if thier DeviceIds are equal.

zardini123 commented 5 months ago

This totally makes sense, thank you!

2. I don't think it actually provides usefull information. Unless devices can dynamically change their name or ids, two DeviceInfos are equal if thier DeviceIds are equal.

Should the hash impl (and the equality impl) only work with the DeviceId fields? Or should the four u16 fields (product_id, vendor_id, etc.) also be included in the comparison?

sidit77 commented 5 months ago

I think technically only the DeviceId field are necessary, but I would keep four u16s and the name in there for now as "sanity check". I don't think the performance impact of a few more "useless" comparisions or hashes will ever be relevant.