2014-10-13 21:09:57

by Arman Uguray

[permalink] [raw]
Subject: [PATCH v2 0/5] Introduce shared/gatt-server.

*v2:
- Split read_by_grp_type_cb into two functions by moving the response PDU
encoding loop into a helper function.

*v1:
- Make gatt-db external to gatt-server, as initially discussed.
- Also implement Read By Group Type request.

Arman Uguray (5):
shared/att: Drop the connection if a request is received while one is
pending.
shared/att: Respond with ERROR_REQUEST_NOT_SUPPORTED for unhandled
requests.
shared/gatt-server: Introduce shared/gatt-server.
shared/gatt-server: Support Exchange MTU request.
shared/gatt-server: Support Read By Group Type request.

Makefile.am | 1 +
src/shared/att.c | 124 +++++++++++------
src/shared/gatt-server.c | 356 +++++++++++++++++++++++++++++++++++++++++++++++
src/shared/gatt-server.h | 40 ++++++
4 files changed, 481 insertions(+), 40 deletions(-)
create mode 100644 src/shared/gatt-server.c
create mode 100644 src/shared/gatt-server.h

--
2.1.0.rc2.206.gedb03e5



2014-10-20 12:09:13

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH v2 0/5] Introduce shared/gatt-server.

Hi Arman,

On Thu, Oct 16, 2014 at 9:49 PM, Arman Uguray <[email protected]> wrote:
> On Mon, Oct 13, 2014 at 2:09 PM, Arman Uguray <[email protected]> wrote:
>> *v2:
>> - Split read_by_grp_type_cb into two functions by moving the response PDU
>> encoding loop into a helper function.
>>
>> *v1:
>> - Make gatt-db external to gatt-server, as initially discussed.
>> - Also implement Read By Group Type request.
>>
>> Arman Uguray (5):
>> shared/att: Drop the connection if a request is received while one is
>> pending.
>> shared/att: Respond with ERROR_REQUEST_NOT_SUPPORTED for unhandled
>> requests.
>> shared/gatt-server: Introduce shared/gatt-server.
>> shared/gatt-server: Support Exchange MTU request.
>> shared/gatt-server: Support Read By Group Type request.
>>
>> Makefile.am | 1 +
>> src/shared/att.c | 124 +++++++++++------
>> src/shared/gatt-server.c | 356 +++++++++++++++++++++++++++++++++++++++++++++++
>> src/shared/gatt-server.h | 40 ++++++
>> 4 files changed, 481 insertions(+), 40 deletions(-)
>> create mode 100644 src/shared/gatt-server.c
>> create mode 100644 src/shared/gatt-server.h
>>
>> --
>> 2.1.0.rc2.206.gedb03e5
>>
>
> ping. Luiz made comments on one of the patches but I believe they've
> been addressed.
> --

This is now pushed, note that Ive made a couple changes both in the
code and the commit message, please next time follow 50/72 format for
the commit message (check HACKING if you don't know what Im talking
about).


--
Luiz Augusto von Dentz

2014-10-16 18:49:41

by Arman Uguray

[permalink] [raw]
Subject: Re: [PATCH v2 0/5] Introduce shared/gatt-server.

On Mon, Oct 13, 2014 at 2:09 PM, Arman Uguray <[email protected]> wrote:
> *v2:
> - Split read_by_grp_type_cb into two functions by moving the response PDU
> encoding loop into a helper function.
>
> *v1:
> - Make gatt-db external to gatt-server, as initially discussed.
> - Also implement Read By Group Type request.
>
> Arman Uguray (5):
> shared/att: Drop the connection if a request is received while one is
> pending.
> shared/att: Respond with ERROR_REQUEST_NOT_SUPPORTED for unhandled
> requests.
> shared/gatt-server: Introduce shared/gatt-server.
> shared/gatt-server: Support Exchange MTU request.
> shared/gatt-server: Support Read By Group Type request.
>
> Makefile.am | 1 +
> src/shared/att.c | 124 +++++++++++------
> src/shared/gatt-server.c | 356 +++++++++++++++++++++++++++++++++++++++++++++++
> src/shared/gatt-server.h | 40 ++++++
> 4 files changed, 481 insertions(+), 40 deletions(-)
> create mode 100644 src/shared/gatt-server.c
> create mode 100644 src/shared/gatt-server.h
>
> --
> 2.1.0.rc2.206.gedb03e5
>

ping. Luiz made comments on one of the patches but I believe they've
been addressed.

2014-10-16 18:47:26

by Arman Uguray

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] shared/att: Drop the connection if a request is received while one is pending.

Hi Luiz,

>>> It seems this code is not following our coding style regarding multi
>>> line comments, check doc/coding-style.txt(M2: Multiple line comment)
>>> the first line should be empty.
>>>
>>
>> I didn't realize that was the comment style. I've seen both styles in
>> daemon code and I think I went with this one because of that. I'd
>> rather have the code be consistent (since shared/att uses this style
>> elsewhere) and maybe do a style correction pass in another patch?
>
> Sure, I can probably fix this myself just made the comment to let you
> know that we do in fact have a coding style for it.
>

Ah got it.


>>> This might cause a problem with our own code when connecting to each
>>> other if bt_att_cancel is used, apparently bt_att_cancel does not send
>>> anything to the remote side which seems to enable sending a new
>>> request without waiting any response for the first one.
>>>
>>
>> I'm confused. Do you mean if bt_att is being used in the client role,
>> it can send multiple requests this way? That is correct, though I
>> don't see how that's related to this patch? Currently bt_att_cancel
>> can be used to cancel waiting for a pending request and send the next
>> one, as you said. In that regard, it doesn't actually "cancel"
>> anything if the PDU has been sent over the air already. I don't know
>> how big of a problem this is, since the remote end should normally
>> detect the error and terminate the connection anyway. If this becomes
>> a problem we can always prevent canceling a request that has already
>> left the queue and is pending. Though, again, this isn't really
>> related to this patch unless I'm missing something.
>
> Sorry for the confusion, this patch is actually fine what we really
> need to fix is bt_att_cancel should not really remove the pending
> request otherwise the remote may disconnect if we proceed with another
> request.
>

Ok. I'll prepare a separate patch for that. We probably want
bt_att_cancel to keep the pending request pointer but it should
perhaps set the response and destroy callbacks to NULL. Same thing
applies to both outgoing requests and indications.

Cheers,
Arman

2014-10-16 07:31:43

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] shared/att: Drop the connection if a request is received while one is pending.

Hi Arman,

On Wed, Oct 15, 2014 at 8:36 PM, Arman Uguray <[email protected]> wrote:
> Hi Luiz,
>
>
>>> + case ATT_OP_TYPE_REQ:
>>> + /* If a request is currently pending, then the sequential
>>> + * protocol was violated. Disconnect the bearer and notify the
>>> + * upper-layer.
>>> + */
>>
>> It seems this code is not following our coding style regarding multi
>> line comments, check doc/coding-style.txt(M2: Multiple line comment)
>> the first line should be empty.
>>
>
> I didn't realize that was the comment style. I've seen both styles in
> daemon code and I think I went with this one because of that. I'd
> rather have the code be consistent (since shared/att uses this style
> elsewhere) and maybe do a style correction pass in another patch?

Sure, I can probably fix this myself just made the comment to let you
know that we do in fact have a coding style for it.

>
>>> + if (att->in_req) {
>>> + util_debug(att->debug_callback, att->debug_data,
>>> + "Received request while another is "
>>> + "pending: 0x%02x", opcode);
>>> + disconnect_cb(att->io, att);
>>> + return false;
>>> + }
>>> +
>>> + att->in_req = true;
>>> +
>>> + /* Fall through to the next case */
>>
>> This might cause a problem with our own code when connecting to each
>> other if bt_att_cancel is used, apparently bt_att_cancel does not send
>> anything to the remote side which seems to enable sending a new
>> request without waiting any response for the first one.
>>
>
> I'm confused. Do you mean if bt_att is being used in the client role,
> it can send multiple requests this way? That is correct, though I
> don't see how that's related to this patch? Currently bt_att_cancel
> can be used to cancel waiting for a pending request and send the next
> one, as you said. In that regard, it doesn't actually "cancel"
> anything if the PDU has been sent over the air already. I don't know
> how big of a problem this is, since the remote end should normally
> detect the error and terminate the connection anyway. If this becomes
> a problem we can always prevent canceling a request that has already
> left the queue and is pending. Though, again, this isn't really
> related to this patch unless I'm missing something.

Sorry for the confusion, this patch is actually fine what we really
need to fix is bt_att_cancel should not really remove the pending
request otherwise the remote may disconnect if we proceed with another
request.


--
Luiz Augusto von Dentz

2014-10-15 17:36:39

by Arman Uguray

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] shared/att: Drop the connection if a request is received while one is pending.

Hi Luiz,


>> + case ATT_OP_TYPE_REQ:
>> + /* If a request is currently pending, then the sequential
>> + * protocol was violated. Disconnect the bearer and notify the
>> + * upper-layer.
>> + */
>
> It seems this code is not following our coding style regarding multi
> line comments, check doc/coding-style.txt(M2: Multiple line comment)
> the first line should be empty.
>

