Increase max size of notification message to 100 char.

Fix bug in message handling that would ignore the last character of the notification.
InfiniTimeOrg · Oct 21, 2020 · 440ae41 · 440ae41
1 parent ef5670c
commit 440ae41
Show file tree

Hide file tree

Showing 6 changed files with 30 additions and 41 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -1,5 +1,5 @@
 cmake_minimum_required(VERSION 3.10)
-project(pinetime VERSION 0.8.2 LANGUAGES C CXX ASM)
+project(pinetime VERSION 0.9.0 LANGUAGES C CXX ASM)
 
 set(NRF_TARGET "nrf52")
 

diff --git a/src/components/ble/AlertNotificationClient.cpp b/src/components/ble/AlertNotificationClient.cpp
@@ -105,25 +105,18 @@ int AlertNotificationClient::OnDescriptorDiscoveryEventCallback(uint16_t connect
 
 void AlertNotificationClient::OnNotification(ble_gap_event *event) {
   if(event->notify_rx.attr_handle == newAlertHandle) {
-    // TODO implement this with more memory safety (and constexpr)
-    static const size_t maxBufferSize{21};
-    static const size_t maxMessageSize{18};
-    size_t bufferSize = min(OS_MBUF_PKTLEN(event->notify_rx.om), maxBufferSize);
+    static constexpr size_t stringTerminatorSize{1}; // end of string '\0'
+    static constexpr size_t headerSize{3};
+    const auto maxMessageSize {NotificationManager::MaximumMessageSize()};
+    const auto maxBufferSize{maxMessageSize + headerSize};
 
-    uint8_t data[bufferSize];
-    os_mbuf_copydata(event->notify_rx.om, 0, bufferSize, data);
+    size_t bufferSize = min(OS_MBUF_PKTLEN(event->notify_rx.om) + stringTerminatorSize, maxBufferSize);
+    char *message = (char *)(&event->notify_rx.om[headerSize]);
+    auto messageSize = min(maxMessageSize, (bufferSize-headerSize));
 
-    char *s = (char *) &data[3];
-    auto messageSize = min(maxMessageSize, (bufferSize-3));
+    message[messageSize-1] = '\0';
 
-    for (uint i = 0; i < messageSize-1; i++) {
-      if (s[i] == 0x00) {
-        s[i] = 0x0A;
-      }
-    }
-    s[messageSize-1] = '\0';
-
-    notificationManager.Push(Pinetime::Controllers::NotificationManager::Categories::SimpleAlert, s, messageSize);
+    notificationManager.Push(Pinetime::Controllers::NotificationManager::Categories::SimpleAlert, message, messageSize);
     systemTask.PushMessage(Pinetime::System::SystemTask::Messages::OnNewNotification);
   }
 }

diff --git a/src/components/ble/AlertNotificationService.cpp b/src/components/ble/AlertNotificationService.cpp
@@ -38,7 +38,7 @@ AlertNotificationService::AlertNotificationService ( System::SystemTask& systemT
                   0
                 }
         },
-        serviceDefinition{
+    serviceDefinition{
                 {
                         /* Device Information Service */
                         .type = BLE_GATT_SVC_TYPE_PRIMARY,
@@ -48,33 +48,25 @@ AlertNotificationService::AlertNotificationService ( System::SystemTask& systemT
                 {
                         0
                 },
-        }, m_systemTask{systemTask}, m_notificationManager{notificationManager} {
+        }, systemTask{systemTask}, notificationManager{notificationManager} {
 }
 
 int AlertNotificationService::OnAlert(uint16_t conn_handle, uint16_t attr_handle,
                                                     struct ble_gatt_access_ctxt *ctxt) {
-
   if (ctxt->op == BLE_GATT_ACCESS_OP_WRITE_CHR) {
-    // TODO implement this with more memory safety (and constexpr)
-    static const size_t maxBufferSize{21};
-    static const size_t maxMessageSize{18};
-    size_t bufferSize = min(OS_MBUF_PKTLEN(ctxt->om), maxBufferSize);
-
-    uint8_t data[bufferSize];
-    os_mbuf_copydata(ctxt->om, 0, bufferSize, data);
+    static constexpr size_t stringTerminatorSize{1}; // end of string '\0'
+    static constexpr size_t headerSize{3};
+    const auto maxMessageSize {NotificationManager::MaximumMessageSize()};
+    const auto maxBufferSize{maxMessageSize + headerSize};
 
-    char *s = (char *) &data[3];
-    auto messageSize = min(maxMessageSize, (bufferSize-3));
+    size_t bufferSize = min(OS_MBUF_PKTLEN(ctxt->om) + stringTerminatorSize, maxBufferSize);
+    char *message = (char *)(&ctxt->om->om_data[headerSize]);
+    auto messageSize = min(maxMessageSize, (bufferSize-headerSize));
 
-    for (uint i = 0; i < messageSize-1; i++) {
-      if (s[i] == 0x00) {
-        s[i] = 0x0A;
-      }
-    }
-    s[messageSize-1] = '\0';
+    message[messageSize-1] = '\0';
 
-    m_notificationManager.Push(Pinetime::Controllers::NotificationManager::Categories::SimpleAlert, s, messageSize);
-    m_systemTask.PushMessage(Pinetime::System::SystemTask::Messages::OnNewNotification);
+    notificationManager.Push(Pinetime::Controllers::NotificationManager::Categories::SimpleAlert, message, messageSize);
+    systemTask.PushMessage(Pinetime::System::SystemTask::Messages::OnNewNotification);
   }
   return 0;
 }
diff --git a/src/components/ble/AlertNotificationService.h b/src/components/ble/AlertNotificationService.h
@@ -32,8 +32,8 @@ namespace Pinetime {
         struct ble_gatt_chr_def characteristicDefinition[2];
         struct ble_gatt_svc_def serviceDefinition[2];
 
-        Pinetime::System::SystemTask &m_systemTask;
-        NotificationManager &m_notificationManager;
+        Pinetime::System::SystemTask &systemTask;
+        NotificationManager &notificationManager;
     };
   }
 }
diff --git a/src/components/ble/NotificationManager.cpp b/src/components/ble/NotificationManager.cpp
@@ -4,10 +4,13 @@
 
 using namespace Pinetime::Controllers;
 
+constexpr uint8_t NotificationManager::MessageSize;
+
+
 void NotificationManager::Push(Pinetime::Controllers::NotificationManager::Categories category,
                                                       const char *message, uint8_t currentMessageSize) {
   // TODO handle edge cases on read/write index
-  auto checkedSize = std::min(currentMessageSize, uint8_t{18});
+  auto checkedSize = std::min(currentMessageSize, MessageSize);
   auto& notif = notifications[writeIndex];
   std::memcpy(notif.message.data(), message, checkedSize);
   notif.message[checkedSize] = '\0';

diff --git a/src/components/ble/NotificationManager.h b/src/components/ble/NotificationManager.h
@@ -8,7 +8,7 @@ namespace Pinetime {
     class NotificationManager {
       public:
         enum class Categories {Unknown, SimpleAlert, Email, News, IncomingCall, MissedCall, Sms, VoiceMail, Schedule, HighProriotyAlert, InstantMessage };
-        static constexpr uint8_t MessageSize{18};
+        static constexpr uint8_t MessageSize{100};
 
         struct Notification {
           using Id = uint8_t;
@@ -28,6 +28,7 @@ namespace Pinetime {
       bool ClearNewNotificationFlag();
       bool AreNewNotificationsAvailable();
 
+      static constexpr uint8_t MaximumMessageSize() { return MessageSize; };
 
       private:
         Notification::Id GetNextId();