timesync code refactoring

include/timesync.h

@@ -8,7 +8,7 @@
 #define TIME_SYNC_FRAME_LENGTH 0x07 // timeserver answer frame length [bytes]
 #define TIME_SYNC_FIXUP 16 // compensation for processing time [milliseconds]
 #define TIMEREQUEST_MAX_SEQNO 0xfe // threshold for wrap around seqno
-#define TIMEREQUEST_FINISH                                                     \
+#define TIMEREQUEST_END                                                        \
   (TIMEREQUEST_MAX_SEQNO + 1) // marker for end of timesync handshake
 #define GPS_UTC_DIFF 315964800
 
@@ -24,11 +24,6 @@ void timesync_init(void);
 void timesync_sendReq(void);
 void timesync_storeReq(uint32_t timestamp, timesync_t timestamp_type);
 void IRAM_ATTR timesync_processReq(void *taskparameter);
-
+void IRAM_ATTR timesync_serverAnswer(void *pUserData, int flag);
-#if (TIME_SYNC_LORASERVER)
-int recv_timeserver_ans(const uint8_t buf[], uint8_t buf_len);
-#elif (TIME_SYNC_LORAWAN)
-void IRAM_ATTR DevTimeAns_Cb(void *pUserData, int flagSuccess);
-#endif
 
 #endif

src/lorawan.cpp

