2014-07-18 21:13:46

by Arman Uguray

[permalink] [raw]
Subject: [PATCH 00/11] Add shared/gatt-helpers

This patch-set introduces src/shared/gatt-helpers, which provides helper
functions for client-side procedures defined for the Generic Attribute Profile.
These helpers are useful, since most of them are performed over several ATT
protocol requests over a wide attribute range (all discovery and long read/write
procedures). These functions also handle the encoding/decoding of the ATT
protocol PDU involved in these requests.

All GATT client procedures except the "Signed Write Without Response" have been
implemented, as this requires signed PDUs to be handled correctly by
bt_att_send, which will be implemented in a future patch-set.

Arman Uguray (11):
shared/gatt: Introduce gatt-helpers.h skeleton.
shared/gatt: Implement bt_gatt_exchange_mtu.
shared/gatt: Implement "Discover All Primary Services" procedure.
shared/gatt: Implement "Discover Primary Service by UUID" procedure.
shared/gatt: Implement "Characteristic Discovery" procedures.
shared/gatt: Implement "Descriptor Discovery" procedure.
shared/gatt: Implement "Read" procedure.
shared/gatt: Implement "Read Long Characteristic Values" procedure.
shared/gatt: Implement "Write Value" and "Write Without Response"
procedures.
shared/gatt: Implement "Write Long Values" procedure.
shared/gatt: Implement notification/indication helper.

Makefile.am | 3 +-
src/shared/gatt-helpers.c | 1445 +++++++++++++++++++++++++++++++++++++++++++++
src/shared/gatt-helpers.h | 135 +++++
3 files changed, 1582 insertions(+), 1 deletion(-)
create mode 100644 src/shared/gatt-helpers.c
create mode 100644 src/shared/gatt-helpers.h

--
2.0.0.526.g5318336



2014-07-23 23:00:42

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 01/11] shared/gatt: Introduce gatt-helpers.h skeleton.

Hi Arman,

>> I am feeling uneasy about doing it this way. Can we try to start with just using a uint8 for the ATT error and see how far that gets us. I have bad experience with overloading values from standards and binary protocols with our own. And I know range wise this is safe (at least at the moment).
>>
>> Or for that case we have an extra parameter indicating the success or failure of a verified write. It might be better to make it procedure specific instead of generalizing it.
>>
>> We do not have to do it all at once. This is why I think we should start just reporting the ATT errors and see how far that gets us. So we learn which are the cases that will actually need special handling.
>>
>> For the cases of protocol errors, there is not much point in telling the higher layers about the exact violation. If it happens, you are in trouble. Nothing you do will fix it.
>>
>
> Now that I look at it, there are 4 errors that I have defined:
> - UNKNOWN, which is only returned when a malloc fails in a call to
> bt_att_send.
> - INVALID_RSP, returned when the server sends a malformed PDU
> - FAILED_ALLOC, returned when a malloc fails in bt_gatt routines/subroutines
> - RELIABLE_WRITE, returned when the reliable write verification fails
>
> The first 3 of these have no good way of recovering from other than
> killing the connection or crashing/exiting and the last one applies to
> only one specific GATT procedure. So I'll go with your suggestion
> here. All existing callback functions will accept a "bool success" and
> a "uint8_t att_ecode" parameter. If att_ecode is 0 but success is
> false, then one of the first 3 conditions above happened. For reliable
> writes I'll add a new callback which also takes in a "bool
> reliable_verified", that if false, indicates that the write failed
> because it couldn't be verified.
>
> The other case that I can think of is the signed write, for which we
> can have a specific callback as well.
>
> Sound good?

sounds good to me. Please go ahead with it.

>>> I think so. I like to keep these procedures more or less inline with
>>> what is defined in the core spec. The distinction between these is
>>> simple enough that the upper layer can correctly determine which one
>>> to use as needed.
>>
>> We can do that of course, but for me the questions is always if it gets really used that way. Will any code really make the distinction and care about it. If not, then this is just bloat that makes the higher layers work for something they should not.
>>
>
> I see value in having these separate routines. I will keep them for
> now, but I'd be happy to remove/revise them later if unifying them
> turns out to be better as the whole thing starts getting clearer as I
> work on it.

Fair enough.

Regards

Marcel


2014-07-23 22:54:26

by Arman Uguray

[permalink] [raw]
Subject: Re: [PATCH 01/11] shared/gatt: Introduce gatt-helpers.h skeleton.

Hi Marcel,

> I am feeling uneasy about doing it this way. Can we try to start with jus=
t using a uint8 for the ATT error and see how far that gets us. I have bad =
experience with overloading values from standards and binary protocols with=
our own. And I know range wise this is safe (at least at the moment).
>
> Or for that case we have an extra parameter indicating the success or fai=
lure of a verified write. It might be better to make it procedure specific =
instead of generalizing it.
>
> We do not have to do it all at once. This is why I think we should start =
just reporting the ATT errors and see how far that gets us. So we learn whi=
ch are the cases that will actually need special handling.
>
> For the cases of protocol errors, there is not much point in telling the =
higher layers about the exact violation. If it happens, you are in trouble.=
Nothing you do will fix it.
>

Now that I look at it, there are 4 errors that I have defined:
- UNKNOWN, which is only returned when a malloc fails in a call to
bt_att_send.
- INVALID_RSP, returned when the server sends a malformed PDU
- FAILED_ALLOC, returned when a malloc fails in bt_gatt routines/subrouti=
nes
- RELIABLE_WRITE, returned when the reliable write verification fails

The first 3 of these have no good way of recovering from other than
killing the connection or crashing/exiting and the last one applies to
only one specific GATT procedure. So I'll go with your suggestion
here. All existing callback functions will accept a "bool success" and
a "uint8_t att_ecode" parameter. If att_ecode is 0 but success is
false, then one of the first 3 conditions above happened. For reliable
writes I'll add a new callback which also takes in a "bool
reliable_verified", that if false, indicates that the write failed
because it couldn't be verified.

The other case that I can think of is the signed write, for which we
can have a specific callback as well.

Sound good?


>> I think so. I like to keep these procedures more or less inline with
>> what is defined in the core spec. The distinction between these is
>> simple enough that the upper layer can correctly determine which one
>> to use as needed.
>
> We can do that of course, but for me the questions is always if it gets r=
eally used that way. Will any code really make the distinction and care abo=
ut it. If not, then this is just bloat that makes the higher layers work fo=
r something they should not.
>

I see value in having these separate routines. I will keep them for
now, but I'd be happy to remove/revise them later if unifying them
turns out to be better as the whole thing starts getting clearer as I
work on it.

Cheers,
Arman

2014-07-23 05:39:30

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 01/11] shared/gatt: Introduce gatt-helpers.h skeleton.

Hi Arman,

>>> + * The callbacks below encode the operation status in a 16-bit unsigned integer,
>>> + * where 0-255 are allocated for ATT protocol errors.
>>> + *
>>> + * - 0x0000: Success
>>> + * - 0x0001: Invalid handle (ATT protocol error)
>>> + * - 0x0100: Unknown failure (bluetoothd defined)
>>
>> do we really need this? I am feeling a bit uneasy about these kind of error splits where we are overloading wire protocol errors with internal errors.
>>
>
> I think they are useful. I think it ends up being cleaner than
> returning two separate arguments in callbacks, one for ATT protocol
> errors and one for application-specific errors and we clearly document
> above that the LSB is for ATT protocol errors only.

I am feeling uneasy about doing it this way. Can we try to start with just using a uint8 for the ATT error and see how far that gets us. I have bad experience with overloading values from standards and binary protocols with our own. And I know range wise this is safe (at least at the moment).

>>> +#define BT_GATT_ERROR_UNKNOWN 0x0100
>>> +#define BT_GATT_ERROR_INVALID_RSP 0x0101
>>
>> Especially with the background of invalid response as listed here, I think the only real result is a disconnect anyway, so we might better introduce a disconnect reason with the disconnect callback. Just an idea.
>>
>
> Well with some errors you want to disconnect but not with all errors.
> For example, in the case of verified writes, if the response PDU
> didn't match the blob that we sent, we would report that the
> verification failed in an error like this but this wouldn't warrant a
> disconnect.

Or for that case we have an extra parameter indicating the success or failure of a verified write. It might be better to make it procedure specific instead of generalizing it.

> In general, though, I agree with having a disconnect callback. In this
> case though, I feel that, from a layering perspective, these helpers
> should just return an error in the result callback and have the upper
> layer interpret that as a requirement for disconnect. After all,
> reporting these helper-defined errors in a result callback is not all
> that different from what you're suggesting.
>
> We can then go and add that exact "disconnect required" callback to
> src/shared/gatt.h when we add it later. I'd like to leave these as
> just GATT procedure helper functions without doing any big state
> keeping here.

We do not have to do it all at once. This is why I think we should start just reporting the ATT errors and see how far that gets us. So we learn which are the cases that will actually need special handling.

For the cases of protocol errors, there is not much point in telling the higher layers about the exact violation. If it happens, you are in trouble. Nothing you do will fix it.

>>> +
>>> +typedef void (*bt_gatt_destroy_func_t)(void *user_data);
>>> +
>>> +typedef void (*bt_gatt_result_callback_t)(uint16_t status, void *user_data);
>>> +typedef void (*bt_gatt_discovery_callback_t)(uint16_t status,
>>> + struct queue *results, void *user_data);
>>
>> Can we please avoid internal data structures exposed here. I would say this needs to provide its own GATT specific data structure for the result. Most likely an allocated array or pointer array with a dedicated free function.
>
> Interesting, I was under the impression that struct queue was to be
> used like GSList or other list structures used elsewhere and it was ok
> to use it here since this is all going into src/shared. Are you
> suggesting something like:
>
> typedef void (*bt_gatt_services_callback_t)(uint16_t status, struct
> bt_gatt_service *services, size_t len, ...);
> typedef void (*bt_gatt_characteristics_callback_t)(uint16_t status,
> struct bt_gatt_characteristic *chrcs, ...);
>
> ...etc?

Yes. I am suggesting something along these lines. It depends on how much information you need to return. Maybe an array of uint16_t handle is enough in some cases. In others we might need a custom structure.

The background here is that long-term we want to get away from GLib dependencies and move over to using ELL. We are in the process of adding kdbus support to ELL and stabilize it. However that will not happen overnight and the internal data structures in src/shared/ are almost identical to ELL's data structures so that gives us a more easier way to port everything over. The less we expose in unit tested code, the easier this transition will be when it finally comes.

>>> +bool bt_gatt_read_value(struct bt_att *att, uint16_t value_handle,
>>> + bt_gatt_read_callback_t callback,
>>> + void *user_data,
>>> + bt_gatt_destroy_func_t destroy);
>>> +bool bt_gatt_read_long_value(struct bt_att *att,
>>> + uint16_t value_handle, uint16_t offset,
>>> + bt_gatt_read_callback_t callback,
>>> + void *user_data,
>>> + bt_gatt_destroy_func_t destroy);
>>
>> Does this need an explicit function? Wouldn't it make more sense to handle the long reads (and writes for that matter) internally.
>>
>
> I think so. I like to keep these procedures more or less inline with
> what is defined in the core spec. The distinction between these is
> simple enough that the upper layer can correctly determine which one
> to use as needed.

We can do that of course, but for me the questions is always if it gets really used that way. Will any code really make the distinction and care about it. If not, then this is just bloat that makes the higher layers work for something they should not.

Regards

Marcel


2014-07-23 01:22:25

by Arman Uguray

[permalink] [raw]
Subject: Re: [PATCH 01/11] shared/gatt: Introduce gatt-helpers.h skeleton.

Hi Marcel,

>> + * The callbacks below encode the operation status in a 16-bit unsigned integer,
>> + * where 0-255 are allocated for ATT protocol errors.
>> + *
>> + * - 0x0000: Success
>> + * - 0x0001: Invalid handle (ATT protocol error)
>> + * - 0x0100: Unknown failure (bluetoothd defined)
>
> do we really need this? I am feeling a bit uneasy about these kind of error splits where we are overloading wire protocol errors with internal errors.
>

I think they are useful. I think it ends up being cleaner than
returning two separate arguments in callbacks, one for ATT protocol
errors and one for application-specific errors and we clearly document
above that the LSB is for ATT protocol errors only.


>> +#define BT_GATT_ERROR_UNKNOWN 0x0100
>> +#define BT_GATT_ERROR_INVALID_RSP 0x0101
>
> Especially with the background of invalid response as listed here, I think the only real result is a disconnect anyway, so we might better introduce a disconnect reason with the disconnect callback. Just an idea.
>

Well with some errors you want to disconnect but not with all errors.
For example, in the case of verified writes, if the response PDU
didn't match the blob that we sent, we would report that the
verification failed in an error like this but this wouldn't warrant a
disconnect.

In general, though, I agree with having a disconnect callback. In this
case though, I feel that, from a layering perspective, these helpers
should just return an error in the result callback and have the upper
layer interpret that as a requirement for disconnect. After all,
reporting these helper-defined errors in a result callback is not all
that different from what you're suggesting.

We can then go and add that exact "disconnect required" callback to
src/shared/gatt.h when we add it later. I'd like to leave these as
just GATT procedure helper functions without doing any big state
keeping here.


>> +
>> +typedef void (*bt_gatt_destroy_func_t)(void *user_data);
>> +
>> +typedef void (*bt_gatt_result_callback_t)(uint16_t status, void *user_data);
>> +typedef void (*bt_gatt_discovery_callback_t)(uint16_t status,
>> + struct queue *results, void *user_data);
>
> Can we please avoid internal data structures exposed here. I would say this needs to provide its own GATT specific data structure for the result. Most likely an allocated array or pointer array with a dedicated free function.

