Need help using a custom delay class

A mentor of mine made a library for non-blocking delays called fancy delay. This is great and all but I would like to use the ready boolean that is built in the class but my knowledge is not sufficient enough for this.

Here is the code,

class FancyDelay{
  unsigned long _period, _next;
  public:
  FancyDelay(unsigned long period) : _period(period) {
    _next = millis() + _period;
  }
  bool ready(){
    if(millis() >= _next){
      _next += _period;
      return true;
    }
    return false;
  }
};

Here is what I want to do with the class,

void loadingScreen(uint16_t textColor, uint16_t bckgndColor){
	drawtext((char *)"\n\n\n\n Now loading", textColor);
	FancyDelay(325);
	//Use of ready boolean here (I think?)
	drawtext((char *)"\n\n\n\n Now loading.", textColor);
	FancyDelay(325);
        //Use of ready boolean here (I think?)
	drawtext((char *)"\n\n\n\n Now loading..", textColor);
	FancyDelay(325);
        //Use of ready boolean here (I think?)
	drawtext((char *)"\n\n\n\n Now loading...", textColor);
	FancyDelay(325);
        //Use of ready boolean here (I think?)
	tft.fillScreen(bckgndColor);
	FancyDelay(325);
        //Use of ready boolean here (I think?)
	}
}

(deleted)

    _next = millis() + _period;

That is not how to use millis.

It will not work if millis overflows in the period.

spycatcher2k:
Thats not an arduino sketch - thats for processing, perhaps going the their forum!

Whandall:

    _next = millis() + _period;

That is not how to use millis.

It will not work if millis overflows in the period.

Appreciate the tips.
My mentor has clarified how to use this and has confirmed that it A) is in fact Arduino code and will work with arduino and that the line below is intentional and works properly.

_next = millis() + _period;

TrainDoc

Yes it works properly now. It just won't work 49 days from now.

'ready' is not a boolean; it's a function that returns a boolean.

I'm not a C++ programmer (C only) but it could look something like

  FancyDelay fd = FancyDelay(325);  // might require 'new' keyword or fd being a global variable. I don't know
  while(fd.ready()==false);

More experienced C++ programmers can comment on this.

Now the above is only to show you how to use the ready() method; the while() as shown will (obviously) block.

If you post the complete library (all files), I can try to play with it (for my learning experience and to try to help you).

traindoctor:
My mentor has clarified how to use this and has confirmed …
and that the line below is intentional and works properly.

_next = millis() + _period;

Your mentor is wrong.

Whandall:
Your mentor is wrong.

I will provide you with a functional example. I will now excuse myself from the thread.

sterretje:
'ready' is not a boolean; it's a function that returns a boolean.

I'm not a C++ programmer (C only) but it could look something like

  FancyDelay fd = FancyDelay(325);  // might require 'new' keyword or fd being a global variable. I don't know

while(fd.ready()==false);



More experienced C++ programmers can comment on this.

Now the above is only to show you how to use the ready() method; the while() as shown will (obviously) block.

If you post the complete library (all files), I can try to play with it (for my learning experience and to try to help you).

Correct, this is how it is used. (Variable fd does not have to be a global variable). My bad for not clarifying how it is was used after I said it had been provided to me. Here's the full library.

MorganS:
Yes it works properly now. It just won't work 49 days from now.

Good catch, I should probably add some additional logic to ready() to catch the edge case, for FancyDelays with long periods, this could result in a whole string of false positives, followed by a long blocking delay every 50 days. I have yet to have any of my Arduino programs run for more than a few days though, as I don't really do any long term sensing applications, and mostly active robot controls.

Whandall:
Your mentor is wrong.

...thanks for keeping me humble :wink:

unchapped:
Good catch, I should probably add some additional logic to ready() to catch the edge case...

That is the wrong approach. Do the test using SUBTRACTION rather than ADDITION, and it works perfectly, even when crossing the rollover. Don't ADD the delay to the current time, then check whether that time has passed. Instead SUBTRACT the current time from the start time of the interval, and compare that to the desired delay and you will ALWAYS get the correct result. No kludge code to deal with corner cases required, because there ARE no corner cases.

Regards,
Ray L.

traindoctor:
(Variable fd does not have to be a global variable).

fd should be global, or at least have persistent scope if you're using it inside of a function, i.e, you either need:

FancyDelay fd(325);

void loop()
  if(fd.ready()) {
      //do something every 325 ms
  }
}

or use the static keyword like so:

