Skip to content
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

Implementation of Characteristic descriptor discovery #74

Merged
merged 13 commits into from
Dec 23, 2015

Conversation

pan-
Copy link
Member

@pan- pan- commented Nov 18, 2015

This is the implementation of ARMmbed/ble#105

The characteristic discovery process has been slightly modified. To discover characteristic descriptors, it is required to know the start handle and end handle of a characteristic. The discovery process handle this now.

Limitations:

  • Currently, Characteristics Descriptors 128 bits UUID are not managed properly.

pan- added 6 commits November 13, 2015 14:53
BLE_GATT_STATUS_SUCCESS.
Report error in the Termination callback
Fix terminate discovery (the replacement of the discovery was done after
the call to terminate).
When searching for a running discovery, dismiss results where the
characteristic is equal to the default characteristic value
Add Discovery::operator!=
Add support of DiscoveredCharacteristic last handle in the characteristic
discovery process
Use const references for passing characteristic discovery callbacks
Fix a bug in btle_discovery.cpp, the discovered descriptors were captured
by value instead of reference.
@@ -183,6 +183,7 @@ static void btle_handler(ble_evt_t *p_ble_evt)
break;
}
nRF5xGap::getInstance().processDisconnectionEvent(handle, reason);
// TODO: close pending discoveries
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a question. Is it required to close pending discovery manually ? If a discovery is running, does the stack will report the discovery that the discovery has failed ? My question apply for service, characteristic and characteristic descriptors discovery.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we should invoke the serviceDiscoveryTermination callback before invoking disconnectionCallback. Not sure yet how to report a failure to service discovery.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have made the changes. I have added a new function in nRF5xServiceDiscovery : void terminate(Gap::Handle_t connectionHandle). It is an overload of terminate, if the handle passed in parameters match the handle of the current discovery, it terminate the discovery otherwise, the discovery continue to run.

Add accessor for ServiceDiscovery and CharacteristicDescriptorDiscover in
GattClient.

Remove friend relationship with bleGattcEventHandler
// Close all pending discoveries for this connection
nRF5xGattClient& gattClient = nRF5xGattClient::getInstance();
gattClient.characteristicDescriptorDiscoverer().terminate(handle, BLE_ERROR_INVALID_STATE);
gattClient.discovery().terminate(handle);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for this improvement. :)

@rgrover
Copy link
Contributor

rgrover commented Dec 9, 2015

I've finished my review for the moment. I'll wait for your updates.

@pan-
Copy link
Member Author

pan- commented Dec 21, 2015

@rgrover, @andresag01 Could you review these changes ?

pan- added a commit that referenced this pull request Dec 23, 2015
Implementation of Characteristic descriptor discovery
@pan- pan- merged commit 05763f7 into ARMmbed:develop Dec 23, 2015
@pan- pan- deleted the characteristicDescriptorDiscovery branch February 15, 2016 17:23
@pan- pan- restored the characteristicDescriptorDiscovery branch February 15, 2016 17:23
@pan- pan- deleted the characteristicDescriptorDiscovery branch March 9, 2016 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants