[PATCH 00/10] bt_att initial implementation

[PATCH 00/10] bt_att initial implementation

[Arman Uguray]

This patch set introduces "struct bt_att" which handles the transport logic for
interactions over the Attribute Protocol. struct bt_att is meant to be
connection-agnostic: It enables a new GATT/ATT stack based around src/shared/io,
so that the library internals of how the connection is managed (glib vs
mainloop) is not tied to the protocol implementation. bt_att will initially
support the client-side protocol and eventually extended to support the server
side.

Major changes introduced in this patch-set:
  - src/shared/att.[h,cc], which introduces "struct bt_att", with initial
    support for client-initiated requests and asynchronous response handling via
    a callback.
  - unit/test-att, which defines unit tests for request/response using a
    socketpair.
  - tools/btatt, which is a command-line tool for debugging the Attribute
    Protocol.

The majority of the above are introduced in patches 1 through 3, which consist
almost entirely of boilerplate taken from src/shared/mgmt.*, unit/test-mgmt, and
tools/btmgmt.c.

The 9 patches introduced here are the initial portion of 20+ patches where each
ATT operation is implemented in groups of 3 patches:
  - 1 patch that implements the request/response or command in src/shared/att
  - 1 patch that adds a unit test to unit/test-att
  - 1 patch that adds a command for the request to tools/btatt

Arman Uguray (10):
  src/shared/att: Introduce struct bt_att.
  unit/test-att: Add unit tests for src/shared/att.
  tools/btatt: Add command-line tool for ATT protocol testing.
  tools/btatt: Add "exchange-mtu" command.
  src/shared/att: Add "Find Information" request and response.
  unit/test-att: Add unit test for "Find Information" request/response.
  tools/btatt: Add "find-information" command.
  src/shared/att: Add "Find By Type Value" request and response.
  unit/test-att: Add unit test for "Find By Type Value"
    request/response.
  tools/btatt: Add "find-by-type-value" command.

 Makefile.am      |  12 +-
 Makefile.tools   |  10 +-
 src/shared/att.c | 689 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/shared/att.h | 124 ++++++++++
 tools/btatt.c    | 626 ++++++++++++++++++++++++++++++++++++++++++++++++++
 unit/test-att.c  | 473 ++++++++++++++++++++++++++++++++++++++
 6 files changed, 1932 insertions(+), 2 deletions(-)
 create mode 100644 src/shared/att.c
 create mode 100644 src/shared/att.h
 create mode 100644 tools/btatt.c
 create mode 100644 unit/test-att.c

-- 
1.9.1.423.g4596e3a

[PATCH 01/10] src/shared/att: Introduce struct bt_att.

[Arman Uguray]

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 | 526 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/shared/att.h |  94 ++++++++++
 3 files changed, 622 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..012a6ad
