Skip to content

Commit afa79bf

Browse files
committed
Timer reset bugfix, drift correction buffer
1 parent 0fec06a commit afa79bf

File tree

1 file changed

+53
-29
lines changed

1 file changed

+53
-29
lines changed

arduino-nixie/arduino-nixie.ino

Lines changed: 53 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ byte timerInitialSecs = 0; //timer original duration setting, seconds - up to 59
138138
unsigned long timerTime = 0; //timestamp of timer target / chrono origin (while running) or duration (while stopped)
139139
unsigned long timerLapTime = 0;
140140
const byte millisCorrectionInterval = 30; //used to calibrate millis() to RTC for timer/chrono purposes
141-
unsigned long millisAtLastCorrection = 0;
141+
unsigned long millisAtLastCheck = 0;
142142
word unoffRemain = 0; //un-off (briefly turn on tubes during full night/away shutoff) timeout counter, seconds
143143
byte displayDim = 2; //dim per display or function: 2=normal, 1=dim, 0=off
144144
byte cleanRemain = 0; //anti-cathode-poisoning clean timeout counter, increments at cleanSpeed ms (see loop()). Start at 11 to run at clock startup
@@ -200,6 +200,7 @@ void loop(){
200200
}
201201
//Every loop cycle, check the RTC and inputs (previously polled, but works fine without and less flicker)
202202
checkRTC(false); //if clock has ticked, decrement timer if running, and updateDisplay
203+
millisApplyDrift();
203204
checkInputs(); //if inputs have changed, this will do things + updateDisplay as needed
204205
doSetHold(false); //if inputs have been held, this will do more things + updateDisplay as needed
205206
cycleTimer();
@@ -452,7 +453,7 @@ void ctrlEvt(byte ctrl, byte evt){
452453
ds3231.setHour(fnSetVal/60);
453454
ds3231.setMinute(fnSetVal%60);
454455
ds3231.setSecond(0);
455-
millisAtLastCorrection = 0; //see ms()
456+
millisAtLastCheck = 0; //see ms()
456457
calcSun(tod.year(),tod.month(),tod.day());
457458
isDSTByHour(tod.year(),tod.month(),tod.day(),fnSetVal/60,true);
458459
}
@@ -803,9 +804,9 @@ void checkRTC(bool force){
803804
if(fnPg >= fnDatePages){ fnPg = 0; fn = fnIsTime; }
804805
force=true;
805806
}
806-
//Temporary-display function timeout: if we're *not* in a permanent one (time, or running timer)
807-
// Stopped timer shouldn't be permanent, but have a much longer timeout, mostly in case someone is waiting to start the chrono in sync with some event, so we'll give that an hour.
808-
else if(fn!=fnIsTime && !(fn==fnIsTimer && timerState&1)){
807+
//Temporary-display function timeout: if we're *not* in a permanent one (time, or running/signaling timer)
808+
// Stopped/non-signaling timer shouldn't be permanent, but have a much longer timeout, mostly in case someone is waiting to start the chrono in sync with some event, so we'll give that an hour.
809+
else if(fn!=fnIsTime && !(fn==fnIsTimer && (timerState&1 || signalRemain>0))){
809810
if((unsigned long)(now-inputLast)>=(fn==fnIsTimer?3600:timeoutTempFn)*1000) { fnSetPg = 0; fn = fnIsTime; force=true; }
810811
}
811812

@@ -842,7 +843,7 @@ void checkRTC(bool force){
842843
//Things to do at specific times
843844
//Timer drift correction: per the millisCorrectionInterval
844845
if(tod.second()%millisCorrectionInterval==0){ //if time:
845-
if(!(rtcDid&1)) millisCorrectForDrift(); bitWrite(rtcDid,0,1); //do if not done, set as done
846+
if(!(rtcDid&1)) millisCheckDrift(); bitWrite(rtcDid,0,1); //do if not done, set as done
846847
} else bitWrite(rtcDid,0,0); //if not time: set as not done
847848
//DST change check: every 2am
848849
if(tod.second()==0 && tod.minute()==0 && tod.hour()==2) autoDST();
@@ -1067,16 +1068,44 @@ bool isDayInRange(byte dstart, byte dend, byte dtest) {
10671068
}
10681069

10691070
// Chrono/Timer
1070-
// There are two timing sources in the UNDB – the Arduino itself (eg millis()), which gives subsecond precision but isn't very accurate, so it's only good for short-term timing and taking action in response to user activity (eg button press hold thresholds); and the DS3231, which is very accurate but only gives seconds (unless you're monitoring its square wave via a digital pin), so it's only good for long-term timing and taking action in response to time of day. The one place we need both short-term precision and long-term accuracy is in the chrono/timer – so I have based it on millis() but with an offset applied to correct for its drift, periodically adjusted per the DS3231.
1071-
unsigned long millisDriftOffset = 0; //The cumulative running offset.
1072-
//unsigned long millisAtLastCorrection (defined at top, so ctrlEvt can reset it when setting RTC). 0 when unreliable (at start and after RTC set).
1071+
// There are two timing sources in the UNDB – the Arduino itself (eg millis()), which gives subsecond precision but isn't very accurate, so it's only good for short-term timing and taking action in response to user activity (eg button press hold thresholds); and the DS3231, which is very accurate but only gives seconds (unless you're monitoring its square wave via a digital pin), so it's only good for long-term timing and taking action in response to time of day. The one place we need both short-term precision and long-term accuracy is in the chrono/timer – so I have based it on millis() but with an offset applied to correct for its drift, periodically adjusted per the DS3231. I also use it for the signal, so the 1/sec measure cycle stays in sync with real time; but we don't need to use it for stuff like button polling.
1072+
unsigned long millisDriftOffset = 0; //The cumulative running offset. Since it's circular, doesn't matter whether signed or not.
1073+
//unsigned long millisAtLastCheck (defined at top, so ctrlEvt can reset it when setting RTC). 0 when unreliable (at start and after RTC set).
10731074
//const byte millisCorrectionInterval (defined at top, so checkRTC can see it)
1075+
int millisDriftBuffer = 0; // Each time we calculate millis() drift, we add it to this signed buffer, which gets applied to millisDriftOffset slowly to smooth the correction and minimize (eliminate?) the chance of a discontinuity, which prevents unsightly glitches in the chrono/timer and the signal performance, and failures in eg detecting button presses.
10741076
// TODO the adjustments are a little noisy in the short term, because of a rolling offset between the loop cycle (slowed down by cycleDisplay's delays) and the rtc ticks. It's kind of academic since the variance is probably only around ±.02sec per adjustment at most (largely the duration of cycleDisplay's delays), which I'd say is within the tolerance of display/button/human limitations, but no point doing it as quickly as once per second I think.
1075-
void millisCorrectForDrift(){
1077+
void millisCheckDrift(){
10761078
unsigned long now = millis();
1077-
if(millisAtLastCorrection){ //if this has a value, check to see how much millis has drifted since then.
1078-
unsigned long millisDrift = now-(millisAtLastCorrection+(millisCorrectionInterval*1000));
1079-
millisDriftOffset -= millisDrift;
1079+
if(millisAtLastCheck){ // if this has a value, check to see how much millis has drifted since then. If this is 0, it means either the RTC was recently set (or the extremely unlikely case that the last sync occurred exactly at millis rollover) so we will hold off until next drift check.
1080+
long millisDrift = now-(millisAtLastCheck+(millisCorrectionInterval*1000)); // Converting difference to a signed long.
1081+
if(abs((long)(millisDrift+millisDriftBuffer))>32767){} // If adding drift to buffer would make it overflow, ignore it this time
1082+
else {
1083+
millisDriftBuffer -= millisDrift;
1084+
// tod = rtc.now();
1085+
// if(tod.hour()<10) Serial.print(F("0"));
1086+
// Serial.print(tod.hour(),DEC);
1087+
// Serial.print(F(":"));
1088+
// if(tod.minute()<10) Serial.print(F("0"));
1089+
// Serial.print(tod.minute(),DEC);
1090+
// Serial.print(F(":"));
1091+
// if(tod.second()<10) Serial.print(F("0"));
1092+
// Serial.print(tod.second(),DEC);
1093+
// Serial.print(F(" millis: "));
1094+
// Serial.print(now,DEC);
1095+
// Serial.print(F(" drift: "));
1096+
// Serial.print(millisDrift,DEC);
1097+
// Serial.print(F(" new buffer: "));
1098+
// Serial.print(millisDriftBuffer,DEC);
1099+
// Serial.println();
1100+
}
1101+
}
1102+
millisAtLastCheck = now;
1103+
}
1104+
void millisApplyDrift(){
1105+
//Applies millisDriftBuffer to millisDriftOffset at the rate of 1ms per loop. See above for details.
1106+
if(millisDriftBuffer){
1107+
millisDriftOffset += (millisDriftBuffer>0? 1: -1);
1108+
millisDriftBuffer -= (millisDriftBuffer>0? 1: -1);
10801109
// tod = rtc.now();
10811110
// if(tod.hour()<10) Serial.print(F("0"));
10821111
// Serial.print(tod.hour(),DEC);
@@ -1086,22 +1115,16 @@ void millisCorrectForDrift(){
10861115
// Serial.print(F(":"));
10871116
// if(tod.second()<10) Serial.print(F("0"));
10881117
// Serial.print(tod.second(),DEC);
1089-
// Serial.print(F(" millis: "));
1090-
// Serial.print(now,DEC);
1091-
// Serial.print(F(" drift: "));
1092-
// Serial.print(millisDrift,DEC);
1093-
// Serial.print(F(" (")); Serial.print(~millisDrift,DEC); //in case of wraparound, this is more human-readable
1094-
// Serial.print(F(") new offset: "));
1118+
// Serial.print(F(" new offset: "));
10951119
// Serial.print(millisDriftOffset,DEC);
1096-
// Serial.print(F(" (")); Serial.print(~millisDriftOffset,DEC); //in case of wraparound, this is more human-readable
1097-
// Serial.print(F(")"));
1120+
// Serial.print(F(" new buffer: "));
1121+
// Serial.print(millisDriftBuffer,DEC);
10981122
// Serial.println();
10991123
}
1100-
millisAtLastCorrection = now;
11011124
}
11021125
unsigned long ms(){
11031126
// Returns millis() with the drift offset applied, for timer/chrono purposes.
1104-
// WARNING: Since the offset is being periodically adjusted, there is the possibility of a discontinuity in ms() output – if we give out a timestamp and then effectively set the clock back, the next timestamp might possibly be earlier than the last one, which could mess up duration math. I tried to think of a way to monitor for that discontinuity – e.g. if now-then is greater than then-now, due to overflow – but it gets tricky since millis() is effectively circular, and that condition occurs naturally at rollover as well – so I think we would need a flag that millisCorrectForDrift sets when it sets the offset backward, and ms clears when the real time has caught up.... or something like that.
1127+
// WARNING: Since the offset is being periodically adjusted, there is the possibility of a discontinuity in ms() output – if we give out a timestamp and then effectively set the clock back, the next timestamp might possibly be earlier than the last one, which could mess up duration math. I tried to think of a way to monitor for that discontinuity – e.g. if now-then is greater than then-now, due to overflow – but it gets tricky since millis() is effectively circular, and that condition occurs naturally at rollover as well – so I think we would need a flag that millisCheckDrift sets when it sets the offset backward, and ms clears when the real time has caught up.... or something like that.
11051128
// So for now we'll restrict use of ms() to the timer/chrono duration – the only place we really need it, and fortunately it's not a problem here, because that duration never exceeds 100 hours so we can easily detect that overflow. And the only time it might really be an issue is if it occurs immediately after the chrono starts, since it would briefly display the overflowed time – in that case we'll just reset the timer to 0 and keep going.
11061129
return (unsigned long)(millis()+millisDriftOffset);
11071130
}
@@ -1710,6 +1733,7 @@ void decToBin(bool binVal[], byte i){
17101733
//Like songs, a signal is made up of measures (usually 1sec each) tracked by the signalRemain counter.
17111734
//Measures are made up of steps such as beeps and relay pulses, tracked by signalMeasureStep.
17121735
//When used with switched relay, it simply turns on at the start, and off at the end, like a clock radio – the measures just wait it out.
1736+
//Timed using ms() instead of millis() – see timer/chrono for details.
17131737
unsigned long signalMeasureStartTime = 0; //to keep track of individual measures
17141738
byte signalMeasureStep = 0; //step number, or 255 if waiting for next measure, or 0 if not signaling
17151739
void signalStart(byte sigFn, byte sigDur){
@@ -1736,7 +1760,7 @@ void signalStart(byte sigFn, byte sigDur){
17361760
// Serial.println();
17371761

17381762
//Start a measure for cycleSignal to pick up. For "quick measures" we won't set signalRemain.
1739-
signalMeasureStartTime = millis();
1763+
signalMeasureStartTime = ms();
17401764
signalMeasureStep = 1; //waiting to start a new measure
17411765
if(sigDur!=0){ //long-duration signal (alarm, sleep, etc) - set signalRemain
17421766
//If switched relay, except if this is a forced fnIsTimer signal (for signaling runout options)
@@ -1784,13 +1808,13 @@ void cycleSignal(){
17841808
bc = 4; bd = 62; break;
17851809
}
17861810
//See if it's time to start a new measure
1787-
if(signalMeasureStep==255 && (unsigned long)(millis()-signalMeasureStartTime)>=measureDur){
1811+
if(signalMeasureStep==255 && (unsigned long)(ms()-signalMeasureStartTime)>=measureDur){
17881812
//Serial.println(F("Starting new measure, sPS -1 -> 1"));
17891813
signalMeasureStartTime += measureDur;
17901814
signalMeasureStep = 1;
17911815
}
17921816
//See if it's time to start a new beep
1793-
if((unsigned long)(millis()-signalMeasureStartTime)>=(signalMeasureStep-1)*bd*2){
1817+
if((unsigned long)(ms()-signalMeasureStartTime)>=(signalMeasureStep-1)*bd*2){
17941818
word piezoPitch = (signalPattern==5 && signalMeasureStep==2? getSignalPitch()*0.7937: //cuckoo: go down major third (2^(-4/12)) on 2nd beep
17951819
(signalPattern==255? 1000: //the pips: use 1000Hz just like the Beeb
17961820
(signalRemain==0 && signalSource==fnIsTimer? getHz(69): //fnIsTimer runout setting: use timer start pitch
@@ -1819,7 +1843,7 @@ void cycleSignal(){
18191843
//We don't follow the beep pattern here, we simply energize the relay for relayPulse time
18201844
//Unlike beeper, we need to use a signalMeasureStep (2) to stop the relay.
18211845
//See if it's time to start a new measure
1822-
if(signalMeasureStep==255 && (unsigned long)(millis()-signalMeasureStartTime)>=measureDur){
1846+
if(signalMeasureStep==255 && (unsigned long)(ms()-signalMeasureStartTime)>=measureDur){
18231847
//Serial.println(F("Starting new measure, sPS -1 -> 1"));
18241848
signalMeasureStartTime += measureDur;
18251849
signalMeasureStep = 1;
@@ -1831,7 +1855,7 @@ void cycleSignal(){
18311855
signalMeasureStep = 2; //set it up to stop
18321856
}
18331857
//See if it's time to stop the pulse
1834-
else if(signalMeasureStep==2 && (unsigned long)(millis()-signalMeasureStartTime)>=relayPulse) {
1858+
else if(signalMeasureStep==2 && (unsigned long)(ms()-signalMeasureStartTime)>=relayPulse) {
18351859
digitalWrite(relayPin,HIGH); updateLEDs(); //LOW = device on
18361860
//Serial.print(millis(),DEC); Serial.println(F(" Relay off, cycleSignal"));
18371861
//Set up for the next event
@@ -1843,7 +1867,7 @@ void cycleSignal(){
18431867
else { //switched relay / default
18441868
//Simply decrement signalRemain until it runs out - signalMeasureStep doesn't matter as long as it stays nonzero as at start
18451869
//See if it's time to start a new measure
1846-
if((unsigned long)(millis()-signalMeasureStartTime)>=measureDur){
1870+
if((unsigned long)(ms()-signalMeasureStartTime)>=measureDur){
18471871
//Serial.println(F("Starting new measure, sPS -1 -> 1"));
18481872
signalMeasureStartTime += measureDur;
18491873
}

0 commit comments

Comments
 (0)