Interesting, I was under the impression that struct queue was to be
used like GSList or other list structures used elsewhere and it was ok
to use it here since this is all going into src/shared. Are you
suggesting something like:

typedef void (*bt_gatt_services_callback_t)(uint16_t status, struct
bt_gatt_service *services, size_t len, ...);
typedef void (*bt_gatt_characteristics_callback_t)(uint16_t status,
struct bt_gatt_characteristic *chrcs, ...);

...etc?


>> +bool bt_gatt_read_value(struct bt_att *att, uint16_t value_handle,
>> + bt_gatt_read_callback_t callback,
>> + void *user_data,
>> + bt_gatt_destroy_func_t destroy);
>> +bool bt_gatt_read_long_value(struct bt_att *att,
>> + uint16_t value_handle, uint16_t offset,
>> + bt_gatt_read_callback_t callback,
>> + void *user_data,
>> + bt_gatt_destroy_func_t destroy);
>
> Does this need an explicit function? Wouldn't it make more sense to handle the long reads (and writes for that matter) internally.
>

I think so. I like to keep these procedures more or less inline with
what is defined in the core spec. The distinction between these is
simple enough that the upper layer can correctly determine which one
to use as needed.

Cheers,
Arman

2014-07-22 23:25:41

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 02/11] shared/gatt: Implement bt_gatt_exchange_mtu.

Hi Arman,

> This patch implements the helper function bt_gatt_exchange_mtu, which performs
> an ATT "Exchange MTU" request and resizes the internal buffer based on the
> result.
> ---
> src/shared/gatt-helpers.c | 87 +++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 85 insertions(+), 2 deletions(-)
>
> diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
> index 3d85868..9cf05cf 100644
> --- a/src/shared/gatt-helpers.c
> +++ b/src/shared/gatt-helpers.c
> @@ -30,14 +30,97 @@
> #include "src/shared/att.h"
> #include "lib/uuid.h"
> #include "src/shared/gatt-helpers.h"
> +#include "src/shared/util.h"
> +
> +#ifndef MIN
> +#define MIN(a, b) ((a) < (b) ? (a) : (b))
> +#endif
> +
> +struct mtu_op {
> + struct bt_att *att;
> + uint16_t client_rx_mtu;
> + bt_gatt_result_callback_t callback;
> + void *user_data;
> + bt_gatt_destroy_func_t destroy;
> +};
> +
> +static void destroy_mtu_op(void *user_data)
> +{
> + struct mtu_op *op = user_data;
> +
> + if (op->destroy)
> + op->destroy(op->user_data);
> +
> + free(op);
> +}
> +
> +static uint16_t process_error(const void *pdu, uint16_t length)
> +{
> + if (!pdu || length != 4)
> + return BT_GATT_ERROR_INVALID_RSP;
> +
> + return (uint16_t)((uint8_t *) pdu)[3];
> +}

I am not convinced by the uint16 status, but in case you do these things, create a uint16_t local variable and assign the uint8_t value to it and then return. It is a clearer, then the double casting.

> +
> +static void mtu_cb(uint8_t opcode, const void *pdu, uint16_t length,
> + void *user_data)
> +{
> + struct mtu_op *op = user_data;
> + uint16_t result = 0;
> + uint16_t server_rx_mtu;
> +
> + if (opcode == BT_ATT_OP_ERROR_RSP) {
> + result = process_error(pdu, length);
> + goto done;
> + }
> +
> + if (opcode != BT_ATT_OP_MTU_RSP || !pdu || length != 2) {
> + result = BT_GATT_ERROR_INVALID_RSP;
> + goto done;
> + }
> +
> + server_rx_mtu = get_le16(pdu);
> + bt_att_set_mtu(op->att, MIN(op->client_rx_mtu, server_rx_mtu));
> +
> +done:
> + if (op->callback)
> + op->callback(result, op->user_data);
> +}
>
> bool bt_gatt_exchange_mtu(struct bt_att *att, uint16_t client_rx_mtu,
> bt_gatt_result_callback_t callback,
> void *user_data,
> bt_gatt_destroy_func_t destroy)
> {
> - /* TODO */
> - return false;
> + struct mtu_op *op;
> + uint8_t pdu[2];
> +
> + if (!att)
> + return false;
> +
> + if (!client_rx_mtu)
> + return false;
> +
> + op = new0(struct mtu_op, 1);
> + if (!op)
> + return false;
> +
> + op->att = att;
> + op->client_rx_mtu = client_rx_mtu;
> + op->callback = callback;
> + op->user_data = user_data;
> + op->destroy = destroy;
> +
> + put_le16(client_rx_mtu, pdu);
> +
> + if (!bt_att_send(att, BT_ATT_OP_MTU_REQ, pdu, sizeof(pdu),
> + mtu_cb, op,
> + destroy_mtu_op)) {
> + free(op);
> + return false;
> + }
> +
> + return true;
> }

Regards

Marcel


2014-07-22 23:23:31

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 01/11] shared/gatt: Introduce gatt-helpers.h skeleton.

Hi Arman,

> This patch introduces the skeleton for src/shared/gatt-helpers, which defines
> helper functions for over-the-air GATT client-side procedures that will be
> frequently used by clients. Most of these functions require several sequential
> ATT protocol requests and it is useful to abstract these details away from the
> upper layer.
> ---
> Makefile.am | 3 +-
> src/shared/gatt-helpers.c | 141 ++++++++++++++++++++++++++++++++++++++++++++++
> src/shared/gatt-helpers.h | 133 +++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 276 insertions(+), 1 deletion(-)
> create mode 100644 src/shared/gatt-helpers.c
> create mode 100644 src/shared/gatt-helpers.h
>
> diff --git a/Makefile.am b/Makefile.am
> index 4588ce8..e4f7df2 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -157,7 +157,8 @@ src_bluetoothd_SOURCES = $(builtin_sources) \
> src/shared/queue.h src/shared/queue.c \
> src/shared/util.h src/shared/util.c \
> src/shared/mgmt.h src/shared/mgmt.c \
> - src/shared/att-types.h src/shared/att.h src/shared/att.c
> + src/shared/att-types.h src/shared/att.h src/shared/att.c \
> + src/shared/gatt-helpers.h src/shared/gatt-helpers.c
> src_bluetoothd_LDADD = lib/libbluetooth-internal.la gdbus/libgdbus-internal.la \
> @GLIB_LIBS@ @DBUS_LIBS@ -ldl -lrt
> src_bluetoothd_LDFLAGS = $(AM_LDFLAGS) -Wl,--export-dynamic \
> diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
> new file mode 100644
> index 0000000..3d85868
> --- /dev/null
> +++ b/src/shared/gatt-helpers.c
> @@ -0,0 +1,141 @@
> +/*
> + *
> + * 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
> + *
> + */
> +
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include "src/shared/queue.h"
> +#include "src/shared/att.h"
> +#include "lib/uuid.h"
> +#include "src/shared/gatt-helpers.h"
> +
> +bool bt_gatt_exchange_mtu(struct bt_att *att, uint16_t client_rx_mtu,
> + bt_gatt_result_callback_t callback,
> + void *user_data,
> + bt_gatt_destroy_func_t destroy)
> +{
> + /* TODO */
> + return false;
> +}
> +
> +bool bt_gatt_discover_primary_services(struct bt_att *att,
> + bt_uuid_t *uuid,
> + bt_gatt_discovery_callback_t callback,
> + void *user_data,
> + bt_gatt_destroy_func_t destroy)
> +{
> + /* TODO */
> + return false;
> +}
> +
> +bool bt_gatt_discover_included_services(struct bt_att *att,
> + uint16_t start, uint16_t end,
> + bt_uuid_t *uuid,
> + bt_gatt_discovery_callback_t callback,
> + void *user_data,
> + bt_gatt_destroy_func_t destroy)
> +{
> + /* TODO */
> + return false;
> +}
> +
> +bool bt_gatt_discover_characteristics(struct bt_att *att,
> + uint16_t start, uint16_t end,
> + bt_uuid_t *uuid,
> + bt_gatt_discovery_callback_t callback,
> + void *user_data,
> + bt_gatt_destroy_func_t destroy)
> +{
> + /* TODO */
> + return false;
> +}
> +
> +bool bt_gatt_discover_descriptors(struct bt_att *att,
> + uint16_t start, uint16_t end,
> + bt_gatt_discovery_callback_t callback,
> + void *user_data,
> + bt_gatt_destroy_func_t destroy)
> +{
> + /* TODO */
> + return false;
> +}
> +
> +bool bt_gatt_read_value(struct bt_att *att, uint16_t value_handle,
> + bt_gatt_read_callback_t callback,
> + void *user_data,
> + bt_gatt_destroy_func_t destroy)
> +{
> + /* TODO */
> + return false;
> +}
> +
> +bool bt_gatt_read_long_value(struct bt_att *att,
> + uint16_t value_handle, uint16_t offset,
> + bt_gatt_read_callback_t callback,
> + void *user_data,
> + bt_gatt_destroy_func_t destroy)
> +{
> + /* TODO */
> + return false;
> +}
> +
> +bool bt_gatt_write_without_response(struct bt_att *att,
> + uint16_t value_handle,
> + bool signed_write,
> + uint8_t *value, uint16_t length)
> +{
> + /* TODO */
> + return false;
> +}
> +
> +bool bt_gatt_write_value(struct bt_att *att, uint16_t value_handle,
> + uint8_t *value, uint16_t length,
> + bt_gatt_result_callback_t callback,
> + void *user_data,
> + bt_gatt_destroy_func_t destroy)
> +{
> + /* TODO */
> + return false;
> +}
> +
> +bool bt_gatt_write_long_value(struct bt_att *att, bool reliable,
> + uint16_t value_handle, uint16_t offset,
> + uint8_t *value, uint16_t length,
> + bt_gatt_result_callback_t callback,
> + void *user_data,
> + bt_gatt_destroy_func_t destroy)
> +{
> + /* TODO */
> + return false;
> +}
> +
> +unsigned int bt_gatt_register(struct bt_att *att, bool indications,
> + bt_gatt_notify_callback_t callback,
> + void *user_data,
> + bt_gatt_destroy_func_t destroy)
> +{
> + /* TODO */
> + return false;
> +}
> diff --git a/src/shared/gatt-helpers.h b/src/shared/gatt-helpers.h
> new file mode 100644
> index 0000000..b711324
> --- /dev/null
> +++ b/src/shared/gatt-helpers.h
> @@ -0,0 +1,133 @@
> +/*
> + *
> + * 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
> + *
> + */
> +
> +/* This file defines helpers for performing client-side procedures defined by
> + * the Generic Attribute Profile.
> + */
> +
> +#include <stdbool.h>
> +#include <stdint.h>
> +
> +struct bt_gatt_service {
> + uint16_t start;
> + uint16_t end;
> + uint8_t uuid[16];
> +};
> +
> +struct bt_gatt_characteristic {
> + uint16_t start;
> + uint16_t end;
> + uint16_t value;
> + uint8_t properties;
> + uint8_t uuid[16];
> +};
> +
> +struct bt_gatt_descriptor {
> + uint16_t handle;
> + uint8_t uuid[16];
> +};
> +
> +/* Operations can report two kinds of errors:
> + *
> + * 1. ATT protocol error codes
> + * 2. bluetoothd defined errors
> + *
> + * The callbacks below encode the operation status in a 16-bit unsigned integer,
> + * where 0-255 are allocated for ATT protocol errors.
> + *
> + * - 0x0000: Success
> + * - 0x0001: Invalid handle (ATT protocol error)
> + * - 0x0100: Unknown failure (bluetoothd defined)

do we really need this? I am feeling a bit uneasy about these kind of error splits where we are overloading wire protocol errors with internal errors.

> + *
> + */
> +#define BT_GATT_ERROR_UNKNOWN 0x0100
> +#define BT_GATT_ERROR_INVALID_RSP 0x0101

Especially with the background of invalid response as listed here, I think the only real result is a disconnect anyway, so we might better introduce a disconnect reason with the disconnect callback. Just an idea.

> +
> +typedef void (*bt_gatt_destroy_func_t)(void *user_data);
> +
> +typedef void (*bt_gatt_result_callback_t)(uint16_t status, void *user_data);
> +typedef void (*bt_gatt_discovery_callback_t)(uint16_t status,
> + struct queue *results, void *user_data);

Can we please avoid internal data structures exposed here. I would say this needs to provide its own GATT specific data structure for the result. Most likely an allocated array or pointer array with a dedicated free function.

