All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/11] Add shared/gatt-helpers
@ 2014-07-25 22:08 Arman Uguray
  2014-07-25 22:08 ` [PATCH v2 01/11] shared/gatt: Introduce gatt-helpers.h skeleton Arman Uguray
                   ` (10 more replies)
  0 siblings, 11 replies; 20+ messages in thread
From: Arman Uguray @ 2014-07-25 22:08 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Arman Uguray

* v2:
   - Addressed various style comments.
   - Changed status reporting from a single uint16 that encodes bluez defined
     errors and ATT protocol errors to a bool that indicates success/failure and
     a uint8 that contains ATT protocol errors. Removed all BT_GATT_ERROR_*.
   - Stopped returning discovery results in a struct queue. Instead, the code
     now uses a GATT-specific data structure as per Marcel's request. This
     structure is a simple singly linked list that doesn't allow external
     modification.

     I'm not quite sure this is the right way to do this; I didn't want to use
     any glib structures here so maybe in the future we can change this to
     simply use some library defined linked list structure that is shared among
     userspace code.

* v1:
   - Fixed 32-bit UUID encoding for the "Discover Primary Service by UUID"
     procedure.
   - Minor cosmetic fix in bt_gatt_exchange_mtu.

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

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

-- 
2.0.0.526.g5318336


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2 01/11] shared/gatt: Introduce gatt-helpers.h skeleton.
  2014-07-25 22:08 [PATCH v2 00/11] Add shared/gatt-helpers Arman Uguray
@ 2014-07-25 22:08 ` Arman Uguray
  2014-07-25 22:47   ` Marcel Holtmann
  2014-07-25 22:08 ` [PATCH v2 02/11] shared/gatt: Implement bt_gatt_exchange_mtu Arman Uguray
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: Arman Uguray @ 2014-07-25 22:08 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Arman Uguray

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

diff --git a/Makefile.am b/Makefile.am
index c73e23a..88fcb49 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -157,7 +157,8 @@ src_bluetoothd_SOURCES = $(builtin_sources) \
 			src/shared/queue.h src/shared/queue.c \
 			src/shared/util.h src/shared/util.c \
 			src/shared/mgmt.h src/shared/mgmt.c \
-			src/shared/att-types.h src/shared/att.h src/shared/att.c
+			src/shared/att-types.h src/shared/att.h src/shared/att.c \
+			src/shared/gatt-helpers.h src/shared/gatt-helpers.c
 src_bluetoothd_LDADD = lib/libbluetooth-internal.la gdbus/libgdbus-internal.la \
 			@GLIB_LIBS@ @DBUS_LIBS@ -ldl -lrt
 src_bluetoothd_LDFLAGS = $(AM_LDFLAGS) -Wl,--export-dynamic \
diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
new file mode 100644
index 0000000..a119de6
--- /dev/null
+++ b/src/shared/gatt-helpers.c
@@ -0,0 +1,141 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2014  Google Inc.
+ *
+ *
+ *  This library is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU Lesser General Public
+ *  License as published by the Free Software Foundation; either
+ *  version 2.1 of the License, or (at your option) any later version.
+ *
+ *  This library is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public
+ *  License along with this library; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include "src/shared/queue.h"
+#include "src/shared/att.h"
+#include "lib/uuid.h"
+#include "src/shared/gatt-helpers.h"
+
+bool bt_gatt_exchange_mtu(struct bt_att *att, uint16_t client_rx_mtu,
+					bt_gatt_result_callback_t callback,
+					void *user_data,
+					bt_gatt_destroy_func_t destroy)
+{
+	/* TODO */
+	return false;
+}
+
+bool bt_gatt_discover_primary_services(struct bt_att *att,
+					bt_uuid_t *uuid,
+					bt_gatt_discovery_callback_t callback,
+					void *user_data,
+					bt_gatt_destroy_func_t destroy)
+{
+	/* TODO */
+	return false;
+}
+
+bool bt_gatt_discover_included_services(struct bt_att *att,
+					uint16_t start, uint16_t end,
+					bt_uuid_t *uuid,
+					bt_gatt_discovery_callback_t callback,
+					void *user_data,
+					bt_gatt_destroy_func_t destroy)
+{
+	/* TODO */
+	return false;
+}
+
+bool bt_gatt_discover_characteristics(struct bt_att *att,
+					uint16_t start, uint16_t end,
+					bt_uuid_t *uuid,
+					bt_gatt_discovery_callback_t callback,
+					void *user_data,
+					bt_gatt_destroy_func_t destroy)
+{
+	/* TODO */
+	return false;
+}
+
+bool bt_gatt_discover_descriptors(struct bt_att *att,
+					uint16_t start, uint16_t end,
+					bt_gatt_discovery_callback_t callback,
+					void *user_data,
+					bt_gatt_destroy_func_t destroy)
+{
+	/* TODO */
+	return false;
+}
+
+bool bt_gatt_read_value(struct bt_att *att, uint16_t value_handle,
+					bt_gatt_read_callback_t callback,
+					void *user_data,
+					bt_gatt_destroy_func_t destroy)
+{
+	/* TODO */
+	return false;
+}
+
+bool bt_gatt_read_long_value(struct bt_att *att,
+					uint16_t value_handle, uint16_t offset,
+					bt_gatt_read_callback_t callback,
+					void *user_data,
+					bt_gatt_destroy_func_t destroy)
+{
+	/* TODO */
+	return false;
+}
+
+bool bt_gatt_write_without_response(struct bt_att *att,
+					uint16_t value_handle,
+                                        bool signed_write,
+					uint8_t *value, uint16_t length)
+{
+	/* TODO */
+	return false;
+}
+
+bool bt_gatt_write_value(struct bt_att *att, uint16_t value_handle,
+					uint8_t *value, uint16_t length,
+					bt_gatt_result_callback_t callback,
+					void *user_data,
+					bt_gatt_destroy_func_t destroy)
+{
+	/* TODO */
+	return false;
+}
+
+bool bt_gatt_write_long_value(struct bt_att *att, bool reliable,
+					uint16_t value_handle, uint16_t offset,
+					uint8_t *value, uint16_t length,
+					bt_gatt_write_long_callback_t callback,
+					void *user_data,
+					bt_gatt_destroy_func_t destroy)
+{
+	/* TODO */
+	return false;
+}
+
+unsigned int bt_gatt_register(struct bt_att *att, bool indications,
+					bt_gatt_notify_callback_t callback,
+					void *user_data,
+					bt_gatt_destroy_func_t destroy)
+{
+	/* TODO */
+	return false;
+}
diff --git a/src/shared/gatt-helpers.h b/src/shared/gatt-helpers.h
new file mode 100644
index 0000000..d3a95ae
--- /dev/null
+++ b/src/shared/gatt-helpers.h
@@ -0,0 +1,127 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2014  Google Inc.
+ *
+ *
+ *  This library is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU Lesser General Public
+ *  License as published by the Free Software Foundation; either
+ *  version 2.1 of the License, or (at your option) any later version.
+ *
+ *  This library is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public
+ *  License along with this library; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+/* This file defines helpers for performing client-side procedures defined by
+ * the Generic Attribute Profile.
+ */
+
+#include <stdbool.h>
+#include <stdint.h>
+
+struct bt_gatt_service {
+	uint16_t start;
+	uint16_t end;
+	uint8_t uuid[16];
+};
+
+struct bt_gatt_characteristic {
+	uint16_t start;
+	uint16_t end;
+	uint16_t value;
+	uint8_t properties;
+	uint8_t uuid[16];
+};
+
+struct bt_gatt_descriptor {
+	uint16_t handle;
+	uint8_t uuid[16];
+};
+
+struct bt_gatt_list;
+
+struct bt_gatt_list *bt_gatt_list_get_next(struct bt_gatt_list *l);
+void *bt_gatt_list_get_data(struct bt_gatt_list *l);
+
+typedef void (*bt_gatt_destroy_func_t)(void *user_data);
+
+typedef void (*bt_gatt_result_callback_t)(bool success, uint8_t att_ecode,
+							void *user_data);
+typedef void (*bt_gatt_discovery_callback_t)(bool success, uint8_t att_ecode,
+						struct bt_gatt_list *results,
+						void *user_data);
+typedef void (*bt_gatt_read_callback_t)(bool success, uint8_t att_ecode,
+					const uint8_t *value, uint16_t length,
+					void *user_data);
+typedef void (*bt_gatt_write_long_callback_t)(bool success, bool reliable_error,
+					uint8_t att_ecode, void *user_data);
+
+typedef void (*bt_gatt_notify_callback_t)(uint16_t value_handle,
+					const uint8_t *value, uint16_t length,
+					void *user_data);
+
+bool bt_gatt_exchange_mtu(struct bt_att *att, uint16_t client_rx_mtu,
+					bt_gatt_result_callback_t callback,
+					void *user_data,
+					bt_gatt_destroy_func_t destroy);
+
+bool bt_gatt_discover_primary_services(struct bt_att *att, bt_uuid_t *uuid,
+					bt_gatt_discovery_callback_t callback,
+					void *user_data,
+					bt_gatt_destroy_func_t destroy);
+bool bt_gatt_discover_included_services(struct bt_att *att,
+					uint16_t start, uint16_t end,
+					bt_uuid_t *uuid,
+					bt_gatt_discovery_callback_t callback,
+					void *user_data,
+					bt_gatt_destroy_func_t destroy);
+bool bt_gatt_discover_characteristics(struct bt_att *att,
+					uint16_t start, uint16_t end,
+					bt_uuid_t *uuid,
+					bt_gatt_discovery_callback_t callback,
+					void *user_data,
+					bt_gatt_destroy_func_t destroy);
+bool bt_gatt_discover_descriptors(struct bt_att *att,
+					uint16_t start, uint16_t end,
+					bt_gatt_discovery_callback_t callback,
+					void *user_data,
+					bt_gatt_destroy_func_t destroy);
+
+bool bt_gatt_read_value(struct bt_att *att, uint16_t value_handle,
+					bt_gatt_read_callback_t callback,
+					void *user_data,
+					bt_gatt_destroy_func_t destroy);
+bool bt_gatt_read_long_value(struct bt_att *att,
+					uint16_t value_handle, uint16_t offset,
+					bt_gatt_read_callback_t callback,
+					void *user_data,
+					bt_gatt_destroy_func_t destroy);
+
+bool bt_gatt_write_without_response(struct bt_att *att, uint16_t value_handle,
+					bool signed_write,
+					uint8_t *value, uint16_t length);
+bool bt_gatt_write_value(struct bt_att *att, uint16_t value_handle,
+					uint8_t *value, uint16_t length,
+					bt_gatt_result_callback_t callback,
+					void *user_data,
+					bt_gatt_destroy_func_t destroy);
+bool bt_gatt_write_long_value(struct bt_att *att, bool reliable,
+					uint16_t value_handle, uint16_t offset,
+					uint8_t *value, uint16_t length,
+					bt_gatt_write_long_callback_t callback,
+					void *user_data,
+					bt_gatt_destroy_func_t destroy);
+
+unsigned int bt_gatt_register(struct bt_att *att, bool indications,
+					bt_gatt_notify_callback_t callback,
+					void *user_data,
+					bt_gatt_destroy_func_t destroy);
-- 
2.0.0.526.g5318336


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 02/11] shared/gatt: Implement bt_gatt_exchange_mtu.
  2014-07-25 22:08 [PATCH v2 00/11] Add shared/gatt-helpers Arman Uguray
  2014-07-25 22:08 ` [PATCH v2 01/11] shared/gatt: Introduce gatt-helpers.h skeleton Arman Uguray
