aboutsummaryrefslogtreecommitdiffstats
path: root/src/components/datetime
diff options
context:
space:
mode:
Diffstat (limited to 'src/components/datetime')
-rw-r--r--src/components/datetime/DateTimeController.cpp77
-rw-r--r--src/components/datetime/DateTimeController.h20
-rw-r--r--src/components/datetime/TODO.md41
3 files changed, 107 insertions, 31 deletions
diff --git a/src/components/datetime/DateTimeController.cpp b/src/components/datetime/DateTimeController.cpp
index 9e9fb6e4..d439821b 100644
--- a/src/components/datetime/DateTimeController.cpp
+++ b/src/components/datetime/DateTimeController.cpp
@@ -1,22 +1,42 @@
#include "components/datetime/DateTimeController.h"
#include <libraries/log/nrf_log.h>
#include <systemtask/SystemTask.h>
+#include <hal/nrf_rtc.h>
+#include "nrf_assert.h"
using namespace Pinetime::Controllers;
namespace {
- char const* DaysStringShort[] = {"--", "MON", "TUE", "WED", "THU", "FRI", "SAT", "SUN"};
- char const* DaysStringShortLow[] = {"--", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat", "Sun"};
- char const* MonthsString[] = {"--", "JAN", "FEB", "MAR", "APR", "MAY", "JUN", "JUL", "AUG", "SEP", "OCT", "NOV", "DEC"};
- char const* MonthsStringLow[] = {"--", "Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"};
+ constexpr const char* const DaysStringShort[] = {"--", "MON", "TUE", "WED", "THU", "FRI", "SAT", "SUN"};
+ constexpr const char* const DaysStringShortLow[] = {"--", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat", "Sun"};
+ constexpr const char* const MonthsString[] = {"--", "JAN", "FEB", "MAR", "APR", "MAY", "JUN", "JUL", "AUG", "SEP", "OCT", "NOV", "DEC"};
+ constexpr const char* const MonthsStringLow[] =
+ {"--", "Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"};
+
+ constexpr int compileTimeAtoi(const char* str) {
+ int result = 0;
+ while (*str >= '0' && *str <= '9') {
+ result = result * 10 + *str - '0';
+ str++;
+ }
+ return result;
+ }
}
DateTime::DateTime(Controllers::Settings& settingsController) : settingsController {settingsController} {
+ mutex = xSemaphoreCreateMutex();
+ ASSERT(mutex != nullptr);
+ xSemaphoreGive(mutex);
+
+ // __DATE__ is a string of the format "MMM DD YYYY", so an offset of 7 gives the start of the year
+ SetTime(compileTimeAtoi(&__DATE__[7]), 1, 1, 0, 0, 0);
}
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
+ UpdateTime(previousSystickCounter, true); // 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) {
@@ -29,15 +49,19 @@ void DateTime::SetTime(uint16_t year, uint8_t month, uint8_t day, uint8_t hour,
/* .tm_year = */ year - 1900,
};
- tm.tm_isdst = -1; // Use DST value from local time zone
- currentDateTime = std::chrono::system_clock::from_time_t(std::mktime(&tm));
-
NRF_LOG_INFO("%d %d %d ", day, month, year);
NRF_LOG_INFO("%d %d %d ", hour, minute, second);
- UpdateTime(previousSystickCounter);
+ tm.tm_isdst = -1; // Use DST value from local time zone
+
+ xSemaphoreTake(mutex, portMAX_DELAY);
+ currentDateTime = std::chrono::system_clock::from_time_t(std::mktime(&tm));
+ UpdateTime(previousSystickCounter, true);
+ xSemaphoreGive(mutex);
- systemTask->PushMessage(System::Messages::OnNewTime);
+ if (systemTask != nullptr) {
+ systemTask->PushMessage(System::Messages::OnNewTime);
+ }
}
void DateTime::SetTimeZone(int8_t timezone, int8_t dst) {
@@ -45,25 +69,34 @@ void DateTime::SetTimeZone(int8_t timezone, int8_t dst) {
dstOffset = dst;
}
-void DateTime::UpdateTime(uint32_t systickCounter) {
+std::chrono::time_point<std::chrono::system_clock, std::chrono::nanoseconds> DateTime::CurrentDateTime() {
+ xSemaphoreTake(mutex, portMAX_DELAY);
+ UpdateTime(nrf_rtc_counter_get(portNRF_RTC_REG), false);
+ xSemaphoreGive(mutex);
+ return currentDateTime;
+}
+
+void DateTime::UpdateTime(uint32_t systickCounter, bool forceUpdate) {
// 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;
}
- /*
- * 1000 ms = 1024 ticks
- */
- auto correctedDelta = systickDelta / 1024;
- auto rest = systickDelta % 1024;
+ auto correctedDelta = systickDelta / configTICK_RATE_HZ;
+ // If a second hasn't passed, there is nothing to do
+ // If the time has been changed, set forceUpdate to trigger internal state updates
+ if (correctedDelta == 0 && !forceUpdate) {
+ return;
+ }
+ auto rest = systickDelta % configTICK_RATE_HZ;
if (systickCounter >= rest) {
previousSystickCounter = systickCounter - rest;
} else {
- previousSystickCounter = 0xffffff - (rest - systickCounter);
+ previousSystickCounter = static_cast<uint32_t>(portNRF_RTC_MAXTICKS) - (rest - systickCounter - 1);
}
currentDateTime += std::chrono::seconds(correctedDelta);
@@ -115,8 +148,8 @@ const char* DateTime::MonthShortToStringLow(Months month) {
return MonthsStringLow[static_cast<uint8_t>(month)];
}
-const char* DateTime::DayOfWeekShortToStringLow() const {
- return DaysStringShortLow[static_cast<uint8_t>(DayOfWeek())];
+const char* DateTime::DayOfWeekShortToStringLow(Days day) {
+ return DaysStringShortLow[static_cast<uint8_t>(day)];
}
void DateTime::Register(Pinetime::System::SystemTask* systemTask) {
@@ -140,9 +173,9 @@ std::string DateTime::FormattedTime() {
hour12 = (hour == 12) ? 12 : hour - 12;
amPmStr = "PM";
}
- sprintf(buff, "%i:%02i %s", hour12, minute, amPmStr);
+ snprintf(buff, sizeof(buff), "%i:%02i %s", hour12, minute, amPmStr);
} else {
- sprintf(buff, "%02i:%02i", hour, minute);
+ snprintf(buff, sizeof(buff), "%02i:%02i", hour, minute);
}
return std::string(buff);
}
diff --git a/src/components/datetime/DateTimeController.h b/src/components/datetime/DateTimeController.h
index 0bf6ac2a..a005f9ac 100644
--- a/src/components/datetime/DateTimeController.h
+++ b/src/components/datetime/DateTimeController.h
@@ -5,6 +5,8 @@
#include <ctime>
#include <string>
#include "components/settings/Settings.h"
+#include <FreeRTOS.h>
+#include <semphr.h>
namespace Pinetime {
namespace System {
@@ -39,14 +41,12 @@ namespace Pinetime {
*
* used to update difference between utc and local time (see UtcOffset())
*
- * parameters are in quarters of an our. Following the BLE CTS specification,
+ * parameters are in quarters of an hour. Following the BLE CTS specification,
* timezone is expected to be constant over DST which will be reported in
* dst field.
*/
void SetTimeZone(int8_t timezone, int8_t dst);
- void UpdateTime(uint32_t systickCounter);
-
uint16_t Year() const {
return 1900 + localTime.tm_year;
}
@@ -122,14 +122,12 @@ namespace Pinetime {
const char* MonthShortToString() const;
const char* DayOfWeekShortToString() const;
static const char* MonthShortToStringLow(Months month);
- const char* DayOfWeekShortToStringLow() const;
+ 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();
- 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() {
+ return CurrentDateTime() - std::chrono::seconds((tzOffset + dstOffset) * 15 * 60);
}
std::chrono::seconds Uptime() const {
@@ -141,10 +139,14 @@ namespace Pinetime {
std::string FormattedTime();
private:
+ void UpdateTime(uint32_t systickCounter, bool forceUpdate);
+
std::tm localTime;
int8_t tzOffset = 0;
int8_t dstOffset = 0;
+ SemaphoreHandle_t mutex = nullptr;
+
uint32_t previousSystickCounter = 0;
std::chrono::time_point<std::chrono::system_clock, std::chrono::nanoseconds> currentDateTime;
std::chrono::seconds uptime {0};
diff --git a/src/components/datetime/TODO.md b/src/components/datetime/TODO.md
new file mode 100644
index 00000000..e9590898
--- /dev/null
+++ b/src/components/datetime/TODO.md
@@ -0,0 +1,41 @@
+# Refactoring needed
+
+## Context
+
+The [PR #2041 - Continuous time updates](https://github.com/InfiniTimeOrg/InfiniTime/pull/2041) highlighted some
+limitations in the design of DateTimeController: the granularity of the time returned by `DateTime::CurrentDateTime()`
+is limited by the frequency at which SystemTask calls `DateTime::UpdateTime()`, which is currently set to 100ms.
+
+@mark9064 provided more details
+in [this comment](https://github.com/InfiniTimeOrg/InfiniTime/pull/2041#issuecomment-2048528967).
+
+The [PR #2041 - Continuous time updates](https://github.com/InfiniTimeOrg/InfiniTime/pull/2041) provided some changes
+to `DateTime` controller that improves the granularity of the time returned by `DateTime::CurrentDateTime()`.
+
+However, the review showed that `DateTime` cannot be `const` anymore, even when it's only used to get the current time,
+since `DateTime::CurrentDateTime()` changes the internal state of the instance.
+
+We tried to identify alternative implementation that would have maintained the "const correctness" but we eventually
+figured that this would lead to a re-design of `DateTime` which was out of scope of the initial PR (Continuous time
+updates and always on display).
+
+So we decided to merge this PR #2041 and agree to fix/improve `DateTime` later on.
+
+## What needs to be done?
+
+Improve/redesign `DateTime` so that it
+
+* provides a very granular (ideally down to the millisecond) date and time via `CurrentDateTime()`.
+* can be declared/passed as `const` when it's only used to **get** the time.
+* limits the use of mutex as much as possible (an ideal implementation would not use any mutex, but this might not be
+ possible).
+* improves the design of `DateTime::Seconds()`, `DateTime::Minutes()`, `DateTime::Hours()`, etc as
+ explained [in this comment](https://github.com/InfiniTimeOrg/InfiniTime/pull/2054#pullrequestreview-2037033105).
+
+Once this redesign is implemented, all instances/references to `DateTime` should be reviewed and updated to use `const`
+where appropriate.
+
+Please check the following PR to get more context about this redesign:
+
+* [#2041 - Continuous time updates by @mark9064](https://github.com/InfiniTimeOrg/InfiniTime/pull/2041)
+* [#2054 - Continuous time update - Alternative implementation to #2041 by @JF002](https://github.com/InfiniTimeOrg/InfiniTime/pull/2054) \ No newline at end of file