[PATCH BlueZ 00/12] core: Use shared/gatt-client.

[PATCH BlueZ 00/12] core: Use shared/gatt-client.

[Arman Uguray]

This patch set integrates shared/gatt-client into bluetoothd. The following
items have been tackled here:

  1. src/device now uses shared/gatt-client to perform service discovery. State
  updates such as disconnections, service changed events, etc. are done using
  callback APIs of the new shared framework and attrib/gatt is no longer used to
  perform GATT operations directly. A GAttrib instance is maintained for
  backwards compatibility with profiles that haven't been converted yet.

  2. A new gatt-callbacks API has been added which mirrors the attio APIs for
  shared/gatt-client. Profiles/plugins are notified using these APIs when the
  bearer disconnects, when services change, and when gatt-client is ready.

  3. The profiles/gatt plugin has been rewritten. Since the GATT service is
  handled by shared/gatt-client, this profile no longer deals with service
  changed events performs out of place MTU exchanges, which used to get
  performed in an arbitrary order. The profile has been renamed to profiles/gap
  as it now only deals with the GAP service, and reads the "Appearance" and
  "Device Name" characteristics using the new shared stack.

Arman Uguray (12):
  attrib/gattrib: Add g_attrib_get_att.
  shared/att: Add bt_att_get_fd.
  shared/att: Directly close fd on ATT violations.
  shared/gatt-client: Call ready_handler only in init.
  core: device: Use bt_att_register_disconnect.
  core: device: Use shared/gatt-client for GATT.
  core: Introduce gatt-callbacks
  profiles/gatt: Don't handle GATT service.
  profiles/gatt: Rename profile to gap.
  profiles/gap: Rewrite using bt_gatt_client.
  profiles/gap: Add Google copyright.
  TODO: Update GAttrib related items.

 Makefile.am              |   1 +
 Makefile.plugins         |   4 +-
 TODO                     |  25 +--
 attrib/gattrib.c         |   8 +
 attrib/gattrib.h         |   3 +
 profiles/gap/gas.c       | 307 +++++++++++++++++++++++++++++++
 profiles/gatt/gas.c      | 457 -----------------------------------------------
 src/device.c             | 456 +++++++++++++++++++++++++++++++---------------
 src/device.h             |   2 -
 src/gatt-callbacks.h     |  42 +++++
 src/shared/att-types.h   |   3 +-
 src/shared/att.c         |  25 ++-
 src/shared/att.h         |   1 +
 src/shared/gatt-client.c |   3 +-
 14 files changed, 711 insertions(+), 626 deletions(-)
 create mode 100644 profiles/gap/gas.c
 delete mode 100644 profiles/gatt/gas.c
 create mode 100644 src/gatt-callbacks.h

-- 
2.1.0.rc2.206.gedb03e5

[PATCH BlueZ 01/12] attrib/gattrib: Add g_attrib_get_att.

[Arman Uguray]

Added the g_attrib_get_att function which returns the underlying bt_att
structure associated with a GAttrib.
---
 attrib/gattrib.c | 8 ++++++++
 attrib/gattrib.h | 3 +++
 2 files changed, 11 insertions(+)

diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index ce7f7b3..a446ae6 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -166,6 +166,14 @@ GIOChannel *g_attrib_get_channel(GAttrib *attrib)
 	return attrib->io;
 }
 