I didn't realize that was the comment style. I've seen both styles in
daemon code and I think I went with this one because of that. I'd
rather have the code be consistent (since shared/att uses this style
elsewhere) and maybe do a style correction pass in another patch?


>> + if (att->in_req) {
>> + util_debug(att->debug_callback, att->debug_data,
>> + "Received request while another is "
>> + "pending: 0x%02x", opcode);
>> + disconnect_cb(att->io, att);
>> + return false;
>> + }
>> +
>> + att->in_req = true;
>> +
>> + /* Fall through to the next case */
>
> This might cause a problem with our own code when connecting to each
> other if bt_att_cancel is used, apparently bt_att_cancel does not send
> anything to the remote side which seems to enable sending a new
> request without waiting any response for the first one.
>

I'm confused. Do you mean if bt_att is being used in the client role,
it can send multiple requests this way? That is correct, though I
don't see how that's related to this patch? Currently bt_att_cancel
can be used to cancel waiting for a pending request and send the next
one, as you said. In that regard, it doesn't actually "cancel"
anything if the PDU has been sent over the air already. I don't know
how big of a problem this is, since the remote end should normally
detect the error and terminate the connection anyway. If this becomes
a problem we can always prevent canceling a request that has already
left the queue and is pending. Though, again, this isn't really
related to this patch unless I'm missing something.

Thanks,
Arman

2014-10-14 10:37:14

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] shared/att: Drop the connection if a request is received while one is pending.

Hi Arman,

