Skip to content

Commit

Permalink
backends/characteristic: make max_write_without_response_size dynamic (
Browse files Browse the repository at this point in the history
…#1586)

It has been observed that the max MTU exchange may not be complete
before the connection is established, at least on Windows.

This reverts the previous attempt to work around this on Windows and
instead makes the max_write_without_response_size dynamic. This way
users can implement a workaround if needed but users who don't need it
won't be punished with a longer connection time. The timeout in the
previous workaround was also too short for some devices so it wasn't
complexly fixing the issue.
  • Loading branch information
dlech authored Jun 1, 2024
1 parent bd8f022 commit 425abb3
Show file tree
Hide file tree
Showing 10 changed files with 43 additions and 44 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ Changed
-------
* Retrieve the BLE address required by ``BleakClientWinRT`` from scan response if advertising is None (WinRT).
* Changed type hint for ``adv`` attribute of ``bleak.backends.winrt.scanner._RawAdvData``.
* ``BleakGATTCharacteristic.max_write_without_response_size`` is now dynamic.

Fixed
-----
Expand Down Expand Up @@ -56,7 +57,7 @@ Fixed
* Fixed scanning silently failing on Windows when Bluetooth is off. Fixes #1535.
* Fixed using wrong value for ``tx_power`` in Android backend. Fixes #1532.
* Fixed 4-character UUIDs not working on ``BleakClient.*_gatt_char`` methods. Fixes #1498.
* Fixed race condition with getting max PDU size on Windows. Fixes #1497.
* Fixed race condition with getting max PDU size on Windows. Fixes #1497. [REVERTED in unreleased]
* Fixed filtering advertisement data by service UUID when multiple apps are scanning. Fixes #1534.

`0.21.1`_ (2023-09-08)
Expand Down
4 changes: 2 additions & 2 deletions bleak/backends/bluezdbus/characteristic.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import List, Union
from typing import Callable, List, Union
from uuid import UUID

from ..characteristic import BleakGATTCharacteristic
Expand Down Expand Up @@ -36,7 +36,7 @@ def __init__(
object_path: str,
service_uuid: str,
service_handle: int,
max_write_without_response_size: int,
max_write_without_response_size: Callable[[], int],
):
super(BleakGATTCharacteristicBlueZDBus, self).__init__(
obj, max_write_without_response_size
Expand Down
2 changes: 1 addition & 1 deletion bleak/backends/bluezdbus/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -696,7 +696,7 @@ async def get_services(
service.handle,
# "MTU" property was added in BlueZ 5.62, otherwise fall
# back to minimum MTU according to Bluetooth spec.
char_props.get("MTU", 23) - 3,
lambda: char_props.get("MTU", 23) - 3,
)

services.add_characteristic(char)
Expand Down
24 changes: 21 additions & 3 deletions bleak/backends/characteristic.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"""
import abc
import enum
from typing import Any, List, Union
from typing import Any, Callable, List, Union
from uuid import UUID

from ..uuids import uuidstr_to_str
Expand All @@ -30,7 +30,7 @@ class GattCharacteristicsFlags(enum.Enum):
class BleakGATTCharacteristic(abc.ABC):
"""Interface for the Bleak representation of a GATT Characteristic"""

def __init__(self, obj: Any, max_write_without_response_size: int):
def __init__(self, obj: Any, max_write_without_response_size: Callable[[], int]):
"""
Args:
obj:
Expand Down Expand Up @@ -86,12 +86,30 @@ def max_write_without_response_size(self) -> int:
Gets the maximum size in bytes that can be used for the *data* argument
of :meth:`BleakClient.write_gatt_char()` when ``response=False``.
In rare cases, a device may take a long time to update this value, so
reading this property may return the default value of ``20`` and reading
it again after a some time may return the expected higher value.
If you *really* need to wait for a higher value, you can do something
like this:
.. code-block:: python
async with asyncio.timeout(10):
while char.max_write_without_response_size == 20:
await asyncio.sleep(0.5)
.. warning:: Linux quirk: For BlueZ versions < 5.62, this property
will always return ``20``.
.. versionadded:: 0.16
"""
return self._max_write_without_response_size

# for backwards compatibility
if isinstance(self._max_write_without_response_size, int):
return self._max_write_without_response_size

return self._max_write_without_response_size()

@property
@abc.abstractmethod
Expand Down
6 changes: 4 additions & 2 deletions bleak/backends/corebluetooth/characteristic.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"""

from enum import Enum
from typing import Dict, List, Optional, Tuple, Union
from typing import Callable, Dict, List, Optional, Tuple, Union

from CoreBluetooth import CBCharacteristic

Expand Down Expand Up @@ -59,7 +59,9 @@ class CBCharacteristicProperties(Enum):
class BleakGATTCharacteristicCoreBluetooth(BleakGATTCharacteristic):
"""GATT Characteristic implementation for the CoreBluetooth backend"""

def __init__(self, obj: CBCharacteristic, max_write_without_response_size: int):
def __init__(
self, obj: CBCharacteristic, max_write_without_response_size: Callable[[], int]
):
super().__init__(obj, max_write_without_response_size)
self.__descriptors: List[BleakGATTDescriptorCoreBluetooth] = []
# self.__props = obj.properties()
Expand Down
2 changes: 1 addition & 1 deletion bleak/backends/corebluetooth/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ async def get_services(self, **kwargs) -> BleakGATTServiceCollection:
services.add_characteristic(
BleakGATTCharacteristicCoreBluetooth(
characteristic,
self._peripheral.maximumWriteValueLengthForType_(
lambda: self._peripheral.maximumWriteValueLengthForType_(
CBCharacteristicWriteWithoutResponse
),
)
Expand Down
4 changes: 2 additions & 2 deletions bleak/backends/p4android/characteristic.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import List, Union
from typing import Callable, List, Union
from uuid import UUID

from ...exc import BleakError
Expand All @@ -15,7 +15,7 @@ def __init__(
java,
service_uuid: str,
service_handle: int,
max_write_without_response_size: int,
max_write_without_response_size: Callable[[], int],
):
super(BleakGATTCharacteristicP4Android, self).__init__(
java, max_write_without_response_size
Expand Down
2 changes: 1 addition & 1 deletion bleak/backends/p4android/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ async def get_services(self) -> BleakGATTServiceCollection:
java_characteristic,
service.uuid,
service.handle,
self.__mtu - 3,
lambda: self.__mtu - 3,
)
services.add_characteristic(characteristic)

Expand Down
8 changes: 6 additions & 2 deletions bleak/backends/winrt/characteristic.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# -*- coding: utf-8 -*-
import sys
from typing import List, Union
from typing import Callable, List, Union
from uuid import UUID

if sys.version_info >= (3, 12):
Expand Down Expand Up @@ -68,7 +68,11 @@
class BleakGATTCharacteristicWinRT(BleakGATTCharacteristic):
"""GATT Characteristic implementation for the .NET backend, implemented with WinRT"""

def __init__(self, obj: GattCharacteristic, max_write_without_response_size: int):
def __init__(
self,
obj: GattCharacteristic,
max_write_without_response_size: Callable[[], int],
):
super().__init__(obj, max_write_without_response_size)
self.__descriptors = []
self.__props = [
Expand Down
32 changes: 3 additions & 29 deletions bleak/backends/winrt/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -383,8 +383,6 @@ def session_status_changed_event_handler(
)
loop.call_soon_threadsafe(handle_session_status_changed, args)

pdu_size_event = asyncio.Event()

def max_pdu_size_changed_handler(sender: GattSession, args):
try:
max_pdu_size = sender.max_pdu_size
Expand All @@ -395,7 +393,6 @@ def max_pdu_size_changed_handler(sender: GattSession, args):
return

logger.debug("max_pdu_size_changed_handler: %d", max_pdu_size)
pdu_size_event.set()

# Start a GATT Session to connect
event = asyncio.Event()
Expand Down Expand Up @@ -492,29 +489,6 @@ def max_pdu_size_changed_handler(sender: GattSession, args):
cache_mode=cache_mode,
)

# There is a race condition where the max_pdu_size_changed event
# might not be received before the get_services() call completes.
# We could put this wait before getting services, but that would
# make the connection time longer. So we put it here instead and
# fix up the characteristics if necessary.
if not pdu_size_event.is_set():
try:
# REVISIT: Devices that don't support > default PDU size
# may be punished by this timeout with a slow connection
# time. We may want to consider an option to ignore this
# timeout for such devices.
async with async_timeout(1):
await pdu_size_event.wait()
except asyncio.TimeoutError:
logger.debug(
"max_pdu_size_changed event not received, using default"
)

for char in self.services.characteristics.values():
char._max_write_without_response_size = (
self._session.max_pdu_size - 3
)

# a connection may not be made until we request info from the
# device, so we have to get services before the GATT session
# is set to active
Expand Down Expand Up @@ -791,10 +765,10 @@ def dispose_on_cancel(future):
f"Could not get GATT descriptors for characteristic {characteristic.uuid} ({characteristic.attribute_handle})",
)

# NB: max_pdu_size might not be valid at this time so we
# start with default size and will update later
new_services.add_characteristic(
BleakGATTCharacteristicWinRT(characteristic, 20)
BleakGATTCharacteristicWinRT(
characteristic, lambda: self._session.max_pdu_size - 3
)
)

for descriptor in descriptors:
Expand Down

0 comments on commit 425abb3

Please sign in to comment.