gioblu / agenda Goto Github PK

I was looking some scheduler libraries and I found this nice small and simple scheduler library.
When looking the Agenda's code, I noticed that Agenda::update() method contains one nasty bug which will lead to stack overflow and totally errorneus behaviour in some cases.

If someone use Agenda::delay() or Agenda::delay_microseconds() methods in any task method, it will lead stack overflow. This could be prevented if task_struct would have some flag to indicate that task is currently running then Agenda::update() method would know that it can't execute same task multiple times.

Bug can be triggered following code:

#include <Agenda.h>

Agenda sched;

void some_task() {
    sched.delay(100);
}
void setup() {
    sched.insert(some_task,1000);
}

void loop() {
    sched.update();
}

Call path will be something like:
loop():
Agenda::update():
some_task():
Agenda::delay():
Agenda::update():
some_task():
Agenda::delay():
Agenda::update():
some_task():
Agenda::delay():
Agenda::update():
... and so on until crash by stack overflow

First of all thanks for sharing your project. I'm going to use it for my first projects with the arduino.
But I think there is a litte bug in this function:

void Agenda::delay(unsigned long delay_time) {
  unsigned long time = micros();
  while(micros() - time > delay_time)
    this->update();
}

This doesn't work like expected. There is no delay ('>' should be '<'). Furthermore it's a good idea to realize two delay-functions (ms and us). Possible solution:

void Agenda::delay_us(unsigned long delay_time_us) {
  unsigned long time = micros();
  while(micros() - time < delay_time_us) {
    this->update();
    yield();  // give time to other tasks
  }
}

void Agenda::delay_ms(unsigned long delay_time_ms) {
  unsigned long time = millis();

  while(millis() - time < delay_time_ms) {
    this->update();
    yield();  // give time to other tasks
  }
}

Greez lucky

As title says fill category with a correct value!