[PATCH bluez 0/2] shared/gatt-client: Fix init when Service Changed lacks Indicate

[PATCH bluez 0/2] shared/gatt-client: Fix init when Service Changed lacks Indicate

[Marian Poltak]

Some devices, e.g. the Aqara Presence Sensor P2 in Matter commissioning
mode, declare the Service Changed characteristic as Read | Notify only.
The client initialization then fails because indications are requested
explicitly, so no services are exported although ServicesResolved is
set.

Patch 1 requests indications only when the characteristic supports
them. Patch 2 adds unit tests for both cases.

---
Marian Poltak (2):
      shared/gatt-client: Fix init when Service Changed lacks Indicate
      unit/test-gatt: Add Service Changed registration tests

 src/shared/gatt-client.c | 15 ++++++++++++++-
 unit/test-gatt.c         | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 1 deletion(-)
---
base-commit: 9659adb6819d7e0284d0d2ba226a3894423d8db4
change-id: 20261112-svc-chngd-notify-only-f1d1d99d3f34

Best regards,
-- 
Marian Poltak <marianpoltak@gmail.com>

[PATCH bluez 0/2] shared/gatt-client: Fix init when Service Changed lacks Indicate

[Marian Poltak via B4 Relay]

Some devices, e.g. the Aqara Presence Sensor P2 in Matter commissioning
mode, declare the Service Changed characteristic as Read | Notify only.
The client initialization then fails because indications are requested
explicitly, so no services are exported although ServicesResolved is
set.

Patch 1 requests indications only when the characteristic supports
them. Patch 2 adds unit tests for both cases.

---
Marian Poltak (2):
      shared/gatt-client: Fix init when Service Changed lacks Indicate
      unit/test-gatt: Add Service Changed registration tests

 src/shared/gatt-client.c | 15 ++++++++++++++-
 unit/test-gatt.c         | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 1 deletion(-)
---
base-commit: 9659adb6819d7e0284d0d2ba226a3894423d8db4
change-id: 20261112-svc-chngd-notify-only-f1d1d99d3f34

Best regards,
-- 
Marian Poltak <marianpoltak@gmail.com>

[PATCH bluez 1/2] shared/gatt-client: Fix init when Service Changed lacks Indicate

[Marian Poltak]

register_service_changed() requests indications explicitly and
register_notify() rejects this when the characteristic does not declare
the Indicate property. Some devices, e.g. the Aqara Presence Sensor P2
in Matter commissioning mode, declare Service Changed as Read | Notify
only, so the client initialization fails and no services are exported,
although ServicesResolved is set.

Request indications only when they are supported, and otherwise pick
the CCC value from the properties as before.

