2015-09-08 12:23:40

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 1/3] shared/gatt-client: Fix not freeing notify data

From: Luiz Augusto von Dentz <[email protected]>

notify_data_write_ccc takes another reference in case it succeed so
the original reference should be dropped to cause the data to be freed
once notify_data_write_ccc completes.
---
src/shared/gatt-client.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index 903afa7..0983852 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -1596,8 +1596,7 @@ static void complete_unregister_notify(void *data)
!notify_data->chrc->ccc_handle)
goto done;

- if (notify_data_write_ccc(notify_data, false, disable_ccc_callback))
- return;
+ notify_data_write_ccc(notify_data, false, disable_ccc_callback);

done:
notify_data_unref(notify_data);
--
2.4.3



2015-09-14 16:39:44

by Jakub Pawlowski

[permalink] [raw]
Subject: Re: [PATCH BlueZ 2/3] core/gatt: Fix not able to read/write on reconnections

Hi Luiz,

On Tue, Sep 8, 2015 at 10:40 AM, Jakub Pawlowski <[email protected]> wrote:
> Hi Luiz,
>
> This patch fixes all my issues with writing just after connecting.
> Thanks alot!
>

So reading and writing works perfectly, even better than I fought - I
have some short flows that execute before services get re-discovered,
but registering for notifications just after connection fails.

I can call DBus method register for notifications before I connect ,
and request will succeed, but I would really be registered for
notifications after btd_gatt_client_ready is called.
This cause a race condition:

1. App Call DBus register for notifications.
2. App call Connect on Device.
3. App Write - write succeds immediately.
4. Device sends back notification which I don't receive in App (I can
see it coming in btmon).
5. Now btd_gatt_client_ready is called, and App is really being
registered for notifications (too late).

I'm working on fixing that. It whould require registering for
notifications right after connecting, before serivces get discovered.


