From 9574ea5a3ecf31fbd42c06824f4c8fa9aac1d104 Mon Sep 17 00:00:00 2001 From: Galdor Takacs Date: Thu, 15 Sep 2022 00:53:37 +0200 Subject: [PATCH 1/6] AlarmController: Add saving alarm time to file Save the set alarm time to the SPI NOR flash, so it does not reset to the default value when the watch resets, e.g. due to watchdog timeout. Size impact: +204 (400332 -> 400536) --- src/components/alarm/AlarmController.cpp | 41 +++++++++++++++++++++++- src/components/alarm/AlarmController.h | 13 +++++++- src/displayapp/screens/Alarm.cpp | 17 ++++++++-- src/displayapp/screens/Alarm.h | 3 ++ src/main.cpp | 2 +- 5 files changed, 70 insertions(+), 6 deletions(-) diff --git a/src/components/alarm/AlarmController.cpp b/src/components/alarm/AlarmController.cpp index 9f4e910519..bded0606de 100644 --- a/src/components/alarm/AlarmController.cpp +++ b/src/components/alarm/AlarmController.cpp @@ -19,11 +19,13 @@ #include "systemtask/SystemTask.h" #include "task.h" #include +#include using namespace Pinetime::Controllers; using namespace std::chrono_literals; -AlarmController::AlarmController(Controllers::DateTime& dateTimeController) : dateTimeController {dateTimeController} { +AlarmController::AlarmController(Controllers::DateTime& dateTimeController, Controllers::FS& fs) + : dateTimeController {dateTimeController}, fs {fs} { } namespace { @@ -35,12 +37,14 @@ namespace { void AlarmController::Init(System::SystemTask* systemTask) { this->systemTask = systemTask; + LoadAlarmFromFile(); alarmTimer = xTimerCreate("Alarm", 1, pdFALSE, this, SetOffAlarm); } void AlarmController::SetAlarmTime(uint8_t alarmHr, uint8_t alarmMin) { hours = alarmHr; minutes = alarmMin; + SaveAlarmToFile(); } void AlarmController::ScheduleAlarm() { @@ -106,3 +110,38 @@ void AlarmController::StopAlerting() { } systemTask->PushMessage(System::Messages::StopRinging); } + +void AlarmController::LoadAlarmFromFile() { + lfs_file_t file; + AlarmData buffer; + + if (fs.FileOpen(&file, "/alarm.dat", LFS_O_RDONLY) != LFS_ERR_OK) { + NRF_LOG_WARNING("[AlarmController] Failed to open alarm data file"); + return; + } + + fs.FileRead(&file, reinterpret_cast(&buffer), sizeof buffer); + fs.FileClose(&file); + if (buffer.version != alarmFormatVersion) { + NRF_LOG_WARNING("[AlarmController] Loaded alarm data has version %u instead of %u, discarding", buffer.version, alarmFormatVersion); + return; + } + + hours = buffer.hours; + minutes = buffer.minutes; + NRF_LOG_INFO("[AlarmController] Loaded alarm data from file"); +} + +void AlarmController::SaveAlarmToFile() { + lfs_file_t file; + AlarmData buffer = {.version = alarmFormatVersion, .hours = hours, .minutes = minutes}; + + if (fs.FileOpen(&file, "/alarm.dat", LFS_O_WRONLY | LFS_O_CREAT) != LFS_ERR_OK) { + NRF_LOG_WARNING("[AlarmController] Failed to open alarm data file for saving"); + return; + } + + fs.FileWrite(&file, reinterpret_cast(&buffer), sizeof buffer); + fs.FileClose(&file); + NRF_LOG_INFO("[AlarmController] Saved alarm data with format version %u to file", buffer.version); +} diff --git a/src/components/alarm/AlarmController.h b/src/components/alarm/AlarmController.h index d630a1289f..bc934f8f99 100644 --- a/src/components/alarm/AlarmController.h +++ b/src/components/alarm/AlarmController.h @@ -29,7 +29,7 @@ namespace Pinetime { namespace Controllers { class AlarmController { public: - AlarmController(Controllers::DateTime& dateTimeController); + AlarmController(Controllers::DateTime& dateTimeController, Controllers::FS& fs); void Init(System::SystemTask* systemTask); void SetAlarmTime(uint8_t alarmHr, uint8_t alarmMin); @@ -57,7 +57,15 @@ namespace Pinetime { } private: + static constexpr uint8_t alarmFormatVersion = 1; + struct AlarmData { + uint8_t version = alarmFormatVersion; + uint8_t hours = 7; + uint8_t minutes = 0; + }; + Controllers::DateTime& dateTimeController; + Controllers::FS& fs; System::SystemTask* systemTask = nullptr; TimerHandle_t alarmTimer; uint8_t hours = 7; @@ -65,6 +73,9 @@ namespace Pinetime { std::chrono::time_point alarmTime; AlarmState state = AlarmState::Not_Set; RecurType recurrence = RecurType::None; + + void LoadAlarmFromFile(); + void SaveAlarmToFile(); }; } } diff --git a/src/displayapp/screens/Alarm.cpp b/src/displayapp/screens/Alarm.cpp index d6371ce66f..0b790bd240 100644 --- a/src/displayapp/screens/Alarm.cpp +++ b/src/displayapp/screens/Alarm.cpp @@ -46,6 +46,9 @@ Alarm::Alarm(DisplayApp* app, System::SystemTask& systemTask) : Screen(app), alarmController {alarmController}, systemTask {systemTask} { + hours = alarmController.Hours(); + minutes = alarmController.Minutes(); + hourCounter.Create(); lv_obj_align(hourCounter.GetObject(), nullptr, LV_ALIGN_IN_TOP_LEFT, 0, 0); if (clockType == Controllers::Settings::ClockType::H12) { @@ -57,12 +60,12 @@ Alarm::Alarm(DisplayApp* app, lv_label_set_align(lblampm, LV_LABEL_ALIGN_CENTER); lv_obj_align(lblampm, lv_scr_act(), LV_ALIGN_CENTER, 0, 30); } - hourCounter.SetValue(alarmController.Hours()); + hourCounter.SetValue(hours); hourCounter.SetValueChangedEventCallback(this, ValueChangedHandler); minuteCounter.Create(); lv_obj_align(minuteCounter.GetObject(), nullptr, LV_ALIGN_IN_TOP_RIGHT, 0, 0); - minuteCounter.SetValue(alarmController.Minutes()); + minuteCounter.SetValue(minutes); minuteCounter.SetValueChangedEventCallback(this, ValueChangedHandler); lv_obj_t* colonLabel = lv_label_create(lv_scr_act(), nullptr); @@ -121,6 +124,7 @@ Alarm::Alarm(DisplayApp* app, } Alarm::~Alarm() { + alarmController.SetAlarmTime(hours, minutes); if (alarmController.State() == AlarmController::AlarmState::Alerting) { StopAlerting(); } @@ -150,6 +154,7 @@ void Alarm::OnButtonEvent(lv_obj_t* obj, lv_event_t event) { } if (obj == enableSwitch) { if (lv_switch_get_state(enableSwitch)) { + alarmController.SetAlarmTime(hours, minutes); alarmController.ScheduleAlarm(); } else { alarmController.DisableAlarm(); @@ -181,6 +186,11 @@ bool Alarm::OnTouchEvent(Pinetime::Applications::TouchEvents event) { } void Alarm::OnValueChanged() { + // Watch out when removing the next line! The selected time is only sent + // to the AlarmController when enabling the toggle and when closing the + // alarm app, to avoid excessive writes to flash. That means when changing + // the time and leaving the app open, the AlarmController is not yet aware + // of the new time and will ring on the previously set time. DisableAlarm(); UpdateAlarmTime(); } @@ -193,7 +203,8 @@ void Alarm::UpdateAlarmTime() { lv_label_set_text_static(lblampm, "AM"); } } - alarmController.SetAlarmTime(hourCounter.GetValue(), minuteCounter.GetValue()); + hours = hourCounter.GetValue(); + minutes = minuteCounter.GetValue(); } void Alarm::SetAlerting() { diff --git a/src/displayapp/screens/Alarm.h b/src/displayapp/screens/Alarm.h index fba9d5d9fb..5cf8f162ef 100644 --- a/src/displayapp/screens/Alarm.h +++ b/src/displayapp/screens/Alarm.h @@ -44,6 +44,9 @@ namespace Pinetime { Controllers::AlarmController& alarmController; System::SystemTask& systemTask; + uint8_t hours = 0; + uint8_t minutes = 0; + lv_obj_t *btnStop, *txtStop, *btnRecur, *txtRecur, *btnInfo, *enableSwitch; lv_obj_t* lblampm = nullptr; lv_obj_t* txtMessage = nullptr; diff --git a/src/main.cpp b/src/main.cpp index 109971bca0..a43267716e 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -111,7 +111,7 @@ Pinetime::Drivers::WatchdogView watchdogView(watchdog); Pinetime::Controllers::NotificationManager notificationManager; Pinetime::Controllers::MotionController motionController; Pinetime::Controllers::TimerController timerController; -Pinetime::Controllers::AlarmController alarmController {dateTimeController}; +Pinetime::Controllers::AlarmController alarmController {dateTimeController, fs}; Pinetime::Controllers::TouchHandler touchHandler(touchPanel, lvgl); Pinetime::Controllers::ButtonHandler buttonHandler; Pinetime::Controllers::BrightnessController brightnessController {}; From 6fed68ad259bf115e395f2bba107669b0cf02af6 Mon Sep 17 00:00:00 2001 From: Galdor Takacs Date: Fri, 16 Sep 2022 00:40:41 +0200 Subject: [PATCH 2/6] AlarmController: Save recurrence and enabled along with alarm time Also reverts the attempt at deferring saving the alarm data for reduced flash writes, in order to reduce complexity. Split `AlarmController.State()` into `.IsAlerting()` and `.IsEnabled()`, since the two properties are conceptually independent and it makes code much cleaner: ```diff - if (alarmController.State() == Controllers::AlarmController::AlarmState::Alerting) { ... + if (alarmController.IsAlerting()) { ... - if (alarmController.State() == AlarmController::AlarmState::Set) { ... + if (alarmController.IsEnabled()) { ... ``` See the removed `switch` for another example. Size impact: +44 (400536 -> 400580) --- src/components/alarm/AlarmController.cpp | 59 ++++++++++++++++-------- src/components/alarm/AlarmController.h | 26 +++++------ src/displayapp/screens/Alarm.cpp | 43 ++++++----------- src/displayapp/screens/Alarm.h | 3 -- src/systemtask/SystemTask.cpp | 8 ++-- 5 files changed, 70 insertions(+), 69 deletions(-) diff --git a/src/components/alarm/AlarmController.cpp b/src/components/alarm/AlarmController.cpp index bded0606de..c153c0a35f 100644 --- a/src/components/alarm/AlarmController.cpp +++ b/src/components/alarm/AlarmController.cpp @@ -37,13 +37,20 @@ namespace { void AlarmController::Init(System::SystemTask* systemTask) { this->systemTask = systemTask; - LoadAlarmFromFile(); alarmTimer = xTimerCreate("Alarm", 1, pdFALSE, this, SetOffAlarm); + LoadAlarmFromFile(); + if (alarm.isEnabled) { + NRF_LOG_INFO("[AlarmController] Loaded alarm was enabled, scheduling"); + ScheduleAlarm(); + } } void AlarmController::SetAlarmTime(uint8_t alarmHr, uint8_t alarmMin) { - hours = alarmHr; - minutes = alarmMin; + if (alarm.hours == alarmHr && alarm.minutes == alarmMin) { + return; + } + alarm.hours = alarmHr; + alarm.minutes = alarmMin; SaveAlarmToFile(); } @@ -57,18 +64,19 @@ void AlarmController::ScheduleAlarm() { tm* tmAlarmTime = std::localtime(&ttAlarmTime); // If the time being set has already passed today,the alarm should be set for tomorrow - if (hours < dateTimeController.Hours() || (hours == dateTimeController.Hours() && minutes <= dateTimeController.Minutes())) { + if (alarm.hours < dateTimeController.Hours() || + (alarm.hours == dateTimeController.Hours() && alarm.minutes <= dateTimeController.Minutes())) { tmAlarmTime->tm_mday += 1; // tm_wday doesn't update automatically tmAlarmTime->tm_wday = (tmAlarmTime->tm_wday + 1) % 7; } - tmAlarmTime->tm_hour = hours; - tmAlarmTime->tm_min = minutes; + tmAlarmTime->tm_hour = alarm.hours; + tmAlarmTime->tm_min = alarm.minutes; tmAlarmTime->tm_sec = 0; // if alarm is in weekday-only mode, make sure it shifts to the next weekday - if (recurrence == RecurType::Weekdays) { + if (alarm.recurrence == RecurType::Weekdays) { if (tmAlarmTime->tm_wday == 0) { // Sunday, shift 1 day tmAlarmTime->tm_mday += 1; } else if (tmAlarmTime->tm_wday == 6) { // Saturday, shift 2 days @@ -83,7 +91,10 @@ void AlarmController::ScheduleAlarm() { xTimerChangePeriod(alarmTimer, secondsToAlarm * configTICK_RATE_HZ, 0); xTimerStart(alarmTimer, 0); - state = AlarmState::Set; + if (!alarm.isEnabled) { + alarm.isEnabled = true; + SaveAlarmToFile(); + } } uint32_t AlarmController::SecondsToAlarm() { @@ -92,18 +103,24 @@ uint32_t AlarmController::SecondsToAlarm() { void AlarmController::DisableAlarm() { xTimerStop(alarmTimer, 0); - state = AlarmState::Not_Set; + isAlerting = false; + if (alarm.isEnabled) { + alarm.isEnabled = false; + SaveAlarmToFile(); + } } void AlarmController::SetOffAlarmNow() { - state = AlarmState::Alerting; + isAlerting = true; systemTask->PushMessage(System::Messages::SetOffAlarm); } void AlarmController::StopAlerting() { - // Alarm state is off unless this is a recurring alarm - if (recurrence == RecurType::None) { - state = AlarmState::Not_Set; + isAlerting = false; + // Disable alarm unless it is recurring + if (alarm.recurrence == RecurType::None) { + alarm.isEnabled = false; + SaveAlarmToFile(); } else { // set next instance ScheduleAlarm(); @@ -111,6 +128,13 @@ void AlarmController::StopAlerting() { systemTask->PushMessage(System::Messages::StopRinging); } +void AlarmController::SetRecurrence(RecurType recurrence) { + if (alarm.recurrence != recurrence) { + alarm.recurrence = recurrence; + SaveAlarmToFile(); + } +} + void AlarmController::LoadAlarmFromFile() { lfs_file_t file; AlarmData buffer; @@ -127,21 +151,18 @@ void AlarmController::LoadAlarmFromFile() { return; } - hours = buffer.hours; - minutes = buffer.minutes; + alarm = buffer; NRF_LOG_INFO("[AlarmController] Loaded alarm data from file"); } void AlarmController::SaveAlarmToFile() { lfs_file_t file; - AlarmData buffer = {.version = alarmFormatVersion, .hours = hours, .minutes = minutes}; - if (fs.FileOpen(&file, "/alarm.dat", LFS_O_WRONLY | LFS_O_CREAT) != LFS_ERR_OK) { NRF_LOG_WARNING("[AlarmController] Failed to open alarm data file for saving"); return; } - fs.FileWrite(&file, reinterpret_cast(&buffer), sizeof buffer); + fs.FileWrite(&file, reinterpret_cast(&alarm), sizeof alarm); fs.FileClose(&file); - NRF_LOG_INFO("[AlarmController] Saved alarm data with format version %u to file", buffer.version); + NRF_LOG_INFO("[AlarmController] Saved alarm data with format version %u to file", alarm.version); } diff --git a/src/components/alarm/AlarmController.h b/src/components/alarm/AlarmController.h index bc934f8f99..bb3776f929 100644 --- a/src/components/alarm/AlarmController.h +++ b/src/components/alarm/AlarmController.h @@ -38,23 +38,23 @@ namespace Pinetime { void SetOffAlarmNow(); uint32_t SecondsToAlarm(); void StopAlerting(); - enum class AlarmState { Not_Set, Set, Alerting }; enum class RecurType { None, Daily, Weekdays }; uint8_t Hours() const { - return hours; + return alarm.hours; } uint8_t Minutes() const { - return minutes; + return alarm.minutes; } - AlarmState State() const { - return state; + bool IsAlerting() const { + return isAlerting; } - RecurType Recurrence() const { - return recurrence; + bool IsEnabled() const { + return alarm.isEnabled; } - void SetRecurrence(RecurType recurType) { - recurrence = recurType; + RecurType Recurrence() const { + return alarm.recurrence; } + void SetRecurrence(RecurType recurrence); private: static constexpr uint8_t alarmFormatVersion = 1; @@ -62,17 +62,17 @@ namespace Pinetime { uint8_t version = alarmFormatVersion; uint8_t hours = 7; uint8_t minutes = 0; + RecurType recurrence = RecurType::None; + bool isEnabled = false; }; + bool isAlerting = false; Controllers::DateTime& dateTimeController; Controllers::FS& fs; System::SystemTask* systemTask = nullptr; TimerHandle_t alarmTimer; - uint8_t hours = 7; - uint8_t minutes = 0; + AlarmData alarm; std::chrono::time_point alarmTime; - AlarmState state = AlarmState::Not_Set; - RecurType recurrence = RecurType::None; void LoadAlarmFromFile(); void SaveAlarmToFile(); diff --git a/src/displayapp/screens/Alarm.cpp b/src/displayapp/screens/Alarm.cpp index 0b790bd240..01773fd592 100644 --- a/src/displayapp/screens/Alarm.cpp +++ b/src/displayapp/screens/Alarm.cpp @@ -46,9 +46,6 @@ Alarm::Alarm(DisplayApp* app, System::SystemTask& systemTask) : Screen(app), alarmController {alarmController}, systemTask {systemTask} { - hours = alarmController.Hours(); - minutes = alarmController.Minutes(); - hourCounter.Create(); lv_obj_align(hourCounter.GetObject(), nullptr, LV_ALIGN_IN_TOP_LEFT, 0, 0); if (clockType == Controllers::Settings::ClockType::H12) { @@ -60,12 +57,13 @@ Alarm::Alarm(DisplayApp* app, lv_label_set_align(lblampm, LV_LABEL_ALIGN_CENTER); lv_obj_align(lblampm, lv_scr_act(), LV_ALIGN_CENTER, 0, 30); } - hourCounter.SetValue(hours); + hourCounter.SetValue(alarmController.Hours()); hourCounter.SetValueChangedEventCallback(this, ValueChangedHandler); minuteCounter.Create(); lv_obj_align(minuteCounter.GetObject(), nullptr, LV_ALIGN_IN_TOP_RIGHT, 0, 0); - minuteCounter.SetValue(minutes); + minuteCounter.SetValue(alarmController.Minutes()); + minuteCounter.SetValueChangedEventCallback(this, ValueChangedHandler); lv_obj_t* colonLabel = lv_label_create(lv_scr_act(), nullptr); @@ -116,7 +114,7 @@ Alarm::Alarm(DisplayApp* app, UpdateAlarmTime(); - if (alarmController.State() == Controllers::AlarmController::AlarmState::Alerting) { + if (alarmController.IsAlerting()) { SetAlerting(); } else { SetSwitchState(LV_ANIM_OFF); @@ -124,15 +122,14 @@ Alarm::Alarm(DisplayApp* app, } Alarm::~Alarm() { - alarmController.SetAlarmTime(hours, minutes); - if (alarmController.State() == AlarmController::AlarmState::Alerting) { + if (alarmController.IsAlerting()) { StopAlerting(); } lv_obj_clean(lv_scr_act()); } void Alarm::DisableAlarm() { - if (alarmController.State() == AlarmController::AlarmState::Set) { + if (alarmController.IsEnabled()) { alarmController.DisableAlarm(); lv_switch_off(enableSwitch, LV_ANIM_ON); } @@ -154,7 +151,6 @@ void Alarm::OnButtonEvent(lv_obj_t* obj, lv_event_t event) { } if (obj == enableSwitch) { if (lv_switch_get_state(enableSwitch)) { - alarmController.SetAlarmTime(hours, minutes); alarmController.ScheduleAlarm(); } else { alarmController.DisableAlarm(); @@ -173,7 +169,7 @@ bool Alarm::OnButtonPushed() { HideInfo(); return true; } - if (alarmController.State() == AlarmController::AlarmState::Alerting) { + if (alarmController.IsAlerting()) { StopAlerting(); return true; } @@ -182,15 +178,10 @@ bool Alarm::OnButtonPushed() { bool Alarm::OnTouchEvent(Pinetime::Applications::TouchEvents event) { // Don't allow closing the screen by swiping while the alarm is alerting - return alarmController.State() == AlarmController::AlarmState::Alerting && event == TouchEvents::SwipeDown; + return alarmController.IsAlerting() && event == TouchEvents::SwipeDown; } void Alarm::OnValueChanged() { - // Watch out when removing the next line! The selected time is only sent - // to the AlarmController when enabling the toggle and when closing the - // alarm app, to avoid excessive writes to flash. That means when changing - // the time and leaving the app open, the AlarmController is not yet aware - // of the new time and will ring on the previously set time. DisableAlarm(); UpdateAlarmTime(); } @@ -203,8 +194,7 @@ void Alarm::UpdateAlarmTime() { lv_label_set_text_static(lblampm, "AM"); } } - hours = hourCounter.GetValue(); - minutes = minuteCounter.GetValue(); + alarmController.SetAlarmTime(hourCounter.GetValue(), minuteCounter.GetValue()); } void Alarm::SetAlerting() { @@ -227,15 +217,10 @@ void Alarm::StopAlerting() { } void Alarm::SetSwitchState(lv_anim_enable_t anim) { - switch (alarmController.State()) { - case AlarmController::AlarmState::Set: - lv_switch_on(enableSwitch, anim); - break; - case AlarmController::AlarmState::Not_Set: - lv_switch_off(enableSwitch, anim); - break; - default: - break; + if (alarmController.IsEnabled()) { + lv_switch_on(enableSwitch, anim); + } else { + lv_switch_off(enableSwitch, anim); } } @@ -252,7 +237,7 @@ void Alarm::ShowInfo() { txtMessage = lv_label_create(btnMessage, nullptr); lv_obj_set_style_local_bg_color(btnMessage, LV_BTN_PART_MAIN, LV_STATE_DEFAULT, LV_COLOR_NAVY); - if (alarmController.State() == AlarmController::AlarmState::Set) { + if (alarmController.IsEnabled()) { auto timeToAlarm = alarmController.SecondsToAlarm(); auto daysToAlarm = timeToAlarm / 86400; diff --git a/src/displayapp/screens/Alarm.h b/src/displayapp/screens/Alarm.h index 5cf8f162ef..fba9d5d9fb 100644 --- a/src/displayapp/screens/Alarm.h +++ b/src/displayapp/screens/Alarm.h @@ -44,9 +44,6 @@ namespace Pinetime { Controllers::AlarmController& alarmController; System::SystemTask& systemTask; - uint8_t hours = 0; - uint8_t minutes = 0; - lv_obj_t *btnStop, *txtStop, *btnRecur, *txtRecur, *btnInfo, *enableSwitch; lv_obj_t* lblampm = nullptr; lv_obj_t* txtMessage = nullptr; diff --git a/src/systemtask/SystemTask.cpp b/src/systemtask/SystemTask.cpp index 1c871fd2b3..5e4b570034 100644 --- a/src/systemtask/SystemTask.cpp +++ b/src/systemtask/SystemTask.cpp @@ -276,7 +276,7 @@ void SystemTask::Work() { case Messages::OnNewTime: ReloadIdleTimer(); displayApp.PushMessage(Pinetime::Applications::Display::Messages::UpdateDateTime); - if (alarmController.State() == Controllers::AlarmController::AlarmState::Set) { + if (alarmController.IsEnabled()) { alarmController.ScheduleAlarm(); } break; @@ -390,8 +390,7 @@ void SystemTask::Work() { case Messages::OnNewHour: using Pinetime::Controllers::AlarmController; if (settingsController.GetNotificationStatus() != Controllers::Settings::Notification::Sleep && - settingsController.GetChimeOption() == Controllers::Settings::ChimesOption::Hours && - alarmController.State() != AlarmController::AlarmState::Alerting) { + settingsController.GetChimeOption() == Controllers::Settings::ChimesOption::Hours && !alarmController.IsAlerting()) { if (state == SystemTaskState::Sleeping) { GoToRunning(); displayApp.PushMessage(Pinetime::Applications::Display::Messages::Clock); @@ -402,8 +401,7 @@ void SystemTask::Work() { case Messages::OnNewHalfHour: using Pinetime::Controllers::AlarmController; if (settingsController.GetNotificationStatus() != Controllers::Settings::Notification::Sleep && - settingsController.GetChimeOption() == Controllers::Settings::ChimesOption::HalfHours && - alarmController.State() != AlarmController::AlarmState::Alerting) { + settingsController.GetChimeOption() == Controllers::Settings::ChimesOption::HalfHours && !alarmController.IsAlerting()) { if (state == SystemTaskState::Sleeping) { GoToRunning(); displayApp.PushMessage(Pinetime::Applications::Display::Messages::Clock); From 9ece9e3c351529a7a9002de4cdffa7bbd0582f6c Mon Sep 17 00:00:00 2001 From: Galdor Takacs Date: Tue, 20 Sep 2022 16:06:24 +0200 Subject: [PATCH 3/6] AlarmController: Code style fixes --- src/components/alarm/AlarmController.cpp | 26 +++++++++++++----------- src/components/alarm/AlarmController.h | 2 ++ src/displayapp/screens/Alarm.cpp | 1 - 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/components/alarm/AlarmController.cpp b/src/components/alarm/AlarmController.cpp index c153c0a35f..99fbc16400 100644 --- a/src/components/alarm/AlarmController.cpp +++ b/src/components/alarm/AlarmController.cpp @@ -136,33 +136,35 @@ void AlarmController::SetRecurrence(RecurType recurrence) { } void AlarmController::LoadAlarmFromFile() { - lfs_file_t file; - AlarmData buffer; + lfs_file_t alarmFile; + AlarmData alarmBuffer; - if (fs.FileOpen(&file, "/alarm.dat", LFS_O_RDONLY) != LFS_ERR_OK) { + if (fs.FileOpen(&alarmFile, "/alarm.dat", LFS_O_RDONLY) != LFS_ERR_OK) { NRF_LOG_WARNING("[AlarmController] Failed to open alarm data file"); return; } - fs.FileRead(&file, reinterpret_cast(&buffer), sizeof buffer); - fs.FileClose(&file); - if (buffer.version != alarmFormatVersion) { - NRF_LOG_WARNING("[AlarmController] Loaded alarm data has version %u instead of %u, discarding", buffer.version, alarmFormatVersion); + fs.FileRead(&alarmFile, reinterpret_cast(&alarmBuffer), sizeof(alarmBuffer)); + fs.FileClose(&alarmFile); + if (alarmBuffer.version != alarmFormatVersion) { + NRF_LOG_WARNING("[AlarmController] Loaded alarm data has version %u instead of %u, discarding", + alarmBuffer.version, + alarmFormatVersion); return; } - alarm = buffer; + alarm = alarmBuffer; NRF_LOG_INFO("[AlarmController] Loaded alarm data from file"); } void AlarmController::SaveAlarmToFile() { - lfs_file_t file; - if (fs.FileOpen(&file, "/alarm.dat", LFS_O_WRONLY | LFS_O_CREAT) != LFS_ERR_OK) { + lfs_file_t alarmFile; + if (fs.FileOpen(&alarmFile, "/alarm.dat", LFS_O_WRONLY | LFS_O_CREAT) != LFS_ERR_OK) { NRF_LOG_WARNING("[AlarmController] Failed to open alarm data file for saving"); return; } - fs.FileWrite(&file, reinterpret_cast(&alarm), sizeof alarm); - fs.FileClose(&file); + fs.FileWrite(&alarmFile, reinterpret_cast(&alarm), sizeof(alarm)); + fs.FileClose(&alarmFile); NRF_LOG_INFO("[AlarmController] Saved alarm data with format version %u to file", alarm.version); } diff --git a/src/components/alarm/AlarmController.h b/src/components/alarm/AlarmController.h index bb3776f929..345e4ca19d 100644 --- a/src/components/alarm/AlarmController.h +++ b/src/components/alarm/AlarmController.h @@ -57,6 +57,8 @@ namespace Pinetime { void SetRecurrence(RecurType recurrence); private: + // Versions 255 is reserved for now, so the version field can be made + // bigger, should it ever be needed. static constexpr uint8_t alarmFormatVersion = 1; struct AlarmData { uint8_t version = alarmFormatVersion; diff --git a/src/displayapp/screens/Alarm.cpp b/src/displayapp/screens/Alarm.cpp index 01773fd592..4143ca1351 100644 --- a/src/displayapp/screens/Alarm.cpp +++ b/src/displayapp/screens/Alarm.cpp @@ -63,7 +63,6 @@ Alarm::Alarm(DisplayApp* app, minuteCounter.Create(); lv_obj_align(minuteCounter.GetObject(), nullptr, LV_ALIGN_IN_TOP_RIGHT, 0, 0); minuteCounter.SetValue(alarmController.Minutes()); - minuteCounter.SetValueChangedEventCallback(this, ValueChangedHandler); lv_obj_t* colonLabel = lv_label_create(lv_scr_act(), nullptr); From ce296ece8335b31fe014d9ec0098689647172fea Mon Sep 17 00:00:00 2001 From: Galdor Takacs Date: Tue, 20 Sep 2022 23:29:42 +0200 Subject: [PATCH 4/6] Alarm: Only set time when enabling alarm and when exiting This avoids writing the time to flash for every single change. Size impact: +16 (400580 -> 400596) --- src/displayapp/screens/Alarm.cpp | 12 +++++++++--- src/displayapp/screens/Alarm.h | 3 +++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/displayapp/screens/Alarm.cpp b/src/displayapp/screens/Alarm.cpp index 4143ca1351..c5ceb8706a 100644 --- a/src/displayapp/screens/Alarm.cpp +++ b/src/displayapp/screens/Alarm.cpp @@ -46,6 +46,9 @@ Alarm::Alarm(DisplayApp* app, System::SystemTask& systemTask) : Screen(app), alarmController {alarmController}, systemTask {systemTask} { + hours = alarmController.Hours(); + minutes = alarmController.Minutes(); + hourCounter.Create(); lv_obj_align(hourCounter.GetObject(), nullptr, LV_ALIGN_IN_TOP_LEFT, 0, 0); if (clockType == Controllers::Settings::ClockType::H12) { @@ -57,12 +60,12 @@ Alarm::Alarm(DisplayApp* app, lv_label_set_align(lblampm, LV_LABEL_ALIGN_CENTER); lv_obj_align(lblampm, lv_scr_act(), LV_ALIGN_CENTER, 0, 30); } - hourCounter.SetValue(alarmController.Hours()); + hourCounter.SetValue(hours); hourCounter.SetValueChangedEventCallback(this, ValueChangedHandler); minuteCounter.Create(); lv_obj_align(minuteCounter.GetObject(), nullptr, LV_ALIGN_IN_TOP_RIGHT, 0, 0); - minuteCounter.SetValue(alarmController.Minutes()); + minuteCounter.SetValue(minutes); minuteCounter.SetValueChangedEventCallback(this, ValueChangedHandler); lv_obj_t* colonLabel = lv_label_create(lv_scr_act(), nullptr); @@ -124,6 +127,7 @@ Alarm::~Alarm() { if (alarmController.IsAlerting()) { StopAlerting(); } + alarmController.SetAlarmTime(hours, minutes); lv_obj_clean(lv_scr_act()); } @@ -150,6 +154,7 @@ void Alarm::OnButtonEvent(lv_obj_t* obj, lv_event_t event) { } if (obj == enableSwitch) { if (lv_switch_get_state(enableSwitch)) { + alarmController.SetAlarmTime(hours, minutes); alarmController.ScheduleAlarm(); } else { alarmController.DisableAlarm(); @@ -193,7 +198,8 @@ void Alarm::UpdateAlarmTime() { lv_label_set_text_static(lblampm, "AM"); } } - alarmController.SetAlarmTime(hourCounter.GetValue(), minuteCounter.GetValue()); + hours = hourCounter.GetValue(); + minutes = minuteCounter.GetValue(); } void Alarm::SetAlerting() { diff --git a/src/displayapp/screens/Alarm.h b/src/displayapp/screens/Alarm.h index fba9d5d9fb..5cf8f162ef 100644 --- a/src/displayapp/screens/Alarm.h +++ b/src/displayapp/screens/Alarm.h @@ -44,6 +44,9 @@ namespace Pinetime { Controllers::AlarmController& alarmController; System::SystemTask& systemTask; + uint8_t hours = 0; + uint8_t minutes = 0; + lv_obj_t *btnStop, *txtStop, *btnRecur, *txtRecur, *btnInfo, *enableSwitch; lv_obj_t* lblampm = nullptr; lv_obj_t* txtMessage = nullptr; From be6bd9089882fc9a631a612b567c00368eef5e47 Mon Sep 17 00:00:00 2001 From: Reinhold Gschweicher Date: Mon, 26 Sep 2022 21:27:31 +0200 Subject: [PATCH 5/6] Revert "Alarm: Only set time when enabling alarm and when exiting" This reverts commit ce296ece8335b31fe014d9ec0098689647172fea. --- src/displayapp/screens/Alarm.cpp | 12 +++--------- src/displayapp/screens/Alarm.h | 3 --- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/src/displayapp/screens/Alarm.cpp b/src/displayapp/screens/Alarm.cpp index c5ceb8706a..4143ca1351 100644 --- a/src/displayapp/screens/Alarm.cpp +++ b/src/displayapp/screens/Alarm.cpp @@ -46,9 +46,6 @@ Alarm::Alarm(DisplayApp* app, System::SystemTask& systemTask) : Screen(app), alarmController {alarmController}, systemTask {systemTask} { - hours = alarmController.Hours(); - minutes = alarmController.Minutes(); - hourCounter.Create(); lv_obj_align(hourCounter.GetObject(), nullptr, LV_ALIGN_IN_TOP_LEFT, 0, 0); if (clockType == Controllers::Settings::ClockType::H12) { @@ -60,12 +57,12 @@ Alarm::Alarm(DisplayApp* app, lv_label_set_align(lblampm, LV_LABEL_ALIGN_CENTER); lv_obj_align(lblampm, lv_scr_act(), LV_ALIGN_CENTER, 0, 30); } - hourCounter.SetValue(hours); + hourCounter.SetValue(alarmController.Hours()); hourCounter.SetValueChangedEventCallback(this, ValueChangedHandler); minuteCounter.Create(); lv_obj_align(minuteCounter.GetObject(), nullptr, LV_ALIGN_IN_TOP_RIGHT, 0, 0); - minuteCounter.SetValue(minutes); + minuteCounter.SetValue(alarmController.Minutes()); minuteCounter.SetValueChangedEventCallback(this, ValueChangedHandler); lv_obj_t* colonLabel = lv_label_create(lv_scr_act(), nullptr); @@ -127,7 +124,6 @@ Alarm::~Alarm() { if (alarmController.IsAlerting()) { StopAlerting(); } - alarmController.SetAlarmTime(hours, minutes); lv_obj_clean(lv_scr_act()); } @@ -154,7 +150,6 @@ void Alarm::OnButtonEvent(lv_obj_t* obj, lv_event_t event) { } if (obj == enableSwitch) { if (lv_switch_get_state(enableSwitch)) { - alarmController.SetAlarmTime(hours, minutes); alarmController.ScheduleAlarm(); } else { alarmController.DisableAlarm(); @@ -198,8 +193,7 @@ void Alarm::UpdateAlarmTime() { lv_label_set_text_static(lblampm, "AM"); } } - hours = hourCounter.GetValue(); - minutes = minuteCounter.GetValue(); + alarmController.SetAlarmTime(hourCounter.GetValue(), minuteCounter.GetValue()); } void Alarm::SetAlerting() { diff --git a/src/displayapp/screens/Alarm.h b/src/displayapp/screens/Alarm.h index 5cf8f162ef..fba9d5d9fb 100644 --- a/src/displayapp/screens/Alarm.h +++ b/src/displayapp/screens/Alarm.h @@ -44,9 +44,6 @@ namespace Pinetime { Controllers::AlarmController& alarmController; System::SystemTask& systemTask; - uint8_t hours = 0; - uint8_t minutes = 0; - lv_obj_t *btnStop, *txtStop, *btnRecur, *txtRecur, *btnInfo, *enableSwitch; lv_obj_t* lblampm = nullptr; lv_obj_t* txtMessage = nullptr; From aa7eda753af125c0ee61a6e48ef4f75d477e21bb Mon Sep 17 00:00:00 2001 From: Reinhold Gschweicher Date: Mon, 26 Sep 2022 21:46:09 +0200 Subject: [PATCH 6/6] AlarmController: save on enabled toggle and on Alarm sceen closing Introduce `SaveAlarm()` member function which saves to file only if the alarm setting is changed. The `alarmChanged` boolean flag mimics the behavior of `settingsChanged` flag from `Settings.h` Additionally save the alarm status when the enabled flag is toggled immediately. To be sure a disabled Alarm stays disabled on a crash. Also mark the `SaveAlarmToFile()` function const, as it doesn't change anything in the `AlarmController` object. --- src/components/alarm/AlarmController.cpp | 29 ++++++++++++++++++------ src/components/alarm/AlarmController.h | 4 +++- src/displayapp/screens/Alarm.cpp | 1 + 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/src/components/alarm/AlarmController.cpp b/src/components/alarm/AlarmController.cpp index 99fbc16400..2cb1f425a8 100644 --- a/src/components/alarm/AlarmController.cpp +++ b/src/components/alarm/AlarmController.cpp @@ -45,13 +45,22 @@ void AlarmController::Init(System::SystemTask* systemTask) { } } +void AlarmController::SaveAlarm() { + + // verify if is necessary to save + if (alarmChanged) { + SaveAlarmToFile(); + } + alarmChanged = false; +} + void AlarmController::SetAlarmTime(uint8_t alarmHr, uint8_t alarmMin) { if (alarm.hours == alarmHr && alarm.minutes == alarmMin) { return; } alarm.hours = alarmHr; alarm.minutes = alarmMin; - SaveAlarmToFile(); + alarmChanged = true; } void AlarmController::ScheduleAlarm() { @@ -93,7 +102,9 @@ void AlarmController::ScheduleAlarm() { if (!alarm.isEnabled) { alarm.isEnabled = true; - SaveAlarmToFile(); + // store enabled alarm immediately + alarmChanged = true; + SaveAlarm(); } } @@ -106,7 +117,9 @@ void AlarmController::DisableAlarm() { isAlerting = false; if (alarm.isEnabled) { alarm.isEnabled = false; - SaveAlarmToFile(); + // store disabled alarm immediately + alarmChanged = true; + SaveAlarm(); } } @@ -120,7 +133,9 @@ void AlarmController::StopAlerting() { // Disable alarm unless it is recurring if (alarm.recurrence == RecurType::None) { alarm.isEnabled = false; - SaveAlarmToFile(); + // store disabled alarm immediately + alarmChanged = true; + SaveAlarm(); } else { // set next instance ScheduleAlarm(); @@ -131,7 +146,7 @@ void AlarmController::StopAlerting() { void AlarmController::SetRecurrence(RecurType recurrence) { if (alarm.recurrence != recurrence) { alarm.recurrence = recurrence; - SaveAlarmToFile(); + alarmChanged = true; } } @@ -157,14 +172,14 @@ void AlarmController::LoadAlarmFromFile() { NRF_LOG_INFO("[AlarmController] Loaded alarm data from file"); } -void AlarmController::SaveAlarmToFile() { +void AlarmController::SaveAlarmToFile() const { lfs_file_t alarmFile; if (fs.FileOpen(&alarmFile, "/alarm.dat", LFS_O_WRONLY | LFS_O_CREAT) != LFS_ERR_OK) { NRF_LOG_WARNING("[AlarmController] Failed to open alarm data file for saving"); return; } - fs.FileWrite(&alarmFile, reinterpret_cast(&alarm), sizeof(alarm)); + fs.FileWrite(&alarmFile, reinterpret_cast(&alarm), sizeof(alarm)); fs.FileClose(&alarmFile); NRF_LOG_INFO("[AlarmController] Saved alarm data with format version %u to file", alarm.version); } diff --git a/src/components/alarm/AlarmController.h b/src/components/alarm/AlarmController.h index 345e4ca19d..408525169d 100644 --- a/src/components/alarm/AlarmController.h +++ b/src/components/alarm/AlarmController.h @@ -32,6 +32,7 @@ namespace Pinetime { AlarmController(Controllers::DateTime& dateTimeController, Controllers::FS& fs); void Init(System::SystemTask* systemTask); + void SaveAlarm(); void SetAlarmTime(uint8_t alarmHr, uint8_t alarmMin); void ScheduleAlarm(); void DisableAlarm(); @@ -68,6 +69,7 @@ namespace Pinetime { bool isEnabled = false; }; bool isAlerting = false; + bool alarmChanged = false; Controllers::DateTime& dateTimeController; Controllers::FS& fs; @@ -77,7 +79,7 @@ namespace Pinetime { std::chrono::time_point alarmTime; void LoadAlarmFromFile(); - void SaveAlarmToFile(); + void SaveAlarmToFile() const; }; } } diff --git a/src/displayapp/screens/Alarm.cpp b/src/displayapp/screens/Alarm.cpp index 4143ca1351..41c72efdf8 100644 --- a/src/displayapp/screens/Alarm.cpp +++ b/src/displayapp/screens/Alarm.cpp @@ -125,6 +125,7 @@ Alarm::~Alarm() { StopAlerting(); } lv_obj_clean(lv_scr_act()); + alarmController.SaveAlarm(); } void Alarm::DisableAlarm() {