Input pairing broken again

Input pairing broken again

[David Woodhouse]

Using the wizard to pair with my Anycom BTM-100 mouse is broken by
commit 58d8ea3c (Make discovery service routine to search for driver
uuids.)

I think the pairing actually works OK, but it takes so long that the
wizard times out, and doesn't bother to connect the new input device.
Looking at the dump at http://david.woodhou.se/btmouse-fail.dump it
seems to be taking about 30 seconds.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: Input pairing broken again

[Luiz Augusto von Dentz]

Hi David,

Could you please provide us the output of bluetoothd -dn and
dbus-monitor --system sender='org.bluez'?

If that is taking too long Im afraid we cannot search for drivers uuis
thus searching for l2cap uuid seems to be the only alternative.

-- 
Luiz Augusto von Dentz
Engenheiro de Computação

Re: Input pairing broken again

[David Woodhouse]

On Fri, 2008-09-12 at 17:21 -0700, David Woodhouse wrote:
> Using the wizard to pair with my Anycom BTM-100 mouse is broken by
> commit 58d8ea3c (Make discovery service routine to search for driver
> uuids.)
> 
> I think the pairing actually works OK, but it takes so long that the
> wizard times out, and doesn't bother to connect the new input device.
> Looking at the dump at http://david.woodhou.se/btmouse-fail.dump it
> seems to be taking about 30 seconds.

diff --git a/src/device.c b/src/device.c
index b90861b..0e615be 100644
--- a/src/device.c
+++ b/src/device.c
@@ -975,8 +975,9 @@ static void browse_cb(sdp_list_t *recs, int err, gpointer user_data)
 	bdaddr_t src;
 	uuid_t uuid;
 
-	/* Public browsing successful or Single record requested */
-	if (err < 0 || (!req->search_uuid && recs))
+	/* If we have a valid response and req->search_uuid == 1, then
+	   public browsing was successful -- we don't need any more */
+	if (err < 0 || (req->search_uuid == 1 && recs))
 		goto done;
 
 	update_services(req, recs);
@@ -984,8 +985,8 @@ static void browse_cb(sdp_list_t *recs, int err, gpointer user_data)
 	adapter_get_address(adapter, &src);
 
 	/* Search for mandatory uuids */
-	if (uuid_list[++req->search_uuid]) {
-		sdp_uuid16_create(&uuid, uuid_list[req->search_uuid]);
+	if (uuid_list[req->search_uuid]) {
+		sdp_uuid16_create(&uuid, uuid_list[req->search_uuid++]);
 		bt_search_service(&src, &device->bdaddr, &uuid, browse_cb, user_data, NULL);
 		return;
 	}
@@ -1057,7 +1061,7 @@ int device_browse(struct btd_device *device, DBusConnection *conn,
 		memcpy(&uuid, search, sizeof(uuid_t));
 		cb = search_cb;
 	} else {
-		sdp_uuid16_create(&uuid, uuid_list[req->search_uuid]);
+		sdp_uuid16_create(&uuid, uuid_list[req->search_uuid++]);
 		init_browse(req);
 		cb = browse_cb;
 	}
@@ -1072,7 +1076,7 @@ int device_browse(struct btd_device *device, DBusConnection *conn,
 						device, NULL);
 
 	return bt_search_service(&src, &device->bdaddr,
-					&uuid, browse_cb, req, NULL);
+					&uuid, cb, req, NULL);
 }
 
 struct btd_adapter *device_get_adapter(struct btd_device *device)


-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: Input pairing broken again

[David Woodhouse]

While we're at it, let's fix init_browse() not to abort completely when
it finds one driver asking for a UUID that another driver already
wanted, and to eliminate duplicates of the UUIDs in uuid_list[] too...

