[PATCH BlueZ v4 00/10] core: Use shared/gatt-client

[PATCH BlueZ v4 00/10] core: Use shared/gatt-client

[Arman Uguray]

*v4:
  - Removed gatt-callbacks in favor of using btd_profile probe and remove
    functions. Added the new btd_profile function "accept" to notify profiles
    that gatt-client is ready.

  - Using device_remove_profile to unregister btd_service's when services are
    removed from the client database. This happens on Service Changed events and
    on disconnection if the device is not bonded.

  - Removed immediated disconnection logic that triggers when no attio callbacks
    are registered after probe.

  - Other minor fixes to shared/gatt and shared/att.

*v3:
  - Renamed device_attach_att_transport to device_attach_att.

*v2:
  - Rebased.

*v1:
  - Addressed comments by Johan and Luiz.


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 (10):
  shared/att: Guard against invalid ref in callbacks
  shared/gatt-client: Guard against invalid access
  core: service: Add start & end handle fields
  core: profile: Add accept function
  core: device: Don't disconnect if attios not set
  core: device: Add getters for GATT db and client
  core: device: Make profile calls in GATT events
  profiles/gatt: Don't handle GATT service.
  profiles/gatt: Rename profile to gap.
  profiles/gap: Rewrite using bt_gatt_client.

 Makefile.plugins         |   4 +-
 profiles/gap/gas.c       | 338 +++++++++++++++++++++++++++++++++++
 profiles/gatt/gas.c      | 457 -----------------------------------------------
 src/device.c             | 285 +++++++++++++++++++++++------
 src/device.h             |   2 +
 src/profile.h            |   2 +
 src/service.c            |  56 ++++++
 src/service.h            |   9 +
 src/shared/att.c         |   5 +
 src/shared/gatt-client.c |  34 ++--
 10 files changed, 663 insertions(+), 529 deletions(-)
 create mode 100644 profiles/gap/gas.c
 delete mode 100644 profiles/gatt/gas.c

-- 
2.2.0.rc0.207.ga3a616c

[PATCH BlueZ v4 01/10] shared/att: Guard against invalid ref in callbacks

[Arman Uguray]

This patch adds a ref-count guard around incoming PDU handling so that
calling bt_att_unref from a callback doesn't free the bt_att instance.
---
 src/shared/att.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/shared/att.c b/src/shared/att.c
index a5bf244..4be0652 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -722,6 +722,8 @@ static bool can_read_data(struct io *io, void *user_data)
 	pdu = att->buf;
 	opcode = pdu[0];
 
+	bt_att_ref(att);
+
 	/* Act on the received PDU based on the opcode type */
 	switch (get_op_type(opcode)) {
 	case ATT_OP_TYPE_RSP:
@@ -745,6 +747,7 @@ static bool can_read_data(struct io *io, void *user_data)
 					"Received request while another is "
 					"pending: 0x%02x", opcode);
 			io_shutdown(att->io);
+			bt_att_unref(att);
 
 			return false;
 		}
@@ -766,6 +769,8 @@ static bool can_read_data(struct io *io, void *user_data)
 		break;
 	}
 
+	bt_att_unref(att);
+
 	return true;
 }
 
-- 
2.2.0.rc0.207.ga3a616c

[PATCH BlueZ v4 02/10] shared/gatt-client: Guard against invalid access

[Arman Uguray]

This patch adds a ref-count based guard around call sites that invoke
a callback set by the upper-layer, to prevent memory errors after the
callback returns.
---
 src/shared/gatt-client.c | 34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index e86c07f..f7a90d1 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -889,6 +889,17 @@ done:
 	op->complete_func(op, success, att_ecode);
 }
 
+static void notify_client_ready(struct bt_gatt_client *client, bool success,
+							uint8_t att_ecode)
+{
+	if (!client->ready_callback)
+		return;
+
+	bt_gatt_client_ref(client);
+	client->ready_callback(success, att_ecode, client->ready_data);
+	bt_gatt_client_unref(client);
+}
+
 static void exchange_mtu_cb(bool success, uint8_t att_ecode, void *user_data)
 {
 	struct discovery_op *op = user_data;
@@ -902,10 +913,7 @@ static void exchange_mtu_cb(bool success, uint8_t att_ecode, void *user_data)
 				att_ecode);
 
 		client->in_init = false;
-
-		if (client->ready_callback)
-			client->ready_callback(success, att_ecode,
-							client->ready_data);
+		notify_client_ready(client, success, att_ecode);
 
 		return;
 	}
@@ -930,9 +938,7 @@ static void exchange_mtu_cb(bool success, uint8_t att_ecode, void *user_data)
 			"Failed to initiate primary service discovery");
 
 	client->in_init = false;
-
-	if (client->ready_callback)
-		client->ready_callback(false, att_ecode, client->ready_data);
+	notify_client_ready(client, false, att_ecode);
 
 	discovery_op_unref(op);
 }
@@ -1134,8 +1140,7 @@ static void service_changed_register_cb(unsigned int id, uint16_t att_ecode,
 			"Registered handler for \"Service Changed\": %u", id);
 
 done:
-	if (client->ready_callback)
-		client->ready_callback(success, att_ecode, client->ready_data);
+	notify_client_ready(client, success, att_ecode);
 }
 
 static void init_complete(struct discovery_op *op, bool success,
@@ -1194,8 +1199,7 @@ fail:
 	op->success = false;
 
 done:
-	if (client->ready_callback)
-		client->ready_callback(success, att_ecode, client->ready_data);
+	notify_client_ready(client, success, att_ecode);
 }
 
 static void init_fail(struct discovery_op *op)
@@ -1331,9 +1335,13 @@ static void enable_ccc_callback(uint8_t opcode, const void *pdu,
 	}
 
 	/* Success! Report success for all remaining requests. */
+	bt_gatt_client_ref(notify_data->client);
+
 	complete_notify_request(notify_data);
 	queue_remove_all(notify_data->chrc->reg_notify_queue, NULL, NULL,
 						complete_notify_request);
