After discussions with Marcel, Claudio, and Lizardo, the need for a common
transport structure for GATT/ATT operations with no GLib or connection-specific
dependencies became apparent. This patch set introduces "struct bt_att", which
will be used by the daemon's GATT client implementation (and eventually server).
The vision is to create a brand new ATT/GATT stack based around "struct io" that
is agnostic to the specifics of the underlying connection. The current vision is
to implement bt_att for the ATT layer, a bt_att for the GATT client/server
transactions using bt_att, and integrate this with a shared GATT database
implementation, e.g. src/shared/gatt-db.
Functionality introduced in this patch-set:
- "struct bt_att", with a set of functions that mimic src/shared/mgmt.
- Handling of locally initiated ATT requests and their responses plumbed
through a callback.
- The "Exchange MTU" request and response.
- The "Error Response".
- Unit tests for request-response using a socketpair. Initial test for
"Exchange MTU" request.
Next steps:
- All remaining ATT requests/responses.
- System for notifications/indications.
- A command-line tool for testing live ATT operations.
- A test suite that uses emulation.
- Locally initiated responses for server mode.
Comments are much welcome.
Arman Uguray (2):
src/shared/att: Introduce struct bt_att.
unit/test-att: Add unit tests for src/shared/att.
src/shared/att.c | 554 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
src/shared/att.h | 97 ++++++++++
unit/test-att.c | 307 ++++++++++++++++++++++++++++++
3 files changed, 958 insertions(+)
create mode 100644 src/shared/att.c
create mode 100644 src/shared/att.h
create mode 100644 unit/test-att.c
--
1.9.1.423.g4596e3a
Hi Arman,
On Tue, Apr 29, 2014 at 11:25 AM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Arman,
>
>> +/* Error response */
>> +#define ATT_OP_ERROR_RESP 0x01
>> +struct bt_att_error_rsp_param {
>> + uint8_t request_opcode;
>> + uint16_t handle;
>> + uint8_t error_code;
>> +};
>> +
>> +/* Exchange MTU */
>> +#define ATT_OP_EXCHANGE_MTU_REQ 0x02
>> +struct bt_att_exchange_mtu_req_param {
>> + uint16_t client_rx_mtu;
>> +};
>> +
>> +#define ATT_OP_EXCHANGE_MTU_RESP 0x03
>> +struct bt_att_exchange_mtu_rsp_param {
>> + uint16_t server_rx_mtu;
>> +};
>
> I would keep the PDU internally and have functions for that e.g.
> bt_att_error_rsp, btw you can probably drop the _param suffix since it
> is quite obvious what it is for.
>
>> +/* Error codes for Error response PDU */
>> +#define ATT_ERROR_INVALID_HANDLE 0x01
>> +#define ATT_ERROR_READ_NOT_PERMITTED 0x02
>> +#define ATT_ERROR_WRITE_NOT_PERMITTED 0x03
>> +#define ATT_ERROR_INVALID_PDU 0x04
>> +#define ATT_ERROR_AUTHENTICATION 0x05
>> +#define ATT_ERROR_REQUEST_NOT_SUPPORTED 0x06
>> +#define ATT_ERROR_INVALID_OFFSET 0x07
>> +#define ATT_ERROR_AUTHORIZATION 0x08
>> +#define ATT_ERROR_PREPARE_QUEUE_FULL 0x09
>> +#define ATT_ERROR_ATTRIBUTE_NOT_FOUND 0x0A
>> +#define ATT_ERROR_ATTRIBUTE_NOT_LONG 0x0B
>> +#define ATT_ERROR_INSUFFICIENT_ENCRYPTION_KEY_SIZE 0x0C
>> +#define ATT_ERROR_INVALID_ATTRIBUTE_VALUE_LEN 0x0D
>> +#define ATT_ERROR_UNLIKELY 0x0E
>> +#define ATT_ERROR_INSUFFICIENT_ENCRYPTION 0x0F
>> +#define ATT_ERROR_UNSUPPORTED_GROUP_TYPE 0x10
>> +#define ATT_ERROR_INSUFFICIENT_RESOURCES 0x11
>> +/* Application defined errors */
>> +#define ATT_ERROR_IO 0x80
>> +#define ATT_ERROR_TIMEOUT 0x81
>> +#define ATT_ERROR_ABORTED 0x82
>
> Perhaps we could map errno to ATT errors, that way this can also be
> made internal to the library.
>
>> +typedef void (*bt_att_destroy_func_t)(void *user_data);
>> +
>> +struct bt_att;
>> +
>> +struct bt_att *bt_att_new(int fd);
>> +
>> +struct bt_att *bt_att_ref(struct bt_att *att);
>> +void bt_att_unref(struct bt_att *att);
>> +
>> +typedef void (*bt_att_debug_func_t)(const char *str, void *user_data);
>> +
>> +bool bt_att_set_debug(struct bt_att *att, bt_att_debug_func_t callback,
>> + void *user_data, bt_att_destroy_func_t destroy);
>> +
>> +bool bt_att_set_close_on_unref(struct bt_att *att, bool do_close);
>> +
>> +uint16_t bt_att_get_mtu(struct bt_att *att);
>> +bool bt_att_set_mtu(struct bt_att *att, uint16_t mtu);
>> +
>> +typedef void (*bt_att_request_func_t)(uint8_t opcode, const void *param,
>> + uint16_t length, void *user_data);
>
> I think we should make the callback receive the bt_att session as well.
>
>> +unsigned int bt_att_send(struct bt_att *att, uint8_t opcode,
>> + const void *param, uint16_t length,
>> + bt_att_request_func_t callback, void *user_data,
>> + bt_att_destroy_func_t destroy);
I forget to mention, but it is probably a good idea to start using
iovec instead of a void * so the caller doesn't have to pack
everything together (which probably mean memcpy), this obviously has
to be handled internally and you probably should end up using writev
or sendmsg, but perhaps bt_att_send can be static as well if you have
proper functions mapping the PDUs as I suggested in the previous
email.
>> +bool bt_att_cancel(struct bt_att *att, unsigned int id);
>> +bool bt_att_cancel_all(struct bt_att *att);
>> --
>> 1.9.1.423.g4596e3a
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Luiz Augusto von Dentz
--
Luiz Augusto von Dentz
Hi Arman,
> +/* Error response */
> +#define ATT_OP_ERROR_RESP 0x01
> +struct bt_att_error_rsp_param {
> + uint8_t request_opcode;
> + uint16_t handle;
> + uint8_t error_code;
> +};
> +
> +/* Exchange MTU */
> +#define ATT_OP_EXCHANGE_MTU_REQ 0x02
> +struct bt_att_exchange_mtu_req_param {
> + uint16_t client_rx_mtu;
> +};
> +
> +#define ATT_OP_EXCHANGE_MTU_RESP 0x03
> +struct bt_att_exchange_mtu_rsp_param {
> + uint16_t server_rx_mtu;
> +};
I would keep the PDU internally and have functions for that e.g.
bt_att_error_rsp, btw you can probably drop the _param suffix since it
is quite obvious what it is for.
> +/* Error codes for Error response PDU */
> +#define ATT_ERROR_INVALID_HANDLE 0x01
> +#define ATT_ERROR_READ_NOT_PERMITTED 0x02
> +#define ATT_ERROR_WRITE_NOT_PERMITTED 0x03
> +#define ATT_ERROR_INVALID_PDU 0x04
> +#define ATT_ERROR_AUTHENTICATION 0x05
> +#define ATT_ERROR_REQUEST_NOT_SUPPORTED 0x06
> +#define ATT_ERROR_INVALID_OFFSET 0x07
> +#define ATT_ERROR_AUTHORIZATION 0x08
> +#define ATT_ERROR_PREPARE_QUEUE_FULL 0x09
> +#define ATT_ERROR_ATTRIBUTE_NOT_FOUND 0x0A
> +#define ATT_ERROR_ATTRIBUTE_NOT_LONG 0x0B
> +#define ATT_ERROR_INSUFFICIENT_ENCRYPTION_KEY_SIZE 0x0C
> +#define ATT_ERROR_INVALID_ATTRIBUTE_VALUE_LEN 0x0D
> +#define ATT_ERROR_UNLIKELY 0x0E
> +#define ATT_ERROR_INSUFFICIENT_ENCRYPTION 0x0F
> +#define ATT_ERROR_UNSUPPORTED_GROUP_TYPE 0x10
> +#define ATT_ERROR_INSUFFICIENT_RESOURCES 0x11
> +/* Application defined errors */
> +#define ATT_ERROR_IO 0x80
> +#define ATT_ERROR_TIMEOUT 0x81
> +#define ATT_ERROR_ABORTED 0x82
Perhaps we could map errno to ATT errors, that way this can also be
made internal to the library.
> +typedef void (*bt_att_destroy_func_t)(void *user_data);
> +
> +struct bt_att;
> +
> +struct bt_att *bt_att_new(int fd);
> +
> +struct bt_att *bt_att_ref(struct bt_att *att);
> +void bt_att_unref(struct bt_att *att);
> +
> +typedef void (*bt_att_debug_func_t)(const char *str, void *user_data);
> +
> +bool bt_att_set_debug(struct bt_att *att, bt_att_debug_func_t callback,
> + void *user_data, bt_att_destroy_func_t destroy);
> +
> +bool bt_att_set_close_on_unref(struct bt_att *att, bool do_close);
> +
> +uint16_t bt_att_get_mtu(struct bt_att *att);
> +bool bt_att_set_mtu(struct bt_att *att, uint16_t mtu);
> +
> +typedef void (*bt_att_request_func_t)(uint8_t opcode, const void *param,
> + uint16_t length, void *user_data);
I think we should make the callback receive the bt_att session as well.
> +unsigned int bt_att_send(struct bt_att *att, uint8_t opcode,
> + const void *param, uint16_t length,
> + bt_att_request_func_t callback, void *user_data,
> + bt_att_destroy_func_t destroy);
> +
> +bool bt_att_cancel(struct bt_att *att, unsigned int id);
> +bool bt_att_cancel_all(struct bt_att *att);
> --
> 1.9.1.423.g4596e3a
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Luiz Augusto von Dentz
Hi Claudio,
On Fri, Apr 25, 2014 at 6:42 AM, Claudio Takahasi
<[email protected]> wrote:
> Hi Arman,
>
> Below are some comments in addition to your findings ...
>
> Remember that incoming and outgoing queues are independent, and there
> isn't "flow control" for notification, it can be sent at any time.
Yes, there will be a separate queue for replies and outgoing requests
and no queue for notifications.
>> +struct bt_att_request {
>> + unsigned int id;
>> + uint16_t opcode;
>> + void *pdu;
>> + uint16_t len;
>> + bt_att_request_func_t callback;
>> + bt_att_destroy_func_t destroy;
>> + void *user_data;
>
> PDU could be declared as "uint8_t pdu[]" at the end the struct, but if
> you are trying to keep it aligned with mgmt.c it should be better to
> keep it as pointer.
I did indeed want to keep it consistent with mgmt.c
> I think ATT_ERROR_IO is BlueZ specific (internal). Please make sure if
> this error is being handled properly by the caller, maybe it should
> not be used at all.
Yes, it's BlueZ specific and since it's an I/O error here, I figured
that reporting an internal error made sense. I made sure that
ATT_ERROR_IO was explicitly declared in the header as a possible
application error (similar to what's in attrib/att.h).
Though, I see that it is not being reported properly in the code here.
The callback should be invoked with ATT_OP_ERROR_RESP with a "struct
bt_att_error_rsp_param". Fixed in next patch set.
>> +static bool handle_exchange_mtu_rsp(struct bt_att *att, uint8_t opcode,
>> + const uint8_t *pdu, uint16_t pdu_length)
>> +{
>> + struct bt_att_exchange_mtu_rsp_param param;
>> +
>> + if (pdu_length != 3)
>> + return false;
>
> I noticed that length is always "1" + param. Where "1" refers to ATT opcode.
In this case, yes, the pdu_length is expected to be 1 + sizeof(param),
but this won't always be true. For example, the "Find Information"
response has a variable length, in which case the pdu_length won't
depend on the length of the param structure. This will be more clear
when I upload that patch later.
>> +
>> + param.server_rx_mtu = get_le16(pdu + 1);
>> +
>> + request_complete(att, ATT_OP_EXCHANGE_MTU_REQ, opcode,
>> + ¶m, sizeof(param));
>> +
>> + return true;
>> +}
>> +
>> +static bool handle_response(struct bt_att *att, uint8_t opcode,
>> + const uint8_t *pdu, uint16_t pdu_length)
>> +{
>
> Is it necessary to inform the opcode? Maybe you could use pdu[0].
You're right, it's a bit unnecessary. Changed this to not take in the opcode.
>
>> + if (opcode == ATT_OP_ERROR_RESP)
>> + return handle_error_rsp(att, opcode, pdu, pdu_length);
>> +
>> + if (opcode == ATT_OP_EXCHANGE_MTU_RESP)
>> + return handle_exchange_mtu_rsp(att, opcode, pdu, pdu_length);
>> +
>> + return false;
>> +}
>> +
>> +static void read_watch_destroy(void *user_data)
>> +{
>> + struct bt_att *att = user_data;
>> +
>> + if (att->destroyed) {
>> + queue_destroy(att->pending_list, NULL);
>> + free(att);
>> + }
>> +}
>> +
>> +static bool can_read_data(struct io *io, void *user_data)
>> +{
>> + struct bt_att *att = user_data;
>> + uint8_t *pdu;
>> + ssize_t bytes_read;
>> + uint16_t opcode;
>> +
>> + bytes_read = read(att->fd, att->buf, att->len);
>> + if (bytes_read < 0)
>> + return false;
>> +
>> + util_hexdump('>', att->buf, bytes_read,
>> + att->debug_callback, att->debug_data);
>> +
>> + if (bytes_read < 1)
>> + return true;
>
> Why "1"? What is the smallest ATT PDU? Notification?
It depends. There are a couple of response PDU's that don't have any
parameters (e.g. Write Response). 1 really means "at least one byte
for the opcode". I defined a macro for this to make it more explicit
in the next patch set.
>> +struct bt_att *bt_att_new(int fd)
>> +{
>> + struct bt_att *att;
>> +
>> + if (fd < 0)
>> + return NULL;
>> +
>> + att = new0(struct bt_att, 1);
>
> Missing memory allocation fail checking.
Done.
> A label for cleanup will be more convenient.
Done.
>> +static bool encode_mtu_req(const void *param, uint16_t length, void *buf,
>> + uint16_t mtu, uint16_t *pdu_length)
>> +{
>> + const struct bt_att_exchange_mtu_req_param *cp = param;
>> + const uint16_t len = 3;
>> +
>> + if (!param || !buf)
>> + return false;
>> +
>> + if (length != sizeof(struct bt_att_exchange_mtu_req_param))
>> + return false;
>> +
>> + if (len > mtu)
>> + return false;
>
> If your plan is to use encode_pdu() as a switch for all possible
> requests, maybe it will be better to move the common parameters
> validation to it.
Done.
>> +bool bt_att_cancel(struct bt_att *att, unsigned int id)
>> +{
>
> Does it make sense to cancel only one ATT request?
> In general if some error happens you need to cancel all requests and
> close the socket.
Not sure. I added this to be consistent with mgmt but we can remove it
until we know if we need it or not.
> I recommend do leave them out until you really need these application
> defined errors. Maybe you can propose a solution which doesn't require
> this workaround.
>> +/* Application defined errors */
>> +#define ATT_ERROR_IO 0x80
>> +#define ATT_ERROR_TIMEOUT 0x81
>> +#define ATT_ERROR_ABORTED 0x82
>>
Maybe leave ATT_ERROR_IO for now?
Cheers,
Arman
Hi Arman,
Below are some comments in addition to your findings ...
Remember that incoming and outgoing queues are independent, and there
isn't "flow control" for notification, it can be sent at any time.
On Tue, Apr 15, 2014 at 10:04 PM, Arman Uguray <[email protected]> wrote:
> This patch introduces struct bt_att, which handles the transport and
> encoding/decoding for the ATT protocol. The structure of the code
> follows that of src/shared/mgmt and lib/mgmt.h, where individual
> parameter structures are defined for all ATT protocol requests, responses,
> commands, indications, and notifications. The serialization and
> endianness conversion for all parameters are handled by bt_att.
>
> struct bt_att is based around struct io and operates on a raw file
> descriptor. The initial patch implements the Exchange MTU request &
> response and the Error Response. Currently, only requests that are
> initiated locally are supported.
> ---
> Makefile.am | 3 +-
> src/shared/att.c | 554 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> src/shared/att.h | 97 ++++++++++
> 3 files changed, 653 insertions(+), 1 deletion(-)
> create mode 100644 src/shared/att.c
> create mode 100644 src/shared/att.h
>
> diff --git a/Makefile.am b/Makefile.am
> index f96c700..0c3424f 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -155,7 +155,8 @@ src_bluetoothd_SOURCES = $(builtin_sources) \
> src/shared/timeout.h src/shared/timeout-glib.c \
> 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/mgmt.h src/shared/mgmt.c \
> + src/shared/att.h src/shared/att.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/att.c b/src/shared/att.c
> new file mode 100644
> index 0000000..04b68a9
> --- /dev/null
> +++ b/src/shared/att.c
> @@ -0,0 +1,554 @@
> +/*
> + *
> + * BlueZ - Bluetooth protocol stack for Linux
> + *
> + * Copyright (C) 2012-2014 Intel Corporation. All rights reserved.
> + *
> + *
> + * 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 <stdio.h>
> +#include <unistd.h>
> +#include <string.h>
> +#include <errno.h>
> +
> +#include "src/shared/io.h"
> +#include "src/shared/queue.h"
> +#include "src/shared/util.h"
> +#include "src/shared/att.h"
> +
> +#define ATT_MAX_VALUE_LEN 512
> +#define ATT_DEFAULT_LE_MTU 23
> +
> +struct bt_att {
> + int ref_count;
> + int fd;
> + bool close_on_unref;
> + struct io *io;
> + bool writer_active;
> + struct queue *request_queue;
> + struct queue *pending_list;
> + unsigned int next_request_id;
> + bool destroyed;
> + void *buf;
> + uint16_t len;
> + bt_att_debug_func_t debug_callback;
> + bt_att_destroy_func_t debug_destroy;
> + void *debug_data;
> +};
> +
> +struct bt_att_request {
> + unsigned int id;
> + uint16_t opcode;
> + void *pdu;
> + uint16_t len;
> + bt_att_request_func_t callback;
> + bt_att_destroy_func_t destroy;
> + void *user_data;
PDU could be declared as "uint8_t pdu[]" at the end the struct, but if
you are trying to keep it aligned with mgmt.c it should be better to
keep it as pointer.
> +};
> +
> +static void destroy_request(void *data)
> +{
> + struct bt_att_request *request = data;
> +
> + if (request->destroy)
> + request->destroy(request->user_data);
> +
> + free(request->pdu);
> + free(request);
> +}
> +
> +static bool match_request_id(const void *a, const void *b)
> +{
> + const struct bt_att_request *request = a;
> + unsigned int id = PTR_TO_UINT(b);
> +
> + return request->id == id;
> +}
> +
> +static bool match_request_opcode(const void *a, const void *b)
> +{
> + const struct bt_att_request *request = a;
> + const uint8_t *opcode = b;
> +
> + return request->opcode == *opcode;
> +}
> +
> +static void write_watch_destroy(void *user_data)
> +{
> + struct bt_att *att = user_data;
> +
> + att->writer_active = false;
> +}
> +
> +static bool can_write_data(struct io *io, void *user_data)
> +{
> + struct bt_att *att = user_data;
> + struct bt_att_request *request;
> + ssize_t bytes_written;
> +
> + if (!queue_isempty(att->pending_list))
> + return false;
> +
> + request = queue_pop_head(att->request_queue);
> + if (!request)
> + return false;
> +
> + bytes_written = write(att->fd, request->pdu, request->len);
> + if (bytes_written < 0) {
> + util_debug(att->debug_callback, att->debug_data,
> + "write failed: %s", strerror(errno));
> + if (request->callback)
> + request->callback(ATT_ERROR_IO, NULL, 0,
> + request->user_data);
I think ATT_ERROR_IO is BlueZ specific (internal). Please make sure if
this error is being handled properly by the caller, maybe it should
not be used at all.
> +
> + destroy_request(request);
> + return true;
> + }
> +
> + util_debug(att->debug_callback, att->debug_data,
> + "ATT command 0x%02x", request->opcode);
> +
> + util_hexdump('<', request->pdu, bytes_written,
> + att->debug_callback, att->debug_data);
> +
> + queue_push_tail(att->pending_list, request);
> +
> + return false;
> +}
> +
> +static void wakeup_writer(struct bt_att *att)
> +{
> + if (!queue_isempty(att->pending_list))
> + return;
> +
> + if (att->writer_active)
> + return;
> +
> + io_set_write_handler(att->io, can_write_data, att, write_watch_destroy);
> +}
> +
> +static void request_complete(struct bt_att *att, uint8_t req_opcode,
> + uint8_t rsp_opcode, const void *param,
> + uint16_t length)
> +{
> + struct bt_att_request *request;
> +
> + request = queue_remove_if(att->pending_list,
> + match_request_opcode, &req_opcode);
> +
> + if (request) {
> + if (request->callback)
> + request->callback(rsp_opcode, param, length,
> + request->user_data);
> +
> + destroy_request(request);
> + }
> +
> + if (att->destroyed)
> + return;
> +
> + wakeup_writer(att);
> +}
> +
> +static bool handle_error_rsp(struct bt_att *att, uint8_t opcode,
> + const uint8_t *pdu, uint16_t pdu_length)
> +{
> + struct bt_att_error_rsp_param param;
> +
> + if (pdu_length != 5)
> + return false;
> +
> + memset(¶m, 0, sizeof(param));
> +
> + param.request_opcode = pdu[1];
> + param.handle = get_le16(pdu + 2);
> + param.error_code = pdu[4];
> +
> + request_complete(att, param.request_opcode, opcode,
> + ¶m, sizeof(param));
> +
> + return true;
> +}
> +
> +static bool handle_exchange_mtu_rsp(struct bt_att *att, uint8_t opcode,
> + const uint8_t *pdu, uint16_t pdu_length)
> +{
> + struct bt_att_exchange_mtu_rsp_param param;
> +
> + if (pdu_length != 3)
> + return false;
I noticed that length is always "1" + param. Where "1" refers to ATT opcode.
> +
> + param.server_rx_mtu = get_le16(pdu + 1);
> +
> + request_complete(att, ATT_OP_EXCHANGE_MTU_REQ, opcode,
> + ¶m, sizeof(param));
> +
> + return true;
> +}
> +
> +static bool handle_response(struct bt_att *att, uint8_t opcode,
> + const uint8_t *pdu, uint16_t pdu_length)
> +{
Is it necessary to inform the opcode? Maybe you could use pdu[0].
> + if (opcode == ATT_OP_ERROR_RESP)
> + return handle_error_rsp(att, opcode, pdu, pdu_length);
> +
> + if (opcode == ATT_OP_EXCHANGE_MTU_RESP)
> + return handle_exchange_mtu_rsp(att, opcode, pdu, pdu_length);
> +
> + return false;
> +}
> +
> +static void read_watch_destroy(void *user_data)
> +{
> + struct bt_att *att = user_data;
> +
> + if (att->destroyed) {
> + queue_destroy(att->pending_list, NULL);
> + free(att);
> + }
> +}
> +
> +static bool can_read_data(struct io *io, void *user_data)
> +{
> + struct bt_att *att = user_data;
> + uint8_t *pdu;
> + ssize_t bytes_read;
> + uint16_t opcode;
> +
> + bytes_read = read(att->fd, att->buf, att->len);
> + if (bytes_read < 0)
> + return false;
> +
> + util_hexdump('>', att->buf, bytes_read,
> + att->debug_callback, att->debug_data);
> +
> + if (bytes_read < 1)
> + return true;
Why "1"? What is the smallest ATT PDU? Notification?
> +
> + pdu = att->buf;
> + opcode = pdu[0];
> +
> + /* The opcode is either a response to a pending request, a new request
> + * from a client, or a notification/indication. Handle them separately
> + * here.
> + */
> + /* TODO: Handle other types of data here. */
> + handle_response(att, opcode, pdu, bytes_read);
> +
> + if (att->destroyed)
> + return false;
> +
> + return true;
> +}
> +
> +struct bt_att *bt_att_new(int fd)
> +{
> + struct bt_att *att;
> +
> + if (fd < 0)
> + return NULL;
> +
> + att = new0(struct bt_att, 1);
Missing memory allocation fail checking.
> + att->fd = fd;
> + att->close_on_unref = false;
> +
> + att->len = ATT_DEFAULT_LE_MTU;
> + att->buf = malloc(att->len);
> + if (!att->buf) {
> + free(att);
> + return NULL;
> + }
> +
> + att->io = io_new(fd);
> + if (!att->io) {
> + free(att->buf);
> + free(att);
> + return NULL;
> + }
> +
> + att->request_queue = queue_new();
> + if (!att->request_queue) {
> + io_destroy(att->io);
> + free(att->buf);
> + free(att);
> + return NULL;
> + }
> +
> + att->pending_list = queue_new();
> + if (!att->pending_list) {
> + queue_destroy(att->request_queue, NULL);
> + free(att->buf);
> + free(att);
> + return NULL;
> + }
> +
> + if (!io_set_read_handler(att->io, can_read_data,
> + att, read_watch_destroy)) {
> + queue_destroy(att->pending_list, NULL);
> + queue_destroy(att->request_queue, NULL);
> + free(att->buf);
> + free(att);
> + return NULL;
> + }
> +
> + att->writer_active = false;
A label for cleanup will be more convenient.
> +
> + return bt_att_ref(att);
> +}
> +
> +struct bt_att *bt_att_ref(struct bt_att *att)
> +{
> + if (!att)
> + return NULL;
> +
> + __sync_fetch_and_add(&att->ref_count, 1);
> +
> + return att;
> +}
> +
> +void bt_att_unref(struct bt_att *att)
> +{
> + if (!att)
> + return;
> +
> + if (__sync_sub_and_fetch(&att->ref_count, 1))
> + return;
> +
> + bt_att_cancel_all(att);
> +
> + queue_destroy(att->request_queue, NULL);
> +
> + io_set_write_handler(att->io, NULL, NULL, NULL);
> + io_set_read_handler(att->io, NULL, NULL, NULL);
> +
> + io_destroy(att->io);
> + att->io = NULL;
> +
> + if (att->close_on_unref)
> + close(att->fd);
> +
> + if (att->debug_destroy)
> + att->debug_destroy(att->debug_data);
> +
> + free(att->buf);
> + att->buf = NULL;
> +
> + att->destroyed = true;
> +}
> +
> +bool bt_att_set_debug(struct bt_att *att, bt_att_debug_func_t callback,
> + void *user_data, bt_att_destroy_func_t destroy)
> +{
> + if (!att)
> + return false;
> +
> + if (att->debug_destroy)
> + att->debug_destroy(att->debug_data);
> +
> + att->debug_callback = callback;
> + att->debug_destroy = destroy;
> + att->debug_data = user_data;
> +
> + return true;
> +}
> +
> +bool bt_att_set_close_on_unref(struct bt_att *att, bool do_close)
> +{
> + if (!att)
> + return false;
> +
> + att->close_on_unref = do_close;
> +
> + return true;
> +}
> +
> +uint16_t bt_att_get_mtu(struct bt_att *att)
> +{
> + if (!att)
> + return 0;
> +
> + return att->len;
> +}
> +
> +bool bt_att_set_mtu(struct bt_att *att, uint16_t mtu)
> +{
> + if (!att)
> + return false;
> +
> + if (mtu <= ATT_DEFAULT_LE_MTU)
> + return false;
> +
> + att->len = mtu;
> +
> + return true;
> +}
> +
> +static bool encode_mtu_req(const void *param, uint16_t length, void *buf,
> + uint16_t mtu, uint16_t *pdu_length)
> +{
> + const struct bt_att_exchange_mtu_req_param *cp = param;
> + const uint16_t len = 3;
> +
> + if (!param || !buf)
> + return false;
> +
> + if (length != sizeof(struct bt_att_exchange_mtu_req_param))
> + return false;
> +
> + if (len > mtu)
> + return false;
If your plan is to use encode_pdu() as a switch for all possible
requests, maybe it will be better to move the common parameters
validation to it.
> +
> + put_le16(cp->client_rx_mtu, buf);
> + *pdu_length = len;
> +
> + return true;
> +}
> +
> +static bool encode_pdu(uint8_t opcode, const void *param, uint16_t length,
> + uint16_t mtu, void *pdu, uint16_t *pdu_length)
> +{
> + uint8_t *bytes = pdu;
> + bytes[0] = opcode;
> +
> + /* The length of the PDU depends on the specific ATT method. Special
> + * case the operations with variable-length PDU's here.
> + */
> + if (length == 0) {
> + *pdu_length = 1;
> + return true;
> + }
> +
> + if (opcode == ATT_OP_EXCHANGE_MTU_REQ) {
> + return encode_mtu_req(param, length, bytes + 1,
> + mtu, pdu_length);
> + }
> +
> + return false;
> +}
> +
> +static struct bt_att_request *create_request(uint8_t opcode, const void *param,
> + uint16_t length, void *buf, uint16_t mtu,
> + bt_att_request_func_t callback, void *user_data,
> + bt_att_destroy_func_t destroy)
> +{
> + struct bt_att_request *request;
> +
> + if (!opcode)
> + return NULL;
> +
> + if (length > 0 && !param)
> + return NULL;
> +
> + request = new0(struct bt_att_request, 1);
> + if (!request)
> + return NULL;
> +
> + if (!encode_pdu(opcode, param, length, mtu, buf, &request->len)) {
> + free(request);
> + return NULL;
> + }
> +
> + /* request->len is at least 1 due to the opcode */
> + request->pdu = malloc(request->len);
> + if (!request->pdu) {
> + free(request);
> + return NULL;
> + }
> +
> + if (request->len > 0)
> + memcpy(request->pdu, buf, request->len);
> +
> + request->opcode = opcode;
> + request->callback = callback;
> + request->destroy = destroy;
> + request->user_data = user_data;
> +
> + return request;
> +}
> +
> +unsigned int bt_att_send(struct bt_att *att, uint8_t opcode,
> + const void *param, uint16_t length,
> + bt_att_request_func_t callback, void *user_data,
> + bt_att_destroy_func_t destroy)
> +{
> + struct bt_att_request *request;
> +
> + if (!att)
> + return 0;
> +
> + request = create_request(opcode, param, length, att->buf, att->len,
> + callback, user_data, destroy);
> +
> + if (!request)
> + return 0;
> +
> + if (att->next_request_id < 1)
> + att->next_request_id = 1;
> +
> + request->id = att->next_request_id++;
> +
> + if (!queue_push_tail(att->request_queue, request)) {
> + free(request->pdu);
> + free(request);
> + return 0;
> + }
> +
> + wakeup_writer(att);
> +
> + return request->id;
> +}
> +
> +bool bt_att_cancel(struct bt_att *att, unsigned int id)
> +{
Does it make sense to cancel only one ATT request?
In general if some error happens you need to cancel all requests and
close the socket.
> + struct bt_att_request *request;
> +
> + if (!request || !id)
> + return false;
> +
> + request = queue_remove_if(att->request_queue, match_request_id,
> + UINT_TO_PTR(id));
> + if (request)
> + goto done;
> +
> + request = queue_remove_if(att->pending_list, match_request_id,
> + UINT_TO_PTR(id));
> + if (request)
> + goto done;
> +
> +done:
> + destroy_request(request);
> +
> + wakeup_writer(att);
> +
> + return true;
> +}
> +
> +bool bt_att_cancel_all(struct bt_att *att)
> +{
> + if (!att)
> + return false;
> +
> + queue_remove_all(att->pending_list, NULL, NULL, destroy_request);
> + queue_remove_all(att->request_queue, NULL, NULL, destroy_request);
> +
> + return true;
> +}
> diff --git a/src/shared/att.h b/src/shared/att.h
> new file mode 100644
> index 0000000..570fbd2
> --- /dev/null
> +++ b/src/shared/att.h
> @@ -0,0 +1,97 @@
> +/*
> + *
> + * BlueZ - Bluetooth protocol stack for Linux
> + *
> + * Copyright (C) 2012-2014 Intel Corporation. All rights reserved.
> + *
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + *
> + */
> +
> +#include <stdbool.h>
> +#include <stdint.h>
> +
> +/* Error response */
> +#define ATT_OP_ERROR_RESP 0x01
> +struct bt_att_error_rsp_param {
> + uint8_t request_opcode;
> + uint16_t handle;
> + uint8_t error_code;
> +};
> +
> +/* Exchange MTU */
> +#define ATT_OP_EXCHANGE_MTU_REQ 0x02
> +struct bt_att_exchange_mtu_req_param {
> + uint16_t client_rx_mtu;
> +};
> +
> +#define ATT_OP_EXCHANGE_MTU_RESP 0x03
> +struct bt_att_exchange_mtu_rsp_param {
> + uint16_t server_rx_mtu;
> +};
> +
> +/* Error codes for Error response PDU */
> +#define ATT_ERROR_INVALID_HANDLE 0x01
> +#define ATT_ERROR_READ_NOT_PERMITTED 0x02
> +#define ATT_ERROR_WRITE_NOT_PERMITTED 0x03
> +#define ATT_ERROR_INVALID_PDU 0x04
> +#define ATT_ERROR_AUTHENTICATION 0x05
> +#define ATT_ERROR_REQUEST_NOT_SUPPORTED 0x06
> +#define ATT_ERROR_INVALID_OFFSET 0x07
> +#define ATT_ERROR_AUTHORIZATION 0x08
> +#define ATT_ERROR_PREPARE_QUEUE_FULL 0x09
> +#define ATT_ERROR_ATTRIBUTE_NOT_FOUND 0x0A
> +#define ATT_ERROR_ATTRIBUTE_NOT_LONG 0x0B
> +#define ATT_ERROR_INSUFFICIENT_ENCRYPTION_KEY_SIZE 0x0C
> +#define ATT_ERROR_INVALID_ATTRIBUTE_VALUE_LEN 0x0D
> +#define ATT_ERROR_UNLIKELY 0x0E
> +#define ATT_ERROR_INSUFFICIENT_ENCRYPTION 0x0F
> +#define ATT_ERROR_UNSUPPORTED_GROUP_TYPE 0x10
> +#define ATT_ERROR_INSUFFICIENT_RESOURCES 0x11
I recommend do leave them out until you really need these application
defined errors. Maybe you can propose a solution which doesn't require
this workaround.
> +/* Application defined errors */
> +#define ATT_ERROR_IO 0x80
> +#define ATT_ERROR_TIMEOUT 0x81
> +#define ATT_ERROR_ABORTED 0x82
> +
> +typedef void (*bt_att_destroy_func_t)(void *user_data);
> +
> +struct bt_att;
> +
> +struct bt_att *bt_att_new(int fd);
> +
> +struct bt_att *bt_att_ref(struct bt_att *att);
> +void bt_att_unref(struct bt_att *att);
> +
> +typedef void (*bt_att_debug_func_t)(const char *str, void *user_data);
> +
> +bool bt_att_set_debug(struct bt_att *att, bt_att_debug_func_t callback,
> + void *user_data, bt_att_destroy_func_t destroy);
> +
> +bool bt_att_set_close_on_unref(struct bt_att *att, bool do_close);
> +
> +uint16_t bt_att_get_mtu(struct bt_att *att);
> +bool bt_att_set_mtu(struct bt_att *att, uint16_t mtu);
> +
> +typedef void (*bt_att_request_func_t)(uint8_t opcode, const void *param,
> + uint16_t length, void *user_data);
> +
> +unsigned int bt_att_send(struct bt_att *att, uint8_t opcode,
> + const void *param, uint16_t length,
> + bt_att_request_func_t callback, void *user_data,
> + bt_att_destroy_func_t destroy);
> +
> +bool bt_att_cancel(struct bt_att *att, unsigned int id);
> +bool bt_att_cancel_all(struct bt_att *att);
> --
> 1.9.1.423.g4596e3a
>
Regards,
Claudio
Caught two bugs in bt_att_cancel:
> +bool bt_att_cancel(struct bt_att *att, unsigned int id)
> +{
> + struct bt_att_request *request;
> +
> + if (!request || !id)
> + return false;
This above if statement should be:
if (!att || !id)
return false;
> +
> + request = queue_remove_if(att->request_queue, match_request_id,
> + UINT_TO_PTR(id));
> + if (request)
> + goto done;
> +
> + request = queue_remove_if(att->pending_list, match_request_id,
> + UINT_TO_PTR(id));
> + if (request)
> + goto done;
The above if statement should be:
if (!request)
return false;
> +
> +done:
> + destroy_request(request);
> +
> + wakeup_writer(att);
> +
> + return true;
> +}
-Arman
This patch introduces unit tests for src/shared/att. The code currently
only tests locally initiated requests and responses.
---
Makefile.am | 9 ++
unit/test-att.c | 307 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 316 insertions(+)
create mode 100644 unit/test-att.c
diff --git a/Makefile.am b/Makefile.am
index 0c3424f..04be6c4 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -262,6 +262,15 @@ unit_test_mgmt_SOURCES = unit/test-mgmt.c \
src/shared/mgmt.h src/shared/mgmt.c
unit_test_mgmt_LDADD = @GLIB_LIBS@
+unit_tests += unit/test-att
+
+unit_test_att_SOURCES = unit/test-att.c \
+ src/shared/io.h src/shared/io-glib.c \
+ src/shared/queue.h src/shared/queue.c \
+ src/shared/util.h src/shared/util.c \
+ src/shared/att.h src/shared/att.c
+unit_test_att_LDADD = @GLIB_LIBS@
+
unit_tests += unit/test-sdp
unit_test_sdp_SOURCES = unit/test-sdp.c \
diff --git a/unit/test-att.c b/unit/test-att.c
new file mode 100644
index 0000000..3a3a77d
--- /dev/null
+++ b/unit/test-att.c
@@ -0,0 +1,307 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2014 Intel Corporation. All rights reserved.
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; 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 <stdlib.h>
+#include <stdbool.h>
+#include <unistd.h>
+#include <sys/socket.h>
+
+#include <glib.h>
+
+#include "src/shared/att.h"
+
+struct context {
+ GMainLoop *main_loop;
+ struct bt_att *att_client;
+ guint server_source;
+ GList *handler_list;
+};
+
+enum action {
+ ACTION_PASSED,
+ ACTION_IGNORE,
+};
+
+struct handler {
+ const void *req_data;
+ uint16_t req_size;
+ const void *resp_data;
+ uint16_t resp_size;
+ enum action action;
+};
+
+static void att_debug(const char *str, void *user_data)
+{
+ const char *prefix = user_data;
+
+ g_printf("%s%s\n", prefix, str);
+}
+
+static void context_quit(struct context *context)
+{
+ g_main_loop_quit(context->main_loop);
+}
+
+static void check_actions(struct context *context, int server_socket,
+ const void *data, uint16_t size)
+{
+ GList *list;
+ ssize_t written;
+
+ for (list = g_list_first(context->handler_list); list;
+ list = g_list_next(list)) {
+ struct handler *handler = list->data;
+
+ if (size != handler->req_size)
+ continue;
+
+ if (memcmp(data, handler->req_data, handler->req_size))
+ continue;
+
+ switch (handler->action) {
+ case ACTION_PASSED:
+ if (!handler->resp_data || !handler->resp_size) {
+ context_quit(context);
+ return;
+ }
+
+ /* Test case involves a response. */
+ written = write(server_socket, handler->resp_data,
+ handler->resp_size);
+ g_assert(written == handler->resp_size);
+ return;
+ case ACTION_IGNORE:
+ return;
+ }
+ }
+
+ g_test_message("Request not handled\n");
+ g_assert_not_reached();
+}
+
+static gboolean server_handler(GIOChannel *channel, GIOCondition cond,
+ gpointer user_data)
+{
+ struct context *context = user_data;
+ unsigned char buf[512];
+ ssize_t result;
+ int fd;
+
+ if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP))
+ return FALSE;
+
+ fd = g_io_channel_unix_get_fd(channel);
+
+ result = read(fd, buf, sizeof(buf));
+ if (result < 0)
+ return FALSE;
+
+ check_actions(context, fd, buf, result);
+
+ return TRUE;
+}
+
+static struct context *create_context(void)
+{
+ struct context *context = g_new0(struct context, 1);
+ GIOChannel *channel;
+ int err, sv[2];
+
+ context->main_loop = g_main_loop_new(NULL, FALSE);
+ g_assert(context->main_loop);
+
+ err = socketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CLOEXEC, 0, sv);
+ g_assert(err == 0);
+
+ channel = g_io_channel_unix_new(sv[0]);
+
+ g_io_channel_set_close_on_unref(channel, TRUE);
+ g_io_channel_set_encoding(channel, NULL, NULL);
+ g_io_channel_set_buffered(channel, FALSE);
+
+ context->server_source = g_io_add_watch(channel,
+ G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
+ server_handler, context);
+ g_assert(context->server_source > 0);
+
+ g_io_channel_unref(channel);
+
+ context->att_client = bt_att_new(sv[1]);
+ g_assert(context->att_client);
+
+ if (g_test_verbose() == TRUE)
+ bt_att_set_debug(context->att_client, att_debug, "att: ", NULL);
+
+ bt_att_set_close_on_unref(context->att_client, true);
+
+ return context;
+}
+
+static void execute_context(struct context *context)
+{
+ g_main_loop_run(context->main_loop);
+
+ g_list_free_full(context->handler_list, g_free);
+
+ g_source_remove(context->server_source);
+
+ bt_att_unref(context->att_client);
+
+ g_main_loop_unref(context->main_loop);
+
+ g_free(context);
+}
+
+static void add_action(struct context *context,
+ const void *req_data, uint16_t req_size,
+ const void *resp_data, uint16_t resp_size,
+ enum action action)
+{
+ struct handler *handler = g_new0(struct handler, 1);
+
+ handler->req_data = req_data;
+ handler->req_size = req_size;
+ handler->resp_data = resp_data;
+ handler->resp_size = resp_size;
+ handler->action = action;
+
+ context->handler_list = g_list_append(context->handler_list, handler);
+}
+
+struct request_test_data {
+ uint8_t req_opcode;
+ const void *req_param;
+ uint16_t req_param_length;
+ uint8_t resp_opcode;
+ const void *resp_param;
+ uint16_t resp_param_length;
+ const void *req_data;
+ uint16_t req_size;
+ const void *resp_data;
+ uint16_t resp_size;
+};
+
+struct request_test {
+ const struct request_test_data *test_data;
+ struct context *context;
+};
+
+/* Exchange MTU request <-> response test */
+static const unsigned char exchange_mtu_req_bytes_1[] = { 0x02, 0x32, 0x00 };
+static const struct bt_att_exchange_mtu_req_param exchange_mtu_req_param_1 = {
+ .client_rx_mtu = 50,
+};
+static const unsigned char exchange_mtu_resp_bytes_1[] = { 0x03, 0x30, 0x00 };
+static const struct bt_att_exchange_mtu_rsp_param exchange_mtu_rsp_param_1 = {
+ .server_rx_mtu = 48,
+};
+static const struct request_test_data request_test_1 = {
+ .req_opcode = ATT_OP_EXCHANGE_MTU_REQ,
+ .req_param = &exchange_mtu_req_param_1,
+ .req_param_length = sizeof(exchange_mtu_req_param_1),
+ .resp_opcode = ATT_OP_EXCHANGE_MTU_RESP,
+ .resp_param = &exchange_mtu_rsp_param_1,
+ .resp_param_length = sizeof(exchange_mtu_rsp_param_1),
+ .req_data = exchange_mtu_req_bytes_1,
+ .req_size = sizeof(exchange_mtu_req_bytes_1),
+ .resp_data = exchange_mtu_resp_bytes_1,
+ .resp_size = sizeof(exchange_mtu_resp_bytes_1),
+};
+
+/* Exchange MTU request <-> error response test */
+static const unsigned char exchange_mtu_req_bytes_2[] = { 0x02, 0x32, 0x00 };
+static const struct bt_att_exchange_mtu_req_param exchange_mtu_req_param_2 = {
+ .client_rx_mtu = 50,
+};
+static const unsigned char exchange_mtu_resp_bytes_2[] = {
+ 0x01, 0x02, 0x00, 0x00, 0x06
+};
+static const struct bt_att_error_rsp_param exchange_mtu_rsp_param_2 = {
+ .request_opcode = ATT_OP_EXCHANGE_MTU_REQ,
+ .handle = 0,
+ .error_code = ATT_ERROR_REQUEST_NOT_SUPPORTED,
+};
+static const struct request_test_data request_test_2 = {
+ .req_opcode = ATT_OP_EXCHANGE_MTU_REQ,
+ .req_param = &exchange_mtu_req_param_2,
+ .req_param_length = sizeof(exchange_mtu_req_param_2),
+ .resp_opcode = ATT_OP_ERROR_RESP,
+ .resp_param = &exchange_mtu_rsp_param_2,
+ .resp_param_length = sizeof(exchange_mtu_rsp_param_2),
+ .req_data = exchange_mtu_req_bytes_2,
+ .req_size = sizeof(exchange_mtu_req_bytes_2),
+ .resp_data = exchange_mtu_resp_bytes_2,
+ .resp_size = sizeof(exchange_mtu_resp_bytes_2),
+};
+
+static void test_request_callback(uint8_t opcode, const void *param,
+ uint16_t length, void *user_data)
+{
+ struct request_test *test = user_data;
+ const struct request_test_data *test_data = test->test_data;
+
+ g_assert(opcode == test_data->resp_opcode);
+ g_assert(length == test_data->resp_param_length);
+
+ g_assert(0 == memcmp(param, test_data->resp_param, length));
+
+ context_quit(test->context);
+}
+
+static void test_request(gconstpointer data)
+{
+ struct request_test *test;
+ const struct request_test_data *test_data = data;
+ struct context *context = create_context();
+ unsigned int req_id = 0;
+
+ add_action(context, test_data->req_data, test_data->req_size,
+ test_data->resp_data, test_data->resp_size,
+ ACTION_PASSED);
+
+ test = malloc(sizeof(struct request_test));
+ test->test_data = test_data;
+ test->context = context;
+
+ req_id = bt_att_send(context->att_client,
+ test_data->req_opcode, test_data->req_param,
+ test_data->req_param_length,
+ test_request_callback, test, free);
+ g_assert(req_id);
+
+ execute_context(context);
+}
+
+int main(int argc, char *argv[])
+{
+ g_test_init(&argc, &argv, NULL);
+
+ g_test_add_data_func("/att/request/1", &request_test_1, test_request);
+ g_test_add_data_func("/att/request/2", &request_test_2, test_request);
+
+ return g_test_run();
+}
--
1.9.1.423.g4596e3a
This patch introduces struct bt_att, which handles the transport and
encoding/decoding for the ATT protocol. The structure of the code
follows that of src/shared/mgmt and lib/mgmt.h, where individual
parameter structures are defined for all ATT protocol requests, responses,
commands, indications, and notifications. The serialization and
endianness conversion for all parameters are handled by bt_att.
struct bt_att is based around struct io and operates on a raw file
descriptor. The initial patch implements the Exchange MTU request &
response and the Error Response. Currently, only requests that are
initiated locally are supported.
---
Makefile.am | 3 +-
src/shared/att.c | 554 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
src/shared/att.h | 97 ++++++++++
3 files changed, 653 insertions(+), 1 deletion(-)
create mode 100644 src/shared/att.c
create mode 100644 src/shared/att.h
diff --git a/Makefile.am b/Makefile.am
index f96c700..0c3424f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -155,7 +155,8 @@ src_bluetoothd_SOURCES = $(builtin_sources) \
src/shared/timeout.h src/shared/timeout-glib.c \
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/mgmt.h src/shared/mgmt.c \
+ src/shared/att.h src/shared/att.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/att.c b/src/shared/att.c
new file mode 100644
index 0000000..04b68a9
--- /dev/null
+++ b/src/shared/att.c
@@ -0,0 +1,554 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2012-2014 Intel Corporation. All rights reserved.
+ *
+ *
+ * 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 <stdio.h>
+#include <unistd.h>
+#include <string.h>
+#include <errno.h>
+
+#include "src/shared/io.h"
+#include "src/shared/queue.h"
+#include "src/shared/util.h"
+#include "src/shared/att.h"
+
+#define ATT_MAX_VALUE_LEN 512
+#define ATT_DEFAULT_LE_MTU 23
+
+struct bt_att {
+ int ref_count;
+ int fd;
+ bool close_on_unref;
+ struct io *io;
+ bool writer_active;
+ struct queue *request_queue;
+ struct queue *pending_list;
+ unsigned int next_request_id;
+ bool destroyed;
+ void *buf;
+ uint16_t len;
+ bt_att_debug_func_t debug_callback;
+ bt_att_destroy_func_t debug_destroy;
+ void *debug_data;
+};
+
+struct bt_att_request {
+ unsigned int id;
+ uint16_t opcode;
+ void *pdu;
+ uint16_t len;
+ bt_att_request_func_t callback;
+ bt_att_destroy_func_t destroy;
+ void *user_data;
+};
+
+static void destroy_request(void *data)
+{
+ struct bt_att_request *request = data;
+
+ if (request->destroy)
+ request->destroy(request->user_data);
+
+ free(request->pdu);
+ free(request);
+}
+
+static bool match_request_id(const void *a, const void *b)
+{
+ const struct bt_att_request *request = a;
+ unsigned int id = PTR_TO_UINT(b);
+
+ return request->id == id;
+}
+
+static bool match_request_opcode(const void *a, const void *b)
+{
+ const struct bt_att_request *request = a;
+ const uint8_t *opcode = b;
+
+ return request->opcode == *opcode;
+}
+
+static void write_watch_destroy(void *user_data)
+{
+ struct bt_att *att = user_data;
+
+ att->writer_active = false;
+}
+
+static bool can_write_data(struct io *io, void *user_data)
+{
+ struct bt_att *att = user_data;
+ struct bt_att_request *request;
+ ssize_t bytes_written;
+
+ if (!queue_isempty(att->pending_list))
+ return false;
+
+ request = queue_pop_head(att->request_queue);
+ if (!request)
+ return false;
+
+ bytes_written = write(att->fd, request->pdu, request->len);
+ if (bytes_written < 0) {
+ util_debug(att->debug_callback, att->debug_data,
+ "write failed: %s", strerror(errno));
+ if (request->callback)
+ request->callback(ATT_ERROR_IO, NULL, 0,
+ request->user_data);
+
+ destroy_request(request);
+ return true;
+ }
+
+ util_debug(att->debug_callback, att->debug_data,
+ "ATT command 0x%02x", request->opcode);
+
+ util_hexdump('<', request->pdu, bytes_written,
+ att->debug_callback, att->debug_data);
+
+ queue_push_tail(att->pending_list, request);
+
+ return false;
+}
+
+static void wakeup_writer(struct bt_att *att)
+{
+ if (!queue_isempty(att->pending_list))
+ return;
+
+ if (att->writer_active)
+ return;
+
+ io_set_write_handler(att->io, can_write_data, att, write_watch_destroy);
+}
+
+static void request_complete(struct bt_att *att, uint8_t req_opcode,
+ uint8_t rsp_opcode, const void *param,
+ uint16_t length)
+{
+ struct bt_att_request *request;
+
+ request = queue_remove_if(att->pending_list,
+ match_request_opcode, &req_opcode);
+
+ if (request) {
+ if (request->callback)
+ request->callback(rsp_opcode, param, length,
+ request->user_data);
+
+ destroy_request(request);
+ }
+
+ if (att->destroyed)
+ return;
+
+ wakeup_writer(att);
+}
+
+static bool handle_error_rsp(struct bt_att *att, uint8_t opcode,
+ const uint8_t *pdu, uint16_t pdu_length)
+{
+ struct bt_att_error_rsp_param param;
+
+ if (pdu_length != 5)
+ return false;
+
+ memset(¶m, 0, sizeof(param));
+
+ param.request_opcode = pdu[1];
+ param.handle = get_le16(pdu + 2);
+ param.error_code = pdu[4];
+
+ request_complete(att, param.request_opcode, opcode,
+ ¶m, sizeof(param));
+
+ return true;
+}
+
+static bool handle_exchange_mtu_rsp(struct bt_att *att, uint8_t opcode,
+ const uint8_t *pdu, uint16_t pdu_length)
+{
+ struct bt_att_exchange_mtu_rsp_param param;
+
+ if (pdu_length != 3)
+ return false;
+
+ param.server_rx_mtu = get_le16(pdu + 1);
+
+ request_complete(att, ATT_OP_EXCHANGE_MTU_REQ, opcode,
+ ¶m, sizeof(param));
+
+ return true;
+}
+
+static bool handle_response(struct bt_att *att, uint8_t opcode,
+ const uint8_t *pdu, uint16_t pdu_length)
+{
+ if (opcode == ATT_OP_ERROR_RESP)
+ return handle_error_rsp(att, opcode, pdu, pdu_length);
+
+ if (opcode == ATT_OP_EXCHANGE_MTU_RESP)
+ return handle_exchange_mtu_rsp(att, opcode, pdu, pdu_length);
+
+ return false;
+}
+
+static void read_watch_destroy(void *user_data)
+{
+ struct bt_att *att = user_data;
+
+ if (att->destroyed) {
+ queue_destroy(att->pending_list, NULL);
+ free(att);
+ }
+}
+
+static bool can_read_data(struct io *io, void *user_data)
+{
+ struct bt_att *att = user_data;
+ uint8_t *pdu;
+ ssize_t bytes_read;
+ uint16_t opcode;
+
+ bytes_read = read(att->fd, att->buf, att->len);
+ if (bytes_read < 0)
+ return false;
+
+ util_hexdump('>', att->buf, bytes_read,
+ att->debug_callback, att->debug_data);
+
+ if (bytes_read < 1)
+ return true;
+
+ pdu = att->buf;
+ opcode = pdu[0];
+
+ /* The opcode is either a response to a pending request, a new request
+ * from a client, or a notification/indication. Handle them separately
+ * here.
+ */
+ /* TODO: Handle other types of data here. */
+ handle_response(att, opcode, pdu, bytes_read);
+
+ if (att->destroyed)
+ return false;
+
+ return true;
+}
+
+struct bt_att *bt_att_new(int fd)
+{
+ struct bt_att *att;
+
+ if (fd < 0)
+ return NULL;
+
+ att = new0(struct bt_att, 1);
+ att->fd = fd;
+ att->close_on_unref = false;
+
+ att->len = ATT_DEFAULT_LE_MTU;
+ att->buf = malloc(att->len);
+ if (!att->buf) {
+ free(att);
+ return NULL;
+ }
+
+ att->io = io_new(fd);
+ if (!att->io) {
+ free(att->buf);
+ free(att);
+ return NULL;
+ }
+
+ att->request_queue = queue_new();
+ if (!att->request_queue) {
+ io_destroy(att->io);
+ free(att->buf);
+ free(att);
+ return NULL;
+ }
+
+ att->pending_list = queue_new();
+ if (!att->pending_list) {
+ queue_destroy(att->request_queue, NULL);
+ free(att->buf);
+ free(att);
+ return NULL;
+ }
+
+ if (!io_set_read_handler(att->io, can_read_data,
+ att, read_watch_destroy)) {
+ queue_destroy(att->pending_list, NULL);
+ queue_destroy(att->request_queue, NULL);
+ free(att->buf);
+ free(att);
+ return NULL;
+ }
+
+ att->writer_active = false;
+
+ return bt_att_ref(att);
+}
+
+struct bt_att *bt_att_ref(struct bt_att *att)
+{
+ if (!att)
+ return NULL;
+
+ __sync_fetch_and_add(&att->ref_count, 1);
+
+ return att;
+}
+
+void bt_att_unref(struct bt_att *att)
+{
+ if (!att)
+ return;
+
+ if (__sync_sub_and_fetch(&att->ref_count, 1))
+ return;
+
+ bt_att_cancel_all(att);
+
+ queue_destroy(att->request_queue, NULL);
+
+ io_set_write_handler(att->io, NULL, NULL, NULL);
+ io_set_read_handler(att->io, NULL, NULL, NULL);
+
+ io_destroy(att->io);
+ att->io = NULL;
+
+ if (att->close_on_unref)
+ close(att->fd);
+
+ if (att->debug_destroy)
+ att->debug_destroy(att->debug_data);
+
+ free(att->buf);
+ att->buf = NULL;
+
+ att->destroyed = true;
+}
+
+bool bt_att_set_debug(struct bt_att *att, bt_att_debug_func_t callback,
+ void *user_data, bt_att_destroy_func_t destroy)
+{
+ if (!att)
+ return false;
+
+ if (att->debug_destroy)
+ att->debug_destroy(att->debug_data);
+
+ att->debug_callback = callback;
+ att->debug_destroy = destroy;
+ att->debug_data = user_data;
+
+ return true;
+}
+
+bool bt_att_set_close_on_unref(struct bt_att *att, bool do_close)
+{
+ if (!att)
+ return false;
+
+ att->close_on_unref = do_close;
+
+ return true;
+}
+
+uint16_t bt_att_get_mtu(struct bt_att *att)
+{
+ if (!att)
+ return 0;
+
+ return att->len;
+}
+
+bool bt_att_set_mtu(struct bt_att *att, uint16_t mtu)
+{
+ if (!att)
+ return false;
+
+ if (mtu <= ATT_DEFAULT_LE_MTU)
+ return false;
+
+ att->len = mtu;
+
+ return true;
+}
+
+static bool encode_mtu_req(const void *param, uint16_t length, void *buf,
+ uint16_t mtu, uint16_t *pdu_length)
+{
+ const struct bt_att_exchange_mtu_req_param *cp = param;
+ const uint16_t len = 3;
+
+ if (!param || !buf)
+ return false;
+
+ if (length != sizeof(struct bt_att_exchange_mtu_req_param))
+ return false;
+
+ if (len > mtu)
+ return false;
+
+ put_le16(cp->client_rx_mtu, buf);
+ *pdu_length = len;
+
+ return true;
+}
+
+static bool encode_pdu(uint8_t opcode, const void *param, uint16_t length,
+ uint16_t mtu, void *pdu, uint16_t *pdu_length)
+{
+ uint8_t *bytes = pdu;
+ bytes[0] = opcode;
+
+ /* The length of the PDU depends on the specific ATT method. Special
+ * case the operations with variable-length PDU's here.
+ */
+ if (length == 0) {
+ *pdu_length = 1;
+ return true;
+ }
+
+ if (opcode == ATT_OP_EXCHANGE_MTU_REQ) {
+ return encode_mtu_req(param, length, bytes + 1,
+ mtu, pdu_length);
+ }
+
+ return false;
+}
+
+static struct bt_att_request *create_request(uint8_t opcode, const void *param,
+ uint16_t length, void *buf, uint16_t mtu,
+ bt_att_request_func_t callback, void *user_data,
+ bt_att_destroy_func_t destroy)
+{
+ struct bt_att_request *request;
+
+ if (!opcode)
+ return NULL;
+
+ if (length > 0 && !param)
+ return NULL;
+
+ request = new0(struct bt_att_request, 1);
+ if (!request)
+ return NULL;
+
+ if (!encode_pdu(opcode, param, length, mtu, buf, &request->len)) {
+ free(request);
+ return NULL;
+ }
+
+ /* request->len is at least 1 due to the opcode */
+ request->pdu = malloc(request->len);
+ if (!request->pdu) {
+ free(request);
+ return NULL;
+ }
+
+ if (request->len > 0)
+ memcpy(request->pdu, buf, request->len);
+
+ request->opcode = opcode;
+ request->callback = callback;
+ request->destroy = destroy;
+ request->user_data = user_data;
+
+ return request;
+}
+
+unsigned int bt_att_send(struct bt_att *att, uint8_t opcode,
+ const void *param, uint16_t length,
+ bt_att_request_func_t callback, void *user_data,
+ bt_att_destroy_func_t destroy)
+{
+ struct bt_att_request *request;
+
+ if (!att)
+ return 0;
+
+ request = create_request(opcode, param, length, att->buf, att->len,
+ callback, user_data, destroy);
+
+ if (!request)
+ return 0;
+
+ if (att->next_request_id < 1)
+ att->next_request_id = 1;
+
+ request->id = att->next_request_id++;
+
+ if (!queue_push_tail(att->request_queue, request)) {
+ free(request->pdu);
+ free(request);
+ return 0;
+ }
+
+ wakeup_writer(att);
+
+ return request->id;
+}
+
+bool bt_att_cancel(struct bt_att *att, unsigned int id)
+{
+ struct bt_att_request *request;
+
+ if (!request || !id)
+ return false;
+
+ request = queue_remove_if(att->request_queue, match_request_id,
+ UINT_TO_PTR(id));
+ if (request)
+ goto done;
+
+ request = queue_remove_if(att->pending_list, match_request_id,
+ UINT_TO_PTR(id));
+ if (request)
+ goto done;
+
+done:
+ destroy_request(request);
+
+ wakeup_writer(att);
+
+ return true;
+}
+
+bool bt_att_cancel_all(struct bt_att *att)
+{
+ if (!att)
+ return false;
+
+ queue_remove_all(att->pending_list, NULL, NULL, destroy_request);
+ queue_remove_all(att->request_queue, NULL, NULL, destroy_request);
+
+ return true;
+}
diff --git a/src/shared/att.h b/src/shared/att.h
new file mode 100644
index 0000000..570fbd2
--- /dev/null
+++ b/src/shared/att.h
@@ -0,0 +1,97 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2012-2014 Intel Corporation. All rights reserved.
+ *
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#include <stdbool.h>
+#include <stdint.h>
+
+/* Error response */
+#define ATT_OP_ERROR_RESP 0x01
+struct bt_att_error_rsp_param {
+ uint8_t request_opcode;
+ uint16_t handle;
+ uint8_t error_code;
+};
+
+/* Exchange MTU */
+#define ATT_OP_EXCHANGE_MTU_REQ 0x02
+struct bt_att_exchange_mtu_req_param {
+ uint16_t client_rx_mtu;
+};
+
+#define ATT_OP_EXCHANGE_MTU_RESP 0x03
+struct bt_att_exchange_mtu_rsp_param {
+ uint16_t server_rx_mtu;
+};
+
+/* Error codes for Error response PDU */
+#define ATT_ERROR_INVALID_HANDLE 0x01
+#define ATT_ERROR_READ_NOT_PERMITTED 0x02
+#define ATT_ERROR_WRITE_NOT_PERMITTED 0x03
+#define ATT_ERROR_INVALID_PDU 0x04
+#define ATT_ERROR_AUTHENTICATION 0x05
+#define ATT_ERROR_REQUEST_NOT_SUPPORTED 0x06
+#define ATT_ERROR_INVALID_OFFSET 0x07
+#define ATT_ERROR_AUTHORIZATION 0x08
+#define ATT_ERROR_PREPARE_QUEUE_FULL 0x09
+#define ATT_ERROR_ATTRIBUTE_NOT_FOUND 0x0A
+#define ATT_ERROR_ATTRIBUTE_NOT_LONG 0x0B
+#define ATT_ERROR_INSUFFICIENT_ENCRYPTION_KEY_SIZE 0x0C
+#define ATT_ERROR_INVALID_ATTRIBUTE_VALUE_LEN 0x0D
+#define ATT_ERROR_UNLIKELY 0x0E
+#define ATT_ERROR_INSUFFICIENT_ENCRYPTION 0x0F
+#define ATT_ERROR_UNSUPPORTED_GROUP_TYPE 0x10
+#define ATT_ERROR_INSUFFICIENT_RESOURCES 0x11
+/* Application defined errors */
+#define ATT_ERROR_IO 0x80
+#define ATT_ERROR_TIMEOUT 0x81
+#define ATT_ERROR_ABORTED 0x82
+
+typedef void (*bt_att_destroy_func_t)(void *user_data);
+
+struct bt_att;
+
+struct bt_att *bt_att_new(int fd);
+
+struct bt_att *bt_att_ref(struct bt_att *att);
+void bt_att_unref(struct bt_att *att);
+
+typedef void (*bt_att_debug_func_t)(const char *str, void *user_data);
+
+bool bt_att_set_debug(struct bt_att *att, bt_att_debug_func_t callback,
+ void *user_data, bt_att_destroy_func_t destroy);
+
+bool bt_att_set_close_on_unref(struct bt_att *att, bool do_close);
+
+uint16_t bt_att_get_mtu(struct bt_att *att);
+bool bt_att_set_mtu(struct bt_att *att, uint16_t mtu);
+
+typedef void (*bt_att_request_func_t)(uint8_t opcode, const void *param,
+ uint16_t length, void *user_data);
+
+unsigned int bt_att_send(struct bt_att *att, uint8_t opcode,
+ const void *param, uint16_t length,
+ bt_att_request_func_t callback, void *user_data,
+ bt_att_destroy_func_t destroy);
+
+bool bt_att_cancel(struct bt_att *att, unsigned int id);
+bool bt_att_cancel_all(struct bt_att *att);
--
1.9.1.423.g4596e3a