On Tue, Oct 14, 2014 at 12:09 AM, Arman Uguray <[email protected]> wrote:
> With this patch, when bt_att is being used for the server role, it now makes
> sure that a second request drops the connection unless a response for a previous
> request has been sent.
> ---
> src/shared/att.c | 101 +++++++++++++++++++++++++++++++++----------------------
> 1 file changed, 61 insertions(+), 40 deletions(-)
>
> diff --git a/src/shared/att.c b/src/shared/att.c
> index de35aef..96f34a3 100644
> --- a/src/shared/att.c
> +++ b/src/shared/att.c
> @@ -64,6 +64,8 @@ struct bt_att {
> bool in_disconn;
> bool need_disconn_cleanup;
>
> + bool in_req; /* There's a pending incoming request */
> +
> uint8_t *buf;
> uint16_t mtu;
>
> @@ -460,6 +462,9 @@ static bool can_write_data(struct io *io, void *user_data)
> case ATT_OP_TYPE_IND:
> att->pending_ind = op;
> break;
> + case ATT_OP_TYPE_RSP:
> + /* Set in_req to false to indicate that no request is pending */
> + att->in_req = false;
> default:
> destroy_att_send_op(op);
> return true;
> @@ -604,6 +609,46 @@ static void handle_notify(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
> bt_att_unref(att);
> }
>
> +static void disconn_handler(void *data, void *user_data)
> +{
> + struct att_disconn *disconn = data;
> +
> + if (disconn->removed)
> + return;
> +
> + if (disconn->callback)
> + disconn->callback(disconn->user_data);
> +}
> +
> +static bool disconnect_cb(struct io *io, void *user_data)
> +{
> + struct bt_att *att = user_data;
> +
> + io_destroy(att->io);
> + att->io = NULL;
> +
> + util_debug(att->debug_callback, att->debug_data,
> + "Physical link disconnected");
> +
> + bt_att_ref(att);
> + att->in_disconn = true;
> + queue_foreach(att->disconn_list, disconn_handler, NULL);
> + att->in_disconn = false;
> +
> + if (att->need_disconn_cleanup) {
> + queue_remove_all(att->disconn_list, match_disconn_removed, NULL,
> + destroy_att_disconn);
> + att->need_disconn_cleanup = false;
> + }
> +
> + bt_att_cancel_all(att);
> + bt_att_unregister_all(att);
> +
> + bt_att_unref(att);
> +
> + return false;
> +}
> +
> static bool can_read_data(struct io *io, void *user_data)
> {
> struct bt_att *att = user_data;
> @@ -635,6 +680,22 @@ static bool can_read_data(struct io *io, void *user_data)
> util_debug(att->debug_callback, att->debug_data,
> "ATT opcode cannot be handled: 0x%02x", opcode);
> break;
> + case ATT_OP_TYPE_REQ:
> + /* If a request is currently pending, then the sequential
> + * protocol was violated. Disconnect the bearer and notify the
> + * upper-layer.
> + */

It seems this code is not following our coding style regarding multi
line comments, check doc/coding-style.txt(M2: Multiple line comment)
the first line should be empty.

> + if (att->in_req) {
> + util_debug(att->debug_callback, att->debug_data,
> + "Received request while another is "
> + "pending: 0x%02x", opcode);
> + disconnect_cb(att->io, att);
> + return false;
> + }
> +
> + att->in_req = true;
> +
> + /* Fall through to the next case */

This might cause a problem with our own code when connecting to each
other if bt_att_cancel is used, apparently bt_att_cancel does not send
anything to the remote side which seems to enable sending a new
request without waiting any response for the first one.

> default:
> /* For all other opcodes notify the upper layer of the PDU and
> * let them act on it.
> @@ -648,46 +709,6 @@ static bool can_read_data(struct io *io, void *user_data)
> return true;
> }
>
> -static void disconn_handler(void *data, void *user_data)
> -{
> - struct att_disconn *disconn = data;
> -
> - if (disconn->removed)
> - return;
> -
> - if (disconn->callback)
> - disconn->callback(disconn->user_data);
> -}
> -
> -static bool disconnect_cb(struct io *io, void *user_data)
> -{
> - struct bt_att *att = user_data;
> -
> - io_destroy(att->io);
> - att->io = NULL;
> -
> - util_debug(att->debug_callback, att->debug_data,
> - "Physical link disconnected");
> -
> - bt_att_ref(att);
> - att->in_disconn = true;
> - queue_foreach(att->disconn_list, disconn_handler, NULL);
> - att->in_disconn = false;
> -
> - if (att->need_disconn_cleanup) {
> - queue_remove_all(att->disconn_list, match_disconn_removed, NULL,
> - destroy_att_disconn);
> - att->need_disconn_cleanup = false;
> - }
> -
> - bt_att_cancel_all(att);
> - bt_att_unregister_all(att);
> -
> - bt_att_unref(att);
> -
> - return false;
> -}
> -
> struct bt_att *bt_att_new(int fd)
> {
> struct bt_att *att;
> --
> 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

2014-10-13 21:10:02

by Arman Uguray

[permalink] [raw]
Subject: [PATCH v2 5/5] shared/gatt-server: Support Read By Group Type request.

This patch adds handling for the Read By Group Type request.
---
src/shared/gatt-server.c | 179 ++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 178 insertions(+), 1 deletion(-)

diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
index c7b0873..d6d0155 100644
--- a/src/shared/gatt-server.c
+++ b/src/shared/gatt-server.c
@@ -45,6 +45,7 @@ struct bt_gatt_server {
uint16_t mtu;

unsigned int mtu_id;
+ unsigned int read_by_grp_type_id;

bt_gatt_server_debug_func_t debug_callback;
bt_gatt_server_destroy_func_t debug_destroy;
@@ -54,6 +55,7 @@ struct bt_gatt_server {
static void gatt_server_unregister_handlers(struct bt_gatt_server *server)
{
bt_att_unregister(server->att, server->mtu_id);
+ bt_att_unregister(server->att, server->read_by_grp_type_id);
}

static void gatt_server_cleanup(struct bt_gatt_server *server)
@@ -71,6 +73,166 @@ static void encode_error_rsp(uint8_t opcode, uint16_t handle, uint8_t ecode,
put_le16(handle, pdu + 1);
}

+static bool get_uuid_le(const uint8_t *uuid, size_t len, bt_uuid_t *out_uuid)
+{
+ uint128_t u128;
+
+ switch (len) {
+ case 2:
+ bt_uuid16_create(out_uuid, get_le16(uuid));
+ return true;
+ case 16:
+ bswap_128(uuid, &u128.data);
+ bt_uuid128_create(out_uuid, u128);
+ return true;
+ default:
+ return false;
+ }
+
+ return false;
+}
+
+static bool encode_read_by_grp_type_rsp(struct gatt_db *db, struct queue *q,
+ uint16_t mtu,
+ uint8_t *pdu, uint16_t *len)
+{
+ int iter = 0;
+ uint16_t start_handle, end_handle;
+ uint8_t *value;
+ int value_len;
+ uint8_t data_val_len;
+
+ *len = 0;
+
+ while (queue_peek_head(q)) {
+ start_handle = PTR_TO_UINT(queue_pop_head(q));
+ value = NULL;
+ value_len = 0;
+
+ /* This should never be deferred to the read callback for
+ * primary/secondary service declarations.
+ */
+ if (!gatt_db_read(db, start_handle, 0,
+ BT_ATT_OP_READ_BY_GRP_TYPE_REQ,
+ NULL, &value,
+ &value_len) || value_len < 0)
+ return false;
+
+ /* Use the first attribute to determine the length of each
+ * attribute data unit. Stop the list when a different attribute
+ * value is seen.
+ */
+ if (iter == 0) {
+ data_val_len = 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)
+ 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);
+
+ iter += data_val_len + 4;
+ }
+
+ *len = iter;
+
+ return true;
+}
+
+static void read_by_grp_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;
+ bt_uuid_t prim, snd;
+ 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 != 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 Grp 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 defines that only the <<Primary Service>> and
+ * <<Secondary Service>> group types can be used for the
+ * "Read By Group Type" request (Core v4.1, Vol 3, sec 2.5.3). Return an
+ * error if any other group type is given.
+ */
+ bt_uuid16_create(&prim, GATT_PRIM_SVC_UUID);
+ bt_uuid16_create(&snd, GATT_SND_SVC_UUID);
+ if (bt_uuid_cmp(&type, &prim) && bt_uuid_cmp(&type, &snd)) {
+ ecode = BT_ATT_ERROR_UNSUPPORTED_GROUP_TYPE;
+ goto error;
+ }
+
+ gatt_db_read_by_group_type(server->db, start, end, type, q);
+
+ if (queue_isempty(q)) {
+ ecode = BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND;
+ goto error;
+ }
+
+ if (!encode_read_by_grp_type_rsp(server->db, q, mtu, rsp_pdu,
+ &rsp_len)) {
+ ecode = BT_ATT_ERROR_UNLIKELY;
+ goto error;
+ }
+
+ rsp_opcode = BT_ATT_OP_READ_BY_GRP_TYPE_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)
{
@@ -97,18 +259,33 @@ static void exchange_mtu_cb(uint8_t opcode, const void *pdu,
/* Set MTU to be the minimum */
server->mtu = final_mtu;
bt_att_set_mtu(server->att, final_mtu);
+
+ util_debug(server->debug_callback, server->debug_data,
+ "MTU exchange complete, with MTU: %u", final_mtu);
}

static bool gatt_server_register_att_handlers(struct bt_gatt_server *server)
{
- /* EXCHANGE MTU request */
+ /* Exchange MTU */
server->mtu_id = bt_att_register(server->att, BT_ATT_OP_MTU_REQ,
exchange_mtu_cb,
server, NULL);
if (!server->mtu_id)
return false;

+ /* Read By Group Type */
+ server->read_by_grp_type_id = bt_att_register(server->att,
+ BT_ATT_OP_READ_BY_GRP_TYPE_REQ,
+ read_by_grp_type_cb,
+ server, NULL);
+ if (!server->read_by_grp_type_id)
+ goto fail;
+
return true;
+
+fail:
+ gatt_server_unregister_handlers(server);
+ return false;
}

struct bt_gatt_server *bt_gatt_server_new(struct gatt_db *db,
--
2.1.0.rc2.206.gedb03e5


2014-10-13 21:10:01

by Arman Uguray

[permalink] [raw]
Subject: [PATCH v2 4/5] shared/gatt-server: Support Exchange MTU request.

This patch adds handling for the exchange MTU request.
---
src/shared/gatt-server.c | 60 +++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 57 insertions(+), 3 deletions(-)

diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
index 544e60e..c7b0873 100644
--- a/src/shared/gatt-server.c
+++ b/src/shared/gatt-server.c
@@ -34,29 +34,83 @@
#define MAX(a, b) ((a) > (b) ? (a) : (b))
#endif

+#ifndef MIN
+#define MIN(a, b) ((a) < (b) ? (a) : (b))
+#endif
+
struct bt_gatt_server {
struct gatt_db *db;
struct bt_att *att;
int ref_count;
uint16_t mtu;

+ unsigned int mtu_id;
+
bt_gatt_server_debug_func_t debug_callback;
bt_gatt_server_destroy_func_t debug_destroy;
void *debug_data;
};

-static bool gatt_server_register_att_handlers(struct bt_gatt_server *server)
+static void gatt_server_unregister_handlers(struct bt_gatt_server *server)
{
- /* TODO */
- return true;
+ bt_att_unregister(server->att, server->mtu_id);
}

static void gatt_server_cleanup(struct bt_gatt_server *server)
{
+ gatt_server_unregister_handlers(server);
bt_att_unref(server->att);
server->att = NULL;
}

+static void encode_error_rsp(uint8_t opcode, uint16_t handle, uint8_t ecode,
+ uint8_t pdu[4])
+{
+ pdu[0] = opcode;
+ pdu[3] = ecode;
+ put_le16(handle, pdu + 1);
+}
+
+static void exchange_mtu_cb(uint8_t opcode, const void *pdu,
+ uint16_t length, void *user_data)
+{
+ struct bt_gatt_server *server = user_data;
+ uint16_t client_rx_mtu;
+ uint16_t final_mtu;
+ uint8_t rsp_pdu[4];
+
+ if (length != 2) {
+ encode_error_rsp(opcode, 0, BT_ATT_ERROR_INVALID_PDU, rsp_pdu);
+ bt_att_send(server->att, BT_ATT_OP_ERROR_RSP, rsp_pdu,
+ sizeof(rsp_pdu), NULL, NULL, NULL);
+ return;
+ }
+
+ client_rx_mtu = get_le16(pdu);
+ final_mtu = MAX(MIN(client_rx_mtu, server->mtu), BT_ATT_DEFAULT_LE_MTU);
+
+ /* Respond with the server MTU */
+ put_le16(server->mtu, rsp_pdu);
+ bt_att_send(server->att, BT_ATT_OP_MTU_RSP, rsp_pdu, 2, NULL, NULL,
+ NULL);
+
+ /* Set MTU to be the minimum */
+ server->mtu = final_mtu;
+ bt_att_set_mtu(server->att, final_mtu);
+}
+
+static bool gatt_server_register_att_handlers(struct bt_gatt_server *server)
+{
+ /* EXCHANGE MTU request */
+ server->mtu_id = bt_att_register(server->att, BT_ATT_OP_MTU_REQ,
+ exchange_mtu_cb,
+ server, NULL);
+ if (!server->mtu_id)
+ return false;
+
+ return true;
+}
+
struct bt_gatt_server *bt_gatt_server_new(struct gatt_db *db,
struct bt_att *att, uint16_t mtu)
{
--
2.1.0.rc2.206.gedb03e5


2014-10-13 21:10:00

by Arman Uguray

[permalink] [raw]
Subject: [PATCH v2 3/5] shared/gatt-server: Introduce shared/gatt-server.

This patch introduces bt_gatt_server which will implement the server-side of the
ATT protocol over a bt_att structure and construct responses based on a gatt_db
structure.
---
Makefile.am | 1 +
src/shared/gatt-server.c | 125 +++++++++++++++++++++++++++++++++++++++++++++++
src/shared/gatt-server.h | 40 +++++++++++++++
3 files changed, 166 insertions(+)
create mode 100644 src/shared/gatt-server.c
create mode 100644 src/shared/gatt-server.h

diff --git a/Makefile.am b/Makefile.am
index 1a1a43f..2dfea28 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -114,6 +114,7 @@ shared_sources = src/shared/io.h src/shared/timeout.h \
src/shared/att.h src/shared/att.c \
src/shared/gatt-helpers.h src/shared/gatt-helpers.c \
src/shared/gatt-client.h src/shared/gatt-client.c \
+ src/shared/gatt-server.h src/shared/gatt-server.c \
src/shared/gatt-db.h src/shared/gatt-db.c \
src/shared/gap.h src/shared/gap.c

diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
new file mode 100644
index 0000000..544e60e
--- /dev/null
+++ b/src/shared/gatt-server.c
@@ -0,0 +1,125 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2014 Google Inc.
+ *
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#include "src/shared/att.h"
+#include "lib/uuid.h"
+#include "src/shared/queue.h"
+#include "src/shared/gatt-db.h"
+#include "src/shared/gatt-server.h"
+#include "src/shared/gatt-helpers.h"
+#include "src/shared/att-types.h"
+#include "src/shared/util.h"
+
+#ifndef MAX
+#define MAX(a, b) ((a) > (b) ? (a) : (b))
+#endif
+
+struct bt_gatt_server {
+ struct gatt_db *db;
+ struct bt_att *att;
+ int ref_count;
+ uint16_t mtu;
+
+ bt_gatt_server_debug_func_t debug_callback;
+ bt_gatt_server_destroy_func_t debug_destroy;
+ void *debug_data;
+};
+
+static bool gatt_server_register_att_handlers(struct bt_gatt_server *server)
+{
+ /* TODO */
+ return true;
+}
+
+static void gatt_server_cleanup(struct bt_gatt_server *server)
+{
+ bt_att_unref(server->att);
+ server->att = NULL;
+}
+
+struct bt_gatt_server *bt_gatt_server_new(struct gatt_db *db,
+ struct bt_att *att, uint16_t mtu)
+{
+ struct bt_gatt_server *server;
+
+ if (!att)
+ return NULL;
+
+ server = new0(struct bt_gatt_server, 1);
+ if (!server)
+ return NULL;
+
+ server->db = db;
+ server->att = bt_att_ref(att);
+ server->mtu = MAX(mtu, BT_ATT_DEFAULT_LE_MTU);
+
+ if (!gatt_server_register_att_handlers(server)) {
+ gatt_db_destroy(server->db);
+ bt_att_unref(att);
+ free(server);
+ return NULL;
+ }
+
+ return bt_gatt_server_ref(server);
+}
+
+struct bt_gatt_server *bt_gatt_server_ref(struct bt_gatt_server *server)
+{
+ if (!server)
+ return NULL;
+
+ __sync_fetch_and_add(&server->ref_count, 1);
+
+ return server;
+}
+
+void bt_gatt_server_unref(struct bt_gatt_server *server)
+{
+ if (__sync_sub_and_fetch(&server->ref_count, 1))
+ return;
+
+ if (server->debug_destroy)
+ server->debug_destroy(server->debug_data);
+
+ gatt_server_cleanup(server);
+
+ free(server);
+}
+
+bool bt_gatt_server_set_debug(struct bt_gatt_server *server,
+ bt_gatt_server_debug_func_t callback,
+ void *user_data,
+ bt_gatt_server_destroy_func_t destroy)
+{
+ if (!server)
+ return false;
+
+ if (server->debug_destroy)
+ server->debug_destroy(server->debug_data);
+
+ server->debug_callback = callback;
+ server->debug_destroy = destroy;
+ server->debug_data = user_data;
+
+ return true;
+}
diff --git a/src/shared/gatt-server.h b/src/shared/gatt-server.h
new file mode 100644
index 0000000..e3c4def
--- /dev/null
+++ b/src/shared/gatt-server.h
@@ -0,0 +1,40 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2014 Google Inc.
+ *
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#include <stdint.h>
+
+struct bt_gatt_server;
+
+struct bt_gatt_server *bt_gatt_server_new(struct gatt_db *db,
+ struct bt_att *att, uint16_t mtu);
+
+struct bt_gatt_server *bt_gatt_server_ref(struct bt_gatt_server *server);
+void bt_gatt_server_unref(struct bt_gatt_server *server);
+
+typedef void (*bt_gatt_server_destroy_func_t)(void *user_data);
+typedef void (*bt_gatt_server_debug_func_t)(const char *str, void *user_data);
+
+bool bt_gatt_server_set_debug(struct bt_gatt_server *server,
+ bt_gatt_server_debug_func_t callback,
+ void *user_data,
+ bt_gatt_server_destroy_func_t destroy);
--
2.1.0.rc2.206.gedb03e5


2014-10-13 21:09:59

by Arman Uguray

[permalink] [raw]
Subject: [PATCH v2 2/5] shared/att: Respond with ERROR_REQUEST_NOT_SUPPORTED for unhandled requests.

With this patch bt_att automatically responds with ERROR_REQUEST_NOT_SUPPORTED
to an incoming request for which no handler has been registered via
bt_att_register.
---
src/shared/att.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)

diff --git a/src/shared/att.c b/src/shared/att.c
index 96f34a3..d086ca8 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -562,6 +562,7 @@ struct notify_data {
uint8_t opcode;
uint8_t *pdu;
ssize_t pdu_len;
+ bool handler_found;
};

static void notify_handler(void *data, void *user_data)
@@ -575,11 +576,27 @@ static void notify_handler(void *data, void *user_data)
if (notify->opcode != not_data->opcode)
return;

+ not_data->handler_found = true;
+
if (notify->callback)
notify->callback(not_data->opcode, not_data->pdu,
not_data->pdu_len, notify->user_data);
}

+static void respond_not_supported(struct bt_att *att, uint8_t opcode)
+{
+ uint8_t pdu[4];
+
+ memset(pdu, 0, sizeof(pdu));
+ pdu[0] = opcode;
+ pdu[1] = 0;
+ pdu[2] = 0;
+ pdu[3] = BT_ATT_ERROR_REQUEST_NOT_SUPPORTED;
+
+ bt_att_send(att, BT_ATT_OP_ERROR_RSP, pdu, sizeof(pdu), NULL, NULL,
+ NULL);
+}
+
static void handle_notify(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
ssize_t pdu_len)
{
@@ -607,6 +624,12 @@ static void handle_notify(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
}

bt_att_unref(att);
+
+ /* If this was a request and no handler was registered for it, respond
+ * with "Not Supported"
+ */
+ if (!data.handler_found && get_op_type(opcode) == ATT_OP_TYPE_REQ)
+ respond_not_supported(att, opcode);
}

static void disconn_handler(void *data, void *user_data)
--
2.1.0.rc2.206.gedb03e5


2014-10-13 21:09:58

by Arman Uguray

[permalink] [raw]
Subject: [PATCH v2 1/5] shared/att: Drop the connection if a request is received while one is pending.

With this patch, when bt_att is being used for the server role, it now makes
sure that a second request drops the connection unless a response for a previous
request has been sent.
---
src/shared/att.c | 101 +++++++++++++++++++++++++++++++++----------------------
1 file changed, 61 insertions(+), 40 deletions(-)

diff --git a/src/shared/att.c b/src/shared/att.c
index de35aef..96f34a3 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -64,6 +64,8 @@ struct bt_att {
bool in_disconn;
bool need_disconn_cleanup;

+ bool in_req; /* There's a pending incoming request */
+
uint8_t *buf;
uint16_t mtu;

@@ -460,6 +462,9 @@ static bool can_write_data(struct io *io, void *user_data)
case ATT_OP_TYPE_IND:
att->pending_ind = op;
break;
+ case ATT_OP_TYPE_RSP:
+ /* Set in_req to false to indicate that no request is pending */
+ att->in_req = false;
default:
destroy_att_send_op(op);
return true;
@@ -604,6 +609,46 @@ static void handle_notify(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
bt_att_unref(att);
}

+static void disconn_handler(void *data, void *user_data)
+{
+ struct att_disconn *disconn = data;
+
+ if (disconn->removed)
+ return;
+
+ if (disconn->callback)
+ disconn->callback(disconn->user_data);
+}
+
+static bool disconnect_cb(struct io *io, void *user_data)
+{
+ struct bt_att *att = user_data;
+
+ io_destroy(att->io);
+ att->io = NULL;
+
+ util_debug(att->debug_callback, att->debug_data,
+ "Physical link disconnected");
+
+ bt_att_ref(att);
+ att->in_disconn = true;
+ queue_foreach(att->disconn_list, disconn_handler, NULL);
+ att->in_disconn = false;
+
+ if (att->need_disconn_cleanup) {
+ queue_remove_all(att->disconn_list, match_disconn_removed, NULL,
+ destroy_att_disconn);
+ att->need_disconn_cleanup = false;
+ }
+
+ bt_att_cancel_all(att);
+ bt_att_unregister_all(att);
+
+ bt_att_unref(att);
+
+ return false;
+}
+
static bool can_read_data(struct io *io, void *user_data)
{
struct bt_att *att = user_data;
@@ -635,6 +680,22 @@ static bool can_read_data(struct io *io, void *user_data)
util_debug(att->debug_callback, att->debug_data,
"ATT opcode cannot be handled: 0x%02x", opcode);
break;
+ case ATT_OP_TYPE_REQ:
+ /* If a request is currently pending, then the sequential
+ * protocol was violated. Disconnect the bearer and notify the
+ * upper-layer.
+ */
+ if (att->in_req) {
+ util_debug(att->debug_callback, att->debug_data,
+ "Received request while another is "
+ "pending: 0x%02x", opcode);
+ disconnect_cb(att->io, att);
+ return false;
+ }
+
+ att->in_req = true;
+
+ /* Fall through to the next case */
default:
/* For all other opcodes notify the upper layer of the PDU and
* let them act on it.
@@ -648,46 +709,6 @@ static bool can_read_data(struct io *io, void *user_data)
return true;
}

-static void disconn_handler(void *data, void *user_data)
-{
- struct att_disconn *disconn = data;
-
- if (disconn->removed)
- return;
-
- if (disconn->callback)
- disconn->callback(disconn->user_data);
-}
-
-static bool disconnect_cb(struct io *io, void *user_data)
-{
- struct bt_att *att = user_data;
-
- io_destroy(att->io);
- att->io = NULL;
-
- util_debug(att->debug_callback, att->debug_data,
- "Physical link disconnected");
-
- bt_att_ref(att);
- att->in_disconn = true;
- queue_foreach(att->disconn_list, disconn_handler, NULL);
- att->in_disconn = false;
-
- if (att->need_disconn_cleanup) {
- queue_remove_all(att->disconn_list, match_disconn_removed, NULL,
- destroy_att_disconn);
- att->need_disconn_cleanup = false;
- }
-
- bt_att_cancel_all(att);
- bt_att_unregister_all(att);
-
- bt_att_unref(att);
-
- return false;
-}
-
struct bt_att *bt_att_new(int fd)
{
struct bt_att *att;
--
2.1.0.rc2.206.gedb03e5