bt: services: gap_default: Extract GAP service from gatt implementation #94235

rkel · 2025-08-07T15:58:36Z

Remove Generic Access Service implementation from gatt files and move it into separate file in services directory.
The proposed name is just gap_default as it is default implementation that may be switched off by the user in purpose of providing the GAP implementation directly in the application.

In the end it serves the same purpose that #93946 but gives the user total freedom of implementation when needed in the same time being 100% compatible with the current solution.

I was testing it manually and now I can enable and disable default GAP implementation.

The main reason of this PR is to discuss if we wish to go into this solution instead of #93946.

To be done:
[v] - GAP service validation during bt_enable.
[ ] - review all configurations that need to be moved to Kconfig.gap_default
[ ] - sample with GAP reimplementation on App side
[ ] - test of the sample (BabbleSim?)
[ ] - update documentation

alwa-nordic · 2025-08-08T09:50:10Z

subsys/bluetooth/services/Kconfig.gap_default

+# Copyright (c) 2025 Koppel Electronic
+# SPDX-License-Identifier: Apache-2.0
+
+config BT_GAP_DEFAULT


This config name is too generic. The GAP Service is only one component of GAP. I suggest something like BT_GAP_SVC_DEFAULT_IMPL.

alwa-nordic · 2025-08-08T09:54:08Z

subsys/bluetooth/services/Kconfig.gap_default

+	help
+	  Enable the default GATT Generic Access Service.
+	  Enabled by default provides basic GAP functionality.
+	  If disabled user can provide its own implementation in the code.


I think we should say "must provide" and maybe "otherwise the system is not Bluetooth qualification compliant" or similar.

I started it from "must" but then I thought that not everybody cares about being qualification compliant. But the wording like:

If disabled, it is the application responsibility to provide its own implementation of GAP service, otherwise the system is not Bluetooth qualification compliant.

Probably explains everything and not "forces" anybody to do anything - freedom :)

alwa-nordic · 2025-08-08T09:55:43Z

subsys/bluetooth/services/gap_svc_default.c

+				 strlen(name));
+}
+
+#if defined(CONFIG_BT_DEVICE_NAME_GATT_WRITABLE)


The configs for the GAP Service should be moved Kconfig.gap_default.

Yes. I am just wondering if it Kconfig.gap_default is good place for it. Maybe we should have Kconfig.gap - with all its configurations and then Kconfig.gap_default.

alwa-nordic · 2025-08-08T10:04:45Z

subsys/bluetooth/services/gap_default.c

+/* This checks if the range entered is valid */
+BUILD_ASSERT(!(CONFIG_BT_PERIPHERAL_PREF_MIN_INT > 3200 &&
+	     CONFIG_BT_PERIPHERAL_PREF_MIN_INT < 0xffff));
+BUILD_ASSERT(!(CONFIG_BT_PERIPHERAL_PREF_MAX_INT > 3200 &&
+	     CONFIG_BT_PERIPHERAL_PREF_MAX_INT < 0xffff));
+BUILD_ASSERT(!(CONFIG_BT_PERIPHERAL_PREF_TIMEOUT > 3200 &&
+	     CONFIG_BT_PERIPHERAL_PREF_TIMEOUT < 0xffff));
+BUILD_ASSERT((CONFIG_BT_PERIPHERAL_PREF_MIN_INT == 0xffff) ||
+	     (CONFIG_BT_PERIPHERAL_PREF_MIN_INT <=
+	     CONFIG_BT_PERIPHERAL_PREF_MAX_INT));
+BUILD_ASSERT((CONFIG_BT_PERIPHERAL_PREF_TIMEOUT * 4U) >
+	     ((1U + CONFIG_BT_PERIPHERAL_PREF_LATENCY) *
+	      CONFIG_BT_PERIPHERAL_PREF_MAX_INT));


I'm not sure whether this is not GAP Service specific or even GATT specific. They are used in conn.c. But on the other hand they are defined under "ATT and GATT options". There seems to be a mistake here one way or the other. @jhedberg, should we move these configs out of "ATT and GATT" into a generic GAP space?

It's clearly not right to have these assertions here while the configs are used in conn.c.

Yes, seems like they should move to some GAP-specific place. Btw, some of those build asserts should probably be converted into range directives in Kconfig.