--- /dev/null
+++ b/src/shared/att.c
@@ -0,0 +1,526 @@
+/*
+ *
+ *  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
+#define ATT_MIN_PDU_LEN			1  /* At least 1 byte for the opcode. */
+
+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_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;
+	struct bt_att_error_rsp_param error_rp;
+	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) {
+			error_rp.request_opcode = request->opcode;
+			error_rp.handle = 0;
+			error_rp.error_code = ATT_ERROR_IO;
+			request->callback(ATT_OP_ERROR_RESP,
+						&error_rp, sizeof(error_rp),
+						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, const uint8_t *pdu,
+							uint16_t pdu_length)
+{
+	struct bt_att_error_rsp_param param;
+
+	if (pdu_length != 5)
+		return false;
+
+	memset(&param, 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, pdu[0],
+							&param, sizeof(param));
+
+	return true;
+}
+
+static bool handle_exchange_mtu_rsp(struct bt_att *att, 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, pdu[0],
+							&param, sizeof(param));
+
+	return true;
+}
+
+static bool handle_response(struct bt_att *att, const uint8_t *pdu,
+							uint16_t pdu_length)
+{
+	uint8_t opcode = pdu[0];
+
+	if (opcode == ATT_OP_ERROR_RESP)
+		return handle_error_rsp(att, pdu, pdu_length);
+
+	if (opcode == ATT_OP_EXCHANGE_MTU_RESP)
+		return handle_exchange_mtu_rsp(att, 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;
+
+	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 < ATT_MIN_PDU_LEN)
+		return true;
+
+	pdu = att->buf;
+
+	/* 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, 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);
+	if (!att)
+		return NULL;
+
+	att->fd = fd;
+	att->close_on_unref = false;
+
+	att->len = ATT_DEFAULT_LE_MTU;
+	att->buf = malloc(att->len);
+	if (!att->buf)
+		goto fail;
+
+	att->io = io_new(fd);
+	if (!att->io)
+		goto fail;
+
+	att->request_queue = queue_new();
+	if (!att->request_queue)
+		goto fail;
+
+	att->pending_list = queue_new();
+	if (!att->pending_list)
+		goto fail;
+
+	if (!io_set_read_handler(att->io, can_read_data,
+						att, read_watch_destroy))
+		goto fail;
+
+	att->writer_active = false;
+
+	return bt_att_ref(att);
+
+fail:
+	queue_destroy(att->pending_list, NULL);
+	queue_destroy(att->request_queue, NULL);
+	io_destroy(att->io);
+	free(att->buf);
+	free(att);
+
+	return NULL;
+}
+
+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 (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;
+
+	if (!bytes)
+		return false;
+
+	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 (!param)
+		return false;
+
+	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_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..1568deb
--- /dev/null
+++ b/src/shared/att.h
@@ -0,0 +1,94 @@
+/*
+ *
+ *  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
+
+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_all(struct bt_att *att);
-- 
1.9.1.423.g4596e3a

[PATCH 02/10] unit/test-att: Add unit tests for src/shared/att.

[Arman Uguray]

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

[PATCH 03/10] tools/btatt: Add command-line tool for ATT protocol testing.

[Arman Uguray]

Initial commit for tools/btatt. Added basic command line options parsing
and added the tool to Makefile.tools as experimental.
---
 Makefile.tools |  10 +++-
 tools/btatt.c  | 151 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 160 insertions(+), 1 deletion(-)
 create mode 100644 tools/btatt.c

diff --git a/Makefile.tools b/Makefile.tools
index 40f076b..13f6691 100644
--- a/Makefile.tools
+++ b/Makefile.tools
@@ -278,7 +278,7 @@ noinst_PROGRAMS += tools/bdaddr tools/avinfo tools/avtest \
 			tools/btmgmt tools/btinfo tools/btattach \
 			tools/btsnoop tools/btproxy tools/btiotest \
 			tools/mpris-player tools/cltest tools/seq2bseq \
-			tools/ibeacon
+			tools/ibeacon tools/btatt
 
 tools_bdaddr_SOURCES = tools/bdaddr.c src/oui.h src/oui.c
 tools_bdaddr_LDADD = lib/libbluetooth-internal.la @UDEV_LIBS@
@@ -342,6 +342,14 @@ tools_ibeacon_SOURCES = tools/ibeacon.c monitor/bt.h \
 				src/shared/queue.h src/shared/queue.c \
 				src/shared/ringbuf.h src/shared/ringbuf.c
 
+tools_btatt_SOURCES = tools/btatt.c src/uuid-helper.c \
+				monitor/mainloop.h monitor/mainloop.c \
+				src/shared/io.h src/shared/io-mainloop.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
+tools_btatt_LDADD = lib/libbluetooth-internal.la
+
 EXTRA_DIST += tools/bdaddr.1
 endif
 
diff --git a/tools/btatt.c b/tools/btatt.c
new file mode 100644
index 0000000..5287f42
--- /dev/null
+++ b/tools/btatt.c
@@ -0,0 +1,151 @@
+/*
+ *  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 <stdbool.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <getopt.h>
+#include <string.h>
+
+#include <bluetooth/bluetooth.h>
+
+#include "monitor/mainloop.h"
+#include "src/shared/att.h"
+
+#define ATT_CID 4
+
+static bool verbose = false;
+
+static struct {
+	char *cmd;
+	void (*func)(struct bt_att *att, int argc, char **argv);
+	char *doc;
+} command[] = {
+	{ }
+};
+
+static void usage(void)
+{
+	int i;
+
+	printf("btatt\n");
+	printf("Usage:\n"
+		"\tbtatt [options] <command> [command parameters]\n");
+
+	printf("Options:\n"
+		"\t-i, --index <id>\tSpecify adapter index, e.g. hci0\n"
+		"\t-d, --dest <addr>\tSpecify the destination address\n"
+		"\t-t, --type [random|public] \tSpecify the LE address type\n"
+		"\t-v, --verbose\tEnable extra logging\n"
+		"\t-h, --help\tDisplay help\n");
+
+	printf("Commands:\n");
+	for (i = 0; command[i].cmd; i++)
+		printf("\t%-15s\t%s\n", command[i].cmd, command[i].doc);
+
+	printf("\n"
+		"For more information on the usage of each command use:\n"
+		"\tbtatt <command> --help\n" );
+}
+
+static struct option main_options[] = {
+	{ "index",	1, 0, 'i' },
+	{ "dest",	1, 0, 'd' },
+	{ "type",	1, 0, 't' },
+	{ "verbose",	0, 0, 'v' },
+	{ "help",	0, 0, 'h' },
+	{ 0, 0, 0, 0}
+};
+
+int main(int argc, char *argv[])
+{
+	int opt, i;
+	int dev_id = -1;
+	bool dest_given = false;
+	uint8_t dest_type = BDADDR_LE_PUBLIC;
+	bdaddr_t src_addr, dst_addr;
+	int fd;
+
+	while ((opt = getopt_long(argc, argv, "+hvt:i:d:",
+						main_options, NULL)) != -1) {
+		switch (opt) {
+		case 'i':
+			dev_id = hci_devid(optarg);
+			if (dev_id < 0) {
+				perror("Invalid adapter");
+				return EXIT_FAILURE;
+			}
+			break;
+		case 'd':
+			if (str2ba(optarg, &dst_addr) < 0) {
+				fprintf(stderr, "Invalid destination address\n");
+				return EXIT_FAILURE;
+			}
+			dest_given = true;
+			break;
+		case 't':
+			if (strcmp(optarg, "random") == 0)
+				dest_type = BDADDR_LE_RANDOM;
+			else if (strcmp(optarg, "public") == 0)
+				dest_type = BDADDR_LE_PUBLIC;
+			else {
+				fprintf(stderr, "Allowed types: random, public\n");
+				return EXIT_FAILURE;
+			}
+			break;
+		case 'v':
+			verbose = true;
+			break;
+		case 'h':
+			usage();
+			return 0;
+		default:
+			break;
+		}
+	}
+
+	argc -= optind;
+	argv += optind;
+	optind = 0;
+
+	if (argc < 1) {
+		usage();
+		return 0;
+	}
+
+	if (dev_id == -1)
+		bacpy(&src_addr, BDADDR_ANY);
+	else if (hci_devba(dev_id, &src_addr) < 0) {
+		perror("Adapter not available");
+		return EXIT_FAILURE;
+	}
+
+	if (!dest_given) {
+		fprintf(stderr, "Destination address required\n");
+		return EXIT_FAILURE;
+	}
+
+	return 0;
+}
-- 
1.9.1.423.g4596e3a

[PATCH 04/10] tools/btatt: Add "exchange-mtu" command.

[Arman Uguray]

Added command for "Exchange MTU" request/response. Also added code for
initiating an L2CAP connection over the ATT channel. Tested using a CSR
uEnergy 1000 board with the following results:

$ btatt -d 00:02:5B:00:15:10 -v exchange-mtu 50
Going to set MTU to 16 bit value: 50
att: ATT command 0x02
att < 02 32 00
att > 03 17 00
Exchange MTU response: Server Rx MTU 23
---
 tools/btatt.c | 191 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 190 insertions(+), 1 deletion(-)

diff --git a/tools/btatt.c b/tools/btatt.c
index 5287f42..a21625f 100644
--- a/tools/btatt.c
+++ b/tools/btatt.c
@@ -30,6 +30,7 @@
 #include <string.h>
 
 #include <bluetooth/bluetooth.h>
+#include <bluetooth/l2cap.h>
 
 #include "monitor/mainloop.h"
 #include "src/shared/att.h"
@@ -38,11 +39,152 @@
 
 static bool verbose = false;
 
+static void handle_error(const void *param, uint16_t len)
+{
+	const struct bt_att_error_rsp_param *rp = param;
+
+	if (len != sizeof(*rp)) {
+		fprintf(stderr, "Invalid error response length (%u)\n", len);
+		return;
+	}
+
+	printf("Request failed:\n\topcode:\t%u\n\thandle:\t%u\n\terror:\t%u\n",
+			rp->request_opcode, rp->handle, rp->error_code);
+}
+
+static void exchange_mtu_cb(uint8_t opcode, const void *param,
+						uint16_t len, void *user_data)
+{
+	const struct bt_att_exchange_mtu_rsp_param *rp;
+
+	if (opcode == ATT_OP_ERROR_RESP) {
+		handle_error(param, len);
+		goto done;
+	}
+
+	if (opcode != ATT_OP_EXCHANGE_MTU_RESP) {
+		fprintf(stderr, "Invalid response opcode (%u)\n", opcode);
+		goto done;
+	}
+
+	rp = param;
+
+	if (len != sizeof(*rp)) {
+		fprintf(stderr, "Invalid \"Exchange MTU\" response length "
+								"(%u)\n", len);
+		goto done;
+	}
+
+	printf("Exchange MTU response: Server Rx MTU: %u\n", rp->server_rx_mtu);
+
+done:
+	mainloop_quit();
+}
+
+static void mtu_usage(void)
+{
+	printf("Usage: btatt exchange-mtu <ATT_MTU>\n");
+}
+
+static struct option mtu_options[] = {
+	{ "help",	0, 0, 'h'},
+	{}
+};
+
+static void cmd_mtu(struct bt_att *att, int argc, char **argv)
+{
+	struct bt_att_exchange_mtu_req_param param;
+	uint16_t mtu;
+	int opt;
+
+	while ((opt = getopt_long(argc, argv, "+h", mtu_options,
+							NULL)) != -1) {
+		switch (opt) {
+		case 'h':
+		default:
+			mtu_usage();
+			exit(EXIT_SUCCESS);
+		}
+	}
+
+	argc -= optind;
+	argv += optind;
+	optind = 0;
+
+	if (argc < 1) {
+		mtu_usage();
+		exit(EXIT_FAILURE);
+	}
+
+	mtu = atoi(argv[0]);
+
+	if (verbose)
+		printf("Going to set MTU to 16 bit value: %u\n", mtu);
+
+	memset(&param, 0, sizeof(param));
+	param.client_rx_mtu = mtu;
+	if (bt_att_send(att, ATT_OP_EXCHANGE_MTU_REQ, &param, sizeof(param),
+					exchange_mtu_cb, NULL, NULL) == 0) {
+		fprintf(stderr, "Unable to send \"Exchange MTU\" request\n");
+		exit(EXIT_FAILURE);
+	}
+}
+
+static int l2cap_le_att_connect(bdaddr_t *src, bdaddr_t *dst, uint8_t dst_type)
+{
+	int sock;
+	struct sockaddr_l2 srcaddr, dstaddr;
+	char srcaddr_str[18], dstaddr_str[18];
+
+	if (verbose) {
+		ba2str(src, srcaddr_str);
+		ba2str(dst, dstaddr_str);
+
+		printf("Opening L2CAP LE connection on ATT channel: "
+					"src: %s dest: %s\n", srcaddr_str, dstaddr_str);
+	}
+
+	sock = socket(PF_BLUETOOTH, SOCK_SEQPACKET, BTPROTO_L2CAP);
+	if (sock < 0) {
+		perror("Failed to create L2CAP socket");
+		return -1;
+	}
+
+	/* Set up source address */
+	memset(&srcaddr, 0, sizeof(srcaddr));
+	srcaddr.l2_family = AF_BLUETOOTH;
+	bacpy(&srcaddr.l2_bdaddr, src);
+	srcaddr.l2_cid = htobs(ATT_CID);
+	srcaddr.l2_bdaddr_type = BDADDR_LE_PUBLIC;
+
+	if (bind(sock, (struct sockaddr *) &srcaddr, sizeof(srcaddr)) < 0) {
+		perror("Failed to bind L2CAP socket");
+		close(sock);
+		return -1;
+	}
+
+	/* Set up destination address */
+	memset(&dstaddr, 0, sizeof(dstaddr));
+	dstaddr.l2_family = AF_BLUETOOTH;
+	bacpy(&dstaddr.l2_bdaddr, dst);
+	dstaddr.l2_cid = htobs(ATT_CID);
+	dstaddr.l2_bdaddr_type = dst_type;
+
+	if (connect(sock, (struct sockaddr *) &dstaddr, sizeof(dstaddr)) < 0) {
+		perror("Failed to connect");
+		close(sock);
+		return -1;
+	}
+
+	return sock;
+}
+
 static struct {
 	char *cmd;
 	void (*func)(struct bt_att *att, int argc, char **argv);
 	char *doc;
 } command[] = {
+	{ "exchange-mtu",	cmd_mtu,	"\"Exchange MTU\" request." },
 	{ }
 };
 
@@ -79,6 +221,13 @@ static struct option main_options[] = {
 	{ 0, 0, 0, 0}
 };
 
+static void att_debug(const char *str, void *user_data)
+{
+	const char *prefix = user_data;
+
+	printf("%s%s\n", prefix, str);
+}
+
 int main(int argc, char *argv[])
 {
 	int opt, i;
@@ -87,6 +236,8 @@ int main(int argc, char *argv[])
 	uint8_t dest_type = BDADDR_LE_PUBLIC;
 	bdaddr_t src_addr, dst_addr;
 	int fd;
+	struct bt_att *att;
+	int exit_status;
 
 	while ((opt = getopt_long(argc, argv, "+hvt:i:d:",
 						main_options, NULL)) != -1) {
@@ -147,5 +298,43 @@ int main(int argc, char *argv[])
 		return EXIT_FAILURE;
 	}
 
-	return 0;
+	mainloop_init();
+
+	fd = l2cap_le_att_connect(&src_addr, &dst_addr, dest_type);
+
+	if (fd < 0)
+		return EXIT_FAILURE;
+
+	att = bt_att_new(fd);
+
+	if (!att) {
+		fprintf(stderr, "Failed to create bt_att.");
+		close(fd);
+		return EXIT_FAILURE;
+	}
+
+	if (verbose)
+		bt_att_set_debug(att, att_debug, "att: ", NULL);
+
+	bt_att_set_close_on_unref(att, true);
+
+	for (i = 0; command[i].cmd; i++) {
+		if (strcmp(command[i].cmd, argv[0]) == 0)
+			break;
+	}
+
+	if (command[i].cmd == NULL) {
+		fprintf(stderr, "Unknown command: %s\n", argv[0]);
+		bt_att_unref(att);
+		return EXIT_FAILURE;
+	}
+
+	command[i].func(att, argc, argv);
+
+	exit_status = mainloop_run();
+
+	bt_att_cancel_all(att);
+	bt_att_unref(att);
+
+	return exit_status;
 }
