> On Wed, Jan 14, 2015 at 11:38 AM, Arman Uguray <armansito@xxxxxxxxxxxx> wrote:
> Hi Luiz,
>
>> On Wed, Jan 14, 2015 at 5:17 AM, Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx> wrote:
>> Hi Arman,
>>
>> On Wed, Jan 14, 2015 at 1:31 AM, Arman Uguray <armansito@xxxxxxxxxxxx> wrote:
>>> This patch fixes the broken GATT UUID management, which incorrectly
>>> remove valid UUIDs during the GATT profile probe process. This is
>>> achieved by splitting UUIDs obtained via GATT and SDP into their own
>>> lists that are internal to btd_device.
>>> ---
>>> plugins/sixaxis.c | 2 +-
>>> src/adapter.c | 8 +-
>>> src/device.c | 367 +++++++++++++++++++++++++++++++++++-------------------
>>> src/device.h | 6 +-
>>> 4 files changed, 248 insertions(+), 135 deletions(-)
>>>
>>> diff --git a/plugins/sixaxis.c b/plugins/sixaxis.c
>>> index ac53ba9..0a4e3d8 100644
>>> --- a/plugins/sixaxis.c
>>> +++ b/plugins/sixaxis.c
>>> @@ -289,7 +289,7 @@ static bool setup_device(int fd, int index, struct btd_adapter *adapter)
>>>
>>> device = btd_adapter_get_device(adapter, &device_bdaddr, BDADDR_BREDR);
>>>
>>> - if (g_slist_find_custom(btd_device_get_uuids(device), HID_UUID,
>>> + if (g_slist_find_custom(btd_device_get_sdp_uuids(device), HID_UUID,
>>> (GCompareFunc)strcasecmp)) {
>>> DBG("device %s already known, skipping", device_addr);
>>> return true;
>>> diff --git a/src/adapter.c b/src/adapter.c
>>> index d7e2550..379a80b 100644
>>> --- a/src/adapter.c
>>> +++ b/src/adapter.c
>>> @@ -2912,9 +2912,13 @@ static void load_devices(struct btd_adapter *adapter)
>>>
>>> /* TODO: register services from pre-loaded list of primaries */
>>>
>>> - list = btd_device_get_uuids(device);
>>> + list = btd_device_get_sdp_uuids(device);
>>> if (list)
>>> - device_probe_profiles(device, list);
>>> + device_probe_profiles(device, list, false);
>>> +
>>> + list = btd_device_get_gatt_uuids(device);
>>> + if (list)
>>> + device_probe_profiles(device, list, true);
>>>
>>> device_exist:
>>> if (key_info) {
>>> diff --git a/src/device.c b/src/device.c
>>> index 4f702ba..9d1a480 100644
>>> --- a/src/device.c
>>> +++ b/src/device.c
>>> @@ -193,7 +193,8 @@ struct btd_device {
>>> uint16_t appearance;
>>> char *modalias;
>>> struct btd_adapter *adapter;
>>> - GSList *uuids;
>>> + GSList *sdp_uuids;
>>> + GSList *gatt_uuids;
>>> GSList *primaries; /* List of primary services */
>>> GSList *services; /* List of btd_service */
>>> GSList *pending; /* Pending services */
>>> @@ -335,6 +336,28 @@ static void update_technologies(GKeyFile *file, struct btd_device *dev)
>>> list, len);
>>> }
>>>
>>> +static char **store_service_uuids(GKeyFile *key_file, gchar *key, GSList *uuids)
>>> +{
>>> + GSList *l;
>>> + char **array;
>>> + int i;
>>> +
>>> + if (!uuids) {
>>> + g_key_file_remove_key(key_file, "General", key, NULL);
>>> + return NULL;
>>> + }
>>> +
>>> + array = g_new0(char *, g_slist_length(uuids) + 1);
>>> +
>>> + for (i = 0, l = uuids; l; l = g_slist_next(l), i++)
>>> + array[i] = l->data;
>>> +
>>> + g_key_file_set_string_list(key_file, "General", key,
>>> + (const char **)array, i);
>>> +
>>> + return array;
>>> +}
>>> +
>>> static gboolean store_device_info_cb(gpointer user_data)
>>> {
>>> struct btd_device *device = user_data;
>>> @@ -344,7 +367,7 @@ static gboolean store_device_info_cb(gpointer user_data)
>>> char device_addr[18];
>>> char *str;
>>> char class[9];
>>> - char **uuids = NULL;
>>> + char **sdp_uuids, **gatt_uuids;
>>> gsize length = 0;
>>>
>>> device->store_id = 0;
>>> @@ -387,18 +410,10 @@ static gboolean store_device_info_cb(gpointer user_data)
>>> g_key_file_set_boolean(key_file, "General", "Blocked",
>>> device->blocked);
>>>
>>> - if (device->uuids) {
>>> - GSList *l;
>>> - int i;
>>> -
>>> - uuids = g_new0(char *, g_slist_length(device->uuids) + 1);
>>> - for (i = 0, l = device->uuids; l; l = g_slist_next(l), i++)
>>> - uuids[i] = l->data;
>>> - g_key_file_set_string_list(key_file, "General", "Services",
>>> - (const char **)uuids, i);
>>> - } else {
>>> - g_key_file_remove_key(key_file, "General", "Services", NULL);
>>> - }
>>> + sdp_uuids = store_service_uuids(key_file, "SDPServices",
>>> + device->sdp_uuids);
>>> + gatt_uuids = store_service_uuids(key_file, "GATTServices",
>>> + device->gatt_uuids);
>>
>> This change is not backward compatible which mean existing storage
>> will not load properly if we change the storage fields, furthermore I
>> think it is better to store the UUIDs per bearer_state since service
>> discovered flag is stored there it seem logical to couple UUIDs founds
>> there as well.
>>
>
> You're right, I'm aware that this is not backwards compatible. I
> didn't build any migration mechanism from the UUIDs field to the new
> fields to sort of initiate this discussion with you. I'm still trying
> to figure out how to resolve the current format. See my comments
> below.
>
>>> if (device->vendor_src) {
>>> g_key_file_set_integer(key_file, "DeviceID", "Source",
>>> @@ -420,7 +435,8 @@ static gboolean store_device_info_cb(gpointer user_data)
>>> g_free(str);
>>>
>>> g_key_file_free(key_file);
>>> - g_free(uuids);
>>> + g_free(sdp_uuids);
>>> + g_free(gatt_uuids);
>>>
>>> return FALSE;
>>> }
>>> @@ -574,7 +590,8 @@ static void device_free(gpointer user_data)
>>> btd_gatt_client_destroy(device->client_dbus);
>>> device->client_dbus = NULL;
>>>
>>> - g_slist_free_full(device->uuids, g_free);
>>> + g_slist_free_full(device->sdp_uuids, g_free);
>>> + g_slist_free_full(device->gatt_uuids, g_free);
>>> g_slist_free_full(device->primaries, g_free);
>>> g_slist_free_full(device->attios, g_free);
>>> g_slist_free_full(device->attios_offline, g_free);
>>> @@ -988,22 +1005,43 @@ static gboolean dev_property_get_uuids(const GDBusPropertyTable *property,
>>> {
>>> struct btd_device *dev = data;
>>> DBusMessageIter entry;
>>> - GSList *l;
>>> + GHashTable *uuids = NULL;
>>> + GSList *sl;
>>> + GList *keys, *l;
>>>
>>> dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
>>> DBUS_TYPE_STRING_AS_STRING, &entry);
>>>
>>> - if (dev->bredr_state.svc_resolved || dev->le_state.svc_resolved)
>>> - l = dev->uuids;
>>> - else if (dev->eir_uuids)
>>> - l = dev->eir_uuids;
>>> - else
>>> - l = dev->uuids;
>>> + if (!dev->bredr_state.svc_resolved && dev->le_state.svc_resolved &&
>>> + dev->eir_uuids) {
>>> + for (sl = dev->eir_uuids; sl; sl = g_slist_next(sl))
>>> + dbus_message_iter_append_basic(&entry, DBUS_TYPE_STRING,
>>> + &sl->data);
>>> +
>>> + goto done;
>>> + }
>>> +
>>> + /* Insert GATT and SDP UUIDs into a set to prevent any duplicates */
>>> + uuids = g_hash_table_new(g_str_hash, bt_uuid_strcmp);
>>>
>>> - for (; l != NULL; l = l->next)
>>> + for (sl = dev->sdp_uuids; sl; sl = g_slist_next(sl))
>>> + g_hash_table_add(uuids, sl->data);
>>> +
>>> + for (sl = dev->gatt_uuids; sl; sl = g_slist_next(sl))
>>> + g_hash_table_add(uuids, sl->data);
>>> +
>>> + /* Obtain a list of UUIDs and sort it */
>>> + keys = g_hash_table_get_keys(uuids);
>>> + keys = g_list_sort(keys, bt_uuid_strcmp);
>>> +
>>> + for (l = keys; l; l = g_list_next(l))
>>> dbus_message_iter_append_basic(&entry, DBUS_TYPE_STRING,
>>> - &l->data);
>>> + &l->data);
>>> +
>>> + g_list_free(keys);
>>> + g_hash_table_destroy(uuids);
>>>
>>> +done:
>>> dbus_message_iter_close_container(iter, &entry);
>>>
>>> return TRUE;
>>> @@ -1130,7 +1168,8 @@ int device_unblock(struct btd_device *device, gboolean silent,
>>> if (!silent) {
>>> g_dbus_emit_property_changed(dbus_conn, device->path,
>>> DEVICE_INTERFACE, "Blocked");
>>> - device_probe_profiles(device, device->uuids);
>>> + device_probe_profiles(device, device->sdp_uuids, false);
>>> + device_probe_profiles(device, device->gatt_uuids, true);
>>> }
>>>
>>> return 0;
>>> @@ -2165,6 +2204,66 @@ failed:
>>> return str;
>>> }
>>>
>>> +static bool device_add_uuid(GSList **uuids, const char *uuid)
>>> +{
>>> + if (g_slist_find_custom(*uuids, uuid, bt_uuid_strcmp))
>>> + return false;
>>> +
>>> + *uuids = g_slist_insert_sorted(*uuids, g_strdup(uuid), bt_uuid_strcmp);
>>> +
>>> + return true;
>>> +}
>>> +
>>> +static bool device_add_uuids(struct btd_device *device, GSList **uuids,
>>> + GSList *new_uuids)
>>> +{
>>> + GSList *l;
>>> + bool changed = false;
>>> +
>>> + for (l = new_uuids; l; l = g_slist_next(l)) {
>>> + if (device_add_uuid(uuids, l->data))
>>> + changed = true;
>>> + }
>>> +
>>> + if (changed)
>>> + g_dbus_emit_property_changed(dbus_conn, device->path,
>>> + DEVICE_INTERFACE, "UUIDs");
>>> +
>>> + return changed;
>>> +}
>>> +
>>> +static bool device_add_sdp_uuid(struct btd_device *device, const char *uuid)
>>> +{
>>> + return device_add_uuid(&device->sdp_uuids, uuid);
>>> +}
>>> +
>>> +static bool device_add_sdp_uuids(struct btd_device *device, GSList *uuids)
>>> +{
>>> + return device_add_uuids(device, &device->sdp_uuids, uuids);
>>> +}
>>> +
>>> +static bool device_add_gatt_uuid(struct btd_device *device, const char *uuid)
>>> +{
>>> + return device_add_uuid(&device->gatt_uuids, uuid);
>>> +}
>>> +
>>> +static bool device_add_gatt_uuids(struct btd_device *device, GSList *uuids)
>>> +{
>>> + return device_add_uuids(device, &device->gatt_uuids, uuids);
>>> +}
>>> +
>>> +static void device_remove_gatt_uuid(struct btd_device *device, const char *uuid)
>>> +{
>>> + GSList *l;
>>> +
>>> + l = g_slist_find_custom(device->gatt_uuids, uuid, bt_uuid_strcmp);
>>> + if (!l)
>>> + return;
>>> +
>>> + g_free(l->data);
>>> + device->gatt_uuids = g_slist_delete_link(device->gatt_uuids, l);
>>> +}
>>> +
>>> static void load_info(struct btd_device *device, const char *local,
>>> const char *peer, GKeyFile *key_file)
>>> {
>>> @@ -2254,30 +2353,40 @@ next:
>>> if (blocked)
>>> device_block(device, FALSE);
>>>
>>> - /* Load device profile list */
>>> - uuids = g_key_file_get_string_list(key_file, "General", "Services",
>>> - NULL, NULL);
>>> + /* Load classic device profile list */
>>> + uuids = g_key_file_get_string_list(key_file, "General", "SDPServices",
>>> + NULL, NULL);
>>> if (uuids) {
>>> char **uuid;
>>>
>>> - for (uuid = uuids; *uuid; uuid++) {
>>> - GSList *match;
>>> + for (uuid = uuids; *uuid; uuid++)
>>> + device_add_sdp_uuid(device, *uuid);
>>>
>>> - match = g_slist_find_custom(device->uuids, *uuid,
>>> - bt_uuid_strcmp);
>>> - if (match)
>>> - continue;
>>> -
>>> - device->uuids = g_slist_insert_sorted(device->uuids,
>>> - g_strdup(*uuid),
>>> - bt_uuid_strcmp);
>>> - }
>>> g_strfreev(uuids);
>>>
>>> /* Discovered services restored from storage */
>>> device->bredr_state.svc_resolved = true;
>>> }
>>>
>>> + /* Load GATT-based device profile list */
>>> + uuids = g_key_file_get_string_list(key_file, "General", "GATTServices",
>>> + NULL, NULL);
>>> + if (uuids) {
>>> + char **uuid;
>>> +
>>> + for (uuid = uuids; *uuid; uuid++)
>>> + device_add_gatt_uuid(device, *uuid);
>>> +
>>> + g_strfreev(uuids);
>>> +
>>> + /*
>>> + * TODO: The GATT-based service UUIDs have been restored from
>>> + * storage but we don't have a populated gatt-db yet. Here we
>>> + * should mark le_state.svc_resolved as true, if we're bonded
>>> + * and we have a populated gatt_db.
>>> + */
>>> + }
>>> +
>>> /* Load device id */
>>> source = g_key_file_get_integer(key_file, "DeviceID", "Source", NULL);
>>> if (source) {
>>> @@ -2411,34 +2520,13 @@ 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;
>>> - bool changed = false;
>>> -
>>> - 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;
>>> -
>>> - changed = true;
>>> - device->uuids = g_slist_insert_sorted(device->uuids,
>>> - g_strdup(l->data),
>>> - bt_uuid_strcmp);
>>> - }
>>> -
>>> - if (changed)
>>> - 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 all_services;
>>> - GSList *uuids;
>>> + GSList *new_uuids;
>>> + GSList *old_uuids;
>>> struct gatt_db_attribute *cur_attr;
>>> char cur_uuid[MAX_LEN_UUID_STR];
>>> };
>>> @@ -2488,20 +2576,31 @@ static void dev_probe_gatt_profile(struct gatt_db_attribute *attr,
>>> {
>>> struct gatt_probe_data *data = user_data;
>>> bt_uuid_t uuid;
>>> - GSList *l = NULL;
>>> + gpointer dup_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;
>>>
>>> - /*
>>> - * 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,
>>> + if (data->all_services) {
>>> + GSList *l;
>>> +
>>> + /*
>>> + * Check if the service is already in the old UUIDs list. If so,
>>> + * remove it.
>>> + */
>>> + l = g_slist_find_custom(data->old_uuids, data->cur_uuid,
>>> + bt_uuid_strcmp);
>>> + if (l) {
>>> + g_free(l->data);
>>> + data->old_uuids = g_slist_delete_link(data->old_uuids,
>>> + l);
>>> + } else {
>>> + data->new_uuids = g_slist_append(data->new_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))
>>> @@ -2512,8 +2611,14 @@ static void dev_probe_gatt_profile(struct gatt_db_attribute *attr,
>>> if (data->all_services)
>>> return;
>>>
>>> - l = g_slist_append(l, g_strdup(data->cur_uuid));
>>> - device_add_uuids(data->dev, l);
>>> + dup_uuid = g_strdup(data->cur_uuid);
>>> + if (!device_add_gatt_uuid(data->dev, dup_uuid)) {
>>> + g_free(dup_uuid);
>>> + return;
>>> + }
>>> +
>>> + g_dbus_emit_property_changed(dbus_conn, data->dev->path,
>>> + DEVICE_INTERFACE, "UUIDs");
>>> }
>>>
>>> static void device_probe_gatt_profile(struct btd_device *device,
>>> @@ -2526,33 +2631,25 @@ static void device_probe_gatt_profile(struct btd_device *device,
>>> data.dev = device;
>>>
>>> dev_probe_gatt_profile(attr, &data);
>>> - g_slist_free_full(data.uuids, g_free);
>>> }
>>>
>>> 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;
>>> + GSList *l, *svc;
>>>
>>> - for (l = dev->services; l != NULL;) {
>>> - service = l->data;
>>> - profile = btd_service_get_profile(service);
>>> + for (l = data->old_uuids; l; l = g_slist_next(l)) {
>>> + device_remove_gatt_uuid(dev, l->data);
>>>
>>> - if (g_slist_find_custom(dev->uuids, profile->remote_uuid,
>>> - bt_uuid_strcmp)) {
>>> - l = g_slist_next(l);
>>> + svc = find_service_with_uuid(dev->services, l->data);
>>> + if (!svc)
>>> continue;
>>> - }
>>>
>>> - /* Service no longer valid, so remove it */
>>> - tmp = l->next;
>>> - dev->services = g_slist_delete_link(dev->services, l);
>>> + service = svc->data;
>>> + dev->services = g_slist_delete_link(dev->services, svc);
>>> dev->pending = g_slist_remove(dev->pending, service);
>>> service_remove(service);
>>> -
>>> - l = tmp;
>>> }
>>> }
>>>
>>> @@ -2560,6 +2657,7 @@ static void device_probe_gatt_profiles(struct btd_device *device)
>>> {
>>> struct gatt_probe_data data;
>>> char addr[18];
>>> + GSList *l;
>>>
>>> ba2str(&device->bdaddr, addr);
>>>
>>> @@ -2573,23 +2671,24 @@ static void device_probe_gatt_profiles(struct btd_device *device)
>>> data.dev = device;
>>> data.all_services = true;
>>>
>>> + /* Copy current list of GATT UUIDs */
>>> + for (l = device->gatt_uuids; l; l = g_slist_next(l))
>>> + data.old_uuids = g_slist_append(data.old_uuids,
>>> + g_strdup(l->data));
>>>
>>> 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;
>>> + /* Whatever remains in data.old_uuids is stale and should be removed */
>>> + remove_invalid_services(&data);
>>>
>>> - device_add_uuids(device, data.uuids);
>>> - g_slist_free_full(data.uuids, g_free);
>>> + /* Update the list with new UUIDs */
>>> + if (!device_add_gatt_uuids(device, data.new_uuids) && data.old_uuids)
>>> + g_dbus_emit_property_changed(dbus_conn, device->path,
>>> + DEVICE_INTERFACE, "UUIDs");
>>>
>>> - remove_invalid_services(&data);
>>> + g_slist_free_full(data.new_uuids, g_free);
>>> + g_slist_free_full(data.old_uuids, g_free);
>>> }
>>>
>>> static void device_accept_gatt_profiles(struct btd_device *device)
>>> @@ -2675,46 +2774,39 @@ 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)
>>> {
>>> struct btd_device *device = user_data;
>>> GSList *l;
>>> - struct gatt_primary *prim;
>>> uint16_t start, end;
>>> + bt_uuid_t uuid;
>>> + char uuid_str[MAX_LEN_UUID_STR];
>>>
>>> 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);
>>>
>>> DBG("start: 0x%04x, end: 0x%04x", start, end);
>>>
>>> /* Remove the corresponding gatt_primary */
>>> l = g_slist_find_custom(device->primaries, attr, prim_attr_cmp);
>>> - if (!l)
>>> - return;
>>> + if (l) {
>>> + g_free(l->data);
>>> + device->primaries = g_slist_delete_link(device->primaries, l);
>>> + }
>>>
>>> - prim = l->data;
>>> - device->primaries = g_slist_delete_link(device->primaries, l);
>>> + bt_uuid_to_string(&uuid, uuid_str, sizeof(uuid_str));
>>>
>>> /* Get the UUID entry to be removed below */
>>> - l = g_slist_find_custom(device->uuids, prim->uuid, bt_uuid_strcmp);
>>> + l = g_slist_find_custom(device->gatt_uuids, uuid_str, bt_uuid_strcmp);
>>>
>>> /*
>>> * Remove the corresponding UUIDs entry and the profile, only if this
>>> * is the last service with this UUID.
>>> */
>>> - if (!g_slist_find_custom(device->primaries, prim->uuid,
>>> - prim_uuid_cmp)) {
>>> + if (!gatt_db_get_service_with_uuid(device->db, &uuid)) {
>>> /*
>>> * If this happened since the db was cleared for a non-bonded
>>> * device, then don't remove the btd_service just yet. We do
>>> @@ -2727,13 +2819,11 @@ static void gatt_service_removed(struct gatt_db_attribute *attr,
>>> device_remove_gatt_profile(device, attr);
>>>
>>> g_free(l->data);
>>> - device->uuids = g_slist_delete_link(device->uuids, l);
>>> + device->gatt_uuids = g_slist_delete_link(device->gatt_uuids, l);
>>> g_dbus_emit_property_changed(dbus_conn, device->path,
>>> DEVICE_INTERFACE, "UUIDs");
>>> }
>>>
>>> - g_free(prim);
>>> -
>>> store_device_info(device);
>>>
>>> btd_gatt_client_service_removed(device->client_dbus, attr);
>>> @@ -2988,8 +3078,13 @@ void device_merge_duplicate(struct btd_device *dev, struct btd_device *dup)
>>> dev->trusted = dup->trusted;
>>> dev->blocked = dup->blocked;
>>>
>>> - for (l = dup->uuids; l; l = g_slist_next(l))
>>> - dev->uuids = g_slist_append(dev->uuids, g_strdup(l->data));
>>> + for (l = dup->sdp_uuids; l; l = g_slist_next(l))
>>> + dev->sdp_uuids = g_slist_append(dev->sdp_uuids,
>>> + g_strdup(l->data));
>>> +
>>> + for (l = dup->gatt_uuids; l; l = g_slist_next(l))
>>> + dev->gatt_uuids = g_slist_append(dev->gatt_uuids,
>>> + g_strdup(l->data));
>>>
>>> if (dev->name[0] == '\0')
>>> strcpy(dev->name, dup->name);
>>> @@ -3240,9 +3335,14 @@ static gboolean record_has_uuid(const sdp_record_t *rec,
>>> return FALSE;
>>> }
>>>
>>> -GSList *btd_device_get_uuids(struct btd_device *device)
>>> +GSList *btd_device_get_sdp_uuids(struct btd_device *device)
>>> {
>>> - return device->uuids;
>>> + return device->sdp_uuids;
>>> +}
>>> +
>>> +GSList *btd_device_get_gatt_uuids(struct btd_device *device)
>>> +{
>>> + return device->gatt_uuids;
>>> }
>>>
>>> struct probe_data {
>>> @@ -3280,7 +3380,8 @@ void device_probe_profile(gpointer a, gpointer b)
>>> if (profile->device_probe == NULL)
>>> return;
>>>
>>> - if (!device_match_profile(device, profile, device->uuids))
>>> + if (!device_match_profile(device, profile, device->sdp_uuids) &&
>>> + !device_match_profile(device, profile, device->gatt_uuids))
>>> return;
>>>
>>> service = service_create(device, profile);
>>> @@ -3318,7 +3419,8 @@ void device_remove_profile(gpointer a, gpointer b)
>>> service_remove(service);
>>> }
>>>
>>> -void device_probe_profiles(struct btd_device *device, GSList *uuids)
>>> +void device_probe_profiles(struct btd_device *device, GSList *uuids,
>>> + bool gatt)
>>> {
>>> struct probe_data d = { device, uuids };
>>> char addr[18];
>>> @@ -3335,7 +3437,10 @@ void device_probe_profiles(struct btd_device *device, GSList *uuids)
>>> btd_profile_foreach(dev_probe, &d);
>>>
>>> add_uuids:
>>> - device_add_uuids(device, uuids);
>>> + if (gatt)
>>> + device_add_gatt_uuids(device, uuids);
>>> + else
>>> + device_add_sdp_uuids(device, uuids);
>>> }
>>>
>>> static void store_sdp_record(GKeyFile *key_file, sdp_record_t *rec)
>>> @@ -3431,7 +3536,7 @@ static int update_record(struct browse_req *req, const char *uuid,
>>> req->records = sdp_list_append(req->records, sdp_copy_record(rec));
>>>
>>> /* Check if UUID is duplicated */
>>> - l = g_slist_find_custom(req->device->uuids, uuid, bt_uuid_strcmp);
>>> + l = g_slist_find_custom(req->device->sdp_uuids, uuid, bt_uuid_strcmp);
>>> if (l == NULL) {
>>> l = g_slist_find_custom(req->profiles_added, uuid,
>>> bt_uuid_strcmp);
>>> @@ -3662,7 +3767,7 @@ static void search_cb(sdp_list_t *recs, int err, gpointer user_data)
>>> * the full list of services and populate a client-role gatt_db over
>>> * BR/EDR.
>>> */
>>> - device_probe_profiles(device, req->profiles_added);
>>> + device_probe_profiles(device, req->profiles_added, false);
>>>
>>> /* Propagate services changes */
>>> g_dbus_emit_property_changed(dbus_conn, req->device->path,
>>> @@ -3927,6 +4032,8 @@ static void gatt_client_ready_cb(bool success, uint8_t att_ecode,
>>>
>>> if (device->browse)
>>> register_gatt_services(device->browse);
>>> + else
>>> + device_probe_gatt_profiles(device);
>>>
>>> device_accept_gatt_profiles(device);
>>>
>>> @@ -5127,13 +5234,13 @@ void btd_device_add_uuid(struct btd_device *device, const char *uuid)
>>> GSList *uuid_list;
>>> char *new_uuid;
>>>
>>> - if (g_slist_find_custom(device->uuids, uuid, bt_uuid_strcmp))
>>> + if (g_slist_find_custom(device->sdp_uuids, uuid, bt_uuid_strcmp))
>>> return;
>>>
>>> new_uuid = g_strdup(uuid);
>>> uuid_list = g_slist_append(NULL, new_uuid);
>>>
>>> - device_probe_profiles(device, uuid_list);
>>> + device_probe_profiles(device, uuid_list, false);
>>>
>>> g_free(new_uuid);
>>> g_slist_free(uuid_list);
>>> diff --git a/src/device.h b/src/device.h
>>> index a7fefee..f250bfa 100644
>>> --- a/src/device.h
>>> +++ b/src/device.h
>>> @@ -60,8 +60,10 @@ struct device_addr_type {
>>> };
>>>
>>> int device_addr_type_cmp(gconstpointer a, gconstpointer b);
>>> -GSList *btd_device_get_uuids(struct btd_device *device);
>>> -void device_probe_profiles(struct btd_device *device, GSList *profiles);
>>> +GSList *btd_device_get_sdp_uuids(struct btd_device *device);
>>> +GSList *btd_device_get_gatt_uuids(struct btd_device *device);
>>> +void device_probe_profiles(struct btd_device *device, GSList *profiles,
>>> + bool gatt);
>>
>> Id leave this API as it is for now and internally classify the UUIDs,
>> the classification function can be used when loading from the storage
>> that way we don't need to break the storage format.
>>
>
> Well, I'm just not sure how to do that classification. Especially in
> the case of device_probe_profiles, which gets passed in a list of
> UUIDs and the case where the UUIDs get loaded from storage, I don't
> know how we can figure out which ones are LE and which ones are
> BR/EDR, unless we do something complicated/inefficient like going
> through the gatt-db and see if there's a matching service for each
> UUID but this still becomes a problem when we load the UUIDs before
> having done discovery (i.e. there's no populated gatt-db).
>
>>> const sdp_record_t *btd_device_get_record(struct btd_device *device,
>>> const char *uuid);
>>> struct gatt_primary *btd_device_get_primary(struct btd_device *device,
>>> --
>>> 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@xxxxxxxxxxxxxxx
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
>>
>> --
>> Luiz Augusto von Dentz
>
> Thanks,
> Arman
ping
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
