2014-09-03 06:58:02

by Lukasz Rymanowski

[permalink] [raw]
Subject: [PATCH v2 1/6] android/gatt: Add API to remove autoconnect GATT based device

---
v2: Fixed issue found by Szymon

android/gatt.c | 24 ++++++++++++++++++++++++
android/gatt.h | 1 +
2 files changed, 25 insertions(+)

diff --git a/android/gatt.c b/android/gatt.c
index aff5dc5..a3486ca 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -6768,3 +6768,27 @@ bool bt_gatt_add_autoconnect(unsigned int id, const bdaddr_t *addr)

return true;
}
+
+void bt_gatt_remove_autoconnect(unsigned int id, const bdaddr_t *addr)
+{
+ struct gatt_device *dev;
+
+ DBG("");
+
+ dev = find_device_by_addr(addr);
+ if (!dev) {
+ error("gatt: Device not found");
+ return;
+ }
+
+ queue_remove(dev->autoconnect_apps, INT_TO_PTR(id));
+
+ if (queue_isempty(dev->autoconnect_apps)) {
+ bt_auto_connect_remove(addr);
+
+ if (dev->state == DEVICE_CONNECT_INIT)
+ device_set_state(dev, DEVICE_DISCONNECTED);
+
+ device_unref(dev);
+ }
+}
diff --git a/android/gatt.h b/android/gatt.h
index 40699b2..3382df9 100644
--- a/android/gatt.h
+++ b/android/gatt.h
@@ -40,3 +40,4 @@ bool bt_gatt_connect_app(unsigned int id, const bdaddr_t *addr);
bool bt_gatt_disconnect_app(unsigned int id, const bdaddr_t *addr);
bool bt_gatt_set_security(const bdaddr_t *bdaddr, int sec_level);
bool bt_gatt_add_autoconnect(unsigned int id, const bdaddr_t *addr);
+void bt_gatt_remove_autoconnect(unsigned int id, const bdaddr_t *addr);
--
1.8.4



2014-09-03 13:46:25

by Lukasz Rymanowski

[permalink] [raw]
Subject: Re: [PATCH v2 2/6] android/gatt: Unsubscribe from autoconnect on unregister

Hi Luiz,

