[PATCH BlueZ v0 0/4] Fix losing Service Changed indication

[PATCH BlueZ v0 0/4] Fix losing Service Changed indication

[Claudio Takahasi]

Apply after:
"gatt: Remove reading Service Changed characteristic after connected"
sent from Andrzej Kaczmarek

This patch series adds Service Changed characteristic handle storage
allowing GATT service to manage Service Changed indication when the
connection is established.

The patch to remove the entry from "gatt" file will be sent in the next
patch series. It is unclear to me if we need to extend the device or
profile drive to indicate that the device is being removed/purged from
the system or if the expected implementation is to move to per-device
storage(TODO file). At the moment, it is not possible to detect in the
drivers remove callback if the device is being removed from the system
or if it bluetoothd is exiting.


Claudio Takahasi (4):
  gatt: Remove unneeded handle range check
  gatt: Ignore Service Changed if not bonded
  gatt: Add storing Service Changed handle
  gatt: Add reading stored Service Changed handle

 profiles/gatt/gas.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 83 insertions(+), 7 deletions(-)

-- 
1.7.12

[PATCH BlueZ v0 1/4] gatt: Remove unneeded handle range check

[Claudio Takahasi]

Since the Service Changed is a read-only control point, it not necessary
to compare the previous value read. This logic was added to compare the
result of the read operation with the received indications. Repeated
indications containing the same range is a valid procedure.
---
 profiles/gatt/gas.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/profiles/gatt/gas.c b/profiles/gatt/gas.c
index d189221..a828fcd 100644
--- a/profiles/gatt/gas.c
+++ b/profiles/gatt/gas.c
@@ -44,7 +44,6 @@ struct gas {
 	struct btd_device *device;
 	struct att_range gap;	/* GAP Primary service range */
 	struct att_range gatt;	/* GATT Primary service range */
-	struct att_range changed; /* Affected handle range */
 	GAttrib *attrib;
 	guint attioid;
 	guint changed_ind;
@@ -152,12 +151,6 @@ static void indication_cb(const uint8_t *pdu, uint16_t len, gpointer user_data)
 	olen = enc_confirmation(opdu, plen);
 	g_attrib_send(gas->attrib, 0, opdu[0], opdu, olen, NULL, NULL, NULL);
 
-	if (gas->changed.start == start && gas->changed.end == end)
-		return;
-
-	gas->changed.start = start;
-	gas->changed.end = end;
-
 	btd_device_gatt_set_service_changed(gas->device, start, end);
 }
 
-- 
1.7.12

[PATCH BlueZ v0 2/4] gatt: Ignore Service Changed if not bonded

[Claudio Takahasi]

This patch fix replying for Service Changed indication when the device
is not bonded. From Erratum 3951: Indications caused by changes to the
Service Changed Characteristic Value shall be considered lost if the
client has not enabled indications the Client Configuration
Characteristic Descriptor.
---
 profiles/gatt/gas.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/profiles/gatt/gas.c b/profiles/gatt/gas.c
index a828fcd..b42aa64 100644
--- a/profiles/gatt/gas.c
+++ b/profiles/gatt/gas.c
@@ -146,6 +146,11 @@ static void indication_cb(const uint8_t *pdu, uint16_t len, gpointer user_data)
 
 	DBG("Service Changed start: 0x%04X end: 0x%04X", start, end);
 
+	if (device_is_bonded(gas->device) == FALSE) {
+		DBG("Ignoring Service Changed: device is not bonded");
+		return;
+	}
+
 	/* Confirming indication received */
 	opdu = g_attrib_get_buffer(gas->attrib, &plen);
 	olen = enc_confirmation(opdu, plen);
-- 
1.7.12

[PATCH BlueZ v0 3/4] gatt: Add storing Service Changed handle

[Claudio Takahasi]

This patch stores the Service Changed control-point handle in order to
be able to listen for Service Changed indications when the connection
is established.
---
 profiles/gatt/gas.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/profiles/gatt/gas.c b/profiles/gatt/gas.c
index b42aa64..8b6a0bd 100644
--- a/profiles/gatt/gas.c
+++ b/profiles/gatt/gas.c
@@ -25,6 +25,9 @@
 #endif
 
 #include <stdbool.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
 
 #include <glib.h>
 #include <bluetooth/uuid.h>
@@ -37,6 +40,7 @@
 #include "btio.h"
 #include "gatt.h"
 #include "log.h"
+#include "textfile.h"
 #include "gas.h"
 
 /* Generic Attribute/Access Service */
@@ -70,6 +74,34 @@ static gint cmp_device(gconstpointer a, gconstpointer b)
 	return (gas->device == device ? 0 : -1);
 }
 