@jhedberg @alwa-nordic - should I just leave this assertions in the gatt.c for the purpose of this PR?

I will do it right now - it looks little odd as now we have only the parameter checking in the gatt.c. But I get it that it is better not to move it to the module that may be disabled.

Copilot

Pull Request Overview

This PR extracts the Generic Access Service (GAP) implementation from the GATT core files into a separate, configurable service module. This allows users to disable the default GAP implementation and provide their own custom implementation when needed.

Key changes:

Creates a new configurable GAP service module in the services directory
Removes GAP implementation from the core GATT files
Maintains 100% compatibility with existing functionality through default enablement

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
subsys/bluetooth/services/gap_default.c	New file containing the extracted GAP service implementation
subsys/bluetooth/services/Kconfig.gap_default	Configuration file for the default GAP service
subsys/bluetooth/services/Kconfig	Includes the new GAP configuration
subsys/bluetooth/services/CMakeLists.txt	Builds the GAP service conditionally
subsys/bluetooth/host/gatt.c	Removes the GAP service implementation
include/zephyr/bluetooth/gap.h	Adds service name definition for consistency

Comments suppressed due to low confidence (1)

Copilot · 2025-08-08T10:06:02Z

subsys/bluetooth/services/gap_svc_default.c

+
+#if defined(CONFIG_BT_DEVICE_APPEARANCE_GATT_WRITABLE)
+	#define GAP_APPEARANCE_PROPS (BT_GATT_CHRC_READ | BT_GATT_CHRC_WRITE)
+#if defined(CONFIG_DEVICE_APPEARANCE_GATT_WRITABLE_AUTHEN)


Configuration macro name is inconsistent with the pattern used elsewhere. Should be CONFIG_BT_DEVICE_APPEARANCE_GATT_WRITABLE_AUTHEN to match the naming convention.

Suggested change

#if defined(CONFIG_DEVICE_APPEARANCE_GATT_WRITABLE_AUTHEN)

#if defined(CONFIG_BT_DEVICE_APPEARANCE_GATT_WRITABLE_AUTHEN)

Copilot · 2025-08-08T10:06:03Z

subsys/bluetooth/services/gap_svc_default.c

+	BT_GATT_CHARACTERISTIC(BT_UUID_GAP_DEVICE_NAME,
+			       BT_GATT_CHRC_READ | BT_GATT_CHRC_WRITE,
+			       BT_GATT_PERM_READ |
+#if defined(CONFIG_DEVICE_NAME_GATT_WRITABLE_AUTHEN)


Configuration macro name is inconsistent with the pattern used elsewhere. Should be CONFIG_BT_DEVICE_NAME_GATT_WRITABLE_AUTHEN to match the naming convention.

Suggested change

#if defined(CONFIG_DEVICE_NAME_GATT_WRITABLE_AUTHEN)

#if defined(CONFIG_BT_DEVICE_NAME_GATT_WRITABLE_AUTHEN)

Copilot · 2025-08-08T10:06:03Z

subsys/bluetooth/services/gap_svc_default.c

+			       BT_GATT_PERM_READ |
+#if defined(CONFIG_DEVICE_NAME_GATT_WRITABLE_AUTHEN)
+			       BT_GATT_PERM_WRITE_AUTHEN,
+#elif defined(CONFIG_DEVICE_NAME_GATT_WRITABLE_ENCRYPT)


Configuration macro name is inconsistent with the pattern used elsewhere. Should be CONFIG_BT_DEVICE_NAME_GATT_WRITABLE_ENCRYPT to match the naming convention.

Suggested change

#elif defined(CONFIG_DEVICE_NAME_GATT_WRITABLE_ENCRYPT)

#elif defined(CONFIG_BT_DEVICE_NAME_GATT_WRITABLE_ENCRYPT)

rkel · 2025-08-08T11:00:09Z

Can we please discuss now mainly if this is the way we wish to go? If yes - I will work on it little more before changing it into valid PR. Now it is rather proof of concept to discuss this vs #93946. I know we can implement both - but to be honest, any of this two just solves my issues. This one I think gives more freedom to the user, but requires (a little) more work from the application perspective if the user wants to overwrite default functionality.
I am saying "a little more" as you can always just copy default implementation, add any checking and application notification and call it a day.

jhedberg · 2025-08-08T18:36:42Z

Can we please discuss now mainly if this is the way we wish to go?

To me it sounds like a reasonable approach at least, i.e. have a default implementation which can be completely overridden by the app if it wants to.

sonarqubecloud · 2025-08-09T21:37:01Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Thalley · 2025-08-11T11:45:33Z

I generally like and approve of the idea that Zephyr provides a GAP service that works out of the box, similar to DIS, BAS, etc., and where users can choose to define and implement their own (similar to the other services).

2 things we need to consider for this approach is

Unlike (most) other services, GAP service shall always be there on a Bluetooth devices (the only mandatory service IIRC).
Unlike (most) other services, there shall only ever be a single GAP service on a device.

Both of the above can be verified at runtime, where we in bt_enable can verify that the service has been registered, as well as verifying in bt_gatt_service_register that no other service with BT_UUID_GAP has been registered.

jhedberg · 2025-08-14T10:39:29Z

Discussed in the Bluetooth WG meeting. Consensus was to move the GAP service to subsys/bluetooth/services

rkel · 2025-10-01T10:56:01Z

Just a rebase before any changes.

jhedberg · 2025-10-02T09:21:42Z

@rkel please be consistent with the commit subject prefix. Take a look at e.g. git log subsys/bluetooth and just use the same style as the majority of existing commits.

rkel · 2025-10-02T13:10:32Z

@Thalley I had added some verification if there is exactly one GAP service in GATT database.

sonarqubecloud · 2025-10-03T06:37:33Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Thalley · 2025-10-03T08:39:17Z

@rkel Please press the "Ready for review" button once you think this is ready for reviews again :) I (and possibly others), will generally wait with reviewing PRs until they leave draft

