Bluetooth: fix GATT service reregistering

* Fixed issue with reregistering of a GATT services. * Added unit tests covering the GATT reregistering scenario. Signed-off-by: Stine Akredalen <[email protected]>
zephyrproject-rtos · Nov 26, 2024 · 599cbb4 · 599cbb4
1 parent c22233a
commit 599cbb4
Show file tree

Hide file tree

Showing 3 changed files with 166 additions and 1 deletion.
diff --git a/include/zephyr/bluetooth/gatt.h b/include/zephyr/bluetooth/gatt.h
@@ -296,8 +296,13 @@ struct bt_gatt_attr {
 	 *  satisfied before calling read() or write().
 	 *
 	 *  @sa bt_gatt_discover_func_t about this field.
+	 *
+	 *  @note The `_auto` flag in this bitfield indicates if the attribute handle was
+	 *  automatically assigned by the stack (1) or manually set by the application (0).
+	 *
 	 */
-	uint16_t perm;
+	uint16_t perm: 15,
+		_auto: 1;
 };
 
 /** @brief GATT Service structure */

diff --git a/subsys/bluetooth/host/gatt.c b/subsys/bluetooth/host/gatt.c
@@ -1307,9 +1307,11 @@ static int gatt_register(struct bt_gatt_service *svc)
 populate:
 	/* Populate the handles and append them to the list */
 	for (; attrs && count; attrs++, count--) {
+		attrs->_auto = 0;
 		if (!attrs->handle) {
 			/* Allocate handle if not set already */
 			attrs->handle = ++handle;
+			attrs->_auto = 1;
 		} else if (attrs->handle > handle) {
 			/* Use existing handle if valid */
 			handle = attrs->handle;
@@ -1727,6 +1729,13 @@ static int gatt_unregister(struct bt_gatt_service *svc)
 		if (is_host_managed_ccc(attr)) {
 			gatt_unregister_ccc(attr->user_data);
 		}
+
+		/* The stack should not clear any handles set by the user. */
+		if (attr->_auto) {
+			LOG_DBG("Pre-allocated handle 0x%04x was not clared.", attr->handle);
+			attr->handle = 0;
+			attr->_auto = 0;
+		}
 	}
 
 	return 0;

diff --git a/tests/bluetooth/gatt/src/main.c b/tests/bluetooth/gatt/src/main.c
@@ -142,6 +142,157 @@ ZTEST(test_gatt, test_gatt_unregister)
 		     "Test service unregister failed");
 }
 
