[PATCH v5 0/3] shared/gatt: Couple fixes and improvements

[PATCH v5 0/3] shared/gatt: Couple fixes and improvements

[Łukasz Rymanowski]

I've back to work on making Android gatt to make use of shared/gatt.
Here is couple of fixes I've done to shared code when doing this work.

Android gatt still needs testing but will be available sooner or later

v2:
 - fix test-gatt for long write seesion testes
 - prepare test-gatt test to support other changes
 - do aggregation of prep writes for long write session
 - support reliable nested long write
 - start to look into characteristic extended prop for reliable session

v3:
 - rebase
 - remove patch adding notification type - that one needs to be reworked

v4:
 - Fix according to Luiz comment

v5:
 - Fix additional Luiz comments

Łukasz Rymanowski (3):
  shared/gatt-server: Add support for long write
  shared/gatt-db: Add API to get extended prop
  shared/gatt-server: Check for ext. charact. prop. on reliable session

 src/shared/gatt-db.c     |  62 +++++++++++++++++++++
 src/shared/gatt-db.h     |   4 ++
 src/shared/gatt-server.c | 139 ++++++++++++++++++++++++++++++++++++++++-------
 unit/test-gatt.c         |  14 +++++
 4 files changed, 198 insertions(+), 21 deletions(-)

-- 
2.5.0

[PATCH v5 1/3] shared/gatt-server: Add support for long write

[Łukasz Rymanowski]

With this patch long write and nested long write reliable is supported.
GATT server is responsible now to do aggregation of prep write data
for long write session.
Note: We consider long write as the consequtive prepare writes with
continues offsets.

E.g. 1

prep_write: handle 1, offset 0, value_len 10
prep_write: handle 1, offset 10, value_len 10
prep_write: handle 2, offset 0, value_len 10
prep_write: handle 2, offset 10, value_len 10

Will result with following calles to app:

exec_write: handle 1: offset 0, value_len 20
exec_write: handle 2: offset 0, value_len 20

E.g. 2

prep_write: handle 1, offset 0, value_len 10
prep_write: handle 1, offset 2, value_len 5
prep_write: handle 2, offset 0, value_len 10
prep_write: handle 2, offset 4, value_len 5

Will result with following calles to app:

exec_write: handle 1: offset 0, value_len 10
exec_write: handle 1: offset 2, value_len 5
exec_write: handle 2: offset 0, value_len 10
exec_write: handle 2: offset 4, value_len 5

E.g. 3
prep_write: handle 1, offset 0, value_len 10
prep_write: handle 1, offset 5, value_len 5
prep_write: handle 1, offset 10, value_len 6

will result with following calles to app:

exec_write: handle 1, offset 0, value 10
exec_write: handle 1, offset 5, value 11
---
 src/shared/gatt-server.c | 91 +++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 71 insertions(+), 20 deletions(-)

diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
index c41273a..a88b62f 100644
--- a/src/shared/gatt-server.c
+++ b/src/shared/gatt-server.c
@@ -1088,11 +1088,77 @@ error:
 	bt_att_send_error_rsp(server->att, opcode, 0, ecode);
 }
 
+static bool create_and_store_prep_data(struct bt_gatt_server *server,
+						uint16_t handle, uint16_t offset,
+						uint16_t length, uint8_t *value)
+{
+	struct prep_write_data *prep_data;
+
+	prep_data = new0(struct prep_write_data, 1);
+	prep_data->length = length;
+	if (prep_data->length) {
+		prep_data->value = malloc(prep_data->length);
+		if (!prep_data->value) {
+			prep_write_data_destroy(prep_data);
+			return false;
+		}
+
+		memcpy(prep_data->value, value, prep_data->length);
+	}
+
+	prep_data->server = server;
+	prep_data->handle = handle;
+	prep_data->offset = offset;
+
+	queue_push_tail(server->prep_queue, prep_data);
+
+	return true;
+}
+
+static bool make_aggregation_of_long_write_data(struct bt_gatt_server *server,
+					struct prep_write_data *prep_data,
+					uint16_t handle, uint16_t length,
+					uint8_t *value)
+{
+	uint16_t new_len;
+
+	new_len = prep_data->length + length;
+
+	prep_data->value = realloc(prep_data->value, new_len);
+	if (!prep_data->value)
+		return false;
+
+	memcpy(prep_data->value + prep_data->length, value, length);
+	prep_data->length = new_len;
+
+	return true;
+}
+
+static bool store_prep_data(struct bt_gatt_server *server,
+						uint16_t handle, uint16_t offset,
+						uint16_t length, uint8_t *value)
+{
+	struct prep_write_data *prep_data = NULL;
+
+	/*
+	 * Now lets check if prep write is a continuation of long write
+	 * If so do aggregation of data
+	 */
+	prep_data = queue_peek_tail(server->prep_queue);
+	if (prep_data && (prep_data->handle == handle) &&
+			(offset == (prep_data->length + prep_data->offset)))
+		return make_aggregation_of_long_write_data(server, prep_data,
+								handle,
+								length, value);
+
+	return create_and_store_prep_data(server, handle, offset,
+								length, value);
+}
+
 static void prep_write_cb(uint8_t opcode, const void *pdu,
 					uint16_t length, void *user_data)
 {
 	struct bt_gatt_server *server = user_data;
-	struct prep_write_data *prep_data = NULL;
 	uint16_t handle = 0;
 	uint16_t offset;
 	struct gatt_db_attribute *attr;
@@ -1126,33 +1192,18 @@ static void prep_write_cb(uint8_t opcode, const void *pdu,
 	if (ecode)
 		goto error;
 
-	prep_data = new0(struct prep_write_data, 1);
-	prep_data->length = length - 4;
-	if (prep_data->length) {
-		prep_data->value = malloc(prep_data->length);
-		if (!prep_data->value) {
-			ecode = BT_ATT_ERROR_INSUFFICIENT_RESOURCES;
-			goto error;
-		}
+	if (!store_prep_data(server, handle, offset, length - 4,
+						&((uint8_t *) pdu)[4])) {
+		ecode = BT_ATT_ERROR_INSUFFICIENT_RESOURCES;
+		goto error;
 	}
 
-	prep_data->server = server;
-	prep_data->handle = handle;
-	prep_data->offset = offset;
-	memcpy(prep_data->value, pdu + 4, prep_data->length);
-
-	queue_push_tail(server->prep_queue, prep_data);
-
 	bt_att_send(server->att, BT_ATT_OP_PREP_WRITE_RSP, pdu, length, NULL,
 								NULL, NULL);
 	return;
 
 error:
-	if (prep_data)
-		prep_write_data_destroy(prep_data);
-
 	bt_att_send_error_rsp(server->att, opcode, handle, ecode);
-
 }
 
 static void exec_next_prep_write(struct bt_gatt_server *server,
-- 
2.5.0

Re: [PATCH v5 1/3] shared/gatt-server: Add support for long write

[Luiz Augusto von Dentz]

Hi Łukasz,

On Sat, Apr 2, 2016 at 11:26 PM, Łukasz Rymanowski
<lukasz.rymanowski@codecoup.pl> wrote:
> With this patch long write and nested long write reliable is supported.
> GATT server is responsible now to do aggregation of prep write data
> for long write session.
> Note: We consider long write as the consequtive prepare writes with
> continues offsets.
>
> E.g. 1
>
> prep_write: handle 1, offset 0, value_len 10
> prep_write: handle 1, offset 10, value_len 10
> prep_write: handle 2, offset 0, value_len 10
> prep_write: handle 2, offset 10, value_len 10
>
> Will result with following calles to app:
>
> exec_write: handle 1: offset 0, value_len 20
> exec_write: handle 2: offset 0, value_len 20
>
> E.g. 2
>
> prep_write: handle 1, offset 0, value_len 10
> prep_write: handle 1, offset 2, value_len 5
> prep_write: handle 2, offset 0, value_len 10
> prep_write: handle 2, offset 4, value_len 5
>
> Will result with following calles to app:
>
> exec_write: handle 1: offset 0, value_len 10
> exec_write: handle 1: offset 2, value_len 5
> exec_write: handle 2: offset 0, value_len 10
> exec_write: handle 2: offset 4, value_len 5
>
> E.g. 3
> prep_write: handle 1, offset 0, value_len 10
> prep_write: handle 1, offset 5, value_len 5
> prep_write: handle 1, offset 10, value_len 6
>
> will result with following calles to app:
>
> exec_write: handle 1, offset 0, value 10
> exec_write: handle 1, offset 5, value 11
> ---
>  src/shared/gatt-server.c | 91 +++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 71 insertions(+), 20 deletions(-)
>
> diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
> index c41273a..a88b62f 100644
> --- a/src/shared/gatt-server.c
> +++ b/src/shared/gatt-server.c
> @@ -1088,11 +1088,77 @@ error:
>         bt_att_send_error_rsp(server->att, opcode, 0, ecode);
>  }
>
> +static bool create_and_store_prep_data(struct bt_gatt_server *server,
> +                                               uint16_t handle, uint16_t offset,
> +                                               uint16_t length, uint8_t *value)
> +{
> +       struct prep_write_data *prep_data;
> +
> +       prep_data = new0(struct prep_write_data, 1);
> +       prep_data->length = length;
> +       if (prep_data->length) {
> +               prep_data->value = malloc(prep_data->length);
> +               if (!prep_data->value) {
> +                       prep_write_data_destroy(prep_data);
> +                       return false;
> +               }
> +
> +               memcpy(prep_data->value, value, prep_data->length);
> +       }
> +
> +       prep_data->server = server;
> +       prep_data->handle = handle;
> +       prep_data->offset = offset;
> +
> +       queue_push_tail(server->prep_queue, prep_data);
> +
> +       return true;
> +}
> +
> +static bool make_aggregation_of_long_write_data(struct bt_gatt_server *server,
> +                                       struct prep_write_data *prep_data,
> +                                       uint16_t handle, uint16_t length,
> +                                       uint8_t *value)
> +{
> +       uint16_t new_len;
> +
> +       new_len = prep_data->length + length;
> +
> +       prep_data->value = realloc(prep_data->value, new_len);
> +       if (!prep_data->value)
> +               return false;
> +
> +       memcpy(prep_data->value + prep_data->length, value, length);
> +       prep_data->length = new_len;
> +
> +       return true;
> +}
> +
> +static bool store_prep_data(struct bt_gatt_server *server,
> +                                               uint16_t handle, uint16_t offset,
> +                                               uint16_t length, uint8_t *value)
> +{
> +       struct prep_write_data *prep_data = NULL;
> +
> +       /*
> +        * Now lets check if prep write is a continuation of long write
> +        * If so do aggregation of data
> +        */
> +       prep_data = queue_peek_tail(server->prep_queue);
> +       if (prep_data && (prep_data->handle == handle) &&
> +                       (offset == (prep_data->length + prep_data->offset)))
> +               return make_aggregation_of_long_write_data(server, prep_data,
> +                                                               handle,
> +                                                               length, value);
> +
> +       return create_and_store_prep_data(server, handle, offset,
> +                                                               length, value);
> +}
> +
>  static void prep_write_cb(uint8_t opcode, const void *pdu,
>                                         uint16_t length, void *user_data)
>  {
>         struct bt_gatt_server *server = user_data;
> -       struct prep_write_data *prep_data = NULL;
>         uint16_t handle = 0;
>         uint16_t offset;
>         struct gatt_db_attribute *attr;
> @@ -1126,33 +1192,18 @@ static void prep_write_cb(uint8_t opcode, const void *pdu,
>         if (ecode)
>                 goto error;
>
> -       prep_data = new0(struct prep_write_data, 1);
> -       prep_data->length = length - 4;
> -       if (prep_data->length) {
> -               prep_data->value = malloc(prep_data->length);
> -               if (!prep_data->value) {
> -                       ecode = BT_ATT_ERROR_INSUFFICIENT_RESOURCES;
> -                       goto error;
> -               }
> +       if (!store_prep_data(server, handle, offset, length - 4,
> +                                               &((uint8_t *) pdu)[4])) {
> +               ecode = BT_ATT_ERROR_INSUFFICIENT_RESOURCES;
> +               goto error;
>         }
>
> -       prep_data->server = server;
> -       prep_data->handle = handle;
> -       prep_data->offset = offset;
> -       memcpy(prep_data->value, pdu + 4, prep_data->length);
> -
> -       queue_push_tail(server->prep_queue, prep_data);
> -
>         bt_att_send(server->att, BT_ATT_OP_PREP_WRITE_RSP, pdu, length, NULL,
>                                                                 NULL, NULL);
>         return;
>
>  error:
> -       if (prep_data)
> -               prep_write_data_destroy(prep_data);
> -
>         bt_att_send_error_rsp(server->att, opcode, handle, ecode);
> -
>  }
>
>  static void exec_next_prep_write(struct bt_gatt_server *server,
> --
> 2.5.0
>

Applied after modifying a few things, please rebase.

-- 
Luiz Augusto von Dentz

[PATCH v5 2/3] shared/gatt-db: Add API to get extended prop

[Łukasz Rymanowski]

This patch adds way to get extended properties from
characteristic extended property descriptor
---
 src/shared/gatt-db.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/shared/gatt-db.h |  4 ++++
 unit/test-gatt.c     | 14 ++++++++++++
 3 files changed, 80 insertions(+)

diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
index cc49458..1a1704b 100644
--- a/src/shared/gatt-db.c
+++ b/src/shared/gatt-db.c
@@ -52,6 +52,8 @@ static const bt_uuid_t characteristic_uuid = { .type = BT_UUID16,
 					.value.u16 = GATT_CHARAC_UUID };
 static const bt_uuid_t included_service_uuid = { .type = BT_UUID16,
 					.value.u16 = GATT_INCLUDE_UUID };
+static const bt_uuid_t ext_desc_uuid = { .type = BT_UUID16,
+				.value.u16 = GATT_CHARAC_EXT_PROPER_UUID };
 
 struct gatt_db {
 	int ref_count;
@@ -1456,6 +1458,66 @@ bool gatt_db_attribute_get_service_data(const struct gatt_db_attribute *attrib,
 	return le_to_uuid(decl->value, decl->value_len, uuid);
 }
 
+struct ext_prop_data {
+	bool present;
+	uint8_t prop;
+};
+
+static void set_ext_prop_data(struct gatt_db_attribute *attrib,
+						int err, const uint8_t *value,
+						size_t length, void *user_data)
+{
+	struct ext_prop_data *ext_prop_data = user_data;
+
+	if (err || (length != sizeof(uint8_t)))
+		return;
+
+	ext_prop_data->prop = value[0];
+}
+
+static void check_reliable_supported(struct gatt_db_attribute *attrib,
+							void *user_data)
+{
+	struct ext_prop_data *ext_prop_data = user_data;
+
+	if (ext_prop_data->present)
+		return;
+
+	if (bt_uuid_cmp(&ext_desc_uuid, &attrib->uuid))
+		return;
+
+	ext_prop_data->present = true;
+	ext_prop_data->prop = gatt_db_attribute_read(attrib, 0,
+						BT_ATT_OP_READ_REQ, NULL,
+						set_ext_prop_data, user_data);
+}
+
+bool gatt_db_attribute_get_characteristic_extended_prop(
+					const struct gatt_db_attribute *attrib,
+					uint8_t *ext_prop)
+{
+	struct ext_prop_data ext_prop_data;
+
+	if (!attrib)
+		return false;
+
+	if (bt_uuid_cmp(&characteristic_uuid, &attrib->uuid))
+		return false;
+
+	memset(&ext_prop_data, 0, sizeof(ext_prop_data));
+
+	/*
+	 * Cast needed for foreach function. We do not change attrib during
+	 * this call
+	 */
+	gatt_db_service_foreach_desc((struct gatt_db_attribute *) attrib,
+						check_reliable_supported,
+						&ext_prop_data);
+
+	*ext_prop = ext_prop_data.prop;
+	return ext_prop_data.present;
+}
+
 bool gatt_db_attribute_get_char_data(const struct gatt_db_attribute *attrib,
 							uint16_t *handle,
 							uint16_t *value_handle,
diff --git a/src/shared/gatt-db.h b/src/shared/gatt-db.h
index 96cceb9..0cb713e 100644
--- a/src/shared/gatt-db.h
+++ b/src/shared/gatt-db.h
@@ -195,6 +195,10 @@ bool gatt_db_attribute_get_service_data(const struct gatt_db_attribute *attrib,
 							bool *primary,
 							bt_uuid_t *uuid);
 
+bool gatt_db_attribute_get_characteristic_extended_prop(
+					const struct gatt_db_attribute *attrib,
+					uint8_t *ext_prop);
+
 bool gatt_db_attribute_get_char_data(const struct gatt_db_attribute *attrib,
 							uint16_t *handle,
 							uint16_t *value_handle,
diff --git a/unit/test-gatt.c b/unit/test-gatt.c
index 0912348..85e4d75 100644
--- a/unit/test-gatt.c
+++ b/unit/test-gatt.c
@@ -4434,5 +4434,19 @@ int main(int argc, char *argv[])
 				0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff),
 			raw_pdu(0x01, 0x16, 0x04, 0x00, 0x03));
 
+	define_test_server("/robustness/no-reliable-characteristic", test_server,
+			ts_large_db_1, NULL,
+			raw_pdu(0x03, 0x00, 0x02),
+			raw_pdu(0x16, 0x82, 0x00, 0x00, 0x00, 0x01, 0x02, 0x03),
+			raw_pdu(0x17, 0x82, 0x00, 0x00, 0x00, 0x01, 0x02, 0x03),
+			raw_pdu(0x16, 0x25, 0x00, 0x00, 0x00, 0x01, 0x02, 0x03),
+			raw_pdu(0x17, 0x25, 0x00, 0x00, 0x00, 0x01, 0x02, 0x03),
+			raw_pdu(0x16, 0x82, 0x00, 0x03, 0x00, 0x04, 0x05, 0x06),
+			raw_pdu(0x17, 0x82, 0x00, 0x03, 0x00, 0x04, 0x05, 0x06),
+			raw_pdu(0x16, 0x25, 0x00, 0x03, 0x00, 0x04, 0x05, 0x06),
+			raw_pdu(0x17, 0x25, 0x00, 0x03, 0x00, 0x04, 0x05, 0x06),
+			raw_pdu(0x18, 0x01),
+			raw_pdu(0x01, 0x18, 0x25, 0x00, 0x06));
+
 	return tester_run();
 }
-- 
2.5.0

Re: [PATCH v5 2/3] shared/gatt-db: Add API to get extended prop

[Luiz Augusto von Dentz]

Hi Łukasz,

On Sat, Apr 2, 2016 at 11:26 PM, Łukasz Rymanowski
<lukasz.rymanowski@codecoup.pl> wrote:
> This patch adds way to get extended properties from
> characteristic extended property descriptor
> ---
>  src/shared/gatt-db.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/shared/gatt-db.h |  4 ++++
>  unit/test-gatt.c     | 14 ++++++++++++
>  3 files changed, 80 insertions(+)
>
> diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
> index cc49458..1a1704b 100644
> --- a/src/shared/gatt-db.c
> +++ b/src/shared/gatt-db.c
> @@ -52,6 +52,8 @@ static const bt_uuid_t characteristic_uuid = { .type = BT_UUID16,
>                                         .value.u16 = GATT_CHARAC_UUID };
>  static const bt_uuid_t included_service_uuid = { .type = BT_UUID16,
>                                         .value.u16 = GATT_INCLUDE_UUID };
> +static const bt_uuid_t ext_desc_uuid = { .type = BT_UUID16,
> +                               .value.u16 = GATT_CHARAC_EXT_PROPER_UUID };
>
>  struct gatt_db {
>         int ref_count;
> @@ -1456,6 +1458,66 @@ bool gatt_db_attribute_get_service_data(const struct gatt_db_attribute *attrib,
>         return le_to_uuid(decl->value, decl->value_len, uuid);
>  }
>
> +struct ext_prop_data {
> +       bool present;
> +       uint8_t prop;
> +};
> +
> +static void set_ext_prop_data(struct gatt_db_attribute *attrib,
> +                                               int err, const uint8_t *value,
> +                                               size_t length, void *user_data)
> +{
> +       struct ext_prop_data *ext_prop_data = user_data;
> +
> +       if (err || (length != sizeof(uint8_t)))
> +               return;
> +
> +       ext_prop_data->prop = value[0];
> +}
> +
> +static void check_reliable_supported(struct gatt_db_attribute *attrib,
> +                                                       void *user_data)
> +{
> +       struct ext_prop_data *ext_prop_data = user_data;
> +
> +       if (ext_prop_data->present)
> +               return;
> +
> +       if (bt_uuid_cmp(&ext_desc_uuid, &attrib->uuid))
> +               return;
> +
> +       ext_prop_data->present = true;
> +       ext_prop_data->prop = gatt_db_attribute_read(attrib, 0,
> +                                               BT_ATT_OP_READ_REQ, NULL,
> +                                               set_ext_prop_data, user_data);
> +}
> +
> +bool gatt_db_attribute_get_characteristic_extended_prop(
> +                                       const struct gatt_db_attribute *attrib,
> +                                       uint8_t *ext_prop)
> +{
> +       struct ext_prop_data ext_prop_data;
> +
> +       if (!attrib)
> +               return false;
> +
> +       if (bt_uuid_cmp(&characteristic_uuid, &attrib->uuid))
> +               return false;
> +
> +       memset(&ext_prop_data, 0, sizeof(ext_prop_data));
> +
> +       /*
> +        * Cast needed for foreach function. We do not change attrib during
> +        * this call
> +        */
> +       gatt_db_service_foreach_desc((struct gatt_db_attribute *) attrib,
> +                                               check_reliable_supported,
> +                                               &ext_prop_data);
> +
> +       *ext_prop = ext_prop_data.prop;
> +       return ext_prop_data.present;
> +}
> +
>  bool gatt_db_attribute_get_char_data(const struct gatt_db_attribute *attrib,
>                                                         uint16_t *handle,
>                                                         uint16_t *value_handle,
> diff --git a/src/shared/gatt-db.h b/src/shared/gatt-db.h
> index 96cceb9..0cb713e 100644
> --- a/src/shared/gatt-db.h
> +++ b/src/shared/gatt-db.h
> @@ -195,6 +195,10 @@ bool gatt_db_attribute_get_service_data(const struct gatt_db_attribute *attrib,
>                                                         bool *primary,
>                                                         bt_uuid_t *uuid);
>
> +bool gatt_db_attribute_get_characteristic_extended_prop(
> +                                       const struct gatt_db_attribute *attrib,
> +                                       uint8_t *ext_prop);
> +

I wonder if it wouldn't be a better idea to extend
gatt_db_attribute_get_char_data to return it there along with the
other properties?

>  bool gatt_db_attribute_get_char_data(const struct gatt_db_attribute *attrib,
>                                                         uint16_t *handle,
>                                                         uint16_t *value_handle,
> diff --git a/unit/test-gatt.c b/unit/test-gatt.c
> index 0912348..85e4d75 100644
> --- a/unit/test-gatt.c
> +++ b/unit/test-gatt.c
> @@ -4434,5 +4434,19 @@ int main(int argc, char *argv[])
>                                 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff),
>                         raw_pdu(0x01, 0x16, 0x04, 0x00, 0x03));
>
> +       define_test_server("/robustness/no-reliable-characteristic", test_server,
> +                       ts_large_db_1, NULL,
> +                       raw_pdu(0x03, 0x00, 0x02),
> +                       raw_pdu(0x16, 0x82, 0x00, 0x00, 0x00, 0x01, 0x02, 0x03),
> +                       raw_pdu(0x17, 0x82, 0x00, 0x00, 0x00, 0x01, 0x02, 0x03),
> +                       raw_pdu(0x16, 0x25, 0x00, 0x00, 0x00, 0x01, 0x02, 0x03),
> +                       raw_pdu(0x17, 0x25, 0x00, 0x00, 0x00, 0x01, 0x02, 0x03),
> +                       raw_pdu(0x16, 0x82, 0x00, 0x03, 0x00, 0x04, 0x05, 0x06),
> +                       raw_pdu(0x17, 0x82, 0x00, 0x03, 0x00, 0x04, 0x05, 0x06),
> +                       raw_pdu(0x16, 0x25, 0x00, 0x03, 0x00, 0x04, 0x05, 0x06),
> +                       raw_pdu(0x17, 0x25, 0x00, 0x03, 0x00, 0x04, 0x05, 0x06),
> +                       raw_pdu(0x18, 0x01),
> +                       raw_pdu(0x01, 0x18, 0x25, 0x00, 0x06));
> +
>         return tester_run();
>  }
> --
> 2.5.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Luiz Augusto von Dentz

Re: [PATCH v5 2/3] shared/gatt-db: Add API to get extended prop

[Łukasz Rymanowski]

Hi Luiz,



On 4 April 2016 at 15:18, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
> Hi Łukasz,
>
> On Sat, Apr 2, 2016 at 11:26 PM, Łukasz Rymanowski
> <lukasz.rymanowski@codecoup.pl> wrote:
>> This patch adds way to get extended properties from
>> characteristic extended property descriptor
>> ---
>>  src/shared/gatt-db.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  src/shared/gatt-db.h |  4 ++++
>>  unit/test-gatt.c     | 14 ++++++++++++
>>  3 files changed, 80 insertions(+)
>>
>> diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
>> index cc49458..1a1704b 100644
>> --- a/src/shared/gatt-db.c
>> +++ b/src/shared/gatt-db.c
>> @@ -52,6 +52,8 @@ static const bt_uuid_t characteristic_uuid = { .type = BT_UUID16,
>>                                         .value.u16 = GATT_CHARAC_UUID };
>>  static const bt_uuid_t included_service_uuid = { .type = BT_UUID16,
>>                                         .value.u16 = GATT_INCLUDE_UUID };
>> +static const bt_uuid_t ext_desc_uuid = { .type = BT_UUID16,
>> +                               .value.u16 = GATT_CHARAC_EXT_PROPER_UUID };
>>
>>  struct gatt_db {
>>         int ref_count;
>> @@ -1456,6 +1458,66 @@ bool gatt_db_attribute_get_service_data(const struct gatt_db_attribute *attrib,
>>         return le_to_uuid(decl->value, decl->value_len, uuid);
>>  }
>>
>> +struct ext_prop_data {
>> +       bool present;
>> +       uint8_t prop;
>> +};
>> +
>> +static void set_ext_prop_data(struct gatt_db_attribute *attrib,
>> +                                               int err, const uint8_t *value,
>> +                                               size_t length, void *user_data)
>> +{
>> +       struct ext_prop_data *ext_prop_data = user_data;
>> +
>> +       if (err || (length != sizeof(uint8_t)))
>> +               return;
>> +
>> +       ext_prop_data->prop = value[0];
>> +}
>> +
>> +static void check_reliable_supported(struct gatt_db_attribute *attrib,
>> +                                                       void *user_data)
>> +{
>> +       struct ext_prop_data *ext_prop_data = user_data;
>> +
>> +       if (ext_prop_data->present)
>> +               return;
>> +
>> +       if (bt_uuid_cmp(&ext_desc_uuid, &attrib->uuid))
>> +               return;
>> +
>> +       ext_prop_data->present = true;
>> +       ext_prop_data->prop = gatt_db_attribute_read(attrib, 0,
>> +                                               BT_ATT_OP_READ_REQ, NULL,
>> +                                               set_ext_prop_data, user_data);
>> +}
>> +
>> +bool gatt_db_attribute_get_characteristic_extended_prop(
>> +                                       const struct gatt_db_attribute *attrib,
>> +                                       uint8_t *ext_prop)
>> +{
>> +       struct ext_prop_data ext_prop_data;
>> +
>> +       if (!attrib)
>> +               return false;
>> +
>> +       if (bt_uuid_cmp(&characteristic_uuid, &attrib->uuid))
>> +               return false;
>> +
>> +       memset(&ext_prop_data, 0, sizeof(ext_prop_data));
>> +
>> +       /*
>> +        * Cast needed for foreach function. We do not change attrib during
>> +        * this call
>> +        */
>> +       gatt_db_service_foreach_desc((struct gatt_db_attribute *) attrib,
>> +                                               check_reliable_supported,
>> +                                               &ext_prop_data);
>> +
>> +       *ext_prop = ext_prop_data.prop;
>> +       return ext_prop_data.present;
>> +}
>> +
>>  bool gatt_db_attribute_get_char_data(const struct gatt_db_attribute *attrib,
>>                                                         uint16_t *handle,
>>                                                         uint16_t *value_handle,
>> diff --git a/src/shared/gatt-db.h b/src/shared/gatt-db.h
>> index 96cceb9..0cb713e 100644
>> --- a/src/shared/gatt-db.h
>> +++ b/src/shared/gatt-db.h
>> @@ -195,6 +195,10 @@ bool gatt_db_attribute_get_service_data(const struct gatt_db_attribute *attrib,
>>                                                         bool *primary,
>>                                                         bt_uuid_t *uuid);
>>
>> +bool gatt_db_attribute_get_characteristic_extended_prop(
>> +                                       const struct gatt_db_attribute *attrib,
>> +                                       uint8_t *ext_prop);
>> +
>
> I wonder if it wouldn't be a better idea to extend
> gatt_db_attribute_get_char_data to return it there along with the
> other properties?

Well we could add *ext_prop to gatt_db_attribute_get_char_data  and if
0x0 it mean there is no extended prop descriptor. Would it be OK?
If so I will do it.

Are there any other comments to this or next patch before I send next version?


>
>>  bool gatt_db_attribute_get_char_data(const struct gatt_db_attribute *attrib,
>>                                                         uint16_t *handle,
>>                                                         uint16_t *value_handle,
>> diff --git a/unit/test-gatt.c b/unit/test-gatt.c
>> index 0912348..85e4d75 100644
>> --- a/unit/test-gatt.c
>> +++ b/unit/test-gatt.c
>> @@ -4434,5 +4434,19 @@ int main(int argc, char *argv[])
>>                                 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff),
>>                         raw_pdu(0x01, 0x16, 0x04, 0x00, 0x03));
>>
>> +       define_test_server("/robustness/no-reliable-characteristic", test_server,
>> +                       ts_large_db_1, NULL,
>> +                       raw_pdu(0x03, 0x00, 0x02),
>> +                       raw_pdu(0x16, 0x82, 0x00, 0x00, 0x00, 0x01, 0x02, 0x03),
>> +                       raw_pdu(0x17, 0x82, 0x00, 0x00, 0x00, 0x01, 0x02, 0x03),
>> +                       raw_pdu(0x16, 0x25, 0x00, 0x00, 0x00, 0x01, 0x02, 0x03),
>> +                       raw_pdu(0x17, 0x25, 0x00, 0x00, 0x00, 0x01, 0x02, 0x03),
>> +                       raw_pdu(0x16, 0x82, 0x00, 0x03, 0x00, 0x04, 0x05, 0x06),
>> +                       raw_pdu(0x17, 0x82, 0x00, 0x03, 0x00, 0x04, 0x05, 0x06),
>> +                       raw_pdu(0x16, 0x25, 0x00, 0x03, 0x00, 0x04, 0x05, 0x06),
>> +                       raw_pdu(0x17, 0x25, 0x00, 0x03, 0x00, 0x04, 0x05, 0x06),
>> +                       raw_pdu(0x18, 0x01),
>> +                       raw_pdu(0x01, 0x18, 0x25, 0x00, 0x06));
>> +
>>         return tester_run();
>>  }
>> --
>> 2.5.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Luiz Augusto von Dentz



-- 
BR / Pozdrawiam
Łukasz

Re: [PATCH v5 2/3] shared/gatt-db: Add API to get extended prop

[Luiz Augusto von Dentz]

Hi Łukasz,

On Tue, Apr 5, 2016 at 10:51 AM, Łukasz Rymanowski
<lukasz.rymanowski@codecoup.pl> wrote:
> Hi Luiz,
>
>
>
> On 4 April 2016 at 15:18, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>> Hi Łukasz,
>>
>> On Sat, Apr 2, 2016 at 11:26 PM, Łukasz Rymanowski
>> <lukasz.rymanowski@codecoup.pl> wrote:
>>> This patch adds way to get extended properties from
>>> characteristic extended property descriptor
>>> ---
>>>  src/shared/gatt-db.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  src/shared/gatt-db.h |  4 ++++
>>>  unit/test-gatt.c     | 14 ++++++++++++
>>>  3 files changed, 80 insertions(+)
>>>
>>> diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
>>> index cc49458..1a1704b 100644
>>> --- a/src/shared/gatt-db.c
>>> +++ b/src/shared/gatt-db.c
>>> @@ -52,6 +52,8 @@ static const bt_uuid_t characteristic_uuid = { .type = BT_UUID16,
>>>                                         .value.u16 = GATT_CHARAC_UUID };
>>>  static const bt_uuid_t included_service_uuid = { .type = BT_UUID16,
>>>                                         .value.u16 = GATT_INCLUDE_UUID };
>>> +static const bt_uuid_t ext_desc_uuid = { .type = BT_UUID16,
>>> +                               .value.u16 = GATT_CHARAC_EXT_PROPER_UUID };
>>>
>>>  struct gatt_db {
>>>         int ref_count;
>>> @@ -1456,6 +1458,66 @@ bool gatt_db_attribute_get_service_data(const struct gatt_db_attribute *attrib,
>>>         return le_to_uuid(decl->value, decl->value_len, uuid);
>>>  }
>>>
>>> +struct ext_prop_data {
>>> +       bool present;
>>> +       uint8_t prop;
>>> +};
>>> +
>>> +static void set_ext_prop_data(struct gatt_db_attribute *attrib,
>>> +                                               int err, const uint8_t *value,
>>> +                                               size_t length, void *user_data)
>>> +{
>>> +       struct ext_prop_data *ext_prop_data = user_data;
>>> +
>>> +       if (err || (length != sizeof(uint8_t)))
>>> +               return;
>>> +
>>> +       ext_prop_data->prop = value[0];
>>> +}
>>> +
>>> +static void check_reliable_supported(struct gatt_db_attribute *attrib,
>>> +                                                       void *user_data)
>>> +{
>>> +       struct ext_prop_data *ext_prop_data = user_data;
>>> +
>>> +       if (ext_prop_data->present)
>>> +               return;
>>> +
>>> +       if (bt_uuid_cmp(&ext_desc_uuid, &attrib->uuid))
>>> +               return;
>>> +
>>> +       ext_prop_data->present = true;
>>> +       ext_prop_data->prop = gatt_db_attribute_read(attrib, 0,
>>> +                                               BT_ATT_OP_READ_REQ, NULL,
>>> +                                               set_ext_prop_data, user_data);
>>> +}
>>> +
>>> +bool gatt_db_attribute_get_characteristic_extended_prop(
>>> +                                       const struct gatt_db_attribute *attrib,
>>> +                                       uint8_t *ext_prop)
>>> +{
>>> +       struct ext_prop_data ext_prop_data;
>>> +
>>> +       if (!attrib)
>>> +               return false;
>>> +
>>> +       if (bt_uuid_cmp(&characteristic_uuid, &attrib->uuid))
>>> +               return false;
>>> +
>>> +       memset(&ext_prop_data, 0, sizeof(ext_prop_data));
>>> +
>>> +       /*
>>> +        * Cast needed for foreach function. We do not change attrib during
>>> +        * this call
>>> +        */
>>> +       gatt_db_service_foreach_desc((struct gatt_db_attribute *) attrib,
>>> +                                               check_reliable_supported,
>>> +                                               &ext_prop_data);
>>> +
>>> +       *ext_prop = ext_prop_data.prop;
>>> +       return ext_prop_data.present;
>>> +}
>>> +
>>>  bool gatt_db_attribute_get_char_data(const struct gatt_db_attribute *attrib,
>>>                                                         uint16_t *handle,
>>>                                                         uint16_t *value_handle,
>>> diff --git a/src/shared/gatt-db.h b/src/shared/gatt-db.h
>>> index 96cceb9..0cb713e 100644
>>> --- a/src/shared/gatt-db.h
>>> +++ b/src/shared/gatt-db.h
>>> @@ -195,6 +195,10 @@ bool gatt_db_attribute_get_service_data(const struct gatt_db_attribute *attrib,
>>>                                                         bool *primary,
>>>                                                         bt_uuid_t *uuid);
>>>
>>> +bool gatt_db_attribute_get_characteristic_extended_prop(
>>> +                                       const struct gatt_db_attribute *attrib,
>>> +                                       uint8_t *ext_prop);
>>> +
>>
>> I wonder if it wouldn't be a better idea to extend
>> gatt_db_attribute_get_char_data to return it there along with the
>> other properties?
>
> Well we could add *ext_prop to gatt_db_attribute_get_char_data  and if
> 0x0 it mean there is no extended prop descriptor. Would it be OK?
> If so I will do it.

Yep, I guess we could do that also we should be able to skip if
ext_prop is NULL which means the caller is not interested in reading
the value.

> Are there any other comments to this or next patch before I send next version?

There were some lines over 80 columns in the last patch, if you could
please fix that.

>
>>
>>>  bool gatt_db_attribute_get_char_data(const struct gatt_db_attribute *attrib,
>>>                                                         uint16_t *handle,
>>>                                                         uint16_t *value_handle,
>>> diff --git a/unit/test-gatt.c b/unit/test-gatt.c
>>> index 0912348..85e4d75 100644
>>> --- a/unit/test-gatt.c
>>> +++ b/unit/test-gatt.c
>>> @@ -4434,5 +4434,19 @@ int main(int argc, char *argv[])
>>>                                 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff),
>>>                         raw_pdu(0x01, 0x16, 0x04, 0x00, 0x03));
>>>
>>> +       define_test_server("/robustness/no-reliable-characteristic", test_server,
>>> +                       ts_large_db_1, NULL,
>>> +                       raw_pdu(0x03, 0x00, 0x02),
>>> +                       raw_pdu(0x16, 0x82, 0x00, 0x00, 0x00, 0x01, 0x02, 0x03),
>>> +                       raw_pdu(0x17, 0x82, 0x00, 0x00, 0x00, 0x01, 0x02, 0x03),
>>> +                       raw_pdu(0x16, 0x25, 0x00, 0x00, 0x00, 0x01, 0x02, 0x03),
>>> +                       raw_pdu(0x17, 0x25, 0x00, 0x00, 0x00, 0x01, 0x02, 0x03),
>>> +                       raw_pdu(0x16, 0x82, 0x00, 0x03, 0x00, 0x04, 0x05, 0x06),
>>> +                       raw_pdu(0x17, 0x82, 0x00, 0x03, 0x00, 0x04, 0x05, 0x06),
>>> +                       raw_pdu(0x16, 0x25, 0x00, 0x03, 0x00, 0x04, 0x05, 0x06),
>>> +                       raw_pdu(0x17, 0x25, 0x00, 0x03, 0x00, 0x04, 0x05, 0x06),
>>> +                       raw_pdu(0x18, 0x01),
>>> +                       raw_pdu(0x01, 0x18, 0x25, 0x00, 0x06));
>>> +
>>>         return tester_run();
>>>  }
>>> --
>>> 2.5.0
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
>>
>> --
>> Luiz Augusto von Dentz
>
>
>
> --
> BR / Pozdrawiam
> Łukasz



-- 
Luiz Augusto von Dentz

[PATCH v5 3/3] shared/gatt-server: Check for ext. charact. prop. on reliable session

[Łukasz Rymanowski]

With this patch we make sure that reliable session is done on
characteristics which does support it.
---
 src/shared/gatt-server.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 47 insertions(+), 1 deletion(-)

diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
index a88b62f..ddfe420 100644
--- a/src/shared/gatt-server.c
+++ b/src/shared/gatt-server.c
@@ -72,6 +72,8 @@ struct prep_write_data {
 	uint16_t handle;
 	uint16_t offset;
 	uint16_t length;
+
+	bool reliable_supported;
 };
 
 static void prep_write_data_destroy(void *user_data)
@@ -1088,6 +1090,22 @@ error:
 	bt_att_send_error_rsp(server->att, opcode, 0, ecode);
 }
 
+static bool is_reliable_supported(const struct bt_gatt_server  *server,
+							uint16_t handle)
+{
+	struct gatt_db_attribute *attr;
+	uint8_t ext_prop;
+
+	attr = gatt_db_get_attribute(server->db, handle);
+	if (!attr)
+		return false;
+
+	if (!gatt_db_attribute_get_characteristic_extended_prop(attr, &ext_prop))
+		return false;
+
+	return (ext_prop & BT_GATT_CHRC_EXT_PROP_RELIABLE_WRITE);
+}
+
 static bool create_and_store_prep_data(struct bt_gatt_server *server,
 						uint16_t handle, uint16_t offset,
 						uint16_t length, uint8_t *value)
@@ -1110,6 +1128,12 @@ static bool create_and_store_prep_data(struct bt_gatt_server *server,
 	prep_data->handle = handle;
 	prep_data->offset = offset;
 
+	/*
+	 * Handle is the value handle. We need characteristic declaration
+	 * handle which in BlueZ is handle_value -1
+	 */
+	prep_data->reliable_supported = is_reliable_supported(server,
+								handle - 1);
 	queue_push_tail(server->prep_queue, prep_data);
 
 	return true;
@@ -1262,6 +1286,14 @@ error:
 								ehandle, err);
 }
 
+static bool find_no_reliable_characteristic(const void *data,
+						const void *match_data)
+{
+	const struct prep_write_data *prep_data = data;
+
+	return !prep_data->reliable_supported;
+}
+
 static void exec_write_cb(uint8_t opcode, const void *pdu,
 					uint16_t length, void *user_data)
 {
@@ -1269,6 +1301,7 @@ static void exec_write_cb(uint8_t opcode, const void *pdu,
 	uint8_t flags;
 	uint8_t ecode;
 	bool write;
+	uint16_t ehandle = 0;
 
 	if (length != 1) {
 		ecode = BT_ATT_ERROR_INVALID_PDU;
@@ -1297,6 +1330,19 @@ static void exec_write_cb(uint8_t opcode, const void *pdu,
 		return;
 	}
 
+	/* If there is more than one prep request, we are in reliable session */
+	if (queue_length(server->prep_queue) > 1) {
+		struct prep_write_data *prep_data;
+
+		prep_data = queue_find(server->prep_queue,
+					find_no_reliable_characteristic, NULL);
+		if (prep_data) {
+			ecode = BT_ATT_ERROR_REQUEST_NOT_SUPPORTED;
+			ehandle = prep_data->handle;
+			goto error;
+		}
+	}
+
 	exec_next_prep_write(server, 0, 0);
 
 	return;
@@ -1304,7 +1350,7 @@ static void exec_write_cb(uint8_t opcode, const void *pdu,
 error:
 	queue_remove_all(server->prep_queue, NULL, NULL,
 						prep_write_data_destroy);
-	bt_att_send_error_rsp(server->att, opcode, 0, ecode);
+	bt_att_send_error_rsp(server->att, opcode, ehandle, ecode);
 }
 
 static void exchange_mtu_cb(uint8_t opcode, const void *pdu,
-- 
2.5.0