Is it possible to pass a member function pointer to setDioXAction?

[sergimn]

I'm trying to develop a C++ class that uses RadioLib. Once I try to specify what function should be called when new data is received with radio->setDio0Action(&this->onReceive);, the compilation error error: invalid use of non-static member function appears. I've tried the following lines without success, each with a slightly different error message:

radio->setDio0Action(&MyClass::onReceive);
radio->setDio0Action(MyClass::onReceive);
radio->setDio0Action(onReceive);

Is there a way to, when a packet is received, access the private variables of an instance? The instance has data such as nº of packets received or the node ID, something that is not compatible with making the method static as some people may suggest.

I found this example and this stackoverflow answer and both indicate that the underlying code has to has support for an extra parameter (the instance we want to get the data from).

In case what I'm asking needs a rework of the code, do you think it should be easy to do? I'm currently using a ESP32 with a SX127x module. Do you know if the underlying hardware supports passing a parameter to the interruption handler?

[jgromes]

The short answer is that you can't do this directly, but it's pretty easy to do indirectly.

Background: Take a look at the definition of setDio0Action method, e.g. in SX127x class:

    /*!
      \brief Set interrupt service routine function to call when DIO0 activates.

      \param func Pointer to interrupt service routine.
    */
    void setDio0Action(void (*func)(void));

You can see that the method expects the argument to be a pointer to void-type function in the global scope taking no arguments, such as

void func(void);

Your class method wil have a different signature however, it will be:

void MyClass::func(void);

That's the reason why your compilation fails in the first and fourth example. Moreover, there's a fundamental issue with examples 2 and 3: assuming MyClass::onReceive isn't static, you can't call it as MyClass::onReceive() - how would the compiler know which instance of MyClass to call?

Solution:

You'd have to create your own setDio0Action() in RadioLib, one that will have the expected signature of the MyClass::onReceive() method. That would of course mean exposing MyClass definition to RadioLib. Maybe you could do some pretty evil void casting to match setDio0Action() definition, but that doesn't seem very clean

However, there's a simpler solution - does setDio0Action() have to call your method directly? You can easily create a wrapper function in the global scope that would call the instance of MyClass you intend to call, e.g.:

MyClass mine;

void callMyClass(void) {
  mine.onReceive();
}

(...)

radio.setDio0Action(callMyClass);

[sergimn]

That's actually a smart solution, thanks for pointing it out. However I forgot to mention that the class is part (well, the only part) of a library that will act as a wrapper to RadioLib and even though I could tell the end user to interact directly with the radio object (that I hoped I could hide putting it in the private section of the class) i think it's a bad design strategy. If I try to put the callMyClass function on the main.c and the radio.setDio0Action(callMyClass); on the constructor method (or any other method, anyways), callMyClass is not on scope so it's not going to compile.

I've tried to look further on the internet and I found another similar question with a more specific answer to the two I provided before but my C++ syntax knowledge is not enough to fully understand the solution and why it works but I still have the feeling that without passing the object, it's not going to work. Note though, that I tried the code and it does work.

I'm starting to think that what I'm looking for is impossible by design of C++, do you suggest something that I can look into it? As of right now the solution with most probability of success that I can think about is just to make it C code or a static class and not let users have multiple instances of it, which is not ideal, but I don't think lots of people would create two instances of my class anyways. Do you know of other projects that use RadioLib that could have taken some similar approach?

[jgromes]

The stackoverflow answer you linked to is the evil void casting I mentioned. Essentially, you pass void* - you say it's a pointer to "something" but you don't define what type that something is: it can be an integer, or it can be your class, it doesn't matter. Once inside the ISR, you then cast that void pointer to the type you want it to be. However, there's nothing stopping you from casting it into the wrong type, which will yield incorrect data in the best case and some hard fault in the worst (and more likely) scenario. This is a very C way of doing things, the disadvantage is that you can cast void* to anything, so essentially you bypass C++ type safety.

