Fixes based on code reviews (formatting, UI code)

src/components/alarm/AlarmController.cpp

@@ -1,8 +1,16 @@
-//
+/*  Copyright (C) 2021 JF, Adam Pigg, Avamander
-// Created by mrussell on 30.08.21.
+    This file is part of InfiniTime.
-//
+    InfiniTime is free software: you can redistribute it and/or modify
-// Copied from Florian's Timer app
+    it under the terms of the GNU General Public License as published
-
+    by the Free Software Foundation, either version 3 of the License, or
+    (at your option) any later version.
+    InfiniTime is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+    You should have received a copy of the GNU General Public License
+    along with this program.  If not, see <https://www.gnu.org/licenses/>.
+*/
 #include "AlarmController.h"
 #include "systemtask/SystemTask.h"
 #include "app_timer.h"
@@ -25,18 +33,19 @@ namespace {
   }
 }
 
-void AlarmController::Init() {
+void AlarmController::Init(System::SystemTask* systemTask) {
   app_timer_create(&alarmAppTimer, APP_TIMER_MODE_SINGLE_SHOT, SetOffAlarm);
+  this->systemTask = systemTask;
 }
 
 void AlarmController::SetAlarm(uint8_t alarmHr, uint8_t alarmMin) {
   hours = alarmHr;
   minutes = alarmMin;
   state = AlarmState::Set;
-  scheduleAlarm();
+  ScheduleAlarm();
 }
 
-void AlarmController::scheduleAlarm() {
+void AlarmController::ScheduleAlarm() {
   // Determine the next time the alarm needs to go off and set the app_timer
   app_timer_stop(alarmAppTimer);
 
@@ -83,15 +92,11 @@ void AlarmController::DisableAlarm() {
 
 void AlarmController::SetOffAlarmNow() {
   state = AlarmState::Alerting;
-  if (systemTask != nullptr) {
+  systemTask->PushMessage(System::Messages::SetOffAlarm);
-    systemTask->PushMessage(System::Messages::SetOffAlarm);
-  }
 }
 
 void AlarmController::StopAlerting() {
-  if (systemTask != nullptr) {
+  systemTask->PushMessage(System::Messages::StopRinging);
-    systemTask->PushMessage(System::Messages::StopRinging);
-  }
 
   // Alarm state is off unless this is a recurring alarm
   if (recurrence == RecurType::None) {
@@ -99,20 +104,6 @@ void AlarmController::StopAlerting() {
   } else {
     state = AlarmState::Set;
     // set next instance
-    scheduleAlarm();
+    ScheduleAlarm();
   }
 }
-
-void AlarmController::ToggleRecurrence() {
-  if (recurrence == AlarmController::RecurType::None) {
-    recurrence = AlarmController::RecurType::Daily;
-  } else if (recurrence == AlarmController::RecurType::Daily) {
-    recurrence = AlarmController::RecurType::Weekdays;
-  } else {
-    recurrence = AlarmController::RecurType::None;
-  }
-}
-
-void AlarmController::Register(Pinetime::System::SystemTask* systemTask) {
-  this->systemTask = systemTask;
-}

src/components/alarm/AlarmController.h

@@ -1,3 +1,16 @@
+/*  Copyright (C) 2021 JF, Adam Pigg, Avamander
+    This file is part of InfiniTime.
+    InfiniTime is free software: you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published
+    by the Free Software Foundation, either version 3 of the License, or
+    (at your option) any later version.
+    InfiniTime is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+    You should have received a copy of the GNU General Public License
+    along with this program.  If not, see <https://www.gnu.org/licenses/>.
+*/
 #pragma once
 
 #include <cstdint>
@@ -13,16 +26,14 @@ namespace Pinetime {
     public:
       AlarmController(Controllers::DateTime& dateTimeController);
 
-      void Init();
+      void Init(System::SystemTask* systemTask);
       void SetAlarm(uint8_t alarmHr, uint8_t alarmMin);
       void DisableAlarm();
       void SetOffAlarmNow();
       uint32_t SecondsToAlarm();
       void StopAlerting();
-      void Register(System::SystemTask* systemTask);
       enum class AlarmState { Not_Set, Set, Alerting };
       enum class RecurType { None, Daily, Weekdays };
-      void ToggleRecurrence();
       uint8_t Hours() const {
         return hours;
       }
@@ -35,6 +46,9 @@ namespace Pinetime {
       RecurType Recurrence() const {
         return recurrence;
       }
+      void SetRecurrence(RecurType recurType) {
+        recurrence = recurType;
+      }
 
     private:
       Controllers::DateTime& dateTimeController;
@@ -42,9 +56,9 @@ namespace Pinetime {
       uint8_t hours;
       uint8_t minutes;
       std::chrono::time_point<std::chrono::system_clock, std::chrono::nanoseconds> alarmTime;
-      AlarmState state = AlarmState::Not_Set; 
+      AlarmState state = AlarmState::Not_Set;
       RecurType recurrence = RecurType::None;
-      void scheduleAlarm();
+      void ScheduleAlarm();
     };
   }
-}
+}

src/displayapp/screens/Alarm.cpp

@@ -1,3 +1,16 @@
+/*  Copyright (C) 2021 JF, Adam Pigg, Avamander
+    This file is part of InfiniTime.
+    InfiniTime is free software: you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published
+    by the Free Software Foundation, either version 3 of the License, or
+    (at your option) any later version.
+    InfiniTime is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+    You should have received a copy of the GNU General Public License
+    along with this program.  If not, see <https://www.gnu.org/licenses/>.
+*/
 #include "Alarm.h"
 #include "Screen.h"
 #include "Symbols.h"
@@ -21,66 +34,61 @@ Alarm::Alarm(DisplayApp* app, Controllers::AlarmController& alarmController)
   alarmMinutes = alarmController.Minutes();
   lv_label_set_text_fmt(time, "%02lu:%02lu", alarmHours, alarmMinutes);
 
-  lv_obj_align(time, lv_scr_act(), LV_ALIGN_IN_LEFT_MID, 0, -20);
+  lv_obj_align(time, lv_scr_act(), LV_ALIGN_CENTER, 0, -25);
 
   btnHoursUp = lv_btn_create(lv_scr_act(), nullptr);
   btnHoursUp->user_data = this;
   lv_obj_set_event_cb(btnHoursUp, btnEventHandler);
-  lv_obj_align(btnHoursUp, lv_scr_act(), LV_ALIGN_IN_LEFT_MID, 20, -80);
+  lv_obj_set_size(btnHoursUp, 60, 40);
-  lv_obj_set_height(btnHoursUp, 40);
+  lv_obj_align(btnHoursUp, lv_scr_act(), LV_ALIGN_IN_LEFT_MID, 20, -85);
-  lv_obj_set_width(btnHoursUp, 60);
   txtHrUp = lv_label_create(btnHoursUp, nullptr);
   lv_label_set_text(txtHrUp, "+");
 
   btnHoursDown = lv_btn_create(lv_scr_act(), nullptr);
   btnHoursDown->user_data = this;
   lv_obj_set_event_cb(btnHoursDown, btnEventHandler);
-  lv_obj_align(btnHoursDown, lv_scr_act(), LV_ALIGN_IN_LEFT_MID, 20, +40);
+  lv_obj_set_size(btnHoursDown, 60, 40);
-  lv_obj_set_height(btnHoursDown, 40);
+  lv_obj_align(btnHoursDown, lv_scr_act(), LV_ALIGN_IN_LEFT_MID, 20, 35);
-  lv_obj_set_width(btnHoursDown, 60);
   txtHrDown = lv_label_create(btnHoursDown, nullptr);
   lv_label_set_text(txtHrDown, "-");
 
   btnMinutesUp = lv_btn_create(lv_scr_act(), nullptr);
   btnMinutesUp->user_data = this;
   lv_obj_set_event_cb(btnMinutesUp, btnEventHandler);
-  lv_obj_align(btnMinutesUp, lv_scr_act(), LV_ALIGN_IN_RIGHT_MID, 10, -80);
+  lv_obj_set_size(btnMinutesUp, 60, 40);
-  lv_obj_set_height(btnMinutesUp, 40);
+  lv_obj_align(btnMinutesUp, lv_scr_act(), LV_ALIGN_IN_RIGHT_MID, -20, -85);
-  lv_obj_set_width(btnMinutesUp, 60);
   txtMinUp = lv_label_create(btnMinutesUp, nullptr);
   lv_label_set_text(txtMinUp, "+");
 
   btnMinutesDown = lv_btn_create(lv_scr_act(), nullptr);
   btnMinutesDown->user_data = this;
   lv_obj_set_event_cb(btnMinutesDown, btnEventHandler);
-  lv_obj_align(btnMinutesDown, lv_scr_act(), LV_ALIGN_IN_RIGHT_MID, 10, +40);
+  lv_obj_set_size(btnMinutesDown, 60, 40);
-  lv_obj_set_height(btnMinutesDown, 40);
+  lv_obj_align(btnMinutesDown, lv_scr_act(), LV_ALIGN_IN_RIGHT_MID, -20, 35);
-  lv_obj_set_width(btnMinutesDown, 60);
   txtMinDown = lv_label_create(btnMinutesDown, nullptr);
   lv_label_set_text(txtMinDown, "-");
 
   btnEnable = lv_btn_create(lv_scr_act(), nullptr);
   btnEnable->user_data = this;
   lv_obj_set_event_cb(btnEnable, btnEventHandler);
-  lv_obj_align(btnEnable, lv_scr_act(), LV_ALIGN_IN_BOTTOM_LEFT, 3, -10);
+  lv_obj_set_size(btnEnable, 115, 50);
-  lv_obj_set_height(btnEnable, 40);
+  lv_obj_align(btnEnable, lv_scr_act(), LV_ALIGN_IN_BOTTOM_LEFT, 0, 0);
   txtEnable = lv_label_create(btnEnable, nullptr);
-  setEnableButtonState();
+  SetEnableButtonState();
 
   btnRecur = lv_btn_create(lv_scr_act(), nullptr);
   btnRecur->user_data = this;
   lv_obj_set_event_cb(btnRecur, btnEventHandler);
-  lv_obj_align(btnRecur, lv_scr_act(), LV_ALIGN_IN_BOTTOM_RIGHT, -3, -10);
+  lv_obj_set_size(btnRecur, 115, 50);
-  lv_obj_set_height(btnRecur, 40);
+  lv_obj_align(btnRecur, lv_scr_act(), LV_ALIGN_IN_BOTTOM_RIGHT, 0, 0);
   txtRecur = lv_label_create(btnRecur, nullptr);
-  setRecurButtonState();
+  SetRecurButtonState();
 
   btnInfo = lv_btn_create(lv_scr_act(), nullptr);
   btnInfo->user_data = this;
   lv_obj_set_event_cb(btnInfo, btnEventHandler);
-  lv_obj_align(btnInfo, lv_scr_act(), LV_ALIGN_CENTER, 30, -80);
+  lv_obj_set_size(btnInfo, 50, 40);
-  lv_obj_set_height(btnInfo, 40);
+  lv_obj_align(btnInfo, lv_scr_act(), LV_ALIGN_CENTER, 0, -85);
-  lv_obj_set_width(btnInfo, 30);
   txtInfo = lv_label_create(btnInfo, nullptr);
   lv_label_set_text(txtInfo, "i");
 }
@@ -100,11 +108,11 @@ void Alarm::OnButtonEvent(lv_obj_t* obj, lv_event_t event) {
       } else {
         alarmController.SetAlarm(alarmHours, alarmMinutes);
       }
-      setEnableButtonState();
+      SetEnableButtonState();
       return;
     }
     if (obj == btnInfo) {
-      showInfo();
+      ShowInfo();
       return;
     }
     if (obj == btnMessage) {
@@ -120,7 +128,7 @@ void Alarm::OnButtonEvent(lv_obj_t* obj, lv_event_t event) {
     // can just do it once when the alarm is re-enabled
     if (alarmController.State() == AlarmController::AlarmState::Set) {
       alarmController.DisableAlarm();
-      setEnableButtonState();
+      SetEnableButtonState();
     }
     if (obj == btnMinutesUp) {
       if (alarmMinutes >= 59) {
@@ -159,17 +167,16 @@ void Alarm::OnButtonEvent(lv_obj_t* obj, lv_event_t event) {
       return;
     }
     if (obj == btnRecur) {
-      alarmController.ToggleRecurrence();
+      ToggleRecurrence();
-      setRecurButtonState();
     }
   }
 }
 
 void Alarm::SetAlerting() {
-  setEnableButtonState();
+  SetEnableButtonState();
 }
 
-void Alarm::setEnableButtonState() {
+void Alarm::SetEnableButtonState() {
   switch (alarmController.State()) {
     case AlarmController::AlarmState::Set:
       lv_label_set_text(txtEnable, "ON");
@@ -185,7 +192,7 @@ void Alarm::setEnableButtonState() {
   }
 }
 
-void Alarm::showInfo() {
+void Alarm::ShowInfo() {
   btnMessage = lv_btn_create(lv_scr_act(), nullptr);
   btnMessage->user_data = this;
   lv_obj_set_event_cb(btnMessage, btnEventHandler);
@@ -210,7 +217,7 @@ void Alarm::showInfo() {
   }
 }
 
-void Alarm::setRecurButtonState() {
+void Alarm::SetRecurButtonState() {
   using Pinetime::Controllers::AlarmController;
   switch (alarmController.Recurrence()) {
     case AlarmController::RecurType::None:
@@ -220,6 +227,21 @@ void Alarm::setRecurButtonState() {
       lv_label_set_text(txtRecur, "DAILY");
       break;
     case AlarmController::RecurType::Weekdays:
-      lv_label_set_text(txtRecur, "WKDAYS");
+      lv_label_set_text(txtRecur, "MON-FRI");
   }
-}
+}
+
+void Alarm::ToggleRecurrence() {
+  using Pinetime::Controllers::AlarmController;
+  switch (alarmController.Recurrence()) {
+    case AlarmController::RecurType::None:
+      alarmController.SetRecurrence(AlarmController::RecurType::Daily);
+      break;
+    case AlarmController::RecurType::Daily:
+      alarmController.SetRecurrence(AlarmController::RecurType::Weekdays);
+      break;
+    case AlarmController::RecurType::Weekdays:
+      alarmController.SetRecurrence(AlarmController::RecurType::None);
+  }
+  SetRecurButtonState();
+}

src/displayapp/screens/Alarm.h

@@ -1,3 +1,16 @@
+/*  Copyright (C) 2021 JF, Adam Pigg, Avamander
+    This file is part of InfiniTime.
+    InfiniTime is free software: you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published
+    by the Free Software Foundation, either version 3 of the License, or
+    (at your option) any later version.
+    InfiniTime is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+    You should have received a copy of the GNU General Public License
+    along with this program.  If not, see <https://www.gnu.org/licenses/>.
+*/
 #pragma once
 
 #include "Screen.h"
@@ -5,27 +18,32 @@
 #include "../LittleVgl.h"
 #include "components/alarm/AlarmController.h"
 
-namespace Pinetime::Applications::Screens {
+namespace Pinetime {
-  class Alarm : public Screen {
+  namespace Applications {
-  public:
+    namespace Screens {
-    Alarm(DisplayApp* app, Controllers::AlarmController& alarmController);
+      class Alarm : public Screen {
-    ~Alarm() override;
+      public:
-    void SetAlerting();
+        Alarm(DisplayApp* app, Controllers::AlarmController& alarmController);
-    void OnButtonEvent(lv_obj_t* obj, lv_event_t event);
+        ~Alarm() override;
+        void SetAlerting();
+        void OnButtonEvent(lv_obj_t* obj, lv_event_t event);
 
-  private:
+      private:
-    bool running;
+        bool running;
-    uint8_t alarmHours = 0;
+        uint8_t alarmHours = 0;
-    uint8_t alarmMinutes = 0;
+        uint8_t alarmMinutes = 0;
-    Controllers::AlarmController& alarmController;
+        Controllers::AlarmController& alarmController;
 
-    lv_obj_t *time, *btnEnable, *txtEnable, *btnMinutesUp, *btnMinutesDown, *btnHoursUp, *btnHoursDown, *txtMinUp, *txtMinDown, *txtHrUp,
+        lv_obj_t *time, *btnEnable, *txtEnable, *btnMinutesUp, *btnMinutesDown, *btnHoursUp, *btnHoursDown, *txtMinUp, *txtMinDown,
-      *txtHrDown, *btnRecur, *txtRecur, *btnMessage, *txtMessage, *btnInfo, *txtInfo;
+          *txtHrUp, *txtHrDown, *btnRecur, *txtRecur, *btnMessage, *txtMessage, *btnInfo, *txtInfo;
 
-    enum class EnableButtonState { On, Off, Alerting };
+        enum class EnableButtonState { On, Off, Alerting };
-    void setEnableButtonState();
+        void SetEnableButtonState();
-    void setRecurButtonState();
+        void SetRecurButtonState();
-    void setAlarm();
+        void SetAlarm();
-    void showInfo();
+        void ShowInfo();
+        void ToggleRecurrence();
+      };
+    };
   };
 }

src/systemtask/SystemTask.cpp

@@ -134,8 +134,7 @@ void SystemTask::Work() {
   motionSensor.SoftReset();
   timerController.Register(this);
   timerController.Init();
-  alarmController.Register(this);
+  alarmController.Init(this);
-  alarmController.Init();
 
   // Reset the TWI device because the motion sensor chip most probably crashed it...
   twiMaster.Sleep();