2017-05-10 19:41:35

by Marcin Kraglak

[permalink] [raw]
Subject: [PATCH v2 BlueZ 1/3] shared/gatt-db: Remove services with match function

Allow remove services using match callback.
---
src/shared/gatt-db.c | 41 +++++++++++++++++++++++++++++++++++++++++
src/shared/gatt-db.h | 7 +++++++
2 files changed, 48 insertions(+)

diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
index 8ef6f3b..ded7561 100644
--- a/src/shared/gatt-db.c
+++ b/src/shared/gatt-db.c
@@ -408,6 +408,47 @@ bool gatt_db_remove_service(struct gatt_db *db,
return true;
}

+struct remove_services_data {
+ gatt_db_attribute_match_cb_t func;
+ void *user_data;
+};
+
+static bool remove_services_match(const void *data, const void *match_data)
+{
+ const struct remove_services_data *remove_svc_data = match_data;
+ const struct gatt_db_service *service = data;
+ struct gatt_db_attribute *attrib = service->attributes[0];
+
+ return remove_svc_data->func(attrib, remove_svc_data->user_data);
+}
+
+bool gatt_db_remove_services(struct gatt_db *db,
+ gatt_db_attribute_match_cb_t func,
+ void *user_data)
+{
+ if (!db)
+ return false;
+
+ if (func) {
+ struct remove_services_data remove_services_data = {
+ .func = func,
+ .user_data = user_data,
+ };
+
+ queue_remove_all(db->services, remove_services_match,
+ &remove_services_data,
+ gatt_db_service_destroy);
+ } else {
+ queue_remove_all(db->services, NULL, NULL,
+ gatt_db_service_destroy);
+ }
+
+ if (gatt_db_isempty(db))
+ db->next_handle = 0;
+
+ return true;
+}
+
bool gatt_db_clear(struct gatt_db *db)
{
return gatt_db_clear_range(db, 1, UINT16_MAX);
diff --git a/src/shared/gatt-db.h b/src/shared/gatt-db.h
index 134ec63..4e049cd 100644
--- a/src/shared/gatt-db.h
+++ b/src/shared/gatt-db.h
@@ -38,6 +38,13 @@ struct gatt_db_attribute *gatt_db_add_service(struct gatt_db *db,

bool gatt_db_remove_service(struct gatt_db *db,
struct gatt_db_attribute *attrib);
+
+typedef bool (*gatt_db_attribute_match_cb_t)(struct gatt_db_attribute *attrib,
+ void *user_data);
+
+bool gatt_db_remove_services(struct gatt_db *db,
+ gatt_db_attribute_match_cb_t func,
+ void *user_data);
bool gatt_db_clear(struct gatt_db *db);
bool gatt_db_clear_range(struct gatt_db *db, uint16_t start_handle,
uint16_t end_handle);
--
2.4.3



2017-05-11 14:22:01

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH v2 BlueZ 3/3] shared/gatt-client: Fix searching secondary services

Hi Marcin,

On Wed, May 10, 2017 at 10:41 PM, Marcin Kraglak
<[email protected]> wrote:
> Search secondary services if there is any cached primary service.
> ---
> src/shared/gatt-client.c | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
> index a919d32..fe9aaeb 100644
> --- a/src/shared/gatt-client.c
> +++ b/src/shared/gatt-client.c
> @@ -1054,6 +1054,18 @@ done:
> discovery_op_complete(op, success, att_ecode);
> }
>
> +static void count_primary_services_cb(struct gatt_db_attribute *attrib,
> + void *user_data)
> +{
> + uint16_t *service_count = (uint16_t *) user_data;
> + bool primary = false;
> +
> + gatt_db_attribute_get_service_data(attrib, NULL, NULL, &primary, NULL);
> +
> + if (primary)
> + (*service_count)++;
> +}
> +
> static void discover_primary_cb(bool success, uint8_t att_ecode,
> struct bt_gatt_result *result,
> void *user_data)
> @@ -1063,6 +1075,7 @@ static void discover_primary_cb(bool success, uint8_t att_ecode,
> struct bt_gatt_iter iter;
> struct gatt_db_attribute *attr;
> uint16_t start, end;
> + uint16_t primary_service_count = 0;
> uint128_t u128;
> bt_uuid_t uuid;
> char uuid_str[MAX_LEN_UUID_STR];
> @@ -1137,7 +1150,10 @@ secondary:
> * functionality of a device and is referenced from at least one
> * primary service on the device.
> */
> - if (queue_isempty(op->pending_svcs))
> + gatt_db_foreach_service(client->db, NULL, count_primary_services_cb,
> + &primary_service_count);
> +
> + if (primary_service_count == 0)

Im not sure I follow, if there isn't any new primary device would that
be any reason to scan for secondary? Or you are saying there could be
new secondary service that we would not be able to find without this?

> goto done;
>
> /* Discover secondary services */
> --
> 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



--
Luiz Augusto von Dentz

2017-05-11 14:24:14

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH v2 BlueZ 2/3] shared/gatt-client: Validate cached services