+ZTEST(test_gatt, test_gatt_reregister)
+{
+	struct bt_gatt_attr local_test_attrs[] = {
+		/* Vendor Primary Service Declaration */
+		BT_GATT_PRIMARY_SERVICE(&test_uuid),
+
+		BT_GATT_CHARACTERISTIC(&test_chrc_uuid.uuid, BT_GATT_CHRC_READ | BT_GATT_CHRC_WRITE,
+				       BT_GATT_PERM_READ_AUTHEN | BT_GATT_PERM_WRITE_AUTHEN,
+				       read_test, write_test, test_value),
+	};
+
+	struct bt_gatt_attr local_test1_attrs[] = {
+		/* Vendor Primary Service Declaration */
+		BT_GATT_PRIMARY_SERVICE(&test1_uuid),
+
+		BT_GATT_CHARACTERISTIC(&test1_nfy_uuid.uuid, BT_GATT_CHRC_NOTIFY, BT_GATT_PERM_NONE,
+				       NULL, NULL, &nfy_enabled),
+		BT_GATT_CCC(test1_ccc_cfg_changed, BT_GATT_PERM_READ | BT_GATT_PERM_WRITE),
+	};
+	struct bt_gatt_service local_test_svc = BT_GATT_SERVICE(local_test_attrs);
+	struct bt_gatt_service local_test1_svc = BT_GATT_SERVICE(local_test1_attrs);
+
+	for (int i = 0; i < 10; i++) {
+
+		/* Attempt to register service A */
+		zassert_false(bt_gatt_service_register(&local_test_svc),
+			      "Test service A registration failed");
+
+		/* Attempt to unregister service A */
+		zassert_false(bt_gatt_service_unregister(&local_test_svc),
+			      "Test service A unregister failed");
+
+		/* Attempt to register service B */
+		zassert_false(bt_gatt_service_register(&local_test1_svc),
+			      "Test service B registration failed");
+
+		/* Attempt to re-register service A */
+		zassert_false(bt_gatt_service_register(&local_test_svc),
+			      "Test service A re-registering failed...");
+
+		/* Clean up */
+		zassert_false(bt_gatt_service_unregister(&local_test_svc),
+			      "Test service A unregister failed");
+		zassert_false(bt_gatt_service_unregister(&local_test1_svc),
+			      "Test service B unregister failed");
+	}
+}
+
+ZTEST(test_gatt, test_gatt_reregister_pre_allocated_handles)
+{
+	struct bt_gatt_attr local_test_attrs[] = {
+		/* Vendor Primary Service Declaration */
+		BT_GATT_PRIMARY_SERVICE(&test_uuid),
+
+		BT_GATT_CHARACTERISTIC(&test_chrc_uuid.uuid, BT_GATT_CHRC_READ | BT_GATT_CHRC_WRITE,
+				       BT_GATT_PERM_READ_AUTHEN | BT_GATT_PERM_WRITE_AUTHEN,
+				       read_test, write_test, test_value),
+	};
+
+	struct bt_gatt_attr local_test1_attrs[] = {
+		/* Vendor Primary Service Declaration */
+		BT_GATT_PRIMARY_SERVICE(&test1_uuid),
+
+		BT_GATT_CHARACTERISTIC(&test1_nfy_uuid.uuid, BT_GATT_CHRC_NOTIFY, BT_GATT_PERM_NONE,
+				       NULL, NULL, &nfy_enabled),
+		BT_GATT_CCC(test1_ccc_cfg_changed, BT_GATT_PERM_READ | BT_GATT_PERM_WRITE),
+	};
+	struct bt_gatt_service local_test_svc = BT_GATT_SERVICE(local_test_attrs);
+	struct bt_gatt_service local_test1_svc = BT_GATT_SERVICE(local_test1_attrs);
+
+	/* Pre-allocate handles for both services */
+	for (int j = 0; j < local_test_svc.attr_count; j++) {
+		local_test_svc.attrs[j].handle = 0x0100 + j;
+	}
+	for (int j = 0; j < local_test1_svc.attr_count; j++) {
+		local_test1_svc.attrs[j].handle = 0x0200 + j;
+	}
+
+	/* Attempt to register service A */
+	zassert_false(bt_gatt_service_register(&local_test_svc),
+		      "Test service A registration failed");
+
+	/* Attempt to unregister service A */
+	zassert_false(bt_gatt_service_unregister(&local_test_svc),
+		      "Test service A unregister failed");
+
+	/* Attempt to register service B */
+	zassert_false(bt_gatt_service_register(&local_test1_svc),
+		      "Test service B registration failed");
+
+	/* Attempt to re-register service A */
+	zassert_false(bt_gatt_service_register(&local_test_svc),
+		      "Test service A re-registering failed...");
+
+	/* Clean up */
+	zassert_false(bt_gatt_service_unregister(&local_test_svc),
+		      "Test service A unregister failed");
+	zassert_false(bt_gatt_service_unregister(&local_test1_svc),
+		      "Test service B unregister failed");
+}
+
+ZTEST(test_gatt, test_gatt_reregister_pre_allocated_handle_single)
+{
+	struct bt_gatt_attr local_test_attrs[] = {
+		/* Vendor Primary Service Declaration */
+		BT_GATT_PRIMARY_SERVICE(&test_uuid),
+
+		BT_GATT_CHARACTERISTIC(&test_chrc_uuid.uuid, BT_GATT_CHRC_READ | BT_GATT_CHRC_WRITE,
+				       BT_GATT_PERM_READ_AUTHEN | BT_GATT_PERM_WRITE_AUTHEN,
+				       read_test, write_test, test_value),
+	};
+
+	struct bt_gatt_attr local_test1_attrs[] = {
+		/* Vendor Primary Service Declaration */
+		BT_GATT_PRIMARY_SERVICE(&test1_uuid),
+
+		BT_GATT_CHARACTERISTIC(&test1_nfy_uuid.uuid, BT_GATT_CHRC_NOTIFY, BT_GATT_PERM_NONE,
+				       NULL, NULL, &nfy_enabled),
+		BT_GATT_CCC(test1_ccc_cfg_changed, BT_GATT_PERM_READ | BT_GATT_PERM_WRITE),
+	};
+	struct bt_gatt_service local_test_svc = BT_GATT_SERVICE(local_test_attrs);
+	struct bt_gatt_service local_test1_svc = BT_GATT_SERVICE(local_test1_attrs);
+
+	/* Pre-allocate handles for service A */
+	for (int j = 0; j < local_test_svc.attr_count; j++) {
+		local_test_svc.attrs[j].handle = 0x0100 + j;
+	}
+
+	/* Attempt to register service A */
+	zassert_false(bt_gatt_service_register(&local_test_svc),
+		      "Test service A registration failed");
+
+	/* Attempt to unregister service A */
+	zassert_false(bt_gatt_service_unregister(&local_test_svc),
+		      "Test service A unregister failed");
+
+	/* Attempt to register service B */
+	zassert_false(bt_gatt_service_register(&local_test1_svc),
+		      "Test service B registration failed");
+
+	/* Attempt to re-register service A */
+	zassert_false(bt_gatt_service_register(&local_test_svc),
+		      "Test service A re-registering failed...");
+
+	/* Clean up */
+	zassert_false(bt_gatt_service_unregister(&local_test_svc),
+		      "Test service A unregister failed");
+	zassert_false(bt_gatt_service_unregister(&local_test1_svc),
+		      "Test service B unregister failed");
+}
+
 static uint8_t count_attr(const struct bt_gatt_attr *attr, uint16_t handle,
 			  void *user_data)
 {