+
+	bt_gatt_client_unref(notify_data->client);
 }
 
 static void disable_ccc_callback(uint8_t opcode, const void *pdu,
@@ -1451,8 +1459,8 @@ static void att_disconnect_cb(int err, void *user_data)
 	client->in_init = false;
 	client->ready = false;
 
-	if (in_init && client->ready_callback)
-		client->ready_callback(false, 0, client->ready_data);
+	if (in_init)
+		notify_client_ready(client, false, 0);
 }
 
 struct bt_gatt_client *bt_gatt_client_new(struct gatt_db *db,
-- 
2.2.0.rc0.207.ga3a616c

[PATCH BlueZ v4 03/10] core: service: Add start & end handle fields

[Arman Uguray]

When GATT services are discovered, added, and removed, profiles will
be notified using btd_profile device_probe and device_remove functions.
Since a single profile may handle multiple services from the same
device, it is useful to distinguish each btd_service by its service
handle range. This patch adds the start_handle and end_handle fields to
btd_service and an accessor for getting them, which reports failure if
the service was initialized without handles.
---
 src/service.c | 38 ++++++++++++++++++++++++++++++++++++++
 src/service.h |  7 +++++++
 2 files changed, 45 insertions(+)

diff --git a/src/service.c b/src/service.c
index 7a480d6..afe592b 100644
--- a/src/service.c
+++ b/src/service.c
@@ -53,6 +53,8 @@ struct btd_service {
 	void			*user_data;
 	btd_service_state_t	state;
 	int			err;
+	uint16_t		start_handle;
+	uint16_t		end_handle;
 };
 
 struct service_state_callback {
@@ -149,6 +151,26 @@ struct btd_service *service_create(struct btd_device *device,
 	return service;
 }
 
+struct btd_service *service_create_gatt(struct btd_device *device,
+						struct btd_profile *profile,
+						uint16_t start_handle,
+						uint16_t end_handle)
+{
+	struct btd_service *service;
+
+	if (!start_handle || !end_handle || start_handle > end_handle)
+		return NULL;
+
+	service = service_create(device, profile);
+	if (!service)
+		return NULL;
+
+	service->start_handle = start_handle;
+	service->end_handle = end_handle;
+
+	return service;
+}
+
 int service_probe(struct btd_service *service)
 {
 	char addr[18];
@@ -282,6 +304,22 @@ int btd_service_get_error(const struct btd_service *service)
 	return service->err;
 }
 
+bool btd_service_get_gatt_handles(const struct btd_service *service,
+							uint16_t *start_handle,
+							uint16_t *end_handle)
+{
+	if (!service || !service->start_handle || !service->end_handle)
+		return false;
+
+	if (start_handle)
+		*start_handle = service->start_handle;
+
+	if (end_handle)
+		*end_handle = service->end_handle;
+
+	return true;
+}
+
 unsigned int btd_service_add_state_cb(btd_service_state_cb cb, void *user_data)
 {
 	struct service_state_callback *state_cb;
diff --git a/src/service.h b/src/service.h
index 5230115..857e688 100644
--- a/src/service.h
+++ b/src/service.h
@@ -44,6 +44,10 @@ void btd_service_unref(struct btd_service *service);
 /* Service management functions used by the core */
 struct btd_service *service_create(struct btd_device *device,
 						struct btd_profile *profile);
+struct btd_service *service_create_gatt(struct btd_device *device,
+						struct btd_profile *profile,
+						uint16_t start_handle,
+						uint16_t end_handle);
 
 int service_probe(struct btd_service *service);
 void service_remove(struct btd_service *service);
@@ -57,6 +61,9 @@ struct btd_device *btd_service_get_device(const struct btd_service *service);
 struct btd_profile *btd_service_get_profile(const struct btd_service *service);
 btd_service_state_t btd_service_get_state(const struct btd_service *service);
 int btd_service_get_error(const struct btd_service *service);
+bool btd_service_get_gatt_handles(const struct btd_service *service,
+							uint16_t *start_handle,
+							uint16_t *end_handle);
 
 unsigned int btd_service_add_state_cb(btd_service_state_cb cb,
 							void *user_data);
-- 
2.2.0.rc0.207.ga3a616c

[PATCH BlueZ v4 04/10] core: profile: Add accept function

[Arman Uguray]

This patch introduces the "accept" function to btd_profile. The purpose
of this function is to notify a profile when a btd_service is ready to
interact with and will be called by btd_device when a bt_gatt_client
becomes ready (MTU exchange and discovery has been completed). This is
initially meant to be optional for non-GATT based profiles but could be
generalized for them in the future.
---
 src/profile.h |  2 ++
 src/service.c | 18 ++++++++++++++++++
 src/service.h |  2 ++
 3 files changed, 22 insertions(+)

diff --git a/src/profile.h b/src/profile.h
index 9aec27e..f5a3ded 100644
--- a/src/profile.h
+++ b/src/profile.h
@@ -42,6 +42,8 @@ struct btd_profile {
 	int (*connect) (struct btd_service *service);
 	int (*disconnect) (struct btd_service *service);
 
+	int (*accept) (struct btd_service *service);
+
 	int (*adapter_probe) (struct btd_profile *p,
 						struct btd_adapter *adapter);
 	void (*adapter_remove) (struct btd_profile *p,
diff --git a/src/service.c b/src/service.c
index afe592b..2ea2b7a 100644
--- a/src/service.c
+++ b/src/service.c
@@ -199,6 +199,24 @@ void service_remove(struct btd_service *service)
 	btd_service_unref(service);
 }
 
+int service_accept(struct btd_service *service)
+{
+	char addr[18];
+	int err;
+
+	if (!service->profile->accept)
+		return 0;
+
+	err = service->profile->accept(service);
+	if (!err)
+		return 0;
+
+	ba2str(device_get_address(service->device), addr);
+	error("%s profile accept failed for %s", service->profile->name, addr);
+
+	return err;
+}
+
 int btd_service_connect(struct btd_service *service)
 {
 	struct btd_profile *profile = service->profile;
diff --git a/src/service.h b/src/service.h
index 857e688..3a0db6e 100644
--- a/src/service.h
+++ b/src/service.h
@@ -52,6 +52,8 @@ struct btd_service *service_create_gatt(struct btd_device *device,
 int service_probe(struct btd_service *service);
 void service_remove(struct btd_service *service);
 
+int service_accept(struct btd_service *service);
+
 /* Connection control API */
 int btd_service_connect(struct btd_service *service);
 int btd_service_disconnect(struct btd_service *service);
-- 
2.2.0.rc0.207.ga3a616c

[PATCH BlueZ v4 05/10] core: device: Don't disconnect if attios not set

[Arman Uguray]

Currently there is no way for external applications to claim ownership
on a GATT connection and as profiles move away from attio.h it doesn't
make much sense to immediately disconnect if no attio callbacks have
been set after probing the profiles (as this will always immediately
disconnect the device). This patch removes this logic from btd_device.
---
 src/device.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/src/device.c b/src/device.c
index e8bdc18..64591d0 100644
--- a/src/device.c
+++ b/src/device.c
@@ -3642,9 +3642,6 @@ static void register_gatt_services(struct browse_req *req)
 
 	device_probe_profiles(device, req->profiles_added);
 
-	if (device->attios == NULL && device->attios_offline == NULL)
-		attio_cleanup(device);
-
 	device_svc_resolved(device, device->bdaddr_type, 0);
 
 	store_services(device);
@@ -5069,11 +5066,6 @@ gboolean btd_device_remove_attio_callback(struct btd_device *device, guint id)
 
 	g_free(attio);
 
-	if (device->attios != NULL || device->attios_offline != NULL)
-		return TRUE;
-
-	attio_cleanup(device);
-
 	return TRUE;
 }
 
-- 
2.2.0.rc0.207.ga3a616c

[PATCH BlueZ v4 06/10] core: device: Add getters for GATT db and client

[Arman Uguray]

This patch adds btd_device_get_gatt_db and btd_device_get_gatt_client
functions.
---
 src/device.c | 16 ++++++++++++++++
 src/device.h |  2 ++
 2 files changed, 18 insertions(+)

diff --git a/src/device.c b/src/device.c
index 64591d0..854d9f3 100644
--- a/src/device.c
+++ b/src/device.c
@@ -4844,6 +4844,22 @@ GSList *btd_device_get_primaries(struct btd_device *device)
 	return device->primaries;
 }
 
+struct gatt_db *btd_device_get_gatt_db(struct btd_device *device)
+{
+	if (!device)
+		return NULL;
+
+	return device->db;
+}
+
+struct bt_gatt_client *btd_device_get_gatt_client(struct btd_device *device)
+{
+	if (!device)
+		return NULL;
+
+	return device->client;
+}
+
 void btd_device_gatt_set_service_changed(struct btd_device *device,
 						uint16_t start, uint16_t end)
 {
diff --git a/src/device.h b/src/device.h
index 487bd27..a7fefee 100644
--- a/src/device.h
+++ b/src/device.h
@@ -67,6 +67,8 @@ 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);
+struct gatt_db *btd_device_get_gatt_db(struct btd_device *device);
+struct bt_gatt_client *btd_device_get_gatt_client(struct btd_device *device);
 void btd_device_gatt_set_service_changed(struct btd_device *device,
 						uint16_t start, uint16_t end);
 bool device_attach_att(struct btd_device *dev, GIOChannel *io);
-- 
2.2.0.rc0.207.ga3a616c

[PATCH BlueZ v4 07/10] core: device: Make profile calls in GATT events

[Arman Uguray]

This patch correctly integrates all profile calls into the various GATT
client events (ready, service-added, service-removed) so that profiles
can perform the necessary updates. The major changes introduced in this
this patch are:

  1. Added new profile probe/remove functions for GATT services, which
     operate on a btd_device's client db and initialize btd_service
     instances with start & end handles:
        - device_probe_gatt_profiles
        - device_probe_gatt_profile
        - device_remove_gatt_profile
        - device_accept_gatt_profiles

  2. device_probe_gatt_profiles is called when the gatt-client becomes
     ready. Profiles are then notified with the "accept" call.

  3. device_probe_gatt_profile is called when a new GATT service is
     added to the db.

  4. device_remove_gatt_profile is called when a GATT service is removed
     from the db.

  5. Profiles are notified that the gatt-client is ready via the
     btd_service "accept" function on a per-service basis. Currently this
     is always called immediately after a probe.
---
 src/device.c | 261 ++++++++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 212 insertions(+), 49 deletions(-)

diff --git a/src/device.c b/src/device.c
index 854d9f3..6242c76 100644
--- a/src/device.c
+++ b/src/device.c
@@ -292,6 +292,27 @@ static GSList *find_service_with_state(GSList *list,
 	return NULL;
 }
 
+static GSList *find_service_with_gatt_handles(GSList *list,
+							uint16_t start_handle,
+							uint16_t end_handle)
+{
+	GSList *l;
+	uint16_t svc_start, svc_end;
+
+	for (l = list; l != NULL; l = g_slist_next(l)) {
+		struct btd_service *service = l->data;
+
+		if (!btd_service_get_gatt_handles(service, &svc_start,
+								&svc_end))
+			continue;
+
+		if (svc_start == start_handle && svc_end == end_handle)
+			return l;
+	}
+
+	return NULL;
+}
+
 static void update_technologies(GKeyFile *file, struct btd_device *dev)
 {
 	const char *list[2];
@@ -2390,15 +2411,168 @@ static void add_primary(struct gatt_db_attribute *attr, void *user_data)
 	*new_services = g_slist_append(*new_services, prim);
 }
 
+static void device_add_uuids(struct btd_device *device, GSList *uuids)
+{
+	GSList *l;
+
+	for (l = uuids; l != NULL; l = g_slist_next(l)) {
+		GSList *match = g_slist_find_custom(device->uuids, l->data,
+							bt_uuid_strcmp);
+		if (match)
+			continue;
+
+		device->uuids = g_slist_insert_sorted(device->uuids,
+						g_strdup(l->data),
+						bt_uuid_strcmp);
+	}
+
+	g_dbus_emit_property_changed(dbus_conn, device->path,
+						DEVICE_INTERFACE, "UUIDs");
+}
+
 static void store_services(struct btd_device *device);
 
+struct gatt_probe_data {
+	struct btd_device *dev;
+	bool add_uuid_to_device;
+	GSList *uuids;
+	struct gatt_db_attribute *cur_attr;
+	char cur_uuid[MAX_LEN_UUID_STR];
+	uint16_t start_handle, end_handle;
+};
+
+static bool device_match_profile(struct btd_device *device,
+					struct btd_profile *profile,
+					GSList *uuids)
+{
+	if (profile->remote_uuid == NULL)
+		return false;
+
+	if (g_slist_find_custom(uuids, profile->remote_uuid,
+							bt_uuid_strcmp) == NULL)
+		return false;
+
+	return true;
+}
+
+static void dev_probe_gatt(struct btd_profile *p, void *user_data)
+{
+	struct gatt_probe_data *data = user_data;
+	struct btd_service *service;
+
+	if (p->device_probe == NULL)
+		return;
+
+	if (!device_match_profile(data->dev, p, g_slist_last(data->uuids)))
+		return;
+
+	service = service_create_gatt(data->dev, p, data->start_handle,
+							data->end_handle);
+	if (!service)
+		return;
+
+	if (service_probe(service) < 0) {
+		btd_service_unref(service);
+		return;
+	}
+
+	data->dev->services = g_slist_append(data->dev->services, service);
+}
+
+static void dev_probe_gatt_profile(struct gatt_db_attribute *attr,
+							void *user_data)
+{
+	struct gatt_probe_data *data = user_data;
+	bt_uuid_t uuid;
+	GSList *l;
+
+	gatt_db_attribute_get_service_data(attr, &data->start_handle,
+							&data->end_handle, NULL,
+							&uuid);
+	bt_uuid_to_string(&uuid, data->cur_uuid, sizeof(data->cur_uuid));
+
+	data->uuids = g_slist_append(data->uuids, g_strdup(data->cur_uuid));
+
+	data->cur_attr = attr;
+
+	btd_profile_foreach(dev_probe_gatt, data);
+
+	if (!data->add_uuid_to_device)
+		return;
+
+	l = g_slist_append(l, g_strdup(data->cur_uuid));
+	device_add_uuids(data->dev, l);
+}
+
+static void device_probe_gatt_profile(struct btd_device *device,
+						struct gatt_db_attribute *attr)
+{
+	struct gatt_probe_data data;
+
+	memset(&data, 0, sizeof(data));
+
+	data.dev = device;
+	data.add_uuid_to_device = true;
+
+	dev_probe_gatt_profile(attr, &data);
+	g_slist_free_full(data.uuids, g_free);
+}
+
+static void device_probe_gatt_profiles(struct btd_device *device)
+{
+	struct gatt_probe_data data;
+	char addr[18];
+
+	ba2str(&device->bdaddr, addr);
+
+	if (device->blocked) {
+		DBG("Skipping profiles for blocked device %s", addr);
+		return;
+	}
+
+	memset(&data, 0, sizeof(data));
+
+	data.dev = device;
+
+	gatt_db_foreach_service(device->db, NULL, dev_probe_gatt_profile,
+									&data);
+	device_add_uuids(device, data.uuids);
+	g_slist_free_full(data.uuids, g_free);
+}
+
+static void device_accept_gatt_profiles(struct btd_device *device)
+{
+	GSList *l;
+
+	for (l = device->services; l != NULL; l = g_slist_next(l))
+		service_accept(l->data);
+}
+
+static void device_remove_gatt_profile(struct btd_device *device,
+						struct gatt_db_attribute *attr)
+{
+	uint16_t start, end;
+	struct btd_service *service;
+	GSList *l;
+
+	gatt_db_attribute_get_service_handles(attr, &start, &end);
+
+	l = find_service_with_gatt_handles(device->services, start, end);
+	if (!l)
+		return;
+
+	service = l->data;
+	device->services = g_slist_delete_link(device->services, l);
+	device->pending = g_slist_remove(device->pending, service);
+	service_remove(service);
+}
+
 static void gatt_service_added(struct gatt_db_attribute *attr, void *user_data)
 {
 	struct btd_device *device = user_data;
-	struct gatt_primary *prim;
 	GSList *new_service = NULL;
-	GSList *profiles_added = NULL;
 	uint16_t start, end;
+	GSList *l;
 
 	if (!bt_gatt_client_is_ready(device->client))
 		return;
@@ -2417,14 +2591,13 @@ static void gatt_service_added(struct gatt_db_attribute *attr, void *user_data)
 
 	device_register_primaries(device, new_service, -1);
 
-	prim = new_service->data;
-	profiles_added = g_slist_append(profiles_added, g_strdup(prim->uuid));
+	device_probe_gatt_profile(device, attr);
 
-	device_probe_profiles(device, profiles_added);
+	l = find_service_with_gatt_handles(device->services, start, end);
+	if (l)
+		service_accept(l->data);
 
 	store_services(device);
-
-	g_slist_free_full(profiles_added, g_free);
 }
 
 static gint prim_attr_cmp(gconstpointer a, gconstpointer b)
@@ -2438,6 +2611,14 @@ static gint prim_attr_cmp(gconstpointer a, gconstpointer b)
 	return !(prim->range.start == start && prim->range.end == end);
 }
 
+static gint prim_uuid_cmp(gconstpointer a, gconstpointer b)
+{
+	const struct gatt_primary *prim = a;
+	const char *uuid = b;
+
+	return bt_uuid_strcmp(prim->uuid, uuid);
+}
+
 static void gatt_service_removed(struct gatt_db_attribute *attr,
 								void *user_data)
 {
@@ -2461,20 +2642,24 @@ static void gatt_service_removed(struct gatt_db_attribute *attr,
 	prim = l->data;
 	device->primaries = g_slist_delete_link(device->primaries, l);
 
-	/* Remove the corresponding UUIDs entry */
-	l = g_slist_find_custom(device->uuids, prim->uuid, bt_uuid_strcmp);
-	device->uuids = g_slist_delete_link(device->uuids, l);
-	g_free(prim);
-
-	store_services(device);
+	device_remove_gatt_profile(device, attr);
 
 	/*
-	 * TODO: Notify the profiles somehow. It may be sufficient for each
-	 * profile to register a service_removed handler.
+	 * Remove the corresponding UUIDs entry, only if this is the last
+	 * service with this UUID.
 	 */
+	l = g_slist_find_custom(device->uuids, prim->uuid, bt_uuid_strcmp);
 
-	g_dbus_emit_property_changed(dbus_conn, device->path,
+	if (!g_slist_find_custom(device->primaries, prim->uuid,
+							prim_uuid_cmp)) {
+		device->uuids = g_slist_delete_link(device->uuids, l);
+		g_dbus_emit_property_changed(dbus_conn, device->path,
 						DEVICE_INTERFACE, "UUIDs");
+	}
+
+	g_free(prim);
+
+	store_services(device);
 }
 
 static struct btd_device *device_new(struct btd_adapter *adapter,
@@ -2975,20 +3160,6 @@ GSList *btd_device_get_uuids(struct btd_device *device)
 	return device->uuids;
 }
 
-static bool device_match_profile(struct btd_device *device,
-					struct btd_profile *profile,
-					GSList *uuids)
-{
-	if (profile->remote_uuid == NULL)
-		return false;
-
-	if (g_slist_find_custom(uuids, profile->remote_uuid,
-							bt_uuid_strcmp) == NULL)
-		return false;
-
-	return true;
-}
-
 struct probe_data {
 	struct btd_device *dev;
 	GSList *uuids;
@@ -3065,7 +3236,6 @@ void device_remove_profile(gpointer a, gpointer b)
 void device_probe_profiles(struct btd_device *device, GSList *uuids)
 {
 	struct probe_data d = { device, uuids };
-	GSList *l;
 	char addr[18];
 
 	ba2str(&device->bdaddr, addr);
@@ -3080,19 +3250,7 @@ void device_probe_profiles(struct btd_device *device, GSList *uuids)
 	btd_profile_foreach(dev_probe, &d);
 
 add_uuids:
-	for (l = uuids; l != NULL; l = g_slist_next(l)) {
-		GSList *match = g_slist_find_custom(device->uuids, l->data,
-							bt_uuid_strcmp);
-		if (match)
-			continue;
-
-		device->uuids = g_slist_insert_sorted(device->uuids,
-						g_strdup(l->data),
-						bt_uuid_strcmp);
-	}
-
-	g_dbus_emit_property_changed(dbus_conn, device->path,
-						DEVICE_INTERFACE, "UUIDs");
+	device_add_uuids(device, uuids);
 }
 
 static void store_sdp_record(GKeyFile *key_file, sdp_record_t *rec)
@@ -3412,6 +3570,13 @@ static void search_cb(sdp_list_t *recs, int err, gpointer user_data)
 	if (primaries)
 		device_register_primaries(device, primaries, ATT_PSM);
 
+	/*
+	 * TODO: The btd_service instances for GATT services need to be
+	 * initialized with the service handles. Eventually this code should
+	 * perform ATT protocol service discovery over the ATT PSM to obtain
+	 * the full list of services and populate a client-role gatt_db over
+	 * BR/EDR.
+	 */
 	device_probe_profiles(device, req->profiles_added);
 
 	/* Propagate services changes */
@@ -3640,7 +3805,7 @@ static void register_gatt_services(struct browse_req *req)
 
 	device_register_primaries(device, services, -1);
 
-	device_probe_profiles(device, req->profiles_added);
+	device_probe_gatt_profiles(device);
 
 	device_svc_resolved(device, device->bdaddr_type, 0);
 
@@ -3675,10 +3840,8 @@ 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.
-	 */
+	device_accept_gatt_profiles(device);
+
 	g_slist_foreach(device->attios, attio_connected, device->attrib);
 }
 
-- 
2.2.0.rc0.207.ga3a616c

[PATCH BlueZ v4 08/10] 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 | 251 ++--------------------------------------------------
 1 file changed, 5 insertions(+), 246 deletions(-)

diff --git a/profiles/gatt/gas.c b/profiles/gatt/gas.c
index b51b4a8..9d5d31e 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,163 +110,12 @@ 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 +128,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 +154,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 +174,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.2.0.rc0.207.ga3a616c

[PATCH BlueZ v4 09/10] 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  | 213 +++++++++++++++++++++++++++++++++++++++++++++++++++
 profiles/gatt/gas.c | 216 ----------------------------------------------------
 3 files changed, 215 insertions(+), 218 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..a4028dd
--- /dev/null
+++ b/profiles/gap/gas.c
@@ -0,0 +1,213 @@
+/*
+ *
+ *  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.
+ */
+
+#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;
+
+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;
+	uint16_t app;
+
+	gas->attrib = g_attrib_ref(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)
+{
+	devices = NULL;
+
+	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 9d5d31e..0000000
--- a/profiles/gatt/gas.c
+++ /dev/null
@@ -1,216 +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;
-	uint16_t app;
-
-	gas->attrib = g_attrib_ref(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.2.0.rc0.207.ga3a616c

[PATCH BlueZ v4 10/10] 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 | 269 +++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 197 insertions(+), 72 deletions(-)

diff --git a/profiles/gap/gas.c b/profiles/gap/gas.c
index a4028dd..5702e47 100644
--- a/profiles/gap/gas.c
+++ b/profiles/gap/gas.c
@@ -19,6 +19,7 @@
 #include <config.h>
 #endif
 
+#include <ctype.h>
 #include <stdbool.h>
 #include <stdlib.h>
 #include <sys/types.h>
@@ -28,38 +29,34 @@
 
 #include <glib.h>
 
-#include "btio/btio.h"
 #include "lib/uuid.h"
+#include "src/shared/util.h"
+#include "src/shared/att.h"
+#include "src/shared/queue.h"
+#include "src/shared/gatt-db.h"
+#include "src/shared/gatt-client.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;
+	struct gatt_db *db;
+	struct bt_gatt_client *client;
+	uint16_t start_handle, end_handle;
 };
 
 static GSList *devices;
 
 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);
+	gatt_db_unref(gas->db);
+	bt_gatt_client_unref(gas->client);
 	g_free(gas);
 }
 
@@ -71,128 +68,256 @@ 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);
+
+	DBG("GAP Device Name: %s", name);
+
+	btd_device_device_set_name(gas->device, name);
+}
 
-	if (status != 0) {
-		error("Read characteristics by UUID failed: %s",
-				att_ecode2str(status));
+static void handle_device_name(struct gas *gas, uint16_t value_handle)
+{
+	if (!bt_gatt_client_read_long_value(gas->client, 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, uint16_t value_handle)
+{
+	if (!bt_gatt_client_read_value(gas->client, 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 bt_uuid_t *uuid)
 {
-	struct gas *gas = user_data;
-	uint16_t app;
+	bt_uuid_t lhs;
 
-	gas->attrib = g_attrib_ref(attrib);
+	bt_uuid16_create(&lhs, u16);
 
-	if (device_get_appearance(gas->device, &app) < 0) {
-		bt_uuid_t uuid;
+	return bt_uuid_cmp(&lhs, uuid) == 0;
+}
 
-		bt_uuid16_create(&uuid, GATT_CHARAC_APPEARANCE);
+static void handle_characteristic(struct gatt_db_attribute *attr,
+								void *user_data)
+{
+	struct gas *gas = user_data;
+	uint16_t value_handle;
+	bt_uuid_t uuid;
 
-		gatt_read_char_by_uuid(gas->attrib, gas->gap.start,
-						gas->gap.end, &uuid,
-						gap_appearance_cb, gas);
+	if (!gatt_db_attribute_get_char_data(attr, NULL, &value_handle, NULL,
+								&uuid)) {
+		error("Failed to obtain characteristic data");
+		return;
 	}
 
-	/* TODO: Read other GAP characteristics - See Core spec page 1739 */
+	if (uuid_cmp(GATT_CHARAC_DEVICE_NAME, &uuid))
+		handle_device_name(gas, value_handle);
+	else if (uuid_cmp(GATT_CHARAC_APPEARANCE, &uuid))
+		handle_appearance(gas, value_handle);
+	else {
+		char uuid_str[MAX_LEN_UUID_STR];
+
+		/* TODO: Support peripheral privacy feature */
+
+		bt_uuid_to_string(&uuid, uuid_str, sizeof(uuid_str));
+		DBG("Unsupported characteristic: %s", uuid_str);
+	}
 }
 
-static void attio_disconnected_cb(gpointer user_data)
+static void handle_gap_service(struct gas *gas)
 {
-	struct gas *gas = user_data;
+	struct gatt_db_attribute *attr;
 
-	g_attrib_unref(gas->attrib);
-	gas->attrib = NULL;
+	attr = gatt_db_get_attribute(gas->db, gas->start_handle);
+	if (!attr) {
+		error("Service with handle 0x%04x not found in db",
+							gas->start_handle);
+		return;
+	}
+
+	gatt_db_service_foreach_char(attr, handle_characteristic, gas);
 }
 
-static int gas_register(struct btd_device *device, struct att_range *gap)
+static int gap_driver_probe(struct btd_service *service)
 {
+	struct btd_device *device = btd_service_get_device(service);
 	struct gas *gas;
+	uint16_t start_handle, end_handle;
+	GSList *l;
+	char addr[18];
+
+	if (!btd_service_get_gatt_handles(service, &start_handle, &end_handle))
+		return -1;
+
+	ba2str(device_get_address(device), addr);
+	DBG("GAP profile probe (%s): start: 0x%04x, end 0x%04x", addr,
+						start_handle, end_handle);
+
+	/*
+	 * There can't be more than one instance of the GAP service on the same
+	 * device.
+	 */
+	l = g_slist_find_custom(devices, device, cmp_device);
+	if (l) {
+		error("More than one GAP service exists on device");
+		return -1;
+	}
 
 	gas = g_new0(struct gas, 1);
-	gas->gap.start = gap->start;
-	gas->gap.end = gap->end;
 
 	gas->device = btd_device_ref(device);
-
+	gas->start_handle = start_handle;
+	gas->end_handle = end_handle;
 	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)
+static void gap_driver_remove(struct btd_service *service)
 {
+	struct btd_device *device = btd_service_get_device(service);
 	struct gas *gas;
+	uint16_t start_handle, end_handle;
 	GSList *l;
+	char addr[18];
+
+	if (!btd_service_get_gatt_handles(service, &start_handle,
+								&end_handle)) {
+		error("Removed service is not a GATT service");
+		return;
+	}
+
+	ba2str(device_get_address(device), addr);
+	DBG("GAP profile remove (%s): start: 0x%04x, end 0x%04x", addr,
+						start_handle, end_handle);
 
 	l = g_slist_find_custom(devices, device, cmp_device);
-	if (l == NULL)
+	if (!l) {
+		error("GAP service not handled by profile");
 		return;
+	}
 
 	gas = l->data;
+
+	if (gas->start_handle != start_handle ||
+						gas->end_handle != end_handle) {
+		error("Removed unknown GAP service");
+		return;
+	}
+
 	devices = g_slist_remove(devices, gas);
 	gas_free(gas);
 }
 
-static int gap_driver_probe(struct btd_service *service)
+static int gap_driver_accept(struct btd_service *service)
 {
 	struct btd_device *device = btd_service_get_device(service);
-	struct gatt_primary *gap;
+	struct gatt_db *db = btd_device_get_gatt_db(device);
+	struct bt_gatt_client *client = btd_device_get_gatt_client(device);
+	struct gas *gas;
+	uint16_t start_handle, end_handle;
+	GSList *l;
+	char addr[18];
 
-	gap = btd_device_get_primary(device, GAP_UUID);
+	if (!btd_service_get_gatt_handles(service, &start_handle,
+								&end_handle)) {
+		error("Service is not a GATT service");
+		return -1;
+	}
+
+	ba2str(device_get_address(device), addr);
+	DBG("GAP profile accept (%s): start: 0x%04x, end 0x%04x", addr,
+						start_handle, end_handle);
 
-	if (gap == NULL) {
-		error("GAP service is mandatory");
-		return -EINVAL;
+	l = g_slist_find_custom(devices, device, cmp_device);
+	if (!l) {
+		error("GAP service not handled by profile");
+		return -1;
 	}
 
-	return gas_register(device, &gap->range);
-}
+	gas = l->data;
 
-static void gap_driver_remove(struct btd_service *service)
-{
-	struct btd_device *device = btd_service_get_device(service);
+	if (gas->start_handle != start_handle ||
+						gas->end_handle != end_handle) {
+		error("Accepting unknown GAP service");
+		return -1;
+	}
 
-	gas_unregister(device);
+	/* Clean-up any old client/db and acquire the new ones */
+	gatt_db_unref(gas->db);
+	bt_gatt_client_unref(gas->client);
+	gas->db = NULL;
+	gas->client = NULL;
+
+	gas->db = gatt_db_ref(db);
+	gas->client = bt_gatt_client_ref(client);
+
+	/* Handle the service */
+	handle_gap_service(gas);
+
+	return 0;
 }
 
 static struct btd_profile gap_profile = {
 	.name		= "gap-profile",
 	.remote_uuid	= GAP_UUID,
 	.device_probe	= gap_driver_probe,
-	.device_remove	= gap_driver_remove
+	.device_remove	= gap_driver_remove,
+	.accept		= gap_driver_accept
 };
 
 static int gap_init(void)
-- 
2.2.0.rc0.207.ga3a616c

Re: [PATCH BlueZ v4 06/10] core: device: Add getters for GATT db and client

[Luiz Augusto von Dentz]

Hi Arman,

On Wed, Dec 17, 2014 at 4:07 AM, Arman Uguray <armansito@chromium.org> wrote:
> This patch adds btd_device_get_gatt_db and btd_device_get_gatt_client
> functions.
> ---
>  src/device.c | 16 ++++++++++++++++
>  src/device.h |  2 ++
>  2 files changed, 18 insertions(+)
>
> diff --git a/src/device.c b/src/device.c
> index 64591d0..854d9f3 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -4844,6 +4844,22 @@ GSList *btd_device_get_primaries(struct btd_device *device)
>         return device->primaries;
>  }
>
> +struct gatt_db *btd_device_get_gatt_db(struct btd_device *device)
> +{
> +       if (!device)
> +               return NULL;
> +
> +       return device->db;
> +}

I guess the db can be get from the gatt_client, also perhaps we should
return a reference otherwise it should be const.

> +struct bt_gatt_client *btd_device_get_gatt_client(struct btd_device *device)
> +{
> +       if (!device)
> +               return NULL;
> +
> +       return device->client;
> +}
> +
>  void btd_device_gatt_set_service_changed(struct btd_device *device,
>                                                 uint16_t start, uint16_t end)
>  {
> diff --git a/src/device.h b/src/device.h
> index 487bd27..a7fefee 100644
> --- a/src/device.h
> +++ b/src/device.h
> @@ -67,6 +67,8 @@ 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);
> +struct gatt_db *btd_device_get_gatt_db(struct btd_device *device);
> +struct bt_gatt_client *btd_device_get_gatt_client(struct btd_device *device);
>  void btd_device_gatt_set_service_changed(struct btd_device *device,
>                                                 uint16_t start, uint16_t end);
>  bool device_attach_att(struct btd_device *dev, GIOChannel *io);
> --
> 2.2.0.rc0.207.ga3a616c
>
> --
> 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 v4 07/10] core: device: Make profile calls in GATT events

[Luiz Augusto von Dentz]

Hi Arman,

On Wed, Dec 17, 2014 at 4:07 AM, Arman Uguray <armansito@chromium.org> wrote:
> This patch correctly integrates all profile calls into the various GATT
> client events (ready, service-added, service-removed) so that profiles
> can perform the necessary updates. The major changes introduced in this
> this patch are:
>
>   1. Added new profile probe/remove functions for GATT services, which
>      operate on a btd_device's client db and initialize btd_service
>      instances with start & end handles:
>         - device_probe_gatt_profiles
>         - device_probe_gatt_profile
>         - device_remove_gatt_profile
>         - device_accept_gatt_profiles
>
>   2. device_probe_gatt_profiles is called when the gatt-client becomes
>      ready. Profiles are then notified with the "accept" call.
>
>   3. device_probe_gatt_profile is called when a new GATT service is
>      added to the db.
>
>   4. device_remove_gatt_profile is called when a GATT service is removed
>      from the db.
>
>   5. Profiles are notified that the gatt-client is ready via the
>      btd_service "accept" function on a per-service basis. Currently this
>      is always called immediately after a probe.

After probe? I thought the idea was to indicate that a connection
becomes available, if it is called after probe that means it is called
just once?

> ---
>  src/device.c | 261 ++++++++++++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 212 insertions(+), 49 deletions(-)
>
> diff --git a/src/device.c b/src/device.c
> index 854d9f3..6242c76 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -292,6 +292,27 @@ static GSList *find_service_with_state(GSList *list,
>         return NULL;
>  }
>
> +static GSList *find_service_with_gatt_handles(GSList *list,
> +                                                       uint16_t start_handle,
> +                                                       uint16_t end_handle)
> +{
> +       GSList *l;
> +       uint16_t svc_start, svc_end;
> +
> +       for (l = list; l != NULL; l = g_slist_next(l)) {
> +               struct btd_service *service = l->data;
> +
> +               if (!btd_service_get_gatt_handles(service, &svc_start,
> +                                                               &svc_end))
> +                       continue;
> +
> +               if (svc_start == start_handle && svc_end == end_handle)
> +                       return l;
> +       }
> +
> +       return NULL;
> +}
> +
>  static void update_technologies(GKeyFile *file, struct btd_device *dev)
>  {
>         const char *list[2];
> @@ -2390,15 +2411,168 @@ static void add_primary(struct gatt_db_attribute *attr, void *user_data)
>         *new_services = g_slist_append(*new_services, prim);
>  }
>
> +static void device_add_uuids(struct btd_device *device, GSList *uuids)
> +{
> +       GSList *l;
> +
> +       for (l = uuids; l != NULL; l = g_slist_next(l)) {
> +               GSList *match = g_slist_find_custom(device->uuids, l->data,
> +                                                       bt_uuid_strcmp);
> +               if (match)
> +                       continue;
> +
> +               device->uuids = g_slist_insert_sorted(device->uuids,
> +                                               g_strdup(l->data),
> +                                               bt_uuid_strcmp);
> +       }
> +
> +       g_dbus_emit_property_changed(dbus_conn, device->path,
> +                                               DEVICE_INTERFACE, "UUIDs");
> +}
> +
>  static void store_services(struct btd_device *device);
>
> +struct gatt_probe_data {
> +       struct btd_device *dev;
> +       bool add_uuid_to_device;
> +       GSList *uuids;
> +       struct gatt_db_attribute *cur_attr;
> +       char cur_uuid[MAX_LEN_UUID_STR];
> +       uint16_t start_handle, end_handle;
> +};
> +
> +static bool device_match_profile(struct btd_device *device,
> +                                       struct btd_profile *profile,
> +                                       GSList *uuids)
> +{
> +       if (profile->remote_uuid == NULL)
> +               return false;
> +
> +       if (g_slist_find_custom(uuids, profile->remote_uuid,
> +                                                       bt_uuid_strcmp) == NULL)
> +               return false;
> +
> +       return true;
> +}
> +
> +static void dev_probe_gatt(struct btd_profile *p, void *user_data)
> +{
> +       struct gatt_probe_data *data = user_data;
> +       struct btd_service *service;
> +
> +       if (p->device_probe == NULL)
> +               return;
> +
> +       if (!device_match_profile(data->dev, p, g_slist_last(data->uuids)))
> +               return;

Using g_slist_last is not very smart since afaik GSList has to iterate
to to access the last.

> +
> +       service = service_create_gatt(data->dev, p, data->start_handle,
> +                                                       data->end_handle);
> +       if (!service)
> +               return;
> +
> +       if (service_probe(service) < 0) {
> +               btd_service_unref(service);
> +               return;
> +       }
> +
> +       data->dev->services = g_slist_append(data->dev->services, service);
> +}
> +
> +static void dev_probe_gatt_profile(struct gatt_db_attribute *attr,
> +                                                       void *user_data)
> +{
> +       struct gatt_probe_data *data = user_data;
> +       bt_uuid_t uuid;
> +       GSList *l;
> +
> +       gatt_db_attribute_get_service_data(attr, &data->start_handle,
> +                                                       &data->end_handle, NULL,
> +                                                       &uuid);
> +       bt_uuid_to_string(&uuid, data->cur_uuid, sizeof(data->cur_uuid));
> +
> +       data->uuids = g_slist_append(data->uuids, g_strdup(data->cur_uuid));

If the order in the list is not really important than use prepend and
g_slist_first in dev_probe_gatt, actually do we even need this list
since we are adding one by one the UUIDs?

> +       data->cur_attr = attr;
> +
> +       btd_profile_foreach(dev_probe_gatt, data);
> +
> +       if (!data->add_uuid_to_device)
> +               return;
> +
> +       l = g_slist_append(l, g_strdup(data->cur_uuid));
> +       device_add_uuids(data->dev, l);
> +}
> +
> +static void device_probe_gatt_profile(struct btd_device *device,
> +                                               struct gatt_db_attribute *attr)
> +{
> +       struct gatt_probe_data data;
> +
> +       memset(&data, 0, sizeof(data));
> +
> +       data.dev = device;
> +       data.add_uuid_to_device = true;
> +
> +       dev_probe_gatt_profile(attr, &data);
> +       g_slist_free_full(data.uuids, g_free);
> +}
> +
> +static void device_probe_gatt_profiles(struct btd_device *device)
> +{
> +       struct gatt_probe_data data;
> +       char addr[18];
> +
> +       ba2str(&device->bdaddr, addr);
> +
> +       if (device->blocked) {
> +               DBG("Skipping profiles for blocked device %s", addr);
> +               return;
> +       }
> +
> +       memset(&data, 0, sizeof(data));
> +
> +       data.dev = device;
> +
> +       gatt_db_foreach_service(device->db, NULL, dev_probe_gatt_profile,
> +                                                                       &data);
> +       device_add_uuids(device, data.uuids);
> +       g_slist_free_full(data.uuids, g_free);
> +}
> +
> +static void device_accept_gatt_profiles(struct btd_device *device)
> +{
> +       GSList *l;
> +
> +       for (l = device->services; l != NULL; l = g_slist_next(l))
> +               service_accept(l->data);
> +}
> +
> +static void device_remove_gatt_profile(struct btd_device *device,
> +                                               struct gatt_db_attribute *attr)
> +{
> +       uint16_t start, end;
> +       struct btd_service *service;
> +       GSList *l;
> +
> +       gatt_db_attribute_get_service_handles(attr, &start, &end);
> +
> +       l = find_service_with_gatt_handles(device->services, start, end);
> +       if (!l)
> +               return;
> +
> +       service = l->data;
> +       device->services = g_slist_delete_link(device->services, l);
> +       device->pending = g_slist_remove(device->pending, service);
> +       service_remove(service);
> +}
> +
>  static void gatt_service_added(struct gatt_db_attribute *attr, void *user_data)
>  {
>         struct btd_device *device = user_data;
> -       struct gatt_primary *prim;
>         GSList *new_service = NULL;
> -       GSList *profiles_added = NULL;
>         uint16_t start, end;
> +       GSList *l;
>
>         if (!bt_gatt_client_is_ready(device->client))
>                 return;
> @@ -2417,14 +2591,13 @@ static void gatt_service_added(struct gatt_db_attribute *attr, void *user_data)
>
>         device_register_primaries(device, new_service, -1);
>
> -       prim = new_service->data;
> -       profiles_added = g_slist_append(profiles_added, g_strdup(prim->uuid));
> +       device_probe_gatt_profile(device, attr);
>
> -       device_probe_profiles(device, profiles_added);
> +       l = find_service_with_gatt_handles(device->services, start, end);
> +       if (l)
> +               service_accept(l->data);
>
>         store_services(device);
> -
> -       g_slist_free_full(profiles_added, g_free);
>  }
>
>  static gint prim_attr_cmp(gconstpointer a, gconstpointer b)
> @@ -2438,6 +2611,14 @@ static gint prim_attr_cmp(gconstpointer a, gconstpointer b)
>         return !(prim->range.start == start && prim->range.end == end);
>  }
>
> +static gint prim_uuid_cmp(gconstpointer a, gconstpointer b)
> +{
> +       const struct gatt_primary *prim = a;
> +       const char *uuid = b;
> +
> +       return bt_uuid_strcmp(prim->uuid, uuid);
> +}
> +
>  static void gatt_service_removed(struct gatt_db_attribute *attr,
>                                                                 void *user_data)
>  {
> @@ -2461,20 +2642,24 @@ static void gatt_service_removed(struct gatt_db_attribute *attr,
>         prim = l->data;
>         device->primaries = g_slist_delete_link(device->primaries, l);
>
> -       /* Remove the corresponding UUIDs entry */
> -       l = g_slist_find_custom(device->uuids, prim->uuid, bt_uuid_strcmp);
> -       device->uuids = g_slist_delete_link(device->uuids, l);
> -       g_free(prim);
> -
> -       store_services(device);
> +       device_remove_gatt_profile(device, attr);
>
>         /*
> -        * TODO: Notify the profiles somehow. It may be sufficient for each
> -        * profile to register a service_removed handler.
> +        * Remove the corresponding UUIDs entry, only if this is the last
> +        * service with this UUID.
>          */
> +       l = g_slist_find_custom(device->uuids, prim->uuid, bt_uuid_strcmp);
>
> -       g_dbus_emit_property_changed(dbus_conn, device->path,
> +       if (!g_slist_find_custom(device->primaries, prim->uuid,
> +                                                       prim_uuid_cmp)) {
> +               device->uuids = g_slist_delete_link(device->uuids, l);
> +               g_dbus_emit_property_changed(dbus_conn, device->path,
>                                                 DEVICE_INTERFACE, "UUIDs");
> +       }
> +
> +       g_free(prim);
> +
> +       store_services(device);
>  }
>
>  static struct btd_device *device_new(struct btd_adapter *adapter,
> @@ -2975,20 +3160,6 @@ GSList *btd_device_get_uuids(struct btd_device *device)
>         return device->uuids;
>  }
>
> -static bool device_match_profile(struct btd_device *device,
> -                                       struct btd_profile *profile,
> -                                       GSList *uuids)
> -{
> -       if (profile->remote_uuid == NULL)
> -               return false;
> -
> -       if (g_slist_find_custom(uuids, profile->remote_uuid,
> -                                                       bt_uuid_strcmp) == NULL)
> -               return false;
> -
> -       return true;
> -}
> -
>  struct probe_data {
>         struct btd_device *dev;
>         GSList *uuids;
> @@ -3065,7 +3236,6 @@ void device_remove_profile(gpointer a, gpointer b)
>  void device_probe_profiles(struct btd_device *device, GSList *uuids)
>  {
>         struct probe_data d = { device, uuids };
> -       GSList *l;
>         char addr[18];
>
>         ba2str(&device->bdaddr, addr);
> @@ -3080,19 +3250,7 @@ void device_probe_profiles(struct btd_device *device, GSList *uuids)
>         btd_profile_foreach(dev_probe, &d);
>
>  add_uuids:
> -       for (l = uuids; l != NULL; l = g_slist_next(l)) {
> -               GSList *match = g_slist_find_custom(device->uuids, l->data,
> -                                                       bt_uuid_strcmp);
> -               if (match)
> -                       continue;
> -
> -               device->uuids = g_slist_insert_sorted(device->uuids,
> -                                               g_strdup(l->data),
> -                                               bt_uuid_strcmp);
> -       }
> -
> -       g_dbus_emit_property_changed(dbus_conn, device->path,
> -                                               DEVICE_INTERFACE, "UUIDs");
> +       device_add_uuids(device, uuids);
>  }
>
>  static void store_sdp_record(GKeyFile *key_file, sdp_record_t *rec)
> @@ -3412,6 +3570,13 @@ static void search_cb(sdp_list_t *recs, int err, gpointer user_data)
>         if (primaries)
>                 device_register_primaries(device, primaries, ATT_PSM);
>
> +       /*
> +        * TODO: The btd_service instances for GATT services need to be
> +        * initialized with the service handles. Eventually this code should
> +        * perform ATT protocol service discovery over the ATT PSM to obtain
> +        * the full list of services and populate a client-role gatt_db over
> +        * BR/EDR.
> +        */
>         device_probe_profiles(device, req->profiles_added);
>
>         /* Propagate services changes */
> @@ -3640,7 +3805,7 @@ static void register_gatt_services(struct browse_req *req)
>
>         device_register_primaries(device, services, -1);
>
> -       device_probe_profiles(device, req->profiles_added);
> +       device_probe_gatt_profiles(device);
>
>         device_svc_resolved(device, device->bdaddr_type, 0);
>
> @@ -3675,10 +3840,8 @@ 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.
> -        */
> +       device_accept_gatt_profiles(device);
> +
>         g_slist_foreach(device->attios, attio_connected, device->attrib);
>  }
>
> --
> 2.2.0.rc0.207.ga3a616c
>
> --
> 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 v4 05/10] core: device: Don't disconnect if attios not set

[Luiz Augusto von Dentz]

Hi Arman,

On Wed, Dec 17, 2014 at 4:07 AM, Arman Uguray <armansito@chromium.org> wrote:
> Currently there is no way for external applications to claim ownership
> on a GATT connection and as profiles move away from attio.h it doesn't
> make much sense to immediately disconnect if no attio callbacks have
> been set after probing the profiles (as this will always immediately
> disconnect the device). This patch removes this logic from btd_device.

I was thinking that if no driver accepts the connection we should
disconnect but it doesn't look like you had done anything in this
direction, for external client using the D-Bus API maybe we need some
method for registering client making only the services with clients
visible.

> ---
>  src/device.c | 8 --------
>  1 file changed, 8 deletions(-)
>
> diff --git a/src/device.c b/src/device.c
> index e8bdc18..64591d0 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -3642,9 +3642,6 @@ static void register_gatt_services(struct browse_req *req)
>
>         device_probe_profiles(device, req->profiles_added);
>
> -       if (device->attios == NULL && device->attios_offline == NULL)
> -               attio_cleanup(device);
> -
>         device_svc_resolved(device, device->bdaddr_type, 0);
>
>         store_services(device);
> @@ -5069,11 +5066,6 @@ gboolean btd_device_remove_attio_callback(struct btd_device *device, guint id)
>
>         g_free(attio);
>
> -       if (device->attios != NULL || device->attios_offline != NULL)
> -               return TRUE;
> -
> -       attio_cleanup(device);
> -
>         return TRUE;
>  }
>
> --
> 2.2.0.rc0.207.ga3a616c
>
> --
> 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 v4 00/10] core: Use shared/gatt-client

[Luiz Augusto von Dentz]

Hi Arman,

On Wed, Dec 17, 2014 at 4:07 AM, Arman Uguray <armansito@chromium.org> wrote:
> *v4:
>   - Removed gatt-callbacks in favor of using btd_profile probe and remove
>     functions. Added the new btd_profile function "accept" to notify profiles
>     that gatt-client is ready.
>
>   - Using device_remove_profile to unregister btd_service's when services are
>     removed from the client database. This happens on Service Changed events and
>     on disconnection if the device is not bonded.
>
>   - Removed immediated disconnection logic that triggers when no attio callbacks
>     are registered after probe.
>
>   - Other minor fixes to shared/gatt and shared/att.
>
> *v3:
>   - Renamed device_attach_att_transport to device_attach_att.
>
> *v2:
>   - Rebased.
>
> *v1:
>   - Addressed comments by Johan and Luiz.
>
>
> 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 (10):
>   shared/att: Guard against invalid ref in callbacks
>   shared/gatt-client: Guard against invalid access
>   core: service: Add start & end handle fields
>   core: profile: Add accept function
>   core: device: Don't disconnect if attios not set
>   core: device: Add getters for GATT db and client
>   core: device: Make profile calls in GATT events
>   profiles/gatt: Don't handle GATT service.
>   profiles/gatt: Rename profile to gap.
>   profiles/gap: Rewrite using bt_gatt_client.
>
>  Makefile.plugins         |   4 +-
>  profiles/gap/gas.c       | 338 +++++++++++++++++++++++++++++++++++
>  profiles/gatt/gas.c      | 457 -----------------------------------------------
>  src/device.c             | 285 +++++++++++++++++++++++------
>  src/device.h             |   2 +
>  src/profile.h            |   2 +
>  src/service.c            |  56 ++++++
>  src/service.h            |   9 +
>  src/shared/att.c         |   5 +
>  src/shared/gatt-client.c |  34 ++--
>  10 files changed, 663 insertions(+), 529 deletions(-)
>  create mode 100644 profiles/gap/gas.c
>  delete mode 100644 profiles/gatt/gas.c
>
> --
> 2.2.0.rc0.207.ga3a616c

Ive applied 1-4, please check the comments for the remaining set.


-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ v4 05/10] core: device: Don't disconnect if attios not set

[Arman Uguray]

Hi Luiz,

> On Wed, Dec 17, 2014 at 5:08 AM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
> Hi Arman,
>
> On Wed, Dec 17, 2014 at 4:07 AM, Arman Uguray <armansito@chromium.org> wrote:
>> Currently there is no way for external applications to claim ownership
>> on a GATT connection and as profiles move away from attio.h it doesn't
>> make much sense to immediately disconnect if no attio callbacks have
>> been set after probing the profiles (as this will always immediately
>> disconnect the device). This patch removes this logic from btd_device.
>
> I was thinking that if no driver accepts the connection we should
> disconnect but it doesn't look like you had done anything in this
> direction, for external client using the D-Bus API maybe we need some
> method for registering client making only the services with clients
> visible.
>

I discussed how to make this possible for external applications on the
mailing list with Marcel and Johan by extending the Profile API
concept to GATT (org.bluez.GattProfile1 or sorts) but this will only
happen down the line.

For profiles, maybe btd_device can choose to disconnect if no profile
returned success during probing? Then again, we end up having a
mundane profile like GAP behave like the rest of the profiles this
way.

For now, I figured we can remove this logic and keep the device
connected (since the user called Connect, it seems reasonable to make
the assumption that they want the device to be connected). In the
future, we'll want to revisit this and perhaps combine btd_profile and
GattProfile1 state to determine auto-connect, keeping the connection
alive, and so on.

Though initially I still want to have the basic doc/gatt-api.txt
implemented before we go down the GattProfile path. I'm open to
suggestions in general though.

>> ---
>>  src/device.c | 8 --------
>>  1 file changed, 8 deletions(-)
>>
>> diff --git a/src/device.c b/src/device.c
>> index e8bdc18..64591d0 100644
>> --- a/src/device.c
>> +++ b/src/device.c
>> @@ -3642,9 +3642,6 @@ static void register_gatt_services(struct browse_req *req)
>>
>>         device_probe_profiles(device, req->profiles_added);
>>
>> -       if (device->attios == NULL && device->attios_offline == NULL)
>> -               attio_cleanup(device);
>> -
>>         device_svc_resolved(device, device->bdaddr_type, 0);
>>
>>         store_services(device);
>> @@ -5069,11 +5066,6 @@ gboolean btd_device_remove_attio_callback(struct btd_device *device, guint id)
>>
>>         g_free(attio);
>>
>> -       if (device->attios != NULL || device->attios_offline != NULL)
>> -               return TRUE;
>> -
>> -       attio_cleanup(device);
>> -
>>         return TRUE;
>>  }
>>
>> --
>> 2.2.0.rc0.207.ga3a616c
>>
>> --
>> 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

Arman

Re: [PATCH BlueZ v4 06/10] core: device: Add getters for GATT db and client

[Arman Uguray]

Hi Luiz,

> On Wed, Dec 17, 2014 at 4:39 AM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
> Hi Arman,
>
> On Wed, Dec 17, 2014 at 4:07 AM, Arman Uguray <armansito@chromium.org> wrote:
>> This patch adds btd_device_get_gatt_db and btd_device_get_gatt_client
>> functions.
>> ---
>>  src/device.c | 16 ++++++++++++++++
>>  src/device.h |  2 ++
>>  2 files changed, 18 insertions(+)
>>
>> diff --git a/src/device.c b/src/device.c
>> index 64591d0..854d9f3 100644
>> --- a/src/device.c
>> +++ b/src/device.c
>> @@ -4844,6 +4844,22 @@ GSList *btd_device_get_primaries(struct btd_device *device)
>>         return device->primaries;
>>  }
>>
>> +struct gatt_db *btd_device_get_gatt_db(struct btd_device *device)
>> +{
>> +       if (!device)
>> +               return NULL;
>> +
>> +       return device->db;
>> +}
>
> I guess the db can be get from the gatt_client, also perhaps we should
> return a reference otherwise it should be const.
>

I think I'll keep this getter but make things const. My idea here is
that the gatt_db will outlive the connection and if the device is
bonded it won't get cleared. So it might still be useful.

>> +struct bt_gatt_client *btd_device_get_gatt_client(struct btd_device *device)
>> +{
>> +       if (!device)
>> +               return NULL;
>> +
>> +       return device->client;
>> +}
>> +
>>  void btd_device_gatt_set_service_changed(struct btd_device *device,
>>                                                 uint16_t start, uint16_t end)
>>  {
>> diff --git a/src/device.h b/src/device.h
>> index 487bd27..a7fefee 100644
>> --- a/src/device.h
>> +++ b/src/device.h
>> @@ -67,6 +67,8 @@ 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);
>> +struct gatt_db *btd_device_get_gatt_db(struct btd_device *device);
>> +struct bt_gatt_client *btd_device_get_gatt_client(struct btd_device *device);
>>  void btd_device_gatt_set_service_changed(struct btd_device *device,
>>                                                 uint16_t start, uint16_t end);
>>  bool device_attach_att(struct btd_device *dev, GIOChannel *io);
>> --
>> 2.2.0.rc0.207.ga3a616c
>>
>> --
>> 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

Arman

Re: [PATCH BlueZ v4 07/10] core: device: Make profile calls in GATT events

[Arman Uguray]

Hi Luiz,

> On Wed, Dec 17, 2014 at 4:57 AM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
> Hi Arman,
>
> On Wed, Dec 17, 2014 at 4:07 AM, Arman Uguray <armansito@chromium.org> wrote:
>> This patch correctly integrates all profile calls into the various GATT
>> client events (ready, service-added, service-removed) so that profiles
>> can perform the necessary updates. The major changes introduced in this
>> this patch are:
>>
>>   1. Added new profile probe/remove functions for GATT services, which
>>      operate on a btd_device's client db and initialize btd_service
>>      instances with start & end handles:
>>         - device_probe_gatt_profiles
>>         - device_probe_gatt_profile
>>         - device_remove_gatt_profile
>>         - device_accept_gatt_profiles
>>
>>   2. device_probe_gatt_profiles is called when the gatt-client becomes
>>      ready. Profiles are then notified with the "accept" call.
>>
>>   3. device_probe_gatt_profile is called when a new GATT service is
>>      added to the db.
>>
>>   4. device_remove_gatt_profile is called when a GATT service is removed
>>      from the db.
>>
>>   5. Profiles are notified that the gatt-client is ready via the
>>      btd_service "accept" function on a per-service basis. Currently this
>>      is always called immediately after a probe.
>
> After probe? I thought the idea was to indicate that a connection
> becomes available, if it is called after probe that means it is called
> just once?
>

At least the way I did it is I call probe every time the gatt-client
becomes ready and, from then on, when a new service is added. If the
gatt-client is ready at that time, I also call accept. My idea here
was that if the device is bonded and we have a live cache, then
profiles will be probed beforehand and then receive the accept call
later when there's a connection and gatt-client becomes ready.

Does this behavior make sense or did you have something different in mind?

>> ---
>>  src/device.c | 261 ++++++++++++++++++++++++++++++++++++++++++++++++-----------
>>  1 file changed, 212 insertions(+), 49 deletions(-)
>>
>> diff --git a/src/device.c b/src/device.c
>> index 854d9f3..6242c76 100644
>> --- a/src/device.c
>> +++ b/src/device.c
>> @@ -292,6 +292,27 @@ static GSList *find_service_with_state(GSList *list,
>>         return NULL;
>>  }
>>
>> +static GSList *find_service_with_gatt_handles(GSList *list,
>> +                                                       uint16_t start_handle,
>> +                                                       uint16_t end_handle)
>> +{
>> +       GSList *l;
>> +       uint16_t svc_start, svc_end;
>> +
>> +       for (l = list; l != NULL; l = g_slist_next(l)) {
>> +               struct btd_service *service = l->data;
>> +
>> +               if (!btd_service_get_gatt_handles(service, &svc_start,
>> +                                                               &svc_end))
>> +                       continue;
>> +
>> +               if (svc_start == start_handle && svc_end == end_handle)
>> +                       return l;
>> +       }
>> +
>> +       return NULL;
>> +}
>> +
>>  static void update_technologies(GKeyFile *file, struct btd_device *dev)
>>  {
>>         const char *list[2];
>> @@ -2390,15 +2411,168 @@ static void add_primary(struct gatt_db_attribute *attr, void *user_data)
>>         *new_services = g_slist_append(*new_services, prim);
>>  }
>>
>> +static void device_add_uuids(struct btd_device *device, GSList *uuids)
>> +{
>> +       GSList *l;
>> +
>> +       for (l = uuids; l != NULL; l = g_slist_next(l)) {
>> +               GSList *match = g_slist_find_custom(device->uuids, l->data,
>> +                                                       bt_uuid_strcmp);
>> +               if (match)
>> +                       continue;
>> +
>> +               device->uuids = g_slist_insert_sorted(device->uuids,
>> +                                               g_strdup(l->data),
>> +                                               bt_uuid_strcmp);
>> +       }
>> +
>> +       g_dbus_emit_property_changed(dbus_conn, device->path,
>> +                                               DEVICE_INTERFACE, "UUIDs");
>> +}
>> +
>>  static void store_services(struct btd_device *device);
>>
>> +struct gatt_probe_data {
>> +       struct btd_device *dev;
>> +       bool add_uuid_to_device;
>> +       GSList *uuids;
>> +       struct gatt_db_attribute *cur_attr;
>> +       char cur_uuid[MAX_LEN_UUID_STR];
>> +       uint16_t start_handle, end_handle;
>> +};
>> +
>> +static bool device_match_profile(struct btd_device *device,
>> +                                       struct btd_profile *profile,
>> +                                       GSList *uuids)
>> +{
>> +       if (profile->remote_uuid == NULL)
>> +               return false;
>> +
>> +       if (g_slist_find_custom(uuids, profile->remote_uuid,
>> +                                                       bt_uuid_strcmp) == NULL)
>> +               return false;
>> +
>> +       return true;
>> +}
>> +
>> +static void dev_probe_gatt(struct btd_profile *p, void *user_data)
>> +{
>> +       struct gatt_probe_data *data = user_data;
>> +       struct btd_service *service;
>> +
>> +       if (p->device_probe == NULL)
>> +               return;
>> +
>> +       if (!device_match_profile(data->dev, p, g_slist_last(data->uuids)))
>> +               return;
>
> Using g_slist_last is not very smart since afaik GSList has to iterate
> to to access the last.
>

I didn't realize that. I'll fix this in v5.

>> +
>> +       service = service_create_gatt(data->dev, p, data->start_handle,
>> +                                                       data->end_handle);
>> +       if (!service)
>> +               return;
>> +
>> +       if (service_probe(service) < 0) {
>> +               btd_service_unref(service);
>> +               return;
>> +       }
>> +
>> +       data->dev->services = g_slist_append(data->dev->services, service);
>> +}
>> +
>> +static void dev_probe_gatt_profile(struct gatt_db_attribute *attr,
>> +                                                       void *user_data)
>> +{
>> +       struct gatt_probe_data *data = user_data;
>> +       bt_uuid_t uuid;
>> +       GSList *l;
>> +
>> +       gatt_db_attribute_get_service_data(attr, &data->start_handle,
>> +                                                       &data->end_handle, NULL,
>> +                                                       &uuid);
>> +       bt_uuid_to_string(&uuid, data->cur_uuid, sizeof(data->cur_uuid));
>> +
>> +       data->uuids = g_slist_append(data->uuids, g_strdup(data->cur_uuid));
>
> If the order in the list is not really important than use prepend and
> g_slist_first in dev_probe_gatt, actually do we even need this list
> since we are adding one by one the UUIDs?
>

We probably may not need it in this case. The list is only needed when
we call device_probe_gatt_profiles so that the UUIDs property can be
updated all at once. So I can conditionally populate this based on
data->add_uuid_to_device.

>> +       data->cur_attr = attr;
>> +
>> +       btd_profile_foreach(dev_probe_gatt, data);
>> +
>> +       if (!data->add_uuid_to_device)
>> +               return;
>> +
>> +       l = g_slist_append(l, g_strdup(data->cur_uuid));
>> +       device_add_uuids(data->dev, l);
>> +}
>> +
>> +static void device_probe_gatt_profile(struct btd_device *device,
>> +                                               struct gatt_db_attribute *attr)
>> +{
>> +       struct gatt_probe_data data;
>> +
>> +       memset(&data, 0, sizeof(data));
>> +
>> +       data.dev = device;
>> +       data.add_uuid_to_device = true;
>> +
>> +       dev_probe_gatt_profile(attr, &data);
>> +       g_slist_free_full(data.uuids, g_free);
>> +}
>> +
>> +static void device_probe_gatt_profiles(struct btd_device *device)
>> +{
>> +       struct gatt_probe_data data;
>> +       char addr[18];
>> +
>> +       ba2str(&device->bdaddr, addr);
>> +
>> +       if (device->blocked) {
>> +               DBG("Skipping profiles for blocked device %s", addr);
>> +               return;
>> +       }
>> +
>> +       memset(&data, 0, sizeof(data));
>> +
>> +       data.dev = device;
>> +
>> +       gatt_db_foreach_service(device->db, NULL, dev_probe_gatt_profile,
>> +                                                                       &data);
>> +       device_add_uuids(device, data.uuids);
>> +       g_slist_free_full(data.uuids, g_free);
>> +}
>> +
>> +static void device_accept_gatt_profiles(struct btd_device *device)
>> +{
>> +       GSList *l;
>> +
>> +       for (l = device->services; l != NULL; l = g_slist_next(l))
>> +               service_accept(l->data);
>> +}
>> +
>> +static void device_remove_gatt_profile(struct btd_device *device,
>> +                                               struct gatt_db_attribute *attr)
>> +{
>> +       uint16_t start, end;
>> +       struct btd_service *service;
>> +       GSList *l;
>> +
>> +       gatt_db_attribute_get_service_handles(attr, &start, &end);
>> +
>> +       l = find_service_with_gatt_handles(device->services, start, end);
>> +       if (!l)
>> +               return;
>> +
>> +       service = l->data;
>> +       device->services = g_slist_delete_link(device->services, l);
>> +       device->pending = g_slist_remove(device->pending, service);
>> +       service_remove(service);
>> +}
>> +
>>  static void gatt_service_added(struct gatt_db_attribute *attr, void *user_data)
>>  {
>>         struct btd_device *device = user_data;
>> -       struct gatt_primary *prim;
>>         GSList *new_service = NULL;
>> -       GSList *profiles_added = NULL;
>>         uint16_t start, end;
>> +       GSList *l;
>>
>>         if (!bt_gatt_client_is_ready(device->client))
>>                 return;
>> @@ -2417,14 +2591,13 @@ static void gatt_service_added(struct gatt_db_attribute *attr, void *user_data)
>>
>>         device_register_primaries(device, new_service, -1);
>>
>> -       prim = new_service->data;
>> -       profiles_added = g_slist_append(profiles_added, g_strdup(prim->uuid));
>> +       device_probe_gatt_profile(device, attr);
>>
>> -       device_probe_profiles(device, profiles_added);
>> +       l = find_service_with_gatt_handles(device->services, start, end);
>> +       if (l)
>> +               service_accept(l->data);
>>
>>         store_services(device);
>> -
>> -       g_slist_free_full(profiles_added, g_free);
>>  }
>>
>>  static gint prim_attr_cmp(gconstpointer a, gconstpointer b)
>> @@ -2438,6 +2611,14 @@ static gint prim_attr_cmp(gconstpointer a, gconstpointer b)
>>         return !(prim->range.start == start && prim->range.end == end);
>>  }
>>
>> +static gint prim_uuid_cmp(gconstpointer a, gconstpointer b)
>> +{
>> +       const struct gatt_primary *prim = a;
>> +       const char *uuid = b;
>> +
>> +       return bt_uuid_strcmp(prim->uuid, uuid);
>> +}
>> +
>>  static void gatt_service_removed(struct gatt_db_attribute *attr,
>>                                                                 void *user_data)
>>  {
>> @@ -2461,20 +2642,24 @@ static void gatt_service_removed(struct gatt_db_attribute *attr,
>>         prim = l->data;
>>         device->primaries = g_slist_delete_link(device->primaries, l);
>>
>> -       /* Remove the corresponding UUIDs entry */
>> -       l = g_slist_find_custom(device->uuids, prim->uuid, bt_uuid_strcmp);
>> -       device->uuids = g_slist_delete_link(device->uuids, l);
>> -       g_free(prim);
>> -
>> -       store_services(device);
>> +       device_remove_gatt_profile(device, attr);
>>
>>         /*
>> -        * TODO: Notify the profiles somehow. It may be sufficient for each
>> -        * profile to register a service_removed handler.
>> +        * Remove the corresponding UUIDs entry, only if this is the last
>> +        * service with this UUID.
>>          */
>> +       l = g_slist_find_custom(device->uuids, prim->uuid, bt_uuid_strcmp);
>>
>> -       g_dbus_emit_property_changed(dbus_conn, device->path,
>> +       if (!g_slist_find_custom(device->primaries, prim->uuid,
>> +                                                       prim_uuid_cmp)) {
>> +               device->uuids = g_slist_delete_link(device->uuids, l);
>> +               g_dbus_emit_property_changed(dbus_conn, device->path,
>>                                                 DEVICE_INTERFACE, "UUIDs");
>> +       }
>> +
>> +       g_free(prim);
>> +
>> +       store_services(device);
>>  }
>>
>>  static struct btd_device *device_new(struct btd_adapter *adapter,
>> @@ -2975,20 +3160,6 @@ GSList *btd_device_get_uuids(struct btd_device *device)
>>         return device->uuids;
>>  }
>>
>> -static bool device_match_profile(struct btd_device *device,
>> -                                       struct btd_profile *profile,
>> -                                       GSList *uuids)
>> -{
>> -       if (profile->remote_uuid == NULL)
>> -               return false;
>> -
>> -       if (g_slist_find_custom(uuids, profile->remote_uuid,
>> -                                                       bt_uuid_strcmp) == NULL)
>> -               return false;
>> -
>> -       return true;
>> -}
>> -
>>  struct probe_data {
>>         struct btd_device *dev;
>>         GSList *uuids;
>> @@ -3065,7 +3236,6 @@ void device_remove_profile(gpointer a, gpointer b)
>>  void device_probe_profiles(struct btd_device *device, GSList *uuids)
>>  {
>>         struct probe_data d = { device, uuids };
>> -       GSList *l;
>>         char addr[18];
>>
>>         ba2str(&device->bdaddr, addr);
>> @@ -3080,19 +3250,7 @@ void device_probe_profiles(struct btd_device *device, GSList *uuids)
>>         btd_profile_foreach(dev_probe, &d);
>>
>>  add_uuids:
>> -       for (l = uuids; l != NULL; l = g_slist_next(l)) {
>> -               GSList *match = g_slist_find_custom(device->uuids, l->data,
>> -                                                       bt_uuid_strcmp);
>> -               if (match)
>> -                       continue;
>> -
>> -               device->uuids = g_slist_insert_sorted(device->uuids,
>> -                                               g_strdup(l->data),
>> -                                               bt_uuid_strcmp);
>> -       }
>> -
>> -       g_dbus_emit_property_changed(dbus_conn, device->path,
>> -                                               DEVICE_INTERFACE, "UUIDs");
>> +       device_add_uuids(device, uuids);
>>  }
>>
>>  static void store_sdp_record(GKeyFile *key_file, sdp_record_t *rec)
>> @@ -3412,6 +3570,13 @@ static void search_cb(sdp_list_t *recs, int err, gpointer user_data)
>>         if (primaries)
>>                 device_register_primaries(device, primaries, ATT_PSM);
>>
>> +       /*
>> +        * TODO: The btd_service instances for GATT services need to be
>> +        * initialized with the service handles. Eventually this code should
>> +        * perform ATT protocol service discovery over the ATT PSM to obtain
>> +        * the full list of services and populate a client-role gatt_db over
>> +        * BR/EDR.
>> +        */
>>         device_probe_profiles(device, req->profiles_added);
>>
>>         /* Propagate services changes */
>> @@ -3640,7 +3805,7 @@ static void register_gatt_services(struct browse_req *req)
>>
>>         device_register_primaries(device, services, -1);
>>
>> -       device_probe_profiles(device, req->profiles_added);
>> +       device_probe_gatt_profiles(device);
>>
>>         device_svc_resolved(device, device->bdaddr_type, 0);
>>
>> @@ -3675,10 +3840,8 @@ 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.
>> -        */
>> +       device_accept_gatt_profiles(device);
>> +
>>         g_slist_foreach(device->attios, attio_connected, device->attrib);
>>  }
>>
>> --
>> 2.2.0.rc0.207.ga3a616c
>>
>> --
>> 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 v4 05/10] core: device: Don't disconnect if attios not set

[Luiz Augusto von Dentz]

Hi Arman,

On Wed, Dec 17, 2014 at 3:42 PM, Arman Uguray <armansito@chromium.org> wrote:
> Hi Luiz,
>
>> On Wed, Dec 17, 2014 at 5:08 AM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>> Hi Arman,
>>
>> On Wed, Dec 17, 2014 at 4:07 AM, Arman Uguray <armansito@chromium.org> wrote:
>>> Currently there is no way for external applications to claim ownership
>>> on a GATT connection and as profiles move away from attio.h it doesn't
>>> make much sense to immediately disconnect if no attio callbacks have
>>> been set after probing the profiles (as this will always immediately
>>> disconnect the device). This patch removes this logic from btd_device.
>>
>> I was thinking that if no driver accepts the connection we should
>> disconnect but it doesn't look like you had done anything in this
>> direction, for external client using the D-Bus API maybe we need some
>> method for registering client making only the services with clients
>> visible.
>>
>
> I discussed how to make this possible for external applications on the
> mailing list with Marcel and Johan by extending the Profile API
> concept to GATT (org.bluez.GattProfile1 or sorts) but this will only
> happen down the line.
>
> For profiles, maybe btd_device can choose to disconnect if no profile
> returned success during probing? Then again, we end up having a
> mundane profile like GAP behave like the rest of the profiles this
> way.

Im not really following here, probing stage is to detect a driver can
be used and on success we create a service instance so it happen only
once, accept in the other hand would be triggered every time the
device connects. Anyway my comments was regarding the fact that we
need to detect if there are multiple clients for the same profile we
would need to do some conflict resolution, since we cannot track D-Bus
client right now I think it is better to not expose services which
have built-in driver such as HoG, etc, later once we introduce
org.bluez.GattProfile1 or similar we may favor external apps if that
makes more sense.

> For now, I figured we can remove this logic and keep the device
> connected (since the user called Connect, it seems reasonable to make
> the assumption that they want the device to be connected). In the
> future, we'll want to revisit this and perhaps combine btd_profile and
> GattProfile1 state to determine auto-connect, keeping the connection
> alive, and so on.

Well in case of classic if the user call Connect and there is no
driver matching/probed we don't attempt to connect at all, except to
update the services, but with LE it is much more common that we got
connected by the remote via advertising and passive scanning, thus it
would look like as an incoming connection, and then we could request
each probed driver to accept the connection.

> Though initially I still want to have the basic doc/gatt-api.txt
> implemented before we go down the GattProfile path. I'm open to
> suggestions in general though.

I guess it is fine to leave it for later but we should keep in mind
what we gonna do about conflicting clients, I figure this patch might
actually be necessary since we cannot track D-Bus clients we may keep
the connection even if no driver accepts it.

>
>>> ---
>>>  src/device.c | 8 --------
>>>  1 file changed, 8 deletions(-)
>>>
>>> diff --git a/src/device.c b/src/device.c
>>> index e8bdc18..64591d0 100644
>>> --- a/src/device.c
>>> +++ b/src/device.c
>>> @@ -3642,9 +3642,6 @@ static void register_gatt_services(struct browse_req *req)
>>>
>>>         device_probe_profiles(device, req->profiles_added);
>>>
>>> -       if (device->attios == NULL && device->attios_offline == NULL)
>>> -               attio_cleanup(device);
>>> -
>>>         device_svc_resolved(device, device->bdaddr_type, 0);
>>>
>>>         store_services(device);
>>> @@ -5069,11 +5066,6 @@ gboolean btd_device_remove_attio_callback(struct btd_device *device, guint id)
>>>
>>>         g_free(attio);
>>>
>>> -       if (device->attios != NULL || device->attios_offline != NULL)
>>> -               return TRUE;
>>> -
>>> -       attio_cleanup(device);
>>> -
>>>         return TRUE;
>>>  }
>>>
>>> --
>>> 2.2.0.rc0.207.ga3a616c
>>>
>>> --
>>> 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
>
> Arman



-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ v4 06/10] core: device: Add getters for GATT db and client

