Return-Path: MIME-Version: 1.0 In-Reply-To: References: <1493121371-29377-1-git-send-email-marcin.kraglak@tieto.com> From: Luiz Augusto von Dentz Date: Wed, 26 Apr 2017 12:07:50 +0300 Message-ID: Subject: Re: [PATCH BlueZ 1/2] shared/gatt-client: Remove services after Service Changed indication To: Marcin Kraglak Cc: "linux-bluetooth@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-bluetooth-owner@vger.kernel.org List-ID: Hi Marcin, On Wed, Apr 26, 2017 at 8:16 AM, Marcin Kraglak wrote: > Hi Luiz, > > On 25 April 2017 at 20:21, Luiz Augusto von Dentz wrote: >> Hi Marcin, >> >> On Tue, Apr 25, 2017 at 2:56 PM, Marcin Kraglak >> wrote: >>> Clear services in range of Service Changed indication. >>> From specification, Version 4.2 [Vol 3, Part G] 2.5.2 Attribute Caching: >>> "The client, upon receiving a Handle Value Indication containing the range of >>> affected Attribute Handles, shall consider the attribute cache invalid over >>> the affected Attribute Handle range". >>> --- >>> src/shared/gatt-client.c | 7 ++++--- >>> 1 file changed, 4 insertions(+), 3 deletions(-) >>> >>> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c >>> index 0134721..7d23702 100644 >>> --- a/src/shared/gatt-client.c >>> +++ b/src/shared/gatt-client.c >>> @@ -1475,8 +1475,6 @@ static void service_changed_complete(struct discovery_op *op, bool success, >>> util_debug(client->debug_callback, client->debug_data, >>> "Failed to discover services within changed range - " >>> "error: 0x%02x", att_ecode); >>> - >>> - gatt_db_clear_range(client->db, start_handle, end_handle); >>> } >>> >>> /* Notify the upper layer of changed services */ >>> @@ -1514,7 +1512,8 @@ static void service_changed_failure(struct discovery_op *op) >>> { >>> struct bt_gatt_client *client = op->client; >>> >>> - gatt_db_clear_range(client->db, op->start, op->end); >>> + util_debug(client->debug_callback, client->debug_data, >>> + "Failed to process Service Changed"); >>> } >>> >>> static void process_service_changed(struct bt_gatt_client *client, >>> @@ -1523,6 +1522,8 @@ static void process_service_changed(struct bt_gatt_client *client, >>> { >>> struct discovery_op *op; >>> >>> + gatt_db_clear_range(client->db, start_handle, end_handle); >>> + >>> op = discovery_op_create(client, start_handle, end_handle, >>> service_changed_complete, >>> service_changed_failure); >>> -- >>> 2.4.3 >> >> The code used to do exactly that, but it turn out it very inefficient >> with devices that just loose the entire database when rebooted so we >> started doing cache validation so we don't remove the services upfront >> but check if the service range has changed so we prevent rediscovering >> everything. >> > I see. Currently cached services, which were not found in new > discovery after reconnect/service change (they were simply removed), > are not unregistered. I guess it is not what we expect? If not I can > fix it in another patch. There is are calls to gatt_db_clear_range, so you are saying these do not affect the D-Bus objects? If that is the case then yes we need to fix it, but is it so that the services really changed because if they don't there is no reason to remove the objects. >> -- >> Luiz Augusto von Dentz > > > > -- > BR > Marcin Kraglak -- Luiz Augusto von Dentz