Return-Path: MIME-Version: 1.0 In-Reply-To: <1458306498-28151-7-git-send-email-lukasz.rymanowski@codecoup.pl> References: <1458306498-28151-1-git-send-email-lukasz.rymanowski@codecoup.pl> <1458306498-28151-7-git-send-email-lukasz.rymanowski@codecoup.pl> Date: Tue, 22 Mar 2016 17:03:47 +0200 Message-ID: Subject: Re: [PATCH v3 6/9] shared/gatt-server: Add support for long write From: Luiz Augusto von Dentz To: =?UTF-8?Q?=C5=81ukasz_Rymanowski?= Cc: "linux-bluetooth@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-bluetooth-owner@vger.kernel.org List-ID: Hi Lukasz, On Fri, Mar 18, 2016 at 3:08 PM, Łukasz Rymanowski wrote: > With this patch long write and nested long write reliable is supported. > GATT server is responsible now to do aggregation of prep write data > for long write session. > Note: We consider long write as the consequtive prepare writes with > continues offsets. > > E.g. 1 > > prep_write: handle 1, offset 0, value_len 10 > prep_write: handle 2, offset 0, value_len 10 > prep_write: handle 1, offset 10, value_len 10 > prep_write: handle 2, offset 10, value_len 10 > > Will result with following calles to app: > > exec_write: handle 1: offset 0, value_len 20 > exec_write: handle 2: offset 0, value_len 20 We should not alter the sequence if the remote is writing in that way we should honor it, so the aggregation shall only be done as long as the remote don't nest anything in between at that point you should stop aggregating and add a new item to the queue. Perhaps you just detect if the prepare was for for the last handle and the offset start from the last point then just append otherwise create a new item. > > E.g. 2 > > prep_write: handle 1, offset 0, value_len 10 > prep_write: handle 1, offset 2, value_len 5 > prep_write: handle 2, offset 0, value_len 10 > prep_write: handle 2, offset 4, value_len 5 > > Will result with following calles to app: > > exec_write: handle 1: offset 0, value_len 10 > exec_write: handle 1: offset 2, value_len 5 > exec_write: handle 2: offset 0, value_len 10 > exec_write: handle 2: offset 4, value_len 5 > --- > src/shared/gatt-server.c | 85 ++++++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 71 insertions(+), 14 deletions(-) > > diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c > index c41273a..0904336 100644 > --- a/src/shared/gatt-server.c > +++ b/src/shared/gatt-server.c > @@ -1088,6 +1088,63 @@ error: > bt_att_send_error_rsp(server->att, opcode, 0, ecode); > } > > +static bool match_attribute_handle(const void *data, const void *match_data) > +{ > + const struct prep_write_data *prep_data = data; > + > + return prep_data->handle == PTR_TO_UINT(match_data); > +} > + > +static bool create_and_store_prep_data(struct bt_gatt_server *server, > + uint16_t handle, uint16_t offset, > + uint16_t length, uint8_t *value) > +{ > + struct prep_write_data *prep_data; > + > + prep_data = new0(struct prep_write_data, 1); > + prep_data->length = length; > + if (prep_data->length) { > + prep_data->value = malloc(prep_data->length); > + if (!prep_data->value) { > + return false; > + } > + > + memcpy(prep_data->value, value, prep_data->length); > + } > + > + prep_data->server = server; > + prep_data->handle = handle; > + prep_data->offset = offset; > + > + queue_push_tail(server->prep_queue, prep_data); > + > + return true; > +} > + > +static bool make_aggregation_of_long_write_data(struct bt_gatt_server *server, > + struct prep_write_data *prep_data, > + uint16_t handle, uint16_t length, > + uint8_t *value) > +{ > + uint8_t *buf; > + uint16_t new_len; > + > + new_len = prep_data->length + length; > + > + buf = malloc(new_len); > + if (!buf) > + return false; > + > + memcpy(buf, prep_data->value, prep_data->length); > + memcpy(buf + prep_data->length, value, length); > + > + free(prep_data->value); > + prep_data->value = buf; > + prep_data->length = new_len; > + > + return true; > +} > + > static void prep_write_cb(uint8_t opcode, const void *pdu, > uint16_t length, void *user_data) > { > @@ -1097,6 +1154,7 @@ static void prep_write_cb(uint8_t opcode, const void *pdu, > uint16_t offset; > struct gatt_db_attribute *attr; > uint8_t ecode; > + bool success; > > if (length < 4) { > ecode = BT_ATT_ERROR_INVALID_PDU; > @@ -1126,22 +1184,21 @@ static void prep_write_cb(uint8_t opcode, const void *pdu, > if (ecode) > goto error; > > - prep_data = new0(struct prep_write_data, 1); > - prep_data->length = length - 4; > - if (prep_data->length) { > - prep_data->value = malloc(prep_data->length); > - if (!prep_data->value) { > - ecode = BT_ATT_ERROR_INSUFFICIENT_RESOURCES; > - goto error; > - } > - } > + prep_data = queue_find(server->prep_queue, match_attribute_handle, > + UINT_TO_PTR(handle)); > > - prep_data->server = server; > - prep_data->handle = handle; > - prep_data->offset = offset; > - memcpy(prep_data->value, pdu + 4, prep_data->length); > + if (prep_data && offset == prep_data->length + prep_data->offset) > + success = make_aggregation_of_long_write_data(server, prep_data, > + handle, length - 4, > + &((uint8_t *) pdu)[4]); > + else > + success = create_and_store_prep_data(server, handle, offset, > + length - 4, &((uint8_t *) pdu)[4]); > > - queue_push_tail(server->prep_queue, prep_data); > + if (!success) { > + ecode = BT_ATT_ERROR_INSUFFICIENT_RESOURCES; > + goto error; > + } > > bt_att_send(server->att, BT_ATT_OP_PREP_WRITE_RSP, pdu, length, NULL, > NULL, NULL); > -- > 2.5.0 > > -- > 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