This patch set implements the server side of GATT discovery procedures. With
this, characteristic, descriptor, and include declaration discovery can be
performed on a device running shared/gatt-server.
Also included are modifications to shared/gatt-db's external read/write handlers
and minor fixes to shared/att.
The TODO has also been updated to reflect the remaining core shared/gatt-server
features. I've already started with most of them but I'm including them there to
track the progress.
Arman Uguray (8):
shared/att: bt_att_cancel should not cancel pending
requests/indications.
shared/gatt-db: Add complete callback to gatt_db_read.
shared/gatt-db: Add complete callback to gatt_db_write.
shared/att-types: Move GATT characteristic property defines to
att-types.
shared/att-types: Add attribute permission bitfield definitions.
shared/gatt-server: Implement "Read By Type" request.
shared/gatt-server: Implement "Find Information" request.
TODO: Update shared/gatt-server items.
TODO | 35 +++-
android/gatt.c | 42 +++--
src/shared/att-types.h | 26 +++
src/shared/att.c | 24 +--
src/shared/gatt-client.h | 9 -
src/shared/gatt-db.c | 14 +-
src/shared/gatt-db.h | 17 +-
src/shared/gatt-server.c | 431 ++++++++++++++++++++++++++++++++++++++++++++++-
8 files changed, 554 insertions(+), 44 deletions(-)
--
2.1.0.rc2.206.gedb03e5
Hi Luiz,
>>>> This patch introduces a completion callback parameter to gatt_db_read,
>>>> which is meant to be used by a gatt_db_read_t implementation to signal
>>>> the end of an asynchronous read operation performed in the upper layer.
>>>> ---
>>>> android/gatt.c | 21 ++++++++++++++++++---
>>>> src/shared/gatt-db.c | 7 +++++--
>>>> src/shared/gatt-db.h | 9 ++++++++-
>>>> src/shared/gatt-server.c | 4 ++--
>>>> 4 files changed, 33 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/android/gatt.c b/android/gatt.c
>>>> index ea20941..e2aa686 100644
>>>> --- a/android/gatt.c
>>>> +++ b/android/gatt.c
>>>> @@ -4694,7 +4694,8 @@ static void read_requested_attributes(void *data, void *user_data)
>>>> resp_data->offset,
>>>> process_data->opcode,
>>>> &process_data->device->bdaddr,
>>>> - &value, &value_len))
>>>> + &value, &value_len,
>>>> + NULL, NULL))
>>>> error = ATT_ECODE_UNLIKELY;
>>>>
>>>> /* We have value here already if no callback will be called */
>>>> @@ -4744,7 +4745,10 @@ static struct pending_trans_data *conn_add_transact(struct app_connection *conn,
>>>> }
>>>>
>>>> static void read_cb(uint16_t handle, uint16_t offset, uint8_t att_opcode,
>>>> - bdaddr_t *bdaddr, void *user_data)
>>>> + bdaddr_t *bdaddr,
>>>> + gatt_db_read_complete_t complete_func,
>>>> + void *complete_data,
>>>> + void *user_data)
>>>> {
>>>> struct pending_trans_data *transaction;
>>>> struct hal_ev_gatt_server_request_read ev;
>>>> @@ -6266,7 +6270,10 @@ static struct gap_srvc_handles gap_srvc_data;
>>>> #define PERIPHERAL_PRIVACY_DISABLE 0x00
>>>>
>>>> static void gap_read_cb(uint16_t handle, uint16_t offset, uint8_t att_opcode,
>>>> - bdaddr_t *bdaddr, void *user_data)
>>>> + bdaddr_t *bdaddr,
>>>> + gatt_db_read_complete_t complete_func,
>>>> + void *complete_data,
>>>> + void *user_data)
>>>> {
>>>> struct pending_request *entry;
>>>> struct gatt_device *dev;
>>>> @@ -6373,6 +6380,8 @@ static void register_gap_service(void)
>>>>
>>>> static void device_info_read_cb(uint16_t handle, uint16_t offset,
>>>> uint8_t att_opcode, bdaddr_t *bdaddr,
>>>> + gatt_db_read_complete_t complete_func,
>>>> + void *complete_data,
>>>> void *user_data)
>>>> {
>>>> struct pending_request *entry;
>>>> @@ -6406,6 +6415,8 @@ done:
>>>>
>>>> static void device_info_read_system_id_cb(uint16_t handle, uint16_t offset,
>>>> uint8_t att_opcode, bdaddr_t *bdaddr,
>>>> + gatt_db_read_complete_t complete_func,
>>>> + void *complete_data,
>>>> void *user_data)
>>>> {
>>>> struct pending_request *entry;
>>>> @@ -6438,6 +6449,8 @@ done:
>>>>
>>>> static void device_info_read_pnp_id_cb(uint16_t handle, uint16_t offset,
>>>> uint8_t att_opcode, bdaddr_t *bdaddr,
>>>> + gatt_db_read_complete_t complete_func,
>>>> + void *complete_data,
>>>> void *user_data)
>>>> {
>>>> struct pending_request *entry;
>>>> @@ -6602,6 +6615,8 @@ static void gatt_srvc_change_write_cb(uint16_t handle, uint16_t offset,
>>>>
>>>> static void gatt_srvc_change_read_cb(uint16_t handle, uint16_t offset,
>>>> uint8_t att_opcode, bdaddr_t *bdaddr,
>>>> + gatt_db_read_complete_t complete_func,
>>>> + void *complete_data,
>>>> void *user_data)
>>>> {
>>>> struct pending_request *entry;
>>>> diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
>>>> index b3f95d2..c342e32 100644
>>>> --- a/src/shared/gatt-db.c
>>>> +++ b/src/shared/gatt-db.c
>>>> @@ -638,7 +638,9 @@ static bool find_service_for_handle(const void *data, const void *user_data)
>>>>
>>>> bool gatt_db_read(struct gatt_db *db, uint16_t handle, uint16_t offset,
>>>> uint8_t att_opcode, bdaddr_t *bdaddr,
>>>> - uint8_t **value, int *length)
>>>> + uint8_t **value, int *length,
>>>> + gatt_db_read_complete_t complete_func,
>>>> + void *complete_data)
>>>> {
>>>> struct gatt_db_service *service;
>>>> uint16_t service_handle;
>>>> @@ -665,7 +667,8 @@ bool gatt_db_read(struct gatt_db *db, uint16_t handle, uint16_t offset,
>>>> if (a->read_func) {
>>>> *value = NULL;
>>>> *length = -1;
>>>> - a->read_func(handle, offset, att_opcode, bdaddr, a->user_data);
>>>> + a->read_func(handle, offset, att_opcode, bdaddr, complete_func,
>>>> + complete_data, a->user_data);
>>>> } else {
>>>> if (offset > a->value_len)
>>>> return false;
>>>> diff --git a/src/shared/gatt-db.h b/src/shared/gatt-db.h
>>>> index 8d18434..1c8739e 100644
>>>> --- a/src/shared/gatt-db.h
>>>> +++ b/src/shared/gatt-db.h
>>>> @@ -30,8 +30,13 @@ uint16_t gatt_db_add_service(struct gatt_db *db, const bt_uuid_t *uuid,
>>>> bool primary, uint16_t num_handles);
>>>> bool gatt_db_remove_service(struct gatt_db *db, uint16_t handle);
>>>>
>>>> +typedef void (*gatt_db_read_complete_t)(uint16_t handle, uint16_t att_ecode,
>>>> + const uint8_t *value, size_t len,
>>>> + void *complete_data);
>>>> typedef void (*gatt_db_read_t) (uint16_t handle, uint16_t offset,
>>>> uint8_t att_opcode, bdaddr_t *bdaddr,
>>>> + gatt_db_read_complete_t complete_func,
>>>> + void *complete_data,
>>>> void *user_data);
>>>>
>>>> typedef void (*gatt_db_write_t) (uint16_t handle, uint16_t offset,
>>>> @@ -81,7 +86,9 @@ void gatt_db_find_information(struct gatt_db *db, uint16_t start_handle,
>>>>
>>>> bool gatt_db_read(struct gatt_db *db, uint16_t handle, uint16_t offset,
>>>> uint8_t att_opcode, bdaddr_t *bdaddr,
>>>> - uint8_t **value, int *length);
>>>> + uint8_t **value, int *length,
>>>> + gatt_db_read_complete_t complete_func,
>>>> + void *complete_data);
>>>
>>> Lets remove value and length here and make it always return via
>>> callback, we can probably use a idle callback if there is a problem to
>>> return the values immediately.
>>>
>>
>> I agree with this, since this'll get rid of a lot duplicate code, and
>> we can probably get away with not using an idle callback initially.
>> One problem though is that the android code makes use of these
>> functions and I don't have an Android build or devices set up to test
>> this. So would you mind taking on this task (or someone that has an
>> android build), change the gatt_db_read signature and make sure all
>> the android code works with passing unit tests, and then I'll rebase
>> my gatt-server patches on top of it?
>
> Let me take care of with gatt_db_attribute, gatt_db_attribute_read
> should be done with callback and Im considering having a cancel for
> these operations in case we need to abort them.
>
OK, I will wait for you to make those changes on the Android side and
rebase my gatt-server patches on top of that.
>> Or if we don't want to hold back on my current patch set, you can
>> merge them and then do the refactor. Up to you.
>>
>>
>>>> bool gatt_db_write(struct gatt_db *db, uint16_t handle, uint16_t offset,
>>>> const uint8_t *value, size_t len,
>>>> diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
>>>> index 657b564..3233b21 100644
>>>> --- a/src/shared/gatt-server.c
>>>> +++ b/src/shared/gatt-server.c
>>>> @@ -114,8 +114,8 @@ static bool encode_read_by_grp_type_rsp(struct gatt_db *db, struct queue *q,
>>>> */
>>>> if (!gatt_db_read(db, start_handle, 0,
>>>> BT_ATT_OP_READ_BY_GRP_TYPE_REQ,
>>>> - NULL, &value,
>>>> - &value_len) || value_len < 0)
>>>> + NULL, &value, &value_len,
>>>> + NULL, NULL) || value_len < 0)
>>>> return false;
>>>>
>>>> /*
>>>> --
>>>> 2.1.0.rc2.206.gedb03e5
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>>>> the body of a message to [email protected]
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>>>
>>>
>>> --
>>> Luiz Augusto von Dentz
>>
>> Cheers,
>> Arman
>
>
>
> --
> Luiz Augusto von Dentz
-Arman
Hi Luiz,
>>>> This patch implements the ATT protocol "Read By Type" request for
>>>> shared/gatt-server. Logic is implemented that allows asynchronous
>>>> reading of non-standard attribute values via the registered read and
>>>> read completion callbacks.
>>>> ---
>>>> src/shared/gatt-server.c | 273 ++++++++++++++++++++++++++++++++++++++++++++++-
>>>> 1 file changed, 270 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
>>>> index 3233b21..6c5ea2b 100644
>>>> --- a/src/shared/gatt-server.c
>>>> +++ b/src/shared/gatt-server.c
>>>> @@ -38,6 +38,16 @@
>>>> #define MIN(a, b) ((a) < (b) ? (a) : (b))
>>>> #endif
>>>>
>>>> +struct async_read_op {
>>>> + struct bt_gatt_server *server;
>>>> + uint8_t opcode;
>>>> + bool done;
>>>> + uint8_t *pdu;
>>>> + size_t pdu_len;
>>>> + int value_len;
>>>> + struct queue *db_data;
>>>> +};
>>>> +
>>>> struct bt_gatt_server {
>>>> struct gatt_db *db;
>>>> struct bt_att *att;
>>>> @@ -46,6 +56,9 @@ struct bt_gatt_server {
>>>>
>>>> unsigned int mtu_id;
>>>> unsigned int read_by_grp_type_id;
>>>> + unsigned int read_by_type_id;
>>>> +
>>>> + struct async_read_op *pending_read_op;
>>>>
>>>> bt_gatt_server_debug_func_t debug_callback;
>>>> bt_gatt_server_destroy_func_t debug_destroy;
>>>> @@ -59,8 +72,12 @@ static void bt_gatt_server_free(struct bt_gatt_server *server)
>>>>
>>>> bt_att_unregister(server->att, server->mtu_id);
>>>> bt_att_unregister(server->att, server->read_by_grp_type_id);
>>>> + bt_att_unregister(server->att, server->read_by_type_id);
>>>> bt_att_unref(server->att);
>>>>
>>>> + if (server->pending_read_op)
>>>> + server->pending_read_op->server = NULL;
>>>> +
>>>> free(server);
>>>> }
>>>>
>>>> @@ -124,21 +141,21 @@ static bool encode_read_by_grp_type_rsp(struct gatt_db *db, struct queue *q,
>>>> * value is seen.
>>>> */
>>>> if (iter == 0) {
>>>> - data_val_len = value_len;
>>>> + data_val_len = MIN(MIN(mtu - 6, 251), value_len);
>>>> pdu[0] = data_val_len + 4;
>>>> iter++;
>>>> } else if (value_len != data_val_len)
>>>> break;
>>>>
>>>> /* Stop if this unit would surpass the MTU */
>>>> - if (iter + data_val_len + 4 > mtu)
>>>> + if (iter + data_val_len + 4 > mtu - 1)
>>>> break;
>>>>
>>>> end_handle = gatt_db_get_end_handle(db, start_handle);
>>>>
>>>> put_le16(start_handle, pdu + iter);
>>>> put_le16(end_handle, pdu + iter + 2);
>>>> - memcpy(pdu + iter + 4, value, value_len);
>>>> + memcpy(pdu + iter + 4, value, data_val_len);
>>>>
>>>> iter += data_val_len + 4;
>>>> }
>>>> @@ -235,6 +252,248 @@ done:
>>>> NULL, NULL, NULL);
>>>> }
>>>>
>>>> +static void async_read_op_destroy(struct async_read_op *op)
>>>> +{
>>>> + if (op->server)
>>>> + op->server->pending_read_op = NULL;
>>>> +
>>>> + queue_destroy(op->db_data, NULL);
>>>> + free(op->pdu);
>>>> + free(op);
>>>> +}
>>>> +
>>>> +static void process_read_by_type(struct async_read_op *op);
>>>> +
>>>> +static void read_by_type_read_complete_cb(uint16_t handle, uint16_t att_ecode,
>>>> + const uint8_t *value, size_t len,
>>>> + void *complete_data)
>>>> +{
>>>> + struct async_read_op *op = complete_data;
>>>> + struct bt_gatt_server *server = op->server;
>>>> + uint16_t mtu;
>>>> +
>>>> + if (!server) {
>>>> + async_read_op_destroy(op);
>>>> + return;
>>>> + }
>>>> +
>>>> + mtu = bt_att_get_mtu(server->att);
>>>> +
>>>> + /* Terminate the operation if there was an error */
>>>> + if (att_ecode) {
>>>> + uint8_t pdu[4];
>>>> +
>>>> + encode_error_rsp(BT_ATT_OP_READ_BY_TYPE_REQ, handle, att_ecode,
>>>> + pdu);
>>>> + bt_att_send(server->att, BT_ATT_OP_ERROR_RSP, pdu, 4, NULL,
>>>> + NULL, NULL);
>>>> + async_read_op_destroy(op);
>>>> + return;
>>>> + }
>>>> +
>>>> + if (op->pdu_len == 0) {
>>>> + op->value_len = MIN(MIN(mtu - 4, 253), len);
>>>> + op->pdu[0] = op->value_len + 2;
>>>> + op->pdu_len++;
>>>> + } else if (len != op->value_len) {
>>>> + op->done = true;
>>>> + goto done;
>>>> + }
>>>> +
>>>> + /* Stop if this would surpass the MTU */
>>>> + if (op->pdu_len + op->value_len + 2 > mtu - 1) {
>>>> + op->done = true;
>>>> + goto done;
>>>> + }
>>>> +
>>>> + /* Encode the current value */
>>>> + put_le16(handle, op->pdu + op->pdu_len);
>>>> + memcpy(op->pdu + op->pdu_len + 2, value, op->value_len);
>>>> +
>>>> + op->pdu_len += op->value_len + 2;
>>>> +
>>>> + if (op->pdu_len == mtu - 1)
>>>> + op->done = true;
>>>
>>> The code above is duplicated and it is actually causing some warnings
>>> comparing unsigned with signed (usually cause by '-' operation) which
>>> I started fixing myself but stop once I realize this was the result of
>>> gatt_db_read having 2 modes to read, sync and async, which I don't
>>> think is a good idea.
>>>
>>
>> Yes, please see my response to your other comment in this patch set.
>>
>>
>>>> +done:
>>>> + process_read_by_type(op);
>>>> +}
>>>> +
>>>> +static void process_read_by_type(struct async_read_op *op)
>>>> +{
>>>> + struct bt_gatt_server *server = op->server;
>>>> + uint16_t mtu = bt_att_get_mtu(server->att);
>>>> + uint8_t rsp_opcode;
>>>> + uint8_t rsp_len;
>>>> + uint8_t ecode;
>>>> + uint16_t ehandle;
>>>> + uint16_t start_handle;
>>>> + uint8_t *value;
>>>> + int value_len;
>>>> + uint32_t perm;
>>>> +
>>>> + if (op->done) {
>>>> + rsp_opcode = BT_ATT_OP_READ_BY_TYPE_RSP;
>>>> + rsp_len = op->pdu_len;
>>>> + goto done;
>>>> + }
>>>> +
>>>> + while (queue_peek_head(op->db_data)) {
>>>> + start_handle = PTR_TO_UINT(queue_pop_head(op->db_data));
>>>> + value = NULL;
>>>> + value_len = 0;
>>>> +
>>>> + if (!gatt_db_get_attribute_permissions(server->db, start_handle,
>>>> + &perm)) {
>>>> + ecode = BT_ATT_ERROR_UNLIKELY;
>>>> + goto error;
>>>> + }
>>>> +
>>>> + /*
>>>> + * Check for the READ access permission. Encryption,
>>>> + * authentication, and authorization permissions need to be
>>>> + * checked by the read handler, since bt_att is agnostic to
>>>> + * connection type and doesn't have security information on it.
>>>> + */
>>>> + if (perm && !(perm & BT_ATT_PERM_READ)) {
>>>> + ecode = BT_ATT_ERROR_READ_NOT_PERMITTED;
>>>> + goto error;
>>>> + }
>>>
>>> I would suggest moving this check to gatt_db_read which should return
>>> -EPERM or the actual code if it is too hard to have errno mapping for
>>> each error code.
>>>
>>
>> gatt_db currently has this nice property that the permissions field is
>> just a uint32_t. This means that while shared/gatt-server can use the
>> new permission macros I added to shared/att-types, the android code
>> can still use the Android platform definitions. Of course, once
>> android starts using shared/gatt-server it'll have to use the new
>> macros but for now we can have gatt-db support both types by keeping
>> the permission checks outside of gatt-db and this would make the whole
>> transition easier. Makes sense?
>
> Well I would like to harmonize the definitions with Android as soon as
> possible since you are defining new ones, which Ive applied btw,
> otherwise later when we start using it in Android it might break
> things, btw Im fine if the server actually handling the permissions,
> what I really want is a central place to handle them.
>
OK, then for now I will perform all the checks in shared/gatt-server.
-Arman
Hi Arman,
On Fri, Oct 24, 2014 at 8:02 PM, Arman Uguray <[email protected]> wrote:
> Hi Luiz,
>
>
> On Fri, Oct 24, 2014 at 2:31 AM, Luiz Augusto von Dentz
> <[email protected]> wrote:
>> Hi Arman,
>>
>> On Tue, Oct 21, 2014 at 12:00 AM, Arman Uguray <[email protected]> wrote:
>>> This patch introduces a completion callback parameter to gatt_db_read,
>>> which is meant to be used by a gatt_db_read_t implementation to signal
>>> the end of an asynchronous read operation performed in the upper layer.
>>> ---
>>> android/gatt.c | 21 ++++++++++++++++++---
>>> src/shared/gatt-db.c | 7 +++++--
>>> src/shared/gatt-db.h | 9 ++++++++-
>>> src/shared/gatt-server.c | 4 ++--
>>> 4 files changed, 33 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/android/gatt.c b/android/gatt.c
>>> index ea20941..e2aa686 100644
>>> --- a/android/gatt.c
>>> +++ b/android/gatt.c
>>> @@ -4694,7 +4694,8 @@ static void read_requested_attributes(void *data, void *user_data)
>>> resp_data->offset,
>>> process_data->opcode,
>>> &process_data->device->bdaddr,
>>> - &value, &value_len))
>>> + &value, &value_len,
>>> + NULL, NULL))
>>> error = ATT_ECODE_UNLIKELY;
>>>
>>> /* We have value here already if no callback will be called */
>>> @@ -4744,7 +4745,10 @@ static struct pending_trans_data *conn_add_transact(struct app_connection *conn,
>>> }
>>>
>>> static void read_cb(uint16_t handle, uint16_t offset, uint8_t att_opcode,
>>> - bdaddr_t *bdaddr, void *user_data)
>>> + bdaddr_t *bdaddr,
>>> + gatt_db_read_complete_t complete_func,
>>> + void *complete_data,
>>> + void *user_data)
>>> {
>>> struct pending_trans_data *transaction;
>>> struct hal_ev_gatt_server_request_read ev;
>>> @@ -6266,7 +6270,10 @@ static struct gap_srvc_handles gap_srvc_data;
>>> #define PERIPHERAL_PRIVACY_DISABLE 0x00
>>>
>>> static void gap_read_cb(uint16_t handle, uint16_t offset, uint8_t att_opcode,
>>> - bdaddr_t *bdaddr, void *user_data)
>>> + bdaddr_t *bdaddr,
>>> + gatt_db_read_complete_t complete_func,
>>> + void *complete_data,
>>> + void *user_data)
>>> {
>>> struct pending_request *entry;
>>> struct gatt_device *dev;
>>> @@ -6373,6 +6380,8 @@ static void register_gap_service(void)
>>>
>>> static void device_info_read_cb(uint16_t handle, uint16_t offset,
>>> uint8_t att_opcode, bdaddr_t *bdaddr,
>>> + gatt_db_read_complete_t complete_func,
>>> + void *complete_data,
>>> void *user_data)
>>> {
>>> struct pending_request *entry;
>>> @@ -6406,6 +6415,8 @@ done:
>>>
>>> static void device_info_read_system_id_cb(uint16_t handle, uint16_t offset,
>>> uint8_t att_opcode, bdaddr_t *bdaddr,
>>> + gatt_db_read_complete_t complete_func,
>>> + void *complete_data,
>>> void *user_data)
>>> {
>>> struct pending_request *entry;
>>> @@ -6438,6 +6449,8 @@ done:
>>>
>>> static void device_info_read_pnp_id_cb(uint16_t handle, uint16_t offset,
>>> uint8_t att_opcode, bdaddr_t *bdaddr,
>>> + gatt_db_read_complete_t complete_func,
>>> + void *complete_data,
>>> void *user_data)
>>> {
>>> struct pending_request *entry;
>>> @@ -6602,6 +6615,8 @@ static void gatt_srvc_change_write_cb(uint16_t handle, uint16_t offset,
>>>
>>> static void gatt_srvc_change_read_cb(uint16_t handle, uint16_t offset,
>>> uint8_t att_opcode, bdaddr_t *bdaddr,
>>> + gatt_db_read_complete_t complete_func,
>>> + void *complete_data,
>>> void *user_data)
>>> {
>>> struct pending_request *entry;
>>> diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
>>> index b3f95d2..c342e32 100644
>>> --- a/src/shared/gatt-db.c
>>> +++ b/src/shared/gatt-db.c
>>> @@ -638,7 +638,9 @@ static bool find_service_for_handle(const void *data, const void *user_data)
>>>
>>> bool gatt_db_read(struct gatt_db *db, uint16_t handle, uint16_t offset,
>>> uint8_t att_opcode, bdaddr_t *bdaddr,
>>> - uint8_t **value, int *length)
>>> + uint8_t **value, int *length,
>>> + gatt_db_read_complete_t complete_func,
>>> + void *complete_data)
>>> {
>>> struct gatt_db_service *service;
>>> uint16_t service_handle;
>>> @@ -665,7 +667,8 @@ bool gatt_db_read(struct gatt_db *db, uint16_t handle, uint16_t offset,
>>> if (a->read_func) {
>>> *value = NULL;
>>> *length = -1;
>>> - a->read_func(handle, offset, att_opcode, bdaddr, a->user_data);
>>> + a->read_func(handle, offset, att_opcode, bdaddr, complete_func,
>>> + complete_data, a->user_data);
>>> } else {
>>> if (offset > a->value_len)
>>> return false;
>>> diff --git a/src/shared/gatt-db.h b/src/shared/gatt-db.h
>>> index 8d18434..1c8739e 100644
>>> --- a/src/shared/gatt-db.h
>>> +++ b/src/shared/gatt-db.h
>>> @@ -30,8 +30,13 @@ uint16_t gatt_db_add_service(struct gatt_db *db, const bt_uuid_t *uuid,
>>> bool primary, uint16_t num_handles);
>>> bool gatt_db_remove_service(struct gatt_db *db, uint16_t handle);
>>>
>>> +typedef void (*gatt_db_read_complete_t)(uint16_t handle, uint16_t att_ecode,
>>> + const uint8_t *value, size_t len,
>>> + void *complete_data);
>>> typedef void (*gatt_db_read_t) (uint16_t handle, uint16_t offset,
>>> uint8_t att_opcode, bdaddr_t *bdaddr,
>>> + gatt_db_read_complete_t complete_func,
>>> + void *complete_data,
>>> void *user_data);
>>>
>>> typedef void (*gatt_db_write_t) (uint16_t handle, uint16_t offset,
>>> @@ -81,7 +86,9 @@ void gatt_db_find_information(struct gatt_db *db, uint16_t start_handle,
>>>
>>> bool gatt_db_read(struct gatt_db *db, uint16_t handle, uint16_t offset,
>>> uint8_t att_opcode, bdaddr_t *bdaddr,
>>> - uint8_t **value, int *length);
>>> + uint8_t **value, int *length,
>>> + gatt_db_read_complete_t complete_func,
>>> + void *complete_data);
>>
>> Lets remove value and length here and make it always return via
>> callback, we can probably use a idle callback if there is a problem to
>> return the values immediately.
>>
>
> I agree with this, since this'll get rid of a lot duplicate code, and
> we can probably get away with not using an idle callback initially.
> One problem though is that the android code makes use of these
> functions and I don't have an Android build or devices set up to test
> this. So would you mind taking on this task (or someone that has an
> android build), change the gatt_db_read signature and make sure all
> the android code works with passing unit tests, and then I'll rebase
> my gatt-server patches on top of it?
Let me take care of with gatt_db_attribute, gatt_db_attribute_read
should be done with callback and Im considering having a cancel for
these operations in case we need to abort them.
> Or if we don't want to hold back on my current patch set, you can
> merge them and then do the refactor. Up to you.
>
>
>>> bool gatt_db_write(struct gatt_db *db, uint16_t handle, uint16_t offset,
>>> const uint8_t *value, size_t len,
>>> diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
>>> index 657b564..3233b21 100644
>>> --- a/src/shared/gatt-server.c
>>> +++ b/src/shared/gatt-server.c
>>> @@ -114,8 +114,8 @@ static bool encode_read_by_grp_type_rsp(struct gatt_db *db, struct queue *q,
>>> */
>>> if (!gatt_db_read(db, start_handle, 0,
>>> BT_ATT_OP_READ_BY_GRP_TYPE_REQ,
>>> - NULL, &value,
>>> - &value_len) || value_len < 0)
>>> + NULL, &value, &value_len,
>>> + NULL, NULL) || value_len < 0)
>>> return false;
>>>
>>> /*
>>> --
>>> 2.1.0.rc2.206.gedb03e5
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>>> the body of a message to [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
>>
>> --
>> Luiz Augusto von Dentz
>
> Cheers,
> Arman
--
Luiz Augusto von Dentz
Hi Arman,
On Fri, Oct 24, 2014 at 8:12 PM, Arman Uguray <[email protected]> wrote:
> Hi Luiz,
>
>
> On Fri, Oct 24, 2014 at 2:25 AM, Luiz Augusto von Dentz
> <[email protected]> wrote:
>> Hi Arman,
>>
>> On Tue, Oct 21, 2014 at 12:00 AM, Arman Uguray <[email protected]> wrote:
>>> This patch implements the ATT protocol "Read By Type" request for
>>> shared/gatt-server. Logic is implemented that allows asynchronous
>>> reading of non-standard attribute values via the registered read and
>>> read completion callbacks.
>>> ---
>>> src/shared/gatt-server.c | 273 ++++++++++++++++++++++++++++++++++++++++++++++-
>>> 1 file changed, 270 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
>>> index 3233b21..6c5ea2b 100644
>>> --- a/src/shared/gatt-server.c
>>> +++ b/src/shared/gatt-server.c
>>> @@ -38,6 +38,16 @@
>>> #define MIN(a, b) ((a) < (b) ? (a) : (b))
>>> #endif
>>>
>>> +struct async_read_op {
>>> + struct bt_gatt_server *server;
>>> + uint8_t opcode;
>>> + bool done;
>>> + uint8_t *pdu;
>>> + size_t pdu_len;
>>> + int value_len;
>>> + struct queue *db_data;
>>> +};
>>> +
>>> struct bt_gatt_server {
>>> struct gatt_db *db;
>>> struct bt_att *att;
>>> @@ -46,6 +56,9 @@ struct bt_gatt_server {
>>>
>>> unsigned int mtu_id;
>>> unsigned int read_by_grp_type_id;
>>> + unsigned int read_by_type_id;
>>> +
>>> + struct async_read_op *pending_read_op;
>>>
>>> bt_gatt_server_debug_func_t debug_callback;
>>> bt_gatt_server_destroy_func_t debug_destroy;
>>> @@ -59,8 +72,12 @@ static void bt_gatt_server_free(struct bt_gatt_server *server)
>>>
>>> bt_att_unregister(server->att, server->mtu_id);
>>> bt_att_unregister(server->att, server->read_by_grp_type_id);
>>> + bt_att_unregister(server->att, server->read_by_type_id);
>>> bt_att_unref(server->att);
>>>
>>> + if (server->pending_read_op)
>>> + server->pending_read_op->server = NULL;
>>> +
>>> free(server);
>>> }
>>>
>>> @@ -124,21 +141,21 @@ static bool encode_read_by_grp_type_rsp(struct gatt_db *db, struct queue *q,
>>> * value is seen.
>>> */
>>> if (iter == 0) {
>>> - data_val_len = value_len;
>>> + data_val_len = MIN(MIN(mtu - 6, 251), value_len);
>>> pdu[0] = data_val_len + 4;
>>> iter++;
>>> } else if (value_len != data_val_len)
>>> break;
>>>
>>> /* Stop if this unit would surpass the MTU */
>>> - if (iter + data_val_len + 4 > mtu)
>>> + if (iter + data_val_len + 4 > mtu - 1)
>>> break;
>>>
>>> end_handle = gatt_db_get_end_handle(db, start_handle);
>>>
>>> put_le16(start_handle, pdu + iter);
>>> put_le16(end_handle, pdu + iter + 2);
>>> - memcpy(pdu + iter + 4, value, value_len);
>>> + memcpy(pdu + iter + 4, value, data_val_len);
>>>
>>> iter += data_val_len + 4;
>>> }
>>> @@ -235,6 +252,248 @@ done:
>>> NULL, NULL, NULL);
>>> }
>>>
>>> +static void async_read_op_destroy(struct async_read_op *op)
>>> +{
>>> + if (op->server)
>>> + op->server->pending_read_op = NULL;
>>> +
>>> + queue_destroy(op->db_data, NULL);
>>> + free(op->pdu);
>>> + free(op);
>>> +}
>>> +
>>> +static void process_read_by_type(struct async_read_op *op);
>>> +
>>> +static void read_by_type_read_complete_cb(uint16_t handle, uint16_t att_ecode,
>>> + const uint8_t *value, size_t len,
>>> + void *complete_data)
>>> +{
>>> + struct async_read_op *op = complete_data;
>>> + struct bt_gatt_server *server = op->server;
>>> + uint16_t mtu;
>>> +
>>> + if (!server) {
>>> + async_read_op_destroy(op);
>>> + return;
>>> + }
>>> +
>>> + mtu = bt_att_get_mtu(server->att);
>>> +
>>> + /* Terminate the operation if there was an error */
>>> + if (att_ecode) {
>>> + uint8_t pdu[4];
>>> +
>>> + encode_error_rsp(BT_ATT_OP_READ_BY_TYPE_REQ, handle, att_ecode,
>>> + pdu);
>>> + bt_att_send(server->att, BT_ATT_OP_ERROR_RSP, pdu, 4, NULL,
>>> + NULL, NULL);
>>> + async_read_op_destroy(op);
>>> + return;
>>> + }
>>> +
>>> + if (op->pdu_len == 0) {
>>> + op->value_len = MIN(MIN(mtu - 4, 253), len);
>>> + op->pdu[0] = op->value_len + 2;
>>> + op->pdu_len++;
>>> + } else if (len != op->value_len) {
>>> + op->done = true;
>>> + goto done;
>>> + }
>>> +
>>> + /* Stop if this would surpass the MTU */
>>> + if (op->pdu_len + op->value_len + 2 > mtu - 1) {
>>> + op->done = true;
>>> + goto done;
>>> + }
>>> +
>>> + /* Encode the current value */
>>> + put_le16(handle, op->pdu + op->pdu_len);
>>> + memcpy(op->pdu + op->pdu_len + 2, value, op->value_len);
>>> +
>>> + op->pdu_len += op->value_len + 2;
>>> +
>>> + if (op->pdu_len == mtu - 1)
>>> + op->done = true;
>>
>> The code above is duplicated and it is actually causing some warnings
>> comparing unsigned with signed (usually cause by '-' operation) which
>> I started fixing myself but stop once I realize this was the result of
>> gatt_db_read having 2 modes to read, sync and async, which I don't
>> think is a good idea.
>>
>
> Yes, please see my response to your other comment in this patch set.
>
>
>>> +done:
>>> + process_read_by_type(op);
>>> +}
>>> +
>>> +static void process_read_by_type(struct async_read_op *op)
>>> +{
>>> + struct bt_gatt_server *server = op->server;
>>> + uint16_t mtu = bt_att_get_mtu(server->att);
>>> + uint8_t rsp_opcode;
>>> + uint8_t rsp_len;
>>> + uint8_t ecode;
>>> + uint16_t ehandle;
>>> + uint16_t start_handle;
>>> + uint8_t *value;
>>> + int value_len;
>>> + uint32_t perm;
>>> +
>>> + if (op->done) {
>>> + rsp_opcode = BT_ATT_OP_READ_BY_TYPE_RSP;
>>> + rsp_len = op->pdu_len;
>>> + goto done;
>>> + }
>>> +
>>> + while (queue_peek_head(op->db_data)) {
>>> + start_handle = PTR_TO_UINT(queue_pop_head(op->db_data));
>>> + value = NULL;
>>> + value_len = 0;
>>> +
>>> + if (!gatt_db_get_attribute_permissions(server->db, start_handle,
>>> + &perm)) {
>>> + ecode = BT_ATT_ERROR_UNLIKELY;
>>> + goto error;
>>> + }
>>> +
>>> + /*
>>> + * Check for the READ access permission. Encryption,
>>> + * authentication, and authorization permissions need to be
>>> + * checked by the read handler, since bt_att is agnostic to
>>> + * connection type and doesn't have security information on it.
>>> + */
>>> + if (perm && !(perm & BT_ATT_PERM_READ)) {
>>> + ecode = BT_ATT_ERROR_READ_NOT_PERMITTED;
>>> + goto error;
>>> + }
>>
>> I would suggest moving this check to gatt_db_read which should return
>> -EPERM or the actual code if it is too hard to have errno mapping for
>> each error code.
>>
>
> gatt_db currently has this nice property that the permissions field is
> just a uint32_t. This means that while shared/gatt-server can use the
> new permission macros I added to shared/att-types, the android code
> can still use the Android platform definitions. Of course, once
> android starts using shared/gatt-server it'll have to use the new
> macros but for now we can have gatt-db support both types by keeping
> the permission checks outside of gatt-db and this would make the whole
> transition easier. Makes sense?
Well I would like to harmonize the definitions with Android as soon as
possible since you are defining new ones, which Ive applied btw,
otherwise later when we start using it in Android it might break
things, btw Im fine if the server actually handling the permissions,
what I really want is a central place to handle them.
--
Luiz Augusto von Dentz
Hi Luiz,
On Fri, Oct 24, 2014 at 2:25 AM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Arman,
>
> On Tue, Oct 21, 2014 at 12:00 AM, Arman Uguray <[email protected]> wrote:
>> This patch implements the ATT protocol "Read By Type" request for
>> shared/gatt-server. Logic is implemented that allows asynchronous
>> reading of non-standard attribute values via the registered read and
>> read completion callbacks.
>> ---
>> src/shared/gatt-server.c | 273 ++++++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 270 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
>> index 3233b21..6c5ea2b 100644
>> --- a/src/shared/gatt-server.c
>> +++ b/src/shared/gatt-server.c
>> @@ -38,6 +38,16 @@
>> #define MIN(a, b) ((a) < (b) ? (a) : (b))
>> #endif
>>
>> +struct async_read_op {
>> + struct bt_gatt_server *server;
>> + uint8_t opcode;
>> + bool done;
>> + uint8_t *pdu;
>> + size_t pdu_len;
>> + int value_len;
>> + struct queue *db_data;
>> +};
>> +
>> struct bt_gatt_server {
>> struct gatt_db *db;
>> struct bt_att *att;
>> @@ -46,6 +56,9 @@ struct bt_gatt_server {
>>
>> unsigned int mtu_id;
>> unsigned int read_by_grp_type_id;
>> + unsigned int read_by_type_id;
>> +
>> + struct async_read_op *pending_read_op;
>>
>> bt_gatt_server_debug_func_t debug_callback;
>> bt_gatt_server_destroy_func_t debug_destroy;
>> @@ -59,8 +72,12 @@ static void bt_gatt_server_free(struct bt_gatt_server *server)
>>
>> bt_att_unregister(server->att, server->mtu_id);
>> bt_att_unregister(server->att, server->read_by_grp_type_id);
>> + bt_att_unregister(server->att, server->read_by_type_id);
>> bt_att_unref(server->att);
>>
>> + if (server->pending_read_op)
>> + server->pending_read_op->server = NULL;
>> +
>> free(server);
>> }
>>
>> @@ -124,21 +141,21 @@ static bool encode_read_by_grp_type_rsp(struct gatt_db *db, struct queue *q,
>> * value is seen.
>> */
>> if (iter == 0) {
>> - data_val_len = value_len;
>> + data_val_len = MIN(MIN(mtu - 6, 251), value_len);
>> pdu[0] = data_val_len + 4;
>> iter++;
>> } else if (value_len != data_val_len)
>> break;
>>
>> /* Stop if this unit would surpass the MTU */
>> - if (iter + data_val_len + 4 > mtu)
>> + if (iter + data_val_len + 4 > mtu - 1)
>> break;
>>
>> end_handle = gatt_db_get_end_handle(db, start_handle);
>>
>> put_le16(start_handle, pdu + iter);
>> put_le16(end_handle, pdu + iter + 2);
>> - memcpy(pdu + iter + 4, value, value_len);
>> + memcpy(pdu + iter + 4, value, data_val_len);
>>
>> iter += data_val_len + 4;
>> }
>> @@ -235,6 +252,248 @@ done:
>> NULL, NULL, NULL);
>> }
>>
>> +static void async_read_op_destroy(struct async_read_op *op)
>> +{
>> + if (op->server)
>> + op->server->pending_read_op = NULL;
>> +
>> + queue_destroy(op->db_data, NULL);
>> + free(op->pdu);
>> + free(op);
>> +}
>> +
>> +static void process_read_by_type(struct async_read_op *op);
>> +
>> +static void read_by_type_read_complete_cb(uint16_t handle, uint16_t att_ecode,
>> + const uint8_t *value, size_t len,
>> + void *complete_data)
>> +{
>> + struct async_read_op *op = complete_data;
>> + struct bt_gatt_server *server = op->server;
>> + uint16_t mtu;
>> +
>> + if (!server) {
>> + async_read_op_destroy(op);
>> + return;
>> + }
>> +
>> + mtu = bt_att_get_mtu(server->att);
>> +
>> + /* Terminate the operation if there was an error */
>> + if (att_ecode) {
>> + uint8_t pdu[4];
>> +
>> + encode_error_rsp(BT_ATT_OP_READ_BY_TYPE_REQ, handle, att_ecode,
>> + pdu);
>> + bt_att_send(server->att, BT_ATT_OP_ERROR_RSP, pdu, 4, NULL,
>> + NULL, NULL);
>> + async_read_op_destroy(op);
>> + return;
>> + }
>> +
>> + if (op->pdu_len == 0) {
>> + op->value_len = MIN(MIN(mtu - 4, 253), len);
>> + op->pdu[0] = op->value_len + 2;
>> + op->pdu_len++;
>> + } else if (len != op->value_len) {
>> + op->done = true;
>> + goto done;
>> + }
>> +
>> + /* Stop if this would surpass the MTU */
>> + if (op->pdu_len + op->value_len + 2 > mtu - 1) {
>> + op->done = true;
>> + goto done;
>> + }
>> +
>> + /* Encode the current value */
>> + put_le16(handle, op->pdu + op->pdu_len);
>> + memcpy(op->pdu + op->pdu_len + 2, value, op->value_len);
>> +
>> + op->pdu_len += op->value_len + 2;
>> +
>> + if (op->pdu_len == mtu - 1)
>> + op->done = true;
>
> The code above is duplicated and it is actually causing some warnings
> comparing unsigned with signed (usually cause by '-' operation) which
> I started fixing myself but stop once I realize this was the result of
> gatt_db_read having 2 modes to read, sync and async, which I don't
> think is a good idea.
>
Yes, please see my response to your other comment in this patch set.
>> +done:
>> + process_read_by_type(op);
>> +}
>> +
>> +static void process_read_by_type(struct async_read_op *op)
>> +{
>> + struct bt_gatt_server *server = op->server;
>> + uint16_t mtu = bt_att_get_mtu(server->att);
>> + uint8_t rsp_opcode;
>> + uint8_t rsp_len;
>> + uint8_t ecode;
>> + uint16_t ehandle;
>> + uint16_t start_handle;
>> + uint8_t *value;
>> + int value_len;
>> + uint32_t perm;
>> +
>> + if (op->done) {
>> + rsp_opcode = BT_ATT_OP_READ_BY_TYPE_RSP;
>> + rsp_len = op->pdu_len;
>> + goto done;
>> + }
>> +
>> + while (queue_peek_head(op->db_data)) {
>> + start_handle = PTR_TO_UINT(queue_pop_head(op->db_data));
>> + value = NULL;
>> + value_len = 0;
>> +
>> + if (!gatt_db_get_attribute_permissions(server->db, start_handle,
>> + &perm)) {
>> + ecode = BT_ATT_ERROR_UNLIKELY;
>> + goto error;
>> + }
>> +
>> + /*
>> + * Check for the READ access permission. Encryption,
>> + * authentication, and authorization permissions need to be
>> + * checked by the read handler, since bt_att is agnostic to
>> + * connection type and doesn't have security information on it.
>> + */
>> + if (perm && !(perm & BT_ATT_PERM_READ)) {
>> + ecode = BT_ATT_ERROR_READ_NOT_PERMITTED;
>> + goto error;
>> + }
>
> I would suggest moving this check to gatt_db_read which should return
> -EPERM or the actual code if it is too hard to have errno mapping for
> each error code.
>
gatt_db currently has this nice property that the permissions field is
just a uint32_t. This means that while shared/gatt-server can use the
new permission macros I added to shared/att-types, the android code
can still use the Android platform definitions. Of course, once
android starts using shared/gatt-server it'll have to use the new
macros but for now we can have gatt-db support both types by keeping
the permission checks outside of gatt-db and this would make the whole
transition easier. Makes sense?
>> + if (!gatt_db_read(server->db, start_handle, 0, op->opcode, NULL,
>> + &value, &value_len,
>> + read_by_type_read_complete_cb, op)) {
>> + ecode = BT_ATT_ERROR_UNLIKELY;
>> + goto error;
>> + }
>> +
>> + /* The read has been deferred to the upper layer if |value_len|
>> + * is less than 0.
>> + */
>> + if (value_len < 0)
>> + return;
>> +
>> + if (op->pdu_len == 0) {
>> + op->value_len = MIN(MIN(mtu - 4, 253), value_len);
>> + op->pdu[0] = op->value_len + 2;
>> + op->pdu_len++;
>> + } else if (value_len != op->value_len)
>> + break;
>> +
>> + /* Stop if this would surpass the MTU */
>> + if (op->pdu_len + op->value_len + 2 > mtu - 1)
>> + break;
>> +
>> + /* Encode the current value */
>> + put_le16(start_handle, op->pdu + op->pdu_len);
>> + memcpy(op->pdu + op->pdu_len + 2, value, op->value_len);
>> +
>> + op->pdu_len += op->value_len + 2;
>> +
>> + if (op->pdu_len == mtu - 1)
>> + break;
>> + }
>> +
>> + rsp_opcode = BT_ATT_OP_READ_BY_TYPE_RSP;
>> + rsp_len = op->pdu_len;
>> +
>> + goto done;
>> +
>> +error:
>> + rsp_opcode = BT_ATT_OP_ERROR_RSP;
>> + rsp_len = 4;
>> + encode_error_rsp(BT_ATT_OP_READ_BY_TYPE_REQ, ehandle, ecode, op->pdu);
>> +
>> +done:
>> + bt_att_send(server->att, rsp_opcode, op->pdu, rsp_len, NULL,
>> + NULL, NULL);
>> + async_read_op_destroy(op);
>> +}
>> +
>> +static void read_by_type_cb(uint8_t opcode, const void *pdu,
>> + uint16_t length, void *user_data)
>> +{
>> + struct bt_gatt_server *server = user_data;
>> + uint16_t start, end;
>> + bt_uuid_t type;
>> + uint8_t rsp_pdu[4];
>> + uint16_t ehandle = 0;
>> + uint8_t ecode;
>> + struct queue *q = NULL;
>> + struct async_read_op *op;
>> +
>> + if (length != 6 && length != 20) {
>> + ecode = BT_ATT_ERROR_INVALID_PDU;
>> + goto error;
>> + }
>> +
>> + q = queue_new();
>> + if (!q) {
>> + ecode = BT_ATT_ERROR_INSUFFICIENT_RESOURCES;
>> + goto error;
>> + }
>> +
>> + start = get_le16(pdu);
>> + end = get_le16(pdu + 2);
>> + get_uuid_le(pdu + 4, length - 4, &type);
>> +
>> + util_debug(server->debug_callback, server->debug_data,
>> + "Read By Type - start: 0x%04x end: 0x%04x",
>> + start, end);
>> +
>> + if (!start || !end) {
>> + ecode = BT_ATT_ERROR_INVALID_HANDLE;
>> + goto error;
>> + }
>> +
>> + ehandle = start;
>> +
>> + if (start > end) {
>> + ecode = BT_ATT_ERROR_INVALID_HANDLE;
>> + goto error;
>> + }
>> +
>> + gatt_db_read_by_type(server->db, start, end, type, q);
>> +
>> + if (queue_isempty(q)) {
>> + ecode = BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND;
>> + goto error;
>> + }
>> +
>> + if (server->pending_read_op) {
>> + ecode = BT_ATT_ERROR_UNLIKELY;
>> + goto error;
>> + }
>> +
>> + op = new0(struct async_read_op, 1);
>> + if (!op) {
>> + ecode = BT_ATT_ERROR_INSUFFICIENT_RESOURCES;
>> + goto error;
>> + }
>> +
>> + op->pdu = malloc(bt_att_get_mtu(server->att));
>> + if (!op->pdu) {
>> + free(op);
>> + ecode = BT_ATT_ERROR_INSUFFICIENT_RESOURCES;
>> + goto error;
>> + }
>> +
>> + op->opcode = opcode;
>> + op->server = server;
>> + op->db_data = q;
>> + server->pending_read_op = op;
>> +
>> + process_read_by_type(op);
>> +
>> + return;
>> +
>> +error:
>> + encode_error_rsp(opcode, ehandle, ecode, rsp_pdu);
>> + queue_destroy(q, NULL);
>> + bt_att_send(server->att, BT_ATT_OP_ERROR_RSP, rsp_pdu, 4,
>> + NULL, NULL, NULL);
>> +}
>> +
>> static void exchange_mtu_cb(uint8_t opcode, const void *pdu,
>> uint16_t length, void *user_data)
>> {
>> @@ -283,6 +542,14 @@ static bool gatt_server_register_att_handlers(struct bt_gatt_server *server)
>> if (!server->read_by_grp_type_id)
>> return false;
>>
>> + /* Read By Type */
>> + server->read_by_type_id = bt_att_register(server->att,
>> + BT_ATT_OP_READ_BY_TYPE_REQ,
>> + read_by_type_cb,
>> + server, NULL);
>> + if (!server->read_by_type_id)
>> + return false;
>> +
>> return true;
>> }
>>
>> --
>> 2.1.0.rc2.206.gedb03e5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Luiz Augusto von Dentz
Cheers,
Arman
Hi Luiz,
On Fri, Oct 24, 2014 at 2:31 AM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Arman,
>
> On Tue, Oct 21, 2014 at 12:00 AM, Arman Uguray <[email protected]> wrote:
>> This patch introduces a completion callback parameter to gatt_db_read,
>> which is meant to be used by a gatt_db_read_t implementation to signal
>> the end of an asynchronous read operation performed in the upper layer.
>> ---
>> android/gatt.c | 21 ++++++++++++++++++---
>> src/shared/gatt-db.c | 7 +++++--
>> src/shared/gatt-db.h | 9 ++++++++-
>> src/shared/gatt-server.c | 4 ++--
>> 4 files changed, 33 insertions(+), 8 deletions(-)
>>
>> diff --git a/android/gatt.c b/android/gatt.c
>> index ea20941..e2aa686 100644
>> --- a/android/gatt.c
>> +++ b/android/gatt.c
>> @@ -4694,7 +4694,8 @@ static void read_requested_attributes(void *data, void *user_data)
>> resp_data->offset,
>> process_data->opcode,
>> &process_data->device->bdaddr,
>> - &value, &value_len))
>> + &value, &value_len,
>> + NULL, NULL))
>> error = ATT_ECODE_UNLIKELY;
>>
>> /* We have value here already if no callback will be called */
>> @@ -4744,7 +4745,10 @@ static struct pending_trans_data *conn_add_transact(struct app_connection *conn,
>> }
>>
>> static void read_cb(uint16_t handle, uint16_t offset, uint8_t att_opcode,
>> - bdaddr_t *bdaddr, void *user_data)
>> + bdaddr_t *bdaddr,
>> + gatt_db_read_complete_t complete_func,
>> + void *complete_data,
>> + void *user_data)
>> {
>> struct pending_trans_data *transaction;
>> struct hal_ev_gatt_server_request_read ev;
>> @@ -6266,7 +6270,10 @@ static struct gap_srvc_handles gap_srvc_data;
>> #define PERIPHERAL_PRIVACY_DISABLE 0x00
>>
>> static void gap_read_cb(uint16_t handle, uint16_t offset, uint8_t att_opcode,
>> - bdaddr_t *bdaddr, void *user_data)
>> + bdaddr_t *bdaddr,
>> + gatt_db_read_complete_t complete_func,
>> + void *complete_data,
>> + void *user_data)
>> {
>> struct pending_request *entry;
>> struct gatt_device *dev;
>> @@ -6373,6 +6380,8 @@ static void register_gap_service(void)
>>
>> static void device_info_read_cb(uint16_t handle, uint16_t offset,
>> uint8_t att_opcode, bdaddr_t *bdaddr,
>> + gatt_db_read_complete_t complete_func,
>> + void *complete_data,
>> void *user_data)
>> {
>> struct pending_request *entry;
>> @@ -6406,6 +6415,8 @@ done:
>>
>> static void device_info_read_system_id_cb(uint16_t handle, uint16_t offset,
>> uint8_t att_opcode, bdaddr_t *bdaddr,
>> + gatt_db_read_complete_t complete_func,
>> + void *complete_data,
>> void *user_data)
>> {
>> struct pending_request *entry;
>> @@ -6438,6 +6449,8 @@ done:
>>
>> static void device_info_read_pnp_id_cb(uint16_t handle, uint16_t offset,
>> uint8_t att_opcode, bdaddr_t *bdaddr,
>> + gatt_db_read_complete_t complete_func,
>> + void *complete_data,
>> void *user_data)
>> {
>> struct pending_request *entry;
>> @@ -6602,6 +6615,8 @@ static void gatt_srvc_change_write_cb(uint16_t handle, uint16_t offset,
>>
>> static void gatt_srvc_change_read_cb(uint16_t handle, uint16_t offset,
>> uint8_t att_opcode, bdaddr_t *bdaddr,
>> + gatt_db_read_complete_t complete_func,
>> + void *complete_data,
>> void *user_data)
>> {
>> struct pending_request *entry;
>> diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
>> index b3f95d2..c342e32 100644
>> --- a/src/shared/gatt-db.c
>> +++ b/src/shared/gatt-db.c
>> @@ -638,7 +638,9 @@ static bool find_service_for_handle(const void *data, const void *user_data)
>>
>> bool gatt_db_read(struct gatt_db *db, uint16_t handle, uint16_t offset,
>> uint8_t att_opcode, bdaddr_t *bdaddr,
>> - uint8_t **value, int *length)
>> + uint8_t **value, int *length,
>> + gatt_db_read_complete_t complete_func,
>> + void *complete_data)
>> {
>> struct gatt_db_service *service;
>> uint16_t service_handle;
>> @@ -665,7 +667,8 @@ bool gatt_db_read(struct gatt_db *db, uint16_t handle, uint16_t offset,
>> if (a->read_func) {
>> *value = NULL;
>> *length = -1;
>> - a->read_func(handle, offset, att_opcode, bdaddr, a->user_data);
>> + a->read_func(handle, offset, att_opcode, bdaddr, complete_func,
>> + complete_data, a->user_data);
>> } else {
>> if (offset > a->value_len)
>> return false;
>> diff --git a/src/shared/gatt-db.h b/src/shared/gatt-db.h
>> index 8d18434..1c8739e 100644
>> --- a/src/shared/gatt-db.h
>> +++ b/src/shared/gatt-db.h
>> @@ -30,8 +30,13 @@ uint16_t gatt_db_add_service(struct gatt_db *db, const bt_uuid_t *uuid,
>> bool primary, uint16_t num_handles);
>> bool gatt_db_remove_service(struct gatt_db *db, uint16_t handle);
>>
>> +typedef void (*gatt_db_read_complete_t)(uint16_t handle, uint16_t att_ecode,
>> + const uint8_t *value, size_t len,
>> + void *complete_data);
>> typedef void (*gatt_db_read_t) (uint16_t handle, uint16_t offset,
>> uint8_t att_opcode, bdaddr_t *bdaddr,
>> + gatt_db_read_complete_t complete_func,
>> + void *complete_data,
>> void *user_data);
>>
>> typedef void (*gatt_db_write_t) (uint16_t handle, uint16_t offset,
>> @@ -81,7 +86,9 @@ void gatt_db_find_information(struct gatt_db *db, uint16_t start_handle,
>>
>> bool gatt_db_read(struct gatt_db *db, uint16_t handle, uint16_t offset,
>> uint8_t att_opcode, bdaddr_t *bdaddr,
>> - uint8_t **value, int *length);
>> + uint8_t **value, int *length,
>> + gatt_db_read_complete_t complete_func,
>> + void *complete_data);
>
> Lets remove value and length here and make it always return via
> callback, we can probably use a idle callback if there is a problem to
> return the values immediately.
>
I agree with this, since this'll get rid of a lot duplicate code, and
we can probably get away with not using an idle callback initially.
One problem though is that the android code makes use of these
functions and I don't have an Android build or devices set up to test
this. So would you mind taking on this task (or someone that has an
android build), change the gatt_db_read signature and make sure all
the android code works with passing unit tests, and then I'll rebase
my gatt-server patches on top of it?
Or if we don't want to hold back on my current patch set, you can
merge them and then do the refactor. Up to you.
>> bool gatt_db_write(struct gatt_db *db, uint16_t handle, uint16_t offset,
>> const uint8_t *value, size_t len,
>> diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
>> index 657b564..3233b21 100644
>> --- a/src/shared/gatt-server.c
>> +++ b/src/shared/gatt-server.c
>> @@ -114,8 +114,8 @@ static bool encode_read_by_grp_type_rsp(struct gatt_db *db, struct queue *q,
>> */
>> if (!gatt_db_read(db, start_handle, 0,
>> BT_ATT_OP_READ_BY_GRP_TYPE_REQ,
>> - NULL, &value,
>> - &value_len) || value_len < 0)
>> + NULL, &value, &value_len,
>> + NULL, NULL) || value_len < 0)
>> return false;
>>
>> /*
>> --
>> 2.1.0.rc2.206.gedb03e5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Luiz Augusto von Dentz
Cheers,
Arman
Hi Arman,
On Tue, Oct 21, 2014 at 12:00 AM, Arman Uguray <[email protected]> wrote:
> This patch introduces a completion callback parameter to gatt_db_read,
> which is meant to be used by a gatt_db_read_t implementation to signal
> the end of an asynchronous read operation performed in the upper layer.
> ---
> android/gatt.c | 21 ++++++++++++++++++---
> src/shared/gatt-db.c | 7 +++++--
> src/shared/gatt-db.h | 9 ++++++++-
> src/shared/gatt-server.c | 4 ++--
> 4 files changed, 33 insertions(+), 8 deletions(-)
>
> diff --git a/android/gatt.c b/android/gatt.c
> index ea20941..e2aa686 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -4694,7 +4694,8 @@ static void read_requested_attributes(void *data, void *user_data)
> resp_data->offset,
> process_data->opcode,
> &process_data->device->bdaddr,
> - &value, &value_len))
> + &value, &value_len,
> + NULL, NULL))
> error = ATT_ECODE_UNLIKELY;
>
> /* We have value here already if no callback will be called */
> @@ -4744,7 +4745,10 @@ static struct pending_trans_data *conn_add_transact(struct app_connection *conn,
> }
>
> static void read_cb(uint16_t handle, uint16_t offset, uint8_t att_opcode,
> - bdaddr_t *bdaddr, void *user_data)
> + bdaddr_t *bdaddr,
> + gatt_db_read_complete_t complete_func,
> + void *complete_data,
> + void *user_data)
> {
> struct pending_trans_data *transaction;
> struct hal_ev_gatt_server_request_read ev;
> @@ -6266,7 +6270,10 @@ static struct gap_srvc_handles gap_srvc_data;
> #define PERIPHERAL_PRIVACY_DISABLE 0x00
>
> static void gap_read_cb(uint16_t handle, uint16_t offset, uint8_t att_opcode,
> - bdaddr_t *bdaddr, void *user_data)
> + bdaddr_t *bdaddr,
> + gatt_db_read_complete_t complete_func,
> + void *complete_data,
> + void *user_data)
> {
> struct pending_request *entry;
> struct gatt_device *dev;
> @@ -6373,6 +6380,8 @@ static void register_gap_service(void)
>
> static void device_info_read_cb(uint16_t handle, uint16_t offset,
> uint8_t att_opcode, bdaddr_t *bdaddr,
> + gatt_db_read_complete_t complete_func,
> + void *complete_data,
> void *user_data)
> {
> struct pending_request *entry;
> @@ -6406,6 +6415,8 @@ done:
>
> static void device_info_read_system_id_cb(uint16_t handle, uint16_t offset,
> uint8_t att_opcode, bdaddr_t *bdaddr,
> + gatt_db_read_complete_t complete_func,
> + void *complete_data,
> void *user_data)
> {
> struct pending_request *entry;
> @@ -6438,6 +6449,8 @@ done:
>
> static void device_info_read_pnp_id_cb(uint16_t handle, uint16_t offset,
> uint8_t att_opcode, bdaddr_t *bdaddr,
> + gatt_db_read_complete_t complete_func,
> + void *complete_data,
> void *user_data)
> {
> struct pending_request *entry;
> @@ -6602,6 +6615,8 @@ static void gatt_srvc_change_write_cb(uint16_t handle, uint16_t offset,
>
> static void gatt_srvc_change_read_cb(uint16_t handle, uint16_t offset,
> uint8_t att_opcode, bdaddr_t *bdaddr,
> + gatt_db_read_complete_t complete_func,
> + void *complete_data,
> void *user_data)
> {
> struct pending_request *entry;
> diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
> index b3f95d2..c342e32 100644
> --- a/src/shared/gatt-db.c
> +++ b/src/shared/gatt-db.c
> @@ -638,7 +638,9 @@ static bool find_service_for_handle(const void *data, const void *user_data)
>
> bool gatt_db_read(struct gatt_db *db, uint16_t handle, uint16_t offset,
> uint8_t att_opcode, bdaddr_t *bdaddr,
> - uint8_t **value, int *length)
> + uint8_t **value, int *length,
> + gatt_db_read_complete_t complete_func,
> + void *complete_data)
> {
> struct gatt_db_service *service;
> uint16_t service_handle;
> @@ -665,7 +667,8 @@ bool gatt_db_read(struct gatt_db *db, uint16_t handle, uint16_t offset,
> if (a->read_func) {
> *value = NULL;
> *length = -1;
> - a->read_func(handle, offset, att_opcode, bdaddr, a->user_data);
> + a->read_func(handle, offset, att_opcode, bdaddr, complete_func,
> + complete_data, a->user_data);
> } else {
> if (offset > a->value_len)
> return false;
> diff --git a/src/shared/gatt-db.h b/src/shared/gatt-db.h
> index 8d18434..1c8739e 100644
> --- a/src/shared/gatt-db.h
> +++ b/src/shared/gatt-db.h
> @@ -30,8 +30,13 @@ uint16_t gatt_db_add_service(struct gatt_db *db, const bt_uuid_t *uuid,
> bool primary, uint16_t num_handles);
> bool gatt_db_remove_service(struct gatt_db *db, uint16_t handle);
>
> +typedef void (*gatt_db_read_complete_t)(uint16_t handle, uint16_t att_ecode,
> + const uint8_t *value, size_t len,
> + void *complete_data);
> typedef void (*gatt_db_read_t) (uint16_t handle, uint16_t offset,
> uint8_t att_opcode, bdaddr_t *bdaddr,
> + gatt_db_read_complete_t complete_func,
> + void *complete_data,
> void *user_data);
>
> typedef void (*gatt_db_write_t) (uint16_t handle, uint16_t offset,
> @@ -81,7 +86,9 @@ void gatt_db_find_information(struct gatt_db *db, uint16_t start_handle,
>
> bool gatt_db_read(struct gatt_db *db, uint16_t handle, uint16_t offset,
> uint8_t att_opcode, bdaddr_t *bdaddr,
> - uint8_t **value, int *length);
> + uint8_t **value, int *length,
> + gatt_db_read_complete_t complete_func,
> + void *complete_data);
Lets remove value and length here and make it always return via
callback, we can probably use a idle callback if there is a problem to
return the values immediately.
> bool gatt_db_write(struct gatt_db *db, uint16_t handle, uint16_t offset,
> const uint8_t *value, size_t len,
> diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
> index 657b564..3233b21 100644
> --- a/src/shared/gatt-server.c
> +++ b/src/shared/gatt-server.c
> @@ -114,8 +114,8 @@ static bool encode_read_by_grp_type_rsp(struct gatt_db *db, struct queue *q,
> */
> if (!gatt_db_read(db, start_handle, 0,
> BT_ATT_OP_READ_BY_GRP_TYPE_REQ,
> - NULL, &value,
> - &value_len) || value_len < 0)
> + NULL, &value, &value_len,
> + NULL, NULL) || value_len < 0)
> return false;
>
> /*
> --
> 2.1.0.rc2.206.gedb03e5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Luiz Augusto von Dentz
Hi Arman,
On Tue, Oct 21, 2014 at 12:00 AM, Arman Uguray <[email protected]> wrote:
> This patch implements the ATT protocol "Read By Type" request for
> shared/gatt-server. Logic is implemented that allows asynchronous
> reading of non-standard attribute values via the registered read and
> read completion callbacks.
> ---
> src/shared/gatt-server.c | 273 ++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 270 insertions(+), 3 deletions(-)
>
> diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
> index 3233b21..6c5ea2b 100644
> --- a/src/shared/gatt-server.c
> +++ b/src/shared/gatt-server.c
> @@ -38,6 +38,16 @@
> #define MIN(a, b) ((a) < (b) ? (a) : (b))
> #endif
>
> +struct async_read_op {
> + struct bt_gatt_server *server;
> + uint8_t opcode;
> + bool done;
> + uint8_t *pdu;
> + size_t pdu_len;
> + int value_len;
> + struct queue *db_data;
> +};
> +
> struct bt_gatt_server {
> struct gatt_db *db;
> struct bt_att *att;
> @@ -46,6 +56,9 @@ struct bt_gatt_server {
>
> unsigned int mtu_id;
> unsigned int read_by_grp_type_id;
> + unsigned int read_by_type_id;
> +
> + struct async_read_op *pending_read_op;
>
> bt_gatt_server_debug_func_t debug_callback;
> bt_gatt_server_destroy_func_t debug_destroy;
> @@ -59,8 +72,12 @@ static void bt_gatt_server_free(struct bt_gatt_server *server)
>
> bt_att_unregister(server->att, server->mtu_id);
> bt_att_unregister(server->att, server->read_by_grp_type_id);
> + bt_att_unregister(server->att, server->read_by_type_id);
> bt_att_unref(server->att);
>
> + if (server->pending_read_op)
> + server->pending_read_op->server = NULL;
> +
> free(server);
> }
>
> @@ -124,21 +141,21 @@ static bool encode_read_by_grp_type_rsp(struct gatt_db *db, struct queue *q,
> * value is seen.
> */
> if (iter == 0) {
> - data_val_len = value_len;
> + data_val_len = MIN(MIN(mtu - 6, 251), value_len);
> pdu[0] = data_val_len + 4;
> iter++;
> } else if (value_len != data_val_len)
> break;
>
> /* Stop if this unit would surpass the MTU */
> - if (iter + data_val_len + 4 > mtu)
> + if (iter + data_val_len + 4 > mtu - 1)
> break;
>
> end_handle = gatt_db_get_end_handle(db, start_handle);
>
> put_le16(start_handle, pdu + iter);
> put_le16(end_handle, pdu + iter + 2);
> - memcpy(pdu + iter + 4, value, value_len);
> + memcpy(pdu + iter + 4, value, data_val_len);
>
> iter += data_val_len + 4;
> }
> @@ -235,6 +252,248 @@ done:
> NULL, NULL, NULL);
> }
>
> +static void async_read_op_destroy(struct async_read_op *op)
> +{
> + if (op->server)
> + op->server->pending_read_op = NULL;
> +
> + queue_destroy(op->db_data, NULL);
> + free(op->pdu);
> + free(op);
> +}
> +
> +static void process_read_by_type(struct async_read_op *op);
> +
> +static void read_by_type_read_complete_cb(uint16_t handle, uint16_t att_ecode,
> + const uint8_t *value, size_t len,
> + void *complete_data)
> +{
> + struct async_read_op *op = complete_data;
> + struct bt_gatt_server *server = op->server;
> + uint16_t mtu;
> +
> + if (!server) {
> + async_read_op_destroy(op);
> + return;
> + }
> +
> + mtu = bt_att_get_mtu(server->att);
> +
> + /* Terminate the operation if there was an error */
> + if (att_ecode) {
> + uint8_t pdu[4];
> +
> + encode_error_rsp(BT_ATT_OP_READ_BY_TYPE_REQ, handle, att_ecode,
> + pdu);
> + bt_att_send(server->att, BT_ATT_OP_ERROR_RSP, pdu, 4, NULL,
> + NULL, NULL);
> + async_read_op_destroy(op);
> + return;
> + }
> +
> + if (op->pdu_len == 0) {
> + op->value_len = MIN(MIN(mtu - 4, 253), len);
> + op->pdu[0] = op->value_len + 2;
> + op->pdu_len++;
> + } else if (len != op->value_len) {
> + op->done = true;
> + goto done;
> + }
> +
> + /* Stop if this would surpass the MTU */
> + if (op->pdu_len + op->value_len + 2 > mtu - 1) {
> + op->done = true;
> + goto done;
> + }
> +
> + /* Encode the current value */
> + put_le16(handle, op->pdu + op->pdu_len);
> + memcpy(op->pdu + op->pdu_len + 2, value, op->value_len);
> +
> + op->pdu_len += op->value_len + 2;
> +
> + if (op->pdu_len == mtu - 1)
> + op->done = true;
The code above is duplicated and it is actually causing some warnings
comparing unsigned with signed (usually cause by '-' operation) which
I started fixing myself but stop once I realize this was the result of
gatt_db_read having 2 modes to read, sync and async, which I don't
think is a good idea.
> +done:
> + process_read_by_type(op);
> +}
> +
> +static void process_read_by_type(struct async_read_op *op)
> +{
> + struct bt_gatt_server *server = op->server;
> + uint16_t mtu = bt_att_get_mtu(server->att);
> + uint8_t rsp_opcode;
> + uint8_t rsp_len;
> + uint8_t ecode;
> + uint16_t ehandle;
> + uint16_t start_handle;
> + uint8_t *value;
> + int value_len;
> + uint32_t perm;
> +
> + if (op->done) {
> + rsp_opcode = BT_ATT_OP_READ_BY_TYPE_RSP;
> + rsp_len = op->pdu_len;
> + goto done;
> + }
> +
> + while (queue_peek_head(op->db_data)) {
> + start_handle = PTR_TO_UINT(queue_pop_head(op->db_data));
> + value = NULL;
> + value_len = 0;
> +
> + if (!gatt_db_get_attribute_permissions(server->db, start_handle,
> + &perm)) {
> + ecode = BT_ATT_ERROR_UNLIKELY;
> + goto error;
> + }
> +
> + /*
> + * Check for the READ access permission. Encryption,
> + * authentication, and authorization permissions need to be
> + * checked by the read handler, since bt_att is agnostic to
> + * connection type and doesn't have security information on it.
> + */
> + if (perm && !(perm & BT_ATT_PERM_READ)) {
> + ecode = BT_ATT_ERROR_READ_NOT_PERMITTED;
> + goto error;
> + }
I would suggest moving this check to gatt_db_read which should return
-EPERM or the actual code if it is too hard to have errno mapping for
each error code.
> + if (!gatt_db_read(server->db, start_handle, 0, op->opcode, NULL,
> + &value, &value_len,
> + read_by_type_read_complete_cb, op)) {
> + ecode = BT_ATT_ERROR_UNLIKELY;
> + goto error;
> + }
> +
> + /* The read has been deferred to the upper layer if |value_len|
> + * is less than 0.
> + */
> + if (value_len < 0)
> + return;
> +
> + if (op->pdu_len == 0) {
> + op->value_len = MIN(MIN(mtu - 4, 253), value_len);
> + op->pdu[0] = op->value_len + 2;
> + op->pdu_len++;
> + } else if (value_len != op->value_len)
> + break;
> +
> + /* Stop if this would surpass the MTU */
> + if (op->pdu_len + op->value_len + 2 > mtu - 1)
> + break;
> +
> + /* Encode the current value */
> + put_le16(start_handle, op->pdu + op->pdu_len);
> + memcpy(op->pdu + op->pdu_len + 2, value, op->value_len);
> +
> + op->pdu_len += op->value_len + 2;
> +
> + if (op->pdu_len == mtu - 1)
> + break;
> + }
> +
> + rsp_opcode = BT_ATT_OP_READ_BY_TYPE_RSP;
> + rsp_len = op->pdu_len;
> +
> + goto done;
> +
> +error:
> + rsp_opcode = BT_ATT_OP_ERROR_RSP;
> + rsp_len = 4;
> + encode_error_rsp(BT_ATT_OP_READ_BY_TYPE_REQ, ehandle, ecode, op->pdu);
> +
> +done:
> + bt_att_send(server->att, rsp_opcode, op->pdu, rsp_len, NULL,
> + NULL, NULL);
> + async_read_op_destroy(op);
> +}
> +
> +static void read_by_type_cb(uint8_t opcode, const void *pdu,
> + uint16_t length, void *user_data)
> +{
> + struct bt_gatt_server *server = user_data;
> + uint16_t start, end;
> + bt_uuid_t type;
> + uint8_t rsp_pdu[4];
> + uint16_t ehandle = 0;
> + uint8_t ecode;
> + struct queue *q = NULL;
> + struct async_read_op *op;
> +
> + if (length != 6 && length != 20) {
> + ecode = BT_ATT_ERROR_INVALID_PDU;
> + goto error;
> + }
> +
> + q = queue_new();
> + if (!q) {
> + ecode = BT_ATT_ERROR_INSUFFICIENT_RESOURCES;
> + goto error;
> + }
> +
> + start = get_le16(pdu);
> + end = get_le16(pdu + 2);
> + get_uuid_le(pdu + 4, length - 4, &type);
> +
> + util_debug(server->debug_callback, server->debug_data,
> + "Read By Type - start: 0x%04x end: 0x%04x",
> + start, end);
> +
> + if (!start || !end) {
> + ecode = BT_ATT_ERROR_INVALID_HANDLE;
> + goto error;
> + }
> +
> + ehandle = start;
> +
> + if (start > end) {
> + ecode = BT_ATT_ERROR_INVALID_HANDLE;
> + goto error;
> + }
> +
> + gatt_db_read_by_type(server->db, start, end, type, q);
> +
> + if (queue_isempty(q)) {
> + ecode = BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND;
> + goto error;
> + }
> +
> + if (server->pending_read_op) {
> + ecode = BT_ATT_ERROR_UNLIKELY;
> + goto error;
> + }
> +
> + op = new0(struct async_read_op, 1);
> + if (!op) {
> + ecode = BT_ATT_ERROR_INSUFFICIENT_RESOURCES;
> + goto error;
> + }
> +
> + op->pdu = malloc(bt_att_get_mtu(server->att));
> + if (!op->pdu) {
> + free(op);
> + ecode = BT_ATT_ERROR_INSUFFICIENT_RESOURCES;
> + goto error;
> + }
> +
> + op->opcode = opcode;
> + op->server = server;
> + op->db_data = q;
> + server->pending_read_op = op;
> +
> + process_read_by_type(op);
> +
> + return;
> +
> +error:
> + encode_error_rsp(opcode, ehandle, ecode, rsp_pdu);
> + queue_destroy(q, NULL);
> + bt_att_send(server->att, BT_ATT_OP_ERROR_RSP, rsp_pdu, 4,
> + NULL, NULL, NULL);
> +}
> +
> static void exchange_mtu_cb(uint8_t opcode, const void *pdu,
> uint16_t length, void *user_data)
> {
> @@ -283,6 +542,14 @@ static bool gatt_server_register_att_handlers(struct bt_gatt_server *server)
> if (!server->read_by_grp_type_id)
> return false;
>
> + /* Read By Type */
> + server->read_by_type_id = bt_att_register(server->att,
> + BT_ATT_OP_READ_BY_TYPE_REQ,
> + read_by_type_cb,
> + server, NULL);
> + if (!server->read_by_type_id)
> + return false;
> +
> return true;
> }
>
> --
> 2.1.0.rc2.206.gedb03e5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Luiz Augusto von Dentz
Hi Luiz,
On Wed, Oct 22, 2014 at 8:40 AM, Arman Uguray <[email protected]> wrote:
> Hi Luiz,
>
>
> On Wed, Oct 22, 2014 at 7:39 AM, Luiz Augusto von Dentz
> <[email protected]> wrote:
>> Hi Arman,
>>
>> On Tue, Oct 21, 2014 at 12:00 AM, Arman Uguray <[email protected]> wrote:
>>> bt_att_cancel and bt_att_cancel_all no longer destroy any pending
>>> request or indication, since this would cause a later bt_att_send to
>>> erroneously send out a request or indication before a
>>> response/confirmation for a pending one is received. Instead, they now
>>> keep the pending operations but clear their response and destroy
>>> callbacks since the upper layer is no longer interested in them.
>>> ---
>>> src/shared/att.c | 24 ++++++++++++++----------
>>> 1 file changed, 14 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/src/shared/att.c b/src/shared/att.c
>>> index 503e06c..c70d396 100644
>>> --- a/src/shared/att.c
>>> +++ b/src/shared/att.c
>>> @@ -1029,15 +1029,17 @@ bool bt_att_cancel(struct bt_att *att, unsigned int id)
>>> return false;
>>>
>>> if (att->pending_req && att->pending_req->id == id) {
>>> - op = att->pending_req;
>>> - att->pending_req = NULL;
>>> - goto done;
>>> + /* Don't cancel the pending request; remove it's handlers */
>>> + att->pending_req->callback = NULL;
>>> + att->pending_req->destroy = NULL;
>>> + return true;
>>> }
>>>
>>> if (att->pending_ind && att->pending_ind->id == id) {
>>> - op = att->pending_ind;
>>> - att->pending_ind = NULL;
>>> - goto done;
>>> + /* Don't cancel the pending indication; remove it's handlers */
>>> + att->pending_ind->callback = NULL;
>>> + att->pending_ind->destroy = NULL;
>>> + return true;
>>> }
>>>
>>> op = queue_remove_if(att->req_queue, match_op_id, UINT_TO_PTR(id));
>>> @@ -1073,13 +1075,15 @@ bool bt_att_cancel_all(struct bt_att *att)
>>> queue_remove_all(att->write_queue, NULL, NULL, destroy_att_send_op);
>>>
>>> if (att->pending_req) {
>>> - destroy_att_send_op(att->pending_req);
>>> - att->pending_req = NULL;
>>> + /* Don't cancel the pending request; remove it's handlers */
>>> + att->pending_req->callback = NULL;
>>> + att->pending_req->destroy = NULL;
>>> }
>>>
>>> if (att->pending_ind) {
>>> - destroy_att_send_op(att->pending_ind);
>>> - att->pending_ind = NULL;
>>> + /* Don't cancel the pending indication; remove it's handlers */
>>> + att->pending_ind->callback = NULL;
>>> + att->pending_ind->destroy = NULL;
>>> }
>>>
>>> return true;
>>> --
>>> 2.1.0.rc2.206.gedb03e5
>>
>> I would like to first finish with Marcin's patches, are you okay with
>> v5? Btw, please make sure you follow the HACKING document when
>> submitting patches so we can be consistent from now on.
>>
>>
>> --
>> Luiz Augusto von Dentz
>
> I'm fine with Marcin's v5, though we may have to wait for v6 since I
> saw some comments fly by from Szymon.
>
> Yeah I'm following HACKING from now on. So far I used the code around
> me (shared/mgmt, etc) as the style guide and might have repeated
> mistakes that already existed in the code, so it might makes sense to
> do a general style fix pass within src/shared at some point.
>
> -Arman
Looks like Marcin's patches have been merged so should we go ahead
with my patch set? Also, I'm adding TODO items for adding packed
structs for ATT PDUs and adding complete GATT unit test coverage in
unit/test-gatt in the next patch set. We should probably wait until we
have enough unit tests before we convert the code to use packed
structs to make sure that we don't break anything.
-Arman
Hi Luiz,
On Wed, Oct 22, 2014 at 7:39 AM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Arman,
>
> On Tue, Oct 21, 2014 at 12:00 AM, Arman Uguray <[email protected]> wrote:
>> bt_att_cancel and bt_att_cancel_all no longer destroy any pending
>> request or indication, since this would cause a later bt_att_send to
>> erroneously send out a request or indication before a
>> response/confirmation for a pending one is received. Instead, they now
>> keep the pending operations but clear their response and destroy
>> callbacks since the upper layer is no longer interested in them.
>> ---
>> src/shared/att.c | 24 ++++++++++++++----------
>> 1 file changed, 14 insertions(+), 10 deletions(-)
>>
>> diff --git a/src/shared/att.c b/src/shared/att.c
>> index 503e06c..c70d396 100644
>> --- a/src/shared/att.c
>> +++ b/src/shared/att.c
>> @@ -1029,15 +1029,17 @@ bool bt_att_cancel(struct bt_att *att, unsigned int id)
>> return false;
>>
>> if (att->pending_req && att->pending_req->id == id) {
>> - op = att->pending_req;
>> - att->pending_req = NULL;
>> - goto done;
>> + /* Don't cancel the pending request; remove it's handlers */
>> + att->pending_req->callback = NULL;
>> + att->pending_req->destroy = NULL;
>> + return true;
>> }
>>
>> if (att->pending_ind && att->pending_ind->id == id) {
>> - op = att->pending_ind;
>> - att->pending_ind = NULL;
>> - goto done;
>> + /* Don't cancel the pending indication; remove it's handlers */
>> + att->pending_ind->callback = NULL;
>> + att->pending_ind->destroy = NULL;
>> + return true;
>> }
>>
>> op = queue_remove_if(att->req_queue, match_op_id, UINT_TO_PTR(id));
>> @@ -1073,13 +1075,15 @@ bool bt_att_cancel_all(struct bt_att *att)
>> queue_remove_all(att->write_queue, NULL, NULL, destroy_att_send_op);
>>
>> if (att->pending_req) {
>> - destroy_att_send_op(att->pending_req);
>> - att->pending_req = NULL;
>> + /* Don't cancel the pending request; remove it's handlers */
>> + att->pending_req->callback = NULL;
>> + att->pending_req->destroy = NULL;
>> }
>>
>> if (att->pending_ind) {
>> - destroy_att_send_op(att->pending_ind);
>> - att->pending_ind = NULL;
>> + /* Don't cancel the pending indication; remove it's handlers */
>> + att->pending_ind->callback = NULL;
>> + att->pending_ind->destroy = NULL;
>> }
>>
>> return true;
>> --
>> 2.1.0.rc2.206.gedb03e5
>
> I would like to first finish with Marcin's patches, are you okay with
> v5? Btw, please make sure you follow the HACKING document when
> submitting patches so we can be consistent from now on.
>
>
> --
> Luiz Augusto von Dentz
I'm fine with Marcin's v5, though we may have to wait for v6 since I
saw some comments fly by from Szymon.
Yeah I'm following HACKING from now on. So far I used the code around
me (shared/mgmt, etc) as the style guide and might have repeated
mistakes that already existed in the code, so it might makes sense to
do a general style fix pass within src/shared at some point.
-Arman
Hi Arman,
On Tue, Oct 21, 2014 at 12:00 AM, Arman Uguray <[email protected]> wrote:
> bt_att_cancel and bt_att_cancel_all no longer destroy any pending
> request or indication, since this would cause a later bt_att_send to
> erroneously send out a request or indication before a
> response/confirmation for a pending one is received. Instead, they now
> keep the pending operations but clear their response and destroy
> callbacks since the upper layer is no longer interested in them.
> ---
> src/shared/att.c | 24 ++++++++++++++----------
> 1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/src/shared/att.c b/src/shared/att.c
> index 503e06c..c70d396 100644
> --- a/src/shared/att.c
> +++ b/src/shared/att.c
> @@ -1029,15 +1029,17 @@ bool bt_att_cancel(struct bt_att *att, unsigned int id)
> return false;
>
> if (att->pending_req && att->pending_req->id == id) {
> - op = att->pending_req;
> - att->pending_req = NULL;
> - goto done;
> + /* Don't cancel the pending request; remove it's handlers */
> + att->pending_req->callback = NULL;
> + att->pending_req->destroy = NULL;
> + return true;
> }
>
> if (att->pending_ind && att->pending_ind->id == id) {
> - op = att->pending_ind;
> - att->pending_ind = NULL;
> - goto done;
> + /* Don't cancel the pending indication; remove it's handlers */
> + att->pending_ind->callback = NULL;
> + att->pending_ind->destroy = NULL;
> + return true;
> }
>
> op = queue_remove_if(att->req_queue, match_op_id, UINT_TO_PTR(id));
> @@ -1073,13 +1075,15 @@ bool bt_att_cancel_all(struct bt_att *att)
> queue_remove_all(att->write_queue, NULL, NULL, destroy_att_send_op);
>
> if (att->pending_req) {
> - destroy_att_send_op(att->pending_req);
> - att->pending_req = NULL;
> + /* Don't cancel the pending request; remove it's handlers */
> + att->pending_req->callback = NULL;
> + att->pending_req->destroy = NULL;
> }
>
> if (att->pending_ind) {
> - destroy_att_send_op(att->pending_ind);
> - att->pending_ind = NULL;
> + /* Don't cancel the pending indication; remove it's handlers */
> + att->pending_ind->callback = NULL;
> + att->pending_ind->destroy = NULL;
> }
>
> return true;
> --
> 2.1.0.rc2.206.gedb03e5
I would like to first finish with Marcin's patches, are you okay with
v5? Btw, please make sure you follow the HACKING document when
submitting patches so we can be consistent from now on.
--
Luiz Augusto von Dentz
shared/gatt-server has been introduced, so removed that item. Added new
items for remaining tasks.
---
TODO | 35 +++++++++++++++++++++++++++++++++--
1 file changed, 33 insertions(+), 2 deletions(-)
diff --git a/TODO b/TODO
index efaec11..b09b047 100644
--- a/TODO
+++ b/TODO
@@ -149,12 +149,43 @@ ATT/GATT (new shared stack)
Priority: Medium
Complexity: C2
-- Introduce shared/gatt-server, which combined with shared/gatt-db, can be used
- as a GATT server implementation.
+- Implement GATT read and write procedures for shared/gatt-server. These map to
+ the following ATT protocol operations:
+
+ Read Request
+ Read Blob Request
+ Write Command
+ Write Request
+ Prepare Write Request
+ Execute Write Request
+
+ Priority: Medium
+ Complexity: C2
+
+- Implement server-initiated ATT protocol operations for shared/gatt-server:
+
+ Handle Value Notification
+ Handle Value Indication
Priority: Medium
Complexity: C2
+- Provide a tool for shared/gatt-server. This tool should demonstrate how a
+ shared/gatt-db can be used together with a shared/gatt-server to implement the
+ GATT server role. This should be written in a way so that it can be easily
+ used in conjunction with a remote instance of tools/btgatt-client (i.e. it
+ should listen for incoming connections, have similar verbose output, etc.)
+
+ Priority: Medium
+ Complexity: C2
+
+- Implement other low-priority ATT protocol operations for shared/gatt-server:
+
+ Read Multiple Request
+
+ Priority: Low
+ Complexity: C1
+
- Implement the server portion of doc/gatt-api.txt using shared/gatt-server once
it exists.
--
2.1.0.rc2.206.gedb03e5
This patch implements the "Find Information" request for the GATT
server role.
---
src/shared/gatt-server.c | 154 ++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 153 insertions(+), 1 deletion(-)
diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
index 6c5ea2b..1388db7 100644
--- a/src/shared/gatt-server.c
+++ b/src/shared/gatt-server.c
@@ -57,6 +57,7 @@ struct bt_gatt_server {
unsigned int mtu_id;
unsigned int read_by_grp_type_id;
unsigned int read_by_type_id;
+ unsigned int find_info_id;
struct async_read_op *pending_read_op;
@@ -73,11 +74,12 @@ static void bt_gatt_server_free(struct bt_gatt_server *server)
bt_att_unregister(server->att, server->mtu_id);
bt_att_unregister(server->att, server->read_by_grp_type_id);
bt_att_unregister(server->att, server->read_by_type_id);
- bt_att_unref(server->att);
+ bt_att_unregister(server->att, server->find_info_id);
if (server->pending_read_op)
server->pending_read_op->server = NULL;
+ bt_att_unref(server->att);
free(server);
}
@@ -494,6 +496,148 @@ error:
NULL, NULL, NULL);
}
+static void put_uuid_le(const bt_uuid_t *src, void *dst)
+{
+ bt_uuid_t uuid;
+
+ switch (src->type) {
+ case BT_UUID16:
+ put_le16(src->value.u16, dst);
+ break;
+ case BT_UUID128:
+ bswap_128(&src->value.u128, dst);
+ break;
+ case BT_UUID32:
+ bt_uuid_to_uuid128(src, &uuid);
+ bswap_128(&uuid.value.u128, dst);
+ break;
+ default:
+ break;
+ }
+}
+
+static bool encode_find_info_rsp(struct gatt_db *db, struct queue *q,
+ uint16_t mtu,
+ uint8_t *pdu, uint16_t *len)
+{
+ uint16_t handle;
+ const bt_uuid_t *type;
+ int uuid_len, cur_uuid_len;
+ int iter = 0;
+
+ *len = 0;
+
+ while (queue_peek_head(q)) {
+ handle = PTR_TO_UINT(queue_pop_head(q));
+ type = gatt_db_get_attribute_type(db, handle);
+ if (!type)
+ return false;
+
+ cur_uuid_len = bt_uuid_len(type);
+
+ if (iter == 0) {
+ switch (cur_uuid_len) {
+ case 2:
+ uuid_len = 2;
+ pdu[0] = 0x01;
+ break;
+ case 4:
+ case 16:
+ uuid_len = 16;
+ pdu[0] = 0x02;
+ break;
+ default:
+ return false;
+ }
+
+ iter++;
+ } else if (cur_uuid_len != uuid_len)
+ break;
+
+ if (iter + uuid_len + 2 > mtu - 1)
+ break;
+
+ put_le16(handle, pdu + iter);
+ put_uuid_le(type, pdu + iter + 2);
+
+ iter += uuid_len + 2;
+ }
+
+ *len = iter;
+
+ return true;
+}
+
+static void find_info_cb(uint8_t opcode, const void *pdu,
+ uint16_t length, void *user_data)
+{
+ struct bt_gatt_server *server = user_data;
+ uint16_t start, end;
+ uint16_t mtu = bt_att_get_mtu(server->att);
+ uint8_t rsp_pdu[mtu];
+ uint16_t rsp_len;
+ uint8_t rsp_opcode;
+ uint8_t ecode = 0;
+ uint16_t ehandle = 0;
+ struct queue *q = NULL;
+
+ if (length != 4) {
+ ecode = BT_ATT_ERROR_INVALID_PDU;
+ goto error;
+ }
+
+ q = queue_new();
+ if (!q) {
+ ecode = BT_ATT_ERROR_INSUFFICIENT_RESOURCES;
+ goto error;
+ }
+
+ start = get_le16(pdu);
+ end = get_le16(pdu + 2);
+
+ util_debug(server->debug_callback, server->debug_data,
+ "Find Info - start: 0x%04x end: 0x%04x",
+ start, end);
+
+ if (!start || !end) {
+ ecode = BT_ATT_ERROR_INVALID_HANDLE;
+ goto error;
+ }
+
+ ehandle = start;
+
+ if (start > end) {
+ ecode = BT_ATT_ERROR_INVALID_HANDLE;
+ goto error;
+ }
+
+ gatt_db_find_information(server->db, start, end, q);
+
+ if (queue_isempty(q)) {
+ ecode = BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND;
+ goto error;
+ }
+
+ if (!encode_find_info_rsp(server->db, q, mtu, rsp_pdu, &rsp_len)) {
+ ecode = BT_ATT_ERROR_UNLIKELY;
+ goto error;
+ }
+
+ rsp_opcode = BT_ATT_OP_FIND_INFO_RSP;
+
+ goto done;
+
+error:
+ rsp_opcode = BT_ATT_OP_ERROR_RSP;
+ rsp_len = 4;
+ encode_error_rsp(opcode, ehandle, ecode, rsp_pdu);
+
+done:
+ queue_destroy(q, NULL);
+ bt_att_send(server->att, rsp_opcode, rsp_pdu, rsp_len,
+ NULL, NULL, NULL);
+}
+
static void exchange_mtu_cb(uint8_t opcode, const void *pdu,
uint16_t length, void *user_data)
{
@@ -550,6 +694,14 @@ static bool gatt_server_register_att_handlers(struct bt_gatt_server *server)
if (!server->read_by_type_id)
return false;
+ /* Find Information */
+ server->find_info_id = bt_att_register(server->att,
+ BT_ATT_OP_FIND_INFO_REQ,
+ find_info_cb,
+ server, NULL);
+ if (!server->find_info_id)
+ return false;
+
return true;
}
--
2.1.0.rc2.206.gedb03e5
This patch introduces a completion callback parameter to gatt_db_read,
which is meant to be used by a gatt_db_read_t implementation to signal
the end of an asynchronous read operation performed in the upper layer.
---
android/gatt.c | 21 ++++++++++++++++++---
src/shared/gatt-db.c | 7 +++++--
src/shared/gatt-db.h | 9 ++++++++-
src/shared/gatt-server.c | 4 ++--
4 files changed, 33 insertions(+), 8 deletions(-)
diff --git a/android/gatt.c b/android/gatt.c
index ea20941..e2aa686 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -4694,7 +4694,8 @@ static void read_requested_attributes(void *data, void *user_data)
resp_data->offset,
process_data->opcode,
&process_data->device->bdaddr,
- &value, &value_len))
+ &value, &value_len,
+ NULL, NULL))
error = ATT_ECODE_UNLIKELY;
/* We have value here already if no callback will be called */
@@ -4744,7 +4745,10 @@ static struct pending_trans_data *conn_add_transact(struct app_connection *conn,
}
static void read_cb(uint16_t handle, uint16_t offset, uint8_t att_opcode,
- bdaddr_t *bdaddr, void *user_data)
+ bdaddr_t *bdaddr,
+ gatt_db_read_complete_t complete_func,
+ void *complete_data,
+ void *user_data)
{
struct pending_trans_data *transaction;
struct hal_ev_gatt_server_request_read ev;
@@ -6266,7 +6270,10 @@ static struct gap_srvc_handles gap_srvc_data;
#define PERIPHERAL_PRIVACY_DISABLE 0x00
static void gap_read_cb(uint16_t handle, uint16_t offset, uint8_t att_opcode,
- bdaddr_t *bdaddr, void *user_data)
+ bdaddr_t *bdaddr,
+ gatt_db_read_complete_t complete_func,
+ void *complete_data,
+ void *user_data)
{
struct pending_request *entry;
struct gatt_device *dev;
@@ -6373,6 +6380,8 @@ static void register_gap_service(void)
static void device_info_read_cb(uint16_t handle, uint16_t offset,
uint8_t att_opcode, bdaddr_t *bdaddr,
+ gatt_db_read_complete_t complete_func,
+ void *complete_data,
void *user_data)
{
struct pending_request *entry;
@@ -6406,6 +6415,8 @@ done:
static void device_info_read_system_id_cb(uint16_t handle, uint16_t offset,
uint8_t att_opcode, bdaddr_t *bdaddr,
+ gatt_db_read_complete_t complete_func,
+ void *complete_data,
void *user_data)
{
struct pending_request *entry;
@@ -6438,6 +6449,8 @@ done:
static void device_info_read_pnp_id_cb(uint16_t handle, uint16_t offset,
uint8_t att_opcode, bdaddr_t *bdaddr,
+ gatt_db_read_complete_t complete_func,
+ void *complete_data,
void *user_data)
{
struct pending_request *entry;
@@ -6602,6 +6615,8 @@ static void gatt_srvc_change_write_cb(uint16_t handle, uint16_t offset,
static void gatt_srvc_change_read_cb(uint16_t handle, uint16_t offset,
uint8_t att_opcode, bdaddr_t *bdaddr,
+ gatt_db_read_complete_t complete_func,
+ void *complete_data,
void *user_data)
{
struct pending_request *entry;
diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
index b3f95d2..c342e32 100644
--- a/src/shared/gatt-db.c
+++ b/src/shared/gatt-db.c
@@ -638,7 +638,9 @@ static bool find_service_for_handle(const void *data, const void *user_data)
bool gatt_db_read(struct gatt_db *db, uint16_t handle, uint16_t offset,
uint8_t att_opcode, bdaddr_t *bdaddr,
- uint8_t **value, int *length)
+ uint8_t **value, int *length,
+ gatt_db_read_complete_t complete_func,
+ void *complete_data)
{
struct gatt_db_service *service;
uint16_t service_handle;
@@ -665,7 +667,8 @@ bool gatt_db_read(struct gatt_db *db, uint16_t handle, uint16_t offset,
if (a->read_func) {
*value = NULL;
*length = -1;
- a->read_func(handle, offset, att_opcode, bdaddr, a->user_data);
+ a->read_func(handle, offset, att_opcode, bdaddr, complete_func,
+ complete_data, a->user_data);
} else {
if (offset > a->value_len)
return false;
diff --git a/src/shared/gatt-db.h b/src/shared/gatt-db.h
index 8d18434..1c8739e 100644
--- a/src/shared/gatt-db.h
+++ b/src/shared/gatt-db.h
@@ -30,8 +30,13 @@ uint16_t gatt_db_add_service(struct gatt_db *db, const bt_uuid_t *uuid,
bool primary, uint16_t num_handles);
bool gatt_db_remove_service(struct gatt_db *db, uint16_t handle);
+typedef void (*gatt_db_read_complete_t)(uint16_t handle, uint16_t att_ecode,
+ const uint8_t *value, size_t len,
+ void *complete_data);
typedef void (*gatt_db_read_t) (uint16_t handle, uint16_t offset,
uint8_t att_opcode, bdaddr_t *bdaddr,
+ gatt_db_read_complete_t complete_func,
+ void *complete_data,
void *user_data);
typedef void (*gatt_db_write_t) (uint16_t handle, uint16_t offset,
@@ -81,7 +86,9 @@ void gatt_db_find_information(struct gatt_db *db, uint16_t start_handle,
bool gatt_db_read(struct gatt_db *db, uint16_t handle, uint16_t offset,
uint8_t att_opcode, bdaddr_t *bdaddr,
- uint8_t **value, int *length);
+ uint8_t **value, int *length,
+ gatt_db_read_complete_t complete_func,
+ void *complete_data);
bool gatt_db_write(struct gatt_db *db, uint16_t handle, uint16_t offset,
const uint8_t *value, size_t len,
diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
index 657b564..3233b21 100644
--- a/src/shared/gatt-server.c
+++ b/src/shared/gatt-server.c
@@ -114,8 +114,8 @@ static bool encode_read_by_grp_type_rsp(struct gatt_db *db, struct queue *q,
*/
if (!gatt_db_read(db, start_handle, 0,
BT_ATT_OP_READ_BY_GRP_TYPE_REQ,
- NULL, &value,
- &value_len) || value_len < 0)
+ NULL, &value, &value_len,
+ NULL, NULL) || value_len < 0)
return false;
/*
--
2.1.0.rc2.206.gedb03e5
This patch introduces a completion callback parameter to gatt_db_write,
which is meant to be used by a gatt_db_write_t implementation to signal
the end of an asynchronous read operation performed in the upper layer.
---
android/gatt.c | 21 +++++++++++++--------
src/shared/gatt-db.c | 7 +++++--
src/shared/gatt-db.h | 8 +++++++-
3 files changed, 25 insertions(+), 11 deletions(-)
diff --git a/android/gatt.c b/android/gatt.c
index e2aa686..80ba728 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -4799,6 +4799,8 @@ failed:
static void write_cb(uint16_t handle, uint16_t offset,
const uint8_t *value, size_t len,
uint8_t att_opcode, bdaddr_t *bdaddr,
+ gatt_db_write_complete_t complete_func,
+ void *complete_data,
void *user_data)
{
uint8_t buf[IPC_MTU];
@@ -5880,7 +5882,8 @@ static void write_cmd_request(const uint8_t *cmd, uint16_t cmd_len,
if (check_device_permissions(dev, cmd[0], permissions))
return;
- gatt_db_write(gatt_db, handle, 0, value, vlen, cmd[0], &dev->bdaddr);
+ gatt_db_write(gatt_db, handle, 0, value, vlen, cmd[0], &dev->bdaddr,
+ NULL, NULL);
}
static void write_signed_cmd_request(const uint8_t *cmd, uint16_t cmd_len,
@@ -5948,7 +5951,7 @@ static void write_signed_cmd_request(const uint8_t *cmd, uint16_t cmd_len,
/* Signature OK, proceed with write */
bt_update_sign_counter(&dev->bdaddr, REMOTE_CSRK, r_sign_cnt);
gatt_db_write(gatt_db, handle, 0, value, vlen, cmd[0],
- &dev->bdaddr);
+ &dev->bdaddr, NULL, NULL);
}
}
@@ -5990,7 +5993,7 @@ static uint8_t write_req_request(const uint8_t *cmd, uint16_t cmd_len,
}
if (!gatt_db_write(gatt_db, handle, 0, value, vlen, cmd[0],
- &dev->bdaddr)) {
+ &dev->bdaddr, NULL, NULL)) {
queue_remove(dev->pending_requests, data);
free(data);
return ATT_ECODE_UNLIKELY;
@@ -6042,7 +6045,7 @@ static uint8_t write_prep_request(const uint8_t *cmd, uint16_t cmd_len,
}
if (!gatt_db_write(gatt_db, handle, offset, value, vlen, cmd[0],
- &dev->bdaddr))
+ &dev->bdaddr, NULL, NULL))
return ATT_ECODE_UNLIKELY;
return 0;
@@ -6583,10 +6586,12 @@ static void register_device_info_service(void)
}
static void gatt_srvc_change_write_cb(uint16_t handle, uint16_t offset,
- const uint8_t *val, size_t len,
- uint8_t att_opcode,
- bdaddr_t *bdaddr,
- void *user_data)
+ const uint8_t *val, size_t len,
+ uint8_t att_opcode,
+ bdaddr_t *bdaddr,
+ gatt_db_write_complete_t complete_func,
+ void *complete_data,
+ void *user_data)
{
struct pending_request *entry;
struct gatt_device *dev;
diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
index c342e32..afb0c75 100644
--- a/src/shared/gatt-db.c
+++ b/src/shared/gatt-db.c
@@ -682,7 +682,9 @@ bool gatt_db_read(struct gatt_db *db, uint16_t handle, uint16_t offset,
bool gatt_db_write(struct gatt_db *db, uint16_t handle, uint16_t offset,
const uint8_t *value, size_t len,
- uint8_t att_opcode, bdaddr_t *bdaddr)
+ uint8_t att_opcode, bdaddr_t *bdaddr,
+ gatt_db_write_complete_t complete_func,
+ void *complete_data)
{
struct gatt_db_service *service;
uint16_t service_handle;
@@ -700,7 +702,8 @@ bool gatt_db_write(struct gatt_db *db, uint16_t handle, uint16_t offset,
return false;
a->write_func(handle, offset, value, len, att_opcode, bdaddr,
- a->user_data);
+ complete_func, complete_data,
+ a->user_data);
return true;
}
diff --git a/src/shared/gatt-db.h b/src/shared/gatt-db.h
index 1c8739e..c940b71 100644
--- a/src/shared/gatt-db.h
+++ b/src/shared/gatt-db.h
@@ -39,9 +39,13 @@ typedef void (*gatt_db_read_t) (uint16_t handle, uint16_t offset,
void *complete_data,
void *user_data);
+typedef void (*gatt_db_write_complete_t)(uint16_t handle, uint16_t att_ecode,
+ void *complete_data);
typedef void (*gatt_db_write_t) (uint16_t handle, uint16_t offset,
const uint8_t *value, size_t len,
uint8_t att_opcode, bdaddr_t *bdaddr,
+ gatt_db_write_complete_t complete_func,
+ void *complete_data,
void *user_data);
uint16_t gatt_db_add_characteristic(struct gatt_db *db, uint16_t handle,
@@ -92,7 +96,9 @@ bool gatt_db_read(struct gatt_db *db, uint16_t handle, uint16_t offset,
bool gatt_db_write(struct gatt_db *db, uint16_t handle, uint16_t offset,
const uint8_t *value, size_t len,
- uint8_t att_opcode, bdaddr_t *bdaddr);
+ uint8_t att_opcode, bdaddr_t *bdaddr,
+ gatt_db_write_complete_t complete_func,
+ void *complete_data);
const bt_uuid_t *gatt_db_get_attribute_type(struct gatt_db *db,
uint16_t handle);
--
2.1.0.rc2.206.gedb03e5
This patch moves GATT characteristic property macros from gatt-client to
att-types. att-types is a logical place for these since gatt-server will
make use of them as well.
---
src/shared/att-types.h | 14 ++++++++++++++
src/shared/gatt-client.h | 9 ---------
2 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/src/shared/att-types.h b/src/shared/att-types.h
index b85c969..b57a5f3 100644
--- a/src/shared/att-types.h
+++ b/src/shared/att-types.h
@@ -73,3 +73,17 @@
#define BT_ATT_ERROR_INSUFFICIENT_ENCRYPTION 0x0F
#define BT_ATT_ERROR_UNSUPPORTED_GROUP_TYPE 0x10
#define BT_ATT_ERROR_INSUFFICIENT_RESOURCES 0x11
+
+/* GATT Characteristic Properties Bitfield values */
+#define BT_GATT_CHRC_PROP_BROADCAST 0x01
+#define BT_GATT_CHRC_PROP_READ 0x02
+#define BT_GATT_CHRC_PROP_WRITE_WITHOUT_RESP 0x04
+#define BT_GATT_CHRC_PROP_WRITE 0x08
+#define BT_GATT_CHRC_PROP_NOTIFY 0x10
+#define BT_GATT_CHRC_PROP_INDICATE 0x20
+#define BT_GATT_CHRC_PROP_AUTH 0x40
+#define BT_GATT_CHRC_PROP_EXT_PROP 0x80
+
+/* GATT Characteristic Extended Properties Bitfield values */
+#define BT_GATT_CHRC_EXT_PROP_RELIABLE_WRITE 0x01
+#define BT_GATT_CHRC_EXT_PROP_WRITABLE_AUX 0x02
diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
index 6807f6b..6b8719f 100644
--- a/src/shared/gatt-client.h
+++ b/src/shared/gatt-client.h
@@ -27,15 +27,6 @@
#define BT_GATT_UUID_SIZE 16
-#define BT_GATT_CHRC_PROP_BROADCAST 0x01
-#define BT_GATT_CHRC_PROP_READ 0x02
-#define BT_GATT_CHRC_PROP_WRITE_WITHOUT_RESP 0x04
-#define BT_GATT_CHRC_PROP_WRITE 0x08
-#define BT_GATT_CHRC_PROP_NOTIFY 0x10
-#define BT_GATT_CHRC_PROP_INDICATE 0x20
-#define BT_GATT_CHRC_PROP_AUTH 0x40
-#define BT_GATT_CHRC_PROP_EXT_PROP 0x80
-
struct bt_gatt_client;
struct bt_gatt_client *bt_gatt_client_new(struct bt_att *att, uint16_t mtu);
--
2.1.0.rc2.206.gedb03e5
This patch implements the ATT protocol "Read By Type" request for
shared/gatt-server. Logic is implemented that allows asynchronous
reading of non-standard attribute values via the registered read and
read completion callbacks.
---
src/shared/gatt-server.c | 273 ++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 270 insertions(+), 3 deletions(-)
diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
index 3233b21..6c5ea2b 100644
--- a/src/shared/gatt-server.c
+++ b/src/shared/gatt-server.c
@@ -38,6 +38,16 @@
#define MIN(a, b) ((a) < (b) ? (a) : (b))
#endif
+struct async_read_op {
+ struct bt_gatt_server *server;
+ uint8_t opcode;
+ bool done;
+ uint8_t *pdu;
+ size_t pdu_len;
+ int value_len;
+ struct queue *db_data;
+};
+
struct bt_gatt_server {
struct gatt_db *db;
struct bt_att *att;
@@ -46,6 +56,9 @@ struct bt_gatt_server {
unsigned int mtu_id;
unsigned int read_by_grp_type_id;
+ unsigned int read_by_type_id;
+
+ struct async_read_op *pending_read_op;
bt_gatt_server_debug_func_t debug_callback;
bt_gatt_server_destroy_func_t debug_destroy;
@@ -59,8 +72,12 @@ static void bt_gatt_server_free(struct bt_gatt_server *server)
bt_att_unregister(server->att, server->mtu_id);
bt_att_unregister(server->att, server->read_by_grp_type_id);
+ bt_att_unregister(server->att, server->read_by_type_id);
bt_att_unref(server->att);
+ if (server->pending_read_op)
+ server->pending_read_op->server = NULL;
+
free(server);
}
@@ -124,21 +141,21 @@ static bool encode_read_by_grp_type_rsp(struct gatt_db *db, struct queue *q,
* value is seen.
*/
if (iter == 0) {
- data_val_len = value_len;
+ data_val_len = MIN(MIN(mtu - 6, 251), value_len);
pdu[0] = data_val_len + 4;
iter++;
} else if (value_len != data_val_len)
break;
/* Stop if this unit would surpass the MTU */
- if (iter + data_val_len + 4 > mtu)
+ if (iter + data_val_len + 4 > mtu - 1)
break;
end_handle = gatt_db_get_end_handle(db, start_handle);
put_le16(start_handle, pdu + iter);
put_le16(end_handle, pdu + iter + 2);
- memcpy(pdu + iter + 4, value, value_len);
+ memcpy(pdu + iter + 4, value, data_val_len);
iter += data_val_len + 4;
}
@@ -235,6 +252,248 @@ done:
NULL, NULL, NULL);
}
+static void async_read_op_destroy(struct async_read_op *op)
+{
+ if (op->server)
+ op->server->pending_read_op = NULL;
+
+ queue_destroy(op->db_data, NULL);
+ free(op->pdu);
+ free(op);
+}
+
+static void process_read_by_type(struct async_read_op *op);
+
+static void read_by_type_read_complete_cb(uint16_t handle, uint16_t att_ecode,
+ const uint8_t *value, size_t len,
+ void *complete_data)
+{
+ struct async_read_op *op = complete_data;
+ struct bt_gatt_server *server = op->server;
+ uint16_t mtu;
+
+ if (!server) {
+ async_read_op_destroy(op);
+ return;
+ }
+
+ mtu = bt_att_get_mtu(server->att);
+
+ /* Terminate the operation if there was an error */
+ if (att_ecode) {
+ uint8_t pdu[4];
+
+ encode_error_rsp(BT_ATT_OP_READ_BY_TYPE_REQ, handle, att_ecode,
+ pdu);
+ bt_att_send(server->att, BT_ATT_OP_ERROR_RSP, pdu, 4, NULL,
+ NULL, NULL);
+ async_read_op_destroy(op);
+ return;
+ }
+
+ if (op->pdu_len == 0) {
+ op->value_len = MIN(MIN(mtu - 4, 253), len);
+ op->pdu[0] = op->value_len + 2;
+ op->pdu_len++;
+ } else if (len != op->value_len) {
+ op->done = true;
+ goto done;
+ }
+
+ /* Stop if this would surpass the MTU */
+ if (op->pdu_len + op->value_len + 2 > mtu - 1) {
+ op->done = true;
+ goto done;
+ }
+
+ /* Encode the current value */
+ put_le16(handle, op->pdu + op->pdu_len);
+ memcpy(op->pdu + op->pdu_len + 2, value, op->value_len);
+
+ op->pdu_len += op->value_len + 2;
+
+ if (op->pdu_len == mtu - 1)
+ op->done = true;
+
+done:
+ process_read_by_type(op);
+}
+
+static void process_read_by_type(struct async_read_op *op)
+{
+ struct bt_gatt_server *server = op->server;
+ uint16_t mtu = bt_att_get_mtu(server->att);
+ uint8_t rsp_opcode;
+ uint8_t rsp_len;
+ uint8_t ecode;
+ uint16_t ehandle;
+ uint16_t start_handle;
+ uint8_t *value;
+ int value_len;
+ uint32_t perm;
+
+ if (op->done) {
+ rsp_opcode = BT_ATT_OP_READ_BY_TYPE_RSP;
+ rsp_len = op->pdu_len;
+ goto done;
+ }
+
+ while (queue_peek_head(op->db_data)) {
+ start_handle = PTR_TO_UINT(queue_pop_head(op->db_data));
+ value = NULL;
+ value_len = 0;
+
+ if (!gatt_db_get_attribute_permissions(server->db, start_handle,
+ &perm)) {
+ ecode = BT_ATT_ERROR_UNLIKELY;
+ goto error;
+ }
+
+ /*
+ * Check for the READ access permission. Encryption,
+ * authentication, and authorization permissions need to be
+ * checked by the read handler, since bt_att is agnostic to
+ * connection type and doesn't have security information on it.
+ */
+ if (perm && !(perm & BT_ATT_PERM_READ)) {
+ ecode = BT_ATT_ERROR_READ_NOT_PERMITTED;
+ goto error;
+ }
+
+ if (!gatt_db_read(server->db, start_handle, 0, op->opcode, NULL,
+ &value, &value_len,
+ read_by_type_read_complete_cb, op)) {
+ ecode = BT_ATT_ERROR_UNLIKELY;
+ goto error;
+ }
+
+ /* The read has been deferred to the upper layer if |value_len|
+ * is less than 0.
+ */
+ if (value_len < 0)
+ return;
+
+ if (op->pdu_len == 0) {
+ op->value_len = MIN(MIN(mtu - 4, 253), value_len);
+ op->pdu[0] = op->value_len + 2;
+ op->pdu_len++;
+ } else if (value_len != op->value_len)
+ break;
+
+ /* Stop if this would surpass the MTU */
+ if (op->pdu_len + op->value_len + 2 > mtu - 1)
+ break;
+
+ /* Encode the current value */
+ put_le16(start_handle, op->pdu + op->pdu_len);
+ memcpy(op->pdu + op->pdu_len + 2, value, op->value_len);
+
+ op->pdu_len += op->value_len + 2;
+
+ if (op->pdu_len == mtu - 1)
+ break;
+ }
+
+ rsp_opcode = BT_ATT_OP_READ_BY_TYPE_RSP;
+ rsp_len = op->pdu_len;
+
+ goto done;
+
+error:
+ rsp_opcode = BT_ATT_OP_ERROR_RSP;
+ rsp_len = 4;
+ encode_error_rsp(BT_ATT_OP_READ_BY_TYPE_REQ, ehandle, ecode, op->pdu);
+
+done:
+ bt_att_send(server->att, rsp_opcode, op->pdu, rsp_len, NULL,
+ NULL, NULL);
+ async_read_op_destroy(op);
+}
+
+static void read_by_type_cb(uint8_t opcode, const void *pdu,
+ uint16_t length, void *user_data)
+{
+ struct bt_gatt_server *server = user_data;
+ uint16_t start, end;
+ bt_uuid_t type;
+ uint8_t rsp_pdu[4];
+ uint16_t ehandle = 0;
+ uint8_t ecode;
+ struct queue *q = NULL;
+ struct async_read_op *op;
+
+ if (length != 6 && length != 20) {
+ ecode = BT_ATT_ERROR_INVALID_PDU;
+ goto error;
+ }
+
+ q = queue_new();
+ if (!q) {
+ ecode = BT_ATT_ERROR_INSUFFICIENT_RESOURCES;
+ goto error;
+ }
+
+ start = get_le16(pdu);
+ end = get_le16(pdu + 2);
+ get_uuid_le(pdu + 4, length - 4, &type);
+
+ util_debug(server->debug_callback, server->debug_data,
+ "Read By Type - start: 0x%04x end: 0x%04x",
+ start, end);
+
+ if (!start || !end) {
+ ecode = BT_ATT_ERROR_INVALID_HANDLE;
+ goto error;
+ }
+
+ ehandle = start;
+
+ if (start > end) {
+ ecode = BT_ATT_ERROR_INVALID_HANDLE;
+ goto error;
+ }
+
+ gatt_db_read_by_type(server->db, start, end, type, q);
+
+ if (queue_isempty(q)) {
+ ecode = BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND;
+ goto error;
+ }
+
+ if (server->pending_read_op) {
+ ecode = BT_ATT_ERROR_UNLIKELY;
+ goto error;
+ }
+
+ op = new0(struct async_read_op, 1);
+ if (!op) {
+ ecode = BT_ATT_ERROR_INSUFFICIENT_RESOURCES;
+ goto error;
+ }
+
+ op->pdu = malloc(bt_att_get_mtu(server->att));
+ if (!op->pdu) {
+ free(op);
+ ecode = BT_ATT_ERROR_INSUFFICIENT_RESOURCES;
+ goto error;
+ }
+
+ op->opcode = opcode;
+ op->server = server;
+ op->db_data = q;
+ server->pending_read_op = op;
+
+ process_read_by_type(op);
+
+ return;
+
+error:
+ encode_error_rsp(opcode, ehandle, ecode, rsp_pdu);
+ queue_destroy(q, NULL);
+ bt_att_send(server->att, BT_ATT_OP_ERROR_RSP, rsp_pdu, 4,
+ NULL, NULL, NULL);
+}
+
static void exchange_mtu_cb(uint8_t opcode, const void *pdu,
uint16_t length, void *user_data)
{
@@ -283,6 +542,14 @@ static bool gatt_server_register_att_handlers(struct bt_gatt_server *server)
if (!server->read_by_grp_type_id)
return false;
+ /* Read By Type */
+ server->read_by_type_id = bt_att_register(server->att,
+ BT_ATT_OP_READ_BY_TYPE_REQ,
+ read_by_type_cb,
+ server, NULL);
+ if (!server->read_by_type_id)
+ return false;
+
return true;
}
--
2.1.0.rc2.206.gedb03e5
GATT doesn't have a standard way of representing attribute permissions
but coming up with values that can be used consistently among code that
uses shared/att is valuable. This patch introduces macros to represent
attribute permissions in a bitfield, with specific bits assigned to
read and write access, encryption, authentication, and authorization
permissions.
---
src/shared/att-types.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/src/shared/att-types.h b/src/shared/att-types.h
index b57a5f3..a6b23e4 100644
--- a/src/shared/att-types.h
+++ b/src/shared/att-types.h
@@ -74,6 +74,18 @@
#define BT_ATT_ERROR_UNSUPPORTED_GROUP_TYPE 0x10
#define BT_ATT_ERROR_INSUFFICIENT_RESOURCES 0x11
+/*
+ * ATT attribute permission bitfield values. Permissions are grouped as
+ * "Access", "Encryption", "Authentication", and "Authorization". A bitmask of
+ * permissions is a byte that encodes a combination of these.
+ */
+#define BT_ATT_PERM_READ 0x01
+#define BT_ATT_PERM_WRITE 0x02
+#define BT_ATT_PERM_ENCRYPT 0x04
+#define BT_ATT_PERM_AUTHEN 0x08
+#define BT_ATT_PERM_AUTHOR 0x10
+#define BT_ATT_PERM_NONE 0x20
+
/* GATT Characteristic Properties Bitfield values */
#define BT_GATT_CHRC_PROP_BROADCAST 0x01
#define BT_GATT_CHRC_PROP_READ 0x02
--
2.1.0.rc2.206.gedb03e5
bt_att_cancel and bt_att_cancel_all no longer destroy any pending
request or indication, since this would cause a later bt_att_send to
erroneously send out a request or indication before a
response/confirmation for a pending one is received. Instead, they now
keep the pending operations but clear their response and destroy
callbacks since the upper layer is no longer interested in them.
---
src/shared/att.c | 24 ++++++++++++++----------
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/src/shared/att.c b/src/shared/att.c
index 503e06c..c70d396 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -1029,15 +1029,17 @@ bool bt_att_cancel(struct bt_att *att, unsigned int id)
return false;
if (att->pending_req && att->pending_req->id == id) {
- op = att->pending_req;
- att->pending_req = NULL;
- goto done;
+ /* Don't cancel the pending request; remove it's handlers */
+ att->pending_req->callback = NULL;
+ att->pending_req->destroy = NULL;
+ return true;
}
if (att->pending_ind && att->pending_ind->id == id) {
- op = att->pending_ind;
- att->pending_ind = NULL;
- goto done;
+ /* Don't cancel the pending indication; remove it's handlers */
+ att->pending_ind->callback = NULL;
+ att->pending_ind->destroy = NULL;
+ return true;
}
op = queue_remove_if(att->req_queue, match_op_id, UINT_TO_PTR(id));
@@ -1073,13 +1075,15 @@ bool bt_att_cancel_all(struct bt_att *att)
queue_remove_all(att->write_queue, NULL, NULL, destroy_att_send_op);
if (att->pending_req) {
- destroy_att_send_op(att->pending_req);
- att->pending_req = NULL;
+ /* Don't cancel the pending request; remove it's handlers */
+ att->pending_req->callback = NULL;
+ att->pending_req->destroy = NULL;
}
if (att->pending_ind) {
- destroy_att_send_op(att->pending_ind);
- att->pending_ind = NULL;
+ /* Don't cancel the pending indication; remove it's handlers */
+ att->pending_ind->callback = NULL;
+ att->pending_ind->destroy = NULL;
}
return true;
--
2.1.0.rc2.206.gedb03e5