[Luiz Augusto von Dentz]

Hi Arman,

On Wed, Dec 17, 2014 at 3:45 PM, Arman Uguray <armansito@chromium.org> wrote:
> Hi Luiz,
>
>> On Wed, Dec 17, 2014 at 4:39 AM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>> Hi Arman,
>>
>> On Wed, Dec 17, 2014 at 4:07 AM, Arman Uguray <armansito@chromium.org> wrote:
>>> This patch adds btd_device_get_gatt_db and btd_device_get_gatt_client
>>> functions.
>>> ---
>>>  src/device.c | 16 ++++++++++++++++
>>>  src/device.h |  2 ++
>>>  2 files changed, 18 insertions(+)
>>>
>>> diff --git a/src/device.c b/src/device.c
>>> index 64591d0..854d9f3 100644
>>> --- a/src/device.c
>>> +++ b/src/device.c
>>> @@ -4844,6 +4844,22 @@ GSList *btd_device_get_primaries(struct btd_device *device)
>>>         return device->primaries;
>>>  }
>>>
>>> +struct gatt_db *btd_device_get_gatt_db(struct btd_device *device)
>>> +{
>>> +       if (!device)
>>> +               return NULL;
>>> +
>>> +       return device->db;
>>> +}
>>
>> I guess the db can be get from the gatt_client, also perhaps we should
>> return a reference otherwise it should be const.
>>
>
> I think I'll keep this getter but make things const. My idea here is
> that the gatt_db will outlive the connection and if the device is
> bonded it won't get cleared. So it might still be useful.

