search

zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.67k stars 6.53k forks source link

bt_gatt_unsubscribe creates write request to CCC and then cancels it #47682

Closed sean-madigan closed 2 years ago

sean-madigan commented 2 years ago

Describe the bug bt_gatt_unsubscribe calls bt_gatt_cancel straight after gatt_write_ccc, this means that the write callback function returns BT_ATT_ERR_UNLIKELY.

This bug was introduced in commit 8d5be19.

Please find link to Nordic Devzone ticket highlighting the issue - https://devzone.nordicsemi.com/f/nordic-q-a/89813/bug-bt_gatt_unsubscribe-creates-write-request-to-ccc-and-then-cancels-it

jori-nordic commented 2 years ago

There is another behavior that's mentioned in the DevZone ticket:

Additionally, there is an edge case of bt_gatt_unsubscribe where a notification received between the write request and the write response (CCC) won't get sent to the application because it removes the subscription from sub->list before getting the write response. Since there are server implementations that send out pending notifications prior to sending the write response,

My opinion is that it is okay to miss those notifications since there was an intent from the app to unsubscribe anyway. What do you guys think @Vudentz @alwa-nordic @jhedberg ?

alwa-nordic commented 2 years ago

Additionally, there is an edge case of bt_gatt_unsubscribe where a notification received between the write request and the write response (CCC) won't get sent to the application because it removes the subscription from sub->list before getting the write response. Since there are server implementations that send out pending notifications prior to sending the write response,

My opinion is that it is okay to miss those notifications since there was an intent from the app to unsubscribe anyway.

It's fine, I think. This choice is a property of Host API, so we can choose. It's a future-problem for when a profile that actually cares about this pops up.