void loop() {
  static FancyDelay fd(325);
  if(fd.ready()) {
      //do something every 325 ms
  }
}

RayLivingston:
That is the wrong approach. Do the test using SUBTRACTION rather than ADDITION, and it works perfectly, even when crossing the rollover. Don't ADD the delay to the current time, then check whether that time has passed. Instead SUBTRACT the current time from the start time of the interval, and compare that to the desired delay and you will ALWAYS get the correct result. No kludge code to deal with corner cases required, because there ARE no corner cases.

Regards,
Ray L.

Thanks Ray, I initially had a fix working with subtraction of millis() from next, cast to a signed long, and then compared to 0, which accomplishes the same thing, but your suggestion is much more elegant.

Whandall:
Your mentor is wrong.

So, do you believe now that your mentor was wrong?

Have a look at how millis() is used to manage timing without blocking in Several things at a time

Seems to me much simpler than using fancy libraries that people don’t understand and which just wrap up the simple stuff to make it appear important.

…R

Robin2:
Seems to me much simpler than using fancy libraries…

Here in the US, the USDA allows marketers to use the work “fancy” to describe ketchup quality. I’d say that calling this class FancyDelay is the moral equivalent.

If you are going to call your class “Fancy” it ought to do something… well, fancy.

Personally, I believe that using a wrapper on millis() timers is very desirable approach. Why muck up your code with a bunch of millis() timers if you are performing a lot of timed events? It wreaks of opportunity for a decent class implementation.

@traindoctor, take a look at how simple this “blink without delay” sketch is simply using a reasonable class for timer elements using callbacks (albeit a difficult concept for beginners):

#include "MillisTimer.h"

MillisTimer blink(toggleLed, 100); // we set the function to run (toggleLed) and the interval at which it is run (100)

void setup() 
{
  pinMode(13, OUTPUT);
}

void loop() 
{
  MillisTimer::update();  // we call the update() function constantly in loop()
}

void toggleLed(void)
{
  digitalWrite(13, !digitalRead(13));
}

MillisTimer.h:

#ifndef MILLISTIMER_H
#define MILLISTIMER_H

#include "Arduino.h"

#define MAX_MILLIS_TIMER_INSTANCES 10

class MillisTimer{
  public:
    MillisTimer(void(*_callback)(void), uint32_t _period);
    ~MillisTimer();
    static void update(uint32_t nowMillis);
    static void update(void);
    
  private:
    void(*callback)(void);
    uint32_t lastMillis;
    uint32_t period;
    static MillisTimer* instance[MAX_MILLIS_TIMER_INSTANCES];
    static size_t num_Instances;
};

#endif

and MillisTimer.cpp:

#include "MillisTimer.h"

size_t MillisTimer::num_Instances = 0;
MillisTimer* MillisTimer::instance[MAX_MILLIS_TIMER_INSTANCES] = {NULL};

MillisTimer::MillisTimer(void(*_callback)(void), uint32_t _period)
{
  period = _period;
  callback = _callback;
  instance[num_Instances++] = this;
}

MillisTimer::~MillisTimer()
{
  MillisTimer* tempArray[num_Instances - 1];
  int index = 0;
  for(int i = 0; i < num_Instances; i++)
  {
    if(instance[i] != this)
    {
      tempArray[index++] = instance[i];
    }
  }
  for(int i = 0; i < num_Instances; i++)
  {
    if(i < num_Instances - 1)
    {
      instance[i] = tempArray[i];
    }
    else
    {
      instance[i] = NULL;
    }
  }
}

void MillisTimer::update(void)
{
  update(millis());
}

void MillisTimer::update(uint32_t nowMillis)
{
  for(int i = 0; i < MillisTimer::num_Instances; i++)
  {
    uint32_t currentMillis = nowMillis;
    if(nowMillis - instance[i]->lastMillis > instance[i]->period)
    {
      instance[i]->callback();
      instance[i]->lastMillis = currentMillis;
    }
  }
}

@Robin2,

Perhaps your tutorial shouldn’t stop short of introducing people to concepts around implementing a class to perform such mundane tasks.

BulldogLowell:
@Robin2,

Perhaps your tutorial shouldn't stop short of introducing people to concepts around implementing a class to perform such mundane tasks.

I would hate to deny you the opportunity of writing a tutorial about building a class. if you do I can then include a link to it.

Keep in mind, however, that the purpose of my tutorial was to show people how to do it - I specifically wanted to avoid creating a convenient black-box.

...R

Robin2:
I specifically wanted to avoid creating a convenient black-box.

Agreed, convenience sucks.