BLE central API

[sandeepmistry]

Based on the outcome of the API meeting, I've added a first draft of a BLE central API in the api/BLE/central folder.

It's probably best to review the examples in the following order:

• scan.ino: Starts scanning for peripherals and prints out advertisement info. of discovered peripheral
• scan_callback.ino: Similar to scan.ino but callback style
• led_control.ino: Controls the LED on the 101 BLE LED example using a local button
• sensortag_button.ino: Listens to simple key notifications from a TI SensorTag and prints out events to the serial monitor. Similar to Tom's Node.js sensorTagSlideshow example.
• peripheral_explorer.ino: Connects to a peripheral and prints out it's services, characteristics, and descriptors. Similar to Tom's Node.js exploreAsync example.

Other item's to discuss:

1. Are there too many API's to access attributes of a peripheral? I've added a few to cater for various use cases, however I understand this might make documentation more difficult.
2. Is there a nicer way to handle reading and writing attributes with values (characteristics and descriptors). See BLERemotePeripheralAttributeWithValue.h. Maybe we can re-use the UDP style, with beginWrite, Print API's, and endWrite.
3. Class names, they are long now, mostly to avoid clashes with the BLEPeripheral API. In case there are use cases for using both peripheral and central mode at the same time in the future.

cc/ @tigoe @damellis @cmaglie @agdl

[sandeepmistry]

@mbanzi it would be valuable to also get Don Coleman's feedback on this. Is it ok to give him read access to this repo or share via another mechanism?

[tigoe]

+1 that. Don would have good feedback.

[tigoe]

A couple thoughts:

1. Yes, work toward shorter class names. How much overlap is there between the central and peripheral libraries, and can they share any common transport or config functions? When it's possible to do something the same way on both, I think that might be helpful.
2. startScanningWithDuplicates, could the WithDuplicates be a parameter of startScanning()? Is there a reason why that'd be a bad idea?

3/ It'd be nice to hide the references on the event handlers if possible. Is there another way to pass the peripheral in?

