Class method callback issues: class vars changed in callback seem isolated?

Hello all,

First of all thanks a lot for taking the time to read my post. I have read the FAQs and the “Before you ask” post of this forum, and I’ll give my best to adhere to the guidelines. If I miss out on something I promise that it was not intentional.

I have a question relating to class method callback in a project (which is actually running on an ESP8266, so not strictly speaking Arduino. I really hope this is OK and that I am not violating the rules by posting here).

To give you a bit of context that may make the problem clearer: the project in question is a library I’m writing that allows to set certain effects to a LED, such as fading, blinking, etc. The whole thing is set up using a strategy design pattern: there is an abstract base class for each LED effect, and then concrete effects classes that implement the former.

In my main sketch, I create an instance of my LED library. The different strategies are then set from WITHIN the library (this might be important to know for what follows). Ie the main sketch will of course call a method on the LED library class, similar to setEffect(MYEFFECT), but the actual strategy switching happens in the library itself. In other words, the different strategies are NOT created in the main sketch and then passed over to the LED library, but the LED library handles this itself.

Now, sometimes I want to know whether one effect has completed a whole “run”: for instance, if the chosen effect is a fade-in effect, I want to know when the LEDs have been fully faded in from their start to their end luminosity. To practice my C++ skills, I wanted to implement this using a callback!

So here’s the more meaty part: the individual effects classes (strategies) for the LEDs have a method to set a callback, and this callback is called whenever the effect has completed one round. This means that in practice, the callback that my library sets, is actually a class method. The callback gets executed correctly after each round when I test it, so that in itself is working. Also, FYI, the callback takes no arguments - it simply signals that one round of the effect in question has completed.

The problem is, that within the callback, some class variables of my LED library are supposed to be modified. Take a counter for an example. If the counter is initialised with 0, the callback is supposed to increase it at every round. Now, if I print the old and new value from within the callback, I can see that it’s increasing. However, when other code from outside the callback uses the counter variable, it stays stuck at whatever value it was before the callbacks! So it feels like I’m essentially not operating on the same instance of my class, even though I passed a pointer to “this” as part of the callback. I hope the following code segments may help you to understand the issue.

The relevant code of the abstract base pattern that contains the code for the setting and calling of the callback is the following:

Extract from effect CPP file:

uint8_t Pattern::cycle(FairyState &fstate) {
  // This method is called when the effect has completed a whole round.
  fstate.cycle++;
  if(cycle_callback != NULL) cycle_callback();    // Call the callback, if defined
  return fstate.cycle;
}

void Pattern::setCycleCallback(const CycleCallback &callback) {
  // Sets the callback function
  this->cycle_callback = callback;
}

Extract from effect header file:

typedef std::function<void ()> CycleCallback;

class Pattern
{
public:
 ...
 void setCycleCallback(const CycleCallback &callback);
protected:
 ...
 uint8_t cycle(FairyState &fstate);
 CycleCallback cycle_callback;
};

The above are parts of the effects class. In my library, I have the following method that is passed as callback to the effects class:

void FairyLights::cycleCallback() {
  // This is a callback that gets calle whenever a pattern has finished, but is only relevant if we are in mix mode to cycle through patterns.
  // Note that fstate is a class variable (struct)
  if(fstate.mode == MIX) {
    this->fstate.mix_mode++;
  }
}

And this is the way the callback is set from my library:

myEffectClass.setCycleCallback(std::bind(&FairyLights::cycleCallback, *this));

As I said, if I print fstate.mix_mode from within the callback, I can see it increasing correctly. But if I access it from anywhere else in my code, it remains unchanged.

I have also tried setting the callback using only “this” (so not “*this”) as the first argument in the std::bind function, but then fstate.mix_mode does not change at all, even from within the callback.

Then I noticed that std::bind actually always copies the arguments, even if a pointer or reference is passed. Hence, I changed it to the following:

myEffectClass.setCycleCallback(std::bind(&FairyLights::cycleCallback, std::ref(*this)));

However, the effect remains the same.

I also tried setting the callback using a lambda function like so:

myEffectClass.setCycleCallback([this] () { this->cycleCallback(); });

However, that doesn’t work either.

Someone gave me a hint I might need to use the volatile keyword for the variables that can be changed by the callback, but I’m not quite sure this is correct. Should the compiler not notice that it can’t optimise away the variables in my case?