Sure no problem, Im just not sure if the driver would be interested in
the db without the client and as it is const it should not be possible
to register callbacks, actually const for the client perhaps is not a
good idea either for the same reason.

>>> +struct bt_gatt_client *btd_device_get_gatt_client(struct btd_device *device)
>>> +{
>>> +       if (!device)
>>> +               return NULL;
>>> +
>>> +       return device->client;
>>> +}
>>> +
>>>  void btd_device_gatt_set_service_changed(struct btd_device *device,
>>>                                                 uint16_t start, uint16_t end)
>>>  {
>>> diff --git a/src/device.h b/src/device.h
>>> index 487bd27..a7fefee 100644
>>> --- a/src/device.h
>>> +++ b/src/device.h
>>> @@ -67,6 +67,8 @@ 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);
>>> +struct gatt_db *btd_device_get_gatt_db(struct btd_device *device);
>>> +struct bt_gatt_client *btd_device_get_gatt_client(struct btd_device *device);
>>>  void btd_device_gatt_set_service_changed(struct btd_device *device,
>>>                                                 uint16_t start, uint16_t end);
>>>  bool device_attach_att(struct btd_device *dev, GIOChannel *io);
>>> --
>>> 2.2.0.rc0.207.ga3a616c
>>>
>>> --
>>> 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
>
> Arman



