[PATCH v4 0/7] shared/gatt-client: Add support for prepare/execute write

[PATCH v4 0/7] shared/gatt-client: Add support for prepare/execute write

[Lukasz Rymanowski]

This is v4 of previous [PATCH 1/2] shared/gatt-client: Add write prepare and execute

v2 handles comments from Luiz plus some clean ups.

Main change is that gatt client can start reliable session by calling write prepare
with id == 0. In return gatt client will get session id which should be used in
following write prepare and execute write.

This set also makes sure that on bt_gatt_client_cancel, reliable session will be
correctly terminated.

Last patch adds means to test this with btgatt-client

v3:
* Forgoten fixup on last patch now is merged.

v4:
* Fix parameters order pointed by Gowtham

*** BLURB HERE ***

Lukasz Rymanowski (7):
  tools/btgatt-client: Minor fix of menu alignment
  shared/gatt-client: Add helper to cancel long write request
  shared/gatt-client: Make usage of cancel_long_write_cb
  shared/gatt-client: Add write prepare and execute
  shared/gatt-client: Add coexistence of long write and prepare write
  shared/gatt-client: Support cancel of prepare write
  tools/btgatt-client: Add prepare and execute write

 src/shared/gatt-client.c | 350 ++++++++++++++++++++++++++++++++++++++++++-----
 src/shared/gatt-client.h |  13 ++
 tools/btgatt-client.c    | 208 +++++++++++++++++++++++++++-
 3 files changed, 537 insertions(+), 34 deletions(-)

-- 
1.8.4

[PATCH v4 1/7] tools/btgatt-client: Minor fix of menu alignment

[Lukasz Rymanowski]

---
 tools/btgatt-client.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/btgatt-client.c b/tools/btgatt-client.c
index c90f265..e2e0d15 100644
--- a/tools/btgatt-client.c
+++ b/tools/btgatt-client.c
@@ -1135,9 +1135,9 @@ static struct {
 	{ "unregister-notify", cmd_unregister_notify,
 						"Unregister a not/ind session"},
 	{ "set-sec-level", cmd_set_sec_level,
-					"Set security level on le connection"},
+				"\tSet security level on le connection"},
 	{ "get-sec-level", cmd_get_sec_level,
-					"Get security level on le connection"},
+				"\tGet security level on le connection"},
 	{ "set-sign-key", cmd_set_sign_key,
 				"\tSet signing key for signed write command"},
 	{ }
-- 
1.8.4

[PATCH v4 2/7] shared/gatt-client: Add helper to cancel long write request

[Lukasz Rymanowski]

---
 src/shared/gatt-client.c | 61 ++++++++++++++++++++++++------------------------
 1 file changed, 30 insertions(+), 31 deletions(-)

diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index b8c4bd8..af12860 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -1769,10 +1769,34 @@ static void cancel_long_write_cb(uint8_t opcode, const void *pdu, uint16_t len,
 	/* Do nothing */
 }
 
+static bool cancel_long_write_req(struct bt_gatt_client *client,
+							struct request *req)
+{
+	uint8_t pdu = 0x00;
+	bool ret;
+
+	/*
+	 * att_id == 0 means that request has been queued and no prepare write
+	 * has been sent so far.Let's just remove if from the queue.
+	 * Otherwise execute write needs to be send.
+	 */
+	if (!req->att_id)
+		ret = queue_remove(client->long_write_queue, req);
+	else
+		ret = !!bt_att_send(client->att, BT_ATT_OP_EXEC_WRITE_REQ, &pdu,
+							sizeof(pdu),
+							cancel_long_write_cb,
+							NULL, NULL);
+
+	if (queue_isempty(client->long_write_queue))
+		client->in_long_write = false;
+
+	return ret;
+}
+
 bool bt_gatt_client_cancel(struct bt_gatt_client *client, unsigned int id)
 {
 	struct request *req;
-	uint8_t pdu = 0x00;
 
 	if (!client || !id || !client->att)
 		return false;
@@ -1788,19 +1812,8 @@ bool bt_gatt_client_cancel(struct bt_gatt_client *client, unsigned int id)
 		return false;
 
 	/* If this was a long-write, we need to abort all prepared writes */
-	if (!req->long_write)
-		return true;
-
-	if (!req->att_id)
-		queue_remove(client->long_write_queue, req);
-	else
-		bt_att_send(client->att, BT_ATT_OP_EXEC_WRITE_REQ,
-							&pdu, sizeof(pdu),
-							cancel_long_write_cb,
-							NULL, NULL);
-
-	if (queue_isempty(client->long_write_queue))
-		client->in_long_write = false;
+	if (req->long_write)
+		return cancel_long_write_req(client, req);
 
 	return true;
 }