On 3 September 2014 13:30, Luiz Augusto von Dentz <[email protected]> wrote:
> Hi Lukasz,
>
> On Wed, Sep 3, 2014 at 9:58 AM, Lukasz Rymanowski
> <[email protected]> wrote:
>> When application does unregister, lets make sure that BfA does not keep
>> any auto connect devices for this app.
>> ---
>> android/gatt.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 43 insertions(+)
>>
>> diff --git a/android/gatt.c b/android/gatt.c
>> index a3486ca..dea253e 100644
>> --- a/android/gatt.c
>> +++ b/android/gatt.c
>> @@ -370,6 +370,13 @@ static bool match_device_by_state(const void *data, const void *user_data)
>> return true;
>> }
>>
>> +static bool match_device_by_ac_id(const void *data, const void *user_data)
>> +{
>> + const struct gatt_device *dev = data;
>> +
>> + return queue_find(dev->autoconnect_apps, match_by_value, user_data);
>> +}
>> +
>> static bool match_pending_device(const void *data, const void *user_data)
>> {
>> const struct gatt_device *dev = data;
>> @@ -442,6 +449,11 @@ static struct gatt_device *find_device_by_state(uint32_t state)
>> UINT_TO_PTR(state));
>> }
>>
>> +static struct gatt_device *find_device_by_autoconnect_app_id(uint32_t id)
>> +{
>> + return queue_find(gatt_devices, match_device_by_ac_id,
>> + UINT_TO_PTR(id));
>> +}
>> static bool match_srvc_by_element_id(const void *data, const void *user_data)
>> {
>> const struct element_id *exp_id = user_data;
>> @@ -1865,10 +1877,41 @@ static bool trigger_connection(struct app_connection *connection)
>> return ret;
>> }
>>
>> +static void clear_autoconnect_devices(int app_id)
>> +{
>> + struct gatt_device *dev;
>> +
>> + /*
>> + * Check if application was registered for any devices to
>> + * reconnect
>> + */
>> + dev = find_device_by_autoconnect_app_id(app_id);
>> + while (dev) {
>> + queue_remove(dev->autoconnect_apps, INT_TO_PTR(app_id));
>> +
>> + if (queue_isempty(dev->autoconnect_apps)) {
>> + bt_auto_connect_remove(&dev->bdaddr);
>> +
>> + if (dev->state == DEVICE_CONNECT_INIT)
>> + device_set_state(dev, DEVICE_DISCONNECTED);
>> +
>> + device_unref(dev);
>> + }
>> +
>> + dev = find_device_by_autoconnect_app_id(app_id);
>> + };
>
> Looks like there is a ';; after the while statement, please fix that.

True. I had do {} while; before sending it to ML and forgot to remove it.

> Also this looks very inefficient since it does
> find_device_by_autoconnect_app_id does a double lookup and we doing it
> in a while loop, I would suggest using queue_foreach(gatt_devices...
> then do queue_remove_if(dev->autoconnect_apps, the result should be
> the same but with a single lookup in each queue.
>

OK, Will use queue_foreach. I did this ways, because I thought that
device_unref can destroy device and remove it from gatt_devices and
this is something we don't want to happen during
queue_foreach(gatt_device..). Anyway, I double check it and it should
not happen (unless there is a bug somewhere ;) )

\Lukasz

>> +}
>> +
>> static uint8_t unregister_app(int client_if)
>> {
>> struct gatt_app *cl;
>>
>> + /*
>> + * Make sure that there is no devices in auto connect list for this
>> + * application
>> + */
>> + clear_autoconnect_devices(client_if);
>> +
>> cl = queue_remove_if(gatt_apps, match_app_by_id, INT_TO_PTR(client_if));
>> if (!cl) {
>> error("gatt: client_if=%d not found", client_if);
>> --
>> 1.8.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Luiz Augusto von Dentz

2014-09-03 11:30:04

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH v2 2/6] android/gatt: Unsubscribe from autoconnect on unregister

Hi Lukasz,

On Wed, Sep 3, 2014 at 9:58 AM, Lukasz Rymanowski
<[email protected]> wrote:
> When application does unregister, lets make sure that BfA does not keep
> any auto connect devices for this app.
> ---
> android/gatt.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 43 insertions(+)
>
> diff --git a/android/gatt.c b/android/gatt.c
> index a3486ca..dea253e 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -370,6 +370,13 @@ static bool match_device_by_state(const void *data, const void *user_data)
> return true;
> }
>
> +static bool match_device_by_ac_id(const void *data, const void *user_data)
> +{
> + const struct gatt_device *dev = data;
> +
> + return queue_find(dev->autoconnect_apps, match_by_value, user_data);
> +}
> +
> static bool match_pending_device(const void *data, const void *user_data)
> {
> const struct gatt_device *dev = data;
> @@ -442,6 +449,11 @@ static struct gatt_device *find_device_by_state(uint32_t state)
> UINT_TO_PTR(state));
> }
>
> +static struct gatt_device *find_device_by_autoconnect_app_id(uint32_t id)
> +{
> + return queue_find(gatt_devices, match_device_by_ac_id,
> + UINT_TO_PTR(id));
> +}
> static bool match_srvc_by_element_id(const void *data, const void *user_data)
> {
> const struct element_id *exp_id = user_data;
> @@ -1865,10 +1877,41 @@ static bool trigger_connection(struct app_connection *connection)
> return ret;
> }
>
> +static void clear_autoconnect_devices(int app_id)
> +{
> + struct gatt_device *dev;
> +
> + /*
> + * Check if application was registered for any devices to
> + * reconnect
> + */
> + dev = find_device_by_autoconnect_app_id(app_id);
> + while (dev) {
> + queue_remove(dev->autoconnect_apps, INT_TO_PTR(app_id));
> +
> + if (queue_isempty(dev->autoconnect_apps)) {
> + bt_auto_connect_remove(&dev->bdaddr);
> +
> + if (dev->state == DEVICE_CONNECT_INIT)
> + device_set_state(dev, DEVICE_DISCONNECTED);
> +
> + device_unref(dev);
> + }
> +
> + dev = find_device_by_autoconnect_app_id(app_id);
> + };

Looks like there is a ';; after the while statement, please fix that.
Also this looks very inefficient since it does
find_device_by_autoconnect_app_id does a double lookup and we doing it
in a while loop, I would suggest using queue_foreach(gatt_devices...
then do queue_remove_if(dev->autoconnect_apps, the result should be
the same but with a single lookup in each queue.

> +}
> +
> static uint8_t unregister_app(int client_if)
> {
> struct gatt_app *cl;
>
> + /*
> + * Make sure that there is no devices in auto connect list for this
> + * application
> + */
> + clear_autoconnect_devices(client_if);
> +
> cl = queue_remove_if(gatt_apps, match_app_by_id, INT_TO_PTR(client_if));
> if (!cl) {
> error("gatt: client_if=%d not found", client_if);
> --
> 1.8.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
Luiz Augusto von Dentz

2014-09-03 06:58:07

by Lukasz Rymanowski

[permalink] [raw]
Subject: [PATCH v2 6/6] android/hidhost: Remove reconnect logic

GATT can handle reconnect now, so lets remove that part of code from
here
---
android/hidhost.c | 26 +++++++-------------------
1 file changed, 7 insertions(+), 19 deletions(-)

diff --git a/android/hidhost.c b/android/hidhost.c
index 3dc3c25..652b118 100644
--- a/android/hidhost.c
+++ b/android/hidhost.c
@@ -109,7 +109,6 @@ struct hid_device {
struct bt_uhid *uhid;
uint8_t last_hid_msg;
struct bt_hog *hog;
- guint reconnect_id;
int sec_level;
};

@@ -125,9 +124,6 @@ static void hid_device_free(void *data)
{
struct hid_device *dev = data;

- if (dev->reconnect_id > 0)
- g_source_remove(dev->reconnect_id);
-
if (dev->ctrl_watch > 0)
g_source_remove(dev->ctrl_watch);

@@ -768,19 +764,6 @@ fail:
hid_device_remove(dev);
}

-static gboolean hog_reconnect(void *user_data)
-{
- struct hid_device *dev = user_data;
-
- DBG("");
-
- dev->reconnect_id = 0;
-
- bt_gatt_connect_app(hog_app, &dev->dst);
-
- return FALSE;
-}
-
static void hog_conn_cb(const bdaddr_t *addr, int err, void *attrib)
{
GSList *l;
@@ -792,11 +775,10 @@ static void hog_conn_cb(const bdaddr_t *addr, int err, void *attrib)
if (err < 0) {
if (!dev)
return;
- if (dev->hog && !dev->reconnect_id) {
+ if (dev->hog) {
bt_hid_notify_state(dev,
HAL_HIDHOST_STATE_DISCONNECTED);
bt_hog_detach(dev->hog);
- dev->reconnect_id = g_idle_add(hog_reconnect, dev);
return;
}
goto fail;
@@ -829,6 +811,9 @@ static void hog_conn_cb(const bdaddr_t *addr, int err, void *attrib)

bt_hid_notify_state(dev, HAL_HIDHOST_STATE_CONNECTED);

+ if (!bt_gatt_add_autoconnect(hog_app, &dev->dst))
+ error("hidhost: Could not add to autoconnect list");
+
return;

fail:
@@ -1497,6 +1482,9 @@ static void hid_unpaired_cb(const bdaddr_t *addr, uint8_t type)
ba2str(addr, address);
DBG("Unpaired device %s", address);

+ if (hog_app)
+ bt_gatt_remove_autoconnect(hog_app, addr);
+
hid_device_remove(dev);
}

--
1.8.4


2014-09-03 06:58:06

by Lukasz Rymanowski

[permalink] [raw]
Subject: [PATCH v2 5/6] android/gatt: Remove device from white list on device destroy

---
android/gatt.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/android/gatt.c b/android/gatt.c
index 0182c5d..b086c0f 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -722,6 +722,8 @@ static void destroy_device(void *data)
queue_destroy(dev->pending_requests, destroy_pending_request);
queue_destroy(dev->autoconnect_apps, NULL);

+ bt_auto_connect_remove(&dev->bdaddr);
+
free(dev);
}

--
1.8.4


2014-09-03 06:58:05

by Lukasz Rymanowski

[permalink] [raw]
Subject: [PATCH v2 4/6] android/gatt: Make sure GATT will reconnect after disconnection

With this patch once remote is disconnected there is decision taken if
BfA should wait for reconnect or not.

Removing device from the autoconnect list has been removed from
connect_cb as it is now handled during disconnection
---
android/gatt.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index 542c9b4..0182c5d 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -650,6 +650,13 @@ static void connection_cleanup(struct gatt_device *device)
queue_remove_all(device->services, NULL, NULL, destroy_service);

device_set_state(device, DEVICE_DISCONNECTED);
+
+ if (!queue_isempty(device->autoconnect_apps)) {
+ auto_connect_le(device);
+ device_set_state(device, DEVICE_CONNECT_INIT);
+ } else {
+ bt_auto_connect_remove(&device->bdaddr);
+ }
}

static void destroy_gatt_app(void *data)
@@ -1463,9 +1470,6 @@ static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)

device_set_state(dev, DEVICE_CONNECTED);

- if (queue_isempty(dev->autoconnect_apps))
- bt_auto_connect_remove(&dev->bdaddr);
-
/* Send exchange mtu request as we assume being client and server */
/* TODO: Dont exchange mtu if no client apps */
send_exchange_mtu_request(dev);
--
1.8.4


2014-09-03 06:58:04

by Lukasz Rymanowski

[permalink] [raw]
Subject: [PATCH v2 3/6] android/gatt: Extract remove_autoconnect_device helper

---
android/gatt.c | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index dea253e..542c9b4 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -1877,6 +1877,16 @@ static bool trigger_connection(struct app_connection *connection)
return ret;
}

+static void remove_autoconnect_device(struct gatt_device *dev)
+{
+ bt_auto_connect_remove(&dev->bdaddr);
+
+ if (dev->state == DEVICE_CONNECT_INIT)
+ device_set_state(dev, DEVICE_DISCONNECTED);
+
+ device_unref(dev);
+}
+
static void clear_autoconnect_devices(int app_id)
{
struct gatt_device *dev;
@@ -1889,14 +1899,10 @@ static void clear_autoconnect_devices(int app_id)
while (dev) {
queue_remove(dev->autoconnect_apps, INT_TO_PTR(app_id));

- if (queue_isempty(dev->autoconnect_apps)) {
- bt_auto_connect_remove(&dev->bdaddr);
-
- if (dev->state == DEVICE_CONNECT_INIT)
- device_set_state(dev, DEVICE_DISCONNECTED);
+ if (queue_isempty(dev->autoconnect_apps))
+ remove_autoconnect_device(dev);

device_unref(dev);
- }

dev = find_device_by_autoconnect_app_id(app_id);
};
@@ -6826,12 +6832,8 @@ void bt_gatt_remove_autoconnect(unsigned int id, const bdaddr_t *addr)

queue_remove(dev->autoconnect_apps, INT_TO_PTR(id));

- if (queue_isempty(dev->autoconnect_apps)) {
- bt_auto_connect_remove(addr);
-
- if (dev->state == DEVICE_CONNECT_INIT)
- device_set_state(dev, DEVICE_DISCONNECTED);
+ if (queue_isempty(dev->autoconnect_apps))
+ remove_autoconnect_device(dev);

device_unref(dev);
- }
}
--
1.8.4


2014-09-03 06:58:03

by Lukasz Rymanowski

[permalink] [raw]
Subject: [PATCH v2 2/6] android/gatt: Unsubscribe from autoconnect on unregister

When application does unregister, lets make sure that BfA does not keep
any auto connect devices for this app.
---
android/gatt.c | 43 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 43 insertions(+)

diff --git a/android/gatt.c b/android/gatt.c
index a3486ca..dea253e 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -370,6 +370,13 @@ static bool match_device_by_state(const void *data, const void *user_data)
return true;
}

+static bool match_device_by_ac_id(const void *data, const void *user_data)
+{
+ const struct gatt_device *dev = data;
+
+ return queue_find(dev->autoconnect_apps, match_by_value, user_data);
+}
+
static bool match_pending_device(const void *data, const void *user_data)
{
const struct gatt_device *dev = data;
@@ -442,6 +449,11 @@ static struct gatt_device *find_device_by_state(uint32_t state)
UINT_TO_PTR(state));
}

