From baca0fc3e59e88420d6c7983ad133fe63c794ec0 Mon Sep 17 00:00:00 2001 From: JF Date: Sat, 28 Mar 2020 19:05:28 +0100 Subject: [PATCH] Encapsulate Notification management in NotificationManager. It implement a static array of notifications to avoid dynamic allocation. --- src/CMakeLists.txt | 2 ++ src/Components/Ble/BleController.cpp | 24 ------------- src/Components/Ble/BleController.h | 11 +----- src/Components/Ble/NotificationManager.cpp | 29 +++++++++++++++ src/Components/Ble/NotificationManager.h | 29 +++++++++++++++ src/DisplayApp/DisplayApp.cpp | 25 ++++++------- src/DisplayApp/DisplayApp.h | 16 ++++----- src/DisplayApp/Screens/Modal.cpp | 41 ++-------------------- src/DisplayApp/Screens/Modal.h | 4 +-- src/DisplayApp/Screens/Tile.cpp | 4 ++- src/SystemTask/SystemTask.cpp | 15 ++++---- src/SystemTask/SystemTask.h | 13 ++++--- src/main.cpp | 8 +++-- 13 files changed, 105 insertions(+), 116 deletions(-) create mode 100644 src/Components/Ble/NotificationManager.cpp create mode 100644 src/Components/Ble/NotificationManager.h diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 5c521b33..e92e3998 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -223,6 +223,7 @@ list(APPEND SOURCE_FILES BLE/BleManager.c Components/Battery/BatteryController.cpp Components/Ble/BleController.cpp + Components/Ble/NotificationManager.cpp Components/DateTime/DateTimeController.cpp Components/Brightness/BrightnessController.cpp drivers/Cst816s.cpp @@ -264,6 +265,7 @@ set(INCLUDE_FILES BLE/BleManager.h Components/Battery/BatteryController.h Components/Ble/BleController.h + Components/Ble/NotificationManager.h Components/DateTime/DateTimeController.h Components/Brightness/BrightnessController.h drivers/Cst816s.h diff --git a/src/Components/Ble/BleController.cpp b/src/Components/Ble/BleController.cpp index fd405896..5fa51688 100644 --- a/src/Components/Ble/BleController.cpp +++ b/src/Components/Ble/BleController.cpp @@ -4,10 +4,6 @@ using namespace Pinetime::Controllers; -Ble::Ble() { - notificationQueue = xQueueCreate(10, sizeof(NotificationMessage)); -} - void Ble::Connect() { isConnected = true; } @@ -16,24 +12,4 @@ void Ble::Disconnect() { isConnected = false; } -void Ble::PushNotification(const char *message, uint8_t size) { - char* messageCopy = static_cast(malloc(sizeof(char) * size)); - std::memcpy(messageCopy, message, size); - NotificationMessage msg; - msg.size = size; - msg.message = messageCopy; - - BaseType_t xHigherPriorityTaskWoken; - xHigherPriorityTaskWoken = pdFALSE; - xQueueSendFromISR(notificationQueue, &msg, &xHigherPriorityTaskWoken); - if (xHigherPriorityTaskWoken) { - /* Actual macro used here is port specific. */ - // TODO : should I do something here? - } -} - -bool Ble::PopNotification(Ble::NotificationMessage& msg) { - return xQueueReceive(notificationQueue, &msg, 0) != 0; -} - diff --git a/src/Components/Ble/BleController.h b/src/Components/Ble/BleController.h index 4f037fc1..31d66986 100644 --- a/src/Components/Ble/BleController.h +++ b/src/Components/Ble/BleController.h @@ -7,22 +7,13 @@ namespace Pinetime { namespace Controllers { class Ble { public: - struct NotificationMessage { - uint8_t size = 0; - const char* message = nullptr; - }; - Ble(); + Ble() = default; bool IsConnected() const {return isConnected;} void Connect(); void Disconnect(); - - void PushNotification(const char* message, uint8_t size); - bool PopNotification(NotificationMessage& msg); - private: bool isConnected = false; - QueueHandle_t notificationQueue; }; } diff --git a/src/Components/Ble/NotificationManager.cpp b/src/Components/Ble/NotificationManager.cpp new file mode 100644 index 00000000..2e02cb15 --- /dev/null +++ b/src/Components/Ble/NotificationManager.cpp @@ -0,0 +1,29 @@ +#include +#include "NotificationManager.h" + +using namespace Pinetime::Controllers; + +void NotificationManager::Push(Pinetime::Controllers::NotificationManager::Categories category, + const char *message, uint8_t messageSize) { + // TODO handle edge cases on read/write index + auto& notif = notifications[writeIndex]; + std::memcpy(notif.message.data(), message, messageSize); + notif.message[messageSize] = '\0'; + notif.category = category; + + writeIndex = (writeIndex + 1 < TotalNbNotifications) ? writeIndex + 1 : 0; + if(!empty && writeIndex == readIndex) + readIndex = writeIndex + 1; +} + +NotificationManager::Notification Pinetime::Controllers::NotificationManager::Pop() { +// TODO handle edge cases on read/write index + NotificationManager::Notification notification = notifications[readIndex]; + + if(readIndex != writeIndex) { + readIndex = (readIndex + 1 < TotalNbNotifications) ? readIndex + 1 : 0; + } + + // TODO Check move optimization on return + return notification; +} diff --git a/src/Components/Ble/NotificationManager.h b/src/Components/Ble/NotificationManager.h new file mode 100644 index 00000000..8edd6828 --- /dev/null +++ b/src/Components/Ble/NotificationManager.h @@ -0,0 +1,29 @@ +#pragma once + +#include + +namespace Pinetime { + namespace Controllers { + class NotificationManager { + public: + enum class Categories {Unknown, SimpleAlert, Email, News, IncomingCall, MissedCall, Sms, VoiceMail, Schedule, HighProriotyAlert, InstantMessage }; + static constexpr uint8_t MessageSize = 18; + + struct Notification { + std::array message; + Categories category = Categories::Unknown; + }; + + void Push(Categories category, const char* message, uint8_t messageSize); + Notification Pop(); + + + private: + static constexpr uint8_t TotalNbNotifications = 5; + std::array notifications; + uint8_t readIndex = 0; + uint8_t writeIndex = 0; + bool empty = true; + }; + } +} \ No newline at end of file diff --git a/src/DisplayApp/DisplayApp.cpp b/src/DisplayApp/DisplayApp.cpp index 2e07cbc5..1b4515e0 100644 --- a/src/DisplayApp/DisplayApp.cpp +++ b/src/DisplayApp/DisplayApp.cpp @@ -15,18 +15,16 @@ #include #include #include +#include #include "../SystemTask/SystemTask.h" using namespace Pinetime::Applications; -DisplayApp::DisplayApp(Pinetime::Drivers::St7789& lcd, - Pinetime::Components::LittleVgl& lvgl, - Pinetime::Drivers::Cst816S& touchPanel, - Controllers::Battery &batteryController, - Controllers::Ble &bleController, - Controllers::DateTime &dateTimeController, - Pinetime::Drivers::WatchdogView& watchdog, - Pinetime::System::SystemTask& systemTask) : +DisplayApp::DisplayApp(Drivers::St7789 &lcd, Components::LittleVgl &lvgl, Drivers::Cst816S &touchPanel, + Controllers::Battery &batteryController, Controllers::Ble &bleController, + Controllers::DateTime &dateTimeController, Drivers::WatchdogView &watchdog, + System::SystemTask &systemTask, + Pinetime::Controllers::NotificationManager& notificationManager) : lcd{lcd}, lvgl{lvgl}, batteryController{batteryController}, @@ -35,7 +33,8 @@ DisplayApp::DisplayApp(Pinetime::Drivers::St7789& lcd, watchdog{watchdog}, touchPanel{touchPanel}, currentScreen{new Screens::Clock(this, dateTimeController, batteryController, bleController) }, - systemTask{systemTask} { + systemTask{systemTask}, + notificationManager{notificationManager} { msgQueue = xQueueCreate(queueSize, itemSize); onClockApp = true; modal.reset(new Screens::Modal(this)); @@ -113,12 +112,8 @@ void DisplayApp::Refresh() { // clockScreen.SetBatteryPercentRemaining(batteryController.PercentRemaining()); break; case Messages::NewNotification: { - Pinetime::Controllers::Ble::NotificationMessage notificationMessage; - if (bleController.PopNotification(notificationMessage)) { - std::string m {notificationMessage.message, notificationMessage.size}; - modal->Show(m); - // TODO delete message - } + auto notification = notificationManager.Pop(); + modal->Show(notification.message.data()); } break; case Messages::TouchEvent: { diff --git a/src/DisplayApp/DisplayApp.h b/src/DisplayApp/DisplayApp.h index ad817331..09f0d1cd 100644 --- a/src/DisplayApp/DisplayApp.h +++ b/src/DisplayApp/DisplayApp.h @@ -17,6 +17,7 @@ #include #include #include +#include #include "TouchEvents.h" @@ -34,14 +35,11 @@ namespace Pinetime { enum class FullRefreshDirections { None, Up, Down }; - DisplayApp(Pinetime::Drivers::St7789& lcd, - Pinetime::Components::LittleVgl& lvgl, - Pinetime::Drivers::Cst816S&, - Controllers::Battery &batteryController, - Controllers::Ble &bleController, - Controllers::DateTime& dateTimeController, - Pinetime::Drivers::WatchdogView& watchdog, - Pinetime::System::SystemTask& systemTask); + DisplayApp(Drivers::St7789 &lcd, Components::LittleVgl &lvgl, Drivers::Cst816S &, + Controllers::Battery &batteryController, Controllers::Ble &bleController, + Controllers::DateTime &dateTimeController, Drivers::WatchdogView &watchdog, + System::SystemTask &systemTask, + Pinetime::Controllers::NotificationManager& notificationManager); void Start(); void PushMessage(Messages msg); @@ -82,7 +80,7 @@ namespace Pinetime { bool onClockApp = false; // TODO find a better way to know that we should handle gestures and button differently for the Clock app. Controllers::BrightnessController brightnessController; std::unique_ptr modal; - + Pinetime::Controllers::NotificationManager& notificationManager; }; } } diff --git a/src/DisplayApp/Screens/Modal.cpp b/src/DisplayApp/Screens/Modal.cpp index ec477b6e..63ae70c0 100644 --- a/src/DisplayApp/Screens/Modal.cpp +++ b/src/DisplayApp/Screens/Modal.cpp @@ -25,42 +25,6 @@ bool Modal::OnButtonPushed() { return true; } -void Modal::Show() { - if(isVisible) return; - isVisible = true; - lv_style_copy(&modal_style, &lv_style_plain_color); - modal_style.body.main_color = modal_style.body.grad_color = LV_COLOR_BLACK; - modal_style.body.opa = LV_OPA_50; - - obj = lv_obj_create(lv_scr_act(), NULL); - lv_obj_set_style(obj, &modal_style); - lv_obj_set_pos(obj, 0, 0); - lv_obj_set_size(obj, LV_HOR_RES, LV_VER_RES); - lv_obj_set_opa_scale_enable(obj, true); /* Enable opacity scaling for the animation */ - - static const char * btns2[] = {"Ok", ""}; - - /* Create the message box as a child of the modal background */ - mbox = lv_mbox_create(obj, NULL); - lv_mbox_add_btns(mbox, btns2); - char versionStr[20]; - sprintf(versionStr, "VERSION: %d.%d.%d", Version::Major(), Version::Minor(), Version::Patch()); - lv_mbox_set_text(mbox, versionStr); -// lv_mbox_set_text(mbox, "Hello world!"); - lv_obj_align(mbox, NULL, LV_ALIGN_CENTER, 0, 0); - lv_obj_set_event_cb(mbox, Modal::mbox_event_cb); - - mbox->user_data = this; - - /* Fade the message box in with an animation */ - lv_anim_t a; - lv_anim_init(&a); - lv_anim_set_time(&a, 500, 0); - lv_anim_set_values(&a, LV_OPA_TRANSP, LV_OPA_COVER); - lv_anim_set_exec_cb(&a, obj, (lv_anim_exec_xcb_t)lv_obj_set_opa_scale); - lv_anim_create(&a); -} - void Modal::Hide() { /* Delete the parent modal background */ lv_obj_del_async(lv_obj_get_parent(mbox)); @@ -83,9 +47,8 @@ void Modal::OnEvent(lv_obj_t *event_obj, lv_event_t evt) { } } -void Modal::Show(const std::string& message) { +void Modal::Show(const char* msg) { if(isVisible) return; - this->message = message; isVisible = true; lv_style_copy(&modal_style, &lv_style_plain_color); modal_style.body.main_color = modal_style.body.grad_color = LV_COLOR_BLACK; @@ -102,7 +65,7 @@ void Modal::Show(const std::string& message) { /* Create the message box as a child of the modal background */ mbox = lv_mbox_create(obj, NULL); lv_mbox_add_btns(mbox, btns2); - lv_mbox_set_text(mbox, message.data()); + lv_mbox_set_text(mbox, msg); lv_obj_align(mbox, NULL, LV_ALIGN_CENTER, 0, 0); lv_obj_set_event_cb(mbox, Modal::mbox_event_cb); diff --git a/src/DisplayApp/Screens/Modal.h b/src/DisplayApp/Screens/Modal.h index b13b5c60..b5425906 100644 --- a/src/DisplayApp/Screens/Modal.h +++ b/src/DisplayApp/Screens/Modal.h @@ -22,8 +22,7 @@ namespace Pinetime { Modal(DisplayApp* app); ~Modal() override; - void Show(); - void Show(const std::string& message); + void Show(const char* msg); void Hide(); bool Refresh() override; @@ -39,7 +38,6 @@ namespace Pinetime { lv_obj_t *info; bool running = true; bool isVisible = false; - std::string message; }; } diff --git a/src/DisplayApp/Screens/Tile.cpp b/src/DisplayApp/Screens/Tile.cpp index 7eb1018c..6c225c9d 100644 --- a/src/DisplayApp/Screens/Tile.cpp +++ b/src/DisplayApp/Screens/Tile.cpp @@ -123,7 +123,9 @@ void Tile::OnObjectEvent(lv_obj_t *obj, lv_event_t event, uint32_t buttonId) { tile->StartClockApp(); break; case 3: - modal->Show(); + char versionStr[20]; + sprintf(versionStr, "VERSION: %d.%d.%d", Version::Major(), Version::Minor(), Version::Patch()); + modal->Show(versionStr); break; case 4: tile->StartSysInfoApp(); diff --git a/src/SystemTask/SystemTask.cpp b/src/SystemTask/SystemTask.cpp index 0080c6b6..e65abb61 100644 --- a/src/SystemTask/SystemTask.cpp +++ b/src/SystemTask/SystemTask.cpp @@ -5,17 +5,19 @@ #include #include #include +#include #include "SystemTask.h" #include "../main.h" using namespace Pinetime::System; -SystemTask::SystemTask(Pinetime::Drivers::SpiMaster &spi, Pinetime::Drivers::St7789 &lcd, - Pinetime::Drivers::Cst816S &touchPanel, Pinetime::Components::LittleVgl &lvgl, - Pinetime::Controllers::Battery &batteryController, Pinetime::Controllers::Ble &bleController, - Pinetime::Controllers::DateTime& dateTimeController) : +SystemTask::SystemTask(Drivers::SpiMaster &spi, Drivers::St7789 &lcd, Drivers::Cst816S &touchPanel, + Components::LittleVgl &lvgl, + Controllers::Battery &batteryController, Controllers::Ble &bleController, + Controllers::DateTime &dateTimeController, + Pinetime::Controllers::NotificationManager& notificationManager) : spi{spi}, lcd{lcd}, touchPanel{touchPanel}, lvgl{lvgl}, batteryController{batteryController}, bleController{bleController}, dateTimeController{dateTimeController}, - watchdog{}, watchdogView{watchdog}{ + watchdog{}, watchdogView{watchdog}, notificationManager{notificationManager} { systemTaksMsgQueue = xQueueCreate(10, 1); } @@ -44,7 +46,8 @@ void SystemTask::Work() { touchPanel.Init(); batteryController.Init(); - displayApp.reset(new Pinetime::Applications::DisplayApp(lcd, lvgl, touchPanel, batteryController, bleController, dateTimeController, watchdogView, *this)); + displayApp.reset(new Pinetime::Applications::DisplayApp(lcd, lvgl, touchPanel, batteryController, bleController, + dateTimeController, watchdogView, *this, notificationManager)); displayApp->Start(); batteryController.Update(); diff --git a/src/SystemTask/SystemTask.h b/src/SystemTask/SystemTask.h index b64fda65..a1ba277a 100644 --- a/src/SystemTask/SystemTask.h +++ b/src/SystemTask/SystemTask.h @@ -16,13 +16,11 @@ namespace Pinetime { enum class Messages {GoToSleep, GoToRunning, OnNewTime, OnNewNotification }; - SystemTask(Pinetime::Drivers::SpiMaster& spi, - Pinetime::Drivers::St7789& lcd, - Pinetime::Drivers::Cst816S& touchPanel, - Pinetime::Components::LittleVgl& lvgl, - Pinetime::Controllers::Battery& batteryController, - Pinetime::Controllers::Ble& bleController, - Pinetime::Controllers::DateTime& dateTimeController); + SystemTask(Drivers::SpiMaster &spi, Drivers::St7789 &lcd, Drivers::Cst816S &touchPanel, + Components::LittleVgl &lvgl, + Controllers::Battery &batteryController, Controllers::Ble &bleController, + Controllers::DateTime &dateTimeController, + Pinetime::Controllers::NotificationManager& manager); void Start(); @@ -45,6 +43,7 @@ namespace Pinetime { bool isSleeping = false; Pinetime::Drivers::Watchdog watchdog; Pinetime::Drivers::WatchdogView watchdogView; + Pinetime::Controllers::NotificationManager& notificationManager; static constexpr uint8_t pinSpiSck = 2; diff --git a/src/main.cpp b/src/main.cpp index a4a759d9..106d19eb 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #if NRF_LOG_ENABLED #include "Logging/NrfLogger.h" @@ -55,6 +56,8 @@ void ble_manager_set_ble_disconnection_callback(void (*disconnection)()); static constexpr uint8_t pinTouchIrq = 28; std::unique_ptr systemTask; +Pinetime::Controllers::NotificationManager notificationManager; + void nrfx_gpiote_evt_handler(nrfx_gpiote_pin_t pin, nrf_gpiote_polarity_t action) { if(pin == pinTouchIrq) { systemTask->OnTouchEvent(); @@ -86,7 +89,7 @@ void OnBleDisconnection() { } void OnNewNotification(const char* message, uint8_t size) { - bleController.PushNotification(message, size); + notificationManager.Push(Pinetime::Controllers::NotificationManager::Categories::SimpleAlert, message, size); systemTask->PushMessage(Pinetime::System::SystemTask::Messages::OnNewNotification); } @@ -128,7 +131,8 @@ int main(void) { debounceTimer = xTimerCreate ("debounceTimer", 200, pdFALSE, (void *) 0, DebounceTimerCallback); - systemTask.reset(new Pinetime::System::SystemTask(spi, lcd, touchPanel, lvgl, batteryController, bleController, dateTimeController)); + systemTask.reset(new Pinetime::System::SystemTask(spi, lcd, touchPanel, lvgl, batteryController, bleController, + dateTimeController, notificationManager)); systemTask->Start(); ble_manager_init();