sandeepmistry / arduino-BLEPeripheral

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

Support for requesting SoC temperature for nrf51822 application. #33

Open bojh opened 8 years ago

bojh commented 8 years ago

For to use/retrieve the temperature value from the nrf51822 SoC, it would be nice to have a:

public void requestTemperature() { this->_device->requestTemperature(); } 

method for the BLEPeripheral class. For to get notified with the value, we can create a new derived MyBLE class overloading BLEPeripheral virtual method:

void MyBLE::BLEDeviceTemperatureReceived(BLEDevice& device, float temperature); 

method. An optional alternative would be to register a callback giving back (only) the temperature.

sandeepmistry commented 8 years ago

@bojh it would be great if there was a more generic API, the nRF8001 and nRf51822 both have temp. sensors on board, but future supported boards might not.

What do you think of?

enum BLEDeviceRequest {
  // ..
  BLEDeviceRequest Address,
  BLEDeviceRequest BatteryLevel,
  BLEDeviceRequest Temperature
};

bool BLEPeripheral::request(BLEDeviceRequest request); // returns true on success, false on failure (no sensors)

void setResposneHandler(BLEDeviceRequest request, BLEDeviceResponseHandler response);

Maybe, it's getting too fancy ..

Note: Right now, if you extend BLEPeripheral you can already override,

virtual void BLEDeviceAddressReceived(BLEDevice& device, const unsigned char* address);
virtual void BLEDeviceTemperatureReceived(BLEDevice& device, float temperature);
virtual void BLEDeviceBatteryLevelReceived(BLEDevice& device, float batteryLevel);

and have acces to the following BLEDevices API's:

    virtual void requestAddress() { }
    virtual void requestTemperature() { }
    virtual void requestBatteryLevel() { }
prjh commented 8 years ago

@sandeepmistry The question is cerainly justified and I allready had it realized, as you have proposed by a derived BLEPeripheral class. On the other hand, I expect also following chipsets (..nrf52x) will also support such universal functions. From my point of view the fancy way seems a little bit more intuitive, ... but BLEDeviceResponseHandler has to be defined and we have different response value types :-) ... but surely depends a little bit on taste.

sandeepmistry commented 8 years ago

@prjh we could make it a union?

union BLEDeviceResponse {
  float f;
  char[6] address;
  // ....
};
typedef void (* BLEDeviceResponseHandler)(BLEDevice& central, BLEDeviceResponse& response);

Slightly unrelated, I've been also thinking about making a breaking change to the API, to expose BLEDevice, changing:

BLEPeripheral                    blePeripheral       = BLEPeripheral(BLE_REQ, BLE_RDY, BLE_RST);

to something like:

// nRF8001
BLENRF8001Device nRF80001            = BLENRF8001Device(BLE_REQ, BLE_RDY, BLE_RST);
BLEPeripheral    blePeripheral       = BLEPeripheral(nRF80001);

// nRF51822
BLENRF51822Device nRF51822            = BLENRF51822Device();
BLEPeripheral    blePeripheral       = BLEPeripheral(nRF51822);

It would make sketches less portable (require one line change), but would support other slave boards like the nRF8001.

sandeepmistry commented 8 years ago

@bojh @prjh anything else to discuss on this?

prjh commented 8 years ago

@sandeepmistry OK the union would al be fine, but I we than have to use a discriminator value (may be BLEDeviceRequest ) to know the callback reason.

Your ideas to the API is OK for me, because the hardware does no change so often for real projects. From compatibility view, I think it's OK to have a individual HW dependend constructor.

bojh commented 8 years ago

OK for me

sandeepmistry commented 8 years ago

Re-opening, as this is not implemented yet :)

bojh commented 7 years ago

I will provide an minimal, incremental solution as PR, making the BLEDevice::requestTemperature() function accessible from the BLEPeripheral class.