+struct bt_att *g_attrib_get_att(GAttrib *attrib)
+{
+	if (!attrib)
+		return NULL;
+
+	return attrib->att;
+}
+
 gboolean g_attrib_set_destroy_function(GAttrib *attrib, GDestroyNotify destroy,
 							gpointer user_data)
 {
diff --git a/attrib/gattrib.h b/attrib/gattrib.h
index 2ed57c1..374bac2 100644
--- a/attrib/gattrib.h
+++ b/attrib/gattrib.h
@@ -31,6 +31,7 @@ extern "C" {
 #define GATTRIB_ALL_REQS 0xFE
 #define GATTRIB_ALL_HANDLES 0x0000
 
+struct bt_att;  /* Forward declaration for compatibility */
 struct _GAttrib;
 typedef struct _GAttrib GAttrib;
 
@@ -47,6 +48,8 @@ void g_attrib_unref(GAttrib *attrib);
 
 GIOChannel *g_attrib_get_channel(GAttrib *attrib);
 
+struct bt_att *g_attrib_get_att(GAttrib *attrib);
+
 gboolean g_attrib_set_destroy_function(GAttrib *attrib,
 		GDestroyNotify destroy, gpointer user_data);
 
-- 
2.1.0.rc2.206.gedb03e5

[PATCH BlueZ 02/12] shared/att: Add bt_att_get_fd.

[Arman Uguray]

This patch adds a getter for a bt_att structure's underlying connection
file descriptor.
---
 src/shared/att.c | 8 ++++++++
 src/shared/att.h | 1 +
 2 files changed, 9 insertions(+)

diff --git a/src/shared/att.c b/src/shared/att.c
index 2bc7682..264cece 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -907,6 +907,14 @@ void bt_att_unref(struct bt_att *att)
 	free(att);
 }
 
+int bt_att_get_fd(struct bt_att *att)
+{
+	if (!att)
+		return -1;
+
+	return att->fd;
+}
+
 bool bt_att_set_close_on_unref(struct bt_att *att, bool do_close)
 {
 	if (!att || !att->io)
diff --git a/src/shared/att.h b/src/shared/att.h
index 99b5a5b..b946b18 100644
--- a/src/shared/att.h
+++ b/src/shared/att.h
@@ -33,6 +33,7 @@ struct bt_att *bt_att_new(int fd);
 struct bt_att *bt_att_ref(struct bt_att *att);
 void bt_att_unref(struct bt_att *att);
 
+int bt_att_get_fd(struct bt_att *att);
 bool bt_att_set_close_on_unref(struct bt_att *att, bool do_close);
 
 typedef void (*bt_att_response_func_t)(uint8_t opcode, const void *pdu,
-- 
2.1.0.rc2.206.gedb03e5

[PATCH BlueZ 03/12] shared/att: Directly close fd on ATT violations.

[Arman Uguray]

The existing code handles ATT protocol request timeouts and invalid
incoming request PDUs by cleaning up the io structure via io_destroy.
This doesn't always work, since this only terminates the connection if
io_set_close_on_destroy has been set. Calling io_destroy to force a
disconnect also has the added side-effect of not invoking the disconnect
callback registered with struct io.

This patch fixes these by calling close directly on the file descriptor
when required by the ATT protocol specification. This both cleans up the
connection regardless of whether or not close_on_unref has been set, and
it triggers the disconnect callback via an EPOLLHUP event.
---
 src/shared/att.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/src/shared/att.c b/src/shared/att.c
index 264cece..d441099 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -416,9 +416,6 @@ static bool timeout_cb(void *user_data)
 	if (!op)
 		return false;
 
-	io_destroy(att->io);
-	att->io = NULL;
-
 	util_debug(att->debug_callback, att->debug_data,
 				"Operation timed out: 0x%02x", op->opcode);
 
@@ -428,6 +425,13 @@ static bool timeout_cb(void *user_data)
 	op->timeout_id = 0;
 	destroy_att_send_op(op);
 
+	/*
+	 * Directly terminate the connection as required by the ATT protocol.
+	 * This should trigger an io disconnect event which will clean up the
+	 * io and notify the upper layer.
+	 */
+	close(att->fd);
+
 	return false;
 }
 
@@ -765,14 +769,15 @@ static bool can_read_data(struct io *io, void *user_data)
 	case ATT_OP_TYPE_REQ:
 		/*
 		 * If a request is currently pending, then the sequential
-		 * protocol was violated. Disconnect the bearer and notify the
-		 * upper-layer.
+		 * protocol was violated. Disconnect the bearer, which will
+		 * promptly notify the upper layer via disconnect handlers.
 		 */
 		if (att->in_req) {
 			util_debug(att->debug_callback, att->debug_data,
 					"Received request while another is "
 					"pending: 0x%02x", opcode);
-			disconnect_cb(att->io, att);
+			close(att->fd);
+
 			return false;
 		}
 
-- 
2.1.0.rc2.206.gedb03e5

[PATCH BlueZ 04/12] shared/gatt-client: Call ready_handler only in init.

[Arman Uguray]

Currently disconnect_cb calls ready_handler to notify the upper layer
if a disconnect occurred during initialization. This patch fixes it so
that the handler is only called if in_init is set.
---
 src/shared/gatt-client.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index cf93d4b..698b7b8 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -1455,6 +1455,7 @@ static void bt_gatt_client_free(struct bt_gatt_client *client)
 static void att_disconnect_cb(void *user_data)
 {
 	struct bt_gatt_client *client = user_data;
+	bool in_init = client->in_init;
 
 	client->disc_id = 0;
 
@@ -1464,7 +1465,7 @@ static void att_disconnect_cb(void *user_data)
 	client->in_init = false;
 	client->ready = false;
 
-	if (client->ready_callback)
+	if (in_init && client->ready_callback)
 		client->ready_callback(false, 0, client->ready_data);
 }
 
-- 
2.1.0.rc2.206.gedb03e5

[PATCH BlueZ 05/12] core: device: Use bt_att_register_disconnect.

[Arman Uguray]

btd_device is now notified of ATT channel disconnections by registering
a disconnect handler with bt_att instead of setting up an io watch with
the GIOChannel.
---
 src/device.c | 34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/src/device.c b/src/device.c
index 025743f..45c7e9c 100644
--- a/src/device.c
+++ b/src/device.c
@@ -46,6 +46,8 @@
 #include "log.h"
 
 #include "src/shared/util.h"
+#include "src/shared/att.h"
+#include "src/shared/gatt-client.h"
 #include "btio/btio.h"
 #include "lib/uuid.h"
 #include "lib/mgmt.h"
@@ -203,6 +205,9 @@ struct btd_device {
 	GSList		*attios_offline;
 	guint		attachid;		/* Attrib server attach */
 
+	struct bt_att *att;	/* The new ATT transport */
+	unsigned int att_disconn_id;
+
 	struct bearer_state bredr_state;
 	struct bearer_state le_state;
 
@@ -221,7 +226,6 @@ struct btd_device {
 	int8_t		rssi;
 
 	GIOChannel	*att_io;
-	guint		cleanup_id;
 	guint		store_id;
 };
 
@@ -466,10 +470,9 @@ static void attio_cleanup(struct btd_device *device)
 		device->attachid = 0;
 	}
 
-	if (device->cleanup_id) {
-		g_source_remove(device->cleanup_id);
-		device->cleanup_id = 0;
-	}
+	if (device->att_disconn_id)
+		bt_att_unregister_disconnect(device->att,
+							device->att_disconn_id);
 
 	if (device->att_io) {
 		g_io_channel_shutdown(device->att_io, FALSE, NULL);
@@ -477,6 +480,11 @@ static void attio_cleanup(struct btd_device *device)
 		device->att_io = NULL;
 	}
 
+	if (device->att) {
+		bt_att_unref(device->att);
+		device->att = NULL;
+	}
+
 	if (device->attrib) {
 		GAttrib *attrib = device->attrib;
 		device->attrib = NULL;
@@ -3401,8 +3409,7 @@ static void attio_disconnected(gpointer data, gpointer user_data)
 		attio->dcfunc(attio->user_data);
 }
 
-static gboolean attrib_disconnected_cb(GIOChannel *io, GIOCondition cond,
-							gpointer user_data)
+static void att_disconnected_cb(void *user_data)
 {
 	struct btd_device *device = user_data;
 	int sock, err = 0;
@@ -3413,7 +3420,7 @@ static gboolean attrib_disconnected_cb(GIOChannel *io, GIOCondition cond,
 	if (device->browse)
 		goto done;
 
-	sock = g_io_channel_unix_get_fd(io);
+	sock = bt_att_get_fd(device->att);
 	len = sizeof(err);
 	getsockopt(sock, SOL_SOCKET, SO_ERROR, &err, &len);
 
@@ -3436,8 +3443,6 @@ static gboolean attrib_disconnected_cb(GIOChannel *io, GIOCondition cond,
 
 done:
 	attio_cleanup(device);
-
-	return FALSE;
 }
 
 static void register_all_services(struct browse_req *req, GSList *services)
@@ -3656,7 +3661,7 @@ bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
 	}
 
 	if (cid == ATT_CID)
-		mtu = ATT_DEFAULT_LE_MTU;
+		mtu = BT_ATT_DEFAULT_LE_MTU;
 
 	attrib = g_attrib_new(io, mtu);
 	if (!attrib) {
@@ -3672,8 +3677,11 @@ bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
 	}
 
 	dev->attrib = attrib;
-	dev->cleanup_id = g_io_add_watch(io, G_IO_HUP,
-					attrib_disconnected_cb, dev);
+	dev->att = bt_att_ref(g_attrib_get_att(attrib));
+
+	dev->att_disconn_id = bt_att_register_disconnect(dev->att,
+						att_disconnected_cb, dev, NULL);
+	bt_att_set_close_on_unref(dev->att, true);
 
 	/*
 	 * Remove the device from the connect_list and give the passive
-- 
2.1.0.rc2.206.gedb03e5

[PATCH BlueZ 06/12] core: device: Use shared/gatt-client for GATT.

[Arman Uguray]

This patch changes the GATT service discovery code path with the
following:
   - As part for device_attach_attrib, a bt_gatt_client structure is
   created that performs MTU exchange and service discovery, caches the
   services in an internal list, and handles the remote GATT service.

   - The device_browse_primary code path obtains service information
   by iterating through bt_gatt_client's services, instead of directly
   initiating primary and secondary service discovery using attrib/gatt.
   If the bt_gatt_client isn't ready at the time, the code defers
   registration of services and profile probing until the ready handler
   is called.

   - The attio connected callbacks are invoked from bt_gatt_client's
   ready handler instead of device_attach_attrib.
---
 src/device.c           | 286 ++++++++++++++++++++++++++-----------------------
 src/device.h           |   2 -
 src/shared/att-types.h |   3 +-
 3 files changed, 152 insertions(+), 139 deletions(-)

diff --git a/src/device.c b/src/device.c
index 45c7e9c..3d0159e 100644
--- a/src/device.c
+++ b/src/device.c
@@ -74,6 +74,10 @@
 #define DISCONNECT_TIMER	2
 #define DISCOVERY_TIMER		1
 
+#ifndef MIN
+#define MIN(a, b) ((a) < (b) ? (a) : (b))
+#endif
+
 static DBusConnection *dbus_conn = NULL;
 unsigned service_state_cb_id;
 
@@ -205,9 +209,12 @@ struct btd_device {
 	GSList		*attios_offline;
 	guint		attachid;		/* Attrib server attach */
 
-	struct bt_att *att;	/* The new ATT transport */
+	struct bt_att *att;			/* The new ATT transport */
+	uint16_t att_mtu;			/* The ATT MTU */
 	unsigned int att_disconn_id;
 
+	struct bt_gatt_client *gatt_client;	/* GATT client cache */
+
 	struct bearer_state bredr_state;
 	struct bearer_state le_state;
 
@@ -463,6 +470,18 @@ static void browse_request_free(struct browse_req *req)
 	g_free(req);
 }
 
+static void gatt_client_cleanup(struct btd_device *device)
+{
+	if (!device->gatt_client)
+		return;
+
+	bt_gatt_client_set_service_changed(device->gatt_client, NULL, NULL,
+									NULL);
+	bt_gatt_client_set_ready_handler(device->gatt_client, NULL, NULL, NULL);
+	bt_gatt_client_unref(device->gatt_client);
+	device->gatt_client = NULL;
+}
+
 static void attio_cleanup(struct btd_device *device)
 {
 	if (device->attachid) {
@@ -480,6 +499,8 @@ static void attio_cleanup(struct btd_device *device)
 		device->att_io = NULL;
 	}
 
+	gatt_client_cleanup(device);
+
 	if (device->att) {
 		bt_att_unref(device->att);
 		device->att = NULL;
@@ -3445,41 +3466,6 @@ done:
 	attio_cleanup(device);
 }
 
-static void register_all_services(struct browse_req *req, GSList *services)
-{
-	struct btd_device *device = req->device;
-
-	btd_device_set_temporary(device, FALSE);
-
-	update_gatt_services(req, device->primaries, services);
-	g_slist_free_full(device->primaries, g_free);
-	device->primaries = NULL;
-
-	device_register_primaries(device, services, -1);
-
-	device_probe_profiles(device, req->profiles_added);
-
-	if (device->attios == NULL && device->attios_offline == NULL)
-		attio_cleanup(device);
-
-	g_dbus_emit_property_changed(dbus_conn, device->path,
-						DEVICE_INTERFACE, "UUIDs");
-
-	device_svc_resolved(device, device->bdaddr_type, 0);
-
-	store_services(device);
-
-	browse_request_free(req);
-}
-
-static int service_by_range_cmp(gconstpointer a, gconstpointer b)
-{
-	const struct gatt_primary *prim = a;
-	const struct att_range *range = b;
-
-	return memcmp(&prim->range, range, sizeof(*range));
-}
-
 static void send_le_browse_response(struct browse_req *req)
 {
 	struct btd_device *dev = req->device;
@@ -3510,122 +3496,152 @@ static void send_le_browse_response(struct browse_req *req)
 	g_dbus_send_reply(dbus_conn, msg, DBUS_TYPE_INVALID);
 }
 
-static void find_included_cb(uint8_t status, GSList *includes, void *user_data)
+/*
+ * TODO: Remove this function once btd_device stops storing gatt_primary
+ * structures for discovered services.
+ */
+static bool populate_primaries(struct btd_device *device, GSList **services)
 {
-	struct included_search *search = user_data;
-	struct btd_device *device = search->req->device;
+	struct bt_gatt_service_iter iter;
+	const bt_gatt_service_t *service;
 	struct gatt_primary *prim;
-	GSList *l;
+	uint128_t u128;
+	bt_uuid_t uuid;
 
-	DBG("status %u", status);
+	/* Create gatt_primary structures for all primary/secondary services */
+	if (!bt_gatt_service_iter_init(&iter, device->gatt_client))
+		return false;
 
-	if (device->attrib == NULL || status) {
-		struct browse_req *req = device->browse;
+	while (bt_gatt_service_iter_next(&iter, &service)) {
+		prim = g_new0(struct gatt_primary, 1);
+		prim->range.start = service->start_handle;
+		prim->range.end = service->end_handle;
 
-		if (status)
-			error("Find included services failed: %s (%d)",
-					att_ecode2str(status), status);
-		else
-			error("Disconnected while doing included discovery");
+		memcpy(u128.data, service->uuid, 16);
+		bt_uuid128_create(&uuid, u128);
+		bt_uuid_to_string(&uuid, prim->uuid, MAX_LEN_UUID_STR + 1);
 
-		if (!req)
-			goto complete;
+		*services = g_slist_append(*services, prim);
+	}
 
+	return true;
+}
+
+static void register_gatt_services(struct browse_req *req)
+{
+	struct btd_device *device = req->device;
+	GSList *services = NULL;
+
+	if (!bt_gatt_client_is_ready(device->gatt_client))
+		return;
+
+	if (!populate_primaries(device, &services)) {
 		send_le_browse_response(req);
 		device->browse = NULL;
 		browse_request_free(req);
-
-		goto complete;
+		return;
 	}
 
-	if (includes == NULL)
-		goto next;
+	btd_device_set_temporary(device, FALSE);
 
-	for (l = includes; l; l = l->next) {
-		struct gatt_included *incl = l->data;
+	update_gatt_services(req, device->primaries, services);
+	g_slist_free_full(device->primaries, g_free);
+	device->primaries = NULL;
 
-		if (g_slist_find_custom(search->services, &incl->range,
-						service_by_range_cmp))
-			continue;
+	device_register_primaries(device, services, -1);
 
-		prim = g_new0(struct gatt_primary, 1);
-		memcpy(prim->uuid, incl->uuid, sizeof(prim->uuid));
-		memcpy(&prim->range, &incl->range, sizeof(prim->range));
+	device_probe_profiles(device, req->profiles_added);
 
-		search->services = g_slist_append(search->services, prim);
-	}
+	if (device->attios == NULL && device->attios_offline == NULL)
+		attio_cleanup(device);
 
-next:
-	search->current = search->current->next;
-	if (search->current == NULL) {
-		register_all_services(search->req, search->services);
-		search->services = NULL;
-		goto complete;
-	}
+	g_dbus_emit_property_changed(dbus_conn, device->path,
+						DEVICE_INTERFACE, "UUIDs");
 
-	prim = search->current->data;
-	gatt_find_included(device->attrib, prim->range.start, prim->range.end,
-					find_included_cb, search);
-	return;
+	device_svc_resolved(device, device->bdaddr_type, 0);
 
-complete:
-	g_slist_free_full(search->services, g_free);
-	g_free(search);
+	store_services(device);
+
+	browse_request_free(req);
 }
 
-static void find_included_services(struct browse_req *req, GSList *services)
+static void gatt_client_ready_cb(bool success, uint8_t att_ecode,
+								void *user_data)
 {
-	struct btd_device *device = req->device;
-	struct included_search *search;
-	struct gatt_primary *prim;
-	GSList *l;
+	struct btd_device *device = user_data;
 
-	DBG("service count %u", g_slist_length(services));
+	DBG("gatt-client ready -- status: %s, ATT error: %u",
+						success ? "success" : "failed",
+						att_ecode);
+
+	if (!success) {
+		if (device->browse) {
+			struct browse_req *req = device->browse;
+			send_le_browse_response(req);
+			device->browse = NULL;
+			browse_request_free(req);
+		}
 
-	if (services == NULL) {
-		DBG("No services found");
-		register_all_services(req, NULL);
 		return;
 	}
 
-	search = g_new0(struct included_search, 1);
-	search->req = req;
+	device->att_mtu = bt_att_get_mtu(device->att);
 
-	/* We have to completely duplicate the data in order to have a
-	 * clearly defined responsibility of freeing regardless of
-	 * failure or success. Otherwise memory leaks are inevitable.
-	 */
-	for (l = services; l; l = g_slist_next(l)) {
-		struct gatt_primary *dup;
+	DBG("gatt-client exchanged MTU: %u", device->att_mtu);
 
-		dup = g_memdup(l->data, sizeof(struct gatt_primary));
+	if (device->browse)
+		register_gatt_services(device->browse);
 
-		search->services = g_slist_append(search->services, dup);
-	}
+	/*
+	 * TODO: Change attio callbacks to accept a gatt-client instead of a
+	 * GAttrib.
+	 */
+	g_slist_foreach(device->attios, attio_connected, device->attrib);
+}
 
-	search->current = search->services;
+static void gatt_client_service_changed(uint16_t start_handle,
+							uint16_t end_handle,
+							void *user_data)
+{
+	struct btd_device *device = user_data;
+
+	DBG("gatt-client: Service Changed: start 0x%04x, end: 0x%04x",
+						start_handle, end_handle);
 
-	prim = search->current->data;
-	gatt_find_included(device->attrib, prim->range.start, prim->range.end,
-					find_included_cb, search);
+	/*
+	 * TODO: Notify clients that services changed so they can handle it
+	 * directly. Remove the profile if a service was removed.
+	 */
+	device_browse_primary(device, NULL);
 }
 
-static void primary_cb(uint8_t status, GSList *services, void *user_data)
+static void gatt_client_init(struct btd_device *device)
 {
-	struct browse_req *req = user_data;
+	gatt_client_cleanup(device);
 
-	DBG("status %u", status);
+	device->gatt_client = bt_gatt_client_new(device->att, device->att_mtu);
+	if (!device->gatt_client) {
+		DBG("Failed to initialize gatt-client");
+		return;
+	}
 
-	if (status) {
-		struct btd_device *device = req->device;
+	if (!bt_gatt_client_set_ready_handler(device->gatt_client,
+							gatt_client_ready_cb,
+							device, NULL)) {
+		DBG("Failed to set ready handler for gatt-client");
+		gatt_client_cleanup(device);
+		return;
+	}
 
-		send_le_browse_response(req);
-		device->browse = NULL;
-		browse_request_free(req);
+	if (!bt_gatt_client_set_service_changed(device->gatt_client,
+						gatt_client_service_changed,
+						device, NULL)) {
+		DBG("Failed to set service changed handler for gatt-client");
+		gatt_client_cleanup(device);
 		return;
 	}
 
-	find_included_services(req, services);
+	DBG("gatt-client created");
 }
 
 bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
@@ -3660,10 +3676,8 @@ bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
 		}
 	}
 
-	if (cid == ATT_CID)
-		mtu = BT_ATT_DEFAULT_LE_MTU;
-
-	attrib = g_attrib_new(io, mtu);
+	dev->att_mtu = MIN(mtu, BT_ATT_MAX_LE_MTU);
+	attrib = g_attrib_new(io, dev->att_mtu);
 	if (!attrib) {
 		error("Unable to create new GAttrib instance");
 		return false;
@@ -3678,11 +3692,12 @@ bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
 
 	dev->attrib = attrib;
 	dev->att = bt_att_ref(g_attrib_get_att(attrib));
-
 	dev->att_disconn_id = bt_att_register_disconnect(dev->att,
 						att_disconnected_cb, dev, NULL);
 	bt_att_set_close_on_unref(dev->att, true);
 
+	gatt_client_init(dev);
+
 	/*
 	 * Remove the device from the connect_list and give the passive
 	 * scanning another chance to be restarted in case there are
@@ -3690,8 +3705,6 @@ bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
 	 */
 	adapter_connect_list_remove(dev->adapter, dev);
 
-	g_slist_foreach(dev->attios, attio_connected, dev->attrib);
-
 	return true;
 }
 
@@ -3845,13 +3858,29 @@ static void att_browse_error_cb(const GError *gerr, gpointer user_data)
 	browse_request_free(req);
 }
 
+static void browse_gatt_client(struct browse_req *req)
+{
+	struct btd_device *device = req->device;
+
+	if (!device->gatt_client) {
+		DBG("No gatt-client currently attached");
+		return;
+	}
+
+	/*
+	 * If gatt-client is ready, then register all services. Otherwise, this
+	 * will be deferred until client becomes ready.
+	 */
+	if (bt_gatt_client_is_ready(device->gatt_client))
+		register_gatt_services(req);
+}
+
 static void att_browse_cb(gpointer user_data)
 {
 	struct att_callbacks *attcb = user_data;
 	struct btd_device *device = attcb->user_data;
 
-	gatt_discover_primary(device->attrib, NULL, primary_cb,
-							device->browse);
+	browse_gatt_client(device->browse);
 }
 
 static int device_browse_primary(struct btd_device *device, DBusMessage *msg)
@@ -3869,7 +3898,7 @@ static int device_browse_primary(struct btd_device *device, DBusMessage *msg)
 	device->browse = req;
 
 	if (device->attrib) {
-		gatt_discover_primary(device->attrib, NULL, primary_cb, req);
+		browse_gatt_client(device->browse);
 		goto done;
 	}
 
@@ -4738,21 +4767,6 @@ GSList *btd_device_get_primaries(struct btd_device *device)
 	return device->primaries;
 }
 
-void btd_device_gatt_set_service_changed(struct btd_device *device,
-						uint16_t start, uint16_t end)
-{
-	GSList *l;
-
-	for (l = device->primaries; l; l = g_slist_next(l)) {
-		struct gatt_primary *prim = l->data;
-
-		if (start <= prim->range.end && end >= prim->range.start)
-			prim->changed = TRUE;
-	}
-
-	device_browse_primary(device, NULL);
-}
-
 void btd_device_add_uuid(struct btd_device *device, const char *uuid)
 {
 	GSList *uuid_list;
diff --git a/src/device.h b/src/device.h
index 2e0473e..00cb502 100644
--- a/src/device.h
+++ b/src/device.h
@@ -67,8 +67,6 @@ const sdp_record_t *btd_device_get_record(struct btd_device *device,
 struct gatt_primary *btd_device_get_primary(struct btd_device *device,
 							const char *uuid);
 GSList *btd_device_get_primaries(struct btd_device *device);
-void btd_device_gatt_set_service_changed(struct btd_device *device,
-						uint16_t start, uint16_t end);
 bool device_attach_attrib(struct btd_device *dev, GIOChannel *io);
 void btd_device_add_uuid(struct btd_device *device, const char *uuid);
 void device_add_eir_uuids(struct btd_device *dev, GSList *uuids);
diff --git a/src/shared/att-types.h b/src/shared/att-types.h
index 3d8829a..8b6d537 100644
--- a/src/shared/att-types.h
+++ b/src/shared/att-types.h
@@ -27,7 +27,8 @@
 #define __packed __attribute__((packed))
 #endif
 
-#define BT_ATT_DEFAULT_LE_MTU 23
+#define BT_ATT_DEFAULT_LE_MTU	23
+#define BT_ATT_MAX_LE_MTU	517
 
 /* ATT protocol opcodes */
 #define BT_ATT_OP_ERROR_RSP	      		0x01
-- 
2.1.0.rc2.206.gedb03e5

[PATCH BlueZ 07/12] core: Introduce gatt-callbacks

[Arman Uguray]

This patch introduces src/gatt-callbacks.h. This defines API functions
to get notified of bt_gatt_client related events, such as ready,
service changed, and disconnects. This is orthogonal and similar to the
existing attio.h functions, which we're aiming to deprecate.
---
 Makefile.am          |   1 +
 src/device.c         | 160 +++++++++++++++++++++++++++++++++++++++++++++++----
 src/gatt-callbacks.h |  42 ++++++++++++++
 3 files changed, 193 insertions(+), 10 deletions(-)
 create mode 100644 src/gatt-callbacks.h

diff --git a/Makefile.am b/Makefile.am
index c84d63a..a3ebe86 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -181,6 +181,7 @@ src_bluetoothd_SOURCES = $(builtin_sources) \
 			src/gatt-dbus.h src/gatt-dbus.c \
 			src/gatt.h src/gatt.c \
 			src/device.h src/device.c src/attio.h \
+			src/gatt-callbacks.h \
 			src/dbus-common.c src/dbus-common.h \
 			src/eir.h src/eir.c
 src_bluetoothd_LDADD = lib/libbluetooth-internal.la \
diff --git a/src/device.c b/src/device.c
index 3d0159e..df87292 100644
--- a/src/device.c
+++ b/src/device.c
@@ -57,6 +57,7 @@
 #include "attrib/gattrib.h"
 #include "attio.h"
 #include "device.h"
+#include "gatt-callbacks.h"
 #include "profile.h"
 #include "service.h"
 #include "dbus-common.h"
@@ -143,6 +144,14 @@ struct attio_data {
 	gpointer user_data;
 };
 
+struct gatt_cb_data {
+	unsigned int id;
+	btd_gatt_client_ready_t ready_func;
+	btd_gatt_client_service_changed_t svc_chngd_func;
+	btd_gatt_disconnect_t disconn_func;
+	void *user_data;
+};
+
 struct svc_callback {
 	unsigned int id;
 	guint idle_id;
@@ -214,6 +223,8 @@ struct btd_device {
 	unsigned int att_disconn_id;
 
 	struct bt_gatt_client *gatt_client;	/* GATT client cache */
+	GSList		*gatt_callbacks;
+	unsigned int	next_gatt_cb_id;
 
 	struct bearer_state bredr_state;
 	struct bearer_state le_state;
@@ -548,6 +559,7 @@ static void device_free(gpointer user_data)
 	g_slist_free_full(device->attios, g_free);
 	g_slist_free_full(device->attios_offline, g_free);
 	g_slist_free_full(device->svc_callbacks, svc_dev_remove);
+	g_slist_free_full(device->gatt_callbacks, g_free);
 
 	attio_cleanup(device);
 
@@ -3430,6 +3442,16 @@ static void attio_disconnected(gpointer data, gpointer user_data)
 		attio->dcfunc(attio->user_data);
 }
 
+static void gatt_disconnected(gpointer data, gpointer user_data)
+{
+	struct gatt_cb_data *gatt_data = data;
+
+	DBG("");
+
+	if (gatt_data->disconn_func)
+		gatt_data->disconn_func(gatt_data->user_data);
+}
+
 static void att_disconnected_cb(void *user_data)
 {
 	struct btd_device *device = user_data;
@@ -3448,6 +3470,7 @@ static void att_disconnected_cb(void *user_data)
 	DBG("%s (%d)", strerror(err), err);
 
 	g_slist_foreach(device->attios, attio_disconnected, NULL);
+	g_slist_foreach(device->gatt_callbacks, gatt_disconnected, NULL);
 
 	if (!device_get_auto_connect(device)) {
 		DBG("Automatic connection disabled");
@@ -3552,7 +3575,14 @@ static void register_gatt_services(struct browse_req *req)
 
 	device_probe_profiles(device, req->profiles_added);
 
-	if (device->attios == NULL && device->attios_offline == NULL)
+	/* TODO: This check seems unnecessary. We may not always want to cleanup
+	 * the connection since there will always be built-in plugins who want
+	 * to interact with remote GATT services. Even if we didn't have those,
+	 * the GATT D-Bus API will need to interact with these, so we should
+	 * later remove this check entirely.
+	 */
+	if (device->attios == NULL && device->attios_offline == NULL &&
+						device->gatt_callbacks == NULL)
 		attio_cleanup(device);
 
 	g_dbus_emit_property_changed(dbus_conn, device->path,
@@ -3565,6 +3595,17 @@ static void register_gatt_services(struct browse_req *req)
 	browse_request_free(req);
 }
 
+static void notify_gatt_client_ready(gpointer data, gpointer user_data)
+{
+	struct gatt_cb_data *gatt_data = data;
+	struct bt_gatt_client *client = user_data;
+
+	DBG("");
+
+	if (gatt_data->ready_func)
+		gatt_data->ready_func(client, gatt_data->user_data);
+}
+
 static void gatt_client_ready_cb(bool success, uint8_t att_ecode,
 								void *user_data)
 {
@@ -3592,11 +3633,30 @@ static void gatt_client_ready_cb(bool success, uint8_t att_ecode,
 	if (device->browse)
 		register_gatt_services(device->browse);
 
-	/*
-	 * TODO: Change attio callbacks to accept a gatt-client instead of a
-	 * GAttrib.
-	 */
 	g_slist_foreach(device->attios, attio_connected, device->attrib);
+	g_slist_foreach(device->gatt_callbacks, notify_gatt_client_ready,
+							device->gatt_client);
+}
+
+struct svc_chngd_data {
+	struct bt_gatt_client *client;
+	uint16_t start_handle;
+	uint16_t end_handle;
+};
+
+static void notify_gatt_svc_chngd(gpointer data, gpointer user_data)
+{
+	struct gatt_cb_data *gatt_data = data;
+	struct svc_chngd_data *svc_data = user_data;
+
+	DBG("start: 0x%04x, end: 0x%04x", svc_data->start_handle,
+							svc_data->end_handle);
+
+	if (gatt_data->svc_chngd_func)
+		gatt_data->svc_chngd_func(svc_data->client,
+							svc_data->start_handle,
+							svc_data->end_handle,
+							gatt_data->user_data);
 }
 
 static void gatt_client_service_changed(uint16_t start_handle,
@@ -3604,14 +3664,17 @@ static void gatt_client_service_changed(uint16_t start_handle,
 							void *user_data)
 {
 	struct btd_device *device = user_data;
+	struct svc_chngd_data data;
 
 	DBG("gatt-client: Service Changed: start 0x%04x, end: 0x%04x",
 						start_handle, end_handle);
 
-	/*
-	 * TODO: Notify clients that services changed so they can handle it
-	 * directly. Remove the profile if a service was removed.
-	 */
+	memset(&data, 0, sizeof(data));
+	data.client = device->gatt_client;
+	data.start_handle = start_handle;
+	data.end_handle = end_handle;
+
+	g_slist_foreach(device->gatt_callbacks, notify_gatt_svc_chngd, &data);
 	device_browse_primary(device, NULL);
 }
 
@@ -4980,7 +5043,8 @@ gboolean btd_device_remove_attio_callback(struct btd_device *device, guint id)
 
 	g_free(attio);
 
-	if (device->attios != NULL || device->attios_offline != NULL)
+	if (device->attios != NULL || device->attios_offline != NULL ||
+						device->gatt_callbacks != NULL)
 		return TRUE;
 
 	attio_cleanup(device);
@@ -4988,6 +5052,82 @@ gboolean btd_device_remove_attio_callback(struct btd_device *device, guint id)
 	return TRUE;
 }
 
+unsigned int btd_device_add_gatt_callbacks(struct btd_device *device,
+			btd_gatt_client_ready_t ready_func,
+			btd_gatt_client_service_changed_t service_changed_func,
+			btd_gatt_disconnect_t disconnect_func,
+			void *user_data)
+{
+	struct gatt_cb_data *gatt_data;
+
+	gatt_data = new0(struct gatt_cb_data, 1);
+	if (!gatt_data)
+		return 0;
+
+	if (device->next_gatt_cb_id < 1)
+		device->next_gatt_cb_id = 1;
+
+	device_set_auto_connect(device, TRUE);
+
+	gatt_data->id = device->next_gatt_cb_id++;
+	gatt_data->ready_func = ready_func;
+	gatt_data->svc_chngd_func = service_changed_func;
+	gatt_data->disconn_func = disconnect_func;
+	gatt_data->user_data = user_data;
+
+	/*
+	 * TODO: The connection might be incoming from attrib-server (see
+	 * btd_device_add_attio_callback). I don't think this is a good place to
+	 * attach the GAttrib to the device. We should come up with a more
+	 * unified flow for attaching the GAttrib, bt_att, and bt_gatt_client
+	 * for incoming and outgoing connections.
+	 */
+	device->gatt_callbacks = g_slist_append(device->gatt_callbacks,
+								gatt_data);
+
+	if (ready_func && bt_gatt_client_is_ready(device->gatt_client))
+		ready_func(device->gatt_client, user_data);
+
+	return gatt_data->id;
+}
+
+static int gatt_cb_id_cmp(gconstpointer a, gconstpointer b)
+{
+	const struct gatt_cb_data *gatt_data = a;
+	guint id = GPOINTER_TO_UINT(b);
+
+	return gatt_data->id - id;
+}
+
+bool btd_device_remove_gatt_callbacks(struct btd_device *device,
+							unsigned int id)
+{
+	struct gatt_cb_data *gatt_data;
+	GSList *l;
+
+	l = g_slist_find_custom(device->gatt_callbacks, GUINT_TO_POINTER(id),
+								gatt_cb_id_cmp);
+	if (!l)
+		return false;
+
+	gatt_data = l->data;
+	device->gatt_callbacks = g_slist_remove(device->gatt_callbacks,
+								gatt_data);
+	free(gatt_data);
+
+	/*
+	 * TODO: Consider removing this check and avoiding cleanup as it seems
+	 * unnecessary.
+	 */
+	if (device->attios != NULL || device->attios_offline != NULL ||
+						device->gatt_callbacks != NULL)
+		return true;
+
+	attio_cleanup(device);
+
+	return true;
+}
+
 void btd_device_set_pnpid(struct btd_device *device, uint16_t source,
 			uint16_t vendor, uint16_t product, uint16_t version)
 {
diff --git a/src/gatt-callbacks.h b/src/gatt-callbacks.h
new file mode 100644
index 0000000..b449fcd
--- /dev/null
+++ b/src/gatt-callbacks.h
@@ -0,0 +1,42 @@
+/*
+ *
+ *  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 <stdint.h>
+#include <stdbool.h>
+
+typedef void (*btd_gatt_client_ready_t) (struct bt_gatt_client *client,
+							void *user_data);
+typedef void (*btd_gatt_client_service_changed_t) (
+						struct bt_gatt_client *client,
+						uint16_t start_handle,
+						uint16_t end_handle,
+						void *user_data);
+typedef void (*btd_gatt_disconnect_t) (void *user_data);
+
+unsigned int btd_device_add_gatt_callbacks(struct btd_device *device,
+			btd_gatt_client_ready_t ready_func,
+			btd_gatt_client_service_changed_t service_changed_func,
+			btd_gatt_disconnect_t disconnect_func,
+			void *user_data);
+bool btd_device_remove_gatt_callbacks(struct btd_device *device,
+							unsigned int id);
-- 
2.1.0.rc2.206.gedb03e5

[PATCH BlueZ 08/12] profiles/gatt: Don't handle GATT service.

[Arman Uguray]

ATT MTU exchange and handling of indications from the "Service Changed"
characteristic are now handled by shared/gatt-client, so this profile
should only deal with the GAP service.
---
 profiles/gatt/gas.c | 248 ++--------------------------------------------------
 1 file changed, 5 insertions(+), 243 deletions(-)

diff --git a/profiles/gatt/gas.c b/profiles/gatt/gas.c
index b51b4a8..75929c1 100644
--- a/profiles/gatt/gas.c
+++ b/profiles/gatt/gas.c
@@ -52,12 +52,8 @@
 struct gas {
 	struct btd_device *device;
 	struct att_range gap;	/* GAP Primary service range */
-	struct att_range gatt;	/* GATT Primary service range */
 	GAttrib *attrib;
 	guint attioid;
-	guint changed_ind;
-	uint16_t changed_handle;
-	uint16_t mtu;
 };
 
 static GSList *devices = NULL;
@@ -80,68 +76,6 @@ static int cmp_device(gconstpointer a, gconstpointer b)
 	return (gas->device == device ? 0 : -1);
 }
 
-static void write_ctp_handle(struct btd_device *device, uint16_t uuid,
-					uint16_t handle)
-{
-	char *filename, group[6], value[7];
-	GKeyFile *key_file;
-	char *data;
-	gsize length = 0;
-
-	filename = btd_device_get_storage_path(device, "gatt");
-	if (!filename) {
-		warn("Unable to get gatt storage path for device");
-		return;
-	}
-
-	key_file = g_key_file_new();
-	g_key_file_load_from_file(key_file, filename, 0, NULL);
-
-	snprintf(group, sizeof(group), "%hu", uuid);
-	snprintf(value, sizeof(value), "0x%4.4X", handle);
-	g_key_file_set_string(key_file, group, "Value", value);
-
-	data = g_key_file_to_data(key_file, &length, NULL);
-	if (length > 0) {
-		create_file(filename, S_IRUSR | S_IWUSR);
-		g_file_set_contents(filename, data, length, NULL);
-	}
-
-	g_free(data);
-	g_free(filename);
-	g_key_file_free(key_file);
-}
-
-static int read_ctp_handle(struct btd_device *device, uint16_t uuid,
-					uint16_t *value)
-{
-	char *filename, group[6];
-	GKeyFile *key_file;
-	char *str;
-	int err = 0;
-
-	filename = btd_device_get_storage_path(device, "gatt");
-	if (!filename) {
-		warn("Unable to get gatt storage path for device");
-		return -ENOENT;
-	}
-
-	snprintf(group, sizeof(group), "%hu", uuid);
-
-	key_file = g_key_file_new();
-	g_key_file_load_from_file(key_file, filename, 0, NULL);
-
-	str = g_key_file_get_string(key_file, group, "Value", NULL);
-	if (str == NULL || sscanf(str, "%hx", value) != 1)
-		err = -ENOENT;
-
-	g_free(str);
-	g_free(filename);
-	g_key_file_free(key_file);
-
-	return err;
-}
-
 static void gap_appearance_cb(guint8 status, const guint8 *pdu, guint16 plen,
 							gpointer user_data)
 {
@@ -176,164 +110,16 @@ done:
 	att_data_list_free(list);
 }
 
-static void indication_cb(const uint8_t *pdu, uint16_t len, gpointer user_data)
-{
-	uint8_t bdaddr_type;
-	struct gas *gas = user_data;
-	uint16_t start, end, olen;
-	size_t plen;
-	uint8_t *opdu;
-
-	if (len < 7) { /* 1-byte opcode + 2-byte handle + 4 range */
-		error("Malformed ATT notification");
-		return;
-	}
-
-	start = get_le16(&pdu[3]);
-	end = get_le16(&pdu[5]);
-
-	DBG("Service Changed start: 0x%04X end: 0x%04X", start, end);
-
-	/* Confirming indication received */
-	opdu = g_attrib_get_buffer(gas->attrib, &plen);
-	olen = enc_confirmation(opdu, plen);
-	g_attrib_send(gas->attrib, 0, opdu, olen, NULL, NULL, NULL);
-
-	bdaddr_type = btd_device_get_bdaddr_type(gas->device);
-	if (!device_is_bonded(gas->device, bdaddr_type)) {
-		DBG("Ignoring Service Changed: device is not bonded");
-		return;
-	}
-
-	btd_device_gatt_set_service_changed(gas->device, start, end);
-}
-
-static void ccc_written_cb(guint8 status, const guint8 *pdu, guint16 plen,
-							gpointer user_data)
-{
-	struct gas *gas = user_data;
-
-	if (status) {
-		error("Write Service Changed CCC failed: %s",
-						att_ecode2str(status));
-		return;
-	}
-
-	DBG("Service Changed indications enabled");
-
-	gas->changed_ind = g_attrib_register(gas->attrib, ATT_OP_HANDLE_IND,
-						gas->changed_handle,
-						indication_cb, gas, NULL);
-
-	write_ctp_handle(gas->device, GATT_CHARAC_SERVICE_CHANGED,
-					gas->changed_handle);
-}
-
-static void write_ccc(GAttrib *attrib, uint16_t handle, gpointer user_data)
-{
-	uint8_t value[2];
-
-	put_le16(GATT_CLIENT_CHARAC_CFG_IND_BIT, value);
-	gatt_write_char(attrib, handle, value, sizeof(value), ccc_written_cb,
-								user_data);
-}
-
-static void discover_ccc_cb(uint8_t status, GSList *descs, void *user_data)
-{
-	struct gas *gas = user_data;
-	struct gatt_desc *desc;
-
-	if (status != 0) {
-		error("Discover Service Changed CCC failed: %s",
-							att_ecode2str(status));
-		return;
-	}
-
-	/* There will be only one descriptor on list and it will be CCC */
-	desc = descs->data;
-
-	DBG("CCC: 0x%04x", desc->handle);
-	write_ccc(gas->attrib, desc->handle, user_data);
-}
-
-static void gatt_characteristic_cb(uint8_t status, GSList *characteristics,
-								void *user_data)
-{
-	struct gas *gas = user_data;
-	struct gatt_char *chr;
-	uint16_t start, end;
-	bt_uuid_t uuid;
-
-	if (status) {
-		error("Discover Service Changed handle: %s", att_ecode2str(status));
-		return;
-	}
-
-	chr = characteristics->data;
-
-	start = chr->value_handle + 1;
-	end = gas->gatt.end;
-
-	if (start > end) {
-		error("Inconsistent database: Service Changed CCC missing");
-		return;
-	}
-
-	gas->changed_handle = chr->value_handle;
-
-	bt_uuid16_create(&uuid, GATT_CLIENT_CHARAC_CFG_UUID);
-
-	gatt_discover_desc(gas->attrib, start, end, &uuid, discover_ccc_cb,
-									gas);
-}
-
-static void exchange_mtu_cb(guint8 status, const guint8 *pdu, guint16 plen,
-							gpointer user_data)
-{
-	struct gas *gas = user_data;
-	uint16_t rmtu;
-
-	if (status) {
-		error("MTU exchange: %s", att_ecode2str(status));
-		return;
-	}
-
-	if (!dec_mtu_resp(pdu, plen, &rmtu)) {
-		error("MTU exchange: protocol error");
-		return;
-	}
-
-	gas->mtu = MIN(rmtu, gas->mtu);
-	if (g_attrib_set_mtu(gas->attrib, gas->mtu))
-		DBG("MTU exchange succeeded: %d", gas->mtu);
-	else
-		DBG("MTU exchange failed");
-}
-
 static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
 {
 	struct gas *gas = user_data;
 	GIOChannel *io;
 	GError *gerr = NULL;
-	uint16_t cid, imtu;
 	uint16_t app;
 
 	gas->attrib = g_attrib_ref(attrib);
 	io = g_attrib_get_channel(attrib);
 
-	if (bt_io_get(io, &gerr, BT_IO_OPT_IMTU, &imtu,
-				BT_IO_OPT_CID, &cid, BT_IO_OPT_INVALID) &&
-							cid == ATT_CID) {
-		gatt_exchange_mtu(gas->attrib, imtu, exchange_mtu_cb, gas);
-		gas->mtu = imtu;
-		DBG("MTU Exchange: Requesting %d", imtu);
-	}
-
-	if (gerr) {
-		error("Could not acquire att imtu and cid: %s", gerr->message);
-		g_error_free(gerr);
-	}
-
 	if (device_get_appearance(gas->device, &app) < 0) {
 		bt_uuid_t uuid;
 
@@ -345,43 +131,23 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
 	}
 
 	/* TODO: Read other GAP characteristics - See Core spec page 1739 */
-
-	/*
-	 * When re-connecting <<Service Changed>> handle and characteristic
-	 * value doesn't need to read again: known information from the
-	 * previous interaction.
-	 */
-	if (gas->changed_handle == 0) {
-		bt_uuid_t uuid;
-
-		bt_uuid16_create(&uuid, GATT_CHARAC_SERVICE_CHANGED);
-
-		gatt_discover_char(gas->attrib, gas->gatt.start, gas->gatt.end,
-					&uuid, gatt_characteristic_cb, gas);
-	}
 }
 
 static void attio_disconnected_cb(gpointer user_data)
 {
 	struct gas *gas = user_data;
 
-	g_attrib_unregister(gas->attrib, gas->changed_ind);
-	gas->changed_ind = 0;
-
 	g_attrib_unref(gas->attrib);
 	gas->attrib = NULL;
 }
 
-static int gas_register(struct btd_device *device, struct att_range *gap,
-						struct att_range *gatt)
+static int gas_register(struct btd_device *device, struct att_range *gap)
 {
 	struct gas *gas;
 
 	gas = g_new0(struct gas, 1);
 	gas->gap.start = gap->start;
 	gas->gap.end = gap->end;
-	gas->gatt.start = gatt->start;
-	gas->gatt.end = gatt->end;
 
 	gas->device = btd_device_ref(device);
 
@@ -391,9 +157,6 @@ static int gas_register(struct btd_device *device, struct att_range *gap,
 						attio_connected_cb,
 						attio_disconnected_cb, gas);
 
-	read_ctp_handle(gas->device, GATT_CHARAC_SERVICE_CHANGED,
-					&gas->changed_handle);
-
 	return 0;
 }
 
@@ -414,17 +177,16 @@ static void gas_unregister(struct btd_device *device)
 static int gatt_driver_probe(struct btd_service *service)
 {
 	struct btd_device *device = btd_service_get_device(service);
-	struct gatt_primary *gap, *gatt;
+	struct gatt_primary *gap;
 
 	gap = btd_device_get_primary(device, GAP_UUID);
-	gatt = btd_device_get_primary(device, GATT_UUID);
 
-	if (gap == NULL || gatt == NULL) {
-		error("GAP and GATT are mandatory");
+	if (gap == NULL) {
+		error("GAP service is mandatory");
 		return -EINVAL;
 	}
 
-	return gas_register(device, &gap->range, &gatt->range);
+	return gas_register(device, &gap->range);
 }
 
 static void gatt_driver_remove(struct btd_service *service)
-- 
2.1.0.rc2.206.gedb03e5

[PATCH BlueZ 09/12] profiles/gatt: Rename profile to gap.

[Arman Uguray]

Since this built in profile only handles the GAP service, this patch renames it
to gap.
---
 Makefile.plugins    |   4 +-
 profiles/gap/gas.c  | 219 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 profiles/gatt/gas.c | 219 ----------------------------------------------------
 3 files changed, 221 insertions(+), 221 deletions(-)
 create mode 100644 profiles/gap/gas.c
 delete mode 100644 profiles/gatt/gas.c

diff --git a/Makefile.plugins b/Makefile.plugins
index 0448b91..52b51c5 100644
--- a/Makefile.plugins
+++ b/Makefile.plugins
@@ -72,8 +72,8 @@ builtin_sources += profiles/health/mcap.h profiles/health/mcap.c \
 			profiles/health/hdp_util.h profiles/health/hdp_util.c
 endif
 
-builtin_modules += gatt
-builtin_sources += profiles/gatt/gas.c
+builtin_modules += gap
+builtin_sources += profiles/gap/gas.c
 
 builtin_modules += scanparam
 builtin_sources += profiles/scanparam/scan.c
diff --git a/profiles/gap/gas.c b/profiles/gap/gas.c
new file mode 100644
index 0000000..91b317f
--- /dev/null
+++ b/profiles/gap/gas.c
@@ -0,0 +1,219 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2012  Instituto Nokia de Tecnologia - INdT
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program 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 General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdbool.h>
+#include <stdlib.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <errno.h>
+
+#include <glib.h>
+
+#include "btio/btio.h"
+#include "lib/uuid.h"
+#include "src/plugin.h"
+#include "src/adapter.h"
+#include "src/device.h"
+#include "src/profile.h"
+#include "src/service.h"
+#include "src/shared/util.h"
+#include "attrib/att.h"
+#include "attrib/gattrib.h"
+#include "src/attio.h"
+#include "attrib/gatt.h"
+#include "src/log.h"
+#include "src/textfile.h"
+
+/* Generic Attribute/Access Service */
+struct gas {
+	struct btd_device *device;
+	struct att_range gap;	/* GAP Primary service range */
+	GAttrib *attrib;
+	guint attioid;
+};
+
+static GSList *devices = NULL;
+
+static void gas_free(struct gas *gas)
+{
+	if (gas->attioid)
+		btd_device_remove_attio_callback(gas->device, gas->attioid);
+
+	g_attrib_unref(gas->attrib);
+	btd_device_unref(gas->device);
+	g_free(gas);
+}
+
+static int cmp_device(gconstpointer a, gconstpointer b)
+{
+	const struct gas *gas = a;
+	const struct btd_device *device = b;
+
+	return (gas->device == device ? 0 : -1);
+}
+
+static void gap_appearance_cb(guint8 status, const guint8 *pdu, guint16 plen,
+							gpointer user_data)
+{
+	struct gas *gas = user_data;
+	struct att_data_list *list =  NULL;
+	uint16_t app;
+	uint8_t *atval;
+
+	if (status != 0) {
+		error("Read characteristics by UUID failed: %s",
+				att_ecode2str(status));
+		return;
+	}
+
+	list = dec_read_by_type_resp(pdu, plen);
+	if (list == NULL)
+		return;
+
+	if (list->len != 4) {
+		error("GAP Appearance value: invalid data");
+		goto done;
+	}
+
+	atval = list->data[0] + 2; /* skip handle value */
+	app = get_le16(atval);
+
+	DBG("GAP Appearance: 0x%04x", app);
+
+	device_set_appearance(gas->device, app);
+
+done:
+	att_data_list_free(list);
+}
+
+static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
+{
+	struct gas *gas = user_data;
+	GIOChannel *io;
+	GError *gerr = NULL;
+	uint16_t app;
+
+	gas->attrib = g_attrib_ref(attrib);
+	io = g_attrib_get_channel(attrib);
+
+	if (device_get_appearance(gas->device, &app) < 0) {
+		bt_uuid_t uuid;
+
+		bt_uuid16_create(&uuid, GATT_CHARAC_APPEARANCE);
+
+		gatt_read_char_by_uuid(gas->attrib, gas->gap.start,
+						gas->gap.end, &uuid,
+						gap_appearance_cb, gas);
+	}
+
+	/* TODO: Read other GAP characteristics - See Core spec page 1739 */
+}
+
+static void attio_disconnected_cb(gpointer user_data)
+{
+	struct gas *gas = user_data;
+
+	g_attrib_unref(gas->attrib);
+	gas->attrib = NULL;
+}
+
+static int gas_register(struct btd_device *device, struct att_range *gap)
+{
+	struct gas *gas;
+
+	gas = g_new0(struct gas, 1);
+	gas->gap.start = gap->start;
+	gas->gap.end = gap->end;
+
+	gas->device = btd_device_ref(device);
+
+	devices = g_slist_append(devices, gas);
+
+	gas->attioid = btd_device_add_attio_callback(device,
+						attio_connected_cb,
+						attio_disconnected_cb, gas);
+
+	return 0;
+}
+
+static void gas_unregister(struct btd_device *device)
+{
+	struct gas *gas;
+	GSList *l;
+
+	l = g_slist_find_custom(devices, device, cmp_device);
+	if (l == NULL)
+		return;
+
+	gas = l->data;
+	devices = g_slist_remove(devices, gas);
+	gas_free(gas);
+}
+
+static int gap_driver_probe(struct btd_service *service)
+{
+	struct btd_device *device = btd_service_get_device(service);
+	struct gatt_primary *gap;
+
+	gap = btd_device_get_primary(device, GAP_UUID);
+
+	if (gap == NULL) {
+		error("GAP service is mandatory");
+		return -EINVAL;
+	}
+
+	return gas_register(device, &gap->range);
+}
+
+static void gap_driver_remove(struct btd_service *service)
+{
+	struct btd_device *device = btd_service_get_device(service);
+
+	gas_unregister(device);
+}
+
+static struct btd_profile gap_profile = {
+	.name		= "gap-profile",
+	.remote_uuid	= GAP_UUID,
+	.device_probe	= gap_driver_probe,
+	.device_remove	= gap_driver_remove
+};
+
+static int gap_init(void)
+{
+	btd_profile_register(&gap_profile);
+
+	return 0;
+}
+
+static void gap_exit(void)
+{
+	btd_profile_unregister(&gap_profile);
+}
+
+BLUETOOTH_PLUGIN_DEFINE(gap, VERSION, BLUETOOTH_PLUGIN_PRIORITY_DEFAULT,
+							gap_init, gap_exit)
diff --git a/profiles/gatt/gas.c b/profiles/gatt/gas.c
deleted file mode 100644
index 75929c1..0000000
--- a/profiles/gatt/gas.c
+++ /dev/null
@@ -1,219 +0,0 @@
-/*
- *
- *  BlueZ - Bluetooth protocol stack for Linux
- *
- *  Copyright (C) 2012  Instituto Nokia de Tecnologia - INdT
- *
- *  This program is free software; you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation; either version 2 of the License, or
- *  (at your option) any later version.
- *
- *  This program 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 General Public License for more details.
- *
- *  You should have received a copy of the GNU General Public License
- *  along with this program; if not, write to the Free Software
- *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
- *
- */
-
-#ifdef HAVE_CONFIG_H
-#include <config.h>
-#endif
-
-#include <stdbool.h>
-#include <stdlib.h>
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <fcntl.h>
-#include <errno.h>
-
-#include <glib.h>
-
-#include "btio/btio.h"
-#include "lib/uuid.h"
-#include "src/plugin.h"
-#include "src/adapter.h"
-#include "src/device.h"
-#include "src/profile.h"
-#include "src/service.h"
-#include "src/shared/util.h"
-#include "attrib/att.h"
-#include "attrib/gattrib.h"
-#include "src/attio.h"
-#include "attrib/gatt.h"
-#include "src/log.h"
-#include "src/textfile.h"
-
-/* Generic Attribute/Access Service */
-struct gas {
-	struct btd_device *device;
-	struct att_range gap;	/* GAP Primary service range */
-	GAttrib *attrib;
-	guint attioid;
-};
-
-static GSList *devices = NULL;
-
-static void gas_free(struct gas *gas)
-{
-	if (gas->attioid)
-		btd_device_remove_attio_callback(gas->device, gas->attioid);
-
-	g_attrib_unref(gas->attrib);
-	btd_device_unref(gas->device);
-	g_free(gas);
-}
-
-static int cmp_device(gconstpointer a, gconstpointer b)
-{
-	const struct gas *gas = a;
-	const struct btd_device *device = b;
-
-	return (gas->device == device ? 0 : -1);
-}
-
-static void gap_appearance_cb(guint8 status, const guint8 *pdu, guint16 plen,
-							gpointer user_data)
-{
-	struct gas *gas = user_data;
-	struct att_data_list *list =  NULL;
-	uint16_t app;
-	uint8_t *atval;
-
-	if (status != 0) {
-		error("Read characteristics by UUID failed: %s",
-				att_ecode2str(status));
-		return;
-	}
-
-	list = dec_read_by_type_resp(pdu, plen);
-	if (list == NULL)
-		return;
-
-	if (list->len != 4) {
-		error("GAP Appearance value: invalid data");
-		goto done;
-	}
-
-	atval = list->data[0] + 2; /* skip handle value */
-	app = get_le16(atval);
-
-	DBG("GAP Appearance: 0x%04x", app);
-
-	device_set_appearance(gas->device, app);
-
-done:
-	att_data_list_free(list);
-}
-
-static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
-{
-	struct gas *gas = user_data;
-	GIOChannel *io;
-	GError *gerr = NULL;
-	uint16_t app;
-
-	gas->attrib = g_attrib_ref(attrib);
-	io = g_attrib_get_channel(attrib);
-
-	if (device_get_appearance(gas->device, &app) < 0) {
-		bt_uuid_t uuid;
-
-		bt_uuid16_create(&uuid, GATT_CHARAC_APPEARANCE);
-
-		gatt_read_char_by_uuid(gas->attrib, gas->gap.start,
-						gas->gap.end, &uuid,
-						gap_appearance_cb, gas);
-	}
-
-	/* TODO: Read other GAP characteristics - See Core spec page 1739 */
-}
-
-static void attio_disconnected_cb(gpointer user_data)
-{
-	struct gas *gas = user_data;
-
-	g_attrib_unref(gas->attrib);
-	gas->attrib = NULL;
-}
-
-static int gas_register(struct btd_device *device, struct att_range *gap)
-{
-	struct gas *gas;
-
-	gas = g_new0(struct gas, 1);
-	gas->gap.start = gap->start;
-	gas->gap.end = gap->end;
-
-	gas->device = btd_device_ref(device);
-
-	devices = g_slist_append(devices, gas);
-
-	gas->attioid = btd_device_add_attio_callback(device,
-						attio_connected_cb,
-						attio_disconnected_cb, gas);
-
-	return 0;
-}
-
-static void gas_unregister(struct btd_device *device)
-{
-	struct gas *gas;
-	GSList *l;
-
-	l = g_slist_find_custom(devices, device, cmp_device);
-	if (l == NULL)
-		return;
-
-	gas = l->data;
-	devices = g_slist_remove(devices, gas);
-	gas_free(gas);
-}
-
-static int gatt_driver_probe(struct btd_service *service)
-{
-	struct btd_device *device = btd_service_get_device(service);
-	struct gatt_primary *gap;
-
-	gap = btd_device_get_primary(device, GAP_UUID);
-
-	if (gap == NULL) {
-		error("GAP service is mandatory");
-		return -EINVAL;
-	}
-
-	return gas_register(device, &gap->range);
-}
-
-static void gatt_driver_remove(struct btd_service *service)
-{
-	struct btd_device *device = btd_service_get_device(service);
-
-	gas_unregister(device);
-}
-
-static struct btd_profile gatt_profile = {
-	.name		= "gap-gatt-profile",
-	.remote_uuid	= GATT_UUID,
-	.device_probe	= gatt_driver_probe,
-	.device_remove	= gatt_driver_remove
-};
-
-static int gatt_init(void)
-{
-	btd_profile_register(&gatt_profile);
-
-	return 0;
-}
-
-static void gatt_exit(void)
-{
-	btd_profile_unregister(&gatt_profile);
-}
-
-BLUETOOTH_PLUGIN_DEFINE(gatt, VERSION, BLUETOOTH_PLUGIN_PRIORITY_DEFAULT,
-					gatt_init, gatt_exit)
-- 
2.1.0.rc2.206.gedb03e5

[PATCH BlueZ 10/12] profiles/gap: Rewrite using bt_gatt_client.

[Arman Uguray]

This patch rewrites the GAP profile to use the shared GATT stack instead
of GAttrib. The profile now also handles the Device Name characteristic.
---
 profiles/gap/gas.c | 213 +++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 150 insertions(+), 63 deletions(-)

diff --git a/profiles/gap/gas.c b/profiles/gap/gas.c
index 91b317f..9813d68 100644
--- a/profiles/gap/gas.c
+++ b/profiles/gap/gas.c
@@ -24,6 +24,7 @@
 #include <config.h>
 #endif
 
+#include <ctype.h>
 #include <stdbool.h>
 #include <stdlib.h>
 #include <sys/types.h>
@@ -41,29 +42,28 @@
 #include "src/profile.h"
 #include "src/service.h"
 #include "src/shared/util.h"
-#include "attrib/att.h"
-#include "attrib/gattrib.h"
-#include "src/attio.h"
-#include "attrib/gatt.h"
 #include "src/log.h"
 #include "src/textfile.h"
+#include "src/shared/att.h"
+#include "src/shared/gatt-client.h"
+#include "src/gatt-callbacks.h"
 
 /* Generic Attribute/Access Service */
 struct gas {
 	struct btd_device *device;
-	struct att_range gap;	/* GAP Primary service range */
-	GAttrib *attrib;
-	guint attioid;
+	struct bt_gatt_client *client;
+	uint16_t start_handle, end_handle;
+	unsigned int gatt_cb_id;
 };
 
 static GSList *devices = NULL;
 
 static void gas_free(struct gas *gas)
 {
-	if (gas->attioid)
-		btd_device_remove_attio_callback(gas->device, gas->attioid);
+	if (gas->gatt_cb_id)
+		btd_device_remove_gatt_callbacks(gas->device,
+							gas->gatt_cb_id);
 
-	g_attrib_unref(gas->attrib);
 	btd_device_unref(gas->device);
 	g_free(gas);
 }
@@ -76,86 +76,181 @@ static int cmp_device(gconstpointer a, gconstpointer b)
 	return (gas->device == device ? 0 : -1);
 }
 
-static void gap_appearance_cb(guint8 status, const guint8 *pdu, guint16 plen,
-							gpointer user_data)
+static char *name2utf8(const uint8_t *name, uint8_t len)
+{
+	char utf8_name[HCI_MAX_NAME_LENGTH + 2];
+	int i;
+
+	if (g_utf8_validate((const char *) name, len, NULL))
+		return g_strndup((char *) name, len);
+
+	memset(utf8_name, 0, sizeof(utf8_name));
+	strncpy(utf8_name, (char *) name, len);
+
+	/* Assume ASCII, and replace all non-ASCII with spaces */
+	for (i = 0; utf8_name[i] != '\0'; i++) {
+		if (!isascii(utf8_name[i]))
+			utf8_name[i] = ' ';
+	}
+
+	/* Remove leading and trailing whitespace characters */
+	g_strstrip(utf8_name);
+
+	return g_strdup(utf8_name);
+}
+
+static void read_device_name_cb(bool success, uint8_t att_ecode,
+					const uint8_t *value, uint16_t length,
+					void *user_data)
 {
 	struct gas *gas = user_data;
-	struct att_data_list *list =  NULL;
-	uint16_t app;
-	uint8_t *atval;
+	char *name = name2utf8(value, length);
 
-	if (status != 0) {
-		error("Read characteristics by UUID failed: %s",
-				att_ecode2str(status));
+	DBG("GAP Device Name: %s", name);
+
+	btd_device_device_set_name(gas->device, name);
+}
+
+static void handle_device_name(struct gas *gas,
+					const bt_gatt_characteristic_t *chrc)
+{
+	if (!bt_gatt_client_read_long_value(gas->client, chrc->value_handle, 0,
+						read_device_name_cb, gas, NULL))
+		DBG("Failed to send request to read device name");
+}
+
+static void read_appearance_cb(bool success, uint8_t att_ecode,
+					const uint8_t *value, uint16_t length,
+					void *user_data)
+{
+	struct gas *gas = user_data;
+	uint16_t appearance;
+
+	if (!success) {
+		DBG("Reading appearance failed with ATT error: %u", att_ecode);
 		return;
 	}
 
-	list = dec_read_by_type_resp(pdu, plen);
-	if (list == NULL)
+	/* The appearance value is a 16-bit unsigned integer */
+	if (length != 2) {
+		DBG("Malformed appearance value");
 		return;
-
-	if (list->len != 4) {
-		error("GAP Appearance value: invalid data");
-		goto done;
 	}
 
-	atval = list->data[0] + 2; /* skip handle value */
-	app = get_le16(atval);
+	appearance = get_le16(value);
 
-	DBG("GAP Appearance: 0x%04x", app);
+	DBG("GAP Appearance: 0x%04x", appearance);
 
-	device_set_appearance(gas->device, app);
+	device_set_appearance(gas->device, appearance);
+}
 
-done:
-	att_data_list_free(list);
+static void handle_appearance(struct gas *gas,
+					const bt_gatt_characteristic_t *chrc)
+{
+	uint16_t appearance;
+
+	if (device_get_appearance(gas->device, &appearance) >= 0)
+		return;
+
+	if (!bt_gatt_client_read_value(gas->client, chrc->value_handle,
+				read_appearance_cb, gas, NULL))
+		DBG("Failed to send request to read appearance");
 }
 
-static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
+static bool uuid_cmp(uint16_t u16, const uint8_t uuid[16])
 {
-	struct gas *gas = user_data;
-	GIOChannel *io;
-	GError *gerr = NULL;
-	uint16_t app;
+	uint128_t u128;
+	bt_uuid_t lhs, rhs;
 
-	gas->attrib = g_attrib_ref(attrib);
-	io = g_attrib_get_channel(attrib);
+	memcpy(u128.data, uuid, sizeof(uint8_t) * 16);
+	bt_uuid16_create(&lhs, u16);
+	bt_uuid128_create(&rhs, u128);
 
-	if (device_get_appearance(gas->device, &app) < 0) {
-		bt_uuid_t uuid;
+	return bt_uuid_cmp(&lhs, &rhs) == 0;
+}
 
-		bt_uuid16_create(&uuid, GATT_CHARAC_APPEARANCE);
+static void reset_gap_service(struct gas *gas)
+{
+	struct bt_gatt_service_iter iter;
+	struct bt_gatt_characteristic_iter chrc_iter;
+	const bt_gatt_service_t *service = NULL;
+	const bt_gatt_characteristic_t *chrc = NULL;
+	bt_uuid_t gap_uuid;
 
-		gatt_read_char_by_uuid(gas->attrib, gas->gap.start,
-						gas->gap.end, &uuid,
-						gap_appearance_cb, gas);
+	bt_string_to_uuid(&gap_uuid, GAP_UUID);
+
+	if (!bt_gatt_service_iter_init(&iter, gas->client)) {
+		DBG("Failed to initialize service iterator");
+		return;
 	}
 
-	/* TODO: Read other GAP characteristics - See Core spec page 1739 */
+	if (!bt_gatt_service_iter_next_by_uuid(&iter, gap_uuid.value.u128.data,
+								&service)) {
+		error("GAP service not found on device");
+		return;
+	}
+
+	gas->start_handle = service->start_handle;
+	gas->end_handle = service->end_handle;
+
+	if (!bt_gatt_characteristic_iter_init(&chrc_iter, service)) {
+		DBG("Failed to initialize characteristic iterator");
+		return;
+	}
+
+	while (bt_gatt_characteristic_iter_next(&chrc_iter, &chrc)) {
+		if (uuid_cmp(GATT_CHARAC_DEVICE_NAME, chrc->uuid))
+			handle_device_name(gas, chrc);
+		else if (uuid_cmp(GATT_CHARAC_APPEARANCE, chrc->uuid))
+			handle_appearance(gas, chrc);
+
+		/*
+		 * TODO: Implement peripheral privacy feature.
+		 */
+	}
+}
+
+static void gatt_client_ready_cb(struct bt_gatt_client *client, void *user_data)
+{
+	struct gas *gas = user_data;
+
+	gas->client = client;
+
+	reset_gap_service(gas);
 }
 
-static void attio_disconnected_cb(gpointer user_data)
+static void gatt_svc_chngd_cb(struct bt_gatt_client *client,
+						uint16_t start_handle,
+						uint16_t end_handle,
+						void *user_data)
 {
 	struct gas *gas = user_data;
 
-	g_attrib_unref(gas->attrib);
-	gas->attrib = NULL;
+	if (!gas->client || !gas->start_handle || !gas->end_handle)
+		return;
+
+	if (gas->end_handle < start_handle || gas->start_handle > end_handle)
+		return;
+
+	DBG("GAP service changed!");
+
+	reset_gap_service(gas);
 }
 
-static int gas_register(struct btd_device *device, struct att_range *gap)
+static int gas_register(struct btd_device *device)
 {
 	struct gas *gas;
 
 	gas = g_new0(struct gas, 1);
-	gas->gap.start = gap->start;
-	gas->gap.end = gap->end;
 
 	gas->device = btd_device_ref(device);
-
 	devices = g_slist_append(devices, gas);
 
-	gas->attioid = btd_device_add_attio_callback(device,
-						attio_connected_cb,
-						attio_disconnected_cb, gas);
+	if (!gas->gatt_cb_id)
+		gas->gatt_cb_id = btd_device_add_gatt_callbacks(device,
+							gatt_client_ready_cb,
+							gatt_svc_chngd_cb,
+							NULL, gas);
 
 	return 0;
 }
@@ -177,16 +272,8 @@ static void gas_unregister(struct btd_device *device)
 static int gap_driver_probe(struct btd_service *service)
 {
 	struct btd_device *device = btd_service_get_device(service);
-	struct gatt_primary *gap;
-
-	gap = btd_device_get_primary(device, GAP_UUID);
-
-	if (gap == NULL) {
-		error("GAP service is mandatory");
-		return -EINVAL;
-	}
 
-	return gas_register(device, &gap->range);
+	return gas_register(device);
 }
 
 static void gap_driver_remove(struct btd_service *service)
-- 
2.1.0.rc2.206.gedb03e5

[PATCH BlueZ 11/12] profiles/gap: Add Google copyright.

[Arman Uguray]

Added Google Inc. to the copyright comment since we mostly rewrote this
profile.
---
 profiles/gap/gas.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/profiles/gap/gas.c b/profiles/gap/gas.c
index 9813d68..6154a0d 100644
--- a/profiles/gap/gas.c
+++ b/profiles/gap/gas.c
@@ -3,6 +3,7 @@
  *  BlueZ - Bluetooth protocol stack for Linux
  *
  *  Copyright (C) 2012  Instituto Nokia de Tecnologia - INdT
+ *  Copyright (C) 2014  Google Inc.
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
-- 
2.1.0.rc2.206.gedb03e5

[PATCH BlueZ 12/12] TODO: Update GAttrib related items.

[Arman Uguray]

Updated TODOs related to the daemon transition from attrib/ to
shared/gatt.
---
 TODO | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/TODO b/TODO
index fec0fa2..2019e3c 100644
--- a/TODO
+++ b/TODO
@@ -141,16 +141,16 @@ ATT/GATT (new shared stack)
   Priority: Low
   Complexity: C4
 
-- Move all daemon plugins and profiles that are GATT based to use
-  shared/gatt-client instead of attrib/*. This is a complicated task that
-  potentially needs a new plugin/profile probing interface and a lot of
-  rewriting that can cause regressions in existing functionality. The biggest
-  challenge here is that an instance of bt_att (shared/att) and GAttrib
-  (attrib/gattrib) cannot coexist on the same file descriptor, since they will
-  cause ATT protocol violations by breaking the sequential request-response
-  structure. A special shared/gatt-client-gattrib implementation may be
-  necessary to move each profile/plugin to the new API before actually switching
-  to the shared/att based implementation.
+- Convert all GATT client-role based profiles to use shared/gatt-client. These
+  profiles are:
+
+    cyclingspeed
+    deviceinfo
+    heartrate
+    input/hog
+    proximity (also has a server component)
+    scanparam
+    thermometer
 
   Priority: Medium
   Complexity: C4
@@ -183,6 +183,11 @@ ATT/GATT (new shared stack)
   Priority: Low
   Complexity: C1
 
+- Replace attrib-server with shared/gatt-db and shared/gatt-server.
+
+  Priority: Medium
+  Complexity: C2
+
 - Implement the server portion of doc/gatt-api.txt using shared/gatt-server once
   it exists.
 
-- 
2.1.0.rc2.206.gedb03e5

Re: [PATCH BlueZ 00/12] core: Use shared/gatt-client.

[Arman Uguray]

Hi all,

> On Mon, Nov 17, 2014 at 11:22 AM, Arman Uguray <armansito@chromium.org> wrote:
> This patch set integrates shared/gatt-client into bluetoothd. The following
> items have been tackled here:
>
>   1. src/device now uses shared/gatt-client to perform service discovery. State
>   updates such as disconnections, service changed events, etc. are done using
>   callback APIs of the new shared framework and attrib/gatt is no longer used to
>   perform GATT operations directly. A GAttrib instance is maintained for
>   backwards compatibility with profiles that haven't been converted yet.
>
>   2. A new gatt-callbacks API has been added which mirrors the attio APIs for
>   shared/gatt-client. Profiles/plugins are notified using these APIs when the
>   bearer disconnects, when services change, and when gatt-client is ready.
>
>   3. The profiles/gatt plugin has been rewritten. Since the GATT service is
>   handled by shared/gatt-client, this profile no longer deals with service
>   changed events performs out of place MTU exchanges, which used to get
>   performed in an arbitrary order. The profile has been renamed to profiles/gap
>   as it now only deals with the GAP service, and reads the "Appearance" and
>   "Device Name" characteristics using the new shared stack.
>
> Arman Uguray (12):
>   attrib/gattrib: Add g_attrib_get_att.
>   shared/att: Add bt_att_get_fd.
>   shared/att: Directly close fd on ATT violations.
>   shared/gatt-client: Call ready_handler only in init.
>   core: device: Use bt_att_register_disconnect.
>   core: device: Use shared/gatt-client for GATT.
>   core: Introduce gatt-callbacks
>   profiles/gatt: Don't handle GATT service.
>   profiles/gatt: Rename profile to gap.
>   profiles/gap: Rewrite using bt_gatt_client.
>   profiles/gap: Add Google copyright.
>   TODO: Update GAttrib related items.
>
>  Makefile.am              |   1 +
>  Makefile.plugins         |   4 +-
>  TODO                     |  25 +--
>  attrib/gattrib.c         |   8 +
>  attrib/gattrib.h         |   3 +
>  profiles/gap/gas.c       | 307 +++++++++++++++++++++++++++++++
>  profiles/gatt/gas.c      | 457 -----------------------------------------------
>  src/device.c             | 456 +++++++++++++++++++++++++++++++---------------
>  src/device.h             |   2 -
>  src/gatt-callbacks.h     |  42 +++++
>  src/shared/att-types.h   |   3 +-
>  src/shared/att.c         |  25 ++-
>  src/shared/att.h         |   1 +
>  src/shared/gatt-client.c |   3 +-
>  14 files changed, 711 insertions(+), 626 deletions(-)
>  create mode 100644 profiles/gap/gas.c
>  delete mode 100644 profiles/gatt/gas.c
>  create mode 100644 src/gatt-callbacks.h
>
> --
> 2.1.0.rc2.206.gedb03e5
>

ping

Re: [PATCH BlueZ 01/12] attrib/gattrib: Add g_attrib_get_att.

[Luiz Augusto von Dentz]

Hi Arman,

On Mon, Nov 17, 2014 at 9:22 PM, Arman Uguray <armansito@chromium.org> wrote:
> Added the g_attrib_get_att function which returns the underlying bt_att
> structure associated with a GAttrib.
> ---
>  attrib/gattrib.c | 8 ++++++++
>  attrib/gattrib.h | 3 +++
>  2 files changed, 11 insertions(+)
>
> diff --git a/attrib/gattrib.c b/attrib/gattrib.c
> index ce7f7b3..a446ae6 100644
> --- a/attrib/gattrib.c
> +++ b/attrib/gattrib.c
> @@ -166,6 +166,14 @@ GIOChannel *g_attrib_get_channel(GAttrib *attrib)
>         return attrib->io;
>  }
>
> +struct bt_att *g_attrib_get_att(GAttrib *attrib)
> +{
> +       if (!attrib)
> +               return NULL;
> +
> +       return attrib->att;
> +}
> +
>  gboolean g_attrib_set_destroy_function(GAttrib *attrib, GDestroyNotify destroy,
>                                                         gpointer user_data)
>  {
> diff --git a/attrib/gattrib.h b/attrib/gattrib.h
> index 2ed57c1..374bac2 100644
> --- a/attrib/gattrib.h
> +++ b/attrib/gattrib.h
> @@ -31,6 +31,7 @@ extern "C" {
>  #define GATTRIB_ALL_REQS 0xFE
>  #define GATTRIB_ALL_HANDLES 0x0000
>
> +struct bt_att;  /* Forward declaration for compatibility */
>  struct _GAttrib;
>  typedef struct _GAttrib GAttrib;
>
> @@ -47,6 +48,8 @@ void g_attrib_unref(GAttrib *attrib);
>
>  GIOChannel *g_attrib_get_channel(GAttrib *attrib);
>
> +struct bt_att *g_attrib_get_att(GAttrib *attrib);
> +
>  gboolean g_attrib_set_destroy_function(GAttrib *attrib,
>                 GDestroyNotify destroy, gpointer user_data);
>
> --
> 2.1.0.rc2.206.gedb03e5

Im not following you here, why you want to expose bt_att via GAttrib,
isn't it better to just have a g_attrib_get_fd if you just need to
access the fd?


-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ 02/12] shared/att: Add bt_att_get_fd.

[Luiz Augusto von Dentz]

Hi Arman,

On Mon, Nov 17, 2014 at 9:22 PM, Arman Uguray <armansito@chromium.org> wrote:
> This patch adds a getter for a bt_att structure's underlying connection
> file descriptor.
> ---
>  src/shared/att.c | 8 ++++++++
>  src/shared/att.h | 1 +
>  2 files changed, 9 insertions(+)
>
> diff --git a/src/shared/att.c b/src/shared/att.c
> index 2bc7682..264cece 100644
> --- a/src/shared/att.c
> +++ b/src/shared/att.c
> @@ -907,6 +907,14 @@ void bt_att_unref(struct bt_att *att)
>         free(att);
>  }
>
> +int bt_att_get_fd(struct bt_att *att)
> +{
> +       if (!att)
> +               return -1;

It probably make sense to return -ENOTCONN since we are not setting
any errno here.

> +       return att->fd;
> +}
> +
>  bool bt_att_set_close_on_unref(struct bt_att *att, bool do_close)
>  {
>         if (!att || !att->io)
> diff --git a/src/shared/att.h b/src/shared/att.h
> index 99b5a5b..b946b18 100644
> --- a/src/shared/att.h
> +++ b/src/shared/att.h
> @@ -33,6 +33,7 @@ struct bt_att *bt_att_new(int fd);
>  struct bt_att *bt_att_ref(struct bt_att *att);
>  void bt_att_unref(struct bt_att *att);
>
> +int bt_att_get_fd(struct bt_att *att);
>  bool bt_att_set_close_on_unref(struct bt_att *att, bool do_close);
>
>  typedef void (*bt_att_response_func_t)(uint8_t opcode, const void *pdu,
> --
> 2.1.0.rc2.206.gedb03e5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ 01/12] attrib/gattrib: Add g_attrib_get_att.

[Arman Uguray]

Hi Luiz,

> On Wed, Nov 19, 2014 at 7:05 AM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
> Hi Arman,
>
> On Mon, Nov 17, 2014 at 9:22 PM, Arman Uguray <armansito@chromium.org> wrote:
>> Added the g_attrib_get_att function which returns the underlying bt_att
>> structure associated with a GAttrib.
>> ---
>>  attrib/gattrib.c | 8 ++++++++
>>  attrib/gattrib.h | 3 +++
>>  2 files changed, 11 insertions(+)
>>
>> diff --git a/attrib/gattrib.c b/attrib/gattrib.c
>> index ce7f7b3..a446ae6 100644
>> --- a/attrib/gattrib.c
>> +++ b/attrib/gattrib.c
>> @@ -166,6 +166,14 @@ GIOChannel *g_attrib_get_channel(GAttrib *attrib)
>>         return attrib->io;
>>  }
>>
>> +struct bt_att *g_attrib_get_att(GAttrib *attrib)
>> +{
>> +       if (!attrib)
>> +               return NULL;
>> +
>> +       return attrib->att;
>> +}
>> +
>>  gboolean g_attrib_set_destroy_function(GAttrib *attrib, GDestroyNotify destroy,
>>                                                         gpointer user_data)
>>  {
>> diff --git a/attrib/gattrib.h b/attrib/gattrib.h
>> index 2ed57c1..374bac2 100644
>> --- a/attrib/gattrib.h
>> +++ b/attrib/gattrib.h
>> @@ -31,6 +31,7 @@ extern "C" {
>>  #define GATTRIB_ALL_REQS 0xFE
>>  #define GATTRIB_ALL_HANDLES 0x0000
>>
>> +struct bt_att;  /* Forward declaration for compatibility */
>>  struct _GAttrib;
>>  typedef struct _GAttrib GAttrib;
>>
>> @@ -47,6 +48,8 @@ void g_attrib_unref(GAttrib *attrib);
>>
>>  GIOChannel *g_attrib_get_channel(GAttrib *attrib);
>>
>> +struct bt_att *g_attrib_get_att(GAttrib *attrib);
>> +
>>  gboolean g_attrib_set_destroy_function(GAttrib *attrib,
>>                 GDestroyNotify destroy, gpointer user_data);
>>
>> --
>> 2.1.0.rc2.206.gedb03e5
>
> Im not following you here, why you want to expose bt_att via GAttrib,
> isn't it better to just have a g_attrib_get_fd if you just need to
> access the fd?
>

That's the whole point; the new code will use bt_att directly while
the old code will go through the GAttrib shim around bt_att until we
transition everything to use shared/gatt. At that point we will no
longer need the shim and we can have src/device create a bt_att
directly.

>
> --
> Luiz Augusto von Dentz

Thanks,
Arman

Re: [PATCH BlueZ 01/12] attrib/gattrib: Add g_attrib_get_att.

[Michael Janssen]

Hi Arman, Luiz:

On Wed Nov 19 2014 at 7:05:50 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Arman,
>
> On Mon, Nov 17, 2014 at 9:22 PM, Arman Uguray <armansito@chromium.org> wrote:
> > Added the g_attrib_get_att function which returns the underlying bt_att
> > structure associated with a GAttrib.
> > ---
> >  attrib/gattrib.c | 8 ++++++++
> >  attrib/gattrib.h | 3 +++
> >  2 files changed, 11 insertions(+)
> >
> > diff --git a/attrib/gattrib.c b/attrib/gattrib.c
> > index ce7f7b3..a446ae6 100644
> > --- a/attrib/gattrib.c
> > +++ b/attrib/gattrib.c
> > @@ -166,6 +166,14 @@ GIOChannel *g_attrib_get_channel(GAttrib *attrib)
> >         return attrib->io;
> >  }
> >
> > +struct bt_att *g_attrib_get_att(GAttrib *attrib)
> > +{
> > +       if (!attrib)
> > +               return NULL;
> > +
> > +       return attrib->att;
> > +}
> > +
> >  gboolean g_attrib_set_destroy_function(GAttrib *attrib, GDestroyNotify destroy,
> >                                                         gpointer user_data)
> >  {
> > diff --git a/attrib/gattrib.h b/attrib/gattrib.h
> > index 2ed57c1..374bac2 100644
> > --- a/attrib/gattrib.h
> > +++ b/attrib/gattrib.h
> > @@ -31,6 +31,7 @@ extern "C" {
> >  #define GATTRIB_ALL_REQS 0xFE
> >  #define GATTRIB_ALL_HANDLES 0x0000
> >
> > +struct bt_att;  /* Forward declaration for compatibility */
> >  struct _GAttrib;
> >  typedef struct _GAttrib GAttrib;
> >
> > @@ -47,6 +48,8 @@ void g_attrib_unref(GAttrib *attrib);
> >
> >  GIOChannel *g_attrib_get_channel(GAttrib *attrib);
> >
> > +struct bt_att *g_attrib_get_att(GAttrib *attrib);
> > +
> >  gboolean g_attrib_set_destroy_function(GAttrib *attrib,
> >                 GDestroyNotify destroy, gpointer user_data);
> >
> > --
> > 2.1.0.rc2.206.gedb03e5
>
> Im not following you here, why you want to expose bt_att via GAttrib,
> isn't it better to just have a g_attrib_get_fd if you just need to
> access the fd?
>
>
> --
> Luiz Augusto von Dentz

It's my understanding that the g_attrib_get_att function is needed in
the transition from GAttrib to bt_att.  For now the GAttrib creates
the bt_att, but eventually device_attach_attrib will create the bt_att
on it's own once GAttrib is not needed anymore.  I don't have an issue
with the reference semantics as long as it's clear.

 Mike

Re: [PATCH BlueZ 06/12] core: device: Use shared/gatt-client for GATT.

[Luiz Augusto von Dentz]

Hi Arman,

On Mon, Nov 17, 2014 at 9:22 PM, Arman Uguray <armansito@chromium.org> wrote:
> This patch changes the GATT service discovery code path with the
> following:
>    - As part for device_attach_attrib, a bt_gatt_client structure is
>    created that performs MTU exchange and service discovery, caches the
>    services in an internal list, and handles the remote GATT service.
>
>    - The device_browse_primary code path obtains service information
>    by iterating through bt_gatt_client's services, instead of directly
>    initiating primary and secondary service discovery using attrib/gatt.
>    If the bt_gatt_client isn't ready at the time, the code defers
>    registration of services and profile probing until the ready handler
>    is called.
>
>    - The attio connected callbacks are invoked from bt_gatt_client's
>    ready handler instead of device_attach_attrib.
> ---

I would suggest that before we start changing the core we make sure we
have completed the TODO items for bt_gatt_client/bt_gatt_server, there
are items such as gatt-client + gatt-server coexistence that haven't
been tested  and I also remember that we had some ideas about a new
object that would take care of both client and server under it, the
caching probably needs to be addressed as well otherwise
bt_gatt_client will always end up discovering the service in every
connection.


>  src/device.c           | 286 ++++++++++++++++++++++++++-----------------------
>  src/device.h           |   2 -
>  src/shared/att-types.h |   3 +-
>  3 files changed, 152 insertions(+), 139 deletions(-)
>
> diff --git a/src/device.c b/src/device.c
> index 45c7e9c..3d0159e 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -74,6 +74,10 @@
>  #define DISCONNECT_TIMER       2
>  #define DISCOVERY_TIMER                1
>
> +#ifndef MIN
> +#define MIN(a, b) ((a) < (b) ? (a) : (b))
> +#endif
> +
>  static DBusConnection *dbus_conn = NULL;
>  unsigned service_state_cb_id;
>
> @@ -205,9 +209,12 @@ struct btd_device {
>         GSList          *attios_offline;
>         guint           attachid;               /* Attrib server attach */
>
> -       struct bt_att *att;     /* The new ATT transport */
> +       struct bt_att *att;                     /* The new ATT transport */
> +       uint16_t att_mtu;                       /* The ATT MTU */
>         unsigned int att_disconn_id;
>
> +       struct bt_gatt_client *gatt_client;     /* GATT client cache */
> +
>         struct bearer_state bredr_state;
>         struct bearer_state le_state;
>
> @@ -463,6 +470,18 @@ static void browse_request_free(struct browse_req *req)
>         g_free(req);
>  }
>
> +static void gatt_client_cleanup(struct btd_device *device)
> +{
> +       if (!device->gatt_client)
> +               return;
> +
> +       bt_gatt_client_set_service_changed(device->gatt_client, NULL, NULL,
> +                                                                       NULL);
> +       bt_gatt_client_set_ready_handler(device->gatt_client, NULL, NULL, NULL);
> +       bt_gatt_client_unref(device->gatt_client);
> +       device->gatt_client = NULL;
> +}
> +
>  static void attio_cleanup(struct btd_device *device)
>  {
>         if (device->attachid) {
> @@ -480,6 +499,8 @@ static void attio_cleanup(struct btd_device *device)
>                 device->att_io = NULL;
>         }
>
> +       gatt_client_cleanup(device);
> +
>         if (device->att) {
>                 bt_att_unref(device->att);
>                 device->att = NULL;
> @@ -3445,41 +3466,6 @@ done:
>         attio_cleanup(device);
>  }
>
> -static void register_all_services(struct browse_req *req, GSList *services)
> -{
> -       struct btd_device *device = req->device;
> -
> -       btd_device_set_temporary(device, FALSE);
> -
> -       update_gatt_services(req, device->primaries, services);
> -       g_slist_free_full(device->primaries, g_free);
> -       device->primaries = NULL;
> -
> -       device_register_primaries(device, services, -1);
> -
> -       device_probe_profiles(device, req->profiles_added);
> -
> -       if (device->attios == NULL && device->attios_offline == NULL)
> -               attio_cleanup(device);
> -
> -       g_dbus_emit_property_changed(dbus_conn, device->path,
> -                                               DEVICE_INTERFACE, "UUIDs");
> -
> -       device_svc_resolved(device, device->bdaddr_type, 0);
> -
> -       store_services(device);
> -
> -       browse_request_free(req);
> -}
> -
> -static int service_by_range_cmp(gconstpointer a, gconstpointer b)
> -{
> -       const struct gatt_primary *prim = a;
> -       const struct att_range *range = b;
> -
> -       return memcmp(&prim->range, range, sizeof(*range));
> -}
> -
>  static void send_le_browse_response(struct browse_req *req)
>  {
>         struct btd_device *dev = req->device;
> @@ -3510,122 +3496,152 @@ static void send_le_browse_response(struct browse_req *req)
>         g_dbus_send_reply(dbus_conn, msg, DBUS_TYPE_INVALID);
>  }
>
> -static void find_included_cb(uint8_t status, GSList *includes, void *user_data)
> +/*
> + * TODO: Remove this function once btd_device stops storing gatt_primary
> + * structures for discovered services.
> + */
> +static bool populate_primaries(struct btd_device *device, GSList **services)
>  {
> -       struct included_search *search = user_data;
> -       struct btd_device *device = search->req->device;
> +       struct bt_gatt_service_iter iter;
> +       const bt_gatt_service_t *service;
>         struct gatt_primary *prim;
> -       GSList *l;
> +       uint128_t u128;
> +       bt_uuid_t uuid;
>
> -       DBG("status %u", status);
> +       /* Create gatt_primary structures for all primary/secondary services */
> +       if (!bt_gatt_service_iter_init(&iter, device->gatt_client))
> +               return false;
>
> -       if (device->attrib == NULL || status) {
> -               struct browse_req *req = device->browse;
> +       while (bt_gatt_service_iter_next(&iter, &service)) {
> +               prim = g_new0(struct gatt_primary, 1);
> +               prim->range.start = service->start_handle;
> +               prim->range.end = service->end_handle;
>
> -               if (status)
> -                       error("Find included services failed: %s (%d)",
> -                                       att_ecode2str(status), status);
> -               else
> -                       error("Disconnected while doing included discovery");
> +               memcpy(u128.data, service->uuid, 16);
> +               bt_uuid128_create(&uuid, u128);
> +               bt_uuid_to_string(&uuid, prim->uuid, MAX_LEN_UUID_STR + 1);
>
> -               if (!req)
> -                       goto complete;
> +               *services = g_slist_append(*services, prim);
> +       }
>
> +       return true;
> +}
> +
> +static void register_gatt_services(struct browse_req *req)
> +{
> +       struct btd_device *device = req->device;
> +       GSList *services = NULL;
> +
> +       if (!bt_gatt_client_is_ready(device->gatt_client))
> +               return;
> +
> +       if (!populate_primaries(device, &services)) {
>                 send_le_browse_response(req);
>                 device->browse = NULL;
>                 browse_request_free(req);
> -
> -               goto complete;
> +               return;
>         }
>
> -       if (includes == NULL)
> -               goto next;
> +       btd_device_set_temporary(device, FALSE);
>
> -       for (l = includes; l; l = l->next) {
> -               struct gatt_included *incl = l->data;
> +       update_gatt_services(req, device->primaries, services);
> +       g_slist_free_full(device->primaries, g_free);
> +       device->primaries = NULL;
>
> -               if (g_slist_find_custom(search->services, &incl->range,
> -                                               service_by_range_cmp))
> -                       continue;
> +       device_register_primaries(device, services, -1);
>
> -               prim = g_new0(struct gatt_primary, 1);
> -               memcpy(prim->uuid, incl->uuid, sizeof(prim->uuid));
> -               memcpy(&prim->range, &incl->range, sizeof(prim->range));
> +       device_probe_profiles(device, req->profiles_added);
>
> -               search->services = g_slist_append(search->services, prim);
> -       }
> +       if (device->attios == NULL && device->attios_offline == NULL)
> +               attio_cleanup(device);
>
> -next:
> -       search->current = search->current->next;
> -       if (search->current == NULL) {
> -               register_all_services(search->req, search->services);
> -               search->services = NULL;
> -               goto complete;
> -       }
> +       g_dbus_emit_property_changed(dbus_conn, device->path,
> +                                               DEVICE_INTERFACE, "UUIDs");
>
> -       prim = search->current->data;
> -       gatt_find_included(device->attrib, prim->range.start, prim->range.end,
> -                                       find_included_cb, search);
> -       return;
> +       device_svc_resolved(device, device->bdaddr_type, 0);
>
> -complete:
> -       g_slist_free_full(search->services, g_free);
> -       g_free(search);
> +       store_services(device);
> +
> +       browse_request_free(req);
>  }
>
> -static void find_included_services(struct browse_req *req, GSList *services)
> +static void gatt_client_ready_cb(bool success, uint8_t att_ecode,
> +                                                               void *user_data)
>  {
> -       struct btd_device *device = req->device;
> -       struct included_search *search;
> -       struct gatt_primary *prim;
> -       GSList *l;
> +       struct btd_device *device = user_data;
>
> -       DBG("service count %u", g_slist_length(services));
> +       DBG("gatt-client ready -- status: %s, ATT error: %u",
> +                                               success ? "success" : "failed",
> +                                               att_ecode);
> +
> +       if (!success) {
> +               if (device->browse) {
> +                       struct browse_req *req = device->browse;
> +                       send_le_browse_response(req);
> +                       device->browse = NULL;
> +                       browse_request_free(req);
> +               }
>
> -       if (services == NULL) {
> -               DBG("No services found");
> -               register_all_services(req, NULL);
>                 return;
>         }
>
> -       search = g_new0(struct included_search, 1);
> -       search->req = req;
> +       device->att_mtu = bt_att_get_mtu(device->att);
>
> -       /* We have to completely duplicate the data in order to have a
> -        * clearly defined responsibility of freeing regardless of
> -        * failure or success. Otherwise memory leaks are inevitable.
> -        */
> -       for (l = services; l; l = g_slist_next(l)) {
> -               struct gatt_primary *dup;
> +       DBG("gatt-client exchanged MTU: %u", device->att_mtu);
>
> -               dup = g_memdup(l->data, sizeof(struct gatt_primary));
> +       if (device->browse)
> +               register_gatt_services(device->browse);
>
> -               search->services = g_slist_append(search->services, dup);
> -       }
> +       /*
> +        * TODO: Change attio callbacks to accept a gatt-client instead of a
> +        * GAttrib.
> +        */
> +       g_slist_foreach(device->attios, attio_connected, device->attrib);
> +}
>
> -       search->current = search->services;
> +static void gatt_client_service_changed(uint16_t start_handle,
> +                                                       uint16_t end_handle,
> +                                                       void *user_data)
> +{
> +       struct btd_device *device = user_data;
> +
> +       DBG("gatt-client: Service Changed: start 0x%04x, end: 0x%04x",
> +                                               start_handle, end_handle);
>
> -       prim = search->current->data;
> -       gatt_find_included(device->attrib, prim->range.start, prim->range.end,
> -                                       find_included_cb, search);
> +       /*
> +        * TODO: Notify clients that services changed so they can handle it
> +        * directly. Remove the profile if a service was removed.
> +        */
> +       device_browse_primary(device, NULL);
>  }
>
> -static void primary_cb(uint8_t status, GSList *services, void *user_data)
> +static void gatt_client_init(struct btd_device *device)
>  {
> -       struct browse_req *req = user_data;
> +       gatt_client_cleanup(device);
>
> -       DBG("status %u", status);
> +       device->gatt_client = bt_gatt_client_new(device->att, device->att_mtu);
> +       if (!device->gatt_client) {
> +               DBG("Failed to initialize gatt-client");
> +               return;
> +       }
>
> -       if (status) {
> -               struct btd_device *device = req->device;
> +       if (!bt_gatt_client_set_ready_handler(device->gatt_client,
> +                                                       gatt_client_ready_cb,
> +                                                       device, NULL)) {
> +               DBG("Failed to set ready handler for gatt-client");
> +               gatt_client_cleanup(device);
> +               return;
> +       }
>
> -               send_le_browse_response(req);
> -               device->browse = NULL;
> -               browse_request_free(req);
> +       if (!bt_gatt_client_set_service_changed(device->gatt_client,
> +                                               gatt_client_service_changed,
> +                                               device, NULL)) {
> +               DBG("Failed to set service changed handler for gatt-client");
> +               gatt_client_cleanup(device);
>                 return;
>         }
>
> -       find_included_services(req, services);
> +       DBG("gatt-client created");
>  }
>
>  bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
> @@ -3660,10 +3676,8 @@ bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
>                 }
>         }
>
> -       if (cid == ATT_CID)
> -               mtu = BT_ATT_DEFAULT_LE_MTU;
> -
> -       attrib = g_attrib_new(io, mtu);
> +       dev->att_mtu = MIN(mtu, BT_ATT_MAX_LE_MTU);
> +       attrib = g_attrib_new(io, dev->att_mtu);
>         if (!attrib) {
>                 error("Unable to create new GAttrib instance");
>                 return false;
> @@ -3678,11 +3692,12 @@ bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
>
>         dev->attrib = attrib;
>         dev->att = bt_att_ref(g_attrib_get_att(attrib));
> -
>         dev->att_disconn_id = bt_att_register_disconnect(dev->att,
>                                                 att_disconnected_cb, dev, NULL);
>         bt_att_set_close_on_unref(dev->att, true);
>
> +       gatt_client_init(dev);
> +
>         /*
>          * Remove the device from the connect_list and give the passive
>          * scanning another chance to be restarted in case there are
> @@ -3690,8 +3705,6 @@ bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
>          */
>         adapter_connect_list_remove(dev->adapter, dev);
>
> -       g_slist_foreach(dev->attios, attio_connected, dev->attrib);
> -
>         return true;
>  }
>
> @@ -3845,13 +3858,29 @@ static void att_browse_error_cb(const GError *gerr, gpointer user_data)
>         browse_request_free(req);
>  }
>
> +static void browse_gatt_client(struct browse_req *req)
> +{
> +       struct btd_device *device = req->device;
> +
> +       if (!device->gatt_client) {
> +               DBG("No gatt-client currently attached");
> +               return;
> +       }
> +
> +       /*
> +        * If gatt-client is ready, then register all services. Otherwise, this
> +        * will be deferred until client becomes ready.
> +        */
> +       if (bt_gatt_client_is_ready(device->gatt_client))
> +               register_gatt_services(req);
> +}
> +
>  static void att_browse_cb(gpointer user_data)
>  {
>         struct att_callbacks *attcb = user_data;
>         struct btd_device *device = attcb->user_data;
>
> -       gatt_discover_primary(device->attrib, NULL, primary_cb,
> -                                                       device->browse);
> +       browse_gatt_client(device->browse);
>  }
>
>  static int device_browse_primary(struct btd_device *device, DBusMessage *msg)
> @@ -3869,7 +3898,7 @@ static int device_browse_primary(struct btd_device *device, DBusMessage *msg)
>         device->browse = req;
>
>         if (device->attrib) {
> -               gatt_discover_primary(device->attrib, NULL, primary_cb, req);
> +               browse_gatt_client(device->browse);
>                 goto done;
>         }
>
> @@ -4738,21 +4767,6 @@ GSList *btd_device_get_primaries(struct btd_device *device)
>         return device->primaries;
>  }
>
> -void btd_device_gatt_set_service_changed(struct btd_device *device,
> -                                               uint16_t start, uint16_t end)
> -{
> -       GSList *l;
> -
> -       for (l = device->primaries; l; l = g_slist_next(l)) {
> -               struct gatt_primary *prim = l->data;
> -
> -               if (start <= prim->range.end && end >= prim->range.start)
> -                       prim->changed = TRUE;
> -       }
> -
> -       device_browse_primary(device, NULL);
> -}
> -
>  void btd_device_add_uuid(struct btd_device *device, const char *uuid)
>  {
>         GSList *uuid_list;
> diff --git a/src/device.h b/src/device.h
> index 2e0473e..00cb502 100644
> --- a/src/device.h
> +++ b/src/device.h
> @@ -67,8 +67,6 @@ const sdp_record_t *btd_device_get_record(struct btd_device *device,
>  struct gatt_primary *btd_device_get_primary(struct btd_device *device,
>                                                         const char *uuid);
>  GSList *btd_device_get_primaries(struct btd_device *device);
> -void btd_device_gatt_set_service_changed(struct btd_device *device,
> -                                               uint16_t start, uint16_t end);
>  bool device_attach_attrib(struct btd_device *dev, GIOChannel *io);
>  void btd_device_add_uuid(struct btd_device *device, const char *uuid);
>  void device_add_eir_uuids(struct btd_device *dev, GSList *uuids);
> diff --git a/src/shared/att-types.h b/src/shared/att-types.h
> index 3d8829a..8b6d537 100644
> --- a/src/shared/att-types.h
> +++ b/src/shared/att-types.h
> @@ -27,7 +27,8 @@
>  #define __packed __attribute__((packed))
>  #endif
>
> -#define BT_ATT_DEFAULT_LE_MTU 23
> +#define BT_ATT_DEFAULT_LE_MTU  23
> +#define BT_ATT_MAX_LE_MTU      517
>
>  /* ATT protocol opcodes */
>  #define BT_ATT_OP_ERROR_RSP                    0x01
> --
> 2.1.0.rc2.206.gedb03e5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ 01/12] attrib/gattrib: Add g_attrib_get_att.

[Luiz Augusto von Dentz]

Hi Arman, Michael,

On Wed, Nov 19, 2014 at 5:15 PM, Michael Janssen <jamuraa@google.com> wrote:
> Hi Arman, Luiz:
>
> On Wed Nov 19 2014 at 7:05:50 AM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
>>
>> Hi Arman,
>>
>> On Mon, Nov 17, 2014 at 9:22 PM, Arman Uguray <armansito@chromium.org> wrote:
>> > Added the g_attrib_get_att function which returns the underlying bt_att
>> > structure associated with a GAttrib.
>> > ---
>> >  attrib/gattrib.c | 8 ++++++++
>> >  attrib/gattrib.h | 3 +++
>> >  2 files changed, 11 insertions(+)
>> >
>> > diff --git a/attrib/gattrib.c b/attrib/gattrib.c
>> > index ce7f7b3..a446ae6 100644
>> > --- a/attrib/gattrib.c
>> > +++ b/attrib/gattrib.c
>> > @@ -166,6 +166,14 @@ GIOChannel *g_attrib_get_channel(GAttrib *attrib)
>> >         return attrib->io;
>> >  }
>> >
>> > +struct bt_att *g_attrib_get_att(GAttrib *attrib)
>> > +{
>> > +       if (!attrib)
>> > +               return NULL;
>> > +
>> > +       return attrib->att;
>> > +}
>> > +
>> >  gboolean g_attrib_set_destroy_function(GAttrib *attrib, GDestroyNotify destroy,
>> >                                                         gpointer user_data)
>> >  {
>> > diff --git a/attrib/gattrib.h b/attrib/gattrib.h
>> > index 2ed57c1..374bac2 100644
>> > --- a/attrib/gattrib.h
>> > +++ b/attrib/gattrib.h
>> > @@ -31,6 +31,7 @@ extern "C" {
>> >  #define GATTRIB_ALL_REQS 0xFE
>> >  #define GATTRIB_ALL_HANDLES 0x0000
>> >
>> > +struct bt_att;  /* Forward declaration for compatibility */
>> >  struct _GAttrib;
>> >  typedef struct _GAttrib GAttrib;
>> >
>> > @@ -47,6 +48,8 @@ void g_attrib_unref(GAttrib *attrib);
>> >
>> >  GIOChannel *g_attrib_get_channel(GAttrib *attrib);
>> >
>> > +struct bt_att *g_attrib_get_att(GAttrib *attrib);
>> > +
>> >  gboolean g_attrib_set_destroy_function(GAttrib *attrib,
>> >                 GDestroyNotify destroy, gpointer user_data);
>> >
>> > --
>> > 2.1.0.rc2.206.gedb03e5
>>
>> Im not following you here, why you want to expose bt_att via GAttrib,
>> isn't it better to just have a g_attrib_get_fd if you just need to
>> access the fd?
>>
>>
>> --
>> Luiz Augusto von Dentz
>
> It's my understanding that the g_attrib_get_att function is needed in
> the transition from GAttrib to bt_att.  For now the GAttrib creates
> the bt_att, but eventually device_attach_attrib will create the bt_att
> on it's own once GAttrib is not needed anymore.  I don't have an issue
> with the reference semantics as long as it's clear.

I see, we might just return a void pointer here instead, although I
still believe this is a bit too soon to start changing things directly
on the core without proper unit tests.


-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ 06/12] core: device: Use shared/gatt-client for GATT.

[Arman Uguray]

Hi Luiz,

> On Wed, Nov 19, 2014 at 7:43 AM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
> Hi Arman,
>
> On Mon, Nov 17, 2014 at 9:22 PM, Arman Uguray <armansito@chromium.org> wrote:
>> This patch changes the GATT service discovery code path with the
>> following:
>>    - As part for device_attach_attrib, a bt_gatt_client structure is
>>    created that performs MTU exchange and service discovery, caches the
>>    services in an internal list, and handles the remote GATT service.
>>
>>    - The device_browse_primary code path obtains service information
>>    by iterating through bt_gatt_client's services, instead of directly
>>    initiating primary and secondary service discovery using attrib/gatt.
>>    If the bt_gatt_client isn't ready at the time, the code defers
>>    registration of services and profile probing until the ready handler
>>    is called.
>>
>>    - The attio connected callbacks are invoked from bt_gatt_client's
>>    ready handler instead of device_attach_attrib.
>> ---
>
> I would suggest that before we start changing the core we make sure we
> have completed the TODO items for bt_gatt_client/bt_gatt_server, there
> are items such as gatt-client + gatt-server coexistence that haven't
> been tested  and I also remember that we had some ideas about a new
> object that would take care of both client and server under it, the
> caching probably needs to be addressed as well otherwise
> bt_gatt_client will always end up discovering the service in every
> connection.
>

I think we have to start somewhere. I think at this point we have the
basic feature set implemented to start converting the core and at
least have behavior that's equivalent (or better) to what the older
code did. I didn't want to immediately involve the server until seeing
how the client fits in and we still have to figure out how not to
break the existing server role profiles (the experimental ones) while
integrating bt_gatt_server (or maybe we don't care since those are all
experimental and we can perhaps just throw out src/attrib-server).
Either way I wanted to tackle this in several steps and move
iteratively.

So, I guess the one big item is the missing caching support, which I
will now get started on, but in the meantime I think we should start
converting the core, see if there are any bugs and edge cases we have
to handle, etc. I want to see how well the plugins work with the new
code and more importantly I want to have the GATT D-Bus API
implemented and merged into the tree (at least as experimental) and
get people to start using it and send feedback.

In the meantime I'm going to start working on two API functions like
bt_gatt_client_new_from_storage & bt_gatt_client_store_attributes so
you can initialize the cache from the contents of a file and commit
them to a file. That should make things easy to integrate into the
core.

>
>>  src/device.c           | 286 ++++++++++++++++++++++++++-----------------------
>>  src/device.h           |   2 -
>>  src/shared/att-types.h |   3 +-
>>  3 files changed, 152 insertions(+), 139 deletions(-)
>>
>> diff --git a/src/device.c b/src/device.c
>> index 45c7e9c..3d0159e 100644
>> --- a/src/device.c
>> +++ b/src/device.c
>> @@ -74,6 +74,10 @@
>>  #define DISCONNECT_TIMER       2
>>  #define DISCOVERY_TIMER                1
>>
>> +#ifndef MIN
>> +#define MIN(a, b) ((a) < (b) ? (a) : (b))
>> +#endif
>> +
>>  static DBusConnection *dbus_conn = NULL;
>>  unsigned service_state_cb_id;
>>
>> @@ -205,9 +209,12 @@ struct btd_device {
>>         GSList          *attios_offline;
>>         guint           attachid;               /* Attrib server attach */
>>
>> -       struct bt_att *att;     /* The new ATT transport */
>> +       struct bt_att *att;                     /* The new ATT transport */
>> +       uint16_t att_mtu;                       /* The ATT MTU */
>>         unsigned int att_disconn_id;
>>
>> +       struct bt_gatt_client *gatt_client;     /* GATT client cache */
>> +
>>         struct bearer_state bredr_state;
>>         struct bearer_state le_state;
>>
>> @@ -463,6 +470,18 @@ static void browse_request_free(struct browse_req *req)
>>         g_free(req);
>>  }
>>
>> +static void gatt_client_cleanup(struct btd_device *device)
>> +{
>> +       if (!device->gatt_client)
>> +               return;
>> +
>> +       bt_gatt_client_set_service_changed(device->gatt_client, NULL, NULL,
>> +                                                                       NULL);
>> +       bt_gatt_client_set_ready_handler(device->gatt_client, NULL, NULL, NULL);
>> +       bt_gatt_client_unref(device->gatt_client);
>> +       device->gatt_client = NULL;
>> +}
>> +
>>  static void attio_cleanup(struct btd_device *device)
>>  {
>>         if (device->attachid) {
>> @@ -480,6 +499,8 @@ static void attio_cleanup(struct btd_device *device)
>>                 device->att_io = NULL;
>>         }
>>
>> +       gatt_client_cleanup(device);
>> +
>>         if (device->att) {
>>                 bt_att_unref(device->att);
>>                 device->att = NULL;
>> @@ -3445,41 +3466,6 @@ done:
>>         attio_cleanup(device);
>>  }
>>
>> -static void register_all_services(struct browse_req *req, GSList *services)
>> -{
>> -       struct btd_device *device = req->device;
>> -
>> -       btd_device_set_temporary(device, FALSE);
>> -
>> -       update_gatt_services(req, device->primaries, services);
>> -       g_slist_free_full(device->primaries, g_free);
>> -       device->primaries = NULL;
>> -
>> -       device_register_primaries(device, services, -1);
>> -
>> -       device_probe_profiles(device, req->profiles_added);
>> -
>> -       if (device->attios == NULL && device->attios_offline == NULL)
>> -               attio_cleanup(device);
>> -
>> -       g_dbus_emit_property_changed(dbus_conn, device->path,
>> -                                               DEVICE_INTERFACE, "UUIDs");
>> -
>> -       device_svc_resolved(device, device->bdaddr_type, 0);
>> -
>> -       store_services(device);
>> -
>> -       browse_request_free(req);
>> -}
>> -
>> -static int service_by_range_cmp(gconstpointer a, gconstpointer b)
>> -{
>> -       const struct gatt_primary *prim = a;
>> -       const struct att_range *range = b;
>> -
>> -       return memcmp(&prim->range, range, sizeof(*range));
>> -}
>> -
>>  static void send_le_browse_response(struct browse_req *req)
>>  {
>>         struct btd_device *dev = req->device;
>> @@ -3510,122 +3496,152 @@ static void send_le_browse_response(struct browse_req *req)
>>         g_dbus_send_reply(dbus_conn, msg, DBUS_TYPE_INVALID);
>>  }
>>
>> -static void find_included_cb(uint8_t status, GSList *includes, void *user_data)
>> +/*
>> + * TODO: Remove this function once btd_device stops storing gatt_primary
>> + * structures for discovered services.
>> + */
>> +static bool populate_primaries(struct btd_device *device, GSList **services)
>>  {
>> -       struct included_search *search = user_data;
>> -       struct btd_device *device = search->req->device;
>> +       struct bt_gatt_service_iter iter;
>> +       const bt_gatt_service_t *service;
>>         struct gatt_primary *prim;
>> -       GSList *l;
>> +       uint128_t u128;
>> +       bt_uuid_t uuid;
>>
>> -       DBG("status %u", status);
>> +       /* Create gatt_primary structures for all primary/secondary services */
>> +       if (!bt_gatt_service_iter_init(&iter, device->gatt_client))
>> +               return false;
>>
>> -       if (device->attrib == NULL || status) {
>> -               struct browse_req *req = device->browse;
>> +       while (bt_gatt_service_iter_next(&iter, &service)) {
>> +               prim = g_new0(struct gatt_primary, 1);
>> +               prim->range.start = service->start_handle;
>> +               prim->range.end = service->end_handle;
>>
>> -               if (status)
>> -                       error("Find included services failed: %s (%d)",
>> -                                       att_ecode2str(status), status);
>> -               else
>> -                       error("Disconnected while doing included discovery");
>> +               memcpy(u128.data, service->uuid, 16);
>> +               bt_uuid128_create(&uuid, u128);
>> +               bt_uuid_to_string(&uuid, prim->uuid, MAX_LEN_UUID_STR + 1);
>>
>> -               if (!req)
>> -                       goto complete;
>> +               *services = g_slist_append(*services, prim);
>> +       }
>>
>> +       return true;
>> +}
>> +
>> +static void register_gatt_services(struct browse_req *req)
>> +{
>> +       struct btd_device *device = req->device;
>> +       GSList *services = NULL;
>> +
>> +       if (!bt_gatt_client_is_ready(device->gatt_client))
>> +               return;
>> +
>> +       if (!populate_primaries(device, &services)) {
>>                 send_le_browse_response(req);
>>                 device->browse = NULL;
>>                 browse_request_free(req);
>> -
>> -               goto complete;
>> +               return;
>>         }
>>
>> -       if (includes == NULL)
>> -               goto next;
>> +       btd_device_set_temporary(device, FALSE);
>>
>> -       for (l = includes; l; l = l->next) {
>> -               struct gatt_included *incl = l->data;
>> +       update_gatt_services(req, device->primaries, services);
>> +       g_slist_free_full(device->primaries, g_free);
>> +       device->primaries = NULL;
>>
>> -               if (g_slist_find_custom(search->services, &incl->range,
>> -                                               service_by_range_cmp))
>> -                       continue;
>> +       device_register_primaries(device, services, -1);
>>
>> -               prim = g_new0(struct gatt_primary, 1);
>> -               memcpy(prim->uuid, incl->uuid, sizeof(prim->uuid));
>> -               memcpy(&prim->range, &incl->range, sizeof(prim->range));
>> +       device_probe_profiles(device, req->profiles_added);
>>
>> -               search->services = g_slist_append(search->services, prim);
>> -       }
>> +       if (device->attios == NULL && device->attios_offline == NULL)
>> +               attio_cleanup(device);
>>
>> -next:
>> -       search->current = search->current->next;
>> -       if (search->current == NULL) {
>> -               register_all_services(search->req, search->services);
>> -               search->services = NULL;
>> -               goto complete;
>> -       }
>> +       g_dbus_emit_property_changed(dbus_conn, device->path,
>> +                                               DEVICE_INTERFACE, "UUIDs");
>>
>> -       prim = search->current->data;
>> -       gatt_find_included(device->attrib, prim->range.start, prim->range.end,
>> -                                       find_included_cb, search);
>> -       return;
>> +       device_svc_resolved(device, device->bdaddr_type, 0);
>>
>> -complete:
>> -       g_slist_free_full(search->services, g_free);
>> -       g_free(search);
>> +       store_services(device);
>> +
>> +       browse_request_free(req);
>>  }
>>
>> -static void find_included_services(struct browse_req *req, GSList *services)
>> +static void gatt_client_ready_cb(bool success, uint8_t att_ecode,
>> +                                                               void *user_data)
>>  {
>> -       struct btd_device *device = req->device;
>> -       struct included_search *search;
>> -       struct gatt_primary *prim;
>> -       GSList *l;
>> +       struct btd_device *device = user_data;
>>
>> -       DBG("service count %u", g_slist_length(services));
>> +       DBG("gatt-client ready -- status: %s, ATT error: %u",
>> +                                               success ? "success" : "failed",
>> +                                               att_ecode);
>> +
>> +       if (!success) {
>> +               if (device->browse) {
>> +                       struct browse_req *req = device->browse;
>> +                       send_le_browse_response(req);
>> +                       device->browse = NULL;
>> +                       browse_request_free(req);
>> +               }
>>
>> -       if (services == NULL) {
>> -               DBG("No services found");
>> -               register_all_services(req, NULL);
>>                 return;
>>         }
>>
>> -       search = g_new0(struct included_search, 1);
>> -       search->req = req;
>> +       device->att_mtu = bt_att_get_mtu(device->att);
>>
>> -       /* We have to completely duplicate the data in order to have a
>> -        * clearly defined responsibility of freeing regardless of
>> -        * failure or success. Otherwise memory leaks are inevitable.
>> -        */
>> -       for (l = services; l; l = g_slist_next(l)) {
>> -               struct gatt_primary *dup;
>> +       DBG("gatt-client exchanged MTU: %u", device->att_mtu);
>>
>> -               dup = g_memdup(l->data, sizeof(struct gatt_primary));
>> +       if (device->browse)
>> +               register_gatt_services(device->browse);
>>
>> -               search->services = g_slist_append(search->services, dup);
>> -       }
>> +       /*
>> +        * TODO: Change attio callbacks to accept a gatt-client instead of a
>> +        * GAttrib.
>> +        */
>> +       g_slist_foreach(device->attios, attio_connected, device->attrib);
>> +}
>>
>> -       search->current = search->services;
>> +static void gatt_client_service_changed(uint16_t start_handle,
>> +                                                       uint16_t end_handle,
>> +                                                       void *user_data)
>> +{
>> +       struct btd_device *device = user_data;
>> +
>> +       DBG("gatt-client: Service Changed: start 0x%04x, end: 0x%04x",
>> +                                               start_handle, end_handle);
>>
>> -       prim = search->current->data;
>> -       gatt_find_included(device->attrib, prim->range.start, prim->range.end,
>> -                                       find_included_cb, search);
>> +       /*
>> +        * TODO: Notify clients that services changed so they can handle it
>> +        * directly. Remove the profile if a service was removed.
>> +        */
>> +       device_browse_primary(device, NULL);
>>  }
>>
>> -static void primary_cb(uint8_t status, GSList *services, void *user_data)
>> +static void gatt_client_init(struct btd_device *device)
>>  {
>> -       struct browse_req *req = user_data;
>> +       gatt_client_cleanup(device);
>>
>> -       DBG("status %u", status);
>> +       device->gatt_client = bt_gatt_client_new(device->att, device->att_mtu);
>> +       if (!device->gatt_client) {
>> +               DBG("Failed to initialize gatt-client");
>> +               return;
>> +       }
>>
>> -       if (status) {
>> -               struct btd_device *device = req->device;
>> +       if (!bt_gatt_client_set_ready_handler(device->gatt_client,
>> +                                                       gatt_client_ready_cb,
>> +                                                       device, NULL)) {
>> +               DBG("Failed to set ready handler for gatt-client");
>> +               gatt_client_cleanup(device);
>> +               return;
>> +       }
>>
>> -               send_le_browse_response(req);
>> -               device->browse = NULL;
>> -               browse_request_free(req);
>> +       if (!bt_gatt_client_set_service_changed(device->gatt_client,
>> +                                               gatt_client_service_changed,
>> +                                               device, NULL)) {
>> +               DBG("Failed to set service changed handler for gatt-client");
>> +               gatt_client_cleanup(device);
>>                 return;
>>         }
>>
>> -       find_included_services(req, services);
>> +       DBG("gatt-client created");
>>  }
>>
>>  bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
>> @@ -3660,10 +3676,8 @@ bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
>>                 }
>>         }
>>
>> -       if (cid == ATT_CID)
>> -               mtu = BT_ATT_DEFAULT_LE_MTU;
>> -
>> -       attrib = g_attrib_new(io, mtu);
>> +       dev->att_mtu = MIN(mtu, BT_ATT_MAX_LE_MTU);
>> +       attrib = g_attrib_new(io, dev->att_mtu);
>>         if (!attrib) {
>>                 error("Unable to create new GAttrib instance");
>>                 return false;
>> @@ -3678,11 +3692,12 @@ bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
>>
>>         dev->attrib = attrib;
>>         dev->att = bt_att_ref(g_attrib_get_att(attrib));
>> -
>>         dev->att_disconn_id = bt_att_register_disconnect(dev->att,
>>                                                 att_disconnected_cb, dev, NULL);
>>         bt_att_set_close_on_unref(dev->att, true);
>>
>> +       gatt_client_init(dev);
>> +
>>         /*
>>          * Remove the device from the connect_list and give the passive
>>          * scanning another chance to be restarted in case there are
>> @@ -3690,8 +3705,6 @@ bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
>>          */
>>         adapter_connect_list_remove(dev->adapter, dev);
>>
>> -       g_slist_foreach(dev->attios, attio_connected, dev->attrib);
>> -
>>         return true;
>>  }
>>
>> @@ -3845,13 +3858,29 @@ static void att_browse_error_cb(const GError *gerr, gpointer user_data)
>>         browse_request_free(req);
>>  }
>>
>> +static void browse_gatt_client(struct browse_req *req)
>> +{
>> +       struct btd_device *device = req->device;
>> +
>> +       if (!device->gatt_client) {
>> +               DBG("No gatt-client currently attached");
>> +               return;
>> +       }
>> +
>> +       /*
>> +        * If gatt-client is ready, then register all services. Otherwise, this
>> +        * will be deferred until client becomes ready.
>> +        */
>> +       if (bt_gatt_client_is_ready(device->gatt_client))
>> +               register_gatt_services(req);
>> +}
>> +
>>  static void att_browse_cb(gpointer user_data)
>>  {
>>         struct att_callbacks *attcb = user_data;
>>         struct btd_device *device = attcb->user_data;
>>
>> -       gatt_discover_primary(device->attrib, NULL, primary_cb,
>> -                                                       device->browse);
>> +       browse_gatt_client(device->browse);
>>  }
>>
>>  static int device_browse_primary(struct btd_device *device, DBusMessage *msg)
>> @@ -3869,7 +3898,7 @@ static int device_browse_primary(struct btd_device *device, DBusMessage *msg)
>>         device->browse = req;
>>
>>         if (device->attrib) {
>> -               gatt_discover_primary(device->attrib, NULL, primary_cb, req);
>> +               browse_gatt_client(device->browse);
>>                 goto done;
>>         }
>>
>> @@ -4738,21 +4767,6 @@ GSList *btd_device_get_primaries(struct btd_device *device)
>>         return device->primaries;
>>  }
>>
>> -void btd_device_gatt_set_service_changed(struct btd_device *device,
>> -                                               uint16_t start, uint16_t end)
>> -{
>> -       GSList *l;
>> -
>> -       for (l = device->primaries; l; l = g_slist_next(l)) {
>> -               struct gatt_primary *prim = l->data;
>> -
>> -               if (start <= prim->range.end && end >= prim->range.start)
>> -                       prim->changed = TRUE;
>> -       }
>> -
>> -       device_browse_primary(device, NULL);
>> -}
>> -
>>  void btd_device_add_uuid(struct btd_device *device, const char *uuid)
>>  {
>>         GSList *uuid_list;
>> diff --git a/src/device.h b/src/device.h
>> index 2e0473e..00cb502 100644
>> --- a/src/device.h
>> +++ b/src/device.h
>> @@ -67,8 +67,6 @@ const sdp_record_t *btd_device_get_record(struct btd_device *device,
>>  struct gatt_primary *btd_device_get_primary(struct btd_device *device,
>>                                                         const char *uuid);
>>  GSList *btd_device_get_primaries(struct btd_device *device);
>> -void btd_device_gatt_set_service_changed(struct btd_device *device,
>> -                                               uint16_t start, uint16_t end);
>>  bool device_attach_attrib(struct btd_device *dev, GIOChannel *io);
>>  void btd_device_add_uuid(struct btd_device *device, const char *uuid);
>>  void device_add_eir_uuids(struct btd_device *dev, GSList *uuids);
>> diff --git a/src/shared/att-types.h b/src/shared/att-types.h
>> index 3d8829a..8b6d537 100644
>> --- a/src/shared/att-types.h
>> +++ b/src/shared/att-types.h
>> @@ -27,7 +27,8 @@
>>  #define __packed __attribute__((packed))
>>  #endif
>>
>> -#define BT_ATT_DEFAULT_LE_MTU 23
>> +#define BT_ATT_DEFAULT_LE_MTU  23
>> +#define BT_ATT_MAX_LE_MTU      517
>>
>>  /* ATT protocol opcodes */
>>  #define BT_ATT_OP_ERROR_RSP                    0x01
>> --
>> 2.1.0.rc2.206.gedb03e5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Luiz Augusto von Dentz

Thanks,
Arman

Re: [PATCH BlueZ 06/12] core: device: Use shared/gatt-client for GATT.

[Luiz Augusto von Dentz]

Hi Arman,

On Wed, Nov 19, 2014 at 6:42 PM, Arman Uguray <armansito@chromium.org> wrote:
> Hi Luiz,
>
>> On Wed, Nov 19, 2014 at 7:43 AM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>> Hi Arman,
>>
>> On Mon, Nov 17, 2014 at 9:22 PM, Arman Uguray <armansito@chromium.org> wrote:
>>> This patch changes the GATT service discovery code path with the
>>> following:
>>>    - As part for device_attach_attrib, a bt_gatt_client structure is
>>>    created that performs MTU exchange and service discovery, caches the
>>>    services in an internal list, and handles the remote GATT service.
>>>
>>>    - The device_browse_primary code path obtains service information
>>>    by iterating through bt_gatt_client's services, instead of directly
>>>    initiating primary and secondary service discovery using attrib/gatt.
>>>    If the bt_gatt_client isn't ready at the time, the code defers
>>>    registration of services and profile probing until the ready handler
>>>    is called.
>>>
>>>    - The attio connected callbacks are invoked from bt_gatt_client's
>>>    ready handler instead of device_attach_attrib.
>>> ---
>>
>> I would suggest that before we start changing the core we make sure we
>> have completed the TODO items for bt_gatt_client/bt_gatt_server, there
>> are items such as gatt-client + gatt-server coexistence that haven't
>> been tested  and I also remember that we had some ideas about a new
>> object that would take care of both client and server under it, the
>> caching probably needs to be addressed as well otherwise
>> bt_gatt_client will always end up discovering the service in every
>> connection.
>>
>
> I think we have to start somewhere. I think at this point we have the
> basic feature set implemented to start converting the core and at
> least have behavior that's equivalent (or better) to what the older
> code did. I didn't want to immediately involve the server until seeing
> how the client fits in and we still have to figure out how not to
> break the existing server role profiles (the experimental ones) while
> integrating bt_gatt_server (or maybe we don't care since those are all
> experimental and we can perhaps just throw out src/attrib-server).
> Either way I wanted to tackle this in several steps and move
> iteratively.

It is not exactly the steps that worries me, but I don't wont to rush
and cause regressions, for example we should not treat client and
server separately since it is valid to have different profiles using
different roles.

> So, I guess the one big item is the missing caching support, which I
> will now get started on, but in the meantime I think we should start
> converting the core, see if there are any bugs and edge cases we have
> to handle, etc. I want to see how well the plugins work with the new
> code and more importantly I want to have the GATT D-Bus API
> implemented and merged into the tree (at least as experimental) and
> get people to start using it and send feedback.

Coexistence of server and client is another one, perhaps we can try to
emulate something in the unit tests.

> In the meantime I'm going to start working on two API functions like
> bt_gatt_client_new_from_storage & bt_gatt_client_store_attributes so
> you can initialize the cache from the contents of a file and commit
> them to a file. That should make things easy to integrate into the
> core.

Yep, something like that will need to be in place and also perhaps
bt_gatt_*attach/bt_gatt_*detach otherwise we can't have the same
connection handled by both client and server at same, this btw could
be solved with a higher level object that takes care of both and
perhaps we can then have bt_gatt_get_client/bt_gatt_get_server.

>>
>>>  src/device.c           | 286 ++++++++++++++++++++++++++-----------------------
>>>  src/device.h           |   2 -
>>>  src/shared/att-types.h |   3 +-
>>>  3 files changed, 152 insertions(+), 139 deletions(-)
>>>
>>> diff --git a/src/device.c b/src/device.c
>>> index 45c7e9c..3d0159e 100644
>>> --- a/src/device.c
>>> +++ b/src/device.c
>>> @@ -74,6 +74,10 @@
>>>  #define DISCONNECT_TIMER       2
>>>  #define DISCOVERY_TIMER                1
>>>
>>> +#ifndef MIN
>>> +#define MIN(a, b) ((a) < (b) ? (a) : (b))
>>> +#endif
>>> +
>>>  static DBusConnection *dbus_conn = NULL;
>>>  unsigned service_state_cb_id;
>>>
>>> @@ -205,9 +209,12 @@ struct btd_device {
>>>         GSList          *attios_offline;
>>>         guint           attachid;               /* Attrib server attach */
>>>
>>> -       struct bt_att *att;     /* The new ATT transport */
>>> +       struct bt_att *att;                     /* The new ATT transport */
>>> +       uint16_t att_mtu;                       /* The ATT MTU */
>>>         unsigned int att_disconn_id;
>>>
>>> +       struct bt_gatt_client *gatt_client;     /* GATT client cache */
>>> +
>>>         struct bearer_state bredr_state;
>>>         struct bearer_state le_state;
>>>
>>> @@ -463,6 +470,18 @@ static void browse_request_free(struct browse_req *req)
>>>         g_free(req);
>>>  }
>>>
>>> +static void gatt_client_cleanup(struct btd_device *device)
>>> +{
>>> +       if (!device->gatt_client)
>>> +               return;
>>> +
>>> +       bt_gatt_client_set_service_changed(device->gatt_client, NULL, NULL,
>>> +                                                                       NULL);
>>> +       bt_gatt_client_set_ready_handler(device->gatt_client, NULL, NULL, NULL);
>>> +       bt_gatt_client_unref(device->gatt_client);
>>> +       device->gatt_client = NULL;
>>> +}
>>> +
>>>  static void attio_cleanup(struct btd_device *device)
>>>  {
>>>         if (device->attachid) {
>>> @@ -480,6 +499,8 @@ static void attio_cleanup(struct btd_device *device)
>>>                 device->att_io = NULL;
>>>         }
>>>
>>> +       gatt_client_cleanup(device);
>>> +
>>>         if (device->att) {
>>>                 bt_att_unref(device->att);
>>>                 device->att = NULL;
>>> @@ -3445,41 +3466,6 @@ done:
>>>         attio_cleanup(device);
>>>  }
>>>
>>> -static void register_all_services(struct browse_req *req, GSList *services)
>>> -{
>>> -       struct btd_device *device = req->device;
>>> -
>>> -       btd_device_set_temporary(device, FALSE);
>>> -
>>> -       update_gatt_services(req, device->primaries, services);
>>> -       g_slist_free_full(device->primaries, g_free);
>>> -       device->primaries = NULL;
>>> -
>>> -       device_register_primaries(device, services, -1);
>>> -
>>> -       device_probe_profiles(device, req->profiles_added);
>>> -
>>> -       if (device->attios == NULL && device->attios_offline == NULL)
>>> -               attio_cleanup(device);
>>> -
>>> -       g_dbus_emit_property_changed(dbus_conn, device->path,
>>> -                                               DEVICE_INTERFACE, "UUIDs");
>>> -
>>> -       device_svc_resolved(device, device->bdaddr_type, 0);
>>> -
>>> -       store_services(device);
>>> -
>>> -       browse_request_free(req);
>>> -}
>>> -
>>> -static int service_by_range_cmp(gconstpointer a, gconstpointer b)
>>> -{
>>> -       const struct gatt_primary *prim = a;
>>> -       const struct att_range *range = b;
>>> -
>>> -       return memcmp(&prim->range, range, sizeof(*range));
>>> -}
>>> -
>>>  static void send_le_browse_response(struct browse_req *req)
>>>  {
>>>         struct btd_device *dev = req->device;
>>> @@ -3510,122 +3496,152 @@ static void send_le_browse_response(struct browse_req *req)
>>>         g_dbus_send_reply(dbus_conn, msg, DBUS_TYPE_INVALID);
>>>  }
>>>
>>> -static void find_included_cb(uint8_t status, GSList *includes, void *user_data)
>>> +/*
>>> + * TODO: Remove this function once btd_device stops storing gatt_primary
>>> + * structures for discovered services.
>>> + */
>>> +static bool populate_primaries(struct btd_device *device, GSList **services)
>>>  {
>>> -       struct included_search *search = user_data;
>>> -       struct btd_device *device = search->req->device;
>>> +       struct bt_gatt_service_iter iter;
>>> +       const bt_gatt_service_t *service;
>>>         struct gatt_primary *prim;
>>> -       GSList *l;
>>> +       uint128_t u128;
>>> +       bt_uuid_t uuid;
>>>
>>> -       DBG("status %u", status);
>>> +       /* Create gatt_primary structures for all primary/secondary services */
>>> +       if (!bt_gatt_service_iter_init(&iter, device->gatt_client))
>>> +               return false;
>>>
>>> -       if (device->attrib == NULL || status) {
>>> -               struct browse_req *req = device->browse;
>>> +       while (bt_gatt_service_iter_next(&iter, &service)) {
>>> +               prim = g_new0(struct gatt_primary, 1);
>>> +               prim->range.start = service->start_handle;
>>> +               prim->range.end = service->end_handle;
>>>
>>> -               if (status)
>>> -                       error("Find included services failed: %s (%d)",
>>> -                                       att_ecode2str(status), status);
>>> -               else
>>> -                       error("Disconnected while doing included discovery");
>>> +               memcpy(u128.data, service->uuid, 16);
>>> +               bt_uuid128_create(&uuid, u128);
>>> +               bt_uuid_to_string(&uuid, prim->uuid, MAX_LEN_UUID_STR + 1);
>>>
>>> -               if (!req)
>>> -                       goto complete;
>>> +               *services = g_slist_append(*services, prim);
>>> +       }
>>>
>>> +       return true;
>>> +}
>>> +
>>> +static void register_gatt_services(struct browse_req *req)
>>> +{
>>> +       struct btd_device *device = req->device;
>>> +       GSList *services = NULL;
>>> +
>>> +       if (!bt_gatt_client_is_ready(device->gatt_client))
>>> +               return;
>>> +
>>> +       if (!populate_primaries(device, &services)) {
>>>                 send_le_browse_response(req);
>>>                 device->browse = NULL;
>>>                 browse_request_free(req);
>>> -
>>> -               goto complete;
>>> +               return;
>>>         }
>>>
>>> -       if (includes == NULL)
>>> -               goto next;
>>> +       btd_device_set_temporary(device, FALSE);
>>>
>>> -       for (l = includes; l; l = l->next) {
>>> -               struct gatt_included *incl = l->data;
>>> +       update_gatt_services(req, device->primaries, services);
>>> +       g_slist_free_full(device->primaries, g_free);
>>> +       device->primaries = NULL;
>>>
>>> -               if (g_slist_find_custom(search->services, &incl->range,
>>> -                                               service_by_range_cmp))
>>> -                       continue;
>>> +       device_register_primaries(device, services, -1);
>>>
>>> -               prim = g_new0(struct gatt_primary, 1);
>>> -               memcpy(prim->uuid, incl->uuid, sizeof(prim->uuid));
>>> -               memcpy(&prim->range, &incl->range, sizeof(prim->range));
>>> +       device_probe_profiles(device, req->profiles_added);
>>>
>>> -               search->services = g_slist_append(search->services, prim);
>>> -       }
>>> +       if (device->attios == NULL && device->attios_offline == NULL)
>>> +               attio_cleanup(device);
>>>
>>> -next:
>>> -       search->current = search->current->next;
>>> -       if (search->current == NULL) {
>>> -               register_all_services(search->req, search->services);
>>> -               search->services = NULL;
>>> -               goto complete;
>>> -       }
>>> +       g_dbus_emit_property_changed(dbus_conn, device->path,
>>> +                                               DEVICE_INTERFACE, "UUIDs");
>>>
>>> -       prim = search->current->data;
>>> -       gatt_find_included(device->attrib, prim->range.start, prim->range.end,
>>> -                                       find_included_cb, search);
>>> -       return;
>>> +       device_svc_resolved(device, device->bdaddr_type, 0);
>>>
>>> -complete:
>>> -       g_slist_free_full(search->services, g_free);
>>> -       g_free(search);
>>> +       store_services(device);
>>> +
>>> +       browse_request_free(req);
>>>  }
>>>
>>> -static void find_included_services(struct browse_req *req, GSList *services)
>>> +static void gatt_client_ready_cb(bool success, uint8_t att_ecode,
>>> +                                                               void *user_data)
>>>  {
>>> -       struct btd_device *device = req->device;
>>> -       struct included_search *search;
>>> -       struct gatt_primary *prim;
>>> -       GSList *l;
>>> +       struct btd_device *device = user_data;
>>>
>>> -       DBG("service count %u", g_slist_length(services));
>>> +       DBG("gatt-client ready -- status: %s, ATT error: %u",
>>> +                                               success ? "success" : "failed",
>>> +                                               att_ecode);
>>> +
>>> +       if (!success) {
>>> +               if (device->browse) {
>>> +                       struct browse_req *req = device->browse;
>>> +                       send_le_browse_response(req);
>>> +                       device->browse = NULL;
>>> +                       browse_request_free(req);
>>> +               }
>>>
>>> -       if (services == NULL) {
>>> -               DBG("No services found");
>>> -               register_all_services(req, NULL);
>>>                 return;
>>>         }
>>>
>>> -       search = g_new0(struct included_search, 1);
>>> -       search->req = req;
>>> +       device->att_mtu = bt_att_get_mtu(device->att);
>>>
>>> -       /* We have to completely duplicate the data in order to have a
>>> -        * clearly defined responsibility of freeing regardless of
>>> -        * failure or success. Otherwise memory leaks are inevitable.
>>> -        */
>>> -       for (l = services; l; l = g_slist_next(l)) {
>>> -               struct gatt_primary *dup;
>>> +       DBG("gatt-client exchanged MTU: %u", device->att_mtu);
>>>
>>> -               dup = g_memdup(l->data, sizeof(struct gatt_primary));
>>> +       if (device->browse)
>>> +               register_gatt_services(device->browse);
>>>
>>> -               search->services = g_slist_append(search->services, dup);
>>> -       }
>>> +       /*
>>> +        * TODO: Change attio callbacks to accept a gatt-client instead of a
>>> +        * GAttrib.
>>> +        */
>>> +       g_slist_foreach(device->attios, attio_connected, device->attrib);
>>> +}
>>>
>>> -       search->current = search->services;
>>> +static void gatt_client_service_changed(uint16_t start_handle,
>>> +                                                       uint16_t end_handle,
>>> +                                                       void *user_data)
>>> +{
>>> +       struct btd_device *device = user_data;
>>> +
>>> +       DBG("gatt-client: Service Changed: start 0x%04x, end: 0x%04x",
>>> +                                               start_handle, end_handle);
>>>
>>> -       prim = search->current->data;
>>> -       gatt_find_included(device->attrib, prim->range.start, prim->range.end,
>>> -                                       find_included_cb, search);
>>> +       /*
>>> +        * TODO: Notify clients that services changed so they can handle it
>>> +        * directly. Remove the profile if a service was removed.
>>> +        */
>>> +       device_browse_primary(device, NULL);
>>>  }
>>>
>>> -static void primary_cb(uint8_t status, GSList *services, void *user_data)
>>> +static void gatt_client_init(struct btd_device *device)
>>>  {
>>> -       struct browse_req *req = user_data;
>>> +       gatt_client_cleanup(device);
>>>
>>> -       DBG("status %u", status);
>>> +       device->gatt_client = bt_gatt_client_new(device->att, device->att_mtu);
>>> +       if (!device->gatt_client) {
>>> +               DBG("Failed to initialize gatt-client");
>>> +               return;
>>> +       }
>>>
>>> -       if (status) {
>>> -               struct btd_device *device = req->device;
>>> +       if (!bt_gatt_client_set_ready_handler(device->gatt_client,
>>> +                                                       gatt_client_ready_cb,
>>> +                                                       device, NULL)) {
>>> +               DBG("Failed to set ready handler for gatt-client");
>>> +               gatt_client_cleanup(device);
>>> +               return;
>>> +       }
>>>
>>> -               send_le_browse_response(req);
>>> -               device->browse = NULL;
>>> -               browse_request_free(req);
>>> +       if (!bt_gatt_client_set_service_changed(device->gatt_client,
>>> +                                               gatt_client_service_changed,
>>> +                                               device, NULL)) {
>>> +               DBG("Failed to set service changed handler for gatt-client");
>>> +               gatt_client_cleanup(device);
>>>                 return;
>>>         }
>>>
>>> -       find_included_services(req, services);
>>> +       DBG("gatt-client created");
>>>  }
>>>
>>>  bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
>>> @@ -3660,10 +3676,8 @@ bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
>>>                 }
>>>         }
>>>
>>> -       if (cid == ATT_CID)
>>> -               mtu = BT_ATT_DEFAULT_LE_MTU;
>>> -
>>> -       attrib = g_attrib_new(io, mtu);
>>> +       dev->att_mtu = MIN(mtu, BT_ATT_MAX_LE_MTU);
>>> +       attrib = g_attrib_new(io, dev->att_mtu);
>>>         if (!attrib) {
>>>                 error("Unable to create new GAttrib instance");
>>>                 return false;
>>> @@ -3678,11 +3692,12 @@ bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
>>>
>>>         dev->attrib = attrib;
>>>         dev->att = bt_att_ref(g_attrib_get_att(attrib));
>>> -
>>>         dev->att_disconn_id = bt_att_register_disconnect(dev->att,
>>>                                                 att_disconnected_cb, dev, NULL);
>>>         bt_att_set_close_on_unref(dev->att, true);
>>>
>>> +       gatt_client_init(dev);
>>> +
>>>         /*
>>>          * Remove the device from the connect_list and give the passive
>>>          * scanning another chance to be restarted in case there are
>>> @@ -3690,8 +3705,6 @@ bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
>>>          */
>>>         adapter_connect_list_remove(dev->adapter, dev);
>>>
>>> -       g_slist_foreach(dev->attios, attio_connected, dev->attrib);
>>> -
>>>         return true;
>>>  }
>>>
>>> @@ -3845,13 +3858,29 @@ static void att_browse_error_cb(const GError *gerr, gpointer user_data)
>>>         browse_request_free(req);
>>>  }
>>>
>>> +static void browse_gatt_client(struct browse_req *req)
>>> +{
>>> +       struct btd_device *device = req->device;
>>> +
>>> +       if (!device->gatt_client) {
>>> +               DBG("No gatt-client currently attached");
>>> +               return;
>>> +       }
>>> +
>>> +       /*
>>> +        * If gatt-client is ready, then register all services. Otherwise, this
>>> +        * will be deferred until client becomes ready.
>>> +        */
>>> +       if (bt_gatt_client_is_ready(device->gatt_client))
>>> +               register_gatt_services(req);
>>> +}
>>> +
>>>  static void att_browse_cb(gpointer user_data)
>>>  {
>>>         struct att_callbacks *attcb = user_data;
>>>         struct btd_device *device = attcb->user_data;
>>>
>>> -       gatt_discover_primary(device->attrib, NULL, primary_cb,
>>> -                                                       device->browse);
>>> +       browse_gatt_client(device->browse);
>>>  }
>>>
>>>  static int device_browse_primary(struct btd_device *device, DBusMessage *msg)
>>> @@ -3869,7 +3898,7 @@ static int device_browse_primary(struct btd_device *device, DBusMessage *msg)
>>>         device->browse = req;
>>>
>>>         if (device->attrib) {
>>> -               gatt_discover_primary(device->attrib, NULL, primary_cb, req);
>>> +               browse_gatt_client(device->browse);
>>>                 goto done;
>>>         }
>>>
>>> @@ -4738,21 +4767,6 @@ GSList *btd_device_get_primaries(struct btd_device *device)
>>>         return device->primaries;
>>>  }
>>>
>>> -void btd_device_gatt_set_service_changed(struct btd_device *device,
>>> -                                               uint16_t start, uint16_t end)
>>> -{
>>> -       GSList *l;
>>> -
>>> -       for (l = device->primaries; l; l = g_slist_next(l)) {
>>> -               struct gatt_primary *prim = l->data;
>>> -
>>> -               if (start <= prim->range.end && end >= prim->range.start)
>>> -                       prim->changed = TRUE;
>>> -       }
>>> -
>>> -       device_browse_primary(device, NULL);
>>> -}
>>> -
>>>  void btd_device_add_uuid(struct btd_device *device, const char *uuid)
>>>  {
>>>         GSList *uuid_list;
>>> diff --git a/src/device.h b/src/device.h
>>> index 2e0473e..00cb502 100644
>>> --- a/src/device.h
>>> +++ b/src/device.h
>>> @@ -67,8 +67,6 @@ const sdp_record_t *btd_device_get_record(struct btd_device *device,
>>>  struct gatt_primary *btd_device_get_primary(struct btd_device *device,
>>>                                                         const char *uuid);
>>>  GSList *btd_device_get_primaries(struct btd_device *device);
>>> -void btd_device_gatt_set_service_changed(struct btd_device *device,
>>> -                                               uint16_t start, uint16_t end);
>>>  bool device_attach_attrib(struct btd_device *dev, GIOChannel *io);
>>>  void btd_device_add_uuid(struct btd_device *device, const char *uuid);
>>>  void device_add_eir_uuids(struct btd_device *dev, GSList *uuids);
>>> diff --git a/src/shared/att-types.h b/src/shared/att-types.h
>>> index 3d8829a..8b6d537 100644
>>> --- a/src/shared/att-types.h
>>> +++ b/src/shared/att-types.h
>>> @@ -27,7 +27,8 @@
>>>  #define __packed __attribute__((packed))
>>>  #endif
>>>
>>> -#define BT_ATT_DEFAULT_LE_MTU 23
>>> +#define BT_ATT_DEFAULT_LE_MTU  23
>>> +#define BT_ATT_MAX_LE_MTU      517
>>>
>>>  /* ATT protocol opcodes */
>>>  #define BT_ATT_OP_ERROR_RSP                    0x01
>>> --
>>> 2.1.0.rc2.206.gedb03e5
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
>>
>> --
>> Luiz Augusto von Dentz
>
> Thanks,
> Arman



-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ 06/12] core: device: Use shared/gatt-client for GATT.

[Arman Uguray]

Hi Luiz,

> On Wed, Nov 19, 2014 at 9:09 AM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
> Hi Arman,
>
> On Wed, Nov 19, 2014 at 6:42 PM, Arman Uguray <armansito@chromium.org> wrote:
>> Hi Luiz,
>>
>>> On Wed, Nov 19, 2014 at 7:43 AM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>>> Hi Arman,
>>>
>>> On Mon, Nov 17, 2014 at 9:22 PM, Arman Uguray <armansito@chromium.org> wrote:
>>>> This patch changes the GATT service discovery code path with the
>>>> following:
>>>>    - As part for device_attach_attrib, a bt_gatt_client structure is
>>>>    created that performs MTU exchange and service discovery, caches the
>>>>    services in an internal list, and handles the remote GATT service.
>>>>
>>>>    - The device_browse_primary code path obtains service information
>>>>    by iterating through bt_gatt_client's services, instead of directly
>>>>    initiating primary and secondary service discovery using attrib/gatt.
>>>>    If the bt_gatt_client isn't ready at the time, the code defers
>>>>    registration of services and profile probing until the ready handler
>>>>    is called.
>>>>
>>>>    - The attio connected callbacks are invoked from bt_gatt_client's
>>>>    ready handler instead of device_attach_attrib.
>>>> ---
>>>
>>> I would suggest that before we start changing the core we make sure we
>>> have completed the TODO items for bt_gatt_client/bt_gatt_server, there
>>> are items such as gatt-client + gatt-server coexistence that haven't
>>> been tested  and I also remember that we had some ideas about a new
>>> object that would take care of both client and server under it, the
>>> caching probably needs to be addressed as well otherwise
>>> bt_gatt_client will always end up discovering the service in every
>>> connection.
>>>
>>
>> I think we have to start somewhere. I think at this point we have the
>> basic feature set implemented to start converting the core and at
>> least have behavior that's equivalent (or better) to what the older
>> code did. I didn't want to immediately involve the server until seeing
>> how the client fits in and we still have to figure out how not to
>> break the existing server role profiles (the experimental ones) while
>> integrating bt_gatt_server (or maybe we don't care since those are all
>> experimental and we can perhaps just throw out src/attrib-server).
>> Either way I wanted to tackle this in several steps and move
>> iteratively.
>
> It is not exactly the steps that worries me, but I don't wont to rush
> and cause regressions, for example we should not treat client and
> server separately since it is valid to have different profiles using
> different roles.
>

At least from what I've seen, the profiles using different roles (such
as proximity) handle the client and server operations somewhat
orthogonally, so that's why I wasn't worried. I prefer not rushing
anything too but I would still like to handle this in steps, so we'll
probably have some profile code during the transition process that
does client stuff through shared/gatt-client and server role stuff
through src/attrib-server. That's why we went through the trouble of
turning GAttrib into a wrapper around bt_att so that we can do this in
small steps :)

>> So, I guess the one big item is the missing caching support, which I
>> will now get started on, but in the meantime I think we should start
>> converting the core, see if there are any bugs and edge cases we have
>> to handle, etc. I want to see how well the plugins work with the new
>> code and more importantly I want to have the GATT D-Bus API
>> implemented and merged into the tree (at least as experimental) and
>> get people to start using it and send feedback.
>
> Coexistence of server and client is another one, perhaps we can try to
> emulate something in the unit tests.
>

A unit test definitely makes sense. Would we want a standalone
unit/test-gatt-coex or would we want to work this into unit/test-gatt?
We need to start writing server role tests as well, so let's decide
where both of those should go.

In addition we could add this as a feature to tools/btgatt-server and
tools/btgatt-client as well. I kind of like that they are separate but
perhaps we'll want to merge these tools in the future.

>> In the meantime I'm going to start working on two API functions like
>> bt_gatt_client_new_from_storage & bt_gatt_client_store_attributes so
>> you can initialize the cache from the contents of a file and commit
>> them to a file. That should make things easy to integrate into the
>> core.
>
> Yep, something like that will need to be in place and also perhaps
> bt_gatt_*attach/bt_gatt_*detach otherwise we can't have the same
> connection handled by both client and server at same, this btw could
> be solved with a higher level object that takes care of both and
> perhaps we can then have bt_gatt_get_client/bt_gatt_get_server.
>

Sorry, I'm not fully understanding the attach/detach bit. Currently
you could do bt_gatt_server_new(att, db) and that "attaches" the
server to the bt_att and when you're done you unref the bt_gatt_server
(same for bt_gatt_client). So you could create both on the same bt_att
now and that should work, they should be able to coexist just fine.

>>>
>>>>  src/device.c           | 286 ++++++++++++++++++++++++++-----------------------
>>>>  src/device.h           |   2 -
>>>>  src/shared/att-types.h |   3 +-
>>>>  3 files changed, 152 insertions(+), 139 deletions(-)
>>>>
>>>> diff --git a/src/device.c b/src/device.c
>>>> index 45c7e9c..3d0159e 100644
>>>> --- a/src/device.c
>>>> +++ b/src/device.c
>>>> @@ -74,6 +74,10 @@
>>>>  #define DISCONNECT_TIMER       2
>>>>  #define DISCOVERY_TIMER                1
>>>>
>>>> +#ifndef MIN
>>>> +#define MIN(a, b) ((a) < (b) ? (a) : (b))
>>>> +#endif
>>>> +
>>>>  static DBusConnection *dbus_conn = NULL;
>>>>  unsigned service_state_cb_id;
>>>>
>>>> @@ -205,9 +209,12 @@ struct btd_device {
>>>>         GSList          *attios_offline;
>>>>         guint           attachid;               /* Attrib server attach */
>>>>
>>>> -       struct bt_att *att;     /* The new ATT transport */
>>>> +       struct bt_att *att;                     /* The new ATT transport */
>>>> +       uint16_t att_mtu;                       /* The ATT MTU */
>>>>         unsigned int att_disconn_id;
>>>>
>>>> +       struct bt_gatt_client *gatt_client;     /* GATT client cache */
>>>> +
>>>>         struct bearer_state bredr_state;
>>>>         struct bearer_state le_state;
>>>>
>>>> @@ -463,6 +470,18 @@ static void browse_request_free(struct browse_req *req)
>>>>         g_free(req);
>>>>  }
>>>>
>>>> +static void gatt_client_cleanup(struct btd_device *device)
>>>> +{
>>>> +       if (!device->gatt_client)
>>>> +               return;
>>>> +
>>>> +       bt_gatt_client_set_service_changed(device->gatt_client, NULL, NULL,
>>>> +                                                                       NULL);
>>>> +       bt_gatt_client_set_ready_handler(device->gatt_client, NULL, NULL, NULL);
>>>> +       bt_gatt_client_unref(device->gatt_client);
>>>> +       device->gatt_client = NULL;
>>>> +}
>>>> +
>>>>  static void attio_cleanup(struct btd_device *device)
>>>>  {
>>>>         if (device->attachid) {
>>>> @@ -480,6 +499,8 @@ static void attio_cleanup(struct btd_device *device)
>>>>                 device->att_io = NULL;
>>>>         }
>>>>
>>>> +       gatt_client_cleanup(device);
>>>> +
>>>>         if (device->att) {
>>>>                 bt_att_unref(device->att);
>>>>                 device->att = NULL;
>>>> @@ -3445,41 +3466,6 @@ done:
>>>>         attio_cleanup(device);
>>>>  }
>>>>
>>>> -static void register_all_services(struct browse_req *req, GSList *services)
>>>> -{
>>>> -       struct btd_device *device = req->device;
>>>> -
>>>> -       btd_device_set_temporary(device, FALSE);
>>>> -
>>>> -       update_gatt_services(req, device->primaries, services);
>>>> -       g_slist_free_full(device->primaries, g_free);
>>>> -       device->primaries = NULL;
>>>> -
>>>> -       device_register_primaries(device, services, -1);
>>>> -
>>>> -       device_probe_profiles(device, req->profiles_added);
>>>> -
>>>> -       if (device->attios == NULL && device->attios_offline == NULL)
>>>> -               attio_cleanup(device);
>>>> -
>>>> -       g_dbus_emit_property_changed(dbus_conn, device->path,
>>>> -                                               DEVICE_INTERFACE, "UUIDs");
>>>> -
>>>> -       device_svc_resolved(device, device->bdaddr_type, 0);
>>>> -
>>>> -       store_services(device);
>>>> -
>>>> -       browse_request_free(req);
>>>> -}
>>>> -
>>>> -static int service_by_range_cmp(gconstpointer a, gconstpointer b)
>>>> -{
>>>> -       const struct gatt_primary *prim = a;
>>>> -       const struct att_range *range = b;
>>>> -
>>>> -       return memcmp(&prim->range, range, sizeof(*range));
>>>> -}
>>>> -
>>>>  static void send_le_browse_response(struct browse_req *req)
>>>>  {
>>>>         struct btd_device *dev = req->device;
>>>> @@ -3510,122 +3496,152 @@ static void send_le_browse_response(struct browse_req *req)
>>>>         g_dbus_send_reply(dbus_conn, msg, DBUS_TYPE_INVALID);
>>>>  }
>>>>
>>>> -static void find_included_cb(uint8_t status, GSList *includes, void *user_data)
>>>> +/*
>>>> + * TODO: Remove this function once btd_device stops storing gatt_primary
>>>> + * structures for discovered services.
>>>> + */
>>>> +static bool populate_primaries(struct btd_device *device, GSList **services)
>>>>  {
>>>> -       struct included_search *search = user_data;
>>>> -       struct btd_device *device = search->req->device;
>>>> +       struct bt_gatt_service_iter iter;
>>>> +       const bt_gatt_service_t *service;
>>>>         struct gatt_primary *prim;
>>>> -       GSList *l;
>>>> +       uint128_t u128;
>>>> +       bt_uuid_t uuid;
>>>>
>>>> -       DBG("status %u", status);
>>>> +       /* Create gatt_primary structures for all primary/secondary services */
>>>> +       if (!bt_gatt_service_iter_init(&iter, device->gatt_client))
>>>> +               return false;
>>>>
>>>> -       if (device->attrib == NULL || status) {
>>>> -               struct browse_req *req = device->browse;
>>>> +       while (bt_gatt_service_iter_next(&iter, &service)) {
>>>> +               prim = g_new0(struct gatt_primary, 1);
>>>> +               prim->range.start = service->start_handle;
>>>> +               prim->range.end = service->end_handle;
>>>>
>>>> -               if (status)
>>>> -                       error("Find included services failed: %s (%d)",
>>>> -                                       att_ecode2str(status), status);
>>>> -               else
>>>> -                       error("Disconnected while doing included discovery");
>>>> +               memcpy(u128.data, service->uuid, 16);
>>>> +               bt_uuid128_create(&uuid, u128);
>>>> +               bt_uuid_to_string(&uuid, prim->uuid, MAX_LEN_UUID_STR + 1);
>>>>
>>>> -               if (!req)
>>>> -                       goto complete;
>>>> +               *services = g_slist_append(*services, prim);
>>>> +       }
>>>>
>>>> +       return true;
>>>> +}
>>>> +
>>>> +static void register_gatt_services(struct browse_req *req)
>>>> +{
>>>> +       struct btd_device *device = req->device;
>>>> +       GSList *services = NULL;
>>>> +
>>>> +       if (!bt_gatt_client_is_ready(device->gatt_client))
>>>> +               return;
>>>> +
>>>> +       if (!populate_primaries(device, &services)) {
>>>>                 send_le_browse_response(req);
>>>>                 device->browse = NULL;
>>>>                 browse_request_free(req);
>>>> -
>>>> -               goto complete;
>>>> +               return;
>>>>         }
>>>>
>>>> -       if (includes == NULL)
>>>> -               goto next;
>>>> +       btd_device_set_temporary(device, FALSE);
>>>>
>>>> -       for (l = includes; l; l = l->next) {
>>>> -               struct gatt_included *incl = l->data;
>>>> +       update_gatt_services(req, device->primaries, services);
>>>> +       g_slist_free_full(device->primaries, g_free);
>>>> +       device->primaries = NULL;
>>>>
>>>> -               if (g_slist_find_custom(search->services, &incl->range,
>>>> -                                               service_by_range_cmp))
>>>> -                       continue;
>>>> +       device_register_primaries(device, services, -1);
>>>>
>>>> -               prim = g_new0(struct gatt_primary, 1);
>>>> -               memcpy(prim->uuid, incl->uuid, sizeof(prim->uuid));
>>>> -               memcpy(&prim->range, &incl->range, sizeof(prim->range));
>>>> +       device_probe_profiles(device, req->profiles_added);
>>>>
>>>> -               search->services = g_slist_append(search->services, prim);
>>>> -       }
>>>> +       if (device->attios == NULL && device->attios_offline == NULL)
>>>> +               attio_cleanup(device);
>>>>
>>>> -next:
>>>> -       search->current = search->current->next;
>>>> -       if (search->current == NULL) {
>>>> -               register_all_services(search->req, search->services);
>>>> -               search->services = NULL;
>>>> -               goto complete;
>>>> -       }
>>>> +       g_dbus_emit_property_changed(dbus_conn, device->path,
>>>> +                                               DEVICE_INTERFACE, "UUIDs");
>>>>
>>>> -       prim = search->current->data;
>>>> -       gatt_find_included(device->attrib, prim->range.start, prim->range.end,
>>>> -                                       find_included_cb, search);
>>>> -       return;
>>>> +       device_svc_resolved(device, device->bdaddr_type, 0);
>>>>
>>>> -complete:
>>>> -       g_slist_free_full(search->services, g_free);
>>>> -       g_free(search);
>>>> +       store_services(device);
>>>> +
>>>> +       browse_request_free(req);
>>>>  }
>>>>
>>>> -static void find_included_services(struct browse_req *req, GSList *services)
>>>> +static void gatt_client_ready_cb(bool success, uint8_t att_ecode,
>>>> +                                                               void *user_data)
>>>>  {
>>>> -       struct btd_device *device = req->device;
>>>> -       struct included_search *search;
>>>> -       struct gatt_primary *prim;
>>>> -       GSList *l;
>>>> +       struct btd_device *device = user_data;
>>>>
>>>> -       DBG("service count %u", g_slist_length(services));
>>>> +       DBG("gatt-client ready -- status: %s, ATT error: %u",
>>>> +                                               success ? "success" : "failed",
>>>> +                                               att_ecode);
>>>> +
>>>> +       if (!success) {
>>>> +               if (device->browse) {
>>>> +                       struct browse_req *req = device->browse;
>>>> +                       send_le_browse_response(req);
>>>> +                       device->browse = NULL;
>>>> +                       browse_request_free(req);
>>>> +               }
>>>>
>>>> -       if (services == NULL) {
>>>> -               DBG("No services found");
>>>> -               register_all_services(req, NULL);
>>>>                 return;
>>>>         }
>>>>
>>>> -       search = g_new0(struct included_search, 1);
>>>> -       search->req = req;
>>>> +       device->att_mtu = bt_att_get_mtu(device->att);
>>>>
>>>> -       /* We have to completely duplicate the data in order to have a
>>>> -        * clearly defined responsibility of freeing regardless of
>>>> -        * failure or success. Otherwise memory leaks are inevitable.
>>>> -        */
>>>> -       for (l = services; l; l = g_slist_next(l)) {
>>>> -               struct gatt_primary *dup;
>>>> +       DBG("gatt-client exchanged MTU: %u", device->att_mtu);
>>>>
>>>> -               dup = g_memdup(l->data, sizeof(struct gatt_primary));
>>>> +       if (device->browse)
>>>> +               register_gatt_services(device->browse);
>>>>
>>>> -               search->services = g_slist_append(search->services, dup);
>>>> -       }
>>>> +       /*
>>>> +        * TODO: Change attio callbacks to accept a gatt-client instead of a
>>>> +        * GAttrib.
>>>> +        */
>>>> +       g_slist_foreach(device->attios, attio_connected, device->attrib);
>>>> +}
>>>>
>>>> -       search->current = search->services;
>>>> +static void gatt_client_service_changed(uint16_t start_handle,
>>>> +                                                       uint16_t end_handle,
>>>> +                                                       void *user_data)
>>>> +{
>>>> +       struct btd_device *device = user_data;
>>>> +
>>>> +       DBG("gatt-client: Service Changed: start 0x%04x, end: 0x%04x",
>>>> +                                               start_handle, end_handle);
>>>>
>>>> -       prim = search->current->data;
>>>> -       gatt_find_included(device->attrib, prim->range.start, prim->range.end,
>>>> -                                       find_included_cb, search);
>>>> +       /*
>>>> +        * TODO: Notify clients that services changed so they can handle it
>>>> +        * directly. Remove the profile if a service was removed.
>>>> +        */
>>>> +       device_browse_primary(device, NULL);
>>>>  }
>>>>
>>>> -static void primary_cb(uint8_t status, GSList *services, void *user_data)
>>>> +static void gatt_client_init(struct btd_device *device)
>>>>  {
>>>> -       struct browse_req *req = user_data;
>>>> +       gatt_client_cleanup(device);
>>>>
>>>> -       DBG("status %u", status);
>>>> +       device->gatt_client = bt_gatt_client_new(device->att, device->att_mtu);
>>>> +       if (!device->gatt_client) {
>>>> +               DBG("Failed to initialize gatt-client");
>>>> +               return;
>>>> +       }
>>>>
>>>> -       if (status) {
>>>> -               struct btd_device *device = req->device;
>>>> +       if (!bt_gatt_client_set_ready_handler(device->gatt_client,
>>>> +                                                       gatt_client_ready_cb,
>>>> +                                                       device, NULL)) {
>>>> +               DBG("Failed to set ready handler for gatt-client");
>>>> +               gatt_client_cleanup(device);
>>>> +               return;
>>>> +       }
>>>>
>>>> -               send_le_browse_response(req);
>>>> -               device->browse = NULL;
>>>> -               browse_request_free(req);
>>>> +       if (!bt_gatt_client_set_service_changed(device->gatt_client,
>>>> +                                               gatt_client_service_changed,
>>>> +                                               device, NULL)) {
>>>> +               DBG("Failed to set service changed handler for gatt-client");
>>>> +               gatt_client_cleanup(device);
>>>>                 return;
>>>>         }
>>>>
>>>> -       find_included_services(req, services);
>>>> +       DBG("gatt-client created");
>>>>  }
>>>>
>>>>  bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
>>>> @@ -3660,10 +3676,8 @@ bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
>>>>                 }
>>>>         }
>>>>
>>>> -       if (cid == ATT_CID)
>>>> -               mtu = BT_ATT_DEFAULT_LE_MTU;
>>>> -
>>>> -       attrib = g_attrib_new(io, mtu);
>>>> +       dev->att_mtu = MIN(mtu, BT_ATT_MAX_LE_MTU);
>>>> +       attrib = g_attrib_new(io, dev->att_mtu);
>>>>         if (!attrib) {
>>>>                 error("Unable to create new GAttrib instance");
>>>>                 return false;
>>>> @@ -3678,11 +3692,12 @@ bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
>>>>
>>>>         dev->attrib = attrib;
>>>>         dev->att = bt_att_ref(g_attrib_get_att(attrib));
>>>> -
>>>>         dev->att_disconn_id = bt_att_register_disconnect(dev->att,
>>>>                                                 att_disconnected_cb, dev, NULL);
>>>>         bt_att_set_close_on_unref(dev->att, true);
>>>>
>>>> +       gatt_client_init(dev);
>>>> +
>>>>         /*
>>>>          * Remove the device from the connect_list and give the passive
>>>>          * scanning another chance to be restarted in case there are
>>>> @@ -3690,8 +3705,6 @@ bool device_attach_attrib(struct btd_device *dev, GIOChannel *io)
>>>>          */
>>>>         adapter_connect_list_remove(dev->adapter, dev);
>>>>
>>>> -       g_slist_foreach(dev->attios, attio_connected, dev->attrib);
>>>> -
>>>>         return true;
>>>>  }
>>>>
>>>> @@ -3845,13 +3858,29 @@ static void att_browse_error_cb(const GError *gerr, gpointer user_data)
>>>>         browse_request_free(req);
>>>>  }
>>>>
>>>> +static void browse_gatt_client(struct browse_req *req)
>>>> +{
>>>> +       struct btd_device *device = req->device;
>>>> +
>>>> +       if (!device->gatt_client) {
>>>> +               DBG("No gatt-client currently attached");
>>>> +               return;
>>>> +       }
>>>> +
>>>> +       /*
>>>> +        * If gatt-client is ready, then register all services. Otherwise, this
>>>> +        * will be deferred until client becomes ready.
>>>> +        */
>>>> +       if (bt_gatt_client_is_ready(device->gatt_client))
>>>> +               register_gatt_services(req);
>>>> +}
>>>> +
>>>>  static void att_browse_cb(gpointer user_data)
>>>>  {
>>>>         struct att_callbacks *attcb = user_data;
>>>>         struct btd_device *device = attcb->user_data;
>>>>
>>>> -       gatt_discover_primary(device->attrib, NULL, primary_cb,
>>>> -                                                       device->browse);
>>>> +       browse_gatt_client(device->browse);
>>>>  }
>>>>
>>>>  static int device_browse_primary(struct btd_device *device, DBusMessage *msg)
>>>> @@ -3869,7 +3898,7 @@ static int device_browse_primary(struct btd_device *device, DBusMessage *msg)
>>>>         device->browse = req;
>>>>
>>>>         if (device->attrib) {
>>>> -               gatt_discover_primary(device->attrib, NULL, primary_cb, req);
>>>> +               browse_gatt_client(device->browse);
>>>>                 goto done;
>>>>         }
>>>>
>>>> @@ -4738,21 +4767,6 @@ GSList *btd_device_get_primaries(struct btd_device *device)
>>>>         return device->primaries;
>>>>  }
>>>>
>>>> -void btd_device_gatt_set_service_changed(struct btd_device *device,
>>>> -                                               uint16_t start, uint16_t end)
>>>> -{
>>>> -       GSList *l;
>>>> -
>>>> -       for (l = device->primaries; l; l = g_slist_next(l)) {
>>>> -               struct gatt_primary *prim = l->data;
>>>> -
>>>> -               if (start <= prim->range.end && end >= prim->range.start)
>>>> -                       prim->changed = TRUE;
>>>> -       }
>>>> -
>>>> -       device_browse_primary(device, NULL);
>>>> -}
>>>> -
>>>>  void btd_device_add_uuid(struct btd_device *device, const char *uuid)
>>>>  {
>>>>         GSList *uuid_list;
>>>> diff --git a/src/device.h b/src/device.h
>>>> index 2e0473e..00cb502 100644
>>>> --- a/src/device.h
>>>> +++ b/src/device.h
>>>> @@ -67,8 +67,6 @@ const sdp_record_t *btd_device_get_record(struct btd_device *device,
>>>>  struct gatt_primary *btd_device_get_primary(struct btd_device *device,
>>>>                                                         const char *uuid);
>>>>  GSList *btd_device_get_primaries(struct btd_device *device);
>>>> -void btd_device_gatt_set_service_changed(struct btd_device *device,
>>>> -                                               uint16_t start, uint16_t end);
>>>>  bool device_attach_attrib(struct btd_device *dev, GIOChannel *io);
>>>>  void btd_device_add_uuid(struct btd_device *device, const char *uuid);
>>>>  void device_add_eir_uuids(struct btd_device *dev, GSList *uuids);
>>>> diff --git a/src/shared/att-types.h b/src/shared/att-types.h
>>>> index 3d8829a..8b6d537 100644
>>>> --- a/src/shared/att-types.h
>>>> +++ b/src/shared/att-types.h
>>>> @@ -27,7 +27,8 @@
>>>>  #define __packed __attribute__((packed))
>>>>  #endif
>>>>
>>>> -#define BT_ATT_DEFAULT_LE_MTU 23
>>>> +#define BT_ATT_DEFAULT_LE_MTU  23
>>>> +#define BT_ATT_MAX_LE_MTU      517
>>>>
>>>>  /* ATT protocol opcodes */
>>>>  #define BT_ATT_OP_ERROR_RSP                    0x01
>>>> --
>>>> 2.1.0.rc2.206.gedb03e5
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>>
>>>
>>> --
>>> Luiz Augusto von Dentz
>>
>> Thanks,
>> Arman
>
>
>
> --
> Luiz Augusto von Dentz

Thanks,
Arman