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

Ignore failed descriptor reads on Windows #362

Merged
merged 3 commits into from
Jan 10, 2024
Merged

Ignore failed descriptor reads on Windows #362

merged 3 commits into from
Jan 10, 2024

Conversation

DavidZemon
Copy link
Contributor

This brings the Windows implementation in line with Linux

Fixes #361

This brings the Windows implementation in line with Linux

Fixes #361
@DavidZemon
Copy link
Contributor Author

I tossed a release commit into the branch on the off chance you'd be willing to get this released today, but would be happy to drop/edit that commit if you're not ready to release yet.

@qwandor qwandor requested a review from qdot January 9, 2024 19:38
@DavidZemon
Copy link
Contributor Author

I've just added another commit in the middle of the branch that adds better low-level handling of the characteristics. After testing my first fix, I found that it was still failing on some devices because GattCommunicationStatus::AccessDenied was being treated as a full-blown error.

Discovery of a peripheral's services should not error out merely because
one of the characteristics encountered an "AccessDenied" or similar
error
@DavidZemon
Copy link
Contributor Author

Release commit has been amended with today's date.

@qdot
Copy link
Contributor

qdot commented Jan 10, 2024

Ok, I gotta say, submitting the CHANGELOG update and updating it's date in the PR daily is a surprisingly good way to make sure I don't forget about this PR. :P

@qdot qdot changed the base branch from master to dev January 10, 2024 20:56
@qdot qdot merged commit 1937ff5 into deviceplug:dev Jan 10, 2024
4 checks passed
@DavidZemon DavidZemon deleted the feat/add-windows-resiliency branch January 10, 2024 20:57
@DavidZemon
Copy link
Contributor Author

Is the release to crates.io automatic? Just wondering when I should I update my project that is using btleplug.

@DavidZemon
Copy link
Contributor Author

Is the release to crates.io automatic? Just wondering when I should I update my project that is using btleplug.

Nevermind - I see this was merged into dev not master, so obviously not going to release automatically.

@qdot
Copy link
Contributor

qdot commented Jan 10, 2024

Nope, releases are done by hand still, but it's published now.

@DavidZemon
Copy link
Contributor Author

Nope, releases are done by hand still, but it's published now.

Thanks! Already have my PR up to use the new version and it's one of the last things for getting the app deployed to prod, so thank you!

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.

Strange and undescribed error thrown once discovering services is called
2 participants