Properly set attribute handles and improve dynamic service changes by h2zero · Pull Request #399 · h2zero/esp-nimble-cpp

h2zero · 2026-03-09T20:24:50Z

This changes how attribute handles are set so they can be correctly identified when there is more than one attribute with the same UUID. Instead of reading from the stack by UUID to get the handles this will now use the registration callback to set them correctly.

This also improves handling of dynamic service changes by properly removing characteristics/descriptors when required and resetting the GATT when advertising is started instead of after the last client disconnects.

Adds NimBLEUUID constructor overload for ble_uuid_t*.
NimBLECharacteristic::getDescriptorByUUID now takes an optional index value to support multiple same-uuid descriptors.

This changes how attribute handles are set so they can be correctly identified when there is more than one attribute with the same UUID. Instead of reading from the stack by UUID to get the handles this will now use the registration callback to set them correctly. This also improves handling of dynamic service changes by properly removing characteristics/descriptors when required and resetting the GATT when advertising is started instead of after the last client disconnects. * Adds NimBLEUUID constructor overload for ble_uuid_t*. * NimBLECharacteristic::getDescriptorByUUID now takes an optional index value to support multiple same-uuid descriptors.

Copilot

Pull request overview

This PR updates how GATT attribute handles are tracked (especially with duplicate UUIDs) by switching to NimBLE’s GATT registration callback, and refines the dynamic service-change/reset flow so the GATT database is rebuilt at server start/advertising time rather than on last disconnect.

Changes:

Add NimBLEUUID constructor overload for const ble_uuid_t*.
Populate service/characteristic/descriptor handles via ble_hs_cfg.gatts_register_cb registration events instead of UUID-based stack lookups.
Extend NimBLECharacteristic::getDescriptorByUUID with an optional index to support multiple descriptors sharing the same UUID.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/NimBLEUUID.h	Adds UUID ctor overload from native UUID pointer.
src/NimBLEUUID.cpp	Implements pointer-based UUID construction with null handling.
src/NimBLEService.cpp	Adjusts service start behavior/logging and handle assignment strategy.
src/NimBLEServer.h	Declares GATT registration callback hook.
src/NimBLEServer.cpp	Implements registration callback and updates GATT reset/start sequencing.
src/NimBLECharacteristic.h / .cpp	Adds indexed descriptor lookup to support duplicate UUID descriptors.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-10T16:06:26Z

src/NimBLEService.cpp

    }

-    NIMBLE_LOGD(LOG_TAG, "Adding %d characteristics for service %s", numChrs, toString().c_str());
+    NIMBLE_LOGD(LOG_TAG, "Adding %d characteristics for service %s", numChrs, getUUID().toString().c_str());


numChrs is a size_t, but this log uses %d. This can trigger format warnings (or errors under -Werror) on some toolchains. Use the correct format specifier or cast to an int explicitly if the value is known to fit.

Suggested change

NIMBLE_LOGD(LOG_TAG, "Adding %d characteristics for service %s", numChrs, getUUID().toString().c_str());

NIMBLE_LOGD(LOG_TAG, "Adding %zu characteristics for service %s", numChrs, getUUID().toString().c_str());

Copilot · 2026-03-10T16:06:27Z

src/NimBLEServer.cpp