@@ -1808,27 +1821,13 @@ bool bt_gatt_client_cancel(struct bt_gatt_client *client, unsigned int id)
 static void cancel_request(void *data)
 {
 	struct request *req = data;
-	uint8_t pdu = 0x00;
 
 	req->removed = true;
 
-	if (!req->long_write) {
-		bt_att_cancel(req->client->att, req->att_id);
-		return;
-	}
-
-	if (!req->att_id)
-		queue_remove(req->client->long_write_queue, req);
-
-	if (queue_isempty(req->client->long_write_queue))
-		req->client->in_long_write = false;
-
-	bt_att_send(req->client->att, BT_ATT_OP_EXEC_WRITE_REQ,
-							&pdu, sizeof(pdu),
-							cancel_long_write_cb,
-							NULL, NULL);
-
 	bt_att_cancel(req->client->att, req->att_id);
+
+	if (req->long_write)
+		cancel_long_write_req(req->client, req);
 }
 
 bool bt_gatt_client_cancel_all(struct bt_gatt_client *client)
-- 
1.8.4

[PATCH v4 3/7] shared/gatt-client: Make usage of cancel_long_write_cb

[Lukasz Rymanowski]

Clear in_long_write flag when cancel write execute is done
---
 src/shared/gatt-client.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index af12860..acb6200 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -1766,14 +1766,16 @@ static bool match_req_id(const void *a, const void *b)
 static void cancel_long_write_cb(uint8_t opcode, const void *pdu, uint16_t len,
 								void *user_data)
 {
-	/* Do nothing */
+	struct bt_gatt_client *client = user_data;
+
+	if (queue_isempty(client->long_write_queue))
+		client->in_long_write = false;
 }
 
 static bool cancel_long_write_req(struct bt_gatt_client *client,
 							struct request *req)
 {
 	uint8_t pdu = 0x00;
-	bool ret;
 
 	/*
 	 * att_id == 0 means that request has been queued and no prepare write
@@ -1781,17 +1783,13 @@ static bool cancel_long_write_req(struct bt_gatt_client *client,
 	 * Otherwise execute write needs to be send.
 	 */
 	if (!req->att_id)
-		ret = queue_remove(client->long_write_queue, req);
-	else
-		ret = !!bt_att_send(client->att, BT_ATT_OP_EXEC_WRITE_REQ, &pdu,
+		return queue_remove(client->long_write_queue, req);
+
+	return !!bt_att_send(client->att, BT_ATT_OP_EXEC_WRITE_REQ, &pdu,
 							sizeof(pdu),
 							cancel_long_write_cb,
-							NULL, NULL);
-
-	if (queue_isempty(client->long_write_queue))
-		client->in_long_write = false;
+							client, NULL);
 
-	return ret;
 }
 
 bool bt_gatt_client_cancel(struct bt_gatt_client *client, unsigned int id)
-- 
1.8.4

[PATCH v4 4/7] shared/gatt-client: Add write prepare and execute

