arduino-BLEPeripheral icon indicating copy to clipboard operation
arduino-BLEPeripheral copied to clipboard

Support for requesting SoC temperature for nrf51822 application.

Open bojh opened this issue 8 years ago • 8 comments

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.

bojh avatar Nov 02 '15 14:11 bojh

@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() { }

sandeepmistry avatar Nov 07 '15 21:11 sandeepmistry

@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.

prjh avatar Nov 08 '15 17:11 prjh

@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 avatar Nov 15 '15 17:11 sandeepmistry

@bojh @prjh anything else to discuss on this?

sandeepmistry avatar Jan 03 '16 19:01 sandeepmistry

@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.

prjh avatar Jan 04 '16 09:01 prjh

OK for me

bojh avatar Jan 04 '16 09:01 bojh

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

sandeepmistry avatar Jan 09 '16 15:01 sandeepmistry

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

bojh avatar Dec 20 '16 14:12 bojh