2015-09-09 16:42:04

by Jakub Pawlowski

[permalink] [raw]
Subject: [PATCH] core/device: Store services after pairing.

Service caching works only for paired devices. Right now caching is
triggered only right after discovery finishes. That means that if you
connect to new device, then pair during this connection, your services
won't be cached until reconnect. This will require full service discovery
which is slow.
This patch fixes that by trying to cache services right after successful
pairing.
---
src/device.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/src/device.c b/src/device.c
index 8184508..d31620e 100644
--- a/src/device.c
+++ b/src/device.c
@@ -5315,6 +5315,11 @@ void device_bonding_complete(struct btd_device *device, uint8_t bdaddr_type,
* request
*/
if (state->svc_resolved && bonding) {
+ /* Attept to store services for this device failed because it
+ * was not paired. Now that we're paired retry. */
+ if (device->bdaddr_type != BDADDR_BREDR)
+ store_gatt_db(device);
+
g_dbus_send_reply(dbus_conn, bonding->msg, DBUS_TYPE_INVALID);
bonding_request_free(bonding);
return;
--
2.1.4



2015-09-14 16:14:10

by Jakub Pawlowski

[permalink] [raw]
Subject: Re: [PATCH] core/device: Store services after pairing.

Hi Luiz,

On Mon, Sep 14, 2015 at 6:15 AM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Jakub,
>
> On Wed, Sep 9, 2015 at 7:42 PM, Jakub Pawlowski <[email protected]> wrote:
>> Service caching works only for paired devices. Right now caching is
>> triggered only right after discovery finishes. That means that if you
>> connect to new device, then pair during this connection, your services
>> won't be cached until reconnect. This will require full service discovery
>> which is slow.
>> This patch fixes that by trying to cache services right after successful
>> pairing.
>> ---
>> src/device.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/src/device.c b/src/device.c
>> index 8184508..d31620e 100644
>> --- a/src/device.c
>> +++ b/src/device.c
>> @@ -5315,6 +5315,11 @@ void device_bonding_complete(struct btd_device *device, uint8_t bdaddr_type,
>> * request
>> */
>> if (state->svc_resolved && bonding) {
>> + /* Attept to store services for this device failed because it
>> + * was not paired. Now that we're paired retry. */
>> + if (device->bdaddr_type != BDADDR_BREDR)
>> + store_gatt_db(device);
>> +
>> g_dbus_send_reply(dbus_conn, bonding->msg, DBUS_TYPE_INVALID);
>> bonding_request_free(bonding);
>> return;
>> --
>> 2.1.4
>
> Id probably put this into gatt_client_ready_cb or even better
> gatt_services_changed since both gatt_service_added and
> gatt_service_removed should also call it.

I've moved calling store_gatt_db to gatt_services_changed, it works
well. Thanks for suggesting that.
store_gatt_db still needs to be called from device_bonding_complete to
handle case when you connected to device
first time, and pair to it after services were discovered.
gatt_services_changed is not called in this case.

> I might actually remove the
> checks for bt_gatt_client_is_ready from them because we should be able
> to operate them right away since we don't have the limitation of not
> being able to read/write anything while bt_gatt_client is not ready.

Yes, that should be beneficial for devices that properly implement
"service changed" characteristic.

>
>
> --
> Luiz Augusto von Dentz

2015-09-14 13:15:13

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH] core/device: Store services after pairing.

Hi Jakub,

On Wed, Sep 9, 2015 at 7:42 PM, Jakub Pawlowski <[email protected]> wrote:
> Service caching works only for paired devices. Right now caching is
> triggered only right after discovery finishes. That means that if you
> connect to new device, then pair during this connection, your services
> won't be cached until reconnect. This will require full service discovery
> which is slow.
> This patch fixes that by trying to cache services right after successful
> pairing.
> ---
> src/device.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/src/device.c b/src/device.c
> index 8184508..d31620e 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -5315,6 +5315,11 @@ void device_bonding_complete(struct btd_device *device, uint8_t bdaddr_type,
> * request
> */
> if (state->svc_resolved && bonding) {
> + /* Attept to store services for this device failed because it
> + * was not paired. Now that we're paired retry. */
> + if (device->bdaddr_type != BDADDR_BREDR)
> + store_gatt_db(device);
> +
> g_dbus_send_reply(dbus_conn, bonding->msg, DBUS_TYPE_INVALID);
> bonding_request_free(bonding);
> return;
> --
> 2.1.4

Id probably put this into gatt_client_ready_cb or even better
gatt_services_changed since both gatt_service_added and
gatt_service_removed should also call it. I might actually remove the
checks for bt_gatt_client_is_ready from them because we should be able
to operate them right away since we don't have the limitation of not
being able to read/write anything while bt_gatt_client is not ready.


--
Luiz Augusto von Dentz