@ 2014-07-25 22:08 ` Arman Uguray
  2014-07-25 22:08 ` [PATCH v2 03/11] shared/gatt: Implement "Discover All Primary Services" procedure Arman Uguray
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Arman Uguray @ 2014-07-25 22:08 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Arman Uguray

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

diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index a119de6..96137bf 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -30,14 +30,96 @@
 #include "src/shared/att.h"
 #include "lib/uuid.h"
 #include "src/shared/gatt-helpers.h"
+#include "src/shared/util.h"
+
+#ifndef MIN
+#define MIN(a, b) ((a) < (b) ? (a) : (b))
+#endif
+
+struct mtu_op {
+	struct bt_att *att;
+	uint16_t client_rx_mtu;
+	bt_gatt_result_callback_t callback;
+	void *user_data;
+	bt_gatt_destroy_func_t destroy;
+};
+
+static void destroy_mtu_op(void *user_data)
+{
+	struct mtu_op *op = user_data;
+
+	if (op->destroy)
+		op->destroy(op->user_data);
+
+	free(op);
+}
+
+static uint8_t process_error(const void *pdu, uint16_t length)
+{
+	if (!pdu || length != 4)
+		return 0;
+
+	return ((uint8_t *) pdu)[3];
+}
+
+static void mtu_cb(uint8_t opcode, const void *pdu, uint16_t length,
+								void *user_data)
+{
+	struct mtu_op *op = user_data;
+	bool success = true;
+	uint8_t att_ecode = 0;
+	uint16_t server_rx_mtu;
+
+	if (opcode == BT_ATT_OP_ERROR_RSP) {
+		success = false;
+		att_ecode = process_error(pdu, length);
+		goto done;
+	}
+
+	if (opcode != BT_ATT_OP_MTU_RSP || !pdu || length != 2) {
+		success = false;
+		goto done;
+	}
+
+	server_rx_mtu = get_le16(pdu);
+	bt_att_set_mtu(op->att, MIN(op->client_rx_mtu, server_rx_mtu));
+
+done:
+	if (op->callback)
+		op->callback(success, att_ecode, op->user_data);
+}
 
 bool bt_gatt_exchange_mtu(struct bt_att *att, uint16_t client_rx_mtu,
 					bt_gatt_result_callback_t callback,
 					void *user_data,
 					bt_gatt_destroy_func_t destroy)
 {
-	/* TODO */
-	return false;
+	struct mtu_op *op;
+	uint8_t pdu[2];
+
+	if (!att || !client_rx_mtu)
+		return false;
+
+	op = new0(struct mtu_op, 1);
+	if (!op)
+		return false;
+
+	op->att = att;
+	op->client_rx_mtu = client_rx_mtu;
+	op->callback = callback;
+	op->user_data = user_data;
+	op->destroy = destroy;
+
+	put_le16(client_rx_mtu, pdu);
+
+	if (!bt_att_send(att, BT_ATT_OP_MTU_REQ, pdu, sizeof(pdu),
+							mtu_cb, op,
+							destroy_mtu_op)) {
+		free(op);
+		return false;
+	}
+
+	return true;
 }
 
 bool bt_gatt_discover_primary_services(struct bt_att *att,
-- 
2.0.0.526.g5318336


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 03/11] shared/gatt: Implement "Discover All Primary Services" procedure.
  2014-07-25 22:08 [PATCH v2 00/11] Add shared/gatt-helpers Arman Uguray
  2014-07-25 22:08 ` [PATCH v2 01/11] shared/gatt: Introduce gatt-helpers.h skeleton Arman Uguray
  2014-07-25 22:08 ` [PATCH v2 02/11] shared/gatt: Implement bt_gatt_exchange_mtu Arman Uguray
