by Arman Uguray

[permalink] [raw]

Subject: [PATCH BlueZ v3 2/8] core: device: Fix GATT profile probing

This patch fixes a recently introduced issue which changed the behavior
of profile probing for GATT-based profiles to create a btd_service
instance for all discovered GATT services to be on a per-UUID basis
rather than per-service.
---
src/device.c | 182 +++++++++++++++++++++++------------------------------------
1 file changed, 70 insertions(+), 112 deletions(-)

diff --git a/src/device.c b/src/device.c
index 75a5984..4f702ba 100644
--- a/src/device.c
+++ b/src/device.c
@@ -295,21 +295,15 @@ 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)
+static GSList *find_service_with_uuid(GSList *list, char *uuid)
{
GSList *l;
- uint16_t svc_start, svc_end;

for (l = list; l != NULL; l = g_slist_next(l)) {
struct btd_service *service = l->data;
+ struct btd_profile *profile = btd_service_get_profile(service);

- if (!btd_service_get_gatt_handles(service, &svc_start,
- &svc_end))
- continue;
-
- if (svc_start == start_handle && svc_end == end_handle)
+ if (bt_uuid_strcmp(profile->remote_uuid, uuid) == 0)
return l;
}

@@ -2420,6 +2414,7 @@ static void add_primary(struct gatt_db_attribute *attr, void *user_data)
static void device_add_uuids(struct btd_device *device, GSList *uuids)
{
GSList *l;
+ bool changed = false;

for (l = uuids; l != NULL; l = g_slist_next(l)) {
GSList *match = g_slist_find_custom(device->uuids, l->data,
@@ -2427,12 +2422,14 @@ static void device_add_uuids(struct btd_device *device, GSList *uuids)
if (match)
continue;

+ changed = true;
device->uuids = g_slist_insert_sorted(device->uuids,
g_strdup(l->data),
bt_uuid_strcmp);
}

- g_dbus_emit_property_changed(dbus_conn, device->path,
+ if (changed)
+ g_dbus_emit_property_changed(dbus_conn, device->path,
DEVICE_INTERFACE, "UUIDs");
}

@@ -2444,8 +2441,6 @@ struct gatt_probe_data {
GSList *uuids;
struct gatt_db_attribute *cur_attr;
char cur_uuid[MAX_LEN_UUID_STR];
- uint16_t start_handle, end_handle;
- GSList *valid_services;
};

static bool device_match_profile(struct btd_device *device,
@@ -2473,8 +2468,7 @@ static void dev_probe_gatt(struct btd_profile *p, void *user_data)
if (!p->remote_uuid || bt_uuid_strcmp(p->remote_uuid, data->cur_uuid))
return;

- service = service_create_gatt(data->dev, p, data->start_handle,
- data->end_handle);
+ service = service_create(data->dev, p);
if (!service)
return;

@@ -2487,72 +2481,6 @@ static void dev_probe_gatt(struct btd_profile *p, void *user_data)
gatt_db_service_set_active(data->cur_attr, false);

data->dev->services = g_slist_append(data->dev->services, service);
-
- if (data->all_services)
- data->valid_services = g_slist_append(data->valid_services,
- service);
-}
-
-static bool match_existing_service(struct gatt_probe_data *data)
-{
- struct btd_device *dev = data->dev;
- struct btd_service *service;
- struct btd_profile *profile;
- uint16_t start, end;
- GSList *l, *tmp;
-
- /*
- * Check if the profiles should be probed for the service in the
- * database.
- */
- for (l = dev->services; l != NULL;) {
- service = l->data;
- profile = btd_service_get_profile(service);
-
- /* If this is local or non-GATT based service, then skip. */
- if (!profile->remote_uuid ||
- !btd_service_get_gatt_handles(service, &start, &end)) {
- l = g_slist_next(l);
- continue;
- }
-
- /* Skip this service if the handle ranges don't overlap. */
- if (start > data->end_handle || end < data->start_handle) {
- l = g_slist_next(l);
- continue;
- }
-
- /*
- * If there is an exact match, then there's no need to probe the
- * profiles. An exact match is when the service handles AND the
- * service UUID match.
- */
- if (start == data->start_handle && end == data->end_handle &&
- !bt_uuid_strcmp(profile->remote_uuid, data->cur_uuid)) {
- if (data->all_services)
- data->valid_services = g_slist_append(
- data->valid_services, service);
- return true;
- }
-
- /*
- * The handles overlap but there is no exact match. This means
- * that the service is no longer valid. Remove it.
- *
- * TODO: The following code is fairly inefficient, especially
- * when we consider all the extra searches that we're already
- * doing. Consider changing the services list to a GList.
- */
- tmp = l->next;
- dev->services = g_slist_delete_link(dev->services, l);
- dev->pending = g_slist_remove(dev->pending, service);
- service_remove(service);
-
- l = tmp;
- }
-
- /* No match found */
- return false;
}

static void dev_probe_gatt_profile(struct gatt_db_attribute *attr,
@@ -2562,22 +2490,27 @@ static void dev_probe_gatt_profile(struct gatt_db_attribute *attr,
bt_uuid_t uuid;
GSList *l = NULL;

- gatt_db_attribute_get_service_data(attr, &data->start_handle,
- &data->end_handle, NULL,
- &uuid);
+ gatt_db_attribute_get_service_uuid(attr, &uuid);
bt_uuid_to_string(&uuid, data->cur_uuid, sizeof(data->cur_uuid));

data->cur_attr = attr;

- /* Don't probe the profiles if a matching service already exists. */
- if (!match_existing_service(data))
- btd_profile_foreach(dev_probe_gatt, data);
-
- if (data->all_services) {
+ /*
+ * If we're probing for all services, store the UUID since device->uuids
+ * was cleared.
+ */
+ if (data->all_services)
data->uuids = g_slist_append(data->uuids,
g_strdup(data->cur_uuid));
+
+ /* Don't probe the profiles if a matching service already exists. */
+ if (find_service_with_uuid(data->dev->services, data->cur_uuid))
+ return;
+
+ btd_profile_foreach(dev_probe_gatt, data);
+
+ if (data->all_services)
return;
- }

l = g_slist_append(l, g_strdup(data->cur_uuid));
device_add_uuids(data->dev, l);
@@ -2600,12 +2533,15 @@ static void remove_invalid_services(struct gatt_probe_data *data)
{
struct btd_device *dev = data->dev;
struct btd_service *service;
+ struct btd_profile *profile;
GSList *l, *tmp;

for (l = dev->services; l != NULL;) {
service = l->data;
+ profile = btd_service_get_profile(service);

- if (g_slist_find(data->valid_services, service)) {
+ if (g_slist_find_custom(dev->uuids, profile->remote_uuid,
+ bt_uuid_strcmp)) {
l = g_slist_next(l);
continue;
}
@@ -2639,11 +2575,21 @@ static void device_probe_gatt_profiles(struct btd_device *device)

gatt_db_foreach_service(device->db, NULL, dev_probe_gatt_profile,
&data);
+
+ /*
+ * Clear the UUIDs list
+ *
+ * FIXME: The management of UUIDs here iss currently broken, as clearing
+ * the entire list here will likely remove UUIDs that shouldn't be
+ * removed (e.g. those obtained via SDP for a dual-mode device).
+ */
+ g_slist_free_full(device->uuids, g_free);
+ device->uuids = NULL;
+
device_add_uuids(device, data.uuids);
g_slist_free_full(data.uuids, g_free);

remove_invalid_services(&data);
- g_slist_free(data.valid_services);
}

static void device_accept_gatt_profiles(struct btd_device *device)
@@ -2657,13 +2603,15 @@ static void device_accept_gatt_profiles(struct btd_device *device)
static void device_remove_gatt_profile(struct btd_device *device,
struct gatt_db_attribute *attr)
{
- uint16_t start, end;
struct btd_service *service;
+ bt_uuid_t uuid;
+ char uuid_str[MAX_LEN_UUID_STR];
GSList *l;

- gatt_db_attribute_get_service_handles(attr, &start, &end);
+ gatt_db_attribute_get_service_uuid(attr, &uuid);
+ bt_uuid_to_string(&uuid, uuid_str, sizeof(uuid_str));

- l = find_service_with_gatt_handles(device->services, start, end);
+ l = find_service_with_uuid(device->services, uuid_str);
if (!l)
return;

@@ -2677,13 +2625,16 @@ static void gatt_service_added(struct gatt_db_attribute *attr, void *user_data)
{
struct btd_device *device = user_data;
GSList *new_service = NULL;
+ bt_uuid_t uuid;
+ char uuid_str[MAX_LEN_UUID_STR];
uint16_t start, end;
GSList *l;

if (!bt_gatt_client_is_ready(device->client))
return;

- gatt_db_attribute_get_service_handles(attr, &start, &end);
+ gatt_db_attribute_get_service_data(attr, &start, &end, NULL, &uuid);
+ bt_uuid_to_string(&uuid, uuid_str, sizeof(uuid_str));

DBG("start: 0x%04x, end: 0x%04x", start, end);

@@ -2695,14 +2646,19 @@ static void gatt_service_added(struct gatt_db_attribute *attr, void *user_data)
if (!new_service)
return;

- device_register_primaries(device, new_service, -1);
+ l = find_service_with_uuid(device->services, uuid_str);

- device_probe_gatt_profile(device, attr);
+ device_register_primaries(device, new_service, -1);

- l = find_service_with_gatt_handles(device->services, start, end);
- if (l)
+ /*
+ * If the profile was probed for the first time then call accept on
+ * the service.
+ */
+ if (!l && (l = find_service_with_uuid(device->services, uuid_str)))
service_accept(l->data);

+ device_probe_gatt_profile(device, attr);
+
store_device_info(device);

btd_gatt_client_service_added(device->client_dbus, attr);
@@ -2750,24 +2706,26 @@ static void gatt_service_removed(struct gatt_db_attribute *attr,
prim = l->data;
device->primaries = g_slist_delete_link(device->primaries, l);

- /*
- * If this happend since the db was cleared for a non-bonded device,
- * then don't remove the btd_service just yet. We do this so that we can
- * avoid re-probing the profile if the same GATT service is found on the
- * device on re-connection. However, if the device is marked as
- * temporary, then we remove it anyway.
- */
- if (device->client || device->temporary == TRUE)
- device_remove_gatt_profile(device, attr);
+ /* Get the UUID entry to be removed below */
+ l = g_slist_find_custom(device->uuids, prim->uuid, bt_uuid_strcmp);

/*
- * Remove the corresponding UUIDs entry, only if this is the last
- * service with this UUID.
+ * Remove the corresponding UUIDs entry and the profile, only if this
+ * is the last service with this UUID.
*/
- l = g_slist_find_custom(device->uuids, prim->uuid, bt_uuid_strcmp);
-
if (!g_slist_find_custom(device->primaries, prim->uuid,
prim_uuid_cmp)) {
+ /*
+ * If this happened since the db was cleared for a non-bonded
+ * device, then don't remove the btd_service just yet. We do
+ * this so that we can avoid re-probing the profile if the same
+ * GATT service is found on the device on re-connection.
+ * However, if the device is marked as temporary, then we remove
+ * it anyway.
+ */
+ if (device->client || device->temporary == TRUE)
+ device_remove_gatt_profile(device, attr);
+
g_free(l->data);
device->uuids = g_slist_delete_link(device->uuids, l);
g_dbus_emit_property_changed(dbus_conn, device->path,
--
2.2.0.rc0.207.ga3a616c

2015-01-14 03:31:00

by Arman Uguray

[permalink] [raw]

Subject: [PATCH BlueZ v3 1/8] shared/gatt-db: Add service getter by UUID

Added gatt_db_get_service_with_uuid which checks if the database
contains a service with the given UUID and returns the first result.
---
src/shared/gatt-db.c | 26 ++++++++++++++++++++++++++
src/shared/gatt-db.h | 3 +++
2 files changed, 29 insertions(+)

diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
index e4a6f73..e8fb52c 100644
--- a/src/shared/gatt-db.c
+++ b/src/shared/gatt-db.c
@@ -1179,6 +1179,32 @@ struct gatt_db_attribute *gatt_db_get_attribute(struct gatt_db *db,
return service->attributes[handle - service_handle];
}

+static bool find_service_with_uuid(const void *data, const void *user_data)
+{
+ const struct gatt_db_service *service = data;
+ const bt_uuid_t *uuid = user_data;
+ bt_uuid_t svc_uuid;
+
+ gatt_db_attribute_get_service_uuid(service->attributes[0], &svc_uuid);
+
+ return bt_uuid_cmp(uuid, &svc_uuid) == 0;
+}
+
+struct gatt_db_attribute *gatt_db_get_service_with_uuid(struct gatt_db *db,
+ const bt_uuid_t *uuid)
+{
+ struct gatt_db_service *service;
+
+ if (!db || !uuid)
+ return NULL;
+
+ service = queue_find(db->services, find_service_with_uuid, uuid);
+ if (!service)
+ return NULL;
+
+ return service->attributes[0];
+}
+
const bt_uuid_t *gatt_db_attribute_get_type(
const struct gatt_db_attribute *attrib)
{
diff --git a/src/shared/gatt-db.h b/src/shared/gatt-db.h
index 1f4005e..dc1b819 100644
--- a/src/shared/gatt-db.h
+++ b/src/shared/gatt-db.h
@@ -151,6 +151,9 @@ bool gatt_db_unregister(struct gatt_db *db, unsigned int id);
struct gatt_db_attribute *gatt_db_get_attribute(struct gatt_db *db,
uint16_t handle);

+struct gatt_db_attribute *gatt_db_get_service_with_uuid(struct gatt_db *db,
+ const bt_uuid_t *uuid);
+
const bt_uuid_t *gatt_db_attribute_get_type(
const struct gatt_db_attribute *attrib);

--
2.2.0.rc0.207.ga3a616c