Skip to content

Commit

Permalink
Check the bluetoothctl version with asyncio.create_subprocess_exec
Browse files Browse the repository at this point in the history
  • Loading branch information
bdraco committed Jul 29, 2022
1 parent 5aabf0a commit 1ac732a
Show file tree
Hide file tree
Showing 8 changed files with 185 additions and 47 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ Changed
* ``BleakScanner()`` arg ``scanning_mode`` is no longer Windows-only and is no longer keyword-only.
* All ``BleakScanner()`` instances in BlueZ backend now use common D-Bus object manager.
* Deprecated ``filters`` kwarg in ``BleakScanner`` in BlueZ backend.
* BlueZ version is now checked on first connection instead of import to avoid blocking the event loop.

Fixed
-----
Expand Down
5 changes: 0 additions & 5 deletions bleak/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,9 @@
import asyncio

from bleak.__version__ import __version__ # noqa: F401
from bleak.backends.bluezdbus import check_bluez_version
from bleak.exc import BleakError

_on_rtd = os.environ.get("READTHEDOCS") == "True"
_on_ci = "CI" in os.environ

_logger = logging.getLogger(__name__)
_logger.addHandler(logging.NullHandler())
Expand All @@ -38,9 +36,6 @@
BleakClientP4Android as BleakClient,
) # noqa: F401
elif platform.system() == "Linux":
if not _on_ci and not check_bluez_version(5, 43):
raise BleakError("Bleak requires BlueZ >= 5.43.")

from bleak.backends.bluezdbus.scanner import (
BleakScannerBlueZDBus as BleakScanner,
) # noqa: F401
Expand Down
29 changes: 1 addition & 28 deletions bleak/backends/bluezdbus/__init__.py
Original file line number Diff line number Diff line change
@@ -1,28 +1 @@
import re
import subprocess

from ...exc import BleakError


def check_bluez_version(major: int, minor: int) -> bool:
"""
Checks the BlueZ version.
Returns:
``True`` if the BlueZ major version is equal to *major* and the minor
version is greater than or equal to *minor*, otherwise ``False``.
"""
# lazy-get the version and store it so we only have to run subprocess once
if not hasattr(check_bluez_version, "version"):
p = subprocess.Popen(["bluetoothctl", "--version"], stdout=subprocess.PIPE)
out, _ = p.communicate()
s = re.search(b"(\\d+).(\\d+)", out.strip(b"'"))

if not s:
raise BleakError(f"Could not determine BlueZ version: {out.decode()}")

setattr(check_bluez_version, "version", tuple(map(int, s.groups())))

bluez_major, bluez_minor = getattr(check_bluez_version, "version")

return bluez_major == major and bluez_minor >= minor
"""BlueZ backend."""
28 changes: 14 additions & 14 deletions bleak/backends/bluezdbus/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from dbus_next.message import Message
from dbus_next.signature import Variant

from bleak.backends.bluezdbus import check_bluez_version, defs
from bleak.backends.bluezdbus import defs
from bleak.backends.bluezdbus.characteristic import BleakGATTCharacteristicBlueZDBus
from bleak.backends.bluezdbus.descriptor import BleakGATTDescriptorBlueZDBus
from bleak.backends.bluezdbus.scanner import BleakScannerBlueZDBus
Expand All @@ -30,7 +30,7 @@
from bleak.backends.device import BLEDevice
from bleak.backends.service import BleakGATTServiceCollection
from bleak.exc import BleakDBusError, BleakError

from .version import BlueZFeatures

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -78,12 +78,6 @@ def __init__(self, address_or_ble_device: Union[BLEDevice, str], **kwargs):
# used to override mtu_size property
self._mtu_size: Optional[int] = None

# BlueZ version features
self._can_write_without_response = check_bluez_version(5, 46)
self._write_without_response_workaround_needed = not check_bluez_version(5, 51)
self._hides_battery_characteristic = check_bluez_version(5, 48)
self._hides_device_name_characteristic = check_bluez_version(5, 48)

# Connectivity methods

