Default callback function in a library

Using Arduino Programming Questions
#1

What is the correct default value to give a function pointer when the function pointer parameter is an optional parameter in a library function ?

Currently I have

void update(callbackFunc func = nullptr);

callbackFunc has previously been defined like this, but I am not sure that int is the right data type

typedef void (*callbackFunc) (int arg1);  //is int right ?

The library function tests for the value in the function like this

  if (func == nullptr)
  {
    _callbackRequired = false;
  }

It works, but is it correct ?

Full code below

TestCallback.cpp

#include "Arduino.h"
#include "TestCallback.h"

TestCallback::TestCallback()
{
  _callbackRequired = false;
}

void TestCallback::begin()
{
  _startCount = 5;
  _currentCount = 5;
  _period = 500;
}

void TestCallback::update(callbackFunc func = nullptr)
{
  if (func == nullptr)
  {
    _callbackRequired = false;
  }
  else
  {
    cb1 = func;
    _callbackRequired = true;
  }
  _currentTime = millis();
  if (_currentTime - _startTime >= _period)
  {
    _previousCount = _currentCount;
    _currentCount--;
    if (_currentCount < 0)
    {
      _currentCount = _startCount;
    }
    _startTime = _currentTime;
    if (_callbackRequired)
    {
      cb1(_currentCount);  //execute the callback function
    }
  }
}

TestCallback.h

#ifndef TestCallback_X
#define TestCallback_X

#include "Arduino.h"

class TestCallback
{
  private:
    int _startCount;
    int _currentCount;
    int _previousCount;
    unsigned long _currentTime;
    unsigned long _startTime;
    unsigned long _period;
    boolean _callbackRequired = true;
    int _number = 123;

  public:
    TestCallback();
    typedef void (*callbackFunc) (int arg1); //create function pointer type
    callbackFunc cb1;                       //an instance of the function pointer type
    void begin();
    void update(callbackFunc func = nullptr);
};
#endif

TestCallback.ino

#include "TestCallback.h"

TestCallback testing;

void setup()
{
  Serial.begin(115200);
  testing.begin();
}

void loop()
{
  testing.update(printCount);
}

void printCount(int count)
{
  Serial.println(count);
}
#2 
typedef void (*callbackFunc) (int arg1);  //is int right ?

for typing a function signature you can just use:

typedef void (*callbackFunc) (int);

but prefer to use your C++, because you do not need to create a new type, it already exists:

using callbackFunc = void (*)(int);

uninitialized class member pointers will be nullptr by default, but I like that you explicitly initialize it.

you don't need all this:

void TestCallback::update(callbackFunc func = nullptr)
{
  if (func == nullptr)
  {
    _callbackRequired = false;
  }
  else
  {
    cb1 = func;
    _callbackRequired = true;
  }

pointers can be truth tested:

if (func) {
  cb1 = func;
}

likewise when you call it... not like this:

if (_callbackRequired)
    {
      cb1(_currentCount);  //execute the callback function
    }

but this:

if (cb1)
  {
    cb1(_currentCount);  //execute the callback function
  }

so you save a variable, _callbackRequired isn't necessary.

#3

Thanks for the feedback. I'm off to experiment

#4

Changes made and it still works !

Unless anyone is really interested I won't post the revised code but if anyone would like to see it I will gladly post it.

#5

UKHeliBob:
Changes made and it still works !

Unless anyone is really interested I won’t post the revised code but if anyone would like to see it I will gladly post it.

One more thing… since you are using a function pointer: If your function is only called from one place, you don’t need to create a whole separate function, you can use a lambda with the same signature:

class TestCallback
{
  private:
    int _startCount;
    int _currentCount;
    int _previousCount;
    unsigned long _currentTime;
    unsigned long _startTime;
    unsigned long _period;
    int _number = 123;

  public:
    TestCallback();
    using callbackFunc = void (*) (int);
    callbackFunc cb1;
    void begin();
    void update(callbackFunc func = nullptr);
};

TestCallback::TestCallback()
{
  
}

void TestCallback::begin()
{
  _startCount = 5;
  _currentCount = 5;
  _period = 500;
}

void TestCallback::update(callbackFunc func)
{
  cb1 = func;
  _currentTime = millis();
  if (_currentTime - _startTime >= _period)
  {
    _previousCount = _currentCount;
    _currentCount--;
    if (_currentCount < 0)
    {
      _currentCount = _startCount;
    }
    _startTime = _currentTime;
    if (cb1)
    {
      cb1(_currentCount);  //execute the callback function
    }
  }
}


TestCallback testing;

void setup() {
  Serial.begin(9600);
  testing.begin();
}

void loop() {
  testing.update([](int count){
    Serial.println(count);
  });
}

//or even like this:

//void loop() {
//  auto printCount = [](int count) {
//    Serial.println(count);
//  };
//  
//  testing.update(printCount);
//}

//or even  more pedantic, like this:
//void loop() {
//  TestCallback::callbackFunc  printCount = [](int count) {
//    Serial.println(count);
//  };
//  
//  testing.update(printCount);
//}
#6

you can use a lambda with the same signature:

I think that I will pass on that suggestion for now, at least until I have got a callback implemented in my larger library.

#7