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
Łukasz Rymanowski (11):
unit/test-gatt: Fix long write testcases
unit/test-gatt: Modify small-db
unit/test-gatt: Extend large-db
unit/test-gatt: Fix long write test
shared/gatt-client: Minor variable rename
shared/gatt-client: Add notify type to notification callback
shared/gatt-server: Fix handle error on execute write
shared/gatt-server: Add support for long write
shared/gatt-server: Support for reliable session nested with long
shared/gatt-db: Add API to get extended prop
shared/gatt-server: Check for ext. charact. prop. on reliable session
profiles/scanparam/scan.c | 5 +-
src/gatt-client.c | 4 +-
src/shared/gatt-client.c | 13 ++--
src/shared/gatt-client.h | 13 +++-
src/shared/gatt-db.c | 62 +++++++++++++++++
src/shared/gatt-db.h | 4 ++
src/shared/gatt-server.c | 169 ++++++++++++++++++++++++++++++++++++++++------
tools/btgatt-client.c | 2 +-
unit/test-gatt.c | 113 +++++++++++++++++++------------
9 files changed, 309 insertions(+), 76 deletions(-)
--
2.5.0
Hi,
On Fri, Nov 27, 2015 at 12:11 PM, Łukasz Rymanowski
<[email protected]> 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);
> +
> 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 6b65525..ceed486 100644
> --- a/unit/test-gatt.c
> +++ b/unit/test-gatt.c
> @@ -4440,5 +4440,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));
> +
>
Hmm, this test should go in separate patch. Some rebasing issue. Will fix.
\Łukasz
> return tester_run();
> }
> --
> 2.5.0
>
--
BR / Pozdrawiam
Łukasz
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 2bd90b0..b939bed 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)
@@ -1109,6 +1127,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 declariation
+ * 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;
@@ -1285,6 +1309,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)
{
@@ -1292,6 +1324,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;
@@ -1320,6 +1353,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;
@@ -1327,7 +1373,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
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 2bd90b0..b939bed 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)
@@ -1109,6 +1127,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 declariation
+ * 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;
@@ -1285,6 +1309,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)
{
@@ -1292,6 +1324,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;
@@ -1320,6 +1353,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;
@@ -1327,7 +1373,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
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 6b65525..ceed486 100644
--- a/unit/test-gatt.c
+++ b/unit/test-gatt.c
@@ -4440,5 +4440,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
With this patch it is possible to do reliable session to one
characteristic nested with long write.
We assume that long write as consequtive prep writes with continous
offsets. Otherwise we treat it as single reliable write.
Note: Long write can be started from non 0 offset as well
With this patch we support following scenarios:
e.g. 1:
(a) prep_write: handle 1, offset 0, value_len 10
(b) prep_write: handle 1, offset 5, value_len 5
(c) prep_write: handle 1, offset 10, value_len 6
Will result with:
(a) exec_write: handle 1, offset 0, value_len 10
(b + c) exec_write: handle 1, offset 10, value_len 11
e.g. 2:
(a) prep_write: handle 1, offset 0, value_len 10
(b) prep_write: handle 1, offset 5, value_len 3
(c) prep_write: handle 1, offset 10, value_len 6
Will result with:
(a) exec_write: handle 1, offset 0, value_len 10
(b) exec_write: handle 1, offset 5, value_len 3
(c) exec_write: handle 1, offset 10, value_len 6
---
src/shared/gatt-server.c | 49 ++++++++++++++++++++++++++++++++----------------
1 file changed, 33 insertions(+), 16 deletions(-)
diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
index c0e7a08..2bd90b0 100644
--- a/src/shared/gatt-server.c
+++ b/src/shared/gatt-server.c
@@ -1088,13 +1088,6 @@ error:
bt_att_send_error_rsp(server->att, opcode, 0, ecode);
}
-static bool match_attribute_handle(const void *data, const void *match_data)
-{
- const struct prep_write_data *prep_data = data;
-
- return prep_data->handle == PTR_TO_UINT(match_data);
-}
-
static bool create_and_store_prep_data(struct bt_gatt_server *server,
uint16_t handle, uint16_t offset,
uint16_t length, uint8_t *value)
@@ -1145,16 +1138,40 @@ static bool make_aggregation_of_long_write_data(struct bt_gatt_server *server,
return true;
}
+struct prep_match_data {
+ uint16_t handle;
+ uint16_t offset;
+ struct prep_write_data *p;
+};
+
+static void match_prep_data(void *data, void *user_data)
+{
+ struct prep_write_data *prep_data = data;
+ struct prep_match_data *s = user_data;
+
+ if (prep_data->handle != s->handle)
+ return;
+
+ /*
+ * We are looking for last prepare write for given handle
+ * with offset we could assume that it is long write
+ */
+ if (prep_data->offset + prep_data->length == s->offset)
+ s->p = prep_data;
+ else
+ s->p = NULL;
+}
+
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;
uint8_t ecode;
bool success;
+ struct prep_match_data match_data;
if (length < 4) {
ecode = BT_ATT_ERROR_INVALID_PDU;
@@ -1184,12 +1201,15 @@ static void prep_write_cb(uint8_t opcode, const void *pdu,
if (ecode)
goto error;
- prep_data = queue_find(server->prep_queue, match_attribute_handle,
- UINT_TO_PTR(handle));
+ match_data.handle = handle;
+ match_data.offset = offset;
+ match_data.p = NULL;
- if (prep_data && offset == prep_data->length + prep_data->offset)
- success = make_aggregation_of_long_write_data(server, prep_data,
- handle, length - 4,
+ queue_foreach(server->prep_queue, match_prep_data, &match_data);
+ if (match_data.p)
+ success = make_aggregation_of_long_write_data(server,
+ match_data.p,
+ handle,length - 4,
&((uint8_t *) pdu)[4]);
else
success = create_and_store_prep_data(server, handle, offset,
@@ -1205,9 +1225,6 @@ static void prep_write_cb(uint8_t opcode, const void *pdu,
return;
error:
- if (prep_data)
- prep_write_data_destroy(prep_data);
-
bt_att_send_error_rsp(server->att, opcode, handle, ecode);
}
--
2.5.0
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 2, offset 0, value_len 10
prep_write: handle 1, offset 10, 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
---
src/shared/gatt-server.c | 85 ++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 71 insertions(+), 14 deletions(-)
diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
index 49dced9..c0e7a08 100644
--- a/src/shared/gatt-server.c
+++ b/src/shared/gatt-server.c
@@ -1088,6 +1088,63 @@ error:
bt_att_send_error_rsp(server->att, opcode, 0, ecode);
}
+static bool match_attribute_handle(const void *data, const void *match_data)
+{
+ const struct prep_write_data *prep_data = data;
+
+ return prep_data->handle == PTR_TO_UINT(match_data);
+}
+
+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) {
+ 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)
+{
+ uint8_t *buf;
+ uint16_t new_len;
+
+ new_len = prep_data->length + length;
+
+ buf = malloc(new_len);
+ if (!buf)
+ return false;
+
+ memcpy(buf, prep_data->value, prep_data->length);
+ memcpy(buf + prep_data->length, value, length);
+
+ free(prep_data->value);
+ prep_data->value = buf;
+ prep_data->length = new_len;
+
+ return true;
+}
+
static void prep_write_cb(uint8_t opcode, const void *pdu,
uint16_t length, void *user_data)
{
@@ -1097,6 +1154,7 @@ static void prep_write_cb(uint8_t opcode, const void *pdu,
uint16_t offset;
struct gatt_db_attribute *attr;
uint8_t ecode;
+ bool success;
if (length < 4) {
ecode = BT_ATT_ERROR_INVALID_PDU;
@@ -1126,22 +1184,21 @@ 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;
- }
- }
+ prep_data = queue_find(server->prep_queue, match_attribute_handle,
+ UINT_TO_PTR(handle));
- prep_data->server = server;
- prep_data->handle = handle;
- prep_data->offset = offset;
- memcpy(prep_data->value, pdu + 4, prep_data->length);
+ if (prep_data && offset == prep_data->length + prep_data->offset)
+ success = make_aggregation_of_long_write_data(server, prep_data,
+ handle, length - 4,
+ &((uint8_t *) pdu)[4]);
+ else
+ success = create_and_store_prep_data(server, handle, offset,
+ length - 4, &((uint8_t *) pdu)[4]);
- queue_push_tail(server->prep_queue, prep_data);
+ if (!success) {
+ ecode = BT_ATT_ERROR_INSUFFICIENT_RESOURCES;
+ goto error;
+ }
bt_att_send(server->att, BT_ATT_OP_PREP_WRITE_RSP, pdu, length, NULL,
NULL, NULL);
--
2.5.0
If there is an error during execute write we should drop all
outstanding prep_write data. This patch fix that.
---
src/shared/gatt-server.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
index a32aeb1..49dced9 100644
--- a/src/shared/gatt-server.c
+++ b/src/shared/gatt-server.c
@@ -1204,6 +1204,9 @@ static void exec_next_prep_write(struct bt_gatt_server *server,
err = BT_ATT_ERROR_UNLIKELY;
error:
+ queue_remove_all(server->prep_queue, NULL, NULL,
+ prep_write_data_destroy);
+
bt_att_send_error_rsp(server->att, BT_ATT_OP_EXEC_WRITE_REQ,
ehandle, err);
}
@@ -1248,6 +1251,8 @@ static void exec_write_cb(uint8_t opcode, const void *pdu,
return;
error:
+ queue_remove_all(server->prep_queue, NULL, NULL,
+ prep_write_data_destroy);
bt_att_send_error_rsp(server->att, opcode, 0, ecode);
}
--
2.5.0
Android expect information either its indication or notification.
---
profiles/scanparam/scan.c | 5 +++--
src/gatt-client.c | 4 ++--
src/shared/gatt-client.c | 13 +++++++++----
src/shared/gatt-client.h | 13 ++++++++++---
tools/btgatt-client.c | 2 +-
unit/test-gatt.c | 14 ++++++++++----
6 files changed, 35 insertions(+), 16 deletions(-)
diff --git a/profiles/scanparam/scan.c b/profiles/scanparam/scan.c
index 4015b3f..fd01135 100644
--- a/profiles/scanparam/scan.c
+++ b/profiles/scanparam/scan.c
@@ -92,8 +92,9 @@ static void write_scan_params(struct scan *scan)
false, value, sizeof(value));
}
-static void refresh_value_cb(uint16_t value_handle, const uint8_t *value,
- uint16_t length, void *user_data)
+static void refresh_value_cb(notify_t type, uint16_t value_handle,
+ const uint8_t *value, uint16_t length,
+ void *user_data)
{
struct scan *scan = user_data;
diff --git a/src/gatt-client.c b/src/gatt-client.c
index 39f6646..e74803c 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -1057,8 +1057,8 @@ static bool match_notify_sender(const void *a, const void *b)
return strcmp(client->owner, sender) == 0;
}
-static void notify_cb(uint16_t value_handle, const uint8_t *value,
- uint16_t length, void *user_data)
+static void notify_cb(notify_t type, uint16_t value_handle,
+ const uint8_t *value, uint16_t length, void *user_data)
{
struct async_dbus_op *op = user_data;
struct notify_client *client = op->data;
diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index 06ac763..76e4a9a 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -1109,7 +1109,7 @@ struct service_changed_op {
static void process_service_changed(struct bt_gatt_client *client,
uint16_t start_handle,
uint16_t end_handle);
-static void service_changed_cb(uint16_t value_handle, const uint8_t *value,
+static void service_changed_cb(notify_t type, uint16_t value_handle, const uint8_t *value,
uint16_t length, void *user_data);
static void complete_notify_request(void *data)
@@ -1446,8 +1446,9 @@ fail:
" after Service Changed");
}
-static void service_changed_cb(uint16_t value_handle, const uint8_t *value,
- uint16_t length, void *user_data)
+static void service_changed_cb(notify_t type, uint16_t value_handle,
+ const uint8_t *value, uint16_t length,
+ void *user_data)
{
struct bt_gatt_client *client = user_data;
struct service_changed_op *op;
@@ -1545,6 +1546,7 @@ static bool gatt_client_init(struct bt_gatt_client *client, uint16_t mtu)
}
struct pdu_data {
+ notify_t type;
const void *pdu;
uint16_t length;
};
@@ -1614,7 +1616,8 @@ static void notify_handler(void *data, void *user_data)
* CCC, there is really no reason not to notify the handlers.
*/
if (notify_data->notify)
- notify_data->notify(value_handle, value, pdu_data->length - 2,
+ notify_data->notify(pdu_data->type, value_handle, value,
+ pdu_data->length - 2,
notify_data->user_data);
}
@@ -1629,6 +1632,8 @@ static void notify_cb(uint8_t opcode, const void *pdu, uint16_t length,
memset(&pdu_data, 0, sizeof(pdu_data));
pdu_data.pdu = pdu;
pdu_data.length = length;
+ pdu_data.type = opcode == BT_ATT_OP_HANDLE_VAL_IND ?
+ BT_GATT_INDICATION : BT_GATT_NOTIFICATION;
queue_foreach(client->notify_list, notify_handler, &pdu_data);
diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
index befa43f..3018a99 100644
--- a/src/shared/gatt-client.h
+++ b/src/shared/gatt-client.h
@@ -27,6 +27,11 @@
#define BT_GATT_UUID_SIZE 16
+typedef enum {
+ BT_GATT_NOTIFICATION,
+ BT_GATT_INDICATION,
+} notify_t;
+
struct bt_gatt_client;
struct bt_gatt_client *bt_gatt_client_new(struct gatt_db *db,
@@ -46,9 +51,11 @@ typedef void (*bt_gatt_client_read_callback_t)(bool success, uint8_t att_ecode,
typedef void (*bt_gatt_client_write_long_callback_t)(bool success,
bool reliable_error, uint8_t att_ecode,
void *user_data);
-typedef void (*bt_gatt_client_notify_callback_t)(uint16_t value_handle,
- const uint8_t *value, uint16_t length,
- void *user_data);
+typedef void (*bt_gatt_client_notify_callback_t)(notify_t type,
+ uint16_t value_handle,
+ const uint8_t *value,
+ uint16_t length,
+ void *user_data);
typedef void (*bt_gatt_client_register_callback_t)(uint16_t att_ecode,
void *user_data);
typedef void (*bt_gatt_client_service_changed_callback_t)(uint16_t start_handle,
diff --git a/tools/btgatt-client.c b/tools/btgatt-client.c
index 0f6a1bd..76fd6d0 100644
--- a/tools/btgatt-client.c
+++ b/tools/btgatt-client.c
@@ -1099,7 +1099,7 @@ static void register_notify_usage(void)
printf("Usage: register-notify <chrc value handle>\n");
}
-static void notify_cb(uint16_t value_handle, const uint8_t *value,
+static void notify_cb(notify_t type, uint16_t value_handle, const uint8_t *value,
uint16_t length, void *user_data)
{
int i;
diff --git a/unit/test-gatt.c b/unit/test-gatt.c
index 87a87b8..6b65525 100644
--- a/unit/test-gatt.c
+++ b/unit/test-gatt.c
@@ -2211,12 +2211,15 @@ static const struct test_step test_long_read_15 = {
.expected_att_ecode = 0x0c
};
-static void notification_cb(uint16_t value_handle, const uint8_t *value,
- uint16_t length, void *user_data)
+static void notification_cb(notify_t type, uint16_t value_handle,
+ const uint8_t *value, uint16_t length,
+ void *user_data)
{
struct context *context = user_data;
const struct test_step *step = context->data->step;
+ g_assert(type == BT_GATT_NOTIFICATION);
+
if (value_handle == step->handle) {
g_assert_cmpint(length, ==, step->length);
@@ -2279,12 +2282,15 @@ static void test_server_indication_confirm(struct context *context)
g_assert(indication_received == 1);
}
-static void indication_cb(uint16_t value_handle, const uint8_t *value,
- uint16_t length, void *user_data)
+static void indication_cb(notify_t type, uint16_t value_handle,
+ const uint8_t *value, uint16_t length,
+ void *user_data)
{
struct context *context = user_data;
const struct test_step *step = context->data->step;
+ g_assert(type == BT_GATT_INDICATION);
+
if (value_handle == step->handle) {
g_assert_cmpint(length, ==, step->length);
--
2.5.0
Use success instead of status to make it easy to read
---
src/shared/gatt-server.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
index ba668e3..a32aeb1 100644
--- a/src/shared/gatt-server.c
+++ b/src/shared/gatt-server.c
@@ -1172,7 +1172,7 @@ static void exec_next_prep_write(struct bt_gatt_server *server,
{
struct prep_write_data *next = NULL;
struct gatt_db_attribute *attr;
- bool status;
+ bool success;
if (err)
goto error;
@@ -1190,7 +1190,7 @@ static void exec_next_prep_write(struct bt_gatt_server *server,
goto error;
}
- status = gatt_db_attribute_write(attr, next->offset,
+ success = gatt_db_attribute_write(attr, next->offset,
next->value, next->length,
BT_ATT_OP_EXEC_WRITE_REQ,
server->att,
@@ -1198,7 +1198,7 @@ static void exec_next_prep_write(struct bt_gatt_server *server,
prep_write_data_destroy(next);
- if (status)
+ if (success)
return;
err = BT_ATT_ERROR_UNLIKELY;
--
2.5.0
This patch change characteristic we are writing too in the nested
long write reliable testcase.
Basically characteristic with handle 0x0072 already have characteristic
extended descriptor required for reliable session
---
unit/test-gatt.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/unit/test-gatt.c b/unit/test-gatt.c
index 68cdcff..87a87b8 100644
--- a/unit/test-gatt.c
+++ b/unit/test-gatt.c
@@ -4005,12 +4005,12 @@ int main(int argc, char *argv[])
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, 0x72, 0x00, 0x00, 0x00, 0x01, 0x02, 0x03),
+ raw_pdu(0x17, 0x72, 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(0x16, 0x72, 0x00, 0x03, 0x00, 0x04, 0x05, 0x06),
+ raw_pdu(0x17, 0x72, 0x00, 0x03, 0x00, 0x04, 0x05, 0x06),
raw_pdu(0x18, 0x01),
raw_pdu(0x19));
--
2.5.0
This patch adds characteristic extended descriptor to characteristic
handle 0x0081. Also fixes testes using this.
We need this to make script pass when following patches start
to look into this descriptor on reliable write session
---
unit/test-gatt.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/unit/test-gatt.c b/unit/test-gatt.c
index 9554bcb..68cdcff 100644
--- a/unit/test-gatt.c
+++ b/unit/test-gatt.c
@@ -223,7 +223,7 @@ struct context {
0x60, 0x00, 0x6B, 0x00, 0x0B, 0xA0), \
raw_pdu(0x10, 0x6C, 0x00, 0xff, 0xff, 0x00, 0x28), \
raw_pdu(0x11, 0x06, 0x70, 0x00, 0x76, 0x00, 0x0B, 0xA0, \
- 0x80, 0x00, 0x85, 0x00, 0x0B, 0xA0), \
+ 0x80, 0x00, 0x86, 0x00, 0x0B, 0xA0), \
raw_pdu(0x10, 0x86, 0x00, 0xff, 0xff, 0x00, 0x28), \
raw_pdu(0x11, 0x14, 0x90, 0x00, 0x96, 0x00, \
0xef, 0xcd, 0xab, 0x89, 0x67, 0x45, 0x23, 0x01, \
@@ -1614,7 +1614,7 @@ static struct gatt_db *make_test_spec_small_db(void)
static struct gatt_db *make_test_spec_large_db_1(void)
{
const struct att_handle_spec specs[] = {
- PRIMARY_SERVICE(0x0080, "a00b", 6),
+ PRIMARY_SERVICE(0x0080, "a00b", 7),
CHARACTERISTIC(0xb008, BT_ATT_PERM_READ | BT_ATT_PERM_WRITE,
BT_GATT_CHRC_PROP_READ |
BT_GATT_CHRC_PROP_WRITE,
@@ -1623,6 +1623,8 @@ static struct gatt_db *make_test_spec_large_db_1(void)
DESCRIPTOR(0xb016, BT_ATT_PERM_READ | BT_ATT_PERM_WRITE, 0x02),
DESCRIPTOR(0xb017, BT_ATT_PERM_READ | BT_ATT_PERM_WRITE |
BT_ATT_PERM_ENCRYPT, 0x03),
+ DESCRIPTOR(GATT_CHARAC_EXT_PROPER_UUID, BT_ATT_PERM_READ, 0x01,
+ 0x00),
SECONDARY_SERVICE(0x0001, "a00d", 6),
INCLUDE(0x0080),
@@ -2438,7 +2440,7 @@ int main(int argc, char *argv[])
0xa0),
raw_pdu(0x07, 0x30, 0x00, 0x32, 0x00, 0x50, 0x00, 0x52,
0x00, 0x60, 0x00, 0x6b, 0x00, 0x70, 0x00, 0x76,
- 0x00, 0x80, 0x00, 0x85, 0x00),
+ 0x00, 0x80, 0x00, 0x86, 0x00),
raw_pdu(0x06, 0x86, 0x00, 0xff, 0xff, 0x00, 0x28, 0x0b,
0xa0),
raw_pdu(0x01, 0x06, 0x86, 0x00, 0x0a));
@@ -2514,7 +2516,7 @@ int main(int argc, char *argv[])
ts_large_db_1, NULL,
raw_pdu(0x03, 0x00, 0x02),
raw_pdu(0x08, 0x01, 0x00, 0xff, 0xff, 0x02, 0x28),
- raw_pdu(0x09, 0x08, 0x02, 0x00, 0x80, 0x00, 0x85, 0x00,
+ raw_pdu(0x09, 0x08, 0x02, 0x00, 0x80, 0x00, 0x86, 0x00,
0x0b, 0xa0, 0x21, 0x00, 0x01, 0x00, 0x06, 0x00,
0x0d, 0xa0),
raw_pdu(0x08, 0x22, 0x00, 0xff, 0xff, 0x02, 0x28),
--
2.5.0
This patch adds Characteristic Extended descriptor to characteristic
with handle 0x0002 nad 0x0014.
Also fixes scripts using small-db.
We need this as next patches will start to look into this descriptor
on reliable write session.
---
unit/test-gatt.c | 51 +++++++++++++++++++++++++++++----------------------
1 file changed, 29 insertions(+), 22 deletions(-)
diff --git a/unit/test-gatt.c b/unit/test-gatt.c
index 70b6de5..9554bcb 100644
--- a/unit/test-gatt.c
+++ b/unit/test-gatt.c
@@ -209,7 +209,7 @@ struct context {
#define PRIMARY_DISC_SMALL_DB \
raw_pdu(0x10, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28), \
- raw_pdu(0x11, 0x06, 0x10, 0xF0, 0x17, 0xF0, 0x00, 0x18, \
+ raw_pdu(0x11, 0x06, 0x10, 0xF0, 0x18, 0xF0, 0x00, 0x18, \
0xFF, 0xFF, 0xFF, 0xFF, 0x0a, 0x18)
#define PRIMARY_DISC_LARGE_DB_1 \
@@ -244,27 +244,29 @@ struct context {
raw_pdu(0x01, 0x10, 0x11, 0x00, 0x0a)
#define INCLUDE_DISC_SMALL_DB \
- raw_pdu(0x08, 0x10, 0xf0, 0x17, 0xf0, 0x02, 0x28), \
+ raw_pdu(0x08, 0x10, 0xf0, 0x18, 0xf0, 0x02, 0x28), \
raw_pdu(0x09, 0x08, 0x11, 0xf0, 0x01, 0x00, 0x0f, 0x00, \
0x0a, 0x18), \
- raw_pdu(0x08, 0x12, 0xf0, 0x17, 0xf0, 0x02, 0x28), \
+ raw_pdu(0x08, 0x12, 0xf0, 0x18, 0xf0, 0x02, 0x28), \
raw_pdu(0x01, 0x08, 0x12, 0xf0, 0x0a), \
raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x02, 0x28), \
raw_pdu(0x01, 0x08, 0x01, 0x00, 0x0a)
#define CHARACTERISTIC_DISC_SMALL_DB \
- raw_pdu(0x08, 0x10, 0xf0, 0x17, 0xf0, 0x03, 0x28), \
+ raw_pdu(0x08, 0x10, 0xf0, 0x18, 0xf0, 0x03, 0x28), \
raw_pdu(0x09, 0x07, 0x12, 0xf0, 0x02, 0x13, 0xf0, 0x00, \
0x2a), \
- raw_pdu(0x08, 0x13, 0xf0, 0x17, 0xf0, 0x03, 0x28), \
+ raw_pdu(0x08, 0x13, 0xf0, 0x18, 0xf0, 0x03, 0x28), \
raw_pdu(0x09, 0x15, 0x14, 0xf0, 0x02, 0x15, 0xf0, 0xef, \
0xcd, 0xab, 0x89, 0x67, 0x45, 0x23, 0x01, 0x00, \
0x00, 0x00, 0x00, 0x09, 0xB0, 0x00, 0x00), \
- raw_pdu(0x08, 0x15, 0xf0, 0x17, 0xf0, 0x03, 0x28), \
- raw_pdu(0x09, 0x07, 0x16, 0xf0, 0x02, 0x17, 0xf0, 0x01, \
+ raw_pdu(0x08, 0x15, 0xf0, 0x18, 0xf0, 0x03, 0x28), \
+ raw_pdu(0x09, 0x07, 0x17, 0xf0, 0x02, 0x18, 0xf0, 0x01, \
0x2a), \
- raw_pdu(0x08, 0x17, 0xf0, 0x17, 0xf0, 0x03, 0x28), \
- raw_pdu(0x01, 0x08, 0x17, 0xf0, 0x0a), \
+ raw_pdu(0x08, 0x18, 0xf0, 0x18, 0xf0, 0x03, 0x28), \
+ raw_pdu(0x01, 0x08, 0x18, 0xf0, 0x0a), \
+ raw_pdu(0x04, 0x16, 0xf0, 0x16, 0xf0), \
+ raw_pdu(0x05, 0x01, 0x16, 0xf0, 0x00, 0x29), \
raw_pdu(0x08, 0x01, 0x00, 0x10, 0x00, 0x03, 0x28), \
raw_pdu(0x09, 0x07, 0x02, 0x00, 0x32, 0x03, 0x00, 0x29, \
0x2a), \
@@ -1560,7 +1562,10 @@ static struct gatt_db *make_test_spec_small_db(void)
BT_ATT_PERM_WRITE, 0x00, 0x00),
DESCRIPTOR_STR(GATT_CHARAC_USER_DESC_UUID, BT_ATT_PERM_READ,
"Manufacturer Name"),
- PRIMARY_SERVICE(0xF010, GAP_UUID, 8),
+ DESCRIPTOR(GATT_CHARAC_EXT_PROPER_UUID, BT_ATT_PERM_READ, 0x01,
+ 0x00),
+
+ PRIMARY_SERVICE(0xF010, GAP_UUID, 9),
INCLUDE(0x0001),
CHARACTERISTIC_STR(GATT_CHARAC_DEVICE_NAME, BT_ATT_PERM_READ,
BT_GATT_CHRC_PROP_READ,
@@ -1568,6 +1573,8 @@ static struct gatt_db *make_test_spec_small_db(void)
CHARACTERISTIC(0000B009-0000-0000-0123-456789abcdef,
BT_ATT_PERM_READ | BT_ATT_PERM_WRITE,
BT_GATT_CHRC_PROP_READ, 0x09),
+ DESCRIPTOR(GATT_CHARAC_EXT_PROPER_UUID, BT_ATT_PERM_READ, 0x01,
+ 0x00),
CHARACTERISTIC(GATT_CHARAC_APPEARANCE, BT_ATT_PERM_READ,
BT_GATT_CHRC_PROP_READ, 0x00, 0x00),
PRIMARY_SERVICE(0xFFFF, DEVICE_INFORMATION_UUID, 1),
@@ -2419,7 +2426,7 @@ int main(int argc, char *argv[])
raw_pdu(0x03, 0x00, 0x02),
raw_pdu(0x06, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28, 0x00,
0x18),
- raw_pdu(0x07, 0x10, 0xf0, 0x17, 0xf0),
+ raw_pdu(0x07, 0x10, 0xf0, 0x18, 0xf0),
raw_pdu(0x06, 0x18, 0xf0, 0xff, 0xff, 0x00, 0x28, 0x00,
0x18),
raw_pdu(0x01, 0x06, 0x18, 0xf0, 0x0a));
@@ -2533,19 +2540,19 @@ int main(int argc, char *argv[])
define_test_server("/TP/GAD/SR/BV-04-C/small/1", test_server,
ts_small_db, NULL,
raw_pdu(0x03, 0x00, 0x02),
- raw_pdu(0x08, 0x10, 0xf0, 0x17, 0xf0, 0x03, 0x28),
+ raw_pdu(0x08, 0x10, 0xf0, 0x18, 0xf0, 0x03, 0x28),
raw_pdu(0x09, 0x07, 0x12, 0xf0, 0x02, 0x13, 0xf0, 0x00,
0x2a),
- raw_pdu(0x08, 0x13, 0xf0, 0x17, 0xf0, 0x03, 0x28),
+ raw_pdu(0x08, 0x13, 0xf0, 0x18, 0xf0, 0x03, 0x28),
raw_pdu(0x09, 0x15, 0x14, 0xf0, 0x02, 0x15, 0xf0, 0xef,
0xcd, 0xab, 0x89, 0x67, 0x45, 0x23,
0x01, 0x00, 0x00, 0x00, 0x00, 0x09,
0xb0, 0x00, 0x00),
- raw_pdu(0x08, 0x15, 0xf0, 0x17, 0xf0, 0x03, 0x28),
- raw_pdu(0x09, 0x07, 0x16, 0xf0, 0x02, 0x17, 0xf0, 0x01,
+ raw_pdu(0x08, 0x15, 0xf0, 0x18, 0xf0, 0x03, 0x28),
+ raw_pdu(0x09, 0x07, 0x17, 0xf0, 0x02, 0x18, 0xf0, 0x01,
0x2a),
- raw_pdu(0x08, 0x17, 0xf0, 0x17, 0xf0, 0x03, 0x28),
- raw_pdu(0x01, 0x08, 0x17, 0xf0, 0x0a));
+ raw_pdu(0x08, 0x18, 0xf0, 0x18, 0xf0, 0x03, 0x28),
+ raw_pdu(0x01, 0x08, 0x18, 0xf0, 0x0a));
define_test_server("/TP/GAD/SR/BV-04-C/small/2", test_server,
ts_small_db, NULL,
@@ -2594,11 +2601,11 @@ int main(int argc, char *argv[])
0xcd, 0xab, 0x89, 0x67, 0x45, 0x23,
0x01, 0x00, 0x00, 0x00, 0x00, 0x09,
0xb0, 0x00, 0x00),
- raw_pdu(0x08, 0x15, 0xf0, 0x17, 0xf0, 0x03, 0x28),
- raw_pdu(0x09, 0x07, 0x16, 0xf0, 0x02, 0x17, 0xf0, 0x01,
+ raw_pdu(0x08, 0x15, 0xf0, 0x18, 0xf0, 0x03, 0x28),
+ raw_pdu(0x09, 0x07, 0x17, 0xf0, 0x02, 0x18, 0xf0, 0x01,
0x2a),
- raw_pdu(0x08, 0x17, 0xf0, 0x17, 0xf0, 0x03, 0x28),
- raw_pdu(0x01, 0x08, 0x17, 0xf0, 0x0a));
+ raw_pdu(0x08, 0x18, 0xf0, 0x18, 0xf0, 0x03, 0x28),
+ raw_pdu(0x01, 0x08, 0x18, 0xf0, 0x0a));
define_test_server("/TP/GAD/SR/BV-05-C/small/2", test_server,
ts_small_db, NULL,
@@ -2798,7 +2805,7 @@ int main(int argc, char *argv[])
0x00),
raw_pdu(0x09, 0x03, 0x15, 0xF0, 0x09),
raw_pdu(0x08, 0x01, 0x00, 0xFF, 0xFF, 0x01, 0x2a),
- raw_pdu(0x09, 0x04, 0x17, 0xF0, 0x00, 0x00));
+ raw_pdu(0x09, 0x04, 0x18, 0xF0, 0x00, 0x00));
define_test_server("/TP/GAR/SR/BV-03-C/large-1", test_server,
ts_large_db_1, NULL,
--
2.5.0
Idea of long write is that each part of data is continuation
of previous one. There shall be not gaps in the offsets between.
If there are gaps in offset then we have reliable write rather than
long write
---
unit/test-gatt.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/unit/test-gatt.c b/unit/test-gatt.c
index 326a32c..70b6de5 100644
--- a/unit/test-gatt.c
+++ b/unit/test-gatt.c
@@ -3852,8 +3852,8 @@ int main(int argc, char *argv[])
raw_pdu(0x17, 0x03, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff),
- raw_pdu(0x16, 0x03, 0x00, 0x3f, 0x00, 0xff),
- raw_pdu(0x17, 0x03, 0x00, 0x3f, 0x00, 0xff),
+ raw_pdu(0x16, 0x03, 0x00, 0x12, 0x00, 0xff),
+ raw_pdu(0x17, 0x03, 0x00, 0x12, 0x00, 0xff),
raw_pdu(0x18, 0x01),
raw_pdu(0x19));
@@ -3866,8 +3866,8 @@ int main(int argc, char *argv[])
raw_pdu(0x17, 0x82, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff),
- raw_pdu(0x16, 0x82, 0x00, 0x3f, 0x00, 0xff),
- raw_pdu(0x17, 0x82, 0x00, 0x3f, 0x00, 0xff),
+ raw_pdu(0x16, 0x82, 0x00, 0x12, 0x00, 0xff),
+ raw_pdu(0x17, 0x82, 0x00, 0x12, 0x00, 0xff),
raw_pdu(0x18, 0x01),
raw_pdu(0x19));
@@ -4374,8 +4374,8 @@ int main(int argc, char *argv[])
raw_pdu(0x17, 0x04, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff),
- raw_pdu(0x16, 0x04, 0x00, 0x3f, 0x00, 0xff),
- raw_pdu(0x17, 0x04, 0x00, 0x3f, 0x00, 0xff),
+ raw_pdu(0x16, 0x04, 0x00, 0x12, 0x00, 0xff),
+ raw_pdu(0x17, 0x04, 0x00, 0x12, 0x00, 0xff),
raw_pdu(0x18, 0x01),
raw_pdu(0x19));
@@ -4388,8 +4388,8 @@ int main(int argc, char *argv[])
raw_pdu(0x17, 0x83, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff),
- raw_pdu(0x16, 0x83, 0x00, 0x3f, 0x00, 0xff),
- raw_pdu(0x17, 0x83, 0x00, 0x3f, 0x00, 0xff),
+ raw_pdu(0x16, 0x83, 0x00, 0x12, 0x00, 0xff),
+ raw_pdu(0x17, 0x83, 0x00, 0x12, 0x00, 0xff),
raw_pdu(0x18, 0x01),
raw_pdu(0x19));
--
2.5.0
Hi Luiz,
On Thu, Dec 3, 2015 at 11:36 AM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Łukasz,
>
> On Fri, Nov 27, 2015 at 1:11 PM, Łukasz Rymanowski
> <[email protected]> wrote:
>> Android expect information either its indication or notification.
>> ---
>> profiles/scanparam/scan.c | 5 +++--
>> src/gatt-client.c | 4 ++--
>> src/shared/gatt-client.c | 13 +++++++++----
>> src/shared/gatt-client.h | 13 ++++++++++---
>> tools/btgatt-client.c | 2 +-
>> unit/test-gatt.c | 14 ++++++++++----
>> 6 files changed, 35 insertions(+), 16 deletions(-)
>>
>> diff --git a/profiles/scanparam/scan.c b/profiles/scanparam/scan.c
>> index 4015b3f..fd01135 100644
>> --- a/profiles/scanparam/scan.c
>> +++ b/profiles/scanparam/scan.c
>> @@ -92,8 +92,9 @@ static void write_scan_params(struct scan *scan)
>> false, value, sizeof(value));
>> }
>>
>> -static void refresh_value_cb(uint16_t value_handle, const uint8_t *value,
>> - uint16_t length, void *user_data)
>> +static void refresh_value_cb(notify_t type, uint16_t value_handle,
>> + const uint8_t *value, uint16_t length,
>> + void *user_data)
>> {
>> struct scan *scan = user_data;
>>
>> diff --git a/src/gatt-client.c b/src/gatt-client.c
>> index 39f6646..e74803c 100644
>> --- a/src/gatt-client.c
>> +++ b/src/gatt-client.c
>> @@ -1057,8 +1057,8 @@ static bool match_notify_sender(const void *a, const void *b)
>> return strcmp(client->owner, sender) == 0;
>> }
>>
>> -static void notify_cb(uint16_t value_handle, const uint8_t *value,
>> - uint16_t length, void *user_data)
>> +static void notify_cb(notify_t type, uint16_t value_handle,
>> + const uint8_t *value, uint16_t length, void *user_data)
>> {
>> struct async_dbus_op *op = user_data;
>> struct notify_client *client = op->data;
>> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
>> index 06ac763..76e4a9a 100644
>> --- a/src/shared/gatt-client.c
>> +++ b/src/shared/gatt-client.c
>> @@ -1109,7 +1109,7 @@ struct service_changed_op {
>> static void process_service_changed(struct bt_gatt_client *client,
>> uint16_t start_handle,
>> uint16_t end_handle);
>> -static void service_changed_cb(uint16_t value_handle, const uint8_t *value,
>> +static void service_changed_cb(notify_t type, uint16_t value_handle, const uint8_t *value,
>> uint16_t length, void *user_data);
>>
>> static void complete_notify_request(void *data)
>> @@ -1446,8 +1446,9 @@ fail:
>> " after Service Changed");
>> }
>>
>> -static void service_changed_cb(uint16_t value_handle, const uint8_t *value,
>> - uint16_t length, void *user_data)
>> +static void service_changed_cb(notify_t type, uint16_t value_handle,
>> + const uint8_t *value, uint16_t length,
>> + void *user_data)
>> {
>> struct bt_gatt_client *client = user_data;
>> struct service_changed_op *op;
>> @@ -1545,6 +1546,7 @@ static bool gatt_client_init(struct bt_gatt_client *client, uint16_t mtu)
>> }
>>
>> struct pdu_data {
>> + notify_t type;
>> const void *pdu;
>> uint16_t length;
>> };
>> @@ -1614,7 +1616,8 @@ static void notify_handler(void *data, void *user_data)
>> * CCC, there is really no reason not to notify the handlers.
>> */
>> if (notify_data->notify)
>> - notify_data->notify(value_handle, value, pdu_data->length - 2,
>> + notify_data->notify(pdu_data->type, value_handle, value,
>> + pdu_data->length - 2,
>> notify_data->user_data);
>> }
>>
>> @@ -1629,6 +1632,8 @@ static void notify_cb(uint8_t opcode, const void *pdu, uint16_t length,
>> memset(&pdu_data, 0, sizeof(pdu_data));
>> pdu_data.pdu = pdu;
>> pdu_data.length = length;
>> + pdu_data.type = opcode == BT_ATT_OP_HANDLE_VAL_IND ?
>> + BT_GATT_INDICATION : BT_GATT_NOTIFICATION;
>>
>> queue_foreach(client->notify_list, notify_handler, &pdu_data);
>>
>> diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
>> index befa43f..3018a99 100644
>> --- a/src/shared/gatt-client.h
>> +++ b/src/shared/gatt-client.h
>> @@ -27,6 +27,11 @@
>>
>> #define BT_GATT_UUID_SIZE 16
>>
>> +typedef enum {
>> + BT_GATT_NOTIFICATION,
>> + BT_GATT_INDICATION,
>> +} notify_t;
>> +
>> struct bt_gatt_client;
>>
>> struct bt_gatt_client *bt_gatt_client_new(struct gatt_db *db,
>> @@ -46,9 +51,11 @@ typedef void (*bt_gatt_client_read_callback_t)(bool success, uint8_t att_ecode,
>> typedef void (*bt_gatt_client_write_long_callback_t)(bool success,
>> bool reliable_error, uint8_t att_ecode,
>> void *user_data);
>> -typedef void (*bt_gatt_client_notify_callback_t)(uint16_t value_handle,
>> - const uint8_t *value, uint16_t length,
>> - void *user_data);
>> +typedef void (*bt_gatt_client_notify_callback_t)(notify_t type,
>> + uint16_t value_handle,
>> + const uint8_t *value,
>> + uint16_t length,
>> + void *user_data);
>
> Maybe I was not very clear last time but I prefer to have a dedicated
> type for indications so we can actually have a return to it. Or
> perhaps we should do this via db since nowadays bt_gatt_client build a
> cache it could in theory support value notification and indication by
> attribute directly in the db.
>
Ah ok so let me understand it:
First option would be to provide two different callbacks (notify_cb
and indication_cb)
to bt_gatt_client_register_notify().
Second option would be to extend struct gatt_db_attribute with notify
and indicate callbacks.
Do I got it right?
>> typedef void (*bt_gatt_client_register_callback_t)(uint16_t att_ecode,
>> void *user_data);
>> typedef void (*bt_gatt_client_service_changed_callback_t)(uint16_t start_handle,
>> diff --git a/tools/btgatt-client.c b/tools/btgatt-client.c
>> index 0f6a1bd..76fd6d0 100644
>> --- a/tools/btgatt-client.c
>> +++ b/tools/btgatt-client.c
>> @@ -1099,7 +1099,7 @@ static void register_notify_usage(void)
>> printf("Usage: register-notify <chrc value handle>\n");
>> }
>>
>> -static void notify_cb(uint16_t value_handle, const uint8_t *value,
>> +static void notify_cb(notify_t type, uint16_t value_handle, const uint8_t *value,
>> uint16_t length, void *user_data)
>> {
>> int i;
>> diff --git a/unit/test-gatt.c b/unit/test-gatt.c
>> index 87a87b8..6b65525 100644
>> --- a/unit/test-gatt.c
>> +++ b/unit/test-gatt.c
>> @@ -2211,12 +2211,15 @@ static const struct test_step test_long_read_15 = {
>> .expected_att_ecode = 0x0c
>> };
>>
>> -static void notification_cb(uint16_t value_handle, const uint8_t *value,
>> - uint16_t length, void *user_data)
>> +static void notification_cb(notify_t type, uint16_t value_handle,
>> + const uint8_t *value, uint16_t length,
>> + void *user_data)
>> {
>> struct context *context = user_data;
>> const struct test_step *step = context->data->step;
>>
>> + g_assert(type == BT_GATT_NOTIFICATION);
>> +
>> if (value_handle == step->handle) {
>> g_assert_cmpint(length, ==, step->length);
>>
>> @@ -2279,12 +2282,15 @@ static void test_server_indication_confirm(struct context *context)
>> g_assert(indication_received == 1);
>> }
>>
>> -static void indication_cb(uint16_t value_handle, const uint8_t *value,
>> - uint16_t length, void *user_data)
>> +static void indication_cb(notify_t type, uint16_t value_handle,
>> + const uint8_t *value, uint16_t length,
>> + void *user_data)
>> {
>> struct context *context = user_data;
>> const struct test_step *step = context->data->step;
>>
>> + g_assert(type == BT_GATT_INDICATION);
>> +
>> if (value_handle == step->handle) {
>> g_assert_cmpint(length, ==, step->length);
>>
>> --
>> 2.5.0
>>
>> --
>> 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
--
BR / Pozdrawiam
Łukasz
Hi Luiz,
On Thu, Dec 3, 2015 at 10:27 AM, Luiz Augusto von Dentz
<[email protected]> wrote:
>
> Hi Łukasz,
>
> On Fri, Nov 27, 2015 at 1:11 PM, Łukasz Rymanowski
> <[email protected]> wrote:
> > Idea of long write is that each part of data is continuation
> > of previous one. There shall be not gaps in the offsets between.
> >
> > If there are gaps in offset then we have reliable write rather than
> > long write
>
> The patch looks fine what I did not understand is what different it
> makes if it is a reliable write or a long write, to me all long writes
> are in fact reliable write since it does use prepare + execute, the
> fact that only one handle is written doesn't change anything.
>
These testes are for Long write and there is a difference between Long Write and
Reliable Write even those two use prepare/execute writes. This can be
found in the
BT Spec Chapter 3 Part G, 4.9.4 and 4.9.5
Basically those procedure belongs to GATT and our gatt server should
understand that.
If we are going to not expose prepare/execute wirtes to application,
then this is GATT server which should
recognize procedure and prepare data to be written to application.
i.e. for long write do aggregation of all
prepare writes, we can not just write part of data as this would cause
some unexpected behavior.
We also need this change, because following patches will start to test
for characteristic extended properties
descriptor and do not allow reliable write on characteristics which
does not have property for this.
Note that for Long Write we don't need this descriptor property, so
these test cases would fail then.
Also, note that there is Long Write procedure for descriptors but
there is no Reliable Write procedure for
descriptors, so we definitely cannot treat long and prepare as the same one.
> > ---
> > unit/test-gatt.c | 16 ++++++++--------
> > 1 file changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/unit/test-gatt.c b/unit/test-gatt.c
> > index 326a32c..70b6de5 100644
> > --- a/unit/test-gatt.c
> > +++ b/unit/test-gatt.c
> > @@ -3852,8 +3852,8 @@ int main(int argc, char *argv[])
> > raw_pdu(0x17, 0x03, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff,
> > 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> > 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff),
> > - raw_pdu(0x16, 0x03, 0x00, 0x3f, 0x00, 0xff),
> > - raw_pdu(0x17, 0x03, 0x00, 0x3f, 0x00, 0xff),
> > + raw_pdu(0x16, 0x03, 0x00, 0x12, 0x00, 0xff),
> > + raw_pdu(0x17, 0x03, 0x00, 0x12, 0x00, 0xff),
> > raw_pdu(0x18, 0x01),
> > raw_pdu(0x19));
> >
> > @@ -3866,8 +3866,8 @@ int main(int argc, char *argv[])
> > raw_pdu(0x17, 0x82, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff,
> > 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> > 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff),
> > - raw_pdu(0x16, 0x82, 0x00, 0x3f, 0x00, 0xff),
> > - raw_pdu(0x17, 0x82, 0x00, 0x3f, 0x00, 0xff),
> > + raw_pdu(0x16, 0x82, 0x00, 0x12, 0x00, 0xff),
> > + raw_pdu(0x17, 0x82, 0x00, 0x12, 0x00, 0xff),
> > raw_pdu(0x18, 0x01),
> > raw_pdu(0x19));
> >
> > @@ -4374,8 +4374,8 @@ int main(int argc, char *argv[])
> > raw_pdu(0x17, 0x04, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff,
> > 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> > 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff),
> > - raw_pdu(0x16, 0x04, 0x00, 0x3f, 0x00, 0xff),
> > - raw_pdu(0x17, 0x04, 0x00, 0x3f, 0x00, 0xff),
> > + raw_pdu(0x16, 0x04, 0x00, 0x12, 0x00, 0xff),
> > + raw_pdu(0x17, 0x04, 0x00, 0x12, 0x00, 0xff),
> > raw_pdu(0x18, 0x01),
> > raw_pdu(0x19));
> >
> > @@ -4388,8 +4388,8 @@ int main(int argc, char *argv[])
> > raw_pdu(0x17, 0x83, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff,
> > 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> > 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff),
> > - raw_pdu(0x16, 0x83, 0x00, 0x3f, 0x00, 0xff),
> > - raw_pdu(0x17, 0x83, 0x00, 0x3f, 0x00, 0xff),
> > + raw_pdu(0x16, 0x83, 0x00, 0x12, 0x00, 0xff),
> > + raw_pdu(0x17, 0x83, 0x00, 0x12, 0x00, 0xff),
> > raw_pdu(0x18, 0x01),
> > raw_pdu(0x19));
> >
> > --
> > 2.5.0
> >
> > --
> > 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
--
BR / Pozdrawiam
Łukasz
Hi Łukasz,
On Fri, Nov 27, 2015 at 1:11 PM, Łukasz Rymanowski
<[email protected]> wrote:
> Android expect information either its indication or notification.
> ---
> profiles/scanparam/scan.c | 5 +++--
> src/gatt-client.c | 4 ++--
> src/shared/gatt-client.c | 13 +++++++++----
> src/shared/gatt-client.h | 13 ++++++++++---
> tools/btgatt-client.c | 2 +-
> unit/test-gatt.c | 14 ++++++++++----
> 6 files changed, 35 insertions(+), 16 deletions(-)
>
> diff --git a/profiles/scanparam/scan.c b/profiles/scanparam/scan.c
> index 4015b3f..fd01135 100644
> --- a/profiles/scanparam/scan.c
> +++ b/profiles/scanparam/scan.c
> @@ -92,8 +92,9 @@ static void write_scan_params(struct scan *scan)
> false, value, sizeof(value));
> }
>
> -static void refresh_value_cb(uint16_t value_handle, const uint8_t *value,
> - uint16_t length, void *user_data)
> +static void refresh_value_cb(notify_t type, uint16_t value_handle,
> + const uint8_t *value, uint16_t length,
> + void *user_data)
> {
> struct scan *scan = user_data;
>
> diff --git a/src/gatt-client.c b/src/gatt-client.c
> index 39f6646..e74803c 100644
> --- a/src/gatt-client.c
> +++ b/src/gatt-client.c
> @@ -1057,8 +1057,8 @@ static bool match_notify_sender(const void *a, const void *b)
> return strcmp(client->owner, sender) == 0;
> }
>
> -static void notify_cb(uint16_t value_handle, const uint8_t *value,
> - uint16_t length, void *user_data)
> +static void notify_cb(notify_t type, uint16_t value_handle,
> + const uint8_t *value, uint16_t length, void *user_data)
> {
> struct async_dbus_op *op = user_data;
> struct notify_client *client = op->data;
> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
> index 06ac763..76e4a9a 100644
> --- a/src/shared/gatt-client.c
> +++ b/src/shared/gatt-client.c
> @@ -1109,7 +1109,7 @@ struct service_changed_op {
> static void process_service_changed(struct bt_gatt_client *client,
> uint16_t start_handle,
> uint16_t end_handle);
> -static void service_changed_cb(uint16_t value_handle, const uint8_t *value,
> +static void service_changed_cb(notify_t type, uint16_t value_handle, const uint8_t *value,
> uint16_t length, void *user_data);
>
> static void complete_notify_request(void *data)
> @@ -1446,8 +1446,9 @@ fail:
> " after Service Changed");
> }
>
> -static void service_changed_cb(uint16_t value_handle, const uint8_t *value,
> - uint16_t length, void *user_data)
> +static void service_changed_cb(notify_t type, uint16_t value_handle,
> + const uint8_t *value, uint16_t length,
> + void *user_data)
> {
> struct bt_gatt_client *client = user_data;
> struct service_changed_op *op;
> @@ -1545,6 +1546,7 @@ static bool gatt_client_init(struct bt_gatt_client *client, uint16_t mtu)
> }
>
> struct pdu_data {
> + notify_t type;
> const void *pdu;
> uint16_t length;
> };
> @@ -1614,7 +1616,8 @@ static void notify_handler(void *data, void *user_data)
> * CCC, there is really no reason not to notify the handlers.
> */
> if (notify_data->notify)
> - notify_data->notify(value_handle, value, pdu_data->length - 2,
> + notify_data->notify(pdu_data->type, value_handle, value,
> + pdu_data->length - 2,
> notify_data->user_data);
> }
>
> @@ -1629,6 +1632,8 @@ static void notify_cb(uint8_t opcode, const void *pdu, uint16_t length,
> memset(&pdu_data, 0, sizeof(pdu_data));
> pdu_data.pdu = pdu;
> pdu_data.length = length;
> + pdu_data.type = opcode == BT_ATT_OP_HANDLE_VAL_IND ?
> + BT_GATT_INDICATION : BT_GATT_NOTIFICATION;
>
> queue_foreach(client->notify_list, notify_handler, &pdu_data);
>
> diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
> index befa43f..3018a99 100644
> --- a/src/shared/gatt-client.h
> +++ b/src/shared/gatt-client.h
> @@ -27,6 +27,11 @@
>
> #define BT_GATT_UUID_SIZE 16
>
> +typedef enum {
> + BT_GATT_NOTIFICATION,
> + BT_GATT_INDICATION,
> +} notify_t;
> +
> struct bt_gatt_client;
>
> struct bt_gatt_client *bt_gatt_client_new(struct gatt_db *db,
> @@ -46,9 +51,11 @@ typedef void (*bt_gatt_client_read_callback_t)(bool success, uint8_t att_ecode,
> typedef void (*bt_gatt_client_write_long_callback_t)(bool success,
> bool reliable_error, uint8_t att_ecode,
> void *user_data);
> -typedef void (*bt_gatt_client_notify_callback_t)(uint16_t value_handle,
> - const uint8_t *value, uint16_t length,
> - void *user_data);
> +typedef void (*bt_gatt_client_notify_callback_t)(notify_t type,
> + uint16_t value_handle,
> + const uint8_t *value,
> + uint16_t length,
> + void *user_data);
Maybe I was not very clear last time but I prefer to have a dedicated
type for indications so we can actually have a return to it. Or
perhaps we should do this via db since nowadays bt_gatt_client build a
cache it could in theory support value notification and indication by
attribute directly in the db.
> typedef void (*bt_gatt_client_register_callback_t)(uint16_t att_ecode,
> void *user_data);
> typedef void (*bt_gatt_client_service_changed_callback_t)(uint16_t start_handle,
> diff --git a/tools/btgatt-client.c b/tools/btgatt-client.c
> index 0f6a1bd..76fd6d0 100644
> --- a/tools/btgatt-client.c
> +++ b/tools/btgatt-client.c
> @@ -1099,7 +1099,7 @@ static void register_notify_usage(void)
> printf("Usage: register-notify <chrc value handle>\n");
> }
>
> -static void notify_cb(uint16_t value_handle, const uint8_t *value,
> +static void notify_cb(notify_t type, uint16_t value_handle, const uint8_t *value,
> uint16_t length, void *user_data)
> {
> int i;
> diff --git a/unit/test-gatt.c b/unit/test-gatt.c
> index 87a87b8..6b65525 100644
> --- a/unit/test-gatt.c
> +++ b/unit/test-gatt.c
> @@ -2211,12 +2211,15 @@ static const struct test_step test_long_read_15 = {
> .expected_att_ecode = 0x0c
> };
>
> -static void notification_cb(uint16_t value_handle, const uint8_t *value,
> - uint16_t length, void *user_data)
> +static void notification_cb(notify_t type, uint16_t value_handle,
> + const uint8_t *value, uint16_t length,
> + void *user_data)
> {
> struct context *context = user_data;
> const struct test_step *step = context->data->step;
>
> + g_assert(type == BT_GATT_NOTIFICATION);
> +
> if (value_handle == step->handle) {
> g_assert_cmpint(length, ==, step->length);
>
> @@ -2279,12 +2282,15 @@ static void test_server_indication_confirm(struct context *context)
> g_assert(indication_received == 1);
> }
>
> -static void indication_cb(uint16_t value_handle, const uint8_t *value,
> - uint16_t length, void *user_data)
> +static void indication_cb(notify_t type, uint16_t value_handle,
> + const uint8_t *value, uint16_t length,
> + void *user_data)
> {
> struct context *context = user_data;
> const struct test_step *step = context->data->step;
>
> + g_assert(type == BT_GATT_INDICATION);
> +
> if (value_handle == step->handle) {
> g_assert_cmpint(length, ==, step->length);
>
> --
> 2.5.0
>
> --
> 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
Hi Łukasz,
On Fri, Nov 27, 2015 at 1:11 PM, Łukasz Rymanowski
<[email protected]> wrote:
> Idea of long write is that each part of data is continuation
> of previous one. There shall be not gaps in the offsets between.
>
> If there are gaps in offset then we have reliable write rather than
> long write
The patch looks fine what I did not understand is what different it
makes if it is a reliable write or a long write, to me all long writes
are in fact reliable write since it does use prepare + execute, the
fact that only one handle is written doesn't change anything.
> ---
> unit/test-gatt.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/unit/test-gatt.c b/unit/test-gatt.c
> index 326a32c..70b6de5 100644
> --- a/unit/test-gatt.c
> +++ b/unit/test-gatt.c
> @@ -3852,8 +3852,8 @@ int main(int argc, char *argv[])
> raw_pdu(0x17, 0x03, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff,
> 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff),
> - raw_pdu(0x16, 0x03, 0x00, 0x3f, 0x00, 0xff),
> - raw_pdu(0x17, 0x03, 0x00, 0x3f, 0x00, 0xff),
> + raw_pdu(0x16, 0x03, 0x00, 0x12, 0x00, 0xff),
> + raw_pdu(0x17, 0x03, 0x00, 0x12, 0x00, 0xff),
> raw_pdu(0x18, 0x01),
> raw_pdu(0x19));
>
> @@ -3866,8 +3866,8 @@ int main(int argc, char *argv[])
> raw_pdu(0x17, 0x82, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff,
> 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff),
> - raw_pdu(0x16, 0x82, 0x00, 0x3f, 0x00, 0xff),
> - raw_pdu(0x17, 0x82, 0x00, 0x3f, 0x00, 0xff),
> + raw_pdu(0x16, 0x82, 0x00, 0x12, 0x00, 0xff),
> + raw_pdu(0x17, 0x82, 0x00, 0x12, 0x00, 0xff),
> raw_pdu(0x18, 0x01),
> raw_pdu(0x19));
>
> @@ -4374,8 +4374,8 @@ int main(int argc, char *argv[])
> raw_pdu(0x17, 0x04, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff,
> 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff),
> - raw_pdu(0x16, 0x04, 0x00, 0x3f, 0x00, 0xff),
> - raw_pdu(0x17, 0x04, 0x00, 0x3f, 0x00, 0xff),
> + raw_pdu(0x16, 0x04, 0x00, 0x12, 0x00, 0xff),
> + raw_pdu(0x17, 0x04, 0x00, 0x12, 0x00, 0xff),
> raw_pdu(0x18, 0x01),
> raw_pdu(0x19));
>
> @@ -4388,8 +4388,8 @@ int main(int argc, char *argv[])
> raw_pdu(0x17, 0x83, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff,
> 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff),
> - raw_pdu(0x16, 0x83, 0x00, 0x3f, 0x00, 0xff),
> - raw_pdu(0x17, 0x83, 0x00, 0x3f, 0x00, 0xff),
> + raw_pdu(0x16, 0x83, 0x00, 0x12, 0x00, 0xff),
> + raw_pdu(0x17, 0x83, 0x00, 0x12, 0x00, 0xff),
> raw_pdu(0x18, 0x01),
> raw_pdu(0x19));
>
> --
> 2.5.0
>
> --
> 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
Hi Luiz,
On 11 March 2016 at 15:30, Luiz Augusto von Dentz <[email protected]> wrote:
> Hi Lukasz,
>
> On Fri, Mar 11, 2016 at 3:27 PM, Łukasz Rymanowski
> <[email protected]> wrote:
>> Hi Luiz,
>>
>> On 3 December 2015 at 22:08, Łukasz Rymanowski
>> <[email protected]> wrote:
>>> Hi Luiz,
>>>
>>> On Thu, Dec 3, 2015 at 10:27 AM, Luiz Augusto von Dentz
>>> <[email protected]> wrote:
>>>>
>>>> Hi Łukasz,
>>>>
>>>> On Fri, Nov 27, 2015 at 1:11 PM, Łukasz Rymanowski
>>>> <[email protected]> wrote:
>>>> > Idea of long write is that each part of data is continuation
>>>> > of previous one. There shall be not gaps in the offsets between.
>>>> >
>>>> > If there are gaps in offset then we have reliable write rather than
>>>> > long write
>>>>
>>>> The patch looks fine what I did not understand is what different it
>>>> makes if it is a reliable write or a long write, to me all long writes
>>>> are in fact reliable write since it does use prepare + execute, the
>>>> fact that only one handle is written doesn't change anything.
>>>>
>>>
>>> These testes are for Long write and there is a difference between Long Write and
>>> Reliable Write even those two use prepare/execute writes. This can be
>>> found in the
>>> BT Spec Chapter 3 Part G, 4.9.4 and 4.9.5
>>>
>>> Basically those procedure belongs to GATT and our gatt server should
>>> understand that.
>>>
>>> If we are going to not expose prepare/execute wirtes to application,
>>> then this is GATT server which should
>>> recognize procedure and prepare data to be written to application.
>>> i.e. for long write do aggregation of all
>>> prepare writes, we can not just write part of data as this would cause
>>> some unexpected behavior.
>>>
>>> We also need this change, because following patches will start to test
>>> for characteristic extended properties
>>> descriptor and do not allow reliable write on characteristics which
>>> does not have property for this.
>>> Note that for Long Write we don't need this descriptor property, so
>>> these test cases would fail then.
>>>
>>> Also, note that there is Long Write procedure for descriptors but
>>> there is no Reliable Write procedure for
>>> descriptors, so we definitely cannot treat long and prepare as the same one.
>>>
>>>
>> Was my explanation clear enough?
>>
>> Just getting back to this work and we have some outstanding patches in
>> this patchset
>> which should be OK to apply.
>
> I guess my point was not really against this patch but more on the
> detection of reliable write vs long write.
Yes I recall that, that is why I'm asking if my explanation is OK.
Note that detection of long/prepare is done in next patch:
[PATCH v2 08/11] shared/gatt-server: Add support for long write
--
BR / Pozdrawiam
Łukasz
Hi Lukasz,
On Fri, Mar 11, 2016 at 3:27 PM, Łukasz Rymanowski
<[email protected]> wrote:
> Hi Luiz,
>
> On 3 December 2015 at 22:08, Łukasz Rymanowski
> <[email protected]> wrote:
>> Hi Luiz,
>>
>> On Thu, Dec 3, 2015 at 10:27 AM, Luiz Augusto von Dentz
>> <[email protected]> wrote:
>>>
>>> Hi Łukasz,
>>>
>>> On Fri, Nov 27, 2015 at 1:11 PM, Łukasz Rymanowski
>>> <[email protected]> wrote:
>>> > Idea of long write is that each part of data is continuation
>>> > of previous one. There shall be not gaps in the offsets between.
>>> >
>>> > If there are gaps in offset then we have reliable write rather than
>>> > long write
>>>
>>> The patch looks fine what I did not understand is what different it
>>> makes if it is a reliable write or a long write, to me all long writes
>>> are in fact reliable write since it does use prepare + execute, the
>>> fact that only one handle is written doesn't change anything.
>>>
>>
>> These testes are for Long write and there is a difference between Long Write and
>> Reliable Write even those two use prepare/execute writes. This can be
>> found in the
>> BT Spec Chapter 3 Part G, 4.9.4 and 4.9.5
>>
>> Basically those procedure belongs to GATT and our gatt server should
>> understand that.
>>
>> If we are going to not expose prepare/execute wirtes to application,
>> then this is GATT server which should
>> recognize procedure and prepare data to be written to application.
>> i.e. for long write do aggregation of all
>> prepare writes, we can not just write part of data as this would cause
>> some unexpected behavior.
>>
>> We also need this change, because following patches will start to test
>> for characteristic extended properties
>> descriptor and do not allow reliable write on characteristics which
>> does not have property for this.
>> Note that for Long Write we don't need this descriptor property, so
>> these test cases would fail then.
>>
>> Also, note that there is Long Write procedure for descriptors but
>> there is no Reliable Write procedure for
>> descriptors, so we definitely cannot treat long and prepare as the same one.
>>
>>
> Was my explanation clear enough?
>
> Just getting back to this work and we have some outstanding patches in
> this patchset
> which should be OK to apply.
I guess my point was not really against this patch but more on the
detection of reliable write vs long write.
Hi Luiz,
On 3 December 2015 at 22:08, Łukasz Rymanowski
<[email protected]> wrote:
> Hi Luiz,
>
> On Thu, Dec 3, 2015 at 10:27 AM, Luiz Augusto von Dentz
> <[email protected]> wrote:
>>
>> Hi Łukasz,
>>
>> On Fri, Nov 27, 2015 at 1:11 PM, Łukasz Rymanowski
>> <[email protected]> wrote:
>> > Idea of long write is that each part of data is continuation
>> > of previous one. There shall be not gaps in the offsets between.
>> >
>> > If there are gaps in offset then we have reliable write rather than
>> > long write
>>
>> The patch looks fine what I did not understand is what different it
>> makes if it is a reliable write or a long write, to me all long writes
>> are in fact reliable write since it does use prepare + execute, the
>> fact that only one handle is written doesn't change anything.
>>
>
> These testes are for Long write and there is a difference between Long Write and
> Reliable Write even those two use prepare/execute writes. This can be
> found in the
> BT Spec Chapter 3 Part G, 4.9.4 and 4.9.5
>
> Basically those procedure belongs to GATT and our gatt server should
> understand that.
>
> If we are going to not expose prepare/execute wirtes to application,
> then this is GATT server which should
> recognize procedure and prepare data to be written to application.
> i.e. for long write do aggregation of all
> prepare writes, we can not just write part of data as this would cause
> some unexpected behavior.
>
> We also need this change, because following patches will start to test
> for characteristic extended properties
> descriptor and do not allow reliable write on characteristics which
> does not have property for this.
> Note that for Long Write we don't need this descriptor property, so
> these test cases would fail then.
>
> Also, note that there is Long Write procedure for descriptors but
> there is no Reliable Write procedure for
> descriptors, so we definitely cannot treat long and prepare as the same one.
>
>
Was my explanation clear enough?
Just getting back to this work and we have some outstanding patches in
this patchset
which should be OK to apply.
>> > ---
>> > unit/test-gatt.c | 16 ++++++++--------
>> > 1 file changed, 8 insertions(+), 8 deletions(-)
>> >
>> > diff --git a/unit/test-gatt.c b/unit/test-gatt.c
>> > index 326a32c..70b6de5 100644
>> > --- a/unit/test-gatt.c
>> > +++ b/unit/test-gatt.c
>> > @@ -3852,8 +3852,8 @@ int main(int argc, char *argv[])
>> > raw_pdu(0x17, 0x03, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff,
>> > 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
>> > 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff),
>> > - raw_pdu(0x16, 0x03, 0x00, 0x3f, 0x00, 0xff),
>> > - raw_pdu(0x17, 0x03, 0x00, 0x3f, 0x00, 0xff),
>> > + raw_pdu(0x16, 0x03, 0x00, 0x12, 0x00, 0xff),
>> > + raw_pdu(0x17, 0x03, 0x00, 0x12, 0x00, 0xff),
>> > raw_pdu(0x18, 0x01),
>> > raw_pdu(0x19));
>> >
>> > @@ -3866,8 +3866,8 @@ int main(int argc, char *argv[])
>> > raw_pdu(0x17, 0x82, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff,
>> > 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
>> > 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff),
>> > - raw_pdu(0x16, 0x82, 0x00, 0x3f, 0x00, 0xff),
>> > - raw_pdu(0x17, 0x82, 0x00, 0x3f, 0x00, 0xff),
>> > + raw_pdu(0x16, 0x82, 0x00, 0x12, 0x00, 0xff),
>> > + raw_pdu(0x17, 0x82, 0x00, 0x12, 0x00, 0xff),
>> > raw_pdu(0x18, 0x01),
>> > raw_pdu(0x19));
>> >
>> > @@ -4374,8 +4374,8 @@ int main(int argc, char *argv[])
>> > raw_pdu(0x17, 0x04, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff,
>> > 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
>> > 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff),
>> > - raw_pdu(0x16, 0x04, 0x00, 0x3f, 0x00, 0xff),
>> > - raw_pdu(0x17, 0x04, 0x00, 0x3f, 0x00, 0xff),
>> > + raw_pdu(0x16, 0x04, 0x00, 0x12, 0x00, 0xff),
>> > + raw_pdu(0x17, 0x04, 0x00, 0x12, 0x00, 0xff),
>> > raw_pdu(0x18, 0x01),
>> > raw_pdu(0x19));
>> >
>> > @@ -4388,8 +4388,8 @@ int main(int argc, char *argv[])
>> > raw_pdu(0x17, 0x83, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff,
>> > 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
>> > 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff),
>> > - raw_pdu(0x16, 0x83, 0x00, 0x3f, 0x00, 0xff),
>> > - raw_pdu(0x17, 0x83, 0x00, 0x3f, 0x00, 0xff),
>> > + raw_pdu(0x16, 0x83, 0x00, 0x12, 0x00, 0xff),
>> > + raw_pdu(0x17, 0x83, 0x00, 0x12, 0x00, 0xff),
>> > raw_pdu(0x18, 0x01),
>> > raw_pdu(0x19));
>> >
>> > --
>> > 2.5.0
>> >
>> > --
>> > 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
>
>
>
>
> --
> BR / Pozdrawiam
> Łukasz
--
BR / Pozdrawiam
Łukasz