@ 2014-07-25 22:08 ` Arman Uguray
  2014-07-26 11:02   ` Marcel Holtmann
  2014-07-25 22:08 ` [PATCH v2 04/11] shared/gatt: Implement "Discover Primary Service by UUID" procedure Arman Uguray
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: Arman Uguray @ 2014-07-25 22:08 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Arman Uguray

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

diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index 96137bf..d427eaf 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -36,6 +36,70 @@
 #define MIN(a, b) ((a) < (b) ? (a) : (b))
 #endif
 
+struct bt_gatt_list {
+	struct bt_gatt_list *next;
+	void *data;
+};
+
+struct list_ptrs {
+	struct bt_gatt_list *head;
+	struct bt_gatt_list *tail;
+};
+
+struct bt_gatt_list *bt_gatt_list_get_next(struct bt_gatt_list *list)
+{
+	return list->next;
+}
+
+void *bt_gatt_list_get_data(struct bt_gatt_list *list)
+{
+	return list->data;
+}
+
+static inline bool list_isempty(struct list_ptrs *list)
+{
+	return !list->head && !list->tail;
+}
+
+static bool list_add(struct list_ptrs *list, void *data)
+{
+	struct bt_gatt_list *item = new0(struct bt_gatt_list, 1);
+	if (!item)
+		return false;
+
+	item->data = data;
+
+	if (list_isempty(list)) {
+		list->head = list->tail = item;
+		return true;
+	}
+
+	list->tail->next = item;
+	list->tail = item;
+
+	return true;
+}
+
+static void list_free(struct list_ptrs *list, bt_gatt_destroy_func_t destroy)
+{
+	struct bt_gatt_list *l, *tmp;
+	l = list->head;
+
+	while (l) {
+		if (destroy)
+			destroy(l->data);
+
+		tmp = l->next;
+		free(l);
+		l = tmp;
+	}
+}
+
+static const uint8_t bt_base_uuid[16] = {
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00,
+	0x80, 0x00, 0x00, 0x80, 0x5F, 0x9B, 0x34, 0xFB
+};
+
 struct mtu_op {
 	struct bt_att *att;
 	uint16_t client_rx_mtu;
@@ -122,14 +186,199 @@ bool bt_gatt_exchange_mtu(struct bt_att *att, uint16_t client_rx_mtu,
 	return true;
 }
 
+struct discovery_op {
+	struct bt_att *att;
+	int ref_count;
+	bt_uuid_t uuid;
+	struct list_ptrs results;
+	bt_gatt_discovery_callback_t callback;
+	void *user_data;
+	bt_gatt_destroy_func_t destroy;
+};
+
+static struct discovery_op* discovery_op_ref(struct discovery_op *op)
+{
+	__sync_fetch_and_add(&op->ref_count, 1);
+
+	return op;
+}
+
+static void discovery_op_unref(void *data)
+{
+	struct discovery_op *op = data;
+
+	if (__sync_sub_and_fetch(&op->ref_count, 1))
+		return;
+
+	if (op->destroy)
+		op->destroy(op->user_data);
+
+	list_free(&op->results, free);
+
+	free(op);
+}
+
+static void put_uuid_le(const bt_uuid_t *src, void *dst)
+{
+	if (src->type == BT_UUID16)
+		put_le16(src->value.u16, dst);
+	else
+		bswap_128(&src->value.u128, dst);
+}
+
+static bool convert_uuid_le(const uint8_t *src, size_t len, uint8_t dst[16])
+{
+	if (len == 16) {
+		bswap_128(src, dst);
+		return true;
+	}
+
+	if (len != 2)
+		return false;
+
+	memcpy(dst, bt_base_uuid, sizeof(bt_base_uuid));
+	dst[2] = src[1];
+	dst[3] = src[0];
+
+	return true;
+}
+
+static void read_by_grp_type_cb(uint8_t opcode, const void *pdu,
+					uint16_t length, void *user_data)
+{
+	struct discovery_op *op = user_data;
+	bool success;
+	uint8_t att_ecode = 0;
+	struct bt_gatt_list *results = NULL;
+	size_t data_length;
+	size_t list_length;
+	uint16_t last_end;
+	int i;
+
+	if (opcode == BT_ATT_OP_ERROR_RSP) {
+		success = false;
+		att_ecode = process_error(pdu, length);
+
+		if (att_ecode == BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND &&
+						!list_isempty(&op->results))
+			goto success;
+
+		goto done;
+	}
+
+	/* PDU must contain at least the following (sans opcode):
+	 * - Attr Data Length (1 octet)
+	 * - Attr Data List (at least 6 octets):
+	 *   -- 2 octets: Attribute handle
+	 *   -- 2 octets: End group handle
+	 *   -- 2 or 16 octets: service UUID
+	 */
+	if (opcode != BT_ATT_OP_READ_BY_GRP_TYPE_RSP || !pdu || length < 7) {
+		success = false;
+		goto done;
+	}
+
+	data_length = ((uint8_t *) pdu)[0];
+	list_length = length - 1;
+
+	if ((list_length % data_length) ||
+				(data_length != 6 && data_length != 20)) {
+		success = false;
+		goto done;
+	}
+
+	for (i = 1; i < length; i += data_length) {
+		struct bt_gatt_service *service;
+
+		service = new0(struct bt_gatt_service, 1);
+		if (!service) {
+			success = false;
+			goto done;
+		}
+
+		service->start = get_le16(pdu + i);
+		last_end = get_le16(pdu + i + 2);
+		service->end = last_end;
+		convert_uuid_le(pdu + i + 4, data_length - 4, service->uuid);
+
+		if (!list_add(&op->results, service)) {
+			success = false;
+			goto done;
+		}
+	}
+
+	if (last_end != 0xffff) {
+		uint8_t pdu[6];
+
+		put_le16(last_end + 1, pdu);
+		put_le16(0xffff, pdu + 2);
+		put_le16(GATT_PRIM_SVC_UUID, pdu + 4);
+
+		if (bt_att_send(op->att, BT_ATT_OP_READ_BY_GRP_TYPE_REQ,
+							pdu, sizeof(pdu),
+							read_by_grp_type_cb,
+							discovery_op_ref(op),
+							discovery_op_unref))
+			return;
+
+		discovery_op_unref(op);
+		success = false;
+		goto done;
+	}
+
+success:
+	/* End of procedure */
+	results = op->results.head;
+	success = true;
+
+done:
+	if (op->callback)
+		op->callback(success, att_ecode, results, op->user_data);
+}
+
 bool bt_gatt_discover_primary_services(struct bt_att *att,
 					bt_uuid_t *uuid,
 					bt_gatt_discovery_callback_t callback,
 					void *user_data,
 					bt_gatt_destroy_func_t destroy)
 {
-	/* TODO */
-	return false;
+	struct discovery_op *op;
+	bool result;
+
+	if (!att)
+		return false;
+
+	op = new0(struct discovery_op, 1);
+	if (!op)
+		return false;
+
+	op->att = att;
+	op->callback = callback;
+	op->user_data = user_data;
+	op->destroy = destroy;
+
+	/* If UUID is NULL, then discover all primary services */
+	if (!uuid) {
+		uint8_t pdu[6];
+
+		put_le16(0x0001, pdu);
+		put_le16(0xffff, pdu + 2);
+		put_le16(GATT_PRIM_SVC_UUID, pdu + 4);
+
+		result = bt_att_send(att, BT_ATT_OP_READ_BY_GRP_TYPE_REQ,
+							pdu, sizeof(pdu),
+							read_by_grp_type_cb,
+							discovery_op_ref(op),
+							discovery_op_unref);
+	} else {
+		free(op);
+		return false;
+	}
+
+	if (!result)
+		free(op);
+
+	return result;
 }
 
 bool bt_gatt_discover_included_services(struct bt_att *att,
-- 
2.0.0.526.g5318336


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 04/11] shared/gatt: Implement "Discover Primary Service by UUID" procedure.
  2014-07-25 22:08 [PATCH v2 00/11] Add shared/gatt-helpers Arman Uguray
                   ` (2 preceding siblings ...)
  2014-07-25 22:08 ` [PATCH v2 03/11] shared/gatt: Implement "Discover All Primary Services" procedure Arman Uguray
@ 2014-07-25 22:08 ` Arman Uguray
  2014-07-25 22:08 ` [PATCH v2 05/11] shared/gatt: Implement "Characteristic Discovery" procedures Arman Uguray
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Arman Uguray @ 2014-07-25 22:08 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Arman Uguray

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

diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index d427eaf..e3ca681 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -220,10 +220,27 @@ static void discovery_op_unref(void *data)
 
 static void put_uuid_le(const bt_uuid_t *src, void *dst)
 {
-	if (src->type == BT_UUID16)
+	bt_uuid_t uuid;
+
+	switch (src->type) {
+	case BT_UUID16:
 		put_le16(src->value.u16, dst);
-	else
+		break;
+	case BT_UUID128:
 		bswap_128(&src->value.u128, dst);
+		break;
+	case BT_UUID32:
+		bt_uuid_to_uuid128(src, &uuid);
+		bswap_128(&uuid.value.u128, dst);
+		break;
+	default:
+		break;
+	}
+}
+
+static inline int get_uuid_len(const bt_uuid_t *uuid)
+{
+	return (uuid->type == BT_UUID16) ? 2 : 16;
 }
 
 static bool convert_uuid_le(const uint8_t *src, size_t len, uint8_t dst[16])
@@ -336,6 +353,89 @@ done:
 		op->callback(success, att_ecode, results, op->user_data);
 }
 
+static void find_by_type_val_cb(uint8_t opcode, const void *pdu,
+					uint16_t length, void *user_data)
+{
+	struct discovery_op *op = user_data;
+	bool success;
+	uint8_t att_ecode = 0;
+	struct bt_gatt_list *results = NULL;
+	uint16_t last_end;
+	int i;
+
+	if (opcode == BT_ATT_OP_ERROR_RSP) {
+		success = false;
+		att_ecode = process_error(pdu, length);
+
+		if (att_ecode == BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND &&
+						!list_isempty(&op->results))
+			goto success;
+
+		goto done;
+	}
+
+	/* PDU must contain 4 bytes and it must be a multiple of 4, where each
+	 * 4 bytes contain the 16-bit attribute and group end handles.
+	 */
+	if (opcode != BT_ATT_OP_FIND_BY_TYPE_VAL_RSP || !pdu || !length ||
+								length % 4) {
+		success = false;
+		goto done;
+	}
+
+	for (i = 0; i < length; i += 4) {
+		struct bt_gatt_service *service;
+		bt_uuid_t uuid;
+
+		service = new0(struct bt_gatt_service, 1);
+		if (!service) {
+			success = false;
+			goto done;
+		}
+
+		service->start = get_le16(pdu + i);
+		last_end = get_le16(pdu + i + 2);
+		service->end = last_end;
+
+		bt_uuid_to_uuid128(&op->uuid, &uuid);
+		memcpy(service->uuid, uuid.value.u128.data, 16);
+
+		if (!list_add(&op->results, service)) {
+			success = false;
+			goto done;
+		}
+	}
+
+	if (last_end != 0xffff) {
+		uint8_t pdu[6 + get_uuid_len(&op->uuid)];
+
+		put_le16(last_end + 1, pdu);
+		put_le16(0xffff, pdu + 2);
+		put_le16(GATT_PRIM_SVC_UUID, pdu + 4);
+		put_uuid_le(&op->uuid, pdu + 6);
+
+		if (bt_att_send(op->att, BT_ATT_OP_FIND_BY_TYPE_VAL_REQ,
+							pdu, sizeof(pdu),
+							find_by_type_val_cb,
+							discovery_op_ref(op),
+							discovery_op_unref))
+			return;
+
+		discovery_op_unref(op);
+		success = false;
+		goto done;
+	}
+
+success:
+	/* End of procedure */
+	results = op->results.head;
+	success = true;
+
+done:
+	if (op->callback)
+		op->callback(success, att_ecode, results, op->user_data);
+}
+
 bool bt_gatt_discover_primary_services(struct bt_att *att,
 					bt_uuid_t *uuid,
 					bt_gatt_discovery_callback_t callback,
@@ -371,8 +471,26 @@ bool bt_gatt_discover_primary_services(struct bt_att *att,
 							discovery_op_ref(op),
 							discovery_op_unref);
 	} else {
-		free(op);
-		return false;
+		uint8_t pdu[6 + get_uuid_len(uuid)];
+
+		if (uuid->type == BT_UUID_UNSPEC) {
+			free(op);
+			return false;
+		}
+
+		/* Discover by UUID */
+		op->uuid = *uuid;
+
+		put_le16(0x0001, pdu);
+		put_le16(0xffff, pdu + 2);
+		put_le16(GATT_PRIM_SVC_UUID, pdu + 4);
+		put_uuid_le(&op->uuid, pdu + 6);
+
+		result = bt_att_send(att, BT_ATT_OP_FIND_BY_TYPE_VAL_REQ,
+							pdu, sizeof(pdu),
+							find_by_type_val_cb,
+							discovery_op_ref(op),
+							discovery_op_unref);
 	}
 
 	if (!result)
-- 
2.0.0.526.g5318336


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 05/11] shared/gatt: Implement "Characteristic Discovery" procedures.
  2014-07-25 22:08 [PATCH v2 00/11] Add shared/gatt-helpers Arman Uguray
                   ` (3 preceding siblings ...)
  2014-07-25 22:08 ` [PATCH v2 04/11] shared/gatt: Implement "Discover Primary Service by UUID" procedure Arman Uguray
@ 2014-07-25 22:08 ` Arman Uguray
  2014-07-25 22:08 ` [PATCH v2 06/11] shared/gatt: Implement "Descriptor Discovery" procedure Arman Uguray
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Arman Uguray @ 2014-07-25 22:08 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Arman Uguray

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

diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index e3ca681..283e6d2 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -510,6 +510,143 @@ bool bt_gatt_discover_included_services(struct bt_att *att,
 	return false;
 }
 
+struct discover_chrcs_op {
+	struct discovery_op data;
+	bool by_uuid;
+	uint16_t end;
+	struct bt_gatt_characteristic *prev_chrc;
+};
+
+static struct discover_chrcs_op *discover_chrcs_op_ref(
+						struct discover_chrcs_op *op)
+{
+	__sync_fetch_and_add(&op->data.ref_count, 1);
+
+	return op;
+}
+
+static void discover_chrcs_op_unref(void *data)
+{
+	struct discover_chrcs_op *op = data;
+
+	if (__sync_sub_and_fetch(&op->data.ref_count, 1))
+		return;
+
+	if (op->data.destroy)
+		op->data.destroy(op->data.user_data);
+
+	list_free(&op->data.results, free);
+
+	free(op);
+}
+
+static void discover_chrcs_cb(uint8_t opcode, const void *pdu,
+					uint16_t length, void *user_data)
+{
+	struct discover_chrcs_op *op = user_data;
+	bool success;
+	uint8_t att_ecode = 0;
+	struct bt_gatt_list *results = NULL;
+	size_t data_length;
+	uint16_t last_handle;
+	int i;
+
+	if (opcode == BT_ATT_OP_ERROR_RSP) {
+		success = false;
+		att_ecode = process_error(pdu, length);
+
+		if (att_ecode == BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND &&
+					!list_isempty(&op->data.results))
+			goto success;
+
+		goto done;
+	}
+
+	/* PDU must contain at least the following (sans opcode):
+	 * - Attr Data Length (1 octet)
+	 * - Attr Data List (at least 7 octets):
+	 *   -- 2 octets: Attribute handle
+	 *   -- 1 octet: Characteristic properties
+	 *   -- 2 octets: Characteristic value handle
+	 *   -- 2 or 16 octets: characteristic UUID
+	 */
+	if (opcode != BT_ATT_OP_READ_BY_TYPE_RSP || !pdu || length < 8) {
+		success = false;
+		goto done;
+	}
+
+	data_length = ((uint8_t *) pdu)[0];
+
+	if (((length - 1) % data_length) ||
+			(data_length != 7 && data_length != 21)) {
+		success = false;
+		goto done;
+	}
+
+	for (i = 1; i < length; i += data_length) {
+		struct bt_gatt_characteristic *chrc;
+		bt_uuid_t uuid;
+
+		chrc = new0(struct bt_gatt_characteristic, 1);
+		if (!chrc) {
+			success = false;
+			goto done;
+		}
+
+		last_handle = get_le16(pdu + i);
+		chrc->start = last_handle;
+		chrc->properties = ((uint8_t *) pdu)[i + 2];
+		chrc->value = get_le16(pdu + i + 3);
+		convert_uuid_le(pdu + i + 5, data_length - 5, chrc->uuid);
+
+		uuid.type = BT_UUID128;
+		memcpy(&uuid.value.u128, chrc->uuid, 16);
+
+		if (op->prev_chrc)
+			op->prev_chrc->end = chrc->start - 1;
+
+		op->prev_chrc = chrc;
+
+		if (!op->by_uuid || !bt_uuid_cmp(&uuid, &op->data.uuid)) {
+			if (!list_add(&op->data.results, chrc)) {
+				success = false;
+				goto done;
+			}
+		}
+	}
+
+	if (last_handle != op->end) {
+		uint8_t pdu[6];
+
+		put_le16(last_handle + 1, pdu);
+		put_le16(op->end, pdu + 2);
+		put_le16(GATT_CHARAC_UUID, pdu + 4);
+
+		if (bt_att_send(op->data.att, BT_ATT_OP_READ_BY_TYPE_REQ,
+						pdu, sizeof(pdu),
+						discover_chrcs_cb,
+						discover_chrcs_op_ref(op),
+						discover_chrcs_op_unref))
+			return;
+
+		discover_chrcs_op_unref(op);
+		success = false;
+		goto done;
+	}
+
+success:
+	results = op->data.results.head;
+	success = true;
+
+	if (op->prev_chrc)
+		op->prev_chrc->end = op->end - 1;
+
+done:
+	if (op->data.callback)
+		op->data.callback(success, att_ecode, results,
+							op->data.user_data);
+}
+
 bool bt_gatt_discover_characteristics(struct bt_att *att,
 					uint16_t start, uint16_t end,
 					bt_uuid_t *uuid,
@@ -517,8 +654,41 @@ bool bt_gatt_discover_characteristics(struct bt_att *att,
 					void *user_data,
 					bt_gatt_destroy_func_t destroy)
 {
-	/* TODO */
-	return false;
+	struct discover_chrcs_op *op;
+	uint8_t pdu[6];
+
+	if (!att)
+		return false;
+
+	op = new0(struct discover_chrcs_op, 1);
+	if (!op)
+		return false;
+
+	if (uuid) {
+		op->by_uuid = true;
+		op->data.uuid = *uuid;
+	}
+
+	op->data.att = att;
+	op->data.callback = callback;
+	op->data.user_data = user_data;
+	op->data.destroy = destroy;
+	op->end = end;
+
+	put_le16(start, pdu);
+	put_le16(end, pdu + 2);
+	put_le16(GATT_CHARAC_UUID, pdu + 4);
+
+	if (!bt_att_send(att, BT_ATT_OP_READ_BY_TYPE_REQ,
+					pdu, sizeof(pdu),
+					discover_chrcs_cb,
+					discover_chrcs_op_ref(op),
+					discover_chrcs_op_unref)) {
+		free(op);
+		return false;
+	}
+
+	return true;
 }
 
 bool bt_gatt_discover_descriptors(struct bt_att *att,
-- 
2.0.0.526.g5318336


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 06/11] shared/gatt: Implement "Descriptor Discovery" procedure.
  2014-07-25 22:08 [PATCH v2 00/11] Add shared/gatt-helpers Arman Uguray
                   ` (4 preceding siblings ...)
  2014-07-25 22:08 ` [PATCH v2 05/11] shared/gatt: Implement "Characteristic Discovery" procedures Arman Uguray
@ 2014-07-25 22:08 ` Arman Uguray
  2014-07-25 22:08 ` [PATCH v2 07/11] shared/gatt: Implement "Read" procedure Arman Uguray
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Arman Uguray @ 2014-07-25 22:08 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Arman Uguray

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

diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index 283e6d2..d02b212 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -691,14 +691,165 @@ bool bt_gatt_discover_characteristics(struct bt_att *att,
 	return true;
 }
 
+struct discover_descs_op {
+	struct discovery_op data;
+	uint16_t end;
+};
+
+static struct discover_descs_op *discover_descs_op_ref(
+						struct discover_descs_op *op)
+{
+	__sync_fetch_and_add(&op->data.ref_count, 1);
+
+	return op;
+}
+
+static void discover_descs_op_unref(void *data)
+{
+	struct discover_descs_op *op = data;
+
+	if (__sync_sub_and_fetch(&op->data.ref_count, 1))
+		return;
+
+	if (op->data.destroy)
+		op->data.destroy(op->data.user_data);
+
+	list_free(&op->data.results, free);
+
+	free(op);
+}
+
+static void discover_descs_cb(uint8_t opcode, const void *pdu,
+					uint16_t length, void *user_data)
+{
+	struct discover_descs_op *op = user_data;
+	bool success;
+	uint8_t att_ecode = 0;
+	struct bt_gatt_list *results = NULL;
+	uint8_t format;
+	uint16_t last_handle;
+	size_t data_length;
+	int i;
+
+	if (opcode == BT_ATT_OP_ERROR_RSP) {
+		success = false;
+		att_ecode = process_error(pdu, length);
+
+		if (att_ecode == BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND &&
+					!list_isempty(&op->data.results))
+			goto success;
+
+		goto done;
+	}
+
+	/* The PDU should contain the following data (sans opcode):
+	 * - Format (1 octet)
+	 * - Attr Data List (at least 4 octets):
+	 *   -- 2 octets: Attribute handle
+	 *   -- 2 or 16 octets: UUID.
+	 */
+	if (opcode != BT_ATT_OP_FIND_INFO_RSP || !pdu || length < 5) {
+		success = false;
+		goto done;
+	}
+
+	format = ((uint8_t *) pdu)[0];
+
+	if (format == 0x01)
+		data_length = 4;
+	else if (format == 0x02)
+		data_length = 18;
+	else {
+		success = false;
+		goto done;
+	}
+
+	if ((length - 1) % data_length) {
+		success = false;
+		goto done;
+	}
+
+	for (i = 1; i < length; i += data_length) {
+		struct bt_gatt_descriptor *descr;
+
+		descr = new0(struct bt_gatt_descriptor, 1);
+		if (!descr) {
+			success = false;
+			goto done;
+		}
+
+		last_handle = get_le16(pdu + i);
+		descr->handle = last_handle;
+		convert_uuid_le(pdu + i + 2, data_length - 2, descr->uuid);
+
+		if (!list_add(&op->data.results, descr)) {
+			success = false;
+			goto done;
+		}
+	}
+
+	if (last_handle != op->end) {
+		uint8_t pdu[4];
+
+		put_le16(last_handle + 1, pdu);
+		put_le16(op->end, pdu + 2);
+
+		if (bt_att_send(op->data.att, BT_ATT_OP_FIND_INFO_REQ,
+						pdu, sizeof(pdu),
+						discover_descs_cb,
+						discover_descs_op_ref(op),
+						discover_descs_op_unref))
+			return;
+
+		discover_descs_op_unref(op);
+		success = false;
+		goto done;
+	}
+
+success:
+	results = op->data.results.head;
+	success = true;
+
+done:
+	if (op->data.callback)
+		op->data.callback(success, att_ecode, results,
+							op->data.user_data);
+}
+
 bool bt_gatt_discover_descriptors(struct bt_att *att,
 					uint16_t start, uint16_t end,
 					bt_gatt_discovery_callback_t callback,
 					void *user_data,
 					bt_gatt_destroy_func_t destroy)
 {
-	/* TODO */
-	return false;
+	struct discover_descs_op *op;
+	uint8_t pdu[4];
+
+	if (!att)
+		return false;
+
+	op = new0(struct discover_descs_op, 1);
+	if (!op)
+		return false;
+
+	op->data.att = att;
+	op->data.callback = callback;
+	op->data.user_data = user_data;
+	op->data.destroy = destroy;
+	op->end = end;
+
+	put_le16(start, pdu);
+	put_le16(end, pdu + 2);
+
+	if (!bt_att_send(att, BT_ATT_OP_FIND_INFO_REQ, pdu, sizeof(pdu),
+						discover_descs_cb,
+						discover_descs_op_ref(op),
+						discover_descs_op_unref)) {
+		free(op);
+		return false;
+	}
+
+	return true;
 }
 
 bool bt_gatt_read_value(struct bt_att *att, uint16_t value_handle,
-- 
2.0.0.526.g5318336


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 07/11] shared/gatt: Implement "Read" procedure.
  2014-07-25 22:08 [PATCH v2 00/11] Add shared/gatt-helpers Arman Uguray
                   ` (5 preceding siblings ...)
  2014-07-25 22:08 ` [PATCH v2 06/11] shared/gatt: Implement "Descriptor Discovery" procedure Arman Uguray
@ 2014-07-25 22:08 ` Arman Uguray
  2014-07-25 22:08 ` [PATCH v2 08/11] shared/gatt: Implement "Read Long Characteristic Values" procedure Arman Uguray
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Arman Uguray @ 2014-07-25 22:08 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Arman Uguray

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

diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index d02b212..4ba17f1 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -852,13 +852,81 @@ bool bt_gatt_discover_descriptors(struct bt_att *att,
 	return true;
 }
 
+struct read_op {
+	bt_gatt_read_callback_t callback;
+	void *user_data;
+	bt_gatt_destroy_func_t destroy;
+};
+
+static void destroy_read_op(void *data)
+{
+	struct read_op *op = data;
+
+	if (op->destroy)
+		op->destroy(op->user_data);
+
+	free(op);
+}
+
+static void read_cb(uint8_t opcode, const void *pdu, uint16_t length,
+								void *user_data)
+{
+	struct read_op *op = user_data;
+	bool success;
+	uint8_t att_ecode = 0;
+	const uint8_t *value = NULL;
+	uint16_t value_len = 0;
+
+	if (opcode == BT_ATT_OP_ERROR_RSP) {
+		success = false;
+		att_ecode = process_error(pdu, length);
+		goto done;
+	}
+
+	if (opcode != BT_ATT_OP_READ_RSP || (!pdu && length)) {
+		success = false;
+		goto done;
+	}
+
+	success = true;
+	value_len = length;
+	if (value_len)
+		value = pdu;
+
+done:
+	if (op->callback)
+		op->callback(success, att_ecode, value, length, op->user_data);
+}
+
 bool bt_gatt_read_value(struct bt_att *att, uint16_t value_handle,
 					bt_gatt_read_callback_t callback,
 					void *user_data,
 					bt_gatt_destroy_func_t destroy)
 {
-	/* TODO */
-	return false;
+	struct read_op *op;
+	uint8_t pdu[2];
+
+	if (!att)
+		return false;
+
+	op = new0(struct read_op, 1);
+	if (!op)
+		return false;
+
+	op->callback = callback;
+	op->user_data = user_data;
+	op->destroy = destroy;
+
+	put_le16(value_handle, pdu);
+
+	if (!bt_att_send(att, BT_ATT_OP_READ_REQ, pdu, sizeof(pdu),
+							read_cb, op,
+							destroy_read_op)) {
+		free(op);
+		return false;
+	}
+
+	return true;
 }
 
 bool bt_gatt_read_long_value(struct bt_att *att,
-- 
2.0.0.526.g5318336


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 08/11] shared/gatt: Implement "Read Long Characteristic Values" procedure.
  2014-07-25 22:08 [PATCH v2 00/11] Add shared/gatt-helpers Arman Uguray
                   ` (6 preceding siblings ...)
  2014-07-25 22:08 ` [PATCH v2 07/11] shared/gatt: Implement "Read" procedure Arman Uguray
@ 2014-07-25 22:08 ` Arman Uguray
  2014-07-25 22:08 ` [PATCH v2 09/11] shared/gatt: Implement "Write Value" and "Write Without Response" procedures Arman Uguray
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Arman Uguray @ 2014-07-25 22:08 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Arman Uguray

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

diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index 4ba17f1..215d35e 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -929,14 +929,214 @@ bool bt_gatt_read_value(struct bt_att *att, uint16_t value_handle,
 	return true;
 }
 
+struct read_long_op {
+	struct bt_att *att;
+	int ref_count;
+	uint16_t value_handle;
+	size_t orig_offset;
+	size_t offset;
+	struct queue *blobs;
+	bt_gatt_read_callback_t callback;
+	void *user_data;
+	bt_gatt_destroy_func_t destroy;
+};
+
+struct blob {
+	uint8_t *data;
+	uint16_t offset;
+	uint16_t length;
+};
+
+static struct blob *create_blob(const uint8_t *data, uint16_t len,
+								uint16_t offset)
+{
+	struct blob *blob;
+
+	blob = new0(struct blob, 1);
+	if (!blob)
+		return NULL;
+
+	blob->data = malloc(len);
+	if (!blob->data) {
+		free(blob);
+		return NULL;
+	}
+
+	memcpy(blob->data, data, len);
+	blob->length = len;
+	blob->offset = offset;
+
+	return blob;
+}
+
+static void destroy_blob(void *data)
+{
+	struct blob *blob = data;
+
+	free(blob->data);
+	free(blob);
+}
+
+static struct read_long_op *read_long_op_ref(struct read_long_op *op)
+{
+	__sync_fetch_and_add(&op->ref_count, 1);
+
+	return op;
+}
+
+static void read_long_op_unref(void *data)
+{
+	struct read_long_op *op = data;
+
+	if (__sync_sub_and_fetch(&op->ref_count, 1))
+		return;
+
+	if (op->destroy)
+		op->destroy(op->user_data);
+
+	queue_destroy(op->blobs, destroy_blob);
+
+	free(op);
+}
+
+static void append_blob(void *data, void *user_data)
+{
+	struct blob *blob = data;
+	uint8_t *value = user_data;
+
+	memcpy(value + blob->offset, blob->data, blob->length);
+}
+
+static void complete_read_long_op(struct read_long_op *op, bool success,
+							uint8_t att_ecode)
+{
+	uint8_t *value = NULL;
+	uint16_t length = 0;
+
+	if (!success)
+		goto done;
+
+	length = op->offset - op->orig_offset;
+
+	if (!length)
+		goto done;
+
+	value = malloc(length);
+	if (!value) {
+		success = false;
+		goto done;
+	}
+
+	queue_foreach(op->blobs, append_blob, value - op->orig_offset);
+
+done:
+	if (op->callback)
+		op->callback(success, att_ecode, value, length, op->user_data);
+
+	free(value);
+}
+
+static void read_long_cb(uint8_t opcode, const void *pdu,
+					uint16_t length, void *user_data)
+{
+	struct read_long_op *op = user_data;
+	struct blob *blob;
+	bool success;
+	uint8_t att_ecode = 0;
+
+	if (opcode == BT_ATT_OP_ERROR_RSP) {
+		success = false;
+		att_ecode = process_error(pdu, length);
+		goto done;
+	}
+
+	if (opcode != BT_ATT_OP_READ_BLOB_RSP || (!pdu && length)) {
+		success = false;
+		goto done;
+	}
+
+	if (!length)
+		goto success;
+
+	blob = create_blob(pdu, length, op->offset);
+	if (!blob) {
+		success = false;
+		goto done;
+	}
+
+	queue_push_tail(op->blobs, blob);
+	op->offset += length;
+	if (op->offset > UINT16_MAX)
+		goto success;
+
+	if (length >= bt_att_get_mtu(op->att) - 1) {
+		uint8_t pdu[4];
+
+		put_le16(op->value_handle, pdu);
+		put_le16(op->offset, pdu + 2);
+
+		if (bt_att_send(op->att, BT_ATT_OP_READ_BLOB_REQ,
+							pdu, sizeof(pdu),
+							read_long_cb,
+							read_long_op_ref(op),
+							read_long_op_unref))
+			return;
+
+		read_long_op_unref(op);
+		success = false;
+		goto done;
+	}
+
+success:
+	success = true;
+
+done:
+	complete_read_long_op(op, success, att_ecode);
+}
+
 bool bt_gatt_read_long_value(struct bt_att *att,
 					uint16_t value_handle, uint16_t offset,
 					bt_gatt_read_callback_t callback,
 					void *user_data,
 					bt_gatt_destroy_func_t destroy)
 {
-	/* TODO */
-	return false;
+	struct read_long_op *op;
+	uint8_t pdu[4];
+
+	if (!att)
+		return false;
+
+	op = new0(struct read_long_op, 1);
+	if (!op)
+		return false;
+
+	op->blobs = queue_new();
+	if (!op->blobs) {
+		free(op);
+		return false;
+	}
+
+	op->att = att;
+	op->value_handle = value_handle;
+	op->orig_offset = offset;
+	op->offset = offset;
+	op->callback = callback;
+	op->user_data = user_data;
+	op->destroy = destroy;
+
+	put_le16(value_handle, pdu);
+	put_le16(offset, pdu + 2);
+
+	if (!bt_att_send(att, BT_ATT_OP_READ_BLOB_REQ, pdu, sizeof(pdu),
+							read_long_cb,
+							read_long_op_ref(op),
+							read_long_op_unref)) {
+		queue_destroy(op->blobs, free);
+		free(op);
+		return false;
+	}
+
+	return true;
 }
 
 bool bt_gatt_write_without_response(struct bt_att *att,
-- 
2.0.0.526.g5318336


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 09/11] shared/gatt: Implement "Write Value" and "Write Without Response" procedures.
  2014-07-25 22:08 [PATCH v2 00/11] Add shared/gatt-helpers Arman Uguray
                   ` (7 preceding siblings ...)
  2014-07-25 22:08 ` [PATCH v2 08/11] shared/gatt: Implement "Read Long Characteristic Values" procedure Arman Uguray
@ 2014-07-25 22:08 ` Arman Uguray
  2014-07-25 22:08 ` [PATCH v2 10/11] shared/gatt: Implement "Write Long Values" procedure Arman Uguray
  2014-07-25 22:08 ` [PATCH v2 11/11] shared/gatt: Implement notification/indication helper Arman Uguray
  10 siblings, 0 replies; 20+ messages in thread
