Bluetooth: Host: Introduce new GAP Device Name APIs #88896

theob-pro · 2025-04-22T12:06:34Z

Introduce new threads safe and less confusing APIs to get/set the Bluetooth GAP Device Name.

Deprecate the old APIs and update the code base to use the new APIs.

If dynamic Bluetooth device name was enabled but the name was never set with `bt_set_name`, we would then store the name set using `CONFIG_BT_DEVICE_NAME`. This can be really confusing because it mean that if someone change the value of `CONFIG_BT_DEVICE_NAME` later on, and flash their device, the old name would still be the one stored in the setting and being used. By not storing it here, users will now have to explicitly set a name using `bt_set_name` when using dynamic Bluetooth device name. Signed-off-by: Théo Battrel <[email protected]>

No longer use `CONFIG_BT_DEVICE_NAME` to set the name when using dynamic device name at Bluetooth initialization and when settings are disabled. This can lead to confusion. When using dynamic device name user will have to explicitly set the GAP device name. Signed-off-by: Théo Battrel <[email protected]>

Introduce new shiny APIs to get and set the Bluetooth GAP Device Name. The following two functions have been added: - `bt_gap_get_device_name` - `bt_gap_set_device_name` Those new APIs are ✨ threads safe ✨ and handle Bluetooth GAP Device Name containing '\0'. To do that they work with byte array instead of char array. Signed-off-by: Théo Battrel <[email protected]>

alxelax · 2025-04-22T13:10:17Z

