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

BlueZ: fix removed devices included in discovered_devices #967

Merged
merged 1 commit into from
Aug 30, 2022

Conversation

dlech
Copy link
Collaborator

@dlech dlech commented Aug 29, 2022

This fixes a regression introduced in v0.15 where device removed from BlueZ while scanning were no longer removed from this list of discovered devices. This caused the discovered_devices property to return devices that could no longer be connected to by BlueZ since they were removed.

Fixes #942.

@dlech
Copy link
Collaborator Author

dlech commented Aug 29, 2022

@bdraco can you please test?

@bdraco
Copy link
Contributor

bdraco commented Aug 29, 2022

Will give it a shot 👍 Thanks

Handles a device being removed from BlueZ.
"""
try:
self._devices[device_path]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
self._devices[device_path]
del self._devices[device_path]

Should this be del?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, of course!

@bdraco
Copy link
Contributor

bdraco commented Aug 30, 2022

Been testing this for a few hours with the above del change and everything is working great.

Devices are going unavailable when they disappear now.

@bdraco
Copy link
Contributor

bdraco commented Aug 30, 2022

To make sure it is effective I did another test by setting up a humidity sensor.

You can see from the graph when I took the batteries out as the device goes unavailable a few minutes later. The holes in the graph are from the latest test where I removed the batteries and later replaced them each time.
Screen Shot 2022-08-29 at 8 44 47 PM

Copy link
Contributor

@bdraco bdraco left a comment

Choose a reason for hiding this comment

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

Tested ok with the suggested change in #967 (comment)

@bdraco
Copy link
Contributor

bdraco commented Aug 30, 2022

Screen Shot 2022-08-29 at 11 21 21 PM

Tested with an extended battery removal as well. Recovered quickly

This all looks good with the del change

…d_devices

This fixes a regression introduced in v0.15 where device removed from
BlueZ while scanning were no longer removed from this list of discovered
devices. This caused the `discovered_devices` property to return
devices that could no longer be connected to by BlueZ since they were
removed.

Fixes #942.
@dlech dlech force-pushed the bluez-fix-device-removed-while-scanning branch from d2441c4 to aa27f4c Compare August 30, 2022 15:32
@dlech
Copy link
Collaborator Author

dlech commented Aug 30, 2022

Thanks for testing!

@dlech dlech merged commit bdfaea1 into develop Aug 30, 2022
@dlech dlech deleted the bluez-fix-device-removed-while-scanning branch August 30, 2022 15:36
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.

Devices are no longer removed from .discovered_devices on the bluez scanner when InterfacesRemoved is fired
2 participants