Continuous time update - Alternative implementation to #2041

This is an alternative implementation to #2041 we talked about in this comment (#2041 (comment)).

This implementation does not change the state of the DateTime controller (previousSystickCounter and currentDateTime fields) in GetCurrentDateTime(). This allows to keep the method GetCurrentDateTime() const.

I also applied a small refactoring of the methods UpdateTime() to avoid trying to lock the same mutex multiple times (FreeRTOS mutexes are not reentrant).

Co-authored-by: 30447455+mark9064@users.noreply.github.com

Author: JF002

src/components/datetime/DateTimeController.cpp

@@ -1,6 +1,7 @@
 #include "components/datetime/DateTimeController.h"
 #include <libraries/log/nrf_log.h>
 #include <systemtask/SystemTask.h>
+#include <nrf_rtc.h>
 
 using namespace Pinetime::Controllers;
 
@@ -12,11 +13,16 @@ namespace {
 }
 
 DateTime::DateTime(Controllers::Settings& settingsController) : settingsController {settingsController} {
+  mutex = xSemaphoreCreateMutex();
+  ASSERT(mutex != nullptr);
+  xSemaphoreGive(mutex);
 }
 
 void DateTime::SetCurrentTime(std::chrono::time_point<std::chrono::system_clock, std::chrono::nanoseconds> t) {
+  xSemaphoreTake(mutex, portMAX_DELAY);
   this->currentDateTime = t;
   UpdateTime(previousSystickCounter); // Update internal state without updating the time
+  xSemaphoreGive(mutex);
 }
 
 void DateTime::SetTime(uint16_t year, uint8_t month, uint8_t day, uint8_t hour, uint8_t minute, uint8_t second) {
@@ -35,7 +41,9 @@ void DateTime::SetTime(uint16_t year, uint8_t month, uint8_t day, uint8_t hour,
   NRF_LOG_INFO("%d %d %d ", day, month, year);
   NRF_LOG_INFO("%d %d %d ", hour, minute, second);
 
+  xSemaphoreTake(mutex, portMAX_DELAY);
   UpdateTime(previousSystickCounter);
+  xSemaphoreGive(mutex);
 
   systemTask->PushMessage(System::Messages::OnNewTime);
 }
@@ -45,29 +53,23 @@ void DateTime::SetTimeZone(int8_t timezone, int8_t dst) {
   dstOffset = dst;
 }
 
-void DateTime::UpdateTime(uint32_t systickCounter) {
+uint32_t DateTime::GetTickFromPreviousSystickCounter(uint32_t systickCounter) const {
   // Handle systick counter overflow
   uint32_t systickDelta = 0;
   if (systickCounter < previousSystickCounter) {
-    systickDelta = 0xffffff - previousSystickCounter;
+    systickDelta = static_cast<uint32_t>(portNRF_RTC_MAXTICKS) - previousSystickCounter;
     systickDelta += systickCounter + 1;
   } else {
     systickDelta = systickCounter - previousSystickCounter;
   }
+  return systickDelta;
+}
 
-  /*
-   * 1000 ms = 1024 ticks
-   */
-  auto correctedDelta = systickDelta / 1024;
-  auto rest = systickDelta % 1024;
-  if (systickCounter >= rest) {
-    previousSystickCounter = systickCounter - rest;
-  } else {
-    previousSystickCounter = 0xffffff - (rest - systickCounter);
-  }
-
-  currentDateTime += std::chrono::seconds(correctedDelta);
-  uptime += std::chrono::seconds(correctedDelta);
+void DateTime::UpdateTime() {
+  xSemaphoreTake(mutex, portMAX_DELAY);
+  uint32_t systick_counter = nrf_rtc_counter_get(portNRF_RTC_REG);
+  UpdateTime(systick_counter);
+  xSemaphoreGive(mutex);
 
   std::time_t currentTime = std::chrono::system_clock::to_time_t(currentDateTime);
   localTime = *std::localtime(&currentTime);
@@ -103,6 +105,23 @@ void DateTime::UpdateTime(uint32_t systickCounter) {
   }
 }
 
+void DateTime::UpdateTime(uint32_t systickCounter) {
+  auto systickDelta = GetTickFromPreviousSystickCounter(systickCounter);
+  auto correctedDelta = systickDelta / configTICK_RATE_HZ;
+
+  /*
+   * 1000 ms = 1024 ticks
+   */
+  auto rest = systickDelta % configTICK_RATE_HZ;
+  if (systickCounter >= rest) {
+    previousSystickCounter = systickCounter - rest;
+  } else {
+    previousSystickCounter = static_cast<uint32_t>(portNRF_RTC_MAXTICKS) - (rest - systickCounter);
+  }
+  currentDateTime += std::chrono::seconds(correctedDelta);
+  uptime += std::chrono::seconds(correctedDelta);
+}
+
 const char* DateTime::MonthShortToString() const {
   return MonthsString[static_cast<uint8_t>(Month())];
 }
@@ -146,3 +165,18 @@ std::string DateTime::FormattedTime() {
   }
   return std::string(buff);
 }
+
+std::chrono::time_point<std::chrono::system_clock, std::chrono::nanoseconds> DateTime::CurrentDateTime() const {
+  xSemaphoreTake(mutex, portMAX_DELAY);
+  uint32_t systick_counter = nrf_rtc_counter_get(portNRF_RTC_REG);
+  auto correctedDelta = GetTickFromPreviousSystickCounter(systick_counter) / configTICK_RATE_HZ;
+  auto result = currentDateTime + std::chrono::seconds(correctedDelta);
+  ;
+  xSemaphoreGive(mutex);
+
+  return result;
+}
+
+std::chrono::time_point<std::chrono::system_clock, std::chrono::nanoseconds> DateTime::UTCDateTime() const {
+  return CurrentDateTime() - std::chrono::seconds((tzOffset + dstOffset) * 15 * 60);
+}

src/components/datetime/DateTimeController.h

@@ -4,6 +4,8 @@
 #include <chrono>
 #include <ctime>
 #include <string>
+#include <FreeRTOS.h>
+#include <semphr.h>
 #include "components/settings/Settings.h"
 
 namespace Pinetime {
@@ -45,7 +47,7 @@ namespace Pinetime {
        */
       void SetTimeZone(int8_t timezone, int8_t dst);
 
-      void UpdateTime(uint32_t systickCounter);
+      void UpdateTime();
 
       uint16_t Year() const {
         return 1900 + localTime.tm_year;
@@ -124,13 +126,9 @@ namespace Pinetime {
       static const char* MonthShortToStringLow(Months month);
       static const char* DayOfWeekShortToStringLow(Days day);
 
-      std::chrono::time_point<std::chrono::system_clock, std::chrono::nanoseconds> CurrentDateTime() const {
-        return currentDateTime;
-      }
+      std::chrono::time_point<std::chrono::system_clock, std::chrono::nanoseconds> CurrentDateTime() const;
 
-      std::chrono::time_point<std::chrono::system_clock, std::chrono::nanoseconds> UTCDateTime() const {
-        return currentDateTime - std::chrono::seconds((tzOffset + dstOffset) * 15 * 60);
-      }
+      std::chrono::time_point<std::chrono::system_clock, std::chrono::nanoseconds> UTCDateTime() const;
 
       std::chrono::seconds Uptime() const {
         return uptime;
@@ -141,6 +139,9 @@ namespace Pinetime {
       std::string FormattedTime();
 
     private:
+      uint32_t GetTickFromPreviousSystickCounter(uint32_t systickCounter) const;
+      void UpdateTime(uint32_t systickCounter);
+
       std::tm localTime;
       int8_t tzOffset = 0;
       int8_t dstOffset = 0;
@@ -154,6 +155,7 @@ namespace Pinetime {
       bool isHalfHourAlreadyNotified = true;
       System::SystemTask* systemTask = nullptr;
       Controllers::Settings& settingsController;
+      SemaphoreHandle_t mutex = nullptr;
     };
   }
 }

src/systemtask/SystemTask.cpp

@@ -1,5 +1,4 @@
 #include "systemtask/SystemTask.h"
-#include <hal/nrf_rtc.h>
 #include <libraries/gpiote/app_gpiote.h>
 #include <libraries/log/nrf_log.h>
 #include "BootloaderVersion.h"
@@ -410,8 +409,7 @@ void SystemTask::Work() {
     }
 
     monitor.Process();
-    uint32_t systick_counter = nrf_rtc_counter_get(portNRF_RTC_REG);
-    dateTimeController.UpdateTime(systick_counter);
+    dateTimeController.UpdateTime();
     NoInit_BackUpTime = dateTimeController.CurrentDateTime();
     if (nrf_gpio_pin_read(PinMap::Button) == 0) {
       watchdog.Reload();