Using cached `time` value may slip off schedule if `run` function takes long time.

[CAHEK7]

In the code below when you cache time before the loop and thread[i]->run(); takes more time than interval to the next thread - the next thread will not be run in that run cycle.

25  unsigned long time = millis(); 
26  int checks = 0; 
27  for(int i = 0; i < MAX_THREADS && checks <= cached_size; i++){ 
28      // Object exists? Is enabled? Timeout exceeded? 
29      if(thread[i]){ 
30          checks++; 
31          if(thread[i]->shouldRun(time)){ 
32              thread[i]->run(); 
33          } 
34      } 
35  } 

This is not so critical for code like this:

void loop() {
  threadController.run();
}

because we run the next cycle right after the previous, but if run function attached to the timer interrupt we may miss whole timer cycle. This becomes more critical if the timer frequency is low.

I guess it would be better to replace if(thread[i]->shouldRun(time)) with thread[i]->shouldRun()) which is implied to call millis() each time.

What do you think?

[ivanseidel]

Not sure why I did like this, but perhaps to reduce millis() calls. I might have thought that that displacement in time could disorganise threads with time.

[CAHEK7]

Agreed that millis() can be heavy, but if you have 2 threads close to each other (for example we have 100ms and 110ms intervals) and the first thread's run function takes more than 10ms to execute, we will skip the second one each first cycle.

[ivanseidel]

True, but this can also happen (and with more frequency because Threads are supposed to run within a short a ammount of time, usually < 5ms): You create 10 threads with 100ms delay on each other. After some time, they get dessincronized by the time each one takes to run.

In the current way, if you add two threads that should run on a fixed period, they will always be executed in the same order, and in the same controller.run()