[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.

With this patch it is possible to start reliable write session by
the gatt-client. First write prepare will return session ID which shall
be used in follow up write prepare request and write execute.
---
 src/shared/gatt-client.c | 261 +++++++++++++++++++++++++++++++++++++++++++++++
 src/shared/gatt-client.h |  13 +++
 2 files changed, 274 insertions(+)

diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index acb6200..30fa883 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -77,6 +77,8 @@ struct bt_gatt_client {
 	struct queue *long_write_queue;
 	bool in_long_write;
 
+	unsigned int reliable_write_session_id;
+
 	/* List of registered disconnect/notification/indication callbacks */
 	struct queue *notify_list;
 	struct queue *notify_chrcs;
@@ -2211,6 +2213,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;
@@ -2596,6 +2599,264 @@ 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);
+}
+
+static struct request *get_reliable_request(struct bt_gatt_client *client,
+							unsigned int id)
+{
+	struct request *req;
+	struct prep_write_op *op;
+
+	op = new0(struct prep_write_op, 1);
+	if (!op)
+		return NULL;
+
+	/* Following prepare writes */
+	if (id != 0)
+		req = queue_find(client->pending_requests, match_req_id,
+							UINT_TO_PTR(id));
+	else
+		req = request_create(client);
+
+	if (!req) {
+		free(op);
+		return NULL;
+	}
+
+	req->data = op;
+
+	return req;
+}
+
+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)
+{
+	struct request *req;
+	struct prep_write_op *op;
+	uint8_t pdu[4 + length];
+
+	if (!client)
+		return 0;
+
+	if (client->in_long_write)
+		return 0;
+
+	/*
+	 * Make sure that client who owns reliable session continues with
+	 * prepare writes or this is brand new reliable session (id == 0)
+	 */
+	if (id != client->reliable_write_session_id) {
+		util_debug(client->debug_callback, client->debug_data,
+			"There is other reliable write session ongoing %u",
+			client->reliable_write_session_id);
+
+		return 0;
+	}
+
+	req = get_reliable_request(client, id);
+	if (!req)
+		return 0;
+
+	op = (struct prep_write_op *)req->data;
+
+	op->callback = callback;
+	op->user_data = user_data;
+	op->destroy = destroy;
+
+	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
+	 * Note that request_unref will be done on write execute
+	 */
+	req->att_id = bt_att_send(client->att, BT_ATT_OP_PREP_WRITE_REQ, pdu,
+					sizeof(pdu), prep_write_cb, req,
+					NULL);
+	if (!req->att_id) {
+		op->destroy = NULL;
+		request_unref(req);
+		return 0;
+	}
+
+	/*
+	 * Store first request id for prepare write and treat it as a session id
+	 * valid until write execute is done
+	 */
+	if (client->reliable_write_session_id == 0)
+		client->reliable_write_session_id = req->id;
+
+	return client->reliable_write_session_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->reliable_write_session_id = 0;
+
+	start_next_long_write(op->client);
+}
+
+unsigned int bt_gatt_client_write_execute(struct bt_gatt_client *client,
+					unsigned int id,
+					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 0;
+
+	if (client->in_long_write)
+		return 0;
+
+	if (client->reliable_write_session_id != id)
+		return 0;
+
+	op = new0(struct write_op, 1);
+	if (!op)
+		return 0;
+
+	req = queue_find(client->pending_requests, match_req_id,
+							UINT_TO_PTR(id));
+	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 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 d4358cf..d7cd2ba 100644
--- a/src/shared/gatt-client.h
+++ b/src/shared/gatt-client.h
@@ -108,6 +108,19 @@ 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,
+				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,
+					bool execute,
+					bt_gatt_client_callback_t callback,
+					void *user_data,
+					bt_gatt_client_destroy_func_t destroy);
 
 unsigned int bt_gatt_client_register_notify(struct bt_gatt_client *client,
 				uint16_t chrc_value_handle,
-- 
1.8.4

[PATCH v4 5/7] shared/gatt-client: Add coexistence of long write and prepare write

[Lukasz Rymanowski]

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.
---
 src/shared/gatt-client.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index 30fa883..7b5c232 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -110,6 +110,7 @@ struct bt_gatt_client {
 struct request {
 	struct bt_gatt_client *client;
 	bool long_write;
+	bool prep_write;
 	bool removed;
 	int ref_count;
 	unsigned int id;
@@ -2566,7 +2567,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->reliable_write_session_id > 0) {
 		queue_push_tail(client->long_write_queue, req);
 		return req->id;
 	}
@@ -2726,6 +2727,7 @@ unsigned int bt_gatt_client_prepare_write(struct bt_gatt_client *client,
 	op->destroy = destroy;
 
 	req->destroy = destroy_prep_write_op;
+	req->prep_write = true;
 
 	put_le16(value_handle, pdu);
 	put_le16(offset, pdu + 2);
-- 
1.8.4

[PATCH v4 6/7] shared/gatt-client: Support cancel of prepare write

[Lukasz Rymanowski]

This patch add support to cancel prepare write done from gatt client
---
 src/shared/gatt-client.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index 7b5c232..4bbf957 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -1795,6 +1795,25 @@ static bool cancel_long_write_req(struct bt_gatt_client *client,
 
 }
 
+static void cancel_prep_write_cb(uint8_t opcode, const void *pdu, uint16_t len,
+								void *user_data)
+{
+	struct bt_gatt_client *client = user_data;
+
+	client->reliable_write_session_id = 0;
+}
+
+static bool cancel_prep_write_session(struct bt_gatt_client *client,
+							struct request *req)
+{
+	uint8_t pdu = 0x00;
+
+	return !!bt_att_send(client->att, BT_ATT_OP_EXEC_WRITE_REQ, &pdu,
+							sizeof(pdu),
+							cancel_prep_write_cb,
+							client, NULL);
+}
+
 bool bt_gatt_client_cancel(struct bt_gatt_client *client, unsigned int id)
 {
 	struct request *req;
@@ -1809,13 +1828,17 @@ bool bt_gatt_client_cancel(struct bt_gatt_client *client, unsigned int id)
 
 	req->removed = true;
 
-	if (!bt_att_cancel(client->att, req->att_id) && !req->long_write)
+	if (!bt_att_cancel(client->att, req->att_id) && !req->long_write &&
+							!req->prep_write)
 		return false;
 
 	/* If this was a long-write, we need to abort all prepared writes */
 	if (req->long_write)
 		return cancel_long_write_req(client, req);
 
+	if (req->prep_write)
+		return cancel_prep_write_session(client, req);
+
 	return true;
 }
 
@@ -1829,6 +1852,9 @@ static void cancel_request(void *data)
 
 	if (req->long_write)
 		cancel_long_write_req(req->client, req);
+
+	if (req->prep_write)
+		cancel_prep_write_session(req->client, req);
 }
 
 bool bt_gatt_client_cancel_all(struct bt_gatt_client *client)
-- 
1.8.4

[PATCH v4 7/7] tools/btgatt-client: Add prepare and execute write

[Lukasz Rymanowski]

---
 tools/btgatt-client.c | 204 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 204 insertions(+)

diff --git a/tools/btgatt-client.c b/tools/btgatt-client.c
index e2e0d15..6a0eae8 100644
--- a/tools/btgatt-client.c
+++ b/tools/btgatt-client.c
@@ -67,6 +67,8 @@ struct client {
 	struct bt_att *att;
 	struct gatt_db *db;
 	struct bt_gatt_client *gatt;
+
+	unsigned int reliable_session_id;
 };
 
 static void print_prompt(void)
@@ -884,6 +886,204 @@ static void cmd_write_long_value(struct client *cli, char *cmd_str)
 	free(value);
 }
 
+static void write_prepare_usage(void)
+{
+	printf("Usage: write-prepare [options] <value_handle> <offset> "
+				"<value>\n"
+				"Options:\n"
+				"\t-s, --session-id\tSession id\n"
+				"e.g.:\n"
+				"\twrite-prepare -s 1 0x0001 00 01 00\n");
+}
+
+static struct option write_prepare_options[] = {
+	{ "session-id",		1, 0, 's' },
+	{ }
+};
+
+static void cmd_write_prepare(struct client *cli, char *cmd_str)
+{
+	int opt, i;
+	char *argvbuf[516];
+	char **argv = argvbuf;
+	int argc = 0;
+	unsigned int id = 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 + 1, &argc)) {
+		printf("Too many arguments\n");
+		write_value_usage();
+		return;
+	}
+
+	/* Add command name for getopt_long */
+	argc++;
+	argv[0] = "write-prepare";
+
+	optind = 0;
+	while ((opt = getopt_long(argc, argv , "s:", write_prepare_options,
+								NULL)) != -1) {
+		switch (opt) {
+		case 's':
+			if (!optarg) {
+				write_prepare_usage();
+				return;
+			}
+
+			id = atoi(optarg);
+
+			break;
+		default:
+			write_prepare_usage();
+			return;
+		}
+	}
+
+	argc -= optind;
+	argv += optind;
+
+	if (argc < 3) {
+		write_prepare_usage();
+		return;
+	}
+
+	if (cli->reliable_session_id != id) {
+		printf("Session id != Ongoing session id (%u!=%u)\n", id,
+						cli->reliable_session_id);
+		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:
+	cli->reliable_session_id =
+			bt_gatt_client_prepare_write(cli->gatt, id,
+							handle, offset,
+							value, length,
+							write_long_cb, NULL,
+							NULL);
+	if (!cli->reliable_session_id)
+		printf("Failed to proceed prepare write\n");
+	else
+		printf("Prepare write success.\n"
+				"Session id: %d to be used on next write\n",
+						cli->reliable_session_id);
+
+	free(value);
+}
+
+static void write_execute_usage(void)
+{
+	printf("Usage: write-execute <session_id> <execute>\n"
+				"e.g.:\n"
+				"\twrite-execute 1 0\n");
+}
+
+static void cmd_write_execute(struct client *cli, char *cmd_str)
+{
+	char *argvbuf[516];
+	char **argv = argvbuf;
+	int argc = 0;
+	char *endptr = NULL;
+	unsigned int session_id;
+	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 < 2) {
+		write_execute_usage();
+		return;
+	}
+
+	session_id = strtol(argv[0], &endptr, 0);
+	if (!endptr || *endptr != '\0') {
+		printf("Invalid session id: %s\n", argv[0]);
+		return;
+	}
+
+	if (session_id != cli->reliable_session_id) {
+		printf("Invalid session id: %u != %u\n", session_id,
+						cli->reliable_session_id);
+		return;
+	}
+
+	execute = !!strtol(argv[1], &endptr, 0);
+	if (!endptr || *endptr != '\0') {
+		printf("Invalid execute: %s\n", argv[1]);
+		return;
+	}
+
+	if (!bt_gatt_client_write_execute(cli->gatt, session_id, execute,
+							write_cb, NULL, NULL))
+		printf("Failed to proceed write execute\n");
+
+	cli->reliable_session_id = 0;
+}
+
 static void register_notify_usage(void)
 {
 	printf("Usage: register-notify <chrc value handle>\n");
@@ -1130,6 +1330,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,
+			"\tWrite prepare characteristic or descriptor value" },
+	{ "write-execute", cmd_write_execute,
+			"\tExecute already prepared write" },
 	{ "register-notify", cmd_register_notify,
 			"\tSubscribe to not/ind from a characteristic" },
 	{ "unregister-notify", cmd_unregister_notify,
-- 
1.8.4

Re: [PATCH v4 4/7] shared/gatt-client: Add write prepare and execute

[Luiz Augusto von Dentz]

Hi Lukasz,

On Mon, Mar 9, 2015 at 10:23 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.
>
> With this patch it is possible to start reliable write session by
> the gatt-client. First write prepare will return session ID which shall
> be used in follow up write prepare request and write execute.
> ---
>  src/shared/gatt-client.c | 261 +++++++++++++++++++++++++++++++++++++++++++++++
>  src/shared/gatt-client.h |  13 +++
>  2 files changed, 274 insertions(+)
>
> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
> index acb6200..30fa883 100644
> --- a/src/shared/gatt-client.c
> +++ b/src/shared/gatt-client.c
> @@ -77,6 +77,8 @@ struct bt_gatt_client {
>         struct queue *long_write_queue;
>         bool in_long_write;
>
> +       unsigned int reliable_write_session_id;
> +
>         /* List of registered disconnect/notification/indication callbacks */
>         struct queue *notify_list;
>         struct queue *notify_chrcs;
> @@ -2211,6 +2213,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;
> @@ -2596,6 +2599,264 @@ 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);
> +}
> +
> +static struct request *get_reliable_request(struct bt_gatt_client *client,
> +                                                       unsigned int id)
> +{
> +       struct request *req;
> +       struct prep_write_op *op;
> +
> +       op = new0(struct prep_write_op, 1);
> +       if (!op)
> +               return NULL;
> +
> +       /* Following prepare writes */
> +       if (id != 0)
> +               req = queue_find(client->pending_requests, match_req_id,
> +                                                       UINT_TO_PTR(id));
> +       else
> +               req = request_create(client);
> +
> +       if (!req) {
> +               free(op);
> +               return NULL;
> +       }
> +
> +       req->data = op;
> +
> +       return req;
> +}
> +
> +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)
> +{
> +       struct request *req;
> +       struct prep_write_op *op;
> +       uint8_t pdu[4 + length];
> +
> +       if (!client)
> +               return 0;
> +
> +       if (client->in_long_write)
> +               return 0;
> +
> +       /*
> +        * Make sure that client who owns reliable session continues with
> +        * prepare writes or this is brand new reliable session (id == 0)
> +        */
> +       if (id != client->reliable_write_session_id) {
> +               util_debug(client->debug_callback, client->debug_data,
> +                       "There is other reliable write session ongoing %u",
> +                       client->reliable_write_session_id);
> +
> +               return 0;
> +       }
> +
> +       req = get_reliable_request(client, id);
> +       if (!req)
> +               return 0;
> +
> +       op = (struct prep_write_op *)req->data;
> +
> +       op->callback = callback;
> +       op->user_data = user_data;
> +       op->destroy = destroy;
> +
> +       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
> +        * Note that request_unref will be done on write execute
> +        */
> +       req->att_id = bt_att_send(client->att, BT_ATT_OP_PREP_WRITE_REQ, pdu,
> +                                       sizeof(pdu), prep_write_cb, req,
> +                                       NULL);
> +       if (!req->att_id) {
> +               op->destroy = NULL;
> +               request_unref(req);
> +               return 0;
> +       }
> +
> +       /*
> +        * Store first request id for prepare write and treat it as a session id
> +        * valid until write execute is done
> +        */
> +       if (client->reliable_write_session_id == 0)
> +               client->reliable_write_session_id = req->id;
> +
> +       return client->reliable_write_session_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->reliable_write_session_id = 0;
> +
> +       start_next_long_write(op->client);
> +}
> +
> +unsigned int bt_gatt_client_write_execute(struct bt_gatt_client *client,
> +                                       unsigned int id,
> +                                       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 0;
> +
> +       if (client->in_long_write)
> +               return 0;
> +
> +       if (client->reliable_write_session_id != id)
> +               return 0;
> +
> +       op = new0(struct write_op, 1);
> +       if (!op)
> +               return 0;
> +
> +       req = queue_find(client->pending_requests, match_req_id,
> +                                                       UINT_TO_PTR(id));
> +       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 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 d4358cf..d7cd2ba 100644
> --- a/src/shared/gatt-client.h
> +++ b/src/shared/gatt-client.h
> @@ -108,6 +108,19 @@ 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,
> +                               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,
> +                                       bool execute,
> +                                       bt_gatt_client_callback_t callback,
> +                                       void *user_data,
> +                                       bt_gatt_client_destroy_func_t destroy);

