From: Miao-chen Chou <[email protected]>
This patch adds gatt_db_cleanup() function to clear client-role GATT DB for the
unpaired device.
---
src/device.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/src/device.c b/src/device.c
index 1b05561e4..da24a9430 100644
--- a/src/device.c
+++ b/src/device.c
@@ -267,6 +267,7 @@ struct pending_sdp_query {
static int device_browse_gatt(struct btd_device *device, DBusMessage *msg);
static int device_browse_sdp(struct btd_device *device, DBusMessage *msg);
+static void store_gatt_db(struct btd_device *device);
static struct pending_sdp_query *pending_sdp_query_new(DBusConnection *conn,
DBusMessage *msg, const struct btd_device *dev)
@@ -546,6 +547,14 @@ static void browse_request_free(struct browse_req *req)
g_free(req);
}
+static void gatt_db_cleanup(struct btd_device* device)
+{
+ if (device->bredr_state.bonded || device->le_state.bonded)
+ return;
+ gatt_db_clear(device->db);
+ store_gatt_db(device);
+}
+
static void gatt_client_cleanup(struct btd_device *device)
{
if (!device->client)
@@ -578,6 +587,7 @@ static void attio_cleanup(struct btd_device *device)
device->att_io = NULL;
}
+ gatt_db_cleanup(device);
gatt_client_cleanup(device);
gatt_server_cleanup(device);
--
2.13.0.219.gdb65acc882-goog
Hi.
On Sun, May 28, 2017 at 11:39 AM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi,
>
> On Sun, May 28, 2017 at 7:54 AM, Giovanni Ortu=C3=B1o <[email protected]=
g> wrote:
>> On Fri, May 26, 2017 at 11:10 AM, Miao-chen Chou <[email protected]> wro=
te:
>>> Hi Luiz,
>>>
>>> The user space apps follow the procedure defined in the spec to
>>> re-register the notification after re-connection for the unpaired
>>> device. Skipping the error doesn't seem to be a good workaround, since
>>> no notification is wanted unless the user space apps explicitly ask
>>> for it, and there can be other implicit notifications even if we skip
>>> one. Also, the Service Changed Characteristic does not matter in the
>>> case of unpaired devices, and there is no such check in BlueZ
>>> currently as well.
>>>
>>
>> +1
>
> BlueZ only re-register if the application had called StartNotify, and
> by _design_ StartNotify persists across reconnection, if the app do
> StopNotify at any point it would not re-register, so what you are
> describing is either a bug or Chrome OS does not call StopNotify thus
> causing bluetoothd still to believe the app want to re-register
> automatically. So the app has all the control here, the only real
> different is that we don't disable the notification on disconnect
> since StartNotify works on top of CCC and we support multiple clients,
> which should be clear by now.
>
> Btw, I did comment earlier that we could a config option to discard
> cache on disconnect.
>
>>> For other system supporting auto-connect, keeping the caches for
>>> unpaired device after disconnection can also be wrong in practice. For
>>> instance, the local device can connect to a different device which has
>>> the exact same address but with totally different GATT services, and
>>> the caching doesn't help in this case.
>
> That is only possible with random addresses, which afaik we don't
> cache since they are considered temporary the whole object is
> destroyed when disconnected:
>
>
> commit ecf2495781f735a7604df7d5d5b2d8ad5330c480
> Author: Luiz Augusto von Dentz <[email protected]>
> Date: Fri Jul 1 17:24:15 2016 +0300
>
> core/device: Don't persist private devices
>
> Device which address is private should not be allowed to reset the
> temporary flag since its settings cannot be stored and the address
> may never be used again after disconnecting.
>
>>> The first workaround that I mentioned is to prevent re-register
>>> subscribed notification clients and clear them, and the second
>>> workaround is to clear the subscribed notification during
>>> disconnection so that there won't be any clients to be re-registered
>>> for notification upon re-connection.
>
> StartNotify don't work like that, we used to remove the services but
> that caused many problems if the connection is not stable which is
> especially problematic if you have to rediscover the attributes over
> and over again.
>
>>> Thanks,
>>> Miao
>>>
>>> On Fri, May 26, 2017 at 1:42 AM, Von Dentz, Luiz
>>> <[email protected]> wrote:
>>>> Hi Miao,
>>>>
>>>> On Fri, May 26, 2017 at 10:20 AM, Miao-chen Chou <[email protected]> w=
rote:
>>>>> Hi,
>>>>>
>>>>> Thanks for your comments. Indeed the auto-connection(reconnection)
>>>>> policy is not enabled in Chrome OS. However, the caching for unpaired
>>>>> device is causing registration failures when users try to register fo=
r
>>>>> the notification after reconnecting to the device which was unpaired
>>>>> in the previous connection, and there is no such failure on Mac and
>>>>> Android afaIk. Please correct me if I was wrong, but I didn't find an=
y
>>>>> logic checking whether the service changed characteristic was
>>>>> presented in the previous connection. The problem we are trying to
>>>>> solve here is that BlueZ returns operation failed error on registerin=
g
>>>>> for notification after reconnecting to an unpaired device. And this i=
s
>>>>> caused by the caching which should be reset according to the spec.
>>>>>
>>>>> [Scenario]
>>>>> connect to the remote device without pairing
>>>>> register for notification
>>>>> disconnect from the remote device
>>>>> reconnect without pairing
>>>>> register for notification --> operation failure with "Already
>>>>> notifying" error message
>>>>
>>>> Well you could just ignore if already notifying, or that still doesn't
>>>> notify? That error perhaps is misleading as it doesn't change anything
>>>> perhaps we should return no error. Btw, if Im not mistake we do
>>>> re-enable the notifications when reconnecting:
>>>>
>>>> bluetoothd[8777]: src/gatt-client.c:register_notify() Re-register
>>>> subscribed notification client
>>>>
>>>> < ACL Data TX: Handle 3585 flags 0x00 dlen 9
>>>> ATT: Write Request (0x12) len 4
>>>> Handle: 0x0011
>>>> Data: 0100
>>>>
>>>>> Other possible workarounds without clearing GATT DB are
>>>>> 1) Skip re-register notify in btd_gatt_client_connected() and clear
>>>>> chrc->notify_clients and client->all_notify_clients for unpaired
>>>>> devices.
>>>>> 2) Clear chrc->service->client->all_notify_clients in
>>>>> btd_gatt_client_disconnected() for unpaired devices.
>>>>
>>>> See above, I think we can just remove the error of already registered
>>>> since we it should already be registered is just a NOP.
>>>>
>>>>> Any comments are appreciated.
>>>>>
>>>>> Thanks,
>>>>> Miao
>>>>>
>>>>> On Thu, May 25, 2017 at 12:44 AM, Von Dentz, Luiz
>>>>> <[email protected]> wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On Thu, May 25, 2017 at 3:10 AM, <[email protected]> wrote:
>>>>>>> From: Miao-chen Chou <[email protected]>
>>>>>>>
>>>>>>> This patch adds gatt_db_cleanup() function to clear client-role GAT=
T DB for the
>>>>>>> unpaired device. According to Core Specification v4.2 [1] and v5.0 =
[2], the
>>>>>>> attributes should be cleared between unbonded devices after disconn=
ection, since
>>>>>>> the attributes are no long valid.
>>>>>>>
>>>>>>> [1] Page 525, Section 2.5.2 Attribute Caching, Part G, Volume 3: "F=
or clients
>>>>>>> that do not have a trusted relationship with the server, the attrib=
ute cache is
>>>>>>> valid only during the connection."
>>>>>>> [2] Page 2227, Section 2.5.2 Attribute Caching, Part G, Volume 3: "=
For clients
>>>>>>> that do not have a trusted relationship with the server, the attrib=
ute cache is
>>>>>>> valid only during the connection."
>>>>>>
>>>>>> This ignores the fact that devices which don't implement service
>>>>>> changed characteristic shall not change its database so the discover=
y
>>>>>> may be skipped:
>>>>>>
>>>>>> BLUETOOTH SPECIFICATION Version 5.0 | Vol 3, Part C
>>>>>> page 2079
>>>>>> "If the client had previously determined that the server did not hav=
e
>>>>>> the =C2=ABService Changed=C2=BB characteristic then service discover=
y may be
>>>>>> skipped."
>>>>>>
>>
>> I think this spec quote is being taken out of context. That paragraph
>> specifically talks about
>> rebonding and as I understand it does not apply to all devices:
>
> I do believe many OSes, including iOS, do cache service discovery
> persistently if Service Changed is no found:
>
> https://github.com/01org/corelibs-arduino101/issues/96
>
>
>>> If a higher layer determines the bond no longer exists on the server, t=
he client must,
>>> after user interaction to confirm the remote device, re-bond, perform s=
ervice discovery
>>> and re-configure the server. If the client had previously determined th=
at the server did
>>> not have the =C2=ABService Changed=C2=BB characteristic then service di=
scovery may be skipped.
>>
>>
>>>>>> Also note that we do persist the cache to be able to persist
>>>>>> configuration such as notifications and indications, perhaps in chro=
me
>>>>>> you don't really have a use for it but in systems where reconnection
>>>>>> is supported, afaik chrome OS doesn't, it is a must have since
>>>>>> notifications and indications would not work during the discovery if
>>>>>> we remove the cache. And please remember that discovery is a very sl=
ow
>>>>>> procedure, in most cases it takes a good 3-5 seconds to rediscover,
>>>>>> depending on db size it may take over 10 seconds and until it is
>>>>>> completed no attribute can be accessed.
>>>>>>
>>
>> We recognize that rediscovery is an expensive procedure and we do have
>> projects that
>> struggle because of this but as Miao-Chen stated not following the
>> spec is not the right
>> solution. Besides, there are other initiatives inside the Bluetooth SIG =
that
>> would allow caching of the database thus solving this problem across
>> all platforms
>> while still following the spec.
>
> The spec don't say cache cannot be persited, or that you must discard
> the cache completely on disconnect, otherwise we would have a
> qualification test testing this, and no new spec will fix this for
> existing devices although we do intend to take advantage of new
> improvements in this area.
>
> For StartNotify please see above, we might add a config option to
> disable caching for non-bonding devices, other systems might still
> want to do more than just recognize this as a problem.
>
>>>>>> We could, however, have a config option to select the behavior of th=
e
>>>>>> cache or have an option for auto-connect so disabling it also disabl=
e
>>>>>> caching.
>>>>>>
>>>>>>> ---
>>>>>>> src/device.c | 13 +++++++++++++
>>>>>>> 1 file changed, 13 insertions(+)
>>>>>>>
>>>>>>> diff --git a/src/device.c b/src/device.c
>>>>>>> index 1b05561e4..f8cde0b43 100644
>>>>>>> --- a/src/device.c
>>>>>>> +++ b/src/device.c
>>>>>>> @@ -267,6 +267,7 @@ struct pending_sdp_query {
>>>>>>>
>>>>>>> static int device_browse_gatt(struct btd_device *device, DBusMessa=
ge *msg);
>>>>>>> static int device_browse_sdp(struct btd_device *device, DBusMessag=
e *msg);
>>>>>>> +static void store_gatt_db(struct btd_device *device);
>>>>>>>
>>>>>>> static struct pending_sdp_query *pending_sdp_query_new(DBusConnect=
ion *conn,
>>>>>>> DBusMessage *msg, const struct btd_=
device *dev)
>>>>>>> @@ -546,6 +547,15 @@ static void browse_request_free(struct browse_=
req *req)
>>>>>>> g_free(req);
>>>>>>> }
>>>>>>>
>>>>>>> +static void gatt_db_cleanup(struct btd_device* device)
>>>>>>> +{
>>>>>>> + if (!device->db)
>>>>>>> + return;
>>>>>>> +
>>>>>>> + gatt_db_clear(device->db);
>>>>>>> + store_gatt_db(device);
>>>>>>> +}
>>>>>>> +
>>>>>>> static void gatt_client_cleanup(struct btd_device *device)
>>>>>>> {
>>>>>>> if (!device->client)
>>>>>>> @@ -578,6 +588,9 @@ static void attio_cleanup(struct btd_device *de=
vice)
>>>>>>> device->att_io =3D NULL;
>>>>>>> }
>>>>>>>
>>>>>>> + if (!device->bredr_state.bonded && !device->le_state.bonded=
)
>>>>>>> + gatt_db_cleanup(device);
>>>>>>> +
>>>>>>> gatt_client_cleanup(device);
>>>>>>> gatt_server_cleanup(device);
Please check the patch I just send adding a config option, that should
do what you did but we retain the possibility to keep the cache always
if the system do want to have quick reconnections.
Hi,
On Sun, May 28, 2017 at 7:54 AM, Giovanni Ortu=C3=B1o <[email protected]>=
wrote:
> On Fri, May 26, 2017 at 11:10 AM, Miao-chen Chou <[email protected]> wrot=
e:
>> Hi Luiz,
>>
>> The user space apps follow the procedure defined in the spec to
>> re-register the notification after re-connection for the unpaired
>> device. Skipping the error doesn't seem to be a good workaround, since
>> no notification is wanted unless the user space apps explicitly ask
>> for it, and there can be other implicit notifications even if we skip
>> one. Also, the Service Changed Characteristic does not matter in the
>> case of unpaired devices, and there is no such check in BlueZ
>> currently as well.
>>
>
> +1
BlueZ only re-register if the application had called StartNotify, and
by _design_ StartNotify persists across reconnection, if the app do
StopNotify at any point it would not re-register, so what you are
describing is either a bug or Chrome OS does not call StopNotify thus
causing bluetoothd still to believe the app want to re-register
automatically. So the app has all the control here, the only real
different is that we don't disable the notification on disconnect
since StartNotify works on top of CCC and we support multiple clients,
which should be clear by now.
Btw, I did comment earlier that we could a config option to discard
cache on disconnect.
>> For other system supporting auto-connect, keeping the caches for
>> unpaired device after disconnection can also be wrong in practice. For
>> instance, the local device can connect to a different device which has
>> the exact same address but with totally different GATT services, and
>> the caching doesn't help in this case.
That is only possible with random addresses, which afaik we don't
cache since they are considered temporary the whole object is
destroyed when disconnected:
commit ecf2495781f735a7604df7d5d5b2d8ad5330c480
Author: Luiz Augusto von Dentz <[email protected]>
Date: Fri Jul 1 17:24:15 2016 +0300
core/device: Don't persist private devices
Device which address is private should not be allowed to reset the
temporary flag since its settings cannot be stored and the address
may never be used again after disconnecting.
>> The first workaround that I mentioned is to prevent re-register
>> subscribed notification clients and clear them, and the second
>> workaround is to clear the subscribed notification during
>> disconnection so that there won't be any clients to be re-registered
>> for notification upon re-connection.
StartNotify don't work like that, we used to remove the services but
that caused many problems if the connection is not stable which is
especially problematic if you have to rediscover the attributes over
and over again.
>> Thanks,
>> Miao
>>
>> On Fri, May 26, 2017 at 1:42 AM, Von Dentz, Luiz
>> <[email protected]> wrote:
>>> Hi Miao,
>>>
>>> On Fri, May 26, 2017 at 10:20 AM, Miao-chen Chou <[email protected]> wr=
ote:
>>>> Hi,
>>>>
>>>> Thanks for your comments. Indeed the auto-connection(reconnection)
>>>> policy is not enabled in Chrome OS. However, the caching for unpaired
>>>> device is causing registration failures when users try to register for
>>>> the notification after reconnecting to the device which was unpaired
>>>> in the previous connection, and there is no such failure on Mac and
>>>> Android afaIk. Please correct me if I was wrong, but I didn't find any
>>>> logic checking whether the service changed characteristic was
>>>> presented in the previous connection. The problem we are trying to
>>>> solve here is that BlueZ returns operation failed error on registering
>>>> for notification after reconnecting to an unpaired device. And this is
>>>> caused by the caching which should be reset according to the spec.
>>>>
>>>> [Scenario]
>>>> connect to the remote device without pairing
>>>> register for notification
>>>> disconnect from the remote device
>>>> reconnect without pairing
>>>> register for notification --> operation failure with "Already
>>>> notifying" error message
>>>
>>> Well you could just ignore if already notifying, or that still doesn't
>>> notify? That error perhaps is misleading as it doesn't change anything
>>> perhaps we should return no error. Btw, if Im not mistake we do
>>> re-enable the notifications when reconnecting:
>>>
>>> bluetoothd[8777]: src/gatt-client.c:register_notify() Re-register
>>> subscribed notification client
>>>
>>> < ACL Data TX: Handle 3585 flags 0x00 dlen 9
>>> ATT: Write Request (0x12) len 4
>>> Handle: 0x0011
>>> Data: 0100
>>>
>>>> Other possible workarounds without clearing GATT DB are
>>>> 1) Skip re-register notify in btd_gatt_client_connected() and clear
>>>> chrc->notify_clients and client->all_notify_clients for unpaired
>>>> devices.
>>>> 2) Clear chrc->service->client->all_notify_clients in
>>>> btd_gatt_client_disconnected() for unpaired devices.
>>>
>>> See above, I think we can just remove the error of already registered
>>> since we it should already be registered is just a NOP.
>>>
>>>> Any comments are appreciated.
>>>>
>>>> Thanks,
>>>> Miao
>>>>
>>>> On Thu, May 25, 2017 at 12:44 AM, Von Dentz, Luiz
>>>> <[email protected]> wrote:
>>>>> Hi,
>>>>>
>>>>> On Thu, May 25, 2017 at 3:10 AM, <[email protected]> wrote:
>>>>>> From: Miao-chen Chou <[email protected]>
>>>>>>
>>>>>> This patch adds gatt_db_cleanup() function to clear client-role GATT=
DB for the
>>>>>> unpaired device. According to Core Specification v4.2 [1] and v5.0 [=
2], the
>>>>>> attributes should be cleared between unbonded devices after disconne=
ction, since
>>>>>> the attributes are no long valid.
>>>>>>
>>>>>> [1] Page 525, Section 2.5.2 Attribute Caching, Part G, Volume 3: "Fo=
r clients
>>>>>> that do not have a trusted relationship with the server, the attribu=
te cache is
>>>>>> valid only during the connection."
>>>>>> [2] Page 2227, Section 2.5.2 Attribute Caching, Part G, Volume 3: "F=
or clients
>>>>>> that do not have a trusted relationship with the server, the attribu=
te cache is
>>>>>> valid only during the connection."
>>>>>
>>>>> This ignores the fact that devices which don't implement service
>>>>> changed characteristic shall not change its database so the discovery
>>>>> may be skipped:
>>>>>
>>>>> BLUETOOTH SPECIFICATION Version 5.0 | Vol 3, Part C
>>>>> page 2079
>>>>> "If the client had previously determined that the server did not have
>>>>> the =C2=ABService Changed=C2=BB characteristic then service discovery=
may be
>>>>> skipped."
>>>>>
>
> I think this spec quote is being taken out of context. That paragraph
> specifically talks about
> rebonding and as I understand it does not apply to all devices:
I do believe many OSes, including iOS, do cache service discovery
persistently if Service Changed is no found:
https://github.com/01org/corelibs-arduino101/issues/96
>> If a higher layer determines the bond no longer exists on the server, th=
e client must,
>> after user interaction to confirm the remote device, re-bond, perform se=
rvice discovery
>> and re-configure the server. If the client had previously determined tha=
t the server did
>> not have the =C2=ABService Changed=C2=BB characteristic then service dis=
covery may be skipped.
>
>
>>>>> Also note that we do persist the cache to be able to persist
>>>>> configuration such as notifications and indications, perhaps in chrom=
e
>>>>> you don't really have a use for it but in systems where reconnection
>>>>> is supported, afaik chrome OS doesn't, it is a must have since
>>>>> notifications and indications would not work during the discovery if
>>>>> we remove the cache. And please remember that discovery is a very slo=
w
>>>>> procedure, in most cases it takes a good 3-5 seconds to rediscover,
>>>>> depending on db size it may take over 10 seconds and until it is
>>>>> completed no attribute can be accessed.
>>>>>
>
> We recognize that rediscovery is an expensive procedure and we do have
> projects that
> struggle because of this but as Miao-Chen stated not following the
> spec is not the right
> solution. Besides, there are other initiatives inside the Bluetooth SIG t=
hat
> would allow caching of the database thus solving this problem across
> all platforms
> while still following the spec.
The spec don't say cache cannot be persited, or that you must discard
the cache completely on disconnect, otherwise we would have a
qualification test testing this, and no new spec will fix this for
existing devices although we do intend to take advantage of new
improvements in this area.
For StartNotify please see above, we might add a config option to
disable caching for non-bonding devices, other systems might still
want to do more than just recognize this as a problem.
>>>>> We could, however, have a config option to select the behavior of the
>>>>> cache or have an option for auto-connect so disabling it also disable
>>>>> caching.
>>>>>
>>>>>> ---
>>>>>> src/device.c | 13 +++++++++++++
>>>>>> 1 file changed, 13 insertions(+)
>>>>>>
>>>>>> diff --git a/src/device.c b/src/device.c
>>>>>> index 1b05561e4..f8cde0b43 100644
>>>>>> --- a/src/device.c
>>>>>> +++ b/src/device.c
>>>>>> @@ -267,6 +267,7 @@ struct pending_sdp_query {
>>>>>>
>>>>>> static int device_browse_gatt(struct btd_device *device, DBusMessag=
e *msg);
>>>>>> static int device_browse_sdp(struct btd_device *device, DBusMessage=
*msg);
>>>>>> +static void store_gatt_db(struct btd_device *device);
>>>>>>
>>>>>> static struct pending_sdp_query *pending_sdp_query_new(DBusConnecti=
on *conn,
>>>>>> DBusMessage *msg, const struct btd_d=
evice *dev)
>>>>>> @@ -546,6 +547,15 @@ static void browse_request_free(struct browse_r=
eq *req)
>>>>>> g_free(req);
>>>>>> }
>>>>>>
>>>>>> +static void gatt_db_cleanup(struct btd_device* device)
>>>>>> +{
>>>>>> + if (!device->db)
>>>>>> + return;
>>>>>> +
>>>>>> + gatt_db_clear(device->db);
>>>>>> + store_gatt_db(device);
>>>>>> +}
>>>>>> +
>>>>>> static void gatt_client_cleanup(struct btd_device *device)
>>>>>> {
>>>>>> if (!device->client)
>>>>>> @@ -578,6 +588,9 @@ static void attio_cleanup(struct btd_device *dev=
ice)
>>>>>> device->att_io =3D NULL;
>>>>>> }
>>>>>>
>>>>>> + if (!device->bredr_state.bonded && !device->le_state.bonded)
>>>>>> + gatt_db_cleanup(device);
>>>>>> +
>>>>>> gatt_client_cleanup(device);
>>>>>> gatt_server_cleanup(device);
>>>>>>
>>>>>> --
>>>>>> 2.13.0.219.gdb65acc882-goog
>>>>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetoot=
h" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> 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
--=20
Luiz Augusto von Dentz
On Fri, May 26, 2017 at 11:10 AM, Miao-chen Chou <[email protected]> wrote:
> Hi Luiz,
>
> The user space apps follow the procedure defined in the spec to
> re-register the notification after re-connection for the unpaired
> device. Skipping the error doesn't seem to be a good workaround, since
> no notification is wanted unless the user space apps explicitly ask
> for it, and there can be other implicit notifications even if we skip
> one. Also, the Service Changed Characteristic does not matter in the
> case of unpaired devices, and there is no such check in BlueZ
> currently as well.
>
+1
> For other system supporting auto-connect, keeping the caches for
> unpaired device after disconnection can also be wrong in practice. For
> instance, the local device can connect to a different device which has
> the exact same address but with totally different GATT services, and
> the caching doesn't help in this case.
>
> The first workaround that I mentioned is to prevent re-register
> subscribed notification clients and clear them, and the second
> workaround is to clear the subscribed notification during
> disconnection so that there won't be any clients to be re-registered
> for notification upon re-connection.
>
> Thanks,
> Miao
>
> On Fri, May 26, 2017 at 1:42 AM, Von Dentz, Luiz
> <[email protected]> wrote:
>> Hi Miao,
>>
>> On Fri, May 26, 2017 at 10:20 AM, Miao-chen Chou <[email protected]> wro=
te:
>>> Hi,
>>>
>>> Thanks for your comments. Indeed the auto-connection(reconnection)
>>> policy is not enabled in Chrome OS. However, the caching for unpaired
>>> device is causing registration failures when users try to register for
>>> the notification after reconnecting to the device which was unpaired
>>> in the previous connection, and there is no such failure on Mac and
>>> Android afaIk. Please correct me if I was wrong, but I didn't find any
>>> logic checking whether the service changed characteristic was
>>> presented in the previous connection. The problem we are trying to
>>> solve here is that BlueZ returns operation failed error on registering
>>> for notification after reconnecting to an unpaired device. And this is
>>> caused by the caching which should be reset according to the spec.
>>>
>>> [Scenario]
>>> connect to the remote device without pairing
>>> register for notification
>>> disconnect from the remote device
>>> reconnect without pairing
>>> register for notification --> operation failure with "Already
>>> notifying" error message
>>
>> Well you could just ignore if already notifying, or that still doesn't
>> notify? That error perhaps is misleading as it doesn't change anything
>> perhaps we should return no error. Btw, if Im not mistake we do
>> re-enable the notifications when reconnecting:
>>
>> bluetoothd[8777]: src/gatt-client.c:register_notify() Re-register
>> subscribed notification client
>>
>> < ACL Data TX: Handle 3585 flags 0x00 dlen 9
>> ATT: Write Request (0x12) len 4
>> Handle: 0x0011
>> Data: 0100
>>
>>> Other possible workarounds without clearing GATT DB are
>>> 1) Skip re-register notify in btd_gatt_client_connected() and clear
>>> chrc->notify_clients and client->all_notify_clients for unpaired
>>> devices.
>>> 2) Clear chrc->service->client->all_notify_clients in
>>> btd_gatt_client_disconnected() for unpaired devices.
>>
>> See above, I think we can just remove the error of already registered
>> since we it should already be registered is just a NOP.
>>
>>> Any comments are appreciated.
>>>
>>> Thanks,
>>> Miao
>>>
>>> On Thu, May 25, 2017 at 12:44 AM, Von Dentz, Luiz
>>> <[email protected]> wrote:
>>>> Hi,
>>>>
>>>> On Thu, May 25, 2017 at 3:10 AM, <[email protected]> wrote:
>>>>> From: Miao-chen Chou <[email protected]>
>>>>>
>>>>> This patch adds gatt_db_cleanup() function to clear client-role GATT =
DB for the
>>>>> unpaired device. According to Core Specification v4.2 [1] and v5.0 [2=
], the
>>>>> attributes should be cleared between unbonded devices after disconnec=
tion, since
>>>>> the attributes are no long valid.
>>>>>
>>>>> [1] Page 525, Section 2.5.2 Attribute Caching, Part G, Volume 3: "For=
clients
>>>>> that do not have a trusted relationship with the server, the attribut=
e cache is
>>>>> valid only during the connection."
>>>>> [2] Page 2227, Section 2.5.2 Attribute Caching, Part G, Volume 3: "Fo=
r clients
>>>>> that do not have a trusted relationship with the server, the attribut=
e cache is
>>>>> valid only during the connection."
>>>>
>>>> This ignores the fact that devices which don't implement service
>>>> changed characteristic shall not change its database so the discovery
>>>> may be skipped:
>>>>
>>>> BLUETOOTH SPECIFICATION Version 5.0 | Vol 3, Part C
>>>> page 2079
>>>> "If the client had previously determined that the server did not have
>>>> the =C2=ABService Changed=C2=BB characteristic then service discovery =
may be
>>>> skipped."
>>>>
I think this spec quote is being taken out of context. That paragraph
specifically talks about
rebonding and as I understand it does not apply to all devices:
> If a higher layer determines the bond no longer exists on the server, the=
client must,
> after user interaction to confirm the remote device, re-bond, perform ser=
vice discovery
> and re-configure the server. If the client had previously determined that=
the server did
> not have the =C2=ABService Changed=C2=BB characteristic then service disc=
overy may be skipped.
>>>> Also note that we do persist the cache to be able to persist
>>>> configuration such as notifications and indications, perhaps in chrome
>>>> you don't really have a use for it but in systems where reconnection
>>>> is supported, afaik chrome OS doesn't, it is a must have since
>>>> notifications and indications would not work during the discovery if
>>>> we remove the cache. And please remember that discovery is a very slow
>>>> procedure, in most cases it takes a good 3-5 seconds to rediscover,
>>>> depending on db size it may take over 10 seconds and until it is
>>>> completed no attribute can be accessed.
>>>>
We recognize that rediscovery is an expensive procedure and we do have
projects that
struggle because of this but as Miao-Chen stated not following the
spec is not the right
solution. Besides, there are other initiatives inside the Bluetooth SIG tha=
t
would allow caching of the database thus solving this problem across
all platforms
while still following the spec.
>>>> We could, however, have a config option to select the behavior of the
>>>> cache or have an option for auto-connect so disabling it also disable
>>>> caching.
>>>>
>>>>> ---
>>>>> src/device.c | 13 +++++++++++++
>>>>> 1 file changed, 13 insertions(+)
>>>>>
>>>>> diff --git a/src/device.c b/src/device.c
>>>>> index 1b05561e4..f8cde0b43 100644
>>>>> --- a/src/device.c
>>>>> +++ b/src/device.c
>>>>> @@ -267,6 +267,7 @@ struct pending_sdp_query {
>>>>>
>>>>> static int device_browse_gatt(struct btd_device *device, DBusMessage=
*msg);
>>>>> static int device_browse_sdp(struct btd_device *device, DBusMessage =
*msg);
>>>>> +static void store_gatt_db(struct btd_device *device);
>>>>>
>>>>> static struct pending_sdp_query *pending_sdp_query_new(DBusConnectio=
n *conn,
>>>>> DBusMessage *msg, const struct btd_de=
vice *dev)
>>>>> @@ -546,6 +547,15 @@ static void browse_request_free(struct browse_re=
q *req)
>>>>> g_free(req);
>>>>> }
>>>>>
>>>>> +static void gatt_db_cleanup(struct btd_device* device)
>>>>> +{
>>>>> + if (!device->db)
>>>>> + return;
>>>>> +
>>>>> + gatt_db_clear(device->db);
>>>>> + store_gatt_db(device);
>>>>> +}
>>>>> +
>>>>> static void gatt_client_cleanup(struct btd_device *device)
>>>>> {
>>>>> if (!device->client)
>>>>> @@ -578,6 +588,9 @@ static void attio_cleanup(struct btd_device *devi=
ce)
>>>>> device->att_io =3D NULL;
>>>>> }
>>>>>
>>>>> + if (!device->bredr_state.bonded && !device->le_state.bonded)
>>>>> + gatt_db_cleanup(device);
>>>>> +
>>>>> gatt_client_cleanup(device);
>>>>> gatt_server_cleanup(device);
>>>>>
>>>>> --
>>>>> 2.13.0.219.gdb65acc882-goog
>>>>>
> --
> 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
Hi Luiz,
The user space apps follow the procedure defined in the spec to
re-register the notification after re-connection for the unpaired
device. Skipping the error doesn't seem to be a good workaround, since
no notification is wanted unless the user space apps explicitly ask
for it, and there can be other implicit notifications even if we skip
one. Also, the Service Changed Characteristic does not matter in the
case of unpaired devices, and there is no such check in BlueZ
currently as well.
For other system supporting auto-connect, keeping the caches for
unpaired device after disconnection can also be wrong in practice. For
instance, the local device can connect to a different device which has
the exact same address but with totally different GATT services, and
the caching doesn't help in this case.
The first workaround that I mentioned is to prevent re-register
subscribed notification clients and clear them, and the second
workaround is to clear the subscribed notification during
disconnection so that there won't be any clients to be re-registered
for notification upon re-connection.
Thanks,
Miao
On Fri, May 26, 2017 at 1:42 AM, Von Dentz, Luiz
<[email protected]> wrote:
> Hi Miao,
>
> On Fri, May 26, 2017 at 10:20 AM, Miao-chen Chou <[email protected]> wrot=
e:
>> Hi,
>>
>> Thanks for your comments. Indeed the auto-connection(reconnection)
>> policy is not enabled in Chrome OS. However, the caching for unpaired
>> device is causing registration failures when users try to register for
>> the notification after reconnecting to the device which was unpaired
>> in the previous connection, and there is no such failure on Mac and
>> Android afaIk. Please correct me if I was wrong, but I didn't find any
>> logic checking whether the service changed characteristic was
>> presented in the previous connection. The problem we are trying to
>> solve here is that BlueZ returns operation failed error on registering
>> for notification after reconnecting to an unpaired device. And this is
>> caused by the caching which should be reset according to the spec.
>>
>> [Scenario]
>> connect to the remote device without pairing
>> register for notification
>> disconnect from the remote device
>> reconnect without pairing
>> register for notification --> operation failure with "Already
>> notifying" error message
>
> Well you could just ignore if already notifying, or that still doesn't
> notify? That error perhaps is misleading as it doesn't change anything
> perhaps we should return no error. Btw, if Im not mistake we do
> re-enable the notifications when reconnecting:
>
> bluetoothd[8777]: src/gatt-client.c:register_notify() Re-register
> subscribed notification client
>
> < ACL Data TX: Handle 3585 flags 0x00 dlen 9
> ATT: Write Request (0x12) len 4
> Handle: 0x0011
> Data: 0100
>
>> Other possible workarounds without clearing GATT DB are
>> 1) Skip re-register notify in btd_gatt_client_connected() and clear
>> chrc->notify_clients and client->all_notify_clients for unpaired
>> devices.
>> 2) Clear chrc->service->client->all_notify_clients in
>> btd_gatt_client_disconnected() for unpaired devices.
>
> See above, I think we can just remove the error of already registered
> since we it should already be registered is just a NOP.
>
>> Any comments are appreciated.
>>
>> Thanks,
>> Miao
>>
>> On Thu, May 25, 2017 at 12:44 AM, Von Dentz, Luiz
>> <[email protected]> wrote:
>>> Hi,
>>>
>>> On Thu, May 25, 2017 at 3:10 AM, <[email protected]> wrote:
>>>> From: Miao-chen Chou <[email protected]>
>>>>
>>>> This patch adds gatt_db_cleanup() function to clear client-role GATT D=
B for the
>>>> unpaired device. According to Core Specification v4.2 [1] and v5.0 [2]=
, the
>>>> attributes should be cleared between unbonded devices after disconnect=
ion, since
>>>> the attributes are no long valid.
>>>>
>>>> [1] Page 525, Section 2.5.2 Attribute Caching, Part G, Volume 3: "For =
clients
>>>> that do not have a trusted relationship with the server, the attribute=
cache is
>>>> valid only during the connection."
>>>> [2] Page 2227, Section 2.5.2 Attribute Caching, Part G, Volume 3: "For=
clients
>>>> that do not have a trusted relationship with the server, the attribute=
cache is
>>>> valid only during the connection."
>>>
>>> This ignores the fact that devices which don't implement service
>>> changed characteristic shall not change its database so the discovery
>>> may be skipped:
>>>
>>> BLUETOOTH SPECIFICATION Version 5.0 | Vol 3, Part C
>>> page 2079
>>> "If the client had previously determined that the server did not have
>>> the =C2=ABService Changed=C2=BB characteristic then service discovery m=
ay be
>>> skipped."
>>>
>>> Also note that we do persist the cache to be able to persist
>>> configuration such as notifications and indications, perhaps in chrome
>>> you don't really have a use for it but in systems where reconnection
>>> is supported, afaik chrome OS doesn't, it is a must have since
>>> notifications and indications would not work during the discovery if
>>> we remove the cache. And please remember that discovery is a very slow
>>> procedure, in most cases it takes a good 3-5 seconds to rediscover,
>>> depending on db size it may take over 10 seconds and until it is
>>> completed no attribute can be accessed.
>>>
>>> We could, however, have a config option to select the behavior of the
>>> cache or have an option for auto-connect so disabling it also disable
>>> caching.
>>>
>>>> ---
>>>> src/device.c | 13 +++++++++++++
>>>> 1 file changed, 13 insertions(+)
>>>>
>>>> diff --git a/src/device.c b/src/device.c
>>>> index 1b05561e4..f8cde0b43 100644
>>>> --- a/src/device.c
>>>> +++ b/src/device.c
>>>> @@ -267,6 +267,7 @@ struct pending_sdp_query {
>>>>
>>>> static int device_browse_gatt(struct btd_device *device, DBusMessage =
*msg);
>>>> static int device_browse_sdp(struct btd_device *device, DBusMessage *=
msg);
>>>> +static void store_gatt_db(struct btd_device *device);
>>>>
>>>> static struct pending_sdp_query *pending_sdp_query_new(DBusConnection=
*conn,
>>>> DBusMessage *msg, const struct btd_dev=
ice *dev)
>>>> @@ -546,6 +547,15 @@ static void browse_request_free(struct browse_req=
*req)
>>>> g_free(req);
>>>> }
>>>>
>>>> +static void gatt_db_cleanup(struct btd_device* device)
>>>> +{
>>>> + if (!device->db)
>>>> + return;
>>>> +
>>>> + gatt_db_clear(device->db);
>>>> + store_gatt_db(device);
>>>> +}
>>>> +
>>>> static void gatt_client_cleanup(struct btd_device *device)
>>>> {
>>>> if (!device->client)
>>>> @@ -578,6 +588,9 @@ static void attio_cleanup(struct btd_device *devic=
e)
>>>> device->att_io =3D NULL;
>>>> }
>>>>
>>>> + if (!device->bredr_state.bonded && !device->le_state.bonded)
>>>> + gatt_db_cleanup(device);
>>>> +
>>>> gatt_client_cleanup(device);
>>>> gatt_server_cleanup(device);
>>>>
>>>> --
>>>> 2.13.0.219.gdb65acc882-goog
>>>>
Hi Miao,
On Fri, May 26, 2017 at 10:20 AM, Miao-chen Chou <[email protected]> wrote:
> Hi,
>
> Thanks for your comments. Indeed the auto-connection(reconnection)
> policy is not enabled in Chrome OS. However, the caching for unpaired
> device is causing registration failures when users try to register for
> the notification after reconnecting to the device which was unpaired
> in the previous connection, and there is no such failure on Mac and
> Android afaIk. Please correct me if I was wrong, but I didn't find any
> logic checking whether the service changed characteristic was
> presented in the previous connection. The problem we are trying to
> solve here is that BlueZ returns operation failed error on registering
> for notification after reconnecting to an unpaired device. And this is
> caused by the caching which should be reset according to the spec.
>
> [Scenario]
> connect to the remote device without pairing
> register for notification
> disconnect from the remote device
> reconnect without pairing
> register for notification --> operation failure with "Already
> notifying" error message
Well you could just ignore if already notifying, or that still doesn't
notify? That error perhaps is misleading as it doesn't change anything
perhaps we should return no error. Btw, if Im not mistake we do
re-enable the notifications when reconnecting:
bluetoothd[8777]: src/gatt-client.c:register_notify() Re-register
subscribed notification client
< ACL Data TX: Handle 3585 flags 0x00 dlen 9
ATT: Write Request (0x12) len 4
Handle: 0x0011
Data: 0100
> Other possible workarounds without clearing GATT DB are
> 1) Skip re-register notify in btd_gatt_client_connected() and clear
> chrc->notify_clients and client->all_notify_clients for unpaired
> devices.
> 2) Clear chrc->service->client->all_notify_clients in
> btd_gatt_client_disconnected() for unpaired devices.
See above, I think we can just remove the error of already registered
since we it should already be registered is just a NOP.
> Any comments are appreciated.
>
> Thanks,
> Miao
>
> On Thu, May 25, 2017 at 12:44 AM, Von Dentz, Luiz
> <[email protected]> wrote:
>> Hi,
>>
>> On Thu, May 25, 2017 at 3:10 AM, <[email protected]> wrote:
>>> From: Miao-chen Chou <[email protected]>
>>>
>>> This patch adds gatt_db_cleanup() function to clear client-role GATT DB=
for the
>>> unpaired device. According to Core Specification v4.2 [1] and v5.0 [2],=
the
>>> attributes should be cleared between unbonded devices after disconnecti=
on, since
>>> the attributes are no long valid.
>>>
>>> [1] Page 525, Section 2.5.2 Attribute Caching, Part G, Volume 3: "For c=
lients
>>> that do not have a trusted relationship with the server, the attribute =
cache is
>>> valid only during the connection."
>>> [2] Page 2227, Section 2.5.2 Attribute Caching, Part G, Volume 3: "For =
clients
>>> that do not have a trusted relationship with the server, the attribute =
cache is
>>> valid only during the connection."
>>
>> This ignores the fact that devices which don't implement service
>> changed characteristic shall not change its database so the discovery
>> may be skipped:
>>
>> BLUETOOTH SPECIFICATION Version 5.0 | Vol 3, Part C
>> page 2079
>> "If the client had previously determined that the server did not have
>> the =C2=ABService Changed=C2=BB characteristic then service discovery ma=
y be
>> skipped."
>>
>> Also note that we do persist the cache to be able to persist
>> configuration such as notifications and indications, perhaps in chrome
>> you don't really have a use for it but in systems where reconnection
>> is supported, afaik chrome OS doesn't, it is a must have since
>> notifications and indications would not work during the discovery if
>> we remove the cache. And please remember that discovery is a very slow
>> procedure, in most cases it takes a good 3-5 seconds to rediscover,
>> depending on db size it may take over 10 seconds and until it is
>> completed no attribute can be accessed.
>>
>> We could, however, have a config option to select the behavior of the
>> cache or have an option for auto-connect so disabling it also disable
>> caching.
>>
>>> ---
>>> src/device.c | 13 +++++++++++++
>>> 1 file changed, 13 insertions(+)
>>>
>>> diff --git a/src/device.c b/src/device.c
>>> index 1b05561e4..f8cde0b43 100644
>>> --- a/src/device.c
>>> +++ b/src/device.c
>>> @@ -267,6 +267,7 @@ struct pending_sdp_query {
>>>
>>> static int device_browse_gatt(struct btd_device *device, DBusMessage *=
msg);
>>> static int device_browse_sdp(struct btd_device *device, DBusMessage *m=
sg);
>>> +static void store_gatt_db(struct btd_device *device);
>>>
>>> static struct pending_sdp_query *pending_sdp_query_new(DBusConnection =
*conn,
>>> DBusMessage *msg, const struct btd_devi=
ce *dev)
>>> @@ -546,6 +547,15 @@ static void browse_request_free(struct browse_req =
*req)
>>> g_free(req);
>>> }
>>>
>>> +static void gatt_db_cleanup(struct btd_device* device)
>>> +{
>>> + if (!device->db)
>>> + return;
>>> +
>>> + gatt_db_clear(device->db);
>>> + store_gatt_db(device);
>>> +}
>>> +
>>> static void gatt_client_cleanup(struct btd_device *device)
>>> {
>>> if (!device->client)
>>> @@ -578,6 +588,9 @@ static void attio_cleanup(struct btd_device *device=
)
>>> device->att_io =3D NULL;
>>> }
>>>
>>> + if (!device->bredr_state.bonded && !device->le_state.bonded)
>>> + gatt_db_cleanup(device);
>>> +
>>> gatt_client_cleanup(device);
>>> gatt_server_cleanup(device);
>>>
>>> --
>>> 2.13.0.219.gdb65acc882-goog
>>>
Hi,
Thanks for your comments. Indeed the auto-connection(reconnection)
policy is not enabled in Chrome OS. However, the caching for unpaired
device is causing registration failures when users try to register for
the notification after reconnecting to the device which was unpaired
in the previous connection, and there is no such failure on Mac and
Android afaIk. Please correct me if I was wrong, but I didn't find any
logic checking whether the service changed characteristic was
presented in the previous connection. The problem we are trying to
solve here is that BlueZ returns operation failed error on registering
for notification after reconnecting to an unpaired device. And this is
caused by the caching which should be reset according to the spec.
[Scenario]
connect to the remote device without pairing
register for notification
disconnect from the remote device
reconnect without pairing
register for notification --> operation failure with "Already
notifying" error message
Other possible workarounds without clearing GATT DB are
1) Skip re-register notify in btd_gatt_client_connected() and clear
chrc->notify_clients and client->all_notify_clients for unpaired
devices.
2) Clear chrc->service->client->all_notify_clients in
btd_gatt_client_disconnected() for unpaired devices.
Any comments are appreciated.
Thanks,
Miao
On Thu, May 25, 2017 at 12:44 AM, Von Dentz, Luiz
<[email protected]> wrote:
> Hi,
>
> On Thu, May 25, 2017 at 3:10 AM, <[email protected]> wrote:
>> From: Miao-chen Chou <[email protected]>
>>
>> This patch adds gatt_db_cleanup() function to clear client-role GATT DB =
for the
>> unpaired device. According to Core Specification v4.2 [1] and v5.0 [2], =
the
>> attributes should be cleared between unbonded devices after disconnectio=
n, since
>> the attributes are no long valid.
>>
>> [1] Page 525, Section 2.5.2 Attribute Caching, Part G, Volume 3: "For cl=
ients
>> that do not have a trusted relationship with the server, the attribute c=
ache is
>> valid only during the connection."
>> [2] Page 2227, Section 2.5.2 Attribute Caching, Part G, Volume 3: "For c=
lients
>> that do not have a trusted relationship with the server, the attribute c=
ache is
>> valid only during the connection."
>
> This ignores the fact that devices which don't implement service
> changed characteristic shall not change its database so the discovery
> may be skipped:
>
> BLUETOOTH SPECIFICATION Version 5.0 | Vol 3, Part C
> page 2079
> "If the client had previously determined that the server did not have
> the =C2=ABService Changed=C2=BB characteristic then service discovery may=
be
> skipped."
>
> Also note that we do persist the cache to be able to persist
> configuration such as notifications and indications, perhaps in chrome
> you don't really have a use for it but in systems where reconnection
> is supported, afaik chrome OS doesn't, it is a must have since
> notifications and indications would not work during the discovery if
> we remove the cache. And please remember that discovery is a very slow
> procedure, in most cases it takes a good 3-5 seconds to rediscover,
> depending on db size it may take over 10 seconds and until it is
> completed no attribute can be accessed.
>
> We could, however, have a config option to select the behavior of the
> cache or have an option for auto-connect so disabling it also disable
> caching.
>
>> ---
>> src/device.c | 13 +++++++++++++
>> 1 file changed, 13 insertions(+)
>>
>> diff --git a/src/device.c b/src/device.c
>> index 1b05561e4..f8cde0b43 100644
>> --- a/src/device.c
>> +++ b/src/device.c
>> @@ -267,6 +267,7 @@ struct pending_sdp_query {
>>
>> static int device_browse_gatt(struct btd_device *device, DBusMessage *m=
sg);
>> static int device_browse_sdp(struct btd_device *device, DBusMessage *ms=
g);
>> +static void store_gatt_db(struct btd_device *device);
>>
>> static struct pending_sdp_query *pending_sdp_query_new(DBusConnection *=
conn,
>> DBusMessage *msg, const struct btd_devic=
e *dev)
>> @@ -546,6 +547,15 @@ static void browse_request_free(struct browse_req *=
req)
>> g_free(req);
>> }
>>
>> +static void gatt_db_cleanup(struct btd_device* device)
>> +{
>> + if (!device->db)
>> + return;
>> +
>> + gatt_db_clear(device->db);
>> + store_gatt_db(device);
>> +}
>> +
>> static void gatt_client_cleanup(struct btd_device *device)
>> {
>> if (!device->client)
>> @@ -578,6 +588,9 @@ static void attio_cleanup(struct btd_device *device)
>> device->att_io =3D NULL;
>> }
>>
>> + if (!device->bredr_state.bonded && !device->le_state.bonded)
>> + gatt_db_cleanup(device);
>> +
>> gatt_client_cleanup(device);
>> gatt_server_cleanup(device);
>>
>> --
>> 2.13.0.219.gdb65acc882-goog
>>
Hi,
On Thu, May 25, 2017 at 3:10 AM, <[email protected]> wrote:
> From: Miao-chen Chou <[email protected]>
>
> This patch adds gatt_db_cleanup() function to clear client-role GATT DB f=
or the
> unpaired device. According to Core Specification v4.2 [1] and v5.0 [2], t=
he
> attributes should be cleared between unbonded devices after disconnection=
, since
> the attributes are no long valid.
>
> [1] Page 525, Section 2.5.2 Attribute Caching, Part G, Volume 3: "For cli=
ents
> that do not have a trusted relationship with the server, the attribute ca=
che is
> valid only during the connection."
> [2] Page 2227, Section 2.5.2 Attribute Caching, Part G, Volume 3: "For cl=
ients
> that do not have a trusted relationship with the server, the attribute ca=
che is
> valid only during the connection."
This ignores the fact that devices which don't implement service
changed characteristic shall not change its database so the discovery
may be skipped:
BLUETOOTH SPECIFICATION Version 5.0 | Vol 3, Part C
page 2079
"If the client had previously determined that the server did not have
the =C2=ABService Changed=C2=BB characteristic then service discovery may b=
e
skipped."
Also note that we do persist the cache to be able to persist
configuration such as notifications and indications, perhaps in chrome
you don't really have a use for it but in systems where reconnection
is supported, afaik chrome OS doesn't, it is a must have since
notifications and indications would not work during the discovery if
we remove the cache. And please remember that discovery is a very slow
procedure, in most cases it takes a good 3-5 seconds to rediscover,
depending on db size it may take over 10 seconds and until it is
completed no attribute can be accessed.
We could, however, have a config option to select the behavior of the
cache or have an option for auto-connect so disabling it also disable
caching.
> ---
> src/device.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/src/device.c b/src/device.c
> index 1b05561e4..f8cde0b43 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -267,6 +267,7 @@ struct pending_sdp_query {
>
> static int device_browse_gatt(struct btd_device *device, DBusMessage *ms=
g);
> static int device_browse_sdp(struct btd_device *device, DBusMessage *msg=
);
> +static void store_gatt_db(struct btd_device *device);
>
> static struct pending_sdp_query *pending_sdp_query_new(DBusConnection *c=
onn,
> DBusMessage *msg, const struct btd_device=
*dev)
> @@ -546,6 +547,15 @@ static void browse_request_free(struct browse_req *r=
eq)
> g_free(req);
> }
>
> +static void gatt_db_cleanup(struct btd_device* device)
> +{
> + if (!device->db)
> + return;
> +
> + gatt_db_clear(device->db);
> + store_gatt_db(device);
> +}
> +
> static void gatt_client_cleanup(struct btd_device *device)
> {
> if (!device->client)
> @@ -578,6 +588,9 @@ static void attio_cleanup(struct btd_device *device)
> device->att_io =3D NULL;
> }
>
> + if (!device->bredr_state.bonded && !device->le_state.bonded)
> + gatt_db_cleanup(device);
> +
> gatt_client_cleanup(device);
> gatt_server_cleanup(device);
>
> --
> 2.13.0.219.gdb65acc882-goog
>
From: Miao-chen Chou <[email protected]>
This patch adds gatt_db_cleanup() function to clear client-role GATT DB for the
unpaired device. According to Core Specification v4.2 [1] and v5.0 [2], the
attributes should be cleared between unbonded devices after disconnection, since
the attributes are no long valid.
[1] Page 525, Section 2.5.2 Attribute Caching, Part G, Volume 3: "For clients
that do not have a trusted relationship with the server, the attribute cache is
valid only during the connection."
[2] Page 2227, Section 2.5.2 Attribute Caching, Part G, Volume 3: "For clients
that do not have a trusted relationship with the server, the attribute cache is
valid only during the connection."
---
src/device.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/src/device.c b/src/device.c
index 1b05561e4..f8cde0b43 100644
--- a/src/device.c
+++ b/src/device.c
@@ -267,6 +267,7 @@ struct pending_sdp_query {
static int device_browse_gatt(struct btd_device *device, DBusMessage *msg);
static int device_browse_sdp(struct btd_device *device, DBusMessage *msg);
+static void store_gatt_db(struct btd_device *device);
static struct pending_sdp_query *pending_sdp_query_new(DBusConnection *conn,
DBusMessage *msg, const struct btd_device *dev)
@@ -546,6 +547,15 @@ static void browse_request_free(struct browse_req *req)
g_free(req);
}
+static void gatt_db_cleanup(struct btd_device* device)
+{
+ if (!device->db)
+ return;
+
+ gatt_db_clear(device->db);
+ store_gatt_db(device);
+}
+
static void gatt_client_cleanup(struct btd_device *device)
{
if (!device->client)
@@ -578,6 +588,9 @@ static void attio_cleanup(struct btd_device *device)
device->att_io = NULL;
}
+ if (!device->bredr_state.bonded && !device->le_state.bonded)
+ gatt_db_cleanup(device);
+
gatt_client_cleanup(device);
gatt_server_cleanup(device);
--
2.13.0.219.gdb65acc882-goog
From: Miao-chen Chou <[email protected]>
This patch adds gatt_db_cleanup() function to clear client-role GATT DB for the
unpaired device. According to Core Specification v4.2 [1] and v5.0 [2], the
attributes should be cleared between unbonded devices after disconnection, since
the attributes are no long valid.
[1] Page 525, Section 2.5.2 Attribute Caching, Part G, Volume 3: "For clients
that do not have a trusted relationship with the server, the attribute cache is
valid only during the connection."
[2] Page 2227, Section 2.5.2 Attribute Caching, Part G, Volume 3: "For clients
that do not have a trusted relationship with the server, the attribute cache is
valid only during the connection."
---
src/device.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/src/device.c b/src/device.c
index 1b05561e4..9a6135e47 100644
--- a/src/device.c
+++ b/src/device.c
@@ -267,6 +267,7 @@ struct pending_sdp_query {
static int device_browse_gatt(struct btd_device *device, DBusMessage *msg);
static int device_browse_sdp(struct btd_device *device, DBusMessage *msg);
+static void store_gatt_db(struct btd_device *device);
static struct pending_sdp_query *pending_sdp_query_new(DBusConnection *conn,
DBusMessage *msg, const struct btd_device *dev)
@@ -546,6 +547,15 @@ static void browse_request_free(struct browse_req *req)
g_free(req);
}
+static void gatt_db_cleanup(struct btd_device* device)
+{
+ if (!device->db)
+ return;
+
+ gatt_db_clear(device->db);
+ store_gatt_db(device);
+}
+
static void gatt_client_cleanup(struct btd_device *device)
{
if (!device->client)
@@ -578,6 +588,9 @@ static void attio_cleanup(struct btd_device *device)
device->att_io = NULL;
}
+ if (device->bredr_state.bonded || device->le_state.bonded)
+ gatt_db_cleanup(device);
+
gatt_client_cleanup(device);
gatt_server_cleanup(device);
--
2.13.0.219.gdb65acc882-goog