From d5e22db335c5748506edd0ac4e6f9bd8c0b90c8c Mon Sep 17 00:00:00 2001 From: srgg Date: Mon, 2 Nov 2026 16:00:02 -0700 Subject: [PATCH] Show a hidden service to connected centrals and announce the change * Show a hidden service to connected centrals and announce the change addService() on a service hidden with removeService(svc, false) only flagged it for the next GATT reset, which start() runs only with no central connected, and the only Service Changed went out after that reset. A connected central could not get the service back and never heard about the hide or the show. Hidden services now stay registered across a reset, addService() makes one visible in place, and hide and show indicate Service Changed for that service's handle range. * Send full-range Service Changed and defer deleting a hidden service ble_svc_gatt_changed() keeps one pending range, so a second hide or show before the first indication went out overwrote the first range and the central never rediscovered that service. Hide and show now indicate the full range, as sendServiceChangedIndication() does. A hidden service now survives a reset, so deleting it freed a definition the stack still pointed to. It is now marked for deletion and freed by resetGATT(). addService() shows a hidden service in place only when no reset is pending, since that reset may still change its database. * Stop resetGATT when ble_gatts_reset refuses ble_gatts_reset() refuses with BLE_HS_EBUSY while an advertising, discovery or connection procedure is active, and leaves every attribute registered. resetGATT() ignored that and freed the services marked for deletion, which the stack still references. It now returns false before freeing anything; the services stay marked changed, so the next start() retries. * Re-send Service Changed to a client as it subscribes Service Changed reaches only the clients connected when it is sent; NimBLE keeps a missed indication for bonded peers alone. A client that disconnects as it goes out keeps the table it last discovered, and CoreBluetooth serves that cached table to its next connection: after a service was shown in place, the first new link saw the old table, 3 of 3 runs. The same holds for the indication start() sends after a GATT reset, with no client connected. Once any Service Changed has been sent, each client that enables it with a CCCD write gets the indication again. A bonded client's restored subscription is left to the host, which sends its pending indication itself. * Indicate Service Changed when a visible service is deleted removeService(svc, true) hid the service at once but indicated nothing: the indication came from start() after the reset, and the reset waits until no client is connected, so a connected client kept the deleted service in its table. The delete now indicates as a hide does, and still marks the reset that frees the service. # Conflicts: # src/NimBLEServer.cpp # src/NimBLEServer.h --- src/NimBLEServer.cpp | 99 ++++++++++++++++++++++++++++++++------------ src/NimBLEServer.h | 1 + 2 files changed, 73 insertions(+), 27 deletions(-) diff --git a/src/NimBLEServer.cpp b/src/NimBLEServer.cpp index b51cfa47..08bfa0d8 100644 --- a/src/NimBLEServer.cpp +++ b/src/NimBLEServer.cpp @@ -54,7 +54,8 @@ NimBLEServer::NimBLEServer() : m_gattsStarted{false}, m_svcChanged{false}, m_registerAppServicesFirst{false}, -# if !MYNEWT_VAL(BLE_EXT_ADV) + m_svcChangedSent{false}, +# if !MYNEWT_VAL(BLE_EXT_ADV) && MYNEWT_VAL(BLE_ROLE_BROADCASTER) m_advertiseOnDisconnect{false}, # endif m_pCallbacks{&defaultCallbacks}, @@ -193,6 +194,7 @@ void NimBLEServer::setServiceChanged() { * @details This should be called when services are added, removed or modified after the server has been started. */ void NimBLEServer::sendServiceChangedIndication() const { + m_svcChangedSent = true; ble_svc_gatt_changed(0x0001, 0xffff); } @@ -210,7 +212,7 @@ void NimBLEServer::gattRegisterCallback(ble_gatt_register_ctxt* ctxt, void* arg) NimBLEUUID uuid(ctxt->svc.svc_def->uuid); args->pSvc = nullptr; for (auto pSvc : NimBLEDevice::getServer()->m_svcVec) { - if (!pSvc->getRemoved() && pSvc->m_handle == 0 && pSvc->getUUID() == uuid) { + if (pSvc->getRemoved() != NIMBLE_ATT_REMOVE_DELETE && pSvc->m_handle == 0 && pSvc->getUUID() == uuid) { pSvc->m_handle = ctxt->svc.handle; NIMBLE_LOGD(LOG_TAG, "Service registered: %s, handle=%d", uuid.toString().c_str(), ctxt->svc.handle); // Set the arg to the service so we know that the following @@ -296,6 +298,13 @@ bool NimBLEServer::start() { return false; } + // resetGATT() registered the hidden services too; hide them again now that they have handles. + for (const auto& svc : m_svcVec) { + if (svc->getRemoved() == NIMBLE_ATT_REMOVE_HIDE) { + ble_gatts_svc_set_visibility(svc->getHandle(), 0); + } + } + # if MYNEWT_VAL(NIMBLE_CPP_LOG_LEVEL) >= 4 ble_gatts_show_local(); @@ -547,6 +556,28 @@ int NimBLEServer::handleGapEvent(ble_gap_event* event, void* arg) { uint8_t subVal = event->subscribe.cur_notify + (event->subscribe.cur_indicate << 1); NIMBLE_LOGI(LOG_TAG, "subscribe event; attr_handle=%d, subscribed: %d", event->subscribe.attr_handle, subVal); + static const ble_uuid16_t gattSvcUuid = BLE_UUID16_INIT(BLE_GATT_SVC_UUID16); + static const ble_uuid16_t svcChangedUuid = BLE_UUID16_INIT(BLE_SVC_GATT_CHR_SERVICE_CHANGED_UUID16); + uint16_t svcChangedHandle = 0; + if (ble_gatts_find_chr(&gattSvcUuid.u, &svcChangedUuid.u, nullptr, &svcChangedHandle) == 0 && + svcChangedHandle == event->subscribe.attr_handle) { + // GATT database changes can leave clients with stale attribute caches. + // NimBLE sends Service Changed indications to connected clients but preserves + // pending indications across disconnections only for bonded clients. + // + // Because unbonded clients cannot be recognized across connections, once any + // Service Changed has been sent the server indicates it to every client that + // enables it. A server may indicate only after the client enables it in the CCCD, + // so this is the earliest opportunity, and it stops a caching client + // (CoreBluetooth) from serving a stale table. A subscription restored for a bonded + // client is skipped: the host sends that client's pending indication itself. + if (pServer->m_svcChangedSent && event->subscribe.cur_indicate && + event->subscribe.reason == BLE_GAP_SUBSCRIBE_REASON_WRITE) { + ble_gatts_indicate(event->subscribe.conn_handle, svcChangedHandle); + } + break; + } + auto pChar = pServer->getCharacteristicByHandle(event->subscribe.attr_handle); if (!pChar) { NIMBLE_LOGE(LOG_TAG, "subscribe event; attr_handle=%d, not found", event->subscribe.attr_handle); @@ -826,8 +857,9 @@ void NimBLEServer::resetCallbacks() { * available and can be re-added in the future. If desired a removed but not deleted service can * be deleted later by calling this method with deleteSvc set to true. * - * @note The service will not be removed from the database until all open connections are closed - * as it requires resetting the GATT server. In the interim the service will have it's visibility disabled. + * @note A service removed but not deleted stays registered with its visibility disabled, through any + * GATT reset, so addService() can show it again while clients are connected. A deleted service + * leaves the database at the next reset, which waits until all open connections are closed. * * @note Advertising will need to be restarted by the user after calling this as we must stop * advertising in order to remove the service. @@ -841,13 +873,10 @@ void NimBLEServer::removeService(NimBLEService* service, bool deleteSvc) { // Otherwise, ignore the call and return. if (service->getRemoved() > 0) { if (deleteSvc) { - for (auto it = m_svcVec.begin(); it != m_svcVec.end(); ++it) { - if ((*it) == service) { - delete *it; - m_svcVec.erase(it); - break; - } - } + // A hidden service is still registered and the stack keeps a pointer to its definition, + // so resetGATT() frees it once the reset has dropped it from the database. + service->setRemoved(NIMBLE_ATT_REMOVE_DELETE); + setServiceChanged(); } return; @@ -859,7 +888,10 @@ void NimBLEServer::removeService(NimBLEService* service, bool deleteSvc) { } service->setRemoved(deleteSvc ? NIMBLE_ATT_REMOVE_DELETE : NIMBLE_ATT_REMOVE_HIDE); - setServiceChanged(); + if (deleteSvc) { + setServiceChanged(); + } + sendServiceChangedIndication(); # if !MYNEWT_VAL(BLE_EXT_ADV) && MYNEWT_VAL(BLE_ROLE_BROADCASTER) NimBLEDevice::getAdvertising()->removeServiceUUID(service->getUUID()); # endif @@ -885,6 +917,15 @@ void NimBLEServer::addService(NimBLEService* service) { return; } + // Hidden but still registered: shown in place, no reset needed. A pending reset may still change + // the database under it, so the service waits for that reset instead. + if (service->getRemoved() == NIMBLE_ATT_REMOVE_HIDE && service->getHandle() != 0 && !m_svcChanged && + ble_gatts_svc_set_visibility(service->getHandle(), 1) == 0) { + service->setRemoved(0); + sendServiceChangedIndication(); + return; + } + service->setRemoved(0); setServiceChanged(); } // addService @@ -892,8 +933,8 @@ void NimBLEServer::addService(NimBLEService* service) { /** * @brief Resets the GATT server, used when services are added/removed after initialization. * @return True if successful. - * @details This will reset the GATT server and re-register all services, characteristics, and - * descriptors that have not been removed. Services, characteristics, and descriptors that have been + * @details This will reset the GATT server and re-register every service that has not been deleted, + * hidden ones included; start() hides those again. Characteristics and descriptors that have been * removed but not deleted will be skipped and have their handles cleared, and those that have been * deleted will be removed from the server's service vector. */ @@ -901,15 +942,19 @@ bool NimBLEServer::resetGATT() { # if MYNEWT_VAL(BLE_ROLE_BROADCASTER) NimBLEDevice::stopAdvertising(); # endif - // esp-idf nimble clears the device name and // appearance during ble_gatts_reset() #ifndef CONFIG_USING_NIMBLE_COMPONENT std::string name(ble_svc_gap_device_name()); uint16_t appearance = ble_svc_gap_device_appearance(); #endif - - ble_gatts_reset(); + // A refused reset leaves every attribute registered, so nothing may be freed below; the next + // start() retries, since services stay marked changed. + int rc = ble_gatts_reset(); + if (rc != 0) { + NIMBLE_LOGE(LOG_TAG, "ble_gatts_reset; rc=%d, %s", rc, NimBLEUtils::returnCodeToString(rc)); + return false; + } // Register the standard GAP (0x1800) and GATT (0x1801) services, restoring the // device name/appearance that ble_gatts_reset() clears. By default this happens @@ -961,17 +1006,17 @@ bool NimBLEServer::resetGATT() { ++chrIt; } - if (pSvc->getRemoved() == 0) { - if (!pSvc->start_internal()) { - NIMBLE_LOGE(LOG_TAG, "Failed to start service: %s", pSvc->getUUID().toString().c_str()); - // When deferring GAP/GATT (registerAppServicesFirst), still register - // them on the failure path so the mandatory GAP/GATT services (and - // the restored name/appearance) are never left out of the database. - if (m_registerAppServicesFirst) { - initGapGattServices(); - } - return false; + // A hidden service stays registered, so addService() can show it again while a client is + // connected — a reset, which needs none connected, is the only way to register one. + if (!pSvc->start_internal()) { + NIMBLE_LOGE(LOG_TAG, "Failed to start service: %s", pSvc->getUUID().toString().c_str()); + // When deferring GAP/GATT (registerAppServicesFirst), still register + // them on the failure path so the mandatory GAP/GATT services (and + // the restored name/appearance) are never left out of the database. + if (m_registerAppServicesFirst) { + initGapGattServices(); } + return false; } pSvc->m_handle = 0; diff --git a/src/NimBLEServer.h b/src/NimBLEServer.h index b4303356..37fab98d 100644 --- a/src/NimBLEServer.h +++ b/src/NimBLEServer.h @@ -132,6 +132,7 @@ class NimBLEServer { bool m_gattsStarted : 1; bool m_svcChanged : 1; bool m_registerAppServicesFirst : 1; + mutable bool m_svcChangedSent : 1; # if !MYNEWT_VAL(BLE_EXT_ADV) && MYNEWT_VAL(BLE_ROLE_BROADCASTER) bool m_advertiseOnDisconnect : 1; # endif