Timer drift over longer runtime periods

[bizprof]

Encountered a timer drift issue with BlockNot, which only manifests itself over longer periods of runtime, depending on the board used and its speed. This has nothing to do with the accuracy (or lack thereof) of the internal clock, but with the way timers are reset.

When running the ResetAll.ino example over longer periods of time, the timers slowly drift off their set intervals, compared to the millis()-based timestamp running in parallel.

In the a.m. example, the myReset timer should trigger every 30 seconds exactly. In reality, it is taking a couple milliseconds longer for each run, as the CPU is processing the hasTriggered() and reset() functions, plus some other code in between resets. Over time, this adds up to whole seconds. With an Arduino UNO, it is noticeable after about 2 hours of runtime.

This might be undesirable for some projects, and it also creates problems when using multiple dependent timers in the same script, as the shorter timers drift off faster than the longer ones.

Proposed solution is to reset timer start times not to the current internal millis() value, but to what it should have been without the additional CPU cycles. Will create a PR for that.

[EasyG0ing1]

@bizprof Thanks, It might be an issue of offsetting the time it takes to perform the various tasks in which case it's going to be completely dependent on the processor, but perhaps I can build in an offset adjuster based on the speed of the CPU...

Stay tuned ...

[EasyG0ing1]

@bizprof Also looking forward to your PR as I continue looking at this.

[bizprof]

Thanks for looking into this, Mike! Will work on the PR today, have to first see what changes you made for the new release. Still early morning here in the UK, should get it done today, hopefully. My approach is to a) calculate the intended start time of a timer when it can be known, to eliminate the drift regardless of CPU speed, and b) to synchronise the start times in resetAllTimers(). While keeping the code backwards-compatible. There would still be a drift in some constellations, but it wouldn't compound itself over time. So, hopefully a compromise we can live with.

[EasyG0ing1]

@bizprof Michael,

All I did in the update was remove the BlockNot(bool type) {} class instantiation construct because for some reason, it was causing problems when you instantiated the class with just the time alone. You could overcome the issue by making sure the time value you passed into the construct was an unsigned long ( 3400UL for example), but it was causing confusion and an issue was created over the problem so I removed that construct. I also modified your resetTimers example so that it is ... simpler ... we can always add another one if you feel it's necessary, but the examples really just need to show how something works and I felt that simplifying it would help more basic users.

I also renamed type to baseUnits

[EasyG0ing1]

@bizprof

On the time drift issue, do you think something like this might help the problem?

in public:

    void resetForAll() {
        startTime = millis();
    }

and then modify:

void resetAllTimers() {
    BlockNot* timer = BlockNot::firstTimer;
    while( timer != nullptr ) {
        timer->BlockNot::resetForAll();
        timer = timer->BlockNot::getNextTimer();
    }
}

???

[EasyG0ing1]

@bizprof

Or perhaps something like this:

in public:

    void setForAll(unsigned long newTime) {
        startTime = newTime;
    }

then modify:

void resetAllTimers() {
    unsigned long now = millis();
    BlockNot* timer = BlockNot::firstTimer;
    while( timer != nullptr ) {
        timer->BlockNot::setForAll(now);
        timer = timer->BlockNot::getNextTimer();
    }
}

[bizprof]

I've been thinking to add a non-mandatory parameter to the reset() and resetAllTimers() functions, which will default to millis(), so nothing changes for existing scripts. But it can be set to a calculated new start time when a timer gets reset by the triggered(...) function:

bool triggered(bool restartOption = true) {
    bool triggered = hasTriggered();
    if (restartOption && triggered) {
        reset( startTime + duration );
    }
    return triggered;
}

And the reset() would turn into:

void reset( const unsigned long newStartTime = millis() ) { resetTimer( newStartTime ); }

Similar approach for resetAllTimers, where all timers will be set to the same new start time (just like you proposed in your last comment):

void resetAllTimers( const unsigned long newStartTime = millis() ) { BlockNot* timer = BlockNot::firstTimer; while( timer != nullptr ) { timer->BlockNot::reset( newStartTime ); timer = timer->BlockNot::getNextTimer(); } }

[EasyG0ing1]

@bizprof

I had actyally already considered adding a different hasTriggered method so that it only adds the current duration to the current startTime. But I quickly realized that would do nothing about the time drift you are seeing currently since it has no dependence on the hasTriggered method.

