* [PATCH v1 00/10] bt_att initial implementation
@ 2014-05-09 20:13 Arman Uguray
2014-05-09 20:13 ` [PATCH v1 01/10] src/shared/att: Introduce struct bt_att Arman Uguray
` (9 more replies)
0 siblings, 10 replies; 17+ messages in thread
From: Arman Uguray @ 2014-05-09 20:13 UTC (permalink / raw
To: linux-bluetooth; +Cc: Arman Uguray
v1:
- Updated the copyright notice in new files to include Google Inc. Retained
the Intel copyright in src/shared/att.c and unit/test-att.c which take a lot
of code from src/shared/mgmt.c and unit/test-mgmt.c respectively.
- Removed ATT_ERROR_IO from src/shared/att as 0x80 is a legitimate
application error that varies among GATT profiles. On I/O errors, the code
now invokes the callback with ATT_OP_ERROR_RESP with NULL parameters.
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 | 684 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
src/shared/att.h | 122 ++++++++++
tools/btatt.c | 627 ++++++++++++++++++++++++++++++++++++++++++++++++++
unit/test-att.c | 474 ++++++++++++++++++++++++++++++++++++++
6 files changed, 1927 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.8.3.2
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v1 01/10] src/shared/att: Introduce struct bt_att.
2014-05-09 20:13 [PATCH v1 00/10] bt_att initial implementation Arman Uguray
@ 2014-05-09 20:13 ` Arman Uguray
2014-05-13 16:43 ` Marcel Holtmann
2014-05-09 20:13 ` [PATCH v1 02/10] unit/test-att: Add unit tests for src/shared/att Arman Uguray
` (8 subsequent siblings)
9 siblings, 1 reply; 17+ messages in thread
From: Arman Uguray @ 2014-05-09 20:13 UTC (permalink / raw
To: linux-bluetooth; +Cc: 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 | 521 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
src/shared/att.h | 92 ++++++++++
3 files changed, 615 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..515d9ec
--- /dev/null
+++ b/src/shared/att.c
@@ -0,0 +1,521 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2014 Google Inc.
+ * Copyright (C) 2014 Intel Corporation.
+ *
+ *
+ * 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;
+ 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_OP_ERROR_RESP, 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, 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, pdu[0],
+ ¶m, 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],
+ ¶m, 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..34cb005
--- /dev/null
+++ b/src/shared/att.h
@@ -0,0 +1,92 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2014 Google Inc.
+ *
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#include <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
+
+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.8.3.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v1 02/10] unit/test-att: Add unit tests for src/shared/att.
2014-05-09 20:13 [PATCH v1 00/10] bt_att initial implementation Arman Uguray
2014-05-09 20:13 ` [PATCH v1 01/10] src/shared/att: Introduce struct bt_att Arman Uguray
@ 2014-05-09 20:13 ` Arman Uguray
2014-05-09 20:13 ` [PATCH v1 03/10] tools/btatt: Add command-line tool for ATT protocol testing Arman Uguray
` (7 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Arman Uguray @ 2014-05-09 20:13 UTC (permalink / raw
To: linux-bluetooth; +Cc: 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 | 308 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 317 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..2ea65f9
--- /dev/null
+++ b/unit/test-att.c
@@ -0,0 +1,308 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2014 Google Inc.
+ * Copyright (C) 2014 Intel Corporation.
+ *
+ *
+ * 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.8.3.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v1 03/10] tools/btatt: Add command-line tool for ATT protocol testing.
2014-05-09 20:13 [PATCH v1 00/10] bt_att initial implementation Arman Uguray
2014-05-09 20:13 ` [PATCH v1 01/10] src/shared/att: Introduce struct bt_att Arman Uguray
2014-05-09 20:13 ` [PATCH v1 02/10] unit/test-att: Add unit tests for src/shared/att Arman Uguray
@ 2014-05-09 20:13 ` Arman Uguray
2014-05-09 20:13 ` [PATCH v1 04/10] tools/btatt: Add "exchange-mtu" command Arman Uguray
` (6 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Arman Uguray @ 2014-05-09 20:13 UTC (permalink / raw
To: linux-bluetooth; +Cc: 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 | 152 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 161 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..476bb94
--- /dev/null
+++ b/tools/btatt.c
@@ -0,0 +1,152 @@
+/*
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2014 Google Inc.
+ *
+ *
+ * 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.8.3.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v1 04/10] tools/btatt: Add "exchange-mtu" command.
2014-05-09 20:13 [PATCH v1 00/10] bt_att initial implementation Arman Uguray
` (2 preceding siblings ...)
2014-05-09 20:13 ` [PATCH v1 03/10] tools/btatt: Add command-line tool for ATT protocol testing Arman Uguray
@ 2014-05-09 20:13 ` Arman Uguray
2014-05-09 20:13 ` [PATCH v1 05/10] src/shared/att: Add "Find Information" request and response Arman Uguray
` (5 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Arman Uguray @ 2014-05-09 20:13 UTC (permalink / raw
To: linux-bluetooth; +Cc: 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 476bb94..de8d300 100644
--- a/tools/btatt.c
+++ b/tools/btatt.c
@@ -31,6 +31,7 @@
#include <string.h>
#include <bluetooth/bluetooth.h>
+#include <bluetooth/l2cap.h>
#include "monitor/mainloop.h"
#include "src/shared/att.h"
@@ -39,11 +40,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(¶m, 0, sizeof(param));
+ param.client_rx_mtu = mtu;
+ if (bt_att_send(att, ATT_OP_EXCHANGE_MTU_REQ, ¶m, 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." },
{ }
};
@@ -80,6 +222,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;
@@ -88,6 +237,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) {
@@ -148,5 +299,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.8.3.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v1 05/10] src/shared/att: Add "Find Information" request and response.
2014-05-09 20:13 [PATCH v1 00/10] bt_att initial implementation Arman Uguray
` (3 preceding siblings ...)
2014-05-09 20:13 ` [PATCH v1 04/10] tools/btatt: Add "exchange-mtu" command Arman Uguray
@ 2014-05-09 20:13 ` Arman Uguray
2014-05-09 20:13 ` [PATCH v1 06/10] unit/test-att: Add unit test for "Find Information" request/response Arman Uguray
` (4 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Arman Uguray @ 2014-05-09 20:13 UTC (permalink / raw
To: linux-bluetooth; +Cc: 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 515d9ec..4e0d839 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -191,6 +191,8 @@ static bool handle_exchange_mtu_rsp(struct bt_att *att, const uint8_t *pdu,
if (pdu_length != 3)
return false;
+ memset(¶m, 0, sizeof(param));
+
param.server_rx_mtu = get_le16(pdu + 1);
request_complete(att, ATT_OP_EXCHANGE_MTU_REQ, pdu[0],
@@ -199,16 +201,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(¶m, 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],
+ ¶m, 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;
}
@@ -395,14 +448,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(struct bt_att_exchange_mtu_req_param))
+ if (length != sizeof(*cp))
return false;
if (len > mtu)
return false;
- put_le16(cp->client_rx_mtu, buf);
+ put_le16(cp->client_rx_mtu, bytes + 1);
+ *pdu_length = len;
+
+ return true;
+}
+
+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->start_handle, bytes + 1);
+ put_le16(cp->end_handle, bytes + 3);
+
*pdu_length = len;
return true;
@@ -429,10 +504,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 34cb005..569116a 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.8.3.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v1 06/10] unit/test-att: Add unit test for "Find Information" request/response.
2014-05-09 20:13 [PATCH v1 00/10] bt_att initial implementation Arman Uguray
` (4 preceding siblings ...)
2014-05-09 20:13 ` [PATCH v1 05/10] src/shared/att: Add "Find Information" request and response Arman Uguray
@ 2014-05-09 20:13 ` Arman Uguray
2014-05-09 20:13 ` [PATCH v1 07/10] tools/btatt: Add "find-information" command Arman Uguray
` (3 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Arman Uguray @ 2014-05-09 20:13 UTC (permalink / raw
To: linux-bluetooth; +Cc: 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 2ea65f9..9649db5 100644
--- a/unit/test-att.c
+++ b/unit/test-att.c
@@ -204,6 +204,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 {
@@ -259,6 +260,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)
{
@@ -268,7 +330,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);
}
@@ -303,6 +374,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.8.3.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v1 07/10] tools/btatt: Add "find-information" command.
2014-05-09 20:13 [PATCH v1 00/10] bt_att initial implementation Arman Uguray
` (5 preceding siblings ...)
2014-05-09 20:13 ` [PATCH v1 06/10] unit/test-att: Add unit test for "Find Information" request/response Arman Uguray
@ 2014-05-09 20:13 ` Arman Uguray
2014-05-09 20:13 ` [PATCH v1 08/10] src/shared/att: Add "Find By Type Value" request and response Arman Uguray
` (2 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Arman Uguray @ 2014-05-09 20:13 UTC (permalink / raw
To: linux-bluetooth; +Cc: 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 de8d300..a6aa281 100644
--- a/tools/btatt.c
+++ b/tools/btatt.c
@@ -34,6 +34,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
@@ -64,7 +66,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;
}
@@ -131,6 +133,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(¶m, 0, sizeof(param));
+ param.start_handle = start;
+ param.end_handle = end;
+
+ if (bt_att_send(att, ATT_OP_FIND_INFORMATION_REQ, ¶m,
+ 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;
@@ -185,7 +311,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.8.3.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v1 08/10] src/shared/att: Add "Find By Type Value" request and response.
2014-05-09 20:13 [PATCH v1 00/10] bt_att initial implementation Arman Uguray
` (6 preceding siblings ...)
2014-05-09 20:13 ` [PATCH v1 07/10] tools/btatt: Add "find-information" command Arman Uguray
@ 2014-05-09 20:13 ` Arman Uguray
2014-05-09 20:13 ` [PATCH v1 09/10] unit/test-att: Add unit test for "Find By Type Value" request/response Arman Uguray
2014-05-09 20:14 ` [PATCH v1 10/10] tools/btatt: Add "find-by-type-value" command Arman Uguray
9 siblings, 0 replies; 17+ messages in thread
From: Arman Uguray @ 2014-05-09 20:13 UTC (permalink / raw
To: linux-bluetooth; +Cc: 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 4e0d839..a97d1ef 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -207,6 +207,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;
@@ -232,6 +237,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(¶m, 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], ¶m,
+ sizeof(param));
+
+ free(handles_info_list);
+
+ return true;
+}
+
static bool handle_response(struct bt_att *att, const uint8_t *pdu,
uint16_t pdu_length)
{
@@ -256,6 +300,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;
}
@@ -483,6 +534,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)
{
@@ -511,6 +593,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 569116a..df11726 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.8.3.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v1 09/10] unit/test-att: Add unit test for "Find By Type Value" request/response.
2014-05-09 20:13 [PATCH v1 00/10] bt_att initial implementation Arman Uguray
` (7 preceding siblings ...)
2014-05-09 20:13 ` [PATCH v1 08/10] src/shared/att: Add "Find By Type Value" request and response Arman Uguray
@ 2014-05-09 20:13 ` Arman Uguray
2014-05-09 20:14 ` [PATCH v1 10/10] tools/btatt: Add "find-by-type-value" command Arman Uguray
9 siblings, 0 replies; 17+ messages in thread
From: Arman Uguray @ 2014-05-09 20:13 UTC (permalink / raw
To: linux-bluetooth; +Cc: 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 9649db5..1b2466f 100644
--- a/unit/test-att.c
+++ b/unit/test-att.c
@@ -202,7 +202,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);
};
@@ -235,11 +235,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 = {
@@ -273,14 +273,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[] = {
@@ -306,7 +306,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
};
@@ -321,6 +321,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)
{
@@ -376,6 +466,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.8.3.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v1 10/10] tools/btatt: Add "find-by-type-value" command.
2014-05-09 20:13 [PATCH v1 00/10] bt_att initial implementation Arman Uguray
` (8 preceding siblings ...)
2014-05-09 20:13 ` [PATCH v1 09/10] unit/test-att: Add unit test for "Find By Type Value" request/response Arman Uguray
@ 2014-05-09 20:14 ` Arman Uguray
9 siblings, 0 replies; 17+ messages in thread
From: Arman Uguray @ 2014-05-09 20:14 UTC (permalink / raw
To: linux-bluetooth; +Cc: 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 a6aa281..0867d9b 100644
--- a/tools/btatt.c
+++ b/tools/btatt.c
@@ -29,6 +29,7 @@
#include <stdio.h>
#include <getopt.h>
#include <string.h>
+#include <errno.h>
#include <bluetooth/bluetooth.h>
#include <bluetooth/l2cap.h>
@@ -51,8 +52,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,
@@ -257,6 +261,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(¶m, 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, ¶m,
+ 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;
@@ -313,6 +470,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.8.3.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v1 01/10] src/shared/att: Introduce struct bt_att.
2014-05-09 20:13 ` [PATCH v1 01/10] src/shared/att: Introduce struct bt_att Arman Uguray
@ 2014-05-13 16:43 ` Marcel Holtmann
2014-05-13 18:44 ` Arman Uguray
0 siblings, 1 reply; 17+ messages in thread
From: Marcel Holtmann @ 2014-05-13 16:43 UTC (permalink / raw
To: Arman Uguray; +Cc: linux-bluetooth
Hi Arman,
> 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 | 521 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> src/shared/att.h | 92 ++++++++++
> 3 files changed, 615 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..515d9ec
> --- /dev/null
> +++ b/src/shared/att.c
> @@ -0,0 +1,521 @@
> +/*
> + *
> + * BlueZ - Bluetooth protocol stack for Linux
> + *
> + * Copyright (C) 2014 Google Inc.
> + * Copyright (C) 2014 Intel Corporation.
> + *
> + *
> + * 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;
I think what you are looking for here is PTR_TO_UINT and UINT_TO_PRT magic. Please use that.
> +
> + 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_OP_ERROR_RESP, 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, 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, pdu[0],
> + ¶m, 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],
> + ¶m, sizeof(param));
> +
> + return true;
> +}
So I wonder if we want to do this internally in att.c or not. My current thinking is that we might just provide an bt_att_set_mtu function that allows changing of the MTU and then then the MTU exchange itself becomes a GATT problem. This means we keep ATT inside att.c pretty stupid to just being a transport.
I like this part of mgmt.c actually. It has no logic of its own besides queuing. Of course I realize that ATT is a bit silly with its one transaction at a time, but then still allow bi-directional transactions.
Here is what I had initially:
unsigned int att_send(struct att *att, uint8_t opcode,
const void *param, uint16_t length,
const uint8_t responses[],
att_callback_func_t callback,
void *user_data, att_destroy_func_t destroy);
The trick is the responses[] callback which would take an array of opcodes that are valid responses for a transaction that is currently in progress. Everything else would be treated as notifications. So all the logic is then in GATT as an user of ATT.
We did a similar style of handling with our AT command parser inside oFono ad that worked out pretty nicely. For commands without response, the array could be just empty. Might want to check if the error response should be included by default if the array is not empty.
> +
> +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..34cb005
> --- /dev/null
> +++ b/src/shared/att.h
> @@ -0,0 +1,92 @@
> +/*
> + *
> + * BlueZ - Bluetooth protocol stack for Linux
> + *
> + * Copyright (C) 2014 Google Inc.
> + *
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + *
> + */
> +
> +#include <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
Please prefix these with BT_ATT. Not sure if we better stick them into monitor/bt.h here.
> +
> +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);
Regards
Marcel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 01/10] src/shared/att: Introduce struct bt_att.
2014-05-13 16:43 ` Marcel Holtmann
@ 2014-05-13 18:44 ` Arman Uguray
2014-05-14 8:17 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 17+ messages in thread
From: Arman Uguray @ 2014-05-13 18:44 UTC (permalink / raw
To: Marcel Holtmann; +Cc: BlueZ development
Hi Marcel,
>> +static bool match_request_opcode(const void *a, const void *b)
>> +{
>> + const struct bt_att_request *request =3D a;
>> + const uint8_t *opcode =3D b;
>
> I think what you are looking for here is PTR_TO_UINT and UINT_TO_PRT magi=
c. Please use that.
Will do.
>> +static bool handle_exchange_mtu_rsp(struct bt_att *att, const uint8_t *=
pdu,
>> + uint16_t pdu_lengt=
h)
>> +{
>> + struct bt_att_exchange_mtu_rsp_param param;
>> +
>> + if (pdu_length !=3D 3)
>> + return false;
>> +
>> + param.server_rx_mtu =3D get_le16(pdu + 1);
>> +
>> + request_complete(att, ATT_OP_EXCHANGE_MTU_REQ, pdu[0],
>> + ¶m, sizeof(par=
am));
>> +
>> + return true;
>> +}
>
> So I wonder if we want to do this internally in att.c or not. My current =
thinking is that we might just provide an bt_att_set_mtu function that allo=
ws changing of the MTU and then then the MTU exchange itself becomes a GATT=
problem. This means we keep ATT inside att.c pretty stupid to just being a=
transport.
This is exactly the idea. This function above simply propagates the
received Server Rx MTU to the upper layer through the callback. The
upper layer is then expected to appropriately set the MTU using
bt_att_set_mtu, which is already introduced in this patch (I realize
now that it doesn't properly resize the buffer; I'll fix this in v2).
> I like this part of mgmt.c actually. It has no logic of its own besides q=
ueuing. Of course I realize that ATT is a bit silly with its one transactio=
n at a time, but then still allow bi-directional transactions.
>
> Here is what I had initially:
>
> unsigned int att_send(struct att *att, uint8_t opcode,
> const void *param, uint16_t length,
> const uint8_t responses[],
> att_callback_func_t callback,
> void *user_data, att_destroy_func_t destr=
oy);
>
> The trick is the responses[] callback which would take an array of opcode=
s that are valid responses for a transaction that is currently in progress.=
Everything else would be treated as notifications. So all the logic is the=
n in GATT as an user of ATT.
>
> We did a similar style of handling with our AT command parser inside oFon=
o ad that worked out pretty nicely. For commands without response, the arra=
y could be just empty. Might want to check if the error response should be =
included by default if the array is not empty.
Actually what I have in mind is even simpler than this. For outgoing
operations that don't solicit a response (such as commands and
outgoing notifications) we simply send them and invoke the callback
with a special opcode such as "BT_ATT_PDU_SENT". For those outgoing
operations that do solicit a response (e.g. requests and indications),
the request remains pending until the response is received. A
responses array is not really necessary, since there is already a 1:1
mapping between ATT requests and their responses, so we know exactly
which request the received response matches (even the error response
contains the request that caused it in its PDU).
For incoming requests, we treat these as events as you said. To
respond to them, the upper layer than just needs to call bt_att_send
with the corresponding response opcode and parameters.
>> +/* 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
>
> Please prefix these with BT_ATT. Not sure if we better stick them into mo=
nitor/bt.h here.
Will do. I say we keep them here for now. On that note: is there a
pattern to which structures get the bt_ prefix and which don't? e.g.
would struct att be a better name then struct bt_att here?
Cheers,
Arman
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 01/10] src/shared/att: Introduce struct bt_att.
2014-05-13 18:44 ` Arman Uguray
@ 2014-05-14 8:17 ` Luiz Augusto von Dentz
2014-05-14 14:54 ` Arman Uguray
0 siblings, 1 reply; 17+ messages in thread
From: Luiz Augusto von Dentz @ 2014-05-14 8:17 UTC (permalink / raw
To: Arman Uguray; +Cc: Marcel Holtmann, BlueZ development
Hi Arman,
On Tue, May 13, 2014 at 9:44 PM, Arman Uguray <armansito@chromium.org> wrot=
e:
> Hi Marcel,
>
>>> +static bool match_request_opcode(const void *a, const void *b)
>>> +{
>>> + const struct bt_att_request *request =3D a;
>>> + const uint8_t *opcode =3D b;
>>
>> I think what you are looking for here is PTR_TO_UINT and UINT_TO_PRT mag=
ic. Please use that.
>
> Will do.
>
>>> +static bool handle_exchange_mtu_rsp(struct bt_att *att, const uint8_t =
*pdu,
>>> + uint16_t pdu_leng=
th)
>>> +{
>>> + struct bt_att_exchange_mtu_rsp_param param;
>>> +
>>> + if (pdu_length !=3D 3)
>>> + return false;
>>> +
>>> + param.server_rx_mtu =3D get_le16(pdu + 1);
>>> +
>>> + request_complete(att, ATT_OP_EXCHANGE_MTU_REQ, pdu[0],
>>> + ¶m, sizeof(pa=
ram));
>>> +
>>> + return true;
>>> +}
>>
>> So I wonder if we want to do this internally in att.c or not. My current=
thinking is that we might just provide an bt_att_set_mtu function that all=
ows changing of the MTU and then then the MTU exchange itself becomes a GAT=
T problem. This means we keep ATT inside att.c pretty stupid to just being =
a transport.
>
> This is exactly the idea. This function above simply propagates the
> received Server Rx MTU to the upper layer through the callback. The
> upper layer is then expected to appropriately set the MTU using
> bt_att_set_mtu, which is already introduced in this patch (I realize
> now that it doesn't properly resize the buffer; I'll fix this in v2).
>
>> I like this part of mgmt.c actually. It has no logic of its own besides =
queuing. Of course I realize that ATT is a bit silly with its one transacti=
on at a time, but then still allow bi-directional transactions.
>>
>> Here is what I had initially:
>>
>> unsigned int att_send(struct att *att, uint8_t opcode,
>> const void *param, uint16_t length,
>> const uint8_t responses[],
>> att_callback_func_t callback,
>> void *user_data, att_destroy_func_t dest=
roy);
>>
>> The trick is the responses[] callback which would take an array of opcod=
es that are valid responses for a transaction that is currently in progress=
. Everything else would be treated as notifications. So all the logic is th=
en in GATT as an user of ATT.
>>
>> We did a similar style of handling with our AT command parser inside oFo=
no ad that worked out pretty nicely. For commands without response, the arr=
ay could be just empty. Might want to check if the error response should be=
included by default if the array is not empty.
>
> Actually what I have in mind is even simpler than this. For outgoing
> operations that don't solicit a response (such as commands and
> outgoing notifications) we simply send them and invoke the callback
> with a special opcode such as "BT_ATT_PDU_SENT". For those outgoing
> operations that do solicit a response (e.g. requests and indications),
> the request remains pending until the response is received. A
> responses array is not really necessary, since there is already a 1:1
> mapping between ATT requests and their responses, so we know exactly
> which request the received response matches (even the error response
> contains the request that caused it in its PDU).
I don't think we need an special opcode when the PDU is sent, the
caller might actually not set any callback and I don't think there is
any special action that can be triggered by having such a thing,
specially because it is in no way an acknowledgment it just mean it
has been dispatched it may still not reach the remote side. In other
words if ATT does not have acknowledgment we should not invent one
either.
> For incoming requests, we treat these as events as you said. To
> respond to them, the upper layer than just needs to call bt_att_send
> with the corresponding response opcode and parameters.
I would not reuse the same function for both requests and responses,
responses should not have any callback and custom data attached for
the same reasons stated above, so I would have att_send_rsp for
responses alone.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 01/10] src/shared/att: Introduce struct bt_att.
2014-05-14 8:17 ` Luiz Augusto von Dentz
@ 2014-05-14 14:54 ` Arman Uguray
2014-05-15 13:07 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 17+ messages in thread
From: Arman Uguray @ 2014-05-14 14:54 UTC (permalink / raw
To: Luiz Augusto von Dentz; +Cc: Marcel Holtmann, BlueZ development
Hi Luiz,
>> Actually what I have in mind is even simpler than this. For outgoing
>> operations that don't solicit a response (such as commands and
>> outgoing notifications) we simply send them and invoke the callback
>> with a special opcode such as "BT_ATT_PDU_SENT". For those outgoing
>> operations that do solicit a response (e.g. requests and indications),
>> the request remains pending until the response is received. A
>> responses array is not really necessary, since there is already a 1:1
>> mapping between ATT requests and their responses, so we know exactly
>> which request the received response matches (even the error response
>> contains the request that caused it in its PDU).
>
> I don't think we need an special opcode when the PDU is sent, the
> caller might actually not set any callback and I don't think there is
> any special action that can be triggered by having such a thing,
> specially because it is in no way an acknowledgment it just mean it
> has been dispatched it may still not reach the remote side. In other
> words if ATT does not have acknowledgment we should not invent one
> either.
Makes sense. The idea I had was that if the caller passes NULL to
bt_att_send for the callback, then no acknowledgement would happen but
if they do, we would confirm with a special PDU, but this will no
longer be necessary if we have a separate function as you suggest
below.
>> For incoming requests, we treat these as events as you said. To
>> respond to them, the upper layer than just needs to call bt_att_send
>> with the corresponding response opcode and parameters.
>
> I would not reuse the same function for both requests and responses,
> responses should not have any callback and custom data attached for
> the same reasons stated above, so I would have att_send_rsp for
> responses alone.
Maybe it would make sense to have one function to send operations that
solicit a response (requests/indications), e.g.
bt_att_send_op_with_rsp and one function for those that don't (for
responses/commands/notifications) like bt_att_send_op_no_rsp. Or do
you think we should have separate functions such as bt_att_send_req,
bt_att_send_rsp, bt_att_send_not, bt_att_send_ind, etc?
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 01/10] src/shared/att: Introduce struct bt_att.
2014-05-14 14:54 ` Arman Uguray
@ 2014-05-15 13:07 ` Luiz Augusto von Dentz
2014-05-15 17:22 ` Arman Uguray
0 siblings, 1 reply; 17+ messages in thread
From: Luiz Augusto von Dentz @ 2014-05-15 13:07 UTC (permalink / raw
To: Arman Uguray; +Cc: Marcel Holtmann, BlueZ development
Hi Arman,
On Wed, May 14, 2014 at 5:54 PM, Arman Uguray <armansito@chromium.org> wrote:
> Hi Luiz,
>
>>> Actually what I have in mind is even simpler than this. For outgoing
>>> operations that don't solicit a response (such as commands and
>>> outgoing notifications) we simply send them and invoke the callback
>>> with a special opcode such as "BT_ATT_PDU_SENT". For those outgoing
>>> operations that do solicit a response (e.g. requests and indications),
>>> the request remains pending until the response is received. A
>>> responses array is not really necessary, since there is already a 1:1
>>> mapping between ATT requests and their responses, so we know exactly
>>> which request the received response matches (even the error response
>>> contains the request that caused it in its PDU).
>>
>> I don't think we need an special opcode when the PDU is sent, the
>> caller might actually not set any callback and I don't think there is
>> any special action that can be triggered by having such a thing,
>> specially because it is in no way an acknowledgment it just mean it
>> has been dispatched it may still not reach the remote side. In other
>> words if ATT does not have acknowledgment we should not invent one
>> either.
>
> Makes sense. The idea I had was that if the caller passes NULL to
> bt_att_send for the callback, then no acknowledgement would happen but
> if they do, we would confirm with a special PDU, but this will no
> longer be necessary if we have a separate function as you suggest
> below.
>
>>> For incoming requests, we treat these as events as you said. To
>>> respond to them, the upper layer than just needs to call bt_att_send
>>> with the corresponding response opcode and parameters.
>>
>> I would not reuse the same function for both requests and responses,
>> responses should not have any callback and custom data attached for
>> the same reasons stated above, so I would have att_send_rsp for
>> responses alone.
>
> Maybe it would make sense to have one function to send operations that
> solicit a response (requests/indications), e.g.
> bt_att_send_op_with_rsp and one function for those that don't (for
> responses/commands/notifications) like bt_att_send_op_no_rsp. Or do
> you think we should have separate functions such as bt_att_send_req,
> bt_att_send_rsp, bt_att_send_not, bt_att_send_ind, etc?
Well personally I would have dedicated function for each opcode, that
way there is very little room for confusion i.e using
bt_att_send_op_with_rsp for an opcode that does not solicit a response
and vice-versa, internally you can still have them using common
functions.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 01/10] src/shared/att: Introduce struct bt_att.
2014-05-15 13:07 ` Luiz Augusto von Dentz
@ 2014-05-15 17:22 ` Arman Uguray
0 siblings, 0 replies; 17+ messages in thread
From: Arman Uguray @ 2014-05-15 17:22 UTC (permalink / raw
To: Luiz Augusto von Dentz; +Cc: Marcel Holtmann, BlueZ development
Hi Luiz,
> Well personally I would have dedicated function for each opcode, that
> way there is very little room for confusion i.e using
> bt_att_send_op_with_rsp for an opcode that does not solicit a response
> and vice-versa, internally you can still have them using common
> functions.
While I see your point, I think I'm not going to go with individual
functions for each opcode at the moment. Especially because, for
incoming PDUs, we need a generic way to propagate the PDU to the upper
layer anyway, and the current opcode + param structure based signature
allows upper layers to set a single callback with a unified signature,
instead of setting a separate callback with a different function
signature for each incoming opcode. I believe the former is cleaner in
general and the current way of having a generic bt_att_send makes
things consistent between incoming and outgoing PDUs. The alternative
would be to have a dedicated function to send outgoing operations and
then a generic callback (opcode + param) for incoming ones, but I
don't prefer this.
We could remove the ambiguity that you mention by having the functions
fail and return false in those cases and by explicitly documenting how
they should be used. We'll document that bt_att_send_op_with_rsp will
return false for all opcodes that don't have control flow and that
multiple operations posted through this function will be queued until
a response for each one has been received. We'll document that
bt_att_send_op[_no_rsp] will send the PDU as soon as the buffer is
ready.
-Arman
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2014-05-15 17:22 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-09 20:13 [PATCH v1 00/10] bt_att initial implementation Arman Uguray
2014-05-09 20:13 ` [PATCH v1 01/10] src/shared/att: Introduce struct bt_att Arman Uguray
2014-05-13 16:43 ` Marcel Holtmann
2014-05-13 18:44 ` Arman Uguray
2014-05-14 8:17 ` Luiz Augusto von Dentz
2014-05-14 14:54 ` Arman Uguray
2014-05-15 13:07 ` Luiz Augusto von Dentz
2014-05-15 17:22 ` Arman Uguray
2014-05-09 20:13 ` [PATCH v1 02/10] unit/test-att: Add unit tests for src/shared/att Arman Uguray
2014-05-09 20:13 ` [PATCH v1 03/10] tools/btatt: Add command-line tool for ATT protocol testing Arman Uguray
2014-05-09 20:13 ` [PATCH v1 04/10] tools/btatt: Add "exchange-mtu" command Arman Uguray
2014-05-09 20:13 ` [PATCH v1 05/10] src/shared/att: Add "Find Information" request and response Arman Uguray
2014-05-09 20:13 ` [PATCH v1 06/10] unit/test-att: Add unit test for "Find Information" request/response Arman Uguray
2014-05-09 20:13 ` [PATCH v1 07/10] tools/btatt: Add "find-information" command Arman Uguray
2014-05-09 20:13 ` [PATCH v1 08/10] src/shared/att: Add "Find By Type Value" request and response Arman Uguray
2014-05-09 20:13 ` [PATCH v1 09/10] unit/test-att: Add unit test for "Find By Type Value" request/response Arman Uguray
2014-05-09 20:14 ` [PATCH v1 10/10] tools/btatt: Add "find-by-type-value" command Arman Uguray
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.