Merge pull request #261 from cyberman54/development

bugfixes for time/date handling and i2c bus access handling

include/globals.h

@@ -40,7 +40,7 @@
 #define SCREEN_MODE (0x80)
 
 // I2C bus access control
-#define I2C_MUTEX_LOCK()    xSemaphoreTake(I2Caccess, (DISPLAYREFRESH_MS / portTICK_PERIOD_MS)) == pdTRUE
+#define I2C_MUTEX_LOCK()    xSemaphoreTake(I2Caccess, (3 * DISPLAYREFRESH_MS / portTICK_PERIOD_MS)) == pdTRUE
 #define I2C_MUTEX_UNLOCK()  xSemaphoreGive(I2Caccess)
 
 // Struct holding devices's runtime configuration

include/rtctime.h

@@ -9,19 +9,11 @@
 #include "gpsread.h"
 #endif
 
-typedef enum {
-  useless = 0,     // waiting for good enough signal
-  dirty = 1,       // time data available but inconfident
-  reserve = 2,     // clock was once synced but now may deviate
-  synced_LORA = 3, // clock driven by LORAWAN network
-  synced_GPS = 4   // best possible quality, clock is driven by GPS
-} clock_state_t;
-
 extern RtcDS3231<TwoWire> Rtc; // make RTC instance globally available
 
 int rtc_init(void);
-int set_rtctime(uint32_t UTCTime);
+int set_rtctime(uint32_t t);
-int set_rtctime(RtcDateTime now);
+int set_rtctime(time_t t);
 void sync_rtctime(void);
 time_t get_rtctime(void);
 float get_rtctemp(void);

src/cyclic.cpp

@@ -9,6 +9,7 @@ static const char TAG[] = "main";
 
 time_t userUTCTime; // Seconds since the UTC epoch
 unsigned long nextLoraTimeSync = millis();