There's a couple projects that have some sort of wrapper around RadioLib, e.g. RFQuack, tinyGS or Meshtastic, but I'm not that familiar with them to tell if it's going to be useful.

This is definitely doable from C++. I think we can safely add a more advanced setDio0Action method that would allow users to specify void* data, but the one with no parameters should remain as well and be the default.

[sergimn]

I think I have something. It's still untested but I already submitted the pull request (#290 ) for ease of code discussion. If it ends up not working we can just close it.

[Jaimi5]

I am currently working with the @sergimn implementation, continuing a LoRaMesh Library LoRaMesher. I am stuck with this problem and how to remove the fork that Sergi did in the past.
After seeing how other projects Meshtastic, tinyGS and disaster.radio they use a global variable at some point of their program.

I do not really like to use a global variable because, in some point, other users could break the library.

I have found some library that works in AVR, for example functional-avr Could there be a possibility to implement the previous pull request with these fixes for the AVR devices?
I think using functional (or maybe another solution I cannot find) will be a good idea for future implementations that uses RadioLib.

Thanks for your time!

[jgromes]

Couple of thoughts on that: first of all, RadioLib runs on more than just AVR devices. Relying on external library that only supports AVR is not an option.

Second, I don't think anything is forcing you to use a global variable. You just have to find a way to specify which instance should be affected by the interrupt. How you implement it is up to you, it doesn't have to use a global variable (though it happens to be the easiest solution, and I agree it is somewhat ugly). For example, you could have some logic which determines the instance you want to call:

MyClass* getInstance() {
  // returns a pointer to the instance you want to call, based on some internal logic
}

void callSomeClass(void) {
  MyClass* mine = getInstance();
  mine->onReceive();
}

(...)

radio.setDio0Action(callSomeClass);

[Jaimi5]

Thanks for the response!

Finally I could find a solution using a private function and an instance as you said (in this case a static instance).

The final code is like this:

//.h
class LoraMesher {
public:
static LoraMesher& getInstance() {
static LoraMesher instance;
    return instance;
}

private:
static void onReceive(void);
};

//.cpp
void IRAM_ATTR LoraMesher::onReceive() {
  BaseType_t xHigherPriorityTaskWoken = pdFALSE;

  xHigherPriorityTaskWoken = xTaskNotifyFromISR(
    LoraMesher::getInstance().ReceivePacket_TaskHandle,
    0,
    eSetValueWithoutOverwrite,
    &xHigherPriorityTaskWoken);

  if (xHigherPriorityTaskWoken == pdTRUE)
    portYIELD_FROM_ISR();
}

radio->setDio0Action(onReceive);

Hope this is well written and could help other peoples too!

PD: Thanks for your support @jgromes, without your library this project could not be what is it today!

[sergimn]

This might be an interesting read, even thought this is already an old question and it does not really satisfy the question asked as the article basically says to not do it

[Jaimi5]

Then is the only solution to have a global variable and pray for the users to not use it?

[Pablo2048]

Hi guys,
I have few things to say:

1. <functional> is standard C++ library, present in some 32 bits Core (ESP8266, ESP32, STM32 and probably others), unfortunately is not present in AVR core, but:
2. I can maybe offer another solution. Let's say we have helper class Executable:

class Executable
{
public:
   virtual void exec(void) = 0;
   virtual ~Executable() = default;
};

radio.setDioAction() can accept pointer to the Executable class, and when is the time for callback, then it calls exec(). Then we can have our MyClass (inherited from Executable) like this:

class MyClass : public Executable
{
public:
  void exec(void) override {
... code to handle ....
}
};

... and finally we can use it like this

MyClass mine;

...

radio.setDio0Action(mine);

[Pablo2048]

I overlooked that @sergimn is on ESP32 - so the simplest solution probably is radio.setDio0Action(std::bind(&MyClass:onReceive, mine));