> +typedef void (*bt_gatt_read_callback_t)(uint16_t status, const uint8_t *value,
> + uint16_t length, void *user_data);
> +
> +typedef void (*bt_gatt_notify_callback_t)(uint16_t value_handle,
> + const uint8_t *value, uint16_t length,
> + void *user_data);
> +
> +bool bt_gatt_exchange_mtu(struct bt_att *att, uint16_t client_rx_mtu,
> + bt_gatt_result_callback_t callback,
> + void *user_data,
> + bt_gatt_destroy_func_t destroy);
> +
> +bool bt_gatt_discover_primary_services(struct bt_att *att, bt_uuid_t *uuid,
> + bt_gatt_discovery_callback_t callback,
> + void *user_data,
> + bt_gatt_destroy_func_t destroy);
> +bool bt_gatt_discover_included_services(struct bt_att *att,
> + uint16_t start, uint16_t end,
> + bt_uuid_t *uuid,
> + bt_gatt_discovery_callback_t callback,
> + void *user_data,
> + bt_gatt_destroy_func_t destroy);
> +bool bt_gatt_discover_characteristics(struct bt_att *att,
> + uint16_t start, uint16_t end,
> + bt_uuid_t *uuid,
> + bt_gatt_discovery_callback_t callback,
> + void *user_data,
> + bt_gatt_destroy_func_t destroy);
> +bool bt_gatt_discover_descriptors(struct bt_att *att,
> + uint16_t start, uint16_t end,
> + bt_gatt_discovery_callback_t callback,
> + void *user_data,
> + bt_gatt_destroy_func_t destroy);
> +
> +bool bt_gatt_read_value(struct bt_att *att, uint16_t value_handle,
> + bt_gatt_read_callback_t callback,
> + void *user_data,
> + bt_gatt_destroy_func_t destroy);
> +bool bt_gatt_read_long_value(struct bt_att *att,
> + uint16_t value_handle, uint16_t offset,
> + bt_gatt_read_callback_t callback,
> + void *user_data,
> + bt_gatt_destroy_func_t destroy);

Does this need an explicit function? Wouldn't it make more sense to handle the long reads (and writes for that matter) internally.

> +
> +bool bt_gatt_write_without_response(struct bt_att *att, uint16_t value_handle,
> + bool signed_write,
> + uint8_t *value, uint16_t length);
> +bool bt_gatt_write_value(struct bt_att *att, uint16_t value_handle,
> + uint8_t *value, uint16_t length,
> + bt_gatt_result_callback_t callback,
> + void *user_data,
> + bt_gatt_destroy_func_t destroy);
> +bool bt_gatt_write_long_value(struct bt_att *att, bool reliable,
> + uint16_t value_handle, uint16_t offset,
> + uint8_t *value, uint16_t length,
> + bt_gatt_result_callback_t callback,
> + void *user_data,
> + bt_gatt_destroy_func_t destroy);
> +
> +unsigned int bt_gatt_register(struct bt_att *att, bool indications,
> + bt_gatt_notify_callback_t callback,
> + void *user_data,
> + bt_gatt_destroy_func_t destroy);

Regards

Marcel


2014-07-22 23:23:29

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 03/11] shared/gatt: Implement "Discover All Primary Services" procedure.

Hi Arman,

> This patch implements bt_gatt_discover_primary_services for the case when no
> UUID is provided.
> ---
> src/shared/gatt-helpers.c | 197 +++++++++++++++++++++++++++++++++++++++++++++-
> src/shared/gatt-helpers.h | 1 +
> 2 files changed, 196 insertions(+), 2 deletions(-)
>
> diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
> index 9cf05cf..4fbf2eb 100644
> --- a/src/shared/gatt-helpers.c
> +++ b/src/shared/gatt-helpers.c
> @@ -36,6 +36,11 @@
> #define MIN(a, b) ((a) < (b) ? (a) : (b))
> #endif
>
> +static uint8_t bt_base_uuid[16] = {
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00,
> + 0x80, 0x00, 0x00, 0x80, 0x5F, 0x9B, 0x34, 0xFB
> +};
> +

as a general rule, make these const as well.

