Check the bluetoothctl version with asyncio.create_subprocess_exec

CHANGELOG.rst

@@ -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
 -----

bleak/__init__.py

@@ -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())
@@ -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

bleak/backends/bluezdbus/__init__.py

@@ -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."""

bleak/backends/bluezdbus/client.py

@@ -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
@@ -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__)
 
@@ -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:
@@ -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)
@@ -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(
@@ -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"))
@@ -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(

bleak/backends/bluezdbus/version.py

@@ -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

requirements_dev.txt

@@ -1,3 +1,4 @@
+asynctest>=0.13.0
 pip>=18.0
 bump2version==1.0.1
 wheel>=0.32.2
@@ -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

tests/bleak/backends/bluezdbus/test_version.py

@@ -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