timeout micros
michalzahor opened this issue · 4 comments
Hi,
I've found issue with micros overflow [ back to zero ] which as I see is solved but the way that it only works if one is using sleep() wake() code - even then not always - it can happen that micros overflows in the middle of waittimeout.
I'd like to see universal solution like:
timeoutSet - store timestamp and delay in micros
void Adafruit_Thermal::timeoutSet(unsigned long x) {
// resumeTime = micros() + x;
lastOperationTS = micros(); //last operation timestamp
waitTime = x; // how many micros to wait for another operation
}
Then wait until waitTime passed or miscros overflow
void Adafruit_Thermal::timeoutWait() {
// while((long)(micros() - resumeTime) < 0); // Rollover-proof
while( (micros() >= lastOperationTS ) && ( (long)(micros() - lastOperationTS - waitTime) < 0 ) ); // Rollover-proof
}
Can you cite an example of the timeout code failing? It's been tested in a variety of situations, not just sleep/wake (e.g. bitmap printing), and is designed to survive the micros() overflow. The first condition in your while() won't wait in an overflow situation.
if writeByte is called at the end of micros() cycle ( max. 4,294,967,295 ) , timeoutSet(BYTE_TIME) will let's say set resumeTime = 4,294,967,290. Next call to writeByte , the micros can / will overflow. So calling timeoutWait()
while((long)(micros() - resumeTime) < 0); will wait cca 70 minutes, because resumeTime is set to 4,294,967,290 but
micros() runs from the beginning.
It could happen calling
printer.normal()
....
//micros 4,294,967,290
... some code ...
//micros 14
....
priner.inverseOn()
My solution is not perfect - because as you said, if this situation happens, it will not wait in timeout function [ but as I see this waiting is not so importat , that it will not hurt functionality if it will not wait always ]
Or why not to use delayMicroseconds() instead of while cycle.
// comment on delayMicroseconds
//This function works very accurately in the range 3 microseconds and up. We cannot assure that //delayMicroseconds will perform precisely for smaller delay-times.
//Currently, the largest value that will produce an accurate delay is 16383. This could change in future Arduino //releases. For delays longer than a few thousand microseconds, you should use delay() instead.
void Adafruit_Thermal::timeoutWait() {
if ( resumeTime <= 16383 )
delayMicroseconds(resumeTime); // Rollover-proof
else
delay ( resumeTime / 1000);
}
I'm cleaning up old issues and am hesitant to change anything here since the current code is pretty well understood and hasn't had a lot of issues with delay overflow. It's good to raise this issue though so folks can find it and reply if they're running into it. If there are enough people hitting this then it might make sense to look at changing some of the delay logic. For now I'll close this one though, thanks for raising the issue!
The code as written is in fact rollover-proof...it's using sign-conversion to pull shenanigans against unsigned values (hence the comparison against 0, rather than a target value, which -would- manifest the rollover plague). It's One Of Those Arduino Tricks™. A little test program demonstrates:
void setup() {
unsigned long
pre = 4294967290, // Hypothetical micros() time pre-rollover
resumeTime = pre + 42, // This will roll over! It's OK.
post = 5; // Hypothetical micros() time post-rollover
Serial.begin(9600);
Serial.println((int)(post - resumeTime)); // Negative until resumeTime
}
void loop() { }