Hi Marcin,
On Tue, Nov 17, 2015 at 1:12 PM, Marcin Kraglak
<marcin.kraglak@xxxxxxxxx> wrote:
> 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.
You are right, will fix, thanks
> 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
--
BR / Pozdrawiam
Łukasz
--
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
