Merge pull request #335 from cyberman54/development

time sync accuracy fixes

README.md

@@ -389,7 +389,9 @@ Note: all settings are stored in NVRAM and will be reloaded when device starts.
 
 0x86 get time/date
 
-	Device answers with it's local time/date (UTC Unix epoch) on Port 2.
+	Device answers with it's local time/date (UTC Unix epoch) on Port 2:
+	bytes 1..4 = local time/date in UTC epoch seconds
+	byte 5 = bits 0..3 timeSource, bits 4..7 timeStatus
 
 0x87 set time/date
 

include/if482.h

@@ -4,7 +4,7 @@
 #include "globals.h"
 
 #define IF482_FRAME_SIZE (17)
-#define IF482_SYNC_FIXUP (3) // calibration to fixup processing time [milliseconds]
+#define IF482_SYNC_FIXUP (2) // calibration to fixup processing time [milliseconds]
 
 String IRAM_ATTR IF482_Frame(time_t tt);
 

include/irqhandler.h

@@ -16,6 +16,8 @@
 #include "timekeeper.h"
 
 void irqHandler(void *pvParameters);
+int mask_user_IRQ();
+int unmask_user_IRQ();
 
 #ifdef HAS_DISPLAY
 #include "display.h"

include/timesync.h

@@ -1,9 +1,9 @@
-#ifndef _TIME_SYNC_LORASERVER_H
-#define _TIME_SYNC_LORASERVER_H
+#ifndef _TIMESYNC_H
+#define _TIMESYNC_H
 
 #include <chrono>
 #include "globals.h"
-#include "timesync.h"
+#include "irqhandler.h"
 #include "timekeeper.h"
 
 //#define TIME_SYNC_TRIGGER 100 // threshold for time sync [milliseconds]

src/irqhandler.cpp

@@ -18,15 +18,14 @@ void irqHandler(void *pvParameters) {
                     &InterruptStatus, // Receives the notification value
                     portMAX_DELAY);   // wait forever
 
-    // interrupt handler to be enabled?
-    if (InterruptStatus & UNMASK_IRQ)
+    if (InterruptStatus & UNMASK_IRQ) // interrupt handler to be enabled?
       mask_irq = false;
-    else if (mask_irq)
-      continue; // suppress processing if interrupt handler is disabled
-
-    // interrupt handler to be disabled?
-    if (InterruptStatus & MASK_IRQ)
+    else if (mask_irq) // suppress processing if interrupt handler is disabled
+      continue;
+    else if (InterruptStatus & MASK_IRQ) { // interrupt handler to be disabled?
       mask_irq = true;
+      continue;
+    }
 
 // button pressed?
 #ifdef HAS_BUTTON
@@ -85,3 +84,16 @@ void IRAM_ATTR ButtonIRQ() {
     portYIELD_FROM_ISR();
 }
 #endif
+
+int mask_user_IRQ() {
+  // begin of time critical section: lock I2C bus to ensure accurate timing
+  if (!I2C_MUTEX_LOCK())
+    return 1; // failure
+  xTaskNotify(irqHandlerTask, MASK_IRQ, eSetBits);
+}
+
+int unmask_user_IRQ() {
+  // end of time critical section: release I2C bus
+  I2C_MUTEX_UNLOCK();
+  xTaskNotify(irqHandlerTask, UNMASK_IRQ, eSetBits);
+}

src/lorawan.cpp

@@ -495,8 +495,7 @@ void IRAM_ATTR user_request_network_time_callback(void *pVoidUserUTCTime,
   }
 
   // begin of time critical section: lock I2C bus to ensure accurate timing
-  // don't move the mutex, will impact accuracy of time up to 1 sec!
-  if (!I2C_MUTEX_LOCK())
+  if (!mask_user_IRQ())
     return; // failure
 
   // Update userUTCTime, considering the difference between the GPS and UTC