> On Tue, Sep 8, 2015 at 5:23 AM, Luiz Augusto von Dentz
> <[email protected]> wrote:
>> From: Luiz Augusto von Dentz <[email protected]>
>>
>> Upon reconnecting the attributes may already be available from cache
>> which mean the client would be able to issue commands while discovery
>> is not complete thus btd_gatt_client_ready was not called yet.
>>
>> To fix now btd_gatt_client_ready is split into btd_gatt_client_connected
>> which is called whenever connected leaving btd_gatt_client_ready to only
>> deal with exporting services found during the discovery.
>> ---
>> src/device.c | 2 ++
>> src/gatt-client.c | 24 +++++++++++++++++-------
>> src/gatt-client.h | 1 +
>> 3 files changed, 20 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/device.c b/src/device.c
>> index dc362d9..2102f23 100644
>> --- a/src/device.c
>> +++ b/src/device.c
>> @@ -4602,6 +4602,8 @@ static void gatt_client_init(struct btd_device *device)
>> }
>>
>> device->gatt_cache_used = !gatt_db_isempty(device->db);
>> +
>> + btd_gatt_client_connected(device->client_dbus);
>> }
>>
>> static void gatt_server_init(struct btd_device *device, struct gatt_db *db)
>> diff --git a/src/gatt-client.c b/src/gatt-client.c
>> index d8a3429..225aa42 100644
>> --- a/src/gatt-client.c
>> +++ b/src/gatt-client.c
>> @@ -1825,20 +1825,14 @@ fail:
>>
>> void btd_gatt_client_ready(struct btd_gatt_client *client)
>> {
>> - struct bt_gatt_client *gatt;
>> -
>> if (!client)
>> return;
>>
>> - gatt = btd_device_get_gatt_client(client->device);
>> - if (!gatt) {
>> + if (!client->gatt) {
>> error("GATT client not initialized");
>> return;
>> }
>>
>> - bt_gatt_client_unref(client->gatt);
>> - client->gatt = bt_gatt_client_ref(gatt);
>> -
>> client->ready = true;
>>
>> DBG("GATT client ready");
>> @@ -1852,6 +1846,22 @@ void btd_gatt_client_ready(struct btd_gatt_client *client)
>> queue_foreach(client->all_notify_clients, register_notify, client);
>> }
>>
>> +void btd_gatt_client_connected(struct btd_gatt_client *client)
>> +{
>> + struct bt_gatt_client *gatt;
>> +
>> + gatt = btd_device_get_gatt_client(client->device);
>> + if (!gatt) {
>> + error("GATT client not initialized");
>> + return;
>> + }
>> +
>> + DBG("Device connected.");
>> +
>> + bt_gatt_client_unref(client->gatt);
>> + client->gatt = bt_gatt_client_ref(gatt);
>> +}
>> +
>> void btd_gatt_client_service_added(struct btd_gatt_client *client,
>> struct gatt_db_attribute *attrib)
>> {
>> diff --git a/src/gatt-client.h b/src/gatt-client.h
>> index a18db17..92a9255 100644
>> --- a/src/gatt-client.h
>> +++ b/src/gatt-client.h
>> @@ -23,6 +23,7 @@ struct btd_gatt_client *btd_gatt_client_new(struct btd_device *device);
>> void btd_gatt_client_destroy(struct btd_gatt_client *client);
>>
>> void btd_gatt_client_ready(struct btd_gatt_client *client);
>> +void btd_gatt_client_connected(struct btd_gatt_client *client);
>> void btd_gatt_client_service_added(struct btd_gatt_client *client,
>> struct gatt_db_attribute *attrib);
>> void btd_gatt_client_service_removed(struct btd_gatt_client *client,
>> --
>> 2.4.3
>>
>> --
>> 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

2015-09-09 11:42:25

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH BlueZ 1/3] shared/gatt-client: Fix not freeing notify data

Hi,

On Tue, Sep 8, 2015 at 3:23 PM, Luiz Augusto von Dentz
<[email protected]> wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> notify_data_write_ccc takes another reference in case it succeed so
> the original reference should be dropped to cause the data to be freed
> once notify_data_write_ccc completes.
> ---
> src/shared/gatt-client.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
> index 903afa7..0983852 100644
> --- a/src/shared/gatt-client.c
> +++ b/src/shared/gatt-client.c
> @@ -1596,8 +1596,7 @@ static void complete_unregister_notify(void *data)
> !notify_data->chrc->ccc_handle)
> goto done;
>
> - if (notify_data_write_ccc(notify_data, false, disable_ccc_callback))
> - return;
> + notify_data_write_ccc(notify_data, false, disable_ccc_callback);
>
> done:
> notify_data_unref(notify_data);
> --
> 2.4.3

Applied.


--
Luiz Augusto von Dentz

2015-09-08 17:40:19

by Jakub Pawlowski

[permalink] [raw]
Subject: Re: [PATCH BlueZ 2/3] core/gatt: Fix not able to read/write on reconnections

Hi Luiz,

This patch fixes all my issues with writing just after connecting.
Thanks alot!

On Tue, Sep 8, 2015 at 5:23 AM, Luiz Augusto von Dentz
<[email protected]> wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> Upon reconnecting the attributes may already be available from cache
> which mean the client would be able to issue commands while discovery
> is not complete thus btd_gatt_client_ready was not called yet.
>
> To fix now btd_gatt_client_ready is split into btd_gatt_client_connected
> which is called whenever connected leaving btd_gatt_client_ready to only
> deal with exporting services found during the discovery.
> ---
> src/device.c | 2 ++
> src/gatt-client.c | 24 +++++++++++++++++-------
> src/gatt-client.h | 1 +
> 3 files changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/src/device.c b/src/device.c
> index dc362d9..2102f23 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -4602,6 +4602,8 @@ static void gatt_client_init(struct btd_device *device)
> }
>
> device->gatt_cache_used = !gatt_db_isempty(device->db);
> +
> + btd_gatt_client_connected(device->client_dbus);
> }
>
> static void gatt_server_init(struct btd_device *device, struct gatt_db *db)
> diff --git a/src/gatt-client.c b/src/gatt-client.c
> index d8a3429..225aa42 100644
> --- a/src/gatt-client.c
> +++ b/src/gatt-client.c
> @@ -1825,20 +1825,14 @@ fail:
>
> void btd_gatt_client_ready(struct btd_gatt_client *client)
> {
> - struct bt_gatt_client *gatt;
> -
> if (!client)
> return;
>
> - gatt = btd_device_get_gatt_client(client->device);
> - if (!gatt) {
> + if (!client->gatt) {
> error("GATT client not initialized");
> return;
> }
>
> - bt_gatt_client_unref(client->gatt);
> - client->gatt = bt_gatt_client_ref(gatt);
> -
> client->ready = true;
>
> DBG("GATT client ready");
> @@ -1852,6 +1846,22 @@ void btd_gatt_client_ready(struct btd_gatt_client *client)
> queue_foreach(client->all_notify_clients, register_notify, client);
> }
>
> +void btd_gatt_client_connected(struct btd_gatt_client *client)
> +{
> + struct bt_gatt_client *gatt;
> +
> + gatt = btd_device_get_gatt_client(client->device);
> + if (!gatt) {
> + error("GATT client not initialized");
> + return;
> + }
> +
> + DBG("Device connected.");
> +
> + bt_gatt_client_unref(client->gatt);
> + client->gatt = bt_gatt_client_ref(gatt);
> +}
> +
> void btd_gatt_client_service_added(struct btd_gatt_client *client,
> struct gatt_db_attribute *attrib)
> {
> diff --git a/src/gatt-client.h b/src/gatt-client.h
> index a18db17..92a9255 100644
> --- a/src/gatt-client.h
> +++ b/src/gatt-client.h
> @@ -23,6 +23,7 @@ struct btd_gatt_client *btd_gatt_client_new(struct btd_device *device);
> void btd_gatt_client_destroy(struct btd_gatt_client *client);
>
> void btd_gatt_client_ready(struct btd_gatt_client *client);
> +void btd_gatt_client_connected(struct btd_gatt_client *client);
> void btd_gatt_client_service_added(struct btd_gatt_client *client,
> struct gatt_db_attribute *attrib);
> void btd_gatt_client_service_removed(struct btd_gatt_client *client,
> --
> 2.4.3
>
> --
> 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

2015-09-08 12:23:42

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 3/3] core/device: Only reload database if empty

From: Luiz Augusto von Dentz <[email protected]>

There is no need to reload the database once it has been populated.
---
src/device.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/device.c b/src/device.c
index 2102f23..8184508 100644
--- a/src/device.c
+++ b/src/device.c
@@ -4717,7 +4717,8 @@ bool device_attach_att(struct btd_device *dev, GIOChannel *io)
dst = device_get_address(dev);
ba2str(dst, dstaddr);

- load_gatt_db(dev, srcaddr, dstaddr);
+ if (gatt_db_isempty(dev->db))
+ load_gatt_db(dev, srcaddr, dstaddr);

gatt_client_init(dev);
gatt_server_init(dev, btd_gatt_database_get_db(database));
--
2.4.3


2015-09-08 12:23:41

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 2/3] core/gatt: Fix not able to read/write on reconnections

From: Luiz Augusto von Dentz <[email protected]>

Upon reconnecting the attributes may already be available from cache
which mean the client would be able to issue commands while discovery
is not complete thus btd_gatt_client_ready was not called yet.

To fix now btd_gatt_client_ready is split into btd_gatt_client_connected
which is called whenever connected leaving btd_gatt_client_ready to only
deal with exporting services found during the discovery.
---
src/device.c | 2 ++
src/gatt-client.c | 24 +++++++++++++++++-------
src/gatt-client.h | 1 +
3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/src/device.c b/src/device.c
index dc362d9..2102f23 100644
--- a/src/device.c
+++ b/src/device.c
@@ -4602,6 +4602,8 @@ static void gatt_client_init(struct btd_device *device)
}