> struct mtu_op {
> struct bt_att *att;
> uint16_t client_rx_mtu;
> @@ -123,14 +128,202 @@ bool bt_gatt_exchange_mtu(struct bt_att *att, uint16_t client_rx_mtu,
> return true;
> }
>
> +struct discovery_op {
> + struct bt_att *att;
> + int ref_count;
> + bt_uuid_t uuid;
> + struct queue *results;
> + bt_gatt_discovery_callback_t callback;
> + void *user_data;
> + bt_gatt_destroy_func_t destroy;
> +};
> +
> +static struct discovery_op* discovery_op_ref(struct discovery_op *op)
> +{
> + __sync_fetch_and_add(&op->ref_count, 1);
> +
> + return op;
> +}
> +
> +static void discovery_op_unref(void *data)
> +{
> + struct discovery_op *op = data;
> +
> + if (__sync_sub_and_fetch(&op->ref_count, 1))
> + return;
> +
> + if (op->destroy)
> + op->destroy(op->user_data);
> +
> + queue_destroy(op->results, free);
> +
> + free(op);
> +}
> +
> +static void put_uuid_le(const bt_uuid_t *src, void *dst)
> +{
> + if (src->type == BT_UUID16)
> + put_le16(src->value.u16, dst);
> + else
> + bswap_128(&src->value.u128, dst);
> +}

Actually 32-bit UUIDs are defined for GATT internally. It just for the wire protocol, the 32-bit UUID always needs to be converted to 128-bit. So you might want to take care of this detail here as well.

> +
> +static bool convert_uuid_le(const uint8_t *src, size_t len, uint8_t dst[16])
> +{
> + if (len == 16) {
> + bswap_128(src, dst);
> + return true;
> + }
> +
> + if (len != 2)
> + return false;
> +
> + memcpy(dst, bt_base_uuid, sizeof(bt_base_uuid));
> + dst[2] = src[1];
> + dst[3] = src[0];
> +
> + return true;
> +}
> +
> +static void read_by_grp_type_cb(uint8_t opcode, const void *pdu,
> + uint16_t length, void *user_data)
> +{
> + struct discovery_op *op = user_data;
> + uint16_t status;
> + struct queue *results = NULL;
> + size_t data_length;
> + size_t list_length;
> + uint16_t last_end;
> + int i;
> +
> + if (opcode == BT_ATT_OP_ERROR_RSP) {
> + status = process_error(pdu, length);
> +
> + if (status == BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND &&
> + !queue_isempty(op->results))
> + goto success;
> +
> + goto done;
> + }
> +
> + /* PDU must contain at least the following (sans opcode):
> + * - Attr Data Length (1 octet)
> + * - Attr Data List (at least 6 octets):
> + * -- 2 octets: Attribute handle
> + * -- 2 octets: End group handle
> + * -- 2 or 16 octets: service UUID
> + */
> + if (opcode != BT_ATT_OP_READ_BY_GRP_TYPE_RSP || !pdu || length < 7) {
> + status = BT_GATT_ERROR_INVALID_RSP;
> + goto done;
> + }
> +
> + data_length = ((uint8_t *) pdu)[0];
> + list_length = length - 1;
> +
> + if ((list_length % data_length) ||
> + (data_length != 6 && data_length != 20)) {
> + status = BT_GATT_ERROR_INVALID_RSP;
> + goto done;
> + }
> +
> + for (i = 1; i < length; i += data_length) {
> + struct bt_gatt_service *service;
> +
> + service = new0(struct bt_gatt_service, 1);
> + if (!service) {
> + status = BT_GATT_ERROR_FAILED_ALLOC;
> + goto done;
> + }
> +
> + service->start = get_le16(pdu + i);
> + last_end = get_le16(pdu + i + 2);
> + service->end = last_end;
> + convert_uuid_le(pdu + i + 4, data_length - 4, service->uuid);
> +
> + queue_push_tail(op->results, service);
> + }
> +
> + if (last_end != 0xFFFF) {

Generally we do 0xffff in lower-case. There might cases in the code that do not follow this as strictly, but we should not introduce new code doing the same thing.

> + uint8_t pdu[6];
> +
> + put_le16(last_end + 1, pdu);
> + put_le16(0xFFFF, pdu + 2);
> + put_le16(GATT_PRIM_SVC_UUID, pdu + 4);

So I have no problem in doing initially manual PDU building this way, but I thing latest in follow up patches we need to introduce packed structs (where it makes sense since the data is static). If we don't, then this get easily out of control and the code becomes really hard to read later on since it is not obvious what PDU you are building.

> +
> + if (bt_att_send(op->att, BT_ATT_OP_READ_BY_GRP_TYPE_REQ,
> + pdu, sizeof(pdu),
> + read_by_grp_type_cb,
> + discovery_op_ref(op),
> + discovery_op_unref))
> + return;
> +
> + discovery_op_unref(op);
> + status = BT_GATT_ERROR_UNKNOWN;
> + goto done;
> + }
> +
> +success:
> + /* End of procedure */
> + results = op->results;
> + status = 0;
> +
> +done:
> + if (op->callback)
> + op->callback(status, results, op->user_data);
> +}
> +
> bool bt_gatt_discover_primary_services(struct bt_att *att,
> bt_uuid_t *uuid,
> bt_gatt_discovery_callback_t callback,
> void *user_data,
> bt_gatt_destroy_func_t destroy)
> {
> - /* TODO */
> - return false;
> + struct discovery_op *op;
> + bool result;
> +
> + if (!att)
> + return false;
> +
> + op = new0(struct discovery_op, 1);
> + if (!op)
> + return false;
> +
> + op->results = queue_new();
> + if (!op->results) {
> + free(op);
> + return false;
> + }
> +
> + op->att = att;
> + op->callback = callback;
> + op->user_data = user_data;
> + op->destroy = destroy;
> +
> + /* If UUID is NULL, then discover all primary services */
> + if (!uuid) {
> + uint8_t pdu[6];
> +
> + put_le16(0x0001, pdu);
> + put_le16(0xFFFF, pdu + 2);
> + put_le16(GATT_PRIM_SVC_UUID, pdu + 4);
> +
> + result = bt_att_send(att, BT_ATT_OP_READ_BY_GRP_TYPE_REQ,
> + pdu, sizeof(pdu),
> + read_by_grp_type_cb,
> + discovery_op_ref(op),
> + discovery_op_unref);
> + } else {
> + free(op);
> + return false;
> + }
> +
> + if (!result) {
> + queue_destroy(op->results, free);
> + free(op);
> + }
> +
> + return result;
> }
>
> bool bt_gatt_discover_included_services(struct bt_att *att,
> diff --git a/src/shared/gatt-helpers.h b/src/shared/gatt-helpers.h
> index b711324..068c4bb 100644
> --- a/src/shared/gatt-helpers.h
> +++ b/src/shared/gatt-helpers.h
> @@ -62,6 +62,7 @@ struct bt_gatt_descriptor {
> */
> #define BT_GATT_ERROR_UNKNOWN 0x0100
> #define BT_GATT_ERROR_INVALID_RSP 0x0101
> +#define BT_GATT_ERROR_FAILED_ALLOC 0x0102
>
> typedef void (*bt_gatt_destroy_func_t)(void *user_data);

Regards

Marcel


2014-07-22 23:23:26

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 11/11] shared/gatt: Implement notification/indication helper.

Hi Arman,

> This patch implements bt_gatt_register, which sets up internal callbacks for
> incoming notifications and indications, parses the PDU and provides it to the
> upper layer. It also automatically sends out a confirmation when an indication
> is received.
> ---
> src/shared/gatt-helpers.c | 64 +++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 62 insertions(+), 2 deletions(-)
>
> diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
> index 5afe91e..f82bb59 100644
> --- a/src/shared/gatt-helpers.c
> +++ b/src/shared/gatt-helpers.c
> @@ -1375,11 +1375,71 @@ bool bt_gatt_write_long_value(struct bt_att *att, bool reliable,
> return true;
> }
>
> +struct notify_data {
> + struct bt_att *att;
> + bt_gatt_notify_callback_t callback;
> + void *user_data;
> + bt_gatt_destroy_func_t destroy;
> +};
> +
> +static void notify_data_destroy(void *data)
> +{
> + struct notify_data *notd = data;
> +
> + if (notd->destroy)
> + notd->destroy(notd->user_data);
> +
> + free(notd);
> +}
> +
> +static void notify_cb(uint8_t opcode, const void *pdu, uint16_t length,
> + void *user_data)
> +{
> + struct notify_data *data = user_data;
> + bool indication;
> + uint16_t value_handle;
> + const uint8_t *value = NULL;
> +
> + indication = (opcode == BT_ATT_OP_HANDLE_VAL_IND);

andy reason for this interim variable?

> + value_handle = get_le16(pdu);
> +
> + if (length > 2)
> + value = pdu + 2;
> +
> + if (data->callback)
> + data->callback(value_handle, value, length - 2, data->user_data);
> +
> + if (indication)

I would just check the opcode here and add a comment on why we are doing it this way.

> + bt_att_send(data->att, BT_ATT_OP_HANDLE_VAL_CONF, NULL, 0,
> + NULL, NULL, NULL);
> +}
> +
> unsigned int bt_gatt_register(struct bt_att *att, bool indications,
> bt_gatt_notify_callback_t callback,
> void *user_data,
> bt_gatt_destroy_func_t destroy)
> {
> - /* TODO */
> - return false;
> + struct notify_data *data;
> + uint8_t opcode;
> + unsigned int id;
> +
> + if (!att)
> + return 0;
> +
> + data = new0(struct notify_data, 1);
> + if (!data)
> + return 0;
> +
> + data->att = att;
> + data->callback = callback;
> + data->user_data = user_data;
> + data->destroy = destroy;
> +
> + opcode = indications ? BT_ATT_OP_HANDLE_VAL_IND : BT_ATT_OP_HANDLE_VAL_NOT;
> +
> + id = bt_att_register(att, opcode, notify_cb, data, notify_data_destroy);
> + if (!id)
> + free(data);
> +
> + return id;
> }

Regards

Marcel


2014-07-21 22:17:17

by Arman Uguray

[permalink] [raw]
Subject: Re: [PATCH 04/11] shared/gatt: Implement "Discover Primary Service by UUID" procedure.

Hi Grzegorz,

>> if (src->type == BT_UUID16)
>> put_le16(src->value.u16, dst);
>> + else if (src->type == BT_UUID32)
>> + put_le32(src->value.u32, dst);
>
> Specification Core 4.1 says that 32bit UUIDS in GATT should be
> converted to 128bit. Please check it [Vol. 3] Part G, Section 2.5.4
>

You're right, thanks for catching that.

2014-07-21 09:39:21

by Grzegorz Kolodziejczyk

[permalink] [raw]
Subject: Re: [PATCH 04/11] shared/gatt: Implement "Discover Primary Service by UUID" procedure.

Hi Arman,

On 18 July 2014 23:13, Arman Uguray <[email protected]> wrote:
> This patch implements bt_gatt_discover_primary_services for the case when a UUID
> is provided.
> ---
> src/shared/gatt-helpers.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 93 insertions(+), 2 deletions(-)
>
> diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
> index 4fbf2eb..fb5a7b6 100644
> --- a/src/shared/gatt-helpers.c
> +++ b/src/shared/gatt-helpers.c
> @@ -164,6 +164,8 @@ static void put_uuid_le(const bt_uuid_t *src, void *dst)
> {
> if (src->type == BT_UUID16)
> put_le16(src->value.u16, dst);
> + else if (src->type == BT_UUID32)
> + put_le32(src->value.u32, dst);

Specification Core 4.1 says that 32bit UUIDS in GATT should be
converted to 128bit. Please check it [Vol. 3] Part G, Section 2.5.4

> else
> bswap_128(&src->value.u128, dst);
> }
> @@ -273,6 +275,82 @@ done:
> op->callback(status, results, op->user_data);
> }
>
> +static void find_by_type_val_cb(uint8_t opcode, const void *pdu,
> + uint16_t length, void *user_data)
> +{
> + struct discovery_op *op = user_data;
> + uint16_t status = 0;
> + struct queue *results = NULL;
> + uint16_t last_end;
> + int i;
> +
> + if (opcode == BT_ATT_OP_ERROR_RSP) {
> + status = process_error(pdu, length);
> +
> + if (status == BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND &&
> + !queue_isempty(op->results))
> + goto success;
> +
> + goto done;
> + }
> +
> + /* PDU must contain 4 bytes and it must be a multiple of 4, where each
> + * 4 bytes contain the 16-bit attribute and group end handles.
> + */
> + if (opcode != BT_ATT_OP_FIND_BY_TYPE_VAL_RSP || !pdu || !length ||
> + length % 4) {
> + status = BT_GATT_ERROR_INVALID_RSP;
> + goto done;
> + }
> +
> + for (i = 0; i < length; i += 4) {
> + struct bt_gatt_service *service;
> + bt_uuid_t uuid;
> +
> + service = new0(struct bt_gatt_service, 1);
> + if (!service)
> + goto done;
> +
> + service->start = get_le16(pdu + i);
> + last_end = get_le16(pdu + i + 2);
> + service->end = last_end;
> +
> + bt_uuid_to_uuid128(&op->uuid, &uuid);
> + memcpy(service->uuid, uuid.value.u128.data, 16);
> +
> + queue_push_tail(op->results, service);
> + }
> +
> + if (last_end != 0xFFFF) {
> + uint8_t pdu[6 + bt_uuid_len(&op->uuid)];
> +
> + put_le16(last_end + 1, pdu);
> + put_le16(0xFFFF, pdu + 2);
> + put_le16(GATT_PRIM_SVC_UUID, pdu + 4);
> + put_uuid_le(&op->uuid, pdu + 6);
> +
> + if (bt_att_send(op->att, BT_ATT_OP_FIND_BY_TYPE_VAL_REQ,
> + pdu, sizeof(pdu),
> + find_by_type_val_cb,
> + discovery_op_ref(op),
> + discovery_op_unref))
> + return;
> +
> + discovery_op_unref(op);
> + status = BT_GATT_ERROR_UNKNOWN;
> + goto done;
> + }
> +
> +success:
> + /* End of procedure */
> + results = op->results;
> + status = 0;
> +
> +done:
> + if (op->callback)
> + op->callback(status, results, op->user_data);
> +}
> +
> bool bt_gatt_discover_primary_services(struct bt_att *att,
> bt_uuid_t *uuid,
> bt_gatt_discovery_callback_t callback,
> @@ -314,8 +392,21 @@ bool bt_gatt_discover_primary_services(struct bt_att *att,
> discovery_op_ref(op),
> discovery_op_unref);
> } else {
> - free(op);
> - return false;
> + uint8_t pdu[6 + bt_uuid_len(uuid)];
> +
> + /* Discover by UUID */
> + op->uuid = *uuid;
> +
> + put_le16(0x0001, pdu);
> + put_le16(0xFFFF, pdu + 2);
> + put_le16(GATT_PRIM_SVC_UUID, pdu + 4);
> + put_uuid_le(&op->uuid, pdu + 6);
> +
> + result = bt_att_send(att, BT_ATT_OP_FIND_BY_TYPE_VAL_REQ,
> + pdu, sizeof(pdu),
> + find_by_type_val_cb,
> + discovery_op_ref(op),
> + discovery_op_unref);
> }
>
> if (!result) {
> --
> 2.0.0.526.g5318336
>
> --
> 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

Best regards,
Grzegorz

2014-07-21 07:37:59

by Grzegorz Kolodziejczyk

[permalink] [raw]
Subject: Re: [PATCH 02/11] shared/gatt: Implement bt_gatt_exchange_mtu.

Hi Arman,


On 18 July 2014 23:13, Arman Uguray <[email protected]> wrote:
>
> This patch implements the helper function bt_gatt_exchange_mtu, which performs
> an ATT "Exchange MTU" request and resizes the internal buffer based on the
> result.
> ---
> src/shared/gatt-helpers.c | 87 +++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 85 insertions(+), 2 deletions(-)
>
> diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
> index 3d85868..9cf05cf 100644
> --- a/src/shared/gatt-helpers.c
> +++ b/src/shared/gatt-helpers.c
> @@ -30,14 +30,97 @@
> #include "src/shared/att.h"
> #include "lib/uuid.h"
> #include "src/shared/gatt-helpers.h"
> +#include "src/shared/util.h"
> +
> +#ifndef MIN
> +#define MIN(a, b) ((a) < (b) ? (a) : (b))
> +#endif
> +
> +struct mtu_op {
> + struct bt_att *att;
> + uint16_t client_rx_mtu;
> + bt_gatt_result_callback_t callback;
> + void *user_data;
> + bt_gatt_destroy_func_t destroy;
> +};
> +
> +static void destroy_mtu_op(void *user_data)
> +{
> + struct mtu_op *op = user_data;
> +
> + if (op->destroy)
> + op->destroy(op->user_data);
> +
> + free(op);
> +}
> +
> +static uint16_t process_error(const void *pdu, uint16_t length)
> +{
> + if (!pdu || length != 4)
> + return BT_GATT_ERROR_INVALID_RSP;
> +
> + return (uint16_t)((uint8_t *) pdu)[3];
> +}
> +
> +static void mtu_cb(uint8_t opcode, const void *pdu, uint16_t length,
> + void *user_data)
> +{
> + struct mtu_op *op = user_data;
> + uint16_t result = 0;
> + uint16_t server_rx_mtu;
> +
> + if (opcode == BT_ATT_OP_ERROR_RSP) {
> + result = process_error(pdu, length);
> + goto done;
> + }
> +
> + if (opcode != BT_ATT_OP_MTU_RSP || !pdu || length != 2) {
> + result = BT_GATT_ERROR_INVALID_RSP;
> + goto done;
> + }
> +
> + server_rx_mtu = get_le16(pdu);
> + bt_att_set_mtu(op->att, MIN(op->client_rx_mtu, server_rx_mtu));
> +
> +done:
> + if (op->callback)
> + op->callback(result, op->user_data);
> +}
>
> bool bt_gatt_exchange_mtu(struct bt_att *att, uint16_t client_rx_mtu,
> bt_gatt_result_callback_t callback,
> void *user_data,
> bt_gatt_destroy_func_t destroy)
> {
> - /* TODO */
> - return false;
> + struct mtu_op *op;
> + uint8_t pdu[2];
> +
> + if (!att)
> + return false;
> +
> + if (!client_rx_mtu)
> + return false;
> +
Would it be better to include att and client_rx_mtu check in one "if"
statement ? (both of them return false)

> + op = new0(struct mtu_op, 1);
> + if (!op)
> + return false;
> +
> + op->att = att;
> + op->client_rx_mtu = client_rx_mtu;
> + op->callback = callback;
> + op->user_data = user_data;
> + op->destroy = destroy;
> +
> + put_le16(client_rx_mtu, pdu);
> +
> + if (!bt_att_send(att, BT_ATT_OP_MTU_REQ, pdu, sizeof(pdu),
> + mtu_cb, op,
> + destroy_mtu_op)) {
> + free(op);
> + return false;
> + }
> +
> + return true;
> }
>
> bool bt_gatt_discover_primary_services(struct bt_att *att,
> --
> 2.0.0.526.g5318336
>
> --
> 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

Best regards,
Grzegorz

2014-07-18 21:13:49

by Arman Uguray

[permalink] [raw]
Subject: [PATCH 03/11] shared/gatt: Implement "Discover All Primary Services" procedure.

This patch implements bt_gatt_discover_primary_services for the case when no
UUID is provided.
---
src/shared/gatt-helpers.c | 197 +++++++++++++++++++++++++++++++++++++++++++++-
src/shared/gatt-helpers.h | 1 +
2 files changed, 196 insertions(+), 2 deletions(-)

diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index 9cf05cf..4fbf2eb 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -36,6 +36,11 @@
#define MIN(a, b) ((a) < (b) ? (a) : (b))
#endif

+static uint8_t bt_base_uuid[16] = {
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00,
+ 0x80, 0x00, 0x00, 0x80, 0x5F, 0x9B, 0x34, 0xFB
+};
+
struct mtu_op {
struct bt_att *att;
uint16_t client_rx_mtu;
@@ -123,14 +128,202 @@ bool bt_gatt_exchange_mtu(struct bt_att *att, uint16_t client_rx_mtu,
return true;
}

+struct discovery_op {
+ struct bt_att *att;
+ int ref_count;
+ bt_uuid_t uuid;
+ struct queue *results;
+ bt_gatt_discovery_callback_t callback;
+ void *user_data;
+ bt_gatt_destroy_func_t destroy;
+};
+
+static struct discovery_op* discovery_op_ref(struct discovery_op *op)
+{
+ __sync_fetch_and_add(&op->ref_count, 1);
+
+ return op;
+}
+
+static void discovery_op_unref(void *data)
+{
+ struct discovery_op *op = data;
+
+ if (__sync_sub_and_fetch(&op->ref_count, 1))
+ return;
+
+ if (op->destroy)
+ op->destroy(op->user_data);
+
+ queue_destroy(op->results, free);
+
+ free(op);
+}
+
+static void put_uuid_le(const bt_uuid_t *src, void *dst)
+{
+ if (src->type == BT_UUID16)
+ put_le16(src->value.u16, dst);
+ else
+ bswap_128(&src->value.u128, dst);
+}
+
+static bool convert_uuid_le(const uint8_t *src, size_t len, uint8_t dst[16])
+{
+ if (len == 16) {
+ bswap_128(src, dst);
+ return true;
+ }
+
+ if (len != 2)
+ return false;
+
+ memcpy(dst, bt_base_uuid, sizeof(bt_base_uuid));
+ dst[2] = src[1];
+ dst[3] = src[0];
+
+ return true;
+}
+
+static void read_by_grp_type_cb(uint8_t opcode, const void *pdu,
+ uint16_t length, void *user_data)
+{
+ struct discovery_op *op = user_data;
+ uint16_t status;
+ struct queue *results = NULL;
+ size_t data_length;
+ size_t list_length;
+ uint16_t last_end;
+ int i;
+
+ if (opcode == BT_ATT_OP_ERROR_RSP) {
+ status = process_error(pdu, length);
+
+ if (status == BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND &&
+ !queue_isempty(op->results))
+ goto success;
+
+ goto done;
+ }
+
+ /* PDU must contain at least the following (sans opcode):
+ * - Attr Data Length (1 octet)
+ * - Attr Data List (at least 6 octets):
+ * -- 2 octets: Attribute handle
+ * -- 2 octets: End group handle
+ * -- 2 or 16 octets: service UUID
+ */
+ if (opcode != BT_ATT_OP_READ_BY_GRP_TYPE_RSP || !pdu || length < 7) {
+ status = BT_GATT_ERROR_INVALID_RSP;
+ goto done;
+ }
+
+ data_length = ((uint8_t *) pdu)[0];
+ list_length = length - 1;
+
+ if ((list_length % data_length) ||
+ (data_length != 6 && data_length != 20)) {
+ status = BT_GATT_ERROR_INVALID_RSP;
+ goto done;
+ }
+
+ for (i = 1; i < length; i += data_length) {
+ struct bt_gatt_service *service;
+
+ service = new0(struct bt_gatt_service, 1);
+ if (!service) {
+ status = BT_GATT_ERROR_FAILED_ALLOC;
+ goto done;
+ }
+
+ service->start = get_le16(pdu + i);
+ last_end = get_le16(pdu + i + 2);
+ service->end = last_end;
+ convert_uuid_le(pdu + i + 4, data_length - 4, service->uuid);
+
+ queue_push_tail(op->results, service);
+ }
+
+ if (last_end != 0xFFFF) {
+ uint8_t pdu[6];
+
+ put_le16(last_end + 1, pdu);
+ put_le16(0xFFFF, pdu + 2);
+ put_le16(GATT_PRIM_SVC_UUID, pdu + 4);
+
+ if (bt_att_send(op->att, BT_ATT_OP_READ_BY_GRP_TYPE_REQ,
+ pdu, sizeof(pdu),
+ read_by_grp_type_cb,
+ discovery_op_ref(op),
+ discovery_op_unref))
+ return;
+
+ discovery_op_unref(op);
+ status = BT_GATT_ERROR_UNKNOWN;
+ goto done;
+ }
+
+success:
+ /* End of procedure */
+ results = op->results;
+ status = 0;
+
+done:
+ if (op->callback)
+ op->callback(status, results, op->user_data);
+}
+
bool bt_gatt_discover_primary_services(struct bt_att *att,
bt_uuid_t *uuid,
bt_gatt_discovery_callback_t callback,
void *user_data,
bt_gatt_destroy_func_t destroy)
{
- /* TODO */
- return false;
+ struct discovery_op *op;
+ bool result;
+
+ if (!att)
+ return false;
+
+ op = new0(struct discovery_op, 1);
+ if (!op)
+ return false;
+
+ op->results = queue_new();
+ if (!op->results) {
+ free(op);
+ return false;
+ }
+
+ op->att = att;
+ op->callback = callback;
+ op->user_data = user_data;
+ op->destroy = destroy;
+
+ /* If UUID is NULL, then discover all primary services */
+ if (!uuid) {
+ uint8_t pdu[6];
+
+ put_le16(0x0001, pdu);
+ put_le16(0xFFFF, pdu + 2);
+ put_le16(GATT_PRIM_SVC_UUID, pdu + 4);
+
+ result = bt_att_send(att, BT_ATT_OP_READ_BY_GRP_TYPE_REQ,
+ pdu, sizeof(pdu),
+ read_by_grp_type_cb,
+ discovery_op_ref(op),
+ discovery_op_unref);
+ } else {
+ free(op);
+ return false;
+ }
+
+ if (!result) {
+ queue_destroy(op->results, free);
+ free(op);
+ }
+
+ return result;
}

bool bt_gatt_discover_included_services(struct bt_att *att,
diff --git a/src/shared/gatt-helpers.h b/src/shared/gatt-helpers.h
index b711324..068c4bb 100644
--- a/src/shared/gatt-helpers.h
+++ b/src/shared/gatt-helpers.h
@@ -62,6 +62,7 @@ struct bt_gatt_descriptor {
*/
#define BT_GATT_ERROR_UNKNOWN 0x0100
#define BT_GATT_ERROR_INVALID_RSP 0x0101
+#define BT_GATT_ERROR_FAILED_ALLOC 0x0102

typedef void (*bt_gatt_destroy_func_t)(void *user_data);

--
2.0.0.526.g5318336


2014-07-18 21:13:57

by Arman Uguray

[permalink] [raw]
Subject: [PATCH 11/11] shared/gatt: Implement notification/indication helper.

This patch implements bt_gatt_register, which sets up internal callbacks for
incoming notifications and indications, parses the PDU and provides it to the
upper layer. It also automatically sends out a confirmation when an indication
is received.
---
src/shared/gatt-helpers.c | 64 +++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 62 insertions(+), 2 deletions(-)

diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index 5afe91e..f82bb59 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -1375,11 +1375,71 @@ bool bt_gatt_write_long_value(struct bt_att *att, bool reliable,
return true;
}

+struct notify_data {
+ struct bt_att *att;
+ bt_gatt_notify_callback_t callback;
+ void *user_data;
+ bt_gatt_destroy_func_t destroy;
+};
+
+static void notify_data_destroy(void *data)
+{
+ struct notify_data *notd = data;
+
+ if (notd->destroy)
+ notd->destroy(notd->user_data);
+
+ free(notd);
+}
+
+static void notify_cb(uint8_t opcode, const void *pdu, uint16_t length,
+ void *user_data)
+{
+ struct notify_data *data = user_data;
+ bool indication;
+ uint16_t value_handle;
+ const uint8_t *value = NULL;
+
+ indication = (opcode == BT_ATT_OP_HANDLE_VAL_IND);
+ value_handle = get_le16(pdu);
+
+ if (length > 2)
+ value = pdu + 2;
+
+ if (data->callback)
+ data->callback(value_handle, value, length - 2, data->user_data);
+
+ if (indication)
+ bt_att_send(data->att, BT_ATT_OP_HANDLE_VAL_CONF, NULL, 0,
+ NULL, NULL, NULL);
+}
+
unsigned int bt_gatt_register(struct bt_att *att, bool indications,
bt_gatt_notify_callback_t callback,
void *user_data,
bt_gatt_destroy_func_t destroy)
{
- /* TODO */
- return false;
+ struct notify_data *data;
+ uint8_t opcode;
+ unsigned int id;
+
+ if (!att)
+ return 0;
+
+ data = new0(struct notify_data, 1);
+ if (!data)
+ return 0;
+
+ data->att = att;
+ data->callback = callback;
+ data->user_data = user_data;
+ data->destroy = destroy;
+
+ opcode = indications ? BT_ATT_OP_HANDLE_VAL_IND : BT_ATT_OP_HANDLE_VAL_NOT;
+
+ id = bt_att_register(att, opcode, notify_cb, data, notify_data_destroy);
+ if (!id)
+ free(data);
+
+ return id;
}
--
2.0.0.526.g5318336


2014-07-18 21:13:51

by Arman Uguray

[permalink] [raw]
Subject: [PATCH 05/11] shared/gatt: Implement "Characteristic Discovery" procedures.

This patch implements bt_gatt_discover_characteristics, which can perform the
characteristic discovery procedures "Discover All Characteristics of a Service"
and "Discover Characteristics by UUID".
---
src/shared/gatt-helpers.c | 174 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 172 insertions(+), 2 deletions(-)

diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index fb5a7b6..ad544f7 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -428,6 +428,136 @@ bool bt_gatt_discover_included_services(struct bt_att *att,
return false;
}

+struct discover_chrcs_op {
+ struct discovery_op data;
+ bool by_uuid;
+ uint16_t end;
+ struct bt_gatt_characteristic *prev_chrc;
+};
+
+static struct discover_chrcs_op *discover_chrcs_op_ref(
+ struct discover_chrcs_op *op)
+{
+ __sync_fetch_and_add(&op->data.ref_count, 1);
+
+ return op;
+}
+
+static void discover_chrcs_op_unref(void *data)
+{
+ struct discover_chrcs_op *op = data;
+
+ if (__sync_sub_and_fetch(&op->data.ref_count, 1))
+ return;
+
+ if (op->data.destroy)
+ op->data.destroy(op->data.user_data);
+
+ queue_destroy(op->data.results, free);
+
+ free(op);
+}
+
+static void discover_chrcs_cb(uint8_t opcode, const void *pdu,
+ uint16_t length, void *user_data)
+{
+ struct discover_chrcs_op *op = user_data;
+ uint16_t status;
+ struct queue *results = NULL;
+ size_t data_length;
+ uint16_t last_handle;
+ int i;
+
+ if (opcode == BT_ATT_OP_ERROR_RSP) {
+ status = process_error(pdu, length);
+
+ if (status == BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND &&
+ !queue_isempty(op->data.results))
+ goto success;
+
+ goto done;
+ }
+
+ /* PDU must contain at least the following (sans opcode):
+ * - Attr Data Length (1 octet)
+ * - Attr Data List (at least 7 octets):
+ * -- 2 octets: Attribute handle
+ * -- 1 octet: Characteristic properties
+ * -- 2 octets: Characteristic value handle
+ * -- 2 or 16 octets: characteristic UUID
+ */
+ if (opcode != BT_ATT_OP_READ_BY_TYPE_RSP || !pdu || length < 8) {
+ status = BT_GATT_ERROR_INVALID_RSP;
+ goto done;
+ }
+
+ data_length = ((uint8_t *) pdu)[0];
+
+ if (((length - 1) % data_length) ||
+ (data_length != 7 && data_length != 21)) {
+ status = BT_GATT_ERROR_INVALID_RSP;
+ goto done;
+ }
+
+ for (i = 1; i < length; i += data_length) {
+ struct bt_gatt_characteristic *chrc;
+ bt_uuid_t uuid;
+
+ chrc = new0(struct bt_gatt_characteristic, 1);
+ if (!chrc) {
+ status = BT_GATT_ERROR_FAILED_ALLOC;
+ goto done;
+ }
+
+ last_handle = get_le16(pdu + i);
+ chrc->start = last_handle;
+ chrc->properties = ((uint8_t *) pdu)[i + 2];
+ chrc->value = get_le16(pdu + i + 3);
+ convert_uuid_le(pdu + i + 5, data_length - 5, chrc->uuid);
+
+ uuid.type = BT_UUID128;
+ memcpy(&uuid.value.u128, chrc->uuid, 16);
+
+ if (op->prev_chrc)
+ op->prev_chrc->end = chrc->start - 1;
+
+ op->prev_chrc = chrc;
+
+ if (!op->by_uuid || !bt_uuid_cmp(&uuid, &op->data.uuid))
+ queue_push_tail(op->data.results, chrc);
+ }
+
+ if (last_handle != op->end) {
+ uint8_t pdu[6];
+
+ put_le16(last_handle + 1, pdu);
+ put_le16(op->end, pdu + 2);
+ put_le16(GATT_CHARAC_UUID, pdu + 4);
+
+ if (bt_att_send(op->data.att, BT_ATT_OP_READ_BY_TYPE_REQ,
+ pdu, sizeof(pdu),
+ discover_chrcs_cb,
+ discover_chrcs_op_ref(op),
+ discover_chrcs_op_unref))
+ return;
+
+ discover_chrcs_op_unref(op);
+ status = BT_GATT_ERROR_UNKNOWN;
+ goto done;
+ }
+
+success:
+ results = op->data.results;
+ status = 0;
+
+ if (op->prev_chrc)
+ op->prev_chrc->end = op->end - 1;
+
+done:
+ if (op->data.callback)
+ op->data.callback(status, results, op->data.user_data);
+}
+
bool bt_gatt_discover_characteristics(struct bt_att *att,
uint16_t start, uint16_t end,
bt_uuid_t *uuid,
@@ -435,8 +565,48 @@ bool bt_gatt_discover_characteristics(struct bt_att *att,
void *user_data,
bt_gatt_destroy_func_t destroy)
{
- /* TODO */
- return false;
+ struct discover_chrcs_op *op;
+ uint8_t pdu[6];
+
+ if (!att)
+ return false;
+
+ op = new0(struct discover_chrcs_op, 1);
+ if (!op)
+ return false;
+
+ op->data.results = queue_new();
+ if (!op->data.results) {
+ free(op);
+ return false;
+ }
+
+ if (uuid) {
+ op->by_uuid = true;
+ op->data.uuid = *uuid;
+ }
+
+ op->data.att = att;
+ op->data.callback = callback;
+ op->data.user_data = user_data;
+ op->data.destroy = destroy;
+ op->end = end;
+
+ put_le16(start, pdu);
+ put_le16(end, pdu + 2);
+ put_le16(GATT_CHARAC_UUID, pdu + 4);
+
+ if (!bt_att_send(att, BT_ATT_OP_READ_BY_TYPE_REQ,
+ pdu, sizeof(pdu),
+ discover_chrcs_cb,
+ discover_chrcs_op_ref(op),
+ discover_chrcs_op_unref)) {
+ queue_destroy(op->data.results, free);
+ free(op);
+ return false;
+ }
+
+ return true;
}

bool bt_gatt_discover_descriptors(struct bt_att *att,
--
2.0.0.526.g5318336


2014-07-18 21:13:56

by Arman Uguray

[permalink] [raw]
Subject: [PATCH 10/11] shared/gatt: Implement "Write Long Values" procedure.

This patch implements bt_gatt_write_long_value.
---
src/shared/gatt-helpers.c | 226 +++++++++++++++++++++++++++++++++++++++++++++-
src/shared/gatt-helpers.h | 1 +
2 files changed, 225 insertions(+), 2 deletions(-)

diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index fdaf973..5afe91e 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -1142,6 +1142,166 @@ bool bt_gatt_write_value(struct bt_att *att, uint16_t value_handle,
return true;
}

+struct write_long_op {
+ struct bt_att *att;
+ int ref_count;
+ bool reliable;
+ uint16_t status;
+ uint16_t value_handle;
+ uint8_t *value;
+ uint16_t length;
+ uint16_t offset;
+ uint16_t index;
+ uint16_t cur_length;
+ bt_gatt_result_callback_t callback;
+ void *user_data;
+ bt_gatt_destroy_func_t destroy;
+};
+
+static struct write_long_op *write_long_op_ref(struct write_long_op *op)
+{
+ __sync_fetch_and_add(&op->ref_count, 1);
+
+ return op;
+}
+
+static void write_long_op_unref(void *data)
+{
+ struct write_long_op *op = data;
+
+ if (__sync_sub_and_fetch(&op->ref_count, 1))
+ return;
+
+ if (op->destroy)
+ op->destroy(op->user_data);
+
+ free(op->value);
+ free(op);
+}
+
+static void execute_write_cb(uint8_t opcode, const void *pdu, uint16_t length,
+ void *user_data)
+{
+ struct write_long_op *op = user_data;
+ uint16_t status = op->status;
+
+ if (opcode == BT_ATT_OP_ERROR_RSP)
+ status = process_error(pdu, length);
+ else if (opcode != BT_ATT_OP_EXEC_WRITE_RSP || pdu || length)
+ status = BT_GATT_ERROR_INVALID_RSP;
+
+ if (op->callback)
+ op->callback(status, op->user_data);
+}
+
+static void complete_write_long_op(struct write_long_op *op, uint16_t status)
+{
+ uint8_t pdu;
+
+ op->status = status;
+
+ if (status) {
+ pdu = 0x00; /* Cancel */
+ } else
+ pdu = 0x01; /* Write */
+
+ if (bt_att_send(op->att, BT_ATT_OP_EXEC_WRITE_REQ, &pdu, sizeof(pdu),
+ execute_write_cb,
+ write_long_op_ref(op),
+ write_long_op_unref))
+ return;
+
+ write_long_op_unref(op);
+ status = BT_GATT_ERROR_UNKNOWN;
+
+ if (op->callback)
+ op->callback(status, op->user_data);
+}
+
+static void prepare_write_cb(uint8_t opcode, const void *pdu, uint16_t length,
+ void *user_data)
+{
+ struct write_long_op *op = user_data;
+ uint16_t status = 0;
+ uint16_t next_index;
+
+ if (opcode == BT_ATT_OP_ERROR_RSP) {
+ status = process_error(pdu, length);
+ goto done;
+ }
+
+ if (opcode != BT_ATT_OP_PREP_WRITE_RSP) {
+ status = BT_GATT_ERROR_INVALID_RSP;
+ goto done;
+ }
+
+ if (op->reliable) {
+ if (!pdu || length != (op->cur_length + 4)) {
+ status = BT_GATT_ERROR_RELIABLE_WRITE;
+ goto done;
+ }
+
+ if (get_le16(pdu) != op->value_handle ||
+ get_le16(pdu + 2) != (op->offset + op->index)) {
+ status = BT_GATT_ERROR_RELIABLE_WRITE;
+ goto done;
+ }
+
+ if (memcmp(pdu + 4, op->value + op->index, op->cur_length)) {
+ status = BT_GATT_ERROR_RELIABLE_WRITE;
+ goto done;
+ }
+ }
+
+ next_index = op->index + op->cur_length;
+ if (next_index == op->length) {
+ /* All bytes written */
+ goto done;
+ }
+
+ /* If the last written length greater than or equal to what can fit
+ * inside a PDU, then there is more data to send.
+ */
+ if (op->cur_length >= bt_att_get_mtu(op->att) - 5) {
+ uint8_t *pdu;
+
+ op->index = next_index;
+ op->cur_length = MIN(op->length - op->index,
+ bt_att_get_mtu(op->att) - 5);
+
+ pdu = malloc(op->cur_length + 4);
+ if (!pdu) {
+ status = BT_GATT_ERROR_FAILED_ALLOC;
+ goto done;
+ }
+
+ put_le16(op->value_handle, pdu);
+ put_le16(op->offset + op->index, pdu + 2);
+ memcpy(pdu + 4, op->value + op->index, op->cur_length);
+
+ if (!bt_att_send(op->att, BT_ATT_OP_PREP_WRITE_REQ,
+ pdu, op->cur_length + 4,
+ prepare_write_cb,
+ write_long_op_ref(op),
+ write_long_op_unref)) {
+ write_long_op_unref(op);
+ status = BT_GATT_ERROR_UNKNOWN;
+ }
+
+ free(pdu);
+
+ /* If status is 0, then the operation should continue.
+ * Otherwise, there was an error and the procedure should be
+ * completed.
+ */
+ if (!status)
+ return;
+ }
+
+done:
+ complete_write_long_op(op, status);
+}
+
bool bt_gatt_write_long_value(struct bt_att *att, bool reliable,
uint16_t value_handle, uint16_t offset,
uint8_t *value, uint16_t length,
@@ -1149,8 +1309,70 @@ bool bt_gatt_write_long_value(struct bt_att *att, bool reliable,
void *user_data,
bt_gatt_destroy_func_t destroy)
{
- /* TODO */
- return false;
+ struct write_long_op *op;
+ uint8_t *pdu;
+ bool status;
+
+ if (!att)
+ return false;
+
+ if ((size_t)(length + offset) > UINT16_MAX)
+ return false;
+
+ /* Don't allow riting a 0-length value using this procedure. The
+ * upper-layer should use bt_gatt_write_value for that instead.
+ */
+ if (!length || !value)
+ return false;
+
+ op = new0(struct write_long_op, 1);
+ if (!op)
+ return false;
+
+ op->value = malloc(length);
+ if (!op->value) {
+ free(op);
+ return false;
+ }
+
+ memcpy(op->value, value, length);
+
+ op->att = att;
+ op->reliable = reliable;
+ op->value_handle = value_handle;
+ op->length = length;
+ op->offset = offset;
+ op->cur_length = MIN(length, bt_att_get_mtu(att) - 5);
+ op->callback = callback;
+ op->user_data = user_data;
+ op->destroy = destroy;
+
+ pdu = malloc(op->cur_length + 4);
+ if (!pdu) {
+ free(op->value);
+ free(op);
+ return false;
+ }
+
+ put_le16(value_handle, pdu);
+ put_le16(offset, pdu + 2);
+ memcpy(pdu + 4, op->value, op->cur_length);
+
+ status = bt_att_send(att, BT_ATT_OP_PREP_WRITE_REQ,
+ pdu, op->cur_length + 4,
+ prepare_write_cb,
+ write_long_op_ref(op),
+ write_long_op_unref);
+
+ free(pdu);
+
+ if (!status) {
+ free(op->value);
+ free(op);
+ return false;
+ }
+
+ return true;
}

unsigned int bt_gatt_register(struct bt_att *att, bool indications,
diff --git a/src/shared/gatt-helpers.h b/src/shared/gatt-helpers.h
index 068c4bb..56aff24 100644
--- a/src/shared/gatt-helpers.h
+++ b/src/shared/gatt-helpers.h
@@ -63,6 +63,7 @@ struct bt_gatt_descriptor {
#define BT_GATT_ERROR_UNKNOWN 0x0100
#define BT_GATT_ERROR_INVALID_RSP 0x0101
#define BT_GATT_ERROR_FAILED_ALLOC 0x0102
+#define BT_GATT_ERROR_RELIABLE_WRITE 0x0103

typedef void (*bt_gatt_destroy_func_t)(void *user_data);

--
2.0.0.526.g5318336


2014-07-18 21:13:52

by Arman Uguray

[permalink] [raw]
Subject: [PATCH 06/11] shared/gatt: Implement "Descriptor Discovery" procedure.

This patch implements bt_gatt_discover_descriptors.
---
src/shared/gatt-helpers.c | 156 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 154 insertions(+), 2 deletions(-)

diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index ad544f7..302cc89 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -609,14 +609,166 @@ bool bt_gatt_discover_characteristics(struct bt_att *att,
return true;
}

+struct discover_descs_op {
+ struct discovery_op data;
+ uint16_t end;
+};
+
+static struct discover_descs_op *discover_descs_op_ref(
+ struct discover_descs_op *op)
+{
+ __sync_fetch_and_add(&op->data.ref_count, 1);
+
+ return op;
+}
+
+static void discover_descs_op_unref(void *data)
+{
+ struct discover_descs_op *op = data;
+
+ if (__sync_sub_and_fetch(&op->data.ref_count, 1))
+ return;
+
+ if (op->data.destroy)
+ op->data.destroy(op->data.user_data);
+
+ queue_destroy(op->data.results, free);
+
+ free(op);
+}
+
+static void discover_descs_cb(uint8_t opcode, const void *pdu,
+ uint16_t length, void *user_data)
+{
+ struct discover_descs_op *op = user_data;
+ uint16_t status;
+ struct queue *results = NULL;
+ uint8_t format;
+ uint16_t last_handle;
+ size_t data_length;
+ int i;
+
+ if (opcode == BT_ATT_OP_ERROR_RSP) {
+ status = process_error(pdu, length);
+
+ if (status == BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND &&
+ !queue_isempty(op->data.results))
+ goto success;
+
+ goto done;
+ }
+
+ /* The PDU should contain the following data (sans opcode):
+ * - Format (1 octet)
+ * - Attr Data List (at least 4 octets):
+ * -- 2 octets: Attribute handle
+ * -- 2 or 16 octets: UUID.
+ */
+ if (opcode != BT_ATT_OP_FIND_INFO_RSP || !pdu || length < 5) {
+ status = BT_GATT_ERROR_INVALID_RSP;
+ goto done;
+ }
+
+ format = ((uint8_t *) pdu)[0];
+
+ if (format == 0x01)
+ data_length = 4;
+ else if (format == 0x02)
+ data_length = 18;
+ else {
+ status = BT_GATT_ERROR_INVALID_RSP;
+ goto done;
+ }
+
+ if ((length - 1) % data_length) {
+ status = BT_GATT_ERROR_INVALID_RSP;
+ goto done;
+ }
+
+ for (i = 1; i < length; i += data_length) {
+ struct bt_gatt_descriptor *descr;
+
+ descr = new0(struct bt_gatt_descriptor, 1);
+ if (!descr) {
+ status = BT_GATT_ERROR_FAILED_ALLOC;
+ goto done;
+ }
+
+ last_handle = get_le16(pdu + i);
+ descr->handle = last_handle;
+ convert_uuid_le(pdu + i + 2, data_length - 2, descr->uuid);
+
+ queue_push_tail(op->data.results, descr);
+ }
+
+ if (last_handle != op->end) {
+ uint8_t pdu[4];
+
+ put_le16(last_handle + 1, pdu);
+ put_le16(op->end, pdu + 2);
+
+ if (bt_att_send(op->data.att, BT_ATT_OP_FIND_INFO_REQ,
+ pdu, sizeof(pdu),
+ discover_descs_cb,
+ discover_descs_op_ref(op),
+ discover_descs_op_unref))
+ return;
+
+ discover_descs_op_unref(op);
+ status = BT_GATT_ERROR_UNKNOWN;
+ goto done;
+ }
+
+success:
+ results = op->data.results;
+ status = 0;
+
+done:
+ if (op->data.callback)
+ op->data.callback(status, results, op->data.user_data);
+}
+
bool bt_gatt_discover_descriptors(struct bt_att *att,
uint16_t start, uint16_t end,
bt_gatt_discovery_callback_t callback,
void *user_data,
bt_gatt_destroy_func_t destroy)
{
- /* TODO */
- return false;
+ struct discover_descs_op *op;
+ uint8_t pdu[4];
+
+ if (!att)
+ return false;
+
+ op = new0(struct discover_descs_op, 1);
+ if (!op)
+ return false;
+
+ op->data.results = queue_new();
+ if (!op->data.results) {
+ free(op);
+ return false;
+ }
+
+ op->data.att = att;
+ op->data.callback = callback;
+ op->data.user_data = user_data;
+ op->data.destroy = destroy;
+ op->end = end;
+
+ put_le16(start, pdu);
+ put_le16(end, pdu + 2);
+
+ if (!bt_att_send(att, BT_ATT_OP_FIND_INFO_REQ, pdu, sizeof(pdu),
+ discover_descs_cb,
+ discover_descs_op_ref(op),
+ discover_descs_op_unref)) {
+ queue_destroy(op->data.results, free);
+ free(op);
+ return false;
+ }
+
+ return true;
}

bool bt_gatt_read_value(struct bt_att *att, uint16_t value_handle,
--
2.0.0.526.g5318336


2014-07-18 21:13:53

by Arman Uguray

[permalink] [raw]
Subject: [PATCH 07/11] shared/gatt: Implement "Read" procedure.

This patch implements bt_gatt_read_value, which can be used to read the value of
a characteristic or descriptor.
---
src/shared/gatt-helpers.c | 70 +++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 68 insertions(+), 2 deletions(-)

diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index 302cc89..d614d3e 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -771,13 +771,79 @@ bool bt_gatt_discover_descriptors(struct bt_att *att,
return true;
}

+struct read_op {
+ bt_gatt_read_callback_t callback;
+ void *user_data;
+ bt_gatt_destroy_func_t destroy;
+};
+
+static void destroy_read_op(void *data)
+{
+ struct read_op *op = data;
+
+ if (op->destroy)
+ op->destroy(op->user_data);
+
+ free(op);
+}
+
+static void read_cb(uint8_t opcode, const void *pdu, uint16_t length,
+ void *user_data)
+{
+ struct read_op *op = user_data;
+ uint16_t status;
+ const uint8_t *value = NULL;
+ uint16_t value_len = 0;
+
+ if (opcode == BT_ATT_OP_ERROR_RSP) {
+ status = process_error(pdu, length);
+ goto done;
+ }
+
+ if (opcode != BT_ATT_OP_READ_RSP || (!pdu && length)) {
+ status = BT_GATT_ERROR_INVALID_RSP;
+ goto done;
+ }
+
+ status = 0;
+ value_len = length;
+ if (value_len)
+ value = pdu;
+
+done:
+ if (op->callback)
+ op->callback(status, value, length, op->user_data);
+}
+
bool bt_gatt_read_value(struct bt_att *att, uint16_t value_handle,
bt_gatt_read_callback_t callback,
void *user_data,
bt_gatt_destroy_func_t destroy)
{
- /* TODO */
- return false;
+ struct read_op *op;
+ uint8_t pdu[2];
+
+ if (!att)
+ return false;
+
+ op = new0(struct read_op, 1);
+ if (!op)
+ return false;
+
+ op->callback = callback;
+ op->user_data = user_data;
+ op->destroy = destroy;
+
+ put_le16(value_handle, pdu);
+
+ if (!bt_att_send(att, BT_ATT_OP_READ_REQ, pdu, sizeof(pdu),
+ read_cb, op,
+ destroy_read_op)) {
+ free(op);
+ return false;
+ }
+
+ return true;
}

bool bt_gatt_read_long_value(struct bt_att *att,
--
2.0.0.526.g5318336


2014-07-18 21:13:55

by Arman Uguray

[permalink] [raw]
Subject: [PATCH 09/11] shared/gatt: Implement "Write Value" and "Write Without Response" procedures.

This patch implements the bt_gatt_write_without_response and bt_gatt_write_value
functions.
---
src/shared/gatt-helpers.c | 80 ++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 75 insertions(+), 5 deletions(-)

diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index 8db80e7..fdaf973 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -1055,11 +1055,58 @@ bool bt_gatt_read_long_value(struct bt_att *att,

bool bt_gatt_write_without_response(struct bt_att *att,
uint16_t value_handle,
- bool signed_write,
+ bool signed_write,
uint8_t *value, uint16_t length)
{
- /* TODO */
- return false;
+ uint8_t pdu[2 + length];
+
+ if (!att)
+ return 0;
+
+ /* TODO: Support this once bt_att_send supports signed writes. */
+ if (signed_write)
+ return 0;
+
+ put_le16(value_handle, pdu);
+ memcpy(pdu + 2, value, length);
+
+ return bt_att_send(att, BT_ATT_OP_WRITE_CMD, pdu, sizeof(pdu),
+ NULL, NULL, NULL);
+}
+
+struct write_op {
+ bt_gatt_result_callback_t callback;
+ void *user_data;
+ bt_gatt_destroy_func_t destroy;
+};
+
+static void destroy_write_op(void *data)
+{
+ struct write_op *op = data;
+
+ if (op->destroy)
+ op->destroy(op->user_data);
+
+ free(op);
+}
+
+static void write_cb(uint8_t opcode, const void *pdu, uint16_t length,
+ void *user_data)
+{
+ struct write_op *op = user_data;
+ uint16_t status = 0;
+
+ if (opcode == BT_ATT_OP_ERROR_RSP) {
+ status = process_error(pdu, length);
+ goto done;
+ }
+
+ if (opcode != BT_ATT_OP_WRITE_RSP || pdu || length)
+ status = BT_GATT_ERROR_INVALID_RSP;
+
+done:
+ if (op->callback)
+ op->callback(status, op->user_data);
}

bool bt_gatt_write_value(struct bt_att *att, uint16_t value_handle,
@@ -1068,8 +1115,31 @@ bool bt_gatt_write_value(struct bt_att *att, uint16_t value_handle,
void *user_data,
bt_gatt_destroy_func_t destroy)
{
- /* TODO */
- return false;
+ struct write_op *op;
+ uint8_t pdu[2 + length];
+
+ if (!att)
+ return false;
+
+ op = new0(struct write_op, 1);
+ if (!op)
+ return false;
+
+ op->callback = callback;
+ op->user_data = user_data;
+ op->destroy = destroy;
+
+ put_le16(value_handle, pdu);
+ memcpy(pdu + 2, value, length);
+
+ if (!bt_att_send(att, BT_ATT_OP_WRITE_REQ, pdu, sizeof(pdu),
+ write_cb, op,
+ destroy_write_op)) {
+ free(op);
+ return false;
+ }
+
+ return true;
}

bool bt_gatt_write_long_value(struct bt_att *att, bool reliable,
--
2.0.0.526.g5318336


2014-07-18 21:13:54

by Arman Uguray

[permalink] [raw]
Subject: [PATCH 08/11] shared/gatt: Implement "Read Long Characteristic Values" procedure.

This patch implements bt_gatt_read_long_value.
---
src/shared/gatt-helpers.c | 201 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 199 insertions(+), 2 deletions(-)

diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index d614d3e..8db80e7 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -846,14 +846,211 @@ bool bt_gatt_read_value(struct bt_att *att, uint16_t value_handle,
return true;
}

+struct read_long_op {
+ struct bt_att *att;
+ int ref_count;
+ uint16_t value_handle;
+ size_t orig_offset;
+ size_t offset;
+ struct queue *blobs;
+ bt_gatt_read_callback_t callback;
+ void *user_data;
+ bt_gatt_destroy_func_t destroy;
+};
+
+struct blob {
+ uint8_t *data;
+ uint16_t offset;
+ uint16_t length;
+};
+
+static struct blob *create_blob(const uint8_t *data, uint16_t len,
+ uint16_t offset)
+{
+ struct blob *blob;
+
+ blob = new0(struct blob, 1);
+ if (!blob)
+ return NULL;
+
+ blob->data = malloc(len);
+ if (!blob->data) {
+ free(blob);
+ return NULL;
+ }
+
+ memcpy(blob->data, data, len);
+ blob->length = len;
+ blob->offset = offset;
+
+ return blob;
+}
+
+static void destroy_blob(void *data)
+{
+ struct blob *blob = data;
+
+ free(blob->data);
+ free(blob);
+}
+
+static struct read_long_op *read_long_op_ref(struct read_long_op *op)
+{
+ __sync_fetch_and_add(&op->ref_count, 1);
+
+ return op;
+}
+
+static void read_long_op_unref(void *data)
+{
+ struct read_long_op *op = data;
+
+ if (__sync_sub_and_fetch(&op->ref_count, 1))
+ return;
+
+ if (op->destroy)
+ op->destroy(op->user_data);
+
+ queue_destroy(op->blobs, destroy_blob);
+
+ free(op);
+}
+
+static void append_blob(void *data, void *user_data)
+{
+ struct blob *blob = data;
+ uint8_t *value = user_data;
+
+ memcpy(value + blob->offset, blob->data, blob->length);
+}
+
+static void complete_read_long_op(struct read_long_op *op, uint16_t status)
+{
+ uint8_t *value = NULL;
+ uint16_t length = 0;
+
+ if (status)
+ goto done;
+
+ length = op->offset - op->orig_offset;
+
+ if (!length)
+ goto done;
+
+ value = malloc(length);
+ if (!value) {
+ status = BT_GATT_ERROR_FAILED_ALLOC;
+ goto done;
+ }
+
+ queue_foreach(op->blobs, append_blob, value - op->orig_offset);
+
+done:
+ if (op->callback)
+ op->callback(status, value, length, op->user_data);
+
+ free(value);
+}
+
+static void read_long_cb(uint8_t opcode, const void *pdu,
+ uint16_t length, void *user_data)
+{
+ struct read_long_op *op = user_data;
+ struct blob *blob;
+ uint16_t status;
+
+ if (opcode == BT_ATT_OP_ERROR_RSP) {
+ status = process_error(pdu, length);
+ goto done;
+ }
+
+ if (opcode != BT_ATT_OP_READ_BLOB_RSP || (!pdu && length)) {
+ status = BT_GATT_ERROR_INVALID_RSP;
+ goto done;
+ }
+
+ if (!length)
+ goto success;
+
+ blob = create_blob(pdu, length, op->offset);
+ if (!blob) {
+ status = BT_GATT_ERROR_FAILED_ALLOC;
+ goto done;
+ }
+
+ queue_push_tail(op->blobs, blob);
+ op->offset += length;
+ if (op->offset > UINT16_MAX)
+ goto success;
+
+ if (length >= bt_att_get_mtu(op->att) - 1) {
+ uint8_t pdu[4];
+
+ put_le16(op->value_handle, pdu);
+ put_le16(op->offset, pdu + 2);
+
+ if (bt_att_send(op->att, BT_ATT_OP_READ_BLOB_REQ,
+ pdu, sizeof(pdu),
+ read_long_cb,
+ read_long_op_ref(op),
+ read_long_op_unref))
+ return;
+
+ read_long_op_unref(op);
+ status = BT_GATT_ERROR_UNKNOWN;
+ goto done;
+ }
+
+success:
+ status = 0;
+
+done:
+ complete_read_long_op(op, status);
+}
+
bool bt_gatt_read_long_value(struct bt_att *att,
uint16_t value_handle, uint16_t offset,
bt_gatt_read_callback_t callback,
void *user_data,
bt_gatt_destroy_func_t destroy)
{
- /* TODO */
- return false;
+ struct read_long_op *op;
+ uint8_t pdu[4];
+
+ if (!att)
+ return false;
+
+ op = new0(struct read_long_op, 1);
+ if (!op)
+ return false;
+
+ op->blobs = queue_new();
+ if (!op->blobs) {
+ free(op);
+ return false;
+ }
+
+ op->att = att;
+ op->value_handle = value_handle;
+ op->orig_offset = offset;
+ op->offset = offset;
+ op->callback = callback;
+ op->user_data = user_data;
+ op->destroy = destroy;
+
+ put_le16(value_handle, pdu);
+ put_le16(offset, pdu + 2);
+
+ if (!bt_att_send(att, BT_ATT_OP_READ_BLOB_REQ, pdu, sizeof(pdu),
+ read_long_cb,
+ read_long_op_ref(op),
+ read_long_op_unref)) {
+ queue_destroy(op->blobs, free);
+ free(op);
+ return false;
+ }
+
+ return true;
}

bool bt_gatt_write_without_response(struct bt_att *att,
--
2.0.0.526.g5318336


2014-07-18 21:13:50

by Arman Uguray

[permalink] [raw]
Subject: [PATCH 04/11] shared/gatt: Implement "Discover Primary Service by UUID" procedure.

This patch implements bt_gatt_discover_primary_services for the case when a UUID
is provided.
---
src/shared/gatt-helpers.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 93 insertions(+), 2 deletions(-)

diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index 4fbf2eb..fb5a7b6 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -164,6 +164,8 @@ static void put_uuid_le(const bt_uuid_t *src, void *dst)
{
if (src->type == BT_UUID16)
put_le16(src->value.u16, dst);
+ else if (src->type == BT_UUID32)
+ put_le32(src->value.u32, dst);
else
bswap_128(&src->value.u128, dst);
}
@@ -273,6 +275,82 @@ done:
op->callback(status, results, op->user_data);
}

+static void find_by_type_val_cb(uint8_t opcode, const void *pdu,
+ uint16_t length, void *user_data)
+{
+ struct discovery_op *op = user_data;
+ uint16_t status = 0;
+ struct queue *results = NULL;
+ uint16_t last_end;
+ int i;
+
+ if (opcode == BT_ATT_OP_ERROR_RSP) {
+ status = process_error(pdu, length);
+
+ if (status == BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND &&
+ !queue_isempty(op->results))
+ goto success;
+
+ goto done;
+ }
+
+ /* PDU must contain 4 bytes and it must be a multiple of 4, where each
+ * 4 bytes contain the 16-bit attribute and group end handles.
+ */
+ if (opcode != BT_ATT_OP_FIND_BY_TYPE_VAL_RSP || !pdu || !length ||
+ length % 4) {
+ status = BT_GATT_ERROR_INVALID_RSP;
+ goto done;
+ }
+
+ for (i = 0; i < length; i += 4) {
+ struct bt_gatt_service *service;
+ bt_uuid_t uuid;
+
+ service = new0(struct bt_gatt_service, 1);
+ if (!service)
+ goto done;
+
+ service->start = get_le16(pdu + i);
+ last_end = get_le16(pdu + i + 2);
+ service->end = last_end;
+
+ bt_uuid_to_uuid128(&op->uuid, &uuid);
+ memcpy(service->uuid, uuid.value.u128.data, 16);
+
+ queue_push_tail(op->results, service);
+ }
+
+ if (last_end != 0xFFFF) {
+ uint8_t pdu[6 + bt_uuid_len(&op->uuid)];
+
+ put_le16(last_end + 1, pdu);
+ put_le16(0xFFFF, pdu + 2);
+ put_le16(GATT_PRIM_SVC_UUID, pdu + 4);
+ put_uuid_le(&op->uuid, pdu + 6);
+
+ if (bt_att_send(op->att, BT_ATT_OP_FIND_BY_TYPE_VAL_REQ,
+ pdu, sizeof(pdu),
+ find_by_type_val_cb,
+ discovery_op_ref(op),
+ discovery_op_unref))
+ return;
+
+ discovery_op_unref(op);
+ status = BT_GATT_ERROR_UNKNOWN;
+ goto done;
+ }
+
+success:
+ /* End of procedure */
+ results = op->results;
+ status = 0;
+
+done:
+ if (op->callback)
+ op->callback(status, results, op->user_data);
+}
+
bool bt_gatt_discover_primary_services(struct bt_att *att,
bt_uuid_t *uuid,
bt_gatt_discovery_callback_t callback,
@@ -314,8 +392,21 @@ bool bt_gatt_discover_primary_services(struct bt_att *att,
discovery_op_ref(op),
discovery_op_unref);
} else {
- free(op);
- return false;
+ uint8_t pdu[6 + bt_uuid_len(uuid)];
+
+ /* Discover by UUID */
+ op->uuid = *uuid;
+
+ put_le16(0x0001, pdu);
+ put_le16(0xFFFF, pdu + 2);
+ put_le16(GATT_PRIM_SVC_UUID, pdu + 4);
+ put_uuid_le(&op->uuid, pdu + 6);
+
+ result = bt_att_send(att, BT_ATT_OP_FIND_BY_TYPE_VAL_REQ,
+ pdu, sizeof(pdu),
+ find_by_type_val_cb,
+ discovery_op_ref(op),
+ discovery_op_unref);
}

if (!result) {
--
2.0.0.526.g5318336


2014-07-18 21:13:48

by Arman Uguray

[permalink] [raw]
Subject: [PATCH 02/11] shared/gatt: Implement bt_gatt_exchange_mtu.

This patch implements the helper function bt_gatt_exchange_mtu, which performs
an ATT "Exchange MTU" request and resizes the internal buffer based on the
result.
---
src/shared/gatt-helpers.c | 87 +++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 85 insertions(+), 2 deletions(-)

diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index 3d85868..9cf05cf 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -30,14 +30,97 @@
#include "src/shared/att.h"
#include "lib/uuid.h"
#include "src/shared/gatt-helpers.h"
+#include "src/shared/util.h"
+
+#ifndef MIN
+#define MIN(a, b) ((a) < (b) ? (a) : (b))
+#endif
+
+struct mtu_op {
+ struct bt_att *att;
+ uint16_t client_rx_mtu;
+ bt_gatt_result_callback_t callback;
+ void *user_data;
+ bt_gatt_destroy_func_t destroy;
+};
+
+static void destroy_mtu_op(void *user_data)
+{
+ struct mtu_op *op = user_data;
+
+ if (op->destroy)
+ op->destroy(op->user_data);
+
+ free(op);
+}
+
+static uint16_t process_error(const void *pdu, uint16_t length)
+{
+ if (!pdu || length != 4)
+ return BT_GATT_ERROR_INVALID_RSP;
+
+ return (uint16_t)((uint8_t *) pdu)[3];
+}
+
+static void mtu_cb(uint8_t opcode, const void *pdu, uint16_t length,
+ void *user_data)
+{
+ struct mtu_op *op = user_data;
+ uint16_t result = 0;
+ uint16_t server_rx_mtu;
+
+ if (opcode == BT_ATT_OP_ERROR_RSP) {
+ result = process_error(pdu, length);
+ goto done;
+ }
+
+ if (opcode != BT_ATT_OP_MTU_RSP || !pdu || length != 2) {
+ result = BT_GATT_ERROR_INVALID_RSP;
+ goto done;
+ }
+
+ server_rx_mtu = get_le16(pdu);
+ bt_att_set_mtu(op->att, MIN(op->client_rx_mtu, server_rx_mtu));
+
+done:
+ if (op->callback)
+ op->callback(result, op->user_data);
+}

bool bt_gatt_exchange_mtu(struct bt_att *att, uint16_t client_rx_mtu,
bt_gatt_result_callback_t callback,
void *user_data,
bt_gatt_destroy_func_t destroy)
{
- /* TODO */
- return false;
+ struct mtu_op *op;
+ uint8_t pdu[2];
+
+ if (!att)
+ return false;
+
+ if (!client_rx_mtu)
+ return false;
+
+ op = new0(struct mtu_op, 1);
+ if (!op)
+ return false;
+
+ op->att = att;
+ op->client_rx_mtu = client_rx_mtu;
+ op->callback = callback;
+ op->user_data = user_data;
+ op->destroy = destroy;
+
+ put_le16(client_rx_mtu, pdu);
+
+ if (!bt_att_send(att, BT_ATT_OP_MTU_REQ, pdu, sizeof(pdu),
+ mtu_cb, op,
+ destroy_mtu_op)) {
+ free(op);
+ return false;
+ }
+
+ return true;
}

bool bt_gatt_discover_primary_services(struct bt_att *att,
--
2.0.0.526.g5318336


2014-07-18 21:13:47

by Arman Uguray

[permalink] [raw]
Subject: [PATCH 01/11] shared/gatt: Introduce gatt-helpers.h skeleton.

This patch introduces the skeleton for src/shared/gatt-helpers, which defines
helper functions for over-the-air GATT client-side procedures that will be
frequently used by clients. Most of these functions require several sequential
ATT protocol requests and it is useful to abstract these details away from the
upper layer.
---
Makefile.am | 3 +-
src/shared/gatt-helpers.c | 141 ++++++++++++++++++++++++++++++++++++++++++++++
src/shared/gatt-helpers.h | 133 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 276 insertions(+), 1 deletion(-)
create mode 100644 src/shared/gatt-helpers.c
create mode 100644 src/shared/gatt-helpers.h

diff --git a/Makefile.am b/Makefile.am
index 4588ce8..e4f7df2 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -157,7 +157,8 @@ src_bluetoothd_SOURCES = $(builtin_sources) \
src/shared/queue.h src/shared/queue.c \
src/shared/util.h src/shared/util.c \
src/shared/mgmt.h src/shared/mgmt.c \
- src/shared/att-types.h src/shared/att.h src/shared/att.c
+ src/shared/att-types.h src/shared/att.h src/shared/att.c \
+ src/shared/gatt-helpers.h src/shared/gatt-helpers.c
src_bluetoothd_LDADD = lib/libbluetooth-internal.la gdbus/libgdbus-internal.la \
@GLIB_LIBS@ @DBUS_LIBS@ -ldl -lrt
src_bluetoothd_LDFLAGS = $(AM_LDFLAGS) -Wl,--export-dynamic \
diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
new file mode 100644
index 0000000..3d85868
--- /dev/null
+++ b/src/shared/gatt-helpers.c
@@ -0,0 +1,141 @@
+/*
+ *
+ * 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
+ *
+ */
+
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include "src/shared/queue.h"
+#include "src/shared/att.h"
+#include "lib/uuid.h"
+#include "src/shared/gatt-helpers.h"
+
+bool bt_gatt_exchange_mtu(struct bt_att *att, uint16_t client_rx_mtu,
+ bt_gatt_result_callback_t callback,
+ void *user_data,
+ bt_gatt_destroy_func_t destroy)
+{
+ /* TODO */
+ return false;
+}
+
+bool bt_gatt_discover_primary_services(struct bt_att *att,
+ bt_uuid_t *uuid,
+ bt_gatt_discovery_callback_t callback,
+ void *user_data,
+ bt_gatt_destroy_func_t destroy)
+{
+ /* TODO */
+ return false;
+}
+
+bool bt_gatt_discover_included_services(struct bt_att *att,
+ uint16_t start, uint16_t end,
+ bt_uuid_t *uuid,
+ bt_gatt_discovery_callback_t callback,
+ void *user_data,
+ bt_gatt_destroy_func_t destroy)
+{
+ /* TODO */
+ return false;
+}
+
+bool bt_gatt_discover_characteristics(struct bt_att *att,
+ uint16_t start, uint16_t end,
+ bt_uuid_t *uuid,
+ bt_gatt_discovery_callback_t callback,
+ void *user_data,
+ bt_gatt_destroy_func_t destroy)
+{
+ /* TODO */
+ return false;
+}
+
+bool bt_gatt_discover_descriptors(struct bt_att *att,
+ uint16_t start, uint16_t end,
+ bt_gatt_discovery_callback_t callback,
+ void *user_data,
+ bt_gatt_destroy_func_t destroy)
+{
+ /* TODO */
+ return false;
+}
+
+bool bt_gatt_read_value(struct bt_att *att, uint16_t value_handle,
+ bt_gatt_read_callback_t callback,
+ void *user_data,
+ bt_gatt_destroy_func_t destroy)
+{
+ /* TODO */
+ return false;
+}
+
+bool bt_gatt_read_long_value(struct bt_att *att,
+ uint16_t value_handle, uint16_t offset,
+ bt_gatt_read_callback_t callback,
+ void *user_data,
+ bt_gatt_destroy_func_t destroy)
+{
+ /* TODO */
+ return false;
+}
+
+bool bt_gatt_write_without_response(struct bt_att *att,
+ uint16_t value_handle,
+ bool signed_write,
+ uint8_t *value, uint16_t length)
+{
+ /* TODO */
+ return false;
+}
+
+bool bt_gatt_write_value(struct bt_att *att, uint16_t value_handle,
+ uint8_t *value, uint16_t length,
+ bt_gatt_result_callback_t callback,
+ void *user_data,
+ bt_gatt_destroy_func_t destroy)
+{
+ /* TODO */
+ return false;
+}
+
+bool bt_gatt_write_long_value(struct bt_att *att, bool reliable,
+ uint16_t value_handle, uint16_t offset,
+ uint8_t *value, uint16_t length,
+ bt_gatt_result_callback_t callback,
+ void *user_data,
+ bt_gatt_destroy_func_t destroy)
+{
+ /* TODO */
+ return false;
+}
+
+unsigned int bt_gatt_register(struct bt_att *att, bool indications,
+ bt_gatt_notify_callback_t callback,
+ void *user_data,
+ bt_gatt_destroy_func_t destroy)
+{
+ /* TODO */
+ return false;
+}
diff --git a/src/shared/gatt-helpers.h b/src/shared/gatt-helpers.h
new file mode 100644
index 0000000..b711324
--- /dev/null
+++ b/src/shared/gatt-helpers.h
@@ -0,0 +1,133 @@
+/*
+ *
+ * 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
+ *
+ */
+
+/* This file defines helpers for performing client-side procedures defined by
+ * the Generic Attribute Profile.
+ */
+
+#include <stdbool.h>
+#include <stdint.h>
+
+struct bt_gatt_service {
+ uint16_t start;
+ uint16_t end;
+ uint8_t uuid[16];
+};
+
+struct bt_gatt_characteristic {
+ uint16_t start;
+ uint16_t end;
+ uint16_t value;
+ uint8_t properties;
+ uint8_t uuid[16];
+};
+
+struct bt_gatt_descriptor {
+ uint16_t handle;
+ uint8_t uuid[16];
+};
+
+/* Operations can report two kinds of errors:
+ *
+ * 1. ATT protocol error codes
+ * 2. bluetoothd defined errors
+ *
+ * The callbacks below encode the operation status in a 16-bit unsigned integer,
+ * where 0-255 are allocated for ATT protocol errors.
+ *
+ * - 0x0000: Success
+ * - 0x0001: Invalid handle (ATT protocol error)
+ * - 0x0100: Unknown failure (bluetoothd defined)
+ *
+ */
+#define BT_GATT_ERROR_UNKNOWN 0x0100
+#define BT_GATT_ERROR_INVALID_RSP 0x0101
+
+typedef void (*bt_gatt_destroy_func_t)(void *user_data);
+
+typedef void (*bt_gatt_result_callback_t)(uint16_t status, void *user_data);
+typedef void (*bt_gatt_discovery_callback_t)(uint16_t status,
+ struct queue *results, void *user_data);
+typedef void (*bt_gatt_read_callback_t)(uint16_t status, const uint8_t *value,
+ uint16_t length, void *user_data);
+
+typedef void (*bt_gatt_notify_callback_t)(uint16_t value_handle,
+ const uint8_t *value, uint16_t length,
+ void *user_data);
+
+bool bt_gatt_exchange_mtu(struct bt_att *att, uint16_t client_rx_mtu,
+ bt_gatt_result_callback_t callback,
+ void *user_data,
+ bt_gatt_destroy_func_t destroy);
+
+bool bt_gatt_discover_primary_services(struct bt_att *att, bt_uuid_t *uuid,
+ bt_gatt_discovery_callback_t callback,
+ void *user_data,
+ bt_gatt_destroy_func_t destroy);
+bool bt_gatt_discover_included_services(struct bt_att *att,
+ uint16_t start, uint16_t end,
+ bt_uuid_t *uuid,
+ bt_gatt_discovery_callback_t callback,
+ void *user_data,
+ bt_gatt_destroy_func_t destroy);
+bool bt_gatt_discover_characteristics(struct bt_att *att,
+ uint16_t start, uint16_t end,
+ bt_uuid_t *uuid,
+ bt_gatt_discovery_callback_t callback,
+ void *user_data,
+ bt_gatt_destroy_func_t destroy);
+bool bt_gatt_discover_descriptors(struct bt_att *att,
+ uint16_t start, uint16_t end,
+ bt_gatt_discovery_callback_t callback,
+ void *user_data,
+ bt_gatt_destroy_func_t destroy);
+
+bool bt_gatt_read_value(struct bt_att *att, uint16_t value_handle,
+ bt_gatt_read_callback_t callback,
+ void *user_data,
+ bt_gatt_destroy_func_t destroy);
+bool bt_gatt_read_long_value(struct bt_att *att,
+ uint16_t value_handle, uint16_t offset,
+ bt_gatt_read_callback_t callback,
+ void *user_data,
+ bt_gatt_destroy_func_t destroy);
+
+bool bt_gatt_write_without_response(struct bt_att *att, uint16_t value_handle,
+ bool signed_write,
+ uint8_t *value, uint16_t length);
+bool bt_gatt_write_value(struct bt_att *att, uint16_t value_handle,
+ uint8_t *value, uint16_t length,
+ bt_gatt_result_callback_t callback,
+ void *user_data,
+ bt_gatt_destroy_func_t destroy);
+bool bt_gatt_write_long_value(struct bt_att *att, bool reliable,
+ uint16_t value_handle, uint16_t offset,
+ uint8_t *value, uint16_t length,
+ bt_gatt_result_callback_t callback,
+ void *user_data,
+ bt_gatt_destroy_func_t destroy);
+
+unsigned int bt_gatt_register(struct bt_att *att, bool indications,
+ bt_gatt_notify_callback_t callback,
+ void *user_data,
+ bt_gatt_destroy_func_t destroy);
--
2.0.0.526.g5318336