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

backends/characteristic: make max_write_without_response_size dynamic #1586

Merged
merged 2 commits into from
Jun 1, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 is now dynamic.
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: "is is" -> "is"

Copy link
Contributor

Choose a reason for hiding this comment

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

I am interpreting "dynamic" to mean that this property may be mutated asynchronously.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That is the idea.

dlech marked this conversation as resolved.
Show resolved Hide resolved

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,
Copy link
Contributor

Choose a reason for hiding this comment

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

Off topic, but it reminds me that it would be nice get rid of name-mangling, IMO. I guess it's only the android client though.

)
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
Loading