-
Notifications
You must be signed in to change notification settings - Fork 3
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
Hashing DeviceId #3
Comments
I'd be interested in contributing changes once there is agreement on the general ideas 😄 |
I'm not sure how true this remains once you start plugging devices in and out. Irc on Linux the device id essentially boils down to some path like On Windows I'm fairly certain that the id is actually unique. The same goes for MacOS if I read the documentation correctly:
I don't think it's necessary to convert the But yeah, I think we would have to create a wrapper type for that. |
Would you think doing this research would be required before implementing If we associate a unique DeviceId to be a unique device, and we assume there could be platforms where a DeviceId could be associated with two different devices, then would the property of equivalence not work either? How would you be able to differentiate those two devices? Personally at that point I would just notate the inconsistency of the platform in the documentation but keep the convenience of equivalence and hashing.
Sure thing. What would the naming convention for a type like this be? |
I don't think that this is a blocker either. The id struct is opaque and platform dependent anyways so changing it later if/when it becomes a problem shouldn't even be a breaking change.
HashableHSTRING? It's an internal implementation detail of the Windows backend so I don't think it really matters tbh as long as it's somewhat reasonable. |
I realized that if Hash could be implemented in HSTRING directly, we wouldn't have to worry with an intermediate type. So I sent out a PR to windows-rs with a HSTRING hash impl. Once that is accepted, then changes can be made here to hash HSTRING directly. The PR: microsoft/windows-rs#2924 It seems the maintainers of windows-rs accept very quickly and release cycle is every couple days, so we wouldn't have to wait long. |
Hi @sidit77! Thank you for this wonderful library.
I have been working to migrating one of my projects to using this. While migrating, I found DeviceId is not hashable.
My thought is, assuming there cannot be more than one unique device associated with the same DeviceId, a DeviceId represents a unique device. If a DeviceId represents a unique device, we can hash DeviceId to quickly know if an identical device is inside a hashing collection, such as a
HashSet
.My reason why having DeviceId available in a hashing collection would be useful is for doing set operations on HID devices. For example, we can get a set of "unused" devices by taking the set difference of a set of "used" devices and all devices. This task can be done easily using a
HashSet
.Research into Implementation
DeviceId is a tuple struct of BackendDeviceId. BackendDeviceId is whatever type that is set by the platform being used, as defined in src/backend/mod.rs.
async-hid/src/lib.rs
Lines 92 to 95 in fc7a832
hidraw
BackendDeviceId is a PathBuf which has Hash impl (source)
async-hid/src/backend/hidraw/mod.rs
Line 163 in fc7a832
iohidmanager
BackendDeviceId is a RegistryEntryId which is a async-hid tuple struct of a u64. Could derive Hash probably with a simple #derive change.
async-hid/src/backend/iohidmanager/mod.rs
Line 192 in fc7a832
async-hid/src/backend/iohidmanager/service.rs
Lines 64 to 66 in fc7a832
winrt
BackendDeviceId is a windows::core::HSTRING. Does not have a Hash impl directly, but could be converted to a OsString (which is hashable) using HSTRING's
to_os_string
function. Not sure if there is any losses or performance issues with this idea. A wrapper type would probably be required to implement Hash that callsto_os_string
upon hashing, unless there is a more pragmatic idea.async-hid/src/backend/winrt/mod.rs
Line 172 in fc7a832
The text was updated successfully, but these errors were encountered: