Hi Luiz,
I did this fix while testing it agains PTS test case "4.6.39
GATT/SR/GAW/BV-10-C [Nested Long Characteristic Value Reliable Writes
- to Server]"
Without this fix bluez behaves as described:
1) PTS do prepare write to CHARACTERISTIC1 with offset = 0 and value
with size = ATT_MTU-5.
2) Bluez do new prepare write for CHARACTERISTIC1
3) PTS do prepare write to CHARACTERISTIC2 with offset = 0 and value
with size = ATT_MTU-5.
4) Bluez do new prepare write for CHARACTERISTIC2
5) PTS do prepare write to CHARACTERISTIC1 with offset = ATT_MTU-5 and
value with size < ATT_MTU-5.
6) Bluez do new prepare write for CHARACTERISTIC1
7) PTS do prepare write to CHARACTERISTIC2 with offset = ATT_MTU-5 and
value with size < ATT_MTU-5.
8) Bluez do new prepare write for CHARACTERISTIC2
9) Execut write is issued with flag "Immediately write all pend-ing
prepared values".
10) PTS do read of those two CHARACTERISTICS and got response with
value from last prepare write to CHARACTERISTIC (this from second
prepare write form offset ATT_MTU-5) cause thos are 4 seperate prepare
writes queues (bluez appends only if last prepare write was to the
same handle).
11) Test result fails cause expected value isn't the same as read.
What's more, spec says"
"If a Characteristic Value is prepared two or more times during this sub-
procedure, then all prepared values are written to the same Characteristic
Value in the order that they were prepared." BLUETOOTH SPECIFICATION
Version 5.0 | Vol 3, Part G (4.9.5)
"Each client’s queued values are separate; the execution of one queue shall not
affect the preparation or execution of any other client’s queued
values." BLUETOOTH SPECIFICATION Version 5.0 | Vol 3, Part F (3.4.6.1)
Regards,
Grzegorz
2018-04-19 17:11 GMT+02:00 Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx>:
> Hi Grzegorz,
>
> On Thu, Apr 19, 2018 at 5:03 PM, Grzegorz Kolodziejczyk
> <grzegorz.kolodziejczyk@xxxxxxxxxxx> wrote:
>> Multiple prepare writes may be done for multiple attributes. Queue
>> mechanism must be aware of handle under which preparation is done.
>> ---
>> src/shared/gatt-server.c | 15 ++++++++++++---
>> 1 file changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
>> index 4b554f665..d1efa83a1 100644
>> --- a/src/shared/gatt-server.c
>> +++ b/src/shared/gatt-server.c
>> @@ -1190,6 +1190,14 @@ static bool prep_data_new(struct bt_gatt_server *server,
>> return true;
>> }
>>
>> +static bool match_prep_attr_handle(const void *data, const void *match_data)
>> +{
>> + const struct prep_write_data *prep_data = data;
>> + const uint16_t *match_handle = match_data;
>> +
>> + return prep_data->handle == *match_handle;
>> +}
>> +
>> static bool store_prep_data(struct bt_gatt_server *server,
>> uint16_t handle, uint16_t offset,
>> uint16_t length, uint8_t *value)
>> @@ -1200,9 +1208,10 @@ static bool store_prep_data(struct bt_gatt_server *server,
>> * Now lets check if prep write is a continuation of long write
>> * If so do aggregation of data
>> */
>> - prep_data = queue_peek_tail(server->prep_queue);
>> - if (prep_data && (prep_data->handle == handle) &&
>> - (offset == (prep_data->length + prep_data->offset)))
>> + prep_data = queue_find(server->prep_queue, match_prep_attr_handle,
>> + &handle);
>
> If I got this right it is not that the code is broken, it just don't
> group together the writes if there are other handles in the prepare
> queue. Afaik that was done because the order we write the handles may
> matter, so it is up to the remote stack to make the order proper if it
> wants long writes to all be executed in sequence, or interleaved which
> means we would have to store new chunks in the order they are
> prepared.
>
>> + if (prep_data && (offset == (prep_data->length + prep_data->offset)))
>> return append_prep_data(prep_data, handle, length, value);
>>
>> return prep_data_new(server, handle, offset, length, value);
>> --
>> 2.13.6
>>
>> --
>> 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
>
>
>
> --
> Luiz Augusto von Dentz
--
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
