[PATCH BlueZ 00/11] Introducing shared/gatt-client

[PATCH BlueZ 00/11] Introducing shared/gatt-client

[Arman Uguray]

This patch set includes the initial implementation of shared/gatt-client, which
will provide a GATT client utility built on top of shared/att and
shared/gatt-helpers. These patches implement a subset of the long-term features
(some of which are listed in the introduced TODO file), including an abstraction
for GATT primitives (service, characteristic, descriptor, etc.) and the initial
attribute discovery process.

Comments are very much welcome.

Arman Uguray (11):
  shared/gatt-helpers: Remove service, characteristic, descriptor
    structures.
  shared/gatt-helpers: Added result count functions.
  shared/gatt-client: Added initial skeleton and simple functions.
  shared/att: Support multiple disconnect handlers.
  shared/att: Add BT_ATT_DEFAULT_LE_MTU macro.
  shared/gatt-helpers: Fixed typo in callback args.
  shared/gatt-client: Add bt_gatt_client_set_debug.
  shared/gatt-client: Implement initial service discovery.
  shared/gatt-client: Add service iterator functions.
  shared/gatt-client: Implement characteristic discovery.
  shared/gatt-client: Implement descriptor discovery.

 Makefile.am               |   3 +-
 src/shared/TODO           |   8 +
 src/shared/att-types.h    |   2 +
 src/shared/att.c          | 161 +++++++++--
 src/shared/att.h          |   9 +-
 src/shared/gatt-client.c  | 663 ++++++++++++++++++++++++++++++++++++++++++++++
 src/shared/gatt-client.h  | 104 ++++++++
 src/shared/gatt-helpers.c | 100 +++++--
 src/shared/gatt-helpers.h |  36 +--
 9 files changed, 1007 insertions(+), 79 deletions(-)
 create mode 100644 src/shared/TODO
 create mode 100644 src/shared/gatt-client.c
 create mode 100644 src/shared/gatt-client.h

-- 
2.1.0.rc2.206.gedb03e5

[PATCH BlueZ 01/11] shared/gatt-helpers: Remove service, characteristic, descriptor structures.

[Arman Uguray]

This patch removes the service, characteristic, and descriptor structures
declared in gatt-helpers.h. These aren't really necessary, especially since
there will be another higher-level version of these for shared/gatt-client.
---
 src/shared/gatt-helpers.c | 51 +++++++++++++++++++++++++----------------------
 src/shared/gatt-helpers.h | 30 +++++++---------------------
 2 files changed, 34 insertions(+), 47 deletions(-)

diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index 047f64c..ff49867 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -134,13 +134,14 @@ struct discovery_op {
 };
 
 bool bt_gatt_iter_next_service(struct bt_gatt_iter *iter,
-						struct bt_gatt_service *service)
+				uint16_t *start_handle, uint16_t *end_handle,
+				uint8_t uuid[16])
 {
 	struct discovery_op *op;
 	const void *pdu_ptr;
-	bt_uuid_t uuid;
+	bt_uuid_t tmp;
 
-	if (!iter->result || !service)
+	if (!iter || !iter->result || !start_handle || !end_handle || !uuid)
 		return false;
 
 	op = iter->result->op;
@@ -148,17 +149,16 @@ bool bt_gatt_iter_next_service(struct bt_gatt_iter *iter,
 
 	switch (iter->result->opcode) {
 	case BT_ATT_OP_READ_BY_GRP_TYPE_RSP:
-		service->start = get_le16(pdu_ptr);
-		service->end = get_le16(pdu_ptr + 2);
-		convert_uuid_le(pdu_ptr + 4, iter->result->data_len - 4,
-								service->uuid);
+		*start_handle = get_le16(pdu_ptr);
+		*end_handle = get_le16(pdu_ptr + 2);
+		convert_uuid_le(pdu_ptr + 4, iter->result->data_len - 4, uuid);
 		break;
 	case BT_ATT_OP_FIND_BY_TYPE_VAL_RSP:
-		service->start = get_le16(pdu_ptr);
-		service->end = get_le16(pdu_ptr + 2);
+		*start_handle = get_le16(pdu_ptr);
+		*end_handle = get_le16(pdu_ptr + 2);
 
-		bt_uuid_to_uuid128(&op->uuid, &uuid);
-		memcpy(service->uuid, uuid.value.u128.data, 16);
+		bt_uuid_to_uuid128(&op->uuid, &tmp);
+		memcpy(uuid, tmp.value.u128.data, 16);
 		break;
 	default:
 		return false;
@@ -175,12 +175,15 @@ bool bt_gatt_iter_next_service(struct bt_gatt_iter *iter,
 }
 
 bool bt_gatt_iter_next_characteristic(struct bt_gatt_iter *iter,
-					struct bt_gatt_characteristic *chrc)
+				uint16_t *start_handle, uint16_t *end_handle,
+				uint16_t *value_handle, uint8_t *properties,
+				uint8_t uuid[16])
 {
 	struct discovery_op *op;
 	const void *pdu_ptr;
 
-	if (!iter->result || !chrc)
+	if (!iter || !iter->result || !start_handle || !end_handle ||
+					!value_handle || !properties || !uuid)
 		return false;
 
 	if (iter->result->opcode != BT_ATT_OP_READ_BY_TYPE_RSP)
@@ -189,10 +192,10 @@ bool bt_gatt_iter_next_characteristic(struct bt_gatt_iter *iter,
 	op = iter->result->op;
 	pdu_ptr = iter->result->pdu + iter->pos;
 
-	chrc->start = get_le16(pdu_ptr);
-	chrc->properties = ((uint8_t *) pdu_ptr)[2];
-	chrc->value = get_le16(pdu_ptr + 3);
-	convert_uuid_le(pdu_ptr + 5, iter->result->data_len - 5, chrc->uuid);
+	*start_handle = get_le16(pdu_ptr);
+	*properties = ((uint8_t *) pdu_ptr)[2];
+	*value_handle = get_le16(pdu_ptr + 3);
+	convert_uuid_le(pdu_ptr + 5, iter->result->data_len - 5, uuid);
 
 	iter->pos += iter->result->data_len;
 	if (iter->pos == iter->result->pdu_len) {
@@ -201,21 +204,21 @@ bool bt_gatt_iter_next_characteristic(struct bt_gatt_iter *iter,
 	}
 
 	if (!iter->result) {
-		chrc->end = op->end_handle;
+		*end_handle = op->end_handle;
 		return true;
 	}
 
-	chrc->end = get_le16(iter->result->pdu + iter->pos) - 1;
+	*end_handle = get_le16(iter->result->pdu + iter->pos) - 1;
 
 	return true;
 }
 
-bool bt_gatt_iter_next_descriptor(struct bt_gatt_iter *iter,
-						struct bt_gatt_descriptor *desc)
+bool bt_gatt_iter_next_descriptor(struct bt_gatt_iter *iter, uint16_t *handle,
+							uint8_t uuid[16])
 {
 	const void *pdu_ptr;
 
-	if (!iter->result || !desc)
+	if (!iter || !iter->result || !handle || !uuid)
 		return false;
 
 	if (iter->result->opcode != BT_ATT_OP_FIND_INFO_RSP)
@@ -223,8 +226,8 @@ bool bt_gatt_iter_next_descriptor(struct bt_gatt_iter *iter,
 
 	pdu_ptr = iter->result->pdu + iter->pos;
 
-	desc->handle = get_le16(pdu_ptr);
-	convert_uuid_le(pdu_ptr + 2, iter->result->data_len - 2, desc->uuid);
+	*handle = get_le16(pdu_ptr);
+	convert_uuid_le(pdu_ptr + 2, iter->result->data_len - 2, uuid);
 
 	iter->pos += iter->result->data_len;
 	if (iter->pos == iter->result->pdu_len) {
diff --git a/src/shared/gatt-helpers.h b/src/shared/gatt-helpers.h
index 95dd8b7..cdf83ba 100644
--- a/src/shared/gatt-helpers.h
+++ b/src/shared/gatt-helpers.h
@@ -28,25 +28,6 @@
 #include <stdbool.h>
 #include <stdint.h>
 
-struct bt_gatt_service {
-	uint16_t start;
-	uint16_t end;
-	uint8_t uuid[16];
-};
-
-struct bt_gatt_characteristic {
-	uint16_t start;
-	uint16_t end;
-	uint16_t value;
-	uint8_t properties;
-	uint8_t uuid[16];
-};
-
-struct bt_gatt_descriptor {
-	uint16_t handle;
-	uint8_t uuid[16];
-};
-
 struct bt_gatt_result;
 
 struct bt_gatt_iter {
@@ -56,11 +37,14 @@ struct bt_gatt_iter {
 
 bool bt_gatt_iter_init(struct bt_gatt_iter *iter, struct bt_gatt_result *result);
 bool bt_gatt_iter_next_service(struct bt_gatt_iter *iter,
-					struct bt_gatt_service *service);
+				uint16_t *start_handle, uint16_t *end_handle,
+				uint8_t uuid[16]);
 bool bt_gatt_iter_next_characteristic(struct bt_gatt_iter *iter,
-				struct bt_gatt_characteristic *characteristic);
-bool bt_gatt_iter_next_descriptor(struct bt_gatt_iter *iter,
-					struct bt_gatt_descriptor *descriptor);
+				uint16_t *start_handle, uint16_t *end_handle,
+				uint16_t *value_handle, uint8_t *properties,
+				uint8_t uuid[16]);
+bool bt_gatt_iter_next_descriptor(struct bt_gatt_iter *iter, uint16_t *handle,
+							uint8_t uuid[16]);
 
 typedef void (*bt_gatt_destroy_func_t)(void *user_data);
 
-- 
2.1.0.rc2.206.gedb03e5

[PATCH BlueZ 02/11] shared/gatt-helpers: Added result count functions.

[Arman Uguray]

Added functions that return the number of services, characteristics, and
descriptors contained in a bt_gatt_result.
---
 src/shared/gatt-helpers.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++
 src/shared/gatt-helpers.h |  4 ++++
 2 files changed, 53 insertions(+)

diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index ff49867..00813f5 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -88,6 +88,55 @@ static void result_destroy(struct bt_gatt_result *result)
 	}
 }
 
+static unsigned int result_element_count(struct bt_gatt_result *result)
+{
+	unsigned int count = 0;
+	struct bt_gatt_result *cur;
+
+	cur = result;
+
+	while (cur) {
+		count += cur->pdu_len / cur->data_len;
+		cur = cur->next;
+	}
+
+	return count;
+}
+
+unsigned int bt_gatt_result_service_count(struct bt_gatt_result *result)
+{
+	if (!result)
+		return 0;
+
+	if (result->opcode != BT_ATT_OP_READ_BY_GRP_TYPE_RSP &&
+			result->opcode != BT_ATT_OP_FIND_BY_TYPE_VAL_RSP)
+		return 0;
+
+	return result_element_count(result);
+}
+
+unsigned int bt_gatt_result_characteristic_count(struct bt_gatt_result *result)
+{
+	if (!result)
+		return 0;
+
+	if (result->opcode != BT_ATT_OP_READ_BY_TYPE_RSP)
+		return 0;
+
+	return result_element_count(result);
+}
+
+unsigned int bt_gatt_result_descriptor_count(struct bt_gatt_result *result)
+{
+	if (!result)
+		return 0;
+
+	if (result->opcode != BT_ATT_OP_FIND_INFO_RSP)
+		return 0;
+
+	return result_element_count(result);
+}
+
 bool bt_gatt_iter_init(struct bt_gatt_iter *iter, struct bt_gatt_result *result)
 {
 	if (!iter || !result)
diff --git a/src/shared/gatt-helpers.h b/src/shared/gatt-helpers.h
index cdf83ba..d63fac1 100644
--- a/src/shared/gatt-helpers.h
+++ b/src/shared/gatt-helpers.h
@@ -35,6 +35,10 @@ struct bt_gatt_iter {
 	uint16_t pos;
 };
 
+unsigned int bt_gatt_result_service_count(struct bt_gatt_result *result);
+unsigned int bt_gatt_result_characteristic_count(struct bt_gatt_result *result);
+unsigned int bt_gatt_result_descriptor_count(struct bt_gatt_result *result);
+
 bool bt_gatt_iter_init(struct bt_gatt_iter *iter, struct bt_gatt_result *result);
 bool bt_gatt_iter_next_service(struct bt_gatt_iter *iter,
 				uint16_t *start_handle, uint16_t *end_handle,
-- 
2.1.0.rc2.206.gedb03e5

[PATCH BlueZ 03/11] shared/gatt-client: Added initial skeleton and simple functions.

[Arman Uguray]

This patch introduces shared/gatt-client, which provides a central/client side
implementation of the Generic Attribute Profile. An instance of bt_gatt_client
will provide a central database of all GATT services, characteristics, and
descriptors that have been discovered on a peripheral and will provide API
functions to obtain information about discovered attributes, registering
handlers for "Service Changed" indications, as well as providing
reference-counted access to "Client Characteristic Configuration" descriptors.
---
 Makefile.am              |   3 +-
 src/shared/TODO          |  13 ++++++
 src/shared/gatt-client.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++
 src/shared/gatt-client.h |  54 +++++++++++++++++++++++++
 4 files changed, 170 insertions(+), 1 deletion(-)
 create mode 100644 src/shared/TODO
 create mode 100644 src/shared/gatt-client.c
 create mode 100644 src/shared/gatt-client.h

diff --git a/Makefile.am b/Makefile.am
index 88fcb49..6aed8e2 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -158,7 +158,8 @@ src_bluetoothd_SOURCES = $(builtin_sources) \
 			src/shared/util.h src/shared/util.c \
 			src/shared/mgmt.h src/shared/mgmt.c \
 			src/shared/att-types.h src/shared/att.h src/shared/att.c \
-			src/shared/gatt-helpers.h src/shared/gatt-helpers.c
+			src/shared/gatt-helpers.h src/shared/gatt-helpers.c \
+			src/shared/gatt-client.h src/shared/gatt-client.c
 src_bluetoothd_LDADD = lib/libbluetooth-internal.la gdbus/libgdbus-internal.la \
 			@GLIB_LIBS@ @DBUS_LIBS@ -ldl -lrt
 src_bluetoothd_LDFLAGS = $(AM_LDFLAGS) -Wl,--export-dynamic \
diff --git a/src/shared/TODO b/src/shared/TODO
new file mode 100644
index 0000000..2881c97
--- /dev/null
+++ b/src/shared/TODO
@@ -0,0 +1,13 @@
+TODOs for shared/gatt-client
+
+* Add util_debug support.
+* Add high-level abstraction for services, characteristics, and descriptors.
+* Implement service discovery.
+* Implement characteristic discovery.
+* Implement descriptor discovery.
+* Handle request timeouts.
+* Make bt_gatt_client observe disconnect events. Handle bonded/non-bonded cases.
+* Add support for auto-connects using a different bt_att.
+* Add functions for reference counted access to enable/disable
+  notifications/indications.
+* Add support for "Service Changed".
diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
new file mode 100644
index 0000000..53a7de1
--- /dev/null
+++ b/src/shared/gatt-client.c
@@ -0,0 +1,101 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2014  Google Inc.
+ *
+ *
+ *  This library is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU Lesser General Public
+ *  License as published by the Free Software Foundation; either
+ *  version 2.1 of the License, or (at your option) any later version.
+ *
+ *  This library is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public
+ *  License along with this library; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#include "src/shared/att.h"
+#include "src/shared/gatt-client.h"
+#include "src/shared/util.h"
+
+struct bt_gatt_client {
+	struct bt_att *att;
+	bool persist;
+};
+
+struct bt_gatt_client *bt_gatt_client_new(struct bt_att *att)
+{
+	struct bt_gatt_client *client;
+
+	if (!att)
+		return NULL;
+
+	client = new0(struct bt_gatt_client, 1);
+	if (!client)
+		return NULL;
+
+	client->att = bt_att_ref(att);
+
+	return client;
+}
+
+void bt_gatt_client_destroy(struct bt_gatt_client *client)
+{
+	if (!client)
+		return;
+
+	bt_att_unref(client->att);
+	free(client);
+}
+
+bool bt_gatt_client_set_persist(struct bt_gatt_client *client,
+							bool do_persist)
+{
+	if (!client)
+		return false;
+
+	client->persist = do_persist;
+
+	return true;
+}
+
+struct bt_att *bt_gatt_client_get_att(struct bt_gatt_client *client)
+{
+	if (!client)
+		return NULL;
+
+	return client->att;
+}
+
+bool bt_gatt_client_init(struct bt_gatt_client *client, uint16_t mtu,
+					bt_gatt_client_callback_t callback,
+					void *user_data,
+					bt_gatt_client_destroy_func_t destroy)
+{
+	// TODO
+}
+
+unsigned int bt_gatt_client_register_service_changed(
+				struct bt_gatt_client *client,
+				uint16_t handle,
+				bt_gatt_client_service_changed_func_t callback,
+				void *user_data,
+				bt_gatt_client_destroy_func_t destroy)
+{
+	// TODO
+	return 0;
+}
+
+bool bt_gatt_client_unregister_service_changed(struct bt_gatt_client *client,
+							unsigned int id)
+{
+	// TODO
+	return false;
+}
diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
new file mode 100644
index 0000000..a978691
--- /dev/null
+++ b/src/shared/gatt-client.h
@@ -0,0 +1,54 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2014  Google Inc.
+ *
+ *
+ *  This library is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU Lesser General Public
+ *  License as published by the Free Software Foundation; either
+ *  version 2.1 of the License, or (at your option) any later version.
+ *
+ *  This library is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public
+ *  License along with this library; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#include <stdbool.h>
+#include <stdint.h>
+
+struct bt_gatt_client;
+
+struct bt_gatt_client *bt_gatt_client_new(struct bt_att *att);
+void bt_gatt_client_destroy(struct bt_gatt_client *client);
+
+bool bt_gatt_client_set_persist(struct bt_gatt_client *client,
+							bool do_persist);
+
+struct bt_att *bt_gatt_client_get_att(struct bt_gatt_client *client);
+
+typedef void (*bt_gatt_client_destroy_func_t)(void *user_data);
+typedef void (*bt_gatt_client_callback_t)(void *user_data);
+typedef void (*bt_gatt_client_service_changed_func_t)(uint16_t handle,
+							void *user_data);
+
+bool bt_gatt_client_init(struct bt_gatt_client *client, uint16_t mtu,
+					bt_gatt_client_callback_t callback,
+					void *user_data,
+					bt_gatt_client_destroy_func_t destroy);
+
+unsigned int bt_gatt_client_register_service_changed(
+				struct bt_gatt_client *client,
+				uint16_t handle,
+				bt_gatt_client_service_changed_func_t callback,
+				void *user_data,
+				bt_gatt_client_destroy_func_t destroy);
+bool bt_gatt_client_unregister_service_changed(struct bt_gatt_client *client,
+							unsigned int id);
-- 
2.1.0.rc2.206.gedb03e5

[PATCH BlueZ 04/11] shared/att: Support multiple disconnect handlers.

[Arman Uguray]

This patch adds support for registering multiple disconnect handlers with an
instance of struct bt_att. Unregistering is achieved via a new function AND
through bt_att_unregister_all and all disconnect callbacks get automatically
unregistered after a single disconnect event.
---
 src/shared/att.c | 155 ++++++++++++++++++++++++++++++++++++++++++++++---------
 src/shared/att.h |   9 ++--
 2 files changed, 137 insertions(+), 27 deletions(-)

diff --git a/src/shared/att.c b/src/shared/att.c
index b5ab742..68a57f9 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -60,6 +60,10 @@ struct bt_att {
 	bool in_notify;
 	bool need_notify_cleanup;
 
+	struct queue *disconn_list;	/* List of disconnect handlers */
+	bool in_disconn;
+	bool need_disconn_cleanup;
+
 	uint8_t *buf;
 	uint16_t mtu;
 
@@ -73,10 +77,6 @@ struct bt_att {
 	bt_att_debug_func_t debug_callback;
 	bt_att_destroy_func_t debug_destroy;
 	void *debug_data;
-
-	bt_att_disconnect_func_t disconn_callback;
-	bt_att_destroy_func_t disconn_destroy;
-	void *disconn_data;
 };
 
 enum att_op_type {
@@ -233,6 +233,46 @@ static void mark_notify_removed(void *data, void *user_data)
 	notify->removed = true;
 }
 
+struct att_disconn {
+	unsigned int id;
+	bool removed;
+	bt_att_disconnect_func_t callback;
+	bt_att_destroy_func_t destroy;
+	void *user_data;
+};
+
+static void destroy_att_disconn(void *data)
+{
+	struct att_disconn *disconn = data;
+
+	if (disconn->destroy)
+		disconn->destroy(disconn->user_data);
+
+	free(disconn);
+}
+
+static bool match_disconn_id(const void *a, const void *b)
+{
+	const struct att_disconn *disconn = a;
+	unsigned int id = PTR_TO_UINT(b);
+
+	return disconn->id == id;
+}
+
+static bool match_disconn_removed(const void *a, const void *b)
+{
+	const struct att_disconn *disconn = a;
+
+	return disconn->removed;
+}
+
+static void mark_disconn_removed(void *data, void *user_data)
+{
+	struct att_disconn *disconn = data;
+
+	disconn->removed = true;
+}
+
 static bool encode_pdu(struct att_send_op *op, const void *pdu,
 						uint16_t length, uint16_t mtu)
 {
@@ -605,21 +645,42 @@ static bool can_read_data(struct io *io, void *user_data)
 	return true;
 }
 
+static void disconn_handler(void *data, void *user_data)
+{
+	struct att_disconn *disconn = data;
+
+	if (disconn->removed)
+		return;
+
+	if (disconn->callback)
+		disconn->callback(disconn->user_data);
+}
+
 static bool disconnect_cb(struct io *io, void *user_data)
 {
 	struct bt_att *att = user_data;
 
-	bt_att_cancel_all(att);
-	bt_att_unregister_all(att);
-
 	io_destroy(att->io);
 	att->io = NULL;
 
 	util_debug(att->debug_callback, att->debug_data,
 						"Physical link disconnected");
 
-	if (att->disconn_callback)
-		att->disconn_callback(att->disconn_data);
+	bt_att_ref(att);
+	att->in_disconn = true;
+	queue_foreach(att->disconn_list, disconn_handler, NULL);
+	att->in_disconn = false;
+
+	if (att->need_disconn_cleanup) {
+		queue_remove_all(att->disconn_list, match_disconn_removed, NULL,
+							destroy_att_disconn);
+		att->need_disconn_cleanup = false;
+	}
+
+	bt_att_cancel_all(att);
+	bt_att_unregister_all(att);
+
+	bt_att_unref(att);
 
 	return false;
 }
@@ -662,6 +723,10 @@ struct bt_att *bt_att_new(int fd)
 	if (!att->notify_list)
 		goto fail;
 
+	att->disconn_list = queue_new();
+	if (!att->disconn_list)
+		goto fail;
+
 	if (!io_set_read_handler(att->io, can_read_data, att, NULL))
 		goto fail;
 
@@ -674,6 +739,8 @@ fail:
 	queue_destroy(att->req_queue, NULL);
 	queue_destroy(att->ind_queue, NULL);
 	queue_destroy(att->write_queue, NULL);
+	queue_destroy(att->notify_list, NULL);
+	queue_destroy(att->disconn_list, NULL);
 	io_destroy(att->io);
 	free(att->buf);
 	free(att);
@@ -709,6 +776,7 @@ void bt_att_unref(struct bt_att *att)
 	queue_destroy(att->ind_queue, NULL);
 	queue_destroy(att->write_queue, NULL);
 	queue_destroy(att->notify_list, NULL);
+	queue_destroy(att->disconn_list, NULL);
 	att->req_queue = NULL;
 	att->ind_queue = NULL;
 	att->write_queue = NULL;
@@ -720,9 +788,6 @@ void bt_att_unref(struct bt_att *att)
 	if (att->debug_destroy)
 		att->debug_destroy(att->debug_data);
 
-	if (att->disconn_destroy)
-		att->disconn_destroy(att->disconn_data);
-
 	free(att->buf);
 	att->buf = NULL;
 
@@ -800,20 +865,57 @@ bool bt_att_set_timeout_cb(struct bt_att *att, bt_att_timeout_func_t callback,
 	return true;
 }
 
-bool bt_att_set_disconnect_cb(struct bt_att *att,
+unsigned int bt_att_register_disconnect_handler(struct bt_att *att,
 					bt_att_disconnect_func_t callback,
 					void *user_data,
 					bt_att_destroy_func_t destroy)
 {
-	if (!att)
+	struct att_disconn *disconn;
+
+	if (!att || !att->io)
+		return 0;
+
+	disconn = new0(struct att_disconn, 1);
+	if (!disconn)
+		return 0;
+
+	disconn->callback = callback;
+	disconn->destroy = destroy;
+	disconn->user_data = user_data;
+
+	if (att->next_reg_id < 1)
+		att->next_reg_id = 1;
+
+	disconn->id = att->next_reg_id++;
+
+	if (!queue_push_tail(att->disconn_list, disconn)) {
+		free(disconn);
+		return 0;
+	}
+
+	return disconn->id;
+}
+
+bool bt_att_unregister_disconnect_handler(struct bt_att *att, unsigned int id)
+{
+	struct att_disconn *disconn;
+
+	if (!att || !id)
 		return false;
 
-	if (att->disconn_destroy)
-		att->disconn_destroy(att->disconn_data);
+	disconn = queue_find(att->disconn_list, match_disconn_id,
+							UINT_TO_PTR(id));
+	if (!disconn)
+		return false;
+
+	if (!att->in_disconn) {
+		queue_remove(att->disconn_list, disconn);
+		destroy_att_disconn(disconn);
+		return true;
+	}
 
-	att->disconn_callback = callback;
-	att->disconn_destroy = destroy;
-	att->disconn_data = user_data;
+	disconn->removed = true;
+	att->need_disconn_cleanup = true;
 
 	return true;
 }
@@ -996,14 +1098,21 @@ bool bt_att_unregister_all(struct bt_att *att)
 	if (!att)
 		return false;
 
-	if (!att->in_notify) {
+	if (att->in_notify) {
+		queue_foreach(att->notify_list, mark_notify_removed, NULL);
+		att->need_notify_cleanup = true;
+	} else {
 		queue_remove_all(att->notify_list, NULL, NULL,
 							destroy_att_notify);
-		return true;
 	}
 
-	queue_foreach(att->notify_list, mark_notify_removed, NULL);
-	att->need_notify_cleanup = true;
+	if (att->in_disconn) {
+		queue_foreach(att->disconn_list, mark_disconn_removed, NULL);
+		att->need_disconn_cleanup = true;
+	} else {
+		queue_remove_all(att->disconn_list, NULL, NULL,
+							destroy_att_disconn);
+	}
 
 	return true;
 }
diff --git a/src/shared/att.h b/src/shared/att.h
index cf44704..3180d2c 100644
--- a/src/shared/att.h
+++ b/src/shared/att.h
@@ -54,10 +54,6 @@ bool bt_att_set_mtu(struct bt_att *att, uint16_t mtu);
 bool bt_att_set_timeout_cb(struct bt_att *att, bt_att_timeout_func_t callback,
 						void *user_data,
 						bt_att_destroy_func_t destroy);
-bool bt_att_set_disconnect_cb(struct bt_att *att,
-					bt_att_disconnect_func_t callback,
-					void *user_data,
-					bt_att_destroy_func_t destroy);
 
 unsigned int bt_att_send(struct bt_att *att, uint8_t opcode,
 					const void *pdu, uint16_t length,
@@ -72,4 +68,9 @@ unsigned int bt_att_register(struct bt_att *att, uint8_t opcode,
 						void *user_data,
 						bt_att_destroy_func_t destroy);
 bool bt_att_unregister(struct bt_att *att, unsigned int id);
+unsigned int bt_att_register_disconnect_handler(struct bt_att *att,
+					bt_att_disconnect_func_t callback,
+					void *user_data,
+					bt_att_destroy_func_t destroy);
+bool bt_att_unregister_disconnect_handler(struct bt_att *att, unsigned int id);
 bool bt_att_unregister_all(struct bt_att *att);
-- 
2.1.0.rc2.206.gedb03e5

[PATCH BlueZ 05/11] shared/att: Add BT_ATT_DEFAULT_LE_MTU macro.

[Arman Uguray]

Moved the ATT_DEFAULT_LE_MTU to att-types.h to make it public.
---
 src/shared/att-types.h | 2 ++
 src/shared/att.c       | 6 +++---
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/shared/att-types.h b/src/shared/att-types.h
index 1637f6f..b85c969 100644
--- a/src/shared/att-types.h
+++ b/src/shared/att-types.h
@@ -23,6 +23,8 @@
 
 #include <stdint.h>
 
+#define BT_ATT_DEFAULT_LE_MTU 23
+
 /* ATT protocol opcodes */
 #define BT_ATT_OP_ERROR_RSP	      		0x01
 #define BT_ATT_OP_MTU_REQ			0x02
diff --git a/src/shared/att.c b/src/shared/att.c
index 68a57f9..58c03d4 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -35,8 +35,8 @@
 #include "src/shared/timeout.h"
 #include "lib/uuid.h"
 #include "src/shared/att.h"
+#include "src/shared/att-types.h"
 
-#define ATT_DEFAULT_LE_MTU		23
 #define ATT_MIN_PDU_LEN			1  /* At least 1 byte for the opcode. */
 #define ATT_OP_CMD_MASK			0x40
 #define ATT_OP_SIGNED_MASK		0x80
@@ -698,7 +698,7 @@ struct bt_att *bt_att_new(int fd)
 
 	att->fd = fd;
 
-	att->mtu = ATT_DEFAULT_LE_MTU;
+	att->mtu = BT_ATT_DEFAULT_LE_MTU;
 	att->buf = malloc(att->mtu);
 	if (!att->buf)
 		goto fail;
@@ -833,7 +833,7 @@ bool bt_att_set_mtu(struct bt_att *att, uint16_t mtu)
 	if (!att)
 		return false;
 
-	if (mtu < ATT_DEFAULT_LE_MTU)
+	if (mtu < BT_ATT_DEFAULT_LE_MTU)
 		return false;
 
 	buf = malloc(mtu);
-- 
2.1.0.rc2.206.gedb03e5

[PATCH BlueZ 06/11] shared/gatt-helpers: Fixed typo in callback args.

[Arman Uguray]

Fixed a small typo in one of the callback arguments.
---
 src/shared/gatt-helpers.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/shared/gatt-helpers.h b/src/shared/gatt-helpers.h
index d63fac1..443e6c5 100644
--- a/src/shared/gatt-helpers.h
+++ b/src/shared/gatt-helpers.h
@@ -54,7 +54,7 @@ typedef void (*bt_gatt_destroy_func_t)(void *user_data);
 
 typedef void (*bt_gatt_result_callback_t)(bool success, uint8_t att_ecode,
 							void *user_data);
-typedef void (*bt_gatt_discovery_callback_t)(bool success, uint8_t att_code,
+typedef void (*bt_gatt_discovery_callback_t)(bool success, uint8_t att_ecode,
 						struct bt_gatt_result *result,
 						void *user_data);
 typedef void (*bt_gatt_read_callback_t)(bool success, uint8_t att_ecode,
-- 
2.1.0.rc2.206.gedb03e5

[PATCH BlueZ 07/11] shared/gatt-client: Add bt_gatt_client_set_debug.

[Arman Uguray]

Added the bt_gatt_client_set_debug function, which sets up a callback to be used
internally by util_debug.
---
 src/shared/TODO          |  1 -
 src/shared/gatt-client.c | 23 +++++++++++++++++++++++
 src/shared/gatt-client.h |  6 ++++++
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/src/shared/TODO b/src/shared/TODO
index 2881c97..d59e153 100644
--- a/src/shared/TODO
+++ b/src/shared/TODO
@@ -1,6 +1,5 @@
 TODOs for shared/gatt-client
 
-* Add util_debug support.
 * Add high-level abstraction for services, characteristics, and descriptors.
 * Implement service discovery.
 * Implement characteristic discovery.
diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index 53a7de1..a0d85e3 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -28,6 +28,10 @@
 struct bt_gatt_client {
 	struct bt_att *att;
 	bool persist;
+
+	bt_gatt_client_debug_func_t debug_callback;
+	bt_gatt_client_destroy_func_t debug_destroy;
+	void *debug_data;
 };
 
 struct bt_gatt_client *bt_gatt_client_new(struct bt_att *att)
@@ -51,6 +55,9 @@ void bt_gatt_client_destroy(struct bt_gatt_client *client)
 	if (!client)
 		return;
 
+	if (client->debug_destroy)
+		client->debug_destroy(client->debug_data);
+
 	bt_att_unref(client->att);
 	free(client);
 }
@@ -74,6 +81,22 @@ struct bt_att *bt_gatt_client_get_att(struct bt_gatt_client *client)
 	return client->att;
 }
 
+bool bt_gatt_client_set_debug(struct bt_gatt_client *client,
+					bt_gatt_client_debug_func_t callback,
+					void *user_data,
+					bt_gatt_client_destroy_func_t destroy) {
+	if (!client)
+		return false;
+
+	if (client->debug_destroy)
+		client->debug_destroy(client->debug_data);
+
+	client->debug_callback = callback;
+	client->debug_destroy = destroy;
+	client->debug_data = user_data;
+
+	return true;
+}
 bool bt_gatt_client_init(struct bt_gatt_client *client, uint16_t mtu,
 					bt_gatt_client_callback_t callback,
 					void *user_data,
diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
index a978691..4038b1e 100644
--- a/src/shared/gatt-client.h
+++ b/src/shared/gatt-client.h
@@ -38,6 +38,12 @@ typedef void (*bt_gatt_client_destroy_func_t)(void *user_data);
 typedef void (*bt_gatt_client_callback_t)(void *user_data);
 typedef void (*bt_gatt_client_service_changed_func_t)(uint16_t handle,
 							void *user_data);
+typedef void (*bt_gatt_client_debug_func_t)(const char *str, void *user_data);
+
+bool bt_gatt_client_set_debug(struct bt_gatt_client *client,
+					bt_gatt_client_debug_func_t callback,
+					void *user_data,
+					bt_gatt_client_destroy_func_t destroy);
 
 bool bt_gatt_client_init(struct bt_gatt_client *client, uint16_t mtu,
 					bt_gatt_client_callback_t callback,
-- 
2.1.0.rc2.206.gedb03e5

[PATCH BlueZ 08/11] shared/gatt-client: Implement initial service discovery.

[Arman Uguray]

This code implements the following portion of attribute discovery:

  - Initial MTU exchange.
  - Primary service discovery.

Discovery results aren't stored yet.
---
 src/shared/TODO          |   1 -
 src/shared/gatt-client.c | 151 ++++++++++++++++++++++++++++++++++++++++++++++-
 src/shared/gatt-client.h |   3 +-
 3 files changed, 152 insertions(+), 3 deletions(-)

diff --git a/src/shared/TODO b/src/shared/TODO
index d59e153..6f51890 100644
--- a/src/shared/TODO
+++ b/src/shared/TODO
@@ -1,7 +1,6 @@
 TODOs for shared/gatt-client
 
 * Add high-level abstraction for services, characteristics, and descriptors.
-* Implement service discovery.
 * Implement characteristic discovery.
 * Implement descriptor discovery.
 * Handle request timeouts.
diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index a0d85e3..a46ff4e 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -23,8 +23,14 @@
 
 #include "src/shared/att.h"
 #include "src/shared/gatt-client.h"
+#include "lib/uuid.h"
+#include "src/shared/gatt-helpers.h"
 #include "src/shared/util.h"
 
+#ifndef MAX
+#define MAX(a, b) ((a) > (b) ? (a) : (b))
+#endif
+
 struct bt_gatt_client {
 	struct bt_att *att;
 	bool persist;
@@ -32,6 +38,8 @@ struct bt_gatt_client {
 	bt_gatt_client_debug_func_t debug_callback;
 	bt_gatt_client_destroy_func_t debug_destroy;
 	void *debug_data;
+
+	bool in_init;
 };
 
 struct bt_gatt_client *bt_gatt_client_new(struct bt_att *att)
@@ -97,12 +105,153 @@ bool bt_gatt_client_set_debug(struct bt_gatt_client *client,
 
 	return true;
 }
+
+struct async_op {
+	struct bt_gatt_client *client;
+	int ref_count;
+	bt_gatt_client_callback_t callback;
+	void *user_data;
+	bt_gatt_client_destroy_func_t destroy;
+};
+
+static struct async_op *async_op_ref(struct async_op *op)
+{
+	__sync_fetch_and_add(&op->ref_count, 1);
+
+	return op;
+}
+
+static void async_op_unref(void *data)
+{
+	struct async_op *op = data;
+
+	if (__sync_sub_and_fetch(&op->ref_count, 1))
+		return;
+
+	if (op->destroy)
+		op->destroy(op->user_data);
+
+	free(data);
+}
+
+static void discover_primary_cb(bool success, uint8_t att_ecode,
+						struct bt_gatt_result *result,
+						void *user_data)
+{
+	struct async_op *op = user_data;
+	struct bt_gatt_client *client = op->client;
+	struct bt_gatt_iter iter;
+	uint16_t start, end;
+	uint8_t uuid[16];
+	char uuid_str[MAX_LEN_UUID_STR];
+	bt_uuid_t tmp;
+
+	if (!success) {
+		util_debug(client->debug_callback, client->debug_data,
+					"Primary service discovery failed."
+					" ATT ECODE: 0x%02x", att_ecode);
+		goto done;
+	}
+
+	if (!result || !bt_gatt_iter_init(&iter, result)) {
+		success = false;
+		att_ecode = 0;
+		goto done;
+	}
+
+	util_debug(client->debug_callback, client->debug_data,
+					"Primary services found: %u",
+					bt_gatt_result_service_count(result));
+
+	while (bt_gatt_iter_next_service(&iter, &start, &end, uuid)) {
+		/* TODO: discover characteristics and store the service. */
+
+		/* Log debug message. */
+		tmp.type = BT_UUID128;
+		memcpy(tmp.value.u128.data, uuid, sizeof(uuid));
+		bt_uuid_to_string(&tmp, uuid_str, sizeof(uuid_str));
+		util_debug(client->debug_callback, client->debug_data,
+				"start: 0x%04x, end: 0x%04x, uuid: %s",
+				start, end, uuid_str);
+	}
+
+done:
+	client->in_init = false;
+
+	if (op->callback)
+		op->callback(success, att_ecode, op->user_data);
+}
+
+static void exchange_mtu_cb(bool success, uint8_t att_ecode, void *user_data)
+{
+	struct async_op *op = user_data;
+	struct bt_gatt_client *client = op->client;
+
+	if (!success) {
+		util_debug(client->debug_callback, client->debug_data,
+				"MTU Exchange failed. ATT ECODE: 0x%02x",
+				att_ecode);
+
+		client->in_init = false;
+
+		if (op->callback)
+			op->callback(success, att_ecode, op->user_data);
+
+		return;
+	}
+
+	util_debug(client->debug_callback, client->debug_data,
+					"MTU exchange complete, with MTU: %u",
+					bt_att_get_mtu(client->att));
+
+	if (bt_gatt_discover_primary_services(client->att, NULL,
+							discover_primary_cb,
+							async_op_ref(op),
+							async_op_unref))
+		return;
+
+	util_debug(client->debug_callback, client->debug_data,
+			"Failed to initiate primary service discovery");
+
+	client->in_init = false;
+
+	if (op->callback)
+		op->callback(false, 0, op->user_data);
+
+	async_op_unref(op);
+}
+
 bool bt_gatt_client_init(struct bt_gatt_client *client, uint16_t mtu,
 					bt_gatt_client_callback_t callback,
 					void *user_data,
 					bt_gatt_client_destroy_func_t destroy)
 {
-	// TODO
+	struct async_op *op;
+
+	if (!client || client->in_init)
+		return false;
+
+	op = new0(struct async_op, 1);
+	if (!op)
+		return false;
+
+	op->client = client;
+	op->callback = callback;
+	op->user_data = user_data;
+	op->destroy = destroy;
+
+	/* Configure the MTU */
+	if (!bt_gatt_exchange_mtu(client->att, MAX(BT_ATT_DEFAULT_LE_MTU, mtu),
+							exchange_mtu_cb,
+							async_op_ref(op),
+							async_op_unref)) {
+		free(op);
+		return false;
+	}
+
+	client->in_init = true;
+
+	return true;
 }
 
 unsigned int bt_gatt_client_register_service_changed(
diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
index 4038b1e..7a46cca 100644
--- a/src/shared/gatt-client.h
+++ b/src/shared/gatt-client.h
@@ -35,7 +35,8 @@ bool bt_gatt_client_set_persist(struct bt_gatt_client *client,
 struct bt_att *bt_gatt_client_get_att(struct bt_gatt_client *client);
 
 typedef void (*bt_gatt_client_destroy_func_t)(void *user_data);
-typedef void (*bt_gatt_client_callback_t)(void *user_data);
+typedef void (*bt_gatt_client_callback_t)(bool success, uint8_t att_ecode,
+							void *user_data);
 typedef void (*bt_gatt_client_service_changed_func_t)(uint16_t handle,
 							void *user_data);
 typedef void (*bt_gatt_client_debug_func_t)(const char *str, void *user_data);
-- 
2.1.0.rc2.206.gedb03e5

[PATCH BlueZ 09/11] shared/gatt-client: Add service iterator functions.

[Arman Uguray]

This patch introduces high-level structures and functions for iterating through,
and storing, data about the discovered services.
---
 src/shared/TODO          |   1 -
 src/shared/gatt-client.c | 153 +++++++++++++++++++++++++++++++++++++++++++++--
 src/shared/gatt-client.h |  42 +++++++++++++
 3 files changed, 191 insertions(+), 5 deletions(-)

diff --git a/src/shared/TODO b/src/shared/TODO
index 6f51890..8ffaacf 100644
--- a/src/shared/TODO
+++ b/src/shared/TODO
@@ -1,6 +1,5 @@
 TODOs for shared/gatt-client
 
-* Add high-level abstraction for services, characteristics, and descriptors.
 * Implement characteristic discovery.
 * Implement descriptor discovery.
 * Handle request timeouts.
diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index a46ff4e..f25b5b7 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -31,6 +31,13 @@
 #define MAX(a, b) ((a) > (b) ? (a) : (b))
 #endif
 
+#define UUID_BYTES BT_GATT_UUID_SIZE * sizeof(uint8_t)
+
+struct service_list {
+	bt_gatt_service_t service;
+	struct service_list *next;
+};
+
 struct bt_gatt_client {
 	struct bt_att *att;
 	bool persist;
@@ -39,9 +46,66 @@ struct bt_gatt_client {
 	bt_gatt_client_destroy_func_t debug_destroy;
 	void *debug_data;
 
+	struct service_list *svc_head, *svc_tail;
 	bool in_init;
+	bool initialized;
+
+	/* GATT service handles */
+	uint16_t gatt_sv_handle;
+	uint16_t svcc_handle;
+	uint16_t svc_ccc_handle;
 };
 
+static bool gatt_client_add_service(struct bt_gatt_client *client,
+						uint16_t start, uint16_t end,
+						uint8_t uuid[BT_GATT_UUID_SIZE])
+{
+	struct service_list *list;
+
+	list = new0(struct service_list, 1);
+	if (!list)
+		return false;
+
+	list->service.start_handle = start;
+	list->service.end_handle = end;
+	memcpy(list->service.uuid, uuid, UUID_BYTES);
+
+	if (!client->svc_head)
+		client->svc_head = client->svc_tail = list;
+	else {
+		client->svc_tail->next = list;
+		client->svc_tail = list;
+	}
+
+	return true;
+}
+
+static void service_destroy_characteristics(bt_gatt_service_t *service)
+{
+	unsigned int i;
+
+	for (i = 0; i < service->num_chrcs; i++)
+		free((bt_gatt_descriptor_t *) service->chrcs[i].descs);
+
+	free((bt_gatt_characteristic_t *) service->chrcs);
+}
+
+static void gatt_client_clear_services(struct bt_gatt_client *client)
+{
+	struct service_list *l, *tmp;
+
+	l = client->svc_head;
+
+	while (l) {
+		service_destroy_characteristics(&l->service);
+		tmp = l;
+		l = tmp->next;
+		free(tmp);
+	}
+
+	client->svc_head = client->svc_tail = NULL;
+}
+
 struct bt_gatt_client *bt_gatt_client_new(struct bt_att *att)
 {
 	struct bt_gatt_client *client;
@@ -142,7 +206,7 @@ static void discover_primary_cb(bool success, uint8_t att_ecode,
 	struct bt_gatt_client *client = op->client;
 	struct bt_gatt_iter iter;
 	uint16_t start, end;
-	uint8_t uuid[16];
+	uint8_t uuid[BT_GATT_UUID_SIZE];
 	char uuid_str[MAX_LEN_UUID_STR];
 	bt_uuid_t tmp;
 
@@ -164,8 +228,6 @@ static void discover_primary_cb(bool success, uint8_t att_ecode,
 					bt_gatt_result_service_count(result));
 
 	while (bt_gatt_iter_next_service(&iter, &start, &end, uuid)) {
-		/* TODO: discover characteristics and store the service. */
-
 		/* Log debug message. */
 		tmp.type = BT_UUID128;
 		memcpy(tmp.value.u128.data, uuid, sizeof(uuid));
@@ -173,11 +235,25 @@ static void discover_primary_cb(bool success, uint8_t att_ecode,
 		util_debug(client->debug_callback, client->debug_data,
 				"start: 0x%04x, end: 0x%04x, uuid: %s",
 				start, end, uuid_str);
+
+		/* Store the service */
+		if (!gatt_client_add_service(client, start, end, uuid)) {
+			util_debug(client->debug_callback, client->debug_data,
+						"Failed to store service");
+
+			gatt_client_clear_services(client);
+
+			success = false;
+			goto done;
+		}
 	}
 
 done:
 	client->in_init = false;
 
+	if (success)
+		client->initialized = true;
+
 	if (op->callback)
 		op->callback(success, att_ecode, op->user_data);
 }
@@ -228,7 +304,7 @@ bool bt_gatt_client_init(struct bt_gatt_client *client, uint16_t mtu,
 {
 	struct async_op *op;
 
-	if (!client || client->in_init)
+	if (!client || client->in_init || client->initialized)
 		return false;
 
 	op = new0(struct async_op, 1);
@@ -254,6 +330,11 @@ bool bt_gatt_client_init(struct bt_gatt_client *client, uint16_t mtu,
 	return true;
 }
 
+bool bt_gatt_client_is_initialized(struct bt_gatt_client *client)
+{
+	return (client && client->initialized);
+}
+
 unsigned int bt_gatt_client_register_service_changed(
 				struct bt_gatt_client *client,
 				uint16_t handle,
@@ -271,3 +352,67 @@ bool bt_gatt_client_unregister_service_changed(struct bt_gatt_client *client,
 	// TODO
 	return false;
 }
+
+bool bt_gatt_service_iter_init(struct bt_gatt_service_iter *iter,
+						struct bt_gatt_client *client)
+{
+	if (!iter || !client)
+		return false;
+
+	if (client->in_init)
+		return false;
+
+	memset(iter, 0, sizeof(*iter));
+	iter->client = client;
+	iter->ptr = NULL;
+
+	return true;
+}
+
+bool bt_gatt_service_iter_next(struct bt_gatt_service_iter *iter,
+						bt_gatt_service_t *service)
+{
+	struct service_list *l;
+
+	if (!iter || !service)
+		return false;
+
+	l = iter->ptr;
+
+	if (!l)
+		l = iter->client->svc_head;
+	else
+		l = l->next;
+
+	if (!l)
+		return false;
+
+	*service = l->service;
+	iter->ptr = l;
+
+	return true;
+}
+
+bool bt_gatt_service_iter_next_by_handle(struct bt_gatt_service_iter *iter,
+						uint16_t start_handle,
+						bt_gatt_service_t *service)
+{
+	while (bt_gatt_service_iter_next(iter, service)) {
+		if (service->start_handle == start_handle)
+			return true;
+	}
+
+	return false;
+}
+
+bool bt_gatt_service_iter_next_by_uuid(struct bt_gatt_service_iter *iter,
+					const uint8_t uuid[BT_GATT_UUID_SIZE],
+					bt_gatt_service_t *service)
+{
+	while (bt_gatt_service_iter_next(iter, service)) {
+		if (memcmp(service->uuid, uuid, UUID_BYTES) == 0)
+			return true;
+	}
+
+	return false;
+}
diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
index 7a46cca..97033f8 100644
--- a/src/shared/gatt-client.h
+++ b/src/shared/gatt-client.h
@@ -23,6 +23,9 @@
 
 #include <stdbool.h>
 #include <stdint.h>
+#include <stddef.h>
+
+#define BT_GATT_UUID_SIZE 16
 
 struct bt_gatt_client;
 
@@ -50,6 +53,7 @@ bool bt_gatt_client_init(struct bt_gatt_client *client, uint16_t mtu,
 					bt_gatt_client_callback_t callback,
 					void *user_data,
 					bt_gatt_client_destroy_func_t destroy);
+bool bt_gatt_client_is_initialized(struct bt_gatt_client *client);
 
 unsigned int bt_gatt_client_register_service_changed(
 				struct bt_gatt_client *client,
@@ -59,3 +63,41 @@ unsigned int bt_gatt_client_register_service_changed(
 				bt_gatt_client_destroy_func_t destroy);
 bool bt_gatt_client_unregister_service_changed(struct bt_gatt_client *client,
 							unsigned int id);
+
+typedef struct {
+	uint16_t handle;
+	uint8_t uuid[BT_GATT_UUID_SIZE];
+} bt_gatt_descriptor_t;
+
+typedef struct {
+	uint16_t handle;
+	uint16_t value_handle;
+	uint8_t properties;
+	uint8_t uuid[BT_GATT_UUID_SIZE];
+	const bt_gatt_descriptor_t *descs;
+	size_t num_descs;
+} bt_gatt_characteristic_t;
+
+typedef struct {
+	uint16_t start_handle;
+	uint16_t end_handle;
+	uint8_t uuid[BT_GATT_UUID_SIZE];
+	const bt_gatt_characteristic_t *chrcs;
+	size_t num_chrcs;
+} bt_gatt_service_t;
+
+struct bt_gatt_service_iter {
+	struct bt_gatt_client *client;
+	void *ptr;
+};
+
+bool bt_gatt_service_iter_init(struct bt_gatt_service_iter *iter,
+						struct bt_gatt_client *client);
+bool bt_gatt_service_iter_next(struct bt_gatt_service_iter *iter,
+						bt_gatt_service_t *service);
+bool bt_gatt_service_iter_next_by_handle(struct bt_gatt_service_iter *iter,
+						uint16_t start_handle,
+						bt_gatt_service_t *service);
+bool bt_gatt_service_iter_next_by_uuid(struct bt_gatt_service_iter *iter,
+					const uint8_t uuid[BT_GATT_UUID_SIZE],
+					bt_gatt_service_t *service);
-- 
2.1.0.rc2.206.gedb03e5

[PATCH BlueZ 10/11] shared/gatt-client: Implement characteristic discovery.

[Arman Uguray]

This patch implements characteristic discovery as part of the client
initialization flow. The characteristics of each service are discovered in
order.
---
 src/shared/TODO          |   1 -
 src/shared/gatt-client.c | 142 ++++++++++++++++++++++++++++++++++++++++++-----
 src/shared/gatt-client.h |   3 +-
 3 files changed, 130 insertions(+), 16 deletions(-)

diff --git a/src/shared/TODO b/src/shared/TODO
index 8ffaacf..da4b95b 100644
--- a/src/shared/TODO
+++ b/src/shared/TODO
@@ -1,6 +1,5 @@
 TODOs for shared/gatt-client
 
-* Implement characteristic discovery.
 * Implement descriptor discovery.
 * Handle request timeouts.
 * Make bt_gatt_client observe disconnect events. Handle bonded/non-bonded cases.
diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index f25b5b7..c560534 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -172,6 +172,7 @@ bool bt_gatt_client_set_debug(struct bt_gatt_client *client,
 
 struct async_op {
 	struct bt_gatt_client *client;
+	struct service_list *cur_service;
 	int ref_count;
 	bt_gatt_client_callback_t callback;
 	void *user_data;
@@ -198,6 +199,113 @@ static void async_op_unref(void *data)
 	free(data);
 }
 
+static void async_op_complete(struct async_op *op, bool success,
+							uint8_t att_ecode)
+{
+	struct bt_gatt_client *client = op->client;
+
+	client->in_init = false;
+
+	if (success)
+		client->initialized = true;
+	else
+		gatt_client_clear_services(client);
+
+	if (op->callback)
+		op->callback(success, att_ecode, op->user_data);
+}
+
+static void uuid_to_string(const uint8_t uuid[BT_GATT_UUID_SIZE],
+						char str[MAX_LEN_UUID_STR])
+{
+	bt_uuid_t tmp;
+
+	tmp.type = BT_UUID128;
+	memcpy(tmp.value.u128.data, uuid, UUID_BYTES);
+	bt_uuid_to_string(&tmp, str, MAX_LEN_UUID_STR * sizeof(char));
+}
+
+static void discover_chrcs_cb(bool success, uint8_t att_ecode,
+						struct bt_gatt_result *result,
+						void *user_data)
+{
+	struct async_op *op = user_data;
+	struct bt_gatt_client *client = op->client;
+	struct bt_gatt_iter iter;
+	char uuid_str[MAX_LEN_UUID_STR];
+	unsigned int chrc_count;
+	unsigned int i;
+	bt_gatt_characteristic_t *chrcs;
+
+	if (!success) {
+		if (att_ecode == BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND) {
+			success = true;
+			goto next;
+		}
+
+		goto done;
+	}
+
+	if (!result || !bt_gatt_iter_init(&iter, result)) {
+		success = false;
+		goto done;
+	}
+
+	chrc_count = bt_gatt_result_characteristic_count(result);
+	util_debug(client->debug_callback, client->debug_data,
+				"Characteristics found: %u", chrc_count);
+
+	if (chrc_count == 0)
+		goto next;
+
+	chrcs = new0(bt_gatt_characteristic_t, chrc_count);
+	if (!chrcs) {
+		success = false;
+		goto done;
+	}
+
+	i = 0;
+	while (bt_gatt_iter_next_characteristic(&iter, &chrcs[i].start_handle,
+							&chrcs[i].end_handle,
+							&chrcs[i].value_handle,
+							&chrcs[i].properties,
+							chrcs[i].uuid)) {
+		uuid_to_string(chrcs[i].uuid, uuid_str);
+		util_debug(client->debug_callback, client->debug_data,
+				"start: 0x%04x, end: 0x%04x, value: 0x%04x, "
+				"props: 0x%02x, uuid: %s",
+				chrcs[i].start_handle, chrcs[i].end_handle,
+				chrcs[i].value_handle, chrcs[i].properties,
+				uuid_str);
+		i++;
+	}
+
+	op->cur_service->service.chrcs = chrcs;
+	op->cur_service->service.num_chrcs = chrc_count;
+
+next:
+	if (!op->cur_service->next)
+		goto done;
+
+	/* Move on to the next service */
+	op->cur_service = op->cur_service->next;
+	if (bt_gatt_discover_characteristics(client->att,
+					op->cur_service->service.start_handle,
+					op->cur_service->service.end_handle,
+					discover_chrcs_cb,
+					async_op_ref(op),
+					async_op_unref))
+		return;
+
+	util_debug(client->debug_callback, client->debug_data,
+				"Failed to start characteristic discovery");
+	async_op_unref(op);
+	success = false;
+
+done:
+	async_op_complete(op, success, att_ecode);
+}
+
 static void discover_primary_cb(bool success, uint8_t att_ecode,
 						struct bt_gatt_result *result,
 						void *user_data)
@@ -208,7 +316,6 @@ static void discover_primary_cb(bool success, uint8_t att_ecode,
 	uint16_t start, end;
 	uint8_t uuid[BT_GATT_UUID_SIZE];
 	char uuid_str[MAX_LEN_UUID_STR];
-	bt_uuid_t tmp;
 
 	if (!success) {
 		util_debug(client->debug_callback, client->debug_data,
@@ -219,7 +326,6 @@ static void discover_primary_cb(bool success, uint8_t att_ecode,
 
 	if (!result || !bt_gatt_iter_init(&iter, result)) {
 		success = false;
-		att_ecode = 0;
 		goto done;
 	}
 
@@ -229,9 +335,7 @@ static void discover_primary_cb(bool success, uint8_t att_ecode,
 
 	while (bt_gatt_iter_next_service(&iter, &start, &end, uuid)) {
 		/* Log debug message. */
-		tmp.type = BT_UUID128;
-		memcpy(tmp.value.u128.data, uuid, sizeof(uuid));
-		bt_uuid_to_string(&tmp, uuid_str, sizeof(uuid_str));
+		uuid_to_string(uuid, uuid_str);
 		util_debug(client->debug_callback, client->debug_data,
 				"start: 0x%04x, end: 0x%04x, uuid: %s",
 				start, end, uuid_str);
@@ -240,22 +344,32 @@ static void discover_primary_cb(bool success, uint8_t att_ecode,
 		if (!gatt_client_add_service(client, start, end, uuid)) {
 			util_debug(client->debug_callback, client->debug_data,
 						"Failed to store service");
-
-			gatt_client_clear_services(client);
-
 			success = false;
 			goto done;
 		}
 	}
 
-done:
-	client->in_init = false;
+	/* Complete the process if the service list is empty */
+	if (!client->svc_head)
+		goto done;
 
-	if (success)
-		client->initialized = true;
+	/* Sequentially discover the characteristics of all services */
+	op->cur_service = client->svc_head;
+	if (bt_gatt_discover_characteristics(client->att,
+					op->cur_service->service.start_handle,
+					op->cur_service->service.end_handle,
+					discover_chrcs_cb,
+					async_op_ref(op),
+					async_op_unref))
+		return;
 
-	if (op->callback)
-		op->callback(success, att_ecode, op->user_data);
+	util_debug(client->debug_callback, client->debug_data,
+				"Failed to start characteristic discovery");
+	async_op_unref(op);
+	success = false;
+
+done:
+	async_op_complete(op, success, att_ecode);
 }
 
 static void exchange_mtu_cb(bool success, uint8_t att_ecode, void *user_data)
diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
index 97033f8..4497465 100644
--- a/src/shared/gatt-client.h
+++ b/src/shared/gatt-client.h
@@ -70,7 +70,8 @@ typedef struct {
 } bt_gatt_descriptor_t;
 
 typedef struct {
-	uint16_t handle;
+	uint16_t start_handle;
+	uint16_t end_handle;
 	uint16_t value_handle;
 	uint8_t properties;
 	uint8_t uuid[BT_GATT_UUID_SIZE];
-- 
2.1.0.rc2.206.gedb03e5

[PATCH BlueZ 11/11] shared/gatt-client: Implement descriptor discovery.

[Arman Uguray]

This patch implements characteristic descriptor discovery as part of the client
initialization flow.
---
 src/shared/TODO          |   1 -
 src/shared/gatt-client.c | 131 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 131 insertions(+), 1 deletion(-)

diff --git a/src/shared/TODO b/src/shared/TODO
index da4b95b..92b3b59 100644
--- a/src/shared/TODO
+++ b/src/shared/TODO
@@ -1,6 +1,5 @@
 TODOs for shared/gatt-client
 
-* Implement descriptor discovery.
 * Handle request timeouts.
 * Make bt_gatt_client observe disconnect events. Handle bonded/non-bonded cases.
 * Add support for auto-connects using a different bt_att.
diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index c560534..81c8141 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -173,6 +173,8 @@ bool bt_gatt_client_set_debug(struct bt_gatt_client *client,
 struct async_op {
 	struct bt_gatt_client *client;
 	struct service_list *cur_service;
+	bt_gatt_characteristic_t *cur_chrc;
+	int cur_chrc_index;
 	int ref_count;
 	bt_gatt_client_callback_t callback;
 	void *user_data;
@@ -227,6 +229,112 @@ static void uuid_to_string(const uint8_t uuid[BT_GATT_UUID_SIZE],
 
 static void discover_chrcs_cb(bool success, uint8_t att_ecode,
 						struct bt_gatt_result *result,
+						void *user_data);
+
+static void discover_descs_cb(bool success, uint8_t att_ecode,
+						struct bt_gatt_result *result,
+						void *user_data)
+{
+	struct async_op *op = user_data;
+	struct bt_gatt_client *client = op->client;
+	struct bt_gatt_iter iter;
+	char uuid_str[MAX_LEN_UUID_STR];
+	unsigned int desc_count;
+	uint16_t desc_start;
+	unsigned int i;
+	bt_gatt_descriptor_t *descs;
+
+	if (!success) {
+		if (att_ecode == BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND) {
+			success = true;
+			goto next;
+		}
+
+		goto done;
+	}
+
+	if (!result || !bt_gatt_iter_init(&iter, result)) {
+		success = false;
+		goto done;
+	}
+
+	desc_count = bt_gatt_result_descriptor_count(result);
+	if (desc_count == 0) {
+		success = false;
+		goto done;
+	}
+
+	util_debug(client->debug_callback, client->debug_data,
+					"Descriptors found: %u", desc_count);
+
+	descs = new0(bt_gatt_descriptor_t, desc_count);
+	if (!descs) {
+		success = false;
+		goto done;
+	}
+
+	i = 0;
+	while (bt_gatt_iter_next_descriptor(&iter, &descs[i].handle,
+							descs[i].uuid)) {
+		uuid_to_string(descs[i].uuid, uuid_str);
+		util_debug(client->debug_callback, client->debug_data,
+						"handle: 0x%04x, uuid: %s",
+						descs[i].handle, uuid_str);
+		i++;
+	}
+
+	op->cur_chrc->num_descs = desc_count;
+	op->cur_chrc->descs = descs;
+
+	for (i = op->cur_chrc_index;
+				i < op->cur_service->service.num_chrcs; i++) {
+		op->cur_chrc_index = i;
+		op->cur_chrc++;
+		desc_start = op->cur_chrc->value_handle + 1;
+		if (desc_start > op->cur_chrc->end_handle)
+			continue;
+
+		if (bt_gatt_discover_descriptors(client->att,
+						desc_start,
+						op->cur_chrc->end_handle,
+						discover_descs_cb,
+						async_op_ref(op),
+						async_op_unref))
+			return;
+
+		util_debug(client->debug_callback, client->debug_data,
+					"Failed to start descriptor discovery");
+		async_op_unref(op);
+		success = false;
+
+		goto done;
+	}
+
+next:
+	if (!op->cur_service->next)
+		goto done;
+
+	/* Move on to the next service */
+	op->cur_service = op->cur_service->next;
+	if (bt_gatt_discover_characteristics(client->att,
+					op->cur_service->service.start_handle,
+					op->cur_service->service.end_handle,
+					discover_chrcs_cb,
+					async_op_ref(op),
+					async_op_unref))
+		return;
+
+	util_debug(client->debug_callback, client->debug_data,
+				"Failed to start characteristic discovery");
+	async_op_unref(op);
+	success = false;
+
+done:
+	async_op_complete(op, success, att_ecode);
+}
+
+static void discover_chrcs_cb(bool success, uint8_t att_ecode,
+						struct bt_gatt_result *result,
 						void *user_data)
 {
 	struct async_op *op = user_data;
@@ -235,6 +343,7 @@ static void discover_chrcs_cb(bool success, uint8_t att_ecode,
 	char uuid_str[MAX_LEN_UUID_STR];
 	unsigned int chrc_count;
 	unsigned int i;
+	uint16_t desc_start;
 	bt_gatt_characteristic_t *chrcs;
 
 	if (!success) {
@@ -283,6 +392,28 @@ static void discover_chrcs_cb(bool success, uint8_t att_ecode,
 	op->cur_service->service.chrcs = chrcs;
 	op->cur_service->service.num_chrcs = chrc_count;
 
+	for (i = 0; i < chrc_count; i++) {
+		op->cur_chrc_index = i;
+		op->cur_chrc = chrcs + i;
+		desc_start = chrcs[i].value_handle + 1;
+		if (desc_start > chrcs[i].end_handle)
+			continue;
+
+		if (bt_gatt_discover_descriptors(client->att,
+						desc_start, chrcs[i].end_handle,
+						discover_descs_cb,
+						async_op_ref(op),
+						async_op_unref))
+			return;
+
+		util_debug(client->debug_callback, client->debug_data,
+					"Failed to start descriptor discovery");
+		async_op_unref(op);
+		success = false;
+
+		goto done;
+	}
+
 next:
 	if (!op->cur_service->next)
 		goto done;
-- 
2.1.0.rc2.206.gedb03e5

Re: [PATCH BlueZ 03/11] shared/gatt-client: Added initial skeleton and simple functions.

[Luiz Augusto von Dentz]

Hi Arman,

On Wed, Aug 20, 2014 at 11:15 PM, Arman Uguray <armansito@chromium.org> wrote:
> This patch introduces shared/gatt-client, which provides a central/client side
> implementation of the Generic Attribute Profile. An instance of bt_gatt_client
> will provide a central database of all GATT services, characteristics, and
> descriptors that have been discovered on a peripheral and will provide API
> functions to obtain information about discovered attributes, registering
> handlers for "Service Changed" indications, as well as providing
> reference-counted access to "Client Characteristic Configuration" descriptors.
> ---
>  Makefile.am              |   3 +-
>  src/shared/TODO          |  13 ++++++
>  src/shared/gatt-client.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++
>  src/shared/gatt-client.h |  54 +++++++++++++++++++++++++
>  4 files changed, 170 insertions(+), 1 deletion(-)
>  create mode 100644 src/shared/TODO
>  create mode 100644 src/shared/gatt-client.c
>  create mode 100644 src/shared/gatt-client.h
>
> diff --git a/Makefile.am b/Makefile.am
> index 88fcb49..6aed8e2 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -158,7 +158,8 @@ src_bluetoothd_SOURCES = $(builtin_sources) \
>                         src/shared/util.h src/shared/util.c \
>                         src/shared/mgmt.h src/shared/mgmt.c \
>                         src/shared/att-types.h src/shared/att.h src/shared/att.c \
> -                       src/shared/gatt-helpers.h src/shared/gatt-helpers.c
> +                       src/shared/gatt-helpers.h src/shared/gatt-helpers.c \
> +                       src/shared/gatt-client.h src/shared/gatt-client.c
>  src_bluetoothd_LDADD = lib/libbluetooth-internal.la gdbus/libgdbus-internal.la \
>                         @GLIB_LIBS@ @DBUS_LIBS@ -ldl -lrt
>  src_bluetoothd_LDFLAGS = $(AM_LDFLAGS) -Wl,--export-dynamic \
> diff --git a/src/shared/TODO b/src/shared/TODO
> new file mode 100644
> index 0000000..2881c97
> --- /dev/null
> +++ b/src/shared/TODO
> @@ -0,0 +1,13 @@
> +TODOs for shared/gatt-client
> +
> +* Add util_debug support.
> +* Add high-level abstraction for services, characteristics, and descriptors.
> +* Implement service discovery.
> +* Implement characteristic discovery.
> +* Implement descriptor discovery.
> +* Handle request timeouts.
> +* Make bt_gatt_client observe disconnect events. Handle bonded/non-bonded cases.
> +* Add support for auto-connects using a different bt_att.
> +* Add functions for reference counted access to enable/disable
> +  notifications/indications.
> +* Add support for "Service Changed".
> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
> new file mode 100644
> index 0000000..53a7de1
> --- /dev/null
> +++ b/src/shared/gatt-client.c
> @@ -0,0 +1,101 @@
> +/*
> + *
> + *  BlueZ - Bluetooth protocol stack for Linux
> + *
> + *  Copyright (C) 2014  Google Inc.
> + *
> + *
> + *  This library is free software; you can redistribute it and/or
> + *  modify it under the terms of the GNU Lesser General Public
> + *  License as published by the Free Software Foundation; either
> + *  version 2.1 of the License, or (at your option) any later version.
> + *
> + *  This library is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + *  Lesser General Public License for more details.
> + *
> + *  You should have received a copy of the GNU Lesser General Public
> + *  License along with this library; if not, write to the Free Software
> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + *
> + */
> +
> +#include "src/shared/att.h"
> +#include "src/shared/gatt-client.h"
> +#include "src/shared/util.h"
> +
> +struct bt_gatt_client {
> +       struct bt_att *att;
> +       bool persist;
> +};
> +
> +struct bt_gatt_client *bt_gatt_client_new(struct bt_att *att)
> +{
> +       struct bt_gatt_client *client;
> +
> +       if (!att)
> +               return NULL;
> +
> +       client = new0(struct bt_gatt_client, 1);
> +       if (!client)
> +               return NULL;
> +
> +       client->att = bt_att_ref(att);
> +
> +       return client;
> +}
> +
> +void bt_gatt_client_destroy(struct bt_gatt_client *client)
> +{
> +       if (!client)
> +               return;
> +
> +       bt_att_unref(client->att);
> +       free(client);
> +}
> +
> +bool bt_gatt_client_set_persist(struct bt_gatt_client *client,
> +                                                       bool do_persist)
> +{
> +       if (!client)
> +               return false;
> +
> +       client->persist = do_persist;
> +
> +       return true;
> +}
> +
> +struct bt_att *bt_gatt_client_get_att(struct bt_gatt_client *client)
> +{
> +       if (!client)
> +               return NULL;
> +
> +       return client->att;
> +}
> +
> +bool bt_gatt_client_init(struct bt_gatt_client *client, uint16_t mtu,
> +                                       bt_gatt_client_callback_t callback,
> +                                       void *user_data,
> +                                       bt_gatt_client_destroy_func_t destroy)
> +{
> +       // TODO
> +}
> +
> +unsigned int bt_gatt_client_register_service_changed(
> +                               struct bt_gatt_client *client,
> +                               uint16_t handle,
> +                               bt_gatt_client_service_changed_func_t callback,
> +                               void *user_data,
> +                               bt_gatt_client_destroy_func_t destroy)
> +{
> +       // TODO
> +       return 0;
> +}
> +
> +bool bt_gatt_client_unregister_service_changed(struct bt_gatt_client *client,
> +                                                       unsigned int id)
> +{
> +       // TODO
> +       return false;
> +}
> diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
> new file mode 100644
> index 0000000..a978691
> --- /dev/null
> +++ b/src/shared/gatt-client.h
> @@ -0,0 +1,54 @@
> +/*
> + *
> + *  BlueZ - Bluetooth protocol stack for Linux
> + *
> + *  Copyright (C) 2014  Google Inc.
> + *
> + *
> + *  This library is free software; you can redistribute it and/or
> + *  modify it under the terms of the GNU Lesser General Public
> + *  License as published by the Free Software Foundation; either
> + *  version 2.1 of the License, or (at your option) any later version.
> + *
> + *  This library is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + *  Lesser General Public License for more details.
> + *
> + *  You should have received a copy of the GNU Lesser General Public
> + *  License along with this library; if not, write to the Free Software
> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + *
> + */
> +
> +#include <stdbool.h>
> +#include <stdint.h>
> +
> +struct bt_gatt_client;
> +
> +struct bt_gatt_client *bt_gatt_client_new(struct bt_att *att);
> +void bt_gatt_client_destroy(struct bt_gatt_client *client);
> +
> +bool bt_gatt_client_set_persist(struct bt_gatt_client *client,
> +                                                       bool do_persist);
> +
> +struct bt_att *bt_gatt_client_get_att(struct bt_gatt_client *client);
> +
> +typedef void (*bt_gatt_client_destroy_func_t)(void *user_data);
> +typedef void (*bt_gatt_client_callback_t)(void *user_data);
> +typedef void (*bt_gatt_client_service_changed_func_t)(uint16_t handle,
> +                                                       void *user_data);
> +
> +bool bt_gatt_client_init(struct bt_gatt_client *client, uint16_t mtu,
> +                                       bt_gatt_client_callback_t callback,
> +                                       void *user_data,
> +                                       bt_gatt_client_destroy_func_t destroy);
> +
> +unsigned int bt_gatt_client_register_service_changed(
> +                               struct bt_gatt_client *client,
> +                               uint16_t handle,
> +                               bt_gatt_client_service_changed_func_t callback,
> +                               void *user_data,
> +                               bt_gatt_client_destroy_func_t destroy);
> +bool bt_gatt_client_unregister_service_changed(struct bt_gatt_client *client,
> +                                                       unsigned int id);
> --
> 2.1.0.rc2.206.gedb03e5

Perhaps I lost track during my holidays but I was expecting a very
different API, one that does not expose bt_att instance to begin with,
it seems like the user will need to managed both bt_att instance and
bt_gatt_client instance at same time? It does seems more logical to me
that gatt-helper and gatt-client should be part of the same API using
the same instance (e.g. bt_gatt), with this type of design you can do
a proper queuing if one is necessary like for example detect if an
operation is already ongoing and just attach the callback and user
data to the existing operation instead of queuing another command in
bt_att.


-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ 03/11] shared/gatt-client: Added initial skeleton and simple functions.

[Arman Uguray]

HI Luiz,

I'm fine with merging some of the gatt-helpers with gatt-client later;
so that gatt-helpers would only be used internally for discovery and
all the read/write functions can accept a bt_gatt_client instead of a
bt_att directly. And, the bt_gatt_client_get_att function could go
away.

I'm building this one step at a time and wanted to get the general
GATT procedures out of the way first; hence I sent out gatt-helpers
before gatt-client.

Let me know if that makes sense.

On Thu, Aug 21, 2014 at 6:23 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Arman,
>
> On Wed, Aug 20, 2014 at 11:15 PM, Arman Uguray <armansito@chromium.org> wrote:
>> This patch introduces shared/gatt-client, which provides a central/client side
>> implementation of the Generic Attribute Profile. An instance of bt_gatt_client
>> will provide a central database of all GATT services, characteristics, and
>> descriptors that have been discovered on a peripheral and will provide API
>> functions to obtain information about discovered attributes, registering
>> handlers for "Service Changed" indications, as well as providing
>> reference-counted access to "Client Characteristic Configuration" descriptors.
>> ---
>>  Makefile.am              |   3 +-
>>  src/shared/TODO          |  13 ++++++
>>  src/shared/gatt-client.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++
>>  src/shared/gatt-client.h |  54 +++++++++++++++++++++++++
>>  4 files changed, 170 insertions(+), 1 deletion(-)
>>  create mode 100644 src/shared/TODO
>>  create mode 100644 src/shared/gatt-client.c
>>  create mode 100644 src/shared/gatt-client.h
>>
>> diff --git a/Makefile.am b/Makefile.am
>> index 88fcb49..6aed8e2 100644
>> --- a/Makefile.am
>> +++ b/Makefile.am
>> @@ -158,7 +158,8 @@ src_bluetoothd_SOURCES = $(builtin_sources) \
>>                         src/shared/util.h src/shared/util.c \
>>                         src/shared/mgmt.h src/shared/mgmt.c \
>>                         src/shared/att-types.h src/shared/att.h src/shared/att.c \
>> -                       src/shared/gatt-helpers.h src/shared/gatt-helpers.c
>> +                       src/shared/gatt-helpers.h src/shared/gatt-helpers.c \
>> +                       src/shared/gatt-client.h src/shared/gatt-client.c
>>  src_bluetoothd_LDADD = lib/libbluetooth-internal.la gdbus/libgdbus-internal.la \
>>                         @GLIB_LIBS@ @DBUS_LIBS@ -ldl -lrt
>>  src_bluetoothd_LDFLAGS = $(AM_LDFLAGS) -Wl,--export-dynamic \
>> diff --git a/src/shared/TODO b/src/shared/TODO
>> new file mode 100644
>> index 0000000..2881c97
>> --- /dev/null
>> +++ b/src/shared/TODO
>> @@ -0,0 +1,13 @@
>> +TODOs for shared/gatt-client
>> +
>> +* Add util_debug support.
>> +* Add high-level abstraction for services, characteristics, and descriptors.
>> +* Implement service discovery.
>> +* Implement characteristic discovery.
>> +* Implement descriptor discovery.
>> +* Handle request timeouts.
>> +* Make bt_gatt_client observe disconnect events. Handle bonded/non-bonded cases.
>> +* Add support for auto-connects using a different bt_att.
>> +* Add functions for reference counted access to enable/disable
>> +  notifications/indications.
>> +* Add support for "Service Changed".
>> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
>> new file mode 100644
>> index 0000000..53a7de1
>> --- /dev/null
>> +++ b/src/shared/gatt-client.c
>> @@ -0,0 +1,101 @@
>> +/*
>> + *
>> + *  BlueZ - Bluetooth protocol stack for Linux
>> + *
>> + *  Copyright (C) 2014  Google Inc.
>> + *
>> + *
>> + *  This library is free software; you can redistribute it and/or
>> + *  modify it under the terms of the GNU Lesser General Public
>> + *  License as published by the Free Software Foundation; either
>> + *  version 2.1 of the License, or (at your option) any later version.
>> + *
>> + *  This library is distributed in the hope that it will be useful,
>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + *  Lesser General Public License for more details.
>> + *
>> + *  You should have received a copy of the GNU Lesser General Public
>> + *  License along with this library; if not, write to the Free Software
>> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
>> + *
>> + */
>> +
>> +#include "src/shared/att.h"
>> +#include "src/shared/gatt-client.h"
>> +#include "src/shared/util.h"
>> +
>> +struct bt_gatt_client {
>> +       struct bt_att *att;
>> +       bool persist;
>> +};
>> +
>> +struct bt_gatt_client *bt_gatt_client_new(struct bt_att *att)
>> +{
>> +       struct bt_gatt_client *client;
>> +
>> +       if (!att)
>> +               return NULL;
>> +
>> +       client = new0(struct bt_gatt_client, 1);
>> +       if (!client)
>> +               return NULL;
>> +
>> +       client->att = bt_att_ref(att);
>> +
>> +       return client;
>> +}
>> +
>> +void bt_gatt_client_destroy(struct bt_gatt_client *client)
>> +{
>> +       if (!client)
>> +               return;
>> +
>> +       bt_att_unref(client->att);
>> +       free(client);
>> +}
>> +
>> +bool bt_gatt_client_set_persist(struct bt_gatt_client *client,
>> +                                                       bool do_persist)
>> +{
>> +       if (!client)
>> +               return false;
>> +
>> +       client->persist = do_persist;
>> +
>> +       return true;
>> +}
>> +
>> +struct bt_att *bt_gatt_client_get_att(struct bt_gatt_client *client)
>> +{
>> +       if (!client)
>> +               return NULL;
>> +
>> +       return client->att;
>> +}
>> +
>> +bool bt_gatt_client_init(struct bt_gatt_client *client, uint16_t mtu,
>> +                                       bt_gatt_client_callback_t callback,
>> +                                       void *user_data,
>> +                                       bt_gatt_client_destroy_func_t destroy)
>> +{
>> +       // TODO
>> +}
>> +
>> +unsigned int bt_gatt_client_register_service_changed(
>> +                               struct bt_gatt_client *client,
>> +                               uint16_t handle,
>> +                               bt_gatt_client_service_changed_func_t callback,
>> +                               void *user_data,
>> +                               bt_gatt_client_destroy_func_t destroy)
>> +{
>> +       // TODO
>> +       return 0;
>> +}
>> +
>> +bool bt_gatt_client_unregister_service_changed(struct bt_gatt_client *client,
>> +                                                       unsigned int id)
>> +{
>> +       // TODO
>> +       return false;
>> +}
>> diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
>> new file mode 100644
>> index 0000000..a978691
>> --- /dev/null
>> +++ b/src/shared/gatt-client.h
>> @@ -0,0 +1,54 @@
>> +/*
>> + *
>> + *  BlueZ - Bluetooth protocol stack for Linux
>> + *
>> + *  Copyright (C) 2014  Google Inc.
>> + *
>> + *
>> + *  This library is free software; you can redistribute it and/or
>> + *  modify it under the terms of the GNU Lesser General Public
>> + *  License as published by the Free Software Foundation; either
>> + *  version 2.1 of the License, or (at your option) any later version.
>> + *
>> + *  This library is distributed in the hope that it will be useful,
>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + *  Lesser General Public License for more details.
>> + *
>> + *  You should have received a copy of the GNU Lesser General Public
>> + *  License along with this library; if not, write to the Free Software
>> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
>> + *
>> + */
>> +
>> +#include <stdbool.h>
>> +#include <stdint.h>
>> +
>> +struct bt_gatt_client;
>> +
>> +struct bt_gatt_client *bt_gatt_client_new(struct bt_att *att);
>> +void bt_gatt_client_destroy(struct bt_gatt_client *client);
>> +
>> +bool bt_gatt_client_set_persist(struct bt_gatt_client *client,
>> +                                                       bool do_persist);
>> +
>> +struct bt_att *bt_gatt_client_get_att(struct bt_gatt_client *client);
>> +
>> +typedef void (*bt_gatt_client_destroy_func_t)(void *user_data);
>> +typedef void (*bt_gatt_client_callback_t)(void *user_data);
>> +typedef void (*bt_gatt_client_service_changed_func_t)(uint16_t handle,
>> +                                                       void *user_data);
>> +
>> +bool bt_gatt_client_init(struct bt_gatt_client *client, uint16_t mtu,
>> +                                       bt_gatt_client_callback_t callback,
>> +                                       void *user_data,
>> +                                       bt_gatt_client_destroy_func_t destroy);
>> +
>> +unsigned int bt_gatt_client_register_service_changed(
>> +                               struct bt_gatt_client *client,
>> +                               uint16_t handle,
>> +                               bt_gatt_client_service_changed_func_t callback,
>> +                               void *user_data,
>> +                               bt_gatt_client_destroy_func_t destroy);
>> +bool bt_gatt_client_unregister_service_changed(struct bt_gatt_client *client,
>> +                                                       unsigned int id);
>> --
>> 2.1.0.rc2.206.gedb03e5
>
> Perhaps I lost track during my holidays but I was expecting a very
> different API, one that does not expose bt_att instance to begin with,
> it seems like the user will need to managed both bt_att instance and
> bt_gatt_client instance at same time? It does seems more logical to me
> that gatt-helper and gatt-client should be part of the same API using
> the same instance (e.g. bt_gatt), with this type of design you can do
> a proper queuing if one is necessary like for example detect if an
> operation is already ongoing and just attach the callback and user
> data to the existing operation instead of queuing another command in
> bt_att.
>
>
> --
> Luiz Augusto von Dentz

Re: [PATCH BlueZ 04/11] shared/att: Support multiple disconnect handlers.

[Marcel Holtmann]

Hi Arman,

> This patch adds support for registering multiple disconnect handlers with an
> instance of struct bt_att. Unregistering is achieved via a new function AND
> through bt_att_unregister_all and all disconnect callbacks get automatically
> unregistered after a single disconnect event.
> ---
> src/shared/att.c | 155 ++++++++++++++++++++++++++++++++++++++++++++++---------
> src/shared/att.h |   9 ++--
> 2 files changed, 137 insertions(+), 27 deletions(-)
> 
> diff --git a/src/shared/att.c b/src/shared/att.c
> index b5ab742..68a57f9 100644
> --- a/src/shared/att.c
> +++ b/src/shared/att.c
> @@ -60,6 +60,10 @@ struct bt_att {
> 	bool in_notify;
> 	bool need_notify_cleanup;
> 
> +	struct queue *disconn_list;	/* List of disconnect handlers */
> +	bool in_disconn;
> +	bool need_disconn_cleanup;
> +
> 	uint8_t *buf;
> 	uint16_t mtu;
> 
> @@ -73,10 +77,6 @@ struct bt_att {
> 	bt_att_debug_func_t debug_callback;
> 	bt_att_destroy_func_t debug_destroy;
> 	void *debug_data;
> -
> -	bt_att_disconnect_func_t disconn_callback;
> -	bt_att_destroy_func_t disconn_destroy;
> -	void *disconn_data;
> };
> 
> enum att_op_type {
> @@ -233,6 +233,46 @@ static void mark_notify_removed(void *data, void *user_data)
> 	notify->removed = true;
> }
> 
> +struct att_disconn {
> +	unsigned int id;
> +	bool removed;
> +	bt_att_disconnect_func_t callback;
> +	bt_att_destroy_func_t destroy;
> +	void *user_data;
> +};
> +
> +static void destroy_att_disconn(void *data)
> +{
> +	struct att_disconn *disconn = data;
> +
> +	if (disconn->destroy)
> +		disconn->destroy(disconn->user_data);
> +
> +	free(disconn);
> +}
> +
> +static bool match_disconn_id(const void *a, const void *b)
> +{
> +	const struct att_disconn *disconn = a;
> +	unsigned int id = PTR_TO_UINT(b);
> +
> +	return disconn->id == id;
> +}
> +
> +static bool match_disconn_removed(const void *a, const void *b)
> +{
> +	const struct att_disconn *disconn = a;
> +
> +	return disconn->removed;
> +}
> +
> +static void mark_disconn_removed(void *data, void *user_data)
> +{
> +	struct att_disconn *disconn = data;
> +
> +	disconn->removed = true;
> +}
> +
> static bool encode_pdu(struct att_send_op *op, const void *pdu,
> 						uint16_t length, uint16_t mtu)
> {
> @@ -605,21 +645,42 @@ static bool can_read_data(struct io *io, void *user_data)
> 	return true;
> }
> 
> +static void disconn_handler(void *data, void *user_data)
> +{
> +	struct att_disconn *disconn = data;
> +
> +	if (disconn->removed)
> +		return;
> +
> +	if (disconn->callback)
> +		disconn->callback(disconn->user_data);
> +}
> +
> static bool disconnect_cb(struct io *io, void *user_data)
> {
> 	struct bt_att *att = user_data;
> 
> -	bt_att_cancel_all(att);
> -	bt_att_unregister_all(att);
> -
> 	io_destroy(att->io);
> 	att->io = NULL;
> 
> 	util_debug(att->debug_callback, att->debug_data,
> 						"Physical link disconnected");
> 
> -	if (att->disconn_callback)
> -		att->disconn_callback(att->disconn_data);
> +	bt_att_ref(att);
> +	att->in_disconn = true;
> +	queue_foreach(att->disconn_list, disconn_handler, NULL);
> +	att->in_disconn = false;
> +
> +	if (att->need_disconn_cleanup) {
> +		queue_remove_all(att->disconn_list, match_disconn_removed, NULL,
> +							destroy_att_disconn);
> +		att->need_disconn_cleanup = false;
> +	}
> +
> +	bt_att_cancel_all(att);
> +	bt_att_unregister_all(att);
> +
> +	bt_att_unref(att);
> 
> 	return false;
> }
> @@ -662,6 +723,10 @@ struct bt_att *bt_att_new(int fd)
> 	if (!att->notify_list)
> 		goto fail;
> 
> +	att->disconn_list = queue_new();
> +	if (!att->disconn_list)
> +		goto fail;
> +
> 	if (!io_set_read_handler(att->io, can_read_data, att, NULL))
> 		goto fail;
> 
> @@ -674,6 +739,8 @@ fail:
> 	queue_destroy(att->req_queue, NULL);
> 	queue_destroy(att->ind_queue, NULL);
> 	queue_destroy(att->write_queue, NULL);
> +	queue_destroy(att->notify_list, NULL);
> +	queue_destroy(att->disconn_list, NULL);
> 	io_destroy(att->io);
> 	free(att->buf);
> 	free(att);
> @@ -709,6 +776,7 @@ void bt_att_unref(struct bt_att *att)
> 	queue_destroy(att->ind_queue, NULL);
> 	queue_destroy(att->write_queue, NULL);
> 	queue_destroy(att->notify_list, NULL);
> +	queue_destroy(att->disconn_list, NULL);
> 	att->req_queue = NULL;
> 	att->ind_queue = NULL;
> 	att->write_queue = NULL;
> @@ -720,9 +788,6 @@ void bt_att_unref(struct bt_att *att)
> 	if (att->debug_destroy)
> 		att->debug_destroy(att->debug_data);
> 
> -	if (att->disconn_destroy)
> -		att->disconn_destroy(att->disconn_data);
> -
> 	free(att->buf);
> 	att->buf = NULL;
> 
> @@ -800,20 +865,57 @@ bool bt_att_set_timeout_cb(struct bt_att *att, bt_att_timeout_func_t callback,
> 	return true;
> }
> 
> -bool bt_att_set_disconnect_cb(struct bt_att *att,
> +unsigned int bt_att_register_disconnect_handler(struct bt_att *att,
> 					bt_att_disconnect_func_t callback,
> 					void *user_data,
> 					bt_att_destroy_func_t destroy)
> {
> -	if (!att)
> +	struct att_disconn *disconn;
> +
> +	if (!att || !att->io)
> +		return 0;
> +
> +	disconn = new0(struct att_disconn, 1);
> +	if (!disconn)
> +		return 0;
> +
> +	disconn->callback = callback;
> +	disconn->destroy = destroy;
> +	disconn->user_data = user_data;
> +
> +	if (att->next_reg_id < 1)
> +		att->next_reg_id = 1;
> +
> +	disconn->id = att->next_reg_id++;
> +
> +	if (!queue_push_tail(att->disconn_list, disconn)) {
> +		free(disconn);
> +		return 0;
> +	}
> +
> +	return disconn->id;
> +}
> +
> +bool bt_att_unregister_disconnect_handler(struct bt_att *att, unsigned int id)
> +{
> +	struct att_disconn *disconn;
> +
> +	if (!att || !id)
> 		return false;
> 
> -	if (att->disconn_destroy)
> -		att->disconn_destroy(att->disconn_data);
> +	disconn = queue_find(att->disconn_list, match_disconn_id,
> +							UINT_TO_PTR(id));
> +	if (!disconn)
> +		return false;
> +
> +	if (!att->in_disconn) {
> +		queue_remove(att->disconn_list, disconn);
> +		destroy_att_disconn(disconn);
> +		return true;
> +	}
> 
> -	att->disconn_callback = callback;
> -	att->disconn_destroy = destroy;
> -	att->disconn_data = user_data;
> +	disconn->removed = true;
> +	att->need_disconn_cleanup = true;
> 
> 	return true;
> }
> @@ -996,14 +1098,21 @@ bool bt_att_unregister_all(struct bt_att *att)
> 	if (!att)
> 		return false;
> 
> -	if (!att->in_notify) {
> +	if (att->in_notify) {
> +		queue_foreach(att->notify_list, mark_notify_removed, NULL);
> +		att->need_notify_cleanup = true;
> +	} else {
> 		queue_remove_all(att->notify_list, NULL, NULL,
> 							destroy_att_notify);
> -		return true;
> 	}
> 
> -	queue_foreach(att->notify_list, mark_notify_removed, NULL);
> -	att->need_notify_cleanup = true;
> +	if (att->in_disconn) {
> +		queue_foreach(att->disconn_list, mark_disconn_removed, NULL);
> +		att->need_disconn_cleanup = true;
> +	} else {
> +		queue_remove_all(att->disconn_list, NULL, NULL,
> +							destroy_att_disconn);
> +	}
> 
> 	return true;
> }
> diff --git a/src/shared/att.h b/src/shared/att.h
> index cf44704..3180d2c 100644
> --- a/src/shared/att.h
> +++ b/src/shared/att.h
> @@ -54,10 +54,6 @@ bool bt_att_set_mtu(struct bt_att *att, uint16_t mtu);
> bool bt_att_set_timeout_cb(struct bt_att *att, bt_att_timeout_func_t callback,
> 						void *user_data,
> 						bt_att_destroy_func_t destroy);
> -bool bt_att_set_disconnect_cb(struct bt_att *att,
> -					bt_att_disconnect_func_t callback,
> -					void *user_data,
> -					bt_att_destroy_func_t destroy);
> 
> unsigned int bt_att_send(struct bt_att *att, uint8_t opcode,
> 					const void *pdu, uint16_t length,
> @@ -72,4 +68,9 @@ unsigned int bt_att_register(struct bt_att *att, uint8_t opcode,
> 						void *user_data,
> 						bt_att_destroy_func_t destroy);
> bool bt_att_unregister(struct bt_att *att, unsigned int id);
> +unsigned int bt_att_register_disconnect_handler(struct bt_att *att,
> +					bt_att_disconnect_func_t callback,
> +					void *user_data,
> +					bt_att_destroy_func_t destroy);
> +bool bt_att_unregister_disconnect_handler(struct bt_att *att, unsigned int id);

lets just make it bt_att_{register,unregister}_disconnect here. At some point we need to try to avoid super long function names.

Regards

Marcel

Re: [PATCH BlueZ 03/11] shared/gatt-client: Added initial skeleton and simple functions.

[Marcel Holtmann]

Hi Arman,

> This patch introduces shared/gatt-client, which provides a central/client side
> implementation of the Generic Attribute Profile. An instance of bt_gatt_client
> will provide a central database of all GATT services, characteristics, and
> descriptors that have been discovered on a peripheral and will provide API
> functions to obtain information about discovered attributes, registering
> handlers for "Service Changed" indications, as well as providing
> reference-counted access to "Client Characteristic Configuration" descriptors.
> ---
> Makefile.am              |   3 +-
> src/shared/TODO          |  13 ++++++
> src/shared/gatt-client.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++
> src/shared/gatt-client.h |  54 +++++++++++++++++++++++++
> 4 files changed, 170 insertions(+), 1 deletion(-)
> create mode 100644 src/shared/TODO
> create mode 100644 src/shared/gatt-client.c
> create mode 100644 src/shared/gatt-client.h
> 
> diff --git a/Makefile.am b/Makefile.am
> index 88fcb49..6aed8e2 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -158,7 +158,8 @@ src_bluetoothd_SOURCES = $(builtin_sources) \
> 			src/shared/util.h src/shared/util.c \
> 			src/shared/mgmt.h src/shared/mgmt.c \
> 			src/shared/att-types.h src/shared/att.h src/shared/att.c \
> -			src/shared/gatt-helpers.h src/shared/gatt-helpers.c
> +			src/shared/gatt-helpers.h src/shared/gatt-helpers.c \
> +			src/shared/gatt-client.h src/shared/gatt-client.c
> src_bluetoothd_LDADD = lib/libbluetooth-internal.la gdbus/libgdbus-internal.la \
> 			@GLIB_LIBS@ @DBUS_LIBS@ -ldl -lrt
> src_bluetoothd_LDFLAGS = $(AM_LDFLAGS) -Wl,--export-dynamic \
> diff --git a/src/shared/TODO b/src/shared/TODO
> new file mode 100644
> index 0000000..2881c97
> --- /dev/null
> +++ b/src/shared/TODO
> @@ -0,0 +1,13 @@
> +TODOs for shared/gatt-client
> +
> +* Add util_debug support.
> +* Add high-level abstraction for services, characteristics, and descriptors.
> +* Implement service discovery.
> +* Implement characteristic discovery.
> +* Implement descriptor discovery.
> +* Handle request timeouts.
> +* Make bt_gatt_client observe disconnect events. Handle bonded/non-bonded cases.
> +* Add support for auto-connects using a different bt_att.
> +* Add functions for reference counted access to enable/disable
> +  notifications/indications.
> +* Add support for "Service Changed".

great idea with the TODO. However please just use top-level TODO and patches modifying the TODO should be all by itself.

> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
> new file mode 100644
> index 0000000..53a7de1
> --- /dev/null
> +++ b/src/shared/gatt-client.c
> @@ -0,0 +1,101 @@
> +/*
> + *
> + *  BlueZ - Bluetooth protocol stack for Linux
> + *
> + *  Copyright (C) 2014  Google Inc.
> + *
> + *
> + *  This library is free software; you can redistribute it and/or
> + *  modify it under the terms of the GNU Lesser General Public
> + *  License as published by the Free Software Foundation; either
> + *  version 2.1 of the License, or (at your option) any later version.
> + *
> + *  This library is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + *  Lesser General Public License for more details.
> + *
> + *  You should have received a copy of the GNU Lesser General Public
> + *  License along with this library; if not, write to the Free Software
> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + *
> + */
> +
> +#include "src/shared/att.h"
> +#include "src/shared/gatt-client.h"
> +#include "src/shared/util.h"
> +
> +struct bt_gatt_client {
> +	struct bt_att *att;
> +	bool persist;
> +};
> +
> +struct bt_gatt_client *bt_gatt_client_new(struct bt_att *att)
> +{
> +	struct bt_gatt_client *client;
> +
> +	if (!att)
> +		return NULL;
> +
> +	client = new0(struct bt_gatt_client, 1);
> +	if (!client)
> +		return NULL;
> +
> +	client->att = bt_att_ref(att);
> +
> +	return client;
> +}
> +
> +void bt_gatt_client_destroy(struct bt_gatt_client *client)
> +{
> +	if (!client)
> +		return;
> +
> +	bt_att_unref(client->att);
> +	free(client);
> +}
> +
> +bool bt_gatt_client_set_persist(struct bt_gatt_client *client,
> +							bool do_persist)
> +{
> +	if (!client)
> +		return false;
> +
> +	client->persist = do_persist;
> +
> +	return true;
> +}
> +
> +struct bt_att *bt_gatt_client_get_att(struct bt_gatt_client *client)
> +{
> +	if (!client)
> +		return NULL;
> +
> +	return client->att;
> +}
> +
> +bool bt_gatt_client_init(struct bt_gatt_client *client, uint16_t mtu,
> +					bt_gatt_client_callback_t callback,
> +					void *user_data,
> +					bt_gatt_client_destroy_func_t destroy)
> +{
> +	// TODO
> +}
> +
> +unsigned int bt_gatt_client_register_service_changed(
> +				struct bt_gatt_client *client,
> +				uint16_t handle,
> +				bt_gatt_client_service_changed_func_t callback,
> +				void *user_data,
> +				bt_gatt_client_destroy_func_t destroy)
> +{
> +	// TODO
> +	return 0;
> +}
> +
> +bool bt_gatt_client_unregister_service_changed(struct bt_gatt_client *client,
> +							unsigned int id)
> +{
> +	// TODO
> +	return false;
> +}
> diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
> new file mode 100644
> index 0000000..a978691
> --- /dev/null
> +++ b/src/shared/gatt-client.h
> @@ -0,0 +1,54 @@
> +/*
> + *
> + *  BlueZ - Bluetooth protocol stack for Linux
> + *
> + *  Copyright (C) 2014  Google Inc.
> + *
> + *
> + *  This library is free software; you can redistribute it and/or
> + *  modify it under the terms of the GNU Lesser General Public
> + *  License as published by the Free Software Foundation; either
> + *  version 2.1 of the License, or (at your option) any later version.
> + *
> + *  This library is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + *  Lesser General Public License for more details.
> + *
> + *  You should have received a copy of the GNU Lesser General Public
> + *  License along with this library; if not, write to the Free Software
> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + *
> + */
> +
> +#include <stdbool.h>
> +#include <stdint.h>
> +
> +struct bt_gatt_client;
> +
> +struct bt_gatt_client *bt_gatt_client_new(struct bt_att *att);
> +void bt_gatt_client_destroy(struct bt_gatt_client *client);

lets keep using bt_gatt_client_{ref,unref} here as well. No _destroy please.

> +
> +bool bt_gatt_client_set_persist(struct bt_gatt_client *client,
> +							bool do_persist);
> +
> +struct bt_att *bt_gatt_client_get_att(struct bt_gatt_client *client);

I really hope that we do not need this. Lets try really hard to avoid it.

> +
> +typedef void (*bt_gatt_client_destroy_func_t)(void *user_data);
> +typedef void (*bt_gatt_client_callback_t)(void *user_data);
> +typedef void (*bt_gatt_client_service_changed_func_t)(uint16_t handle,
> +							void *user_data);
> +
> +bool bt_gatt_client_init(struct bt_gatt_client *client, uint16_t mtu,
> +					bt_gatt_client_callback_t callback,
> +					void *user_data,
> +					bt_gatt_client_destroy_func_t destroy);

Do not bother with it like this. Let do it like this:

	client = bt_gatt_client_new(att);

	bt_gatt_client_set_mtu(client, mtu);
	bt_gatt_client_set_ready_handler(client, callback, my_data, my_destroy);

Since we are not multi-threaded this will be safe and a lot cleaner to read.

> +
> +unsigned int bt_gatt_client_register_service_changed(
> +				struct bt_gatt_client *client,
> +				uint16_t handle,
> +				bt_gatt_client_service_changed_func_t callback,
> +				void *user_data,
> +				bt_gatt_client_destroy_func_t destroy);
> +bool bt_gatt_client_unregister_service_changed(struct bt_gatt_client *client,
> +							unsigned int id);

Are we expecting more than one here. If not, then better use bt_gatt_client_set_changed_handler.

Actually it make make sense to set ready handler and services changed handler with the same call. Try it out.

Regards

Marcel

Re: [PATCH BlueZ 03/11] shared/gatt-client: Added initial skeleton and simple functions.

[Arman Uguray]

Hi Marcel,


> great idea with the TODO. However please just use top-level TODO and patches modifying the TODO should be all by itself.
>

The top-level TODO has an ATT/GATT section though I don't know how
up-to-date that is. For now, I can create a separate shared/gatt
section for the work that I'm doing.


>> +struct bt_att *bt_gatt_client_get_att(struct bt_gatt_client *client);
>
> I really hope that we do not need this. Lets try really hard to avoid it.
>

I'll go ahead and remove this.


>> +bool bt_gatt_client_init(struct bt_gatt_client *client, uint16_t mtu,
>> +                                     bt_gatt_client_callback_t callback,
>> +                                     void *user_data,
>> +                                     bt_gatt_client_destroy_func_t destroy);
>
> Do not bother with it like this. Let do it like this:
>
>         client = bt_gatt_client_new(att);
>
>         bt_gatt_client_set_mtu(client, mtu);
>         bt_gatt_client_set_ready_handler(client, callback, my_data, my_destroy);
>
> Since we are not multi-threaded this will be safe and a lot cleaner to read.
>

We can have a single ready handler but I kind of like a general
initialize function that first does the MTU exchange and then
discovers all the attributes (this is what bt_gatt_client_init does
though it's not really clear from the name that it also performs
discovery, so I can change that).


>> +
>> +unsigned int bt_gatt_client_register_service_changed(
>> +                             struct bt_gatt_client *client,
>> +                             uint16_t handle,
>> +                             bt_gatt_client_service_changed_func_t callback,
>> +                             void *user_data,
>> +                             bt_gatt_client_destroy_func_t destroy);
>> +bool bt_gatt_client_unregister_service_changed(struct bt_gatt_client *client,
>> +                                                     unsigned int id);
>
> Are we expecting more than one here. If not, then better use bt_gatt_client_set_changed_handler.
>

My idea here was that each profile/plugin can register its own handler
for the handles that they are interested in and get notified of a
changed service. We could also have a single service changed handler
here and the daemon code can then use that to go and probe each
plugin/profile based on that.


> Actually it make make sense to set ready handler and services changed handler with the same call. Try it out.
>

Having separate functions would be a bit cleaner I think. I'll send a
revised patch and we can discuss it there.

-Arman

Re: [PATCH BlueZ 03/11] shared/gatt-client: Added initial skeleton and simple functions.

[Marcel Holtmann]

Hi Arman,

>> great idea with the TODO. However please just use top-level TODO and patches modifying the TODO should be all by itself.
>> 
> 
> The top-level TODO has an ATT/GATT section though I don't know how
> up-to-date that is. For now, I can create a separate shared/gatt
> section for the work that I'm doing.

create a new section if you find it clearer. Otherwise stick it into the existing section.

>>> +struct bt_att *bt_gatt_client_get_att(struct bt_gatt_client *client);
>> 
>> I really hope that we do not need this. Lets try really hard to avoid it.
>> 
> 
> I'll go ahead and remove this.
> 
> 
>>> +bool bt_gatt_client_init(struct bt_gatt_client *client, uint16_t mtu,
>>> +                                     bt_gatt_client_callback_t callback,
>>> +                                     void *user_data,
>>> +                                     bt_gatt_client_destroy_func_t destroy);
>> 
>> Do not bother with it like this. Let do it like this:
>> 
>>        client = bt_gatt_client_new(att);
>> 
>>        bt_gatt_client_set_mtu(client, mtu);
>>        bt_gatt_client_set_ready_handler(client, callback, my_data, my_destroy);
>> 
>> Since we are not multi-threaded this will be safe and a lot cleaner to read.
>> 
> 
> We can have a single ready handler but I kind of like a general
> initialize function that first does the MTU exchange and then
> discovers all the attributes (this is what bt_gatt_client_init does
> though it's not really clear from the name that it also performs
> discovery, so I can change that).

I do not like the fact of _new() an object and have to call _init() to use. Actually calling _new() should trigger all needed transaction. We might want to just do this:

	client = bt_gatt_client_new(att, mtu);

And it will start the service discovery right away. As I said, it is fine to start the service discovery and set the ready handler later. Since we are main loop and single threaded this is totally fine.

>>> +
>>> +unsigned int bt_gatt_client_register_service_changed(
>>> +                             struct bt_gatt_client *client,
>>> +                             uint16_t handle,
>>> +                             bt_gatt_client_service_changed_func_t callback,
>>> +                             void *user_data,
>>> +                             bt_gatt_client_destroy_func_t destroy);
>>> +bool bt_gatt_client_unregister_service_changed(struct bt_gatt_client *client,
>>> +                                                     unsigned int id);
>> 
>> Are we expecting more than one here. If not, then better use bt_gatt_client_set_changed_handler.
>> 
> 
> My idea here was that each profile/plugin can register its own handler
> for the handles that they are interested in and get notified of a
> changed service. We could also have a single service changed handler
> here and the daemon code can then use that to go and probe each
> plugin/profile based on that.

I am not sure about this. The services changed should be handled centrally in the client itself. It might be better than allow to register a changed notifier on the bt_gatt_service that we get.

Lets not bring handles into the game right here. It might be better to push this detail to later when the code is more complete.

Regards

Marcel

Re: [PATCH BlueZ 03/11] shared/gatt-client: Added initial skeleton and simple functions.

[Arman Uguray]

Hi Marcel,

> I do not like the fact of _new() an object and have to call _init() to use. Actually calling _new() should trigger all needed transaction. We might want to just do this:
>
>         client = bt_gatt_client_new(att, mtu);
>
> And it will start the service discovery right away. As I said, it is fine to start the service discovery and set the ready handler later. Since we are main loop and single threaded this is totally fine.
>

Sounds good to me.


>> My idea here was that each profile/plugin can register its own handler
>> for the handles that they are interested in and get notified of a
>> changed service. We could also have a single service changed handler
>> here and the daemon code can then use that to go and probe each
>> plugin/profile based on that.
>
> I am not sure about this. The services changed should be handled centrally in the client itself. It might be better than allow to register a changed notifier on the bt_gatt_service that we get.
>

What I meant here is that the client, as you said, will handle the
change internally by rediscovering the involved services, updating its
cache, and so on. It's nice to then notify the upper layer about the
changes that occurred so that they can perform any necessary updates
in their state.


> Lets not bring handles into the game right here. It might be better to push this detail to later when the code is more complete.
>

I will add code for this handler when I add the service change
handling later then. For now, I'll go ahead and remove the
register/unregister functions that I introduced.

-Arman