aioble.BaseClientCharacteristic.write() ignores response=None parameter

[martin-ringaly]

(https://github.com/micropython/micropython-lib/blob/master/micropython/bluetooth/aioble/aioble/client.py#273)

async def write(self, data, response=None, timeout_ms=1000):
        self._check(_FLAG_WRITE | _FLAG_WRITE_NO_RESPONSE)

        # If the response arg is unset, then default it to true if we only support write-with-response.
        if response is None:
            p = self.properties
            response = (p & _FLAG_WRITE) and not (p & _FLAG_WRITE_NO_RESPONSE)

The comment says to 'default the response arg to true if we only support write-with-response' but shouldn't the code line be as below:

response = (p & _FLAG_WRITE_NO_RESPONSE) and not (p & _FLAG_WRITE)

The consquence (I think) is quite serious as all writes will wait for a response, even if none is requested, which can cause serious delays, problems with timings, etc.

[ekspla]

If you don't want to use write-with-response, why don't you use response=False?

[martin-ringaly]

Thanks @ekspla,

I think passing response=False probably would work as a workaround, but passing a Python False down into the ble.gattc_write(self._connection()._conn_handle, self._value_handle, data, response) C code doesn't look very clean and relies on it being interpreted as a zero mode flag, which coincidentally matches #define MP_BLUETOOTH_WRITE_MODE_NO_RESPONSE (0). That looks brittle to me, so for now I may add a 'fixed' frozen aioble to my forked micropython firmware build. Even more brittle, but perhaps a little cleaner.

Your workaround also doesn't fix what looks like an easy to fix bug in the aioble code (unless I am hallucinating a problem).

[ekspla]

There is no meaning in None used in this case. Using None to control the
default/implicit argument is very common in Python. So, the explicit argument
in this case should be either True or False.

That being said, I am not sure which one of the two cases is correct/favourable
to the majority of users of this library. I am also not sure if this is a bug.

I understood that the original behaviour was not favourable to you,
so I've suggested you to use the explicit argument, False.

[martin-ringaly]

Hi @ekspla, you're right. I had misread the # If the response arg is unset, then default it to true if we only support write-with-response comment, so the code line response = (p & _FLAG_WRITE) and not (p & _FLAG_WRITE_NO_RESPONSE) is logically correct. Then I found it a little weird that the response argument is passed down to the C layer as a Python True/False bool, while the C code seemed to be expecting int values, but on inspection these int values are 0 or 1, so it all works.

I'll close this issue.