diff --git a/src/device.c b/src/device.c
index b90861b..cae29f8 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1022,11 +1023,29 @@ static void init_browse(struct browse_req *req)
 
 		for (i = 0; driver->uuids[i]; i++) {
 			char *uuid;
-
+			int j;
+
+			/* Eliminate duplicates of UUIDs in uuid_list[]... */
+			if (strlen(driver->uuids[i]) == 36 && 
+			    !strncmp(driver->uuids[i], "0000", 4) &&
+			    !strcasecmp(driver->uuids[i] + 8, 
+					"-0000-1000-8000-00805F9B34FB")) {
+				uint16_t uuid16 = strtol(driver->uuids[i],
+							 NULL, 16);
+				for (j = 0; uuid_list[j]; j++) {
+					if (uuid16 == uuid_list[j])
+						continue;
+				}
+					
+			}
+			/* ... and of UUIDs another driver already asked for */
 			if (g_slist_find_custom(req->uuids, driver->uuids[i],
-					(GCompareFunc) strcasecmp))
-				return;
-
+						(GCompareFunc) strcasecmp)) {
+				printf("match on %s: return\n", driver->uuids[i]);
+				continue;
+			}
+			printf("Add uuid %s for driver %s\n",
+			       driver->uuids[i], driver->name);
 			uuid = g_strdup(driver->uuids[i]);
 			req->uuids = g_slist_append(req->uuids, uuid);
 		}

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: Input pairing broken again

[Luiz Augusto von Dentz]

Both seems good, but please use debug instead of printf. Btw does your
first patch solves the problem?

On Fri, Sep 12, 2008 at 11:38 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> While we're at it, let's fix init_browse() not to abort completely when
> it finds one driver asking for a UUID that another driver already
> wanted, and to eliminate duplicates of the UUIDs in uuid_list[] too...
>
> diff --git a/src/device.c b/src/device.c
> index b90861b..cae29f8 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -1022,11 +1023,29 @@ static void init_browse(struct browse_req *req)
>
>                for (i = 0; driver->uuids[i]; i++) {
>                        char *uuid;
> -
> +                       int j;
> +
> +                       /* Eliminate duplicates of UUIDs in uuid_list[]... */
> +                       if (strlen(driver->uuids[i]) == 36 &&
> +                           !strncmp(driver->uuids[i], "0000", 4) &&
> +                           !strcasecmp(driver->uuids[i] + 8,
> +                                       "-0000-1000-8000-00805F9B34FB")) {
> +                               uint16_t uuid16 = strtol(driver->uuids[i],
> +                                                        NULL, 16);
> +                               for (j = 0; uuid_list[j]; j++) {
> +                                       if (uuid16 == uuid_list[j])
> +                                               continue;
> +                               }
> +
> +                       }
> +                       /* ... and of UUIDs another driver already asked for */
>                        if (g_slist_find_custom(req->uuids, driver->uuids[i],
> -                                       (GCompareFunc) strcasecmp))
> -                               return;
> -
> +                                               (GCompareFunc) strcasecmp)) {
> +                               printf("match on %s: return\n", driver->uuids[i]);
> +                               continue;
> +                       }
> +                       printf("Add uuid %s for driver %s\n",
> +                              driver->uuids[i], driver->name);
>                        uuid = g_strdup(driver->uuids[i]);
>                        req->uuids = g_slist_append(req->uuids, uuid);
>                }
>
> --
> David Woodhouse                            Open Source Technology Centre
> David.Woodhouse@intel.com                              Intel Corporation
>
>



-- 
Luiz Augusto von Dentz
Engenheiro de Computação

Re: Input pairing broken again

[David Woodhouse]

On Fri, 2008-09-12 at 23:46 -0300, Luiz Augusto von Dentz wrote:
> Both seems good, but please use debug instead of printf. Btw does your
> first patch solves the problem?

Oops. I meant to remove the printf before I sent the patch :)

Have committed both to git://git.infradead.org/~dwmw2/bluez.git without
the printf.

Before my first patch, look closely at the handling of req->search_uuid
in browse_cb().

Note that it's incremented _every_ time we end up back in browse_cb().

Even when we've already finished going through the uuid_list[] array,
and we're supposed to be iterating through req->uuids.

So what happens is we handle everything in the uuid_list[] until we
reach the zero at the end. Then we handle the first item from
req->uuids, then we trawl through the memory _after_ uuid_list[],
treating it as more uuids to search for until we reach another zero.
Then we handle the second item from req->uuids, then...

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: Input pairing broken again

[Luiz Augusto von Dentz]

Yep, it was obviously wrong, please send a pull request to Marcel so
we can have this fixes upstream.


-- 
Luiz Augusto von Dentz
Engenheiro de Computação