All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH BlueZ v1 00/14] Rewrite local GATT server using shared/gatt
@ 2015-02-12  3:17 Arman Uguray
  2015-02-12  3:17 ` [PATCH BlueZ v1 01/14] shared/att: Add bt_att_get_fd Arman Uguray
                   ` (14 more replies)
  0 siblings, 15 replies; 22+ messages in thread
From: Arman Uguray @ 2015-02-12  3:17 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: luiz.dentz, Arman Uguray

*v1: Addressed comments by jamuraa and vudentz:
  - Now passing bt_att instead of bdaddr_t in gatt_db callbacks and functions.
    I have not run the tests on the android side here, so I would appreciate it
    if you can run them.
  - Renamed src/gatt-server to src/gatt-database for now. Added TODO item for
    refactoring this later.
  - Updated the TODO items for GATT.

This patch set includes patches that rewrite the local GATT server using
shared/gatt. This in effect invalidates the existing src/attrib-server in
favor of a new src/gatt-server.

Arman Uguray (14):
  CHROMIUM: shared/att: Add bt_att_get_fd
  CHROMIUM: shared/gatt: Pass bt_att instead of bdaddr_t
  CHROMIUM: core: Introduce btd_gatt_database
  CHROMIUM: core: Attach gatt-server to bt_att
  CHROMIUM: core: gatt: Add GATT/GAP services to local db
  CHROMIUM: core: Add GATT UUIDs to Adapter1.UUIDs
  CHROMIUM: core: Support per-client CCC state
  CHROMIUM: core: Setup added/removed handlers in GATT database
  CHROMIUM: core: Add Service Changed characteristic
  CHROMIUM: core: device: Add getter for GATT server
  CHROMIUM: core: gatt-server: Send "Service Changed"
  CHROMIUM: core: adapter: Send UUIDs changed for GATT services
  CHROMIUM: shared/gatt: Don't incorrectly terminate discovery
  TODO: Update GATT items.

 Makefile.am               |   1 +
 TODO                      |  54 ++--
 android/gatt.c            |  98 ++++--
 src/adapter.c             |  59 +++-
 src/device.c              |  52 +++-
 src/device.h              |   1 +
 src/gatt-client.c         |   1 +
 src/gatt-database.c       | 766 ++++++++++++++++++++++++++++++++++++++++++++++
 src/gatt-database.h       |  29 ++
 src/main.c                |   3 +
 src/shared/att.c          |   8 +
 src/shared/att.h          |   2 +
 src/shared/gatt-client.c  |   6 +-
 src/shared/gatt-db.c      |  10 +-
 src/shared/gatt-db.h      |   8 +-
 src/shared/gatt-helpers.c |   3 +-
 src/shared/gatt-server.c  |  32 +-
 tools/btgatt-server.c     |  18 +-
 unit/test-gatt.c          |   5 +-
 19 files changed, 1052 insertions(+), 104 deletions(-)
 create mode 100644 src/gatt-database.c
 create mode 100644 src/gatt-database.h

-- 
2.2.0.rc0.207.ga3a616c


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

* [PATCH BlueZ v1 01/14] shared/att: Add bt_att_get_fd
  2015-02-12  3:17 [PATCH BlueZ v1 00/14] Rewrite local GATT server using shared/gatt Arman Uguray
@ 2015-02-12  3:17 ` Arman Uguray
  2015-02-12 13:40   ` Luiz Augusto von Dentz
  2015-02-12  3:17 ` [PATCH BlueZ v1 02/14] shared/gatt: Pass bt_att instead of bdaddr_t Arman Uguray
                   ` (13 subsequent siblings)
  14 siblings, 1 reply; 22+ messages in thread
From: Arman Uguray @ 2015-02-12  3:17 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: luiz.dentz, Arman Uguray

Added the bt_att_get_fd function which returns the underlying file
descriptor of a bt_att.
---
 src/shared/att.c          | 8 ++++++++
 src/shared/att.h          | 2 ++
 src/shared/gatt-client.c  | 3 ++-
 src/shared/gatt-helpers.c | 3 ++-
 4 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/src/shared/att.c b/src/shared/att.c
index a98909e..8bf1bab 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -889,6 +889,14 @@ bool bt_att_set_close_on_unref(struct bt_att *att, bool do_close)
 	return io_set_close_on_destroy(att->io, do_close);
 }
 
