Hi Lukasz,
On 16 November 2015 at 22:08, Łukasz Rymanowski
<lukasz.rymanowski@xxxxxxxxxxx> wrote:
>
> With current implementation application layer has no idea when
> long write session ends as it gets write callback with execute
> write for each prepare write data.
> With this patch, after data are completly received by gatt-server it
> starts to call write callback in proper order with opcode
> BT_ATT_OP_PREP_WRITE_REQ. After that, each characteristic gets
> BT_ATT_OP_EXEC_WRITE_REQ so it knows that complete characteristic value
> has been received and data can be marked as valid.
> ---
> src/shared/gatt-server.c | 78 ++++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 73 insertions(+), 5 deletions(-)
>
> diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
> index 329f53f..931d619 100644
> --- a/src/shared/gatt-server.c
> +++ b/src/shared/gatt-server.c
> @@ -104,6 +104,8 @@ struct bt_gatt_server {
> struct queue *prep_queue;
> unsigned int max_prep_queue_len;
>
> + struct queue *exec_queue;
> +
> struct async_read_op *pending_read_op;
> struct async_write_op *pending_write_op;
>
> @@ -137,6 +139,7 @@ static void bt_gatt_server_free(struct bt_gatt_server *server)
> server->pending_write_op->server = NULL;
>
> queue_destroy(server->prep_queue, prep_write_data_destroy);
> + queue_destroy(server->exec_queue, NULL);
>
> gatt_db_unref(server->db);
> bt_att_unref(server->att);
> @@ -1155,10 +1158,61 @@ error:
>
> }
>
> +static void exec_next_write(struct bt_gatt_server *server);
> +
> +static void exec_write_complete_cb(struct gatt_db_attribute *attr, int err,
> + void *user_data)
> +{
> + struct bt_gatt_server *server = user_data;
> + uint16_t handle = gatt_db_attribute_get_handle(attr);
> +
> + if (err) {
> + queue_remove_all(server->exec_queue, NULL, NULL, NULL);
> + bt_att_send_error_rsp(server->att, BT_ATT_OP_EXEC_WRITE_REQ,
> + handle, err);
> + }
> +
> + exec_next_write(server);
> +}
> +
> +static void exec_next_write(struct bt_gatt_server *server)
> +{
> + uint16_t *handle;
> + struct gatt_db_attribute *attr;
> + bool status;
> + int err;
> +
> + handle = queue_pop_head(server->exec_queue);
> + if (!handle) {
> + bt_att_send(server->att, BT_ATT_OP_EXEC_WRITE_RSP, NULL, 0,
> + NULL, NULL, NULL);
> + }
> +
> + attr = gatt_db_get_attribute(server->db, PTR_TO_UINT(handle));
> + if (!attr) {
> + err = BT_ATT_ERROR_UNLIKELY;
> + goto error;
> + }
> +
> + status = gatt_db_attribute_write(attr, 0, NULL , 0,
> + BT_ATT_OP_EXEC_WRITE_REQ,
> + server->att,
> + exec_write_complete_cb, server);
> + if (status)
> + return;
> +
> + err = BT_ATT_ERROR_UNLIKELY;
> +
> +error:
> + queue_remove_all(server->exec_queue, NULL, NULL, NULL);
> + bt_att_send_error_rsp(server->att, BT_ATT_OP_EXEC_WRITE_REQ,
> + PTR_TO_UINT(handle), err);
> +}
> +
> static void exec_next_prep_write(struct bt_gatt_server *server,
> uint16_t ehandle, int err);
>
> -static void exec_write_complete_cb(struct gatt_db_attribute *attr, int err,
> +static void prep_write_complete_cb(struct gatt_db_attribute *attr, int err,
> void *user_data)
> {
> struct bt_gatt_server *server = user_data;
> @@ -1167,6 +1221,11 @@ static void exec_write_complete_cb(struct gatt_db_attribute *attr, int err,
> exec_next_prep_write(server, handle, err);
> }
>
> +static bool match_attribute_handle(const void *data, const void *match_data)
> +{
> + return PTR_TO_UINT(data) == PTR_TO_UINT(match_data);
> +}
> +
> static void exec_next_prep_write(struct bt_gatt_server *server,
> uint16_t ehandle, int err)
> {
> @@ -1177,10 +1236,17 @@ static void exec_next_prep_write(struct bt_gatt_server *server,
> if (err)
Shouldn't you remove all elements of server->exec_queue here? Looking
at the next patch, all attributes in exec_queue shall be notified that
exec write is canceled too.
And just open question: with introducing two types of write requests
in reliable write procedure: BT_ATT_OP_EXEC_WRITE_REQ and
BT_ATT_OP_PREP_WRITE_REQ, we could avoid using two queues and pass
prepare writes immediatelly to specific service implementation instead
of storing all values in prepare writes in server.
> goto error;
>
>
> + /*
> + * Remember to which handles we need to send execute after reliable
> + * or long write session is done
> + */
> + if (ehandle && !queue_find(server->exec_queue, match_attribute_handle,
> + UINT_TO_PTR(ehandle)))
> + queue_push_tail(server->exec_queue, UINT_TO_PTR(ehandle));
> +
> next = queue_pop_head(server->prep_queue);
> if (!next) {
> - bt_att_send(server->att, BT_ATT_OP_EXEC_WRITE_RSP, NULL, 0,
> - NULL, NULL, NULL);
> + exec_next_write(server);
> return;
> }
>
> @@ -1192,9 +1258,9 @@ static void exec_next_prep_write(struct bt_gatt_server *server,
>
> status = gatt_db_attribute_write(attr, next->offset,
> next->value, next->length,
> - BT_ATT_OP_EXEC_WRITE_REQ,
> + BT_ATT_OP_PREP_WRITE_REQ,
> server->att,
> - exec_write_complete_cb, server);
> + prep_write_complete_cb, server);
>
> prep_write_data_destroy(next);
>
> @@ -1396,6 +1462,8 @@ struct bt_gatt_server *bt_gatt_server_new(struct gatt_db *db,
> server->max_prep_queue_len = DEFAULT_MAX_PREP_QUEUE_LEN;
> server->prep_queue = queue_new();
>
> + server->exec_queue = queue_new();
> +
> if (!gatt_server_register_att_handlers(server)) {
> bt_gatt_server_free(server);
> return NULL;
> --
> 2.5.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
BR
Marcin Kraglak
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
