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 battery workaround #393

Closed
dlech opened this issue Dec 13, 2020 · 8 comments
Closed

BlueZ battery workaround #393

dlech opened this issue Dec 13, 2020 · 8 comments
Labels
Backend: BlueZ Issues and PRs relating to the BlueZ backend bug Something isn't working

Comments

@dlech
Copy link
Collaborator

dlech commented Dec 13, 2020

  • bleak version: 0.10.0
  • Python version: N/A
  • Operating System: Linux
  • BlueZ version (bluetoothctl -v) in case of Linux: >= 5.48 <5.55

Description

Currently, we have a workaround for the standard battery service in the BlueZ backend based on BlueZ version.

# Special handling for BlueZ >= 5.48, where Battery Service (0000180f-0000-1000-8000-00805f9b34fb:)
# has been moved to interface org.bluez.Battery1 instead of as a regular service.
if str(char_specifier) == "00002a19-0000-1000-8000-00805f9b34fb" and (
self._bluez_version[0] == 5 and self._bluez_version[1] >= 48
):
props = await self._get_device_properties(
interface=defs.BATTERY_INTERFACE
)
# Simulate regular characteristics read to be consistent over all platforms.
value = bytearray([props.get("Percentage", "")])
logger.debug(
"Read Battery Level {0} | {1}: {2}".format(
char_specifier, self._device_path, value
)
)
return value

However, I have just learned that there is a -P battery option to bluetoothd that can disable this feature. So the version check will actually break things for someone who has set this option.

So the logic should probably be changed so that rather than looking at the BlueZ version, just say: if we are looking for the battery service there is no matching D-Bus GATT service, then check if the D-Bus org.bluez.Battery1 interface exists.


For the curious, you can disable the BlueZ battery plugin by running sudo systemctl edit bluetooth.service and adding the following:

[Service]
ExecStart=
ExecStart=-/usr/lib/bluetooth/bluetoothd -P battery
@dlech dlech added bug Something isn't working Backend: BlueZ Issues and PRs relating to the BlueZ backend labels Dec 13, 2020
@dlech
Copy link
Collaborator Author

dlech commented Jan 9, 2021

Based on #411 (comment), I'm starting to think that maybe we should just remove the battery service workaround instead and recommend that users disable the BlueZ battery plugin if they want to use devices with the battery service with Bleak. It means the OS will loose access to the battery service, but it will prevent BlueZ from interfering with Bleak.

@hbldh
Copy link
Owner

hbldh commented Jan 11, 2021

I agree. The current Battery API does make the BlueZ backend behave different from the other OS backends. If it can be disabled, then it will produce a more coherent solution for all backends.

But it does require users to make non-trivial changes in the OS. Even though it is a simple fix, it does add a strange step to the installation procedure for Linux.

@karlp
Copy link
Contributor

karlp commented Apr 27, 2022

hrm, I'm kinda of the opinion that the whole reason to use a Dbus based library is so that the existing OS stuff still works. If wanted to shut the OS out of my code, I'd use something speaking HCI directly, and just take over the device?

However, as #389 got merged, is it possible this issue is just outdated and should be closed now?

@dlech
Copy link
Collaborator Author

dlech commented Apr 27, 2022

I assume you are advocating for leaving the Battery Service workaround in place?

In that case, we still should fix Bleak so that it depends on the D-Bus object being present or not rather than on the BlueZ version as it currently does.

@karlp
Copy link
Contributor

karlp commented Apr 28, 2022

I don't actually have a strong opinion on the battery service itself but yes, I would say that bleak should make the battery service look "normal" cross platform, regardless of bluez version or the presence/lack thereof of the battery plugin. My reasoning being that that leaves the system interfaces "as is" which to me is the major appeal of a dbus based BLE toolkit rather than a HCI based toolkit.

@dlech
Copy link
Collaborator Author

dlech commented Sep 3, 2022

Up until now, we missed bluez/bluez@713f6f0 which exposes the battery service in BlueZ again starting in BlueZ v5.55. The version check for this is fixed in #976. So eventually, we can just remove the battery workaround completely when we drop support for BlueZ < 5.55.

@natrajmuthusamy
Copy link

Thanks dlech...

@dlech
Copy link
Collaborator Author

dlech commented Mar 17, 2023

Closing since the workaround is no longer needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backend: BlueZ Issues and PRs relating to the BlueZ backend bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants