From 70c6e89f195f89a1a705100dd198ac443d08c40c Mon Sep 17 00:00:00 2001 From: h2zero Date: Tue, 26 Nov 2024 18:22:37 -0700 Subject: [PATCH] Refactor service changed handling This makes the services changes notification more accurate by waiting until the changes have taken effect and the server re-started before indicating the change. --- src/NimBLEServer.cpp | 12 +++-- src/NimBLEService.cpp | 106 ++++++++++++++++++++---------------------- 2 files changed, 58 insertions(+), 60 deletions(-) diff --git a/src/NimBLEServer.cpp b/src/NimBLEServer.cpp index 1a9baac..1e13c3f 100644 --- a/src/NimBLEServer.cpp +++ b/src/NimBLEServer.cpp @@ -153,13 +153,12 @@ NimBLEAdvertising* NimBLEServer::getAdvertising() const { # endif /** - * @brief Sends a service changed notification and resets the GATT server. + * @brief Called when the services are added/removed and sets a flag to indicate they should be reloaded. + * @details This has no effect if the GATT server was not already started. */ void NimBLEServer::serviceChanged() { if (m_gattsStarted) { m_svcChanged = true; - ble_svc_gatt_changed(0x0001, 0xffff); - resetGATT(); } } // serviceChanged @@ -205,6 +204,12 @@ void NimBLEServer::start() { } } + // If the services have changed indicate it now + if (m_svcChanged) { + m_svcChanged = false; + ble_svc_gatt_changed(0x0001, 0xffff); + } + m_gattsStarted = true; } // start @@ -876,7 +881,6 @@ void NimBLEServer::resetGATT() { ++it; } - m_svcChanged = false; m_gattsStarted = false; } // resetGATT diff --git a/src/NimBLEService.cpp b/src/NimBLEService.cpp index a0b9601..c7a83a7 100644 --- a/src/NimBLEService.cpp +++ b/src/NimBLEService.cpp @@ -90,84 +90,78 @@ void NimBLEService::dump() const { */ bool NimBLEService::start() { NIMBLE_LOGD(LOG_TAG, ">> start(): Starting service: %s", toString().c_str()); - - // Rebuild the service definition if the server attributes have changed. - if (getServer()->m_svcChanged) { - if (m_pSvcDef->characteristics) { - if (m_pSvcDef->characteristics->descriptors) { - delete[] m_pSvcDef->characteristics->descriptors; - } - delete[] m_pSvcDef->characteristics; + // If started previously, clear everything and start over + if (m_pSvcDef->characteristics) { + if (m_pSvcDef->characteristics->descriptors) { + delete[] m_pSvcDef->characteristics->descriptors; } - m_pSvcDef->type = 0; + delete[] m_pSvcDef->characteristics; } - if (!m_pSvcDef->type) { - m_pSvcDef->type = BLE_GATT_SVC_TYPE_PRIMARY; - size_t numChrs = 0; + size_t numChrs = 0; + for (const auto& chr : m_vChars) { + if (chr->getRemoved()) { + continue; + } + ++numChrs; + } + + NIMBLE_LOGD(LOG_TAG, "Adding %d characteristics for service %s", numChrs, toString().c_str()); + if (numChrs) { + int i = 0; + + // Nimble requires the last characteristic to have it's uuid = 0 to indicate the end + // of the characteristics for the service. We create 1 extra and set it to null + // for this purpose. + ble_gatt_chr_def* pChrs = new ble_gatt_chr_def[numChrs + 1]{}; for (const auto& chr : m_vChars) { if (chr->getRemoved()) { continue; } - ++numChrs; - } - NIMBLE_LOGD(LOG_TAG, "Adding %d characteristics for service %s", numChrs, toString().c_str()); - if (numChrs) { - int i = 0; - - // Nimble requires the last characteristic to have it's uuid = 0 to indicate the end - // of the characteristics for the service. We create 1 extra and set it to null - // for this purpose. - ble_gatt_chr_def* pChrs = new ble_gatt_chr_def[numChrs + 1]{}; - for (const auto& chr : m_vChars) { - if (chr->getRemoved()) { + size_t numDscs = 0; + for (const auto& dsc : chr->m_vDescriptors) { + if (dsc->getRemoved()) { continue; } + ++numDscs; + } - size_t numDscs = 0; + if (numDscs) { + int j = 0; + + // Must have last descriptor uuid = 0 so we have to create 1 extra + ble_gatt_dsc_def* pDscs = new ble_gatt_dsc_def[numDscs + 1]{}; for (const auto& dsc : chr->m_vDescriptors) { if (dsc->getRemoved()) { continue; } - ++numDscs; + + pDscs[j].uuid = dsc->getUUID().getBase(); + pDscs[j].att_flags = dsc->getProperties(); + pDscs[j].min_key_size = 0; + pDscs[j].access_cb = NimBLEServer::handleGattEvent; + pDscs[j].arg = dsc; + ++j; } - if (numDscs) { - int j = 0; - - // Must have last descriptor uuid = 0 so we have to create 1 extra - ble_gatt_dsc_def* pDscs = new ble_gatt_dsc_def[numDscs + 1]{}; - for (const auto& dsc : chr->m_vDescriptors) { - if (dsc->getRemoved()) { - continue; - } - - pDscs[j].uuid = dsc->getUUID().getBase(); - pDscs[j].att_flags = dsc->getProperties(); - pDscs[j].min_key_size = 0; - pDscs[j].access_cb = NimBLEServer::handleGattEvent; - pDscs[j].arg = dsc; - ++j; - } - - pChrs[i].descriptors = pDscs; - } - - pChrs[i].uuid = chr->getUUID().getBase(); - pChrs[i].access_cb = NimBLEServer::handleGattEvent; - pChrs[i].arg = chr; - pChrs[i].flags = chr->getProperties(); - pChrs[i].min_key_size = 0; - pChrs[i].val_handle = &chr->m_handle; - ++i; + pChrs[i].descriptors = pDscs; } - m_pSvcDef->characteristics = pChrs; + pChrs[i].uuid = chr->getUUID().getBase(); + pChrs[i].access_cb = NimBLEServer::handleGattEvent; + pChrs[i].arg = chr; + pChrs[i].flags = chr->getProperties(); + pChrs[i].min_key_size = 0; + pChrs[i].val_handle = &chr->m_handle; + ++i; } + + m_pSvcDef->characteristics = pChrs; } - int rc = ble_gatts_count_cfg(m_pSvcDef); + m_pSvcDef->type = BLE_GATT_SVC_TYPE_PRIMARY; + int rc = ble_gatts_count_cfg(m_pSvcDef); if (rc != 0) { NIMBLE_LOGE(LOG_TAG, "ble_gatts_count_cfg failed, rc= %d, %s", rc, NimBLEUtils::returnCodeToString(rc)); return false;