+        for (auto i = 0; i < NimBLEDevice::getServer()->m_svcVec.size(); i++) {
+            auto pSvc = NimBLEDevice::getServer()->getServiceByUUID(uuid, i);
+            if (pSvc != nullptr && pSvc->m_handle == 0) {
+                pSvc->m_handle = ctxt->svc.handle;
+                NIMBLE_LOGD(LOG_TAG, "Service registered: %s, handle=%d", uuid.toString().c_str(), ctxt->svc.handle);


These for (auto i = 0; i < ...size(); i++) loops deduce i as int and compare to .size() (size_t), which can trigger signed/unsigned comparison warnings. Prefer size_t i = 0 (or a range-based loop) for the index variable.

Copilot · 2026-03-10T16:06:27Z

src/NimBLEServer.cpp

+void NimBLEServer::gattRegisterCallback(ble_gatt_register_ctxt* ctxt, void* arg) {
+    gattRegisterCallbackArgs* args = static_cast<gattRegisterCallbackArgs*>(arg);
+
+    if (ctxt->op == BLE_GATT_REGISTER_OP_SVC) {
+        NimBLEUUID uuid(ctxt->svc.svc_def->uuid);
+        for (auto i = 0; i < NimBLEDevice::getServer()->m_svcVec.size(); i++) {
+            auto pSvc = NimBLEDevice::getServer()->getServiceByUUID(uuid, i);
+            if (pSvc != nullptr && pSvc->m_handle == 0) {
+                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
+                // characteristics and descriptors belong to this service
+                args->pSvc = pSvc;
+                break;
+            }


In gattRegisterCallback(), the state in args is not cleared when a service UUID isn’t found. If a BLE_GATT_REGISTER_OP_SVC event doesn’t match any entry in m_svcVec, args->pSvc (and potentially args->pChar) can retain the previous pointers, causing subsequent CHR/DSC registrations to be attributed to the wrong service/characteristic. Reset args->pSvc/args->pChar to nullptr at the start of each SVC op (and consider clearing pChar at each CHR op) before attempting to match.

Copilot · 2026-03-10T16:06:27Z

src/NimBLEServer.cpp

+    gattRegisterCallbackArgs args{};
+    ble_hs_cfg.gatts_register_arg = &args;


ble_hs_cfg.gatts_register_arg is set to the address of a stack variable (gattRegisterCallbackArgs args{}) in NimBLEServer::start(). If the host keeps and uses gatts_register_arg after start() returns, this becomes a dangling pointer. Store the callback args with static/member lifetime (or clear ble_hs_cfg.gatts_register_arg immediately after ble_gatts_start() if it is guaranteed synchronous).

Suggested change

gattRegisterCallbackArgs args{};

ble_hs_cfg.gatts_register_arg = &args;

static gattRegisterCallbackArgs s_args{};

ble_hs_cfg.gatts_register_arg = &s_args;

Copilot · 2026-03-10T16:06:27Z

src/NimBLEService.cpp

+    // to update the service/characteristic/descriptor definitions. If characteristics or descriptors
+    // have been added/removed since the last server start then this service will be started on gatt reset.
+    if (getServer()->m_svcChanged) {
+        NIMBLE_LOGW(LOG_TAG, "<< start(): GATT server already started, cannot start service");


The warning message here is misleading: the condition checks getServer()->m_svcChanged (i.e., services changed after the GATT server started), not simply that the server is already started. Consider updating the log text to indicate a GATT reset/restart is required due to dynamic service changes.

Suggested change

NIMBLE_LOGW(LOG_TAG, "<< start(): GATT server already started, cannot start service");

NIMBLE_LOGW(LOG_TAG, "<< start(): GATT services changed after start; GATT reset/restart required before adding this service");

h2zero requested a review from Copilot March 10, 2026 16:00

Copilot started reviewing on behalf of h2zero March 10, 2026 16:01 View session

Copilot AI reviewed Mar 10, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Properly set attribute handles and improve dynamic service changes#399

Properly set attribute handles and improve dynamic service changes#399
h2zero wants to merge 1 commit intomasterfrom
bugfix/att-handles

h2zero commented Mar 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	NIMBLE_LOGD(LOG_TAG, "Adding %d characteristics for service %s", numChrs, getUUID().toString().c_str());
	NIMBLE_LOGD(LOG_TAG, "Adding %zu characteristics for service %s", numChrs, getUUID().toString().c_str());

		gattRegisterCallbackArgs args{};
		ble_hs_cfg.gatts_register_arg = &args;

	NIMBLE_LOGW(LOG_TAG, "<< start(): GATT server already started, cannot start service");
	NIMBLE_LOGW(LOG_TAG, "<< start(): GATT services changed after start; GATT reset/restart required before adding this service");

Uh oh!

Conversation

h2zero commented Mar 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants