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

BLE::init() now takes in a callback paramter #91

Merged
merged 3 commits into from
Oct 30, 2015
Merged

BLE::init() now takes in a callback paramter #91

merged 3 commits into from
Oct 30, 2015

Conversation

rgrover
Copy link
Contributor

@rgrover rgrover commented Oct 28, 2015

There's a completion callback.
This will require corresponding change to ble-nrf51822. Demos will need to be adapted.

Please review @pan- @andresag01 @marcuschangarm @jaustin

Rohit Grover added 3 commits October 28, 2015 11:39
- There's a new type: BLE::InitializationCompleteCallback_t
- init() now takes a completion callback. This is an optional parameter, if no callback is setup the application can still determine the status of initialization using BLE::hasInitialized() (see below).
- There's a new API: BLEInstanceBase::hasInitialized() which transports need to implement.
- BLEInstanceBase.h is no longer included from BLE.h. We use a forward declaration of BLEInstanceBase instead. This is required us to move implementations of BLE methods out of the header and into BLE.cpp.
- There's a new API:  BLE::getInstanceID(), which simply returns the ID of an instance.
- There are new error types around initialization.
@rgrover
Copy link
Contributor Author

rgrover commented Oct 28, 2015

please note that this changes the minor number.

@rgrover
Copy link
Contributor Author

rgrover commented Oct 28, 2015

refer to #90

@marcuschangarm
Copy link
Contributor

I would bump the major version number for something like this. Looks good though.

@pan-
Copy link
Member

pan- commented Oct 28, 2015

It looks good to me but I'm worried about the default parameter in the init member function.
Of course, it will not break the existing examples and the existing code at compile time but with this "silent" change, the existing code can be already broken at runtime if you use a target which does not initialize immediately.

@marcuschangarm
Copy link
Contributor

@pan- that was actually my point with the major version change - with the version bump it wont be a silent change.

@rgrover
Copy link
Contributor Author

rgrover commented Oct 30, 2015

@pan- I believe we will be updating all examples to reflect the new init API; so the impact to future dual-chip targets would be minimized. We'll also inform our existing target-owners.
I'm happy with the idea of bumping the major version number.
We should allow the default parameter for init so that people using nRF don't get surprized when they upgrade to the latest of BLE_API

rgrover added a commit that referenced this pull request Oct 30, 2015
BLE::init() now takes in a callback paramter
@rgrover rgrover merged commit e46cef5 into ARMmbed:develop Oct 30, 2015
rgrover pushed a commit to ARMmbed/ble-nrf51822 that referenced this pull request Oct 30, 2015
=============

* Update init() to match the changes around initializationCompleteCallback.
  Also implemented hasInitialized().
  Refer to ARMmbed/ble#91 and ARMmbed/ble#90.

* Some changes for memory savings. Certain singletons are now allocated
  dynamically; so some memory may not be needed if the application exercises
  limited functionality. Also reduced the size of some global tables for
  memory savings; affected tables/constants: UUID_TABLE_MAX_ENTRIES (down to 4),
  DEVICE_MANAGER_MAX_BONDS (down to 2).
rgrover pushed a commit to ARMmbed/ble-nrf51822 that referenced this pull request Oct 30, 2015
=============

* Update init() to match the changes around initializationCompleteCallback.
  Also implemented hasInitialized().
  Refer to ARMmbed/ble#91 and ARMmbed/ble#90.

* Some changes for memory savings. Certain singletons are now allocated
  dynamically; so some memory may not be needed if the application exercises
  limited functionality. Also reduced the size of some global tables for
  memory savings; affected tables/constants: UUID_TABLE_MAX_ENTRIES (down to 4),
  DEVICE_MANAGER_MAX_BONDS (down to 2).
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.

3 participants