backends/characteristic: make max_write_without_response_size dynamic #1586
Conversation
|
Hi @JPHutchins, this is a different approach to trying to fix your issue. It is now up to the caller to opt in to the workaround by adding the following code after connection: async with asyncio.timeout(10):
while char.max_write_without_response_size == 20:
await asyncio.sleep(0.5)I feel like this is better because you could still do other things with the device at the same time while waiting for this. And you can make the timeout as long or short as you want. |
|
Great! I think that my main question is whether or not it's always OK to use MTU property on Windows and Mac? On Linux we are using the _acquire_mtu() method and it seems OK. Should I use the MTU property or the max write without response size for reliable MTU? |
|
|
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 wan't complexly fixing the issue.
dlech
force-pushed
the
max-write-without-response-size-rework
branch
from
b9e722d to
d89ae79
Compare
OK! Is it correct that the device-level MTU is valid as soon as the device is "connected", whereas we may have to await the arrival of the |
CHANGELOG.rst
Outdated
| @@ -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. | |||
There was a problem hiding this comment.
I am interpreting "dynamic" to mean that this property may be mutated asynchronously.
| self.__mtu - 3, | ||
| lambda: self.__mtu - 3, |
There was a problem hiding this comment.
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.
No, the information is coming from the same place |
|
Thanks for the review! |
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 wan't complexly fixing the issue.