I recall suggesting to remove execute parameter here and use
bt_gatt_client_cancel, that anyway should support cancelling it, note
that if execute 0x00 fails it would probably block any further
operation to use prepare so I wonder if there is actually a case where
cancel can fail, except perhaps if the queue is empty but then we
would need a special error code to identify that and discard the
prepare queue anyway since in that case it would not block further
prepares.

>  unsigned int bt_gatt_client_register_notify(struct bt_gatt_client *client,
>                                 uint16_t chrc_value_handle,
> --
> 1.8.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Luiz Augusto von Dentz

Re: [PATCH v4 4/7] shared/gatt-client: Add write prepare and execute

[Lukasz Rymanowski]

Hi Luiz,

On Mon, Mar 9, 2015 at 12:06 PM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Lukasz,
>
> On Mon, Mar 9, 2015 at 10:23 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.
>>
>> With this patch it is possible to start reliable write session by
>> the gatt-client. First write prepare will return session ID which shall
>> be used in follow up write prepare request and write execute.
>> ---
>>  src/shared/gatt-client.c | 261 +++++++++++++++++++++++++++++++++++++++++++++++
>>  src/shared/gatt-client.h |  13 +++
>>  2 files changed, 274 insertions(+)
>>
>> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
>> index acb6200..30fa883 100644
>> --- a/src/shared/gatt-client.c
>> +++ b/src/shared/gatt-client.c
>> @@ -77,6 +77,8 @@ struct bt_gatt_client {
>>         struct queue *long_write_queue;
>>         bool in_long_write;
>>
>> +       unsigned int reliable_write_session_id;
>> +
>>         /* List of registered disconnect/notification/indication callbacks */
>>         struct queue *notify_list;
>>         struct queue *notify_chrcs;
>> @@ -2211,6 +2213,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;
>> @@ -2596,6 +2599,264 @@ 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);
>> +}
>> +
>> +static struct request *get_reliable_request(struct bt_gatt_client *client,
>> +                                                       unsigned int id)
>> +{
>> +       struct request *req;
>> +       struct prep_write_op *op;
>> +
>> +       op = new0(struct prep_write_op, 1);
>> +       if (!op)
>> +               return NULL;
>> +
>> +       /* Following prepare writes */
>> +       if (id != 0)
>> +               req = queue_find(client->pending_requests, match_req_id,
>> +                                                       UINT_TO_PTR(id));
>> +       else
>> +               req = request_create(client);
>> +
>> +       if (!req) {
>> +               free(op);
>> +               return NULL;
>> +       }
>> +
>> +       req->data = op;
>> +
>> +       return req;
>> +}
>> +
>> +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)
>> +{
>> +       struct request *req;
>> +       struct prep_write_op *op;
>> +       uint8_t pdu[4 + length];
>> +
>> +       if (!client)
>> +               return 0;
>> +
>> +       if (client->in_long_write)
>> +               return 0;
>> +
>> +       /*
>> +        * Make sure that client who owns reliable session continues with
>> +        * prepare writes or this is brand new reliable session (id == 0)
>> +        */
>> +       if (id != client->reliable_write_session_id) {
>> +               util_debug(client->debug_callback, client->debug_data,
>> +                       "There is other reliable write session ongoing %u",
>> +                       client->reliable_write_session_id);
>> +
>> +               return 0;
>> +       }
>> +
>> +       req = get_reliable_request(client, id);
>> +       if (!req)
>> +               return 0;
>> +
>> +       op = (struct prep_write_op *)req->data;
>> +
>> +       op->callback = callback;
>> +       op->user_data = user_data;
>> +       op->destroy = destroy;
>> +
>> +       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
>> +        * Note that request_unref will be done on write execute
>> +        */
>> +       req->att_id = bt_att_send(client->att, BT_ATT_OP_PREP_WRITE_REQ, pdu,
>> +                                       sizeof(pdu), prep_write_cb, req,
>> +                                       NULL);
>> +       if (!req->att_id) {
>> +               op->destroy = NULL;
>> +               request_unref(req);
>> +               return 0;
>> +       }
>> +
>> +       /*
>> +        * Store first request id for prepare write and treat it as a session id
>> +        * valid until write execute is done
>> +        */
>> +       if (client->reliable_write_session_id == 0)
>> +               client->reliable_write_session_id = req->id;
>> +
>> +       return client->reliable_write_session_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->reliable_write_session_id = 0;
>> +
>> +       start_next_long_write(op->client);
>> +}
>> +
>> +unsigned int bt_gatt_client_write_execute(struct bt_gatt_client *client,
>> +                                       unsigned int id,
>> +                                       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 0;
>> +
>> +       if (client->in_long_write)
>> +               return 0;
>> +
>> +       if (client->reliable_write_session_id != id)
>> +               return 0;
>> +
>> +       op = new0(struct write_op, 1);
>> +       if (!op)
>> +               return 0;
>> +
>> +       req = queue_find(client->pending_requests, match_req_id,
>> +                                                       UINT_TO_PTR(id));
>> +       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 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 d4358cf..d7cd2ba 100644
>> --- a/src/shared/gatt-client.h
>> +++ b/src/shared/gatt-client.h
>> @@ -108,6 +108,19 @@ 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,
>> +                               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,
>> +                                       bool execute,
>> +                                       bt_gatt_client_callback_t callback,
>> +                                       void *user_data,
>> +                                       bt_gatt_client_destroy_func_t destroy);
>
> I recall suggesting to remove execute parameter here and use
> bt_gatt_client_cancel, that anyway should support cancelling it, note
> that if execute 0x00 fails it would probably block any further
> operation to use prepare so I wonder if there is actually a case where
> cancel can fail, except perhaps if the queue is empty but then we
> would need a special error code to identify that and discard the
> prepare queue anyway since in that case it would not block further
> prepares.
>