-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ v4 05/10] core: device: Don't disconnect if attios not set

[Arman Uguray]

Hi Luiz,

On Wed, Dec 17, 2014 at 12:00 PM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Arman,
>
> On Wed, Dec 17, 2014 at 3:42 PM, Arman Uguray <armansito@chromium.org> wrote:
>> Hi Luiz,
>>
>>> On Wed, Dec 17, 2014 at 5:08 AM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>>> Hi Arman,
>>>
>>> On Wed, Dec 17, 2014 at 4:07 AM, Arman Uguray <armansito@chromium.org> wrote:
>>>> Currently there is no way for external applications to claim ownership
>>>> on a GATT connection and as profiles move away from attio.h it doesn't
>>>> make much sense to immediately disconnect if no attio callbacks have
>>>> been set after probing the profiles (as this will always immediately
>>>> disconnect the device). This patch removes this logic from btd_device.
>>>
>>> I was thinking that if no driver accepts the connection we should
>>> disconnect but it doesn't look like you had done anything in this
>>> direction, for external client using the D-Bus API maybe we need some
>>> method for registering client making only the services with clients
>>> visible.
>>>
>>
>> I discussed how to make this possible for external applications on the
>> mailing list with Marcel and Johan by extending the Profile API
>> concept to GATT (org.bluez.GattProfile1 or sorts) but this will only
>> happen down the line.
>>
>> For profiles, maybe btd_device can choose to disconnect if no profile
>> returned success during probing? Then again, we end up having a
>> mundane profile like GAP behave like the rest of the profiles this
>> way.
>
> Im not really following here, probing stage is to detect a driver can
> be used and on success we create a service instance so it happen only
> once, accept in the other hand would be triggered every time the
> device connects. Anyway my comments was regarding the fact that we
> need to detect if there are multiple clients for the same profile we
> would need to do some conflict resolution, since we cannot track D-Bus
> client right now I think it is better to not expose services which
> have built-in driver such as HoG, etc, later once we introduce
> org.bluez.GattProfile1 or similar we may favor external apps if that
> makes more sense.
>
>> For now, I figured we can remove this logic and keep the device
>> connected (since the user called Connect, it seems reasonable to make
>> the assumption that they want the device to be connected). In the
>> future, we'll want to revisit this and perhaps combine btd_profile and
>> GattProfile1 state to determine auto-connect, keeping the connection
>> alive, and so on.
>
> Well in case of classic if the user call Connect and there is no
> driver matching/probed we don't attempt to connect at all, except to
> update the services, but with LE it is much more common that we got
> connected by the remote via advertising and passive scanning, thus it
> would look like as an incoming connection, and then we could request
> each probed driver to accept the connection.
>
>> Though initially I still want to have the basic doc/gatt-api.txt
>> implemented before we go down the GattProfile path. I'm open to
>> suggestions in general though.
>
> I guess it is fine to leave it for later but we should keep in mind
> what we gonna do about conflicting clients, I figure this patch might
> actually be necessary since we cannot track D-Bus clients we may keep
> the connection even if no driver accepts it.
>