In ScanCallback, I don't understand this line:

 for (int i = 0; peripheral.hasAdvertisedService(i); i++) {

Is there a typo there? wouldn't it be i < something in the middle?

Other than those notes, looks good so far. But I definitely think @cmaglie and @damellis should weigh in if they haven't already.

[sandeepmistry]

Thanks for the feedback!

How much overlap is there between the central and peripheral libraries, and can they share any common transport or config functions?

I'd like to as a minimum distinguish between a local and remote, peripherals and centrals. For example, if the Arduino is in peripheral mode BLEPeripheral refers to the local peripheral type and BLERemoteCentral is used for centrals connected to the local peripheral. Similar for central mode, BLECentral is the local central, BLERemotePeripheral would be used for the peripheral's you are connecting to. Maybe there's a nicer term than Remote to distinguish both.

For attributes services, characteristics, and descriptors, BLEService, BLECharacteristic and BLEDescriptor could be re-used. However, it might not make sense in peripheral mode to subscribe to your own characteristic. If we go with a similar approach to above, you could have local attributes and remote attributes with different API's.

startScanningWithDuplicates, could the WithDuplicates be a parameter of startScanning()? Is there a reason why that'd be a bad idea?

I'd be up for that. I was remembering some suggestions Arturo had about the CurieIMU library using bool as an argument to enable/disable features. This rule probably doesn't apply here though.

3/ It'd be nice to hide the references on the event handlers if possible. Is there another way to pass the peripheral in?

Yes, we can change it to do that. I was trying to avoid copying peripheral instances. However, we can figure out a nicer to handle copies internally in the library. (Note: while working on this API, I was prototyping it on the BTLC1000 shield at the same time). The current CurieBLE library uses references, so we should update it as well.

Is there a typo there? wouldn't it be i < something in the middle?

No typo, but it's inconsistent. peripheral.advertisedServiceCount() is a better fit:

 for (int i = 0; peripheral.advertisedServiceCount(); i++) {

the current hasAdvertisedService API returns a bool to see if advertised service i exists before printing it out.

I'll hold off making the changes discussed above until we get a bit more feedback from others.

[tigoe]

I think the local/remote thing is confusing. We don't distinguish between local/remote clients with the client/server libraries in WiFi, do we? Maybe the same principle should apply here.

"However, it might not make sense in peripheral mode to subscribe to your own characteristic." You never know, someone will find a use for it.

"The current CurieBLE library uses references, so we should update it as well." Yes, we should.

 for (int i = 0; peripheral.advertisedServiceCount(); i++

I think having the middle parameter NOT as a comparison and NOT related to i will be confusing to many coders. We're so used to the (i=0; i<limit; i++) model that to throw in a limit that has no relation to the iterator is confusing. I still don't understand it myself. Is there a way to phrase it that makes what it's doing more apparent? Maybe it's actually a while loop instead?

[sandeepmistry]

We don't distinguish between local/remote clients with the client/server libraries in WiFi, do we? Maybe the same principle should apply here.

"The current CurieBLE library uses references, so we should update it as well." Yes, we should

Excellent point! I think we can make same happen on BLE then. It'll make the internal book keeping a bit trickier and probably use more RAM. However, on ARM and ARC there's much more RAM compared to AVR - we should put it to good use.

I'll try to prototype universal classes for BLE central and peripheral in about 2 weeks after I wrap up my current project. Same applies for removing references. I don't expect any issues, just want to see what the performance/memory trade off will be.

I think having the middle parameter NOT as a comparison and NOT related to i will be confusing to many coders.

Sorry, there was a typo in my updated loop snippet, I actually mean't:

 for (int i = 0; i < peripheral.advertisedServiceCount(); i++) {
  // ...
}

[tigoe]

+1 to all that. glad we're on the same page.

[sandeepmistry]

It's been awhile!

I've pushed an updated draft to the BLE folder. It includes the items we've discussed above and an combined central + peripheral API. I would recommend taking a look at the examples folder

There are a few breaking changes to the original BLEPeripheral API:

• Now there is a global BLE (of type BLEDevice) instance created for you. No more BLEPeripheral blePeripheral; declaration needed. BLEDevice can represent the local device or remote, some methods are no-ops depending on what it represents. For example, BLE.connect() does nothing because you can't connect to yourself.
• BLE.begin() must be called at start of sketch to initialize hardware. Then BLE.startAdvertising() needs to be called to advertise. Before blePeripheral.begin() was called last and started advertising.
• There's no more .addAttribute(...), replaced by: BLE.addService(service), service.addCharacteristic(characteristic), and characteristic.addDescriptor(descriptor)
• characteristic.setValue(...) has been renamed to characteristic.writeValue(...)
• Callback signatures no longer use references

We'll need to have a sketch upgrade guide or think about adding backwards compatibility where possible.

I'm still not happy with the BLEAttributeWithValue base type, need to think about using Stream - but also need to be careful of sizes of characteristics. For example, if the remote characteristic only expects 1 bytes, what should characteristic.write(1) do? The current approach has a characteristic.writeByte(1) to make the caller think about sizes.

Another concern is there are a lot of new API's, so the documentation will become larger.

cc/ @tigoe @cmaglie @agdl @kdecoi

[tigoe]

Are these breaking changes going to be back-ported to BLE-Peripheral? I would recommend doing so in the interests of compatibility.

[tigoe]

In the central sensorTag application, why don't you have to check that the characteristic exists before you subscribe to it?

[tigoe]

Rest of the examples look good. I don't love having breaking changes, but they definitely are improvements. RE: central, the API makes sense, though I wish there was a way to simplify the scan() -> connect -> discover services -> discover characteristics -> take action sequence. But that is a problem of BLE, not this API.

[sandeepmistry]

Good question, I'm in two minds:

• Back port them
• Create a new library for the nRF51 and nRF52 which includes both central and peripheral

The last two breaking changes will use much more RAM, so nRF8001 + Uno might not be useable. How many nRF8001 based boards do you see in your courses? I think Chi-Hung mentioned it's end of life.

[tigoe]

I see some 8001’s, but not always with uno.

On Aug 30, 2016, at 3:34 PM, Sandeep Mistry [email protected] wrote:

Good question, I'm in two minds:

Back port them Create a new library for the nRF51 and nRF52 which includes both central and peripheral The last two breaking changes will use much more RAM, so nRF8001 + Uno might not be useable. How many nRF8001 based boards do you see in your courses? I think Chi-Hung mentioned it's end of life.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/arduino/ArduinoCore-API/issues/7#issuecomment-243553606, or mute the thread https://github.com/notifications/unsubscribe-auth/AAXOnxfGrH62kJuvZGL4S6Y0MIhtW7fRks5qlIXfgaJpZM4IQc2T.

[sandeepmistry]

In the central sensorTag application, why don't you have to check that the characteristic exists before you subscribe to it?

I'll change it to:

  if (!simpleKeyCharacteristic) {
    Serial.println("no simple key characteristic found!");
    peripheral.disconnect();
    return;
  } else if (!simpleKeyCharacteristic.subscribe()) {
    Serial.println("subscription failed!");
    peripheral.disconnect();
    return;
  } else {
    Serial.println("Subscribed");
  }

I don't love having breaking changes, but they definitely are improvements.

Yes it's not great, maybe 1/2 or more them can be avoided by adding backwards compatible API's. They'll probably be a cost to code space and/or RAM though.

For example, we can keep a legacy BLEPeripheral type that uses BLE internally.

Need to think about the removing the references on callbacks, this is the trickiest.

RE: central, the API makes sense, though I wish there was a way to simplify the scan() -> connect -> discover services -> discover characteristics -> take action sequence. But that is a problem of BLE, not this API.

Do you think we should add support to connecting to a BT address if you know it ahead of time and don't want to scan? I'm a fan of the Apple model where you always have to scan.

I've combined discover service and characteristics into peripheral.discoverAttributes(), we could have another convenience API like peripheral.connectAndDiscoverAttributes().

[sandeepmistry]

we can keep a legacy BLEPeripheral type that uses BLE internally.

The more I think about it, this is the way to go ... best of both worlds.

[sandeepmistry]

I've added a BLE. setAdvertisedService(service) based on feedback from @cmaglie.

@tigoe were going to give Intel read access to this repo soon.

[tigoe]

Good. I look forward to integrating all of this. When we're definitely settled on the API I might bug you to help me modify the MTT examples, though it looks like the changes will be minimal :)

I like what you've done here, it's clean and simple.

[sandeepmistry]

@tigoe going back to this:

RE: central, the API makes sense, though I wish there was a way to simplify the scan() -> connect -> discover services -> discover characteristics -> take action sequence. But that is a problem of BLE, not this API.

One more thought, we can potentially remove the peripheral.discoverAttributes() call. See led_control.ino for more context.

Instead, anytime you try to retrieve the service/characteristic/descriptor count or query if one exists we could send a request to the peripheral then.

What do you think? It will result in one less call for the user to call.

[tigoe]

I like that. I assume you could still discover attributes if you want to, but you could also skip to the others if you prefer and get an error code, right?

[sandeepmistry]

Good point, I forget about the error case.

A snippet from the peripheral_explorer.ino example.

  // loop the services of the peripheral and explore each
  for (int i = 0; i < peripheral.serviceCount(); i++) {
    BLEService service = peripheral.service(i);

    exploreService(service);
  }

So peripheral.serviceCount() could return -1 on error. (0 for no services - a really odd case).

I like your approach => if attributes haven't been discovered by peripheral.discoverAttributes(), peripheral.serviceCount() (and related API's) will trigger the attribute discovery first.

[tigoe]

Sounds good to me.

[sandeepmistry]

@tigoe one thing that's missing from the current API spec is the ability to start scanning with a filter. Here's a proposal, feedback from everyone is welcome

// starts scanning and filters out duplicates
BLE.startScanning();

// starts scanning for a peripheral advertising service UUID string
BLE.startScanningForServiceUuid(serviceUuid); 

// starts scanning for a peripheral advertising local name string
BLE.startScanningForLocalName(localName); 

Need to also revisit how scanning without filtering for duplicates fits in to this. It would be useful when scanning for iBeacons etc.

cc/ @SidLeung @noelpaz @sgbihu @agdl

[SidLeung]

startScanningForServiceUuid and startScanningForServiceUuid are scanning with filter. startScanning - scan with no filter.

Liang adds comment here, post existing API here for review.

[sgbihu]

The below is my proposal. Please review it. If this is acceptable. We can implement in this way. /** * @brief Start scanning for peripherals without filter * * @param none * * @return none * * @note none */ void startScanning();

/**
 * @brief   Start scanning for peripherals and filter by device name in ADV
 *
 * @param   name    The device's local name.
 *
 * @return  none
 *
 * @note  none
 */
void startScanning(String name);

/**
 * @brief   Start scanning for peripherals and filter by service in ADV
 *
 * @param   service    The service
 *
 * @return  none
 *
 * @note  none
 */
void startScanning(BLEService& service);

[tigoe]

I still feel like it's better to overload the startScanning() function to support either of the other two. @damellis might have good reason why not though. I can see common uses of all three, but the function names are rather long.

First off, is there a reason to call it startScanning() rather than just scan() (or scanFor...())?

Maybe: scan() - scan for anything scan(String name) - scan for localName scan(int UUID) - scan for service UUID scan(int macAddr) - ? Ambiguous overload; is it useful at all? Maybe not.

THen retain setEventHandler and BLE.available as shown in scan.ino and scan_callback.ino?

[tigoe]

void startScanning(BLEService& service);

At the risk of too much repetition: get rid of the references. Always. Always. Always.

[sandeepmistry]

First off, is there a reason to call it startScanning() rather than just scan() (or scanFor...())?

That's good feedback, should we go with stopScan() to stop?

scan() - scan for anything scan(String name) - scan for localName scan(int UUID) - scan for service UUID scan(int macAddr) - ? Ambiguous overload; is it useful at all? Maybe not.

The underlying issue, is we are treating the UUID and address as strings, along with the name.

@sgbihu's suggestion of startScanning(BLEService& service); avoids this, but forces the user to create a BLEService object:

BLE.scanFor(BLEService("180f")); // scan for the peripherals advertising the battery service

Another question, is do we really need to support built-in filtering by local name, is service UUID enough? (CoreBluetooth on OS X/iOS just has the option for scan for UUID, not name).

Here's an updated proposal, assuming we want to keep scanning by name:

BLE.scan();
BLE.scan(int withDuplicates); // option to filter out duplicate addresses

BLE.scanForUUID(String uuid);
BLE.scanForUUID(String uuid, int withDuplicates);

BLE.scanFoName(String name);
BLE.scanFoName(String name, int withDuplicates);

BLE.scanForAddress(String address);

BLE.stopScan();

[tigoe]

I like your updated proposal @sandeepmistry. Solves all the issues, I think, pretty well.

How much does it add to the memory footprint to keep scanForName()? We may not really need it, but it is convenient. If it's a big memory hog, we could kill it, but if it's not a big deal, I'd keep it.

[sandeepmistry]

Thanks @tigoe!

I noticed there is some case inconsistency with the rest of the API's, so BLE.scanForUUID(String uuid); needs to be changed to BLE.scanForUuid(String uuid);.

How much does it add to the memory footprint to keep scanForName()?

The max local name is 29 bytes, if I'm not mistaken. So I think it's ok to keep.

@sgbihu @SidLeung @noelpaz does the above proposal look good to you? Do you have any feedback. Let me know, then I can update the headers and examples in this repo.

[sandeepmistry]

I've pushed the changes discussed above to master: https://github.com/arduino/ArduinoCore-API/commit/7a043a6e3d36dd1f1a2a4eab32bad50219797e29