Hi Marcin,

On Wed, May 10, 2017 at 10:41 PM, Marcin Kraglak
<[email protected]> wrote:
> Remove services that were not found in current discovery.
> ---
> src/shared/gatt-client.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
> index 0134721..a919d32 100644
> --- a/src/shared/gatt-client.c
> +++ b/src/shared/gatt-client.c
> @@ -316,6 +316,7 @@ struct discovery_op {
> struct queue *pending_svcs;
> struct queue *pending_chrcs;
> struct queue *svcs;
> + struct queue *found_svcs;
> struct queue *ext_prop_desc;
> struct gatt_db_attribute *cur_svc;
> bool success;
> @@ -332,13 +333,31 @@ static void discovery_op_free(struct discovery_op *op)
> queue_destroy(op->pending_svcs, NULL);
> queue_destroy(op->pending_chrcs, free);
> queue_destroy(op->svcs, NULL);
> + queue_destroy(op->found_svcs, NULL);
> queue_destroy(op->ext_prop_desc, NULL);
> free(op);
> }
>
> +static bool validate_svc_cb(struct gatt_db_attribute *attrib, void *user_data)
> +{
> + struct discovery_op *op = user_data;
> + uint16_t start_h;
> +
> + gatt_db_attribute_get_service_data(attrib, &start_h, NULL, NULL, NULL);
> +
> + if (op->start > start_h || op->end < start_h)
> + return false;
> +
> + return (queue_find(op->found_svcs, NULL, attrib) == NULL);
> +}
> +
> static void discovery_op_complete(struct discovery_op *op, bool success,
> uint8_t err)
> {
> + struct bt_gatt_client *client = op->client;
> +
> + gatt_db_remove_services(client->db, validate_svc_cb, op);
> +
> /* Reset remaining range */
> if (op->last != UINT16_MAX)
> gatt_db_clear_range(op->client->db, op->last + 1, UINT16_MAX);
> @@ -358,6 +377,7 @@ static struct discovery_op *discovery_op_create(struct bt_gatt_client *client,
> op->pending_svcs = queue_new();
> op->pending_chrcs = queue_new();
> op->svcs = queue_new();
> + op->found_svcs = queue_new();
> op->ext_prop_desc = queue_new();
> op->client = client;
> op->complete_func = complete_func;
> @@ -988,6 +1008,8 @@ static void discover_secondary_cb(bool success, uint8_t att_ecode,
> if (!gatt_db_service_get_active(attr))
> queue_push_tail(op->pending_svcs, attr);
>
> + queue_push_tail(op->found_svcs, attr);
> +
> /* Update last handle */
> if (end > op->last)
> op->last = end;
> @@ -1101,6 +1123,8 @@ static void discover_primary_cb(bool success, uint8_t att_ecode,
> if (!gatt_db_service_get_active(attr))
> queue_push_tail(op->pending_svcs, attr);
>
> + queue_push_tail(op->found_svcs, attr);
> +
> /* Update last handle */
> if (end > op->last)
> op->last = end;
> --
> 2.4.3

My set will actually make this much simpler to detect, which is one of
the reason I start changing the way we do discovery, we basically can
reset services left on pending_svcs so not need to introduce a new
list.


--
Luiz Augusto von Dentz

2017-05-10 19:41:37

by Marcin Kraglak

[permalink] [raw]
Subject: [PATCH v2 BlueZ 3/3] shared/gatt-client: Fix searching secondary services

Search secondary services if there is any cached primary service.
---
src/shared/gatt-client.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index a919d32..fe9aaeb 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -1054,6 +1054,18 @@ done:
discovery_op_complete(op, success, att_ecode);
}

+static void count_primary_services_cb(struct gatt_db_attribute *attrib,
+ void *user_data)
+{
+ uint16_t *service_count = (uint16_t *) user_data;
+ bool primary = false;
+
+ gatt_db_attribute_get_service_data(attrib, NULL, NULL, &primary, NULL);
+
+ if (primary)
+ (*service_count)++;
+}
+
static void discover_primary_cb(bool success, uint8_t att_ecode,
struct bt_gatt_result *result,
void *user_data)
@@ -1063,6 +1075,7 @@ static void discover_primary_cb(bool success, uint8_t att_ecode,
struct bt_gatt_iter iter;
struct gatt_db_attribute *attr;
uint16_t start, end;
+ uint16_t primary_service_count = 0;
uint128_t u128;
bt_uuid_t uuid;
char uuid_str[MAX_LEN_UUID_STR];
@@ -1137,7 +1150,10 @@ secondary:
* functionality of a device and is referenced from at least one
* primary service on the device.
*/
- if (queue_isempty(op->pending_svcs))
+ gatt_db_foreach_service(client->db, NULL, count_primary_services_cb,
+ &primary_service_count);
+
+ if (primary_service_count == 0)
goto done;

/* Discover secondary services */
--
2.4.3


2017-05-10 19:41:36

by Marcin Kraglak

[permalink] [raw]
Subject: [PATCH v2 BlueZ 2/3] shared/gatt-client: Validate cached services

Remove services that were not found in current discovery.
---
src/shared/gatt-client.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)

diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index 0134721..a919d32 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -316,6 +316,7 @@ struct discovery_op {
struct queue *pending_svcs;
struct queue *pending_chrcs;
struct queue *svcs;
+ struct queue *found_svcs;
struct queue *ext_prop_desc;
struct gatt_db_attribute *cur_svc;
bool success;
@@ -332,13 +333,31 @@ static void discovery_op_free(struct discovery_op *op)
queue_destroy(op->pending_svcs, NULL);
queue_destroy(op->pending_chrcs, free);
queue_destroy(op->svcs, NULL);
+ queue_destroy(op->found_svcs, NULL);
queue_destroy(op->ext_prop_desc, NULL);
free(op);
}

+static bool validate_svc_cb(struct gatt_db_attribute *attrib, void *user_data)
+{
+ struct discovery_op *op = user_data;
+ uint16_t start_h;
+
+ gatt_db_attribute_get_service_data(attrib, &start_h, NULL, NULL, NULL);
+
+ if (op->start > start_h || op->end < start_h)
+ return false;
+
+ return (queue_find(op->found_svcs, NULL, attrib) == NULL);
+}
+
static void discovery_op_complete(struct discovery_op *op, bool success,
uint8_t err)
{
+ struct bt_gatt_client *client = op->client;
+
+ gatt_db_remove_services(client->db, validate_svc_cb, op);
+
/* Reset remaining range */
if (op->last != UINT16_MAX)
gatt_db_clear_range(op->client->db, op->last + 1, UINT16_MAX);
@@ -358,6 +377,7 @@ static struct discovery_op *discovery_op_create(struct bt_gatt_client *client,
op->pending_svcs = queue_new();
op->pending_chrcs = queue_new();
op->svcs = queue_new();
+ op->found_svcs = queue_new();
op->ext_prop_desc = queue_new();
op->client = client;
op->complete_func = complete_func;
@@ -988,6 +1008,8 @@ static void discover_secondary_cb(bool success, uint8_t att_ecode,
if (!gatt_db_service_get_active(attr))
queue_push_tail(op->pending_svcs, attr);

+ queue_push_tail(op->found_svcs, attr);
+
/* Update last handle */
if (end > op->last)
op->last = end;
@@ -1101,6 +1123,8 @@ static void discover_primary_cb(bool success, uint8_t att_ecode,
if (!gatt_db_service_get_active(attr))
queue_push_tail(op->pending_svcs, attr);

+ queue_push_tail(op->found_svcs, attr);
+
/* Update last handle */
if (end > op->last)
op->last = end;
--
2.4.3