+static struct gatt_device *find_device_by_autoconnect_app_id(uint32_t id)
+{
+ return queue_find(gatt_devices, match_device_by_ac_id,
+ UINT_TO_PTR(id));
+}
static bool match_srvc_by_element_id(const void *data, const void *user_data)
{
const struct element_id *exp_id = user_data;
@@ -1865,10 +1877,41 @@ static bool trigger_connection(struct app_connection *connection)
return ret;
}

+static void clear_autoconnect_devices(int app_id)
+{
+ struct gatt_device *dev;
+
+ /*
+ * Check if application was registered for any devices to
+ * reconnect
+ */
+ dev = find_device_by_autoconnect_app_id(app_id);
+ while (dev) {
+ queue_remove(dev->autoconnect_apps, INT_TO_PTR(app_id));
+
+ if (queue_isempty(dev->autoconnect_apps)) {
+ bt_auto_connect_remove(&dev->bdaddr);
+
+ if (dev->state == DEVICE_CONNECT_INIT)
+ device_set_state(dev, DEVICE_DISCONNECTED);
+
+ device_unref(dev);
+ }
+
+ dev = find_device_by_autoconnect_app_id(app_id);
+ };
+}
+
static uint8_t unregister_app(int client_if)
{
struct gatt_app *cl;

+ /*
+ * Make sure that there is no devices in auto connect list for this
+ * application
+ */
+ clear_autoconnect_devices(client_if);
+
cl = queue_remove_if(gatt_apps, match_app_by_id, INT_TO_PTR(client_if));
if (!cl) {
error("gatt: client_if=%d not found", client_if);
--
1.8.4