sandeepmistry / arduino-BLEPeripheral

An Arduino library for creating custom BLE peripherals with Nordic Semiconductor's nRF8001 or nR51822.
MIT License
462 stars 179 forks source link

Objects encapsulating services: callbacks vs. listeners #107

Open bsiever opened 7 years ago

bsiever commented 7 years ago

Hi,

First, thanks for a great library (and tools for nRF development)!

Second, a question/thought about the API:

It looks like the only callback mechanism for characteristic events is through c-style functions set by the setEventHandler(), correct? Or am I missing something?

I was trying to create classes for self-contained services that may have multiple instances. For example, a "light color" service that would support a single tri-color LED. Several instances of the service could be used to support multiple LEDs. A person creating a device just instantiates an object for each LED they have on their device.

The callback mechanism doesn't work very well for completely self-contained instances due to the way events are dispatched (event handlers have to be a static function, so they don't work well when there may be multiple instances of an object). As a workaround I modified the library to allow both BLECharacteristics and BLEPeripherals to have listeners. Each characteristic can have a single listener and the peripheral can have a chain of BLEDeviceEventListeners. Does anyone have any suggestions for a better approach? And/or is there any interest in adding this kind of functionality to the BLEPeripheral library?

sandeepmistry commented 7 years ago

@bsiever

It looks like the only callback mechanism for characteristic events is through c-style functions set by the setEventHandler(), correct? Or am I missing something?

Yes, that's correct.

Each characteristic can have a single listener and the peripheral can have a chain of BLEDeviceEventListeners. Does anyone have any suggestions for a better approach? And/or is there any interest in adding this kind of functionality to the BLEPeripheral library?

It's still unclear to me why you need more than one listener per event. Could you please provide more details.

Does anyone have any suggestions for a better approach?

Since the object reference is passed into the callback, your library/code could have it's own registry and check the address of the objects for book keeping potentially.

And/or is there any interest in adding this kind of functionality to the BLEPeripheral library?

Depends on how much RAM it's going to take, things are pretty tight on the Uno at the moment.

bsiever commented 7 years ago

@sandeepmistry

It's still unclear to me why you need more than one listener per event. Could you please provide more details.

It's sometimes desirable for standalone services to get GAP events, especially connect/disconnect events. For example, if someone wanted to create a motor control service they may want to automatically stop the motor following disconnect events. Including two instances of this service would make a differential drive robot pretty simple. Something like:

// Instances are declared somewhere
MotorControlService leftWheel(LeftSpeedPin, LeftDirectionPin, "LEFT");
MotorControlService rightWheel(RightSpeedPin, RightDirectionPin, "RIGHT");
...
// The instances are added to the device
leftWheel.addTo(blePeripheral);
rightWheel.addTo(blePeripheral);
...
// Somewhere there are calls to change motor speeds: Ex: leftWheel.setSpeed(10);

My goal is/was to be able to share functionally complete services that are easy to integrate/combine. The constructors would add each instance to the BLEPeripheral call-back chain, so each instance could deal with the disconnect.

The current callback mechanism (and that used by Nordic) requires one to add a service and also add calls to pass GAP/GATT events. They may require something like:

...
// In the BLEDeviceEventListener:
   leftWheel.peripheralEvent(event);
   rightWheel.peripheralEvent(event);
   // One for each "service" included
...
  // And something comparable for characteristic events (but contained in the service's class

Keeping the event dispatch decisions in BLEPeripheral and BLECharacteristic helps avoid having multiple levels of dispatch. If other people may be interested in this OOP approach to services, it would be nice to integrate it into the BLEPeripheral library itself. If there's not much interest (and I suspect there isn't), then I'll go with another approach, like creating a hierarchy on top of the BLEPeripheral classes.

Depends on how much RAM it's going to take, things are pretty tight on the Uno at the moment.

The "passive overhead" (overhead even when it's not in use) is one more pointer per BLECharacteristic and one more pointer per BLEPeripheral. Code overhead is also pretty small: a few extra checks/loops in both BLEPeripheral and BLECharacteristic.

My current approach to the listener chain in BLEPeripheral assumes that services aren't added/removed dynamically (it uses a simple singly linked list).

Thanks for the feedback/input. Feel free to close the topic if you think either a) there'd be little actual use of this feature and/or b) the overhead exceeds the potential benefit.

sandeepmistry commented 7 years ago

@bsiever thanks for the info.

For now, I prefer to leave this issue open, and see how many members of the community request this feature.