Skip to content

Commit

Permalink
BleakClient.write_gatt_char: change handling of default response
Browse files Browse the repository at this point in the history
Previously, some backends would select the "best" response type based
on the reported characteristics while some did not. For the ones that
didn't, it was a commonly reported issue for the write to not work.

This adds a recommendation to the docs to always be explicit about
the requested response type and moves the common logic for picking
the "best" type in the default case to the BleakClient class to avoid
duplicated code.

The logic is simplified to prefer a write with response if available
and no longer reports a warning for devices that don't properly specify
properties (since it would create much noise with many writes).

Fixes: hbldh#909
  • Loading branch information
dlech committed Jul 20, 2023
1 parent 718d272 commit 401e71a
Show file tree
Hide file tree
Showing 11 changed files with 78 additions and 155 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ Changed
* Improved error messages when failing to get services in WinRT backend.
* Improved error messages with enum values in WinRT backend. Fixes #1284.
* Scanner backends modified to allow multiple advertisement callbacks. Merged #1367.
* Changed default handling of the ``response`` argument in ``BleakClient.write_gatt_char``.
Fixes #909.

Fixed
-----
Expand Down
55 changes: 49 additions & 6 deletions bleak/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -650,23 +650,66 @@ async def write_gatt_char(
self,
char_specifier: Union[BleakGATTCharacteristic, int, str, uuid.UUID],
data: Union[bytes, bytearray, memoryview],
response: bool = False,
response: bool = None,
) -> None:
"""
Perform a write operation on the specified GATT characteristic.
There are two possible kinds of writes. *Write with response* (sometimes
called a *Request*) will write the data then wait for a response from
the remote device. *Write without response* (sometimes called *Command*)
will queue data to be written and return immediately.
Each characteristic may support one kind or the other or both or neither.
Consult the device's documentation or inspect the properties of the
characteristic to find out which kind of writes are supported.
.. tip:: Explicit is better than implicit. Best practice is to always
include an explicit ``response=True`` or ``response=False``
when calling this method.
Args:
char_specifier:
The characteristic to write to, specified by either integer
handle, UUID or directly by the BleakGATTCharacteristic object
representing it.
handle, UUID or directly by the :class:`~bleak.backends.characteristic.BleakGATTCharacteristic`
object representing it. If a device has more than one characteristic
with the same UUID, then attempting to use the UUID wil fail and
a characteristic object must be used instead.
data:
The data to send.
The data to send. When a write-with-response operation is used,
the length of the data is limited to 512 bytes. When a
write-without-response operation is used, the length of the
data is limited to :attr:`~bleak.backends.characteristic.BleakGATTCharacteristic.max_write_without_response_size`.
Any type that supports the buffer protocol can be passed.
response:
If write-with-response operation should be done. Defaults to ``False``.
If ``True``, a write-with-response operation will be used. If
``False``, a write-without-response operation will be used.
If omitted or ``None``, the "best" operation will be used
based on the reported properties of the characteristic.
.. versionchanged:: 0.21
The default behavior when ``response=`` is omitted was changed.
Example::
MY_CHAR_UUID = "1234"
...
await client.write_gatt_char(MY_CHAR_UUID, b"\x00\x01\x02\x03", response=True)
"""
await self._backend.write_gatt_char(char_specifier, data, response)
if isinstance(char_specifier, BleakGATTCharacteristic):
characteristic = char_specifier
else:
characteristic = self.services.get_characteristic(char_specifier)

if not characteristic:
raise BleakError("Characteristic {char_specifier} was not found!")

if response is None:
# if not specified, prefer write-with-response over write-without-
# response if it is available since it is the more reliable write.
response = "write" in characteristic.properties

await self._backend.write_gatt_char(characteristic, data, response)

