Passing function pointer down through multiple functions

[DreamingInsanity]

This may have been a little bit of a complicated project to start with especially since I knew nothing about C++ but I'm enjoying it (for the most part).

I've got another problem, with probably a simple fix, I just don't know how to. Due to the fact that I decided to support two different architectures for my program, the Bluetooth libraries I use differ significantly. One of the big differences is that one has built in callback, and the other doesn't. Because of the major differences I often create a class that will allow me to run a function that performs differently depending on the hardware.
Here's a dumbed down version of a bit of code I have:

typedef void (*Callback)(const char *data);

void _callback(const char *data) {
    Serial.println(data); //On setup, this will print "test1". It will never print "print me" however.
}

#ifdef A
class CallBack : public BLECharacteristicCallbacks
{
    Callback *x;

public:
    CallBack();

    CallBack(Callback *cb) {
        x = cb;
        (*x)("test1");
    };

    void onWrite() {
        (*x)("print me"); //Depending on how I pass down the function (ie. pointers, the object itself) this sometimes causes a crash.
    };
};
#else
class Callback {
.....
};
#endif

class Bluetooth
{
    Callback *x;

public:
    Bluetooth();
    
    void setup() {
        CallBack(x); //Do something with this
    };

    void createCallback(Callback *cb) {
        x = cb;
    };
};

class MainClass
{
    Bluetooth b;

public:
    MainClass();

    void setup()  {
        b.setup();
    };

    void setCallback(Callback cb) {
        b.createCallback(&cb);
    }
};

(The main class, in my program, is in a separate file).
It is run by firstly calling "setCallback". In this case, I pass in "_callback" as the argument. This sets the "x" variable of Bluetooth to a pointer to that function.
"setup" is then called, which calls Bluetooth's setup function, which in turn creates an instance of the Callback class, with the function pointer as a argument.

In the constructor of the Callback class I call that function with "test". When looking at the console I see "test". However, once onWrite is called (which is called when Bluetooth receives data), it will either do nothing / crash the program.
I read somewhere that once a function has returned, the pointer to that function becomes invalid or something along those lines. If that's the case then once the Main class "setup" function has returned, the pointer to "_callback" is no longer going to be valid which is why when "onWrite" is called, it crashes or does nothing.

What's a better way of doing this?

[markyrocks]

Whats a better way of doing this....Id say start by learning the basics of setting up a class and methods. Then learn about calling said methods etc. This is just a mess. Since you don't have a main function posted its impossible to tell the order in which things are happening. Naming everything callback is a terrible idea. Thanks for posting the dumbed down version, I could only imagine the insanity that is happening in the more complicated version.

[helios]

Here's the problem: the type Callback is already a function pointer. The type Callback * is not a function pointer, it's a data pointer. Specifically, it's a pointer to a function pointer.
Simply put, every time you wrote Callback * you should have written Callback:

class CallBack : public BLECharacteristicCallbacks
{
    Callback x;

public:
    CallBack();

    CallBack(Callback cb) {
        x = cb;
        x("test1");
    };

    void onWrite() {
        x("print me");
    };
};

//...

class Bluetooth
{
    Callback x;

public:
    Bluetooth();
    
    void setup() {
        CallBack(x); //Do something with this
    };

    void createCallback(Callback cb) {
        x = cb;
    };
};

class MainClass
{
    Bluetooth b;

public:
    MainClass();

    void setup()  {
        b.setup();
    };

    void setCallback(Callback cb) {
        b.createCallback(cb);
    }
};

Also, avoid names that differ only in the case of the letters. "Callback" and "CallBack" are far too easy to confuse. A better name for the CallBack class might be, I don't know, WriteFunctor or something.

[DreamingInsanity]

I called it "dumbed down" because I stripped away the unnecessary Bluetooth which adds nothing to the problem. Taking away the Bluetooth code gives code almost the same as the example above.
This is the problem as close to the source as possible:
On lots of MCU's Bluetooth is not built in. Instead you use a separate module which transmits data through serial communication. Anyway, to read it, you must do something like this:

while (ble.available() > 0)
{
    data += (char)ble.read();
}

It's a simple as you read it byte by byte from a buffer. Then, to implement a callback I can just call a function with "data" as an argument.
For my other MCU, it has built in Bluetooth. To add a callback it's as simple as:

pCharacteristic->setCallbacks(Callback());

Where "Callback" is a class that extends "BLECharacteristicCallbacks". The class has an "onWrite" function which is called when it receives data. After that, I can then call any other callback with the data.
If I weren't to include the option to allow the user to change the callback this would be easy but it would be a nice feature.

I'm sure the basics of classes and methods will allow me to improve my code but I still feel I'll end up in a similar predicament. I need the "Bluetooth" class so I can create methods like a "setup" method that runs different code depending on the MCU, and to keep Bluetooth code away from the main class. I also need a callback class to pass to the Bluetooth code. So, either way, I still have two extra classes which means I will still have a similar problem, right? (unless I moved everything into one file just to make a global variable)

