diff --git a/README.md b/README.md index e4f6707f..d3d1130f 100644 --- a/README.md +++ b/README.md @@ -9,8 +9,8 @@ Fast open-source firmware for the [PineTime smartwatch](https://pine64.org/devic - [Getting started with InfiniTime](doc/gettingStarted/gettingStarted-1.0.md) - [Updating the software](doc/gettingStarted/updating-software.md) - [About the firmware and bootloader](doc/gettingStarted/about-software.md) -- [PineTimeStyle Watch face](https://wiki.pine64.org/wiki/PineTimeStyle) - - [Weather integration](https://wiki.pine64.org/wiki/Infinitime-Weather) +- [PineTimeStyle Watch face](https://pine64.org/documentation/PineTime/Watchfaces/PineTimeStyle) + - [Weather integration](https://pine64.org/documentation/PineTime/Software/InfiniTime_weather/) ### Companion apps diff --git a/src/components/ble/SimpleWeatherService.cpp b/src/components/ble/SimpleWeatherService.cpp index 146152f8..504cad14 100644 --- a/src/components/ble/SimpleWeatherService.cpp +++ b/src/components/ble/SimpleWeatherService.cpp @@ -80,7 +80,7 @@ int WeatherCallback(uint16_t /*connHandle*/, uint16_t /*attrHandle*/, struct ble return static_cast(arg)->OnCommand(ctxt); } -SimpleWeatherService::SimpleWeatherService(const DateTime& dateTimeController) : dateTimeController(dateTimeController) { +SimpleWeatherService::SimpleWeatherService(DateTime& dateTimeController) : dateTimeController(dateTimeController) { } void SimpleWeatherService::Init() { diff --git a/src/components/ble/SimpleWeatherService.h b/src/components/ble/SimpleWeatherService.h index 4bbefcfc..03d2f6ff 100644 --- a/src/components/ble/SimpleWeatherService.h +++ b/src/components/ble/SimpleWeatherService.h @@ -40,7 +40,7 @@ namespace Pinetime { class SimpleWeatherService { public: - explicit SimpleWeatherService(const DateTime& dateTimeController); + explicit SimpleWeatherService(DateTime& dateTimeController); void Init(); @@ -140,7 +140,7 @@ namespace Pinetime { uint16_t eventHandle {}; - const Pinetime::Controllers::DateTime& dateTimeController; + Pinetime::Controllers::DateTime& dateTimeController; std::optional currentWeather; std::optional forecast; diff --git a/src/components/datetime/DateTimeController.cpp b/src/components/datetime/DateTimeController.cpp index f0ccb5e5..7f58c9b3 100644 --- a/src/components/datetime/DateTimeController.cpp +++ b/src/components/datetime/DateTimeController.cpp @@ -1,6 +1,8 @@ #include "components/datetime/DateTimeController.h" #include #include +#include +#include "nrf_assert.h" using namespace Pinetime::Controllers; @@ -12,11 +14,16 @@ namespace { } DateTime::DateTime(Controllers::Settings& settingsController) : settingsController {settingsController} { + mutex = xSemaphoreCreateMutex(); + ASSERT(mutex != nullptr); + xSemaphoreGive(mutex); } void DateTime::SetCurrentTime(std::chrono::time_point 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,13 +36,15 @@ 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); } @@ -45,25 +54,34 @@ void DateTime::SetTimeZone(int8_t timezone, int8_t dst) { dstOffset = dst; } -void DateTime::UpdateTime(uint32_t systickCounter) { +std::chrono::time_point 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(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(portNRF_RTC_MAXTICKS) - (rest - systickCounter - 1); } currentDateTime += std::chrono::seconds(correctedDelta); diff --git a/src/components/datetime/DateTimeController.h b/src/components/datetime/DateTimeController.h index f719df7d..5a453f20 100644 --- a/src/components/datetime/DateTimeController.h +++ b/src/components/datetime/DateTimeController.h @@ -5,6 +5,8 @@ #include #include #include "components/settings/Settings.h" +#include +#include namespace Pinetime { namespace System { @@ -45,8 +47,6 @@ namespace Pinetime { */ void SetTimeZone(int8_t timezone, int8_t dst); - void UpdateTime(uint32_t systickCounter); - uint16_t Year() const { return 1900 + localTime.tm_year; } @@ -124,12 +124,10 @@ namespace Pinetime { static const char* MonthShortToStringLow(Months month); static const char* DayOfWeekShortToStringLow(Days day); - std::chrono::time_point CurrentDateTime() const { - return currentDateTime; - } + std::chrono::time_point CurrentDateTime(); - std::chrono::time_point UTCDateTime() const { - return currentDateTime - std::chrono::seconds((tzOffset + dstOffset) * 15 * 60); + std::chrono::time_point 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 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 diff --git a/src/displayapp/DisplayApp.cpp b/src/displayapp/DisplayApp.cpp index e13897bf..96b9afe0 100644 --- a/src/displayapp/DisplayApp.cpp +++ b/src/displayapp/DisplayApp.cpp @@ -126,6 +126,7 @@ void DisplayApp::Start(System::BootErrors error) { bootError = error; lvgl.Init(); + motorController.Init(); if (error == System::BootErrors::TouchController) { LoadNewScreen(Apps::Error, DisplayApp::FullRefreshDirections::None); @@ -151,7 +152,6 @@ void DisplayApp::Process(void* instance) { void DisplayApp::InitHw() { brightnessController.Init(); ApplyBrightness(); - motorController.Init(); lcd.Init(); } diff --git a/src/displayapp/screens/WatchFaceAnalog.h b/src/displayapp/screens/WatchFaceAnalog.h index 2eee657e..958ff64d 100644 --- a/src/displayapp/screens/WatchFaceAnalog.h +++ b/src/displayapp/screens/WatchFaceAnalog.h @@ -75,7 +75,7 @@ namespace Pinetime { BatteryIcon batteryIcon; - const Controllers::DateTime& dateTimeController; + Controllers::DateTime& dateTimeController; const Controllers::Battery& batteryController; const Controllers::Ble& bleController; Controllers::NotificationManager& notificationManager; diff --git a/src/drivers/Bma421.cpp b/src/drivers/Bma421.cpp index aff62b8d..74d47d06 100644 --- a/src/drivers/Bma421.cpp +++ b/src/drivers/Bma421.cpp @@ -126,13 +126,6 @@ Bma421::Values Bma421::Process() { uint32_t steps = 0; bma423_step_counter_output(&steps, &bma); - int32_t temperature; - bma4_get_temperature(&temperature, BMA4_DEG, &bma); - temperature = temperature / 1000; - - uint8_t activity = 0; - bma423_activity_output(&activity, &bma); - // X and Y axis are swapped because of the way the sensor is mounted in the PineTime return {steps, data.y, data.x, data.z}; } diff --git a/src/drivers/St7789.cpp b/src/drivers/St7789.cpp index 12e95a41..c22f2199 100644 --- a/src/drivers/St7789.cpp +++ b/src/drivers/St7789.cpp @@ -1,3 +1,4 @@ +#include #include "drivers/St7789.h" #include #include @@ -16,10 +17,9 @@ void St7789::Init() { HardwareReset(); SoftwareReset(); SleepOut(); - ColMod(); + PixelFormat(); MemoryDataAccessControl(); - ColumnAddressSet(); - RowAddressSet(); + SetAddrWindow(0, 0, Width, Height); // P8B Mirrored version does not need display inversion. #ifndef DRIVER_DISPLAY_MIRROR DisplayInversionOn(); @@ -97,8 +97,9 @@ void St7789::SleepIn() { sleepIn = true; } -void St7789::ColMod() { - WriteCommand(static_cast(Commands::ColMod)); +void St7789::PixelFormat() { + WriteCommand(static_cast(Commands::PixelFormat)); + // 65K colours, 16-bit per pixel WriteData(0x55); } @@ -118,22 +119,6 @@ void St7789::MemoryDataAccessControl() { #endif } -void St7789::ColumnAddressSet() { - WriteCommand(static_cast(Commands::ColumnAddressSet)); - WriteData(0x00); - WriteData(0x00); - WriteData(Width >> 8u); - WriteData(Width & 0xffu); -} - -void St7789::RowAddressSet() { - WriteCommand(static_cast(Commands::RowAddressSet)); - WriteData(0x00); - WriteData(0x00); - WriteData(320u >> 8u); - WriteData(320u & 0xffu); -} - void St7789::DisplayInversionOn() { WriteCommand(static_cast(Commands::DisplayInversionOn)); } @@ -148,16 +133,23 @@ void St7789::DisplayOn() { void St7789::SetAddrWindow(uint16_t x0, uint16_t y0, uint16_t x1, uint16_t y1) { WriteCommand(static_cast(Commands::ColumnAddressSet)); - WriteData(x0 >> 8); - WriteData(x0 & 0xff); - WriteData(x1 >> 8); - WriteData(x1 & 0xff); + uint8_t colArgs[] = { + static_cast(x0 >> 8), // x start MSB + static_cast(x0), // x start LSB + static_cast(x1 >> 8), // x end MSB + static_cast(x1) // x end LSB + }; + WriteData(colArgs, sizeof(colArgs)); WriteCommand(static_cast(Commands::RowAddressSet)); - WriteData(y0 >> 8); - WriteData(y0 & 0xff); - WriteData(y1 >> 8); - WriteData(y1 & 0xff); + uint8_t rowArgs[] = { + static_cast(y0 >> 8), // y start MSB + static_cast(y0), // y start LSB + static_cast(y1 >> 8), // y end MSB + static_cast(y1) // y end LSB + }; + memcpy(addrWindowArgs, rowArgs, sizeof(rowArgs)); + WriteData(addrWindowArgs, sizeof(addrWindowArgs)); } void St7789::WriteToRam(const uint8_t* data, size_t size) { @@ -179,8 +171,12 @@ void St7789::DisplayOff() { void St7789::VerticalScrollStartAddress(uint16_t line) { verticalScrollingStartAddress = line; WriteCommand(static_cast(Commands::VerticalScrollStartAddress)); - WriteData(line >> 8u); - WriteData(line & 0x00ffu); + uint8_t args[] = { + static_cast(line >> 8), // Frame memory line pointer MSB + static_cast(line) // Frame memory line pointer LSB + }; + memcpy(verticalScrollArgs, args, sizeof(args)); + WriteData(verticalScrollArgs, sizeof(verticalScrollArgs)); } void St7789::Uninit() { diff --git a/src/drivers/St7789.h b/src/drivers/St7789.h index 45d4b56d..844e0180 100644 --- a/src/drivers/St7789.h +++ b/src/drivers/St7789.h @@ -40,7 +40,7 @@ namespace Pinetime { void SleepOut(); void EnsureSleepOutPostDelay(); void SleepIn(); - void ColMod(); + void PixelFormat(); void MemoryDataAccessControl(); void DisplayInversionOn(); void NormalModeOn(); @@ -68,16 +68,17 @@ namespace Pinetime { MemoryDataAccessControl = 0x36, VerticalScrollDefinition = 0x33, VerticalScrollStartAddress = 0x37, - ColMod = 0x3a, + PixelFormat = 0x3a, VdvSet = 0xc4, }; void WriteData(uint8_t data); void WriteData(const uint8_t* data, size_t size); - void ColumnAddressSet(); static constexpr uint16_t Width = 240; static constexpr uint16_t Height = 320; - void RowAddressSet(); + + uint8_t addrWindowArgs[4]; + uint8_t verticalScrollArgs[2]; }; } } diff --git a/src/systemtask/SystemTask.cpp b/src/systemtask/SystemTask.cpp index e3d40d35..a56c2591 100644 --- a/src/systemtask/SystemTask.cpp +++ b/src/systemtask/SystemTask.cpp @@ -410,8 +410,6 @@ void SystemTask::Work() { } monitor.Process(); - uint32_t systick_counter = nrf_rtc_counter_get(portNRF_RTC_REG); - dateTimeController.UpdateTime(systick_counter); NoInit_BackUpTime = dateTimeController.CurrentDateTime(); if (nrf_gpio_pin_read(PinMap::Button) == 0) { watchdog.Reload();