Skip to content

Commit

Permalink
backends/winrt: fix max pdu update race
Browse files Browse the repository at this point in the history
Users have reported that the max pdu size was wrong on some devices.
This was happening because the max_pdu_size_changed event was happeing
after the get_services() call returned.

This adds an event to wait for the max_pdu_size_changed event to happen
and updates the characteristics in the service dictionary after the
fact to ensure they have the correct value.

The service_explorer example is also updated to show the value to help
with future troubleshooting.

Fixes: #1497
  • Loading branch information
dlech committed Apr 29, 2024
1 parent 29f5dcd commit 6795a2e
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 19 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,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.

`0.21.1`_ (2023-09-08)
======================
Expand Down
32 changes: 29 additions & 3 deletions bleak/backends/winrt/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,8 @@ 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 @@ -391,6 +393,7 @@ 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 @@ -487,6 +490,29 @@ 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 @@ -763,10 +789,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, self._session.max_pdu_size - 3
)
BleakGATTCharacteristicWinRT(characteristic, 20)
)

for descriptor in descriptors:
Expand Down
29 changes: 13 additions & 16 deletions examples/service_explorer.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,24 +51,21 @@ async def main(args: argparse.Namespace):
if "read" in char.properties:
try:
value = await client.read_gatt_char(char.uuid)
logger.info(
" [Characteristic] %s (%s), Value: %r",
char,
",".join(char.properties),
value,
)
extra = f", Value: {value}"
except Exception as e:
logger.error(
" [Characteristic] %s (%s), Error: %s",
char,
",".join(char.properties),
e,
)

extra = f", Error: {e}"
else:
logger.info(
" [Characteristic] %s (%s)", char, ",".join(char.properties)
)
extra = ""

if "write-without-response" in char.properties:
extra += f", Max write w/o rsp size: {char.max_write_without_response_size}"

logger.info(
" [Characteristic] %s (%s)%s",
char,
",".join(char.properties),
extra,
)

for descriptor in char.descriptors:
try:
Expand Down

0 comments on commit 6795a2e

Please sign in to comment.