[markyrocks]

I edited this to more closely resemble what you're after. Not sure if its the answer to your problems but it is functional aswell..

#include <Windows.h>
#include<iostream>
using namespace std;

#define Callback(x) void(*x)()


void _callbackA() {
    cout << "callbackA" << "\n";
}

void _callbackB() {
    cout << "callbackB" << "\n";
}


class CallBack 
{
    

public:
    CallBack() {}

    Callback(x);

    CallBack(Callback(cb)) {
        x = cb;
    };

    void onWrite() {
      
    };
};


class Bluetooth
{
    Callback(x);

public:
    Bluetooth() {}

    void setup(int t) {
        if (t == 1) {
            x=_callbackA;
        }
        else {
            x=_callbackB;
        }
       (*x)();
    }

};

class MainClass
{
    Bluetooth b{};

public:
    MainClass() {}

    void setup(int bb) {
        b.setup(bb);
    };

};


int main() {

    MainClass m{};
    m.setup(2);

    CallBack cb(_callbackA);

    cb.x();
}

[DreamingInsanity]

Thanks - that gave me an idea to just try passing down the function object not a pointer. I did say originally that I tried it but I guess I didn't because it works now!

[JLBorges]

Avoid identifiers that begin with an underscore (like __callback above) in the global namespace.

In addition, some identifiers are reserved for use by C++ implementations and shall not be used otherwise; no diagnostic is required.

Each identifier that contains a double underscore __ or begins with an underscore followed by an uppercase letter is reserved to the implementation for any use.

Each identifier that begins with an underscore is reserved to the implementation for use as a name in the global namespace.
https://eel.is/c++draft/lex.name#3

Using the standard polymorphic call wrapper std::function<> https://en.cppreference.com/w/cpp/utility/functional/function
would offer a great deal more flexibility.

For example:

#include <iostream>
#include <functional>

struct blue_tooth
{
    using on_data_arrived_callback = std::function< void( const char* ) > ;
    on_data_arrived_callback callback_fn ;

    template < typename CALLABLE >
    explicit blue_tooth( CALLABLE&& cb_fn ) : callback_fn( std::move(cb_fn) ) {}

    template < typename CALLABLE >
    void set_callback( CALLABLE&& cb_fn ) { callback_fn = std::move(cb_fn) ; }

    void on_data_arrived( const char* data ) { callback_fn(data) ; }
};

void foo( const char* cstr ) { std::cout << "foo( " << cstr << " )\n" ; }

struct XXX { int bar( const char* cstr ) { std::cout << "XXX::bar( " << cstr << " )\n" ; return 5 ; } };

int main()
{
    blue_tooth bt(foo) ; // set call back to a free function
    bt.on_data_arrived( "data - #one" ) ; // foo( data - #one )

    // set call back to a bound member function
    XXX x ;
    bt.set_callback( std::bind( &XXX::bar, std::ref(x), std::placeholders::_1 ) ) ;
    bt.on_data_arrived( "more data - #two" ) ; // XXX::bar( more data - #two )

    // set call back to a closure object
    bt.set_callback( []( const char* cstr ) { std::cout << "a_closure_object( " << cstr << " )\n" ; } ) ;
    bt.on_data_arrived( "even more data - #three" ) ; // a_closure_object( even more data - #three )
}

http://coliru.stacked-crooked.com/a/83574ccac4105b6b
https://rextester.com/PMJZ45863

[dhayden]

You have Callback CallBack and _callback. No wonder you can't keep it straight :)

Looking at your code:

typedef void (*Callback)(const char *data);   //Callback is a pointer to a function

void _callback(const char *data) {
    Serial.println(data); //On setup, this will print "test1". It will never print "print me" however.
}

#ifdef A
class CallBack : public BLECharacteristicCallbacks
{
    Callback *x;   // x is a pointer to a pointer to a function.

public:
    CallBack();

    CallBack(Callback *cb) {
        x = cb;    // x points to what cb points to
        (*x)("test1");
    };

    void onWrite() {
        (*x)("print me"); //Depending on how I pass down the function (ie. pointers, the object itself) this sometimes causes a crash.
    };
};
#else
class Callback {
.....
};
#endif

class Bluetooth
{
    Callback *x;

public:
    Bluetooth();
    
    void setup() {
        CallBack(x); //Do something with this.
    };

    void createCallback(Callback *cb) {
        x = cb;     // x points to what cb pointer to.
    };
};

class MainClass
{
    Bluetooth b;

public:
    MainClass();

    void setup()  {
        b.setup();
    };

    void setCallback(Callback cb) {
        b.createCallback(&cb);   // NOW B.x POINTS TO THE PARAMETER cb, WHICH IMMEDIATELY GOES OUT OF SCOPE
    }
};