Hi Arman,
On Fri, Oct 24, 2014 at 8:02 PM, Arman Uguray <armansito@xxxxxxxxxxxx> wrote:
> Hi Luiz,
>
>
> On Fri, Oct 24, 2014 at 2:31 AM, Luiz Augusto von Dentz
> <luiz.dentz@xxxxxxxxx> wrote:
>> Hi Arman,
>>
>> On Tue, Oct 21, 2014 at 12:00 AM, Arman Uguray <armansito@xxxxxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
>>
>> --
>> Luiz Augusto von Dentz
>
> Cheers,
> Arman
--
Luiz Augusto von Dentz
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