As we discussed on IRC, I'm leaving this patch in. We'll regulate
conflicts between external apps and internal profiles by not exporing
claimed services.

>>
>>>> ---
>>>>  src/device.c | 8 --------
>>>>  1 file changed, 8 deletions(-)
>>>>
>>>> diff --git a/src/device.c b/src/device.c
>>>> index e8bdc18..64591d0 100644
>>>> --- a/src/device.c
>>>> +++ b/src/device.c
>>>> @@ -3642,9 +3642,6 @@ static void register_gatt_services(struct browse_req *req)
>>>>
>>>>         device_probe_profiles(device, req->profiles_added);
>>>>
>>>> -       if (device->attios == NULL && device->attios_offline == NULL)
>>>> -               attio_cleanup(device);
>>>> -
>>>>         device_svc_resolved(device, device->bdaddr_type, 0);
>>>>
>>>>         store_services(device);
>>>> @@ -5069,11 +5066,6 @@ gboolean btd_device_remove_attio_callback(struct btd_device *device, guint id)
>>>>
>>>>         g_free(attio);
>>>>
>>>> -       if (device->attios != NULL || device->attios_offline != NULL)
>>>> -               return TRUE;
>>>> -
>>>> -       attio_cleanup(device);
>>>> -
>>>>         return TRUE;
>>>>  }
>>>>
>>>> --
>>>> 2.2.0.rc0.207.ga3a616c
>>>>
>>>> --
>>>> 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
>>
>> Arman
>
>
>
> --
> Luiz Augusto von Dentz