@@ -471,10 +471,8 @@ void myRxCallback(void *pUserData, uint8_t port, const uint8_t *pMsg,
 // timeserver answer -> call timesync processor
 #if (TIME_SYNC_LORASERVER)
   case TIMEPORT:
-    // store LMIC time when we received the timesync answer
-    timesync_storeReq(osticks2ms(os_getTime()), timesync_rx);
     // get and store gwtime from payload
-    recv_timeserver_ans(pMsg, nMsg);
+    timesync_serverAnswer(&pMsg, nMsg);
 #endif
 
     // decode any piggybacked downlink MAC commands if we want to print those

src/main.cpp

@@ -33,7 +33,7 @@ IDLE          0     0     ESP32 arduino scheduler -> runs wifi sniffer
 
 lmictask      1     2     MCCI LMiC LORAWAN stack
 clockloop     1     4     generates realtime telegrams for external clock
-timesync_req  1     3     processes realtime time sync requests
+timesync_proc 1     3     processes realtime time sync requests
 irqhandler    1     1     cyclic tasks (i.e. displayrefresh) triggered by timers
 gpsloop       1     1     reads data from GPS via serial or i2c
 lorasendtask  1     1     feeds data from lora sendqueue to lmcic

src/timesync.cpp

@@ -30,16 +30,16 @@ static uint8_t time_sync_seqNo = (uint8_t)random(TIMEREQUEST_MAX_SEQNO),
 static uint16_t timestamp_msec;
 static uint32_t timestamp_sec,
     timesync_timestamp[TIME_SYNC_SAMPLES][no_of_timestamps];
-static TaskHandle_t timeSyncReqTask = NULL;
+static TaskHandle_t timeSyncProcTask = NULL;
 
 // create task for timeserver handshake processing, called from main.cpp
 void timesync_init() {
   xTaskCreatePinnedToCore(timesync_processReq, // task function
-                          "timesync_req",      // name of task
+                          "timesync_proc",     // name of task
                           2048,                // stack size of task
                           (void *)1,           // task parameter
                           3,                   // priority of the task
-                          &timeSyncReqTask,    // task handle
+                          &timeSyncProcTask,   // task handle
                           1);                  // CPU core
 }
 
@@ -53,19 +53,19 @@ void timesync_sendReq(void) {
     ESP_LOGI(TAG, "[%0.3f] Timeserver sync request seqNo#%d started",
              millis() / 1000.0, time_sync_seqNo);
     sample_idx = 0;
-    xTaskNotifyGive(timeSyncReqTask);
+    xTaskNotifyGive(timeSyncProcTask);
   }
 }
 
 // task for processing time sync request
 void IRAM_ATTR timesync_processReq(void *taskparameter) {
 
-  uint32_t rcv_seq_no = TIMEREQUEST_FINISH, time_offset_ms;
+  uint32_t seqNo = TIMEREQUEST_END, time_offset_ms;
 
   //  this task is an endless loop, waiting in blocked mode, until it is
   //  unblocked by timesync_sendReq(). It then waits to be notified from
-  //  recv_timesync_ans(), which is called from RX callback in lorawan.cpp, each
+  //  timesync_serverAnswer(), which is called from LMIC each time a timestamp 
-  //  time a timestamp from timeserver arrived.
+  //  from the timesource via LORAWAN arrived.
 
   // --- asnychronous part: generate and collect timestamps from gateway ---
 
@@ -90,49 +90,45 @@ void IRAM_ATTR timesync_processReq(void *taskparameter) {
 // send timesync request to timeserver or networkserver
 #if (TIME_SYNC_LORASERVER)
       // timesync option 1: use external timeserver (for LoRAWAN < 1.0.3)
+      // ask timeserver
       payload.reset();
       payload.addByte(time_sync_seqNo);
       SendPayload(TIMEPORT, prio_high);
 #elif (TIME_SYNC_LORAWAN)
       // timesync option 2: use LoRAWAN network time (requires LoRAWAN >= 1.0.3)
-      LMIC_requestNetworkTime(DevTimeAns_Cb, &time_sync_seqNo);
+      // ask networkserver
-      // open a receive window to trigger DevTimeAns
+      LMIC_requestNetworkTime(timesync_serverAnswer, &time_sync_seqNo);
-      LMIC_sendAlive();
 #endif
 
+      // open a receive window to immediately get the answer (Class A device)
+      LMIC_sendAlive();
+
       // wait until a timestamp was received
-      while (rcv_seq_no != time_sync_seqNo) {
+      if (xTaskNotifyWait(0x00, ULONG_MAX, &seqNo,
-        if (xTaskNotifyWait(0x00, ULONG_MAX, &rcv_seq_no,
+                          pdMS_TO_TICKS(TIME_SYNC_TIMEOUT * 1000)) == pdFALSE) {
-                            pdMS_TO_TICKS(TIME_SYNC_TIMEOUT * 1000)) ==
-            pdFALSE) {
         ESP_LOGW(TAG, "[%0.3f] Timesync handshake error: timeout",
                  millis() / 1000.0);
-          goto finish; // no valid sequence received before timeout
+        goto Fail; // no valid sequence received before timeout
-        }
       }
 
-      // calculate time diff from received timestamp
+      // check if we are in handshake with server
+      if (seqNo != time_sync_seqNo) {
+        ESP_LOGW(TAG, "[%0.3f] Timesync handshake aborted", millis() / 1000.0);
+        goto Fail;
+      }
+
+      // calculate time diff with received timestamp
       time_offset_ms += timesync_timestamp[sample_idx][timesync_rx] -
                         timesync_timestamp[sample_idx][timesync_tx];
 
-      // increment and maybe wrap around seqNo, keeping it in time port range
+      // increment and wrap around seqNo, keeping it in time port range
       WRAP(time_sync_seqNo, TIMEREQUEST_MAX_SEQNO);
       // increment index for timestamp array
       sample_idx++;
 
       // if last cycle, finish after, else pause until next cycle
-      if (i < TIME_SYNC_SAMPLES - 1) { // wait for next cycle
+      if (i < TIME_SYNC_SAMPLES - 1)
         vTaskDelay(pdMS_TO_TICKS(TIME_SYNC_CYCLE * 1000));
-      } else {
-#if (TIME_SYNC_LORASERVER)
-        // send finish char for closing timesync handshake
-        payload.reset();
-        payload.addByte(TIMEREQUEST_FINISH);
-        SendPayload(RCMDPORT, prio_high);
-        // open a receive window to get last time_sync_answer instantly
-        LMIC_sendAlive();
-#endif
-      }
 
     } // end of for loop to collect timestamp samples
 
@@ -154,12 +150,20 @@ void IRAM_ATTR timesync_processReq(void *taskparameter) {
                   time_offset_ms / 1000,
               time_offset_ms % 1000, _lora);
 
+    // send timerequest end char to show timesync was successful
+    payload.reset();
+    payload.addByte(TIMEREQUEST_END);
+    SendPayload(RCMDPORT, prio_high);
+    goto Finish;
+
+  Fail:
+    // set retry timer
+    timesyncer.attach(TIME_SYNC_INTERVAL_RETRY * 60, timeSync);
+
+  Finish:
     // end of time critical section: release app irq lock
     unmask_user_IRQ();
 
-  finish:
-    timeSyncPending = false;
-
   } // infinite while(1)
 }
 
