* [PATCH 1/2] shared/gatt-client: Add write prepare and execute
@ 2015-01-14 8:17 Lukasz Rymanowski
2015-01-14 8:17 ` [PATCH 2/2] tools/btgatt-client: Add prepare and execute write Lukasz Rymanowski
2015-01-14 12:56 ` [PATCH 1/2] shared/gatt-client: Add write prepare and execute Luiz Augusto von Dentz
0 siblings, 2 replies; 9+ messages in thread
From: Lukasz Rymanowski @ 2015-01-14 8:17 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Lukasz Rymanowski
For Android we need explicit write prepare and execute commands in GATT
client. This patch adds that.
Note that till now prepare and execute has been used only in long write.
This patch makes sure that long write is not proceeded when prepare write
has been called. Instead long write commands will be queued and
once write execute command is done, queued long write requests will be
proceeded.
It does not work in other way. Meaning, when long write is ongoing,
prepare write will not be proceeded nor queued. This feature can be
added later on if really needed.
Conflicts:
src/shared/gatt-client.c
---
src/shared/gatt-client.c | 215 ++++++++++++++++++++++++++++++++++++++++++++++-
src/shared/gatt-client.h | 11 +++
2 files changed, 225 insertions(+), 1 deletion(-)
diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index 1acd34f..c7c29cf 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -75,6 +75,8 @@ struct bt_gatt_client {
struct queue *long_write_queue;
bool in_long_write;
+ bool in_prep_write;
+
/* List of registered disconnect/notification/indication callbacks */
struct queue *notify_list;
struct queue *notify_chrcs;
@@ -2172,6 +2174,7 @@ unsigned int bt_gatt_client_write_without_response(
}
struct write_op {
+ struct bt_gatt_client *client;
bt_gatt_client_callback_t callback;
void *user_data;
bt_gatt_destroy_func_t destroy;
@@ -2524,7 +2527,7 @@ unsigned int bt_gatt_client_write_long_value(struct bt_gatt_client *client,
req->destroy = long_write_op_free;
req->long_write = true;
- if (client->in_long_write) {
+ if (client->in_long_write || client->in_prep_write) {
queue_push_tail(client->long_write_queue, req);
return req->id;
}
@@ -2557,6 +2560,216 @@ unsigned int bt_gatt_client_write_long_value(struct bt_gatt_client *client,
return req->id;
}
+struct prep_write_op {
+ bt_gatt_client_write_long_callback_t callback;
+ void *user_data;
+ bt_gatt_destroy_func_t destroy;
+ uint8_t *pdu;
+ uint16_t pdu_len;
+};
+
+static void destroy_prep_write_op(void *data)
+{
+ struct prep_write_op *op = data;
+
+ if (op->destroy)
+ op->destroy(op->user_data);
+
+ free(op->pdu);
+ free(op);
+}
+
+static void prep_write_cb(uint8_t opcode, const void *pdu, uint16_t length,
+ void *user_data)
+{
+ struct request *req = user_data;
+ struct prep_write_op *op = req->data;
+ bool success;
+ uint8_t att_ecode;
+ bool reliable_error;
+
+ if (opcode == BT_ATT_OP_ERROR_RSP) {
+ success = false;
+ reliable_error = false;
+ att_ecode = process_error(pdu, length);
+ goto done;
+ }
+
+ if (opcode != BT_ATT_OP_PREP_WRITE_RSP) {
+ success = false;
+ reliable_error = false;
+ att_ecode = 0;
+ goto done;
+ }
+
+ if (!pdu || length != op->pdu_len ||
+ memcmp(pdu, op->pdu, op->pdu_len)) {
+ success = false;
+ reliable_error = true;
+ att_ecode = 0;
+ goto done;
+ }
+
+ success = true;
+ reliable_error = false;
+ att_ecode = 0;
+
+done:
+ if (op->callback)
+ op->callback(success, reliable_error, att_ecode, op->user_data);
+}
+
+unsigned int bt_gatt_client_prepare_write(struct bt_gatt_client *client,
+ uint16_t value_handle, uint16_t offset,
+ uint8_t *value, uint16_t length,
+ bt_gatt_client_write_long_callback_t callback,
+ void *user_data,
+ bt_gatt_client_destroy_func_t destroy)
+{
+ struct request *req;
+ struct prep_write_op *op;
+ uint8_t pdu[4 + length];
+
+ if (!client)
+ return 0;
+
+ if (client->in_long_write)
+ return 0;
+
+ op = new0(struct prep_write_op, 1);
+ if (!op)
+ return 0;
+
+ req = request_create(client);
+ if (!req) {
+ free(op);
+ return 0;
+ }
+
+ op->callback = callback;
+ op->user_data = user_data;
+ op->destroy = destroy;
+
+ req->data = op;
+ req->destroy = destroy_prep_write_op;
+
+ put_le16(value_handle, pdu);
+ put_le16(offset, pdu + 2);
+ memcpy(pdu + 4, value, length);
+
+ /*
+ * Before sending command we need to remember pdu as we need to validate
+ * it in the response. Store handle, offset and value. Therefore
+ * increase length by 4 (handle + offset) as we need it in couple places
+ * below
+ */
+ length += 4;
+
+ op->pdu = malloc(length);
+ if (!op->pdu) {
+ op->destroy = NULL;
+ request_unref(req);
+ return 0;
+ }
+
+ memcpy(op->pdu, pdu, length);
+ op->pdu_len = length;
+
+ /* Now we are ready to send command */
+ req->att_id = bt_att_send(client->att, BT_ATT_OP_PREP_WRITE_REQ, pdu,
+ sizeof(pdu), prep_write_cb, req,
+ request_unref);
+ if (!req->att_id) {
+ op->destroy = NULL;
+ request_unref(req);
+ return 0;
+ }
+
+ client->in_prep_write = true;
+
+ return req->id;
+}
+
+static void exec_write_cb(uint8_t opcode, const void *pdu, uint16_t length,
+ void *user_data)
+{
+ struct request *req = user_data;
+ struct write_op *op = req->data;
+ bool success;
+ uint8_t att_ecode;
+
+ if (opcode == BT_ATT_OP_ERROR_RSP) {
+ success = false;
+ att_ecode = process_error(pdu, length);
+ goto done;
+ }
+
+ if (opcode != BT_ATT_OP_EXEC_WRITE_RSP || pdu || length) {
+ success = false;
+ att_ecode = 0;
+ goto done;
+ }
+
+ success = true;
+ att_ecode = 0;
+
+done:
+ if (op->callback)
+ op->callback(success, att_ecode, op->user_data);
+
+ op->client->in_prep_write = false;
+
+ start_next_long_write(op->client);
+}
+
+unsigned int bt_gatt_client_write_execute(struct bt_gatt_client *client,
+ bool execute,
+ bt_gatt_client_callback_t callback,
+ void *user_data,
+ bt_gatt_client_destroy_func_t destroy)
+{
+ struct request *req;
+ struct write_op *op;
+ uint8_t pdu;
+
+ if (!client)
+ return false;
+
+ if (client->in_long_write || !client->in_prep_write)
+ return false;
+
+ op = new0(struct write_op, 1);
+ if (!op)
+ return false;
+
+ req = request_create(client);
+ if (!req) {
+ free(op);
+ return 0;
+ }
+
+ op->client = client;
+ op->callback = callback;
+ op->user_data = user_data;
+ op->destroy = destroy;
+
+ pdu = execute ? 0x01 : 0x00;
+
+ req->data = op;
+ req->destroy = destroy_write_op;
+
+ req->att_id = bt_att_send(client->att, BT_ATT_OP_EXEC_WRITE_REQ, &pdu,
+ sizeof(pdu), exec_write_cb,
+ req, request_unref);
+ if (!req->att_id) {
+ op->destroy = NULL;
+ request_unref(req);
+ return 0;
+ }
+
+ return req->att_id;
+}
+
static bool match_notify_chrc_value_handle(const void *a, const void *b)
{
const struct notify_chrc *chrc = a;
diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
index 9a00feb..45c02c8 100644
--- a/src/shared/gatt-client.h
+++ b/src/shared/gatt-client.h
@@ -109,6 +109,17 @@ unsigned int bt_gatt_client_write_long_value(struct bt_gatt_client *client,
bt_gatt_client_write_long_callback_t callback,
void *user_data,
bt_gatt_client_destroy_func_t destroy);
+unsigned int bt_gatt_client_prepare_write(struct bt_gatt_client *client,
+ uint16_t value_handle, uint16_t offset,
+ uint8_t *value, uint16_t length,
+ bt_gatt_client_write_long_callback_t callback,
+ void *user_data,
+ bt_gatt_client_destroy_func_t destroy);
+unsigned int bt_gatt_client_write_execute(struct bt_gatt_client *client,
+ bool execute,
+ bt_gatt_client_callback_t callback,
+ void *user_data,
+ bt_gatt_client_destroy_func_t destroy);
bool bt_gatt_client_register_notify(struct bt_gatt_client *client,
uint16_t chrc_value_handle,
--
1.8.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] tools/btgatt-client: Add prepare and execute write
2015-01-14 8:17 [PATCH 1/2] shared/gatt-client: Add write prepare and execute Lukasz Rymanowski
@ 2015-01-14 8:17 ` Lukasz Rymanowski
2015-01-14 12:56 ` [PATCH 1/2] shared/gatt-client: Add write prepare and execute Luiz Augusto von Dentz
1 sibling, 0 replies; 9+ messages in thread
From: Lukasz Rymanowski @ 2015-01-14 8:17 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Lukasz Rymanowski
---
tools/btgatt-client.c | 138 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 138 insertions(+)
diff --git a/tools/btgatt-client.c b/tools/btgatt-client.c
index 62c4d3e..1cfeaba 100644
--- a/tools/btgatt-client.c
+++ b/tools/btgatt-client.c
@@ -831,6 +831,140 @@ static void cmd_write_long_value(struct client *cli, char *cmd_str)
free(value);
}
+static void write_prepare_usage(void)
+{
+ printf("Usage: write-prepare <value_handle> <offset> <value>\n"
+ "e.g.:\n"
+ "\twrite-prepare 0x0001 00 01 00\n");
+}
+
+static void cmd_write_prepare(struct client *cli, char *cmd_str)
+{
+ int i;
+ char *argvbuf[516];
+ char **argv = argvbuf;
+ int argc = 0;
+ uint16_t handle;
+ uint16_t offset;
+ char *endptr = NULL;
+ unsigned int length;
+ uint8_t *value = NULL;
+
+ if (!bt_gatt_client_is_ready(cli->gatt)) {
+ printf("GATT client not initialized\n");
+ return;
+ }
+
+ if (!parse_args(cmd_str, 514, argv, &argc)) {
+ printf("Too many arguments\n");
+ write_value_usage();
+ return;
+ }
+
+ if (argc < 3) {
+ write_prepare_usage();
+ return;
+ }
+
+ handle = strtol(argv[0], &endptr, 0);
+ if (!endptr || *endptr != '\0' || !handle) {
+ printf("Invalid handle: %s\n", argv[0]);
+ return;
+ }
+
+ endptr = NULL;
+ offset = strtol(argv[1], &endptr, 0);
+ if (!endptr || *endptr != '\0' || errno == ERANGE) {
+ printf("Invalid offset: %s\n", argv[1]);
+ return;
+ }
+
+ /*
+ * First two arguments are handle and offset. What remains is the value
+ * length
+ */
+ length = argc - 2;
+
+ if (length == 0)
+ goto done;
+
+ if (length > UINT16_MAX) {
+ printf("Write value too long\n");
+ return;
+ }
+
+ value = malloc(length);
+ if (!value) {
+ printf("Failed to allocate memory for value\n");
+ return;
+ }
+
+ for (i = 2; i < argc; i++) {
+ if (strlen(argv[i]) != 2) {
+ printf("Invalid value byte: %s\n", argv[i]);
+ free(value);
+ return;
+ }
+
+ value[i-2] = strtol(argv[i], &endptr, 0);
+ if (endptr == argv[i] || *endptr != '\0' || errno == ERANGE) {
+ printf("Invalid value byte: %s\n", argv[i]);
+ free(value);
+ return;
+ }
+ }
+
+done:
+ if (!bt_gatt_client_prepare_write(cli->gatt, handle, offset, value,
+ length, write_long_cb, NULL,
+ NULL))
+ printf("Failed to proceed prepare write\n");
+
+ free(value);
+}
+
+static void write_execute_usage(void)
+{
+ printf("Usage: write-execute <execute>\n"
+ "e.g.:\n"
+ "\twrite-execute 1\n");
+}
+
+static void cmd_write_execute(struct client *cli, char *cmd_str)
+{
+ char *argvbuf[516];
+ char **argv = argvbuf;
+ int argc = 0;
+ char *endptr = NULL;
+ bool execute;
+
+ if (!bt_gatt_client_is_ready(cli->gatt)) {
+ printf("GATT client not initialized\n");
+ return;
+ }
+
+ if (!parse_args(cmd_str, 514, argv, &argc)) {
+ printf("Too many arguments\n");
+ write_value_usage();
+ return;
+ }
+
+ if (argc < 1) {
+ write_execute_usage();
+ return;
+ }
+
+ execute = !!strtol(argv[0], &endptr, 0);
+ if (!endptr || *endptr != '\0') {
+ printf("Invalid execute: %s\n", argv[0]);
+ return;
+ }
+
+ if (!bt_gatt_client_write_execute(cli->gatt, execute, write_cb,
+ NULL, NULL))
+ printf("Failed to proceed write execute\n");
+}
+
static void register_notify_usage(void)
{
printf("Usage: register-notify <chrc value handle>\n");
@@ -955,6 +1089,10 @@ static struct {
"\tWrite a characteristic or descriptor value" },
{ "write-long-value", cmd_write_long_value,
"Write long characteristic or descriptor value" },
+ { "write-prepare", cmd_write_prepare,
+ "Write prepare characteristic or descriptor value" },
+ { "write-execute", cmd_write_execute,
+ "Execute already prepared write" },
{ "register-notify", cmd_register_notify,
"\tSubscribe to not/ind from a characteristic" },
{ "unregister-notify", cmd_unregister_notify,
--
1.8.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] shared/gatt-client: Add write prepare and execute
2015-01-14 8:17 [PATCH 1/2] shared/gatt-client: Add write prepare and execute Lukasz Rymanowski
2015-01-14 8:17 ` [PATCH 2/2] tools/btgatt-client: Add prepare and execute write Lukasz Rymanowski
@ 2015-01-14 12:56 ` Luiz Augusto von Dentz
2015-01-14 13:52 ` Lukasz Rymanowski
1 sibling, 1 reply; 9+ messages in thread
From: Luiz Augusto von Dentz @ 2015-01-14 12:56 UTC (permalink / raw)
To: Lukasz Rymanowski; +Cc: linux-bluetooth
Hi Lukasz,
On Wed, Jan 14, 2015 at 6:17 AM, Lukasz Rymanowski
<lukasz.rymanowski@tieto.com> wrote:
> For Android we need explicit write prepare and execute commands in GATT
> client. This patch adds that.
>
> Note that till now prepare and execute has been used only in long write.
>
> This patch makes sure that long write is not proceeded when prepare write
> has been called. Instead long write commands will be queued and
> once write execute command is done, queued long write requests will be
> proceeded.
>
> It does not work in other way. Meaning, when long write is ongoing,
> prepare write will not be proceeded nor queued. This feature can be
> added later on if really needed.
>
> Conflicts:
> src/shared/gatt-client.c
> ---
> src/shared/gatt-client.c | 215 ++++++++++++++++++++++++++++++++++++++++++++++-
> src/shared/gatt-client.h | 11 +++
> 2 files changed, 225 insertions(+), 1 deletion(-)
>
> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
> index 1acd34f..c7c29cf 100644
> --- a/src/shared/gatt-client.c
> +++ b/src/shared/gatt-client.c
> @@ -75,6 +75,8 @@ struct bt_gatt_client {
> struct queue *long_write_queue;
> bool in_long_write;
>
> + bool in_prep_write;
> +
> /* List of registered disconnect/notification/indication callbacks */
> struct queue *notify_list;
> struct queue *notify_chrcs;
> @@ -2172,6 +2174,7 @@ unsigned int bt_gatt_client_write_without_response(
> }
>
> struct write_op {
> + struct bt_gatt_client *client;
> bt_gatt_client_callback_t callback;
> void *user_data;
> bt_gatt_destroy_func_t destroy;
> @@ -2524,7 +2527,7 @@ unsigned int bt_gatt_client_write_long_value(struct bt_gatt_client *client,
> req->destroy = long_write_op_free;
> req->long_write = true;
>
> - if (client->in_long_write) {
> + if (client->in_long_write || client->in_prep_write) {
> queue_push_tail(client->long_write_queue, req);
> return req->id;
> }
> @@ -2557,6 +2560,216 @@ unsigned int bt_gatt_client_write_long_value(struct bt_gatt_client *client,
> return req->id;
> }
>
> +struct prep_write_op {
> + bt_gatt_client_write_long_callback_t callback;
> + void *user_data;
> + bt_gatt_destroy_func_t destroy;
> + uint8_t *pdu;
> + uint16_t pdu_len;
> +};
> +
> +static void destroy_prep_write_op(void *data)
> +{
> + struct prep_write_op *op = data;
> +
> + if (op->destroy)
> + op->destroy(op->user_data);
> +
> + free(op->pdu);
> + free(op);
> +}
> +
> +static void prep_write_cb(uint8_t opcode, const void *pdu, uint16_t length,
> + void *user_data)
> +{
> + struct request *req = user_data;
> + struct prep_write_op *op = req->data;
> + bool success;
> + uint8_t att_ecode;
> + bool reliable_error;
> +
> + if (opcode == BT_ATT_OP_ERROR_RSP) {
> + success = false;
> + reliable_error = false;
> + att_ecode = process_error(pdu, length);
> + goto done;
> + }
> +
> + if (opcode != BT_ATT_OP_PREP_WRITE_RSP) {
> + success = false;
> + reliable_error = false;
> + att_ecode = 0;
> + goto done;
> + }
> +
> + if (!pdu || length != op->pdu_len ||
> + memcmp(pdu, op->pdu, op->pdu_len)) {
> + success = false;
> + reliable_error = true;
> + att_ecode = 0;
> + goto done;
> + }
> +
> + success = true;
> + reliable_error = false;
> + att_ecode = 0;
> +
> +done:
> + if (op->callback)
> + op->callback(success, reliable_error, att_ecode, op->user_data);
> +}
> +
> +unsigned int bt_gatt_client_prepare_write(struct bt_gatt_client *client,
> + uint16_t value_handle, uint16_t offset,
> + uint8_t *value, uint16_t length,
> + bt_gatt_client_write_long_callback_t callback,
> + void *user_data,
> + bt_gatt_client_destroy_func_t destroy)
> +{
> + struct request *req;
> + struct prep_write_op *op;
> + uint8_t pdu[4 + length];
> +
> + if (!client)
> + return 0;
> +
> + if (client->in_long_write)
> + return 0;
> +
> + op = new0(struct prep_write_op, 1);
> + if (!op)
> + return 0;
> +
> + req = request_create(client);
> + if (!req) {
> + free(op);
> + return 0;
> + }
> +
> + op->callback = callback;
> + op->user_data = user_data;
> + op->destroy = destroy;
> +
> + req->data = op;
> + req->destroy = destroy_prep_write_op;
> +
> + put_le16(value_handle, pdu);
> + put_le16(offset, pdu + 2);
> + memcpy(pdu + 4, value, length);
> +
> + /*
> + * Before sending command we need to remember pdu as we need to validate
> + * it in the response. Store handle, offset and value. Therefore
> + * increase length by 4 (handle + offset) as we need it in couple places
> + * below
> + */
> + length += 4;
> +
> + op->pdu = malloc(length);
> + if (!op->pdu) {
> + op->destroy = NULL;
> + request_unref(req);
> + return 0;
> + }
> +
> + memcpy(op->pdu, pdu, length);
> + op->pdu_len = length;
> +
> + /* Now we are ready to send command */
> + req->att_id = bt_att_send(client->att, BT_ATT_OP_PREP_WRITE_REQ, pdu,
> + sizeof(pdu), prep_write_cb, req,
> + request_unref);
> + if (!req->att_id) {
> + op->destroy = NULL;
> + request_unref(req);
> + return 0;
> + }
> +
> + client->in_prep_write = true;
> +
> + return req->id;
> +}
> +
> +static void exec_write_cb(uint8_t opcode, const void *pdu, uint16_t length,
> + void *user_data)
> +{
> + struct request *req = user_data;
> + struct write_op *op = req->data;
> + bool success;
> + uint8_t att_ecode;
> +
> + if (opcode == BT_ATT_OP_ERROR_RSP) {
> + success = false;
> + att_ecode = process_error(pdu, length);
> + goto done;
> + }
> +
> + if (opcode != BT_ATT_OP_EXEC_WRITE_RSP || pdu || length) {
> + success = false;
> + att_ecode = 0;
> + goto done;
> + }
> +
> + success = true;
> + att_ecode = 0;
> +
> +done:
> + if (op->callback)
> + op->callback(success, att_ecode, op->user_data);
> +
> + op->client->in_prep_write = false;
> +
> + start_next_long_write(op->client);
> +}
> +
> +unsigned int bt_gatt_client_write_execute(struct bt_gatt_client *client,
> + bool execute,
> + bt_gatt_client_callback_t callback,
> + void *user_data,
> + bt_gatt_client_destroy_func_t destroy)
> +{
> + struct request *req;
> + struct write_op *op;
> + uint8_t pdu;
> +
> + if (!client)
> + return false;
> +
> + if (client->in_long_write || !client->in_prep_write)
> + return false;
> +
> + op = new0(struct write_op, 1);
> + if (!op)
> + return false;
> +
> + req = request_create(client);
> + if (!req) {
> + free(op);
> + return 0;
> + }
> +
> + op->client = client;
> + op->callback = callback;
> + op->user_data = user_data;
> + op->destroy = destroy;
> +
> + pdu = execute ? 0x01 : 0x00;
> +
> + req->data = op;
> + req->destroy = destroy_write_op;
> +
> + req->att_id = bt_att_send(client->att, BT_ATT_OP_EXEC_WRITE_REQ, &pdu,
> + sizeof(pdu), exec_write_cb,
> + req, request_unref);
> + if (!req->att_id) {
> + op->destroy = NULL;
> + request_unref(req);
> + return 0;
> + }
> +
> + return req->att_id;
> +}
> +
> static bool match_notify_chrc_value_handle(const void *a, const void *b)
> {
> const struct notify_chrc *chrc = a;
> diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
> index 9a00feb..45c02c8 100644
> --- a/src/shared/gatt-client.h
> +++ b/src/shared/gatt-client.h
> @@ -109,6 +109,17 @@ unsigned int bt_gatt_client_write_long_value(struct bt_gatt_client *client,
> bt_gatt_client_write_long_callback_t callback,
> void *user_data,
> bt_gatt_client_destroy_func_t destroy);
> +unsigned int bt_gatt_client_prepare_write(struct bt_gatt_client *client,
> + uint16_t value_handle, uint16_t offset,
> + uint8_t *value, uint16_t length,
> + bt_gatt_client_write_long_callback_t callback,
> + void *user_data,
> + bt_gatt_client_destroy_func_t destroy);
> +unsigned int bt_gatt_client_write_execute(struct bt_gatt_client *client,
> + bool execute,
> + bt_gatt_client_callback_t callback,
> + void *user_data,
> + bt_gatt_client_destroy_func_t destroy);
I think it is better to leave execute flag to be handled internally by
bt_gatt_client_cancel (which seems already handle that).
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] shared/gatt-client: Add write prepare and execute
2015-01-14 12:56 ` [PATCH 1/2] shared/gatt-client: Add write prepare and execute Luiz Augusto von Dentz
@ 2015-01-14 13:52 ` Lukasz Rymanowski
2015-01-19 16:09 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 9+ messages in thread
From: Lukasz Rymanowski @ 2015-01-14 13:52 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
Hi Luiz,
On 14 January 2015 at 13:56, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Lukasz,
>
> On Wed, Jan 14, 2015 at 6:17 AM, Lukasz Rymanowski
> <lukasz.rymanowski@tieto.com> wrote:
>> For Android we need explicit write prepare and execute commands in GATT
>> client. This patch adds that.
>>
>> Note that till now prepare and execute has been used only in long write.
>>
>> This patch makes sure that long write is not proceeded when prepare write
>> has been called. Instead long write commands will be queued and
>> once write execute command is done, queued long write requests will be
>> proceeded.
>>
>> It does not work in other way. Meaning, when long write is ongoing,
>> prepare write will not be proceeded nor queued. This feature can be
>> added later on if really needed.
>>
>> Conflicts:
>> src/shared/gatt-client.c
>> ---
>> src/shared/gatt-client.c | 215 ++++++++++++++++++++++++++++++++++++++++++++++-
>> src/shared/gatt-client.h | 11 +++
>> 2 files changed, 225 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
>> index 1acd34f..c7c29cf 100644
>> --- a/src/shared/gatt-client.c
>> +++ b/src/shared/gatt-client.c
>> @@ -75,6 +75,8 @@ struct bt_gatt_client {
>> struct queue *long_write_queue;
>> bool in_long_write;
>>
>> + bool in_prep_write;
>> +
>> /* List of registered disconnect/notification/indication callbacks */
>> struct queue *notify_list;
>> struct queue *notify_chrcs;
>> @@ -2172,6 +2174,7 @@ unsigned int bt_gatt_client_write_without_response(
>> }
>>
>> struct write_op {
>> + struct bt_gatt_client *client;
>> bt_gatt_client_callback_t callback;
>> void *user_data;
>> bt_gatt_destroy_func_t destroy;
>> @@ -2524,7 +2527,7 @@ unsigned int bt_gatt_client_write_long_value(struct bt_gatt_client *client,
>> req->destroy = long_write_op_free;
>> req->long_write = true;
>>
>> - if (client->in_long_write) {
>> + if (client->in_long_write || client->in_prep_write) {
>> queue_push_tail(client->long_write_queue, req);
>> return req->id;
>> }
>> @@ -2557,6 +2560,216 @@ unsigned int bt_gatt_client_write_long_value(struct bt_gatt_client *client,
>> return req->id;
>> }
>>
>> +struct prep_write_op {
>> + bt_gatt_client_write_long_callback_t callback;
>> + void *user_data;
>> + bt_gatt_destroy_func_t destroy;
>> + uint8_t *pdu;
>> + uint16_t pdu_len;
>> +};
>> +
>> +static void destroy_prep_write_op(void *data)
>> +{
>> + struct prep_write_op *op = data;
>> +
>> + if (op->destroy)
>> + op->destroy(op->user_data);
>> +
>> + free(op->pdu);
>> + free(op);
>> +}
>> +
>> +static void prep_write_cb(uint8_t opcode, const void *pdu, uint16_t length,
>> + void *user_data)
>> +{
>> + struct request *req = user_data;
>> + struct prep_write_op *op = req->data;
>> + bool success;
>> + uint8_t att_ecode;
>> + bool reliable_error;
>> +
>> + if (opcode == BT_ATT_OP_ERROR_RSP) {
>> + success = false;
>> + reliable_error = false;
>> + att_ecode = process_error(pdu, length);
>> + goto done;
>> + }
>> +
>> + if (opcode != BT_ATT_OP_PREP_WRITE_RSP) {
>> + success = false;
>> + reliable_error = false;
>> + att_ecode = 0;
>> + goto done;
>> + }
>> +
>> + if (!pdu || length != op->pdu_len ||
>> + memcmp(pdu, op->pdu, op->pdu_len)) {
>> + success = false;
>> + reliable_error = true;
>> + att_ecode = 0;
>> + goto done;
>> + }
>> +
>> + success = true;
>> + reliable_error = false;
>> + att_ecode = 0;
>> +
>> +done:
>> + if (op->callback)
>> + op->callback(success, reliable_error, att_ecode, op->user_data);
>> +}
>> +
>> +unsigned int bt_gatt_client_prepare_write(struct bt_gatt_client *client,
>> + uint16_t value_handle, uint16_t offset,
>> + uint8_t *value, uint16_t length,
>> + bt_gatt_client_write_long_callback_t callback,
>> + void *user_data,
>> + bt_gatt_client_destroy_func_t destroy)
>> +{
>> + struct request *req;
>> + struct prep_write_op *op;
>> + uint8_t pdu[4 + length];
>> +
>> + if (!client)
>> + return 0;
>> +
>> + if (client->in_long_write)
>> + return 0;
>> +
>> + op = new0(struct prep_write_op, 1);
>> + if (!op)
>> + return 0;
>> +
>> + req = request_create(client);
>> + if (!req) {
>> + free(op);
>> + return 0;
>> + }
>> +
>> + op->callback = callback;
>> + op->user_data = user_data;
>> + op->destroy = destroy;
>> +
>> + req->data = op;
>> + req->destroy = destroy_prep_write_op;
>> +
>> + put_le16(value_handle, pdu);
>> + put_le16(offset, pdu + 2);
>> + memcpy(pdu + 4, value, length);
>> +
>> + /*
>> + * Before sending command we need to remember pdu as we need to validate
>> + * it in the response. Store handle, offset and value. Therefore
>> + * increase length by 4 (handle + offset) as we need it in couple places
>> + * below
>> + */
>> + length += 4;
>> +
>> + op->pdu = malloc(length);
>> + if (!op->pdu) {
>> + op->destroy = NULL;
>> + request_unref(req);
>> + return 0;
>> + }
>> +
>> + memcpy(op->pdu, pdu, length);
>> + op->pdu_len = length;
>> +
>> + /* Now we are ready to send command */
>> + req->att_id = bt_att_send(client->att, BT_ATT_OP_PREP_WRITE_REQ, pdu,
>> + sizeof(pdu), prep_write_cb, req,
>> + request_unref);
>> + if (!req->att_id) {
>> + op->destroy = NULL;
>> + request_unref(req);
>> + return 0;
>> + }
>> +
>> + client->in_prep_write = true;
>> +
>> + return req->id;
>> +}
>> +
>> +static void exec_write_cb(uint8_t opcode, const void *pdu, uint16_t length,
>> + void *user_data)
>> +{
>> + struct request *req = user_data;
>> + struct write_op *op = req->data;
>> + bool success;
>> + uint8_t att_ecode;
>> +
>> + if (opcode == BT_ATT_OP_ERROR_RSP) {
>> + success = false;
>> + att_ecode = process_error(pdu, length);
>> + goto done;
>> + }
>> +
>> + if (opcode != BT_ATT_OP_EXEC_WRITE_RSP || pdu || length) {
>> + success = false;
>> + att_ecode = 0;
>> + goto done;
>> + }
>> +
>> + success = true;
>> + att_ecode = 0;
>> +
>> +done:
>> + if (op->callback)
>> + op->callback(success, att_ecode, op->user_data);
>> +
>> + op->client->in_prep_write = false;
>> +
>> + start_next_long_write(op->client);
>> +}
>> +
>> +unsigned int bt_gatt_client_write_execute(struct bt_gatt_client *client,
>> + bool execute,
>> + bt_gatt_client_callback_t callback,
>> + void *user_data,
>> + bt_gatt_client_destroy_func_t destroy)
>> +{
>> + struct request *req;
>> + struct write_op *op;
>> + uint8_t pdu;
>> +
>> + if (!client)
>> + return false;
>> +
>> + if (client->in_long_write || !client->in_prep_write)
>> + return false;
>> +
>> + op = new0(struct write_op, 1);
>> + if (!op)
>> + return false;
>> +
>> + req = request_create(client);
>> + if (!req) {
>> + free(op);
>> + return 0;
>> + }
>> +
>> + op->client = client;
>> + op->callback = callback;
>> + op->user_data = user_data;
>> + op->destroy = destroy;
>> +
>> + pdu = execute ? 0x01 : 0x00;
>> +
>> + req->data = op;
>> + req->destroy = destroy_write_op;
>> +
>> + req->att_id = bt_att_send(client->att, BT_ATT_OP_EXEC_WRITE_REQ, &pdu,
>> + sizeof(pdu), exec_write_cb,
>> + req, request_unref);
>> + if (!req->att_id) {
>> + op->destroy = NULL;
>> + request_unref(req);
>> + return 0;
>> + }
>> +
>> + return req->att_id;
>> +}
>> +
>> static bool match_notify_chrc_value_handle(const void *a, const void *b)
>> {
>> const struct notify_chrc *chrc = a;
>> diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
>> index 9a00feb..45c02c8 100644
>> --- a/src/shared/gatt-client.h
>> +++ b/src/shared/gatt-client.h
>> @@ -109,6 +109,17 @@ unsigned int bt_gatt_client_write_long_value(struct bt_gatt_client *client,
>> bt_gatt_client_write_long_callback_t callback,
>> void *user_data,
>> bt_gatt_client_destroy_func_t destroy);
>> +unsigned int bt_gatt_client_prepare_write(struct bt_gatt_client *client,
>> + uint16_t value_handle, uint16_t offset,
>> + uint8_t *value, uint16_t length,
>> + bt_gatt_client_write_long_callback_t callback,
>> + void *user_data,
>> + bt_gatt_client_destroy_func_t destroy);
>> +unsigned int bt_gatt_client_write_execute(struct bt_gatt_client *client,
>> + bool execute,
>> + bt_gatt_client_callback_t callback,
>> + void *user_data,
>> + bt_gatt_client_destroy_func_t destroy);
>
> I think it is better to leave execute flag to be handled internally by
> bt_gatt_client_cancel (which seems already handle that).
Ah I see that bt_gatt_client_cancel send write execute 0x00 if there
was ongoing long write.
Maybe we could consider do same when does prepare write "manulay"
However I would keep write_execute with execute flag as it is now.
Android has API for that and it just makes things easy. Otherwise,
Android GATT part would have to keep track on the req_id of prepared
writes in order to use it when application does write execute 0x00. Is
that fine?
BTW. What happens in case we have really long write. Then in between
prepare writes user sends regular write request and then user cancel
that regular write before long write is finished?
BR
\Lukasz
>
>
>
> --
> Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] shared/gatt-client: Add write prepare and execute
2015-01-14 13:52 ` Lukasz Rymanowski
@ 2015-01-19 16:09 ` Luiz Augusto von Dentz
2015-01-21 8:28 ` Lukasz Rymanowski
0 siblings, 1 reply; 9+ messages in thread
From: Luiz Augusto von Dentz @ 2015-01-19 16:09 UTC (permalink / raw)
To: Lukasz Rymanowski; +Cc: linux-bluetooth
Hi Lukasz,
On Wed, Jan 14, 2015 at 3:52 PM, Lukasz Rymanowski
<lukasz.rymanowski@tieto.com> wrote:
> Hi Luiz,
>
> On 14 January 2015 at 13:56, Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
>> Hi Lukasz,
>>
>> On Wed, Jan 14, 2015 at 6:17 AM, Lukasz Rymanowski
>> <lukasz.rymanowski@tieto.com> wrote:
>>> For Android we need explicit write prepare and execute commands in GATT
>>> client. This patch adds that.
>>>
>>> Note that till now prepare and execute has been used only in long write.
>>>
>>> This patch makes sure that long write is not proceeded when prepare write
>>> has been called. Instead long write commands will be queued and
>>> once write execute command is done, queued long write requests will be
>>> proceeded.
>>>
>>> It does not work in other way. Meaning, when long write is ongoing,
>>> prepare write will not be proceeded nor queued. This feature can be
>>> added later on if really needed.
>>>
>>> Conflicts:
>>> src/shared/gatt-client.c
>>> ---
>>> src/shared/gatt-client.c | 215 ++++++++++++++++++++++++++++++++++++++++++++++-
>>> src/shared/gatt-client.h | 11 +++
>>> 2 files changed, 225 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
>>> index 1acd34f..c7c29cf 100644
>>> --- a/src/shared/gatt-client.c
>>> +++ b/src/shared/gatt-client.c
>>> @@ -75,6 +75,8 @@ struct bt_gatt_client {
>>> struct queue *long_write_queue;
>>> bool in_long_write;
>>>
>>> + bool in_prep_write;
>>> +
>>> /* List of registered disconnect/notification/indication callbacks */
>>> struct queue *notify_list;
>>> struct queue *notify_chrcs;
>>> @@ -2172,6 +2174,7 @@ unsigned int bt_gatt_client_write_without_response(
>>> }
>>>
>>> struct write_op {
>>> + struct bt_gatt_client *client;
>>> bt_gatt_client_callback_t callback;
>>> void *user_data;
>>> bt_gatt_destroy_func_t destroy;
>>> @@ -2524,7 +2527,7 @@ unsigned int bt_gatt_client_write_long_value(struct bt_gatt_client *client,
>>> req->destroy = long_write_op_free;
>>> req->long_write = true;
>>>
>>> - if (client->in_long_write) {
>>> + if (client->in_long_write || client->in_prep_write) {
>>> queue_push_tail(client->long_write_queue, req);
>>> return req->id;
>>> }
>>> @@ -2557,6 +2560,216 @@ unsigned int bt_gatt_client_write_long_value(struct bt_gatt_client *client,
>>> return req->id;
>>> }
>>>
>>> +struct prep_write_op {
>>> + bt_gatt_client_write_long_callback_t callback;
>>> + void *user_data;
>>> + bt_gatt_destroy_func_t destroy;
>>> + uint8_t *pdu;
>>> + uint16_t pdu_len;
>>> +};
>>> +
>>> +static void destroy_prep_write_op(void *data)
>>> +{
>>> + struct prep_write_op *op = data;
>>> +
>>> + if (op->destroy)
>>> + op->destroy(op->user_data);
>>> +
>>> + free(op->pdu);
>>> + free(op);
>>> +}
>>> +
>>> +static void prep_write_cb(uint8_t opcode, const void *pdu, uint16_t length,
>>> + void *user_data)
>>> +{
>>> + struct request *req = user_data;
>>> + struct prep_write_op *op = req->data;
>>> + bool success;
>>> + uint8_t att_ecode;
>>> + bool reliable_error;
>>> +
>>> + if (opcode == BT_ATT_OP_ERROR_RSP) {
>>> + success = false;
>>> + reliable_error = false;
>>> + att_ecode = process_error(pdu, length);
>>> + goto done;
>>> + }
>>> +
>>> + if (opcode != BT_ATT_OP_PREP_WRITE_RSP) {
>>> + success = false;
>>> + reliable_error = false;
>>> + att_ecode = 0;
>>> + goto done;
>>> + }
>>> +
>>> + if (!pdu || length != op->pdu_len ||
>>> + memcmp(pdu, op->pdu, op->pdu_len)) {
>>> + success = false;
>>> + reliable_error = true;
>>> + att_ecode = 0;
>>> + goto done;
>>> + }
>>> +
>>> + success = true;
>>> + reliable_error = false;
>>> + att_ecode = 0;
>>> +
>>> +done:
>>> + if (op->callback)
>>> + op->callback(success, reliable_error, att_ecode, op->user_data);
>>> +}
>>> +
>>> +unsigned int bt_gatt_client_prepare_write(struct bt_gatt_client *client,
>>> + uint16_t value_handle, uint16_t offset,
>>> + uint8_t *value, uint16_t length,
>>> + bt_gatt_client_write_long_callback_t callback,
>>> + void *user_data,
>>> + bt_gatt_client_destroy_func_t destroy)
>>> +{
>>> + struct request *req;
>>> + struct prep_write_op *op;
>>> + uint8_t pdu[4 + length];
>>> +
>>> + if (!client)
>>> + return 0;
>>> +
>>> + if (client->in_long_write)
>>> + return 0;
>>> +
>>> + op = new0(struct prep_write_op, 1);
>>> + if (!op)
>>> + return 0;
>>> +
>>> + req = request_create(client);
>>> + if (!req) {
>>> + free(op);
>>> + return 0;
>>> + }
>>> +
>>> + op->callback = callback;
>>> + op->user_data = user_data;
>>> + op->destroy = destroy;
>>> +
>>> + req->data = op;
>>> + req->destroy = destroy_prep_write_op;
>>> +
>>> + put_le16(value_handle, pdu);
>>> + put_le16(offset, pdu + 2);
>>> + memcpy(pdu + 4, value, length);
>>> +
>>> + /*
>>> + * Before sending command we need to remember pdu as we need to validate
>>> + * it in the response. Store handle, offset and value. Therefore
>>> + * increase length by 4 (handle + offset) as we need it in couple places
>>> + * below
>>> + */
>>> + length += 4;
>>> +
>>> + op->pdu = malloc(length);
>>> + if (!op->pdu) {
>>> + op->destroy = NULL;
>>> + request_unref(req);
>>> + return 0;
>>> + }
>>> +
>>> + memcpy(op->pdu, pdu, length);
>>> + op->pdu_len = length;
>>> +
>>> + /* Now we are ready to send command */
>>> + req->att_id = bt_att_send(client->att, BT_ATT_OP_PREP_WRITE_REQ, pdu,
>>> + sizeof(pdu), prep_write_cb, req,
>>> + request_unref);
>>> + if (!req->att_id) {
>>> + op->destroy = NULL;
>>> + request_unref(req);
>>> + return 0;
>>> + }
>>> +
>>> + client->in_prep_write = true;
>>> +
>>> + return req->id;
>>> +}
>>> +
>>> +static void exec_write_cb(uint8_t opcode, const void *pdu, uint16_t length,
>>> + void *user_data)
>>> +{
>>> + struct request *req = user_data;
>>> + struct write_op *op = req->data;
>>> + bool success;
>>> + uint8_t att_ecode;
>>> +
>>> + if (opcode == BT_ATT_OP_ERROR_RSP) {
>>> + success = false;
>>> + att_ecode = process_error(pdu, length);
>>> + goto done;
>>> + }
>>> +
>>> + if (opcode != BT_ATT_OP_EXEC_WRITE_RSP || pdu || length) {
>>> + success = false;
>>> + att_ecode = 0;
>>> + goto done;
>>> + }
>>> +
>>> + success = true;
>>> + att_ecode = 0;
>>> +
>>> +done:
>>> + if (op->callback)
>>> + op->callback(success, att_ecode, op->user_data);
>>> +
>>> + op->client->in_prep_write = false;
>>> +
>>> + start_next_long_write(op->client);
>>> +}
>>> +
>>> +unsigned int bt_gatt_client_write_execute(struct bt_gatt_client *client,
>>> + bool execute,
>>> + bt_gatt_client_callback_t callback,
>>> + void *user_data,
>>> + bt_gatt_client_destroy_func_t destroy)
>>> +{
>>> + struct request *req;
>>> + struct write_op *op;
>>> + uint8_t pdu;
>>> +
>>> + if (!client)
>>> + return false;
>>> +
>>> + if (client->in_long_write || !client->in_prep_write)
>>> + return false;
>>> +
>>> + op = new0(struct write_op, 1);
>>> + if (!op)
>>> + return false;
>>> +
>>> + req = request_create(client);
>>> + if (!req) {
>>> + free(op);
>>> + return 0;
>>> + }
>>> +
>>> + op->client = client;
>>> + op->callback = callback;
>>> + op->user_data = user_data;
>>> + op->destroy = destroy;
>>> +
>>> + pdu = execute ? 0x01 : 0x00;
>>> +
>>> + req->data = op;
>>> + req->destroy = destroy_write_op;
>>> +
>>> + req->att_id = bt_att_send(client->att, BT_ATT_OP_EXEC_WRITE_REQ, &pdu,
>>> + sizeof(pdu), exec_write_cb,
>>> + req, request_unref);
>>> + if (!req->att_id) {
>>> + op->destroy = NULL;
>>> + request_unref(req);
>>> + return 0;
>>> + }
>>> +
>>> + return req->att_id;
>>> +}
>>> +
>>> static bool match_notify_chrc_value_handle(const void *a, const void *b)
>>> {
>>> const struct notify_chrc *chrc = a;
>>> diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
>>> index 9a00feb..45c02c8 100644
>>> --- a/src/shared/gatt-client.h
>>> +++ b/src/shared/gatt-client.h
>>> @@ -109,6 +109,17 @@ unsigned int bt_gatt_client_write_long_value(struct bt_gatt_client *client,
>>> bt_gatt_client_write_long_callback_t callback,
>>> void *user_data,
>>> bt_gatt_client_destroy_func_t destroy);
>>> +unsigned int bt_gatt_client_prepare_write(struct bt_gatt_client *client,
>>> + uint16_t value_handle, uint16_t offset,
>>> + uint8_t *value, uint16_t length,
>>> + bt_gatt_client_write_long_callback_t callback,
>>> + void *user_data,
>>> + bt_gatt_client_destroy_func_t destroy);
>>> +unsigned int bt_gatt_client_write_execute(struct bt_gatt_client *client,
>>> + bool execute,
>>> + bt_gatt_client_callback_t callback,
>>> + void *user_data,
>>> + bt_gatt_client_destroy_func_t destroy);
>>
>> I think it is better to leave execute flag to be handled internally by
>> bt_gatt_client_cancel (which seems already handle that).
>
> Ah I see that bt_gatt_client_cancel send write execute 0x00 if there
> was ongoing long write.
> Maybe we could consider do same when does prepare write "manulay"
One thing Im not getting is what bt_gatt_client_prepare_write has to
be so different than bt_gatt_client_write_long_value, IMO it would
make sense to reuse code perhaps by calling
bt_gatt_client_prepare_write in bt_gatt_client_write_long_value as it
seems the only difference is that bt_gatt_client_write_long_value auto
execute while bt_gatt_client_prepare_write requires
bt_gatt_client_write_execute to be called manually.
> However I would keep write_execute with execute flag as it is now.
> Android has API for that and it just makes things easy. Otherwise,
> Android GATT part would have to keep track on the req_id of prepared
> writes in order to use it when application does write execute 0x00. Is
> that fine?
Don't you have to track it anyway, in case it is still outstanding in
the queue you would have to wait the prepare write to complete to send
the execute but if it still in the queue there is no reason even to
proceed with prepare.
> BTW. What happens in case we have really long write. Then in between
> prepare writes user sends regular write request and then user cancel
> that regular write before long write is finished?
Well there is no cancel at ATT level, besides execute 0x00, so we can
only remove from the outgoing queue, but perhaps you had something
else in mind.
>
> BR
> \Lukasz
>>
>>
>>
>> --
>> Luiz Augusto von Dentz
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] shared/gatt-client: Add write prepare and execute
2015-01-19 16:09 ` Luiz Augusto von Dentz
@ 2015-01-21 8:28 ` Lukasz Rymanowski
2015-01-21 9:27 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 9+ messages in thread
From: Lukasz Rymanowski @ 2015-01-21 8:28 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
Hi Luiz,
On 19 January 2015 at 17:09, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Lukasz,
>
> On Wed, Jan 14, 2015 at 3:52 PM, Lukasz Rymanowski
> <lukasz.rymanowski@tieto.com> wrote:
>> Hi Luiz,
>>
>> On 14 January 2015 at 13:56, Luiz Augusto von Dentz
>> <luiz.dentz@gmail.com> wrote:
>>> Hi Lukasz,
>>>
>>> On Wed, Jan 14, 2015 at 6:17 AM, Lukasz Rymanowski
>>> <lukasz.rymanowski@tieto.com> wrote:
>>>> For Android we need explicit write prepare and execute commands in GATT
>>>> client. This patch adds that.
>>>>
>>>> Note that till now prepare and execute has been used only in long write.
>>>>
>>>> This patch makes sure that long write is not proceeded when prepare write
>>>> has been called. Instead long write commands will be queued and
>>>> once write execute command is done, queued long write requests will be
>>>> proceeded.
>>>>
>>>> It does not work in other way. Meaning, when long write is ongoing,
>>>> prepare write will not be proceeded nor queued. This feature can be
>>>> added later on if really needed.
>>>>
>>>> Conflicts:
>>>> src/shared/gatt-client.c
>>>> ---
>>>> src/shared/gatt-client.c | 215 ++++++++++++++++++++++++++++++++++++++++++++++-
>>>> src/shared/gatt-client.h | 11 +++
>>>> 2 files changed, 225 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
>>>> index 1acd34f..c7c29cf 100644
>>>> --- a/src/shared/gatt-client.c
>>>> +++ b/src/shared/gatt-client.c
>>>> @@ -75,6 +75,8 @@ struct bt_gatt_client {
>>>> struct queue *long_write_queue;
>>>> bool in_long_write;
>>>>
>>>> + bool in_prep_write;
>>>> +
>>>> /* List of registered disconnect/notification/indication callbacks */
>>>> struct queue *notify_list;
>>>> struct queue *notify_chrcs;
>>>> @@ -2172,6 +2174,7 @@ unsigned int bt_gatt_client_write_without_response(
>>>> }
>>>>
>>>> struct write_op {
>>>> + struct bt_gatt_client *client;
>>>> bt_gatt_client_callback_t callback;
>>>> void *user_data;
>>>> bt_gatt_destroy_func_t destroy;
>>>> @@ -2524,7 +2527,7 @@ unsigned int bt_gatt_client_write_long_value(struct bt_gatt_client *client,
>>>> req->destroy = long_write_op_free;
>>>> req->long_write = true;
>>>>
>>>> - if (client->in_long_write) {
>>>> + if (client->in_long_write || client->in_prep_write) {
>>>> queue_push_tail(client->long_write_queue, req);
>>>> return req->id;
>>>> }
>>>> @@ -2557,6 +2560,216 @@ unsigned int bt_gatt_client_write_long_value(struct bt_gatt_client *client,
>>>> return req->id;
>>>> }
>>>>
>>>> +struct prep_write_op {
>>>> + bt_gatt_client_write_long_callback_t callback;
>>>> + void *user_data;
>>>> + bt_gatt_destroy_func_t destroy;
>>>> + uint8_t *pdu;
>>>> + uint16_t pdu_len;
>>>> +};
>>>> +
>>>> +static void destroy_prep_write_op(void *data)
>>>> +{
>>>> + struct prep_write_op *op = data;
>>>> +
>>>> + if (op->destroy)
>>>> + op->destroy(op->user_data);
>>>> +
>>>> + free(op->pdu);
>>>> + free(op);
>>>> +}
>>>> +
>>>> +static void prep_write_cb(uint8_t opcode, const void *pdu, uint16_t length,
>>>> + void *user_data)
>>>> +{
>>>> + struct request *req = user_data;
>>>> + struct prep_write_op *op = req->data;
>>>> + bool success;
>>>> + uint8_t att_ecode;
>>>> + bool reliable_error;
>>>> +
>>>> + if (opcode == BT_ATT_OP_ERROR_RSP) {
>>>> + success = false;
>>>> + reliable_error = false;
>>>> + att_ecode = process_error(pdu, length);
>>>> + goto done;
>>>> + }
>>>> +
>>>> + if (opcode != BT_ATT_OP_PREP_WRITE_RSP) {
>>>> + success = false;
>>>> + reliable_error = false;
>>>> + att_ecode = 0;
>>>> + goto done;
>>>> + }
>>>> +
>>>> + if (!pdu || length != op->pdu_len ||
>>>> + memcmp(pdu, op->pdu, op->pdu_len)) {
>>>> + success = false;
>>>> + reliable_error = true;
>>>> + att_ecode = 0;
>>>> + goto done;
>>>> + }
>>>> +
>>>> + success = true;
>>>> + reliable_error = false;
>>>> + att_ecode = 0;
>>>> +
>>>> +done:
>>>> + if (op->callback)
>>>> + op->callback(success, reliable_error, att_ecode, op->user_data);
>>>> +}
>>>> +
>>>> +unsigned int bt_gatt_client_prepare_write(struct bt_gatt_client *client,
>>>> + uint16_t value_handle, uint16_t offset,
>>>> + uint8_t *value, uint16_t length,
>>>> + bt_gatt_client_write_long_callback_t callback,
>>>> + void *user_data,
>>>> + bt_gatt_client_destroy_func_t destroy)
>>>> +{
>>>> + struct request *req;
>>>> + struct prep_write_op *op;
>>>> + uint8_t pdu[4 + length];
>>>> +
>>>> + if (!client)
>>>> + return 0;
>>>> +
>>>> + if (client->in_long_write)
>>>> + return 0;
>>>> +
>>>> + op = new0(struct prep_write_op, 1);
>>>> + if (!op)
>>>> + return 0;
>>>> +
>>>> + req = request_create(client);
>>>> + if (!req) {
>>>> + free(op);
>>>> + return 0;
>>>> + }
>>>> +
>>>> + op->callback = callback;
>>>> + op->user_data = user_data;
>>>> + op->destroy = destroy;
>>>> +
>>>> + req->data = op;
>>>> + req->destroy = destroy_prep_write_op;
>>>> +
>>>> + put_le16(value_handle, pdu);
>>>> + put_le16(offset, pdu + 2);
>>>> + memcpy(pdu + 4, value, length);
>>>> +
>>>> + /*
>>>> + * Before sending command we need to remember pdu as we need to validate
>>>> + * it in the response. Store handle, offset and value. Therefore
>>>> + * increase length by 4 (handle + offset) as we need it in couple places
>>>> + * below
>>>> + */
>>>> + length += 4;
>>>> +
>>>> + op->pdu = malloc(length);
>>>> + if (!op->pdu) {
>>>> + op->destroy = NULL;
>>>> + request_unref(req);
>>>> + return 0;
>>>> + }
>>>> +
>>>> + memcpy(op->pdu, pdu, length);
>>>> + op->pdu_len = length;
>>>> +
>>>> + /* Now we are ready to send command */
>>>> + req->att_id = bt_att_send(client->att, BT_ATT_OP_PREP_WRITE_REQ, pdu,
>>>> + sizeof(pdu), prep_write_cb, req,
>>>> + request_unref);
>>>> + if (!req->att_id) {
>>>> + op->destroy = NULL;
>>>> + request_unref(req);
>>>> + return 0;
>>>> + }
>>>> +
>>>> + client->in_prep_write = true;
>>>> +
>>>> + return req->id;
>>>> +}
>>>> +
>>>> +static void exec_write_cb(uint8_t opcode, const void *pdu, uint16_t length,
>>>> + void *user_data)
>>>> +{
>>>> + struct request *req = user_data;
>>>> + struct write_op *op = req->data;
>>>> + bool success;
>>>> + uint8_t att_ecode;
>>>> +
>>>> + if (opcode == BT_ATT_OP_ERROR_RSP) {
>>>> + success = false;
>>>> + att_ecode = process_error(pdu, length);
>>>> + goto done;
>>>> + }
>>>> +
>>>> + if (opcode != BT_ATT_OP_EXEC_WRITE_RSP || pdu || length) {
>>>> + success = false;
>>>> + att_ecode = 0;
>>>> + goto done;
>>>> + }
>>>> +
>>>> + success = true;
>>>> + att_ecode = 0;
>>>> +
>>>> +done:
>>>> + if (op->callback)
>>>> + op->callback(success, att_ecode, op->user_data);
>>>> +
>>>> + op->client->in_prep_write = false;
>>>> +
>>>> + start_next_long_write(op->client);
>>>> +}
>>>> +
>>>> +unsigned int bt_gatt_client_write_execute(struct bt_gatt_client *client,
>>>> + bool execute,
>>>> + bt_gatt_client_callback_t callback,
>>>> + void *user_data,
>>>> + bt_gatt_client_destroy_func_t destroy)
>>>> +{
>>>> + struct request *req;
>>>> + struct write_op *op;
>>>> + uint8_t pdu;
>>>> +
>>>> + if (!client)
>>>> + return false;
>>>> +
>>>> + if (client->in_long_write || !client->in_prep_write)
>>>> + return false;
>>>> +
>>>> + op = new0(struct write_op, 1);
>>>> + if (!op)
>>>> + return false;
>>>> +
>>>> + req = request_create(client);
>>>> + if (!req) {
>>>> + free(op);
>>>> + return 0;
>>>> + }
>>>> +
>>>> + op->client = client;
>>>> + op->callback = callback;
>>>> + op->user_data = user_data;
>>>> + op->destroy = destroy;
>>>> +
>>>> + pdu = execute ? 0x01 : 0x00;
>>>> +
>>>> + req->data = op;
>>>> + req->destroy = destroy_write_op;
>>>> +
>>>> + req->att_id = bt_att_send(client->att, BT_ATT_OP_EXEC_WRITE_REQ, &pdu,
>>>> + sizeof(pdu), exec_write_cb,
>>>> + req, request_unref);
>>>> + if (!req->att_id) {
>>>> + op->destroy = NULL;
>>>> + request_unref(req);
>>>> + return 0;
>>>> + }
>>>> +
>>>> + return req->att_id;
>>>> +}
>>>> +
>>>> static bool match_notify_chrc_value_handle(const void *a, const void *b)
>>>> {
>>>> const struct notify_chrc *chrc = a;
>>>> diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
>>>> index 9a00feb..45c02c8 100644
>>>> --- a/src/shared/gatt-client.h
>>>> +++ b/src/shared/gatt-client.h
>>>> @@ -109,6 +109,17 @@ unsigned int bt_gatt_client_write_long_value(struct bt_gatt_client *client,
>>>> bt_gatt_client_write_long_callback_t callback,
>>>> void *user_data,
>>>> bt_gatt_client_destroy_func_t destroy);
>>>> +unsigned int bt_gatt_client_prepare_write(struct bt_gatt_client *client,
>>>> + uint16_t value_handle, uint16_t offset,
>>>> + uint8_t *value, uint16_t length,
>>>> + bt_gatt_client_write_long_callback_t callback,
>>>> + void *user_data,
>>>> + bt_gatt_client_destroy_func_t destroy);
>>>> +unsigned int bt_gatt_client_write_execute(struct bt_gatt_client *client,
>>>> + bool execute,
>>>> + bt_gatt_client_callback_t callback,
>>>> + void *user_data,
>>>> + bt_gatt_client_destroy_func_t destroy);
>>>
>>> I think it is better to leave execute flag to be handled internally by
>>> bt_gatt_client_cancel (which seems already handle that).
>>
>> Ah I see that bt_gatt_client_cancel send write execute 0x00 if there
>> was ongoing long write.
>> Maybe we could consider do same when does prepare write "manulay"
>
> One thing Im not getting is what bt_gatt_client_prepare_write has to
> be so different than bt_gatt_client_write_long_value, IMO it would
> make sense to reuse code perhaps by calling
> bt_gatt_client_prepare_write in bt_gatt_client_write_long_value as it
> seems the only difference is that bt_gatt_client_write_long_value auto
> execute while bt_gatt_client_prepare_write requires
> bt_gatt_client_write_execute to be called manually.
>
Yes. Write long make use of write prepare as this is one of the use case.
Other use case for wrtie prepare is e.g. you what write a set of
characterisctics in "atomic" way.
In this use case you would like to have control when to send write execute.
IMHO we should not mix those use cases.
>> However I would keep write_execute with execute flag as it is now.
>> Android has API for that and it just makes things easy. Otherwise,
>> Android GATT part would have to keep track on the req_id of prepared
>> writes in order to use it when application does write execute 0x00. Is
>> that fine?
>
> Don't you have to track it anyway, in case it is still outstanding in
> the queue you would have to wait the prepare write to complete to send
> the execute but if it still in the queue there is no reason even to
> proceed with prepare.
Actually Android waits for the write complete so I would not expect
write execute before
all write prepare are done. I would not be paranoid on that, unless we
found out Android does different.
>
>> BTW. What happens in case we have really long write. Then in between
>> prepare writes user sends regular write request and then user cancel
>> that regular write before long write is finished?
>
> Well there is no cancel at ATT level, besides execute 0x00, so we can
> only remove from the outgoing queue, but perhaps you had something
> else in mind.
>
Nevermind, I double now check and code is fine.
I thought that by canceling some regular write we can incorrectly
cancel ongoing long write.
But it will not happen since long_write flag is in the struct request
which is defined by request_id used by client.
\Lukasz
>>
>> BR
>> \Lukasz
>>>
>>>
>>>
>>> --
>>> Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] shared/gatt-client: Add write prepare and execute
2015-01-21 8:28 ` Lukasz Rymanowski
@ 2015-01-21 9:27 ` Luiz Augusto von Dentz
2015-01-21 10:06 ` Lukasz Rymanowski
0 siblings, 1 reply; 9+ messages in thread
From: Luiz Augusto von Dentz @ 2015-01-21 9:27 UTC (permalink / raw)
To: Lukasz Rymanowski; +Cc: linux-bluetooth
Hi Lukasz,
On Wed, Jan 21, 2015 at 10:28 AM, Lukasz Rymanowski
<lukasz.rymanowski@tieto.com> wrote:
>>>> I think it is better to leave execute flag to be handled internally by
>>>> bt_gatt_client_cancel (which seems already handle that).
>>>
>>> Ah I see that bt_gatt_client_cancel send write execute 0x00 if there
>>> was ongoing long write.
>>> Maybe we could consider do same when does prepare write "manulay"
>>
>> One thing Im not getting is what bt_gatt_client_prepare_write has to
>> be so different than bt_gatt_client_write_long_value, IMO it would
>> make sense to reuse code perhaps by calling
>> bt_gatt_client_prepare_write in bt_gatt_client_write_long_value as it
>> seems the only difference is that bt_gatt_client_write_long_value auto
>> execute while bt_gatt_client_prepare_write requires
>> bt_gatt_client_write_execute to be called manually.
>>
> Yes. Write long make use of write prepare as this is one of the use case.
> Other use case for wrtie prepare is e.g. you what write a set of
> characterisctics in "atomic" way.
> In this use case you would like to have control when to send write execute.
> IMHO we should not mix those use cases.
The problem is that in case of error you cannot assume anything from
execute, so I would not classify it as atomic as you may have to read
all the values to verify what has happened. For Android perhaps this
is not a problem since it doesn't keep everything is sync, it leaves
to the application to do whatever it is pleased, anyway my point is
that internally in gatt-client.c this could be made to reuse code
whether it is for the same handle or different handles it is something
we can handle generically I suppose.
>>> However I would keep write_execute with execute flag as it is now.
>>> Android has API for that and it just makes things easy. Otherwise,
>>> Android GATT part would have to keep track on the req_id of prepared
>>> writes in order to use it when application does write execute 0x00. Is
>>> that fine?
>>
>> Don't you have to track it anyway, in case it is still outstanding in
>> the queue you would have to wait the prepare write to complete to send
>> the execute but if it still in the queue there is no reason even to
>> proceed with prepare.
>
> Actually Android waits for the write complete so I would not expect
> write execute before
> all write prepare are done. I would not be paranoid on that, unless we
> found out Android does different.
>
>>
>>> BTW. What happens in case we have really long write. Then in between
>>> prepare writes user sends regular write request and then user cancel
>>> that regular write before long write is finished?
>>
>> Well there is no cancel at ATT level, besides execute 0x00, so we can
>> only remove from the outgoing queue, but perhaps you had something
>> else in mind.
>>
> Nevermind, I double now check and code is fine.
> I thought that by canceling some regular write we can incorrectly
> cancel ongoing long write.
> But it will not happen since long_write flag is in the struct request
> which is defined by request_id used by client.
>
> \Lukasz
>
>>>
>>> BR
>>> \Lukasz
>>>>
>>>>
>>>>
>>>> --
>>>> Luiz Augusto von Dentz
>>
>>
>>
>> --
>> Luiz Augusto von Dentz
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] shared/gatt-client: Add write prepare and execute
2015-01-21 9:27 ` Luiz Augusto von Dentz
@ 2015-01-21 10:06 ` Lukasz Rymanowski
2015-01-21 11:10 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 9+ messages in thread
From: Lukasz Rymanowski @ 2015-01-21 10:06 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
Hi Luiz,
On 21 January 2015 at 10:27, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Lukasz,
>
> On Wed, Jan 21, 2015 at 10:28 AM, Lukasz Rymanowski
> <lukasz.rymanowski@tieto.com> wrote:
>>>>> I think it is better to leave execute flag to be handled internally by
>>>>> bt_gatt_client_cancel (which seems already handle that).
>>>>
>>>> Ah I see that bt_gatt_client_cancel send write execute 0x00 if there
>>>> was ongoing long write.
>>>> Maybe we could consider do same when does prepare write "manulay"
>>>
>>> One thing Im not getting is what bt_gatt_client_prepare_write has to
>>> be so different than bt_gatt_client_write_long_value, IMO it would
>>> make sense to reuse code perhaps by calling
>>> bt_gatt_client_prepare_write in bt_gatt_client_write_long_value as it
>>> seems the only difference is that bt_gatt_client_write_long_value auto
>>> execute while bt_gatt_client_prepare_write requires
>>> bt_gatt_client_write_execute to be called manually.
>>>
>> Yes. Write long make use of write prepare as this is one of the use case.
>> Other use case for wrtie prepare is e.g. you what write a set of
>> characterisctics in "atomic" way.
>> In this use case you would like to have control when to send write execute.
>> IMHO we should not mix those use cases.
>
> The problem is that in case of error you cannot assume anything from
> execute, so I would not classify it as atomic as you may have to read
> all the values to verify what has happened. For Android perhaps this
> is not a problem since it doesn't keep everything is sync, it leaves
> to the application to do whatever it is pleased, anyway my point is
> that internally in gatt-client.c this could be made to reuse code
> whether it is for the same handle or different handles it is something
> we can handle generically I suppose.
Ok. You mentioned previously that we could remove execute parameter
from write execute and reuse bt_gatt_client_cancel to cover write
execute 0x00
Main problem here is if all prepare writes are already done. At that
time there is actually no outstanding request in the queue to be
cancel. To support write execute 0x00 we would have to add some
parameter to bt_gatt_client_cancel. Is that fine for you? For me
execute parameter in write_execute is much cleaner. I do really
believe that what we do now is OK. We have ATT methods write
prepare/execute and we expose over gatt-client two different scenarios
:)
>
>>>> However I would keep write_execute with execute flag as it is now.
>>>> Android has API for that and it just makes things easy. Otherwise,
>>>> Android GATT part would have to keep track on the req_id of prepared
>>>> writes in order to use it when application does write execute 0x00. Is
>>>> that fine?
>>>
>>> Don't you have to track it anyway, in case it is still outstanding in
>>> the queue you would have to wait the prepare write to complete to send
>>> the execute but if it still in the queue there is no reason even to
>>> proceed with prepare.
>>
>> Actually Android waits for the write complete so I would not expect
>> write execute before
>> all write prepare are done. I would not be paranoid on that, unless we
>> found out Android does different.
>>
>>>
>>>> BTW. What happens in case we have really long write. Then in between
>>>> prepare writes user sends regular write request and then user cancel
>>>> that regular write before long write is finished?
>>>
>>> Well there is no cancel at ATT level, besides execute 0x00, so we can
>>> only remove from the outgoing queue, but perhaps you had something
>>> else in mind.
>>>
>> Nevermind, I double now check and code is fine.
>> I thought that by canceling some regular write we can incorrectly
>> cancel ongoing long write.
>> But it will not happen since long_write flag is in the struct request
>> which is defined by request_id used by client.
>>
>> \Lukasz
>>
>>>>
>>>> BR
>>>> \Lukasz
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Luiz Augusto von Dentz
>>>
>>>
>>>
>>> --
>>> Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] shared/gatt-client: Add write prepare and execute
2015-01-21 10:06 ` Lukasz Rymanowski
@ 2015-01-21 11:10 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 9+ messages in thread
From: Luiz Augusto von Dentz @ 2015-01-21 11:10 UTC (permalink / raw)
To: Lukasz Rymanowski; +Cc: linux-bluetooth
Hi Lukasz,
On Wed, Jan 21, 2015 at 12:06 PM, Lukasz Rymanowski
<lukasz.rymanowski@tieto.com> wrote:
> Hi Luiz,
>
> On 21 January 2015 at 10:27, Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
>> Hi Lukasz,
>>
>> On Wed, Jan 21, 2015 at 10:28 AM, Lukasz Rymanowski
>> <lukasz.rymanowski@tieto.com> wrote:
>>>>>> I think it is better to leave execute flag to be handled internally by
>>>>>> bt_gatt_client_cancel (which seems already handle that).
>>>>>
>>>>> Ah I see that bt_gatt_client_cancel send write execute 0x00 if there
>>>>> was ongoing long write.
>>>>> Maybe we could consider do same when does prepare write "manulay"
>>>>
>>>> One thing Im not getting is what bt_gatt_client_prepare_write has to
>>>> be so different than bt_gatt_client_write_long_value, IMO it would
>>>> make sense to reuse code perhaps by calling
>>>> bt_gatt_client_prepare_write in bt_gatt_client_write_long_value as it
>>>> seems the only difference is that bt_gatt_client_write_long_value auto
>>>> execute while bt_gatt_client_prepare_write requires
>>>> bt_gatt_client_write_execute to be called manually.
>>>>
>>> Yes. Write long make use of write prepare as this is one of the use case.
>>> Other use case for wrtie prepare is e.g. you what write a set of
>>> characterisctics in "atomic" way.
>>> In this use case you would like to have control when to send write execute.
>>> IMHO we should not mix those use cases.
>>
>> The problem is that in case of error you cannot assume anything from
>> execute, so I would not classify it as atomic as you may have to read
>> all the values to verify what has happened. For Android perhaps this
>> is not a problem since it doesn't keep everything is sync, it leaves
>> to the application to do whatever it is pleased, anyway my point is
>> that internally in gatt-client.c this could be made to reuse code
>> whether it is for the same handle or different handles it is something
>> we can handle generically I suppose.
>
> Ok. You mentioned previously that we could remove execute parameter
> from write execute and reuse bt_gatt_client_cancel to cover write
> execute 0x00
>
> Main problem here is if all prepare writes are already done. At that
> time there is actually no outstanding request in the queue to be
> cancel. To support write execute 0x00 we would have to add some
> parameter to bt_gatt_client_cancel. Is that fine for you? For me
> execute parameter in write_execute is much cleaner. I do really
> believe that what we do now is OK. We have ATT methods write
> prepare/execute and we expose over gatt-client two different scenarios
> :)
Well then anyone could cancel or execute write without consent of the
original caller of prepare write so this could cause bugs if any
application misbehave, IMO it would be much better if prepare write id
is also passed to execute write which means we would have to hold a
prepare write id that identify the whole operation, the means the API
would look like this:
unsigned int bt_gatt_client_prepare_write(struct bt_gatt_client *client,
unsigned int id,
uint16_t value_handle, uint16_t offset,
uint8_t *value, uint16_t length,
bt_gatt_client_write_long_callback_t callback,
void *user_data,
bt_gatt_client_destroy_func_t destroy);
unsigned int bt_gatt_client_write_execute(struct bt_gatt_client *client,
unsigned int id,
bt_gatt_client_callback_t callback,
void *user_data,
bt_gatt_client_destroy_func_t destroy);
It entirely up to you whether you will let applications reuse the same
id in Android, but from the API point of view I think this would be
better and bt_gatt_client_write_long_value can then just create it own
queue, obviously this will affect how we handle ids on
bt_gatt_client_cancel because this would be queued at bt_gatt_client
level not in bt_att.
>>
>>>>> However I would keep write_execute with execute flag as it is now.
>>>>> Android has API for that and it just makes things easy. Otherwise,
>>>>> Android GATT part would have to keep track on the req_id of prepared
>>>>> writes in order to use it when application does write execute 0x00. Is
>>>>> that fine?
>>>>
>>>> Don't you have to track it anyway, in case it is still outstanding in
>>>> the queue you would have to wait the prepare write to complete to send
>>>> the execute but if it still in the queue there is no reason even to
>>>> proceed with prepare.
>>>
>>> Actually Android waits for the write complete so I would not expect
>>> write execute before
>>> all write prepare are done. I would not be paranoid on that, unless we
>>> found out Android does different.
>>>
>>>>
>>>>> BTW. What happens in case we have really long write. Then in between
>>>>> prepare writes user sends regular write request and then user cancel
>>>>> that regular write before long write is finished?
>>>>
>>>> Well there is no cancel at ATT level, besides execute 0x00, so we can
>>>> only remove from the outgoing queue, but perhaps you had something
>>>> else in mind.
>>>>
>>> Nevermind, I double now check and code is fine.
>>> I thought that by canceling some regular write we can incorrectly
>>> cancel ongoing long write.
>>> But it will not happen since long_write flag is in the struct request
>>> which is defined by request_id used by client.
>>>
>>> \Lukasz
>>>
>>>>>
>>>>> BR
>>>>> \Lukasz
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Luiz Augusto von Dentz
>>>>
>>>>
>>>>
>>>> --
>>>> Luiz Augusto von Dentz
>>
>>
>>
>> --
>> Luiz Augusto von Dentz-- Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-01-21 11:10 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-14 8:17 [PATCH 1/2] shared/gatt-client: Add write prepare and execute Lukasz Rymanowski
2015-01-14 8:17 ` [PATCH 2/2] tools/btgatt-client: Add prepare and execute write Lukasz Rymanowski
2015-01-14 12:56 ` [PATCH 1/2] shared/gatt-client: Add write prepare and execute Luiz Augusto von Dentz
2015-01-14 13:52 ` Lukasz Rymanowski
2015-01-19 16:09 ` Luiz Augusto von Dentz
2015-01-21 8:28 ` Lukasz Rymanowski
2015-01-21 9:27 ` Luiz Augusto von Dentz
2015-01-21 10:06 ` Lukasz Rymanowski
2015-01-21 11:10 ` Luiz Augusto von Dentz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.