Fixing BLE GATT Characteristic notification and Characteristic Descriptor read #4464
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 3 commits
October 29, 2020 13:41
…ions When registering a notification on a characteristic, the 2902 descriptor (CCCD) value is set to 1 (or 2 for indication). According to the BLUETOOTH CORE SPECIFICATION Version 5.2, Revision Date 2019-12-31, section 4.12.3 "Write Characteristic Descriptors" (page 1588), the characteristic descriptor write must expect a response. Currently, the descriptor write is performed without expecting a reponse, which prevents the notification to be functional with some BLE stacks. This commit modify the write to expect the response. Signed-off-by: Jimmy Durand Wesolowski <[email protected]>
This commits prevents a permanent wait when calling BLERemoteDescriptor readValue function, on the m_semaphoreReadDescrEvt semaphore. ESP32 BLE stack calls to remote characteristic - notification, - value read - value write and remote characteristic descriptor - value read are asynchronous. When such a call is performed by this library, a semaphore is taken prior to the BLE stack read or write operation, and waited on after it. Releasing the semaphore is done by the characteristic event handling function (gattClientEventHandler), when the appropriate event is received. However, the characteristic descriptor events are discarded, and the value read semaphore is never released. This commits forwards the GATT client events from the remote characteristic down to their remote characteristic descriptor, and implements their event handling. Adding a semaphore for the remote characteristic descriptor value write will be done in a separate commit. Signed-off-by: Jimmy Durand Wesolowski <[email protected]>
This adds a semaphore to characteristic descriptor value write, to mimic the value read function, and to ensure completion of the operation before we carry on. Signed-off-by: Jimmy Durand Wesolowski <[email protected]>
JimmyDurandWesolowski
changed the title
JimmyDurandWesolowski
changed the title
chegewara
reviewed
Oct 30, 2020
| @@ -468,7 +475,7 @@ void BLERemoteCharacteristic::registerForNotify(notify_callback notifyCallback, | |||
| if(!notifications) val[0] = 0x02; | |||
| BLERemoteDescriptor* desc = getDescriptor(BLEUUID((uint16_t)0x2902)); | |||
| if (desc != nullptr) | |||
| desc->writeValue(val, 2); | |||
| desc->writeValue(val, 2, true); | |||
There was a problem hiding this comment.
What is the reason to response await here?
There was a problem hiding this comment.
I gave the details in the commit that introduces the change. For convenience, the commit message is:
When registering a notification on a characteristic, the 2902 descriptor
(CCCD) value is set to 1 (or 2 for indication).
According to the BLUETOOTH CORE SPECIFICATION Version 5.2, Revision Date
2019-12-31, section 4.12.3 "Write Characteristic Descriptors" (page 1588),
the characteristic descriptor write must expect a response.
Currently, the descriptor write is performed without expecting a reponse,
which prevents the notification to be functional with some BLE stacks.
This commit modify the write to expect the response.
There was a problem hiding this comment.
Ok, thanks.
Commit aproved.
There was a problem hiding this comment.
Thank you.
chegewara
approved these changes
Oct 30, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These commits fix some issues with GATT Characteristic and Characteristic Descriptor issues.