member function of several individual objects passed as handler to another member function

Question

My thread seems to be similar to my previous one, which I managed to handle with a lambda function. But again I have a problem and I guess, that lambda functions are not the solution to the problem, which I actually didn't fully understand yet.

What I required: In my arduino projects I often had a need to change a variable's value manually during runtime (like a duty cycle, led's state or what ever). So I implemented a class that provides that functionality in an easy-to-use way, meaning I can access global variables by parsing an input string (through serial, tcp, telnet, w.e.) formatted in my own protocol. Another requirement was that it should also be possible to link an event to such an variable which is executed when the variable is accessed through my interface.

What I have done: In an simplified version my "class Parameter" has attributes for the address of my variable, the name I choose to access it and a pointer to the function I want to bind. The corresponding read- and write-methods handle to access the actual value at the given address. The original version also has attributes with information about access-level and datatype. Also separate events are bound to read and write events, but I'll leave that part out, as well as the fact that all relevant functions are overloaded to access variables of different datatypes. So I'll stick to the simplified version as it is short enough to demonstrate my problem.

For my class to work I need a function 'new_parameter(,)' to store my parameters in a vector and functions 'read_param()'/'write_param()' to actually access a specific parameter via string. Those last functions represent my parser, but I'll leave that out as well.

Below is the code that seems to work well, even though I'm sure that there are better ways to do it. For a MCVE it is compiled with g++ 7.3.0, but finally it has to compile with avr-g++.

//parameter.h
using namespace std;

class Parameter {
  public:
    Parameter(int *p, std::string n, void (*e)());
    long int address;
    std::string name;
    void (*event)();

    int read();
    void write(int v);
};
Parameter::Parameter (int *p, std::string n, void (*e)()) {
  address = (long int) p;
  name=n;
  event=e;
}

int Parameter::read () {
  if(event!=NULL){
    event();
  }
  int value = *reinterpret_cast<int*>(address);
  return value;
}

void Parameter::write (int v) {
  if(event!=NULL){
    event();
  }
  *(int*)(address)=v;
}

std::vector<Parameter> parameters;

void new_parameter (int *p, std::string n="defaultname", void (*e)()=NULL) {
  parameters.push_back(Parameter(p,n,e));
}

int read_param(std::string n) {
  for (int i=0;i<parameters.size();i++) {
    if (parameters[i].name == n) {
      int v = parameters[i].read();
      return v;
    }
  }
  return -1;
}

void write_param(std::string n, int v) {
  for (int i=0;i<parameters.size();i++) {
    if (parameters[i].name == n) {
      parameters[i].write(v);
      break;
    }
  }
}


//simple_main.cpp
#include <vector>
#include <string>
#include <locale>
#include <functional>
#include "device.h"

//a global variable
int variable1=-1;
//an event executed when global variable is accessed
void variable1_event () {printf("variable1 event\n");}

int main () {
  //create parameter object for variable
    new_parameter(&variable1,"variable1",variable1_event);

  //read parameter
  printf("1: %i\n",variable1);
  printf("2: %i\n",read_param("variable1"));

  //change value
  write_param("variable1",10);

  //read parameter
  printf("3: %i\n",variable1);
  printf("4: %i\n",read_param("variable1"));
}

When executed main() has the following output:

1: -1
variable1 event
2: -1
variable1 event
3: 10
variable1 event
4: 10

This met my requirements in the past. So far, so good!

In my current project I have a variable number of slave devices connected to my mcu (ESP32) with I2C, each having an identical set of parameters (e.g. set-temperature for temperature control) that I now want to access through my previously demonstrated 'class Parameter'. As the slave devices are of the same kind creating a 'class Device' is an obvious solution. Then I create any number of objects, depending on how many i2c-slaves are connected. Using a 'class Device' means, my parameter will now be pointing to an attribute and the corresponding event-function is now an event-method. The thing is that this event-method has to transmit data to a specific slave and therefor can't be static, as it has to be called with different i2c-addresses (right?). I've tried everything in my ability, but didn't make it to work yet.

This is my simplified version of 'class Device':

//parameter.h
#define MAX_DEVICES 4

int device_count=0;

class Device {
    public:
    Device();
        Device(uint8_t i2c_address);

        bool is_default;
        uint8_t i2c_address;
        int data;

        void i2c_write();
};
Device::Device () {
  is_default=true;
}
Device::Device (uint8_t i2c) {
    is_default=false;
    i2c_address=i2c;
}

Device devices [MAX_DEVICES];

void Device::i2c_write () {
    printf("call to i2c_write (address %i, data %i)\n",i2c_address,data);
}

int get_free_index () {
    for (int i=0; i<MAX_DEVICES; i++) {
        if (devices[i].is_default) return i;
    }
    return -1;
}