Copilot

Pull Request Overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-03T08:39:54Z

subsys/bluetooth/services/Kconfig.gap_svc_default

+	prompt "Security requirements"
+	default DEVICE_APPEARANCE_GATT_WRITABLE_AUTHEN
+	help
+	  Select security requirements for writing device name by remote GATT


The help text incorrectly refers to 'device name' but this choice is for device appearance security requirements. Should be 'Select security requirements for writing device appearance by remote GATT clients.'

Suggested change

Select security requirements for writing device name by remote GATT

Select security requirements for writing device appearance by remote GATT

Copilot · 2025-10-03T08:39:54Z

subsys/bluetooth/host/gatt_gap_svc_validate.c

@@ -0,0 +1,55 @@
+/* gatt_gap_validate.c - GAP service inside GATT validation functions */


The filename in the comment is incorrect. Should be 'gatt_gap_svc_validate.c' to match the actual filename.

Suggested change

/* gatt_gap_validate.c - GAP service inside GATT validation functions */

/* gatt_gap_svc_validate.c - GAP service inside GATT validation functions */

Remove Generic Access Service implementation from gatt files and move it into separate file in services directory. The proposed name is just gap_default as it is default implementation that may be switched off by the user in purpose of providing the GAP implementation directly in the application. Signed-off-by: Radosław Koppel <[email protected]>

…NULL Set the unused argument of the name characteristic to NULL. This removes the reference from GAP service to the hci_core. Signed-off-by: Radosław Koppel <[email protected]>

Implement the verification if there is exactly one GAP service in GATT database. The verification is performed as soon as the GATT database is available and forbids to enable the Bluetooth if the configuration is not compatible with the Bluetooth Specification. The verification may be disabled by the user for better performance or deliberate implementation that is not compliant with the specification. Signed-off-by: Radosław Koppel <[email protected]>

Disable the GATT GAP service validation for the test. Signed-off-by: Radosław Koppel <[email protected]>

WIP Signed-off-by: Radosław Koppel <[email protected]>

sonarqubecloud · 2025-11-29T00:21:00Z

Quality Gate passed

Issues
5 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
7.4% Duplication on New Code

See analysis details on SonarQube Cloud

rkel mentioned this pull request Aug 7, 2025

bt: host: gatt: Implement GATT GAP callbacks #93946

Closed

rkel requested review from Thalley, alwa-nordic and cvinayak August 7, 2025 16:03

alwa-nordic reviewed Aug 8, 2025