Arman

Re: [PATCH BlueZ v4 06/10] core: device: Add getters for GATT db and client

[Arman Uguray]

Hi Luiz,

> On Wed, Dec 17, 2014 at 12:19 PM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
> Hi Arman,
>
> On Wed, Dec 17, 2014 at 3:45 PM, Arman Uguray <armansito@chromium.org> wrote:
>> Hi Luiz,
>>
>>> On Wed, Dec 17, 2014 at 4:39 AM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>>> Hi Arman,
>>>
>>> On Wed, Dec 17, 2014 at 4:07 AM, Arman Uguray <armansito@chromium.org> wrote:
>>>> This patch adds btd_device_get_gatt_db and btd_device_get_gatt_client
>>>> functions.
>>>> ---
>>>>  src/device.c | 16 ++++++++++++++++
>>>>  src/device.h |  2 ++
>>>>  2 files changed, 18 insertions(+)
>>>>
>>>> diff --git a/src/device.c b/src/device.c
>>>> index 64591d0..854d9f3 100644
>>>> --- a/src/device.c
>>>> +++ b/src/device.c
>>>> @@ -4844,6 +4844,22 @@ GSList *btd_device_get_primaries(struct btd_device *device)
>>>>         return device->primaries;
>>>>  }
>>>>
>>>> +struct gatt_db *btd_device_get_gatt_db(struct btd_device *device)
>>>> +{
>>>> +       if (!device)
>>>> +               return NULL;
>>>> +
>>>> +       return device->db;
>>>> +}
>>>
>>> I guess the db can be get from the gatt_client, also perhaps we should
>>> return a reference otherwise it should be const.
>>>
>>
>> I think I'll keep this getter but make things const. My idea here is
>> that the gatt_db will outlive the connection and if the device is
>> bonded it won't get cleared. So it might still be useful.
>
> Sure no problem, Im just not sure if the driver would be interested in
> the db without the client and as it is const it should not be possible
> to register callbacks, actually const for the client perhaps is not a
> good idea either for the same reason.
>

