Use unsigned long for representing times.

[edgar-bonet]

All the time calculations made on signed longs were wrong after 2^31 ms ~ 24.9 days. This patch fixes this by using unsigned longs. As unsigned numbers follow the rules of modular arithmetic, the computations are correct event after the millis() rollover in ~ 49.7 days.

Fixes issue #3: Improper use of signed integers.

[edgar-bonet]

This patch is untested. Here is what it does:

• All longs are turned into unsigned longs.
• Instead of using -1 to mean “now”, runned() and shouldRun() are overloaded with inline versions that take no parameters:

void runned() { runned(millis()); }
bool shouldRun() { return shouldRun(millis()); }

this generates smaller and faster code than the current version, because there is no need to test for the parameter being −1.

• The test for knowing whether it's time to run is a little bit more complex. Timestamps cannot be directly compared, because of the possible rollover. Durations (i.e. time differences), however, can be meaningfully compared. The test could be written

bool time_to_run = (signed long)(time - _cached_next_run) >= 0;

and it would work as expected if compiled with gcc. But since this is implementation-defined behavior, it should be avoided in portable code. Instead of casting to a signed number, the same result can be achieved by testing what would be the sign bit:

// If the "sign" bit is set the signed difference would be negative
bool time_remaining = (time - _cached_next_run) & 0x80000000;

which is as fast as a comparison. Then, shouldRun() returns !time_remaining && enabled.

I am not sure I will be able to test all this today. If you want to test yourself, on an AVR-based Arduino you can time-travel to just before the millis() rollover as follows:

extern unsigned long timer0_millis;

void setup() {
    timer0_millis = -1000;  // rollover in one second
    ...
}

On the Dues this does not work, because the equivalent variable is declared as static. You may make it work if you modify the Arduino core by removing the static qualifier from _dwTickCount in hardware/arduino/sam/system/libsam/source/timetick.c.

[edgar-bonet]

Update: I tested the patched library to see how it deals with the millis() rollover. It runs as expected, keeping a regular pace across the rollover. Here is my test program:

#include <Thread.h>
#include <ThreadController.h>

const int ledPin = 13;
extern unsigned long timer0_millis;

// First thread: toggle the LED every 200 ms.
void blink_led() {
    static bool ledState;
    ledState = !ledState;
    digitalWrite(ledPin, ledState);
}
Thread blink_thread(blink_led, 200);

// Second thread: print the current time every second.
void print_time() {
    Serial.println(millis());
}
Thread print_thread(print_time, 1000);

// Controller for both threads.
ThreadController scheduler;

void setup() {
    pinMode(ledPin, OUTPUT);
    Serial.begin(9600);
    Serial.println("\rStarting");
    scheduler.add(&blink_thread);
    scheduler.add(&print_thread);

    // Test across the millis() rollover.
    timer0_millis = -4300;  // time-travel to rollover minus 4.3 s
    blink_thread.run();     // update _cached_next_run
    print_thread.run();     // ditto
}

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

[ivanseidel]

Awesome! That is just perfect. And with 100% compatibility...

Will merge.

Thanks for the effort

[ivanseidel]

fixes #3