This patch set implements searching of included services.
It is based on previous patches to gatt-helper.
Additionally flag primary is introduced in bt_gatt_service_t
to know type of service. It will be needed in Android
gatt part too.
Included service discovery start right after primary discovery
finish. If include service declaration was found, and it was
not found in primary discovery, it's just simply added as
secondary service.
There is TODO part: we should check if included services
have other includes too, because nested includes are
allowed SPEC. We can consider recursive calls.
Comments are welcome
Marcin
Marcin Kraglak (6):
shared/gatt: Distinguish Primary from Secondary services
tools/btgatt-client: Print type of service
shared/gatt: Discover included services
shared/gatt: Add Secondary services to service_list
shared/gatt: Add gatt-client include service iterator
tools/btgatt-client: Print found include services
src/shared/gatt-client.c | 203 +++++++++++++++++++++++++++++++++++++++++++++--
src/shared/gatt-client.h | 18 +++++
tools/btgatt-client.c | 17 +++-
3 files changed, 231 insertions(+), 7 deletions(-)
--
1.9.3
Hi Arman,
On 8 October 2014 01:04, Arman Uguray <[email protected]> wrote:
> Hi Marcin,
>
>
>> If included service was found and doesn't exist on service list,
>> create Secondary Service and insert it.
>
> I guess this is somewhat of an optimization but wouldn't it be more
> robust to actually perform secondary service discovery? I mean after
> discovering all the primary services, should we also send a bunch of
> Read By Group Type requests with the secondary service UUID? You could
> modify bt_gatt_discover_primary_services so that it accepts a boolean
> parameter (perhaps rename it to bt_gatt_discover_services) and uses
> the correct UUID based on that. Do you think that would be a better
> approach?
If we discover secondary first, then we won't have to recursive search
for include services.
I didn't implement this just because it wasn't listed in gatt feature
requirements in SPEC.
But I think that is good approach. I'll send patches
>
>
>> ---
>> src/shared/gatt-client.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 51 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
>> index 16b1630..54e6fec 100644
>> --- a/src/shared/gatt-client.c
>> +++ b/src/shared/gatt-client.c
>> @@ -471,6 +471,7 @@ static void discover_incl_cb(bool success, uint8_t att_ecode,
>> &includes[i].start_handle,
>> &includes[i].end_handle,
>> includes[i].uuid)) {
>> + struct service_list *service;
>> uuid_to_string(includes[i].uuid, uuid_str);
>> util_debug(client->debug_callback, client->debug_data,
>> "handle: 0x%04x, start: 0x%04x, end: 0x%04x,"
>> @@ -478,7 +479,56 @@ static void discover_incl_cb(bool success, uint8_t att_ecode,
>> includes[i].start_handle,
>> includes[i].end_handle, uuid_str);
>>
>> - /* TODO check if included service is on list */
>> +
>> + /*
>> + * Check if included service was found previously in this
>> + * session
>> + */
>> + for (service = op->result_head; service; service =
>> + service->next) {
>> + if (service->service.start_handle ==
>> + includes[i].start_handle)
>> + break;
>> + }
>> +
>> + /*
>> + * Check if included service was found in previous session
>> + */
>> + if (!service) {
>> + for (service = client->svc_head; service; service =
>> + service->next) {
>> + if (service->service.start_handle ==
>> + includes[i].start_handle)
>> + break;
>> + }
>> + }
>> +
>> + if (!service) {
>> + service = new0(struct service_list, 1);
>> + if (!service) {
>> + free(includes);
>> + success = false;
>> + goto done;
>> + }
>> +
>> + service->service.primary = false;
>> + service->service.start_handle =
>> + includes[i].start_handle;
>> + service->service.end_handle = includes[i].end_handle;
>> + memcpy(service->service.uuid, includes[i].uuid,
>> + UUID_BYTES);
>> +
>> + service_list_insert_services(&op->result_head,
>> + &op->result_tail,
>> + service, service);
>> +
>> + /*
>> + * TODO: Newly created Secondary Service can contain
>> + * Included Services too. They should be discovered
>> + * before characteristic discovery.
>> + */
>> + }
>> +
>> i++;
>> }
>>
>> --
>> 1.9.3
>>
BR
Marcin
>> --
>> 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 Marcin,
> @@ -1635,6 +1635,36 @@ bool bt_gatt_characteristic_iter_next(struct bt_gatt_characteristic_iter *iter,
> return true;
> }
>
> +bool bt_gatt_include_service_iter_init(struct bt_gatt_include_service_iter *i,
> + const bt_gatt_service_t *service)
I would use "iter" instead of "i" to keep it consistent with the rest
of the code (here and elsewhere).
Thanks,
Arman
Hi Marcin,
> If included service was found and doesn't exist on service list,
> create Secondary Service and insert it.
I guess this is somewhat of an optimization but wouldn't it be more
robust to actually perform secondary service discovery? I mean after
discovering all the primary services, should we also send a bunch of
Read By Group Type requests with the secondary service UUID? You could
modify bt_gatt_discover_primary_services so that it accepts a boolean
parameter (perhaps rename it to bt_gatt_discover_services) and uses
the correct UUID based on that. Do you think that would be a better
approach?
> ---
> src/shared/gatt-client.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 51 insertions(+), 1 deletion(-)
>
> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
> index 16b1630..54e6fec 100644
> --- a/src/shared/gatt-client.c
> +++ b/src/shared/gatt-client.c
> @@ -471,6 +471,7 @@ static void discover_incl_cb(bool success, uint8_t att_ecode,
> &includes[i].start_handle,
> &includes[i].end_handle,
> includes[i].uuid)) {
> + struct service_list *service;
> uuid_to_string(includes[i].uuid, uuid_str);
> util_debug(client->debug_callback, client->debug_data,
> "handle: 0x%04x, start: 0x%04x, end: 0x%04x,"
> @@ -478,7 +479,56 @@ static void discover_incl_cb(bool success, uint8_t att_ecode,
> includes[i].start_handle,
> includes[i].end_handle, uuid_str);
>
> - /* TODO check if included service is on list */
> +
> + /*
> + * Check if included service was found previously in this
> + * session
> + */
> + for (service = op->result_head; service; service =
> + service->next) {
> + if (service->service.start_handle ==
> + includes[i].start_handle)
> + break;
> + }
> +
> + /*
> + * Check if included service was found in previous session
> + */
> + if (!service) {
> + for (service = client->svc_head; service; service =
> + service->next) {
> + if (service->service.start_handle ==
> + includes[i].start_handle)
> + break;
> + }
> + }
> +
> + if (!service) {
> + service = new0(struct service_list, 1);
> + if (!service) {
> + free(includes);
> + success = false;
> + goto done;
> + }
> +
> + service->service.primary = false;
> + service->service.start_handle =
> + includes[i].start_handle;
> + service->service.end_handle = includes[i].end_handle;
> + memcpy(service->service.uuid, includes[i].uuid,
> + UUID_BYTES);
> +
> + service_list_insert_services(&op->result_head,
> + &op->result_tail,
> + service, service);
> +
> + /*
> + * TODO: Newly created Secondary Service can contain
> + * Included Services too. They should be discovered
> + * before characteristic discovery.
> + */
> + }
> +
> i++;
> }
>
> --
> 1.9.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
---
src/shared/gatt-client.c | 116 +++++++++++++++++++++++++++++++++++++++++++++--
src/shared/gatt-client.h | 7 +++
2 files changed, 119 insertions(+), 4 deletions(-)
diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index 724fde2..16b1630 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -69,6 +69,8 @@ struct service_list {
bt_gatt_service_t service;
struct chrc_data *chrcs;
size_t num_chrcs;
+ bt_gatt_included_service_t *includes;
+ size_t num_includes;
struct service_list *next;
};
@@ -240,6 +242,14 @@ static void service_destroy_characteristics(struct service_list *service)
free(service->chrcs);
}
+static void service_destroy_includes(struct service_list *service)
+{
+ free(service->includes);
+
+ service->includes = NULL;
+ service->num_includes = 0;
+}
+
static void service_list_clear(struct service_list **head,
struct service_list **tail)
{
@@ -252,6 +262,7 @@ static void service_list_clear(struct service_list **head,
while (l) {
service_destroy_characteristics(l);
+ service_destroy_includes(l);
tmp = l;
l = tmp->next;
free(tmp);
@@ -280,6 +291,7 @@ static void service_list_clear_range(struct service_list **head,
}
service_destroy_characteristics(cur);
+ service_destroy_includes(cur);
if (!prev)
*head = cur->next;
@@ -413,6 +425,101 @@ static int uuid_cmp(const uint8_t uuid128[16], uint16_t uuid16)
return memcmp(uuid128, rhs_uuid, sizeof(rhs_uuid));
}
+static void discover_incl_cb(bool success, uint8_t att_ecode,
+ struct bt_gatt_result *result,
+ void *user_data)
+{
+ struct discovery_op *op = user_data;
+ struct bt_gatt_client *client = op->client;
+ struct bt_gatt_iter iter;
+ char uuid_str[MAX_LEN_UUID_STR];
+ bt_gatt_included_service_t *includes;
+ unsigned int includes_count, i;
+
+ if (!success) {
+ if (att_ecode == BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND) {
+ success = true;
+ goto next;
+ }
+
+ goto done;
+ }
+
+ if (!result || !bt_gatt_iter_init(&iter, result)) {
+ success = false;
+ goto done;
+ }
+
+ includes_count = bt_gatt_result_included_count(result);
+ if (includes_count == 0) {
+ success = false;
+ goto done;
+ }
+
+ includes = new0(bt_gatt_included_service_t, includes_count);
+ if (!includes) {
+ success = false;
+ goto done;
+ }
+
+ util_debug(client->debug_callback, client->debug_data,
+ "Included services found: %u",
+ includes_count);
+
+ i = 0;
+ while (bt_gatt_iter_next_included_service(&iter, &includes[i].handle,
+ &includes[i].start_handle,
+ &includes[i].end_handle,
+ includes[i].uuid)) {
+ uuid_to_string(includes[i].uuid, uuid_str);
+ util_debug(client->debug_callback, client->debug_data,
+ "handle: 0x%04x, start: 0x%04x, end: 0x%04x,"
+ "uuid: %s", includes[i].handle,
+ includes[i].start_handle,
+ includes[i].end_handle, uuid_str);
+
+ /* TODO check if included service is on list */
+ i++;
+ }
+
+ op->cur_service->includes = includes;
+ op->cur_service->num_includes = includes_count;
+
+next:
+ if (!op->cur_service->next) {
+ op->cur_service = op->result_head;
+ if (bt_gatt_discover_characteristics(client->att,
+ op->cur_service->service.start_handle,
+ op->cur_service->service.end_handle,
+ discover_chrcs_cb,
+ discovery_op_ref(op),
+ discovery_op_unref))
+ return;
+
+ util_debug(client->debug_callback, client->debug_data,
+ "Failed to start characteristic discovery");
+ discovery_op_unref(op);
+ success = false;
+ goto done;
+ }
+
+ op->cur_service = op->cur_service->next;
+ if (bt_gatt_discover_included_services(client->att,
+ op->cur_service->service.start_handle,
+ op->cur_service->service.end_handle,
+ discover_incl_cb,
+ discovery_op_ref(op),
+ discovery_op_unref))
+ return;
+ util_debug(client->debug_callback, client->debug_data,
+ "Failed to start included discovery");
+ discovery_op_unref(op);
+ success = false;
+
+done:
+ op->complete_func(op, success, att_ecode);
+}
+
static void discover_descs_cb(bool success, uint8_t att_ecode,
struct bt_gatt_result *result,
void *user_data)
@@ -517,6 +624,7 @@ done:
op->complete_func(op, success, att_ecode);
}
+
static void discover_chrcs_cb(bool success, uint8_t att_ecode,
struct bt_gatt_result *result,
void *user_data)
@@ -686,18 +794,18 @@ static void discover_primary_cb(bool success, uint8_t att_ecode,
if (!op->result_head)
goto done;
- /* Sequentially discover the characteristics of all services */
+ /* Sequentially discover included services */
op->cur_service = op->result_head;
- if (bt_gatt_discover_characteristics(client->att,
+ if (bt_gatt_discover_included_services(client->att,
op->cur_service->service.start_handle,
op->cur_service->service.end_handle,
- discover_chrcs_cb,
+ discover_incl_cb,
discovery_op_ref(op),
discovery_op_unref))
return;
util_debug(client->debug_callback, client->debug_data,
- "Failed to start characteristic discovery");
+ "Failed to start included services discovery");
discovery_op_unref(op);
success = false;
diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
index 22d4dc0..05b4838 100644
--- a/src/shared/gatt-client.h
+++ b/src/shared/gatt-client.h
@@ -96,6 +96,13 @@ typedef struct {
size_t num_descs;
} bt_gatt_characteristic_t;
+typedef struct {
+ uint16_t handle;
+ uint16_t start_handle;
+ uint16_t end_handle;
+ uint8_t uuid[BT_GATT_UUID_SIZE];
+} bt_gatt_included_service_t;
+
struct bt_gatt_service_iter {
struct bt_gatt_client *client;
void *ptr;
--
1.9.3
---
tools/btgatt-client.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/tools/btgatt-client.c b/tools/btgatt-client.c
index 5c692ff..a217819 100644
--- a/tools/btgatt-client.c
+++ b/tools/btgatt-client.c
@@ -174,7 +174,9 @@ static void print_uuid(const uint8_t uuid[16])
static void print_service(const bt_gatt_service_t *service)
{
struct bt_gatt_characteristic_iter iter;
+ struct bt_gatt_include_service_iter include_iter;
const bt_gatt_characteristic_t *chrc;
+ const bt_gatt_included_service_t *incl;
size_t i;
if (!bt_gatt_characteristic_iter_init(&iter, service)) {
@@ -182,12 +184,24 @@ static void print_service(const bt_gatt_service_t *service)
return;
}
+ if (!bt_gatt_include_service_iter_init(&include_iter, service)) {
+ PRLOG("Failed to initialize include service iterator\n");
+ return;
+ }
printf(COLOR_RED "service %s" COLOR_OFF " - start: 0x%04x, "
"end: 0x%04x, uuid: ",
service->primary ? "primary" : "second.",
service->start_handle, service->end_handle);
print_uuid(service->uuid);
+ while (bt_gatt_include_service_iter_next(&include_iter, &incl)) {
+ printf("\t " COLOR_GREEN "include" COLOR_OFF " - handle: "
+ "0x%04x, - start: 0x%04x, end: 0x%04x,"
+ "uuid: ", incl->handle,
+ incl->start_handle, incl->end_handle);
+ print_uuid(incl->uuid);
+ }
+
while (bt_gatt_characteristic_iter_next(&iter, &chrc)) {
printf("\t " COLOR_YELLOW "charac" COLOR_OFF
" - start: 0x%04x, end: 0x%04x, "
--
1.9.3
---
tools/btgatt-client.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/btgatt-client.c b/tools/btgatt-client.c
index d900e08..5c692ff 100644
--- a/tools/btgatt-client.c
+++ b/tools/btgatt-client.c
@@ -182,8 +182,9 @@ static void print_service(const bt_gatt_service_t *service)
return;
}
- printf(COLOR_RED "service" COLOR_OFF " - start: 0x%04x, "
+ printf(COLOR_RED "service %s" COLOR_OFF " - start: 0x%04x, "
"end: 0x%04x, uuid: ",
+ service->primary ? "primary" : "second.",
service->start_handle, service->end_handle);
print_uuid(service->uuid);
--
1.9.3
If included service was found and doesn't exist on service list,
create Secondary Service and insert it.
---
src/shared/gatt-client.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 51 insertions(+), 1 deletion(-)
diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index 16b1630..54e6fec 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -471,6 +471,7 @@ static void discover_incl_cb(bool success, uint8_t att_ecode,
&includes[i].start_handle,
&includes[i].end_handle,
includes[i].uuid)) {
+ struct service_list *service;
uuid_to_string(includes[i].uuid, uuid_str);
util_debug(client->debug_callback, client->debug_data,
"handle: 0x%04x, start: 0x%04x, end: 0x%04x,"
@@ -478,7 +479,56 @@ static void discover_incl_cb(bool success, uint8_t att_ecode,
includes[i].start_handle,
includes[i].end_handle, uuid_str);
- /* TODO check if included service is on list */
+
+ /*
+ * Check if included service was found previously in this
+ * session
+ */
+ for (service = op->result_head; service; service =
+ service->next) {
+ if (service->service.start_handle ==
+ includes[i].start_handle)
+ break;
+ }
+
+ /*
+ * Check if included service was found in previous session
+ */
+ if (!service) {
+ for (service = client->svc_head; service; service =
+ service->next) {
+ if (service->service.start_handle ==
+ includes[i].start_handle)
+ break;
+ }
+ }
+
+ if (!service) {
+ service = new0(struct service_list, 1);
+ if (!service) {
+ free(includes);
+ success = false;
+ goto done;
+ }
+
+ service->service.primary = false;
+ service->service.start_handle =
+ includes[i].start_handle;
+ service->service.end_handle = includes[i].end_handle;
+ memcpy(service->service.uuid, includes[i].uuid,
+ UUID_BYTES);
+
+ service_list_insert_services(&op->result_head,
+ &op->result_tail,
+ service, service);
+
+ /*
+ * TODO: Newly created Secondary Service can contain
+ * Included Services too. They should be discovered
+ * before characteristic discovery.
+ */
+ }
+
i++;
}
--
1.9.3
---
src/shared/gatt-client.c | 30 ++++++++++++++++++++++++++++++
src/shared/gatt-client.h | 10 ++++++++++
2 files changed, 40 insertions(+)
diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index 54e6fec..b823c28 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -1635,6 +1635,36 @@ bool bt_gatt_characteristic_iter_next(struct bt_gatt_characteristic_iter *iter,
return true;
}
+bool bt_gatt_include_service_iter_init(struct bt_gatt_include_service_iter *i,
+ const bt_gatt_service_t *service)
+{
+ if (!i || !service)
+ return false;
+
+ memset(i, 0, sizeof(*i));
+ i->service = (struct service_list *) service;
+
+ return true;
+}
+
+bool bt_gatt_include_service_iter_next(struct bt_gatt_include_service_iter *i,
+ const bt_gatt_included_service_t **incl)
+{
+ struct service_list *service;
+
+ if (!i || !incl)
+ return false;
+
+ service = i->service;
+
+ if (i->pos >= service->num_includes)
+ return false;
+
+ *incl = &service->includes[i->pos++];
+
+ return true;
+}
+
struct read_op {
bt_gatt_client_read_callback_t callback;
void *user_data;
diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
index 05b4838..df6515e 100644
--- a/src/shared/gatt-client.h
+++ b/src/shared/gatt-client.h
@@ -113,6 +113,11 @@ struct bt_gatt_characteristic_iter {
size_t pos;
};
+struct bt_gatt_include_service_iter {
+ void *service;
+ size_t pos;
+};
+
bool bt_gatt_service_iter_init(struct bt_gatt_service_iter *iter,
struct bt_gatt_client *client);
bool bt_gatt_service_iter_next(struct bt_gatt_service_iter *iter,
@@ -129,6 +134,11 @@ bool bt_gatt_characteristic_iter_init(struct bt_gatt_characteristic_iter *iter,
bool bt_gatt_characteristic_iter_next(struct bt_gatt_characteristic_iter *iter,
const bt_gatt_characteristic_t **chrc);
+bool bt_gatt_include_service_iter_init(struct bt_gatt_include_service_iter *i,
+ const bt_gatt_service_t *service);
+bool bt_gatt_include_service_iter_next(struct bt_gatt_include_service_iter *i,
+ const bt_gatt_included_service_t **inc);
+
typedef void (*bt_gatt_client_read_callback_t)(bool success, uint8_t att_ecode,
const uint8_t *value, uint16_t length,
void *user_data);
--
1.9.3
Add flag primary to distinguish service as primary or secondary.
---
src/shared/gatt-client.c | 7 +++++--
src/shared/gatt-client.h | 1 +
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index 03d722f..724fde2 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -202,7 +202,8 @@ static void mark_notify_data_invalid_if_in_range(void *data, void *user_data)
static bool service_list_add_service(struct service_list **head,
struct service_list **tail,
- uint16_t start, uint16_t end,
+ bool primary, uint16_t start,
+ uint16_t end,
uint8_t uuid[BT_GATT_UUID_SIZE])
{
struct service_list *list;
@@ -211,6 +212,7 @@ static bool service_list_add_service(struct service_list **head,
if (!list)
return false;
+ list->service.primary = primary;
list->service.start_handle = start;
list->service.end_handle = end;
memcpy(list->service.uuid, uuid, UUID_BYTES);
@@ -668,7 +670,8 @@ static void discover_primary_cb(bool success, uint8_t att_ecode,
/* Store the service */
if (!service_list_add_service(&op->result_head,
- &op->result_tail, start, end, uuid)) {
+ &op->result_tail, true, start, end,
+ uuid)) {
util_debug(client->debug_callback, client->debug_data,
"Failed to store service");
success = false;
diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
index 6807f6b..22d4dc0 100644
--- a/src/shared/gatt-client.h
+++ b/src/shared/gatt-client.h
@@ -75,6 +75,7 @@ bool bt_gatt_client_set_debug(struct bt_gatt_client *client,
bt_gatt_client_destroy_func_t destroy);
typedef struct {
+ bool primary;
uint16_t start_handle;
uint16_t end_handle;
uint8_t uuid[BT_GATT_UUID_SIZE];
--
1.9.3