-- 
1.9.1.423.g4596e3a

[PATCH 05/10] src/shared/att: Add "Find Information" request and response.

[Arman Uguray]

This patch adds support for the "Find Information" request and response
with their corresponding parameter structures.
---
 src/shared/att.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
 src/shared/att.h | 14 +++++++++
 2 files changed, 99 insertions(+), 8 deletions(-)

diff --git a/src/shared/att.c b/src/shared/att.c
index 012a6ad..25112c7 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -196,6 +196,8 @@ static bool handle_exchange_mtu_rsp(struct bt_att *att, const uint8_t *pdu,
 	if (pdu_length != 3)
 		return false;
 
+	memset(&param, 0, sizeof(param));
+
 	param.server_rx_mtu = get_le16(pdu + 1);
 
 	request_complete(att, ATT_OP_EXCHANGE_MTU_REQ, pdu[0],
@@ -204,16 +206,67 @@ static bool handle_exchange_mtu_rsp(struct bt_att *att, const uint8_t *pdu,
 	return true;
 }
 
+static bool handle_find_info_rsp(struct bt_att *att, const uint8_t *pdu,
+							uint16_t pdu_length)
+{
+	struct bt_att_find_information_rsp_param param;
+	uint8_t *information_data;
+
+	if (pdu_length < 6)
+		return false;
+
+	memset(&param, 0, sizeof(param));
+
+	param.format = pdu[1];
+	param.length = pdu_length - 2;
+
+	/* param.length is at least 4, as checked above */
+	information_data = malloc(param.length);
+	if (!information_data)
+		return false;
+
+	memcpy(information_data, pdu + 2, param.length);
+
+	param.information_data = information_data;
+
+	request_complete(att, ATT_OP_FIND_INFORMATION_REQ, pdu[0],
+							&param, sizeof(param));
+
+	free(information_data);
+
+	return true;
+}
+
 static bool handle_response(struct bt_att *att, const uint8_t *pdu,
 							uint16_t pdu_length)
 {
 	uint8_t opcode = pdu[0];
+	uint8_t req_opcode;
 
-	if (opcode == ATT_OP_ERROR_RESP)
+	switch (opcode) {
+	case ATT_OP_ERROR_RESP:
 		return handle_error_rsp(att, pdu, pdu_length);
 
-	if (opcode == ATT_OP_EXCHANGE_MTU_RESP)
-		return handle_exchange_mtu_rsp(att, pdu, pdu_length);
+	case ATT_OP_EXCHANGE_MTU_RESP:
+		if (!handle_exchange_mtu_rsp(att, pdu, pdu_length)) {
+			req_opcode = ATT_OP_EXCHANGE_MTU_REQ;
+			goto fail;
+		}
+		return true;
+
+	case ATT_OP_FIND_INFORMATION_RESP:
+		if (!handle_find_info_rsp(att, pdu, pdu_length)) {
+			req_opcode = ATT_OP_FIND_INFORMATION_REQ;
+			goto fail;
+		}
+		return true;
+
+	default:
+		break;
+	}
+
+fail:
+	request_complete(att, req_opcode, ATT_OP_ERROR_RESP, NULL, 0);
 
 	return false;
 }
@@ -400,14 +453,36 @@ static bool encode_mtu_req(const void *param, uint16_t length, void *buf,
 {
 	const struct bt_att_exchange_mtu_req_param *cp = param;
 	const uint16_t len = 3;
+	uint8_t *bytes = buf;
+
+	if (length != sizeof(*cp))
+		return false;
+
+	if (len > mtu)
+		return false;
+
+	put_le16(cp->client_rx_mtu, bytes + 1);
+	*pdu_length = len;
+
+	return true;
+}
 
-	if (length != sizeof(struct bt_att_exchange_mtu_req_param))
+static bool encode_find_info_req(const void *param, uint16_t length, void *buf,
+						uint16_t mtu, uint16_t *pdu_length)
+{
+	const struct bt_att_find_information_req_param *cp = param;
+	const uint16_t len = 5;
+	uint8_t *bytes = buf;
+
+	if (length != sizeof(*cp))
 		return false;
 
 	if (len > mtu)
 		return false;
 
-	put_le16(cp->client_rx_mtu, buf);
+	put_le16(cp->start_handle, bytes + 1);
+	put_le16(cp->end_handle, bytes + 3);
+
 	*pdu_length = len;
 
 	return true;
@@ -434,10 +509,12 @@ static bool encode_pdu(uint8_t opcode, const void *param, uint16_t length,
 	if (!param)
 		return false;
 
-	if (opcode == ATT_OP_EXCHANGE_MTU_REQ) {
-		return encode_mtu_req(param, length, bytes + 1,
+	if (opcode == ATT_OP_EXCHANGE_MTU_REQ)
+		return encode_mtu_req(param, length, bytes, mtu, pdu_length);
+
+	if (opcode == ATT_OP_FIND_INFORMATION_REQ)
+		return encode_find_info_req(param, length, bytes,
 							mtu, pdu_length);
-	}
 
 	return false;
 }
diff --git a/src/shared/att.h b/src/shared/att.h
index 1568deb..b9b4037 100644
--- a/src/shared/att.h
+++ b/src/shared/att.h
@@ -43,6 +43,20 @@ struct bt_att_exchange_mtu_rsp_param {
 	uint16_t server_rx_mtu;
 };
 
+/* Find Information */
+#define ATT_OP_FIND_INFORMATION_REQ	0x04
+struct bt_att_find_information_req_param {
+	uint16_t start_handle;
+	uint16_t end_handle;
+};
+
+#define ATT_OP_FIND_INFORMATION_RESP	0x05
+struct bt_att_find_information_rsp_param {
+	uint8_t format;
+	const uint8_t *information_data;
+	uint16_t length;
+};
+
 /* Error codes for Error response PDU */
 #define ATT_ERROR_INVALID_HANDLE			0x01
 #define ATT_ERROR_READ_NOT_PERMITTED			0x02
-- 
1.9.1.423.g4596e3a

[PATCH 06/10] unit/test-att: Add unit test for "Find Information" request/response.

[Arman Uguray]

Added test case for the "Find Information" request/response.
---
 unit/test-att.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 74 insertions(+), 1 deletion(-)

diff --git a/unit/test-att.c b/unit/test-att.c
index 3a3a77d..e45342a 100644
--- a/unit/test-att.c
+++ b/unit/test-att.c
@@ -203,6 +203,7 @@ struct request_test_data {
 	uint16_t req_size;
 	const  void *resp_data;
 	uint16_t resp_size;
+	bool (*compare_func)(const void *a, const void *b);
 };
 
 struct request_test {
@@ -258,6 +259,67 @@ static const struct request_test_data request_test_2 = {
 	.resp_size = sizeof(exchange_mtu_resp_bytes_2),
 };
 
+/* Find Information request <-> response */
+static bool compare_find_info_rsp(const void *a, const void *b)
+{
+	const struct bt_att_find_information_rsp_param *p1 = a;
+	const struct bt_att_find_information_rsp_param *p2 = b;
+
+	if (p1->format != p2->format || p1->length != p2->length)
+		return false;
+
+	return memcmp(p1->information_data, p2->information_data,
+							p2->length) == 0;
+}
+
+static const unsigned char find_info_req_bytes[] = {
+	0x04, 0x01, 0x00, 0xff, 0xff
+};
+static const struct bt_att_find_information_req_param find_info_req_param = {
+	.start_handle = 0x0001,
+	.end_handle = 0xffff,
+};
+static const unsigned char find_info_resp_bytes_1[] = {
+	0x05, 0x01, 0x01, 0x00, 0x0d, 0x18
+};
+static const uint8_t find_info_resp_information_data_1[] = {
+	0x01, 0x00, 0x0d, 0x18
+};
+static const struct bt_att_find_information_rsp_param find_info_rsp_param = {
+	.format = 0x01,
+	.information_data = find_info_resp_information_data_1,
+	.length = sizeof(find_info_resp_information_data_1),
+};
+static const struct request_test_data request_test_3 = {
+	.req_opcode = ATT_OP_FIND_INFORMATION_REQ,
+	.req_param = &find_info_req_param,
+	.req_param_length = sizeof(find_info_req_param),
+	.resp_opcode = ATT_OP_FIND_INFORMATION_RESP,
+	.resp_param = &find_info_rsp_param,
+	.resp_param_length = sizeof(find_info_rsp_param),
+	.req_data = find_info_req_bytes,
+	.req_size = sizeof(find_info_req_bytes),
+	.resp_data = find_info_resp_bytes_1,
+	.resp_size = sizeof(find_info_resp_bytes_1),
+	.compare_func = compare_find_info_rsp,
+};
+
+/* Find Information request <-> invalid response */
+static const unsigned char find_info_resp_bytes_2[] = {
+	0x05, 0x01
+};
+
+static const struct request_test_data request_test_4 = {
+	.req_opcode = ATT_OP_FIND_INFORMATION_REQ,
+	.req_param = &find_info_req_param,
+	.req_param_length = sizeof(find_info_req_param),
+	.resp_opcode = ATT_OP_ERROR_RESP,
+	.req_data = find_info_req_bytes,
+	.req_size = sizeof(find_info_req_bytes),
+	.resp_data = find_info_resp_bytes_2,
+	.resp_size = sizeof(find_info_resp_bytes_2),
+};
+
 static void test_request_callback(uint8_t opcode, const void *param,
 					uint16_t length, void *user_data)
 {
@@ -267,7 +329,16 @@ static void test_request_callback(uint8_t opcode, const void *param,
 	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));
+	if (length == 0 || !param) {
+		g_assert(length == 0 && !param);
+		context_quit(test->context);
+		return;
+	}
+
+	if (test_data->compare_func)
+		g_assert(test_data->compare_func(param, test_data->resp_param));
+	else
+		g_assert(0 == memcmp(param, test_data->resp_param, length));
 
 	context_quit(test->context);
 }
@@ -302,6 +373,8 @@ int main(int argc, char *argv[])
 
 	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);
+	g_test_add_data_func("/att/request/3", &request_test_3, test_request);
+	g_test_add_data_func("/att/request/4", &request_test_4, test_request);
 
 	return g_test_run();
 }
-- 
1.9.1.423.g4596e3a

[PATCH 07/10] tools/btatt: Add "find-information" command.

[Arman Uguray]

Added command for the "Find Information" request. Sample run:

$ btatt -d 00:02:5B:00:15:10 -v find-information 0x0001 0xffff
att: ATT command 0x04
att: < 04 01 00 ff ff
att: > 05 01 01 00 00 28 02 00 03 28 03 00 00 2a 04 00 03 28 05 00 01 2a
Find Information response:
	Format: 0x01
	Information Data:
		handle: 0x0001, uuid: 2800
		handle: 0x0002, uuid: 2803
		handle: 0x0003, uuid: 2a00
		handle: 0x0004, uuid: 2803
		handle: 0x0005, uuid: 2a01
---
 tools/btatt.c | 131 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 129 insertions(+), 2 deletions(-)

diff --git a/tools/btatt.c b/tools/btatt.c
index a21625f..517ef6b 100644
--- a/tools/btatt.c
+++ b/tools/btatt.c
@@ -33,6 +33,8 @@
 #include <bluetooth/l2cap.h>
 
 #include "monitor/mainloop.h"
+#include "lib/uuid.h"
+#include "src/shared/util.h"
 #include "src/shared/att.h"
 
 #define ATT_CID 4
@@ -63,7 +65,7 @@ static void exchange_mtu_cb(uint8_t opcode, const void *param,
 	}
 
 	if (opcode != ATT_OP_EXCHANGE_MTU_RESP) {
-		fprintf(stderr, "Invalid response opcode (%u)\n", opcode);
+		fprintf(stderr, "Invalid response opcode (0x%02x)\n", opcode);
 		goto done;
 	}
 
@@ -130,6 +132,130 @@ static void cmd_mtu(struct bt_att *att, int argc, char **argv)
 	}
 }
 
+static void find_info_cb(uint8_t opcode, const void *param,
+						uint16_t len, void *user_data)
+{
+	const struct bt_att_find_information_rsp_param *rp;
+	bt_uuid_t uuid;
+	char uuidstr[MAX_LEN_UUID_STR];
+	int pair_count, pair_size;
+	uint128_t u128;
+	const uint8_t *data_ptr;
+	int i;
+
+	if (opcode == ATT_OP_ERROR_RESP) {
+		handle_error(param, len);
+		goto done;
+	}
+
+	if (opcode != ATT_OP_FIND_INFORMATION_RESP) {
+		fprintf(stderr, "Invalid response opcode (0x%02x)\n", opcode);
+		goto done;
+	}
+
+	rp = param;
+
+	if (len != sizeof(*rp)) {
+		fprintf(stderr, "Invalid \"Find Information\" response length "
+								"(%u)\n", len);
+		goto done;
+	}
+
+	pair_size = 2;
+	if (rp->format == 0x01)
+		pair_size += 2;
+	else if (rp->format == 0x02)
+		pair_size += 16;
+	else {
+		fprintf(stderr, "Invalid \"Find Information\" response format "
+						"(0x%02x)\n", rp->format);
+		goto done;
+	}
+
+	pair_count = rp->length / pair_size;
+
+	printf("Find Information response:\n" "\tFormat: 0x%02x\n", rp->format);
+	printf("\tInformation Data:\n");
+
+	data_ptr = rp->information_data;
+	for (i = 0; i < pair_count; i++) {
+		printf("\t\thandle: 0x%04x", get_le16(data_ptr));
+
+		if (rp->format == 0x01) {
+			bt_uuid16_create(&uuid, get_le16(data_ptr + 2));
+		} else {
+			bswap_128(data_ptr + 2, &u128);
+			bt_uuid128_create(&uuid, u128);
+		}
+
+		bt_uuid_to_string(&uuid, uuidstr, MAX_LEN_UUID_STR);
+		printf(", uuid: %s\n", uuidstr);
+
+		data_ptr += pair_size;
+	}
+
+done:
+	mainloop_quit();
+}
+
+static void find_info_usage(void)
+{
+	printf("Usage: btatt find-information <start handle> <end handle>\n");
+}
+
+static struct option find_info_options[] = {
+	{ "help", 	0, 0, 'h'},
+	{}
+};
+
+static void cmd_find_info(struct bt_att *att, int argc, char **argv)
+{
+	struct bt_att_find_information_req_param param;
+	uint16_t start, end;
+	int opt;
+
+	while ((opt = getopt_long(argc, argv, "+h", find_info_options,
+								NULL)) != -1) {
+		switch (opt) {
+		case 'h':
+		default:
+			find_info_usage();
+			exit(EXIT_SUCCESS);
+		}
+	}
+
+	argc -= optind;
+	argv += optind;
+	optind = 0;
+
+	if (argc < 2) {
+		find_info_usage();
+		exit(EXIT_FAILURE);
+	}
+
+	start = strtol(argv[0], NULL, 0);
+	if (!start) {
+		fprintf(stderr, "Invalid start handle: %s\n", argv[0]);
+		exit(EXIT_FAILURE);
+	}
+
+	end = strtol(argv[1], NULL, 0);
+	if (!end) {
+		fprintf(stderr, "Invalid end handle: %s\n", argv[1]);
+		exit(EXIT_FAILURE);
+	}
+
+	memset(&param, 0, sizeof(param));
+	param.start_handle = start;
+	param.end_handle = end;
+
+	if (bt_att_send(att, ATT_OP_FIND_INFORMATION_REQ, &param,
+				sizeof(param), find_info_cb, NULL, NULL) == 0) {
+		fprintf(stderr, "Unable to send \"Find Information\" request\n");
+		exit(EXIT_FAILURE);
+	}
+}
+
 static int l2cap_le_att_connect(bdaddr_t *src, bdaddr_t *dst, uint8_t dst_type)
 {
 	int sock;
@@ -184,7 +310,8 @@ static struct {
 	void (*func)(struct bt_att *att, int argc, char **argv);
 	char *doc;
 } command[] = {
-	{ "exchange-mtu",	cmd_mtu,	"\"Exchange MTU\" request." },
+	{ "exchange-mtu", cmd_mtu, "\"Exchange MTU\" request" },
+	{ "find-information", cmd_find_info, "\"Find Information\" request" },
 	{ }
 };
 
-- 
1.9.1.423.g4596e3a

[PATCH 08/10] src/shared/att: Add "Find By Type Value" request and response.

[Arman Uguray]

This patch adds support for the "Find By Type Value" request and
response with their corresponding parameter structures.
---
 src/shared/att.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/shared/att.h | 16 +++++++++++
 2 files changed, 102 insertions(+)

diff --git a/src/shared/att.c b/src/shared/att.c
index 25112c7..44e2abd 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -212,6 +212,11 @@ static bool handle_find_info_rsp(struct bt_att *att, const uint8_t *pdu,
 	struct bt_att_find_information_rsp_param param;
 	uint8_t *information_data;
 
+	/* PDU must contain at least the following:
+	 * - Attribute Opcode (1 octet)
+	 * - Format (1 octet)
+	 * - Information Data (4 to MTU-2 octets)
+	 */
 	if (pdu_length < 6)
 		return false;
 
@@ -237,6 +242,45 @@ static bool handle_find_info_rsp(struct bt_att *att, const uint8_t *pdu,
 	return true;
 }
 
+static bool handle_find_by_type_value_rsp(struct bt_att *att,
+							const uint8_t *pdu,
+							uint16_t pdu_length)
+{
+	struct bt_att_find_by_type_value_rsp_param param;
+	uint8_t *handles_info_list;
+
+	/* PDU must contain at least the following:
+	 * - Attribute Opcode (1 octet)
+	 * - Handles Information List (4 to MTU-1 octets)
+	 */
+	if (pdu_length < 5)
+		return false;
+
+	/* Each Handle Information field is composed of 4 octets. Return error,
+	 * if the length of the field is not a multiple of 4.
+	 */
+	if ((pdu_length - 1) % 4)
+		return false;
+
+	memset(&param, 0, sizeof(param));
+
+	param.length = pdu_length - 1;
+	handles_info_list = malloc(param.length);
+	if (!handles_info_list)
+		return false;
+
+	memcpy(handles_info_list, pdu + 1, param.length);
+
+	param.handles_information_list = handles_info_list;
+
+	request_complete(att, ATT_OP_FIND_BY_TYPE_VALUE_REQ, pdu[0], &param,
+								sizeof(param));
+
+	free(handles_info_list);
+
+	return true;
+}
+
 static bool handle_response(struct bt_att *att, const uint8_t *pdu,
 							uint16_t pdu_length)
 {
@@ -261,6 +305,13 @@ static bool handle_response(struct bt_att *att, const uint8_t *pdu,
 		}
 		return true;
 
+	case ATT_OP_FIND_BY_TYPE_VALUE_RESP:
+		if (!handle_find_by_type_value_rsp(att, pdu, pdu_length)) {
+			req_opcode = ATT_OP_FIND_BY_TYPE_VALUE_REQ;
+			goto fail;
+		}
+		return true;
+
 	default:
 		break;
 	}
@@ -488,6 +539,37 @@ static bool encode_find_info_req(const void *param, uint16_t length, void *buf,
 	return true;
 }
 
+static bool encode_find_by_type_value_req(const void *param, uint16_t length,
+							void *buf, uint16_t mtu,
+							uint16_t *pdu_length)
+{
+	const struct bt_att_find_by_type_value_req_param *cp = param;
+	const uint16_t len =  7 + cp->length;
+	uint8_t *bytes = buf;
+
+	if (length != sizeof(*cp))
+		return false;
+
+	if (len > mtu)
+		return false;
+
+	put_le16(cp->start_handle, bytes + 1);
+	put_le16(cp->end_handle, bytes + 3);
+	put_le16(cp->type, bytes + 5);
+
+	*pdu_length = len;
+
+	if (cp->length == 0)
+		return true;
+
+	if (!cp->value)
+		return false;
+
+	memcpy(bytes + 7, cp->value, cp->length);
+
+	return true;
+}
+
 static bool encode_pdu(uint8_t opcode, const void *param, uint16_t length,
 				uint16_t mtu, void *pdu, uint16_t *pdu_length)
 {
@@ -516,6 +598,10 @@ static bool encode_pdu(uint8_t opcode, const void *param, uint16_t length,
 		return encode_find_info_req(param, length, bytes,
 							mtu, pdu_length);
 
+	if (opcode == ATT_OP_FIND_BY_TYPE_VALUE_REQ)
+		return encode_find_by_type_value_req(param, length, bytes,
+							mtu, pdu_length);
+
 	return false;
 }
 
diff --git a/src/shared/att.h b/src/shared/att.h
index b9b4037..82a8321 100644
--- a/src/shared/att.h
+++ b/src/shared/att.h
@@ -57,6 +57,22 @@ struct bt_att_find_information_rsp_param {
 	uint16_t length;
 };
 
+/* Find By Type Value */
+#define ATT_OP_FIND_BY_TYPE_VALUE_REQ	0x06
+struct bt_att_find_by_type_value_req_param {
+	uint16_t start_handle;
+	uint16_t end_handle;
+	uint16_t type;  /* 2 octet UUID */
+	const uint8_t *value;
+	uint16_t length;  /* MAX length: (ATT_MTU - 7) */
+};
+
+#define ATT_OP_FIND_BY_TYPE_VALUE_RESP	0x07
+struct bt_att_find_by_type_value_rsp_param {
+	const uint8_t *handles_information_list;
+	uint16_t length;
+};
+
 /* Error codes for Error response PDU */
 #define ATT_ERROR_INVALID_HANDLE			0x01
 #define ATT_ERROR_READ_NOT_PERMITTED			0x02
-- 
1.9.1.423.g4596e3a

[PATCH 09/10] unit/test-att: Add unit test for "Find By Type Value" request/response.

[Arman Uguray]

Added test cases for the "Find By Type Value" request/response.
---
 unit/test-att.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 99 insertions(+), 6 deletions(-)

diff --git a/unit/test-att.c b/unit/test-att.c
index e45342a..86864c8 100644
--- a/unit/test-att.c
+++ b/unit/test-att.c
@@ -201,7 +201,7 @@ struct request_test_data {
 	uint16_t resp_param_length;
 	const void *req_data;
 	uint16_t req_size;
-	const  void *resp_data;
+	const void *resp_data;
 	uint16_t resp_size;
 	bool (*compare_func)(const void *a, const void *b);
 };
@@ -234,11 +234,11 @@ static const struct request_test_data request_test_1 = {
 };
 
 /* Exchange MTU request <-> error response test */
-static const unsigned char exchange_mtu_req_bytes_2[] = { 0x02, 0x32, 0x00 };
+static const uint8_t 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[] = {
+static const uint8_t exchange_mtu_resp_bytes_2[] = {
 	0x01, 0x02, 0x00, 0x00, 0x06
 };
 static const struct bt_att_error_rsp_param exchange_mtu_rsp_param_2 = {
@@ -272,14 +272,14 @@ static bool compare_find_info_rsp(const void *a, const void *b)
 							p2->length) == 0;
 }
 
-static const unsigned char find_info_req_bytes[] = {
+static const uint8_t find_info_req_bytes[] = {
 	0x04, 0x01, 0x00, 0xff, 0xff
 };
 static const struct bt_att_find_information_req_param find_info_req_param = {
 	.start_handle = 0x0001,
 	.end_handle = 0xffff,
 };
-static const unsigned char find_info_resp_bytes_1[] = {
+static const uint8_t find_info_resp_bytes_1[] = {
 	0x05, 0x01, 0x01, 0x00, 0x0d, 0x18
 };
 static const uint8_t find_info_resp_information_data_1[] = {
@@ -305,7 +305,7 @@ static const struct request_test_data request_test_3 = {
 };
 
 /* Find Information request <-> invalid response */
-static const unsigned char find_info_resp_bytes_2[] = {
+static const uint8_t find_info_resp_bytes_2[] = {
 	0x05, 0x01
 };
 
@@ -320,6 +320,96 @@ static const struct request_test_data request_test_4 = {
 	.resp_size = sizeof(find_info_resp_bytes_2),
 };
 
+/* Find By Type Value request <-> response */
+static bool compare_find_by_type_val_rsp(const void *a, const void *b)
+{
+	const struct bt_att_find_by_type_value_rsp_param *p1 = a;
+	const struct bt_att_find_by_type_value_rsp_param *p2 = b;
+
+	if (p1->length != p2->length)
+		return false;
+
+	return memcmp(p1->handles_information_list,
+				p2->handles_information_list, p2->length) == 0;
+}
+
+static const uint8_t find_by_type_val_req_bytes_1[] = {
+	0x06, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28, 0x01, 0x02, 0x03, 0x04
+};
+static const uint8_t find_by_type_val_req_att_value[] = {
+	0x01, 0x02, 0x03, 0x04
+};
+static const struct bt_att_find_by_type_value_req_param
+						find_by_type_val_rqp_1 = {
+	.start_handle = 0x0001,
+	.end_handle = 0xffff,
+	.type = 0x2800,
+	.value = find_by_type_val_req_att_value,
+	.length = sizeof(find_by_type_val_req_att_value),
+};
+static const uint8_t find_by_type_val_rsp_bytes_1[] = {
+	0x07, 0x01, 0x00, 0x02, 0x00, 0x02, 0x00, 0x03, 0x00
+};
+static const struct bt_att_find_by_type_value_rsp_param find_by_type_val_rpp = {
+	.handles_information_list = find_by_type_val_rsp_bytes_1 + 1,
+	.length = sizeof(find_by_type_val_rsp_bytes_1) - 1,
+};
+
+static const struct request_test_data request_test_5 = {
+	.req_opcode = ATT_OP_FIND_BY_TYPE_VALUE_REQ,
+	.req_param = &find_by_type_val_rqp_1,
+	.req_param_length = sizeof(find_by_type_val_rqp_1),
+	.resp_opcode = ATT_OP_FIND_BY_TYPE_VALUE_RESP,
+	.resp_param = &find_by_type_val_rpp,
+	.resp_param_length = sizeof(find_by_type_val_rpp),
+	.req_data = find_by_type_val_req_bytes_1,
+	.req_size = sizeof(find_by_type_val_req_bytes_1),
+	.resp_data = find_by_type_val_rsp_bytes_1,
+	.resp_size = sizeof(find_by_type_val_rsp_bytes_1),
+	.compare_func = compare_find_by_type_val_rsp,
+};
+
+/* Find By Type Value request <-> invalid response */
+static const uint8_t find_by_type_val_rsp_bytes_2[] = {
+	0x07, 0x01, 0x00, 0x02
+};
+
+static const struct request_test_data request_test_6 = {
+	.req_opcode = ATT_OP_FIND_BY_TYPE_VALUE_REQ,
+	.req_param = &find_by_type_val_rqp_1,
+	.req_param_length = sizeof(find_by_type_val_rqp_1),
+	.resp_opcode = ATT_OP_ERROR_RESP,
+	.req_data = find_by_type_val_req_bytes_1,
+	.req_size = sizeof(find_by_type_val_req_bytes_1),
+	.resp_data = find_by_type_val_rsp_bytes_2,
+	.resp_size = sizeof(find_by_type_val_rsp_bytes_2),
+};
+
+/* Find By Type Value request no value <-> response */
+static const uint8_t find_by_type_val_req_bytes_2[] = {
+	0x06, 0x01, 0x00, 0xff, 0xff, 0x00, 0x28
+};
+static const struct bt_att_find_by_type_value_req_param
+						find_by_type_val_rqp_2 = {
+	.start_handle = 0x0001,
+	.end_handle = 0xffff,
+	.type = 0x2800,
+};
+
+static const struct request_test_data request_test_7 = {
+	.req_opcode = ATT_OP_FIND_BY_TYPE_VALUE_REQ,
+	.req_param = &find_by_type_val_rqp_2,
+	.req_param_length = sizeof(find_by_type_val_rqp_2),
+	.resp_opcode = ATT_OP_FIND_BY_TYPE_VALUE_RESP,
+	.resp_param = &find_by_type_val_rpp,
+	.resp_param_length = sizeof(find_by_type_val_rpp),
+	.req_data = find_by_type_val_req_bytes_2,
+	.req_size = sizeof(find_by_type_val_req_bytes_2),
+	.resp_data = find_by_type_val_rsp_bytes_1,
+	.resp_size = sizeof(find_by_type_val_rsp_bytes_1),
+	.compare_func = compare_find_by_type_val_rsp,
+};
+
 static void test_request_callback(uint8_t opcode, const void *param,
 					uint16_t length, void *user_data)
 {
@@ -375,6 +465,9 @@ int main(int argc, char *argv[])
 	g_test_add_data_func("/att/request/2", &request_test_2, test_request);
 	g_test_add_data_func("/att/request/3", &request_test_3, test_request);
 	g_test_add_data_func("/att/request/4", &request_test_4, test_request);
+	g_test_add_data_func("/att/request/5", &request_test_5, test_request);
+	g_test_add_data_func("/att/request/6", &request_test_6, test_request);
+	g_test_add_data_func("/att/request/7", &request_test_7, test_request);
 
 	return g_test_run();
 }
-- 
1.9.1.423.g4596e3a

[PATCH 10/10] tools/btatt: Add "find-by-type-value" command.

[Arman Uguray]

Added command for the "Find By Type Value" request. Sample runs:

$ btatt -d 00:02:5B:00:15:10 -v find-by-type-value 0x0001 0xffff
att: ATT command 0x06
att: < 06 01 00 ff ff 00 28
att: > 01 06 01 00 0a
Error response:
	opcode: 0x06
	handle: 0x0001
	error: 0x0a

$ btatt -d 00:02:5B:00:15:10 -v find-by-type-value 0x0001 0xffff 01 18
att: ATT command 0x06
att: < 06 01 00 ff ff 00 28 01 08
att: > 07 08 00 08 00
Find By Type Value response:
	Handle Information List:
		Attr Handle: 0x0008, Grp End Handle: 0x0008
---
 tools/btatt.c | 163 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 161 insertions(+), 2 deletions(-)

diff --git a/tools/btatt.c b/tools/btatt.c
index 517ef6b..25f3adc 100644
--- a/tools/btatt.c
+++ b/tools/btatt.c
@@ -28,6 +28,7 @@
 #include <stdio.h>
 #include <getopt.h>
 #include <string.h>
+#include <errno.h>
 
 #include <bluetooth/bluetooth.h>
 #include <bluetooth/l2cap.h>
@@ -50,8 +51,11 @@ static void handle_error(const void *param, uint16_t len)
 		return;
 	}
 
-	printf("Request failed:\n\topcode:\t%u\n\thandle:\t%u\n\terror:\t%u\n",
-			rp->request_opcode, rp->handle, rp->error_code);
+	printf("Error response:\n"
+				"\topcode:\t0x%02x\n"
+				"\thandle:\t0x%04x\n"
+				"\terror:\t0x%02x\n",
+				rp->request_opcode, rp->handle, rp->error_code);
 }
 
 static void exchange_mtu_cb(uint8_t opcode, const void *param,
@@ -256,6 +260,159 @@ static void cmd_find_info(struct bt_att *att, int argc, char **argv)
 	}
 }
 
+static void find_by_type_val_cb(uint8_t opcode, const void *param,
+						uint16_t len, void *user_data)
+{
+	const struct bt_att_find_by_type_value_rsp_param *rp;
+	uint16_t att_handle, grp_handle;
+	const uint8_t *data_ptr;
+	int num_handle_infos;
+	int i;
+
+	if (opcode == ATT_OP_ERROR_RESP) {
+		handle_error(param, len);
+		goto done;
+	}
+
+	if (opcode != ATT_OP_FIND_BY_TYPE_VALUE_RESP) {
+		fprintf(stderr, "Invalid response opcode (0x%02x)\n", opcode);
+		goto done;
+	}
+
+	rp = param;
+
+	if (len != sizeof(*rp)) {
+		fprintf(stderr, "Invalid \"Find By Type Value\" response length"
+								" (%u)\n", len);
+		goto done;
+	}
+
+	if (rp->length == 0) {
+		fprintf(stderr, "\"Handle Info. List\" field is empty!\n");
+		goto done;
+	}
+
+	if (rp->length % 4) {
+		fprintf(stderr, "Malformed response\n");
+		goto done;
+	}
+
+	num_handle_infos = rp->length / 4;
+
+	printf("Find By Type Value response:\n");
+	printf("\tHandle Information List:\n");
+
+	data_ptr = rp->handles_information_list;
+	for (i = 0; i < num_handle_infos; i++) {
+		printf("\t\tAttr Handle: 0x%04x, Grp End Handle: 0x%04x\n",
+							get_le16(data_ptr),
+							get_le16(data_ptr + 2));
+
+		data_ptr += 4;
+	}
+
+done:
+	mainloop_quit();
+}
+
+static void find_by_type_val_usage(void)
+{
+	printf("Usage: btatt find-by-type-value <start handle> <end handle> "
+							"<uuid> <value>\n");
+
+	printf("e.g. btatt find-by-type-value 0x0001 0xFFFF 0x280C 00 01\n");
+}
+
+static struct option find_by_type_val_options[] = {
+	{ "help",	0, 0, 'h'},
+	{}
+};
+
+static void cmd_find_by_type_val(struct bt_att *att, int argc, char **argv)
+{
+	struct bt_att_find_by_type_value_req_param param;
+	uint16_t start, end, type, length;
+	uint8_t *value = NULL;
+	int opt;
+
+	while ((opt = getopt_long(argc, argv, "+h", find_by_type_val_options,
+								NULL)) != -1) {
+		switch (opt) {
+		case 'h':
+		default:
+			find_by_type_val_usage();
+			exit(EXIT_SUCCESS);
+		}
+	}
+
+	argc -= optind;
+	argv += optind;
+
+	if (argc < 3) {
+		find_by_type_val_usage();
+		exit(EXIT_FAILURE);
+	}
+
+	start = strtol(argv[0], NULL, 0);
+	if (!start) {
+		fprintf(stderr, "Invalid start handle: %s\n", argv[0]);
+		exit(EXIT_FAILURE);
+	}
+
+	end = strtol(argv[1], NULL, 0);
+	if (!end) {
+		fprintf(stderr, "Invalid end handle: %s\n", argv[1]);
+		exit(EXIT_FAILURE);
+	}
+
+	type = strtol(argv[2], NULL, 0);
+	if (!type) {
+		fprintf(stderr, "Invalid 16-bit UUID: %s\n", argv[2]);
+		exit(EXIT_FAILURE);
+	}
+
+	length = argc - 3;
+
+	if (length > 0) {
+		int i;
+
+		value = malloc(length);
+
+		for (i = 3; i < argc; i++) {
+			if (strlen(argv[i]) != 2) {
+				fprintf(stderr, "Invalid value byte: %s\n",
+								argv[i]);
+				free(value);
+				exit(EXIT_FAILURE);
+			}
+
+			value[i-3] = strtol(argv[i], NULL, 16);
+			if (errno) {
+				perror("Error parsing value");
+				free(value);
+				exit(EXIT_FAILURE);
+			}
+		}
+	}
+
+	memset(&param, 0, sizeof(param));
+	param.start_handle = start;
+	param.end_handle = end;
+	param.type = type;
+	param.value = value;
+	param.length = length;
+
+	if (!bt_att_send(att, ATT_OP_FIND_BY_TYPE_VALUE_REQ, &param,
+			sizeof(param), find_by_type_val_cb, NULL, NULL)) {
+		fprintf(stderr, "Unable to send \"Find By Type Value\" "
+								"request\n");
+		free(value);
+		exit(EXIT_FAILURE);
+	}
+
+	free(value);
+}
+
 static int l2cap_le_att_connect(bdaddr_t *src, bdaddr_t *dst, uint8_t dst_type)
 {
 	int sock;
@@ -312,6 +469,8 @@ static struct {
 } command[] = {
 	{ "exchange-mtu", cmd_mtu, "\"Exchange MTU\" request" },
 	{ "find-information", cmd_find_info, "\"Find Information\" request" },
+	{ "find-by-type-value", cmd_find_by_type_val,
+						"\"Find By Type Value\" request" },
 	{ }
 };
 
-- 
1.9.1.423.g4596e3a

Re: [PATCH 00/10] bt_att initial implementation

[Luiz Augusto von Dentz]

Hi Arman,

On Tue, May 6, 2014 at 7:45 AM, Arman Uguray <armansito@chromium.org> wrote:
> This patch set introduces "struct bt_att" which handles the transport logic for
> interactions over the Attribute Protocol. struct bt_att is meant to be
> connection-agnostic: It enables a new GATT/ATT stack based around src/shared/io,
> so that the library internals of how the connection is managed (glib vs
> mainloop) is not tied to the protocol implementation. bt_att will initially
> support the client-side protocol and eventually extended to support the server
> side.
>
> Major changes introduced in this patch-set:
>   - src/shared/att.[h,cc], which introduces "struct bt_att", with initial
>     support for client-initiated requests and asynchronous response handling via
>     a callback.
>   - unit/test-att, which defines unit tests for request/response using a
>     socketpair.
>   - tools/btatt, which is a command-line tool for debugging the Attribute
>     Protocol.
>
> The majority of the above are introduced in patches 1 through 3, which consist
> almost entirely of boilerplate taken from src/shared/mgmt.*, unit/test-mgmt, and
> tools/btmgmt.c.
>
> The 9 patches introduced here are the initial portion of 20+ patches where each
> ATT operation is implemented in groups of 3 patches:
>   - 1 patch that implements the request/response or command in src/shared/att
>   - 1 patch that adds a unit test to unit/test-att
>   - 1 patch that adds a command for the request to tools/btatt
>
> Arman Uguray (10):
>   src/shared/att: Introduce struct bt_att.
>   unit/test-att: Add unit tests for src/shared/att.
>   tools/btatt: Add command-line tool for ATT protocol testing.
>   tools/btatt: Add "exchange-mtu" command.
>   src/shared/att: Add "Find Information" request and response.
>   unit/test-att: Add unit test for "Find Information" request/response.
>   tools/btatt: Add "find-information" command.
>   src/shared/att: Add "Find By Type Value" request and response.
>   unit/test-att: Add unit test for "Find By Type Value"
>     request/response.
>   tools/btatt: Add "find-by-type-value" command.
>
>  Makefile.am      |  12 +-
>  Makefile.tools   |  10 +-
>  src/shared/att.c | 689 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/shared/att.h | 124 ++++++++++
>  tools/btatt.c    | 626 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  unit/test-att.c  | 473 ++++++++++++++++++++++++++++++++++++++
>  6 files changed, 1932 insertions(+), 2 deletions(-)
>  create mode 100644 src/shared/att.c
>  create mode 100644 src/shared/att.h
>  create mode 100644 tools/btatt.c
>  create mode 100644 unit/test-att.c
>
> --
> 1.9.1.423.g4596e3a

Apparently you have ignored my comments regarding the API, it is
probably not the best thing to base everything on mgmt API since that
is a internal protocol which does not need encoding as everything is
passed in host order, also unlike mgmt you have GATT on top o ATT so
it is beneficial to use iovec in the API.

Regarding unit tests, I think you should take a look at test-av*, and
there exists a test specification for ATT/GATT so we should start with
it since is what PTS uses for qualification:
https://www.bluetooth.org/docman/handlers/downloaddoc.ashx?doc_id=229871

You can use the name and description of the unit test following the
test specification:

/TP/GAC/CL/BV-01-C: Verify that a Generic Attribute Profile client can
generate an Exchange MTU Request command  to configure ATT_MTU over
LE.

Re: [PATCH 00/10] bt_att initial implementation

[Luiz Augusto von Dentz]

Hi Arman,

On Tue, May 6, 2014 at 11:29 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Arman,
>
> On Tue, May 6, 2014 at 7:45 AM, Arman Uguray <armansito@chromium.org> wrote:
>> This patch set introduces "struct bt_att" which handles the transport logic for
>> interactions over the Attribute Protocol. struct bt_att is meant to be
>> connection-agnostic: It enables a new GATT/ATT stack based around src/shared/io,
>> so that the library internals of how the connection is managed (glib vs
>> mainloop) is not tied to the protocol implementation. bt_att will initially
>> support the client-side protocol and eventually extended to support the server
>> side.
>>
>> Major changes introduced in this patch-set:
>>   - src/shared/att.[h,cc], which introduces "struct bt_att", with initial
>>     support for client-initiated requests and asynchronous response handling via
>>     a callback.
>>   - unit/test-att, which defines unit tests for request/response using a
>>     socketpair.
>>   - tools/btatt, which is a command-line tool for debugging the Attribute
>>     Protocol.
>>
>> The majority of the above are introduced in patches 1 through 3, which consist
>> almost entirely of boilerplate taken from src/shared/mgmt.*, unit/test-mgmt, and
>> tools/btmgmt.c.
>>
>> The 9 patches introduced here are the initial portion of 20+ patches where each
>> ATT operation is implemented in groups of 3 patches:
>>   - 1 patch that implements the request/response or command in src/shared/att
>>   - 1 patch that adds a unit test to unit/test-att
>>   - 1 patch that adds a command for the request to tools/btatt
>>
>> Arman Uguray (10):
>>   src/shared/att: Introduce struct bt_att.
>>   unit/test-att: Add unit tests for src/shared/att.
>>   tools/btatt: Add command-line tool for ATT protocol testing.
>>   tools/btatt: Add "exchange-mtu" command.
>>   src/shared/att: Add "Find Information" request and response.
>>   unit/test-att: Add unit test for "Find Information" request/response.
>>   tools/btatt: Add "find-information" command.
>>   src/shared/att: Add "Find By Type Value" request and response.
>>   unit/test-att: Add unit test for "Find By Type Value"
>>     request/response.
>>   tools/btatt: Add "find-by-type-value" command.
>>
>>  Makefile.am      |  12 +-
>>  Makefile.tools   |  10 +-
>>  src/shared/att.c | 689 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  src/shared/att.h | 124 ++++++++++
>>  tools/btatt.c    | 626 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  unit/test-att.c  | 473 ++++++++++++++++++++++++++++++++++++++
>>  6 files changed, 1932 insertions(+), 2 deletions(-)
>>  create mode 100644 src/shared/att.c
>>  create mode 100644 src/shared/att.h
>>  create mode 100644 tools/btatt.c
>>  create mode 100644 unit/test-att.c
>>
>> --
>> 1.9.1.423.g4596e3a
>
> Apparently you have ignored my comments regarding the API, it is
> probably not the best thing to base everything on mgmt API since that
> is a internal protocol which does not need encoding as everything is
> passed in host order, also unlike mgmt you have GATT on top o ATT so
> it is beneficial to use iovec in the API.

So I had a little chat offline with Johan regarding this and perhaps
you don't really need to introduce iovec into the API since GATT/ATT
framing is quite simple, so please ignore this comment.

> Regarding unit tests, I think you should take a look at test-av*, and
> there exists a test specification for ATT/GATT so we should start with
> it since is what PTS uses for qualification:
> https://www.bluetooth.org/docman/handlers/downloaddoc.ashx?doc_id=229871
>
> You can use the name and description of the unit test following the
> test specification:
>
> /TP/GAC/CL/BV-01-C: Verify that a Generic Attribute Profile client can
> generate an Exchange MTU Request command  to configure ATT_MTU over
> LE.

This is still relevant and perhaps it should involve testing gatt-db
as well since the testing spec also involves the server, which leads
to the question if we should keep them apart? I guess this should be
done in a single place since we will be controlling the fd, or the
idea is that bt_att should access gatt_db internally to handle
incoming requests?



-- 
Luiz Augusto von Dentz

Re: [PATCH 00/10] bt_att initial implementation

[Marcel Holtmann]

Hi Luiz,

>> This patch set introduces "struct bt_att" which handles the transport logic for
>> interactions over the Attribute Protocol. struct bt_att is meant to be
>> connection-agnostic: It enables a new GATT/ATT stack based around src/shared/io,
>> so that the library internals of how the connection is managed (glib vs
>> mainloop) is not tied to the protocol implementation. bt_att will initially
>> support the client-side protocol and eventually extended to support the server
>> side.
>> 
>> Major changes introduced in this patch-set:
>>  - src/shared/att.[h,cc], which introduces "struct bt_att", with initial
>>    support for client-initiated requests and asynchronous response handling via
>>    a callback.
>>  - unit/test-att, which defines unit tests for request/response using a
>>    socketpair.
>>  - tools/btatt, which is a command-line tool for debugging the Attribute
>>    Protocol.
>> 
>> The majority of the above are introduced in patches 1 through 3, which consist
>> almost entirely of boilerplate taken from src/shared/mgmt.*, unit/test-mgmt, and
>> tools/btmgmt.c.
>> 
>> The 9 patches introduced here are the initial portion of 20+ patches where each
>> ATT operation is implemented in groups of 3 patches:
>>  - 1 patch that implements the request/response or command in src/shared/att
>>  - 1 patch that adds a unit test to unit/test-att
>>  - 1 patch that adds a command for the request to tools/btatt
>> 
>> Arman Uguray (10):
>>  src/shared/att: Introduce struct bt_att.
>>  unit/test-att: Add unit tests for src/shared/att.
>>  tools/btatt: Add command-line tool for ATT protocol testing.
>>  tools/btatt: Add "exchange-mtu" command.
>>  src/shared/att: Add "Find Information" request and response.
>>  unit/test-att: Add unit test for "Find Information" request/response.
>>  tools/btatt: Add "find-information" command.
>>  src/shared/att: Add "Find By Type Value" request and response.
>>  unit/test-att: Add unit test for "Find By Type Value"
>>    request/response.
>>  tools/btatt: Add "find-by-type-value" command.
>> 
>> Makefile.am      |  12 +-
>> Makefile.tools   |  10 +-
>> src/shared/att.c | 689 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> src/shared/att.h | 124 ++++++++++
>> tools/btatt.c    | 626 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> unit/test-att.c  | 473 ++++++++++++++++++++++++++++++++++++++
>> 6 files changed, 1932 insertions(+), 2 deletions(-)
>> create mode 100644 src/shared/att.c
>> create mode 100644 src/shared/att.h
>> create mode 100644 tools/btatt.c
>> create mode 100644 unit/test-att.c
>> 
>> --
>> 1.9.1.423.g4596e3a
> 
> Apparently you have ignored my comments regarding the API, it is
> probably not the best thing to base everything on mgmt API since that
> is a internal protocol which does not need encoding as everything is
> passed in host order, also unlike mgmt you have GATT on top o ATT so
> it is beneficial to use iovec in the API.

what now? mgmt is little-endian and not host order. I do not design host order protocols ;)

Actually ATT and mgmt are pretty much similar. ATT is one transaction at a time. It is send a request, get response. If that transaction model is failing, you disconnect.

I am pretty much in favor of just being plain simple when it comes to pure ATT. The GATT layer can make semantic sense out of the ATT PDUs. For all intense and purposes ATT is just some PDUs that are exchanged.

Regards

Marcel

Re: [PATCH 00/10] bt_att initial implementation

[Arman Uguray]

Hi Luiz,

>> Apparently you have ignored my comments regarding the API, it is
>> probably not the best thing to base everything on mgmt API since that
>> is a internal protocol which does not need encoding as everything is
>> passed in host order, also unlike mgmt you have GATT on top o ATT so
>> it is beneficial to use iovec in the API.

Sorry, I must have missed those comments in my inbox somewhere, so I
didn't mean to ignore them. That said, as Marcel also pointed out,
bt_att is only meant for ATT protocol transactions. It doesn't do any
state keeping apart from some internal queues for ATT request
control-flow, but that's pretty much it. The intended use is to create
a bt_att around a file descriptor, use bt_att_send with the opcode and
the relevant parameters to send requests/commands, bt_att_reply to
send replies to incoming requests/indications, and some other function
to subscribe to events. In that regard, the structure of
src/shared/mgmt is remarkably suitable for bt_att. The encoding of the
PDU, including the necessary endianness conversion, is handled by
bt_att.

That last bit of course is different from mgmt, where the parameters
structure definition itself encodes the data being sent over the
socket, whereas bt_att performs encoding/decoding logic. Either way, I
think this definition makes for a nice API.

>> Regarding unit tests, I think you should take a look at test-av*, and
>> there exists a test specification for ATT/GATT so we should start with
>> it since is what PTS uses for qualification:
>> https://www.bluetooth.org/docman/handlers/downloaddoc.ashx?doc_id=229871
>>
>> You can use the name and description of the unit test following the
>> test specification:
>>
>> /TP/GAC/CL/BV-01-C: Verify that a Generic Attribute Profile client can
>> generate an Exchange MTU Request command  to configure ATT_MTU over
>> LE.
>
> This is still relevant and perhaps it should involve testing gatt-db
> as well since the testing spec also involves the server, which leads
> to the question if we should keep them apart? I guess this should be
> done in a single place since we will be controlling the fd, or the
> idea is that bt_att should access gatt_db internally to handle
> incoming requests?

This is a very good point. Currently the idea is to have a bt_gatt
which defines GATT client-side functions for primary service
discovery, read/write requests etc, which would then internally use
bt_att for ATT protocol transactions, and gatt_db which solely serves
as a medium for storage of local/remote attributes. Basically, a
complete implementation of a GATT client/server would create and
manage individual instances of bt_gatt/bt_att for all requests and
gatt_db to explicitly store the results of those requests. In essence,
the database implementation and bt_gatt/bt_att won't need to
explicitly depend on each other.

This is why it might make sense to hold off on writing PTS style tests
until we have the complete picture in place. For now, I went with a
simple set of unit tests to make sure that basic PDU encoding/decoding
is done correctly for bt_att. As we then start putting gatt_db and
bt_gatt together, I say we then write tests in the fashion of
test-av*.

Cheers,
Arman

Re: [PATCH 00/10] bt_att initial implementation

[Luiz Augusto von Dentz]

Hi Arman,

On Wed, May 7, 2014 at 9:53 AM, Arman Uguray <armansito@chromium.org> wrote:
> Hi Luiz,
>
>>> Apparently you have ignored my comments regarding the API, it is
>>> probably not the best thing to base everything on mgmt API since that
>>> is a internal protocol which does not need encoding as everything is
>>> passed in host order, also unlike mgmt you have GATT on top o ATT so
>>> it is beneficial to use iovec in the API.
>
> Sorry, I must have missed those comments in my inbox somewhere, so I
> didn't mean to ignore them. That said, as Marcel also pointed out,
> bt_att is only meant for ATT protocol transactions. It doesn't do any
> state keeping apart from some internal queues for ATT request
> control-flow, but that's pretty much it. The intended use is to create
> a bt_att around a file descriptor, use bt_att_send with the opcode and
> the relevant parameters to send requests/commands, bt_att_reply to
> send replies to incoming requests/indications, and some other function
> to subscribe to events. In that regard, the structure of
> src/shared/mgmt is remarkably suitable for bt_att. The encoding of the
> PDU, including the necessary endianness conversion, is handled by
> bt_att.
>
> That last bit of course is different from mgmt, where the parameters
> structure definition itself encodes the data being sent over the
> socket, whereas bt_att performs encoding/decoding logic. Either way, I
> think this definition makes for a nice API.

If you can really tell by opcode how to encode/decode the data that
should be fine, regarding the API style I would favor functions vs use
of opcode + struct, I understand in MGMT case we did this to define
the protocol at PDU level so anyone could use those to implement a
library thus why it is placed under lib/. Anyway Im not against either
style since we can still change if we need.

>>> Regarding unit tests, I think you should take a look at test-av*, and
>>> there exists a test specification for ATT/GATT so we should start with
>>> it since is what PTS uses for qualification:
>>> https://www.bluetooth.org/docman/handlers/downloaddoc.ashx?doc_id=229871
>>>
>>> You can use the name and description of the unit test following the
>>> test specification:
>>>
>>> /TP/GAC/CL/BV-01-C: Verify that a Generic Attribute Profile client can
>>> generate an Exchange MTU Request command  to configure ATT_MTU over
>>> LE.
>>
>> This is still relevant and perhaps it should involve testing gatt-db
>> as well since the testing spec also involves the server, which leads
>> to the question if we should keep them apart? I guess this should be
>> done in a single place since we will be controlling the fd, or the
>> idea is that bt_att should access gatt_db internally to handle
>> incoming requests?
>
> This is a very good point. Currently the idea is to have a bt_gatt
> which defines GATT client-side functions for primary service
> discovery, read/write requests etc, which would then internally use
> bt_att for ATT protocol transactions, and gatt_db which solely serves
> as a medium for storage of local/remote attributes. Basically, a
> complete implementation of a GATT client/server would create and
> manage individual instances of bt_gatt/bt_att for all requests and
> gatt_db to explicitly store the results of those requests. In essence,
> the database implementation and bt_gatt/bt_att won't need to
> explicitly depend on each other.

I believe bt_gatt will actually need to depend on gatt_db at some
point because it makes it a lot more convenient to do the
qualification tests without having to implement the interaction with
gatt_db on the test itself and it is also more convenient in terms of
API since you don't have to create extra callbacks to interact with
the db.

> This is why it might make sense to hold off on writing PTS style tests
> until we have the complete picture in place. For now, I went with a
> simple set of unit tests to make sure that basic PDU encoding/decoding
> is done correctly for bt_att. As we then start putting gatt_db and
> bt_gatt together, I say we then write tests in the fashion of
> test-av*.

Well you risk having to do the same test more than once, in case of
ATT I don't think that is a problem since there doesn't seems to be
tests specific for it but in case of GATT I would stick with test spec
since the beginning.


-- 
Luiz Augusto von Dentz

Re: [PATCH 00/10] bt_att initial implementation

[Arman Uguray]

Hi Luiz,

>> That last bit of course is different from mgmt, where the parameters
>> structure definition itself encodes the data being sent over the
>> socket, whereas bt_att performs encoding/decoding logic. Either way, I
>> think this definition makes for a nice API.
>
> If you can really tell by opcode how to encode/decode the data that
> should be fine, regarding the API style I would favor functions vs use
> of opcode + struct, I understand in MGMT case we did this to define
> the protocol at PDU level so anyone could use those to implement a
> library thus why it is placed under lib/. Anyway Im not against either
> style since we can still change if we need.

Yes, the opcode tells you exactly what the final PDU will look like. I
had to make a decision between opcode + struct vs functions and I went
with the former as it felt more consistent. Though, as you said, we
can always change things up later if we feel like there's a better
way.

>> This is a very good point. Currently the idea is to have a bt_gatt
>> which defines GATT client-side functions for primary service
>> discovery, read/write requests etc, which would then internally use
>> bt_att for ATT protocol transactions, and gatt_db which solely serves
>> as a medium for storage of local/remote attributes. Basically, a
>> complete implementation of a GATT client/server would create and
>> manage individual instances of bt_gatt/bt_att for all requests and
>> gatt_db to explicitly store the results of those requests. In essence,
>> the database implementation and bt_gatt/bt_att won't need to
>> explicitly depend on each other.
>
> I believe bt_gatt will actually need to depend on gatt_db at some
> point because it makes it a lot more convenient to do the
> qualification tests without having to implement the interaction with
> gatt_db on the test itself and it is also more convenient in terms of
> API since you don't have to create extra callbacks to interact with
> the db.

That's true. It mostly depends on whether the code is implementing a
GATT client or server so we should make bt_att and gatt_db not depend
on each other, so as to make the code reusable for both client/server.
Since bt_gatt is going to end up being mostly client-only, I'm fine
with an instance of bt_gatt creating its own internal gatt_db.

>> This is why it might make sense to hold off on writing PTS style tests
>> until we have the complete picture in place. For now, I went with a
>> simple set of unit tests to make sure that basic PDU encoding/decoding
>> is done correctly for bt_att. As we then start putting gatt_db and
>> bt_gatt together, I say we then write tests in the fashion of
>> test-av*.
>
> Well you risk having to do the same test more than once, in case of
> ATT I don't think that is a problem since there doesn't seems to be
> tests specific for it but in case of GATT I would stick with test spec
> since the beginning.

Exactly what I'm thinking. For GATT we should plan tests based on the
test spec as we develop the code.

-Arman

Re: [PATCH 01/10] src/shared/att: Introduce struct bt_att.

[Johan Hedberg]

Hi Arman,

On Mon, May 05, 2014, Arman Uguray wrote:
> + *  Copyright (C) 2012-2014  Intel Corporation. All rights reserved.

It's ok to have this if you've copied code from elsewhere where there
was Intel copyright, however probably a lot of the code is by you in
which case I suppose you'll want to have Google's copyright here. Same
goes for the other files you're adding.

Johan

Re: [PATCH 01/10] src/shared/att: Introduce struct bt_att.

[Arman Uguray]

Hi Johan,

>> + *  Copyright (C) 2012-2014  Intel Corporation. All rights reserved.
>
> It's ok to have this if you've copied code from elsewhere where there
> was Intel copyright, however probably a lot of the code is by you in
> which case I suppose you'll want to have Google's copyright here. Same
> goes for the other files you're adding.

Thanks for pointing that out, I changed the copyright line to read:

      Copyright (C) 2014  Google Inc.

I will include this as part of v1.