void new_device (uint8_t i2c) {
    int new_index=get_free_index();
    if (new_index>=0) {
    devices[new_index]=Device(i2c);
//    new_parameter(&devices[new_index].data, "device"+std::to_string(new_index)+"data",  devices[new_index].i2c_transmit)
  }
    else printf("Error: exceeded maximum number of engines\n");
}

See my advanced main function below to see how I'd like to handle my devices.

//advanced_main.cpp
#include <vector>
#include <string>
#include <locale>
#include <functional>
#include "parameter2.h"
#include "device2.h"

int variable1=-1;
void variable1_event () {printf("variable1 event\n");}

int main () {
  //create parameter object for variable
    new_parameter(&variable1,"variable1",variable1_event);
  new_device(10);
  new_device(10);

  //read/write parameter
  printf("1: %i\n",read_param("variable1"));
  printf("2: %i\n",read_param("device0data"));
  printf("3: %i\n",read_param("device1data"));
  write_param("variable1",10);
  write_param("device0data",20);
  write_param("device1data",30);
  printf("4: %i\n",read_param("variable1"));
  printf("5: %i\n",read_param("device0data"));
  printf("6: %i\n",read_param("device1data"));
}

The output I would expect if this was working is:

variable1 event
1: -1
call to i2c_transmit (address 19, data 123)
2: 123
call to i2c_transmit (address 23, data 123)
3: 123
variable1 event
call to i2c_transmit (address 19, data 123)
call to i2c_transmit (address 23, data 123)
variable1 event
4: 10
call to i2c_transmit (address 19, data 20)
5: 20
call to i2c_transmit (address 23, data 30)
6: 30

but actually it doesn't even compile in this version:

device.h:40:120: error: invalid use of non-static member function ‘void Device::i2c_transmit()’
devices[new_index].data, "device"+std::to_string(new_index)+"data",  devices[new_index].i2c_transmit)

All the other ways I tried to pass the member function 'i2c_transmit()' to the constructor of 'class parameter' didn't work either and even though I often understand why, i've no idea HOW it works...

Is it trivial to create a local object, store a copy of this object to a global array and only work on this copy? I guess, thats what my above code does. I also tried to declare 'Device devices [MAX_DEVICES];' as static, but it didn't work. I tried using a lambda function, but also had no luck... It's hard to tell what else I have tried yet, but I think I have a problem in the general structure anyways. I'm open to new suggestions, but as 'class Parameter' is part of a library I'd like this class to not be changed!

Answer 1

void (*event)() is a badly designed callback. Callbacks in C-style code have both a function pointer and a void*, like this:

void (*event)(void*);
void* state;

Answer 2

You could add a parameter to the call back, as mentioned in the other answer. Another option is to eliminate the callbacks altogether and use OOP design. Create a wrapper class around an int, with virtual functions for get and set.

class Parameter {
  public:
    Parameter(String n, int default_value = 0) :
        name(n), value(default_value) {}

    // Virtual functions that can be overridden but have default functionality
    virtual int read() {
        return value;
    }
    virtual int write(int new_value) {
        return value = new_value;
    }

    String name;
  protected:
    int value;

    // Below are static functions and variables
    // ----
    // std lib is not supported on Arduino, so use an array 
    // Arduino programs are small enough that you should be
    // able to make an educated guess at MAX_PARAMETERS
    // Also note it is an array of pointers for virtual functions to work
    static Parameter *all_values[MAX_PARAMETERS];
    static int parameter_count;
  public:
    static bool add_param(Parameter *p) {
        if (parameter_count < MAX_PARAMETERS) {
            all_values[parameter_count++] = p;
            return true;
        }
        return false;
    }
    static Parameter * find_param(String name) {
         for (int i = 0; i < parameter_count; i++) {
             if (all_values[i]->name == name) return all_values[i];
         }
         return nullptr;
    }
};
Parameter * Parameter::all_values[MAX_PARAMETERS];
int Parameter::parameter_count = 0;

For the i2c parameters, you can extend this class

class Device : public Parameter
{
  protected:
     uint8_t address;
  public:
    Device(std::string n, uint8_t i2c_address) :
        address(i2c_address), Parameter(n) {}

    // Override these
    int read() {
        // Add your code here to read the value from the bus....
        return value;
    }
    int write(int new_value) {
        Parameter::write(new_value);
        // Add your code here to write the value to the bus....
        return value;
    }
};

To use it you could do something like:

// Create and add regular param
Parameter::add_param(new Parameter("test1"));
// Create and add i2c device param
Parameter::add_param(new Device("test2", 99));

Parameter::find_param("test1")->write(100);
int x = Parameter::find_param("test1")->read();

Parameter::find_param("test2")->write(123);
int y = Parameter::find_param("test2")->read();

// You can also use temp vars to simplify
Parameter *some_device = Parameter::find_param("test2");
some_device->write(100);
int z = some_device->read();

I know Arduinos are very limited on resources, so I would be interested to see how this code performs on the device.