View reviewed changes

alwa-nordic requested a review from Copilot August 8, 2025 10:05

Copilot AI reviewed Aug 8, 2025

View reviewed changes

rkel force-pushed the bt_gap_default_service branch 2 times, most recently from ace01bc to 38b4d8a Compare August 9, 2025 21:29

alwa-nordic added the Bluetooth Review Discussion in the Bluetooth WG meeting required label Aug 14, 2025

jhedberg added area: Bluetooth and removed Bluetooth Review Discussion in the Bluetooth WG meeting required labels Aug 14, 2025

rkel force-pushed the bt_gap_default_service branch from 38b4d8a to 3214e90 Compare October 1, 2025 10:55

rkel force-pushed the bt_gap_default_service branch 3 times, most recently from 9689adc to 5c141cb Compare October 2, 2025 09:14

rkel force-pushed the bt_gap_default_service branch 2 times, most recently from 47420b3 to b34b4eb Compare October 2, 2025 11:05

rkel force-pushed the bt_gap_default_service branch from cb3839c to b511a1e Compare October 3, 2025 05:56

rkel force-pushed the bt_gap_default_service branch from b511a1e to 0305318 Compare October 3, 2025 06:12

Thalley requested a review from Copilot October 3, 2025 08:39

Copilot AI reviewed Oct 3, 2025

View reviewed changes

rkel force-pushed the bt_gap_default_service branch 2 times, most recently from 2aa9db0 to 232bca6 Compare November 28, 2025 22:20

rkel added 4 commits November 28, 2025 23:43

Bluetooth: gap_svc_default: Set user_data for name characteristic to …

ccf0665

…NULL Set the unused argument of the name characteristic to NULL. This removes the reference from GAP service to the hci_core. Signed-off-by: Radosław Koppel <[email protected]>

tests: bluetooth: host: conn: Disable GATT GAP svc validation

8042113

Disable the GATT GAP service validation for the test. Signed-off-by: Radosław Koppel <[email protected]>

rkel force-pushed the bt_gap_default_service branch from 232bca6 to 8042113 Compare November 28, 2025 22:43

tests: gap local reimplementation

8e6793a

WIP Signed-off-by: Radosław Koppel <[email protected]>

	#if defined(CONFIG_DEVICE_APPEARANCE_GATT_WRITABLE_AUTHEN)
	#if defined(CONFIG_BT_DEVICE_APPEARANCE_GATT_WRITABLE_AUTHEN)

	#if defined(CONFIG_DEVICE_NAME_GATT_WRITABLE_AUTHEN)
	#if defined(CONFIG_BT_DEVICE_NAME_GATT_WRITABLE_AUTHEN)

	#elif defined(CONFIG_DEVICE_NAME_GATT_WRITABLE_ENCRYPT)
	#elif defined(CONFIG_BT_DEVICE_NAME_GATT_WRITABLE_ENCRYPT)

	Select security requirements for writing device name by remote GATT
	Select security requirements for writing device appearance by remote GATT

		@@ -0,0 +1,55 @@
		/* gatt_gap_validate.c - GAP service inside GATT validation functions */

	/* gatt_gap_validate.c - GAP service inside GATT validation functions */
	/* gatt_gap_svc_validate.c - GAP service inside GATT validation functions */

bt: services: gap_default: Extract GAP service from gatt implementation #94235

Are you sure you want to change the base?

bt: services: gap_default: Extract GAP service from gatt implementation #94235

Conversation

rkel commented Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rkel Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

rkel commented Aug 8, 2025

Uh oh!

jhedberg commented Aug 8, 2025

Uh oh!

sonarqubecloud bot commented Aug 9, 2025

Quality Gate passed

Uh oh!

Thalley commented Aug 11, 2025

Uh oh!

jhedberg commented Aug 14, 2025

Uh oh!

rkel commented Oct 1, 2025

Uh oh!

jhedberg commented Oct 2, 2025

Uh oh!

rkel commented Oct 2, 2025

Uh oh!

sonarqubecloud bot commented Oct 3, 2025

Quality Gate passed

Uh oh!

Thalley commented Oct 3, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Nov 29, 2025

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

rkel commented Aug 7, 2025 •

edited

Loading

rkel Aug 8, 2025 •

edited

Loading