+static inline int create_filename(char *buf, size_t size,
+				const bdaddr_t *bdaddr, const char *name)
+{
+	char addr[18];
+
+	ba2str(bdaddr, addr);
+
+	return create_name(buf, size, STORAGEDIR, addr, name);
+}
+
+static int write_ctp_handle(bdaddr_t *sba, bdaddr_t *dba, uint8_t bdaddr_type,
+						uint16_t uuid, uint16_t handle)
+{
+	char filename[PATH_MAX + 1], addr[18], key[27], value[7];
+
+	create_filename(filename, PATH_MAX, sba, "gatt");
+
+	create_file(filename, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+
+	ba2str(dba, addr);
+
+	snprintf(key, sizeof(key), "%17s#%hhu#0x%4.4x", addr, bdaddr_type,
+									uuid);
+	snprintf(value, sizeof(value), "0x%4.4x", handle);
+
+	return textfile_put(filename, key, value);
+}
+
 static void gap_appearance_cb(guint8 status, const guint8 *pdu, guint16 plen,
 							gpointer user_data)
 {
@@ -107,6 +139,10 @@ done:
 static void ccc_written_cb(guint8 status, const guint8 *pdu, guint16 plen,
 							gpointer user_data)
 {
+	struct gas *gas = user_data;
+	bdaddr_t sba, dba;
+	uint8_t bdaddr_type;
+
 	if (status) {
 		error("Write Service Changed CCC failed: %s",
 						att_ecode2str(status));
@@ -114,6 +150,12 @@ static void ccc_written_cb(guint8 status, const guint8 *pdu, guint16 plen,
 	}
 
 	DBG("Service Changed indications enabled");
+
+	adapter_get_address(device_get_adapter(gas->device), &sba);
+	device_get_address(gas->device, &dba, &bdaddr_type);
+
+	write_ctp_handle(&sba, &dba, bdaddr_type, GATT_CHARAC_SERVICE_CHANGED,
+							gas->changed_handle);
 }
 
 static void write_ccc(GAttrib *attrib, uint16_t handle, gpointer user_data)
-- 
1.7.12

[PATCH BlueZ v0 4/4] gatt: Add reading stored Service Changed handle

[Claudio Takahasi]

This patch avoids discovering Service Changed handle when the ATT
connection is established and there is a previous stored handle.
---
 profiles/gatt/gas.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/profiles/gatt/gas.c b/profiles/gatt/gas.c
index 8b6a0bd..da6d555 100644
--- a/profiles/gatt/gas.c
+++ b/profiles/gatt/gas.c
@@ -25,9 +25,11 @@
 #endif
 
 #include <stdbool.h>
+#include <stdlib.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
+#include <errno.h>
 
 #include <glib.h>
 #include <bluetooth/uuid.h>
@@ -102,6 +104,32 @@ static int write_ctp_handle(bdaddr_t *sba, bdaddr_t *dba, uint8_t bdaddr_type,
 	return textfile_put(filename, key, value);
 }
 
+static int read_ctp_handle(bdaddr_t *sba, bdaddr_t *dba, uint8_t bdaddr_type,
+						uint16_t uuid, uint16_t *value)
+{
+	char filename[PATH_MAX + 1], addr[18], key[27];
+	char *str;
+
+	create_filename(filename, PATH_MAX, sba, "gatt");
+
+	ba2str(dba, addr);
+	snprintf(key, sizeof(key), "%17s#%hhu#0x%04x", addr, bdaddr_type,
+									uuid);
+
+	str = textfile_get(filename, key);
+	if (str == NULL)
+		return -errno;
+
+	if (sscanf(str, "%hx", value) != 1) {
+		free(str);
+		return -ENOENT;
+	}
+
+	free(str);
+
+	return 0;
+}
+
 static void gap_appearance_cb(guint8 status, const guint8 *pdu, guint16 plen,
 							gpointer user_data)
 {
@@ -349,6 +377,8 @@ int gas_register(struct btd_device *device, struct att_range *gap,
 						struct att_range *gatt)
 {
 	struct gas *gas;
+	bdaddr_t sba, dba;
+	uint8_t bdaddr_type;
 
 	gas = g_new0(struct gas, 1);
 	gas->gap.start = gap->start;
@@ -364,6 +394,12 @@ int gas_register(struct btd_device *device, struct att_range *gap,
 						attio_connected_cb,
 						attio_disconnected_cb, gas);
 
+	adapter_get_address(device_get_adapter(gas->device), &sba);
+	device_get_address(gas->device, &dba, &bdaddr_type);
+
+	read_ctp_handle(&sba, &dba, bdaddr_type, GATT_CHARAC_SERVICE_CHANGED,
+							&gas->changed_handle);
+
 	return 0;
 }
 
-- 
1.7.12

Re: [PATCH BlueZ v0 0/4] Fix losing Service Changed indication

[Johan Hedberg]

Hi Claudio,

On Fri, Sep 28, 2012, Claudio Takahasi wrote:
> Apply after:
> "gatt: Remove reading Service Changed characteristic after connected"
> sent from Andrzej Kaczmarek
> 
> This patch series adds Service Changed characteristic handle storage
> allowing GATT service to manage Service Changed indication when the
> connection is established.
> 
> The patch to remove the entry from "gatt" file will be sent in the next
> patch series. It is unclear to me if we need to extend the device or
> profile drive to indicate that the device is being removed/purged from
> the system or if the expected implementation is to move to per-device
> storage(TODO file). At the moment, it is not possible to detect in the
> drivers remove callback if the device is being removed from the system
> or if it bluetoothd is exiting.
> 
> 
> Claudio Takahasi (4):
>   gatt: Remove unneeded handle range check
>   gatt: Ignore Service Changed if not bonded
>   gatt: Add storing Service Changed handle
>   gatt: Add reading stored Service Changed handle
> 
>  profiles/gatt/gas.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 83 insertions(+), 7 deletions(-)

All patches in this set have been applied. Thanks.

Johan