From 0231e38adb77eb97840c844d6c34ccd559f6cce8 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Mon, 31 Oct 2022 15:56:24 -0500 Subject: [PATCH 1/6] winrt/client: fix crash when getting services Python 3.11 crashes on Windows when connecting to a device: Traceback (most recent call last): File "C:\Users\david\Documents\GitHub\Pybricks\bleak\examples\service_explorer.py", line 64, in asyncio.run(main(sys.argv[1] if len(sys.argv) == 2 else ADDRESS)) File "C:\Users\david\AppData\Local\Programs\Python\Python311\Lib\asyncio\runners.py", line 190, in run return runner.run(main) ^^^^^^^^^^^^^^^^ File "C:\Users\david\AppData\Local\Programs\Python\Python311\Lib\asyncio\runners.py", line 118, in run return self._loop.run_until_complete(task) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "C:\Users\david\AppData\Local\Programs\Python\Python311\Lib\asyncio\base_events.py", line 650, in run_until_complete return future.result() ^^^^^^^^^^^^^^^ File "C:\Users\david\Documents\GitHub\Pybricks\bleak\examples\service_explorer.py", line 29, in main async with BleakClient(address) as client: File "C:\Users\david\Documents\GitHub\Pybricks\bleak\bleak\__init__.py", line 433, in __aenter__ await self.connect() File "C:\Users\david\Documents\GitHub\Pybricks\bleak\bleak\__init__.py", line 471, in connect return await self._backend.connect(**kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ await self.get_services() File "C:\Users\david\Documents\GitHub\Pybricks\bleak\bleak\backends\winrt\client.py", line 574, in get_services File "C:\Users\david\AppData\Local\Programs\Python\Python311\Lib\asyncio\tasks.py", line 418, in wait return await _wait(fs, timeout, return_when, loop) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "C:\Users\david\AppData\Local\Programs\Python\Python311\Lib\asyncio\tasks.py", line 522, in _wait f.add_done_callback(_on_completion) ^^^^^^^^^^^^^^^^^^^ AttributeError: '_bleak_winrt_Windows_Foundation.IAsyncOperation' object has no attribute 'add_done_callback' asyncio.wait() requires an asyncio.Task or asyncio.Future, but we were passing a WinRT IAsyncResult object. So we have to wrap it in a coroutine and then in a task to avoid the error. --- CHANGELOG.rst | 4 ++++ bleak/backends/winrt/client.py | 8 ++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 529c2267..c99b76d8 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -10,6 +10,10 @@ and this project adheres to `Semantic Versioning BleakGATTServiceCollection: services_changed_event.wait() ) self._services_changed_events.append(services_changed_event) - get_services_task = self._requester.get_gatt_services_async(*args) + + async def get_services(): + return await self._requester.get_gatt_services_async(*args) + + get_services_task = asyncio.create_task(get_services()) try: await asyncio.wait( @@ -590,7 +594,7 @@ async def get_services(self, **kwargs) -> BleakGATTServiceCollection: args = [BluetoothCacheMode.UNCACHED] services: Sequence[GattDeviceService] = _ensure_success( - get_services_task.get_results(), + get_services_task.result(), "services", "Could not get GATT services", ) From 4a79290720b9eb2c89813272d87dfed75defbac2 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Mon, 31 Oct 2022 16:32:09 -0500 Subject: [PATCH 2/6] winrt/client: change get services retry cache mode This changes the cache mode when getting services after a ServicesChanged event in the WinRT backend. This matches the recommendation of the docs. https://learn.microsoft.com/en-us/uwp/api/windows.devices.bluetooth.bluetoothledevice.gattserviceschanged?view=winrt-20348 --- CHANGELOG.rst | 1 + bleak/backends/winrt/client.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index c99b76d8..d36919f2 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -13,6 +13,7 @@ and this project adheres to `Semantic Versioning Date: Mon, 31 Oct 2022 20:31:55 -0500 Subject: [PATCH 3/6] winrt/client: add FutureLike wrapper for IAsyncOperation The fix from #1101 causes the following when run with `python3.10 -X dev`. ERROR:asyncio:Exception in callback Future.set_result(, <_bleak_winrt...001C33C60B100>) handle: , <_bleak_winrt...001C33C60B100>)> Traceback (most recent call last): File "C:\Users\david\AppData\Local\Programs\Python\Python310\lib\asyncio\events.py", line 80, in _run self._context.run(self._callback, *self._args) asyncio.exceptions.InvalidStateError: invalid state ERROR:asyncio:Exception in callback Future.set_result(, <_bleak_winrt...001C33C60BFC0>) handle: , <_bleak_winrt...001C33C60BFC0>)> Traceback (most recent call last): File "C:\Users\david\AppData\Local\Programs\Python\Python310\lib\asyncio\events.py", line 80, in _run self._context.run(self._callback, *self._args) asyncio.exceptions.InvalidStateError: invalid state This adds a new FutureLike wrapper to make the IAsyncOperation object look like an asyncio.Future instead of wrapping it in a task. --- bleak/backends/winrt/client.py | 78 +++++++++++++++++++++++++++++++--- 1 file changed, 72 insertions(+), 6 deletions(-) diff --git a/bleak/backends/winrt/client.py b/bleak/backends/winrt/client.py index 07b495f9..f6072dd4 100644 --- a/bleak/backends/winrt/client.py +++ b/bleak/backends/winrt/client.py @@ -6,10 +6,12 @@ """ import asyncio +import functools import logging import sys import uuid import warnings +from ctypes import pythonapi from typing import Any, Dict, List, Optional, Sequence, Union, cast import async_timeout @@ -44,11 +46,15 @@ DevicePairingResultStatus, DeviceUnpairingResultStatus, ) -from bleak_winrt.windows.foundation import EventRegistrationToken +from bleak_winrt.windows.foundation import ( + AsyncStatus, + EventRegistrationToken, + IAsyncOperation, +) from bleak_winrt.windows.storage.streams import Buffer from ... import BleakScanner -from ...exc import PROTOCOL_ERROR_CODES, BleakError, BleakDeviceNotFoundError +from ...exc import PROTOCOL_ERROR_CODES, BleakDeviceNotFoundError, BleakError from ..characteristic import BleakGATTCharacteristic from ..client import BaseBleakClient, NotifyCallback from ..device import BLEDevice @@ -569,10 +575,9 @@ async def get_services(self, **kwargs) -> BleakGATTServiceCollection: ) self._services_changed_events.append(services_changed_event) - async def get_services(): - return await self._requester.get_gatt_services_async(*args) - - get_services_task = asyncio.create_task(get_services()) + get_services_task = FutureLike( + self._requester.get_gatt_services_async(*args) + ) try: await asyncio.wait( @@ -889,3 +894,64 @@ async def stop_notify( event_handler_token = self._notification_callbacks.pop(characteristic.handle) characteristic.obj.remove_value_changed(event_handler_token) + + +class FutureLike: + """ + Wraps a WinRT IAsyncOperation in a "future-like" object so that it can + be passed to Python APIs. + + Needed until https://github.com/pywinrt/pywinrt/issues/14 + """ + + _asyncio_future_blocking = True + + def __init__(self, async_result: IAsyncOperation) -> None: + self._async_result = async_result + self._callbacks = [] + self._loop = asyncio.get_running_loop() + + def call_callbacks(op: IAsyncOperation, status: AsyncStatus): + for c in self._callbacks: + c(self) + + async_result.completed = functools.partial( + self._loop.call_soon_threadsafe, call_callbacks + ) + + def result(self) -> Any: + return self._async_result.get_results() + + def done(self) -> bool: + return self._async_result.status != AsyncStatus.STARTED + + def cancelled(self) -> bool: + return self._async_result.status == AsyncStatus.CANCELED + + def add_done_callback(self, callback, *, context=None) -> None: + self._callbacks.append(callback) + + def remove_done_callback(self, callback) -> None: + self._callbacks.remove(callback) + + def cancel(self, msg=None) -> bool: + if self._async_result.status != AsyncStatus.STARTED: + return False + self._async_result.cancel() + return True + + def exception(self) -> Optional[Exception]: + if self._async_result.status == AsyncStatus.STARTED: + raise asyncio.InvalidStateError + if self._async_result.status == AsyncStatus.COMPLETED: + return None + if self._async_result.status == AsyncStatus.CANCELED: + raise asyncio.CancelledError + if self._async_result.status == AsyncStatus.ERROR: + try: + pythonapi.PyErr_SetFromWindowsErr(self._async_result.error_code) + except OSError as e: + return e + + def get_loop(self) -> asyncio.AbstractEventLoop: + return self._loop From 431cfc9650968a65efcdc990eaa0c26c0b4e0144 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Tue, 1 Nov 2022 13:26:37 -0500 Subject: [PATCH 4/6] winrt/client: fix logger.warn deprecation warning When run with python3.10 -dev X, we get a warning that warn is deprecated and we should replace it with warning. --- bleak/backends/winrt/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bleak/backends/winrt/client.py b/bleak/backends/winrt/client.py index f6072dd4..64659804 100644 --- a/bleak/backends/winrt/client.py +++ b/bleak/backends/winrt/client.py @@ -256,7 +256,7 @@ async def connect(self, **kwargs) -> bool: def handle_services_changed(): if not self._services_changed_events: - logger.warn("%s: unhandled services changed event", self.address) + logger.warning("%s: unhandled services changed event", self.address) else: for event in self._services_changed_events: event.set() From 9c9dac6ad85d83213fb8b7536d1faf9106e11080 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Thu, 3 Nov 2022 10:40:00 -0500 Subject: [PATCH 5/6] bluezdbus/manager: ignore KeyError when properties removed Apparently, BlueZ will sometimes request to remove properties that were not set in the first place. Fixes: #1107 --- CHANGELOG.rst | 1 + bleak/backends/bluezdbus/manager.py | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index d36919f2..8d8b3202 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,7 @@ Fixed ------ * Fixed crash when getting services in WinRT backend. * Fixed cache mode when retrying get services in WinRT backend. +* Fixed ``KeyError`` crash in BlueZ backend when removing non-existent property. Fixes #1107. `0.19.1`_ (2022-10-29) ====================== diff --git a/bleak/backends/bluezdbus/manager.py b/bleak/backends/bluezdbus/manager.py index 0ccf6f39..8ee6e5d0 100644 --- a/bleak/backends/bluezdbus/manager.py +++ b/bleak/backends/bluezdbus/manager.py @@ -818,7 +818,12 @@ def _parse_msg(self, message: Message): self_interface.update(unpack_variants(changed)) for name in invalidated: - del self_interface[name] + try: + del self_interface[name] + except KeyError: + # sometimes there BlueZ tries to remove properties + # that were never added + pass # then call any callbacks so they will be called with the # updated state From 4371cb483b9d21fb46506012ab1e78d78a03073e Mon Sep 17 00:00:00 2001 From: David Lechner Date: Sun, 6 Nov 2022 15:08:22 -0600 Subject: [PATCH 6/6] v0.19.2 --- CHANGELOG.rst | 10 +++++++--- pyproject.toml | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 8d8b3202..74d402ff 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -10,10 +10,13 @@ and this project adheres to `Semantic Versioning "] license = "MIT"