From 42ee21a2f78689078a01f9e223774a83868f0622 Mon Sep 17 00:00:00 2001
From: Klaus K Wilting <klaus.k.wilting@deutschebahn.com>
Date: Wed, 11 Mar 2020 23:49:06 +0100
Subject: [PATCH] timesync code sanitziations

---
 include/timesync.h |  4 ++--
 src/lorawan.cpp    |  2 +-
 src/timekeeper.cpp | 25 +++++++++----------------
 src/timesync.cpp   | 22 +++++++++++++---------
 4 files changed, 25 insertions(+), 28 deletions(-)

diff --git a/include/timesync.h b/include/timesync.h
index dd385029..e2a8d9c4 100644
--- a/include/timesync.h
+++ b/include/timesync.h
@@ -20,8 +20,8 @@ enum timesync_t {
 };
 
 void timesync_init(void);
-void timesync_sendReq(void);
-void timesync_storeReq(uint32_t timestamp, timesync_t timestamp_type);
+void timesync_request(void);
+void timesync_store(uint32_t timestamp, timesync_t timestamp_type);
 void IRAM_ATTR timesync_processReq(void *taskparameter);
 void IRAM_ATTR timesync_serverAnswer(void *pUserData, int flag);
 
diff --git a/src/lorawan.cpp b/src/lorawan.cpp
index 933a1ffd..ee9c525c 100644
--- a/src/lorawan.cpp
+++ b/src/lorawan.cpp
@@ -251,7 +251,7 @@ void lora_send(void *pvParameters) {
         // if last packet sent was a timesync request, store TX timestamp
         if (SendBuffer.MessagePort == TIMEPORT)
           // store LMIC time when we started transmit of timesync request
-          timesync_storeReq(osticks2ms(os_getTime()), timesync_tx);
+          timesync_store(osticks2ms(os_getTime()), timesync_tx);
 #endif
 
         ESP_LOGI(TAG, "%d byte(s) sent to LORA", SendBuffer.MessageSize);
diff --git a/src/timekeeper.cpp b/src/timekeeper.cpp
index 4e788d04..ada140a6 100644
--- a/src/timekeeper.cpp
+++ b/src/timekeeper.cpp
@@ -30,30 +30,27 @@ void calibrateTime(void) {
 
   // kick off asychronous lora timesync if we have
 #if (HAS_LORA) && (TIME_SYNC_LORASERVER) || (TIME_SYNC_LORAWAN)
-  timesync_sendReq();
+  timesync_request();
 #endif
 
-  // only if we lost time, we try to fallback to local time source RTS or GPS
+  // (only!) if we lost time, we try to fallback to local time source RTS or GPS
   if (timeSource == _unsynced) {
 
 // has RTC -> fallback to RTC time
 #ifdef HAS_RTC
     t = get_rtctime();
-    if (t) {
-      timeSource = _rtc;
-      goto finish;
-    }
+    timeSource = _rtc;
 #endif
 
 // no RTC -> fallback to GPS time
 #if (HAS_GPS)
-    // fetch recent time from last NMEA record
-    t = fetch_gpsTime(&t_msec);
-    if (t) {
-      timeSource = _gps;
-      goto finish;
-    }
+    t = get_gpstime(&t_msec);
+    timeSource = _gps;
 #endif
+
+    if (t)
+      setMyTime((uint32_t)t, t_msec, timeSource); // set time
+
   } // fallback
 
   else
@@ -61,10 +58,6 @@ void calibrateTime(void) {
     // no fallback time source available -> we can't set time
     return;
 
-finish:
-
-  setMyTime((uint32_t)t, t_msec, timeSource); // set time
-
 } // calibrateTime()
 
 // adjust system time, calibrate RTC and RTC_INT pps
diff --git a/src/timesync.cpp b/src/timesync.cpp
index ce2c5e5a..d9937922 100644
--- a/src/timesync.cpp
+++ b/src/timesync.cpp
@@ -42,7 +42,7 @@ void timesync_init() {
 }
 
 // kickoff asnychronous timesync handshake
-void timesync_sendReq(void) {
+void timesync_request(void) {
   // exit if a timesync handshake is already running
   if (timeSyncPending)
     return;
@@ -60,7 +60,7 @@ void IRAM_ATTR timesync_processReq(void *taskparameter) {
   uint32_t rcv_seqNo = TIME_SYNC_END_FLAG, 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
+  //  unblocked by timesync_request(). It then waits to be notified from
   //  timesync_serverAnswer(), which is called from LMIC each time a timestamp
   //  from the timesource via LORAWAN arrived.
 
@@ -158,7 +158,7 @@ void IRAM_ATTR timesync_processReq(void *taskparameter) {
 }
 
 // store incoming timestamps
-void timesync_storeReq(uint32_t timestamp, timesync_t timestamp_type) {
+void timesync_store(uint32_t timestamp, timesync_t timestamp_type) {
   ESP_LOGD(TAG, "[%0.3f] seq#%d[%d]: timestamp(t%d)=%d", millis() / 1000.0,
            time_sync_seqNo, sample_idx, timestamp_type, timestamp);
   timesync_timestamp[sample_idx][timestamp_type] = timestamp;
@@ -174,6 +174,9 @@ void IRAM_ATTR timesync_serverAnswer(void *pUserData, int flag) {
   // mask application irq to ensure accurate timing
   mask_user_IRQ();
 
+  // store LMIC time when we received the timesync answer
+  ostime_t rxTime = osticks2ms(os_getTime());
+
   int rc = 0;
   uint8_t rcv_seqNo = *(uint8_t *)pUserData;
   uint16_t timestamp_msec = 0;
@@ -184,8 +187,8 @@ void IRAM_ATTR timesync_serverAnswer(void *pUserData, int flag) {
   // pUserData: contains pointer to payload buffer
   // flag: length of buffer
 
-  // store LMIC time when we received the timesync answer
-  timesync_storeReq(osticks2ms(os_getTime()), timesync_rx);
+  // Store the instant the time request of the node was received on the gateway
+  timesync_store(rxTime, timesync_rx);
 
   //  parse timesync_answer:
   //  byte    meaning
@@ -240,8 +243,9 @@ void IRAM_ATTR timesync_serverAnswer(void *pUserData, int flag) {
 
   // 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);
+  // Add delay between the instant the time was received on the gateway and the
+  // current time on the node
+  timestamp_msec = rxTime - lmicTime.tLocal;
   goto Finish;
 
 #endif // (TIME_SYNC_LORAWAN)
@@ -250,8 +254,8 @@ Finish:
   // check if calculated time is recent
   if (timeIsValid(timestamp_sec)) {
     // store time received from gateway
-    timesync_storeReq(timestamp_sec, gwtime_sec);
-    timesync_storeReq(timestamp_msec, gwtime_msec);
+    timesync_store(timestamp_sec, gwtime_sec);
+    timesync_store(timestamp_msec, gwtime_msec);
     // success
     rc = 1;
   } else {