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

make sure disconnect is getting called by using finally #91

Closed
wants to merge 2 commits into from
Closed

Conversation

flecmart
Copy link

I added finally blocks to the logic to make sure BleakClient disconnects as suggested in https://bleak.readthedocs.io/en/latest/usage.html

@mjmccans
Copy link

For what it is worth, I think @flecmart's changes are good, subject to some minor formatting cleanup, and is a better way to ensure that disconnect is called. I have been using these changes for a little while with my script and addon and they are working well so far.

@lone-cloud
Copy link

You can't raise exceptions like that. They must be derived from the base Exception class.

@flecmart
Copy link
Author

flecmart commented Aug 5, 2022

You're right @lone-cloud - I fixed the raising of exceptions in this PR. Thank's for the hint. I can't confirm that my changes make the sensor work in the newest home assistant release - there is still the conflicting bleak versions.

@lone-cloud
Copy link

I already tested your changes in #93 and I know that it won't work. We believe that this custom component needs to be rewritten using the new bluetooth integration. I'm leaving for vacation tomorrow, so I won't have time to mess around with it until next Tuesday at the earliest.

@flecmart
Copy link
Author

flecmart commented Aug 5, 2022

Yeah, I was reading through that issue, too. I think you're right about moving to the official bluetooth integration. Did not wan't to have a broken PR here, though :-)

@flecmart flecmart closed this Sep 16, 2022
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.

3 participants