+int bt_att_get_fd(struct bt_att *att)
+{
+	if (!att)
+		return -1;
+
+	return att->fd;
+}
+
 bool bt_att_set_debug(struct bt_att *att, bt_att_debug_func_t callback,
 				void *user_data, bt_att_destroy_func_t destroy)
 {
diff --git a/src/shared/att.h b/src/shared/att.h
index cd00a1e..db423fe 100644
--- a/src/shared/att.h
+++ b/src/shared/att.h
@@ -35,6 +35,8 @@ void bt_att_unref(struct bt_att *att);
 
 bool bt_att_set_close_on_unref(struct bt_att *att, bool do_close);
 
+int bt_att_get_fd(struct bt_att *att);
+
 typedef void (*bt_att_response_func_t)(uint8_t opcode, const void *pdu,
 					uint16_t length, void *user_data);
 typedef void (*bt_att_notify_func_t)(uint8_t opcode, const void *pdu,
diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index bfb9427..d5a277b 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -21,8 +21,9 @@
  *
  */
 
-#include "src/shared/att.h"
 #include "lib/uuid.h"
+#include "lib/bluetooth.h"
+#include "src/shared/att.h"
 #include "src/shared/gatt-helpers.h"
 #include "src/shared/util.h"
 #include "src/shared/queue.h"
diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index a33f960..b469116 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -26,9 +26,10 @@
 #include <config.h>
 #endif
 
+#include "lib/uuid.h"
+#include "lib/bluetooth.h"
 #include "src/shared/queue.h"
 #include "src/shared/att.h"
-#include "lib/uuid.h"
 #include "src/shared/gatt-helpers.h"
 #include "src/shared/util.h"
 
-- 
2.2.0.rc0.207.ga3a616c


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

* [PATCH BlueZ v1 02/14] shared/gatt: Pass bt_att instead of bdaddr_t
  2015-02-12  3:17 [PATCH BlueZ v1 00/14] Rewrite local GATT server using shared/gatt Arman Uguray
  2015-02-12  3:17 ` [PATCH BlueZ v1 01/14] shared/att: Add bt_att_get_fd Arman Uguray
@ 2015-02-12  3:17 ` Arman Uguray
  2015-02-12  3:17 ` [PATCH BlueZ v1 03/14] core: Introduce btd_gatt_database Arman Uguray
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 22+ messages in thread
From: Arman Uguray @ 2015-02-12  3:17 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: luiz.dentz, Arman Uguray

Replaced the bdaddr_t parameter of gatt_db's read/write functions
and callbacks with a bt_att parameter. The BDADDR information can
be obtained from the bt_att's underlying file descriptor.
---
 android/gatt.c           | 98 ++++++++++++++++++++++++++++++++++++------------
 src/gatt-client.c        |  1 +
 src/shared/gatt-db.c     | 10 ++---
 src/shared/gatt-db.h     |  8 ++--
 src/shared/gatt-server.c | 32 +++++++++-------
 tools/btgatt-server.c    | 18 ++++-----
 unit/test-gatt.c         |  5 ++-
 7 files changed, 113 insertions(+), 59 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index 8e58e41..fd67b22 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -44,6 +44,7 @@
 #include "utils.h"
 #include "src/shared/util.h"
 #include "src/shared/queue.h"
+#include "src/shared/att.h"
 #include "src/shared/gatt-db.h"
 #include "attrib/gattrib.h"
 #include "attrib/att.h"
@@ -4849,6 +4850,7 @@ static void read_requested_attributes(void *data, void *user_data)
 {
 	struct pending_request *resp_data = data;
 	struct request_processing_data *process_data = user_data;
+	struct bt_att *att = g_attrib_get_att(process_data->device->attrib);
 	struct gatt_db_attribute *attrib;
 	uint32_t permissions;
 	uint8_t error;
@@ -4879,8 +4881,7 @@ static void read_requested_attributes(void *data, void *user_data)
 	}
 
 	gatt_db_attribute_read(attrib, resp_data->offset, process_data->opcode,
-						&process_data->device->bdaddr,
-						attribute_read_cb, resp_data);
+					att, attribute_read_cb, resp_data);
 }
 
 static void process_dev_pending_requests(struct gatt_device *device,
@@ -4926,8 +4927,29 @@ static struct pending_trans_data *conn_add_transact(struct app_connection *conn,
 	return transaction;
 }
 
+static bool get_dst_addr(struct bt_att *att, bdaddr_t *dst)
+{
+	GIOChannel *io = NULL;
+	GError *gerr = NULL;
+
+	io = g_io_channel_unix_new(bt_att_get_fd(att));
+	if (!io)
+		return false;
+
+	bt_io_get(io, &gerr, BT_IO_OPT_DEST_BDADDR, dst, BT_IO_OPT_INVALID);
+	if (gerr) {
+		error("gatt: bt_io_get: %s", gerr->message);
+		g_error_free(gerr);
+		g_io_channel_unref(io);
+		return false;
+	}
+
+	g_io_channel_unref(io);
+	return true;
+}
+
 static void read_cb(struct gatt_db_attribute *attrib, unsigned int id,
-			uint16_t offset, uint8_t opcode, bdaddr_t *bdaddr,
+			uint16_t offset, uint8_t opcode, struct bt_att *att,
 			void *user_data)
 {
 	struct pending_trans_data *transaction;
@@ -4935,6 +4957,7 @@ static void read_cb(struct gatt_db_attribute *attrib, unsigned int id,
 	struct gatt_app *app;
 	struct app_connection *conn;
 	int32_t app_id = PTR_TO_INT(user_data);
+	bdaddr_t bdaddr;
 
 	DBG("id %u", id);
 
@@ -4944,7 +4967,12 @@ static void read_cb(struct gatt_db_attribute *attrib, unsigned int id,
 		goto failed;
 	}
 
-	conn = find_conn(bdaddr, app->id);
+	if (!get_dst_addr(att, &bdaddr)) {
+		error("gatt: read_cb, could not obtain dst BDADDR");
+		goto failed;
+	}
+
+	conn = find_conn(&bdaddr, app->id);
 	if (!conn) {
 		error("gatt: read_cb, cound not found connection");
 		goto failed;
@@ -4957,7 +4985,7 @@ static void read_cb(struct gatt_db_attribute *attrib, unsigned int id,
 	if (!transaction)
 		goto failed;
 
-	bdaddr2android(bdaddr, ev.bdaddr);
+	bdaddr2android(&bdaddr, ev.bdaddr);
 	ev.conn_id = conn->id;
 	ev.attr_handle = gatt_db_attribute_get_handle(attrib);
 	ev.offset = offset;
@@ -4976,7 +5004,7 @@ failed:
 
 static void write_cb(struct gatt_db_attribute *attrib, unsigned int id,
 			uint16_t offset, const uint8_t *value, size_t len,
-			uint8_t opcode, bdaddr_t *bdaddr, void *user_data)
+			uint8_t opcode, struct bt_att *att, void *user_data)
 {
 	uint8_t buf[IPC_MTU];
 	struct hal_ev_gatt_server_request_write *ev = (void *) buf;
@@ -4984,6 +5012,7 @@ static void write_cb(struct gatt_db_attribute *attrib, unsigned int id,
 	struct gatt_app *app;
 	int32_t app_id = PTR_TO_INT(user_data);
 	struct app_connection *conn;
+	bdaddr_t bdaddr;
 
 	DBG("id %u", id);
 
@@ -4993,7 +5022,12 @@ static void write_cb(struct gatt_db_attribute *attrib, unsigned int id,
 		goto failed;
 	}
 
-	conn = find_conn(bdaddr, app->id);
+	if (!get_dst_addr(att, &bdaddr)) {
+		error("gatt: write_cb, could not obtain dst BDADDR");
+		goto failed;
+	}
+
+	conn = find_conn(&bdaddr, app->id);
 	if (!conn) {
 		error("gatt: write_cb could not found connection");
 		goto failed;
@@ -5013,7 +5047,7 @@ static void write_cb(struct gatt_db_attribute *attrib, unsigned int id,
 
 	memset(ev, 0, sizeof(*ev));
 
-	bdaddr2android(bdaddr, &ev->bdaddr);
+	bdaddr2android(&bdaddr, &ev->bdaddr);
 	ev->attr_handle = gatt_db_attribute_get_handle(attrib);
 	ev->offset = offset;
 
@@ -6405,8 +6439,9 @@ static void write_cmd_request(const uint8_t *cmd, uint16_t cmd_len,
 	if (check_device_permissions(dev, cmd[0], permissions))
 		return;
 
-	gatt_db_attribute_write(attrib, 0, value, vlen, cmd[0], &dev->bdaddr,
-							write_confirm, NULL);
+	gatt_db_attribute_write(attrib, 0, value, vlen, cmd[0],
+						g_attrib_get_att(dev->attrib),
+						write_confirm, NULL);
 }
 
 static void write_signed_cmd_request(const uint8_t *cmd, uint16_t cmd_len,
@@ -6478,7 +6513,8 @@ static void write_signed_cmd_request(const uint8_t *cmd, uint16_t cmd_len,
 		/* Signature OK, proceed with write */
 		bt_update_sign_counter(&dev->bdaddr, REMOTE_CSRK, r_sign_cnt);
 		gatt_db_attribute_write(attrib, 0, value, vlen, cmd[0],
-					&dev->bdaddr, write_confirm, NULL);
+						g_attrib_get_att(dev->attrib),
+						write_confirm, NULL);
 	}
 }
 
@@ -6537,8 +6573,8 @@ static uint8_t write_req_request(const uint8_t *cmd, uint16_t cmd_len,
 	}
 
 	if (!gatt_db_attribute_write(attrib, 0, value, vlen, cmd[0],
-					&dev->bdaddr, attribute_write_cb,
-					data)) {
+						g_attrib_get_att(dev->attrib),
+						attribute_write_cb, data)) {
 		queue_remove(dev->pending_requests, data);
 		free(data);
 		return ATT_ECODE_UNLIKELY;
@@ -6596,8 +6632,8 @@ static uint8_t write_prep_request(const uint8_t *cmd, uint16_t cmd_len,
 	data->length = vlen;
 
 	if (!gatt_db_attribute_write(attrib, offset, value, vlen, cmd[0],
-					&dev->bdaddr, attribute_write_cb,
-					data)) {
+						g_attrib_get_att(dev->attrib),
+						attribute_write_cb, data)) {
 		queue_remove(dev->pending_requests, data);
 		g_free(data->value);
 		free(data);
@@ -6817,7 +6853,7 @@ static struct gap_srvc_handles gap_srvc_data;
 
 static void device_name_read_cb(struct gatt_db_attribute *attrib,
 					unsigned int id, uint16_t offset,
-					uint8_t opcode, bdaddr_t *bdaddr,
+					uint8_t opcode, struct bt_att *att,
 					void *user_data)
 {
 	const char *name = bt_get_adapter_name();
@@ -6893,7 +6929,7 @@ static void register_gap_service(void)
 
 static void device_info_read_cb(struct gatt_db_attribute *attrib,
 					unsigned int id, uint16_t offset,
-					uint8_t opcode, bdaddr_t *bdaddr,
+					uint8_t opcode, struct bt_att *att,
 					void *user_data)
 {
 	char *buf = user_data;
@@ -6903,7 +6939,7 @@ static void device_info_read_cb(struct gatt_db_attribute *attrib,
 
 static void device_info_read_system_id_cb(struct gatt_db_attribute *attrib,
 					unsigned int id, uint16_t offset,
-					uint8_t opcode, bdaddr_t *bdaddr,
+					uint8_t opcode, struct bt_att *att,
 					void *user_data)
 {
 	uint8_t pdu[8];
@@ -6915,7 +6951,7 @@ static void device_info_read_system_id_cb(struct gatt_db_attribute *attrib,
 
 static void device_info_read_pnp_id_cb(struct gatt_db_attribute *attrib,
 					unsigned int id, uint16_t offset,
-					uint8_t opcode, bdaddr_t *bdaddr,
+					uint8_t opcode, struct bt_att *att,
 					void *user_data)
 {
 	uint8_t pdu[7];
@@ -7029,18 +7065,24 @@ static void register_device_info_service(void)
 static void gatt_srvc_change_write_cb(struct gatt_db_attribute *attrib,
 					unsigned int id, uint16_t offset,
 					const uint8_t *value, size_t len,
-					uint8_t opcode, bdaddr_t *bdaddr,
+					uint8_t opcode, struct bt_att *att,
 					void *user_data)
 {
 	struct gatt_device *dev;
+	bdaddr_t bdaddr;
 
-	dev = find_device_by_addr(bdaddr);
+	if (!get_dst_addr(att, &bdaddr)) {
+		error("gatt: srvc_change_write_cb, could not obtain BDADDR");
+		return;
+	}
+
+	dev = find_device_by_addr(&bdaddr);
 	if (!dev) {
 		error("gatt: Could not find device ?!");
 		return;
 	}
 
-	if (!bt_device_is_bonded(bdaddr)) {
+	if (!bt_device_is_bonded(&bdaddr)) {
 		gatt_db_attribute_write_result(attrib, id,
 						ATT_ECODE_AUTHORIZATION);
 		return;
@@ -7054,20 +7096,26 @@ static void gatt_srvc_change_write_cb(struct gatt_db_attribute *attrib,
 	}
 
 	/* Set services changed indication value */
-	bt_store_gatt_ccc(bdaddr, get_le16(value));
+	bt_store_gatt_ccc(&bdaddr, get_le16(value));
 
 	gatt_db_attribute_write_result(attrib, id, 0);
 }
 
 static void gatt_srvc_change_read_cb(struct gatt_db_attribute *attrib,
 					unsigned int id, uint16_t offset,
-					uint8_t opcode, bdaddr_t *bdaddr,
+					uint8_t opcode, struct bt_att *att,
 					void *user_data)
 {
 	struct gatt_device *dev;
 	uint8_t pdu[2];
+	bdaddr_t bdaddr;
 
-	dev = find_device_by_addr(bdaddr);
+	if (!get_dst_addr(att, &bdaddr)) {
+		error("gatt: srvc_change_read_cb, could not obtain BDADDR");
+		return;
+	}
+
+	dev = find_device_by_addr(&bdaddr);
 	if (!dev) {
 		error("gatt: Could not find device ?!");
 		return;
diff --git a/src/gatt-client.c b/src/gatt-client.c
index 9811bd8..06988c6 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -34,6 +34,7 @@
 #include "adapter.h"
 #include "device.h"
 #include "lib/uuid.h"
+#include "lib/bluetooth.h"
 #include "src/shared/queue.h"
 #include "src/shared/att.h"
 #include "src/shared/gatt-db.h"
diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
index f72d58e..42ce5c2 100644
--- a/src/shared/gatt-db.c
+++ b/src/shared/gatt-db.c
@@ -28,8 +28,8 @@
 #include "src/shared/util.h"
 #include "src/shared/queue.h"
 #include "src/shared/timeout.h"
+#include "src/shared/att.h"
 #include "src/shared/gatt-db.h"
-#include "src/shared/att-types.h"
 
 #ifndef MAX
 #define MAX(a, b) ((a) > (b) ? (a) : (b))
@@ -1427,7 +1427,7 @@ static bool read_timeout(void *user_data)
 }
 
 bool gatt_db_attribute_read(struct gatt_db_attribute *attrib, uint16_t offset,
-				uint8_t opcode, bdaddr_t *bdaddr,
+				uint8_t opcode, struct bt_att *att,
 				gatt_db_attribute_read_t func, void *user_data)
 {
 	uint8_t *value;
@@ -1451,7 +1451,7 @@ bool gatt_db_attribute_read(struct gatt_db_attribute *attrib, uint16_t offset,
 
 		queue_push_tail(attrib->pending_reads, p);
 
-		attrib->read_func(attrib, p->id, offset, opcode, bdaddr,
+		attrib->read_func(attrib, p->id, offset, opcode, att,
 							attrib->user_data);
 		return true;
 	}
@@ -1522,7 +1522,7 @@ static bool write_timeout(void *user_data)
 
 bool gatt_db_attribute_write(struct gatt_db_attribute *attrib, uint16_t offset,
 					const uint8_t *value, size_t len,
-					uint8_t opcode, bdaddr_t *bdaddr,
+					uint8_t opcode, struct bt_att *att,
 					gatt_db_attribute_write_t func,
 					void *user_data)
 {
@@ -1546,7 +1546,7 @@ bool gatt_db_attribute_write(struct gatt_db_attribute *attrib, uint16_t offset,
 		queue_push_tail(attrib->pending_writes, p);
 
 		attrib->write_func(attrib, p->id, offset, value, len, opcode,
-						bdaddr, attrib->user_data);
+							att, attrib->user_data);
 		return true;
 	}
 
diff --git a/src/shared/gatt-db.h b/src/shared/gatt-db.h
index 37df4d5..75f0668 100644
--- a/src/shared/gatt-db.h
+++ b/src/shared/gatt-db.h
@@ -50,13 +50,13 @@ struct gatt_db_attribute *gatt_db_insert_service(struct gatt_db *db,
 
 typedef void (*gatt_db_read_t) (struct gatt_db_attribute *attrib,
 					unsigned int id, uint16_t offset,
-					uint8_t opcode, bdaddr_t *bdaddr,
+					uint8_t opcode, struct bt_att *att,
 					void *user_data);
 
 typedef void (*gatt_db_write_t) (struct gatt_db_attribute *attrib,
 					unsigned int id, uint16_t offset,
 					const uint8_t *value, size_t len,
-					uint8_t opcode, bdaddr_t *bdaddr,
+					uint8_t opcode, struct bt_att *att,
 					void *user_data);
 
 struct gatt_db_attribute *
@@ -196,7 +196,7 @@ typedef void (*gatt_db_attribute_read_t) (struct gatt_db_attribute *attrib,
 						size_t length, void *user_data);
 
 bool gatt_db_attribute_read(struct gatt_db_attribute *attrib, uint16_t offset,
-				uint8_t opcode, bdaddr_t *bdaddr,
+				uint8_t opcode, struct bt_att *att,
 				gatt_db_attribute_read_t func, void *user_data);
 
 bool gatt_db_attribute_read_result(struct gatt_db_attribute *attrib,
@@ -208,7 +208,7 @@ typedef void (*gatt_db_attribute_write_t) (struct gatt_db_attribute *attrib,
 
 bool gatt_db_attribute_write(struct gatt_db_attribute *attrib, uint16_t offset,
 					const uint8_t *value, size_t len,
-					uint8_t opcode, bdaddr_t *bdaddr,
+					uint8_t opcode, struct bt_att *att,
 					gatt_db_attribute_write_t func,
 					void *user_data);
 
diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
index 8f7b5cd..1415b27 100644
--- a/src/shared/gatt-server.c
+++ b/src/shared/gatt-server.c
@@ -24,8 +24,9 @@
 #include <sys/uio.h>
 #include <errno.h>
 
-#include "src/shared/att.h"
 #include "lib/uuid.h"
+#include "lib/bluetooth.h"
+#include "src/shared/att.h"
 #include "src/shared/queue.h"
 #include "src/shared/gatt-db.h"
 #include "src/shared/gatt-server.h"
@@ -168,8 +169,9 @@ static void attribute_read_cb(struct gatt_db_attribute *attrib, int err,
 }
 
 static bool encode_read_by_grp_type_rsp(struct gatt_db *db, struct queue *q,
-						uint16_t mtu,
-						uint8_t *pdu, uint16_t *len)
+						struct bt_att *att,
+						uint16_t mtu, uint8_t *pdu,
+						uint16_t *len)
 {
 	int iter = 0;
 	uint16_t start_handle, end_handle;
@@ -190,7 +192,7 @@ static bool encode_read_by_grp_type_rsp(struct gatt_db *db, struct queue *q,
 		 */
 		if (!gatt_db_attribute_read(attrib, 0,
 						BT_ATT_OP_READ_BY_GRP_TYPE_REQ,
-						NULL, attribute_read_cb,
+						att, attribute_read_cb,
 						&value) || !value.iov_len)
 			return false;
 
@@ -291,8 +293,8 @@ static void read_by_grp_type_cb(uint8_t opcode, const void *pdu,
 		goto error;
 	}
 
-	if (!encode_read_by_grp_type_rsp(server->db, q, mtu, rsp_pdu,
-								&rsp_len)) {
+	if (!encode_read_by_grp_type_rsp(server->db, q, server->att, mtu,
+							rsp_pdu, &rsp_len)) {
 		ecode = BT_ATT_ERROR_UNLIKELY;
 		goto error;
 	}
@@ -406,8 +408,8 @@ static void process_read_by_type(struct async_read_op *op)
 		goto error;
 	}
 
-	if (gatt_db_attribute_read(attr, 0, op->opcode, NULL,
-				read_by_type_read_complete_cb, op))
+	if (gatt_db_attribute_read(attr, 0, op->opcode, server->att,
+					read_by_type_read_complete_cb, op))
 		return;
 
 	ecode = BT_ATT_ERROR_UNLIKELY;
@@ -813,7 +815,8 @@ static void write_cb(uint8_t opcode, const void *pdu,
 	server->pending_write_op = op;
 
 	if (gatt_db_attribute_write(attr, 0, pdu + 2, length - 2, opcode,
-						NULL, write_complete_cb, op))
+							server->att,
+							write_complete_cb, op))
 		return;
 
 	if (op)
@@ -925,7 +928,7 @@ static void handle_read_req(struct bt_gatt_server *server, uint8_t opcode,
 	op->server = server;
 	server->pending_read_op = op;
 
-	if (gatt_db_attribute_read(attr, offset, opcode, NULL,
+	if (gatt_db_attribute_read(attr, offset, opcode, server->att,
 							read_complete_cb, op))
 		return;
 
@@ -999,7 +1002,6 @@ static void read_multiple_complete_cb(struct gatt_db_attribute *attr, int err,
 	struct read_multiple_resp_data *data = user_data;
 	struct gatt_db_attribute *next_attr;
 	uint32_t perm;
-
 	uint16_t handle = gatt_db_attribute_get_handle(attr);
 
 	if (err != 0) {
@@ -1051,7 +1053,8 @@ static void read_multiple_complete_cb(struct gatt_db_attribute *attr, int err,
 		return;
 	}
 
-	if (!gatt_db_attribute_read(next_attr, 0, BT_ATT_OP_READ_MULT_REQ, NULL,
+	if (!gatt_db_attribute_read(next_attr, 0, BT_ATT_OP_READ_MULT_REQ,
+					data->server->att,
 					read_multiple_complete_cb, data)) {
 		bt_att_send_error_rsp(data->server->att,
 						BT_ATT_OP_READ_MULT_REQ,
@@ -1107,7 +1110,7 @@ static void read_multiple_cb(uint8_t opcode, const void *pdu,
 		goto error;
 	}
 
-	if (gatt_db_attribute_read(attr, 0, opcode, NULL,
+	if (gatt_db_attribute_read(attr, 0, opcode, server->att,
 					read_multiple_complete_cb, &data))
 		return;
 
@@ -1234,7 +1237,8 @@ static void exec_next_prep_write(struct bt_gatt_server *server,
 
 	status = gatt_db_attribute_write(attr, next->offset,
 						next->value, next->length,
-						BT_ATT_OP_EXEC_WRITE_REQ, NULL,
+						BT_ATT_OP_EXEC_WRITE_REQ,
+						server->att,
 						exec_write_complete_cb, server);
 
 	prep_write_data_destroy(next);
diff --git a/tools/btgatt-server.c b/tools/btgatt-server.c
index d27cf10..868fe32 100644
--- a/tools/btgatt-server.c
+++ b/tools/btgatt-server.c
@@ -125,7 +125,7 @@ static void gatt_debug_cb(const char *str, void *user_data)
 
 static void gap_device_name_read_cb(struct gatt_db_attribute *attrib,
 					unsigned int id, uint16_t offset,
-					uint8_t opcode, bdaddr_t *bdaddr,
+					uint8_t opcode, struct bt_att *att,
 					void *user_data)
 {
 	struct server *server = user_data;
@@ -152,7 +152,7 @@ done:
 static void gap_device_name_write_cb(struct gatt_db_attribute *attrib,
 					unsigned int id, uint16_t offset,
 					const uint8_t *value, size_t len,
-					uint8_t opcode, bdaddr_t *bdaddr,
+					uint8_t opcode, struct bt_att *att,
 					void *user_data)
 {
 	struct server *server = user_data;
@@ -196,7 +196,7 @@ done:
 
 static void gap_device_name_ext_prop_read_cb(struct gatt_db_attribute *attrib,
 					unsigned int id, uint16_t offset,
-					uint8_t opcode, bdaddr_t *bdaddr,
+					uint8_t opcode, struct bt_att *att,
 					void *user_data)
 {
 	uint8_t value[2];
@@ -211,7 +211,7 @@ static void gap_device_name_ext_prop_read_cb(struct gatt_db_attribute *attrib,
 
 static void gatt_service_changed_cb(struct gatt_db_attribute *attrib,
 					unsigned int id, uint16_t offset,
-					uint8_t opcode, bdaddr_t *bdaddr,
+					uint8_t opcode, struct bt_att *att,
 					void *user_data)
 {
 	PRLOG("Service Changed Read called\n");
@@ -221,7 +221,7 @@ static void gatt_service_changed_cb(struct gatt_db_attribute *attrib,
 
 static void gatt_svc_chngd_ccc_read_cb(struct gatt_db_attribute *attrib,
 					unsigned int id, uint16_t offset,
-					uint8_t opcode, bdaddr_t *bdaddr,
+					uint8_t opcode, struct bt_att *att,
 					void *user_data)
 {
 	struct server *server = user_data;
@@ -238,7 +238,7 @@ static void gatt_svc_chngd_ccc_read_cb(struct gatt_db_attribute *attrib,
 static void gatt_svc_chngd_ccc_write_cb(struct gatt_db_attribute *attrib,
 					unsigned int id, uint16_t offset,
 					const uint8_t *value, size_t len,
-					uint8_t opcode, bdaddr_t *bdaddr,
+					uint8_t opcode, struct bt_att *att,
 					void *user_data)
 {
 	struct server *server = user_data;
@@ -272,7 +272,7 @@ done:
 
 static void hr_msrmt_ccc_read_cb(struct gatt_db_attribute *attrib,
 					unsigned int id, uint16_t offset,
-					uint8_t opcode, bdaddr_t *bdaddr,
+					uint8_t opcode, struct bt_att *att,
 					void *user_data)
 {
 	struct server *server = user_data;
@@ -326,7 +326,7 @@ static void update_hr_msrmt_simulation(struct server *server)
 static void hr_msrmt_ccc_write_cb(struct gatt_db_attribute *attrib,
 					unsigned int id, uint16_t offset,
 					const uint8_t *value, size_t len,
-					uint8_t opcode, bdaddr_t *bdaddr,
+					uint8_t opcode, struct bt_att *att,
 					void *user_data)
 {
 	struct server *server = user_data;
@@ -366,7 +366,7 @@ done:
 static void hr_control_point_write_cb(struct gatt_db_attribute *attrib,
 					unsigned int id, uint16_t offset,
 					const uint8_t *value, size_t len,
-					uint8_t opcode, bdaddr_t *bdaddr,
+					uint8_t opcode, struct bt_att *att,
 					void *user_data)
 {
 	struct server *server = user_data;
diff --git a/unit/test-gatt.c b/unit/test-gatt.c
index 6c98cfa..fb0ddbc 100644
--- a/unit/test-gatt.c
+++ b/unit/test-gatt.c
@@ -36,6 +36,7 @@
 #include <glib.h>
 
 #include "lib/uuid.h"
+#include "lib/bluetooth.h"
 #include "src/shared/util.h"
 #include "src/shared/att.h"
 #include "src/shared/gatt-helpers.h"
@@ -762,8 +763,8 @@ static struct gatt_db_attribute *add_char_with_value(struct gatt_db *db,
 
 	g_assert(attrib != NULL);
 
-	gatt_db_attribute_write(attrib, 0, value, len, 0x00, NULL, att_write_cb,
-									NULL);
+	gatt_db_attribute_write(attrib, 0, value, len, 0x00, NULL,
+							att_write_cb, NULL);
 
 	return attrib;
 }
-- 
2.2.0.rc0.207.ga3a616c


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

* [PATCH BlueZ v1 03/14] core: Introduce btd_gatt_database
  2015-02-12  3:17 [PATCH BlueZ v1 00/14] Rewrite local GATT server using shared/gatt Arman Uguray
  2015-02-12  3:17 ` [PATCH BlueZ v1 01/14] shared/att: Add bt_att_get_fd Arman Uguray
  2015-02-12  3:17 ` [PATCH BlueZ v1 02/14] shared/gatt: Pass bt_att instead of bdaddr_t Arman Uguray
@ 2015-02-12  3:17 ` Arman Uguray
  2015-02-13 16:06   ` Luiz Augusto von Dentz
  2015-02-12  3:17 ` [PATCH BlueZ v1 04/14] core: Attach gatt-server to bt_att Arman Uguray
                   ` (11 subsequent siblings)
  14 siblings, 1 reply; 22+ messages in thread
From: Arman Uguray @ 2015-02-12  3:17 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: luiz.dentz, Arman Uguray

This patch introduces src/gatt-database.* which handles incoming ATT
connections, manages per-adapter shared/gatt-db instances, and routes
connections to the corresponding device object. This is the layer that
will perform all the CCC management and Service Changed handling.
---
 Makefile.am         |   1 +
 src/adapter.c       |   8 ++-
 src/gatt-database.c | 204 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/gatt-database.h |  26 +++++++
 src/main.c          |   3 +
 5 files changed, 240 insertions(+), 2 deletions(-)
 create mode 100644 src/gatt-database.c
 create mode 100644 src/gatt-database.h

diff --git a/Makefile.am b/Makefile.am
index 60811f1..4407355 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -167,6 +167,7 @@ src_bluetoothd_SOURCES = $(builtin_sources) \
 			src/sdpd-server.c src/sdpd-request.c \
 			src/sdpd-service.c src/sdpd-database.c \
 			src/attrib-server.h src/attrib-server.c \
+			src/gatt-database.h src/gatt-database.c \
 			src/sdp-xml.h src/sdp-xml.c \
 			src/sdp-client.h src/sdp-client.c \
 			src/textfile.h src/textfile.c \
diff --git a/src/adapter.c b/src/adapter.c
index 1839286..0253d08 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -68,6 +68,7 @@
 #include "attrib/att.h"
 #include "attrib/gatt.h"
 #include "attrib-server.h"
+#include "gatt-database.h"
 #include "eir.h"
 
 #define ADAPTER_INTERFACE	"org.bluez.Adapter1"
@@ -302,6 +303,7 @@ static void dev_class_changed_callback(uint16_t index, uint16_t length,
 	appearance[1] = rp->val[1] & 0x1f;	/* removes service class */
 	appearance[2] = rp->val[2];
 
+	/* TODO: Do this through btd_gatt_database instead */
 	attrib_gap_set(adapter, GATT_CHARAC_APPEARANCE, appearance, 2);
 }
 
@@ -4014,6 +4016,7 @@ static void convert_sdp_entry(char *key, char *value, void *user_data)
 	if (record_has_uuid(rec, att_uuid))
 		goto failed;
 
+	/* TODO: Do this through btd_gatt_database */
 	if (!gatt_parse_record(rec, &uuid, &psm, &start, &end))
 		goto failed;
 
@@ -4548,7 +4551,7 @@ static void adapter_remove(struct btd_adapter *adapter)
 	adapter->devices = NULL;
 
 	unload_drivers(adapter);
-	btd_adapter_gatt_server_stop(adapter);
+	btd_gatt_database_unregister_adapter(adapter);
 
 	g_slist_free(adapter->pin_callbacks);
 	adapter->pin_callbacks = NULL;
@@ -6590,7 +6593,8 @@ static int adapter_register(struct btd_adapter *adapter)
 		agent_unref(agent);
 	}
 
-	btd_adapter_gatt_server_start(adapter);
+	if (!btd_gatt_database_register_adapter(adapter))
+		error("Failed to register adapter with GATT server");
 
 	load_config(adapter);
 	fix_storage(adapter);
diff --git a/src/gatt-database.c b/src/gatt-database.c
new file mode 100644
index 0000000..57bdf1a
--- /dev/null
+++ b/src/gatt-database.c
@@ -0,0 +1,204 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2015  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.
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdint.h>
+#include <stdlib.h>
+
+#include "lib/uuid.h"
+#include "btio/btio.h"
+#include "src/shared/util.h"
+#include "src/shared/queue.h"
+#include "src/shared/att.h"
+#include "src/shared/gatt-db.h"
+#include "log.h"
+#include "adapter.h"
+#include "device.h"
+#include "gatt-database.h"
+
+#ifndef ATT_CID
+#define ATT_CID 4
+#endif
+
+static struct queue *servers = NULL;
+
+struct btd_gatt_database {
+	struct btd_adapter *adapter;
+	struct gatt_db *db;
+	GIOChannel *le_io;
+};
+
+static void gatt_server_free(void *data)
+{
+	struct btd_gatt_database *server = data;
+
+	if (server->le_io) {
+		g_io_channel_shutdown(server->le_io, FALSE, NULL);
+		g_io_channel_unref(server->le_io);
+	}
+
+	gatt_db_unref(server->db);
+	btd_adapter_unref(server->adapter);
+	free(server);
+}
+
+bool btd_gatt_database_init(void)
+{
+
+	info("Initializing GATT server");
+
+	servers = queue_new();
+	if (!servers) {
+		error("Failed to set up local GATT server");
+		return false;
+	}
+
+	return true;
+}
+
+void btd_gatt_database_cleanup(void)
+{
+	info("Cleaning up GATT server");
+
+	queue_destroy(servers, gatt_server_free);
+	servers = NULL;
+}
+
+static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
+{
+	struct btd_adapter *adapter;
+	struct btd_device *device;
+	uint8_t dst_type;
+	bdaddr_t src, dst;
+
+	DBG("New incoming LE ATT connection");
+
+	if (gerr) {
+		error("%s", gerr->message);
+		return;
+	}
+
+	bt_io_get(io, &gerr, BT_IO_OPT_SOURCE_BDADDR, &src,
+						BT_IO_OPT_DEST_BDADDR, &dst,
+						BT_IO_OPT_DEST_TYPE, &dst_type,
+						BT_IO_OPT_INVALID);
+	if (gerr) {
+		error("bt_io_get: %s", gerr->message);
+		g_error_free(gerr);
+		return;
+	}
+
+	adapter = adapter_find(&src);
+	if (!adapter)
+		return;
+
+	device = btd_adapter_get_device(adapter, &dst, dst_type);
+	if (!device)
+		return;
+
+	device_attach_att(device, io);
+}
+
+static bool match_adapter(const void *a, const void *b)
+{
+	const struct btd_gatt_database *server = a;
+	const struct btd_adapter *adapter = b;
+
+	return server->adapter == adapter;
+}
+
+bool btd_gatt_database_register_adapter(struct btd_adapter *adapter)
+{
+	struct btd_gatt_database *server;
+	GError *gerr = NULL;
+	const bdaddr_t *addr;
+
+	if (!adapter)
+		return false;
+
+	if (!servers) {
+		error("GATT server not initialized");
+		return false;
+	}
+
+	if (queue_find(servers, match_adapter, adapter)) {
+		error("Adapter already registered with GATT server");
+		return false;
+	}
+
+	server = new0(struct btd_gatt_database, 1);
+	if (!server)
+		return false;
+
+	server->adapter = btd_adapter_ref(adapter);
+	server->db = gatt_db_new();
+	if (!server->db)
+		goto fail;
+
+	addr = btd_adapter_get_address(adapter);
+	server->le_io = bt_io_listen(connect_cb, NULL, NULL, NULL, &gerr,
+					BT_IO_OPT_SOURCE_BDADDR, addr,
+					BT_IO_OPT_SOURCE_TYPE, BDADDR_LE_PUBLIC,
+					BT_IO_OPT_CID, ATT_CID,
+					BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
+					BT_IO_OPT_INVALID);
+	if (!server->le_io) {
+		error("Failed to start listening: %s", gerr->message);
+		g_error_free(gerr);
+		goto fail;
+	}
+
+	queue_push_tail(servers, server);
+
+	/* TODO: Set up GAP/GATT services */
+
+	return true;
+
+fail:
+	gatt_server_free(server);
+
+	return false;
+}
+
+void btd_gatt_database_unregister_adapter(struct btd_adapter *adapter)
+{
+	if (!adapter || !servers)
+		return;
+
+	queue_remove_all(servers, match_adapter, adapter, gatt_server_free);
+}
+
+struct gatt_db *btd_gatt_database_get_db(struct btd_adapter *adapter)
+{
+	struct btd_gatt_database *server;
+
+	if (!servers) {
+		error("GATT server not initialized");
+		return false;
+	}
+
+	server = queue_find(servers, match_adapter, adapter);
+	if (!server)
+		return false;
+
+	return server->db;
+}
diff --git a/src/gatt-database.h b/src/gatt-database.h
new file mode 100644
index 0000000..05e4ab9
--- /dev/null
+++ b/src/gatt-database.h
@@ -0,0 +1,26 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2015  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.
+ *
+ */
+
+bool btd_gatt_database_init(void);
+void btd_gatt_database_cleanup(void);
+
+bool btd_gatt_database_register_adapter(struct btd_adapter *adapter);
+void btd_gatt_database_unregister_adapter(struct btd_adapter *adapter);
+
+struct gatt_db *btd_gatt_database_get_db(struct btd_adapter *adapter);
diff --git a/src/main.c b/src/main.c
index 061060d..4ccc18f 100644
--- a/src/main.c
+++ b/src/main.c
@@ -56,6 +56,7 @@
 #include "agent.h"
 #include "profile.h"
 #include "gatt.h"
+#include "gatt-database.h"
 #include "systemd.h"
 
 #define BLUEZ_NAME "org.bluez"
@@ -579,6 +580,7 @@ int main(int argc, char *argv[])
 	g_dbus_set_flags(gdbus_flags);
 
 	gatt_init();
+	btd_gatt_database_init();
 
 	if (adapter_init() < 0) {
 		error("Adapter handling initialization failed");
@@ -642,6 +644,7 @@ int main(int argc, char *argv[])
 
 	adapter_cleanup();
 
+	btd_gatt_database_cleanup();
 	gatt_cleanup();
 
 	rfkill_exit();
-- 
2.2.0.rc0.207.ga3a616c


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

* [PATCH BlueZ v1 04/14] core: Attach gatt-server to bt_att
  2015-02-12  3:17 [PATCH BlueZ v1 00/14] Rewrite local GATT server using shared/gatt Arman Uguray
                   ` (2 preceding siblings ...)
  2015-02-12  3:17 ` [PATCH BlueZ v1 03/14] core: Introduce btd_gatt_database Arman Uguray
@ 2015-02-12  3:17 ` Arman Uguray
  2015-02-12  3:17 ` [PATCH BlueZ v1 05/14] core: gatt: Add GATT/GAP services to local db Arman Uguray
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 22+ messages in thread
From: Arman Uguray @ 2015-02-12  3:17 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: luiz.dentz, Arman Uguray

With this patch, btd_device now creates a bt_gatt_server and attaches
it to the ATT transport which coexists with a bt_gatt_client.
---
 src/device.c | 44 ++++++++++++++++++++++++++++----------------
 1 file changed, 28 insertions(+), 16 deletions(-)

diff --git a/src/device.c b/src/device.c
index a28d6fb..0af4d0f 100644
--- a/src/device.c
+++ b/src/device.c
@@ -51,11 +51,13 @@
 #include "src/shared/queue.h"
 #include "src/shared/gatt-db.h"
 #include "src/shared/gatt-client.h"
+#include "src/shared/gatt-server.h"
 #include "btio/btio.h"
 #include "lib/mgmt.h"
 #include "attrib/att.h"
 #include "hcid.h"
 #include "adapter.h"
+#include "gatt-database.h"
 #include "attrib/gattrib.h"
 #include "attio.h"
 #include "device.h"
@@ -210,7 +212,6 @@ struct btd_device {
 	GAttrib		*attrib;
 	GSList		*attios;
 	GSList		*attios_offline;
-	guint		attachid;		/* Attrib server attach */
 
 	struct bt_att *att;			/* The new ATT transport */
 	uint16_t att_mtu;			/* The ATT MTU */
@@ -223,6 +224,7 @@ struct btd_device {
 	 */
 	struct gatt_db *db;			/* GATT db cache */
 	struct bt_gatt_client *client;		/* GATT client instance */
+	struct bt_gatt_server *server;		/* GATT server instance */
 
 	struct btd_gatt_client *client_dbus;
 
@@ -515,6 +517,15 @@ static void gatt_client_cleanup(struct btd_device *device)
 		gatt_db_clear(device->db);
 }
 
+static void gatt_server_cleanup(struct btd_device *device)
+{
+	if (!device->server)
+		return;
+
+	bt_gatt_server_unref(device->server);
+	device->server = NULL;
+}
+
 static void attio_cleanup(struct btd_device *device)
 {
 	if (device->att_disconn_id)
@@ -528,6 +539,7 @@ static void attio_cleanup(struct btd_device *device)
 	}
 
 	gatt_client_cleanup(device);
+	gatt_server_cleanup(device);
 
 	if (device->att) {
 		bt_att_unref(device->att);
@@ -538,14 +550,6 @@ static void attio_cleanup(struct btd_device *device)
 		GAttrib *attrib = device->attrib;
 
 		device->attrib = NULL;
-
-		if (device->attachid) {
-			guint attachid = device->attachid;
-
-			device->attachid = 0;
-			attrib_channel_detach(attrib, attachid);
-		}
-
 		g_attrib_cancel_all(attrib);
 		g_attrib_unref(attrib);
 	}
@@ -3950,6 +3954,20 @@ static void gatt_client_init(struct btd_device *device)
 	}
 }
 
+static void gatt_server_init(struct btd_device *device, struct gatt_db *db)
+{
+	if (!db) {
+		error("No local GATT database exists for this adapter");
+		return;
+	}
+
+	gatt_server_cleanup(device);
+
+	device->server = bt_gatt_server_new(db, device->att, device->att_mtu);
+	if (!device->server)
+		error("Failed to initialize bt_gatt_server");
+}
+
 bool device_attach_att(struct btd_device *dev, GIOChannel *io)
 {
 	GError *gerr = NULL;
@@ -3989,13 +4007,6 @@ bool device_attach_att(struct btd_device *dev, GIOChannel *io)
 		return false;
 	}
 
-	dev->attachid = attrib_channel_attach(attrib);
-	if (dev->attachid == 0) {
-		g_attrib_unref(attrib);
-		error("Attribute server attach failure!");
-		return false;
-	}
-
 	dev->attrib = attrib;
 	dev->att = g_attrib_get_att(attrib);
 
@@ -4006,6 +4017,7 @@ bool device_attach_att(struct btd_device *dev, GIOChannel *io)
 	bt_att_set_close_on_unref(dev->att, true);
 
 	gatt_client_init(dev);
+	gatt_server_init(dev, btd_gatt_database_get_db(dev->adapter));
 
 	/*
 	 * Remove the device from the connect_list and give the passive
-- 
2.2.0.rc0.207.ga3a616c


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

* [PATCH BlueZ v1 05/14] core: gatt: Add GATT/GAP services to local db
  2015-02-12  3:17 [PATCH BlueZ v1 00/14] Rewrite local GATT server using shared/gatt Arman Uguray
                   ` (3 preceding siblings ...)
  2015-02-12  3:17 ` [PATCH BlueZ v1 04/14] core: Attach gatt-server to bt_att Arman Uguray
@ 2015-02-12  3:17 ` Arman Uguray
  2015-02-12  3:17 ` [PATCH BlueZ v1 06/14] core: Add GATT UUIDs to Adapter1.UUIDs Arman Uguray
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 22+ messages in thread
From: Arman Uguray @ 2015-02-12  3:17 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: luiz.dentz, Arman Uguray

This patch adds the GATT & GAP services to the local GATT database.
---
 src/gatt-database.c | 114 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 113 insertions(+), 1 deletion(-)

diff --git a/src/gatt-database.c b/src/gatt-database.c
index 57bdf1a..2da5a09 100644
--- a/src/gatt-database.c
+++ b/src/gatt-database.c
@@ -39,6 +39,9 @@
 #define ATT_CID 4
 #endif
 
+#define UUID_GAP	0x1800
+#define UUID_GATT	0x1801
+
 static struct queue *servers = NULL;
 
 struct btd_gatt_database {
@@ -126,6 +129,115 @@ static bool match_adapter(const void *a, const void *b)
 	return server->adapter == adapter;
 }
 
+static void gap_device_name_read_cb(struct gatt_db_attribute *attrib,
+					unsigned int id, uint16_t offset,
+					uint8_t opcode, struct bt_att *att,
+					void *user_data)
+{
+	struct btd_gatt_database *server = user_data;
+	uint8_t error = 0;
+	size_t len = 0;
+	const uint8_t *value = NULL;
+	const char *device_name;
+
+	DBG("GAP Device Name read request\n");
+
+	device_name = btd_adapter_get_name(server->adapter);
+	len = strlen(device_name);
+
+	if (offset > len) {
+		error = BT_ATT_ERROR_INVALID_OFFSET;
+		goto done;
+	}
+
+	len -= offset;
+	value = len ? (const uint8_t *) &device_name[offset] : NULL;
+
+done:
+	gatt_db_attribute_read_result(attrib, id, error, value, len);
+}
+
+static void gap_appearance_read_cb(struct gatt_db_attribute *attrib,
+					unsigned int id, uint16_t offset,
+					uint8_t opcode, struct bt_att *att,
+					void *user_data)
+{
+	struct btd_gatt_database *server = user_data;
+	uint8_t error = 0;
+	size_t len = 2;
+	const uint8_t *value = NULL;
+	uint8_t appearance[2];
+	uint32_t dev_class;
+
+	DBG("GAP Appearance read request\n");
+
+	dev_class = btd_adapter_get_class(server->adapter);
+
+	if (offset > 2) {
+		error = BT_ATT_ERROR_INVALID_OFFSET;
+		goto done;
+	}
+
+	appearance[0] = dev_class & 0x00ff;
+	appearance[1] = (dev_class >> 8) & 0x001f;
+
+	len -= offset;
+	value = len ? &appearance[offset] : NULL;
+
+done:
+	gatt_db_attribute_read_result(attrib, id, error, value, len);
+}
+
+static void populate_gap_service(struct btd_gatt_database *server)
+{
+	bt_uuid_t uuid;
+	struct gatt_db_attribute *service;
+
+	/* Add the GAP service */
+	bt_uuid16_create(&uuid, UUID_GAP);
+	service = gatt_db_add_service(server->db, &uuid, true, 5);
+
+	/*
+	 * Device Name characteristic.
+	 */
+	bt_uuid16_create(&uuid, GATT_CHARAC_DEVICE_NAME);
+	gatt_db_service_add_characteristic(service, &uuid, BT_ATT_PERM_READ,
+							BT_GATT_CHRC_PROP_READ,
+							gap_device_name_read_cb,
+							NULL, server);
+
+	/*
+	 * Device Appearance characteristic.
+	 */
+	bt_uuid16_create(&uuid, GATT_CHARAC_APPEARANCE);
+	gatt_db_service_add_characteristic(service, &uuid, BT_ATT_PERM_READ,
+							BT_GATT_CHRC_PROP_READ,
+							gap_appearance_read_cb,
+							NULL, server);
+
+	gatt_db_service_set_active(service, true);
+}
+
+static void populate_gatt_service(struct btd_gatt_database *server)
+{
+	bt_uuid_t uuid;
+	struct gatt_db_attribute *service;
+
+	/* Add the GATT service */
+	bt_uuid16_create(&uuid, UUID_GATT);
+	service = gatt_db_add_service(server->db, &uuid, true, 1);
+
+	/* TODO: Add "Service Changed" characteristic and handle CCC */
+
+	gatt_db_service_set_active(service, true);
+}
+
+static void register_core_services(struct btd_gatt_database *server)
+{
+	populate_gap_service(server);
+	populate_gatt_service(server);
+}
+
 bool btd_gatt_database_register_adapter(struct btd_adapter *adapter)
 {
 	struct btd_gatt_database *server;
@@ -169,7 +281,7 @@ bool btd_gatt_database_register_adapter(struct btd_adapter *adapter)
 
 	queue_push_tail(servers, server);
 
-	/* TODO: Set up GAP/GATT services */
+	register_core_services(server);
 
 	return true;
 
-- 
2.2.0.rc0.207.ga3a616c


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

* [PATCH BlueZ v1 06/14] core: Add GATT UUIDs to Adapter1.UUIDs
  2015-02-12  3:17 [PATCH BlueZ v1 00/14] Rewrite local GATT server using shared/gatt Arman Uguray
                   ` (4 preceding siblings ...)
  2015-02-12  3:17 ` [PATCH BlueZ v1 05/14] core: gatt: Add GATT/GAP services to local db Arman Uguray
@ 2015-02-12  3:17 ` Arman Uguray
  2015-02-12  3:17 ` [PATCH BlueZ v1 07/14] core: Support per-client CCC state Arman Uguray
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 22+ messages in thread
From: Arman Uguray @ 2015-02-12  3:17 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: luiz.dentz, Arman Uguray

Modified src/adapter so that the UUIDs property includes UUIDs that
were added to the local GATT database.
---
 src/adapter.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/src/adapter.c b/src/adapter.c
index 0253d08..c903274 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -53,6 +53,9 @@
 #include "lib/mgmt.h"
 #include "src/shared/mgmt.h"
 #include "src/shared/util.h"
+#include "src/shared/queue.h"
+#include "src/shared/att.h"
+#include "src/shared/gatt-db.h"
 
 #include "hcid.h"
 #include "sdpd.h"
@@ -2189,16 +2192,37 @@ static gboolean property_get_discovering(const GDBusPropertyTable *property,
 	return TRUE;
 }
 
+static void add_gatt_uuid(struct gatt_db_attribute *attrib, void *user_data)
+{
+	DBusMessageIter *iter = user_data;
+	bt_uuid_t uuid, u128;
+	char uuidstr[MAX_LEN_UUID_STR + 1];
+	const char *ptr = uuidstr;
+
+	if (!gatt_db_service_get_active(attrib))
+		return;
+
+	if (!gatt_db_attribute_get_service_uuid(attrib, &uuid))
+		return;
+
+	bt_uuid_to_uuid128(&uuid, &u128);
+	bt_uuid_to_string(&u128, uuidstr, sizeof(uuidstr));
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &ptr);
+}
+
 static gboolean property_get_uuids(const GDBusPropertyTable *property,
 					DBusMessageIter *iter, void *user_data)
 {
 	struct btd_adapter *adapter = user_data;
 	DBusMessageIter entry;
 	sdp_list_t *l;
+	struct gatt_db *db;
 
 	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
 					DBUS_TYPE_STRING_AS_STRING, &entry);
 
+	/* SDP records */
 	for (l = adapter->services; l != NULL; l = l->next) {
 		sdp_record_t *rec = l->data;
 		char *uuid;
@@ -2212,6 +2236,11 @@ static gboolean property_get_uuids(const GDBusPropertyTable *property,
 		free(uuid);
 	}
 
+	/* GATT services */
+	db = btd_gatt_database_get_db(adapter);
+	if (db)
+		gatt_db_foreach_service(db, NULL, add_gatt_uuid, &entry);
+
 	dbus_message_iter_close_container(iter, &entry);
 
 	return TRUE;
-- 
2.2.0.rc0.207.ga3a616c


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

* [PATCH BlueZ v1 07/14] core: Support per-client CCC state
  2015-02-12  3:17 [PATCH BlueZ v1 00/14] Rewrite local GATT server using shared/gatt Arman Uguray
                   ` (5 preceding siblings ...)
  2015-02-12  3:17 ` [PATCH BlueZ v1 06/14] core: Add GATT UUIDs to Adapter1.UUIDs Arman Uguray
@ 2015-02-12  3:17 ` Arman Uguray
  2015-02-12  3:17 ` [PATCH BlueZ v1 08/14] core: Setup added/removed handlers in GATT database Arman Uguray
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 22+ messages in thread
From: Arman Uguray @ 2015-02-12  3:17 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: luiz.dentz, Arman Uguray

Added support to btd_gatt_database to track the states of CCC
descriptors on a per-client device basis. Added a new API for creating a
CCC entry for a given GATT service.
---
 src/gatt-database.c | 297 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 src/gatt-database.h |   3 +
 2 files changed, 295 insertions(+), 5 deletions(-)

diff --git a/src/gatt-database.c b/src/gatt-database.c
index 2da5a09..7b72515 100644
--- a/src/gatt-database.c
+++ b/src/gatt-database.c
@@ -48,8 +48,141 @@ struct btd_gatt_database {
 	struct btd_adapter *adapter;
 	struct gatt_db *db;
 	GIOChannel *le_io;
+	struct queue *device_states;
 };
 
+struct device_state {
+	bdaddr_t bdaddr;
+	uint8_t bdaddr_type;
+	struct queue *ccc_states;
+};
+
+struct ccc_state {
+	uint16_t handle;
+	uint8_t value[2];
+};
+
+struct device_info {
+	bdaddr_t bdaddr;
+	uint8_t bdaddr_type;
+};
+
+static bool dev_state_match(const void *a, const void *b)
+{
+	const struct device_state *dev_state = a;
+	const struct device_info *dev_info = b;
+
+	return bacmp(&dev_state->bdaddr, &dev_info->bdaddr) == 0 &&
+				dev_state->bdaddr_type == dev_info->bdaddr_type;
+}
+
+static struct device_state *find_device_state(struct btd_gatt_database *server,
+							bdaddr_t *bdaddr,
+							uint8_t bdaddr_type)
+{
+	struct device_info dev_info;
+
+	memset(&dev_info, 0, sizeof(dev_info));
+
+	bacpy(&dev_info.bdaddr, bdaddr);
+	dev_info.bdaddr_type = bdaddr_type;
+
+	return queue_find(server->device_states, dev_state_match, &dev_info);
+}
+
+static bool ccc_state_match(const void *a, const void *b)
+{
+	const struct ccc_state *ccc = a;
+	uint16_t handle = PTR_TO_UINT(b);
+
+	return ccc->handle == handle;
+}
+
+static struct ccc_state *find_ccc_state(struct device_state *dev_state,
+								uint16_t handle)
+{
+	return queue_find(dev_state->ccc_states, ccc_state_match,
+							UINT_TO_PTR(handle));
+}
+
+static struct device_state *device_state_create(bdaddr_t *bdaddr,
+							uint8_t bdaddr_type)
+{
+	struct device_state *dev_state;
+
+	dev_state = new0(struct device_state, 1);
+	if (!dev_state)
+		return NULL;
+
+	dev_state->ccc_states = queue_new();
+	if (!dev_state->ccc_states) {
+		free(dev_state);
+		return NULL;
+	}
+
+	bacpy(&dev_state->bdaddr, bdaddr);
+	dev_state->bdaddr_type = bdaddr_type;
+
+	return dev_state;
+}
+
+static struct device_state *get_device_state(struct btd_gatt_database *server,
+							bdaddr_t *bdaddr,
+							uint8_t bdaddr_type)
+{
+	struct device_state *dev_state;
+
+	/*
+	 * Find and return a device state. If a matching state doesn't exist,
+	 * then create a new one.
+	 */
+	dev_state = find_device_state(server, bdaddr, bdaddr_type);
+	if (dev_state)
+		return dev_state;
+
+	dev_state = device_state_create(bdaddr, bdaddr_type);
+	if (!dev_state)
+		return NULL;
+
+	queue_push_tail(server->device_states, dev_state);
+
+	return dev_state;
+}
+
+static struct ccc_state *get_ccc_state(struct btd_gatt_database *server,
+							bdaddr_t *bdaddr,
+							uint8_t bdaddr_type,
+							uint16_t handle)
+{
+	struct device_state *dev_state;
+	struct ccc_state *ccc;
+
+	dev_state = get_device_state(server, bdaddr, bdaddr_type);
+	if (!dev_state)
+		return NULL;
+
+	ccc = find_ccc_state(dev_state, handle);
+	if (ccc)
+		return ccc;
+
+	ccc = new0(struct ccc_state, 1);
+	if (!ccc)
+		return NULL;
+
+	ccc->handle = handle;
+	queue_push_tail(dev_state->ccc_states, ccc);
+
+	return ccc;
+}
+
+static void device_state_free(void *data)
+{
+	struct device_state *state = data;
+
+	queue_destroy(state->ccc_states, free);
+	free(state);
+}
+
 static void gatt_server_free(void *data)
 {
 	struct btd_gatt_database *server = data;
@@ -59,6 +192,8 @@ static void gatt_server_free(void *data)
 		g_io_channel_unref(server->le_io);
 	}
 
+	/* TODO: Persistently store CCC states before freeing them */
+	queue_destroy(server->device_states, device_state_free);
 	gatt_db_unref(server->db);
 	btd_adapter_unref(server->adapter);
 	free(server);
@@ -266,6 +401,10 @@ bool btd_gatt_database_register_adapter(struct btd_adapter *adapter)
 	if (!server->db)
 		goto fail;
 
+	server->device_states = queue_new();
+	if (!server->device_states)
+		goto fail;
+
 	addr = btd_adapter_get_address(adapter);
 	server->le_io = bt_io_listen(connect_cb, NULL, NULL, NULL, &gerr,
 					BT_IO_OPT_SOURCE_BDADDR, addr,
@@ -299,18 +438,166 @@ void btd_gatt_database_unregister_adapter(struct btd_adapter *adapter)
 	queue_remove_all(servers, match_adapter, adapter, gatt_server_free);
 }
 
-struct gatt_db *btd_gatt_database_get_db(struct btd_adapter *adapter)
+static struct btd_gatt_database *get_server(struct btd_adapter *adapter)
 {
-	struct btd_gatt_database *server;
-
 	if (!servers) {
 		error("GATT server not initialized");
 		return false;
 	}
 
-	server = queue_find(servers, match_adapter, adapter);
-	if (!server)
+	return queue_find(servers, match_adapter, adapter);
+}
+
+struct gatt_db *btd_gatt_database_get_db(struct btd_adapter *adapter)
+{
+	struct btd_gatt_database *server;
+
+	server = get_server(adapter);
+	if (!server) {
+		error("Adapter not registered");
 		return false;
+	}
 
 	return server->db;
 }
+
+static bool get_dst_info(struct bt_att *att, bdaddr_t *dst, uint8_t *dst_type)
+{
+	GIOChannel *io = NULL;
+	GError *gerr = NULL;
+
+	io = g_io_channel_unix_new(bt_att_get_fd(att));
+	if (!io)
+		return false;
+
+	bt_io_get(io, &gerr, BT_IO_OPT_DEST_BDADDR, dst,
+						BT_IO_OPT_DEST_TYPE, dst_type,
+						BT_IO_OPT_INVALID);
+	if (gerr) {
+		error("gatt: bt_io_get: %s", gerr->message);
+		g_error_free(gerr);
+		g_io_channel_unref(io);
+		return false;
+	}
+
+	g_io_channel_unref(io);
+	return true;
+}
+
+static void gatt_ccc_read_cb(struct gatt_db_attribute *attrib,
+					unsigned int id, uint16_t offset,
+					uint8_t opcode, struct bt_att *att,
+					void *user_data)
+{
+	struct btd_gatt_database *server = user_data;
+	struct ccc_state *ccc;
+	uint16_t handle;
+	uint8_t ecode = 0;
+	const uint8_t *value = NULL;
+	size_t len = 0;
+	bdaddr_t bdaddr;
+	uint8_t bdaddr_type;
+
+	handle = gatt_db_attribute_get_handle(attrib);
+
+	DBG("CCC read called for handle: 0x%04x", handle);
+
+	if (offset > 2) {
+		ecode = BT_ATT_ERROR_INVALID_OFFSET;
+		goto done;
+	}
+
+	if (!get_dst_info(att, &bdaddr, &bdaddr_type)) {
+		ecode = BT_ATT_ERROR_UNLIKELY;
+		goto done;
+	}
+
+	ccc = get_ccc_state(server, &bdaddr, bdaddr_type, handle);
+	if (!ccc) {
+		ecode = BT_ATT_ERROR_UNLIKELY;
+		goto done;
+	}
+
+	len -= offset;
+	value = len ? &ccc->value[offset] : NULL;
+
+done:
+	gatt_db_attribute_read_result(attrib, id, ecode, value, len);
+}
+
+static void gatt_ccc_write_cb(struct gatt_db_attribute *attrib,
+					unsigned int id, uint16_t offset,
+					const uint8_t *value, size_t len,
+					uint8_t opcode, struct bt_att *att,
+					void *user_data)
+{
+	struct btd_gatt_database *server = user_data;
+	struct ccc_state *ccc;
+	uint16_t handle;
+	uint8_t ecode = 0;
+	bdaddr_t bdaddr;
+	uint8_t bdaddr_type;
+
+	handle = gatt_db_attribute_get_handle(attrib);
+
+	DBG("CCC read called for handle: 0x%04x", handle);
+
+	if (!value || len != 2) {
+		ecode = BT_ATT_ERROR_INVALID_ATTRIBUTE_VALUE_LEN;
+		goto done;
+	}
+
+	if (offset > 2) {
+		ecode = BT_ATT_ERROR_INVALID_OFFSET;
+		goto done;
+	}
+
+	if (!get_dst_info(att, &bdaddr, &bdaddr_type)) {
+		ecode = BT_ATT_ERROR_UNLIKELY;
+		goto done;
+	}
+
+	ccc = get_ccc_state(server, &bdaddr, bdaddr_type, handle);
+	if (!ccc) {
+		ecode = BT_ATT_ERROR_UNLIKELY;
+		goto done;
+	}
+
+	/*
+	 * TODO: Perform this after checking with a callback to the upper
+	 * layer.
+	 */
+	ccc->value[0] = value[0];
+	ccc->value[1] = value[1];
+
+done:
+	gatt_db_attribute_write_result(attrib, id, ecode);
+}
+
+struct gatt_db_attribute *btd_gatt_database_add_ccc(struct btd_adapter *adapter,
+							uint16_t service_handle)
+{
+	struct btd_gatt_database *server;
+	struct gatt_db_attribute *service;
+	bt_uuid_t uuid;
+
+	if (!adapter || !service_handle)
+		return NULL;
+
+	server = get_server(adapter);
+	if (!server) {
+		error("Adapter not registered");
+		return NULL;
+	}
+
+	service = gatt_db_get_attribute(server->db, service_handle);
+	if (!service) {
+		error("No service exists with handle: 0x%04x", service_handle);
+		return NULL;
+	}
+
+	bt_uuid16_create(&uuid, GATT_CLIENT_CHARAC_CFG_UUID);
+	return gatt_db_service_add_descriptor(service, &uuid,
+				BT_ATT_PERM_READ | BT_ATT_PERM_WRITE,
+				gatt_ccc_read_cb, gatt_ccc_write_cb, server);
+}
diff --git a/src/gatt-database.h b/src/gatt-database.h
index 05e4ab9..4ba12c5 100644
--- a/src/gatt-database.h
+++ b/src/gatt-database.h
@@ -24,3 +24,6 @@ bool btd_gatt_database_register_adapter(struct btd_adapter *adapter);
 void btd_gatt_database_unregister_adapter(struct btd_adapter *adapter);
 
 struct gatt_db *btd_gatt_database_get_db(struct btd_adapter *adapter);
+
+struct gatt_db_attribute *btd_gatt_database_add_ccc(struct btd_adapter *adapter,
+						uint16_t service_handle);
-- 
2.2.0.rc0.207.ga3a616c


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

* [PATCH BlueZ v1 08/14] core: Setup added/removed handlers in GATT database
  2015-02-12  3:17 [PATCH BlueZ v1 00/14] Rewrite local GATT server using shared/gatt Arman Uguray
                   ` (6 preceding siblings ...)
  2015-02-12  3:17 ` [PATCH BlueZ v1 07/14] core: Support per-client CCC state Arman Uguray
@ 2015-02-12  3:17 ` Arman Uguray
  2015-02-12  3:17 ` [PATCH BlueZ v1 09/14] core: Add Service Changed characteristic Arman Uguray
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 22+ messages in thread
From: Arman Uguray @ 2015-02-12  3:17 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: luiz.dentz, Arman Uguray

Registered service added/removed handlers in btd_gatt_database.
---
 src/gatt-database.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/src/gatt-database.c b/src/gatt-database.c
index 7b72515..b8dc361 100644
--- a/src/gatt-database.c
+++ b/src/gatt-database.c
@@ -47,6 +47,7 @@ static struct queue *servers = NULL;
 struct btd_gatt_database {
 	struct btd_adapter *adapter;
 	struct gatt_db *db;
+	unsigned int db_id;
 	GIOChannel *le_io;
 	struct queue *device_states;
 };
@@ -194,6 +195,7 @@ static void gatt_server_free(void *data)
 
 	/* TODO: Persistently store CCC states before freeing them */
 	queue_destroy(server->device_states, device_state_free);
+	gatt_db_unregister(server->db, server->db_id);
 	gatt_db_unref(server->db);
 	btd_adapter_unref(server->adapter);
 	free(server);
@@ -373,6 +375,43 @@ static void register_core_services(struct btd_gatt_database *server)
 	populate_gatt_service(server);
 }
 
+static void gatt_db_service_added(struct gatt_db_attribute *attrib,
+								void *user_data)
+{
+	/* TODO: Send out service changed signal */
+}
+
+static bool ccc_match_service(const void *data, const void *match_data)
+{
+	const struct ccc_state *ccc = data;
+	const struct gatt_db_attribute *attrib = match_data;
+	uint16_t start, end;
+
+	if (!gatt_db_attribute_get_service_handles(attrib, &start, &end))
+		return false;
+
+	return ccc->handle >= start && ccc->handle <= end;
+}
+
+static void remove_device_ccc(void *data, void *user_data)
+{
+	struct device_state *state = data;
+
+	queue_remove_all(state->ccc_states, ccc_match_service, user_data, free);
+}
+
+static void gatt_db_service_removed(struct gatt_db_attribute *attrib,
+								void *user_data)
+{
+	struct btd_gatt_database *server = user_data;
+
+	DBG("Local GATT service removed");
+
+	queue_foreach(server->device_states, remove_device_ccc, attrib);
+
+	/* TODO: Send out service changed signal */
+}
+
 bool btd_gatt_database_register_adapter(struct btd_adapter *adapter)
 {
 	struct btd_gatt_database *server;
@@ -405,6 +444,12 @@ bool btd_gatt_database_register_adapter(struct btd_adapter *adapter)
 	if (!server->device_states)
 		goto fail;
 
+	server->db_id = gatt_db_register(server->db, gatt_db_service_added,
+							gatt_db_service_removed,
+							server, NULL);
+	if (!server->db_id)
+		goto fail;
+
 	addr = btd_adapter_get_address(adapter);
 	server->le_io = bt_io_listen(connect_cb, NULL, NULL, NULL, &gerr,
 					BT_IO_OPT_SOURCE_BDADDR, addr,
-- 
2.2.0.rc0.207.ga3a616c


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

* [PATCH BlueZ v1 09/14] core: Add Service Changed characteristic
  2015-02-12  3:17 [PATCH BlueZ v1 00/14] Rewrite local GATT server using shared/gatt Arman Uguray
                   ` (7 preceding siblings ...)
  2015-02-12  3:17 ` [PATCH BlueZ v1 08/14] core: Setup added/removed handlers in GATT database Arman Uguray
@ 2015-02-12  3:17 ` Arman Uguray
  2015-02-12  3:17 ` [PATCH BlueZ v1 10/14] core: device: Add getter for GATT server Arman Uguray
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 22+ messages in thread
From: Arman Uguray @ 2015-02-12  3:17 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: luiz.dentz, Arman Uguray

This patch exports the "Service Changed" characteristic and its CCC
descriptor in btd_gatt_server.
---
 src/gatt-database.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/gatt-database.c b/src/gatt-database.c
index b8dc361..48c8321 100644
--- a/src/gatt-database.c
+++ b/src/gatt-database.c
@@ -50,6 +50,8 @@ struct btd_gatt_database {
 	unsigned int db_id;
 	GIOChannel *le_io;
 	struct queue *device_states;
+	struct gatt_db_attribute *svc_chngd;
+	struct gatt_db_attribute *svc_chngd_ccc;
 };
 
 struct device_state {
@@ -359,12 +361,20 @@ static void populate_gatt_service(struct btd_gatt_database *server)
 {
 	bt_uuid_t uuid;
 	struct gatt_db_attribute *service;
+	uint16_t start_handle;
 
 	/* Add the GATT service */
 	bt_uuid16_create(&uuid, UUID_GATT);
-	service = gatt_db_add_service(server->db, &uuid, true, 1);
+	service = gatt_db_add_service(server->db, &uuid, true, 4);
+	gatt_db_attribute_get_service_handles(service, &start_handle, NULL);
 
-	/* TODO: Add "Service Changed" characteristic and handle CCC */
+	bt_uuid16_create(&uuid, GATT_CHARAC_SERVICE_CHANGED);
+	server->svc_chngd = gatt_db_service_add_characteristic(service, &uuid,
+				BT_ATT_PERM_READ, BT_GATT_CHRC_PROP_INDICATE,
+				NULL, NULL, server);
+
+	server->svc_chngd_ccc = btd_gatt_database_add_ccc(server->adapter,
+								start_handle);
 
 	gatt_db_service_set_active(service, true);
 }
-- 
2.2.0.rc0.207.ga3a616c


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

* [PATCH BlueZ v1 10/14] core: device: Add getter for GATT server
  2015-02-12  3:17 [PATCH BlueZ v1 00/14] Rewrite local GATT server using shared/gatt Arman Uguray
                   ` (8 preceding siblings ...)
  2015-02-12  3:17 ` [PATCH BlueZ v1 09/14] core: Add Service Changed characteristic Arman Uguray
@ 2015-02-12  3:17 ` Arman Uguray
  2015-02-12  3:17 ` [PATCH BlueZ v1 11/14] core: gatt-server: Send "Service Changed" Arman Uguray
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 22+ messages in thread
From: Arman Uguray @ 2015-02-12  3:17 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: luiz.dentz, Arman Uguray

Added btd_device_get_gatt_server function.
---
 src/device.c | 8 ++++++++
 src/device.h | 1 +
 2 files changed, 9 insertions(+)

diff --git a/src/device.c b/src/device.c
index 0af4d0f..7acda88 100644
--- a/src/device.c
+++ b/src/device.c
@@ -5092,6 +5092,14 @@ struct bt_gatt_client *btd_device_get_gatt_client(struct btd_device *device)
 	return device->client;
 }
 
+struct bt_gatt_server *btd_device_get_gatt_server(struct btd_device *device)
+{
+	if (!device)
+		return NULL;
+
+	return device->server;
+}
+
 void btd_device_gatt_set_service_changed(struct btd_device *device,
 						uint16_t start, uint16_t end)
 {
diff --git a/src/device.h b/src/device.h
index a7fefee..8edd0df 100644
--- a/src/device.h
+++ b/src/device.h
@@ -69,6 +69,7 @@ struct gatt_primary *btd_device_get_primary(struct btd_device *device,
 GSList *btd_device_get_primaries(struct btd_device *device);
 struct gatt_db *btd_device_get_gatt_db(struct btd_device *device);
 struct bt_gatt_client *btd_device_get_gatt_client(struct btd_device *device);
+struct bt_gatt_server *btd_device_get_gatt_server(struct btd_device *device);
 void btd_device_gatt_set_service_changed(struct btd_device *device,
 						uint16_t start, uint16_t end);
 bool device_attach_att(struct btd_device *dev, GIOChannel *io);
-- 
2.2.0.rc0.207.ga3a616c


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

* [PATCH BlueZ v1 11/14] core: gatt-server: Send "Service Changed"
  2015-02-12  3:17 [PATCH BlueZ v1 00/14] Rewrite local GATT server using shared/gatt Arman Uguray
                   ` (9 preceding siblings ...)
  2015-02-12  3:17 ` [PATCH BlueZ v1 10/14] core: device: Add getter for GATT server Arman Uguray
@ 2015-02-12  3:17 ` Arman Uguray
  2015-02-12  3:17 ` [PATCH BlueZ v1 12/14] core: adapter: Send UUIDs changed for GATT services Arman Uguray
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 22+ messages in thread
From: Arman Uguray @ 2015-02-12  3:17 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: luiz.dentz, Arman Uguray

With this patch, the local GATT server sends out "Service Changed"
indications to devices that have configured the corresponding CCC
descriptor, when a local attribute database is modified.
---
 src/gatt-database.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 111 insertions(+), 3 deletions(-)

diff --git a/src/gatt-database.c b/src/gatt-database.c
index 48c8321..6a3d095 100644
--- a/src/gatt-database.c
+++ b/src/gatt-database.c
@@ -30,6 +30,7 @@
 #include "src/shared/queue.h"
 #include "src/shared/att.h"
 #include "src/shared/gatt-db.h"
+#include "src/shared/gatt-server.h"
 #include "log.h"
 #include "adapter.h"
 #include "device.h"
@@ -385,10 +386,117 @@ static void register_core_services(struct btd_gatt_database *server)
 	populate_gatt_service(server);
 }
 
+struct not_data {
+	struct btd_gatt_database *server;
+	uint16_t handle, ccc_handle;
+	const uint8_t *value;
+	uint16_t len;
+	bool indicate;
+};
+
+static void conf_cb(void *user_data)
+{
+	DBG("GATT server received confirmation");
+}
+
+static void send_notification_to_device(void *data, void *user_data)
+{
+	struct device_state *device_state = data;
+	struct not_data *not_data = user_data;
+	struct ccc_state *ccc;
+	struct btd_device *device;
+
+	ccc = find_ccc_state(device_state, not_data->ccc_handle);
+	if (!ccc)
+		return;
+
+	if (!ccc->value[0] || (not_data->indicate && !(ccc->value[0] & 0x02)))
+		return;
+
+	device = btd_adapter_get_device(not_data->server->adapter,
+						&device_state->bdaddr,
+						device_state->bdaddr_type);
+	if (!device)
+		return;
+
+	/*
+	 * TODO: If the device is not connected but bonded, send the
+	 * notification/indication when it becomes connected.
+	 */
+
+	if (!not_data->indicate) {
+		DBG("GATT server sending notification");
+		bt_gatt_server_send_notification(
+					btd_device_get_gatt_server(device),
+					not_data->handle, not_data->value,
+					not_data->len);
+		return;
+	}
+
+	DBG("GATT server sending indication");
+	bt_gatt_server_send_indication(btd_device_get_gatt_server(device),
+							not_data->handle,
+							not_data->value,
+							not_data->len, conf_cb,
+							NULL, NULL);
+}
+
+static void send_notification_to_devices(struct btd_gatt_database *server,
+					uint16_t handle, const uint8_t *value,
+					uint16_t len, uint16_t ccc_handle,
+					bool indicate)
+{
+	struct not_data not_data;
+
+	memset(&not_data, 0, sizeof(not_data));
+
+	not_data.server = server;
+	not_data.handle = handle;
+	not_data.ccc_handle = ccc_handle;
+	not_data.value = value;
+	not_data.len = len;
+	not_data.indicate = indicate;
+
+	queue_foreach(server->device_states, send_notification_to_device,
+								&not_data);
+}
+
+static void send_service_changed(struct btd_gatt_database *server,
+					struct gatt_db_attribute *attrib)
+{
+	uint16_t start, end;
+	uint8_t value[4];
+	uint16_t handle, ccc_handle;
+
+	if (!gatt_db_attribute_get_service_handles(attrib, &start, &end)) {
+		error("Failed to obtain changed service handles");
+		return;
+	}
+
+	handle = gatt_db_attribute_get_handle(server->svc_chngd);
+	ccc_handle = gatt_db_attribute_get_handle(server->svc_chngd_ccc);
+
+	if (!handle || !ccc_handle) {
+		error("Failed to obtain handles for \"Service Changed\""
+							" characteristic");
+		return;
+	}
+
+	put_le16(start, value);
+	put_le16(end, value + 2);
+
+	send_notification_to_devices(server, handle, value, sizeof(value),
+							ccc_handle, true);
+}
+
 static void gatt_db_service_added(struct gatt_db_attribute *attrib,
 								void *user_data)
 {
-	/* TODO: Send out service changed signal */
+	struct btd_gatt_database *server = user_data;
+
+	DBG("GATT Service added to local database");
+
+	send_service_changed(server, attrib);
 }
 
 static bool ccc_match_service(const void *data, const void *match_data)
@@ -417,9 +525,9 @@ static void gatt_db_service_removed(struct gatt_db_attribute *attrib,
 
 	DBG("Local GATT service removed");
 
-	queue_foreach(server->device_states, remove_device_ccc, attrib);
+	send_service_changed(server, attrib);
 
-	/* TODO: Send out service changed signal */
+	queue_foreach(server->device_states, remove_device_ccc, attrib);
 }
 
 bool btd_gatt_database_register_adapter(struct btd_adapter *adapter)
-- 
2.2.0.rc0.207.ga3a616c


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

* [PATCH BlueZ v1 12/14] core: adapter: Send UUIDs changed for GATT services
  2015-02-12  3:17 [PATCH BlueZ v1 00/14] Rewrite local GATT server using shared/gatt Arman Uguray
                   ` (10 preceding siblings ...)
  2015-02-12  3:17 ` [PATCH BlueZ v1 11/14] core: gatt-server: Send "Service Changed" Arman Uguray
@ 2015-02-12  3:17 ` Arman Uguray
  2015-02-12  3:17 ` [PATCH BlueZ v1 13/14] shared/gatt: Don't incorrectly terminate discovery Arman Uguray
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 22+ messages in thread
From: Arman Uguray @ 2015-02-12  3:17 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: luiz.dentz, Arman Uguray

btd_adapter now sends a PropertiesChanged signal for the "UUIDs"
property when its associated gatt_db is modified.
---
 src/adapter.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/src/adapter.c b/src/adapter.c
index c903274..b6057d9 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -226,6 +226,8 @@ struct btd_adapter {
 	unsigned int pair_device_id;
 	guint pair_device_timeout;
 
+	unsigned int db_id;		/* Service event handler for GATT db */
+
 	bool is_default;		/* true if adapter is default one */
 };
 
@@ -4555,6 +4557,7 @@ static struct btd_adapter *btd_adapter_new(uint16_t index)
 static void adapter_remove(struct btd_adapter *adapter)
 {
 	GSList *l;
+	struct gatt_db *db;
 
 	DBG("Removing adapter %s", adapter->path);
 
@@ -4580,6 +4583,11 @@ static void adapter_remove(struct btd_adapter *adapter)
 	adapter->devices = NULL;
 
 	unload_drivers(adapter);
+
+	db = btd_gatt_database_get_db(adapter);
+	gatt_db_unregister(db, adapter->db_id);
+	adapter->db_id = 0;
+
 	btd_gatt_database_unregister_adapter(adapter);
 
 	g_slist_free(adapter->pin_callbacks);
@@ -6589,9 +6597,18 @@ static int set_did(struct btd_adapter *adapter, uint16_t vendor,
 	return -EIO;
 }
 
+static void services_modified(struct gatt_db_attribute *attrib, void *user_data)
+{
+	struct btd_adapter *adapter = user_data;
+
+	g_dbus_emit_property_changed(dbus_conn, adapter->path,
+						ADAPTER_INTERFACE, "UUIDs");
+}
+
 static int adapter_register(struct btd_adapter *adapter)
 {
 	struct agent *agent;
+	struct gatt_db *db;
 
 	if (powering_down)
 		return -EBUSY;
@@ -6625,6 +6642,11 @@ static int adapter_register(struct btd_adapter *adapter)
 	if (!btd_gatt_database_register_adapter(adapter))
 		error("Failed to register adapter with GATT server");
 
+	db = btd_gatt_database_get_db(adapter);
+	adapter->db_id = gatt_db_register(db, services_modified,
+							services_modified,
+							adapter, NULL);
+
 	load_config(adapter);
 	fix_storage(adapter);
 	load_drivers(adapter);
-- 
2.2.0.rc0.207.ga3a616c


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

* [PATCH BlueZ v1 13/14] shared/gatt: Don't incorrectly terminate discovery
  2015-02-12  3:17 [PATCH BlueZ v1 00/14] Rewrite local GATT server using shared/gatt Arman Uguray
                   ` (11 preceding siblings ...)
  2015-02-12  3:17 ` [PATCH BlueZ v1 12/14] core: adapter: Send UUIDs changed for GATT services Arman Uguray
@ 2015-02-12  3:17 ` Arman Uguray
  2015-02-12  3:17 ` [PATCH BlueZ v1 14/14] TODO: Update GATT items Arman Uguray
  2015-02-16  9:13 ` [PATCH BlueZ v1 00/14] Rewrite local GATT server using shared/gatt Luiz Augusto von Dentz
  14 siblings, 0 replies; 22+ messages in thread
From: Arman Uguray @ 2015-02-12  3:17 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: luiz.dentz, Arman Uguray

bt_gatt_client terminates discovery if no primary services are found
within the given range. This behavior is incorrect, as the given
handle range may contain secondary services and those should be
discovered regardless.
---
 src/shared/gatt-client.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index d5a277b..03f6725 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -915,7 +915,7 @@ static void discover_primary_cb(bool success, uint8_t att_ecode,
 		util_debug(client->debug_callback, client->debug_data,
 					"Primary service discovery failed."
 					" ATT ECODE: 0x%02x", att_ecode);
-		goto done;
+		goto secondary;
 	}
 
 	if (!result || !bt_gatt_iter_init(&iter, result)) {
@@ -948,6 +948,7 @@ static void discover_primary_cb(bool success, uint8_t att_ecode,
 		queue_push_tail(op->pending_svcs, attr);
 	}
 
+secondary:
 	/* Discover secondary services */
 	if (bt_gatt_discover_secondary_services(client->att, NULL,
 							op->start, op->end,
-- 
2.2.0.rc0.207.ga3a616c


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

* [PATCH BlueZ v1 14/14] TODO: Update GATT items.
  2015-02-12  3:17 [PATCH BlueZ v1 00/14] Rewrite local GATT server using shared/gatt Arman Uguray
                   ` (12 preceding siblings ...)
  2015-02-12  3:17 ` [PATCH BlueZ v1 13/14] shared/gatt: Don't incorrectly terminate discovery Arman Uguray
@ 2015-02-12  3:17 ` Arman Uguray
  2015-02-16  9:13 ` [PATCH BlueZ v1 00/14] Rewrite local GATT server using shared/gatt Luiz Augusto von Dentz
  14 siblings, 0 replies; 22+ messages in thread
From: Arman Uguray @ 2015-02-12  3:17 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: luiz.dentz, Arman Uguray

Updated the GATT/ATT related TODO items:

  - Removed item about disconnect handling as this is already done via
    bt_att.
  - Long-term client caching is currently done in memory. Updated this
    to mean peristent.
  - GAttrib has already been turned into a shim around bt_att. Updated
    the related item to involve updating profiles only.
  - GATT client D-Bus API has been implemented.
  - Added item for missing portion of Service Changed support.
  - Added item for GATT related refactors discussed over IRC.
  - Added item for supporting the Includes property in GATT client.
---
 TODO | 54 ++++++++++++++++++++++++++++++------------------------
 1 file changed, 30 insertions(+), 24 deletions(-)

diff --git a/TODO b/TODO
index e489221..db80b95 100644
--- a/TODO
+++ b/TODO
@@ -136,41 +136,19 @@ ATT/GATT (new shared stack)
   Priority: Medium
   Complexity: C1
 
-- Introduce a handler interface to shared/gatt-client which can be used by the
-  upper layer to determine when the link has been disconnected or an ATT
-  protocol request times out.
+- Persist client attribute cache across reboots.
 
   Priority: Medium
-  Complexity: C2
-
-- Introduce long-term caching of attributes to shared/gatt-client, such that the
-  services, characteristics, and descriptors obtained from a peripheral are
-  remembered in the case of bonding. This may involve storing data about GATT
-  services to disk.
-
-  Priority: Low
   Complexity: C4
 
 - Move all daemon plugins and profiles that are GATT based to use
   shared/gatt-client instead of attrib/*. This is a complicated task that
   potentially needs a new plugin/profile probing interface and a lot of
-  rewriting that can cause regressions in existing functionality. The biggest
-  challenge here is that an instance of bt_att (shared/att) and GAttrib
-  (attrib/gattrib) cannot coexist on the same file descriptor, since they will
-  cause ATT protocol violations by breaking the sequential request-response
-  structure. A special shared/gatt-client-gattrib implementation may be
-  necessary to move each profile/plugin to the new API before actually switching
-  to the shared/att based implementation.
+  rewriting that can cause regressions in existing functionality.
 
   Priority: Medium
   Complexity: C4
 
-- Implement the client portion of doc/gatt-api.txt using shared/gatt-client once
-  plugin/profile code uses it.
-
-  Priority: Medium
-  Complexity: C2
-
 - Introduce a way for shared/gatt-server to check security permissions on the
   current connection through bt_att.
 
@@ -190,6 +168,34 @@ ATT/GATT (new shared stack)
   Priority: Medium
   Complexity: C4
 
+- Send out indications from the "Service Changed" characteristic upon
+  reconnection if a bonded device is not connected when the local database is
+  modified.
+
+  Priority: High
+  Complexity: C2
+
+- Unify the GATT server and client D-Bus implementations into a single module.
+  While these don't share a lot of code, keeping them all in src/gatt-dbus seems
+  to make more sense from an organizational perspective.
+
+  Priority: Low
+  Complexity: C1
+
+- Isolate all GATT code inside the daemon into its own module and perform
+  interaction with other modules (e.g. src/device.c) via callbacks. This
+  includes client/server management, tracking incoming/outgoing connections for
+  ATT, and callbacks to perform profile probing.
+
+  Priority: Low
+  Complexity: C4
+
+- Support included services in the GATT D-Bus client API.
+
+  Priority: Medium
+  Complexity: C1
+
+
 ATT/GATT (old/outdated)
 =======================
 
-- 
2.2.0.rc0.207.ga3a616c


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

* Re: [PATCH BlueZ v1 01/14] shared/att: Add bt_att_get_fd
  2015-02-12  3:17 ` [PATCH BlueZ v1 01/14] shared/att: Add bt_att_get_fd Arman Uguray
@ 2015-02-12 13:40   ` Luiz Augusto von Dentz
  2015-02-12 18:21     ` Arman Uguray
  0 siblings, 1 reply; 22+ messages in thread
From: Luiz Augusto von Dentz @ 2015-02-12 13:40 UTC (permalink / raw)
  To: Arman Uguray; +Cc: linux-bluetooth

Hi Arman,

On Thu, Feb 12, 2015 at 5:17 AM, Arman Uguray <armansito@chromium.org> wrote:
> Added the bt_att_get_fd function which returns the underlying file
> descriptor of a bt_att.
> ---
>  src/shared/att.c          | 8 ++++++++
>  src/shared/att.h          | 2 ++
>  src/shared/gatt-client.c  | 3 ++-
>  src/shared/gatt-helpers.c | 3 ++-
>  4 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/src/shared/att.c b/src/shared/att.c
> index a98909e..8bf1bab 100644
> --- a/src/shared/att.c
> +++ b/src/shared/att.c
> @@ -889,6 +889,14 @@ bool bt_att_set_close_on_unref(struct bt_att *att, bool do_close)
>         return io_set_close_on_destroy(att->io, do_close);
>  }
>
> +int bt_att_get_fd(struct bt_att *att)
> +{
> +       if (!att)
> +               return -1;
> +
> +       return att->fd;
> +}
> +
>  bool bt_att_set_debug(struct bt_att *att, bt_att_debug_func_t callback,
>                                 void *user_data, bt_att_destroy_func_t destroy)
>  {
> diff --git a/src/shared/att.h b/src/shared/att.h
> index cd00a1e..db423fe 100644
> --- a/src/shared/att.h
> +++ b/src/shared/att.h
> @@ -35,6 +35,8 @@ void bt_att_unref(struct bt_att *att);
>
>  bool bt_att_set_close_on_unref(struct bt_att *att, bool do_close);
>
> +int bt_att_get_fd(struct bt_att *att);
> +
>  typedef void (*bt_att_response_func_t)(uint8_t opcode, const void *pdu,
>                                         uint16_t length, void *user_data);
>  typedef void (*bt_att_notify_func_t)(uint8_t opcode, const void *pdu,
> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
> index bfb9427..d5a277b 100644
> --- a/src/shared/gatt-client.c
> +++ b/src/shared/gatt-client.c
> @@ -21,8 +21,9 @@
>   *
>   */
>
> -#include "src/shared/att.h"
>  #include "lib/uuid.h"
> +#include "lib/bluetooth.h"
> +#include "src/shared/att.h"
>  #include "src/shared/gatt-helpers.h"
>  #include "src/shared/util.h"
>  #include "src/shared/queue.h"
> diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
> index a33f960..b469116 100644
> --- a/src/shared/gatt-helpers.c
> +++ b/src/shared/gatt-helpers.c
> @@ -26,9 +26,10 @@
>  #include <config.h>
>  #endif
>
> +#include "lib/uuid.h"
> +#include "lib/bluetooth.h"
>  #include "src/shared/queue.h"
>  #include "src/shared/att.h"
> -#include "lib/uuid.h"
>  #include "src/shared/gatt-helpers.h"
>  #include "src/shared/util.h"

It seems there changes are not really necessary, are they? At least
for these changes you don't really need bluetooth.h.


-- 
Luiz Augusto von Dentz

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

* Re: [PATCH BlueZ v1 01/14] shared/att: Add bt_att_get_fd
  2015-02-12 13:40   ` Luiz Augusto von Dentz
@ 2015-02-12 18:21     ` Arman Uguray
  0 siblings, 0 replies; 22+ messages in thread
From: Arman Uguray @ 2015-02-12 18:21 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

Hi Luiz,

> On Thu, Feb 12, 2015 at 5:40 AM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
> Hi Arman,
>
> On Thu, Feb 12, 2015 at 5:17 AM, Arman Uguray <armansito@chromium.org> wrote:
>> Added the bt_att_get_fd function which returns the underlying file
>> descriptor of a bt_att.
>> ---
>>  src/shared/att.c          | 8 ++++++++
>>  src/shared/att.h          | 2 ++
>>  src/shared/gatt-client.c  | 3 ++-
>>  src/shared/gatt-helpers.c | 3 ++-
>>  4 files changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/shared/att.c b/src/shared/att.c
>> index a98909e..8bf1bab 100644
>> --- a/src/shared/att.c
>> +++ b/src/shared/att.c
>> @@ -889,6 +889,14 @@ bool bt_att_set_close_on_unref(struct bt_att *att, bool do_close)
>>         return io_set_close_on_destroy(att->io, do_close);
>>  }
>>
>> +int bt_att_get_fd(struct bt_att *att)
>> +{
>> +       if (!att)
>> +               return -1;
>> +
>> +       return att->fd;
>> +}
>> +
>>  bool bt_att_set_debug(struct bt_att *att, bt_att_debug_func_t callback,
>>                                 void *user_data, bt_att_destroy_func_t destroy)
>>  {
>> diff --git a/src/shared/att.h b/src/shared/att.h
>> index cd00a1e..db423fe 100644
>> --- a/src/shared/att.h
>> +++ b/src/shared/att.h
>> @@ -35,6 +35,8 @@ void bt_att_unref(struct bt_att *att);
>>
>>  bool bt_att_set_close_on_unref(struct bt_att *att, bool do_close);
>>
>> +int bt_att_get_fd(struct bt_att *att);
>> +
>>  typedef void (*bt_att_response_func_t)(uint8_t opcode, const void *pdu,
>>                                         uint16_t length, void *user_data);
>>  typedef void (*bt_att_notify_func_t)(uint8_t opcode, const void *pdu,
>> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
>> index bfb9427..d5a277b 100644
>> --- a/src/shared/gatt-client.c
>> +++ b/src/shared/gatt-client.c
>> @@ -21,8 +21,9 @@
>>   *
>>   */
>>
>> -#include "src/shared/att.h"
>>  #include "lib/uuid.h"
>> +#include "lib/bluetooth.h"
>> +#include "src/shared/att.h"
>>  #include "src/shared/gatt-helpers.h"
>>  #include "src/shared/util.h"
>>  #include "src/shared/queue.h"
>> diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
>> index a33f960..b469116 100644
>> --- a/src/shared/gatt-helpers.c
>> +++ b/src/shared/gatt-helpers.c
>> @@ -26,9 +26,10 @@
>>  #include <config.h>
>>  #endif
>>
>> +#include "lib/uuid.h"
>> +#include "lib/bluetooth.h"
>>  #include "src/shared/queue.h"
>>  #include "src/shared/att.h"
>> -#include "lib/uuid.h"
>>  #include "src/shared/gatt-helpers.h"
>>  #include "src/shared/util.h"
>
> It seems there changes are not really necessary, are they? At least
> for these changes you don't really need bluetooth.h.
>

You're right, looks like I forgot to revert this bit when I revised
the patches. These should remain as before.

>
> --
> Luiz Augusto von Dentz

Thanks,
Arman

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

* Re: [PATCH BlueZ v1 03/14] core: Introduce btd_gatt_database
  2015-02-12  3:17 ` [PATCH BlueZ v1 03/14] core: Introduce btd_gatt_database Arman Uguray
@ 2015-02-13 16:06   ` Luiz Augusto von Dentz
  2015-02-13 16:21     ` Arman Uguray
  0 siblings, 1 reply; 22+ messages in thread
From: Luiz Augusto von Dentz @ 2015-02-13 16:06 UTC (permalink / raw)
  To: Arman Uguray; +Cc: linux-bluetooth

Hi Arman,

On Thu, Feb 12, 2015 at 5:17 AM, Arman Uguray <armansito@chromium.org> wrote:
> This patch introduces src/gatt-database.* which handles incoming ATT
> connections, manages per-adapter shared/gatt-db instances, and routes
> connections to the corresponding device object. This is the layer that
> will perform all the CCC management and Service Changed handling.
> ---
>  Makefile.am         |   1 +
>  src/adapter.c       |   8 ++-
>  src/gatt-database.c | 204 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/gatt-database.h |  26 +++++++
>  src/main.c          |   3 +
>  5 files changed, 240 insertions(+), 2 deletions(-)
>  create mode 100644 src/gatt-database.c
>  create mode 100644 src/gatt-database.h
>
> diff --git a/Makefile.am b/Makefile.am
> index 60811f1..4407355 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -167,6 +167,7 @@ src_bluetoothd_SOURCES = $(builtin_sources) \
>                         src/sdpd-server.c src/sdpd-request.c \
>                         src/sdpd-service.c src/sdpd-database.c \
>                         src/attrib-server.h src/attrib-server.c \
> +                       src/gatt-database.h src/gatt-database.c \
>                         src/sdp-xml.h src/sdp-xml.c \
>                         src/sdp-client.h src/sdp-client.c \
>                         src/textfile.h src/textfile.c \
> diff --git a/src/adapter.c b/src/adapter.c
> index 1839286..0253d08 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -68,6 +68,7 @@
>  #include "attrib/att.h"
>  #include "attrib/gatt.h"
>  #include "attrib-server.h"
> +#include "gatt-database.h"
>  #include "eir.h"
>
>  #define ADAPTER_INTERFACE      "org.bluez.Adapter1"
> @@ -302,6 +303,7 @@ static void dev_class_changed_callback(uint16_t index, uint16_t length,
>         appearance[1] = rp->val[1] & 0x1f;      /* removes service class */
>         appearance[2] = rp->val[2];
>
> +       /* TODO: Do this through btd_gatt_database instead */
>         attrib_gap_set(adapter, GATT_CHARAC_APPEARANCE, appearance, 2);
>  }
>
> @@ -4014,6 +4016,7 @@ static void convert_sdp_entry(char *key, char *value, void *user_data)
>         if (record_has_uuid(rec, att_uuid))
>                 goto failed;
>
> +       /* TODO: Do this through btd_gatt_database */
>         if (!gatt_parse_record(rec, &uuid, &psm, &start, &end))
>                 goto failed;
>
> @@ -4548,7 +4551,7 @@ static void adapter_remove(struct btd_adapter *adapter)
>         adapter->devices = NULL;
>
>         unload_drivers(adapter);
> -       btd_adapter_gatt_server_stop(adapter);
> +       btd_gatt_database_unregister_adapter(adapter);
>
>         g_slist_free(adapter->pin_callbacks);
>         adapter->pin_callbacks = NULL;
> @@ -6590,7 +6593,8 @@ static int adapter_register(struct btd_adapter *adapter)
>                 agent_unref(agent);
>         }
>
> -       btd_adapter_gatt_server_start(adapter);
> +       if (!btd_gatt_database_register_adapter(adapter))
> +               error("Failed to register adapter with GATT server");
>
>         load_config(adapter);
>         fix_storage(adapter);
> diff --git a/src/gatt-database.c b/src/gatt-database.c
> new file mode 100644
> index 0000000..57bdf1a
> --- /dev/null
> +++ b/src/gatt-database.c
> @@ -0,0 +1,204 @@
> +/*
> + *
> + *  BlueZ - Bluetooth protocol stack for Linux
> + *
> + *  Copyright (C) 2015  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.
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include <stdint.h>
> +#include <stdlib.h>
> +
> +#include "lib/uuid.h"
> +#include "btio/btio.h"
> +#include "src/shared/util.h"
> +#include "src/shared/queue.h"
> +#include "src/shared/att.h"
> +#include "src/shared/gatt-db.h"
> +#include "log.h"
> +#include "adapter.h"
> +#include "device.h"
> +#include "gatt-database.h"
> +
> +#ifndef ATT_CID
> +#define ATT_CID 4
> +#endif
> +
> +static struct queue *servers = NULL;
> +
> +struct btd_gatt_database {
> +       struct btd_adapter *adapter;
> +       struct gatt_db *db;
> +       GIOChannel *le_io;
> +};
> +
> +static void gatt_server_free(void *data)
> +{
> +       struct btd_gatt_database *server = data;
> +
> +       if (server->le_io) {
> +               g_io_channel_shutdown(server->le_io, FALSE, NULL);
> +               g_io_channel_unref(server->le_io);
> +       }
> +
> +       gatt_db_unref(server->db);
> +       btd_adapter_unref(server->adapter);
> +       free(server);
> +}
> +
> +bool btd_gatt_database_init(void)
> +{
> +
> +       info("Initializing GATT server");
> +
> +       servers = queue_new();
> +       if (!servers) {
> +               error("Failed to set up local GATT server");
> +               return false;
> +       }
> +
> +       return true;
> +}
> +
> +void btd_gatt_database_cleanup(void)
> +{
> +       info("Cleaning up GATT server");
> +
> +       queue_destroy(servers, gatt_server_free);
> +       servers = NULL;
> +}
> +
> +static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
> +{
> +       struct btd_adapter *adapter;
> +       struct btd_device *device;
> +       uint8_t dst_type;
> +       bdaddr_t src, dst;
> +
> +       DBG("New incoming LE ATT connection");
> +
> +       if (gerr) {
> +               error("%s", gerr->message);
> +               return;
> +       }
> +
> +       bt_io_get(io, &gerr, BT_IO_OPT_SOURCE_BDADDR, &src,
> +                                               BT_IO_OPT_DEST_BDADDR, &dst,
> +                                               BT_IO_OPT_DEST_TYPE, &dst_type,
> +                                               BT_IO_OPT_INVALID);
> +       if (gerr) {
> +               error("bt_io_get: %s", gerr->message);
> +               g_error_free(gerr);
> +               return;
> +       }
> +
> +       adapter = adapter_find(&src);
> +       if (!adapter)
> +               return;
> +
> +       device = btd_adapter_get_device(adapter, &dst, dst_type);
> +       if (!device)
> +               return;
> +
> +       device_attach_att(device, io);
> +}
> +
> +static bool match_adapter(const void *a, const void *b)
> +{
> +       const struct btd_gatt_database *server = a;
> +       const struct btd_adapter *adapter = b;
> +
> +       return server->adapter == adapter;
> +}
> +
> +bool btd_gatt_database_register_adapter(struct btd_adapter *adapter)
> +{
> +       struct btd_gatt_database *server;
> +       GError *gerr = NULL;
> +       const bdaddr_t *addr;
> +
> +       if (!adapter)
> +               return false;
> +
> +       if (!servers) {
> +               error("GATT server not initialized");
> +               return false;
> +       }
> +
> +       if (queue_find(servers, match_adapter, adapter)) {
> +               error("Adapter already registered with GATT server");
> +               return false;
> +       }
> +
> +       server = new0(struct btd_gatt_database, 1);
> +       if (!server)
> +               return false;
> +
> +       server->adapter = btd_adapter_ref(adapter);
> +       server->db = gatt_db_new();
> +       if (!server->db)
> +               goto fail;
> +
> +       addr = btd_adapter_get_address(adapter);
> +       server->le_io = bt_io_listen(connect_cb, NULL, NULL, NULL, &gerr,
> +                                       BT_IO_OPT_SOURCE_BDADDR, addr,
> +                                       BT_IO_OPT_SOURCE_TYPE, BDADDR_LE_PUBLIC,
> +                                       BT_IO_OPT_CID, ATT_CID,
> +                                       BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
> +                                       BT_IO_OPT_INVALID);
> +       if (!server->le_io) {
> +               error("Failed to start listening: %s", gerr->message);
> +               g_error_free(gerr);
> +               goto fail;
> +       }
> +
> +       queue_push_tail(servers, server);
> +
> +       /* TODO: Set up GAP/GATT services */
> +
> +       return true;
> +
> +fail:
> +       gatt_server_free(server);
> +
> +       return false;
> +}
> +
> +void btd_gatt_database_unregister_adapter(struct btd_adapter *adapter)
> +{
> +       if (!adapter || !servers)
> +               return;
> +
> +       queue_remove_all(servers, match_adapter, adapter, gatt_server_free);
> +}
> +
> +struct gatt_db *btd_gatt_database_get_db(struct btd_adapter *adapter)
> +{
> +       struct btd_gatt_database *server;
> +
> +       if (!servers) {
> +               error("GATT server not initialized");
> +               return false;
> +       }
> +
> +       server = queue_find(servers, match_adapter, adapter);
> +       if (!server)
> +               return false;
> +
> +       return server->db;
> +}
> diff --git a/src/gatt-database.h b/src/gatt-database.h
> new file mode 100644
> index 0000000..05e4ab9
> --- /dev/null
> +++ b/src/gatt-database.h
> @@ -0,0 +1,26 @@
> +/*
> + *
> + *  BlueZ - Bluetooth protocol stack for Linux
> + *
> + *  Copyright (C) 2015  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.
> + *
> + */
> +
> +bool btd_gatt_database_init(void);
> +void btd_gatt_database_cleanup(void);
> +
> +bool btd_gatt_database_register_adapter(struct btd_adapter *adapter);
> +void btd_gatt_database_unregister_adapter(struct btd_adapter *adapter);
> +
> +struct gatt_db *btd_gatt_database_get_db(struct btd_adapter *adapter);

Did not like this API, imo it is better to have
btd_gatt_database_new() and declare struct btd_gatt_database,
otherwise for every use we need a lookup which is not efficient.
Perhaps if you don't have time look at this today I may find sometime
during the weekend or Monday morning.

> diff --git a/src/main.c b/src/main.c
> index 061060d..4ccc18f 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -56,6 +56,7 @@
>  #include "agent.h"
>  #include "profile.h"
>  #include "gatt.h"
> +#include "gatt-database.h"
>  #include "systemd.h"
>
>  #define BLUEZ_NAME "org.bluez"
> @@ -579,6 +580,7 @@ int main(int argc, char *argv[])
>         g_dbus_set_flags(gdbus_flags);
>
>         gatt_init();
> +       btd_gatt_database_init();
>
>         if (adapter_init() < 0) {
>                 error("Adapter handling initialization failed");
> @@ -642,6 +644,7 @@ int main(int argc, char *argv[])
>
>         adapter_cleanup();
>
> +       btd_gatt_database_cleanup();
>         gatt_cleanup();
>
>         rfkill_exit();
> --
> 2.2.0.rc0.207.ga3a616c
>



-- 
Luiz Augusto von Dentz

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

* Re: [PATCH BlueZ v1 03/14] core: Introduce btd_gatt_database
  2015-02-13 16:06   ` Luiz Augusto von Dentz
@ 2015-02-13 16:21     ` Arman Uguray
  2015-02-17 12:03       ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 22+ messages in thread
From: Arman Uguray @ 2015-02-13 16:21 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

Hi Luiz,

> On Fri, Feb 13, 2015 at 8:06 AM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
> Hi Arman,
>
> On Thu, Feb 12, 2015 at 5:17 AM, Arman Uguray <armansito@chromium.org> wrote:
>> This patch introduces src/gatt-database.* which handles incoming ATT
>> connections, manages per-adapter shared/gatt-db instances, and routes
>> connections to the corresponding device object. This is the layer that
>> will perform all the CCC management and Service Changed handling.
>> ---
>>  Makefile.am         |   1 +
>>  src/adapter.c       |   8 ++-
>>  src/gatt-database.c | 204 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  src/gatt-database.h |  26 +++++++
>>  src/main.c          |   3 +
>>  5 files changed, 240 insertions(+), 2 deletions(-)
>>  create mode 100644 src/gatt-database.c
>>  create mode 100644 src/gatt-database.h
>>
>> diff --git a/Makefile.am b/Makefile.am
>> index 60811f1..4407355 100644
>> --- a/Makefile.am
>> +++ b/Makefile.am
>> @@ -167,6 +167,7 @@ src_bluetoothd_SOURCES = $(builtin_sources) \
>>                         src/sdpd-server.c src/sdpd-request.c \
>>                         src/sdpd-service.c src/sdpd-database.c \
>>                         src/attrib-server.h src/attrib-server.c \
>> +                       src/gatt-database.h src/gatt-database.c \
>>                         src/sdp-xml.h src/sdp-xml.c \
>>                         src/sdp-client.h src/sdp-client.c \
>>                         src/textfile.h src/textfile.c \
>> diff --git a/src/adapter.c b/src/adapter.c
>> index 1839286..0253d08 100644
>> --- a/src/adapter.c
>> +++ b/src/adapter.c
>> @@ -68,6 +68,7 @@
>>  #include "attrib/att.h"
>>  #include "attrib/gatt.h"
>>  #include "attrib-server.h"
>> +#include "gatt-database.h"
>>  #include "eir.h"
>>
>>  #define ADAPTER_INTERFACE      "org.bluez.Adapter1"
>> @@ -302,6 +303,7 @@ static void dev_class_changed_callback(uint16_t index, uint16_t length,
>>         appearance[1] = rp->val[1] & 0x1f;      /* removes service class */
>>         appearance[2] = rp->val[2];
>>
>> +       /* TODO: Do this through btd_gatt_database instead */
>>         attrib_gap_set(adapter, GATT_CHARAC_APPEARANCE, appearance, 2);
>>  }
>>
>> @@ -4014,6 +4016,7 @@ static void convert_sdp_entry(char *key, char *value, void *user_data)
>>         if (record_has_uuid(rec, att_uuid))
>>                 goto failed;
>>
>> +       /* TODO: Do this through btd_gatt_database */
>>         if (!gatt_parse_record(rec, &uuid, &psm, &start, &end))
>>                 goto failed;
>>
>> @@ -4548,7 +4551,7 @@ static void adapter_remove(struct btd_adapter *adapter)
>>         adapter->devices = NULL;
>>
>>         unload_drivers(adapter);
>> -       btd_adapter_gatt_server_stop(adapter);
>> +       btd_gatt_database_unregister_adapter(adapter);
>>
>>         g_slist_free(adapter->pin_callbacks);
>>         adapter->pin_callbacks = NULL;
>> @@ -6590,7 +6593,8 @@ static int adapter_register(struct btd_adapter *adapter)
>>                 agent_unref(agent);
>>         }
>>
>> -       btd_adapter_gatt_server_start(adapter);
>> +       if (!btd_gatt_database_register_adapter(adapter))
>> +               error("Failed to register adapter with GATT server");
>>
>>         load_config(adapter);
>>         fix_storage(adapter);
>> diff --git a/src/gatt-database.c b/src/gatt-database.c
>> new file mode 100644
>> index 0000000..57bdf1a
>> --- /dev/null
>> +++ b/src/gatt-database.c
>> @@ -0,0 +1,204 @@
>> +/*
>> + *
>> + *  BlueZ - Bluetooth protocol stack for Linux
>> + *
>> + *  Copyright (C) 2015  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.
>> + *
>> + */
>> +
>> +#ifdef HAVE_CONFIG_H
>> +#include <config.h>
>> +#endif
>> +
>> +#include <stdint.h>
>> +#include <stdlib.h>
>> +
>> +#include "lib/uuid.h"
>> +#include "btio/btio.h"
>> +#include "src/shared/util.h"
>> +#include "src/shared/queue.h"
>> +#include "src/shared/att.h"
>> +#include "src/shared/gatt-db.h"
>> +#include "log.h"
>> +#include "adapter.h"
>> +#include "device.h"
>> +#include "gatt-database.h"
>> +
>> +#ifndef ATT_CID
>> +#define ATT_CID 4
>> +#endif
>> +
>> +static struct queue *servers = NULL;
>> +
>> +struct btd_gatt_database {
>> +       struct btd_adapter *adapter;
>> +       struct gatt_db *db;
>> +       GIOChannel *le_io;
>> +};
>> +
>> +static void gatt_server_free(void *data)
>> +{
>> +       struct btd_gatt_database *server = data;
>> +
>> +       if (server->le_io) {
>> +               g_io_channel_shutdown(server->le_io, FALSE, NULL);
>> +               g_io_channel_unref(server->le_io);
>> +       }
>> +
>> +       gatt_db_unref(server->db);
>> +       btd_adapter_unref(server->adapter);
>> +       free(server);
>> +}
>> +
>> +bool btd_gatt_database_init(void)
>> +{
>> +
>> +       info("Initializing GATT server");
>> +
>> +       servers = queue_new();
>> +       if (!servers) {
>> +               error("Failed to set up local GATT server");
>> +               return false;
>> +       }
>> +
>> +       return true;
>> +}
>> +
>> +void btd_gatt_database_cleanup(void)
>> +{
>> +       info("Cleaning up GATT server");
>> +
>> +       queue_destroy(servers, gatt_server_free);
>> +       servers = NULL;
>> +}
>> +
>> +static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
>> +{
>> +       struct btd_adapter *adapter;
>> +       struct btd_device *device;
>> +       uint8_t dst_type;
>> +       bdaddr_t src, dst;
>> +
>> +       DBG("New incoming LE ATT connection");
>> +
>> +       if (gerr) {
>> +               error("%s", gerr->message);
>> +               return;
>> +       }
>> +
>> +       bt_io_get(io, &gerr, BT_IO_OPT_SOURCE_BDADDR, &src,
>> +                                               BT_IO_OPT_DEST_BDADDR, &dst,
>> +                                               BT_IO_OPT_DEST_TYPE, &dst_type,
>> +                                               BT_IO_OPT_INVALID);
>> +       if (gerr) {
>> +               error("bt_io_get: %s", gerr->message);
>> +               g_error_free(gerr);
>> +               return;
>> +       }
>> +
>> +       adapter = adapter_find(&src);
>> +       if (!adapter)
>> +               return;
>> +
>> +       device = btd_adapter_get_device(adapter, &dst, dst_type);
>> +       if (!device)
>> +               return;
>> +
>> +       device_attach_att(device, io);
>> +}
>> +
>> +static bool match_adapter(const void *a, const void *b)
>> +{
>> +       const struct btd_gatt_database *server = a;
>> +       const struct btd_adapter *adapter = b;
>> +
>> +       return server->adapter == adapter;
>> +}
>> +
>> +bool btd_gatt_database_register_adapter(struct btd_adapter *adapter)
>> +{
>> +       struct btd_gatt_database *server;
>> +       GError *gerr = NULL;
>> +       const bdaddr_t *addr;
>> +
>> +       if (!adapter)
>> +               return false;
>> +
>> +       if (!servers) {
>> +               error("GATT server not initialized");
>> +               return false;
>> +       }
>> +
>> +       if (queue_find(servers, match_adapter, adapter)) {
>> +               error("Adapter already registered with GATT server");
>> +               return false;
>> +       }
>> +
>> +       server = new0(struct btd_gatt_database, 1);
>> +       if (!server)
>> +               return false;
>> +
>> +       server->adapter = btd_adapter_ref(adapter);
>> +       server->db = gatt_db_new();
>> +       if (!server->db)
>> +               goto fail;
>> +
>> +       addr = btd_adapter_get_address(adapter);
>> +       server->le_io = bt_io_listen(connect_cb, NULL, NULL, NULL, &gerr,
>> +                                       BT_IO_OPT_SOURCE_BDADDR, addr,
>> +                                       BT_IO_OPT_SOURCE_TYPE, BDADDR_LE_PUBLIC,
>> +                                       BT_IO_OPT_CID, ATT_CID,
>> +                                       BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
>> +                                       BT_IO_OPT_INVALID);
>> +       if (!server->le_io) {
>> +               error("Failed to start listening: %s", gerr->message);
>> +               g_error_free(gerr);
>> +               goto fail;
>> +       }
>> +
>> +       queue_push_tail(servers, server);
>> +
>> +       /* TODO: Set up GAP/GATT services */
>> +
>> +       return true;
>> +
>> +fail:
>> +       gatt_server_free(server);
>> +
>> +       return false;
>> +}
>> +
>> +void btd_gatt_database_unregister_adapter(struct btd_adapter *adapter)
>> +{
>> +       if (!adapter || !servers)
>> +               return;
>> +
>> +       queue_remove_all(servers, match_adapter, adapter, gatt_server_free);
>> +}
>> +
>> +struct gatt_db *btd_gatt_database_get_db(struct btd_adapter *adapter)
>> +{
>> +       struct btd_gatt_database *server;
>> +
>> +       if (!servers) {
>> +               error("GATT server not initialized");
>> +               return false;
>> +       }
>> +
>> +       server = queue_find(servers, match_adapter, adapter);
>> +       if (!server)
>> +               return false;
>> +
>> +       return server->db;
>> +}
>> diff --git a/src/gatt-database.h b/src/gatt-database.h
>> new file mode 100644
>> index 0000000..05e4ab9
>> --- /dev/null
>> +++ b/src/gatt-database.h
>> @@ -0,0 +1,26 @@
>> +/*
>> + *
>> + *  BlueZ - Bluetooth protocol stack for Linux
>> + *
>> + *  Copyright (C) 2015  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.
>> + *
>> + */
>> +
>> +bool btd_gatt_database_init(void);
>> +void btd_gatt_database_cleanup(void);
>> +
>> +bool btd_gatt_database_register_adapter(struct btd_adapter *adapter);
>> +void btd_gatt_database_unregister_adapter(struct btd_adapter *adapter);
>> +
>> +struct gatt_db *btd_gatt_database_get_db(struct btd_adapter *adapter);
>
> Did not like this API, imo it is better to have
> btd_gatt_database_new() and declare struct btd_gatt_database,
> otherwise for every use we need a lookup which is not efficient.
> Perhaps if you don't have time look at this today I may find sometime
> during the weekend or Monday morning.
>

I basically just followed src/attrib-server.c as an example, then
again that code didn't seem the cleanest to me either. I won't have
time to look at it today but I can revise it on Tuesday when I'm back,
though feel free to take a jab at it.

>> diff --git a/src/main.c b/src/main.c
>> index 061060d..4ccc18f 100644
>> --- a/src/main.c
>> +++ b/src/main.c
>> @@ -56,6 +56,7 @@
>>  #include "agent.h"
>>  #include "profile.h"
>>  #include "gatt.h"
>> +#include "gatt-database.h"
>>  #include "systemd.h"
>>
>>  #define BLUEZ_NAME "org.bluez"
>> @@ -579,6 +580,7 @@ int main(int argc, char *argv[])
>>         g_dbus_set_flags(gdbus_flags);
>>
>>         gatt_init();
>> +       btd_gatt_database_init();
>>
>>         if (adapter_init() < 0) {
>>                 error("Adapter handling initialization failed");
>> @@ -642,6 +644,7 @@ int main(int argc, char *argv[])
>>
>>         adapter_cleanup();
>>
>> +       btd_gatt_database_cleanup();
>>         gatt_cleanup();
>>
>>         rfkill_exit();
>> --
>> 2.2.0.rc0.207.ga3a616c
>>
>
>
>
> --
> Luiz Augusto von Dentz

Arman

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

* Re: [PATCH BlueZ v1 00/14] Rewrite local GATT server using shared/gatt
  2015-02-12  3:17 [PATCH BlueZ v1 00/14] Rewrite local GATT server using shared/gatt Arman Uguray
                   ` (13 preceding siblings ...)
  2015-02-12  3:17 ` [PATCH BlueZ v1 14/14] TODO: Update GATT items Arman Uguray
@ 2015-02-16  9:13 ` Luiz Augusto von Dentz
  14 siblings, 0 replies; 22+ messages in thread
From: Luiz Augusto von Dentz @ 2015-02-16  9:13 UTC (permalink / raw)
  To: Arman Uguray; +Cc: linux-bluetooth

Hi Arman,

On Thu, Feb 12, 2015 at 5:17 AM, Arman Uguray <armansito@chromium.org> wrote:
> *v1: Addressed comments by jamuraa and vudentz:
>   - Now passing bt_att instead of bdaddr_t in gatt_db callbacks and functions.
>     I have not run the tests on the android side here, so I would appreciate it
>     if you can run them.
>   - Renamed src/gatt-server to src/gatt-database for now. Added TODO item for
>     refactoring this later.
>   - Updated the TODO items for GATT.
>
> This patch set includes patches that rewrite the local GATT server using
> shared/gatt. This in effect invalidates the existing src/attrib-server in
> favor of a new src/gatt-server.
>
> Arman Uguray (14):
>   CHROMIUM: shared/att: Add bt_att_get_fd
>   CHROMIUM: shared/gatt: Pass bt_att instead of bdaddr_t
>   CHROMIUM: core: Introduce btd_gatt_database
>   CHROMIUM: core: Attach gatt-server to bt_att
>   CHROMIUM: core: gatt: Add GATT/GAP services to local db
>   CHROMIUM: core: Add GATT UUIDs to Adapter1.UUIDs
>   CHROMIUM: core: Support per-client CCC state
>   CHROMIUM: core: Setup added/removed handlers in GATT database
>   CHROMIUM: core: Add Service Changed characteristic
>   CHROMIUM: core: device: Add getter for GATT server
>   CHROMIUM: core: gatt-server: Send "Service Changed"
>   CHROMIUM: core: adapter: Send UUIDs changed for GATT services
>   CHROMIUM: shared/gatt: Don't incorrectly terminate discovery
>   TODO: Update GATT items.
>
>  Makefile.am               |   1 +
>  TODO                      |  54 ++--
>  android/gatt.c            |  98 ++++--
>  src/adapter.c             |  59 +++-
>  src/device.c              |  52 +++-
>  src/device.h              |   1 +
>  src/gatt-client.c         |   1 +
>  src/gatt-database.c       | 766 ++++++++++++++++++++++++++++++++++++++++++++++
>  src/gatt-database.h       |  29 ++
>  src/main.c                |   3 +
>  src/shared/att.c          |   8 +
>  src/shared/att.h          |   2 +
>  src/shared/gatt-client.c  |   6 +-
>  src/shared/gatt-db.c      |  10 +-
>  src/shared/gatt-db.h      |   8 +-
>  src/shared/gatt-helpers.c |   3 +-
>  src/shared/gatt-server.c  |  32 +-
>  tools/btgatt-server.c     |  18 +-
>  unit/test-gatt.c          |   5 +-
>  19 files changed, 1052 insertions(+), 104 deletions(-)
>  create mode 100644 src/gatt-database.c
>  create mode 100644 src/gatt-database.h
>
> --
> 2.2.0.rc0.207.ga3a616c

First 2 are now applied, I reworking the btd_gatt_database API.



-- 
Luiz Augusto von Dentz

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

* Re: [PATCH BlueZ v1 03/14] core: Introduce btd_gatt_database
  2015-02-13 16:21     ` Arman Uguray
@ 2015-02-17 12:03       ` Luiz Augusto von Dentz
  2015-02-18  0:43         ` Arman Uguray
  0 siblings, 1 reply; 22+ messages in thread
From: Luiz Augusto von Dentz @ 2015-02-17 12:03 UTC (permalink / raw)
  To: Arman Uguray; +Cc: linux-bluetooth

Hi Arman,

On Fri, Feb 13, 2015 at 6:21 PM, Arman Uguray <armansito@chromium.org> wrote:
> Hi Luiz,
>
>> On Fri, Feb 13, 2015 at 8:06 AM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>> Hi Arman,
>>
>> On Thu, Feb 12, 2015 at 5:17 AM, Arman Uguray <armansito@chromium.org> wrote:
>>> This patch introduces src/gatt-database.* which handles incoming ATT
>>> connections, manages per-adapter shared/gatt-db instances, and routes
>>> connections to the corresponding device object. This is the layer that
>>> will perform all the CCC management and Service Changed handling.
>>> ---
>>>  Makefile.am         |   1 +
>>>  src/adapter.c       |   8 ++-
>>>  src/gatt-database.c | 204 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  src/gatt-database.h |  26 +++++++
>>>  src/main.c          |   3 +
>>>  5 files changed, 240 insertions(+), 2 deletions(-)
>>>  create mode 100644 src/gatt-database.c
>>>  create mode 100644 src/gatt-database.h
>>>
>>> diff --git a/Makefile.am b/Makefile.am
>>> index 60811f1..4407355 100644
>>> --- a/Makefile.am
>>> +++ b/Makefile.am
>>> @@ -167,6 +167,7 @@ src_bluetoothd_SOURCES = $(builtin_sources) \
>>>                         src/sdpd-server.c src/sdpd-request.c \
>>>                         src/sdpd-service.c src/sdpd-database.c \
>>>                         src/attrib-server.h src/attrib-server.c \
>>> +                       src/gatt-database.h src/gatt-database.c \
>>>                         src/sdp-xml.h src/sdp-xml.c \
>>>                         src/sdp-client.h src/sdp-client.c \
>>>                         src/textfile.h src/textfile.c \
>>> diff --git a/src/adapter.c b/src/adapter.c
>>> index 1839286..0253d08 100644
>>> --- a/src/adapter.c
>>> +++ b/src/adapter.c
>>> @@ -68,6 +68,7 @@
>>>  #include "attrib/att.h"
>>>  #include "attrib/gatt.h"
>>>  #include "attrib-server.h"
>>> +#include "gatt-database.h"
>>>  #include "eir.h"
>>>
>>>  #define ADAPTER_INTERFACE      "org.bluez.Adapter1"
>>> @@ -302,6 +303,7 @@ static void dev_class_changed_callback(uint16_t index, uint16_t length,
>>>         appearance[1] = rp->val[1] & 0x1f;      /* removes service class */
>>>         appearance[2] = rp->val[2];
>>>
>>> +       /* TODO: Do this through btd_gatt_database instead */
>>>         attrib_gap_set(adapter, GATT_CHARAC_APPEARANCE, appearance, 2);
>>>  }
>>>
>>> @@ -4014,6 +4016,7 @@ static void convert_sdp_entry(char *key, char *value, void *user_data)
>>>         if (record_has_uuid(rec, att_uuid))
>>>                 goto failed;
>>>
>>> +       /* TODO: Do this through btd_gatt_database */
>>>         if (!gatt_parse_record(rec, &uuid, &psm, &start, &end))
>>>                 goto failed;
>>>
>>> @@ -4548,7 +4551,7 @@ static void adapter_remove(struct btd_adapter *adapter)
>>>         adapter->devices = NULL;
>>>
>>>         unload_drivers(adapter);
>>> -       btd_adapter_gatt_server_stop(adapter);
>>> +       btd_gatt_database_unregister_adapter(adapter);
>>>
>>>         g_slist_free(adapter->pin_callbacks);
>>>         adapter->pin_callbacks = NULL;
>>> @@ -6590,7 +6593,8 @@ static int adapter_register(struct btd_adapter *adapter)
>>>                 agent_unref(agent);
>>>         }
>>>
>>> -       btd_adapter_gatt_server_start(adapter);
>>> +       if (!btd_gatt_database_register_adapter(adapter))
>>> +               error("Failed to register adapter with GATT server");
>>>
>>>         load_config(adapter);
>>>         fix_storage(adapter);
>>> diff --git a/src/gatt-database.c b/src/gatt-database.c
>>> new file mode 100644
>>> index 0000000..57bdf1a
>>> --- /dev/null
>>> +++ b/src/gatt-database.c
>>> @@ -0,0 +1,204 @@
>>> +/*
>>> + *
>>> + *  BlueZ - Bluetooth protocol stack for Linux
>>> + *
>>> + *  Copyright (C) 2015  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.
>>> + *
>>> + */
>>> +
>>> +#ifdef HAVE_CONFIG_H
>>> +#include <config.h>
>>> +#endif
>>> +
>>> +#include <stdint.h>
>>> +#include <stdlib.h>
>>> +
>>> +#include "lib/uuid.h"
>>> +#include "btio/btio.h"
>>> +#include "src/shared/util.h"
>>> +#include "src/shared/queue.h"
>>> +#include "src/shared/att.h"
>>> +#include "src/shared/gatt-db.h"
>>> +#include "log.h"
>>> +#include "adapter.h"
>>> +#include "device.h"
>>> +#include "gatt-database.h"
>>> +
>>> +#ifndef ATT_CID
>>> +#define ATT_CID 4
>>> +#endif
>>> +
>>> +static struct queue *servers = NULL;
>>> +
>>> +struct btd_gatt_database {
>>> +       struct btd_adapter *adapter;
>>> +       struct gatt_db *db;
>>> +       GIOChannel *le_io;
>>> +};
>>> +
>>> +static void gatt_server_free(void *data)
>>> +{
>>> +       struct btd_gatt_database *server = data;
>>> +
>>> +       if (server->le_io) {
>>> +               g_io_channel_shutdown(server->le_io, FALSE, NULL);
>>> +               g_io_channel_unref(server->le_io);
>>> +       }
>>> +
>>> +       gatt_db_unref(server->db);
>>> +       btd_adapter_unref(server->adapter);
>>> +       free(server);
>>> +}
>>> +
>>> +bool btd_gatt_database_init(void)
>>> +{
>>> +
>>> +       info("Initializing GATT server");
>>> +
>>> +       servers = queue_new();
>>> +       if (!servers) {
>>> +               error("Failed to set up local GATT server");
>>> +               return false;
>>> +       }
>>> +
>>> +       return true;
>>> +}
>>> +
>>> +void btd_gatt_database_cleanup(void)
>>> +{
>>> +       info("Cleaning up GATT server");
>>> +
>>> +       queue_destroy(servers, gatt_server_free);
>>> +       servers = NULL;
>>> +}
>>> +
>>> +static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
>>> +{
>>> +       struct btd_adapter *adapter;
>>> +       struct btd_device *device;
>>> +       uint8_t dst_type;
>>> +       bdaddr_t src, dst;
>>> +
>>> +       DBG("New incoming LE ATT connection");
>>> +
>>> +       if (gerr) {
>>> +               error("%s", gerr->message);
>>> +               return;
>>> +       }
>>> +
>>> +       bt_io_get(io, &gerr, BT_IO_OPT_SOURCE_BDADDR, &src,
>>> +                                               BT_IO_OPT_DEST_BDADDR, &dst,
>>> +                                               BT_IO_OPT_DEST_TYPE, &dst_type,
>>> +                                               BT_IO_OPT_INVALID);
>>> +       if (gerr) {
>>> +               error("bt_io_get: %s", gerr->message);
>>> +               g_error_free(gerr);
>>> +               return;
>>> +       }
>>> +
>>> +       adapter = adapter_find(&src);
>>> +       if (!adapter)
>>> +               return;
>>> +
>>> +       device = btd_adapter_get_device(adapter, &dst, dst_type);
>>> +       if (!device)
>>> +               return;
>>> +
>>> +       device_attach_att(device, io);
>>> +}
>>> +
>>> +static bool match_adapter(const void *a, const void *b)
>>> +{
>>> +       const struct btd_gatt_database *server = a;
>>> +       const struct btd_adapter *adapter = b;
>>> +
>>> +       return server->adapter == adapter;
>>> +}
>>> +
>>> +bool btd_gatt_database_register_adapter(struct btd_adapter *adapter)
>>> +{
>>> +       struct btd_gatt_database *server;
>>> +       GError *gerr = NULL;
>>> +       const bdaddr_t *addr;
>>> +
>>> +       if (!adapter)
>>> +               return false;
>>> +
>>> +       if (!servers) {
>>> +               error("GATT server not initialized");
>>> +               return false;
>>> +       }
>>> +
>>> +       if (queue_find(servers, match_adapter, adapter)) {
>>> +               error("Adapter already registered with GATT server");
>>> +               return false;
>>> +       }
>>> +
>>> +       server = new0(struct btd_gatt_database, 1);
>>> +       if (!server)
>>> +               return false;
>>> +
>>> +       server->adapter = btd_adapter_ref(adapter);
>>> +       server->db = gatt_db_new();
>>> +       if (!server->db)
>>> +               goto fail;
>>> +
>>> +       addr = btd_adapter_get_address(adapter);
>>> +       server->le_io = bt_io_listen(connect_cb, NULL, NULL, NULL, &gerr,
>>> +                                       BT_IO_OPT_SOURCE_BDADDR, addr,
>>> +                                       BT_IO_OPT_SOURCE_TYPE, BDADDR_LE_PUBLIC,
>>> +                                       BT_IO_OPT_CID, ATT_CID,
>>> +                                       BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
>>> +                                       BT_IO_OPT_INVALID);
>>> +       if (!server->le_io) {
>>> +               error("Failed to start listening: %s", gerr->message);
>>> +               g_error_free(gerr);
>>> +               goto fail;
>>> +       }
>>> +
>>> +       queue_push_tail(servers, server);
>>> +
>>> +       /* TODO: Set up GAP/GATT services */
>>> +
>>> +       return true;
>>> +
>>> +fail:
>>> +       gatt_server_free(server);
>>> +
>>> +       return false;
>>> +}
>>> +
>>> +void btd_gatt_database_unregister_adapter(struct btd_adapter *adapter)
>>> +{
>>> +       if (!adapter || !servers)
>>> +               return;
>>> +
>>> +       queue_remove_all(servers, match_adapter, adapter, gatt_server_free);
>>> +}
>>> +
>>> +struct gatt_db *btd_gatt_database_get_db(struct btd_adapter *adapter)
>>> +{
>>> +       struct btd_gatt_database *server;
>>> +
>>> +       if (!servers) {
>>> +               error("GATT server not initialized");
>>> +               return false;
>>> +       }
>>> +
>>> +       server = queue_find(servers, match_adapter, adapter);
>>> +       if (!server)
>>> +               return false;
>>> +
>>> +       return server->db;
>>> +}
>>> diff --git a/src/gatt-database.h b/src/gatt-database.h
>>> new file mode 100644
>>> index 0000000..05e4ab9
>>> --- /dev/null
>>> +++ b/src/gatt-database.h
>>> @@ -0,0 +1,26 @@
>>> +/*
>>> + *
>>> + *  BlueZ - Bluetooth protocol stack for Linux
>>> + *
>>> + *  Copyright (C) 2015  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.
>>> + *
>>> + */
>>> +
>>> +bool btd_gatt_database_init(void);
>>> +void btd_gatt_database_cleanup(void);
>>> +
>>> +bool btd_gatt_database_register_adapter(struct btd_adapter *adapter);
>>> +void btd_gatt_database_unregister_adapter(struct btd_adapter *adapter);
>>> +
>>> +struct gatt_db *btd_gatt_database_get_db(struct btd_adapter *adapter);
>>
>> Did not like this API, imo it is better to have
>> btd_gatt_database_new() and declare struct btd_gatt_database,
>> otherwise for every use we need a lookup which is not efficient.
>> Perhaps if you don't have time look at this today I may find sometime
>> during the weekend or Monday morning.
>>
>
> I basically just followed src/attrib-server.c as an example, then
> again that code didn't seem the cleanest to me either. I won't have
> time to look at it today but I can revise it on Tuesday when I'm back,
> though feel free to take a jab at it.
>
>>> diff --git a/src/main.c b/src/main.c
>>> index 061060d..4ccc18f 100644
>>> --- a/src/main.c
>>> +++ b/src/main.c
>>> @@ -56,6 +56,7 @@
>>>  #include "agent.h"
>>>  #include "profile.h"
>>>  #include "gatt.h"
>>> +#include "gatt-database.h"
>>>  #include "systemd.h"
>>>
>>>  #define BLUEZ_NAME "org.bluez"
>>> @@ -579,6 +580,7 @@ int main(int argc, char *argv[])
>>>         g_dbus_set_flags(gdbus_flags);
>>>
>>>         gatt_init();
>>> +       btd_gatt_database_init();
>>>
>>>         if (adapter_init() < 0) {
>>>                 error("Adapter handling initialization failed");
>>> @@ -642,6 +644,7 @@ int main(int argc, char *argv[])
>>>
>>>         adapter_cleanup();
>>>
>>> +       btd_gatt_database_cleanup();
>>>         gatt_cleanup();
>>>
>>>         rfkill_exit();
>>> --
>>> 2.2.0.rc0.207.ga3a616c
>>>

I applied this one, not that I did changed the API quite a bit so we
don't need to do extra lookups, I also left comments how you should
proceed, for instance it probably makes more sense to store the
bt_gatt_server instance inside btd_gatt_database, at least it does not
sound useful in device.c. Also it is probably necessary to have
btd_adapter_get_database but the rest should be pretty straight
forward.


-- 
Luiz Augusto von Dentz

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

* Re: [PATCH BlueZ v1 03/14] core: Introduce btd_gatt_database
  2015-02-17 12:03       ` Luiz Augusto von Dentz
@ 2015-02-18  0:43         ` Arman Uguray
  0 siblings, 0 replies; 22+ messages in thread
From: Arman Uguray @ 2015-02-18  0:43 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

Hi Luiz,

> On Tue, Feb 17, 2015 at 4:03 AM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
> Hi Arman,
>
> On Fri, Feb 13, 2015 at 6:21 PM, Arman Uguray <armansito@chromium.org> wrote:
>> Hi Luiz,
>>
>>> On Fri, Feb 13, 2015 at 8:06 AM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>>> Hi Arman,
>>>
>>> On Thu, Feb 12, 2015 at 5:17 AM, Arman Uguray <armansito@chromium.org> wrote:
>>>> This patch introduces src/gatt-database.* which handles incoming ATT
>>>> connections, manages per-adapter shared/gatt-db instances, and routes
>>>> connections to the corresponding device object. This is the layer that
>>>> will perform all the CCC management and Service Changed handling.
>>>> ---
>>>>  Makefile.am         |   1 +
>>>>  src/adapter.c       |   8 ++-
>>>>  src/gatt-database.c | 204 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  src/gatt-database.h |  26 +++++++
>>>>  src/main.c          |   3 +
>>>>  5 files changed, 240 insertions(+), 2 deletions(-)
>>>>  create mode 100644 src/gatt-database.c
>>>>  create mode 100644 src/gatt-database.h
>>>>
>>>> diff --git a/Makefile.am b/Makefile.am
>>>> index 60811f1..4407355 100644
>>>> --- a/Makefile.am
>>>> +++ b/Makefile.am
>>>> @@ -167,6 +167,7 @@ src_bluetoothd_SOURCES = $(builtin_sources) \
>>>>                         src/sdpd-server.c src/sdpd-request.c \
>>>>                         src/sdpd-service.c src/sdpd-database.c \
>>>>                         src/attrib-server.h src/attrib-server.c \
>>>> +                       src/gatt-database.h src/gatt-database.c \
>>>>                         src/sdp-xml.h src/sdp-xml.c \
>>>>                         src/sdp-client.h src/sdp-client.c \
>>>>                         src/textfile.h src/textfile.c \
>>>> diff --git a/src/adapter.c b/src/adapter.c
>>>> index 1839286..0253d08 100644
>>>> --- a/src/adapter.c
>>>> +++ b/src/adapter.c
>>>> @@ -68,6 +68,7 @@
>>>>  #include "attrib/att.h"
>>>>  #include "attrib/gatt.h"
>>>>  #include "attrib-server.h"
>>>> +#include "gatt-database.h"
>>>>  #include "eir.h"
>>>>
>>>>  #define ADAPTER_INTERFACE      "org.bluez.Adapter1"
>>>> @@ -302,6 +303,7 @@ static void dev_class_changed_callback(uint16_t index, uint16_t length,
>>>>         appearance[1] = rp->val[1] & 0x1f;      /* removes service class */
>>>>         appearance[2] = rp->val[2];
>>>>
>>>> +       /* TODO: Do this through btd_gatt_database instead */
>>>>         attrib_gap_set(adapter, GATT_CHARAC_APPEARANCE, appearance, 2);
>>>>  }
>>>>
>>>> @@ -4014,6 +4016,7 @@ static void convert_sdp_entry(char *key, char *value, void *user_data)
>>>>         if (record_has_uuid(rec, att_uuid))
>>>>                 goto failed;
>>>>
>>>> +       /* TODO: Do this through btd_gatt_database */
>>>>         if (!gatt_parse_record(rec, &uuid, &psm, &start, &end))
>>>>                 goto failed;
>>>>
>>>> @@ -4548,7 +4551,7 @@ static void adapter_remove(struct btd_adapter *adapter)
>>>>         adapter->devices = NULL;
>>>>
>>>>         unload_drivers(adapter);
>>>> -       btd_adapter_gatt_server_stop(adapter);
>>>> +       btd_gatt_database_unregister_adapter(adapter);
>>>>
>>>>         g_slist_free(adapter->pin_callbacks);
>>>>         adapter->pin_callbacks = NULL;
>>>> @@ -6590,7 +6593,8 @@ static int adapter_register(struct btd_adapter *adapter)
>>>>                 agent_unref(agent);
>>>>         }
>>>>
>>>> -       btd_adapter_gatt_server_start(adapter);
>>>> +       if (!btd_gatt_database_register_adapter(adapter))
>>>> +               error("Failed to register adapter with GATT server");
>>>>
>>>>         load_config(adapter);
>>>>         fix_storage(adapter);
>>>> diff --git a/src/gatt-database.c b/src/gatt-database.c
>>>> new file mode 100644
>>>> index 0000000..57bdf1a
>>>> --- /dev/null
>>>> +++ b/src/gatt-database.c
>>>> @@ -0,0 +1,204 @@
>>>> +/*
>>>> + *
>>>> + *  BlueZ - Bluetooth protocol stack for Linux
>>>> + *
>>>> + *  Copyright (C) 2015  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.
>>>> + *
>>>> + */
>>>> +
>>>> +#ifdef HAVE_CONFIG_H
>>>> +#include <config.h>
>>>> +#endif
>>>> +
>>>> +#include <stdint.h>
>>>> +#include <stdlib.h>
>>>> +
>>>> +#include "lib/uuid.h"
>>>> +#include "btio/btio.h"
>>>> +#include "src/shared/util.h"
>>>> +#include "src/shared/queue.h"
>>>> +#include "src/shared/att.h"
>>>> +#include "src/shared/gatt-db.h"
>>>> +#include "log.h"
>>>> +#include "adapter.h"
>>>> +#include "device.h"
>>>> +#include "gatt-database.h"
>>>> +
>>>> +#ifndef ATT_CID
>>>> +#define ATT_CID 4
>>>> +#endif
>>>> +
>>>> +static struct queue *servers = NULL;
>>>> +
>>>> +struct btd_gatt_database {
>>>> +       struct btd_adapter *adapter;
>>>> +       struct gatt_db *db;
>>>> +       GIOChannel *le_io;
>>>> +};
>>>> +
>>>> +static void gatt_server_free(void *data)
>>>> +{
>>>> +       struct btd_gatt_database *server = data;
>>>> +
>>>> +       if (server->le_io) {
>>>> +               g_io_channel_shutdown(server->le_io, FALSE, NULL);
>>>> +               g_io_channel_unref(server->le_io);
>>>> +       }
>>>> +
>>>> +       gatt_db_unref(server->db);
>>>> +       btd_adapter_unref(server->adapter);
>>>> +       free(server);
>>>> +}
>>>> +
>>>> +bool btd_gatt_database_init(void)
>>>> +{
>>>> +
>>>> +       info("Initializing GATT server");
>>>> +
>>>> +       servers = queue_new();
>>>> +       if (!servers) {
>>>> +               error("Failed to set up local GATT server");
>>>> +               return false;
>>>> +       }
>>>> +
>>>> +       return true;
>>>> +}
>>>> +
>>>> +void btd_gatt_database_cleanup(void)
>>>> +{
>>>> +       info("Cleaning up GATT server");
>>>> +
>>>> +       queue_destroy(servers, gatt_server_free);
>>>> +       servers = NULL;
>>>> +}
>>>> +
>>>> +static void connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
>>>> +{
>>>> +       struct btd_adapter *adapter;
>>>> +       struct btd_device *device;
>>>> +       uint8_t dst_type;
>>>> +       bdaddr_t src, dst;
>>>> +
>>>> +       DBG("New incoming LE ATT connection");
>>>> +
>>>> +       if (gerr) {
>>>> +               error("%s", gerr->message);
>>>> +               return;
>>>> +       }
>>>> +
>>>> +       bt_io_get(io, &gerr, BT_IO_OPT_SOURCE_BDADDR, &src,
>>>> +                                               BT_IO_OPT_DEST_BDADDR, &dst,
>>>> +                                               BT_IO_OPT_DEST_TYPE, &dst_type,
>>>> +                                               BT_IO_OPT_INVALID);
>>>> +       if (gerr) {
>>>> +               error("bt_io_get: %s", gerr->message);
>>>> +               g_error_free(gerr);
>>>> +               return;
>>>> +       }
>>>> +
>>>> +       adapter = adapter_find(&src);
>>>> +       if (!adapter)
>>>> +               return;
>>>> +
>>>> +       device = btd_adapter_get_device(adapter, &dst, dst_type);
>>>> +       if (!device)
>>>> +               return;
>>>> +
>>>> +       device_attach_att(device, io);
>>>> +}
>>>> +
>>>> +static bool match_adapter(const void *a, const void *b)
>>>> +{
>>>> +       const struct btd_gatt_database *server = a;
>>>> +       const struct btd_adapter *adapter = b;
>>>> +
>>>> +       return server->adapter == adapter;
>>>> +}
>>>> +
>>>> +bool btd_gatt_database_register_adapter(struct btd_adapter *adapter)
>>>> +{
>>>> +       struct btd_gatt_database *server;
>>>> +       GError *gerr = NULL;
>>>> +       const bdaddr_t *addr;
>>>> +
>>>> +       if (!adapter)
>>>> +               return false;
>>>> +
>>>> +       if (!servers) {
>>>> +               error("GATT server not initialized");
>>>> +               return false;
>>>> +       }
>>>> +
>>>> +       if (queue_find(servers, match_adapter, adapter)) {
>>>> +               error("Adapter already registered with GATT server");
>>>> +               return false;
>>>> +       }
>>>> +
>>>> +       server = new0(struct btd_gatt_database, 1);
>>>> +       if (!server)
>>>> +               return false;
>>>> +
>>>> +       server->adapter = btd_adapter_ref(adapter);
>>>> +       server->db = gatt_db_new();
>>>> +       if (!server->db)
>>>> +               goto fail;
>>>> +
>>>> +       addr = btd_adapter_get_address(adapter);
>>>> +       server->le_io = bt_io_listen(connect_cb, NULL, NULL, NULL, &gerr,
>>>> +                                       BT_IO_OPT_SOURCE_BDADDR, addr,
>>>> +                                       BT_IO_OPT_SOURCE_TYPE, BDADDR_LE_PUBLIC,
>>>> +                                       BT_IO_OPT_CID, ATT_CID,
>>>> +                                       BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
>>>> +                                       BT_IO_OPT_INVALID);
>>>> +       if (!server->le_io) {
>>>> +               error("Failed to start listening: %s", gerr->message);
>>>> +               g_error_free(gerr);
>>>> +               goto fail;
>>>> +       }
>>>> +
>>>> +       queue_push_tail(servers, server);
>>>> +
>>>> +       /* TODO: Set up GAP/GATT services */
>>>> +
>>>> +       return true;
>>>> +
>>>> +fail:
>>>> +       gatt_server_free(server);
>>>> +
>>>> +       return false;
>>>> +}
>>>> +
>>>> +void btd_gatt_database_unregister_adapter(struct btd_adapter *adapter)
>>>> +{
>>>> +       if (!adapter || !servers)
>>>> +               return;
>>>> +
>>>> +       queue_remove_all(servers, match_adapter, adapter, gatt_server_free);
>>>> +}
>>>> +
>>>> +struct gatt_db *btd_gatt_database_get_db(struct btd_adapter *adapter)
>>>> +{
>>>> +       struct btd_gatt_database *server;
>>>> +
>>>> +       if (!servers) {
>>>> +               error("GATT server not initialized");
>>>> +               return false;
>>>> +       }
>>>> +
>>>> +       server = queue_find(servers, match_adapter, adapter);
>>>> +       if (!server)
>>>> +               return false;
>>>> +
>>>> +       return server->db;
>>>> +}
>>>> diff --git a/src/gatt-database.h b/src/gatt-database.h
>>>> new file mode 100644
>>>> index 0000000..05e4ab9
>>>> --- /dev/null
>>>> +++ b/src/gatt-database.h
>>>> @@ -0,0 +1,26 @@
>>>> +/*
>>>> + *
>>>> + *  BlueZ - Bluetooth protocol stack for Linux
>>>> + *
>>>> + *  Copyright (C) 2015  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.
>>>> + *
>>>> + */
>>>> +
>>>> +bool btd_gatt_database_init(void);
>>>> +void btd_gatt_database_cleanup(void);
>>>> +
>>>> +bool btd_gatt_database_register_adapter(struct btd_adapter *adapter);
>>>> +void btd_gatt_database_unregister_adapter(struct btd_adapter *adapter);
>>>> +
>>>> +struct gatt_db *btd_gatt_database_get_db(struct btd_adapter *adapter);
>>>
>>> Did not like this API, imo it is better to have
>>> btd_gatt_database_new() and declare struct btd_gatt_database,
>>> otherwise for every use we need a lookup which is not efficient.
>>> Perhaps if you don't have time look at this today I may find sometime
>>> during the weekend or Monday morning.
>>>
>>
>> I basically just followed src/attrib-server.c as an example, then
>> again that code didn't seem the cleanest to me either. I won't have
>> time to look at it today but I can revise it on Tuesday when I'm back,
>> though feel free to take a jab at it.
>>
>>>> diff --git a/src/main.c b/src/main.c
>>>> index 061060d..4ccc18f 100644
>>>> --- a/src/main.c
>>>> +++ b/src/main.c
>>>> @@ -56,6 +56,7 @@
>>>>  #include "agent.h"
>>>>  #include "profile.h"
>>>>  #include "gatt.h"
>>>> +#include "gatt-database.h"
>>>>  #include "systemd.h"
>>>>
>>>>  #define BLUEZ_NAME "org.bluez"
>>>> @@ -579,6 +580,7 @@ int main(int argc, char *argv[])
>>>>         g_dbus_set_flags(gdbus_flags);
>>>>
>>>>         gatt_init();
>>>> +       btd_gatt_database_init();
>>>>
>>>>         if (adapter_init() < 0) {
>>>>                 error("Adapter handling initialization failed");
>>>> @@ -642,6 +644,7 @@ int main(int argc, char *argv[])
>>>>
>>>>         adapter_cleanup();
>>>>
>>>> +       btd_gatt_database_cleanup();
>>>>         gatt_cleanup();
>>>>
>>>>         rfkill_exit();
>>>> --
>>>> 2.2.0.rc0.207.ga3a616c
>>>>
>
> I applied this one, not that I did changed the API quite a bit so we
> don't need to do extra lookups, I also left comments how you should
> proceed, for instance it probably makes more sense to store the
> bt_gatt_server instance inside btd_gatt_database, at least it does not
> sound useful in device.c. Also it is probably necessary to have
> btd_adapter_get_database but the rest should be pretty straight
> forward.
>

I'm taking a second look at storing the bt_gatt_server in
btd_gatt_database, and I wonder if btd_device makes more sense the way
the code currently is. Basically, we currently have the bt_att
transport in btd_device and this likely won't change until we have
cleaned up all the GAttrib dependencies. Currently there are two ways
an ATT transport can be created:

  1. Outgoing connection: e.g. via a D-Bus call to Device1.Connect, or
outgoing auto-connect.
  2. Incoming connection: btd_gatt_server's listening socket receives
the connection event.

In both cases we want to create both a bt_gatt_client and a
bt_gatt_server. The way things are right now, it's much cleaner to
create and store the bt_gatt_server alongside the bt_gatt_client
within btd_device, since the client code already lives in btd_device.
Otherwise, we'll have to create some kind of callback mechanism (or
btd_gatt_database function) for attaching a server, which will lead to
more spaghetti code. I already don't like the whole device_attach_att
code path, but this is necessary for backwards compatibility reasons
given all the profiles that still use GAttrib directly through
btd_device.

I suggest keeping bt_gatt_server inside btd_device for now. I'm
already adding a TODO item for moving GATT related code to its own
module as you suggested, so we can do a simple clean-up once the
dependencies on the older code have been removed.

>
> --
> Luiz Augusto von Dentz

Thanks,
Arman

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

end of thread, other threads:[~2015-02-18  0:43 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-12  3:17 [PATCH BlueZ v1 00/14] Rewrite local GATT server using shared/gatt Arman Uguray
2015-02-12  3:17 ` [PATCH BlueZ v1 01/14] shared/att: Add bt_att_get_fd Arman Uguray
2015-02-12 13:40   ` Luiz Augusto von Dentz
2015-02-12 18:21     ` Arman Uguray
2015-02-12  3:17 ` [PATCH BlueZ v1 02/14] shared/gatt: Pass bt_att instead of bdaddr_t Arman Uguray
2015-02-12  3:17 ` [PATCH BlueZ v1 03/14] core: Introduce btd_gatt_database Arman Uguray
2015-02-13 16:06   ` Luiz Augusto von Dentz
2015-02-13 16:21     ` Arman Uguray
2015-02-17 12:03       ` Luiz Augusto von Dentz
2015-02-18  0:43         ` Arman Uguray
2015-02-12  3:17 ` [PATCH BlueZ v1 04/14] core: Attach gatt-server to bt_att Arman Uguray
2015-02-12  3:17 ` [PATCH BlueZ v1 05/14] core: gatt: Add GATT/GAP services to local db Arman Uguray
2015-02-12  3:17 ` [PATCH BlueZ v1 06/14] core: Add GATT UUIDs to Adapter1.UUIDs Arman Uguray
2015-02-12  3:17 ` [PATCH BlueZ v1 07/14] core: Support per-client CCC state Arman Uguray
2015-02-12  3:17 ` [PATCH BlueZ v1 08/14] core: Setup added/removed handlers in GATT database Arman Uguray
2015-02-12  3:17 ` [PATCH BlueZ v1 09/14] core: Add Service Changed characteristic Arman Uguray
2015-02-12  3:17 ` [PATCH BlueZ v1 10/14] core: device: Add getter for GATT server Arman Uguray
2015-02-12  3:17 ` [PATCH BlueZ v1 11/14] core: gatt-server: Send "Service Changed" Arman Uguray
2015-02-12  3:17 ` [PATCH BlueZ v1 12/14] core: adapter: Send UUIDs changed for GATT services Arman Uguray
2015-02-12  3:17 ` [PATCH BlueZ v1 13/14] shared/gatt: Don't incorrectly terminate discovery Arman Uguray
2015-02-12  3:17 ` [PATCH BlueZ v1 14/14] TODO: Update GATT items Arman Uguray
2015-02-16  9:13 ` [PATCH BlueZ v1 00/14] Rewrite local GATT server using shared/gatt Luiz Augusto von Dentz

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.