Repository navigation
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2058 +/- ##
===========================================
- Coverage 65.74% 65.59% -0.15%
===========================================
Files 53 53
Lines 5047 5060 +13
Branches 611 615 +4
===========================================
+ Hits 3318 3319 +1
- Misses 1507 1516 +9
- Partials 222 225 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
|
Interestingly enough, this is what WebBluetooth has decided to do on Linux as well. whatwg/bluetooth#672 |
a635fd4 to
f9fb1ed
Compare
| """ | ||
| return False if self._bus is None else self._is_connected | ||
|
|
||
| async def _acquire_mtu(self) -> None: |
There was a problem hiding this comment.
I did some research and it looks like there are a decent number of folks calling this. So we should leave the _acquire_mtu() method but just have it raise DeprecationWarning.
And there is a clean workaround to avoid the warning while also supporting older versions of Bleak if they need to.
if hasattr(backend, "_mtu_size"):
await backend._acquire_mtu()There was a problem hiding this comment.
Kept _acquire_mtu(); it now only issues a DeprecationWarning. _mtu_size stays removed so the hasattr check works, and I added that snippet to the Linux backend docs.
There was a problem hiding this comment.
I think I would delete the entire file. The whole point was to show how to do the workaround.
Or update it to show how to support multiple versions of Bleak with the suggested:
if hasattr(backend, "_mtu_size"):
await backend._acquire_mtu()There was a problem hiding this comment.
Deleted the example; the hasattr snippet is in the Linux backend docs instead.
| @@ -107,7 +106,6 @@ def __init__( | |||
| self._notification_fds: dict[str, int] = {} | |||
|
|
|||
| # used to override mtu_size property | |||
There was a problem hiding this comment.
need to delete the comment too
mtu_size returned the default of 23 with a warning unless the private _acquire_mtu() was called first, which calls AcquireWrite or AcquireNotify on a characteristic only to read the MTU from the reply. BlueZ 5.62 added an "MTU" property to org.bluez.GattCharacteristic1, which max_write_without_response_size already reads. All characteristics of a connection share the same ATT MTU, so mtu_size now reads the "MTU" property of a characteristic in the service collection. Without the property (BlueZ < 5.62) or without services, mtu_size logs a warning and returns 23. The _mtu_size override is removed. _acquire_mtu() is kept for existing callers but does nothing except issue a DeprecationWarning. Code that supports older versions of Bleak can call it only when _mtu_size exists. The mtu_size.py example, which only showed the _acquire_mtu() workaround, is removed. The integration test no longer expects a warning on BlueZ.
f9fb1ed to
e2ec9c6
Compare
BlueZ emits no PropertiesChanged for a characteristic's "MTU" on a reconnect to a device whose GATT objects it kept from an earlier connection, and leaves the property out while the device is disconnected. A properties cache filled in the meantime is stale or lacks it for the whole connection, so mtu_size and max_write_without_response_size read the default of 23. The manager now reads "MTU" with Properties.Get once services are resolved and keeps it per device until the device disconnects; mtu_size and every characteristic's max_write_without_response_size read that value, falling back to the cached property, then to 23.
|
Pushed 99e42fe, which reads "MTU" from BlueZ with Reading it from the cache, as the first commit did, turned out to be wrong on reconnects: BlueZ registers the MTU exchange callback that emits |
|
The patch link is broken. Are you saying that BlueZ is not sending the properties changed signal when the MTU property changes? Otherwise we should be able to get the property from the BlueZManager rather than calling Properties.Get. |
|
Sorry, fixed now: https://patch.msgid.link/20261114-gatt-client-mtu-reconnect-v1-1-a86edc1eb208@gmail.com Yes, on a reconnect BlueZ doesn't send it. So the manager's cache can't be relied on in that window: a process that starts while the device is disconnected gets the objects without "MTU" and never receives it, and one that was already running keeps the previous connection's value. On BlueZ 5.82 that's With the BlueZ patch the signal is sent and the manager's value would be correct. But until that is accepted this reads it once with |
|
|
||
| # used to override mtu_size property | ||
| # the ATT MTU of the connection, read from BlueZ once services are resolved | ||
| self._mtu_size: Optional[int] = None |
There was a problem hiding this comment.
Adding this back in now breaks our suggested workaround of testing if this attribute exists.
| await self._get_services( | ||
| dangerous_use_bleak_cache=dangerous_use_bleak_cache | ||
| ) | ||
| self._mtu_size = await manager.refresh_mtu(self._device_path) |
There was a problem hiding this comment.
Since MTU can change after this, there isn't much point in saving the value here.
I would also add a comment explaining the BlueZ bug this is working around.
| if self._mtu_size is not None: | ||
| return self._mtu_size |
There was a problem hiding this comment.
Since the MTU can change at any time, we need to be reading the current MTU here. For example, reading the one from the first characteristic.
|
|
||
| Fixed | ||
| ----- | ||
| * Fixed ``BleakClient.mtu_size`` and ``BleakGATTCharacteristic.max_write_without_response_size`` reading the default MTU in BlueZ backend when reconnecting to a device that BlueZ kept from an earlier connection. The "MTU" property is now read from BlueZ after connecting rather than from the properties cache. |
There was a problem hiding this comment.
This is way too long for a changelog line.
On the BlueZ backend,
BleakClient.mtu_sizereturns the default of 23 with a warning unless the private_acquire_mtu()is called first, which calls AcquireWrite/AcquireNotify on a characteristic only to read the MTU from the reply.BlueZ has no MTU property on the device, but 5.62 added an "MTU" property to
org.bluez.GattCharacteristic1, whichmax_write_without_response_sizealready reads. It is the ATT MTU of the connection, somtu_sizenow reads it. It is read from BlueZ withProperties.Getonce services are resolved, rather than from the properties cache: BlueZ emits noPropertiesChangedfor "MTU" on a reconnect to a device whose GATT objects it kept from an earlier connection, and omits the property while the device is disconnected, so a process that connects within that window would otherwise read the default for the whole connection (inmtu_sizeand inmax_write_without_response_size, which this also fixes). The manager keeps the value per device until it disconnects. With BlueZ < 5.62, or without services,mtu_sizelogs a warning naming the minimum BlueZ version and returns 23.The
_mtu_sizeoverride is removed._acquire_mtu()is kept but deprecated: it does nothing except issue aDeprecationWarning. Code that supports older versions of Bleak can call it onlyif hasattr(client._backend, "_mtu_size"), as the Linux backend docs now show. Themtu_size.pyexample is removed, and the changelog is updated.The BlueZ side is fixed in https://lore.kernel.org/all/20261114-gatt-client-mtu-reconnect-v1-1-a86edc1eb208@gmail.com/, which makes BlueZ emit "MTU" on reconnection; the live read here is still needed for BlueZ releases without it.
Testing: on BlueZ 5.82 (Raspberry Pi OS), a second process connecting within ~30 s of the first one's disconnect reads
max_write_without_response_size20 with bleak 3.1.0, though the link MTU is 517; with this branch (99e42fe) on the same stock BlueZ it readsmtu_size517 andmax_write_without_response_size514. Thetest_mtu_sizeintegration test no longer expects a warning on BlueZ.poe lint,poe typecheckand the unit tests pass.