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

Hashable DeviceId #4

Merged
merged 2 commits into from
Apr 13, 2024
Merged

Hashable DeviceId #4

merged 2 commits into from
Apr 13, 2024

Conversation

zardini123
Copy link
Contributor

Implements Hash for DeviceId.

As mentioned in #3, all underlying platform-specific types used by DeviceId already implement Hash except windows-rs HSTRING. This PR utilizes the recent addition of HSTRING Hash impl to windows-rs (the windows-rs commit). Due to utilizing a more recent windows-rs version with API changes, additional changes were made to async-hid to satisfy those changes.

As of writing, the commit has been merged into windows-rs master (the PR, but a new release has not been released yet (will be version 0.56.0). Will update this PR when windows-rs releases a change.

Resolves #3

@zardini123
Copy link
Contributor Author

After attempting to include changes in a personal project, I found DeviceInfo::enumerate hangs indefinitely. From further analysis, the upgraded windows-rs crate these changes require (due to HSTRING hash contribution) result in some changes somewhere with the FindAllAsyncAqsFilter that hangs indefinitely. Going back to async-hid's original dependency version of windows-rs solves the problem.

let devices = DeviceInformation::FindAllAsyncAqsFilter(DEVICE_SELECTOR)?

Instead of spending the time to solve the reason of this indefinite hanging issue with windows-rs, I will instead implement a HashableHSTRING wrapper type as suggested by @sidit77 in #3 (comment).

@zardini123
Copy link
Contributor Author

Manually tested, and found Hash implementation works. Changes are now ready for review.

@zardini123 zardini123 marked this pull request as ready for review April 2, 2024 02:00
@zardini123
Copy link
Contributor Author

Let me know if there is any changes that need to be done for this to be merged. Thanks!

@sidit77
Copy link
Owner

sidit77 commented Apr 8, 2024

I'm a bit busy right now without access to my PC but I'll try to review it as soon as I'm able.

@zardini123 zardini123 changed the title Hashable deviceid (and upgrades to newer windows-rs version) Hashable deviceid (via wrapper type) Apr 8, 2024
@zardini123 zardini123 changed the title Hashable deviceid (via wrapper type) Hashable DeviceId Apr 8, 2024
@zardini123
Copy link
Contributor Author

No problem, thanks @sidit77!

@sidit77 sidit77 merged commit faafba3 into sidit77:main Apr 13, 2024
@sidit77
Copy link
Owner

sidit77 commented Apr 13, 2024

Thanks for your contribution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hashing DeviceId
2 participants