From f90f2254f55086589d1d378d35a54085e2620cb6 Mon Sep 17 00:00:00 2001 From: JF Date: Wed, 28 Oct 2020 18:23:09 +0100 Subject: [PATCH 1/2] Reset BLE services on disconnect, do not start advertising if a connection is already established. --- .../ble/AlertNotificationClient.cpp | 13 ++++++++ src/components/ble/AlertNotificationClient.h | 17 ++++++----- src/components/ble/CurrentTimeClient.cpp | 30 ++++++++++++------- src/components/ble/CurrentTimeClient.h | 7 ++++- src/components/ble/NimbleController.cpp | 10 ++++--- 5 files changed, 54 insertions(+), 23 deletions(-) diff --git a/src/components/ble/AlertNotificationClient.cpp b/src/components/ble/AlertNotificationClient.cpp index 40970e0b..29bc2f73 100644 --- a/src/components/ble/AlertNotificationClient.cpp +++ b/src/components/ble/AlertNotificationClient.cpp @@ -139,3 +139,16 @@ uint16_t AlertNotificationClient::EndHandle() const { uint16_t AlertNotificationClient::NewAlerthandle() const { return newAlertHandle; } + +void AlertNotificationClient::Reset() { + ansStartHandle = 0; + ansEndHandle = 0; + supportedNewAlertCategoryHandle = 0; + supportedUnreadAlertCategoryHandle = 0; + newAlertHandle = 0; + newAlertDescriptorHandle = 0; + newAlertDefHandle = 0; + unreadAlertStatusHandle = 0; + controlPointHandle = 0; + isDiscovered = false; +} diff --git a/src/components/ble/AlertNotificationClient.h b/src/components/ble/AlertNotificationClient.h index ca4f4e94..ebd5d56f 100644 --- a/src/components/ble/AlertNotificationClient.h +++ b/src/components/ble/AlertNotificationClient.h @@ -27,6 +27,7 @@ namespace Pinetime { bool IsDiscovered() const; uint16_t StartHandle() const; uint16_t EndHandle() const; + void Reset(); static constexpr const ble_uuid16_t &Uuid() { return ansServiceUuid; } @@ -64,15 +65,15 @@ namespace Pinetime { .value = controlPointId }; - uint16_t ansStartHandle; - uint16_t ansEndHandle; - uint16_t supportedNewAlertCategoryHandle; - uint16_t supportedUnreadAlertCategoryHandle; - uint16_t newAlertHandle; + uint16_t ansStartHandle = 0; + uint16_t ansEndHandle = 0; + uint16_t supportedNewAlertCategoryHandle = 0; + uint16_t supportedUnreadAlertCategoryHandle = 0; + uint16_t newAlertHandle = 0; uint16_t newAlertDescriptorHandle = 0; - uint16_t newAlertDefHandle; - uint16_t unreadAlertStatusHandle; - uint16_t controlPointHandle; + uint16_t newAlertDefHandle = 0; + uint16_t unreadAlertStatusHandle = 0; + uint16_t controlPointHandle = 0; bool isDiscovered = false; Pinetime::System::SystemTask &systemTask; Pinetime::Controllers::NotificationManager ¬ificationManager; diff --git a/src/components/ble/CurrentTimeClient.cpp b/src/components/ble/CurrentTimeClient.cpp index 7a225f4b..24027e5f 100644 --- a/src/components/ble/CurrentTimeClient.cpp +++ b/src/components/ble/CurrentTimeClient.cpp @@ -32,16 +32,17 @@ bool CurrentTimeClient::OnDiscoveryEvent(uint16_t connectionHandle, const ble_ga int CurrentTimeClient::OnCharacteristicDiscoveryEvent(uint16_t conn_handle, const ble_gatt_error *error, const ble_gatt_chr *characteristic) { - if(characteristic == nullptr && error->status == BLE_HS_EDONE) { - NRF_LOG_INFO("CTS Characteristic discovery complete"); - return 0; - } - - if(characteristic != nullptr && ble_uuid_cmp(((ble_uuid_t*)¤tTimeCharacteristicUuid), &characteristic->uuid.u) == 0) { - NRF_LOG_INFO("CTS Characteristic discovered : 0x%x", characteristic->val_handle); - currentTimeHandle = characteristic->val_handle; - } + if (characteristic == nullptr && error->status == BLE_HS_EDONE) { + NRF_LOG_INFO("CTS Characteristic discovery complete"); return 0; + } + + if (characteristic != nullptr && ble_uuid_cmp(((ble_uuid_t *) ¤tTimeCharacteristicUuid), &characteristic->uuid.u) == 0) { + NRF_LOG_INFO("CTS Characteristic discovered : 0x%x", characteristic->val_handle); + isCharacteristicDiscovered = true; + currentTimeHandle = characteristic->val_handle; + } + return 0; } int CurrentTimeClient::OnCurrentTimeReadResult(uint16_t conn_handle, const ble_gatt_error *error, const ble_gatt_attr *attribute) { @@ -74,4 +75,13 @@ uint16_t CurrentTimeClient::EndHandle() const { uint16_t CurrentTimeClient::CurrentTimeHandle() const { return currentTimeHandle; -} \ No newline at end of file +} + +void CurrentTimeClient::Reset() { + isDiscovered = false; + isCharacteristicDiscovered = false; +} + +bool CurrentTimeClient::IsCharacteristicDiscovered() const { + return isCharacteristicDiscovered; +} diff --git a/src/components/ble/CurrentTimeClient.h b/src/components/ble/CurrentTimeClient.h index 639ec831..fcc124c2 100644 --- a/src/components/ble/CurrentTimeClient.h +++ b/src/components/ble/CurrentTimeClient.h @@ -12,11 +12,13 @@ namespace Pinetime { public: explicit CurrentTimeClient(DateTime& dateTimeController); void Init(); + void Reset(); bool OnDiscoveryEvent(uint16_t connectionHandle, const ble_gatt_error *error, const ble_gatt_svc *service); int OnCharacteristicDiscoveryEvent(uint16_t conn_handle, const ble_gatt_error *error, const ble_gatt_chr *characteristic); int OnCurrentTimeReadResult(uint16_t conn_handle, const ble_gatt_error *error, const ble_gatt_attr *attribute); bool IsDiscovered() const; + bool IsCharacteristicDiscovered() const; uint16_t StartHandle() const; uint16_t EndHandle() const; uint16_t CurrentTimeHandle() const; @@ -46,11 +48,14 @@ namespace Pinetime { .value = currentTimeCharacteristicId }; - uint16_t currentTimeHandle; DateTime& dateTimeController; bool isDiscovered = false; uint16_t ctsStartHandle; uint16_t ctsEndHandle; + + bool isCharacteristicDiscovered = false; + uint16_t currentTimeHandle; + }; } } \ No newline at end of file diff --git a/src/components/ble/NimbleController.cpp b/src/components/ble/NimbleController.cpp index 022cc510..577c897a 100644 --- a/src/components/ble/NimbleController.cpp +++ b/src/components/ble/NimbleController.cpp @@ -108,7 +108,7 @@ void NimbleController::Init() { } void NimbleController::StartAdvertising() { - if(ble_gap_adv_active()) return; + if(bleController.IsConnected() || ble_gap_conn_active() || ble_gap_adv_active()) return; ble_svc_gap_device_name_set(deviceName); @@ -197,6 +197,8 @@ int NimbleController::OnGAPEvent(ble_gap_event *event) { NRF_LOG_INFO("disconnect; reason=%d", event->disconnect.reason); /* Connection terminated; resume advertising. */ + currentTimeClient.Reset(); + alertNotificationClient.Reset(); connectionHandle = BLE_HS_CONN_HANDLE_NONE; bleController.Disconnect(); StartAdvertising(); @@ -289,10 +291,10 @@ int NimbleController::OnDiscoveryEvent(uint16_t i, const ble_gatt_error *error, int NimbleController::OnCTSCharacteristicDiscoveryEvent(uint16_t connectionHandle, const ble_gatt_error *error, const ble_gatt_chr *characteristic) { - if(characteristic == nullptr && error->status == BLE_HS_EDONE) { + if(characteristic == nullptr && error->status == BLE_HS_EDONE && currentTimeClient.IsCharacteristicDiscovered()) { NRF_LOG_INFO("CTS characteristic Discovery complete"); - ble_gattc_read(connectionHandle, currentTimeClient.CurrentTimeHandle(), CurrentTimeReadCallback, this); - return 0; + auto res = ble_gattc_read(connectionHandle, currentTimeClient.CurrentTimeHandle(), CurrentTimeReadCallback, this); + return res; } return currentTimeClient.OnCharacteristicDiscoveryEvent(connectionHandle, error, characteristic); } From 29f8074fcb844cf9668a5bf071e9cffa47299c99 Mon Sep 17 00:00:00 2001 From: JF Date: Thu, 29 Oct 2020 16:06:01 +0100 Subject: [PATCH 2/2] Refactoring of BLE service discovery : it is now implemented into the classes of the services. --- src/CMakeLists.txt | 3 + .../ble/AlertNotificationClient.cpp | 154 +++++++++++------- src/components/ble/AlertNotificationClient.h | 18 +- src/components/ble/BleClient.h | 12 ++ src/components/ble/CurrentTimeClient.cpp | 114 ++++++++----- src/components/ble/CurrentTimeClient.h | 14 +- src/components/ble/NimbleController.cpp | 108 +----------- src/components/ble/NimbleController.h | 3 + src/components/ble/ServiceDiscovery.cpp | 31 ++++ src/components/ble/ServiceDiscovery.h | 24 +++ 10 files changed, 254 insertions(+), 227 deletions(-) create mode 100644 src/components/ble/BleClient.h create mode 100644 src/components/ble/ServiceDiscovery.cpp create mode 100644 src/components/ble/ServiceDiscovery.h diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 4d691ede..6c9ce24b 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -369,6 +369,7 @@ list(APPEND SOURCE_FILES components/ble/MusicService.cpp components/ble/BatteryInformationService.cpp components/ble/ImmediateAlertService.cpp + components/ble/ServiceDiscovery.cpp components/firmwarevalidator/FirmwareValidator.cpp drivers/Cst816s.cpp FreeRTOS/port.c @@ -447,6 +448,8 @@ set(INCLUDE_FILES components/firmwarevalidator/FirmwareValidator.h components/ble/BatteryInformationService.h components/ble/ImmediateAlertService.h + components/ble/ServiceDiscovery.h + components/ble/BleClient.h drivers/Cst816s.h FreeRTOS/portmacro.h FreeRTOS/portmacro_cmsis.h diff --git a/src/components/ble/AlertNotificationClient.cpp b/src/components/ble/AlertNotificationClient.cpp index 29bc2f73..abe41099 100644 --- a/src/components/ble/AlertNotificationClient.cpp +++ b/src/components/ble/AlertNotificationClient.cpp @@ -3,84 +3,127 @@ #include "AlertNotificationClient.h" - using namespace Pinetime::Controllers; constexpr ble_uuid16_t AlertNotificationClient::ansServiceUuid; - constexpr ble_uuid16_t AlertNotificationClient::supportedNewAlertCategoryUuid; -constexpr ble_uuid16_t AlertNotificationClient::supportedUnreadAlertCategoryUuid ; +constexpr ble_uuid16_t AlertNotificationClient::supportedUnreadAlertCategoryUuid; constexpr ble_uuid16_t AlertNotificationClient::newAlertUuid; constexpr ble_uuid16_t AlertNotificationClient::unreadAlertStatusUuid; constexpr ble_uuid16_t AlertNotificationClient::controlPointUuid; -int Pinetime::Controllers::NewAlertSubcribeCallback(uint16_t conn_handle, - const struct ble_gatt_error *error, - struct ble_gatt_attr *attr, - void *arg) { - auto client = static_cast(arg); - return client->OnNewAlertSubcribe(conn_handle, error, attr); +namespace { + int + OnDiscoveryEventCallback(uint16_t conn_handle, const struct ble_gatt_error *error, const struct ble_gatt_svc *service, + void *arg) { + auto client = static_cast(arg); + return client->OnDiscoveryEvent(conn_handle, error, service); + } + + int OnAlertNotificationCharacteristicDiscoveredCallback(uint16_t conn_handle, const struct ble_gatt_error *error, + const struct ble_gatt_chr *chr, void *arg) { + auto client = static_cast(arg); + return client->OnCharacteristicsDiscoveryEvent(conn_handle, error, chr); + } + + int OnAlertNotificationDescriptorDiscoveryEventCallback(uint16_t conn_handle, + const struct ble_gatt_error *error, + uint16_t chr_val_handle, + const struct ble_gatt_dsc *dsc, + void *arg) { + auto client = static_cast(arg); + return client->OnDescriptorDiscoveryEventCallback(conn_handle, error, chr_val_handle, dsc); + } + + int NewAlertSubcribeCallback(uint16_t conn_handle, + const struct ble_gatt_error *error, + struct ble_gatt_attr *attr, + void *arg) { + auto client = static_cast(arg); + return client->OnNewAlertSubcribe(conn_handle, error, attr); + } } -AlertNotificationClient::AlertNotificationClient(Pinetime::System::SystemTask& systemTask, - Pinetime::Controllers::NotificationManager& notificationManager) : - systemTask{systemTask}, notificationManager{notificationManager}{ - +AlertNotificationClient::AlertNotificationClient(Pinetime::System::SystemTask &systemTask, + Pinetime::Controllers::NotificationManager ¬ificationManager) : + systemTask{systemTask}, notificationManager{notificationManager} { } -bool AlertNotificationClient::OnDiscoveryEvent(uint16_t connectionHandle, const ble_gatt_error *error, const ble_gatt_svc *service) { - if(service == nullptr && error->status == BLE_HS_EDONE) { - NRF_LOG_INFO("ANS Discovery complete"); +bool AlertNotificationClient::OnDiscoveryEvent(uint16_t connectionHandle, const ble_gatt_error *error, + const ble_gatt_svc *service) { + if (service == nullptr && error->status == BLE_HS_EDONE) { + if (isDiscovered) { + NRF_LOG_INFO("ANS Discovery found, starting characteristics discovery"); + + ble_gattc_disc_all_chrs(connectionHandle, ansStartHandle, ansEndHandle, + OnAlertNotificationCharacteristicDiscoveredCallback, this); + } else { + NRF_LOG_INFO("ANS not found"); + onServiceDiscovered(connectionHandle); + } return true; } - if(service != nullptr && ble_uuid_cmp(((ble_uuid_t*)&ansServiceUuid), &service->uuid.u) == 0) { - NRF_LOG_INFO("ANS discovered : 0x%x", service->start_handle); - ansStartHandle = service->start_handle; - ansEndHandle = service->end_handle; - isDiscovered = true; + if (service != nullptr && ble_uuid_cmp(((ble_uuid_t *) &ansServiceUuid), &service->uuid.u) == 0) { + NRF_LOG_INFO("ANS discovered : 0x%x - 0x%x", service->start_handle, service->end_handle); + ansStartHandle = service->start_handle; + ansEndHandle = service->end_handle; + isDiscovered = true; } return false; } int AlertNotificationClient::OnCharacteristicsDiscoveryEvent(uint16_t connectionHandle, const ble_gatt_error *error, - const ble_gatt_chr *characteristic) { - if(error->status != 0 && error->status != BLE_HS_EDONE) { + const ble_gatt_chr *characteristic) { + if (error->status != 0 && error->status != BLE_HS_EDONE) { NRF_LOG_INFO("ANS Characteristic discovery ERROR"); + onServiceDiscovered(connectionHandle); return 0; } - if(characteristic == nullptr && error->status == BLE_HS_EDONE) { + if (characteristic == nullptr && error->status == BLE_HS_EDONE) { NRF_LOG_INFO("ANS Characteristic discovery complete"); + if (isCharacteristicDiscovered) { + ble_gattc_disc_all_dscs(connectionHandle, + newAlertHandle, ansEndHandle, + OnAlertNotificationDescriptorDiscoveryEventCallback, this); + } else + onServiceDiscovered(connectionHandle); } else { - if(characteristic != nullptr && ble_uuid_cmp(((ble_uuid_t*)&supportedNewAlertCategoryUuid), &characteristic->uuid.u) == 0) { + if (characteristic != nullptr && + ble_uuid_cmp(((ble_uuid_t *) &supportedNewAlertCategoryUuid), &characteristic->uuid.u) == 0) { NRF_LOG_INFO("ANS Characteristic discovered : supportedNewAlertCategoryUuid"); supportedNewAlertCategoryHandle = characteristic->val_handle; - } else if(characteristic != nullptr && ble_uuid_cmp(((ble_uuid_t*)&supportedUnreadAlertCategoryUuid), &characteristic->uuid.u) == 0) { + } else if (characteristic != nullptr && + ble_uuid_cmp(((ble_uuid_t *) &supportedUnreadAlertCategoryUuid), &characteristic->uuid.u) == 0) { NRF_LOG_INFO("ANS Characteristic discovered : supportedUnreadAlertCategoryUuid"); supportedUnreadAlertCategoryHandle = characteristic->val_handle; - } else if(characteristic != nullptr && ble_uuid_cmp(((ble_uuid_t*)&newAlertUuid), &characteristic->uuid.u) == 0) { + } else if (characteristic != nullptr && + ble_uuid_cmp(((ble_uuid_t *) &newAlertUuid), &characteristic->uuid.u) == 0) { NRF_LOG_INFO("ANS Characteristic discovered : newAlertUuid"); newAlertHandle = characteristic->val_handle; newAlertDefHandle = characteristic->def_handle; - } else if(characteristic != nullptr && ble_uuid_cmp(((ble_uuid_t*)&unreadAlertStatusUuid), &characteristic->uuid.u) == 0) { + isCharacteristicDiscovered = true; + } else if (characteristic != nullptr && + ble_uuid_cmp(((ble_uuid_t *) &unreadAlertStatusUuid), &characteristic->uuid.u) == 0) { NRF_LOG_INFO("ANS Characteristic discovered : unreadAlertStatusUuid"); unreadAlertStatusHandle = characteristic->val_handle; - } else if(characteristic != nullptr && ble_uuid_cmp(((ble_uuid_t*)&controlPointUuid), &characteristic->uuid.u) == 0) { + } else if (characteristic != nullptr && + ble_uuid_cmp(((ble_uuid_t *) &controlPointUuid), &characteristic->uuid.u) == 0) { NRF_LOG_INFO("ANS Characteristic discovered : controlPointUuid"); controlPointHandle = characteristic->val_handle; - }else - NRF_LOG_INFO("ANS Characteristic discovered : 0x%x", characteristic->val_handle); - } + } else NRF_LOG_INFO("ANS Characteristic discovered : 0x%x", characteristic->val_handle); + } return 0; } int AlertNotificationClient::OnNewAlertSubcribe(uint16_t connectionHandle, const ble_gatt_error *error, ble_gatt_attr *attribute) { - if(error->status == 0) { + if (error->status == 0) { NRF_LOG_INFO("ANS New alert subscribe OK"); } else { NRF_LOG_INFO("ANS New alert subscribe ERROR"); } + onServiceDiscovered(connectionHandle); return 0; } @@ -88,35 +131,40 @@ int AlertNotificationClient::OnNewAlertSubcribe(uint16_t connectionHandle, const int AlertNotificationClient::OnDescriptorDiscoveryEventCallback(uint16_t connectionHandle, const ble_gatt_error *error, uint16_t characteristicValueHandle, const ble_gatt_dsc *descriptor) { - if(error->status == 0) { - if(characteristicValueHandle == newAlertHandle && ble_uuid_cmp(((ble_uuid_t*)&newAlertUuid), &descriptor->uuid.u)) { - if(newAlertDescriptorHandle == 0) { + if (error->status == 0) { + if (characteristicValueHandle == newAlertHandle && + ble_uuid_cmp(((ble_uuid_t *) &newAlertUuid), &descriptor->uuid.u)) { + if (newAlertDescriptorHandle == 0) { NRF_LOG_INFO("ANS Descriptor discovered : %d", descriptor->handle); newAlertDescriptorHandle = descriptor->handle; + isDescriptorFound = true; uint8_t value[2]; value[0] = 1; value[1] = 0; ble_gattc_write_flat(connectionHandle, newAlertDescriptorHandle, value, sizeof(value), NewAlertSubcribeCallback, this); } } + } else { + if (!isDescriptorFound) + onServiceDiscovered(connectionHandle); } return 0; } void AlertNotificationClient::OnNotification(ble_gap_event *event) { - if(event->notify_rx.attr_handle == newAlertHandle) { + if (event->notify_rx.attr_handle == newAlertHandle) { constexpr size_t stringTerminatorSize = 1; // end of string '\0' constexpr size_t headerSize = 3; - const auto maxMessageSize {NotificationManager::MaximumMessageSize()}; + const auto maxMessageSize{NotificationManager::MaximumMessageSize()}; const auto maxBufferSize{maxMessageSize + headerSize}; const auto dbgPacketLen = OS_MBUF_PKTLEN(event->notify_rx.om); size_t bufferSize = min(dbgPacketLen + stringTerminatorSize, maxBufferSize); - auto messageSize = min(maxMessageSize, (bufferSize-headerSize)); + auto messageSize = min(maxMessageSize, (bufferSize - headerSize)); NotificationManager::Notification notif; - os_mbuf_copydata(event->notify_rx.om, headerSize, messageSize-1, notif.message.data()); - notif.message[messageSize-1] = '\0'; + 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)); @@ -124,22 +172,6 @@ void AlertNotificationClient::OnNotification(ble_gap_event *event) { } } -bool AlertNotificationClient::IsDiscovered() const { - return isDiscovered; -} - -uint16_t AlertNotificationClient::StartHandle() const { - return ansStartHandle; -} - -uint16_t AlertNotificationClient::EndHandle() const { - return ansEndHandle; -} - -uint16_t AlertNotificationClient::NewAlerthandle() const { - return newAlertHandle; -} - void AlertNotificationClient::Reset() { ansStartHandle = 0; ansEndHandle = 0; @@ -151,4 +183,12 @@ void AlertNotificationClient::Reset() { unreadAlertStatusHandle = 0; controlPointHandle = 0; isDiscovered = false; + isCharacteristicDiscovered = false; + isDescriptorFound = false; +} + +void AlertNotificationClient::Discover(uint16_t connectionHandle, std::function onServiceDiscovered) { + NRF_LOG_INFO("[ANS] Starting discovery"); + this->onServiceDiscovered = onServiceDiscovered; + ble_gattc_disc_svc_by_uuid(connectionHandle, &ansServiceUuid.u, OnDiscoveryEventCallback, this); } diff --git a/src/components/ble/AlertNotificationClient.h b/src/components/ble/AlertNotificationClient.h index ebd5d56f..bc0df51e 100644 --- a/src/components/ble/AlertNotificationClient.h +++ b/src/components/ble/AlertNotificationClient.h @@ -3,16 +3,12 @@ #include #include #include +#include "BleClient.h" namespace Pinetime { namespace Controllers { - int NewAlertSubcribeCallback(uint16_t conn_handle, - const struct ble_gatt_error *error, - struct ble_gatt_attr *attr, - void *arg); - - class AlertNotificationClient { + class AlertNotificationClient : public BleClient { public: explicit AlertNotificationClient(Pinetime::System::SystemTask &systemTask, Pinetime::Controllers::NotificationManager ¬ificationManager); @@ -24,14 +20,9 @@ namespace Pinetime { int OnDescriptorDiscoveryEventCallback(uint16_t connectionHandle, const ble_gatt_error *error, uint16_t characteristicValueHandle, const ble_gatt_dsc *descriptor); void OnNotification(ble_gap_event *event); - bool IsDiscovered() const; - uint16_t StartHandle() const; - uint16_t EndHandle() const; void Reset(); + void Discover(uint16_t connectionHandle, std::function lambda) override; - static constexpr const ble_uuid16_t &Uuid() { return ansServiceUuid; } - - uint16_t NewAlerthandle() const; private: static constexpr uint16_t ansServiceId{0x1811}; static constexpr uint16_t supportedNewAlertCategoryId = 0x2a47; @@ -77,6 +68,9 @@ namespace Pinetime { bool isDiscovered = false; Pinetime::System::SystemTask &systemTask; Pinetime::Controllers::NotificationManager ¬ificationManager; + std::function onServiceDiscovered; + bool isCharacteristicDiscovered = false; + bool isDescriptorFound = false; }; } } \ No newline at end of file diff --git a/src/components/ble/BleClient.h b/src/components/ble/BleClient.h new file mode 100644 index 00000000..559f6c8d --- /dev/null +++ b/src/components/ble/BleClient.h @@ -0,0 +1,12 @@ +#pragma once + +#include + +namespace Pinetime { + namespace Controllers{ + class BleClient { + public: + virtual void Discover(uint16_t connectionHandle, std::function lambda) = 0; + }; + } +} \ No newline at end of file diff --git a/src/components/ble/CurrentTimeClient.cpp b/src/components/ble/CurrentTimeClient.cpp index 24027e5f..92f9374b 100644 --- a/src/components/ble/CurrentTimeClient.cpp +++ b/src/components/ble/CurrentTimeClient.cpp @@ -6,7 +6,25 @@ using namespace Pinetime::Controllers; constexpr ble_uuid16_t CurrentTimeClient::ctsServiceUuid; constexpr ble_uuid16_t CurrentTimeClient::currentTimeCharacteristicUuid; -CurrentTimeClient::CurrentTimeClient(DateTime& dateTimeController) : dateTimeController{dateTimeController} { +namespace { + int OnDiscoveryEventCallback(uint16_t conn_handle, const struct ble_gatt_error *error, const struct ble_gatt_svc *service, void *arg) { + auto client = static_cast(arg); + return client->OnDiscoveryEvent(conn_handle, error, service); + } + + int OnCurrentTimeCharacteristicDiscoveredCallback(uint16_t conn_handle, const struct ble_gatt_error *error, + const struct ble_gatt_chr *chr, void *arg) { + auto client = static_cast(arg); + return client->OnCharacteristicDiscoveryEvent(conn_handle, error, chr); + } + + int CurrentTimeReadCallback(uint16_t conn_handle, const struct ble_gatt_error *error, struct ble_gatt_attr *attr, void *arg) { + auto client = static_cast(arg); + return client->OnCurrentTimeReadResult(conn_handle, error, attr); + } +} + +CurrentTimeClient::CurrentTimeClient(DateTime &dateTimeController) : dateTimeController{dateTimeController} { } @@ -14,30 +32,47 @@ void CurrentTimeClient::Init() { } -bool CurrentTimeClient::OnDiscoveryEvent(uint16_t connectionHandle, const ble_gatt_error *error, const ble_gatt_svc *service) { - if(service == nullptr && error->status == BLE_HS_EDONE) { - NRF_LOG_INFO("CTS Discovery complete"); - return true; - } +bool CurrentTimeClient::OnDiscoveryEvent(uint16_t connectionHandle, const ble_gatt_error *error, + const ble_gatt_svc *service) { + if (service == nullptr && error->status == BLE_HS_EDONE) { + if (isDiscovered) { + NRF_LOG_INFO("CTS found, starting characteristics discovery"); - if(service != nullptr && ble_uuid_cmp(((ble_uuid_t*)&ctsServiceUuid), &service->uuid.u) == 0) { - NRF_LOG_INFO("CTS discovered : 0x%x", service->start_handle); - isDiscovered = true; - ctsStartHandle = service->start_handle; - ctsEndHandle = service->end_handle; - return false; + ble_gattc_disc_all_chrs(connectionHandle, ctsStartHandle, ctsEndHandle, + OnCurrentTimeCharacteristicDiscoveredCallback, this); + } else { + NRF_LOG_INFO("CTS not found"); + onServiceDiscovered(connectionHandle); } + return true; + } + + if (service != nullptr && ble_uuid_cmp(((ble_uuid_t *) &ctsServiceUuid), &service->uuid.u) == 0) { + NRF_LOG_INFO("CTS discovered : 0x%x - 0x%x", service->start_handle, service->end_handle); + isDiscovered = true; + ctsStartHandle = service->start_handle; + ctsEndHandle = service->end_handle; return false; + } + return false; } int CurrentTimeClient::OnCharacteristicDiscoveryEvent(uint16_t conn_handle, const ble_gatt_error *error, const ble_gatt_chr *characteristic) { if (characteristic == nullptr && error->status == BLE_HS_EDONE) { - NRF_LOG_INFO("CTS Characteristic discovery complete"); + if (isCharacteristicDiscovered) { + NRF_LOG_INFO("CTS Characteristic discovery complete, fetching time"); + ble_gattc_read(conn_handle, currentTimeHandle, CurrentTimeReadCallback, this); + } else { + NRF_LOG_INFO("CTS Characteristic discovery unsuccessful"); + onServiceDiscovered(conn_handle); + } + return 0; } - if (characteristic != nullptr && ble_uuid_cmp(((ble_uuid_t *) ¤tTimeCharacteristicUuid), &characteristic->uuid.u) == 0) { + if (characteristic != nullptr && + ble_uuid_cmp(((ble_uuid_t *) ¤tTimeCharacteristicUuid), &characteristic->uuid.u) == 0) { NRF_LOG_INFO("CTS Characteristic discovered : 0x%x", characteristic->val_handle); isCharacteristicDiscovered = true; currentTimeHandle = characteristic->val_handle; @@ -45,36 +80,23 @@ int CurrentTimeClient::OnCharacteristicDiscoveryEvent(uint16_t conn_handle, cons return 0; } -int CurrentTimeClient::OnCurrentTimeReadResult(uint16_t conn_handle, const ble_gatt_error *error, const ble_gatt_attr *attribute) { - if(error->status == 0) { - // TODO check that attribute->handle equals the handle discovered in OnCharacteristicDiscoveryEvent - CtsData result; - os_mbuf_copydata(attribute->om, 0, sizeof(CtsData), &result); - NRF_LOG_INFO("Received data: %d-%d-%d %d:%d:%d", result.year, - result.month, result.dayofmonth, - result.hour, result.minute, result.second); - dateTimeController.SetTime(result.year, result.month, result.dayofmonth, - 0, result.hour, result.minute, result.second, nrf_rtc_counter_get(portNRF_RTC_REG)); - } else { - NRF_LOG_INFO("Error retrieving current time: %d", error->status); - } - return 0; -} +int CurrentTimeClient::OnCurrentTimeReadResult(uint16_t conn_handle, const ble_gatt_error *error, + const ble_gatt_attr *attribute) { + if (error->status == 0) { + // TODO check that attribute->handle equals the handle discovered in OnCharacteristicDiscoveryEvent + CtsData result; + os_mbuf_copydata(attribute->om, 0, sizeof(CtsData), &result); + NRF_LOG_INFO("Received data: %d-%d-%d %d:%d:%d", result.year, + result.month, result.dayofmonth, + result.hour, result.minute, result.second); + dateTimeController.SetTime(result.year, result.month, result.dayofmonth, + 0, result.hour, result.minute, result.second, nrf_rtc_counter_get(portNRF_RTC_REG)); + } else { + NRF_LOG_INFO("Error retrieving current time: %d", error->status); + } -bool CurrentTimeClient::IsDiscovered() const { - return isDiscovered; -} - -uint16_t CurrentTimeClient::StartHandle() const { - return ctsStartHandle; -} - -uint16_t CurrentTimeClient::EndHandle() const { - return ctsEndHandle; -} - -uint16_t CurrentTimeClient::CurrentTimeHandle() const { - return currentTimeHandle; + onServiceDiscovered(conn_handle); + return 0; } void CurrentTimeClient::Reset() { @@ -82,6 +104,8 @@ void CurrentTimeClient::Reset() { isCharacteristicDiscovered = false; } -bool CurrentTimeClient::IsCharacteristicDiscovered() const { - return isCharacteristicDiscovered; +void CurrentTimeClient::Discover(uint16_t connectionHandle, std::function onServiceDiscovered) { + NRF_LOG_INFO("[CTS] Starting discovery"); + this->onServiceDiscovered = onServiceDiscovered; + ble_gattc_disc_svc_by_uuid(connectionHandle, &ctsServiceUuid.u, OnDiscoveryEventCallback, this); } diff --git a/src/components/ble/CurrentTimeClient.h b/src/components/ble/CurrentTimeClient.h index fcc124c2..93139399 100644 --- a/src/components/ble/CurrentTimeClient.h +++ b/src/components/ble/CurrentTimeClient.h @@ -3,12 +3,13 @@ #include #include "components/datetime/DateTimeController.h" +#include "BleClient.h" #include namespace Pinetime { namespace Controllers { - class CurrentTimeClient { + class CurrentTimeClient : public BleClient { public: explicit CurrentTimeClient(DateTime& dateTimeController); void Init(); @@ -17,14 +18,11 @@ namespace Pinetime { int OnCharacteristicDiscoveryEvent(uint16_t conn_handle, const ble_gatt_error *error, const ble_gatt_chr *characteristic); int OnCurrentTimeReadResult(uint16_t conn_handle, const ble_gatt_error *error, const ble_gatt_attr *attribute); - bool IsDiscovered() const; - bool IsCharacteristicDiscovered() const; - uint16_t StartHandle() const; - uint16_t EndHandle() const; - uint16_t CurrentTimeHandle() const; static constexpr const ble_uuid16_t* Uuid() { return &CurrentTimeClient::ctsServiceUuid; } static constexpr const ble_uuid16_t* CurrentTimeCharacteristicUuid() { return &CurrentTimeClient::currentTimeCharacteristicUuid; } - private: + void Discover(uint16_t connectionHandle, std::function lambda) override; + + private: typedef struct __attribute__((packed)) { uint16_t year; uint8_t month; @@ -55,7 +53,7 @@ namespace Pinetime { bool isCharacteristicDiscovered = false; uint16_t currentTimeHandle; - + std::function onServiceDiscovered; }; } } \ No newline at end of file diff --git a/src/components/ble/NimbleController.cpp b/src/components/ble/NimbleController.cpp index 577c897a..af7f4029 100644 --- a/src/components/ble/NimbleController.cpp +++ b/src/components/ble/NimbleController.cpp @@ -1,10 +1,7 @@ - #include "components/datetime/DateTimeController.h" - #include #include "components/ble/NotificationManager.h" #include - #include "NimbleController.h" #include "MusicService.h" #include @@ -14,14 +11,8 @@ #include #include - - using namespace Pinetime::Controllers; -// TODO I'm not satisfied by how this code looks like (AlertNotificationClient and CurrentTimeClient must -// expose too much data, too many callbacks -> NimbleController -> CTS/ANS client. -// Let's try to improve this code (and keep it working!) - NimbleController::NimbleController(Pinetime::System::SystemTask& systemTask, Pinetime::Controllers::Ble& bleController, DateTime& dateTimeController, @@ -40,8 +31,8 @@ NimbleController::NimbleController(Pinetime::System::SystemTask& systemTask, currentTimeService{dateTimeController}, musicService{systemTask}, batteryInformationService{batteryController}, - immediateAlertService{systemTask, notificationManager} { - + immediateAlertService{systemTask, notificationManager}, + serviceDiscovery({¤tTimeClient, &alertNotificationClient}) { } int GAPEventCallback(struct ble_gap_event *event, void *arg) { @@ -49,33 +40,6 @@ int GAPEventCallback(struct ble_gap_event *event, void *arg) { return nimbleController->OnGAPEvent(event); } -int CurrentTimeCharacteristicDiscoveredCallback(uint16_t conn_handle, const struct ble_gatt_error *error, - const struct ble_gatt_chr *chr, void *arg) { - auto client = static_cast(arg); - return client->OnCTSCharacteristicDiscoveryEvent(conn_handle, error, chr); -} - -int AlertNotificationCharacteristicDiscoveredCallback(uint16_t conn_handle, const struct ble_gatt_error *error, - const struct ble_gatt_chr *chr, void *arg) { - auto client = static_cast(arg); - return client->OnANSCharacteristicDiscoveryEvent(conn_handle, error, chr); -} - -int CurrentTimeReadCallback(uint16_t conn_handle, const struct ble_gatt_error *error, - struct ble_gatt_attr *attr, void *arg) { - auto client = static_cast(arg); - return client->OnCurrentTimeReadResult(conn_handle, error, attr); -} - -int AlertNotificationDescriptorDiscoveryEventCallback(uint16_t conn_handle, - const struct ble_gatt_error *error, - uint16_t chr_val_handle, - const struct ble_gatt_dsc *dsc, - void *arg) { - auto client = static_cast(arg); - return client->OnANSDescriptorDiscoveryEventCallback(conn_handle, error, chr_val_handle, dsc); -} - void NimbleController::Init() { while (!ble_hs_synced()) {} @@ -158,15 +122,6 @@ void NimbleController::StartAdvertising() { // the application has been woken up, for example. } -int OnAllSvrDisco(uint16_t conn_handle, - const struct ble_gatt_error *error, - const struct ble_gatt_svc *service, - void *arg) { - auto nimbleController = static_cast(arg); - return nimbleController->OnDiscoveryEvent(conn_handle, error, service); - return 0; -} - int NimbleController::OnGAPEvent(ble_gap_event *event) { switch (event->type) { case BLE_GAP_EVENT_ADV_COMPLETE: @@ -271,65 +226,8 @@ int NimbleController::OnGAPEvent(ble_gap_event *event) { return 0; } -int NimbleController::OnDiscoveryEvent(uint16_t i, const ble_gatt_error *error, const ble_gatt_svc *service) { - if(service == nullptr && error->status == BLE_HS_EDONE) { - NRF_LOG_INFO("Service Discovery complete"); - if(currentTimeClient.IsDiscovered()) { - ble_gattc_disc_all_chrs(connectionHandle, currentTimeClient.StartHandle(), currentTimeClient.EndHandle(), - CurrentTimeCharacteristicDiscoveredCallback, this); - - } else if(alertNotificationClient.IsDiscovered()) { - ble_gattc_disc_all_chrs(connectionHandle, alertNotificationClient.StartHandle(), alertNotificationClient.EndHandle(), - AlertNotificationCharacteristicDiscoveredCallback, this); - } - } - - alertNotificationClient.OnDiscoveryEvent(i, error, service); - currentTimeClient.OnDiscoveryEvent(i, error, service); - return 0; -} - -int NimbleController::OnCTSCharacteristicDiscoveryEvent(uint16_t connectionHandle, const ble_gatt_error *error, - const ble_gatt_chr *characteristic) { - if(characteristic == nullptr && error->status == BLE_HS_EDONE && currentTimeClient.IsCharacteristicDiscovered()) { - NRF_LOG_INFO("CTS characteristic Discovery complete"); - auto res = ble_gattc_read(connectionHandle, currentTimeClient.CurrentTimeHandle(), CurrentTimeReadCallback, this); - return res; - } - return currentTimeClient.OnCharacteristicDiscoveryEvent(connectionHandle, error, characteristic); -} - -int NimbleController::OnANSCharacteristicDiscoveryEvent(uint16_t connectionHandle, const ble_gatt_error *error, - const ble_gatt_chr *characteristic) { - if(characteristic == nullptr && error->status == BLE_HS_EDONE) { - NRF_LOG_INFO("ANS characteristic Discovery complete"); - ble_gattc_disc_all_dscs(connectionHandle, - alertNotificationClient.NewAlerthandle(), alertNotificationClient.EndHandle(), - AlertNotificationDescriptorDiscoveryEventCallback, this); - return 0; - } - return alertNotificationClient.OnCharacteristicsDiscoveryEvent(connectionHandle, error, characteristic); -} - -int NimbleController::OnCurrentTimeReadResult(uint16_t connectionHandle, const ble_gatt_error *error, ble_gatt_attr *attribute) { - currentTimeClient.OnCurrentTimeReadResult(connectionHandle, error, attribute); - - if (alertNotificationClient.IsDiscovered()) { - ble_gattc_disc_all_chrs(connectionHandle, alertNotificationClient.StartHandle(), - alertNotificationClient.EndHandle(), - AlertNotificationCharacteristicDiscoveredCallback, this); - } - return 0; -} - -int NimbleController::OnANSDescriptorDiscoveryEventCallback(uint16_t connectionHandle, const ble_gatt_error *error, - uint16_t characteristicValueHandle, - const ble_gatt_dsc *descriptor) { - return alertNotificationClient.OnDescriptorDiscoveryEventCallback(connectionHandle, error, characteristicValueHandle, descriptor); -} - void NimbleController::StartDiscovery() { - ble_gattc_disc_all_svcs(connectionHandle, OnAllSvrDisco, this); + serviceDiscovery.StartDiscovery(connectionHandle); } diff --git a/src/components/ble/NimbleController.h b/src/components/ble/NimbleController.h index 9d20caff..8ddec1f9 100644 --- a/src/components/ble/NimbleController.h +++ b/src/components/ble/NimbleController.h @@ -11,6 +11,7 @@ #include "MusicService.h" #include "BatteryInformationService.h" #include "ImmediateAlertService.h" +#include "ServiceDiscovery.h" #include namespace Pinetime { @@ -71,6 +72,8 @@ namespace Pinetime { .value = {0x23, 0xD1, 0xBC, 0xEA, 0x5F, 0x78, 0x23, 0x15, 0xDE, 0xEF, 0x12, 0x12, 0x30, 0x15, 0x00, 0x00} }; + + ServiceDiscovery serviceDiscovery; }; } } diff --git a/src/components/ble/ServiceDiscovery.cpp b/src/components/ble/ServiceDiscovery.cpp new file mode 100644 index 00000000..4b29d89c --- /dev/null +++ b/src/components/ble/ServiceDiscovery.cpp @@ -0,0 +1,31 @@ +#include +#include "ServiceDiscovery.h" +using namespace Pinetime::Controllers; + +ServiceDiscovery::ServiceDiscovery(std::array&& clients) : clients{clients} { + +} + +void ServiceDiscovery::StartDiscovery(uint16_t connectionHandle) { + NRF_LOG_INFO("[Discovery] Starting discovery"); + clientIterator = clients.begin(); + DiscoverNextService(connectionHandle); +} + +void ServiceDiscovery::OnServiceDiscovered(uint16_t connectionHandle) { + clientIterator++; + if(clientIterator != clients.end()) { + DiscoverNextService(connectionHandle); + } else { + NRF_LOG_INFO("End of service discovery"); + } +} + +void ServiceDiscovery::DiscoverNextService(uint16_t connectionHandle) { + NRF_LOG_INFO("[Discovery] Discover next service"); + + auto discoverNextService = [this](uint16_t connectionHandle){ + this->OnServiceDiscovered(connectionHandle); + }; + (*clientIterator)->Discover(connectionHandle, discoverNextService); +} \ No newline at end of file diff --git a/src/components/ble/ServiceDiscovery.h b/src/components/ble/ServiceDiscovery.h new file mode 100644 index 00000000..c86fc4ec --- /dev/null +++ b/src/components/ble/ServiceDiscovery.h @@ -0,0 +1,24 @@ +#pragma once + +#include +#include +#include +#include "BleClient.h" + +namespace Pinetime { + namespace Controllers { + class ServiceDiscovery { + public: + ServiceDiscovery(std::array&& bleClients); + + void StartDiscovery(uint16_t connectionHandle); + + + private: + BleClient** clientIterator; + std::array clients; + void OnServiceDiscovered(uint16_t connectionHandle); + void DiscoverNextService(uint16_t connectionHandle); + }; + } +}