Yes, you did suggest that, and I missed to comment on that in my cover letter.
bt_gatt_client_cancel does support canceling of prepare write. I guess
it  will  be used  mostly when cancel_all is used.

In case of, let say, controlled prepare write session, having callback
from execute gives you and idea
when gatt client is ready to send another request. It also fits
android design as we need to send notification to framework once
write is done. We do send this notification in the write_cb.

Also, having two ways of  doing write execute 0x00 ( with
bt_gatt_client_cancel and (imho) more intuitive way
bt_gatt_client_execute_write(0x00) )
should not be a big problem, especially that this is our internal API.

BR
Lukasz


>>  unsigned int bt_gatt_client_register_notify(struct bt_gatt_client *client,
>>                                 uint16_t chrc_value_handle,
>> --
>> 1.8.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Luiz Augusto von Dentz
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 4/7] shared/gatt-client: Add write prepare and execute

[Luiz Augusto von Dentz]

Hi Lukasz,

On Mon, Mar 9, 2015 at 10:44 PM, Lukasz Rymanowski
<lukasz.rymanowski@gmail.com> wrote:
> Hi Luiz,
>
> On Mon, Mar 9, 2015 at 12:06 PM, Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
>> Hi Lukasz,
>>
>> On Mon, Mar 9, 2015 at 10:23 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.
>>>
>>> With this patch it is possible to start reliable write session by
>>> the gatt-client. First write prepare will return session ID which shall
>>> be used in follow up write prepare request and write execute.
>>> ---
>>>  src/shared/gatt-client.c | 261 +++++++++++++++++++++++++++++++++++++++++++++++
>>>  src/shared/gatt-client.h |  13 +++
>>>  2 files changed, 274 insertions(+)
>>>
>>> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
>>> index acb6200..30fa883 100644
>>> --- a/src/shared/gatt-client.c
>>> +++ b/src/shared/gatt-client.c
>>> @@ -77,6 +77,8 @@ struct bt_gatt_client {
>>>         struct queue *long_write_queue;
>>>         bool in_long_write;
>>>
>>> +       unsigned int reliable_write_session_id;
>>> +
>>>         /* List of registered disconnect/notification/indication callbacks */
>>>         struct queue *notify_list;
>>>         struct queue *notify_chrcs;
>>> @@ -2211,6 +2213,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;
>>> @@ -2596,6 +2599,264 @@ 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);
>>> +}
>>> +
>>> +static struct request *get_reliable_request(struct bt_gatt_client *client,
>>> +                                                       unsigned int id)
>>> +{
>>> +       struct request *req;
>>> +       struct prep_write_op *op;
>>> +
>>> +       op = new0(struct prep_write_op, 1);
>>> +       if (!op)
>>> +               return NULL;
>>> +
>>> +       /* Following prepare writes */
>>> +       if (id != 0)
>>> +               req = queue_find(client->pending_requests, match_req_id,
>>> +                                                       UINT_TO_PTR(id));
>>> +       else
>>> +               req = request_create(client);
>>> +
>>> +       if (!req) {
>>> +               free(op);
>>> +               return NULL;
>>> +       }
>>> +
>>> +       req->data = op;
>>> +
>>> +       return req;
>>> +}
>>> +
>>> +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)
>>> +{
>>> +       struct request *req;
>>> +       struct prep_write_op *op;
>>> +       uint8_t pdu[4 + length];
>>> +
>>> +       if (!client)
>>> +               return 0;
>>> +
>>> +       if (client->in_long_write)
>>> +               return 0;
>>> +
>>> +       /*
>>> +        * Make sure that client who owns reliable session continues with
>>> +        * prepare writes or this is brand new reliable session (id == 0)
>>> +        */
>>> +       if (id != client->reliable_write_session_id) {
>>> +               util_debug(client->debug_callback, client->debug_data,
>>> +                       "There is other reliable write session ongoing %u",
>>> +                       client->reliable_write_session_id);
>>> +
>>> +               return 0;
>>> +       }
>>> +
>>> +       req = get_reliable_request(client, id);
>>> +       if (!req)
>>> +               return 0;
>>> +
>>> +       op = (struct prep_write_op *)req->data;
>>> +
>>> +       op->callback = callback;
>>> +       op->user_data = user_data;
>>> +       op->destroy = destroy;
>>> +
>>> +       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
>>> +        * Note that request_unref will be done on write execute
>>> +        */
>>> +       req->att_id = bt_att_send(client->att, BT_ATT_OP_PREP_WRITE_REQ, pdu,
>>> +                                       sizeof(pdu), prep_write_cb, req,
>>> +                                       NULL);
>>> +       if (!req->att_id) {
>>> +               op->destroy = NULL;
>>> +               request_unref(req);
>>> +               return 0;
>>> +       }
>>> +
>>> +       /*
>>> +        * Store first request id for prepare write and treat it as a session id
>>> +        * valid until write execute is done
>>> +        */
>>> +       if (client->reliable_write_session_id == 0)
>>> +               client->reliable_write_session_id = req->id;
>>> +
>>> +       return client->reliable_write_session_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->reliable_write_session_id = 0;
>>> +
>>> +       start_next_long_write(op->client);
>>> +}
>>> +
>>> +unsigned int bt_gatt_client_write_execute(struct bt_gatt_client *client,
>>> +                                       unsigned int id,
>>> +                                       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 0;
>>> +
>>> +       if (client->in_long_write)
>>> +               return 0;
>>> +
>>> +       if (client->reliable_write_session_id != id)
>>> +               return 0;
>>> +
>>> +       op = new0(struct write_op, 1);
>>> +       if (!op)
>>> +               return 0;
>>> +
>>> +       req = queue_find(client->pending_requests, match_req_id,
>>> +                                                       UINT_TO_PTR(id));
>>> +       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 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 d4358cf..d7cd2ba 100644
>>> --- a/src/shared/gatt-client.h
>>> +++ b/src/shared/gatt-client.h
>>> @@ -108,6 +108,19 @@ 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,
>>> +                               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,
>>> +                                       bool execute,
>>> +                                       bt_gatt_client_callback_t callback,
>>> +                                       void *user_data,
>>> +                                       bt_gatt_client_destroy_func_t destroy);
>>
>> I recall suggesting to remove execute parameter here and use
>> bt_gatt_client_cancel, that anyway should support cancelling it, note
>> that if execute 0x00 fails it would probably block any further
>> operation to use prepare so I wonder if there is actually a case where
>> cancel can fail, except perhaps if the queue is empty but then we
>> would need a special error code to identify that and discard the
>> prepare queue anyway since in that case it would not block further
>> prepares.
>>
>
> Yes, you did suggest that, and I missed to comment on that in my cover letter.
> bt_gatt_client_cancel does support canceling of prepare write. I guess
> it  will  be used  mostly when cancel_all is used.
>
> In case of, let say, controlled prepare write session, having callback
> from execute gives you and idea
> when gatt client is ready to send another request. It also fits
> android design as we need to send notification to framework once
> write is done. We do send this notification in the write_cb.

You can send the notification right away can't you? att will queue the
requests anyway, so it is not that this will broken anything actually.

> Also, having two ways of  doing write execute 0x00 ( with
> bt_gatt_client_cancel and (imho) more intuitive way
> bt_gatt_client_execute_write(0x00) )
> should not be a big problem, especially that this is our internal API.


Well I suspect a newbie could do things like execute 0x00 and then
cancel which would probably crash given that the request would be
freed before the response.

> BR
> Lukasz
>
>
>>>  unsigned int bt_gatt_client_register_notify(struct bt_gatt_client *client,
>>>                                 uint16_t chrc_value_handle,
>>> --
>>> 1.8.4
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
>>
>> --
>> Luiz Augusto von Dentz
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Luiz Augusto von Dentz