device->gatt_cache_used = !gatt_db_isempty(device->db);
+
+ btd_gatt_client_connected(device->client_dbus);
}

static void gatt_server_init(struct btd_device *device, struct gatt_db *db)
diff --git a/src/gatt-client.c b/src/gatt-client.c
index d8a3429..225aa42 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -1825,20 +1825,14 @@ fail:

void btd_gatt_client_ready(struct btd_gatt_client *client)
{
- struct bt_gatt_client *gatt;
-
if (!client)
return;

- gatt = btd_device_get_gatt_client(client->device);
- if (!gatt) {
+ if (!client->gatt) {
error("GATT client not initialized");
return;
}

- bt_gatt_client_unref(client->gatt);
- client->gatt = bt_gatt_client_ref(gatt);
-
client->ready = true;

DBG("GATT client ready");
@@ -1852,6 +1846,22 @@ void btd_gatt_client_ready(struct btd_gatt_client *client)
queue_foreach(client->all_notify_clients, register_notify, client);
}

+void btd_gatt_client_connected(struct btd_gatt_client *client)
+{
+ struct bt_gatt_client *gatt;
+
+ gatt = btd_device_get_gatt_client(client->device);
+ if (!gatt) {
+ error("GATT client not initialized");
+ return;
+ }
+
+ DBG("Device connected.");
+
+ bt_gatt_client_unref(client->gatt);
+ client->gatt = bt_gatt_client_ref(gatt);
+}
+
void btd_gatt_client_service_added(struct btd_gatt_client *client,
struct gatt_db_attribute *attrib)
{
diff --git a/src/gatt-client.h b/src/gatt-client.h
index a18db17..92a9255 100644
--- a/src/gatt-client.h
+++ b/src/gatt-client.h
@@ -23,6 +23,7 @@ struct btd_gatt_client *btd_gatt_client_new(struct btd_device *device);
void btd_gatt_client_destroy(struct btd_gatt_client *client);

void btd_gatt_client_ready(struct btd_gatt_client *client);
+void btd_gatt_client_connected(struct btd_gatt_client *client);
void btd_gatt_client_service_added(struct btd_gatt_client *client,
struct gatt_db_attribute *attrib);
void btd_gatt_client_service_removed(struct btd_gatt_client *client,
--
2.4.3