Re: [PATCH] core/gatt-client: unregister DBus services on disconnect

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jakub,

On Fri, Sep 4, 2015 at 1:39 AM, Jakub Pawlowski <jpawlowski@xxxxxxxxxx> wrote:
> Currently when LE device is disconnected, all services and characteristics
> are still registered and visible through DBus. Those objects aren't
> usable, reading properties or calling methods on them is not usable at
> all. However those services cause issues for clients re-connecting to
> device that try to read/write characteristic right after connecting.
> Read/write operation right after reconnecting ends with "Not connected"
> error, due to dev->client not being ready. Workaround is to wait a fixed
> time, around 1 second after connecting before issuing read/write commands.
> Unregistering services and characteristic from  DBus after disconnect
> fixes this issue.
>
> This patch also fixes bug:
> 1. Connect to unpaired LE device, or paired LE device that have
>    "Service Changed" notifications broken (i.e. all Android phones
>    including version L).

Isn't this solve by the cache validation?

> 2. Disconnect from device, add more services to its GATT database.

Ditto.

> 3. Re-connect to the device. Discovery of primary services will find
>    newly added services, and bluetoothd will keep them in its internal
>    structures.

You are saying this is not reflected in D-Bus, iirc I tested this and
should cause new objects being added and the old being removed.

> 4. Newly addes services will never be exported to DBus due to check
>    in btd_gatt_client_ready if (queue_isempty(client->services))

Sounds like a different bug

> Having client->services cleared when disconnecting fixes this bug.

That has some shortcomings for notifications, since you always remove
the object on disconnect the configuration may stay active in the
remote then once reconnecting they would be lost, and this is used by
a lot profiles, in fact I don't think we can even implement a heart
rate properly without this behavior since it may start notifying
changes as soon it connects.

Instead what I would probably do is drop the line that checks if queue
is empty and start and using gatt_db_service_set_claimed so the next
time we can skip them. For the 'Not connected' error we may have to
find another solution, either we delay the connected signal or we
change the logic so that cache validation don't hold other commands
the problem is that we may find out the attribute was removed during
cache validation which can cause other errors but I guess there is no
better way.

> ---
>  src/gatt-client.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/src/gatt-client.c b/src/gatt-client.c
> index 3356ee4..abcec7c 100644
> --- a/src/gatt-client.c
> +++ b/src/gatt-client.c
> @@ -1951,6 +1951,8 @@ void btd_gatt_client_disconnected(struct btd_gatt_client *client)
>         queue_foreach(client->all_notify_clients, clear_notify_id, NULL);
>         queue_foreach(client->services, cancel_ops, client->gatt);
>
> +       queue_remove_all(client->services, NULL, NULL, unregister_service);
> +
>         bt_gatt_client_unref(client->gatt);
>         client->gatt = NULL;
>  }
> --
> 2.1.4




-- 
Luiz Augusto von Dentz
--
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



[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux