From 599cbb4e37bf273a596ec812ea8de82cdee9171b Mon Sep 17 00:00:00 2001 From: Stine Akredalen Date: Mon, 25 Nov 2024 15:08:12 +0100 Subject: [PATCH] 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 --- include/zephyr/bluetooth/gatt.h | 7 +- subsys/bluetooth/host/gatt.c | 9 ++ tests/bluetooth/gatt/src/main.c | 151 ++++++++++++++++++++++++++++++++ 3 files changed, 166 insertions(+), 1 deletion(-) diff --git a/include/zephyr/bluetooth/gatt.h b/include/zephyr/bluetooth/gatt.h index c72cb821a726ade..2f8144c33ddb4ec 100644 --- 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 index 023216073e03f36..2b01c7a0b19fafb 100644 --- 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 index 3487fa6c036eddf..a13068100e681f6 100644 --- 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) {