-
Notifications
You must be signed in to change notification settings - Fork 3k
Nordic: Set handle of user description descriptors. #5589
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Nordic: Set handle of user description descriptors. #5589
Conversation
@paul-szczepanek-arm @nvlsianpu Could you review ? |
@@ -65,6 +65,7 @@ ble_error_t nRF5xGattServer::addService(GattService &service) | |||
return BLE_ERROR_NO_MEM; | |||
} | |||
GattCharacteristic *p_char = service.getCharacteristic(i); | |||
GattAttribute *description_descriptor = NULL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should use the existing code practice of prefixing the pointer with p_
following the naming here I'd go with p_user_desc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
@@ -83,6 +84,7 @@ ble_error_t nRF5xGattServer::addService(GattService &service) | |||
for (uint8_t j = 0; j < p_char->getDescriptorCount(); j++) { | |||
GattAttribute *p_desc = p_char->getDescriptor(j); | |||
if (p_desc->getUUID() == BLE_UUID_DESCRIPTOR_CHAR_USER_DESC) { | |||
description_descriptor = p_desc; | |||
userDescriptionDescriptorValuePtr = p_desc->getValuePtr(); | |||
userDescriptionDescriptorValueLen = p_desc->getLength(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why It is expected that tere could be more than one descriptor per characteristic at input? Shouldn't this be an error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Nordic code for the GattServer can be improved in many ways; raising an error if there is multiple user description descriptors may be one of them but it is not the purpose of this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right. I Just spotted this at first sight.
352ed33
to
a665f3e
Compare
/morph build |
Build : SUCCESSBuild number : 603 Triggering tests/morph test |
Exporter Build : ABORTEDBuild number : 215 |
Test : SUCCESSBuild number : 420 |
/morph export-build |
Exporter Build : SUCCESSBuild number : 222 |
Description
Fix a bug in Nordic GattServer implementation: Handle of user description descriptors were not populated when a service was added to the GattServer.
Status
READY
Migrations
NO