So, it feels like I’m quite close, but at the same time I have no clue what else to try. I thought by passing a pointer to “this” I could be sure that I’m referring to the correct (and only) instance of my library?

I would be very grateful for any help! And please forgive me if I made any obvious mistakes - I consider myself very much a beginner when it comes to C++.

Thanks a bunch!

It's difficult, for me anyway, to debug from snippets -- especially someone else's code. Since I have a feeling that you whole code is long and perhaps convoluted, I'd suggest creating an MCVE. This is the smallest, simplest, COMPLETE, code that compiles and demonstrates the problem at hand. It doesn't even need to look like your full code, just demonstrate the same problem. I'd also suggest accompanying that with a short, concise description of the problem.

EDIT:
Also, post you MCVE as a single complete file. Don't break it into .h, .cpp, .ino files since we'd just have to put all those together again to compile it. Just have your class declaration at the top, followed by class definitions (implementations), then object instantiation, then use by setup() / loop().

Edit #2:
This might be helpful: Standard C++

Thanks for your reply gfvalvo, I just wanted to let you know that I will create an MCVE illustrating my issue and post it here by tomorrow or the day after tomorrow since that is when I will have time to continue working on this.

Thanks again!

So, plot twist!

I tried to recreate my issue with a minimal working code example and guess what - it works as expected!

Here is the code (copy-pasted since it is short) for the .ino sketch:

#include <functional>

typedef std::function<void ()> MyCallback;

typedef struct MyStruct
{
 uint8_t counter;
};

class Beta
{
    protected:
        MyCallback callback;
    public:
        void setCallback(const MyCallback &callback) {
            this->callback = callback;
        }
        void doCallback() {
            callback();
        }
};

class Alpha
{
    public:
        MyStruct mystruct;
        Beta beta;
        
        void setCounter(uint8_t value) {
            mystruct.counter = value;
        }
        
        uint8_t getCounter() {
            return mystruct.counter;
        }
        
        MyStruct getStruct() {
            return mystruct;
        }
        
        void Callback() {
            mystruct.counter++;
        }
        
        void setCallback() {
            beta.setCallback(std::bind(&Alpha::Callback, std::ref(*this)));
        }
        
        void doCallback() {
            beta.doCallback();
        }
};

Alpha alpha;

void setup() {
  // put your setup code here, to run once:
  alpha.setCounter(0);
  alpha.setCallback();
  
  Serial.begin(115200);
  Serial.println("Set counter to: " + alpha.getCounter());
  
}

void loop() {
  // put your main code here, to run repeatedly:
  alpha.doCallback();
  Serial.print("New value: ");
  Serial.println(alpha.getCounter());
  delay(1000);
}

The main characteristics of my LED library are reproduced: a class Beta encapsulated in a class Alpha. Alpha sets a callback to a member function of itself (Alpha) in Beta. From the main sketch, an event is triggered to launch the callback in Beta. In Beta, the callback, which is as explained a member function of Alpha, is called. This member function of Alpha alters a class variable of Alpha.
As you can see when running the sketch, I call the callback every second and the variable which is changed increments correctly when accessed out of the main sketch.

I’m actually quite happy because I considered this callback part quite complicated, but it seems that my error must lie somewhere else (I have a few ideas …). I just still don’t know why, in my “real” code, the class variable increments correctly within the callback, but stays fixed outside of it. I’ll have to go over it with a blank mind I guess and come back to the forum once I narrowed it down more.

In any case, thanks for reading and for your help!

Now, if I print the old and new value from within the callback, I can see that it's increasing. However, when other code from outside the callback uses the counter variable, it stays stuck at whatever value it was before the callbacks!

Sounds like a compiler optimization problem. Most people here run into them with interrupts, but those are just a special case of callback functions so it's the same concept.

The problem is most likely because your other function thinks the value of the variable won't change, so it's optimizing away access to it and simply using a cached value that it never updates. Mark the counter variable volatile. That will prevent the compiler from optimizing away accesses.

Thanks for all your answers.

I have found the solution!

The problem was not at all how I implemented the callback itself. But due to a bad design decision (which I have rectified now), I was overwriting the values updated by the callback in another part of a method that was also run at every update of the LEDs, but after each potential callback.

Thanks to everyone who took the time to read my post and answer, very much appreciated!