Fixes: 52fdca35b8e7 ("shared/gatt-client: Allow registering notify with explicit CCC value")
Assisted-by: Claude:claude-opus-5-5
---
 src/shared/gatt-client.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index 7bebc50dc..ea9f7167c 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -1887,6 +1887,8 @@ static bool register_service_changed(struct bt_gatt_client *client)
 {
 	bt_uuid_t uuid;
 	struct gatt_db_attribute *attr = NULL;
+	uint8_t properties;
+	uint16_t ccc_value = 0;
 
 	bt_uuid16_create(&uuid, SVC_CHNGD_UUID);
 
@@ -1898,6 +1900,17 @@ static bool register_service_changed(struct bt_gatt_client *client)
 	if (!attr)
 		return true;
 
+	/*
+	 * "Service Changed" shall be indicated, but some devices only
+	 * declare the notify property. Request indications only when they
+	 * are supported, otherwise let register_notify() pick the CCC value
+	 * from the properties.
+	 */
+	if (gatt_db_attribute_get_char_data(attr, NULL, NULL, &properties,
+							NULL, NULL) &&
+				(properties & BT_GATT_CHRC_PROP_INDICATE))
+		ccc_value = 0x0002;
+
 	/*
 	 * Register an indication handler for the "Service Changed"
 	 * characteristic and report ready only if the handler is registered
@@ -1905,7 +1918,7 @@ static bool register_service_changed(struct bt_gatt_client *client)
 	 */
 	client->svc_chngd_ind_id = register_notify(client,
 					gatt_db_attribute_get_handle(attr),
-					0x0002,
+					ccc_value,
 					service_changed_register_cb,
 					service_changed_cb,
 					client, NULL);

-- 
2.56.0

[PATCH bluez 1/2] shared/gatt-client: Fix init when Service Changed lacks Indicate

[Marian Poltak via B4 Relay]

From: Marian Poltak <marianpoltak@gmail.com>

register_service_changed() requests indications explicitly and
register_notify() rejects this when the characteristic does not declare
the Indicate property. Some devices, e.g. the Aqara Presence Sensor P2
in Matter commissioning mode, declare Service Changed as Read | Notify
only, so the client initialization fails and no services are exported,
although ServicesResolved is set.

Request indications only when they are supported, and otherwise pick
the CCC value from the properties as before.

Fixes: 52fdca35b8e7 ("shared/gatt-client: Allow registering notify with explicit CCC value")
Assisted-by: Claude:claude-opus-5-5
---
 src/shared/gatt-client.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index 7bebc50dc..ea9f7167c 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -1887,6 +1887,8 @@ static bool register_service_changed(struct bt_gatt_client *client)
 {
 	bt_uuid_t uuid;
 	struct gatt_db_attribute *attr = NULL;
+	uint8_t properties;
+	uint16_t ccc_value = 0;
 
 	bt_uuid16_create(&uuid, SVC_CHNGD_UUID);
 
@@ -1898,6 +1900,17 @@ static bool register_service_changed(struct bt_gatt_client *client)
 	if (!attr)
 		return true;
 
+	/*
+	 * "Service Changed" shall be indicated, but some devices only
+	 * declare the notify property. Request indications only when they
+	 * are supported, otherwise let register_notify() pick the CCC value
+	 * from the properties.
+	 */
+	if (gatt_db_attribute_get_char_data(attr, NULL, NULL, &properties,
+							NULL, NULL) &&
+				(properties & BT_GATT_CHRC_PROP_INDICATE))
+		ccc_value = 0x0002;
+
 	/*
 	 * Register an indication handler for the "Service Changed"
 	 * characteristic and report ready only if the handler is registered
@@ -1905,7 +1918,7 @@ static bool register_service_changed(struct bt_gatt_client *client)
 	 */
 	client->svc_chngd_ind_id = register_notify(client,
 					gatt_db_attribute_get_handle(attr),
-					0x0002,
+					ccc_value,
 					service_changed_register_cb,
 					service_changed_cb,
 					client, NULL);

-- 
2.56.0

[PATCH bluez 2/2] unit/test-gatt: Add Service Changed registration tests

[Marian Poltak]

Add client tests enabling Service Changed when the characteristic
declares Indicate, and when it declares Notify only.

Assisted-by: Claude:claude-opus-5-5
---
 unit/test-gatt.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/unit/test-gatt.c b/unit/test-gatt.c
index bf6074107..45a257ad1 100644
--- a/unit/test-gatt.c
+++ b/unit/test-gatt.c
@@ -303,6 +303,25 @@ struct context {
 		DESCRIPTOR_DISC_SMALL_DB
 
 
+/*
+ * GATT service (0x0001-0x0004) with a Service Changed characteristic
+ * declaring the given properties, value handle 0x0003 and a CCC at 0x0004.
+ */
+#define SVC_CHNGD_DISCOVERY_PDUS(props)					\
+		raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28),	\
+		raw_pdu(0x11, 0x06, 0x01, 0x00, 0x04, 0x00, 0x01, 0x18),\
+		raw_pdu(0x10, 0x05, 0x00, 0xff, 0xff, 0x00, 0x28),	\
+		raw_pdu(0x01, 0x10, 0x05, 0x00, 0x0a),			\
+		raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x01, 0x28),	\
+		raw_pdu(0x01, 0x10, 0x01, 0x00, 0x0a),			\
+		raw_pdu(0x08, 0x01, 0x00, 0x04, 0x00, 0x02, 0x28),	\
+		raw_pdu(0x01, 0x08, 0x01, 0x00, 0x0a),			\
+		raw_pdu(0x08, 0x01, 0x00, 0x04, 0x00, 0x03, 0x28),	\
+		raw_pdu(0x09, 0x07, 0x02, 0x00, props, 0x03, 0x00, 0x05,\
+				0x2a),					\
+		raw_pdu(0x08, 0x03, 0x00, 0x04, 0x00, 0x03, 0x28),	\
+		raw_pdu(0x01, 0x08, 0x03, 0x00, 0x0a)
+
 #define SERVER_MTU_EXCHANGE_PDU raw_pdu(0x02, 0x17, 0x00)
 
 static bt_uuid_t uuid_16 = {
@@ -3736,6 +3755,26 @@ int main(int argc, char *argv[])
 			raw_pdu(),
 			raw_pdu(0x1B, 0x03, 0x00, 0x01, 0x02, 0x03));
 
+	/* Service Changed with Indicate: indications are enabled */
+	define_test_client("/robustness/svc-chngd-indicate", test_client,
+			NULL, NULL,
+			CLIENT_INIT_PDUS,
+			SVC_CHNGD_DISCOVERY_PDUS(0x22),
+			raw_pdu(0x12, 0x04, 0x00, 0x02, 0x00),
+			raw_pdu(0x13));
+
+	/*
+	 * Service Changed declaring Notify only (not compliant, seen on
+	 * real devices): notifications are enabled instead of failing the
+	 * client initialization.
+	 */
+	define_test_client("/robustness/svc-chngd-notify-only", test_client,
+			NULL, NULL,
+			CLIENT_INIT_PDUS,
+			SVC_CHNGD_DISCOVERY_PDUS(0x12),
+			raw_pdu(0x12, 0x04, 0x00, 0x01, 0x00),
+			raw_pdu(0x13));
+
 	define_test_server("/TP/GAN/SR/BV-01-C", test_server, ts_small_db,
 			&test_notification_server_1,
 			raw_pdu(0x03, 0x00, 0x02),

-- 
2.56.0

[PATCH bluez 2/2] unit/test-gatt: Add Service Changed registration tests

[Marian Poltak via B4 Relay]

From: Marian Poltak <marianpoltak@gmail.com>

Add client tests enabling Service Changed when the characteristic
declares Indicate, and when it declares Notify only.

Assisted-by: Claude:claude-opus-5-5
---
 unit/test-gatt.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/unit/test-gatt.c b/unit/test-gatt.c
index bf6074107..45a257ad1 100644
--- a/unit/test-gatt.c
+++ b/unit/test-gatt.c
@@ -303,6 +303,25 @@ struct context {
 		DESCRIPTOR_DISC_SMALL_DB
 
 
+/*
+ * GATT service (0x0001-0x0004) with a Service Changed characteristic
+ * declaring the given properties, value handle 0x0003 and a CCC at 0x0004.
+ */
+#define SVC_CHNGD_DISCOVERY_PDUS(props)					\
+		raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28),	\
+		raw_pdu(0x11, 0x06, 0x01, 0x00, 0x04, 0x00, 0x01, 0x18),\
+		raw_pdu(0x10, 0x05, 0x00, 0xff, 0xff, 0x00, 0x28),	\
+		raw_pdu(0x01, 0x10, 0x05, 0x00, 0x0a),			\
+		raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x01, 0x28),	\
+		raw_pdu(0x01, 0x10, 0x01, 0x00, 0x0a),			\
+		raw_pdu(0x08, 0x01, 0x00, 0x04, 0x00, 0x02, 0x28),	\
+		raw_pdu(0x01, 0x08, 0x01, 0x00, 0x0a),			\
+		raw_pdu(0x08, 0x01, 0x00, 0x04, 0x00, 0x03, 0x28),	\
+		raw_pdu(0x09, 0x07, 0x02, 0x00, props, 0x03, 0x00, 0x05,\
+				0x2a),					\
+		raw_pdu(0x08, 0x03, 0x00, 0x04, 0x00, 0x03, 0x28),	\
+		raw_pdu(0x01, 0x08, 0x03, 0x00, 0x0a)
+
 #define SERVER_MTU_EXCHANGE_PDU raw_pdu(0x02, 0x17, 0x00)
 
 static bt_uuid_t uuid_16 = {
@@ -3736,6 +3755,26 @@ int main(int argc, char *argv[])
 			raw_pdu(),
 			raw_pdu(0x1B, 0x03, 0x00, 0x01, 0x02, 0x03));
 
+	/* Service Changed with Indicate: indications are enabled */
+	define_test_client("/robustness/svc-chngd-indicate", test_client,
+			NULL, NULL,
+			CLIENT_INIT_PDUS,
+			SVC_CHNGD_DISCOVERY_PDUS(0x22),
+			raw_pdu(0x12, 0x04, 0x00, 0x02, 0x00),
+			raw_pdu(0x13));
+
+	/*
+	 * Service Changed declaring Notify only (not compliant, seen on
+	 * real devices): notifications are enabled instead of failing the
+	 * client initialization.
+	 */
+	define_test_client("/robustness/svc-chngd-notify-only", test_client,
+			NULL, NULL,
+			CLIENT_INIT_PDUS,
+			SVC_CHNGD_DISCOVERY_PDUS(0x12),
+			raw_pdu(0x12, 0x04, 0x00, 0x01, 0x00),
+			raw_pdu(0x13));
+
 	define_test_server("/TP/GAN/SR/BV-01-C", test_server, ts_small_db,
 			&test_notification_server_1,
 			raw_pdu(0x03, 0x00, 0x02),

-- 
2.56.0

RE: shared/gatt-client: Fix init when Service Changed lacks Indicate

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1023 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/series/1209830/

---Test result---

Test Summary:
CheckPatch                    PASS      1.06 seconds
GitLint                       PASS      0.70 seconds
BuildEll                      PASS      21.45 seconds
BluezMake                     PASS      334.11 seconds
MakeCheck                     PASS      14.71 seconds
MakeDistcheck                 PASS      158.82 seconds
CheckValgrind                 PASS      370.80 seconds
CheckSmatch                   PASS      333.70 seconds
bluezmakeextell               PASS      100.48 seconds
TestFunctional                PASS      1199.57 seconds
IncrementalBuild              PASS      347.86 seconds
ScanBuild                     PASS      1188.30 seconds



https://github.com/bluez/bluez/pull/2817

---
Regards,
Linux Bluetooth