@@ -514,7 +513,7 @@ void IRAM_ATTR user_request_network_time_callback(void *pVoidUserUTCTime,
   setMyTime(*pUserUTCTime + requestDelaySec, 0);
 
   // end of time critical section: release I2C bus
-  I2C_MUTEX_UNLOCK();
+  unmask_user_IRQ();
 
 } // user_request_network_time_callback
 #endif // TIME_SYNC_LORAWAN

src/main.cpp

@@ -415,7 +415,6 @@ void setup() {
      !defined HAS_GPS && !defined HAS_RTC)
 #warning you did not specify a time source, time will not be synched
 #endif
-#else
   // start pps timepulse
   ESP_LOGI(TAG, "Starting Timekeeper...");
   assert(timepulse_init()); // setup timepulse

src/rcommand.cpp

@@ -277,6 +277,7 @@ void get_time(uint8_t val[]) {
   ESP_LOGI(TAG, "Remote command: get time");
   payload.reset();
   payload.addTime(now());
+  payload.addByte(timeStatus() << 4 | timeSource);
   SendPayload(STATUSPORT, prio_high);
 };
 

src/timekeeper.cpp

@@ -175,9 +175,9 @@ TickType_t tx_Ticks(uint32_t framesize, unsigned long baud, uint32_t config,
   return round(txTime);
 }
 
-#if defined HAS_IF482 || defined HAS_DCF77
+#if (defined HAS_IF482 || defined HAS_DCF77)
 
-#if defined HAS_DCF77 && defined HAS_IF482
+#if (defined HAS_DCF77 && defined HAS_IF482)
 #error You must define at most one of IF482 or DCF77!
 #endif
 

src/timesync.cpp

@@ -111,8 +111,7 @@ void process_timesync_req(void *taskparameter) {
   } // for
 
   // begin of time critical section: lock I2C bus to ensure accurate timing
-  // don't move the mutex, will impact accuracy of time up to 1 sec!
-  if (!I2C_MUTEX_LOCK())
+  if (!mask_user_IRQ())
     goto error; // failure
 
   // average time offset from collected diffs
@@ -133,9 +132,10 @@ void process_timesync_req(void *taskparameter) {
   setMyTime(time_to_set, time_to_set_fraction_msec);
 
   // end of time critical section: release I2C bus
-  I2C_MUTEX_UNLOCK();
+  unmask_user_IRQ();
 
 finish:
+
   lora_time_sync_pending = false;
   timeSyncReqTask = NULL;
   vTaskDelete(NULL); // end task
@@ -215,7 +215,8 @@ int recv_timesync_ans(uint8_t buf[], uint8_t buf_len) {
 // adjust system time, calibrate RTC and RTC_INT pps
 void IRAM_ATTR setMyTime(uint32_t t_sec, uint16_t t_msec) {
 
-  time_t time_to_set = (time_t)t_sec;
+  // advance time 1 sec wait time
+  time_t time_to_set = (time_t)(t_sec + 1);
 
   ESP_LOGD(TAG, "[%0.3f] Calculated UTC epoch time: %d.%03d sec",
            millis() / 1000.0, time_to_set, t_msec);
@@ -225,14 +226,13 @@ void IRAM_ATTR setMyTime(uint32_t t_sec, uint16_t t_msec) {
     // wait until top of second with millisecond precision
     vTaskDelay(pdMS_TO_TICKS(1000 - t_msec));
 
-#ifdef HAS_RTC
-    time_to_set++; // advance time 1 sec wait time
 // set RTC time and calibrate RTC_INT pulse on top of second
+#ifdef HAS_RTC
     set_rtctime(time_to_set, no_mutex);
 #endif
 
-#if (!defined GPS_INT && !defined RTC_INT)
 // sync pps timer to top of second
+#if (!defined GPS_INT && !defined RTC_INT)
     timerWrite(ppsIRQ, 0); // reset pps timer
     CLOCKIRQ();            // fire clock pps, this advances time 1 sec
 #endif