The reason why I don't think that adding duration to startTime should be a universal thing for all calls to TRIGGERED is because there could be times when code calls TRIGGERED at some time long after the duration has actually passed and it might be necessary to simply reset the timer to trigger again at current millis() + duration. So I think it then becomes necessary to have a separate method to reset the timer so that the new startTime is the current startTime plus the set duration ... like this:

    bool triggeredWithPrecision() {
        bool triggered = hasTriggered();
        if (triggered) startTime += duration;
        return triggered;
    }

And a macro could be created to call that method ... something like TRIGGERED_P

But that still does nothing about the drift you are seeing ...

Tell me what the difference is in your new resetAllTimers as opposed to the one I proposed?

[EasyG0ing1]

@bizprof

OK, I think maybe this will fix the drift problem ... queing from your proposed change:

Change the following methods only:

public:

    void reset(const unsigned long newStartTime = millis()) {
        resetTimer(newStartTime); 
    }

private:

    void resetTimer(const unsigned long newStartTime) {
        if (enabled) {
            startTime = newStartTime;
            onceTriggered = false;
        }
    }

external:

void resetAllTimers(const unsigned long newStartTime = millis()) {
    BlockNot* timer = BlockNot::firstTimer;
    while( timer != nullptr ) {
        timer->BlockNot::reset(newStartTime);
        timer = timer->BlockNot::getNextTimer();
    }
}

I'm not entirely sure what declaring the method argument as constant actually does in terms of helping things in this context, so I'm not sure if that should be done to all of the methods in similar fashion. Also, I can make these changes and push right away ... I might also include the new hasTriggered ... what are your thoughts?

[bizprof]

Agree on that a timer can potentially be triggered long after it was due. E.g., if there is a delay() in the script, or a wait for an external event etc. Maybe have separate reset() and restart() functions to address this. From my use case, a drift per se is absolutely fine, as long as it does not compound over time.

The difference between the resetAllTimers() proposals is that I am thinking of including a parameter newStartTime that would be passed through to the individual reset() calls. Will send over my PR in a couple minutes, maybe that makes it a bit clearer...

[EasyG0ing1]

@bizprof

No need for a PR ... look at my last post here, I think we're on the same page.

[bizprof]

Sorry didn't see your last comment until after I created the PR. Just ignore it, if you make similar changes. Have included some cosmetic changes (eliminate repetitive code, add function getStartTime(), moved setNextTimer() to private section.

Also changed the ResetAll example, it had a compiler warning. Plus added the parameter to the resetAllTimers call. That is now showing no compounding timer drift, which is what we wanted.

The const addition to a function argument tells the compiler that this is a variable that won't be changed during the call, which allows for some optimisation, it's just good coding practice, nothing else.

[EasyG0ing1]

OK, I'm checking it out now. As I'm sure you've realized, C++ is not my forte. I have spent my working career as a network engineer, but writing code has been a hobby of mine since I was 12 back in the Commodore 64 days and I've always done coding in that capacity even throughout my network engineering career. I've only recently taken coding more seriously and I started doing that with Java (though I consider my Java skills to be somewhere between intermediate to advanced - heavily on the intermediate side). I have issues with C++ - mainly I don't think in binary nor hexadecimal and that seems to be a requirement if you really want to master the language, and second ... things like the issue we had with the constructs for instantiating BlockNot where one of them passed in a single boolean as the argument ... in Java, it would not have been an issue instantiating the class with a numeric value as Java would have known the difference between an argument give of TRUE vs 1000 ... but here, it got confused and so I had to remove the construct ... C++ is very rudimentary and rigid and I'm not that use to it nor do I like it very much.

Can't wait for the day when all microcontrollers will be at least as powerful as the current Raspberry Pi Pico and at the $4 price but where we can code in different languages without any performance hits. I know very little about Python ... but it is very popular.

In that vein of my lack of knowledge about C++, I appreciate your efforts with this project and your patience with me.

:-)

Mike

[bizprof]

Thanks for your feedback, Mike, very much appreciated!

I've been in IT for way too long, and never thought I'd take up coding ever again. But then I came across a Nixie clock project on https://nixietester.com/shop/, which used an Arduino to drive the clock. The rest, as they say, is history...

Had to use many different languages throughout the years, starting with IBM/360 Assembler, Fortran, PL/1, Turbo Pascal, ABAP, and, more recently, php/MySQL. Now its mostly Arduino C++, plus JavaScript on the front end. Totally agree with you, C++ is cumbersome and hard to use when you're new to it. It's been a very steep learning curve for me. Learned most of it online. Stackexchange, W3schools (very nice that you can run and modify their example code in an online sandbox), learncpp.com. And, most of all, from reading others' code!