+unsigned long nextRTCTimeSync = millis() + TIME_WRITE_INTERVAL_RTC * 60000;
 
 // do all housekeeping
 void doHousekeeping() {
@@ -34,6 +35,19 @@ void doHousekeeping() {
   }
 #endif
 
+// do cyclic write back system time to RTC if we have an external time source
+#if (defined TIME_SYNC_INTERVAL_LORA || defined TIME_SYNC_INTERVAL_GPS) &&     \
+    defined HAS_RTC
+  if ((millis() >= nextRTCTimeSync) && (timeStatus() == timeSet)) {
+    nextRTCTimeSync = millis() + TIME_WRITE_INTERVAL_RTC *
+                                     60000; // set up next time sync period
+    if (!set_rtctime(now())) // epoch time
+      ESP_LOGE(TAG, "RTC set time failure");
+    else
+      ESP_LOGI(TAG, "RTC time updated");
+  }
+#endif
+
   // task storage debugging //
   ESP_LOGD(TAG, "Wifiloop %d bytes left | Taskstate = %d",
            uxTaskGetStackHighWaterMark(wifiSwitchTask),

src/display.cpp

@@ -59,66 +59,72 @@ void DisplayKey(const uint8_t *key, uint8_t len, bool lsb) {
 
 void init_display(const char *Productname, const char *Version) {
 
-  // show startup screen
+  // block i2c bus access
-  uint8_t buf[32];
+  if (I2C_MUTEX_LOCK()) {
-  u8x8.begin();
-  u8x8.setFont(u8x8_font_chroma48medium8_r);
-  u8x8.clear();
-  u8x8.setFlipMode(0);
-  u8x8.setInverseFont(1);
-  u8x8.draw2x2String(0, 0, Productname);
-  u8x8.setInverseFont(0);
-  u8x8.draw2x2String(2, 2, Productname);
-  delay(1500);
-  u8x8.clear();
-  u8x8.setFlipMode(1);
-  u8x8.setInverseFont(1);
-  u8x8.draw2x2String(0, 0, Productname);
-  u8x8.setInverseFont(0);
-  u8x8.draw2x2String(2, 2, Productname);
-  delay(1500);
 
-  u8x8.setFlipMode(0);
+    // show startup screen
-  u8x8.clear();
+    uint8_t buf[32];
+    u8x8.begin();
+    u8x8.setFont(u8x8_font_chroma48medium8_r);
+    u8x8.clear();
+    u8x8.setFlipMode(0);
+    u8x8.setInverseFont(1);
+    u8x8.draw2x2String(0, 0, Productname);
+    u8x8.setInverseFont(0);
+    u8x8.draw2x2String(2, 2, Productname);
+    delay(500);
+    u8x8.clear();
+    u8x8.setFlipMode(1);
+    u8x8.setInverseFont(1);
+    u8x8.draw2x2String(0, 0, Productname);
+    u8x8.setInverseFont(0);
+    u8x8.draw2x2String(2, 2, Productname);
+    delay(500);
+
+    u8x8.setFlipMode(0);
+    u8x8.clear();
 
 #ifdef DISPLAY_FLIP
-  u8x8.setFlipMode(1);
+    u8x8.setFlipMode(1);
 #endif
 
 // Display chip information
 #ifdef VERBOSE
-  esp_chip_info_t chip_info;
+    esp_chip_info_t chip_info;
-  esp_chip_info(&chip_info);
+    esp_chip_info(&chip_info);
-  u8x8.printf("ESP32 %d cores\nWiFi%s%s\n", chip_info.cores,
+    u8x8.printf("ESP32 %d cores\nWiFi%s%s\n", chip_info.cores,
-              (chip_info.features & CHIP_FEATURE_BT) ? "/BT" : "",
+                (chip_info.features & CHIP_FEATURE_BT) ? "/BT" : "",
-              (chip_info.features & CHIP_FEATURE_BLE) ? "/BLE" : "");
+                (chip_info.features & CHIP_FEATURE_BLE) ? "/BLE" : "");
-  u8x8.printf("ESP Rev.%d\n", chip_info.revision);
+    u8x8.printf("ESP Rev.%d\n", chip_info.revision);
-  u8x8.printf("%dMB %s Flash\n", spi_flash_get_chip_size() / (1024 * 1024),
+    u8x8.printf("%dMB %s Flash\n", spi_flash_get_chip_size() / (1024 * 1024),
-              (chip_info.features & CHIP_FEATURE_EMB_FLASH) ? "int." : "ext.");
+                (chip_info.features & CHIP_FEATURE_EMB_FLASH) ? "int."
+                                                              : "ext.");
 #endif // VERBOSE
 
-  u8x8.print(Productname);
+    u8x8.print(Productname);
-  u8x8.print(" v");
+    u8x8.print(" v");
-  u8x8.println(PROGVERSION);
+    u8x8.println(PROGVERSION);
 
 #ifdef HAS_LORA
-  u8x8.println("DEVEUI:");
+    u8x8.println("DEVEUI:");
-  os_getDevEui((u1_t *)buf);
+    os_getDevEui((u1_t *)buf);
-  DisplayKey(buf, 8, true);
+    DisplayKey(buf, 8, true);
+    delay(3000);
 #endif // HAS_LORA
-
+    u8x8.clear();
-  delay(3000);
+    u8x8.setPowerSave(!cfg.screenon); // set display off if disabled
-  u8x8.clear();
+    u8x8.draw2x2String(0, 0, "PAX:0");
-  u8x8.setPowerSave(!cfg.screenon); // set display off if disabled
-  u8x8.draw2x2String(0, 0, "PAX:0");
 #ifdef BLECOUNTER
-  u8x8.setCursor(0, 3);
+    u8x8.setCursor(0, 3);
-  u8x8.printf("BLTH:0");
+    u8x8.printf("BLTH:0");
 #endif
-  u8x8.setCursor(0, 4);
+    u8x8.setCursor(0, 4);
-  u8x8.printf("WIFI:0");
+    u8x8.printf("WIFI:0");
-  u8x8.setCursor(0, 5);
+    u8x8.setCursor(0, 5);
-  u8x8.printf(!cfg.rssilimit ? "RLIM:off " : "RLIM:%d", cfg.rssilimit);
+    u8x8.printf(!cfg.rssilimit ? "RLIM:off " : "RLIM:%d", cfg.rssilimit);
+
+    I2C_MUTEX_UNLOCK(); // release i2c bus access
+  }
 
 } // init_display
 

src/gpsread.cpp

@@ -70,9 +70,12 @@ time_t tmConvert_t(uint16_t YYYY, uint8_t MM, uint8_t DD, uint8_t hh,
 time_t get_gpstime(void) {
   // never call now() in this function, this would cause a recursion!
   time_t t = 0;
-  if (gps.time.age() < 1500) {
+  if ((gps.time.age() < 1500) && (gps.time.isValid())) {
     t = tmConvert_t(gps.date.year(), gps.date.month(), gps.date.day(),
                     gps.time.hour(), gps.time.minute(), gps.time.second());
+    ESP_LOGD(TAG, "GPS time: %d/%d/%d %d:%d:%d", gps.date.year(),
+             gps.date.month(), gps.date.day(), gps.time.hour(),
+             gps.time.minute(), gps.time.second());
   } else {
     ESP_LOGW(TAG, "GPS has no confident time");
   }

src/if482.cpp

@@ -95,17 +95,22 @@ int if482_init(void) {
   IF482.begin(HAS_IF482);
 
   // use external rtc 1Hz clock for triggering IF482 telegram
-  Rtc.SetSquareWavePinClockFrequency(DS3231SquareWaveClock_1Hz);
+  if (I2C_MUTEX_LOCK()) {
-  Rtc.SetSquareWavePin(DS3231SquareWavePin_ModeClock);
+    Rtc.SetSquareWavePinClockFrequency(DS3231SquareWaveClock_1Hz);
+    Rtc.SetSquareWavePin(DS3231SquareWavePin_ModeClock);
+    I2C_MUTEX_UNLOCK();
+  } else {
+    ESP_LOGE(TAG, "I2c bus busy - IF482 initialization error");
+    return 0;
+  }
   pinMode(RTC_INT, INPUT_PULLUP);
-
-  ESP_LOGI(TAG, "IF482 generator initialized");
-
   return 1;
 
 } // if482_init
 
-String if482Telegram(time_t t) {
+String if482Telegram(time_t tt) {
+
+  time_t t = myTZ.toLocal(tt);
 
   char mon;
   char buf[14] = "000000F000000";
@@ -123,7 +128,8 @@ String if482Telegram(time_t t) {
     break;
   } // switch
 
-  if (!timeNotSet) // do we have valid time?
+  if ((timeStatus() == timeSet) ||
+      (timeStatus() == timeNeedsSync)) // do we have valid time?
     snprintf(buf, sizeof buf, "%02u%02u%02u%1u%02u%02u%02u", year(t) - 2000,
              month(t), day(t), weekday(t), hour(t), minute(t), second(t));
 
@@ -137,15 +143,18 @@ void if482_loop(void *pvParameters) {
 
   TickType_t wakeTime;
   time_t t, tt;
-  const TickType_t shotTime = pdMS_TO_TICKS(IF482_OFFSET);
+  const TickType_t timeOffset =
+      pdMS_TO_TICKS(IF482_OFFSET); // duration of telegram transmit
+  const TickType_t startTime = xTaskGetTickCount(); // now
 
-  // wait until begin of a new second to sync clock signal and absolute time
+  // wait until begin of a new second
   t = tt = now();
   do {
     tt = now();
   } while (t == tt);
 
-  const TickType_t startOffset = xTaskGetTickCount();
+  // take timestamp at moment of start of new second
+  const TickType_t shotTime = xTaskGetTickCount() - startTime - timeOffset;
 
   // task remains in blocked state until it is notified by isr
   for (;;) {
@@ -155,13 +164,10 @@ void if482_loop(void *pvParameters) {
         &wakeTime,      // receives moment of call from isr
         portMAX_DELAY); // wait forever (missing error handling here...)
 
-    t = myTZ.toLocal(now());
+    // now we're synced to start of second tt and wait
-    wakeTime -= startOffset;
+    // until it's time to start transmit telegram for tt+1
-
+    vTaskDelayUntil(&wakeTime, shotTime); // sets waketime to moment of shot
-    // now we're synced to start of second t and wait
+    IF482.print(if482Telegram(now() + 1));
-    // until it's time to start transmit telegram for t+1
-    vTaskDelayUntil(&wakeTime, shotTime);
-    IF482.print(if482Telegram(t + 1));
   }
   vTaskDelete(IF482Task); // shoud never be reached
 } // if482_loop()

src/lmic_config.h

@@ -34,7 +34,7 @@
 // faster or slower. This causes the transceiver to be earlier switched on,
 // so consuming more power. You may sharpen (reduce) this value if you are
 // limited on battery.
-#define CLOCK_ERROR_PROCENTAGE 3
+#define CLOCK_ERROR_PROCENTAGE 30
 
 // Set this to 1 to enable some basic debug output (using printf) about
 // RF settings used during transmission and reception. Set to 2 to

src/lorawan.cpp

@@ -457,7 +457,7 @@ void user_request_network_time_callback(void *pVoidUserUTCTime,
   setTime(*pUserUTCTime);
   ESP_LOGI(TAG, "LoRaWAN network has set the system time");
 #ifdef HAS_RTC
-  if (set_rtctime(*pUserUTCTime))
+  if (!set_rtctime(*pUserUTCTime)) // epoch time
     ESP_LOGE(TAG, "RTC set time failure");
 #endif
 }

src/main.cpp

@@ -92,15 +92,11 @@ void setup() {
 
   char features[100] = "";
 
-  if (I2Caccess == NULL) // Check that semaphore has not already been created
+  I2Caccess = xSemaphoreCreateMutex(); // for access management of i2c bus
-  {
+  if ((I2Caccess) != NULL)
-    I2Caccess = xSemaphoreCreateMutex(); // Create a mutex semaphore we will use
+    xSemaphoreGive((I2Caccess)); // Flag the i2c bus available for use
-                                         // to manage the i2c bus
-    if ((I2Caccess) != NULL)
-      xSemaphoreGive((I2Caccess)); // Flag the i2c bus available for use
-  }
 
-  // disable brownout detection
+    // disable brownout detection
 #ifdef DISABLE_BROWNOUT
   // register with brownout is at address DR_REG_RTCCNTL_BASE + 0xd4
   (*((uint32_t volatile *)ETS_UNCACHED_ADDR((DR_REG_RTCCNTL_BASE + 0xd4)))) = 0;
@@ -188,31 +184,6 @@ void setup() {
                           0);           // CPU core
 #endif
 
-// initialize RTC
-#ifdef HAS_RTC
-  strcat_P(features, " RTC");
-  assert(rtc_init());
-  setSyncProvider(&get_rtctime);
-  if (timeStatus() != timeSet)
-    ESP_LOGI(TAG, "Unable to sync system time with RTC");
-  else
-    ESP_LOGI(TAG, "RTC has set the system time");
-  setSyncInterval(TIME_SYNC_INTERVAL_RTC * 60);
-#endif // HAS_RTC
-
-#ifdef HAS_IF482
-  strcat_P(features, " IF482");
-  assert(if482_init());
-  ESP_LOGI(TAG, "Starting IF482 Generator...");
-  xTaskCreatePinnedToCore(if482_loop,  // task function
-                          "if482loop", // name of task
-                          2048,        // stack size of task
-                          (void *)1,   // parameter of the task
-                          3,           // priority of the task
-                          &IF482Task,  // task handle
-                          0);          // CPU core
-#endif                                 // HAS_IF482
-
 // initialize wifi antenna
 #ifdef HAS_ANTENNA_SWITCH
   strcat_P(features, " ANT");
@@ -312,7 +283,7 @@ void setup() {
 #ifdef HAS_DISPLAY
   strcat_P(features, " OLED");
   DisplayState = cfg.screenon;
-  init_display(PRODUCTNAME, PROGVERSION);
+  init_display(PRODUCTNAME, PROGVERSION); // note: blocking call
 
   // setup display refresh trigger IRQ using esp32 hardware timer
   // https://techtutorialsx.com/2017/10/07/esp32-arduino-timer-interrupts/
@@ -350,6 +321,18 @@ void setup() {
   strcat_P(features, " LPPPKD");
 #endif
 
+// initialize RTC
+#ifdef HAS_RTC
+  strcat_P(features, " RTC");
+  assert(rtc_init());
+  setSyncProvider(&get_rtctime);
+  if (timeStatus() != timeSet)
+    ESP_LOGI(TAG, "Unable to sync system time with RTC");
+  else
+    ESP_LOGI(TAG, "RTC has set the system time");
+  setSyncInterval(TIME_SYNC_INTERVAL_RTC * 60);
+#endif // HAS_RTC
+
   // show compiled features
   ESP_LOGI(TAG, "Features:%s", features);
 
@@ -429,18 +412,27 @@ void setup() {
   else {
     ESP_LOGI(TAG, "GPS has set the system time");
 #ifdef HAS_RTC
-    if (set_rtctime(now()))
+    if (!set_rtctime(now())) // epoch time
       ESP_LOGE(TAG, "RTC set time failure");
 #endif
   }
   setSyncInterval(TIME_SYNC_INTERVAL_GPS * 60);
 #endif
 
-// start RTC interrupt
 #if defined HAS_IF482 && defined RTC_INT
+  strcat_P(features, " IF482");
+  assert(if482_init());
+  ESP_LOGI(TAG, "Starting IF482 Generator...");
+  xTaskCreatePinnedToCore(if482_loop,  // task function
+                          "if482loop", // name of task
+                          2048,        // stack size of task
+                          (void *)1,   // parameter of the task
+                          3,           // priority of the task
+                          &IF482Task,  // task handle
+                          0);          // CPU core
+
   // setup external interupt for active low RTC INT pin
   assert(IF482Task != NULL); // has if482loop task started?
-  ESP_LOGI(TAG, "Starting IF482 output...");
   attachInterrupt(digitalPinToInterrupt(RTC_INT), IF482IRQ, FALLING);
 #endif
 

src/paxcounter.conf

@@ -82,9 +82,10 @@
 #define RESPONSE_TIMEOUT_MS             60000   // firmware binary server connection timeout [milliseconds]
 
 // settings for syncing time of node and external time sources
-#define TIME_SYNC_INTERVAL_GPS          60      // sync time each ... minutes with GPS [default = 60], comment out means off          
+#define TIME_SYNC_INTERVAL_GPS          60      // sync time each .. minutes from source GPS [default = 60], comment out means off          
-#define TIME_SYNC_INTERVAL_RTC          60      // sync time each ... minutes with RTC [default = 60], comment out means off          
+#define TIME_SYNC_INTERVAL_RTC          60      // sync time each .. minutes from RTC [default = 60], comment out means off          
-//#define TIME_SYNC_INTERVAL_LORA       60      // sync time each ... minutes with LORA network [default = 60], comment out means off          
+#define TIME_WRITE_INTERVAL_RTC         60      // write time each .. minutes from GPS/LORA to RTC [default = 60], comment out means off          
+//#define TIME_SYNC_INTERVAL_LORA       60      // sync time each .. minutes from LORA network [default = 60], comment out means off          
 #define IF482_OFFSET                    984     // 1sec minus IF482 serial transmit time [ms]: e.g. 9 bits * 17 bytes * 1/9600 bps = 16ms
 
 // time zone, see https://github.com/JChristensen/Timezone/blob/master/examples/WorldClock/WorldClock.ino

src/rtctime.cpp

@@ -57,27 +57,19 @@ error:
 
 } // rtc_init()
 
-int set_rtctime(uint32_t t) {
+int set_rtctime(time_t t) { // t is epoch time starting 1.1.1970
-  // return = 0 -> error / return = 1 -> success
-  // block i2c bus access
   if (I2C_MUTEX_LOCK()) {
     Rtc.SetDateTime(RtcDateTime(t));
     I2C_MUTEX_UNLOCK(); // release i2c bus access
-    return 1;
+    return 1;           // success
   }
-  return 0;
+  return 0; // failure
 } // set_rtctime()
 
-int set_rtctime(RtcDateTime t) {
+int set_rtctime(uint32_t t) { // t is epoch seconds starting 1.1.1970
-  // return = 0 -> error / return = 1 -> success
+  return set_rtctime(static_cast<time_t>(t));
-  // block i2c bus access
+  // set_rtctime()
-  if (I2C_MUTEX_LOCK()) {
+}
-    Rtc.SetDateTime(t);
-    I2C_MUTEX_UNLOCK(); // release i2c bus access
-    return 1;
-  }
-  return 0;
-} // set_rtctime()
 
 time_t get_rtctime(void) {
   // never call now() in this function, this would cause a recursion!