@@ -172,10 +176,26 @@ void timesync_storeReq(uint32_t timestamp, timesync_t timestamp_type) {
   timesync_timestamp[sample_idx][timestamp_type] = timestamp;
 }
 
+// callback function to receive network time server answer
+void IRAM_ATTR timesync_serverAnswer(void *pUserData, int flag) {
+  // if no timesync handshake is pending then exit
+  if (!timeSyncPending)
+    return;
+
+  // mask application irq to ensure accurate timing
+  mask_user_IRQ();
+
+  int rc = 0;
+  uint32_t timestamp_sec;
+  uint16_t timestamp_msec;
+
 #if (TIME_SYNC_LORASERVER)
-// evaluate timerserver's timestamp answer, called by myRxCallback() in
+
-// lorawan.cpp
+  // store LMIC time when we received the timesync answer
-int recv_timeserver_ans(const uint8_t buf[], const uint8_t buf_len) {
+  timesync_storeReq(osticks2ms(os_getTime()), timesync_rx);
+
+  // pUserData: contains pointer to payload buffer
+  // flag: length of buffer
 
   /*
     parse 6 byte timesync_answer:
@@ -186,72 +206,55 @@ int recv_timeserver_ans(const uint8_t buf[], const uint8_t buf_len) {
     6       1/250ths fractions of current second
     */
 
-  // if no timesync handshake is pending then exit
+  // Explicit conversion from void* to uint8_t* to avoid compiler errors
-  if (!timeSyncPending)
+  uint8_t *p = (uint8_t *)pUserData;
-    return 0; // failure
+  // Get payload buffer from pUserData
+  uint8_t *buf = p;
 
   // extract 1 byte timerequest sequence number from payload
   uint8_t seqNo = buf[0];
   buf++;
 
   // if no time is available or spurious buffer then exit
-  if (buf_len != TIME_SYNC_FRAME_LENGTH) {
+  if (flag != TIME_SYNC_FRAME_LENGTH) {
-    if (seqNo == 0xff)
+    if (seqNo == TIMEREQUEST_END)
       ESP_LOGI(TAG, "[%0.3f] Timeserver error: no confident time available",
                millis() / 1000.0);
     else
       ESP_LOGW(TAG, "[%0.3f] Timeserver error: spurious data received",
                millis() / 1000.0);
-    return 0; // failure
+    goto Exit; // failure
   }
 
-  else { // we received a probably valid time frame
+  // pointer to 4 bytes msb order
-
+  uint32_t *timestamp_ptr;
-    // pointers to 4 bytes msb order
-    uint32_t timestamp_sec, *timestamp_ptr;
-
   // extract 4 bytes containing gateway time in UTC seconds since unix
   // epoch and convert it to uint32_t, octet order is big endian
   timestamp_ptr = (uint32_t *)buf;
   // swap byte order from msb to lsb, note: this is a platform dependent hack
   timestamp_sec = __builtin_bswap32(*timestamp_ptr);
   buf += 4;
-
   // extract 1 byte containing fractional seconds in 2^-8 second steps
   // one step being 1/250th sec * 1000 = 4msec
-    uint16_t timestamp_msec = buf[0] * 4;
+  timestamp_msec = buf[0] * 4;
-    // calculate absolute time received from gateway
-    time_t t = timestamp_sec + timestamp_msec / 1000;
 
-    // we guess timepoint is recent if it is newer than code compile date
+  goto Finish;
-    if (timeIsValid(t)) {
-      ESP_LOGD(TAG, "[%0.3f] Timesync request seq#%d rcvd at %0.3f",
-               millis() / 1000.0, seqNo, osticks2ms(os_getTime()) / 1000.0);
-
-      // store time received from gateway
-      timesync_storeReq(timestamp_sec, gwtime_sec);
-      timesync_storeReq(timestamp_msec, gwtime_msec);
-
-      // inform processing task
-      xTaskNotify(timeSyncReqTask, seqNo, eSetBits);
-
-      return 1; // success
-    } else {
-      ESP_LOGW(TAG, "[%0.3f] Timeserver error: outdated time received",
-               millis() / 1000.0);
-      return 0; // failure
-    }
-  }
-}
 
 #elif (TIME_SYNC_LORAWAN)
 
-void IRAM_ATTR DevTimeAns_Cb(void *pUserData, int flagSuccess) {
+  // pUserData: contains pointer to SeqNo
-  // Explicit conversion from void* to uint8_t* to avoid compiler errors
+  // flagSuccess: indicates if we got a recent time from the network
-  uint8_t *seqNo = (uint8_t *)pUserData;
 
-  // mask application irq to ensure accurate timing
+  // Explicit conversion from void* to uint8_t* to avoid compiler errors
-  mask_user_IRQ();
+  uint8_t *p = (uint8_t *)pUserData;
+  // Get seqNo from pUserData
+  uint8_t seqNo = *p;
+
+  if (flag != 1) {
+    ESP_LOGW(TAG, "[%0.3f] Network did not answer time request",
+             millis() / 1000.0);
+    goto Exit;
+  }
 
   // A struct that will be populated by LMIC_getNetworkTimeReference.
   // It contains the following fields:
@@ -261,39 +264,38 @@ void IRAM_ATTR DevTimeAns_Cb(void *pUserData, int flagSuccess) {
   //              the gateway received the time request
   lmic_time_reference_t lmicTime;
 
-  if (flagSuccess != 1) {
-    ESP_LOGW(TAG, "Network did not answer time request");
-    goto Finish;
-  }
-
-  if (time_sync_seqNo != *seqNo) {
-    ESP_LOGW(TAG, "Network timesync handshake failed, seqNo#%u, *seqNo");
-    goto Finish;
-  }
-
   // Populate lmic_time_reference
   if ((LMIC_getNetworkTimeReference(&lmicTime)) != 1) {
-    ESP_LOGW(TAG, "Network time request failed");
+    ESP_LOGW(TAG, "[%0.3f] Network time request failed", millis() / 1000.0);
+    goto Exit;
+  }
+
+  // Calculate UTCTime, considering the difference between GPS and UTC time
+  timestamp_sec = lmicTime.tNetwork + GPS_UTC_DIFF;
+  // Add delay between the instant the time was transmitted and the current time
+  timestamp_msec = osticks2ms(os_getTime() - lmicTime.tLocal);
   goto Finish;
-}
 
-// Calculate UTCTime, considering the difference between GPS and UTC time
+#endif // (TIME_SYNC_LORAWAN)
-timestamp_sec = lmicTime.tNetwork + GPS_UTC_DIFF;
-// Add delay between the instant the time was transmitted and the current time
-timestamp_msec = osticks2ms(os_getTime() - lmicTime.tLocal);
 
-// store time received from gateway
+Finish:
-timesync_storeReq(timestamp_sec, gwtime_sec);
+  // check if calucalted time is recent
-timesync_storeReq(timestamp_msec, gwtime_msec);
+  if (timeIsValid(timestamp_sec)) {
+    // store time received from gateway
+    timesync_storeReq(timestamp_sec, gwtime_sec);
+    timesync_storeReq(timestamp_msec, gwtime_msec);
+    // success
+    rc = 1;
+  } else {
+    ESP_LOGW(TAG, "[%0.3f] Timeserver error: outdated time received",
+             millis() / 1000.0);
+  }
 
-// inform processing task
+Exit:
-xTaskNotify(timeSyncReqTask, *seqNo, eSetBits);
-
-Finish :
   // end of time critical section: release app irq lock
   unmask_user_IRQ();
+  // inform processing task
+  xTaskNotify(timeSyncProcTask, rc ? seqNo : TIMEREQUEST_END, eSetBits);
 }
 
-#endif
-
 #endif // HAS_LORA