But whenever you think you might have mastered a concept reasonably well, something new pops up. Spent hours just reading up on C++ concepts, without writing a single line of code. I found JavaScript much easier (as long as compact size or max performance are not required). It doesn't enforce strict data types, and it offers some very handy things, like being able to iterate through all instances of an object without bending over backwards. Haven't done much Python, just a couple smaller scripts on the Raspberry Pi.

The Arduino IDE is yet another fine mess, appearing to be platform-independent, when it really is not. Like we've seen with the STL and forward_list. Anyway, it's all we have right now, so better than nothing!

Will give the BlockNot() constructor some thought, there might be an easier way to streamline things...

Keep up the good work! :-)

Michael

[EasyG0ing1]

Issue settled with the 1.7.0 update

[EasyG0ing1]

@bizprof

I have used turbo pascal but that was a long time ago. I've done a lot with SQL ... even wrote some database apps for businesses ... one of them ran the entire manufacturing process for a company that built fasteners for aircraft ... highly specialized parts. That held them for about 15 years until their needs grew into some enterprise-level software. I did that one in VBA, Access, and a SQL Server on the back end. The Access front-end development process was stupid easy compared to any other language I've used. And that was my first experience with modern databases. I have done some light PHP / MySQL before and when I worked for the City of Victorville they were converting from an old school serial connected server to a SQL driven PeopleSoft application and during that process, I got to dabble quite a bit in COBOL and SQL and Crystal Reports which I rather liked.

You mentioned how easy it is to iterate through all instances of an object in Java script ...

Well, in straight Java, if I have an array of any object at all ... let's say it's a File object, you would declare it like this:

List<File> fileList = new ArrayList<>();

Then to iterate through it after you've got however many items in there (and these are dynamic arrays so no need to ever mess with element sizes) this is how you iterate through the list:

for (File aFile : fileList) {
     //Right here you have the next File object in the list and you can do whatever you want to with it
    if (aFile.exists()) aFile.delete();  //or whatever
}

Java is REALLY nice as languages go. I've been enjoying learning and using it.

[EasyG0ing1]

@bizprof - Michael, can you take a look at the current open Issue for BlockNot? There is some trouble with the library and it is directly connected to your addition ... and I'm kinda stumped on how to fix it ... see my search for assistance here and read more detail about the problem:

[bizprof]

Hi Mike, I will take a look at the multiple definition issue. Did the code sample that wouldn't compile get posted somewhere? Usually, multiple declarations are avoided through the #ifdef in the very beginning of a module. If it's already there, it won't be included again. That is good enough for most cases. So, would be great to see a code sample that doesn't compile, and I can take it from there.

[bizprof]

There is a nice write-up about this technique. I did not know it was called "#include guard": https://en.m.wikipedia.org/wiki/Include_guard

[EasyG0ing1]

@bizprof - if you look in the other issue created here for BlockNot, he posted a ver simple code set that will show you the problem.

I was able to split BlockNot up into a header and cpp file, but for some reason, It's not working ... as in ... it doesn't seem to be assigning or retaining values in the variables. I'll attach the files here if you wanna have a look.

I really need to learn more C++ ... lol BlockNot.zip

[bizprof]

Ok, thanks, only just now saw the other issue. Will take a look at what is going wrong.

[EasyG0ing1]

@bizprof - I figured out the variable problem ... I was declaring them in the header and the cpp file and apparently I am only supposed to declare them in the header.

[bizprof]

That sounds good, Mike! There is one thing the BlockNot class is structurally not designed for, and that is to have separate sets (or blocks) of timers, where you could reset all the timers in one set, but not in the other. "All timers" means, well, ALL the timers in a project.

[EasyG0ing1]

@bizprof - RIGHT and that is how I state it in the documentation ... at least I think I do, I'll check before I push this update.

Looks like I have it working properly now and it compiles and runs that multi-file example that was posted in the other Issue ... so I think we have success. Sorry for dragging you into this ... lol

Any way I could get your email address? I wanted to shoot you an email about this rather than re-open this issue but I couldn't find your email anywhere on your profile.

[bizprof]

...sigh of relief... :-) Feel free to email me directly on github@biz-prof.com, not sure why it's not showing in my profile, I'll check

[EasyG0ing1]

Issue fixed in 1.8.0 update