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?
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
--
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
