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.

Sign up for GitHub

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

Jump to bottom

Changed Service identification to handles instead of UUID #449

Merged
merged 8 commits into from
Mar 1, 2021
Merged

Changed Service identification to handles instead of UUID #449

merged 8 commits into from
Mar 1, 2021

Conversation

hbldh
Copy link
Owner

@hbldh hbldh commented Feb 19, 2021 •
edited
Loading

Identifying Services by UUID is wrong, since a peripheral can have multiple services with the same UUID. The proper identifier should be the handle instead. From previously having thought the service handles was not present in the APIs that Bleak use, I have now done a depper investigation and foudn that I was wrong, they were there and they were quite easily obtained as well.

This should once and for all prevent the This service is already present in this BleakGATTServiceCollection! error from occuring for users.

  • Windows backend
  • BlueZ backend
  • CoreBluetooth backend (tested in OS X 10.11 only)

Fixes #445 and #362.

This PR also removes all __str__ methods from subclasses of BleakGATT* in favour of the ones in the abstract base classes.

@hbldh hbldh added bug Something isn't working enhancement New feature or request Backend: pythonnet Issues or PRs relating to the .NET/pythonnet backend Backend: BlueZ Issues and PRs relating to the BlueZ backend Backend: Core Bluetooth Issues and PRs relating to the Core Bluetooth backend labels Feb 19, 2021
@hbldh hbldh self-assigned this Feb 19, 2021
@hbldh hbldh changed the title Changed Service identification to handles for Windows backend. Changed Service identification to handles instead of UUID Feb 21, 2021
@hbldh hbldh marked this pull request as ready for review February 21, 2021 19:34
@hbldh hbldh requested a review from dlech February 21, 2021 19:34
dlech
dlech reviewed Feb 23, 2021
View reviewed changes
Copy link
Collaborator

@dlech dlech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems straightforward. Just a few minor comments inline.

bleak/backends/bluezdbus/utils.py Outdated Show resolved Hide resolved
bleak/backends/bluezdbus/utils.py Outdated Show resolved Hide resolved
bleak/backends/characteristic.py Show resolved Hide resolved
bleak/backends/corebluetooth/service.py Show resolved Hide resolved
bleak/backends/service.py Show resolved Hide resolved
examples/service_explorer.py Outdated Show resolved Hide resolved
dlech
dlech reviewed Feb 25, 2021
View reviewed changes
examples/service_explorer.py Outdated Show resolved Hide resolved
bleak/backends/service.py Outdated Show resolved Hide resolved
@hbldh hbldh merged commit 4a3f862 into develop Mar 1, 2021
@hbldh hbldh deleted the feature/service_handles branch March 1, 2021 08:26
@AGlass0fMilk
Copy link

Fantastic work!!! 👍

@kevincar kevincar mentioned this pull request Mar 30, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dlech dlech dlech left review comments

Assignees

@hbldh hbldh

Labels
Backend: BlueZ Issues and PRs relating to the BlueZ backend Backend: Core Bluetooth Issues and PRs relating to the Core Bluetooth backend Backend: pythonnet Issues or PRs relating to the .NET/pythonnet backend bug Something isn't working enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

bleak.exc.BleakError: This service is already present in this BleakGATTServiceCollection!
3 participants
@hbldh @AGlass0fMilk @dlech