Trying to create n-time Timer

The constructor for the timer class has a ‘one_shot’ parameter allowing you to specify whether you want your Timer to fire once, or repeatedly until stop() is called.

In the following code I’m attempting to create an NTimesTimer, such that it will fire n-times and then stop/dispose itself:

class NTimesTimer {
public:
        NTimesTimer(int n, int delay, void (*callback)(void), void (*onComplete)(void));
        void start();
private:
        int _n;
        void (*_callback)(void);
        void (*_onComplete)(void);
        void _countdown();
        Timer _timer;
};

NTimesTimer::NTimesTimer(int n, int delay, void (*callback)(void), void (*onComplete)(void))
        : _timer(delay, &NTimesTimer::_countdown, *this) {
        _n = n;
        _callback = callback;
        _onComplete = onComplete;
}

void NTimesTimer::start() {
        _timer.start();
}

void NTimesTimer::_countdown() {
        if ( _n > 0 ) {
                _n = _n - 1;
                if ( _callback ) {
                        _callback();
                }
        } else if ( _onComplete ) {
                _onComplete();
                _timer.dispose();
        }
}

It mostly works except that I am not able to use it more than once. For example, this works:

NTimesTimer t =  NTimesTimer(5, 1000, _blink, _complete);
void _blink() {
    digitalWrite(D7, !digitalRead(D7));
}
void _complete() {
    Serial.println("complete!");
}
void setup() {
    t.start();  
}

However, if I attempt to do it like so, it does not:

void foo() {
    if (digitalRead(D1) == HIGH) {
        NTimesTimer t =  NTimesTimer(5, 1000, _blink, _complete);
        t.start();
    }
}

There may be something fairly obvious wrong with this, but my limited experience with C/C++ prohibits me from seeing it.

Any thoughts?

After you called _timer.dispose() your local Timer will be non-existent for any further use.
Either you don’t dispose the timer or you have to instantiate a Timer in start() each time anew.

If you go for the former, I’d suggest to also provide a destructor that does the disposing once your object dies.

@ScruffR, shouldn’t my foo function be instantiating a new NTimesTimer every time?

Wouldn’t the NTimesTimer object called “t” get destroyed once it goes out of scope (i.e. local within foo()).

1 Like

Yes, of course, I missed that :blush: I just read that bit: “It mostly works except that I am not able to use it more than once.” , and the class implementation.

So @BulldogLowell has got the point - I think.

How does it behave this way?

NTimesTimer* t;
void foo() {
    if (digitalRead(D1) == HIGH) {
        t = new NTimesTimer(5, 1000, _blink, _complete);
        t->start();
    }
}
1 Like

What puzzles me is the constructor definition. It looks like it calls _timer constructor as its base class. My C++ knowledge is bit rusty, so please correct me if I am wrong.

This is the suspicious part:

NTimesTimer::NTimesTimer(int n, int delay, void (*callback)(void), void (*onComplete)(void)) *: _timer(delay, &NTimesTimer::_countdown, this) {

That part of the timer declaration is most likely correct. I tested it when I posted this:

The part after the colon on a C++ constructor implementation can initialize the base class as well as any class member variables. And if the member variable is a class, and it has a constructor that takes parameters, you can set the constructor parameters there.

1 Like

When I use a pointer, it works the first time, but subsequent instantiations/starts don’t seem to have any effect.

Ok, so the issue I’m currently encountering is that I don’t think it is possible to call timer.dispose() from within a timer callback. It’s possible this is the source of my difficulties above.

Edit: Yup that was it. Once I moved the call to ->dispose() outside of the timer callback, it started working. Not ideal, but workable.

OK, I think I found a way to go.
First, as suggested earlier I added a destructor which does the disposing, and only stop the timer when finished.
Additionally I think there is sligtly flaw in the logic here

void NTimesTimer::_countdown() {
  if ( _n > 0 ) {
    _n = _n - 1;
    if ( _callback ) {
      _callback();
    }
  } else if ( _onComplete ) {
    _onComplete();
    _timer.dispose();  // <-- this should happen, even if `onComplete == NULL
  }
}

Try this one

#pragma SPARK_NO_PREPROCESSOR;
#include "Particle.h"

class NTimesTimer {
public:
    NTimesTimer(int n, int delay, void (*callback)(void), void (*onComplete)(void))
    : _timer(delay, &NTimesTimer::_countdown, *this) 
    {
        _n = n;
        _callback = callback;
        _onComplete = onComplete;
    }
    ~NTimesTimer()
    {
        _timer.dispose();
    }
    
    void start() { _timer.start(); }
private:
    Timer _timer;
    int _n;
    void (*_callback)(void);
    void (*_onComplete)(void);
    void _countdown()
    {
        if ( _n > 0 ) 
        {
            _n--;
            if ( _callback ) 
                _callback();
        } 
        else  
        {
            _timer.stop();
            if(_onComplete)
                _onComplete();
        }
    }
};

NTimesTimer* t; 
volatile bool retrigger = true;

void dmy()
{
    Serial.println(System.freeMemory());
}

void end()
{
    digitalWrite(D7, !digitalRead(D7));
    retrigger = true;
}

void setup()                    
{
  Serial1.begin(9600);            
  pinMode(D7, OUTPUT);
}

void loop()
{
    if(retrigger)
    {
        delete(t);
        retrigger = false;
        t = new NTimesTimer(5, 500, dmy, end);
        t->start();
    }
}

Ah good catch on the _timer.dispose() call.

In my current use case for the above class, I’ll only ever have one of them running at a single time. So I ended up reusing the same timer, and eliminating the need to dispose, etc.

Thanks for the help!

OK, but now above code works just fine.