From: Luiz Augusto von Dentz <[email protected]>
This adds a timeout (1 second) to attribute operations since these can
cause unexpected transport disconnections if they take too long to
complete.
---
src/shared/gatt-db.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 61 insertions(+), 5 deletions(-)
diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
index bab1202..a39eec2 100644
--- a/src/shared/gatt-db.c
+++ b/src/shared/gatt-db.c
@@ -22,14 +22,17 @@
*/
#include <stdbool.h>
+#include <errno.h>
#include "lib/uuid.h"
#include "src/shared/util.h"
#include "src/shared/queue.h"
+#include "src/shared/timeout.h"
#include "src/shared/gatt-db.h"
#define MAX_CHAR_DECL_VALUE_LEN 19
#define MAX_INCLUDED_VALUE_LEN 6
+#define ATTRIBUTE_TIMEOUT 1000
static const bt_uuid_t primary_service_uuid = { .type = BT_UUID16,
.value.u16 = GATT_PRIM_SVC_UUID };
@@ -46,13 +49,17 @@ struct gatt_db {
};
struct pending_read {
+ struct gatt_db_attribute *attrib;
unsigned int id;
+ unsigned int timeout_id;
gatt_db_attribute_read_t func;
void *user_data;
};
struct pending_write {
+ struct gatt_db_attribute *attrib;
unsigned int id;
+ unsigned int timeout_id;
gatt_db_attribute_write_t func;
void *user_data;
};
@@ -773,6 +780,30 @@ bool gatt_db_attribute_get_permissions(struct gatt_db_attribute *attrib,
return true;
}
+static void pending_read_result(struct pending_read *p, int err,
+ const uint8_t *data, size_t length)
+{
+ if (p->timeout_id > 0)
+ timeout_remove(p->timeout_id);
+
+ p->func(p->attrib, err, data, length, p->user_data);
+
+ free(p);
+}
+
+static bool read_timeout(void *user_data)
+{
+ struct pending_read *p = user_data;
+
+ p->timeout_id = 0;
+
+ queue_remove(p->attrib->pending_reads, p);
+
+ pending_read_result(p, -ETIMEDOUT, NULL, 0);
+
+ return false;
+}
+
bool gatt_db_attribute_read(struct gatt_db_attribute *attrib, uint16_t offset,
uint8_t opcode, bdaddr_t *bdaddr,
gatt_db_attribute_read_t func, void *user_data)
@@ -789,7 +820,10 @@ bool gatt_db_attribute_read(struct gatt_db_attribute *attrib, uint16_t offset,
if (!p)
return false;
+ p->attrib = attrib;
p->id = ++attrib->read_id;
+ p->timeout_id = timeout_add(ATTRIBUTE_TIMEOUT, read_timeout,
+ p, NULL);
p->func = func;
p->user_data = user_data;
@@ -834,11 +868,32 @@ bool gatt_db_attribute_read_result(struct gatt_db_attribute *attrib,
if (!p)
return false;
- p->func(attrib, err, value, length, p->user_data);
+ pending_read_result(p, err, value, length);
+
+ return true;
+}
+
+static void pending_write_result(struct pending_write *p, int err)
+{
+ if (p->timeout_id > 0)
+ timeout_remove(p->timeout_id);
+
+ p->func(p->attrib, err, p->user_data);
free(p);
+}
- return true;
+static bool write_timeout(void *user_data)
+{
+ struct pending_write *p = user_data;
+
+ p->timeout_id = 0;
+
+ queue_remove(p->attrib->pending_writes, p);
+
+ pending_write_result(p, -ETIMEDOUT);
+
+ return false;
}
bool gatt_db_attribute_write(struct gatt_db_attribute *attrib, uint16_t offset,
@@ -857,7 +912,10 @@ bool gatt_db_attribute_write(struct gatt_db_attribute *attrib, uint16_t offset,
if (!p)
return false;
+ p->attrib = attrib;
p->id = ++attrib->write_id;
+ p->timeout_id = timeout_add(ATTRIBUTE_TIMEOUT, write_timeout,
+ p, NULL);
p->func = func;
p->user_data = user_data;
@@ -900,9 +958,7 @@ bool gatt_db_attribute_write_result(struct gatt_db_attribute *attrib,
if (!p)
return false;
- p->func(attrib, err, p->user_data);
-
- free(p);
+ pending_write_result(p, err);
return true;
}
--
1.9.3
Hi Luiz,
> On Wed, Nov 12, 2014 at 1:42 AM, Luiz Augusto von Dentz <[email protected]> wrote:
> Hi Arman,
>
> On Tue, Nov 11, 2014 at 11:47 PM, Arman Uguray <[email protected]> wrote:
>> Hi Luiz,
>>
>>> On Tue, Nov 11, 2014 at 6:22 AM, Luiz Augusto von Dentz <[email protected]> wrote:
>>> From: Luiz Augusto von Dentz <[email protected]>
>>>
>>> This adds a timeout (1 second) to attribute operations since these can
>>> cause unexpected transport disconnections if they take too long to
>>> complete.
>>> ---
>>> src/shared/gatt-db.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++----
>>> 1 file changed, 61 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
>>> index bab1202..a39eec2 100644
>>> --- a/src/shared/gatt-db.c
>>> +++ b/src/shared/gatt-db.c
>>> @@ -22,14 +22,17 @@
>>> */
>>>
>>> #include <stdbool.h>
>>> +#include <errno.h>
>>>
>>> #include "lib/uuid.h"
>>> #include "src/shared/util.h"
>>> #include "src/shared/queue.h"
>>> +#include "src/shared/timeout.h"
>>> #include "src/shared/gatt-db.h"
>>>
>>> #define MAX_CHAR_DECL_VALUE_LEN 19
>>> #define MAX_INCLUDED_VALUE_LEN 6
>>> +#define ATTRIBUTE_TIMEOUT 1000
>>>
>>> static const bt_uuid_t primary_service_uuid = { .type = BT_UUID16,
>>> .value.u16 = GATT_PRIM_SVC_UUID };
>>> @@ -46,13 +49,17 @@ struct gatt_db {
>>> };
>>>
>>> struct pending_read {
>>> + struct gatt_db_attribute *attrib;
>>> unsigned int id;
>>> + unsigned int timeout_id;
>>> gatt_db_attribute_read_t func;
>>> void *user_data;
>>> };
>>>
>>> struct pending_write {
>>> + struct gatt_db_attribute *attrib;
>>> unsigned int id;
>>> + unsigned int timeout_id;
>>> gatt_db_attribute_write_t func;
>>> void *user_data;
>>> };
>>> @@ -773,6 +780,30 @@ bool gatt_db_attribute_get_permissions(struct gatt_db_attribute *attrib,
>>> return true;
>>> }
>>>
>>> +static void pending_read_result(struct pending_read *p, int err,
>>> + const uint8_t *data, size_t length)
>>> +{
>>> + if (p->timeout_id > 0)
>>> + timeout_remove(p->timeout_id);
>>> +
>>> + p->func(p->attrib, err, data, length, p->user_data);
>>> +
>>> + free(p);
>>> +}
>>> +
>>> +static bool read_timeout(void *user_data)
>>> +{
>>> + struct pending_read *p = user_data;
>>> +
>>> + p->timeout_id = 0;
>>> +
>>> + queue_remove(p->attrib->pending_reads, p);
>>> +
>>> + pending_read_result(p, -ETIMEDOUT, NULL, 0);
>>> +
>>> + return false;
>>> +}
>>> +
>>> bool gatt_db_attribute_read(struct gatt_db_attribute *attrib, uint16_t offset,
>>> uint8_t opcode, bdaddr_t *bdaddr,
>>> gatt_db_attribute_read_t func, void *user_data)
>>> @@ -789,7 +820,10 @@ bool gatt_db_attribute_read(struct gatt_db_attribute *attrib, uint16_t offset,
>>> if (!p)
>>> return false;
>>>
>>> + p->attrib = attrib;
>>> p->id = ++attrib->read_id;
>>> + p->timeout_id = timeout_add(ATTRIBUTE_TIMEOUT, read_timeout,
>>> + p, NULL);
>>> p->func = func;
>>> p->user_data = user_data;
>>>
>>> @@ -834,11 +868,32 @@ bool gatt_db_attribute_read_result(struct gatt_db_attribute *attrib,
>>> if (!p)
>>> return false;
>>>
>>> - p->func(attrib, err, value, length, p->user_data);
>>> + pending_read_result(p, err, value, length);
>>> +
>>> + return true;
>>> +}
>>> +
>>> +static void pending_write_result(struct pending_write *p, int err)
>>> +{
>>> + if (p->timeout_id > 0)
>>> + timeout_remove(p->timeout_id);
>>> +
>>> + p->func(p->attrib, err, p->user_data);
>>>
>>> free(p);
>>> +}
>>>
>>> - return true;
>>> +static bool write_timeout(void *user_data)
>>> +{
>>> + struct pending_write *p = user_data;
>>> +
>>> + p->timeout_id = 0;
>>> +
>>> + queue_remove(p->attrib->pending_writes, p);
>>> +
>>> + pending_write_result(p, -ETIMEDOUT);
>>> +
>>> + return false;
>>> }
>>>
>>> bool gatt_db_attribute_write(struct gatt_db_attribute *attrib, uint16_t offset,
>>> @@ -857,7 +912,10 @@ bool gatt_db_attribute_write(struct gatt_db_attribute *attrib, uint16_t offset,
>>> if (!p)
>>> return false;
>>>
>>> + p->attrib = attrib;
>>> p->id = ++attrib->write_id;
>>> + p->timeout_id = timeout_add(ATTRIBUTE_TIMEOUT, write_timeout,
>>> + p, NULL);
>>> p->func = func;
>>> p->user_data = user_data;
>>>
>>> @@ -900,9 +958,7 @@ bool gatt_db_attribute_write_result(struct gatt_db_attribute *attrib,
>>> if (!p)
>>> return false;
>>>
>>> - p->func(attrib, err, p->user_data);
>>> -
>>> - free(p);
>>> + pending_write_result(p, err);
>>>
>>> return true;
>>> }
>>> --
>>> 1.9.3
>>>
>>> --
>>> 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
>>
>> Overall I'm OK with this patch. One thing we need to figure out now
>> though is deciding how we should report an errno like -ETIMEDOUT in an
>> ATT Error Response. Right now I report 0xff if the error is negative
>> or larger than UINT8_MAX (see
>> src/shared/gatt-server.c:att_ecode_from_error), though we may want to
>> map this to a more reasonable ATT application error.
>
> Yes we need to come up with mapping between errno and ATT error code
> and I figure right now you are just using 0xff, in case of Android I
> was planning to use ATT_ECODE_TIMEOUT (0x81) but then I realize that
> in Android case the application themselves should be using this range
> so I guess we should stick with Unlikely Error. Btw 0xff actually
> means out of range according to
> https://www.bluetooth.org/DocMan/handlers/DownloadDoc.ashx?doc_id=282152
> so we might want to change it to something like this:
>
> if (!err || (err > 0 && err < UINT8_MAX))
> return err;
>
> switch (err) {
> case -ENOENT:
> return BT_ATT_ERROR_INVALID_HANDLE ;
> case -ENOMEM:
> return BT_ATT_ERROR_INSUFFICIENT_RESOURCES;
> case -EOVERFLOW:
> return 0xff;
> case -EALREADY:
> return 0xfe;
> ...
> default:
> return ATT_ECODE_UNLIKELY;
> }
>
> Anyway I would leave this to bt_gatt_* or bt_att to convert, actually
> maybe doing in the later is better and create dedicated API like e.g.
> bt_att_error_rsp which takes care of enconding, in fact I always
> preferred to do the encoding in bt_att.
>
OK, I'm including this in my next tools/btgatt-server patch set. I'm
adding a packed struct for error PDUs (with a TODO for adding the same
for others) and putting an encode function to att.h in the bt_att
namespace.
> --
> Luiz Augusto von Dentz
Cheers,
Arman
Hi,
On Tue, Nov 11, 2014 at 4:22 PM, Luiz Augusto von Dentz
<[email protected]> wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> This adds a timeout (1 second) to attribute operations since these can
> cause unexpected transport disconnections if they take too long to
> complete.
> ---
> src/shared/gatt-db.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 61 insertions(+), 5 deletions(-)
>
> diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
> index bab1202..a39eec2 100644
> --- a/src/shared/gatt-db.c
> +++ b/src/shared/gatt-db.c
> @@ -22,14 +22,17 @@
> */
>
> #include <stdbool.h>
> +#include <errno.h>
>
> #include "lib/uuid.h"
> #include "src/shared/util.h"
> #include "src/shared/queue.h"
> +#include "src/shared/timeout.h"
> #include "src/shared/gatt-db.h"
>
> #define MAX_CHAR_DECL_VALUE_LEN 19
> #define MAX_INCLUDED_VALUE_LEN 6
> +#define ATTRIBUTE_TIMEOUT 1000
>
> static const bt_uuid_t primary_service_uuid = { .type = BT_UUID16,
> .value.u16 = GATT_PRIM_SVC_UUID };
> @@ -46,13 +49,17 @@ struct gatt_db {
> };
>
> struct pending_read {
> + struct gatt_db_attribute *attrib;
> unsigned int id;
> + unsigned int timeout_id;
> gatt_db_attribute_read_t func;
> void *user_data;
> };
>
> struct pending_write {
> + struct gatt_db_attribute *attrib;
> unsigned int id;
> + unsigned int timeout_id;
> gatt_db_attribute_write_t func;
> void *user_data;
> };
> @@ -773,6 +780,30 @@ bool gatt_db_attribute_get_permissions(struct gatt_db_attribute *attrib,
> return true;
> }
>
> +static void pending_read_result(struct pending_read *p, int err,
> + const uint8_t *data, size_t length)
> +{
> + if (p->timeout_id > 0)
> + timeout_remove(p->timeout_id);
> +
> + p->func(p->attrib, err, data, length, p->user_data);
> +
> + free(p);
> +}
> +
> +static bool read_timeout(void *user_data)
> +{
> + struct pending_read *p = user_data;
> +
> + p->timeout_id = 0;
> +
> + queue_remove(p->attrib->pending_reads, p);
> +
> + pending_read_result(p, -ETIMEDOUT, NULL, 0);
> +
> + return false;
> +}
> +
> bool gatt_db_attribute_read(struct gatt_db_attribute *attrib, uint16_t offset,
> uint8_t opcode, bdaddr_t *bdaddr,
> gatt_db_attribute_read_t func, void *user_data)
> @@ -789,7 +820,10 @@ bool gatt_db_attribute_read(struct gatt_db_attribute *attrib, uint16_t offset,
> if (!p)
> return false;
>
> + p->attrib = attrib;
> p->id = ++attrib->read_id;
> + p->timeout_id = timeout_add(ATTRIBUTE_TIMEOUT, read_timeout,
> + p, NULL);
> p->func = func;
> p->user_data = user_data;
>
> @@ -834,11 +868,32 @@ bool gatt_db_attribute_read_result(struct gatt_db_attribute *attrib,
> if (!p)
> return false;
>
> - p->func(attrib, err, value, length, p->user_data);
> + pending_read_result(p, err, value, length);
> +
> + return true;
> +}
> +
> +static void pending_write_result(struct pending_write *p, int err)
> +{
> + if (p->timeout_id > 0)
> + timeout_remove(p->timeout_id);
> +
> + p->func(p->attrib, err, p->user_data);
>
> free(p);
> +}
>
> - return true;
> +static bool write_timeout(void *user_data)
> +{
> + struct pending_write *p = user_data;
> +
> + p->timeout_id = 0;
> +
> + queue_remove(p->attrib->pending_writes, p);
> +
> + pending_write_result(p, -ETIMEDOUT);
> +
> + return false;
> }
>
> bool gatt_db_attribute_write(struct gatt_db_attribute *attrib, uint16_t offset,
> @@ -857,7 +912,10 @@ bool gatt_db_attribute_write(struct gatt_db_attribute *attrib, uint16_t offset,
> if (!p)
> return false;
>
> + p->attrib = attrib;
> p->id = ++attrib->write_id;
> + p->timeout_id = timeout_add(ATTRIBUTE_TIMEOUT, write_timeout,
> + p, NULL);
> p->func = func;
> p->user_data = user_data;
>
> @@ -900,9 +958,7 @@ bool gatt_db_attribute_write_result(struct gatt_db_attribute *attrib,
> if (!p)
> return false;
>
> - p->func(attrib, err, p->user_data);
> -
> - free(p);
> + pending_write_result(p, err);
>
> return true;
> }
> --
> 1.9.3
Pushed.
--
Luiz Augusto von Dentz
Hi Arman,
On Tue, Nov 11, 2014 at 11:47 PM, Arman Uguray <[email protected]> wrote:
> Hi Luiz,
>
>> On Tue, Nov 11, 2014 at 6:22 AM, Luiz Augusto von Dentz <[email protected]> wrote:
>> From: Luiz Augusto von Dentz <[email protected]>
>>
>> This adds a timeout (1 second) to attribute operations since these can
>> cause unexpected transport disconnections if they take too long to
>> complete.
>> ---
>> src/shared/gatt-db.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++----
>> 1 file changed, 61 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
>> index bab1202..a39eec2 100644
>> --- a/src/shared/gatt-db.c
>> +++ b/src/shared/gatt-db.c
>> @@ -22,14 +22,17 @@
>> */
>>
>> #include <stdbool.h>
>> +#include <errno.h>
>>
>> #include "lib/uuid.h"
>> #include "src/shared/util.h"
>> #include "src/shared/queue.h"
>> +#include "src/shared/timeout.h"
>> #include "src/shared/gatt-db.h"
>>
>> #define MAX_CHAR_DECL_VALUE_LEN 19
>> #define MAX_INCLUDED_VALUE_LEN 6
>> +#define ATTRIBUTE_TIMEOUT 1000
>>
>> static const bt_uuid_t primary_service_uuid = { .type = BT_UUID16,
>> .value.u16 = GATT_PRIM_SVC_UUID };
>> @@ -46,13 +49,17 @@ struct gatt_db {
>> };
>>
>> struct pending_read {
>> + struct gatt_db_attribute *attrib;
>> unsigned int id;
>> + unsigned int timeout_id;
>> gatt_db_attribute_read_t func;
>> void *user_data;
>> };
>>
>> struct pending_write {
>> + struct gatt_db_attribute *attrib;
>> unsigned int id;
>> + unsigned int timeout_id;
>> gatt_db_attribute_write_t func;
>> void *user_data;
>> };
>> @@ -773,6 +780,30 @@ bool gatt_db_attribute_get_permissions(struct gatt_db_attribute *attrib,
>> return true;
>> }
>>
>> +static void pending_read_result(struct pending_read *p, int err,
>> + const uint8_t *data, size_t length)
>> +{
>> + if (p->timeout_id > 0)
>> + timeout_remove(p->timeout_id);
>> +
>> + p->func(p->attrib, err, data, length, p->user_data);
>> +
>> + free(p);
>> +}
>> +
>> +static bool read_timeout(void *user_data)
>> +{
>> + struct pending_read *p = user_data;
>> +
>> + p->timeout_id = 0;
>> +
>> + queue_remove(p->attrib->pending_reads, p);
>> +
>> + pending_read_result(p, -ETIMEDOUT, NULL, 0);
>> +
>> + return false;
>> +}
>> +
>> bool gatt_db_attribute_read(struct gatt_db_attribute *attrib, uint16_t offset,
>> uint8_t opcode, bdaddr_t *bdaddr,
>> gatt_db_attribute_read_t func, void *user_data)
>> @@ -789,7 +820,10 @@ bool gatt_db_attribute_read(struct gatt_db_attribute *attrib, uint16_t offset,
>> if (!p)
>> return false;
>>
>> + p->attrib = attrib;
>> p->id = ++attrib->read_id;
>> + p->timeout_id = timeout_add(ATTRIBUTE_TIMEOUT, read_timeout,
>> + p, NULL);
>> p->func = func;
>> p->user_data = user_data;
>>
>> @@ -834,11 +868,32 @@ bool gatt_db_attribute_read_result(struct gatt_db_attribute *attrib,
>> if (!p)
>> return false;
>>
>> - p->func(attrib, err, value, length, p->user_data);
>> + pending_read_result(p, err, value, length);
>> +
>> + return true;
>> +}
>> +
>> +static void pending_write_result(struct pending_write *p, int err)
>> +{
>> + if (p->timeout_id > 0)
>> + timeout_remove(p->timeout_id);
>> +
>> + p->func(p->attrib, err, p->user_data);
>>
>> free(p);
>> +}
>>
>> - return true;
>> +static bool write_timeout(void *user_data)
>> +{
>> + struct pending_write *p = user_data;
>> +
>> + p->timeout_id = 0;
>> +
>> + queue_remove(p->attrib->pending_writes, p);
>> +
>> + pending_write_result(p, -ETIMEDOUT);
>> +
>> + return false;
>> }
>>
>> bool gatt_db_attribute_write(struct gatt_db_attribute *attrib, uint16_t offset,
>> @@ -857,7 +912,10 @@ bool gatt_db_attribute_write(struct gatt_db_attribute *attrib, uint16_t offset,
>> if (!p)
>> return false;
>>
>> + p->attrib = attrib;
>> p->id = ++attrib->write_id;
>> + p->timeout_id = timeout_add(ATTRIBUTE_TIMEOUT, write_timeout,
>> + p, NULL);
>> p->func = func;
>> p->user_data = user_data;
>>
>> @@ -900,9 +958,7 @@ bool gatt_db_attribute_write_result(struct gatt_db_attribute *attrib,
>> if (!p)
>> return false;
>>
>> - p->func(attrib, err, p->user_data);
>> -
>> - free(p);
>> + pending_write_result(p, err);
>>
>> return true;
>> }
>> --
>> 1.9.3
>>
>> --
>> 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
>
> Overall I'm OK with this patch. One thing we need to figure out now
> though is deciding how we should report an errno like -ETIMEDOUT in an
> ATT Error Response. Right now I report 0xff if the error is negative
> or larger than UINT8_MAX (see
> src/shared/gatt-server.c:att_ecode_from_error), though we may want to
> map this to a more reasonable ATT application error.
Yes we need to come up with mapping between errno and ATT error code
and I figure right now you are just using 0xff, in case of Android I
was planning to use ATT_ECODE_TIMEOUT (0x81) but then I realize that
in Android case the application themselves should be using this range
so I guess we should stick with Unlikely Error. Btw 0xff actually
means out of range according to
https://www.bluetooth.org/DocMan/handlers/DownloadDoc.ashx?doc_id=282152
so we might want to change it to something like this:
if (!err || (err > 0 && err < UINT8_MAX))
return err;
switch (err) {
case -ENOENT:
return BT_ATT_ERROR_INVALID_HANDLE ;
case -ENOMEM:
return BT_ATT_ERROR_INSUFFICIENT_RESOURCES;
case -EOVERFLOW:
return 0xff;
case -EALREADY:
return 0xfe;
...
default:
return ATT_ECODE_UNLIKELY;
}
Anyway I would leave this to bt_gatt_* or bt_att to convert, actually
maybe doing in the later is better and create dedicated API like e.g.
bt_att_error_rsp which takes care of enconding, in fact I always
preferred to do the encoding in bt_att.
--
Luiz Augusto von Dentz
Hi Luiz,
> On Tue, Nov 11, 2014 at 6:22 AM, Luiz Augusto von Dentz <[email protected]> wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> This adds a timeout (1 second) to attribute operations since these can
> cause unexpected transport disconnections if they take too long to
> complete.
> ---
> src/shared/gatt-db.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 61 insertions(+), 5 deletions(-)
>
> diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
> index bab1202..a39eec2 100644
> --- a/src/shared/gatt-db.c
> +++ b/src/shared/gatt-db.c
> @@ -22,14 +22,17 @@
> */
>
> #include <stdbool.h>
> +#include <errno.h>
>
> #include "lib/uuid.h"
> #include "src/shared/util.h"
> #include "src/shared/queue.h"
> +#include "src/shared/timeout.h"
> #include "src/shared/gatt-db.h"
>
> #define MAX_CHAR_DECL_VALUE_LEN 19
> #define MAX_INCLUDED_VALUE_LEN 6
> +#define ATTRIBUTE_TIMEOUT 1000
>
> static const bt_uuid_t primary_service_uuid = { .type = BT_UUID16,
> .value.u16 = GATT_PRIM_SVC_UUID };
> @@ -46,13 +49,17 @@ struct gatt_db {
> };
>
> struct pending_read {
> + struct gatt_db_attribute *attrib;
> unsigned int id;
> + unsigned int timeout_id;
> gatt_db_attribute_read_t func;
> void *user_data;
> };
>
> struct pending_write {
> + struct gatt_db_attribute *attrib;
> unsigned int id;
> + unsigned int timeout_id;
> gatt_db_attribute_write_t func;
> void *user_data;
> };
> @@ -773,6 +780,30 @@ bool gatt_db_attribute_get_permissions(struct gatt_db_attribute *attrib,
> return true;
> }
>
> +static void pending_read_result(struct pending_read *p, int err,
> + const uint8_t *data, size_t length)
> +{
> + if (p->timeout_id > 0)
> + timeout_remove(p->timeout_id);
> +
> + p->func(p->attrib, err, data, length, p->user_data);
> +
> + free(p);
> +}
> +
> +static bool read_timeout(void *user_data)
> +{
> + struct pending_read *p = user_data;
> +
> + p->timeout_id = 0;
> +
> + queue_remove(p->attrib->pending_reads, p);
> +
> + pending_read_result(p, -ETIMEDOUT, NULL, 0);
> +
> + return false;
> +}
> +
> bool gatt_db_attribute_read(struct gatt_db_attribute *attrib, uint16_t offset,
> uint8_t opcode, bdaddr_t *bdaddr,
> gatt_db_attribute_read_t func, void *user_data)
> @@ -789,7 +820,10 @@ bool gatt_db_attribute_read(struct gatt_db_attribute *attrib, uint16_t offset,
> if (!p)
> return false;
>
> + p->attrib = attrib;
> p->id = ++attrib->read_id;
> + p->timeout_id = timeout_add(ATTRIBUTE_TIMEOUT, read_timeout,
> + p, NULL);
> p->func = func;
> p->user_data = user_data;
>
> @@ -834,11 +868,32 @@ bool gatt_db_attribute_read_result(struct gatt_db_attribute *attrib,
> if (!p)
> return false;
>
> - p->func(attrib, err, value, length, p->user_data);
> + pending_read_result(p, err, value, length);
> +
> + return true;
> +}
> +
> +static void pending_write_result(struct pending_write *p, int err)
> +{
> + if (p->timeout_id > 0)
> + timeout_remove(p->timeout_id);
> +
> + p->func(p->attrib, err, p->user_data);
>
> free(p);
> +}
>
> - return true;
> +static bool write_timeout(void *user_data)
> +{
> + struct pending_write *p = user_data;
> +
> + p->timeout_id = 0;
> +
> + queue_remove(p->attrib->pending_writes, p);
> +
> + pending_write_result(p, -ETIMEDOUT);
> +
> + return false;
> }
>
> bool gatt_db_attribute_write(struct gatt_db_attribute *attrib, uint16_t offset,
> @@ -857,7 +912,10 @@ bool gatt_db_attribute_write(struct gatt_db_attribute *attrib, uint16_t offset,
> if (!p)
> return false;
>
> + p->attrib = attrib;
> p->id = ++attrib->write_id;
> + p->timeout_id = timeout_add(ATTRIBUTE_TIMEOUT, write_timeout,
> + p, NULL);
> p->func = func;
> p->user_data = user_data;
>
> @@ -900,9 +958,7 @@ bool gatt_db_attribute_write_result(struct gatt_db_attribute *attrib,
> if (!p)
> return false;
>
> - p->func(attrib, err, p->user_data);
> -
> - free(p);
> + pending_write_result(p, err);
>
> return true;
> }
> --
> 1.9.3
>
> --
> 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
Overall I'm OK with this patch. One thing we need to figure out now
though is deciding how we should report an errno like -ETIMEDOUT in an
ATT Error Response. Right now I report 0xff if the error is negative
or larger than UINT8_MAX (see
src/shared/gatt-server.c:att_ecode_from_error), though we may want to
map this to a more reasonable ATT application error.
Cheers,
Arman