From 07b6812f61cf5b7726fbf6015c2c60caa12c7f20 Mon Sep 17 00:00:00 2001 From: JF Date: Thu, 22 Oct 2020 10:43:42 +0200 Subject: [PATCH] Notifications : Fix copy when the messages is spread across multiple os_mbuf. --- src/components/ble/AlertNotificationClient.cpp | 15 +++++++++------ src/components/ble/AlertNotificationService.cpp | 15 +++++++++------ src/components/ble/ImmediateAlertService.cpp | 8 +++++++- src/components/ble/NotificationManager.cpp | 11 ++--------- src/components/ble/NotificationManager.h | 2 +- 5 files changed, 28 insertions(+), 23 deletions(-) diff --git a/src/components/ble/AlertNotificationClient.cpp b/src/components/ble/AlertNotificationClient.cpp index 8fbbafb6..40970e0b 100644 --- a/src/components/ble/AlertNotificationClient.cpp +++ b/src/components/ble/AlertNotificationClient.cpp @@ -105,18 +105,21 @@ int AlertNotificationClient::OnDescriptorDiscoveryEventCallback(uint16_t connect void AlertNotificationClient::OnNotification(ble_gap_event *event) { if(event->notify_rx.attr_handle == newAlertHandle) { - static constexpr size_t stringTerminatorSize{1}; // end of string '\0' - static constexpr size_t headerSize{3}; + constexpr size_t stringTerminatorSize = 1; // end of string '\0' + constexpr size_t headerSize = 3; const auto maxMessageSize {NotificationManager::MaximumMessageSize()}; const auto maxBufferSize{maxMessageSize + headerSize}; - size_t bufferSize = min(OS_MBUF_PKTLEN(event->notify_rx.om) + stringTerminatorSize, maxBufferSize); - char *message = (char *)(&event->notify_rx.om[headerSize]); + const auto dbgPacketLen = OS_MBUF_PKTLEN(event->notify_rx.om); + size_t bufferSize = min(dbgPacketLen + stringTerminatorSize, maxBufferSize); auto messageSize = min(maxMessageSize, (bufferSize-headerSize)); - message[messageSize-1] = '\0'; + NotificationManager::Notification notif; + os_mbuf_copydata(event->notify_rx.om, headerSize, messageSize-1, notif.message.data()); + notif.message[messageSize-1] = '\0'; + notif.category = Pinetime::Controllers::NotificationManager::Categories::SimpleAlert; + notificationManager.Push(std::move(notif)); - 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 index 7e432b82..32711b92 100644 --- a/src/components/ble/AlertNotificationService.cpp +++ b/src/components/ble/AlertNotificationService.cpp @@ -54,18 +54,21 @@ AlertNotificationService::AlertNotificationService ( System::SystemTask& systemT 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) { - static constexpr size_t stringTerminatorSize{1}; // end of string '\0' - static constexpr size_t headerSize{3}; + constexpr size_t stringTerminatorSize = 1; // end of string '\0' + constexpr size_t headerSize = 3; const auto maxMessageSize {NotificationManager::MaximumMessageSize()}; const auto maxBufferSize{maxMessageSize + headerSize}; - size_t bufferSize = min(OS_MBUF_PKTLEN(ctxt->om) + stringTerminatorSize, maxBufferSize); - char *message = (char *)(&ctxt->om->om_data[headerSize]); + const auto dbgPacketLen = OS_MBUF_PKTLEN(ctxt->om); + size_t bufferSize = min(dbgPacketLen + stringTerminatorSize, maxBufferSize); auto messageSize = min(maxMessageSize, (bufferSize-headerSize)); - message[messageSize-1] = '\0'; + NotificationManager::Notification notif; + os_mbuf_copydata(ctxt->om, headerSize, messageSize-1, notif.message.data()); + notif.message[messageSize-1] = '\0'; + notif.category = Pinetime::Controllers::NotificationManager::Categories::SimpleAlert; + notificationManager.Push(std::move(notif)); - notificationManager.Push(Pinetime::Controllers::NotificationManager::Categories::SimpleAlert, message, messageSize); systemTask.PushMessage(Pinetime::System::SystemTask::Messages::OnNewNotification); } return 0; diff --git a/src/components/ble/ImmediateAlertService.cpp b/src/components/ble/ImmediateAlertService.cpp index 3b7f47bf..e2cee308 100644 --- a/src/components/ble/ImmediateAlertService.cpp +++ b/src/components/ble/ImmediateAlertService.cpp @@ -1,4 +1,5 @@ #include +#include #include "ImmediateAlertService.h" #include "AlertNotificationService.h" @@ -67,7 +68,12 @@ int ImmediateAlertService::OnAlertLevelChanged(uint16_t connectionHandle, uint16 if(context->op == BLE_GATT_ACCESS_OP_WRITE_CHR) { auto alertLevel = static_cast(context->om->om_data[0]); auto* alertString = ToString(alertLevel); - notificationManager.Push(Pinetime::Controllers::NotificationManager::Categories::SimpleAlert, alertString, strlen(alertString)); + + NotificationManager::Notification notif; + std::memcpy(notif.message.data(), alertString, strlen(alertString)); + notif.category = Pinetime::Controllers::NotificationManager::Categories::SimpleAlert; + notificationManager.Push(std::move(notif)); + systemTask.PushMessage(Pinetime::System::SystemTask::Messages::OnNewNotification); } } diff --git a/src/components/ble/NotificationManager.cpp b/src/components/ble/NotificationManager.cpp index 11555bee..67711723 100644 --- a/src/components/ble/NotificationManager.cpp +++ b/src/components/ble/NotificationManager.cpp @@ -7,17 +7,10 @@ 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, MessageSize); - auto& notif = notifications[writeIndex]; - std::memcpy(notif.message.data(), message, checkedSize); - notif.message[checkedSize] = '\0'; - notif.category = category; +void NotificationManager::Push(NotificationManager::Notification &¬if) { notif.id = GetNextId(); notif.valid = true; - + notifications[writeIndex] = std::move(notif); writeIndex = (writeIndex + 1 < TotalNbNotifications) ? writeIndex + 1 : 0; if(!empty) readIndex = (readIndex + 1 < TotalNbNotifications) ? readIndex + 1 : 0; diff --git a/src/components/ble/NotificationManager.h b/src/components/ble/NotificationManager.h index 64c6df6d..49fe8306 100644 --- a/src/components/ble/NotificationManager.h +++ b/src/components/ble/NotificationManager.h @@ -20,7 +20,7 @@ namespace Pinetime { }; Notification::Id nextId {0}; - void Push(Categories category, const char* message, uint8_t messageSize); + void Push(Notification&& notif); Notification GetLastNotification(); Notification GetNext(Notification::Id id); Notification GetPrevious(Notification::Id id);