subsys/bluetooth/mesh/pb_gatt_srv.c


 dev_name:
 	if (IS_ENABLED(CONFIG_BT_MESH_PB_GATT_USE_DEVICE_NAME)) {
+		uint8_t name[BT_GAP_DEVICE_NAME_MAX_SIZE];


I'm pretty sure after {} name array will disappear and stack might be used for anything else. Device might have garbage instead of name.

Even more, if you allocate memory on stack within this function, after leaving it the stack will be released and name will be again garbage.

This memory should be kept until Host copies in its buffer or global.
Probably place where struct bt_data is allocated should be sufficient.

You are right, I did the update too quickly. It should be good now.

alxelax · 2025-04-22T13:14:33Z

subsys/bluetooth/mesh/pb_gatt_srv.c


 dev_name:
 	if (IS_ENABLED(CONFIG_BT_MESH_PB_GATT_USE_DEVICE_NAME)) {
+		uint8_t name[BT_GAP_DEVICE_NAME_MAX_SIZE];


It will be nice to have BT_GAP_DEVICE_NAME_MAX_SIZE configured over KConfig despite it has specified size. I doubt in that all customers will consume all 237 bytes. Due to this, mesh functions will have stack memory overconsumption.

I created DEVICE_NAME_SIZE in mesh/gatt.h which conditionally take the value of BT_GAP_DEVICE_NAME_MAX_SIZE or CONFIG_BT_GAP_DEVICE_NAME_DYNAMIC_MAX if CONFIG_BT_GAP_DEVICE_NAME_DYNAMIC is enabled.

alxelax · 2025-04-22T13:25:39Z

subsys/bluetooth/mesh/pb_gatt_srv.c

 dev_name:
 	if (IS_ENABLED(CONFIG_BT_MESH_PB_GATT_USE_DEVICE_NAME)) {
+		uint8_t name[BT_GAP_DEVICE_NAME_MAX_SIZE];
+		size_t name_size = bt_gap_get_device_name(name, sizeof(name));


-ENOMEM should be checked and\or converted to zero.

Could it be static assertion instead of size checking?

I added a check, I don't think that using assertion would be right here.

alxelax · 2025-04-22T13:28:36Z

subsys/bluetooth/mesh/gatt.h

@PavelVPV, would you have quite a look at this diff either?

Update the Host subsys functions using the GAP Device Name to use the new APIs. Signed-off-by: Théo Battrel <[email protected]>

Update the Mesh functions using the GAP Device Name to use the new APIs. Signed-off-by: Théo Battrel <[email protected]>

Update samples to use the new Bluetooth GAP Device name APIs added in a previous commit. Signed-off-by: Théo Battrel <[email protected]>

Update the Bluetooth Host tests that uses the GAP Device Name to use the new APIs. Signed-off-by: Théo Battrel <[email protected]>

The following Kconfig APIs are deprecated: - `CONFIG_BT_DEVICE_NAME` - `CONFIG_BT_DEVICE_DYNAMIC` - `CONFIG_BT_DEVICE_NAME_MAX` The following C APIs are deprecated: - `bt_get_name()` - `bt_set_name()` They are deprecated because the current APIs are not thread safe and can be confusing. The Kconfig APIs are replaced by the `BT_GAP_DEVICE_NAME_TYPE` choice. When selecting `CONFIG_BT_GAP_DEVICE_NAME_STATIC`, the name must be set using `CONFIG_BT_GAP_DEVICE_NAME`. When selecting `CONFIG_BT_GAP_DEVICE_NAME_DYNAMIC`, the GAP device name must be set using `bt_gap_set_device_name()` and retrieved using `bt_gap_get_device_name()`. The the maximum size of the GAP device name can now be set using `CONFIG_BT_GAP_DEVICE_NAME_DYNAMIC_MAX`. Signed-off-by: Théo Battrel <[email protected]>

PavelVPV

I know it is in the draft state but will put my comments here to not forget them.

I think there is a confusion in the new API usage.

By reading the API description and the code, it is assumed that the name (which is called as buf) is not a string (copied using memcpy, second argument to new functions called size, not length). So I assume (and as I see in the changes) a user must exclude NULL-terminator from the buf's size when passing it to the new API (they can't just use strlen, for example). However, I see also that a string literals are used as they were used before in Kconfig options (e.g. CONFIG_BT_GAP_DEVICE_NAME="sdp_client") which automatically includes NULL-terminator.

Also, the previous API was assuming name to be a string, so the new API doesn't fully match to the old one, which requires a migration guide.

I think it would be better for user experience if the new API matches the old one with regards to NULL-terminator. It is much more beneficial and less bug-prone for users.

PavelVPV · 2025-04-23T05:59:28Z

subsys/bluetooth/host/gap/device_name_dynamic.c

+		LOG_DBG("Device name is too big for the provided buffer.");
+		return -ENOMEM;


k_mutex_unlock is missing.

PavelVPV · 2025-04-23T06:10:51Z

subsys/bluetooth/host/gap/device_name_dynamic.c

+	int err;
+
+	if (size > BT_GAP_DEVICE_NAME_MAX_SIZE) {
+		return -ENOBUFS;


Wouldn't EINVAL be more appropriate here as showing that the argument is incorrect? ENOBUFS more related to pools.

PavelVPV · 2025-04-23T08:15:09Z

subsys/bluetooth/mesh/proxy_srv.c

+                size_t name_size = bt_gap_get_device_name(name, sizeof(name));
+		if (name_size < 0) {


size_t can't be negative. Below as well.

PavelVPV · 2025-04-23T08:15:37Z

subsys/bluetooth/mesh/proxy_srv.c

+                uint8_t name[DEVICE_NAME_SIZE];
+                size_t name_size = bt_gap_get_device_name(name, sizeof(name));
+		if (name_size < 0) {
+			LOG_DBG("Failed to get name (err %d)", name_size);


Suggested change

LOG_DBG("Failed to get name (err %d)", name_size);

LOG_ERR("Failed to get name (err %d)", name_size);

Below as well.

PavelVPV · 2025-04-23T08:17:59Z

subsys/bluetooth/mesh/proxy_srv.c

 		sd[0].type = BT_DATA_NAME_COMPLETE;
-		sd[0].data_len = BT_DEVICE_NAME_LEN;
-		sd[0].data = BT_DEVICE_NAME;
+		sd[0].data_len = name_size;
+		sd[0].data = name;


This is not going to work, the name will be invalid after the block.

PavelVPV · 2025-04-23T08:33:21Z

subsys/bluetooth/mesh/pb_gatt_srv.c

+        size_t device_name_size = bt_gap_get_device_name(device_name, sizeof(device_name));
+	if (device_name_size < 0) {
+		LOG_DBG("Failed to get name (err %d)", device_name_size);
+		return -ENOMEM;
+	}
+
+	prov_sd_len = gatt_prov_adv_create(prov_sd, device_name, device_name_size);


This code shows how the old option (BT_LE_ADV_OPT_USE_NAME and BT_LE_ADV_OPT_FORCE_NAME_IN_AD) that auto-included device name in advs and scan responses was so useful even though it was not intended behavior (though could be converted to). Now users should include the name manually allocating extra space on stack, doing memcpy, etc while host could do this under the hood more optimal. We didn't gain anything from removing that feature, but created more problems for users really.

PavelVPV · 2025-04-23T08:42:51Z

subsys/bluetooth/mesh/gatt.h

+#if defined(CONFIG_BT_GAP_DEVICE_NAME_DYNAMIC)
+#define DEVICE_NAME_SIZE CONFIG_BT_GAP_DEVICE_NAME_DYNAMIC_MAX
+#else
+#define DEVICE_NAME_SIZE BT_GAP_DEVICE_NAME_MAX_SIZE


Since we change this API, do we really need split between dynamic and not-dynamic name lengths? Can't we use one as a Kconfig option limiting its range through range option?

github-actions · 2025-06-23T00:42:24Z

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

theob-pro added 3 commits April 22, 2025 13:57

theob-pro force-pushed the new-gap-dev-name-api branch 3 times, most recently from c2f125b to 3a284b2 Compare April 22, 2025 13:00

alxelax reviewed Apr 22, 2025

View reviewed changes

theob-pro force-pushed the new-gap-dev-name-api branch 2 times, most recently from 7039fe9 to e43e524 Compare April 22, 2025 14:43

theob-pro added 2 commits April 22, 2025 19:07

Bluetooth: Host: Use the new Bluetooth GAP Device Name APIs

4e88d00

Update the Host subsys functions using the GAP Device Name to use the new APIs. Signed-off-by: Théo Battrel <[email protected]>

Bluetooth: Mesh: Use the new Bluetooth GAP Device Name APIs

4afcc19

Update the Mesh functions using the GAP Device Name to use the new APIs. Signed-off-by: Théo Battrel <[email protected]>

theob-pro force-pushed the new-gap-dev-name-api branch 4 times, most recently from e995b1e to aa04032 Compare April 22, 2025 18:46

theob-pro added 3 commits April 22, 2025 21:03

Samples: Bluetooth: Update to use new Bluetooth GAP Device Name APIs

a24fd14

Update samples to use the new Bluetooth GAP Device name APIs added in a previous commit. Signed-off-by: Théo Battrel <[email protected]>

Tests: Bluetooth: Use the new Bluetooth GAP Device Name APIs

94f8eef

Update the Bluetooth Host tests that uses the GAP Device Name to use the new APIs. Signed-off-by: Théo Battrel <[email protected]>

theob-pro force-pushed the new-gap-dev-name-api branch from aa04032 to 67ef981 Compare April 22, 2025 19:03

PavelVPV reviewed Apr 23, 2025

View reviewed changes

github-actions bot added the Stale label Jun 23, 2025

theob-pro closed this Jun 30, 2025

		LOG_DBG("Device name is too big for the provided buffer.");
		return -ENOMEM;

		size_t name_size = bt_gap_get_device_name(name, sizeof(name));
		if (name_size < 0) {

	LOG_DBG("Failed to get name (err %d)", name_size);
	LOG_ERR("Failed to get name (err %d)", name_size);

Bluetooth: Host: Introduce new GAP Device Name APIs #88896

Bluetooth: Host: Introduce new GAP Device Name APIs #88896

Uh oh!

Conversation

theob-pro commented Apr 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PavelVPV left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jun 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants