Revert "Util Timer now uses time offsets (not variables based on current system time) - avoids drift"

gfwilliams · gfwilliams · commit dd408b6de6dc · 2020-10-09T15:46:51.000+01:00
This reverts commit 46b2a48.
diff --git a/ChangeLog b/ChangeLog
@@ -8,7 +8,6 @@
             nRF52: Fix issue where analogRead would stop E.getBattery from working
             Fix setWatch debounce lastTime regression from 2v07 (fix #1902)
             Util Timer no longer uses RTC - works based on estimated time from the hardware timer itself (fix #1749, ref #1444)
-            Util Timer now uses time offsets (not variables based on current system time) - avoids drift
             nRF52: CPU now sleeps when while UART/BLE data is waiting to be sent (fix #1938)
             JSON.stringify now checks for potential stack overflow when stringifying (fix #1940)
             Check for Stack overflow when Garbage Collecting giant linked list (fix #1765)
diff --git a/src/jsserial.c b/src/jsserial.c
@@ -51,10 +51,10 @@ void jsserialSoftwareFunc(
   JsSysTime time;
   UtilTimerTask task;
   if (jstGetLastPinTimerTask(inf->pinTX, &task)) {
-    time = task.timeLeft + bitTime; // leave one bit of time for a stop bit
+    time = task.time + bitTime; // leave one bit of time for a stop bit
   } else {
     // no timer - just start in a little while!
-    time = jshGetTimeFromMilliseconds(1);
+    time = jshGetSystemTime()+jshGetTimeFromMilliseconds(1);
   }
   //bool outState = 1;
   int outCount = 0;
diff --git a/src/jstimer.c b/src/jstimer.c
@@ -29,11 +29,10 @@ unsigned int utilTimerBit;
 bool utilTimerInIRQ = false;
 unsigned int utilTimerData;
 uint16_t utilTimerReload0H, utilTimerReload0L, utilTimerReload1H, utilTimerReload1L;
-/// When we last started the timer, how far in the future were we meant to get called?
+/// When did the timer last run? This is used for timekeeping. Should be jshGetSystemTime() cropped to 32 bits
+volatile int utilTimerTime;
+/// When we rescheduled the timer, how far in the future were we meant to get called?
 int utilTimerPeriod;
-/** When we last started the timer, this was the RTC time. We need this if we need to
-reschedule the timer */
-JsSysTime utilTimerScheduleTime;
 
 
 #ifndef SAVE_ON_FLASH
@@ -94,14 +93,11 @@ void jstUtilTimerInterruptHandler() {
   jshPinOutput(31,1);
   if (utilTimerOn) {
     utilTimerInIRQ = true;
-    // Update the time of each timer task with the actual timer delay
-    unsigned char t = utilTimerTasksTail;
-    while (t!=utilTimerTasksHead) {
-      utilTimerTasks[t].timeLeft -= utilTimerPeriod;
-      t = (t+1) & (UTILTIMERTASK_TASKS-1);
-    }
+    // Increment estimated timer time
+    // TODO: use SysTick to estimate time in this fn and update utilTimerTime accordingly
+    utilTimerTime += utilTimerPeriod;
     // execute any timers that are due
-    while (utilTimerTasksTail!=utilTimerTasksHead && utilTimerTasks[utilTimerTasksTail].timeLeft<=0) {
+    while (utilTimerTasksTail!=utilTimerTasksHead && (utilTimerTasks[utilTimerTasksTail].time - utilTimerTime)<=0) {
       UtilTimerTask *task = &utilTimerTasks[utilTimerTasksTail];
       void (*executeFn)(JsSysTime time, void* userdata) = 0;
       void *executeData = 0;
@@ -171,12 +167,13 @@ void jstUtilTimerInterruptHandler() {
       // If we need to repeat
       if (task->repeatInterval) {
         // update time (we know time > task->time)
-        task->timeLeft += task->repeatInterval;
+        task->time += task->repeatInterval;
+
         // do an in-place bubble sort to ensure that times are still in the right order
         unsigned char ta = utilTimerTasksTail;
         unsigned char tb = (ta+1) & (UTILTIMERTASK_TASKS-1);
         while (tb != utilTimerTasksHead) {
-          if (utilTimerTasks[ta].timeLeft > utilTimerTasks[tb].timeLeft) {
+          if ((utilTimerTasks[ta].time-utilTimerTime) > (utilTimerTasks[tb].time-utilTimerTime)) {
             UtilTimerTask task = utilTimerTasks[ta];
             utilTimerTasks[ta] = utilTimerTasks[tb];
             utilTimerTasks[tb] = task;
@@ -195,9 +192,8 @@ void jstUtilTimerInterruptHandler() {
 
     // re-schedule the timer if there is something left to do
     if (utilTimerTasksTail != utilTimerTasksHead) {
-      utilTimerPeriod = utilTimerTasks[utilTimerTasksTail].timeLeft;
+      utilTimerPeriod = utilTimerTasks[utilTimerTasksTail].time - utilTimerTime;
       if (utilTimerPeriod<0) utilTimerPeriod=0;
-      utilTimerScheduleTime = jshGetSystemTime();
       jshUtilTimerReschedule(utilTimerPeriod);
     } else {
       utilTimerOn = false;
@@ -229,13 +225,9 @@ static bool utilTimerIsFull() {
 /* Restart the utility timer with the right period. This should not normally
 need to be called by anything outside jstimer.c */
 void  jstRestartUtilTimer() {
-  // disabling here stops jshUtilTimerStart trying to schedule the timer including the time that has already passed
-  jshUtilTimerDisable();
-  // Work out how long we've got to go
-  utilTimerPeriod = utilTimerTasks[utilTimerTasksTail].timeLeft;
+  utilTimerTime = (int)jshGetSystemTime();
+  utilTimerPeriod = utilTimerTasks[utilTimerTasksTail].time - utilTimerTime;
   if (utilTimerPeriod<0) utilTimerPeriod=0;
-  utilTimerScheduleTime = jshGetSystemTime();
-  // reschedule
   jshUtilTimerStart(utilTimerPeriod);
 }
 
@@ -247,11 +239,9 @@ bool utilTimerInsertTask(UtilTimerTask *task) {
 
   if (!utilTimerInIRQ) jshInterruptOff();
 
-  // time between when utilTimerTasks was last updated and time now
-  int timeDiff = jshGetSystemTime()-utilTimerScheduleTime;
   // find out where to insert
   unsigned char insertPos = utilTimerTasksTail;
-  while (insertPos != utilTimerTasksHead && (utilTimerTasks[insertPos].timeLeft-timeDiff) < task->timeLeft)
+  while (insertPos != utilTimerTasksHead && utilTimerTasks[insertPos].time < task->time)
     insertPos = (insertPos+1) & (UTILTIMERTASK_TASKS-1);
 
   bool haveChangedTimer = insertPos==utilTimerTasksTail;
@@ -261,21 +251,18 @@ bool utilTimerInsertTask(UtilTimerTask *task) {
   while (i != insertPos) {
     unsigned char next = (i+UTILTIMERTASK_TASKS-1) & (UTILTIMERTASK_TASKS-1);
     utilTimerTasks[i] = utilTimerTasks[next];
-    if (haveChangedTimer) {
-      // if we changed the timer, we're rescheduling so must change
-      // the times in all tasks
-      utilTimerTasks[i].timeLeft -= timeDiff;
-    }
     i = next;
   }
   // add new item
   utilTimerTasks[insertPos] = *task;
   // increase task list size
   utilTimerTasksHead = (utilTimerTasksHead+1) & (UTILTIMERTASK_TASKS-1);
+
+  //jsiConsolePrint("Head is %d\n", utilTimerTasksHead);
   // now set up timer if not already set up...
   if (!utilTimerOn || haveChangedTimer) {
-    jstRestartUtilTimer();
     utilTimerOn = true;
+    jstRestartUtilTimer();
   }
 
   if (!utilTimerInIRQ) jshInterruptOn();
@@ -381,7 +368,7 @@ bool jstGetLastBufferTimerTask(JsVar *var, UtilTimerTask *task) {
 bool jstPinOutputAtTime(JsSysTime time, Pin *pins, int pinCount, uint8_t value) {
   assert(pinCount<=UTILTIMERTASK_PIN_COUNT);
   UtilTimerTask task;
-  task.timeLeft = (int)(time - jshGetSystemTime());
+  task.time = (int)time;
   task.repeatInterval = 0;
   task.type = UET_SET;
   int i;
@@ -430,10 +417,10 @@ bool jstPinPWM(JsVarFloat freq, JsVarFloat dutyCycle, Pin pin) {
   }
   if (ptaskon && ptaskoff) {
     // Great! We have PWM... Just update it
-    if (ptaskoff->timeLeft > ptaskon->timeLeft)
-      ptaskoff->timeLeft = ptaskon->timeLeft + pulseLength;
+    if (ptaskoff->time > ptaskon->time)
+      ptaskoff->time = ptaskon->time + pulseLength;
     else
-      ptaskoff->timeLeft = ptaskon->timeLeft + pulseLength - (unsigned int)period;
+      ptaskoff->time = ptaskon->time + pulseLength - (unsigned int)period;
     ptaskon->repeatInterval = (unsigned int)period;
     ptaskoff->repeatInterval = (unsigned int)period;
     /* don't bother rescheduling - everything will work out next time
@@ -448,12 +435,12 @@ bool jstPinPWM(JsVarFloat freq, JsVarFloat dutyCycle, Pin pin) {
   if (ptaskon || ptaskoff) {
     while (utilTimerRemoveTask(jstPinTaskChecker, (void*)&pin));
   }
-
+  JsSysTime time = jshGetSystemTime();
   UtilTimerTask taskon, taskoff;
   taskon.data.set.value = 1;
   taskoff.data.set.value = 0;
-  taskon.timeLeft = 0;
-  taskoff.timeLeft = (int)(taskon.timeLeft + pulseLength);
+  taskon.time = (int)time;
+  taskoff.time = (int)(time + pulseLength);
   taskon.repeatInterval = (unsigned int)period;
   taskoff.repeatInterval = (unsigned int)period;
   taskon.type = UET_SET;
@@ -475,7 +462,7 @@ bool jstPinPWM(JsVarFloat freq, JsVarFloat dutyCycle, Pin pin) {
 /// Execute the given function repeatedly after the given time period
 bool jstExecuteFn(UtilTimerTaskExecFn fn, void *userdata, JsSysTime startTime, uint32_t period) {
   UtilTimerTask task;
-  task.timeLeft = (int)(startTime - jshGetSystemTime());
+  task.time = (int)startTime;
   task.repeatInterval = period;
   task.type = UET_EXECUTE;
   task.data.execute.fn = fn;
@@ -496,7 +483,7 @@ bool jstStopExecuteFn(UtilTimerTaskExecFn fn, void *userdata) {
 /// Set the utility timer so we're woken up in whatever time period
 bool jstSetWakeUp(JsSysTime period) {
   UtilTimerTask task;
-  task.timeLeft = (int)period;
+  task.time = (int)(jshGetSystemTime() + period);
   task.repeatInterval = 0;
   task.type = UET_WAKEUP;
 
@@ -508,11 +495,11 @@ bool jstSetWakeUp(JsSysTime period) {
   jshInterruptOff();
   if (utilTimerTasksTail!=utilTimerTasksHead) {
     hasTimer = true;
-    nextTime = utilTimerTasks[utilTimerTasksTail].timeLeft;
+    nextTime = utilTimerTasks[utilTimerTasksTail].time;
   }
   jshInterruptOn();
 
-  if (hasTimer && task.timeLeft >= nextTime) {
+  if (hasTimer && (task.time-utilTimerTime) >= (nextTime-utilTimerTime)) {
     // we already had a timer, and it's going to wake us up sooner.
     // don't create a WAKEUP timer task
     return true;
@@ -549,7 +536,7 @@ bool jstStartSignal(JsSysTime startTime, JsSysTime period, Pin pin, JsVar *curre
   if (!jshIsPinValid(pin)) return false;
   UtilTimerTask task;
   task.repeatInterval = (unsigned int)period;
-  task.timeLeft = (int)(startTime + period - jshGetSystemTime());
+  task.time = (int)(startTime + period);
   task.type = type;
   if (UET_IS_BUFFER_WRITE_EVENT(type)) {
     task.data.buffer.pinFunction = jshGetCurrentPinFunction(pin);
@@ -589,14 +576,19 @@ bool jstStopBufferTimerTask(JsVar *var) {
 void jstReset() {
   jshUtilTimerDisable();
   utilTimerTasksTail = utilTimerTasksHead = 0;
+  utilTimerTime = (int)jshGetSystemTime();
   utilTimerPeriod = 0;
-  utilTimerScheduleTime = jshGetSystemTime();
 }
 
 /** when system time is changed, also change the time in the timers.
 This should be done with interrupts off */
 void jstSystemTimeChanged(JsSysTime diff) {
-  utilTimerScheduleTime += diff;
+  unsigned char t = utilTimerTasksTail;
+  while (t!=utilTimerTasksHead) {
+    utilTimerTasks[t].time += diff;
+    t = (t+1) & (UTILTIMERTASK_TASKS-1);
+  }
+  utilTimerTime += diff;
 }
 
 void jstDumpUtilityTimers() {
@@ -607,15 +599,17 @@ void jstDumpUtilityTimers() {
     uTimerTasks[i] = utilTimerTasks[i];
   unsigned char uTimerTasksHead = utilTimerTasksHead;
   unsigned char uTimerTasksTail = utilTimerTasksTail;
+  int uTimerTime = utilTimerTime;
   jshInterruptOn();
 
+  jsiConsolePrintf("Current timer difference %d us", (int)(1000*jshGetMillisecondsFromTime(jshGetSystemTime()-uTimerTime)));
   unsigned char t = uTimerTasksTail;
   bool hadTimers = false;
   while (t!=uTimerTasksHead) {
     hadTimers = true;
 
     UtilTimerTask task = uTimerTasks[t];
-    jsiConsolePrintf("%08d us", (int)(1000*jshGetMillisecondsFromTime(task.timeLeft)));
+    jsiConsolePrintf("%08d us", (int)(1000*jshGetMillisecondsFromTime(task.time-uTimerTime)));
     jsiConsolePrintf(", repeat %08d us", (int)(1000*jshGetMillisecondsFromTime(task.repeatInterval)));
     jsiConsolePrintf(" : ");
 
diff --git a/src/jstimer.h b/src/jstimer.h
@@ -89,7 +89,7 @@ typedef union UtilTimerTaskData {
 } UtilTimerTaskData;
 
 typedef struct UtilTimerTask {
-  int timeLeft; // how far in the future should we execute the timer (JshSysTime, cropped to 32 bits)
+  int time; // time at which to set pins (JshSysTime, cropped to 32 bits)
   unsigned int repeatInterval; // if nonzero, repeat the timer
   UtilTimerTaskData data; // data used when timer is hit
   UtilTimerEventType type; // the type of this task - do we set pin(s) or read/write data
diff --git a/targets/nrf5x/jshardware.c b/targets/nrf5x/jshardware.c
@@ -1398,10 +1398,10 @@ void jshPinPulse(Pin pin, bool pulsePolarity, JsVarFloat pulseTime) {
     if (!jstGetLastPinTimerTask(pin, &task)) {
       // no timer - just start the pulse now!
       jshPinOutput(pin, pulsePolarity);
-      task.timeLeft = 0;
+      task.time = jshGetSystemTime();
     }
     // Now set the end of the pulse to happen on a timer
-    jstPinOutputAtTime(jshGetSystemTime() + task.timeLeft + jshGetTimeFromMilliseconds(pulseTime), &pin, 1, !pulsePolarity);
+    jstPinOutputAtTime(task.time + jshGetTimeFromMilliseconds(pulseTime), &pin, 1, !pulsePolarity);
   }
 }
 
@@ -2300,19 +2300,10 @@ void jshUtilTimerReschedule(JsSysTime period) {
     period = NRF_TIMER_MAX;
   }
   //jsiConsolePrintf("Sleep for %d %d -> %d\n", (uint32_t)(t>>32), (uint32_t)(t), (uint32_t)(period));
-  if (utilTimerActive) {
-    /* If running, get the current timer value and attempt to use it to schedule the period
-     * based on what it is right now */
-    nrf_timer_task_trigger(NRF_TIMER1, NRF_TIMER_TASK_STOP);
-    nrf_timer_task_trigger(NRF_TIMER1, NRF_TIMER_TASK_CAPTURE3);
-    uint32_t currentValue = nrf_timer_cc_read(NRF_TIMER1, NRF_TIMER_CC_CHANNEL3);
-    if (period < currentValue+2) period = currentValue+2;
-    nrf_timer_cc_write(NRF_TIMER1, NRF_TIMER_CC_CHANNEL0, (uint32_t)period);
-    nrf_timer_task_trigger(NRF_TIMER1, NRF_TIMER_TASK_START);
-  } else {
-    nrf_timer_task_trigger(NRF_TIMER1, NRF_TIMER_TASK_CLEAR);
-    nrf_timer_cc_write(NRF_TIMER1, NRF_TIMER_CC_CHANNEL0, (uint32_t)period);
-  }
+  if (utilTimerActive) nrf_timer_task_trigger(NRF_TIMER1, NRF_TIMER_TASK_STOP);
+  nrf_timer_task_trigger(NRF_TIMER1, NRF_TIMER_TASK_CLEAR);
+  nrf_timer_cc_write(NRF_TIMER1, NRF_TIMER_CC_CHANNEL0, (uint32_t)period);
+  if (utilTimerActive) nrf_timer_task_trigger(NRF_TIMER1, NRF_TIMER_TASK_START);
 }
 
 /// Start the timer and get it to interrupt after 'period'