async def connect(self, **kwargs) -> bool:
Expand All @@ -105,6 +99,10 @@ async def connect(self, **kwargs) -> bool:
if self.is_connected:
raise BleakError("Client is already connected")

if not BlueZFeatures.checked_bluez_version:
await BlueZFeatures.check_bluez_version()
if not BlueZFeatures.supported_version:
raise BleakError("Bleak requires BlueZ >= 5.43.")
# A Discover must have been run before connecting to any devices.
# Find the desired device before trying to connect.
timeout = kwargs.get("timeout", self._timeout)
Expand Down Expand Up @@ -625,8 +623,9 @@ async def read_gatt_char(
if not characteristic:
# 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._hides_battery_characteristic
if (
str(char_specifier) == "00002a19-0000-1000-8000-00805f9b34fb"
and BlueZFeatures.hides_battery_characteristic
):
reply = await self._bus.call(
Message(
Expand All @@ -647,8 +646,9 @@ async def read_gatt_char(
)
)
return value
if str(char_specifier) == "00002a00-0000-1000-8000-00805f9b34fb" and (
self._hides_device_name_characteristic
if (
str(char_specifier) == "00002a00-0000-1000-8000-00805f9b34fb"
and BlueZFeatures.hides_device_name_characteristic
):
# Simulate regular characteristics read to be consistent over all platforms.
value = bytearray(self._properties["Name"].encode("ascii"))
Expand Down Expand Up @@ -780,9 +780,9 @@ async def write_gatt_char(
)

# See docstring for details about this handling.
if not response and not self._can_write_without_response:
if not response and not BlueZFeatures.can_write_without_response:
raise BleakError("Write without response requires at least BlueZ 5.46")
if response or not self._write_without_response_workaround_needed:
if response or not BlueZFeatures.write_without_response_workaround_needed:
# TODO: Add OnValueUpdated handler for response=True?
reply = await self._bus.call(
Message(
Expand Down
62 changes: 62 additions & 0 deletions bleak/backends/bluezdbus/version.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import asyncio
import contextlib
import logging
import re
from typing import Optional

logger = logging.getLogger(__name__)


async def _get_bluetoothctl_version():
"""Get the version of bluetoothctl."""
with contextlib.suppress(Exception):
proc = await asyncio.create_subprocess_exec(
"bluetoothctl", "--version", stdout=asyncio.subprocess.PIPE
)
out = await proc.stdout.read()
version = re.search(b"(\\d+).(\\d+)", out.strip(b"'"))
await proc.wait()
return version
return None


class BlueZFeatures:
"""Check which features are supported by the BlueZ backend."""

checked_bluez_version = False
supported_version = True
can_write_without_response = True
write_without_response_workaround_needed = False
hides_battery_characteristic = True
hides_device_name_characteristic = True
_check_bluez_event: Optional[asyncio.Event] = None

@classmethod
async def check_bluez_version(cls) -> None:
"""Check the bluez version."""
if cls._check_bluez_event:
# If there is already a check in progress
# it wins, wait for it instead
await cls._check_bluez_event.wait()
return
cls._check_bluez_event = asyncio.Event()
version_output = await _get_bluetoothctl_version()
if version_output:
major, minor = tuple(map(int, version_output.groups()))
cls.supported_version = major == 5 and minor >= 34
cls.can_write_without_response = major == 5 and minor >= 46
cls.write_without_response_workaround_needed = not (
major == 5 and minor >= 51
)
cls.hides_battery_characteristic = major == 5 and minor >= 48
cls.hides_device_name_characteristic = major == 5 and minor >= 48
else:
# Its possible they may be running inside a container where
# bluetoothctl is not available and they only have access to the
# BlueZ D-Bus API.
logging.warning(
"Could not determine BlueZ version, bluetoothctl not available, assuming 5.51+"
)

cls._check_bluez_event.set()
cls.checked_bluez_version = True
2 changes: 2 additions & 0 deletions requirements_dev.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
asynctest>=0.13.0
pip>=18.0
bump2version==1.0.1
wheel>=0.32.2
Expand All @@ -11,5 +12,6 @@ Sphinx>=1.7.7
sphinx-rtd-theme>=1.0.0
PyYAML>=3.13
pytest>=3.9.2
pytest-asyncio>=0.19.0
pytest-cov>=2.5.1
typing-extensions>=4.2.0
Empty file.
105 changes: 105 additions & 0 deletions tests/bleak/backends/bluezdbus/test_version.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
#!/usr/bin/env python

"""Tests for `bleak.backends.bluezdbus.version` package."""

import sys
from unittest.mock import Mock, patch

import pytest

if sys.version_info[:2] < (3, 8):
from asynctest.mock import CoroutineMock as AsyncMock
else:
from unittest.mock import AsyncMock

from bleak.backends.bluezdbus.version import BlueZFeatures


@pytest.mark.asyncio
@pytest.mark.parametrize(
"version,can_write_without_response,write_without_response_workaround_needed,hides_battery_characteristic,hides_device_name_characteristic",
[
(b"bluetoothctl: 5.34", False, False, False, False),
(b"bluetoothctl: 5.46", True, False, False, False),
(b"bluetoothctl: 5.48", True, False, True, True),
(b"bluetoothctl: 5.51", True, True, True, True),
(b"bluetoothctl: 5.63", True, True, True, True),
(b"", True, True, True, True),
],
)
async def test_bluez_version(
version,
can_write_without_response,
write_without_response_workaround_needed,
hides_battery_characteristic,
hides_device_name_characteristic,
):
"""Test we can determine supported feature from bluetoothctl."""
mock_proc = Mock(
wait=AsyncMock(), stdout=Mock(read=AsyncMock(return_value=version))
)
with patch(
"bleak.backends.bluezdbus.version.asyncio.create_subprocess_exec",
AsyncMock(return_value=mock_proc),
):
BlueZFeatures._check_bluez_event = None
await BlueZFeatures.check_bluez_version()
assert BlueZFeatures.checked_bluez_version is True
assert BlueZFeatures.can_write_without_response == can_write_without_response
assert (
not BlueZFeatures.write_without_response_workaround_needed
== write_without_response_workaround_needed
)
assert BlueZFeatures.hides_battery_characteristic == hides_battery_characteristic
assert (
BlueZFeatures.hides_device_name_characteristic
== hides_device_name_characteristic
)


@pytest.mark.asyncio
async def test_bluez_version_only_happens_once():
"""Test we can determine supported feature from bluetoothctl."""
BlueZFeatures.checked_bluez_version = False
BlueZFeatures._check_bluez_event = None
mock_proc = Mock(
wait=AsyncMock(),
stdout=Mock(read=AsyncMock(return_value=b"bluetoothctl: 5.46")),
)
with patch(
"bleak.backends.bluezdbus.version.asyncio.create_subprocess_exec",
AsyncMock(return_value=mock_proc),
):
await BlueZFeatures.check_bluez_version()

assert BlueZFeatures.checked_bluez_version is True

with patch(
"bleak.backends.bluezdbus.version.asyncio.create_subprocess_exec",
side_effect=Exception,
):
await BlueZFeatures.check_bluez_version()

assert BlueZFeatures.checked_bluez_version is True


@pytest.mark.asyncio
async def test_exception_checking_bluez_features_does_not_block_forever():
"""Test an exception while checking BlueZ features does not stall a second check."""
BlueZFeatures.checked_bluez_version = False
BlueZFeatures._check_bluez_event = None
with patch(
"bleak.backends.bluezdbus.version.asyncio.create_subprocess_exec",
side_effect=OSError,
):
await BlueZFeatures.check_bluez_version()

assert BlueZFeatures.checked_bluez_version is True

with patch(
"bleak.backends.bluezdbus.version.asyncio.create_subprocess_exec",
side_effect=OSError,
):
await BlueZFeatures.check_bluez_version()

assert BlueZFeatures.checked_bluez_version is True

0 comments on commit 1ac732a

Please sign in to comment.