From: Arman Uguray @ 2014-07-25 22:08 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Arman Uguray

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

diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index 215d35e..a1e8d79 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -1141,11 +1141,60 @@ bool bt_gatt_read_long_value(struct bt_att *att,
 
 bool bt_gatt_write_without_response(struct bt_att *att,
 					uint16_t value_handle,
-                                        bool signed_write,
+					bool signed_write,
 					uint8_t *value, uint16_t length)
 {
-	/* TODO */
-	return false;
+	uint8_t pdu[2 + length];
+
+	if (!att)
+		return 0;
+
+	/* TODO: Support this once bt_att_send supports signed writes. */
+	if (signed_write)
+		return 0;
+
+	put_le16(value_handle, pdu);
+	memcpy(pdu + 2, value, length);
+
+	return bt_att_send(att, BT_ATT_OP_WRITE_CMD, pdu, sizeof(pdu),
+							NULL, NULL, NULL);
+}
+
+struct write_op {
+	bt_gatt_result_callback_t callback;
+	void *user_data;
+	bt_gatt_destroy_func_t destroy;
+};
+
+static void destroy_write_op(void *data)
+{
+	struct write_op *op = data;
+
+	if (op->destroy)
+		op->destroy(op->user_data);
+
+	free(op);
+}
+
+static void write_cb(uint8_t opcode, const void *pdu, uint16_t length,
+								void *user_data)
+{
+	struct write_op *op = user_data;
+	bool success = true;
+	uint8_t att_ecode = 0;
+
+	if (opcode == BT_ATT_OP_ERROR_RSP) {
+		success = false;
+		att_ecode = process_error(pdu, length);
+		goto done;
+	}
+
+	if (opcode != BT_ATT_OP_WRITE_RSP || pdu || length)
+		success = false;
+
+done:
+	if (op->callback)
+		op->callback(success, att_ecode, op->user_data);
 }
 
 bool bt_gatt_write_value(struct bt_att *att, uint16_t value_handle,
@@ -1154,8 +1203,31 @@ bool bt_gatt_write_value(struct bt_att *att, uint16_t value_handle,
 					void *user_data,
 					bt_gatt_destroy_func_t destroy)
 {
-	/* TODO */
-	return false;
+	struct write_op *op;
+	uint8_t pdu[2 + length];
+
+	if (!att)
+		return false;
+
+	op = new0(struct write_op, 1);
+	if (!op)
+		return false;
+
+	op->callback = callback;
+	op->user_data = user_data;
+	op->destroy = destroy;
+
+	put_le16(value_handle, pdu);
+	memcpy(pdu + 2, value, length);
+
+	if (!bt_att_send(att, BT_ATT_OP_WRITE_REQ, pdu, sizeof(pdu),
+							write_cb, op,
+							destroy_write_op)) {
+		free(op);
+		return false;
+	}
+
+	return true;
 }
 
 bool bt_gatt_write_long_value(struct bt_att *att, bool reliable,
-- 
2.0.0.526.g5318336


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 10/11] shared/gatt: Implement "Write Long Values" procedure.
  2014-07-25 22:08 [PATCH v2 00/11] Add shared/gatt-helpers Arman Uguray
                   ` (8 preceding siblings ...)
  2014-07-25 22:08 ` [PATCH v2 09/11] shared/gatt: Implement "Write Value" and "Write Without Response" procedures Arman Uguray
@ 2014-07-25 22:08 ` Arman Uguray
  2014-07-25 22:08 ` [PATCH v2 11/11] shared/gatt: Implement notification/indication helper Arman Uguray
  10 siblings, 0 replies; 20+ messages in thread
From: Arman Uguray @ 2014-07-25 22:08 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Arman Uguray

This patch implements bt_gatt_write_long_value.
---
 src/shared/gatt-helpers.c | 240 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 238 insertions(+), 2 deletions(-)

diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index a1e8d79..59fddab 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -1230,6 +1230,180 @@ bool bt_gatt_write_value(struct bt_att *att, uint16_t value_handle,
 	return true;
 }
 
+struct write_long_op {
+	struct bt_att *att;
+	int ref_count;
+	bool reliable;
+	bool success;
+	uint8_t att_ecode;
+	bool reliable_error;
+	uint16_t value_handle;
+	uint8_t *value;
+	uint16_t length;
+	uint16_t offset;
+	uint16_t index;
+	uint16_t cur_length;
+	bt_gatt_write_long_callback_t callback;
+	void *user_data;
+	bt_gatt_destroy_func_t destroy;
+};
+
+static struct write_long_op *write_long_op_ref(struct write_long_op *op)
+{
+	__sync_fetch_and_add(&op->ref_count, 1);
+
+	return op;
+}
+
+static void write_long_op_unref(void *data)
+{
+	struct write_long_op *op = data;
+
+	if (__sync_sub_and_fetch(&op->ref_count, 1))
+		return;
+
+	if (op->destroy)
+		op->destroy(op->user_data);
+
+	free(op->value);
+	free(op);
+}
+
+static void execute_write_cb(uint8_t opcode, const void *pdu, uint16_t length,
+								void *user_data)
+{
+	struct write_long_op *op = user_data;
+	bool success = op->success;
+	uint8_t att_ecode = op->att_ecode;
+
+	if (opcode == BT_ATT_OP_ERROR_RSP) {
+		success = false;
+		att_ecode = process_error(pdu, length);
+	} else if (opcode != BT_ATT_OP_EXEC_WRITE_RSP || pdu || length)
+		success = false;
+
+	if (op->callback)
+		op->callback(success, op->reliable_error, att_ecode,
+								op->user_data);
+}
+
+static void complete_write_long_op(struct write_long_op *op, bool success,
+					uint8_t att_ecode, bool reliable_error)
+{
+	uint8_t pdu;
+
+	op->success = success;
+	op->att_ecode = att_ecode;
+	op->reliable_error = reliable_error;
+
+	if (success)
+		pdu = 0x01;  /* Write */
+	else
+		pdu = 0x00;  /* Cancel */
+
+	if (bt_att_send(op->att, BT_ATT_OP_EXEC_WRITE_REQ, &pdu, sizeof(pdu),
+						execute_write_cb,
+						write_long_op_ref(op),
+						write_long_op_unref))
+		return;
+
+	write_long_op_unref(op);
+	success = false;
+
+	if (op->callback)
+		op->callback(success, reliable_error, att_ecode, op->user_data);
+}
+
+static void prepare_write_cb(uint8_t opcode, const void *pdu, uint16_t length,
+								void *user_data)
+{
+	struct write_long_op *op = user_data;
+	bool success = true;
+	bool reliable_error = false;
+	uint8_t att_ecode = 0;
+	uint16_t next_index;
+
+	if (opcode == BT_ATT_OP_ERROR_RSP) {
+		success = false;
+		att_ecode = process_error(pdu, length);
+		goto done;
+	}
+
+	if (opcode != BT_ATT_OP_PREP_WRITE_RSP) {
+		success = false;
+		goto done;
+	}
+
+	if (op->reliable) {
+		if (!pdu || length != (op->cur_length + 4)) {
+			success = false;
+			reliable_error = true;
+			goto done;
+		}
+
+		if (get_le16(pdu) != op->value_handle ||
+				get_le16(pdu + 2) != (op->offset + op->index)) {
+			success = false;
+			reliable_error = true;
+			goto done;
+		}
+
+		if (memcmp(pdu + 4, op->value + op->index, op->cur_length)) {
+			success = false;
+			reliable_error = true;
+			goto done;
+		}
+	}
+
+	next_index = op->index + op->cur_length;
+	if (next_index == op->length) {
+		/* All bytes written */
+		goto done;
+	}
+
+	/* If the last written length greater than or equal to what can fit
+	 * inside a PDU, then there is more data to send.
+	 */
+	if (op->cur_length >= bt_att_get_mtu(op->att) - 5) {
+		uint8_t *pdu;
+
+		op->index = next_index;
+		op->cur_length = MIN(op->length - op->index,
+						bt_att_get_mtu(op->att) - 5);
+
+		pdu = malloc(op->cur_length + 4);
+		if (!pdu) {
+			success = false;
+			goto done;
+		}
+
+		put_le16(op->value_handle, pdu);
+		put_le16(op->offset + op->index, pdu + 2);
+		memcpy(pdu + 4, op->value + op->index, op->cur_length);
+
+		if (!bt_att_send(op->att, BT_ATT_OP_PREP_WRITE_REQ,
+							pdu, op->cur_length + 4,
+							prepare_write_cb,
+							write_long_op_ref(op),
+							write_long_op_unref)) {
+			write_long_op_unref(op);
+			success = false;
+		}
+
+		free(pdu);
+
+		/* If so far successful, then the operation should continue.
+		 * Otherwise, there was an error and the procedure should be
+		 * completed.
+		 */
+		if (success)
+			return;
+	}
+
+done:
+	complete_write_long_op(op, success, att_ecode, reliable_error);
+}
+
 bool bt_gatt_write_long_value(struct bt_att *att, bool reliable,
 					uint16_t value_handle, uint16_t offset,
 					uint8_t *value, uint16_t length,
@@ -1237,8 +1411,70 @@ bool bt_gatt_write_long_value(struct bt_att *att, bool reliable,
 					void *user_data,
 					bt_gatt_destroy_func_t destroy)
 {
-	/* TODO */
-	return false;
+	struct write_long_op *op;
+	uint8_t *pdu;
+	bool status;
+
+	if (!att)
+		return false;
+
+	if ((size_t)(length + offset) > UINT16_MAX)
+		return false;
+
+	/* Don't allow riting a 0-length value using this procedure. The
+	 * upper-layer should use bt_gatt_write_value for that instead.
+	 */
+	if (!length || !value)
+		return false;
+
+	op = new0(struct write_long_op, 1);
+	if (!op)
+		return false;
+
+	op->value = malloc(length);
+	if (!op->value) {
+		free(op);
+		return false;
+	}
+
+	memcpy(op->value, value, length);
+
+	op->att = att;
+	op->reliable = reliable;
+	op->value_handle = value_handle;
+	op->length = length;
+	op->offset = offset;
+	op->cur_length = MIN(length, bt_att_get_mtu(att) - 5);
+	op->callback = callback;
+	op->user_data = user_data;
+	op->destroy = destroy;
+
+	pdu = malloc(op->cur_length + 4);
+	if (!pdu) {
+		free(op->value);
+		free(op);
+		return false;
+	}
+
+	put_le16(value_handle, pdu);
+	put_le16(offset, pdu + 2);
+	memcpy(pdu + 4, op->value, op->cur_length);
+
+	status = bt_att_send(att, BT_ATT_OP_PREP_WRITE_REQ,
+							pdu, op->cur_length + 4,
+							prepare_write_cb,
+							write_long_op_ref(op),
+							write_long_op_unref);
+
+	free(pdu);
+
+	if (!status) {
+		free(op->value);
+		free(op);
+		return false;
+	}
+
+	return true;
 }
 
 unsigned int bt_gatt_register(struct bt_att *att, bool indications,
-- 
2.0.0.526.g5318336


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 11/11] shared/gatt: Implement notification/indication helper.
  2014-07-25 22:08 [PATCH v2 00/11] Add shared/gatt-helpers Arman Uguray
                   ` (9 preceding siblings ...)
  2014-07-25 22:08 ` [PATCH v2 10/11] shared/gatt: Implement "Write Long Values" procedure Arman Uguray
@ 2014-07-25 22:08 ` Arman Uguray
  10 siblings, 0 replies; 20+ messages in thread
From: Arman Uguray @ 2014-07-25 22:08 UTC (permalink / raw
  To: linux-bluetooth; +Cc: Arman Uguray

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

diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index 59fddab..489b8c4 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -1477,11 +1477,69 @@ bool bt_gatt_write_long_value(struct bt_att *att, bool reliable,
 	return true;
 }
 
+struct notify_data {
+	struct bt_att *att;
+	bt_gatt_notify_callback_t callback;
+	void *user_data;
+	bt_gatt_destroy_func_t destroy;
+};
+
+static void notify_data_destroy(void *data)
+{
+	struct notify_data *notd = data;
+
+	if (notd->destroy)
+		notd->destroy(notd->user_data);
+
+	free(notd);
+}
+
+static void notify_cb(uint8_t opcode, const void *pdu, uint16_t length,
+								void *user_data)
+{
+	struct notify_data *data = user_data;
+	uint16_t value_handle;
+	const uint8_t *value = NULL;
+
+	value_handle = get_le16(pdu);
+
+	if (length > 2)
+		value = pdu + 2;
+
+	if (data->callback)
+		data->callback(value_handle, value, length - 2, data->user_data);
+
+	if (opcode == BT_ATT_OP_HANDLE_VAL_IND)
+		bt_att_send(data->att, BT_ATT_OP_HANDLE_VAL_CONF, NULL, 0,
+							NULL, NULL, NULL);
+}
+
 unsigned int bt_gatt_register(struct bt_att *att, bool indications,
 					bt_gatt_notify_callback_t callback,
 					void *user_data,
 					bt_gatt_destroy_func_t destroy)
 {
-	/* TODO */
-	return false;
+	struct notify_data *data;
+	uint8_t opcode;
+	unsigned int id;
+
+	if (!att)
+		return 0;
+
+	data = new0(struct notify_data, 1);
+	if (!data)
+		return 0;
+
+	data->att = att;
+	data->callback = callback;
+	data->user_data = user_data;
+	data->destroy = destroy;
+
+	opcode = indications ? BT_ATT_OP_HANDLE_VAL_IND : BT_ATT_OP_HANDLE_VAL_NOT;
+
+	id = bt_att_register(att, opcode, notify_cb, data, notify_data_destroy);
+	if (!id)
+		free(data);
+
+	return id;
 }
-- 
2.0.0.526.g5318336


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 01/11] shared/gatt: Introduce gatt-helpers.h skeleton.
  2014-07-25 22:08 ` [PATCH v2 01/11] shared/gatt: Introduce gatt-helpers.h skeleton Arman Uguray
@ 2014-07-25 22:47   ` Marcel Holtmann
  2014-07-26  0:01     ` Arman Uguray
  0 siblings, 1 reply; 20+ messages in thread
From: Marcel Holtmann @ 2014-07-25 22:47 UTC (permalink / raw
  To: Arman Uguray; +Cc: linux-bluetooth

Hi Arman,

> This patch introduces the skeleton for src/shared/gatt-helpers, which defines
> helper functions for over-the-air GATT client-side procedures that will be
> frequently used by clients. Most of these functions require several sequential
> ATT protocol requests and it is useful to abstract these details away from the
> upper layer.

<snip>

> 
> +struct bt_gatt_service {
> +	uint16_t start;
> +	uint16_t end;
> +	uint8_t uuid[16];
> +};
> +
> +struct bt_gatt_characteristic {
> +	uint16_t start;
> +	uint16_t end;
> +	uint16_t value;
> +	uint8_t properties;
> +	uint8_t uuid[16];
> +};
> +
> +struct bt_gatt_descriptor {
> +	uint16_t handle;
> +	uint8_t uuid[16];
> +};
> +
> +struct bt_gatt_list;
> +
> +struct bt_gatt_list *bt_gatt_list_get_next(struct bt_gatt_list *l);
> +void *bt_gatt_list_get_data(struct bt_gatt_list *l);
> +
> +typedef void (*bt_gatt_destroy_func_t)(void *user_data);
> +
> +typedef void (*bt_gatt_result_callback_t)(bool success, uint8_t att_ecode,
> +							void *user_data);
> +typedef void (*bt_gatt_discovery_callback_t)(bool success, uint8_t att_ecode,
> +						struct bt_gatt_list *results,
> +						void *user_data);

what we have used a lot is the concept of an iterator in a lot of protocols that have complex return types. It makes it generic, flexible and most important it results in readable code.

	struct bt_gatt_result;
	struct bt_gatt_iter;

	bool bt_gatt_iter_init(struct bt_gatt_iter *iter,
				struct bt_gatt_result *result);

	bool bt_gatt_iter_next_uuid(struct bt_gatt_iter *iter,
					uint8_t *len, const uint8_t **uuid);


This means we could process the result of a list of UUIDs as simple as this:

	if (!bt_gatt_iter_init(&iter, result)
		return;

	while (bt_gatt_iter_next_uuid(&iter, &len, &uuid))
		printf("len %d uuid %p\n", len, uuid);

Possible other types could be _next_handle_range etc. If needed we could even allow one level nesting here.

One interesting advantage is that the bt_gatt_result could just store the result PDU as it is an we just return pointers to the next field. That would help us avoid having to copy data around.

For internal representation we might choose to allow linking multiple bt_gatt_result together and only return the head item. And then just let the bt_gatt_iter point to the next one when moved forward.

I went through the whole patchset and besides this detail, it looks pretty solid. I let others have a look at it as well. So we could start applying patches and then change to an iterator approach or you fix it up before applying. What do you think?

Regards

Marcel


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 01/11] shared/gatt: Introduce gatt-helpers.h skeleton.
  2014-07-25 22:47   ` Marcel Holtmann
@ 2014-07-26  0:01     ` Arman Uguray
  2014-07-26  1:07       ` Arman Uguray
  2014-07-26  3:35       ` Marcel Holtmann
  0 siblings, 2 replies; 20+ messages in thread
From: Arman Uguray @ 2014-07-26  0:01 UTC (permalink / raw
  To: Marcel Holtmann; +Cc: Arman Uguray, BlueZ development

Hi Marcel,

> what we have used a lot is the concept of an iterator in a lot of protocols that have complex return types. It makes it generic, flexible and most important it results in readable code.
>
>         struct bt_gatt_result;
>         struct bt_gatt_iter;
>
>         bool bt_gatt_iter_init(struct bt_gatt_iter *iter,
>                                 struct bt_gatt_result *result);
>

I actually like this approach better. I guess this way, we can use the
returned result structures as the basis of a GATT client database. I'm
not yet convinced if src/shared/gatt-db is the correct approach for
the client implementation. I still need to figure out the kinks but
this sounds good.

> One interesting advantage is that the bt_gatt_result could just store the result PDU as it is an we just return pointers to the next field. That would help us avoid having to copy data around.
>

We could. Though we have to copy the data from the PDU at least once
at some point if the caller wants to store that data somewhere (since
the PDU array is temporarily provided by bt_att).

> For internal representation we might choose to allow linking multiple bt_gatt_result together and only return the head item. And then just let the bt_gatt_iter point to the next one when moved forward.
>

This sounds good. I suppose we would then also provide a free function
for bt_gatt_result, instead of cleaning it up internally after the
callback returns?

> I went through the whole patchset and besides this detail, it looks pretty solid. I let others have a look at it as well. So we could start applying patches and then change to an iterator approach or you fix it up before applying. What do you think?
>

I say let's merge these the way they are now. It's easier for me to
submit one small patch that implements the iterator approach than
having to rebase all of these. A single patch later would be easier to
review as well.

Cheers,
Arman

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 01/11] shared/gatt: Introduce gatt-helpers.h skeleton.
  2014-07-26  0:01     ` Arman Uguray
@ 2014-07-26  1:07       ` Arman Uguray
  2014-07-26  3:35       ` Marcel Holtmann
  1 sibling, 0 replies; 20+ messages in thread
From: Arman Uguray @ 2014-07-26  1:07 UTC (permalink / raw
  To: Marcel Holtmann; +Cc: Arman Uguray, BlueZ development

Hi Marcel,

I gave some thought to the iterator approach (even if we do this in an
upcoming patch set, we might as well continue the discussion here). I
think there is benefit to at least some processing of the PDU, since
the format of its contents vary a lot between the different discovery
procedures (i.e. all primary services, primary services by UUID,
characteristics, and descriptors). This is what I have in mind:

struct bt_gatt_result;
struct bt_gatt_iter;

typedef enum {
        BT_GATT_RESULT_TYPE_SERVICE,
        BT_GATT_RESULT_TYPE_CHARACTERISTIC,
        BT_GATT_RESULT_TYPE_DESCRIPTOR,
} bt_gatt_result_type_t;

bt_gatt_result_type_t bt_gatt_result_get_type(struct bt_gatt_result *result);
void bt_gatt_result_destroy(struct bt_gatt_result *result);

bool bt_gatt_iter_init(struct bt_gatt_iter *iter, struct
bt_gatt_result *result);
bool bt_gatt_iter_next_service(struct bt_gatt_iter *iter,
                                        struct bt_gatt_service **service);
bool bt_gatt_iter_next_characteristic(struct bt_gatt_iter *iter,
                                struct bt_gatt_characteristic **characteristic);
bool bt_gatt_iter_next_descriptor(struct bt_gatt_iter *iter,
                                        struct bt_gatt_descriptor **descriptor);

We keep the initial bt_gatt_service, bt_gatt_characteristic,
bt_gatt_descriptor structures and allow iterating through a linked
list of arrays of these structures. I think the data contained in
those structures are essential and providing anything less isn't very
useful, though we can always add other iterator functions in the
future.

Any thoughts/suggestions?

Cheers,
Arman

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 01/11] shared/gatt: Introduce gatt-helpers.h skeleton.
  2014-07-26  0:01     ` Arman Uguray
  2014-07-26  1:07       ` Arman Uguray
@ 2014-07-26  3:35       ` Marcel Holtmann
  1 sibling, 0 replies; 20+ messages in thread
From: Marcel Holtmann @ 2014-07-26  3:35 UTC (permalink / raw
  To: Arman Uguray; +Cc: Arman Uguray, BlueZ development

Hi Arman,

>> what we have used a lot is the concept of an iterator in a lot of protocols that have complex return types. It makes it generic, flexible and most important it results in readable code.
>> 
>>        struct bt_gatt_result;
>>        struct bt_gatt_iter;
>> 
>>        bool bt_gatt_iter_init(struct bt_gatt_iter *iter,
>>                                struct bt_gatt_result *result);
>> 
> 
> I actually like this approach better. I guess this way, we can use the
> returned result structures as the basis of a GATT client database. I'm
> not yet convinced if src/shared/gatt-db is the correct approach for
> the client implementation. I still need to figure out the kinks but
> this sounds good.

not sure that bt_gatt_result should be used as a client database. It feels wrong since I think that lifetime of bt_gatt_result should be limited to the callback. Then again, trying something different and see how it works out never hurts.

>> One interesting advantage is that the bt_gatt_result could just store the result PDU as it is an we just return pointers to the next field. That would help us avoid having to copy data around.
>> 
> 
> We could. Though we have to copy the data from the PDU at least once
> at some point if the caller wants to store that data somewhere (since
> the PDU array is temporarily provided by bt_att).

As less copying of data and allocation of new structures as possible should be the key. There might be users that only need part of the information. They should have the ability to skip parts of the response without any extra memory allocation or copy of data.

>> For internal representation we might choose to allow linking multiple bt_gatt_result together and only return the head item. And then just let the bt_gatt_iter point to the next one when moved forward.
>> 
> 
> This sounds good. I suppose we would then also provide a free function
> for bt_gatt_result, instead of cleaning it up internally after the
> callback returns?

I think bt_gatt_result should only be valid in the context of the callback. That is normally how the result variable and iterators work. Their scope is limited. This makes them so easy to use and keeps the code simple.

>> I went through the whole patchset and besides this detail, it looks pretty solid. I let others have a look at it as well. So we could start applying patches and then change to an iterator approach or you fix it up before applying. What do you think?
>> 
> 
> I say let's merge these the way they are now. It's easier for me to
> submit one small patch that implements the iterator approach than
> having to rebase all of these. A single patch later would be easier to
> review as well.

I applied all patches now.

Regards

Marcel


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 03/11] shared/gatt: Implement "Discover All Primary Services" procedure.
  2014-07-25 22:08 ` [PATCH v2 03/11] shared/gatt: Implement "Discover All Primary Services" procedure Arman Uguray
@ 2014-07-26 11:02   ` Marcel Holtmann
  2014-07-26 20:03     ` Arman Uguray
  0 siblings, 1 reply; 20+ messages in thread
From: Marcel Holtmann @ 2014-07-26 11:02 UTC (permalink / raw
  To: Arman Uguray; +Cc: linux-bluetooth

Hi Arman,

> This patch implements bt_gatt_discover_primary_services for the case when no
> UUID is provided.
> ---
> src/shared/gatt-helpers.c | 253 +++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 251 insertions(+), 2 deletions(-)
> 
> diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
> index 96137bf..d427eaf 100644
> --- a/src/shared/gatt-helpers.c
> +++ b/src/shared/gatt-helpers.c
> @@ -36,6 +36,70 @@
> #define MIN(a, b) ((a) < (b) ? (a) : (b))
> #endif
> 
> +struct bt_gatt_list {
> +	struct bt_gatt_list *next;
> +	void *data;
> +};
> +
> +struct list_ptrs {
> +	struct bt_gatt_list *head;
> +	struct bt_gatt_list *tail;
> +};
> +
> +struct bt_gatt_list *bt_gatt_list_get_next(struct bt_gatt_list *list)
> +{
> +	return list->next;
> +}
> +
> +void *bt_gatt_list_get_data(struct bt_gatt_list *list)
> +{
> +	return list->data;
> +}
> +
> +static inline bool list_isempty(struct list_ptrs *list)
> +{
> +	return !list->head && !list->tail;
> +}
> +
> +static bool list_add(struct list_ptrs *list, void *data)
> +{
> +	struct bt_gatt_list *item = new0(struct bt_gatt_list, 1);
> +	if (!item)
> +		return false;
> +
> +	item->data = data;
> +
> +	if (list_isempty(list)) {
> +		list->head = list->tail = item;
> +		return true;
> +	}
> +
> +	list->tail->next = item;
> +	list->tail = item;
> +
> +	return true;
> +}
> +
> +static void list_free(struct list_ptrs *list, bt_gatt_destroy_func_t destroy)
> +{
> +	struct bt_gatt_list *l, *tmp;
> +	l = list->head;
> +
> +	while (l) {
> +		if (destroy)
> +			destroy(l->data);
> +
> +		tmp = l->next;
> +		free(l);
> +		l = tmp;
> +	}
> +}
> +
> +static const uint8_t bt_base_uuid[16] = {
> +	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00,
> +	0x80, 0x00, 0x00, 0x80, 0x5F, 0x9B, 0x34, 0xFB
> +};
> +
> struct mtu_op {
> 	struct bt_att *att;
> 	uint16_t client_rx_mtu;
> @@ -122,14 +186,199 @@ bool bt_gatt_exchange_mtu(struct bt_att *att, uint16_t client_rx_mtu,
> 	return true;
> }
> 
> +struct discovery_op {
> +	struct bt_att *att;
> +	int ref_count;
> +	bt_uuid_t uuid;
> +	struct list_ptrs results;
> +	bt_gatt_discovery_callback_t callback;
> +	void *user_data;
> +	bt_gatt_destroy_func_t destroy;
> +};
> +
> +static struct discovery_op* discovery_op_ref(struct discovery_op *op)
> +{
> +	__sync_fetch_and_add(&op->ref_count, 1);
> +
> +	return op;
> +}
> +
> +static void discovery_op_unref(void *data)
> +{
> +	struct discovery_op *op = data;
> +
> +	if (__sync_sub_and_fetch(&op->ref_count, 1))
> +		return;
> +
> +	if (op->destroy)
> +		op->destroy(op->user_data);
> +
> +	list_free(&op->results, free);
> +
> +	free(op);
> +}
> +
> +static void put_uuid_le(const bt_uuid_t *src, void *dst)
> +{
> +	if (src->type == BT_UUID16)
> +		put_le16(src->value.u16, dst);
> +	else
> +		bswap_128(&src->value.u128, dst);
> +}
> +
> +static bool convert_uuid_le(const uint8_t *src, size_t len, uint8_t dst[16])
> +{
> +	if (len == 16) {
> +		bswap_128(src, dst);
> +		return true;
> +	}
> +
> +	if (len != 2)
> +		return false;
> +
> +	memcpy(dst, bt_base_uuid, sizeof(bt_base_uuid));
> +	dst[2] = src[1];
> +	dst[3] = src[0];
> +
> +	return true;
> +}
> +
> +static void read_by_grp_type_cb(uint8_t opcode, const void *pdu,
> +					uint16_t length, void *user_data)
> +{
> +	struct discovery_op *op = user_data;
> +	bool success;
> +	uint8_t att_ecode = 0;
> +	struct bt_gatt_list *results = NULL;
> +	size_t data_length;
> +	size_t list_length;
> +	uint16_t last_end;
> +	int i;
> +
> +	if (opcode == BT_ATT_OP_ERROR_RSP) {
> +		success = false;
> +		att_ecode = process_error(pdu, length);
> +
> +		if (att_ecode == BT_ATT_ERROR_ATTRIBUTE_NOT_FOUND &&
> +						!list_isempty(&op->results))
> +			goto success;
> +
> +		goto done;
> +	}
> +
> +	/* PDU must contain at least the following (sans opcode):
> +	 * - Attr Data Length (1 octet)
> +	 * - Attr Data List (at least 6 octets):
> +	 *   -- 2 octets: Attribute handle
> +	 *   -- 2 octets: End group handle
> +	 *   -- 2 or 16 octets: service UUID
> +	 */
> +	if (opcode != BT_ATT_OP_READ_BY_GRP_TYPE_RSP || !pdu || length < 7) {
> +		success = false;
> +		goto done;
> +	}
> +
> +	data_length = ((uint8_t *) pdu)[0];
> +	list_length = length - 1;
> +
> +	if ((list_length % data_length) ||
> +				(data_length != 6 && data_length != 20)) {
> +		success = false;
> +		goto done;
> +	}
> +
> +	for (i = 1; i < length; i += data_length) {
> +		struct bt_gatt_service *service;
> +
> +		service = new0(struct bt_gatt_service, 1);
> +		if (!service) {
> +			success = false;
> +			goto done;
> +		}
> +
> +		service->start = get_le16(pdu + i);
> +		last_end = get_le16(pdu + i + 2);
> +		service->end = last_end;
> +		convert_uuid_le(pdu + i + 4, data_length - 4, service->uuid);
> +
> +		if (!list_add(&op->results, service)) {
> +			success = false;
> +			goto done;
> +		}
> +	}
> +
> +	if (last_end != 0xffff) {
> +		uint8_t pdu[6];
> +
> +		put_le16(last_end + 1, pdu);
> +		put_le16(0xffff, pdu + 2);
> +		put_le16(GATT_PRIM_SVC_UUID, pdu + 4);
> +
> +		if (bt_att_send(op->att, BT_ATT_OP_READ_BY_GRP_TYPE_REQ,
> +							pdu, sizeof(pdu),
> +							read_by_grp_type_cb,
> +							discovery_op_ref(op),
> +							discovery_op_unref))
> +			return;
> +
> +		discovery_op_unref(op);
> +		success = false;
> +		goto done;

I think we need to fix bt_att_send to return false when we got disconnected in between the individual commands. I did run some tests with the code here and if we receive a disconnect, we get stuck and the callback is never called. So when we have running longer procedures, we need to handle the cases where we loose the connection in the middle of it.

Regards

Marcel


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 03/11] shared/gatt: Implement "Discover All Primary Services" procedure.
  2014-07-26 11:02   ` Marcel Holtmann
@ 2014-07-26 20:03     ` Arman Uguray
  2014-07-26 20:54       ` Marcel Holtmann
  0 siblings, 1 reply; 20+ messages in thread
From: Arman Uguray @ 2014-07-26 20:03 UTC (permalink / raw
  To: Marcel Holtmann; +Cc: BlueZ development

Hi Marcel,

> I think we need to fix bt_att_send to return false when we got disconnect=
ed in between the individual commands. I did run some tests with the code h=
ere and if we receive a disconnect, we get stuck and the callback is never =
called. So when we have running longer procedures, we need to handle the ca=
ses where we loose the connection in the middle of it.
>

I'm actually aware of this issue and was trying to decide who is
responsible for handling disconnects. I'm guessing that bt_att can use
io_set_disconnect_handler to crate a handler for this case, in which I
guess the right thing to do is to mark the bt_att structure as
invalidated, cancel all pending ATT operations, and perhaps notify the
user with a special callback (similar to the timeout callback)? The
user will then have to create a new connection and pass the fd to
bt_att_new to obtain a new instance.

Does that make sense or is there a better place to handle disconnects
in general?

-Arman

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 03/11] shared/gatt: Implement "Discover All Primary Services" procedure.
  2014-07-26 20:03     ` Arman Uguray
@ 2014-07-26 20:54       ` Marcel Holtmann
  2014-07-26 22:10         ` Arman Uguray
  0 siblings, 1 reply; 20+ messages in thread
From: Marcel Holtmann @ 2014-07-26 20:54 UTC (permalink / raw
  To: Arman Uguray; +Cc: BlueZ development

Hi Arman,

>> I think we need to fix bt_att_send to return false when we got disconnected in between the individual commands. I did run some tests with the code here and if we receive a disconnect, we get stuck and the callback is never called. So when we have running longer procedures, we need to handle the cases where we loose the connection in the middle of it.
>> 
> 
> I'm actually aware of this issue and was trying to decide who is
> responsible for handling disconnects. I'm guessing that bt_att can use
> io_set_disconnect_handler to crate a handler for this case, in which I
> guess the right thing to do is to mark the bt_att structure as
> invalidated, cancel all pending ATT operations, and perhaps notify the
> user with a special callback (similar to the timeout callback)? The
> user will then have to create a new connection and pass the fd to
> bt_att_new to obtain a new instance.

I think for now we need to make sure the bt_att internally detects the disconnect and sets itself an internal state as disconnected. So besides calling any configured disconnect callback, we have to make sure that the transport is marked as invalid. And in case it is invalid all future bt_att_send should return false. That is just the basic and should make sure that the bt_gatt_* helpers will call their callback with a failed status.

That said, my general direction here is that I want to get to having bt_gatt as main functionality handling the details of everything GATT related.

	struct bt_gatt_server;

	struct bt_gatt_server *bt_gatt_server_new(struct bt_att *att, struct bt_gatt_db *db);

	struct bt_gatt_client;

	struct bt_gatt_client *bt_gatt_client_new(struct bt_att *att);

Both of these can use the same bt_att transport and will just carry out any functionality we need. The server should take care of needed notifications on new connections and the client should handle caching properly. Maybe the GATT helpers you are currently working will be converted to become an actual expose functionality of bt_gatt_client in the long run.

In addition to these two fine grained server and client control structs, I want the magic do everything for us and do it correct bt_gatt support. Meaning it will listen for incoming or auto-connect connections and also provided outgoing connections.

	struct bt_gatt;

	struct bt_gatt *bt_gatt_new(void);

But this is something we most likely should do last and focus on GATT client and server support first. If they can share the same bt_att and we have that working smoothly, the rest should fall into place all by itself.

On a side note, this is how I am currently restructuring SDP as well. The only difference is that SDP server and SDP client do not share a transport. You can only be one role. So that talks directly to a file descriptor instead of a transport abstraction like bt_att. Otherwise it is exactly the same.

This would mean that long term a GAP implementation just has to instantiate one bt_sdp and one bt_gatt and after that everything GAP related is taken care of.

If you want a view along the road, then the step after that is bt_gap that will include bt_sdp, bt_gatt and also handle mgmt. At that point we should be able to run the whole Bluetooth core functionality for BlueZ and also BlueZ for Android from exactly the same code base. The only difference will be then that Android only supports one controller and BlueZ will actually support many controllers. So the difference is the number of bt_gap instances.

Anyway, maybe this is a bit too much right now anyway. My point is that I do not have an exact map on how to get there. I just know where we need to get to. The details we have to figure out along way. That is also why I started testing your code with some of my peripherals that I have lying around. So it might be good that some point soon you start adding ATT and GATT unit tests. Preferable also some that actually match what PTS is testing. I think you can derive some inspiration from AVDTP, AVRCP or SDP test cases. I think ATT should be dead simple to test using a socketpair (with a special hack for the MTU exchange).

Regards

Marcel


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 03/11] shared/gatt: Implement "Discover All Primary Services" procedure.
  2014-07-26 20:54       ` Marcel Holtmann
@ 2014-07-26 22:10         ` Arman Uguray
  0 siblings, 0 replies; 20+ messages in thread
From: Arman Uguray @ 2014-07-26 22:10 UTC (permalink / raw
  To: Marcel Holtmann; +Cc: BlueZ development

Hi Marcel,

All of what you just mentioned is pretty much what I've had in mind,
so that's great. I'm currently only focusing on bt_gatt_client
initially and the helpers that I'm currently adding will be internally
used by it. I'm also planning to add a command-line tool and build PTS
style tests based on the helpers and bt_att directly.

My intention is that bt_gatt_client will handle all of the GATT
service discovery magic, as well as service changed events, caching,
etc. for us and provide simple API functions to enable/disable
notifications etc. Then, in the BlueZ daemon we can create a D-Bus API
layer and revise the plugins entirely to access all attributes through
the core bt_gatt_client instance.

So yeah, I think we're thinking in the same direction here :)

Cheers,
Arman

On Sat, Jul 26, 2014 at 1:54 PM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> Hi Arman,
>
>>> I think we need to fix bt_att_send to return false when we got disconne=
cted in between the individual commands. I did run some tests with the code=
 here and if we receive a disconnect, we get stuck and the callback is neve=
r called. So when we have running longer procedures, we need to handle the =
cases where we loose the connection in the middle of it.
>>>
>>
>> I'm actually aware of this issue and was trying to decide who is
>> responsible for handling disconnects. I'm guessing that bt_att can use
>> io_set_disconnect_handler to crate a handler for this case, in which I
>> guess the right thing to do is to mark the bt_att structure as
>> invalidated, cancel all pending ATT operations, and perhaps notify the
>> user with a special callback (similar to the timeout callback)? The
>> user will then have to create a new connection and pass the fd to
>> bt_att_new to obtain a new instance.
>
> I think for now we need to make sure the bt_att internally detects the di=
sconnect and sets itself an internal state as disconnected. So besides call=
ing any configured disconnect callback, we have to make sure that the trans=
port is marked as invalid. And in case it is invalid all future bt_att_send=
 should return false. That is just the basic and should make sure that the =
bt_gatt_* helpers will call their callback with a failed status.
>
> That said, my general direction here is that I want to get to having bt_g=
att as main functionality handling the details of everything GATT related.
>
>         struct bt_gatt_server;
>
>         struct bt_gatt_server *bt_gatt_server_new(struct bt_att *att, str=
uct bt_gatt_db *db);
>
>         struct bt_gatt_client;
>
>         struct bt_gatt_client *bt_gatt_client_new(struct bt_att *att);
>
> Both of these can use the same bt_att transport and will just carry out a=
ny functionality we need. The server should take care of needed notificatio=
ns on new connections and the client should handle caching properly. Maybe =
the GATT helpers you are currently working will be converted to become an a=
ctual expose functionality of bt_gatt_client in the long run.
>
> In addition to these two fine grained server and client control structs, =
I want the magic do everything for us and do it correct bt_gatt support. Me=
aning it will listen for incoming or auto-connect connections and also prov=
ided outgoing connections.
>
>         struct bt_gatt;
>
>         struct bt_gatt *bt_gatt_new(void);
>
> But this is something we most likely should do last and focus on GATT cli=
ent and server support first. If they can share the same bt_att and we have=
 that working smoothly, the rest should fall into place all by itself.
>
> On a side note, this is how I am currently restructuring SDP as well. The=
 only difference is that SDP server and SDP client do not share a transport=
. You can only be one role. So that talks directly to a file descriptor ins=
tead of a transport abstraction like bt_att. Otherwise it is exactly the sa=
me.
>
> This would mean that long term a GAP implementation just has to instantia=
te one bt_sdp and one bt_gatt and after that everything GAP related is take=
n care of.
>
> If you want a view along the road, then the step after that is bt_gap tha=
t will include bt_sdp, bt_gatt and also handle mgmt. At that point we shoul=
d be able to run the whole Bluetooth core functionality for BlueZ and also =
BlueZ for Android from exactly the same code base. The only difference will=
 be then that Android only supports one controller and BlueZ will actually =
support many controllers. So the difference is the number of bt_gap instanc=
es.
>
> Anyway, maybe this is a bit too much right now anyway. My point is that I=
 do not have an exact map on how to get there. I just know where we need to=
 get to. The details we have to figure out along way. That is also why I st=
arted testing your code with some of my peripherals that I have lying aroun=
d. So it might be good that some point soon you start adding ATT and GATT u=
nit tests. Preferable also some that actually match what PTS is testing. I =
think you can derive some inspiration from AVDTP, AVRCP or SDP test cases. =
I think ATT should be dead simple to test using a socketpair (with a specia=
l hack for the MTU exchange).
>
> Regards
>
> Marcel
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2014-07-26 22:10 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-25 22:08 [PATCH v2 00/11] Add shared/gatt-helpers Arman Uguray
2014-07-25 22:08 ` [PATCH v2 01/11] shared/gatt: Introduce gatt-helpers.h skeleton Arman Uguray
2014-07-25 22:47   ` Marcel Holtmann
2014-07-26  0:01     ` Arman Uguray
2014-07-26  1:07       ` Arman Uguray
2014-07-26  3:35       ` Marcel Holtmann
2014-07-25 22:08 ` [PATCH v2 02/11] shared/gatt: Implement bt_gatt_exchange_mtu Arman Uguray
2014-07-25 22:08 ` [PATCH v2 03/11] shared/gatt: Implement "Discover All Primary Services" procedure Arman Uguray
2014-07-26 11:02   ` Marcel Holtmann
2014-07-26 20:03     ` Arman Uguray
2014-07-26 20:54       ` Marcel Holtmann
2014-07-26 22:10         ` Arman Uguray
2014-07-25 22:08 ` [PATCH v2 04/11] shared/gatt: Implement "Discover Primary Service by UUID" procedure Arman Uguray
2014-07-25 22:08 ` [PATCH v2 05/11] shared/gatt: Implement "Characteristic Discovery" procedures Arman Uguray
2014-07-25 22:08 ` [PATCH v2 06/11] shared/gatt: Implement "Descriptor Discovery" procedure Arman Uguray
2014-07-25 22:08 ` [PATCH v2 07/11] shared/gatt: Implement "Read" procedure Arman Uguray
2014-07-25 22:08 ` [PATCH v2 08/11] shared/gatt: Implement "Read Long Characteristic Values" procedure Arman Uguray
2014-07-25 22:08 ` [PATCH v2 09/11] shared/gatt: Implement "Write Value" and "Write Without Response" procedures Arman Uguray
2014-07-25 22:08 ` [PATCH v2 10/11] shared/gatt: Implement "Write Long Values" procedure Arman Uguray
2014-07-25 22:08 ` [PATCH v2 11/11] shared/gatt: Implement notification/indication helper 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.