*v1:
- Fixed potential invalid access while obtaining end group handle.
- Fixed some compile warnings.
Arman Uguray (7):
shared/gatt-client: Fix bug in service changed queue.
shared/gatt-client: Fix bug in service changed handling.
tools/btgatt-client: Fix read-long-value offset parsing.
tools/btgatt-client: Fix off-by-one error.
tools/btgatt-client: Print prompt on service changed.
shared/gatt-db: Fix range handling in read by grp type.
TODO: Add new shared/gatt TODO items.
TODO | 15 +++++++++++++++
src/shared/gatt-client.c | 14 +++++++++-----
src/shared/gatt-db.c | 11 ++++++++++-
tools/btgatt-client.c | 11 +++++++----
4 files changed, 41 insertions(+), 10 deletions(-)
--
2.1.0.rc2.206.gedb03e5
Hi Arman,
On Mon, Oct 27, 2014 at 4:35 PM, Arman Uguray <[email protected]> wrote:
> *v1:
> - Fixed potential invalid access while obtaining end group handle.
> - Fixed some compile warnings.
>
> Arman Uguray (7):
> shared/gatt-client: Fix bug in service changed queue.
> shared/gatt-client: Fix bug in service changed handling.
> tools/btgatt-client: Fix read-long-value offset parsing.
> tools/btgatt-client: Fix off-by-one error.
> tools/btgatt-client: Print prompt on service changed.
> shared/gatt-db: Fix range handling in read by grp type.
> TODO: Add new shared/gatt TODO items.
>
> TODO | 15 +++++++++++++++
> src/shared/gatt-client.c | 14 +++++++++-----
> src/shared/gatt-db.c | 11 ++++++++++-
> tools/btgatt-client.c | 11 +++++++----
> 4 files changed, 41 insertions(+), 10 deletions(-)
>
> --
> 2.1.0.rc2.206.gedb03e5
I went ahead and applied these set and on top of it I made the
conversion to use pass 0 as base to strtol so it can auto detect the
base since this is convenient in a command line tool.
--
Luiz Augusto von Dentz
Hi Arman,
On Mon, Oct 27, 2014 at 4:35 PM, Arman Uguray <[email protected]> wrote:
> This patch fixes the way read-long-value command parses the "offset"
> argument by interpreting it as base 10 instead of 16, which is more
> intuitive.
> ---
> tools/btgatt-client.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/btgatt-client.c b/tools/btgatt-client.c
> index b7bfa24..e7cd31c 100644
> --- a/tools/btgatt-client.c
> +++ b/tools/btgatt-client.c
> @@ -486,8 +486,8 @@ static void cmd_read_long_value(struct client *cli, char *cmd_str)
> }
>
> endptr = NULL;
> - offset = strtol(argv[1], &endptr, 16);
> - if (!endptr || *endptr != '\0' || !handle) {
> + offset = strtol(argv[1], &endptr, 10);
> + if (!endptr || *endptr != '\0') {
> printf("Invalid offset: %s\n", argv[1]);
> return;
> }
> --
> 2.1.0.rc2.206.gedb03e5
Perhaps we should use base 0 here and let strtol figure what base it is:
'If base is zero or 16, the string may then include a "0x" prefix, and
the number will be read in base 16; otherwise, a zero base is taken as
10 (decimal) unless the next character is '0', in which case it is
taken as 8 (octal).'
--
Luiz Augusto von Dentz
Added items for adding packed structs for ATT PDUs and improving unit test
coverage.
---
TODO | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/TODO b/TODO
index fe5ebe5..b5a9612 100644
--- a/TODO
+++ b/TODO
@@ -105,6 +105,21 @@ ATT/GATT (new shared stack)
Priority: Medium
Complexity: C1
+- Add complete GATT test coverage in unit/test-gatt following the GATT test
+ spec. This could use shared/gatt-client and shared/gatt-server at the same
+ time to test both against eachother. We should definitely have tests for
+ gatt-server and gatt-client simultaneously on one side of the connection.
+
+ Priority: High
+ Complexity: C4
+
+- Define packed structs for ATT protocol PDUs in shared/att-types to improve
+ readability. We should probably do this once there are extensive unit tests
+ for gatt-client/gatt-server so that we don't accidentally break working code.
+
+ Priority: Medium
+ Complexity: C2
+
- Introduce a handler interface to shared/gatt-client which can be used by the
upper layer to determine when the link has been disconnected or an ATT
protocol request times out.
--
2.1.0.rc2.206.gedb03e5
The service changed handler now prints the command line prompt if the
changed service list empty.
---
tools/btgatt-client.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tools/btgatt-client.c b/tools/btgatt-client.c
index ea9acb7..5eedfd0 100644
--- a/tools/btgatt-client.c
+++ b/tools/btgatt-client.c
@@ -304,8 +304,11 @@ static void service_changed_cb(uint16_t start_handle, uint16_t end_handle,
printf("\nService Changed handled - start: 0x%04x end: 0x%04x\n",
start_handle, end_handle);
- if (!bt_gatt_service_iter_next_by_handle(&iter, start_handle, &service))
+ if (!bt_gatt_service_iter_next_by_handle(&iter, start_handle,
+ &service)) {
+ print_prompt();
return;
+ }
print_service(service);
--
2.1.0.rc2.206.gedb03e5
This patch fixes the way read-long-value command parses the "offset"
argument by interpreting it as base 10 instead of 16, which is more
intuitive.
---
tools/btgatt-client.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/btgatt-client.c b/tools/btgatt-client.c
index b7bfa24..e7cd31c 100644
--- a/tools/btgatt-client.c
+++ b/tools/btgatt-client.c
@@ -486,8 +486,8 @@ static void cmd_read_long_value(struct client *cli, char *cmd_str)
}
endptr = NULL;
- offset = strtol(argv[1], &endptr, 16);
- if (!endptr || *endptr != '\0' || !handle) {
+ offset = strtol(argv[1], &endptr, 10);
+ if (!endptr || *endptr != '\0') {
printf("Invalid offset: %s\n", argv[1]);
return;
}
--
2.1.0.rc2.206.gedb03e5
This patch fixes a bug where the upper layer never got notified of a
service changed event if no new services are added within a changed
service range. This also fixes a bug where an empty changed service list
or ATT error caused pending service changed events to not get processed.
---
src/shared/gatt-client.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index faae71c..8689368 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -961,20 +961,20 @@ static void service_changed_complete(struct discovery_op *op, bool success,
{
struct bt_gatt_client *client = op->client;
struct service_changed_op *next_sc_op;
- uint16_t start_handle, end_handle;
+ uint16_t start_handle = 0, end_handle = 0;
client->in_svc_chngd = false;
- if (!success) {
+ if (!success && att_ecode != BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND) {
util_debug(client->debug_callback, client->debug_data,
"Failed to discover services within changed range - "
"error: 0x%02x", att_ecode);
- return;
+ goto next;
}
/* No new services in the modified range */
if (!op->result_head || !op->result_tail)
- return;
+ goto next;
start_handle = op->result_head->service.start_handle;
end_handle = op->result_tail->service.end_handle;
@@ -984,6 +984,7 @@ static void service_changed_complete(struct discovery_op *op, bool success,
service_list_insert_services(&client->svc_head, &client->svc_tail,
op->result_head, op->result_tail);
+next:
/* Notify the upper layer of changed services */
if (client->svc_chngd_callback)
client->svc_chngd_callback(start_handle, end_handle,
@@ -999,7 +1000,7 @@ static void service_changed_complete(struct discovery_op *op, bool success,
}
/* Check if the GATT service is not present or has remained unchanged */
- if (!client->svc_chngd_val_handle ||
+ if (!start_handle || !client->svc_chngd_val_handle ||
client->svc_chngd_val_handle < start_handle ||
client->svc_chngd_val_handle > end_handle)
return;
--
2.1.0.rc2.206.gedb03e5
This patch fixes bug in gatt_db_read_by_grp_type in which the range end handle
was being ignored. This caused incorrect results to be sent for a database
where services exists beyond the end handle provided in the request.
---
src/shared/gatt-db.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
index b3f95d2..65c5759 100644
--- a/src/shared/gatt-db.c
+++ b/src/shared/gatt-db.c
@@ -428,6 +428,7 @@ static void read_by_group_type(void *data, void *user_data)
{
struct read_by_group_type_data *search_data = user_data;
struct gatt_db_service *service = data;
+ uint16_t grp_start, grp_end;
if (!service->active)
return;
@@ -439,7 +440,15 @@ static void read_by_group_type(void *data, void *user_data)
if (bt_uuid_cmp(&search_data->uuid, &service->attributes[0]->uuid))
return;
- if (service->attributes[0]->handle < search_data->start_handle)
+ grp_start = service->attributes[0]->handle;
+ grp_end = grp_start + service->num_handles - 1;
+
+ if (grp_end < search_data->start_handle ||
+ grp_start > search_data->end_handle)
+ return;
+
+ if (service->attributes[0]->handle < search_data->start_handle ||
+ service->attributes[0]->handle > search_data->end_handle)
return;
/* Remember size of uuid */
--
2.1.0.rc2.206.gedb03e5
This patch fixes an error in write-long-value command that caused an
extra invalid byte to get sent at the end of the long write value.
---
tools/btgatt-client.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/btgatt-client.c b/tools/btgatt-client.c
index e7cd31c..ea9acb7 100644
--- a/tools/btgatt-client.c
+++ b/tools/btgatt-client.c
@@ -709,7 +709,7 @@ static void cmd_write_long_value(struct client *cli, char *cmd_str)
return;
}
- length = argc - 1;
+ length = argc - 2;
if (length > 0) {
if (length > UINT16_MAX) {
--
2.1.0.rc2.206.gedb03e5
This patch fixes a bug where the handle range for queued service changed
events weren't getting stored. This caused service discovery to be
initiated for the 0x0000-0x0000 range, which caused an error response.
---
src/shared/gatt-client.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index 57d3e1f..faae71c 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -1095,6 +1095,9 @@ static void service_changed_cb(uint16_t value_handle, const uint8_t *value,
if (!op)
return;
+ op->start_handle = start;
+ op->end_handle = end;
+
queue_push_tail(client->svc_chngd_queue, op);
}
--
2.1.0.rc2.206.gedb03e5