That's a good point. I guess I'll leave this one in like this though.
Eventually when we have a better, more centralized in-memory cache
that holds all the gatt-db's then profiles can get it from there.
Until then, they can get it from btd_device if they want to.

>>>> +struct bt_gatt_client *btd_device_get_gatt_client(struct btd_device *device)
>>>> +{
>>>> +       if (!device)
>>>> +               return NULL;
>>>> +
>>>> +       return device->client;
>>>> +}
>>>> +
>>>>  void btd_device_gatt_set_service_changed(struct btd_device *device,
>>>>                                                 uint16_t start, uint16_t end)
>>>>  {
>>>> diff --git a/src/device.h b/src/device.h
>>>> index 487bd27..a7fefee 100644
>>>> --- a/src/device.h
>>>> +++ b/src/device.h
>>>> @@ -67,6 +67,8 @@ 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);
>>>> +struct gatt_db *btd_device_get_gatt_db(struct btd_device *device);
>>>> +struct bt_gatt_client *btd_device_get_gatt_client(struct btd_device *device);
>>>>  void btd_device_gatt_set_service_changed(struct btd_device *device,
>>>>                                                 uint16_t start, uint16_t end);
>>>>  bool device_attach_att(struct btd_device *dev, GIOChannel *io);
>>>> --
>>>> 2.2.0.rc0.207.ga3a616c
>>>>
>>>> --
>>>> 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
>>
>> Arman
>
>
>
> --
> Luiz Augusto von Dentz
> --
> 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

Arman