[PATCH BlueZ v2 0/5] core: gatt: Support enabling notifications

[PATCH BlueZ v2 0/5] core: gatt: Support enabling notifications

[Arman Uguray]

*v2: Cancel ongoing CCC writes in bt_gatt_client_free.

*v1: Make bt_gatt_register_notify cancellable via bt_gatt_unregister_notify.
     notify_client_free now unregisters its notify id, which will cancel any
     pending CCC write, as well as unregister a registered notify callback.

This patch sets brings support for enabling notifications while in GATT
client-role. The changes that are introduced are:

  1. Implemented the StartNotify and StopNotify methods of the
     GattCharacteristic1 interface. This are internally tied to
     bt_gatt_client_register_notify and bt_gatt_client_unregister_notify.
     These also manage notification sessions on a per dbus sender basis.

  2. The exported GATT API objects are not removed in the event of a disconnect,
     if the devices are bonded. All notification sessions are also persisted and
     automatically re-enabled on reconnection. Also, adding new notification
     sessions via StartNotify is allowed during disconnects.

Arman Uguray (5):
  shared/gatt: Make register_notify cancellable
  core: gatt: Implement GattCharacteristic1.StartNotify
  core: gatt: Implement GattCharacteristic1.StopNotify
  core: device: Don't check ready in service_removed
  core: gatt: Keep objects over disconnects

 src/device.c             |  22 ++-
 src/gatt-client.c        | 435 +++++++++++++++++++++++++++++++++++++++++++++--
 src/shared/gatt-client.c | 129 ++++++++------
 src/shared/gatt-client.h |   7 +-
 tools/btgatt-client.c    |  17 +-
 5 files changed, 530 insertions(+), 80 deletions(-)

-- 
2.2.0.rc0.207.ga3a616c

[PATCH BlueZ v2 1/5] shared/gatt: Make register_notify cancellable

[Arman Uguray]

This patch makes CCC writes via bt_gatt_client_register_notify
cancellable. The following changes have been introduced:

  1. bt_gatt_client_register_notify now returns the id immediately
     instead of returning it in a callback. The callback is still
     used to communicate ATT protocol errors.

  2. A notify callback is immediately registered, so that if the
     remote end sends any ATT notifications/indications, the caller
     will start receiving them right away.

Change-Id: I14785bda0267b49312e6665fed53a9b13f271c50
---
 src/shared/gatt-client.c | 129 ++++++++++++++++++++++++++++-------------------
 src/shared/gatt-client.h |   7 ++-
 tools/btgatt-client.c    |  17 ++++---
 3 files changed, 91 insertions(+), 62 deletions(-)

diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index 04fb4cb..cbc5382 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -168,9 +168,10 @@ struct notify_data {
 	struct bt_gatt_client *client;
 	bool invalid;
 	unsigned int id;
+	unsigned int att_id;
 	int ref_count;
 	struct notify_chrc *chrc;
-	bt_gatt_client_notify_id_callback_t callback;
+	bt_gatt_client_register_callback_t callback;
 	bt_gatt_client_notify_callback_t notify;
 	void *user_data;
 	bt_gatt_client_destroy_func_t destroy;
@@ -1011,22 +1012,20 @@ struct service_changed_op {
 	uint16_t end_handle;
 };
 
-static void service_changed_reregister_cb(unsigned int id, uint16_t att_ecode,
-								void *user_data)
+static void service_changed_reregister_cb(uint16_t att_ecode, void *user_data)
 {
 	struct bt_gatt_client *client = user_data;
 
-	if (!id || att_ecode) {
+	if (!att_ecode) {
 		util_debug(client->debug_callback, client->debug_data,
-			"Failed to register handler for \"Service Changed\"");
+			"Re-registered handler for \"Service Changed\" after "
+			"change in GATT service");
 		return;
 	}
 
-	client->svc_chngd_ind_id = id;
-
 	util_debug(client->debug_callback, client->debug_data,
-		"Re-registered handler for \"Service Changed\" after change in "
-		"GATT service");
+		"Failed to register handler for \"Service Changed\"");
+	client->svc_chngd_ind_id = 0;
 }
 
 static void process_service_changed(struct bt_gatt_client *client,
@@ -1090,11 +1089,12 @@ static void service_changed_complete(struct discovery_op *op, bool success,
 	/* The GATT service was modified. Re-register the handler for
 	 * indications from the "Service Changed" characteristic.
 	 */
-	if (bt_gatt_client_register_notify(client,
+	client->svc_chngd_ind_id = bt_gatt_client_register_notify(client,
 					gatt_db_attribute_get_handle(attr),
 					service_changed_reregister_cb,
 					service_changed_cb,
-					client, NULL))
+					client, NULL);
+	if (client->svc_chngd_ind_id)
 		return;
 
 	util_debug(client->debug_callback, client->debug_data,
@@ -1184,24 +1184,24 @@ static void service_changed_cb(uint16_t value_handle, const uint8_t *value,
 	queue_push_tail(client->svc_chngd_queue, op);
 }
 
-static void service_changed_register_cb(unsigned int id, uint16_t att_ecode,
-								void *user_data)
+static void service_changed_register_cb(uint16_t att_ecode, void *user_data)
 {
 	bool success;
 	struct bt_gatt_client *client = user_data;
 
-	if (!id || att_ecode) {
+	if (att_ecode) {
 		util_debug(client->debug_callback, client->debug_data,
 			"Failed to register handler for \"Service Changed\"");
 		success = false;
+		client->svc_chngd_ind_id = 0;
 		goto done;
 	}
 
-	client->svc_chngd_ind_id = id;
 	client->ready = true;
 	success = true;
 	util_debug(client->debug_callback, client->debug_data,
-			"Registered handler for \"Service Changed\": %u", id);
+			"Registered handler for \"Service Changed\": %u",
+			client->svc_chngd_ind_id);
 
 done:
 	notify_client_ready(client, success, att_ecode);
@@ -1211,7 +1211,6 @@ static void init_complete(struct discovery_op *op, bool success,
 							uint8_t att_ecode)
 {
 	struct bt_gatt_client *client = op->client;
-	bool registered;
 	struct gatt_db_attribute *attr = NULL;
 	bt_uuid_t uuid;
 
@@ -1235,14 +1234,14 @@ static void init_complete(struct discovery_op *op, bool success,
 	 * the handler using the existing framework.
 	 */
 	client->ready = true;
-	registered = bt_gatt_client_register_notify(client,
+	client->svc_chngd_ind_id = bt_gatt_client_register_notify(client,
 					gatt_db_attribute_get_handle(attr),
 					service_changed_register_cb,
 					service_changed_cb,
 					client, NULL);
 	client->ready = false;
 
-	if (registered)
+	if (client->svc_chngd_ind_id)
 		return;
 
 	util_debug(client->debug_callback, client->debug_data,
@@ -1301,24 +1300,15 @@ static void complete_notify_request(void *data)
 	/* Increment the per-characteristic ref count of notify handlers */
 	__sync_fetch_and_add(&notify_data->chrc->notify_count, 1);
 
-	/* Add the handler to the bt_gatt_client's general list */
-	queue_push_tail(notify_data->client->notify_list,
-						notify_data_ref(notify_data));
-
-	/* Assign an ID to the handler and notify the caller that it was
-	 * successfully registered.
-	 */
-	if (notify_data->client->next_reg_id < 1)
-		notify_data->client->next_reg_id = 1;
-
-	notify_data->id = notify_data->client->next_reg_id++;
-	notify_data->callback(notify_data->id, 0, notify_data->user_data);
+	notify_data->att_id = 0;
+	notify_data->callback(0, notify_data->user_data);
 }
 
 static bool notify_data_write_ccc(struct notify_data *notify_data, bool enable,
 						bt_att_response_func_t callback)
 {
 	uint8_t pdu[4];
+	unsigned int att_id;
 
 	assert(notify_data->chrc->ccc_handle);
 	memset(pdu, 0, sizeof(pdu));
@@ -1338,13 +1328,13 @@ static bool notify_data_write_ccc(struct notify_data *notify_data, bool enable,
 			return false;
 	}
 
-	notify_data->chrc->ccc_write_id = bt_att_send(notify_data->client->att,
-						BT_ATT_OP_WRITE_REQ,
-						pdu, sizeof(pdu),
-						callback,
-						notify_data, notify_data_unref);
+	att_id = bt_att_send(notify_data->client->att, BT_ATT_OP_WRITE_REQ,
+						pdu, sizeof(pdu), callback,
+						notify_data_ref(notify_data),
+						notify_data_unref);
+	notify_data->chrc->ccc_write_id = notify_data->att_id = att_id;
 
-	return !!notify_data->chrc->ccc_write_id;
+	return !!att_id;
 }
 
 static uint8_t process_error(const void *pdu, uint16_t length)
@@ -1377,7 +1367,8 @@ static void enable_ccc_callback(uint8_t opcode, const void *pdu,
 		 * the next one in the queue. If there was an error sending the
 		 * write request, then just move on to the next queued entry.
 		 */
-		notify_data->callback(0, att_ecode, notify_data->user_data);
+		queue_remove(notify_data->client->notify_list, notify_data);
+		notify_data->callback(att_ecode, notify_data->user_data);
 
 		while ((notify_data = queue_pop_head(
 					notify_data->chrc->reg_notify_queue))) {
@@ -1426,6 +1417,16 @@ static void complete_unregister_notify(void *data)
 {
 	struct notify_data *notify_data = data;
 
+	/*
+	 * If a procedure to enable the CCC is still pending, then cancel it and
+	 * return.
+	 */
+	if (notify_data->att_id) {
+		bt_att_cancel(notify_data->client->att, notify_data->att_id);
+		notify_data_unref(notify_data);
+		return;
+	}
+
 	if (__sync_sub_and_fetch(&notify_data->chrc->notify_count, 1)) {
 		notify_data_unref(notify_data);
 		return;
@@ -1449,6 +1450,10 @@ static void notify_handler(void *data, void *user_data)
 	if (pdu_data->length > 2)
 		value = pdu_data->pdu + 2;
 
+	/*
+	 * Even if the notify data has a pending ATT request to write to the
+	 * CCC, there is really no reason not to notify the handlers.
+	 */
 	if (notify_data->notify)
 		notify_data->notify(value_handle, value, pdu_data->length - 2,
 							notify_data->user_data);
@@ -1475,10 +1480,22 @@ static void notify_cb(uint8_t opcode, const void *pdu, uint16_t length,
 	bt_gatt_client_unref(client);
 }
 
+static void notify_data_cleanup(void *data)
+{
+	struct notify_data *notify_data = data;
+
+	if (notify_data->att_id)
+		bt_att_cancel(notify_data->client->att, notify_data->att_id);
+
+	notify_data_unref(notify_data);
+}
+
 static void bt_gatt_client_free(struct bt_gatt_client *client)
 {
 	bt_gatt_client_cancel_all(client);
 
+	queue_destroy(client->notify_list, notify_data_cleanup);
+
 	if (client->ready_destroy)
 		client->ready_destroy(client->ready_data);
 
@@ -1496,7 +1513,6 @@ static void bt_gatt_client_free(struct bt_gatt_client *client)
 
 	queue_destroy(client->svc_chngd_queue, free);
 	queue_destroy(client->long_write_queue, request_unref);
-	queue_destroy(client->notify_list, notify_data_unref);
 	queue_destroy(client->notify_chrcs, notify_chrc_free);
 	queue_destroy(client->pending_requests, request_unref);
 
@@ -2569,9 +2585,9 @@ static bool match_notify_chrc_value_handle(const void *a, const void *b)
 	return chrc->value_handle == value_handle;
 }
 
-bool bt_gatt_client_register_notify(struct bt_gatt_client *client,
+unsigned int bt_gatt_client_register_notify(struct bt_gatt_client *client,
 				uint16_t chrc_value_handle,
-				bt_gatt_client_notify_id_callback_t callback,
+				bt_gatt_client_register_callback_t callback,
 				bt_gatt_client_notify_callback_t notify,
 				void *user_data,
 				bt_gatt_client_destroy_func_t destroy)
@@ -2580,10 +2596,10 @@ bool bt_gatt_client_register_notify(struct bt_gatt_client *client,
 	struct notify_chrc *chrc = NULL;
 
 	if (!client || !client->db || !chrc_value_handle || !callback)
-		return false;
+		return 0;
 
 	if (!bt_gatt_client_is_ready(client) || client->in_svc_chngd)
-		return false;
+		return 0;
 
 	/* Check if a characteristic ref count has been started already */
 	chrc = queue_find(client->notify_chrcs, match_notify_chrc_value_handle,
@@ -2596,17 +2612,16 @@ bool bt_gatt_client_register_notify(struct bt_gatt_client *client,
 		 */
 		chrc = notify_chrc_create(client, chrc_value_handle);
 		if (!chrc)
-			return false;
-
+			return 0;
 	}
 
 	/* Fail if we've hit the maximum allowed notify sessions */
 	if (chrc->notify_count == INT_MAX)
-		return false;
+		return 0;
 
 	notify_data = new0(struct notify_data, 1);
 	if (!notify_data)
-		return false;
+		return 0;
 
 	notify_data->client = client;
 	notify_data->ref_count = 1;
@@ -2616,13 +2631,24 @@ bool bt_gatt_client_register_notify(struct bt_gatt_client *client,
 	notify_data->user_data = user_data;
 	notify_data->destroy = destroy;
 
+	/* Add the handler to the bt_gatt_client's general list */
+	queue_push_tail(client->notify_list, notify_data);
+
+	/* Assign an ID to the handler and notify the caller that it was
+	 * successfully registered.
+	 */
+	if (client->next_reg_id < 1)
+		client->next_reg_id = 1;
+
+	notify_data->id = client->next_reg_id++;
+
 	/*
 	 * If a write to the CCC descriptor is in progress, then queue this
 	 * request.
 	 */
 	if (chrc->ccc_write_id) {
 		queue_push_tail(chrc->reg_notify_queue, notify_data);
-		return true;
+		return notify_data->id;
 	}
 
 	/*
@@ -2630,16 +2656,17 @@ bool bt_gatt_client_register_notify(struct bt_gatt_client *client,
 	 */
 	if (chrc->notify_count > 0) {
 		complete_notify_request(notify_data);
-		return true;
+		return notify_data->id;
 	}
 
 	/* Write to the CCC descriptor */
 	if (!notify_data_write_ccc(notify_data, true, enable_ccc_callback)) {
+		queue_remove(client->notify_list, notify_data);
 		free(notify_data);
-		return false;
+		return 0;
 	}
 
-	return true;
+	return notify_data->id;
 }
 
 bool bt_gatt_client_unregister_notify(struct bt_gatt_client *client,
diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
index 9a00feb..b84fa13 100644
--- a/src/shared/gatt-client.h
+++ b/src/shared/gatt-client.h
@@ -49,8 +49,7 @@ typedef void (*bt_gatt_client_write_long_callback_t)(bool success,
 typedef void (*bt_gatt_client_notify_callback_t)(uint16_t value_handle,
 					const uint8_t *value, uint16_t length,
 					void *user_data);
-typedef void (*bt_gatt_client_notify_id_callback_t)(unsigned int id,
-							uint16_t att_ecode,
+typedef void (*bt_gatt_client_register_callback_t)(uint16_t att_ecode,
 							void *user_data);
 typedef void (*bt_gatt_client_service_changed_callback_t)(uint16_t start_handle,
 							uint16_t end_handle,
@@ -110,9 +109,9 @@ unsigned int bt_gatt_client_write_long_value(struct bt_gatt_client *client,
 				void *user_data,
 				bt_gatt_client_destroy_func_t destroy);
 
-bool bt_gatt_client_register_notify(struct bt_gatt_client *client,
+unsigned int bt_gatt_client_register_notify(struct bt_gatt_client *client,
 				uint16_t chrc_value_handle,
-				bt_gatt_client_notify_id_callback_t callback,
+				bt_gatt_client_register_callback_t callback,
 				bt_gatt_client_notify_callback_t notify,
 				void *user_data,
 				bt_gatt_client_destroy_func_t destroy);
diff --git a/tools/btgatt-client.c b/tools/btgatt-client.c
index 62c4d3e..8bda89b 100644
--- a/tools/btgatt-client.c
+++ b/tools/btgatt-client.c
@@ -856,16 +856,15 @@ static void notify_cb(uint16_t value_handle, const uint8_t *value,
 	PRLOG("\n");
 }
 
-static void register_notify_cb(unsigned int id, uint16_t att_ecode,
-								void *user_data)
+static void register_notify_cb(uint16_t att_ecode, void *user_data)
 {
-	if (!id) {
+	if (att_ecode) {
 		PRLOG("Failed to register notify handler "
 					"- error code: 0x%02x\n", att_ecode);
 		return;
 	}
 
-	PRLOG("Registered notify handler with id: %u\n", id);
+	PRLOG("Registered notify handler!");
 }
 
 static void cmd_register_notify(struct client *cli, char *cmd_str)
@@ -873,6 +872,7 @@ static void cmd_register_notify(struct client *cli, char *cmd_str)
 	char *argv[2];
 	int argc = 0;
 	uint16_t value_handle;
+	unsigned int id;
 	char *endptr = NULL;
 
 	if (!bt_gatt_client_is_ready(cli->gatt)) {
@@ -891,12 +891,15 @@ static void cmd_register_notify(struct client *cli, char *cmd_str)
 		return;
 	}
 
-	if (!bt_gatt_client_register_notify(cli->gatt, value_handle,
+	id = bt_gatt_client_register_notify(cli->gatt, value_handle,
 							register_notify_cb,
-							notify_cb, NULL, NULL))
+							notify_cb, NULL, NULL);
+	if (!id) {
 		printf("Failed to register notify handler\n");
+		return;
+	}
 
-	printf("\n");
+	PRLOG("Registering notify handler with id: %u\n", id);
 }
 
 static void unregister_notify_usage(void)
-- 
2.2.0.rc0.207.ga3a616c

[PATCH BlueZ v2 2/5] core: gatt: Implement GattCharacteristic1.StartNotify

[Arman Uguray]

This patch implements the StartNotify method of
org.bluez.GattCharacteristic1. Each call to StartNotify assigns a
session to the D-Bus sender which internally registers a unique
notify handler with the bt_gatt_client. Each received notification
or indication causes a PropertiesChanged signal to be emitted for the
"Value" property.

The notify handler gets automatically unregistered when sender
disconnects from D-Bus.

Change-Id: Ic6d2a04b3dab7fa7d12df6d58938ddf8362a1a8e
---
 src/gatt-client.c | 254 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 245 insertions(+), 9 deletions(-)

diff --git a/src/gatt-client.c b/src/gatt-client.c
index cb8ddf6..4f5022a 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -88,6 +88,9 @@ struct characteristic {
 	unsigned int write_id;
 
 	struct queue *descs;
+
+	bool notifying;
+	struct queue *notify_clients;
 };
 
 struct descriptor {
@@ -231,7 +234,9 @@ static void async_dbus_op_free(void *data)
 {
 	struct async_dbus_op *op = data;
 
-	dbus_message_unref(op->msg);
+	if (op->msg)
+		dbus_message_unref(op->msg);
+
 	free(op);
 }
 
@@ -689,12 +694,8 @@ static gboolean characteristic_value_exists(const GDBusPropertyTable *property,
 static gboolean characteristic_get_notifying(const GDBusPropertyTable *property,
 					DBusMessageIter *iter, void *data)
 {
-	dbus_bool_t notifying = FALSE;
-
-	/*
-	 * TODO: Return the correct value here once StartNotify and StopNotify
-	 * methods are implemented.
-	 */
+	struct characteristic *chrc = data;
+	dbus_bool_t notifying = chrc->notifying ? TRUE : FALSE;
 
 	dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN, &notifying);
 
@@ -925,11 +926,238 @@ fail:
 	return btd_error_not_supported(msg);
 }
 
+struct notify_client {
+	struct characteristic *chrc;
+	int ref_count;
+	char *owner;
+	guint watch;
+	unsigned int notify_id;
+};
+
+static void notify_client_free(struct notify_client *client)
+{
+	DBG("owner %s", client->owner);
+
+	g_dbus_remove_watch(btd_get_dbus_connection(), client->watch);
+	bt_gatt_client_unregister_notify(client->chrc->service->client->gatt,
+							client->notify_id);
+	free(client->owner);
+	free(client);
+}
+
+static void notify_client_unref(void *data)
+{
+	struct notify_client *client = data;
+
+	DBG("owner %s", client->owner);
+
+	if (__sync_sub_and_fetch(&client->ref_count, 1))
+		return;
+
+	notify_client_free(client);
+}
+
+static struct notify_client *notify_client_ref(struct notify_client *client)
+{
+	DBG("owner %s", client->owner);
+
+	__sync_fetch_and_add(&client->ref_count, 1);
+
+	return client;
+}
+
+static bool match_notifying(const void *a, const void *b)
+{
+	const struct notify_client *client = a;
+
+	return !!client->notify_id;
+}
+
+static void update_notifying(struct characteristic *chrc)
+{
+	if (!chrc->notifying)
+		return;
+
+	if (queue_find(chrc->notify_clients, match_notifying, NULL))
+		return;
+
+	chrc->notifying = false;
+
+	g_dbus_emit_property_changed(btd_get_dbus_connection(), chrc->path,
+						GATT_CHARACTERISTIC_IFACE,
+						"Notifying");
+}
+
+static void notify_client_disconnect(DBusConnection *conn, void *user_data)
+{
+	struct notify_client *client = user_data;
+	struct characteristic *chrc = client->chrc;
+	struct bt_gatt_client *gatt = chrc->service->client->gatt;
+
+	DBG("owner %s", client->owner);
+
+	queue_remove(chrc->notify_clients, client);
+	bt_gatt_client_unregister_notify(gatt, client->notify_id);
+
+	update_notifying(chrc);
+
+	notify_client_unref(client);
+}
+
+static struct notify_client *notify_client_create(struct characteristic *chrc,
+							const char *owner)
+{
+	struct notify_client *client;
+
+	client = new0(struct notify_client, 1);
+	if (!client)
+		return NULL;
+
+	client->chrc = chrc;
+	client->owner = strdup(owner);
+	if (!client->owner) {
+		free(client);
+		return NULL;
+	}
+
+	client->watch = g_dbus_add_disconnect_watch(btd_get_dbus_connection(),
+						owner, notify_client_disconnect,
+						client, NULL);
+	if (!client->watch) {
+		free(client->owner);
+		free(client);
+		return NULL;
+	}
+
+	return notify_client_ref(client);
+}
+
+static bool match_notify_sender(const void *a, const void *b)
+{
+	const struct notify_client *client = a;
+	const char *sender = b;
+
+	return strcmp(client->owner, sender) == 0;
+}
+
+static void notify_cb(uint16_t value_handle, const uint8_t *value,
+					uint16_t length, void *user_data)
+{
+	struct async_dbus_op *op = user_data;
+	struct notify_client *client = op->data;
+	struct characteristic *chrc = client->chrc;
+
+	/*
+	 * Even if the value didn't change, we want to send a PropertiesChanged
+	 * signal so that we propagate the notification/indication to
+	 * applications.
+	 */
+	gatt_db_attribute_reset(chrc->attr);
+	gatt_db_attribute_write(chrc->attr, 0, value, length, 0, NULL,
+						write_characteristic_cb, chrc);
+}
+
+static void register_notify_cb(uint16_t att_ecode, void *user_data)
+{
+	struct async_dbus_op *op = user_data;
+	struct notify_client *client = op->data;
+	struct characteristic *chrc = client->chrc;
+	DBusMessage *reply;
+
+	/* Make sure that an interim disconnect did not remove the client */
+	if (!queue_find(chrc->notify_clients, NULL, client)) {
+		bt_gatt_client_unregister_notify(chrc->service->client->gatt,
+							client->notify_id);
+		notify_client_unref(client);
+
+		reply = btd_error_failed(op->msg,
+						"Characteristic not available");
+		goto done;
+	}
+
+	/*
+	 * Drop the reference count that we added when registering the callback.
+	 */
+	notify_client_unref(client);
+
+	if (att_ecode) {
+		queue_remove(chrc->notify_clients, client);
+		notify_client_free(client);
+
+		reply = create_gatt_dbus_error(op->msg, att_ecode);
+
+		goto done;
+	}
+
+	if (!chrc->notifying) {
+		chrc->notifying = true;
+		g_dbus_emit_property_changed(btd_get_dbus_connection(),
+					chrc->path, GATT_CHARACTERISTIC_IFACE,
+					"Notifying");
+	}
+
+	reply = g_dbus_create_reply(op->msg, DBUS_TYPE_INVALID);
+
+done:
+	if (reply)
+		g_dbus_send_message(btd_get_dbus_connection(), reply);
+	else
+		error("Failed to construct D-Bus message reply");
+}
+
 static DBusMessage *characteristic_start_notify(DBusConnection *conn,
 					DBusMessage *msg, void *user_data)
 {
-	/* TODO: Implement */
-	return btd_error_failed(msg, "Not implemented");
+	struct characteristic *chrc = user_data;
+	struct bt_gatt_client *gatt = chrc->service->client->gatt;
+	const char *sender = dbus_message_get_sender(msg);
+	struct async_dbus_op *op;
+	struct notify_client *client;
+
+	if (!(chrc->props & BT_GATT_CHRC_PROP_NOTIFY ||
+				chrc->props & BT_GATT_CHRC_PROP_INDICATE))
+		return btd_error_not_supported(msg);
+
+	/* Each client can only have one active notify session. */
+	client = queue_find(chrc->notify_clients, match_notify_sender, sender);
+	if (client)
+		return client->notify_id ?
+				btd_error_failed(msg, "Already notifying") :
+				btd_error_in_progress(msg);
+
+	client = notify_client_create(chrc, sender);
+	if (!client)
+		return btd_error_failed(msg, "Failed allocate notify session");
+
+	op = new0(struct async_dbus_op, 1);
+	if (!op) {
+		notify_client_unref(client);
+		return btd_error_failed(msg, "Failed to initialize request");
+	}
+
+	/*
+	 * Add to the ref count so that a disconnect during register won't free
+	 * the client instance.
+	 */
+	op->data = notify_client_ref(client);
+	op->msg = dbus_message_ref(msg);
+
+	queue_push_tail(chrc->notify_clients, client);
+
+	client->notify_id = bt_gatt_client_register_notify(gatt,
+						chrc->value_handle,
+						register_notify_cb, notify_cb,
+						op, async_dbus_op_free);
+	if (client->notify_id)
+		return NULL;
+
+	queue_remove(chrc->notify_clients, client);
+	async_dbus_op_free(op);
+
+	/* Directly free the client */
+	notify_client_free(client);
+
+	return btd_error_failed(msg, "Failed to register notify session");
 }
 
 static DBusMessage *characteristic_stop_notify(DBusConnection *conn,
@@ -1022,6 +1250,13 @@ static struct characteristic *characteristic_create(
 		return NULL;
 	}
 
+	chrc->notify_clients = queue_new();
+	if (!chrc->notify_clients) {
+		queue_destroy(chrc->descs, NULL);
+		free(chrc);
+		return NULL;
+	}
+
 	chrc->service = service;
 
 	gatt_db_attribute_get_char_data(attr, &chrc->handle,
@@ -1064,6 +1299,7 @@ static void unregister_characteristic(void *data)
 	if (chrc->write_id)
 		bt_gatt_client_cancel(gatt, chrc->write_id);
 
+	queue_remove_all(chrc->notify_clients, NULL, NULL, notify_client_unref);
 	queue_remove_all(chrc->descs, NULL, NULL, unregister_descriptor);
 
 	g_dbus_unregister_interface(btd_get_dbus_connection(), chrc->path,
-- 
2.2.0.rc0.207.ga3a616c

[PATCH BlueZ v2 3/5] core: gatt: Implement GattCharacteristic1.StopNotify

[Arman Uguray]

This patch implements the StopNotify method of
org.bluez.GattCharacteristic1.

Change-Id: If6173f101341697e035c0cd07971da4b74a37a25
---
 src/gatt-client.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/src/gatt-client.c b/src/gatt-client.c
index 4f5022a..e157ac2 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -1163,8 +1163,25 @@ static DBusMessage *characteristic_start_notify(DBusConnection *conn,
 static DBusMessage *characteristic_stop_notify(DBusConnection *conn,
 					DBusMessage *msg, void *user_data)
 {
-	/* TODO: Implement */
-	return btd_error_failed(msg, "Not implemented");
+	struct characteristic *chrc = user_data;
+	struct bt_gatt_client *gatt = chrc->service->client->gatt;
+	const char *sender = dbus_message_get_sender(msg);
+	struct notify_client *client;
+
+	if (!chrc->notifying)
+		return btd_error_failed(msg, "Not notifying");
+
+	client = queue_remove_if(chrc->notify_clients, match_notify_sender,
+							(void *) sender);
+	if (!client)
+		return btd_error_failed(msg, "No notify session started");
+
+	bt_gatt_client_unregister_notify(gatt, client->notify_id);
+	update_notifying(chrc);
+
+	notify_client_unref(client);
+
+	return dbus_message_new_method_return(msg);
 }
 
 static void append_desc_path(void *data, void *user_data)
-- 
2.2.0.rc0.207.ga3a616c

[PATCH BlueZ v2 4/5] core: device: Don't check ready in service_removed

[Arman Uguray]

shared/gatt-client clears a given gatt-db if there's an error during its
init sequence, even if the given gatt-db was previously populated (e.g.
from a cache). This is to make sure that the database contents are at no
point invalid.

This patch removes a check for bt_gatt_client_is_ready and the
corresponding early-return from btd_device's service_removed handler, so
that other layers can be notified of invalidated gatt_db_attribute
pointers.

Change-Id: I8e188d0d0f81398b89f63183ed0708830b6dd9c5
---
 src/device.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/src/device.c b/src/device.c
index 7c8ae74..29a8f23 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2927,8 +2927,21 @@ static void gatt_service_removed(struct gatt_db_attribute *attr,
 	bt_uuid_t uuid;
 	char uuid_str[MAX_LEN_UUID_STR];
 
-	if (!bt_gatt_client_is_ready(device->client))
-		return;
+	/*
+	 * NOTE: shared/gatt-client clears the database in case of failure. This
+	 * triggers the service_removed callback for all affected services.
+	 * Hence, this function will be called in the following cases:
+	 *
+	 *    1. When a GATT service gets removed due to "Service Changed".
+	 *
+	 *    2. When a GATT service gets removed when the database get cleared
+	 *       upon disconnection with a non-bonded device.
+	 *
+	 *    3. When a GATT service gets removed when the database get cleared
+	 *       by shared/gatt-client when its initialization procedure fails,
+	 *       e.g. due to an ATT protocol error or an unexpected disconnect.
+	 *       In this case the gatt-client will not be ready.
+	 */
 
 	gatt_db_attribute_get_service_data(attr, &start, &end, NULL, &uuid);
 
-- 
2.2.0.rc0.207.ga3a616c

[PATCH BlueZ v2 5/5] core: gatt: Keep objects over disconnects

[Arman Uguray]

This patch changes the way that the life-time for exported GATT API
objects are managed, so that they are kept alive in the case of a
disconnection if the devices are bonded. In this case, all API method
calls return an error except for StartNotify/StopNotify.

All notification sessions that were initiated during and in-between
connections are maintained and a notification callback is registered
for each session immediately on reconnection.

Change-Id: Ib7ac72a91dd8dc4b631c0948b4bd41af0bd4ff7b
---
 src/device.c      |   5 ++
 src/gatt-client.c | 222 +++++++++++++++++++++++++++++++++++++++++++++---------
 2 files changed, 191 insertions(+), 36 deletions(-)

diff --git a/src/device.c b/src/device.c
index 29a8f23..059e3a4 100644
--- a/src/device.c
+++ b/src/device.c
@@ -529,6 +529,11 @@ static void gatt_client_cleanup(struct btd_device *device)
 	bt_gatt_client_unref(device->client);
 	device->client = NULL;
 
+	/*
+	 * TODO: Once GATT over BR/EDR is properly supported, we should check
+	 * the bonding state for the correct bearer based on the transport over
+	 * which GATT is being done.
+	 */
 	if (!device->le_state.bonded)
 		gatt_db_clear(device->db);
 }
diff --git a/src/gatt-client.c b/src/gatt-client.c
index e157ac2..f03a278 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -58,6 +58,7 @@ struct btd_gatt_client {
 	struct bt_gatt_client *gatt;
 
 	struct queue *services;
+	struct queue *all_notify_clients;
 };
 
 struct service {
@@ -387,6 +388,9 @@ static DBusMessage *descriptor_read_value(DBusConnection *conn,
 	struct bt_gatt_client *gatt = desc->chrc->service->client->gatt;
 	struct async_dbus_op *op;
 
+	if (!gatt)
+		return btd_error_failed(msg, "Not connected");
+
 	if (desc->read_id)
 		return btd_error_in_progress(msg);
 
@@ -521,6 +525,9 @@ static DBusMessage *descriptor_write_value(DBusConnection *conn,
 	uint8_t *value = NULL;
 	size_t value_len = 0;
 
+	if (!gatt)
+		return btd_error_failed(msg, "Not connected");
+
 	if (desc->write_id)
 		return btd_error_in_progress(msg);
 
@@ -815,6 +822,9 @@ static DBusMessage *characteristic_read_value(DBusConnection *conn,
 	struct bt_gatt_client *gatt = chrc->service->client->gatt;
 	struct async_dbus_op *op;
 
+	if (!gatt)
+		return btd_error_failed(msg, "Not connected");
+
 	if (chrc->read_id)
 		return btd_error_in_progress(msg);
 
@@ -859,6 +869,9 @@ static DBusMessage *characteristic_write_value(DBusConnection *conn,
 	size_t value_len = 0;
 	bool supported = false;
 
+	if (!gatt)
+		return btd_error_failed(msg, "Not connected");
+
 	if (chrc->write_id)
 		return btd_error_in_progress(msg);
 
@@ -992,12 +1005,11 @@ static void notify_client_disconnect(DBusConnection *conn, void *user_data)
 {
 	struct notify_client *client = user_data;
 	struct characteristic *chrc = client->chrc;
-	struct bt_gatt_client *gatt = chrc->service->client->gatt;
 
 	DBG("owner %s", client->owner);
 
 	queue_remove(chrc->notify_clients, client);
-	bt_gatt_client_unregister_notify(gatt, client->notify_id);
+	queue_remove(chrc->service->client->all_notify_clients, client);
 
 	update_notifying(chrc);
 
@@ -1057,34 +1069,41 @@ static void notify_cb(uint16_t value_handle, const uint8_t *value,
 						write_characteristic_cb, chrc);
 }
 
-static void register_notify_cb(uint16_t att_ecode, void *user_data)
+static DBusMessage *create_notify_reply(struct async_dbus_op *op,
+						bool success, uint8_t att_ecode)
 {
-	struct async_dbus_op *op = user_data;
-	struct notify_client *client = op->data;
-	struct characteristic *chrc = client->chrc;
-	DBusMessage *reply;
+	DBusMessage *reply = NULL;
 
-	/* Make sure that an interim disconnect did not remove the client */
-	if (!queue_find(chrc->notify_clients, NULL, client)) {
-		bt_gatt_client_unregister_notify(chrc->service->client->gatt,
-							client->notify_id);
-		notify_client_unref(client);
+	if (!op->msg)
+		return NULL;
 
+	if (success)
+		reply = g_dbus_create_reply(op->msg, DBUS_TYPE_INVALID);
+	else if (att_ecode)
+		reply = create_gatt_dbus_error(op->msg, att_ecode);
+	else
 		reply = btd_error_failed(op->msg,
 						"Characteristic not available");
-		goto done;
-	}
 
-	/*
-	 * Drop the reference count that we added when registering the callback.
-	 */
-	notify_client_unref(client);
+	if (!reply)
+		error("Failed to construct D-Bus message reply");
+
+	return reply;
+}
+
+static void register_notify_cb(uint16_t att_ecode, void *user_data)
+{
+	struct async_dbus_op *op = user_data;
+	struct notify_client *client = op->data;
+	struct characteristic *chrc = client->chrc;
+	DBusMessage *reply;
 
 	if (att_ecode) {
 		queue_remove(chrc->notify_clients, client);
+		queue_remove(chrc->service->client->all_notify_clients, client);
 		notify_client_free(client);
 
-		reply = create_gatt_dbus_error(op->msg, att_ecode);
+		reply = create_notify_reply(op, false, att_ecode);
 
 		goto done;
 	}
@@ -1096,7 +1115,7 @@ static void register_notify_cb(uint16_t att_ecode, void *user_data)
 					"Notifying");
 	}
 
-	reply = g_dbus_create_reply(op->msg, DBUS_TYPE_INVALID);
+	reply = create_notify_reply(op, true, 0);
 
 done:
 	if (reply)
@@ -1129,20 +1148,35 @@ static DBusMessage *characteristic_start_notify(DBusConnection *conn,
 	if (!client)
 		return btd_error_failed(msg, "Failed allocate notify session");
 
-	op = new0(struct async_dbus_op, 1);
-	if (!op) {
-		notify_client_unref(client);
-		return btd_error_failed(msg, "Failed to initialize request");
-	}
+	queue_push_tail(chrc->notify_clients, client);
+	queue_push_tail(chrc->service->client->all_notify_clients, client);
 
 	/*
-	 * Add to the ref count so that a disconnect during register won't free
-	 * the client instance.
+	 * If the device is currently not connected, return success. We will
+	 * automatically try and register all clients when a GATT client becomes
+	 * ready.
 	 */
-	op->data = notify_client_ref(client);
-	op->msg = dbus_message_ref(msg);
+	if (!gatt) {
+		DBusMessage *reply;
 
-	queue_push_tail(chrc->notify_clients, client);
+		reply = g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
+		if (reply)
+			return reply;
+
+		/*
+		 * Clean up and respond with an error instead of timing out to
+		 * avoid any ambiguities.
+		 */
+		error("Failed to construct D-Bus message reply");
+		goto fail;
+	}
+
+	op = new0(struct async_dbus_op, 1);
+	if (!op)
+		goto fail;
+
+	op->data = client;
+	op->msg = dbus_message_ref(msg);
 
 	client->notify_id = bt_gatt_client_register_notify(gatt,
 						chrc->value_handle,
@@ -1151,9 +1185,12 @@ static DBusMessage *characteristic_start_notify(DBusConnection *conn,
 	if (client->notify_id)
 		return NULL;
 
-	queue_remove(chrc->notify_clients, client);
 	async_dbus_op_free(op);
 
+fail:
+	queue_remove(chrc->notify_clients, client);
+	queue_remove(chrc->service->client->all_notify_clients, client);
+
 	/* Directly free the client */
 	notify_client_free(client);
 
@@ -1176,6 +1213,7 @@ static DBusMessage *characteristic_stop_notify(DBusConnection *conn,
 	if (!client)
 		return btd_error_failed(msg, "No notify session started");
 
+	queue_remove(chrc->service->client->all_notify_clients, client);
 	bt_gatt_client_unregister_notify(gatt, client->notify_id);
 	update_notifying(chrc);
 
@@ -1680,6 +1718,13 @@ struct btd_gatt_client *btd_gatt_client_new(struct btd_device *device)
 		return NULL;
 	}
 
+	client->all_notify_clients = queue_new();
+	if (!client->all_notify_clients) {
+		queue_destroy(client->services, NULL);
+		free(client);
+		return NULL;
+	}
+
 	client->device = device;
 	ba2str(device_get_address(device), client->devaddr);
 
@@ -1694,11 +1739,44 @@ void btd_gatt_client_destroy(struct btd_gatt_client *client)
 		return;
 
 	queue_destroy(client->services, unregister_service);
+	queue_destroy(client->all_notify_clients, NULL);
 	bt_gatt_client_unref(client->gatt);
 	gatt_db_unref(client->db);
 	free(client);
 }
 
+static void register_notify(void *data, void *user_data)
+{
+	struct notify_client *notify_client = data;
+	struct btd_gatt_client *client = user_data;
+	struct async_dbus_op *op;
+
+	DBG("Re-register subscribed notification client");
+
+	op = new0(struct async_dbus_op, 1);
+	if (!op)
+		goto fail;
+
+	op->data = notify_client;
+
+	notify_client->notify_id = bt_gatt_client_register_notify(client->gatt,
+					notify_client->chrc->value_handle,
+					register_notify_cb, notify_cb,
+					op, async_dbus_op_free);
+	if (notify_client->notify_id)
+		return;
+
+	async_dbus_op_free(op);
+
+fail:
+	DBG("Failed to re-register notification client");
+
+	queue_remove(notify_client->chrc->notify_clients, client);
+	queue_remove(client->all_notify_clients, client);
+
+	notify_client_free(notify_client);
+}
+
 void btd_gatt_client_ready(struct btd_gatt_client *client)
 {
 	struct bt_gatt_client *gatt;
@@ -1717,7 +1795,19 @@ void btd_gatt_client_ready(struct btd_gatt_client *client)
 
 	client->ready = true;
 
-	create_services(client);
+	DBG("GATT client ready");
+
+	if (queue_isempty(client->services)) {
+		DBG("Exporting services");
+		create_services(client);
+		return;
+	}
+
+	/*
+	 * Services have already been created before. Re-enable notifications
+	 * for any pre-registered notification sessions.
+	 */
+	queue_foreach(client->all_notify_clients, register_notify, client);
 }
 
 void btd_gatt_client_service_added(struct btd_gatt_client *client,
@@ -1755,18 +1845,78 @@ void btd_gatt_client_service_removed(struct btd_gatt_client *client,
 						unregister_service);
 }
 
+static void clear_notify_id(void *data, void *user_data)
+{
+	struct notify_client *client = data;
+
+	client->notify_id = 0;
+}
+
+static void cancel_desc_ops(void *data, void *user_data)
+{
+	struct descriptor *desc = data;
+	struct bt_gatt_client *gatt = user_data;
+
+	if (desc->read_id) {
+		bt_gatt_client_cancel(gatt, desc->read_id);
+		desc->read_id = 0;
+	}
+
+	if (desc->write_id) {
+		bt_gatt_client_cancel(gatt, desc->write_id);
+		desc->write_id = 0;
+	}
+}
+
+static void cancel_chrc_ops(void *data, void *user_data)
+{
+	struct characteristic *chrc = data;
+	struct bt_gatt_client *gatt = user_data;
+
+	if (chrc->read_id) {
+		bt_gatt_client_cancel(gatt, chrc->read_id);
+		chrc->read_id = 0;
+	}
+
+	if (chrc->write_id) {
+		bt_gatt_client_cancel(gatt, chrc->write_id);
+		chrc->write_id = 0;
+	}
+
+	queue_foreach(chrc->descs, cancel_desc_ops, user_data);
+}
+
+static void cancel_ops(void *data, void *user_data)
+{
+	struct service *service = data;
+
+	queue_foreach(service->chrcs, cancel_chrc_ops, user_data);
+}
+
 void btd_gatt_client_disconnected(struct btd_gatt_client *client)
 {
-	if (!client)
+	if (!client || !client->gatt)
 		return;
 
-	DBG("Device disconnected. Cleaning up");
+	DBG("Device disconnected. Cleaning up.");
 
 	/*
 	 * Remove all services. We'll recreate them when a new bt_gatt_client
-	 * becomes ready.
+	 * becomes ready. Leave the services around if the device is bonded.
+	 * TODO: Once GATT over BR/EDR is properly supported, we should pass the
+	 * correct bdaddr_type based on the transport over which GATT is being
+	 * done.
 	 */
-	queue_remove_all(client->services, NULL, NULL, unregister_service);
+	if (!device_is_bonded(client->device, BDADDR_LE_PUBLIC)) {
+		DBG("Device not bonded. Removing exported services.");
+		queue_remove_all(client->services, NULL, NULL,
+							unregister_service);
+	} else {
+		DBG("Device is bonded. Keeping exported services up.");
+		queue_foreach(client->all_notify_clients, clear_notify_id,
+									NULL);
+		queue_foreach(client->services, cancel_ops, client->gatt);
+	}
 
 	bt_gatt_client_unref(client->gatt);
 	client->gatt = NULL;
-- 
2.2.0.rc0.207.ga3a616c

Re: [PATCH BlueZ v2 0/5] core: gatt: Support enabling notifications

[Luiz Augusto von Dentz]

Hi Arman,

On Sat, Jan 31, 2015 at 2:29 AM, Arman Uguray <armansito@chromium.org> wrote:
> *v2: Cancel ongoing CCC writes in bt_gatt_client_free.
>
> *v1: Make bt_gatt_register_notify cancellable via bt_gatt_unregister_notify.
>      notify_client_free now unregisters its notify id, which will cancel any
>      pending CCC write, as well as unregister a registered notify callback.
>
> This patch sets brings support for enabling notifications while in GATT
> client-role. The changes that are introduced are:
>
>   1. Implemented the StartNotify and StopNotify methods of the
>      GattCharacteristic1 interface. This are internally tied to
>      bt_gatt_client_register_notify and bt_gatt_client_unregister_notify.
>      These also manage notification sessions on a per dbus sender basis.
>
>   2. The exported GATT API objects are not removed in the event of a disconnect,
>      if the devices are bonded. All notification sessions are also persisted and
>      automatically re-enabled on reconnection. Also, adding new notification
>      sessions via StartNotify is allowed during disconnects.
>
> Arman Uguray (5):
>   shared/gatt: Make register_notify cancellable
>   core: gatt: Implement GattCharacteristic1.StartNotify
>   core: gatt: Implement GattCharacteristic1.StopNotify
>   core: device: Don't check ready in service_removed
>   core: gatt: Keep objects over disconnects
>
>  src/device.c             |  22 ++-
>  src/gatt-client.c        | 435 +++++++++++++++++++++++++++++++++++++++++++++--
>  src/shared/gatt-client.c | 129 ++++++++------
>  src/shared/gatt-client.h |   7 +-
>  tools/btgatt-client.c    |  17 +-
>  5 files changed, 530 insertions(+), 80 deletions(-)
>
> --
> 2.2.0.rc0.207.ga3a616c

Pushed, but note that I had made a couple of changes. You should
really stop sending with your internal change-id and I really mean it
since it wasn't the first time, I know this is because you had these
patches for a while but we should really try to revert this trend now
that most API is upstream.

Also, lets try a little bit the test cases running valgrind, Id just
had a smoke test with a HoG device and while it worked (luckily the
device has battery service and supported notifications) and this
showed up:

http://fpaste.org/180066/87602814/

I managed to fix the problems but Im not that happy that it too a
little while to figure out them because the code is a bit convoluted
in a few places, which is another reason things don't get applied as
quickly, furthermore the use of the same filename is really bad when
debugging and Im very tempted to move this code to gatt-dbus.{c,h} or
create gatt-dbus-client.{c,h}.


-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ v2 0/5] core: gatt: Support enabling notifications

[Arman Uguray]

Hi Luiz,

> On Mon, Feb 2, 2015 at 5:53 AM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
> Hi Arman,
>
> On Sat, Jan 31, 2015 at 2:29 AM, Arman Uguray <armansito@chromium.org> wrote:
>> *v2: Cancel ongoing CCC writes in bt_gatt_client_free.
>>
>> *v1: Make bt_gatt_register_notify cancellable via bt_gatt_unregister_notify.
>>      notify_client_free now unregisters its notify id, which will cancel any
>>      pending CCC write, as well as unregister a registered notify callback.
>>
>> This patch sets brings support for enabling notifications while in GATT
>> client-role. The changes that are introduced are:
>>
>>   1. Implemented the StartNotify and StopNotify methods of the
>>      GattCharacteristic1 interface. This are internally tied to
>>      bt_gatt_client_register_notify and bt_gatt_client_unregister_notify.
>>      These also manage notification sessions on a per dbus sender basis.
>>
>>   2. The exported GATT API objects are not removed in the event of a disconnect,
>>      if the devices are bonded. All notification sessions are also persisted and
>>      automatically re-enabled on reconnection. Also, adding new notification
>>      sessions via StartNotify is allowed during disconnects.
>>
>> Arman Uguray (5):
>>   shared/gatt: Make register_notify cancellable
>>   core: gatt: Implement GattCharacteristic1.StartNotify
>>   core: gatt: Implement GattCharacteristic1.StopNotify
>>   core: device: Don't check ready in service_removed
>>   core: gatt: Keep objects over disconnects
>>
>>  src/device.c             |  22 ++-
>>  src/gatt-client.c        | 435 +++++++++++++++++++++++++++++++++++++++++++++--
>>  src/shared/gatt-client.c | 129 ++++++++------
>>  src/shared/gatt-client.h |   7 +-
>>  tools/btgatt-client.c    |  17 +-
>>  5 files changed, 530 insertions(+), 80 deletions(-)
>>
>> --
>> 2.2.0.rc0.207.ga3a616c
>
> Pushed, but note that I had made a couple of changes. You should
> really stop sending with your internal change-id and I really mean it
> since it wasn't the first time, I know this is because you had these
> patches for a while but we should really try to revert this trend now
> that most API is upstream.
>

This started happening again after I moved things around on my
machine, though I did remove the Change-Id lines in v3, looks like you
applied v2. Sorry about the confusion.

> Also, lets try a little bit the test cases running valgrind, Id just
> had a smoke test with a HoG device and while it worked (luckily the
> device has battery service and supported notifications) and this
> showed up:
>
> http://fpaste.org/180066/87602814/
>

Thanks for doing that. Since I've been debugging on ChromiumOS devices
and not my Ubuntu desktop, I've run into a few issues with running
valgrind that I'm trying to resolve, hopefully things should be fixed
soon.

> I managed to fix the problems but Im not that happy that it too a
> little while to figure out them because the code is a bit convoluted
> in a few places, which is another reason things don't get applied as
> quickly, furthermore the use of the same filename is really bad when
> debugging and Im very tempted to move this code to gatt-dbus.{c,h} or
> create gatt-dbus-client.{c,h}.
>

I thought the src vs src/shared namespacing would be sufficient but we
could rename this to gatt-dbus-client or gatt-client-dbus to prevent
confusion with the term "D-Bus client".

>
> --
> Luiz Augusto von Dentz

Thanks,
Arman