async def start_notify(
self,
Expand Down
55 changes: 3 additions & 52 deletions bleak/backends/bluezdbus/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -811,66 +811,17 @@ async def read_gatt_descriptor(self, handle: int, **kwargs) -> bytearray:

async def write_gatt_char(
self,
char_specifier: Union[BleakGATTCharacteristicBlueZDBus, int, str, UUID],
characteristic: BleakGATTCharacteristic,
data: Union[bytes, bytearray, memoryview],
response: bool = False,
response: bool,
) -> None:
"""Perform a write operation on the specified GATT characteristic.
.. note::
The version check below is for the "type" option to the
"Characteristic.WriteValue" method that was added to `Bluez in 5.51
<https://git.kernel.org/pub/scm/bluetooth/bluez.git/commit?id=fa9473bcc48417d69cc9ef81d41a72b18e34a55a>`_
Before that commit, ``Characteristic.WriteValue`` was only "Write with
response". ``Characteristic.AcquireWrite`` was `added in Bluez 5.46
<https://git.kernel.org/pub/scm/bluetooth/bluez.git/commit/doc/gatt-api.txt?id=f59f3dedb2c79a75e51a3a0d27e2ae06fefc603e>`_
which can be used to "Write without response", but for older versions
of Bluez, it is not possible to "Write without response".
Args:
char_specifier (BleakGATTCharacteristicBlueZDBus, int, str or UUID): The characteristic to write
to, specified by either integer handle, UUID or directly by the
BleakGATTCharacteristicBlueZDBus object representing it.
data (bytes or bytearray): The data to send.
response (bool): If write-with-response operation should be done. Defaults to `False`.
"""
if not self.is_connected:
raise BleakError("Not connected")

if not isinstance(char_specifier, BleakGATTCharacteristicBlueZDBus):
characteristic = self.services.get_characteristic(char_specifier)
else:
characteristic = char_specifier

if not characteristic:
raise BleakError("Characteristic {0} was not found!".format(char_specifier))
if (
"write" not in characteristic.properties
and "write-without-response" not in characteristic.properties
):
raise BleakError(
"Characteristic %s does not support write operations!"
% str(characteristic.uuid)
)
if not response and "write-without-response" not in characteristic.properties:
response = True
# Force response here, since the device only supports that.
if (
response
and "write" not in characteristic.properties
and "write-without-response" in characteristic.properties
):
response = False
logger.warning(
"Characteristic %s does not support Write with response. Trying without..."
% str(characteristic.uuid)
)

# See docstring for details about this handling.
if not response and not BlueZFeatures.can_write_without_response:
raise BleakError("Write without response requires at least BlueZ 5.46")

if response or not BlueZFeatures.write_without_response_workaround_needed:
while True:
assert self._bus
Expand Down
16 changes: 7 additions & 9 deletions bleak/backends/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,19 +183,17 @@ async def read_gatt_descriptor(self, handle: int, **kwargs) -> bytearray:
@abc.abstractmethod
async def write_gatt_char(
self,
char_specifier: Union[BleakGATTCharacteristic, int, str, uuid.UUID],
characteristic: BleakGATTCharacteristic,
data: Union[bytes, bytearray, memoryview],
response: bool = False,
response: bool,
) -> None:
"""Perform a write operation on the specified GATT characteristic.
"""
Perform a write operation on the specified GATT characteristic.
Args:
char_specifier (BleakGATTCharacteristic, int, str or UUID): The characteristic to write
to, specified by either integer handle, UUID or directly by the
BleakGATTCharacteristic object representing it.
data (bytes or bytearray): The data to send.
response (bool): If write-with-response operation should be done. Defaults to `False`.
characteristic: The characteristic to write to.
data: The data to send.
response: If write-with-response operation should be done.
"""
raise NotImplementedError()

Expand Down
21 changes: 2 additions & 19 deletions bleak/backends/corebluetooth/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -307,27 +307,10 @@ async def read_gatt_descriptor(

async def write_gatt_char(
self,
char_specifier: Union[BleakGATTCharacteristic, int, str, uuid.UUID],
characteristic: BleakGATTCharacteristic,
data: Union[bytes, bytearray, memoryview],
response: bool = False,
response: bool,
) -> None:
"""Perform a write operation of the specified GATT characteristic.
Args:
char_specifier (BleakGATTCharacteristic, int, str or UUID): The characteristic to write
to, specified by either integer handle, UUID or directly by the
BleakGATTCharacteristic object representing it.
data (bytes or bytearray): The data to send.
response (bool): If write-with-response operation should be done. Defaults to `False`.
"""
if not isinstance(char_specifier, BleakGATTCharacteristic):
characteristic = self.services.get_characteristic(char_specifier)
else:
characteristic = char_specifier
if not characteristic:
raise BleakError("Characteristic {} was not found!".format(char_specifier))

value = NSData.alloc().initWithBytes_length_(data, len(data))
await self._delegate.write_characteristic(
characteristic.obj,
Expand Down
43 changes: 2 additions & 41 deletions bleak/backends/p4android/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -370,49 +370,10 @@ async def read_gatt_descriptor(

async def write_gatt_char(
self,
char_specifier: Union[BleakGATTCharacteristicP4Android, int, str, uuid.UUID],
characteristic: BleakGATTCharacteristic,
data: bytearray,
response: bool = False,
response: bool,
) -> None:
"""Perform a write operation on the specified GATT characteristic.
Args:
char_specifier (BleakGATTCharacteristicP4Android, int, str or UUID): The characteristic to write
to, specified by either integer handle, UUID or directly by the
BleakGATTCharacteristicP4Android object representing it.
data (bytes or bytearray): The data to send.
response (bool): If write-with-response operation should be done. Defaults to `False`.
"""
if not isinstance(char_specifier, BleakGATTCharacteristicP4Android):
characteristic = self.services.get_characteristic(char_specifier)
else:
characteristic = char_specifier

if not characteristic:
raise BleakError(f"Characteristic {char_specifier} was not found!")

if (
"write" not in characteristic.properties
and "write-without-response" not in characteristic.properties
):
raise BleakError(
f"Characteristic {str(characteristic.uuid)} does not support write operations!"
)
if not response and "write-without-response" not in characteristic.properties:
response = True
# Force response here, since the device only supports that.
if (
response
and "write" not in characteristic.properties
and "write-without-response" in characteristic.properties
):
response = False
logger.warning(
"Characteristic %s does not support Write with response. Trying without..."
% str(characteristic.uuid)
)

if response:
characteristic.obj.setWriteType(
defs.BluetoothGattCharacteristic.WRITE_TYPE_DEFAULT
Expand Down
21 changes: 2 additions & 19 deletions bleak/backends/winrt/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -824,30 +824,13 @@ async def read_gatt_descriptor(self, handle: int, **kwargs) -> bytearray:

async def write_gatt_char(
self,
char_specifier: Union[BleakGATTCharacteristic, int, str, uuid.UUID],
characteristic: BleakGATTCharacteristic,
data: Union[bytes, bytearray, memoryview],
response: bool = False,
response: bool,
) -> None:
"""Perform a write operation of the specified GATT characteristic.
Args:
char_specifier (BleakGATTCharacteristic, int, str or UUID): The characteristic to write
to, specified by either integer handle, UUID or directly by the
BleakGATTCharacteristic object representing it.
data (bytes or bytearray): The data to send.
response (bool): If write-with-response operation should be done. Defaults to `False`.
"""
if not self.is_connected:
raise BleakError("Not connected")

if not isinstance(char_specifier, BleakGATTCharacteristic):
characteristic = self.services.get_characteristic(char_specifier)
else:
characteristic = char_specifier
if not characteristic:
raise BleakError(f"Characteristic {char_specifier} was not found!")

response = (
GattWriteOption.WRITE_WITH_RESPONSE
if response
Expand Down
2 changes: 1 addition & 1 deletion examples/mtu_size.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def callback(device: BLEDevice, adv: AdvertisementData) -> None:
for chunk in (
data[i : i + chunk_size] for i in range(0, len(data), chunk_size)
):
await client.write_gatt_char(CHAR_UUID, chunk)
await client.write_gatt_char(CHAR_UUID, chunk, response=False)


if __name__ == "__main__":
Expand Down
12 changes: 7 additions & 5 deletions examples/philips_hue.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,25 +53,25 @@ async def main(address):
print(f"Paired: {paired}")

print("Turning Light off...")
await client.write_gatt_char(LIGHT_CHARACTERISTIC, b"\x00")
await client.write_gatt_char(LIGHT_CHARACTERISTIC, b"\x00", response=False)
await asyncio.sleep(1.0)
print("Turning Light on...")
await client.write_gatt_char(LIGHT_CHARACTERISTIC, b"\x01")
await client.write_gatt_char(LIGHT_CHARACTERISTIC, b"\x01", response=False)
await asyncio.sleep(1.0)

print("Setting color to RED...")
color = convert_rgb([255, 0, 0])
await client.write_gatt_char(COLOR_CHARACTERISTIC, color)
await client.write_gatt_char(COLOR_CHARACTERISTIC, color, response=False)
await asyncio.sleep(1.0)

print("Setting color to GREEN...")
color = convert_rgb([0, 255, 0])
await client.write_gatt_char(COLOR_CHARACTERISTIC, color)
await client.write_gatt_char(COLOR_CHARACTERISTIC, color, response=False)
await asyncio.sleep(1.0)

print("Setting color to BLUE...")
color = convert_rgb([0, 0, 255])
await client.write_gatt_char(COLOR_CHARACTERISTIC, color)
await client.write_gatt_char(COLOR_CHARACTERISTIC, color, response=False)
await asyncio.sleep(1.0)

for brightness in range(256):
Expand All @@ -83,6 +83,7 @@ async def main(address):
brightness,
]
),
response=False,
)
await asyncio.sleep(0.2)

Expand All @@ -94,6 +95,7 @@ async def main(address):
40,
]
),
response=False,
)


Expand Down
4 changes: 2 additions & 2 deletions examples/sensortag.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ async def notification_handler(characteristic, data):
value = await client.read_gatt_char(IO_DATA_CHAR_UUID)
print("I/O Data Pre-Write Value: {0}".format(value))

await client.write_gatt_char(IO_DATA_CHAR_UUID, write_value)
await client.write_gatt_char(IO_DATA_CHAR_UUID, write_value, response=True)

value = await client.read_gatt_char(IO_DATA_CHAR_UUID)
print("I/O Data Post-Write Value: {0}".format(value))
Expand All @@ -134,7 +134,7 @@ async def notification_handler(characteristic, data):
value = await client.read_gatt_char(IO_CONFIG_CHAR_UUID)
print("I/O Config Pre-Write Value: {0}".format(value))

await client.write_gatt_char(IO_CONFIG_CHAR_UUID, write_value)
await client.write_gatt_char(IO_CONFIG_CHAR_UUID, write_value, response=True)

value = await client.read_gatt_char(IO_CONFIG_CHAR_UUID)
print("I/O Config Post-Write Value: {0}".format(value))
Expand Down
2 changes: 1 addition & 1 deletion examples/uart_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ def handle_rx(_: BleakGATTCharacteristic, data: bytearray):
# property to split the data into chunks that will fit.

for s in sliced(data, rx_char.max_write_without_response_size):
await client.write_gatt_char(rx_char, s)
await client.write_gatt_char(rx_char, s, response=False)

print("sent:", data)

Expand Down

0 comments on commit 401e71a

Please sign in to comment.