All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH BlueZ v1 00/17] Implement GATT server D-Bus API
@ 2015-02-26  5:13 Arman Uguray
  2015-02-26  5:13 ` [PATCH BlueZ v1 01/17] shared/gatt: Call bt_att_cancel_all in unref Arman Uguray
                   ` (16 more replies)
  0 siblings, 17 replies; 28+ messages in thread
From: Arman Uguray @ 2015-02-26  5:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Arman Uguray

*v1: - Removed src/gatt.* based on email discussion.

     - After some discussions on IRC, made ObjectManager mandatory on a
       per-service-subtree basis to reduce the number of unnecessary
       object processing and caching. One initial idea was to pass the
       service subtree directly to RegisterService but one of the
       concerns was that we should allow applications to use existing
       tooling for the standard ObjectManager interface.

     - Minor fixes based on comments, also fixed things so that
       descriptors with CCC and CEP UUIDs cannot be registered by
       external apps.

     - Included a fix for recent crashes seen in unit/test-gatt.

This patch set implements the GATT server D-Bus API and provides an
example application in Python to demonstrate how the API can be used
to export GATT services of various functionality.

Some of the basics of the code is very similar to what was already in
src/gatt-dbus.c, especially the bits that use gdbus/client to fetch
and interact with remote objects. However, src/gatt-dbus had a lot of
code that used the now-defunct src/gatt (which was going to be the new
GATT abstraction for bluetoothd before we built shared/gatt), so I
decided to remove gatt-dbus entirely and start from scratch with
src/gatt-manager.

Apart from these the code is fairly straight-forward. New additions to
src/gatt-database have been made to allow upper layers to store CCC
descriptors and send out notifications. The API automatically creates
CCC and CEP (Characteristic Extended Properties) descriptors based on
characteristic properties.

Arman Uguray (17):
  shared/gatt: Call bt_att_cancel_all in unref
  gdbus/client: Don't GetManagedObjects w/o handlers
  gdbus/client: Allow specifying ObjectManager path
  doc/gatt-api.txt: New ObjectManager requirements
  core/gatt: Add GattManager1 stubs
  core/gatt: Implement GattManager1.RegisterService
  core/gatt: Register characteristics
  core/gatt: Support ReadValue for characteristics
  core/gatt: Support WriteValue for characteristics
  core/gatt: Make CCC addition API public
  core/gatt: Create CCC for external characteristic
  core/gatt: Add btd_gatt_database_notify function
  core/gatt: Send not/ind for D-Bus characteristics
  core/gatt: Create CEP for external characteristic
  core/gatt: Register descriptors
  core/gatt: Support descriptor reads/writes
  tools: Added a Python example for GATT server API

 Makefile.am              |    3 +-
 client/main.c            |    2 +-
 doc/gatt-api.txt         |   74 ++-
 gdbus/client.c           |   26 +-
 gdbus/gdbus.h            |    5 +-
 plugins/hostname.c       |    2 +-
 profiles/iap/main.c      |    2 +-
 src/adapter.c            |   18 +-
 src/gatt-database.c      |  114 +++-
 src/gatt-database.h      |   16 +
 src/gatt-dbus.c          |  658 ----------------------
 src/gatt-dbus.h          |   25 -
 src/gatt-manager.c       | 1383 ++++++++++++++++++++++++++++++++++++++++++++++
 src/gatt-manager.h       |   23 +
 src/gatt.c               |  321 -----------
 src/gatt.h               |  121 ----
 src/main.c               |    5 -
 src/shared/gatt-client.c |    9 +
 tools/bluetooth-player.c |    2 +-
 tools/gap-tester.c       |    3 +-
 tools/gatt-example       |  463 ++++++++++++++++
 tools/gatt-service.c     |    2 +-
 tools/mpris-proxy.c      |    2 +-
 tools/obexctl.c          |    2 +-
 unit/test-gdbus-client.c |   39 +-
 25 files changed, 2130 insertions(+), 1190 deletions(-)
 delete mode 100644 src/gatt-dbus.c
 delete mode 100644 src/gatt-dbus.h
 create mode 100644 src/gatt-manager.c
 create mode 100644 src/gatt-manager.h
 delete mode 100644 src/gatt.c
 delete mode 100644 src/gatt.h
 create mode 100755 tools/gatt-example

-- 
2.2.0.rc0.207.ga3a616c


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

* [PATCH BlueZ v1 01/17] shared/gatt: Call bt_att_cancel_all in unref
  2015-02-26  5:13 [PATCH BlueZ v1 00/17] Implement GATT server D-Bus API Arman Uguray
@ 2015-02-26  5:13 ` Arman Uguray
  2015-02-26  8:44   ` Luiz Augusto von Dentz
  2015-02-26  5:13 ` [PATCH BlueZ v1 02/17] gdbus/client: Don't GetManagedObjects w/o handlers Arman Uguray
                   ` (15 subsequent siblings)
  16 siblings, 1 reply; 28+ messages in thread
From: Arman Uguray @ 2015-02-26  5:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Arman Uguray

This patch fixes a potential invalid access that can occur if bt_att
outlives bt_gatt_client and if there are pending discovery requests
when bt_gatt_client_unref is called.

This patch fixes this by canceling all ATT operations that are handled
by the bt_att in bt_gatt_client_unref. The proper fix, however, is to
make the discovery procedures cancelable and to cancel those instead of
canceling everything. A TODO has been added to fix this later.
---
 src/shared/gatt-client.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index cc972d6..92e72e2 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -1529,6 +1529,15 @@ static void bt_gatt_client_free(struct bt_gatt_client *client)
 		bt_att_unregister_disconnect(client->att, client->disc_id);
 		bt_att_unregister(client->att, client->notify_id);
 		bt_att_unregister(client->att, client->ind_id);
+
+		/*
+		 * TODO: If we free bt_gatt_client while there is an ongoing
+		 * discovery procedure, the discovery callback may cause an
+		 * invalid access. To avoid this, we cancel all ongoing ATT
+		 * operations but the proper fix here is to make discovery
+		 * procedures cancelable.
+		 */
+		bt_att_cancel_all(client->att);
 		bt_att_unref(client->att);
 	}
 
-- 
2.2.0.rc0.207.ga3a616c


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

* [PATCH BlueZ v1 02/17] gdbus/client: Don't GetManagedObjects w/o handlers
  2015-02-26  5:13 [PATCH BlueZ v1 00/17] Implement GATT server D-Bus API Arman Uguray
  2015-02-26  5:13 ` [PATCH BlueZ v1 01/17] shared/gatt: Call bt_att_cancel_all in unref Arman Uguray
@ 2015-02-26  5:13 ` Arman Uguray
  2015-02-26  5:13 ` [PATCH BlueZ v1 03/17] gdbus/client: Allow specifying ObjectManager path Arman Uguray
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Arman Uguray @ 2015-02-26  5:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Arman Uguray

The client code currently issues GetManagedObjects if new handlers are
set via g_dbus_client_set_proxy_handlers. An application may set these
to NULL before unref'ing a client or to simply prevent further events.
Hence, there is no need to refresh objects or properties if all handlers
are NULL.
---
 gdbus/client.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gdbus/client.c b/gdbus/client.c
index 238b348..cd5c767 100644
--- a/gdbus/client.c
+++ b/gdbus/client.c
@@ -1374,7 +1374,8 @@ gboolean g_dbus_client_set_proxy_handlers(GDBusClient *client,
 	client->property_changed = property_changed;
 	client->user_data = user_data;
 
-	get_managed_objects(client);
+	if (proxy_added || proxy_removed || property_changed)
+		get_managed_objects(client);
 
 	return TRUE;
 }
-- 
2.2.0.rc0.207.ga3a616c


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

* [PATCH BlueZ v1 03/17] gdbus/client: Allow specifying ObjectManager path
  2015-02-26  5:13 [PATCH BlueZ v1 00/17] Implement GATT server D-Bus API Arman Uguray
  2015-02-26  5:13 ` [PATCH BlueZ v1 01/17] shared/gatt: Call bt_att_cancel_all in unref Arman Uguray
  2015-02-26  5:13 ` [PATCH BlueZ v1 02/17] gdbus/client: Don't GetManagedObjects w/o handlers Arman Uguray
@ 2015-02-26  5:13 ` Arman Uguray
  2015-02-26 15:38   ` Marcel Holtmann
  2015-02-26  5:13 ` [PATCH BlueZ v1 04/17] doc/gatt-api.txt: New ObjectManager requirements Arman Uguray
                   ` (13 subsequent siblings)
  16 siblings, 1 reply; 28+ messages in thread
From: Arman Uguray @ 2015-02-26  5:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Arman Uguray

GDBusClient currently hard-codes "/" as the remote ObjectManager path.
This is generally incorrect, as an application can choose to expose an
ObjectManager at any well-known path. This patch fixes this by allowing
the user to pass in the ObjectManager path to g_dbus_client_new.
---
 client/main.c            |  2 +-
 gdbus/client.c           | 23 ++++++++++++++++-------
 gdbus/gdbus.h            |  5 +++--
 plugins/hostname.c       |  2 +-
 profiles/iap/main.c      |  2 +-
 tools/bluetooth-player.c |  2 +-
 tools/gap-tester.c       |  3 ++-
 tools/gatt-service.c     |  2 +-
 tools/mpris-proxy.c      |  2 +-
 tools/obexctl.c          |  2 +-
 unit/test-gdbus-client.c | 39 ++++++++++++++++++++++++++-------------
 11 files changed, 54 insertions(+), 30 deletions(-)

diff --git a/client/main.c b/client/main.c
index 809c372..9fd3d54 100644
--- a/client/main.c
+++ b/client/main.c
@@ -1769,7 +1769,7 @@ int main(int argc, char *argv[])
 	rl_redisplay();
 
 	signal = setup_signalfd();
-	client = g_dbus_client_new(dbus_conn, "org.bluez", "/org/bluez");
+	client = g_dbus_client_new(dbus_conn, "org.bluez", "/org/bluez", NULL);
 
 	g_dbus_client_set_connect_watch(client, connect_handler, NULL);
 	g_dbus_client_set_disconnect_watch(client, disconnect_handler, NULL);
diff --git a/gdbus/client.c b/gdbus/client.c
index cd5c767..1381e37 100644
--- a/gdbus/client.c
+++ b/gdbus/client.c
@@ -42,6 +42,7 @@ struct GDBusClient {
 	DBusConnection *dbus_conn;
 	char *service_name;
 	char *base_path;
+	char *om_path;
 	guint watch;
 	guint added_watch;
 	guint removed_watch;
@@ -1118,9 +1119,10 @@ static void get_managed_objects(GDBusClient *client)
 	if (client->get_objects_call != NULL)
 		return;
 
-	msg = dbus_message_new_method_call(client->service_name, "/",
-					DBUS_INTERFACE_DBUS ".ObjectManager",
-							"GetManagedObjects");
+	msg = dbus_message_new_method_call(client->service_name,
+						client->om_path,
+						DBUS_INTERFACE_OBJECT_MANAGER,
+						"GetManagedObjects");
 	if (msg == NULL)
 		return;
 
@@ -1196,8 +1198,9 @@ static DBusHandlerResult message_filter(DBusConnection *connection,
 	return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 }
 
-GDBusClient *g_dbus_client_new(DBusConnection *connection,
-					const char *service, const char *path)
+GDBusClient *g_dbus_client_new(DBusConnection *connection, const char *service,
+							const char *path,
+							const char *om_path)
 {
 	GDBusClient *client;
 	unsigned int i;
@@ -1215,9 +1218,14 @@ GDBusClient *g_dbus_client_new(DBusConnection *connection,
 		return NULL;
 	}
 
+	/* If no ObjectManager path is given, then default to "/" */
+	if (!om_path)
+		om_path = "/";
+
 	client->dbus_conn = dbus_connection_ref(connection);
 	client->service_name = g_strdup(service);
 	client->base_path = g_strdup(path);
+	client->om_path = g_strdup(om_path);
 	client->connected = FALSE;
 
 	client->match_rules = g_ptr_array_sized_new(1);
@@ -1228,13 +1236,13 @@ GDBusClient *g_dbus_client_new(DBusConnection *connection,
 						service_disconnect,
 						client, NULL);
 	client->added_watch = g_dbus_add_signal_watch(connection, service,
-						"/",
+						client->om_path,
 						DBUS_INTERFACE_OBJECT_MANAGER,
 						"InterfacesAdded",
 						interfaces_added,
 						client, NULL);
 	client->removed_watch = g_dbus_add_signal_watch(connection, service,
-						"/",
+						client->om_path,
 						DBUS_INTERFACE_OBJECT_MANAGER,
 						"InterfacesRemoved",
 						interfaces_removed,
@@ -1308,6 +1316,7 @@ void g_dbus_client_unref(GDBusClient *client)
 
 	g_free(client->service_name);
 	g_free(client->base_path);
+	g_free(client->om_path);
 
 	g_free(client);
 }
diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
index 551c306..1d6d55b 100644
--- a/gdbus/gdbus.h
+++ b/gdbus/gdbus.h
@@ -353,8 +353,9 @@ gboolean g_dbus_proxy_set_property_watch(GDBusProxy *proxy,
 gboolean g_dbus_proxy_set_removed_watch(GDBusProxy *proxy,
 			GDBusProxyFunction destroy, void *user_data);
 
-GDBusClient *g_dbus_client_new(DBusConnection *connection,
-					const char *service, const char *path);
+GDBusClient *g_dbus_client_new(DBusConnection *connection, const char *service,
+							const char *path,
+							const char *om_path);
 
 GDBusClient *g_dbus_client_ref(GDBusClient *client);
 void g_dbus_client_unref(GDBusClient *client);
diff --git a/plugins/hostname.c b/plugins/hostname.c
index d4d72d3..e1f6cf5 100644
--- a/plugins/hostname.c
+++ b/plugins/hostname.c
@@ -275,7 +275,7 @@ static int hostname_init(void)
 	read_dmi_fallback();
 
 	hostname_client = g_dbus_client_new(conn, "org.freedesktop.hostname1",
-						"/org/freedesktop/hostname1");
+					"/org/freedesktop/hostname1", NULL);
 	if (!hostname_client)
 		return -EIO;
 
diff --git a/profiles/iap/main.c b/profiles/iap/main.c
index 0e8f43f..2b7a9e7 100644
--- a/profiles/iap/main.c
+++ b/profiles/iap/main.c
@@ -447,7 +447,7 @@ int main(int argc, char *argv[])
 
 	signal = setup_signalfd();
 
-	client = g_dbus_client_new(dbus_conn, "org.bluez", "/org/bluez");
+	client = g_dbus_client_new(dbus_conn, "org.bluez", "/org/bluez", NULL);
 
 	g_dbus_client_set_connect_watch(client, connect_handler, client);
 	g_dbus_client_set_disconnect_watch(client, disconnect_handler, NULL);
diff --git a/tools/bluetooth-player.c b/tools/bluetooth-player.c
index f10d9be..875fa7c 100644
--- a/tools/bluetooth-player.c
+++ b/tools/bluetooth-player.c
@@ -1430,7 +1430,7 @@ int main(int argc, char *argv[])
 
 	input = setup_standard_input();
 	signal = setup_signalfd();
-	client = g_dbus_client_new(dbus_conn, "org.bluez", "/org/bluez");
+	client = g_dbus_client_new(dbus_conn, "org.bluez", "/org/bluez", NULL);
 
 	g_dbus_client_set_connect_watch(client, connect_handler, NULL);
 	g_dbus_client_set_disconnect_watch(client, disconnect_handler, NULL);
diff --git a/tools/gap-tester.c b/tools/gap-tester.c
index 2a0be91..31260d1 100644
--- a/tools/gap-tester.c
+++ b/tools/gap-tester.c
@@ -108,7 +108,8 @@ static void test_setup(const void *test_data)
 {
 	dbus_conn = g_dbus_setup_private(DBUS_BUS_SYSTEM, NULL, NULL);
 
-	dbus_client = g_dbus_client_new(dbus_conn, "org.bluez", "/org/bluez");
+	dbus_client = g_dbus_client_new(dbus_conn, "org.bluez", "/org/bluez",
+									NULL);
 
 	g_dbus_client_set_connect_watch(dbus_client, connect_handler, NULL);
 	g_dbus_client_set_disconnect_watch(dbus_client,
diff --git a/tools/gatt-service.c b/tools/gatt-service.c
index 6bca404..8aaa520 100644
--- a/tools/gatt-service.c
+++ b/tools/gatt-service.c
@@ -521,7 +521,7 @@ int main(int argc, char *argv[])
 
 	create_services();
 
-	client = g_dbus_client_new(connection, "org.bluez", "/org/bluez");
+	client = g_dbus_client_new(connection, "org.bluez", "/org/bluez", NULL);
 
 	g_dbus_client_set_connect_watch(client, connect_handler, NULL);
 
diff --git a/tools/mpris-proxy.c b/tools/mpris-proxy.c
index 397f064..a4eafb8 100644
--- a/tools/mpris-proxy.c
+++ b/tools/mpris-proxy.c
@@ -2568,7 +2568,7 @@ int main(int argc, char *argv[])
 	sigaction(SIGTERM, &sa, NULL);
 	sigaction(SIGINT,  &sa, NULL);
 
-	client = g_dbus_client_new(sys, BLUEZ_BUS_NAME, BLUEZ_PATH);
+	client = g_dbus_client_new(sys, BLUEZ_BUS_NAME, BLUEZ_PATH, NULL);
 
 	g_dbus_client_set_connect_watch(client, connect_handler, NULL);
 	g_dbus_client_set_disconnect_watch(client, disconnect_handler, NULL);
diff --git a/tools/obexctl.c b/tools/obexctl.c
index b4fdc1c..26c061b 100644
--- a/tools/obexctl.c
+++ b/tools/obexctl.c
@@ -2498,7 +2498,7 @@ int main(int argc, char *argv[])
 	input = setup_standard_input();
 	signal = setup_signalfd();
 	client = g_dbus_client_new(dbus_conn, "org.bluez.obex",
-							"/org/bluez/obex");
+						"/org/bluez/obex", NULL);
 
 	g_dbus_client_set_connect_watch(client, connect_handler, NULL);
 	g_dbus_client_set_disconnect_watch(client, disconnect_handler, NULL);
diff --git a/unit/test-gdbus-client.c b/unit/test-gdbus-client.c
index d0b6ce7..d352fda 100644
--- a/unit/test-gdbus-client.c
+++ b/unit/test-gdbus-client.c
@@ -153,7 +153,8 @@ static void simple_client(void)
 		return;
 
 	context->dbus_client = g_dbus_client_new(context->dbus_conn,
-						SERVICE_NAME, SERVICE_PATH);
+						SERVICE_NAME, SERVICE_PATH,
+						NULL);
 
 	g_dbus_client_set_connect_watch(context->dbus_client,
 						connect_handler, context);
@@ -177,7 +178,8 @@ static void client_connect_disconnect(void)
 				methods, signals, properties, NULL, NULL);
 
 	context->dbus_client = g_dbus_client_new(context->dbus_conn,
-						SERVICE_NAME, SERVICE_PATH);
+						SERVICE_NAME, SERVICE_PATH,
+						NULL);
 
 	g_dbus_client_set_connect_watch(context->dbus_client,
 						connect_handler, context);
@@ -323,7 +325,8 @@ static void client_get_dict_property(void)
 				NULL, NULL);
 
 	context->dbus_client = g_dbus_client_new(context->dbus_conn,
-						SERVICE_NAME, SERVICE_PATH);
+						SERVICE_NAME, SERVICE_PATH,
+						NULL);
 
 	g_dbus_client_set_disconnect_watch(context->dbus_client,
 						disconnect_handler, context);
@@ -385,7 +388,8 @@ static void client_get_string_property(void)
 				context, NULL);
 
 	context->dbus_client = g_dbus_client_new(context->dbus_conn,
-						SERVICE_NAME, SERVICE_PATH);
+						SERVICE_NAME, SERVICE_PATH,
+						NULL);
 
 	g_dbus_client_set_disconnect_watch(context->dbus_client,
 						disconnect_handler, context);
@@ -446,7 +450,8 @@ static void client_get_boolean_property(void)
 				NULL, NULL);
 
 	context->dbus_client = g_dbus_client_new(context->dbus_conn,
-						SERVICE_NAME, SERVICE_PATH);
+						SERVICE_NAME, SERVICE_PATH,
+						NULL);
 
 	g_dbus_client_set_proxy_handlers(context->dbus_client,
 						proxy_get_boolean,
@@ -523,7 +528,8 @@ static void client_get_array_property(void)
 				NULL, NULL);
 
 	context->dbus_client = g_dbus_client_new(context->dbus_conn,
-						SERVICE_NAME, SERVICE_PATH);
+						SERVICE_NAME, SERVICE_PATH,
+						NULL);
 
 	g_dbus_client_set_proxy_handlers(context->dbus_client, proxy_get_array,
 						NULL, NULL, context);
@@ -584,7 +590,8 @@ static void client_get_uint64_property(void)
 				NULL, NULL);
 
 	context->dbus_client = g_dbus_client_new(context->dbus_conn,
-						SERVICE_NAME, SERVICE_PATH);
+						SERVICE_NAME, SERVICE_PATH,
+						NULL);
 
 	g_dbus_client_set_proxy_handlers(context->dbus_client,
 						proxy_get_uint64,
@@ -684,7 +691,8 @@ static void client_set_string_property(void)
 				context, NULL);
 
 	context->dbus_client = g_dbus_client_new(context->dbus_conn,
-						SERVICE_NAME, SERVICE_PATH);
+						SERVICE_NAME, SERVICE_PATH,
+						NULL);
 
 	g_dbus_client_set_disconnect_watch(context->dbus_client,
 						disconnect_handler, context);
@@ -767,7 +775,8 @@ static void client_string_changed(void)
 				context, NULL);
 
 	context->dbus_client = g_dbus_client_new(context->dbus_conn,
-						SERVICE_NAME, SERVICE_PATH);
+						SERVICE_NAME, SERVICE_PATH,
+						NULL);
 
 	g_dbus_client_set_disconnect_watch(context->dbus_client,
 						disconnect_handler, context);
@@ -836,7 +845,8 @@ static void client_check_order(void)
 				context, NULL);
 
 	context->dbus_client = g_dbus_client_new(context->dbus_conn,
-						SERVICE_NAME, SERVICE_PATH);
+						SERVICE_NAME, SERVICE_PATH,
+						NULL);
 
 	g_dbus_client_set_disconnect_watch(context->dbus_client,
 						disconnect_handler, context);
@@ -896,7 +906,8 @@ static void client_proxy_removed(void)
 				context, NULL);
 
 	context->dbus_client = g_dbus_client_new(context->dbus_conn,
-						SERVICE_NAME, SERVICE_PATH);
+						SERVICE_NAME, SERVICE_PATH,
+						NULL);
 
 	g_dbus_client_set_proxy_handlers(context->dbus_client,
 						proxy_set_removed, NULL, NULL,
@@ -951,7 +962,8 @@ static void client_force_disconnect(void)
 					context, NULL);
 
 	context->dbus_client = g_dbus_client_new(context->dbus_conn,
-						SERVICE_NAME1, SERVICE_PATH);
+						SERVICE_NAME1, SERVICE_PATH,
+						NULL);
 
 	g_dbus_client_set_proxy_handlers(context->dbus_client,
 					proxy_force_disconnect, NULL, NULL,
@@ -1004,7 +1016,8 @@ static void client_ready(void)
 				context, NULL);
 
 	context->dbus_client = g_dbus_client_new(context->dbus_conn,
-						SERVICE_NAME, SERVICE_PATH);
+						SERVICE_NAME, SERVICE_PATH,
+						NULL);
 
 	g_dbus_client_set_ready_watch(context->dbus_client, client_ready_watch,
 								context);
-- 
2.2.0.rc0.207.ga3a616c


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

* [PATCH BlueZ v1 04/17] doc/gatt-api.txt: New ObjectManager requirements
  2015-02-26  5:13 [PATCH BlueZ v1 00/17] Implement GATT server D-Bus API Arman Uguray
                   ` (2 preceding siblings ...)
  2015-02-26  5:13 ` [PATCH BlueZ v1 03/17] gdbus/client: Allow specifying ObjectManager path Arman Uguray
@ 2015-02-26  5:13 ` Arman Uguray
  2015-02-26  5:13 ` [PATCH BlueZ v1 05/17] core/gatt: Add GattManager1 stubs Arman Uguray
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Arman Uguray @ 2015-02-26  5:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Arman Uguray

After some discussion it was decided to require an ObjectManager
interface implementation on a per-service basis to reduce the
overhead of heaving to process and cache potentially many non-GATT
related objects. This patch updates the documentation to reflect this.
---
 doc/gatt-api.txt | 74 +++++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 62 insertions(+), 12 deletions(-)

diff --git a/doc/gatt-api.txt b/doc/gatt-api.txt
index 67938d0..088d285 100644
--- a/doc/gatt-api.txt
+++ b/doc/gatt-api.txt
@@ -8,7 +8,9 @@ application. Remote refers to GATT services exported by the peer.
 BlueZ acts as a proxy, translating ATT operations to D-Bus method calls and
 Properties (or the opposite). Support for D-Bus Object Manager is mandatory for
 external services to allow seamless GATT declarations (Service, Characteristic
-and Descriptors) discovery.
+and Descriptors) discovery. Each GATT service tree is required to export a D-Bus
+Object Manager at its root that is solely responsible for the objects that
+belong to that service.
 
 Releasing a registered GATT service is not defined yet. Any API extension
 should avoid breaking the defined API, and if possible keep an unified GATT
@@ -229,12 +231,63 @@ Methods		void Release()
 GATT Manager hierarchy
 ======================
 
-GATT Manager allows external applications to register GATT based
-services. Services must follow the API for Service and Characteristic
-described above.
-
-Local GATT services, characteristics and characteristic descriptors are
-discovered automatically using the D-Bus Object Manager interface.
+GATT Manager allows external applications to register GATT services and
+profiles.
+
+Registering a profile allows applications to subscribe to *remote* services.
+These must implement the GattProfile1 interface defined above.
+
+Registering a service allows applications to publish a *local* GATT service,
+which then becomes available to remote devices. A GATT service is represented by
+a D-Bus object hierarchy where the root node corresponds to a service and the
+child nodes represent characteristics and descriptors that belong to that
+service. Each node must implement one of GattService1, GattCharacteristic1,
+or GattDescriptor1 interfaces described above, based on the attribute it
+represents. Each node must also implement the standard D-Bus Properties
+interface to expose their properties. These objects collectively represent a
+GATT service definition.
+
+To make service registration simple, BlueZ requires that all objects that belong
+to a GATT service be grouped under a D-Bus Object Manager that solely manages
+the objects of that service. Hence, the standard DBus.ObjectManager interface
+must be available on the root service path. An example application hierarchy
+containing two separate GATT services may look like this:
+
+-> /com/example
+  |
+  -> /com/example/service0
+  | |   - org.freedesktop.DBus.ObjectManager
+  | |   - org.freedesktop.DBus.Properties
+  | |   - org.bluez.GattService1
+  | |
+  | -> /com/example/service0/char0
+  | |     - org.freedesktop.DBus.Properties
+  | |     - org.bluez.GattCharacteristic1
+  | |
+  | -> /com/example/service0/char1
+  |   |   - org.freedesktop.DBus.Properties
+  |   |   - org.bluez.GattCharacteristic1
+  |   |
+  |   -> /com/example/service0/char1/desc0
+  |       - org.freedesktop.DBus.Properties
+  |       - org.bluez.GattDescriptor1
+  |
+  -> /com/example/service1
+    |   - org.freedesktop.DBus.ObjectManager
+    |   - org.freedesktop.DBus.Properties
+    |   - org.bluez.GattService1
+    |
+    -> /com/example/service1/char0
+        - org.freedesktop.DBus.Properties
+        - org.bluez.GattCharacteristic1
+
+When a service is registered, BlueZ will automatically obtain information about
+all objects using the service's Object Manager. Once a service has been
+registered, the objects of a service should not be removed. If BlueZ receives an
+InterfacesRemoved signal from a service's Object Manager, it will immediately
+unregister the service. Similarly, if the application disconnects from the bus,
+all of its registered services will be automatically unregistered.
+InterfacesAdded signals will be ignored.
 
 Service		org.bluez
 Interface	org.bluez.GattManager1 [Experimental]
@@ -242,11 +295,8 @@ Object path	[variable prefix]/{hci0,hci1,...}
 
 Methods		void RegisterService(object service, dict options)
 
-			Registers remote application service exported under
-			interface GattService1. Characteristic objects must
-			be hierarchical to their service and must use the
-			interface GattCharacteristic1. D-Bus Object Manager
-			is used to fetch the exported objects.
+			Registers a local GATT service hierarchy as described
+			above.
 
 			"service" object path together with the D-Bus system
 			bus connection ID define the identification of the
-- 
2.2.0.rc0.207.ga3a616c


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

* [PATCH BlueZ v1 05/17] core/gatt: Add GattManager1 stubs
  2015-02-26  5:13 [PATCH BlueZ v1 00/17] Implement GATT server D-Bus API Arman Uguray
                   ` (3 preceding siblings ...)
  2015-02-26  5:13 ` [PATCH BlueZ v1 04/17] doc/gatt-api.txt: New ObjectManager requirements Arman Uguray
@ 2015-02-26  5:13 ` Arman Uguray
  2015-02-26  5:13 ` [PATCH BlueZ v1 06/17] core/gatt: Implement GattManager1.RegisterService Arman Uguray
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Arman Uguray @ 2015-02-26  5:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Arman Uguray

This patch introduces src/gatt-manager, which will implement the
org.bluez.GattManager1 API outlined in doc/gatt-api.txt. The old
src/gatt-dbus code has been removed to start from a clean slate.
---
 Makefile.am        |   3 +-
 src/adapter.c      |  18 +-
 src/gatt-dbus.c    | 658 -----------------------------------------------------
 src/gatt-dbus.h    |  25 --
 src/gatt-manager.c | 121 ++++++++++
 src/gatt-manager.h |  23 ++
 src/gatt.c         | 321 --------------------------
 src/gatt.h         | 121 ----------
 src/main.c         |   5 -
 9 files changed, 162 insertions(+), 1133 deletions(-)
 delete mode 100644 src/gatt-dbus.c
 delete mode 100644 src/gatt-dbus.h
 create mode 100644 src/gatt-manager.c
 create mode 100644 src/gatt-manager.h
 delete mode 100644 src/gatt.c
 delete mode 100644 src/gatt.h

diff --git a/Makefile.am b/Makefile.am
index dd8cda2..20c0ab6 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -168,6 +168,7 @@ src_bluetoothd_SOURCES = $(builtin_sources) \
 			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/gatt-manager.h src/gatt-manager.c \
 			src/sdp-xml.h src/sdp-xml.c \
 			src/sdp-client.h src/sdp-client.c \
 			src/textfile.h src/textfile.c \
@@ -180,8 +181,6 @@ src_bluetoothd_SOURCES = $(builtin_sources) \
 			src/adapter.h src/adapter.c \
 			src/profile.h src/profile.c \
 			src/service.h src/service.c \
-			src/gatt-dbus.h src/gatt-dbus.c \
-			src/gatt.h src/gatt.c \
 			src/gatt-client.h src/gatt-client.c \
 			src/device.h src/device.c src/attio.h \
 			src/dbus-common.c src/dbus-common.h \
diff --git a/src/adapter.c b/src/adapter.c
index 3353297..b9ec0a9 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -72,6 +72,7 @@
 #include "attrib/gatt.h"
 #include "attrib-server.h"
 #include "gatt-database.h"
+#include "gatt-manager.h"
 #include "eir.h"
 
 #define ADAPTER_INTERFACE	"org.bluez.Adapter1"
@@ -208,6 +209,7 @@ struct btd_adapter {
 	sdp_list_t *services;		/* Services associated to adapter */
 
 	struct btd_gatt_database *database;
+	struct btd_gatt_manager *manager;
 
 	gboolean initialized;
 
@@ -4591,6 +4593,10 @@ static void adapter_remove(struct btd_adapter *adapter)
 	adapter->db_id = 0;
 
 	btd_gatt_database_destroy(adapter->database);
+	adapter->database = NULL;
+
+	btd_gatt_manager_destroy(adapter->manager);
+	adapter->manager = NULL;
 
 	g_slist_free(adapter->pin_callbacks);
 	adapter->pin_callbacks = NULL;
@@ -6642,8 +6648,18 @@ static int adapter_register(struct btd_adapter *adapter)
 	}
 
 	adapter->database = btd_gatt_database_new(adapter);
-	if (!adapter->database)
+	if (!adapter->database) {
 		error("Failed to create GATT database for adapter");
+		return -EINVAL;
+	}
+
+	adapter->manager = btd_gatt_manager_new(adapter);
+	if (!adapter->manager) {
+		error("Failed to register GattManager1 interface for adapter");
+		btd_gatt_database_destroy(adapter->database);
+		adapter->database = NULL;
+		return -EINVAL;
+	}
 
 	db = btd_gatt_database_get_db(adapter->database);
 	adapter->db_id = gatt_db_register(db, services_modified,
diff --git a/src/gatt-dbus.c b/src/gatt-dbus.c
deleted file mode 100644
index c22e8af..0000000
--- a/src/gatt-dbus.c
+++ /dev/null
@@ -1,658 +0,0 @@
-/*
- *
- *  BlueZ - Bluetooth protocol stack for Linux
- *
- *  Copyright (C) 2014  Instituto Nokia de Tecnologia - INdT
- *
- *
- *  This program is free software; you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation; either version 2 of the License, or
- *  (at your option) any later version.
- *
- *  This program is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *  GNU General Public License for more details.
- *
- *  You should have received a copy of the GNU General Public License
- *  along with this program; if not, write to the Free Software
- *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
- *
- */
-
-#ifdef HAVE_CONFIG_H
-#include <config.h>
-#endif
-
-#include <stdint.h>
-#include <errno.h>
-
-#include <glib.h>
-#include <dbus/dbus.h>
-#include <gdbus/gdbus.h>
-
-#include "adapter.h"
-#include "device.h"
-#include "lib/uuid.h"
-#include "dbus-common.h"
-#include "log.h"
-
-#include "error.h"
-#include "attrib/gattrib.h"
-#include "attrib/att.h"
-#include "attrib/gatt.h"
-#include "gatt.h"
-#include "gatt-dbus.h"
-
-#define GATT_MGR_IFACE			"org.bluez.GattManager1"
-#define GATT_SERVICE_IFACE		"org.bluez.GattService1"
-#define GATT_CHR_IFACE			"org.bluez.GattCharacteristic1"
-#define GATT_DESCRIPTOR_IFACE		"org.bluez.GattDescriptor1"
-
-struct external_service {
-	char *owner;
-	char *path;
-	DBusMessage *reg;
-	GDBusClient *client;
-	GSList *proxies;
-	struct btd_attribute *service;
-};
-
-struct proxy_write_data {
-	btd_attr_write_result_t result_cb;
-	void *user_data;
-};
-
-/*
- * Attribute to Proxy hash table. Used to map incoming
- * ATT operations to its external characteristic proxy.
- */
-static GHashTable *proxy_hash;
-
-static GSList *external_services;
-
-static int external_service_path_cmp(gconstpointer a, gconstpointer b)
-{
-	const struct external_service *esvc = a;
-	const char *path = b;
-
-	return g_strcmp0(esvc->path, path);
-}
-
-static gboolean external_service_destroy(void *user_data)
-{
-	struct external_service *esvc = user_data;
-
-	g_dbus_client_unref(esvc->client);
-
-	if (esvc->reg)
-		dbus_message_unref(esvc->reg);
-
-	g_free(esvc->owner);
-	g_free(esvc->path);
-	g_free(esvc);
-
-	return FALSE;
-}
-
-static void external_service_free(void *user_data)
-{
-	struct external_service *esvc = user_data;
-
-	/*
-	 * Set callback to NULL to avoid potential race condition
-	 * when calling remove_service and GDBusClient unref.
-	 */
-	g_dbus_client_set_disconnect_watch(esvc->client, NULL, NULL);
-
-	external_service_destroy(user_data);
-}
-
-static void remove_service(DBusConnection *conn, void *user_data)
-{
-	struct external_service *esvc = user_data;
-
-	external_services = g_slist_remove(external_services, esvc);
-
-	if (esvc->service)
-		btd_gatt_remove_service(esvc->service);
-
-	/*
-	 * Do not run in the same loop, this may be a disconnect
-	 * watch call and GDBusClient should not be destroyed.
-	 */
-	g_idle_add(external_service_destroy, esvc);
-}
-
-static int proxy_path_cmp(gconstpointer a, gconstpointer b)
-{
-	GDBusProxy *proxy1 = (GDBusProxy *) a;
-	GDBusProxy *proxy2 = (GDBusProxy *) b;
-	const char *path1 = g_dbus_proxy_get_path(proxy1);
-	const char *path2 = g_dbus_proxy_get_path(proxy2);
-
-	return g_strcmp0(path1, path2);
-}
-
-static uint8_t flags_string2int(const char *proper)
-{
-	uint8_t value;
-
-	/* Regular Properties: See core spec 4.1 page 2183 */
-	if (!strcmp("broadcast", proper))
-		value = GATT_CHR_PROP_BROADCAST;
-	else if (!strcmp("read", proper))
-		value = GATT_CHR_PROP_READ;
-	else if (!strcmp("write-without-response", proper))
-		value = GATT_CHR_PROP_WRITE_WITHOUT_RESP;
-	else if (!strcmp("write", proper))
-		value = GATT_CHR_PROP_WRITE;
-	else if (!strcmp("notify", proper))
-		value = GATT_CHR_PROP_NOTIFY;
-	else if (!strcmp("indicate", proper))
-		value = GATT_CHR_PROP_INDICATE;
-	else if (!strcmp("authenticated-signed-writes", proper))
-		value = GATT_CHR_PROP_AUTH;
-	else
-		value = 0;
-
-	/* TODO: Extended properties. Ref core spec 4.1 page 2185  */
-
-	return value;
-}
-
-static uint8_t flags_get_bitmask(DBusMessageIter *iter)
-{
-	DBusMessageIter istr;
-	uint8_t propmask = 0, prop;
-	const char *str;
-
-	if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_ARRAY)
-		goto fail;
-
-	dbus_message_iter_recurse(iter, &istr);
-
-	do {
-		if (dbus_message_iter_get_arg_type(&istr) != DBUS_TYPE_STRING)
-			goto fail;
-
-		dbus_message_iter_get_basic(&istr, &str);
-		prop = flags_string2int(str);
-		if (!prop)
-			goto fail;
-
-		propmask |= prop;
-	} while (dbus_message_iter_next(&istr));
-
-	return propmask;
-
-fail:
-	error("Characteristic Flags: Invalid argument!");
-
-	return 0;
-}
-
-static void proxy_added(GDBusProxy *proxy, void *user_data)
-{
-	struct external_service *esvc = user_data;
-	const char *interface, *path;
-
-	interface = g_dbus_proxy_get_interface(proxy);
-	path = g_dbus_proxy_get_path(proxy);
-
-	if (!g_str_has_prefix(path, esvc->path))
-		return;
-
-	if (g_strcmp0(interface, GATT_CHR_IFACE) != 0 &&
-			g_strcmp0(interface, GATT_SERVICE_IFACE) != 0 &&
-			g_strcmp0(interface, GATT_DESCRIPTOR_IFACE) != 0)
-		return;
-
-	DBG("path %s iface %s", path, interface);
-
-	/*
-	 * Object path follows a hierarchical organization. Add the
-	 * proxies sorted by path helps the logic to register the
-	 * object path later.
-	 */
-	esvc->proxies = g_slist_insert_sorted(esvc->proxies, proxy,
-							proxy_path_cmp);
-}
-
-static void proxy_removed(GDBusProxy *proxy, void *user_data)
-{
-	struct external_service *esvc = user_data;
-	const char *interface, *path;
-
-	interface = g_dbus_proxy_get_interface(proxy);
-	path = g_dbus_proxy_get_path(proxy);
-
-	DBG("path %s iface %s", path, interface);
-
-	esvc->proxies = g_slist_remove(esvc->proxies, proxy);
-}
-
-static void proxy_read_cb(struct btd_attribute *attr,
-				btd_attr_read_result_t result, void *user_data)
-{
-	DBusMessageIter iter, array;
-	GDBusProxy *proxy;
-	uint8_t *value;
-	int len;
-
-	/*
-	 * Remote device is trying to read the informed attribute,
-	 * "Value" should be read from the proxy. GDBusProxy tracks
-	 * properties changes automatically, it is not necessary to
-	 * get the value directly from the GATT server.
-	 */
-	proxy = g_hash_table_lookup(proxy_hash, attr);
-	if (!proxy) {
-		result(-ENOENT, NULL, 0, user_data);
-		return;
-	}
-
-	if (!g_dbus_proxy_get_property(proxy, "Value", &iter)) {
-		/* Unusual situation, read property will checked earlier */
-		result(-EPERM, NULL, 0, user_data);
-		return;
-	}
-
-	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_ARRAY) {
-		DBG("External service inconsistent!");
-		result(-EPERM, NULL, 0, user_data);
-		return;
-	}
-
-	dbus_message_iter_recurse(&iter, &array);
-	dbus_message_iter_get_fixed_array(&array, &value, &len);
-
-	DBG("attribute: %p read %d bytes", attr, len);
-
-	result(0, value, len, user_data);
-}
-
-static void proxy_write_reply(const DBusError *derr, void *user_data)
-{
-	struct proxy_write_data *wdata = user_data;
-	int err;
-
-	/*
-	 * Security requirements shall be handled by the core. If external
-	 * applications returns an error, the reasons will be restricted to
-	 * invalid argument or application specific errors.
-	 */
-
-	if (!dbus_error_is_set(derr)) {
-		err = 0;
-		goto done;
-	}
-
-	DBG("Write reply: %s", derr->message);
-
-	if (dbus_error_has_name(derr, DBUS_ERROR_NO_REPLY))
-		err = -ETIMEDOUT;
-	else if (dbus_error_has_name(derr, ERROR_INTERFACE ".InvalidArguments"))
-		err = -EINVAL;
-	else
-		err = -EPROTO;
-
-done:
-	if (wdata && wdata->result_cb)
-		wdata->result_cb(err, wdata->user_data);
-}
-
-static void proxy_write_cb(struct btd_attribute *attr,
-					const uint8_t *value, size_t len,
-					btd_attr_write_result_t result,
-					void *user_data)
-{
-	GDBusProxy *proxy;
-
-	proxy = g_hash_table_lookup(proxy_hash, attr);
-	if (!proxy) {
-		result(-ENOENT, user_data);
-		return;
-	}
-
-	/*
-	 * "result" callback defines if the core wants to receive the
-	 * operation result, allowing to select ATT Write Request or Write
-	 * Command. Descriptors requires Write Request operation. For
-	 * Characteristics, the implementation will define which operations
-	 * are allowed based on the properties/flags.
-	 * TODO: Write Long Characteristics/Descriptors.
-	 */
-
-	if (result) {
-		struct proxy_write_data *wdata;
-
-		wdata = g_new0(struct proxy_write_data, 1);
-		wdata->result_cb = result;
-		wdata->user_data = user_data;
-
-		if (!g_dbus_proxy_set_property_array(proxy, "Value",
-						DBUS_TYPE_BYTE, value, len,
-						proxy_write_reply,
-						wdata, g_free)) {
-			g_free(wdata);
-			result(-ENOENT, user_data);
-		}
-	} else {
-		/*
-		 * Caller is not interested in the Set method call result.
-		 * This flow implements the ATT Write Command scenario, where
-		 * the remote doesn't receive ATT response.
-		 */
-		g_dbus_proxy_set_property_array(proxy, "Value", DBUS_TYPE_BYTE,
-						value, len, proxy_write_reply,
-						NULL, NULL);
-	}
-
-	DBG("Server: Write attribute callback %s",
-					g_dbus_proxy_get_path(proxy));
-
-}
-
-static int register_external_service(struct external_service *esvc,
-							GDBusProxy *proxy)
-{
-	DBusMessageIter iter;
-	const char *str, *path, *iface;
-	bt_uuid_t uuid;
-
-	path = g_dbus_proxy_get_path(proxy);
-	iface = g_dbus_proxy_get_interface(proxy);
-	if (g_strcmp0(esvc->path, path) != 0 ||
-			g_strcmp0(iface, GATT_SERVICE_IFACE) != 0)
-		return -EINVAL;
-
-	if (!g_dbus_proxy_get_property(proxy, "UUID", &iter))
-		return -EINVAL;
-
-	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
-		return -EINVAL;
-
-	dbus_message_iter_get_basic(&iter, &str);
-
-	if (bt_string_to_uuid(&uuid, str) < 0)
-		return -EINVAL;
-
-	esvc->service = btd_gatt_add_service(&uuid);
-	if (!esvc->service)
-		return -EINVAL;
-
-	return 0;
-}
-
-static int add_char(GDBusProxy *proxy, const bt_uuid_t *uuid)
-{
-	DBusMessageIter iter;
-	struct btd_attribute *attr;
-	btd_attr_write_t write_cb;
-	btd_attr_read_t read_cb;
-	uint8_t propmask = 0;
-
-	/*
-	 * Optional property. If is not informed, read and write
-	 * procedures will be allowed. Upper-layer should handle
-	 * characteristic requirements.
-	 */
-	if (g_dbus_proxy_get_property(proxy, "Flags", &iter))
-		propmask = flags_get_bitmask(&iter);
-	else
-		propmask = GATT_CHR_PROP_WRITE_WITHOUT_RESP
-						| GATT_CHR_PROP_WRITE
-						| GATT_CHR_PROP_READ;
-	if (!propmask)
-		return -EINVAL;
-
-	if (propmask & GATT_CHR_PROP_READ)
-		read_cb = proxy_read_cb;
-	else
-		read_cb = NULL;
-
-	if (propmask & (GATT_CHR_PROP_WRITE | GATT_CHR_PROP_WRITE_WITHOUT_RESP))
-		write_cb = proxy_write_cb;
-	else
-		write_cb = NULL;
-
-	attr = btd_gatt_add_char(uuid, propmask, read_cb, write_cb);
-	if (!attr)
-		return -ENOMEM;
-
-	g_hash_table_insert(proxy_hash, attr, g_dbus_proxy_ref(proxy));
-
-	return 0;
-}
-
-static int add_char_desc(GDBusProxy *proxy, const bt_uuid_t *uuid)
-{
-	struct btd_attribute *attr;
-
-	attr = btd_gatt_add_char_desc(uuid, proxy_read_cb, proxy_write_cb);
-	if (!attr)
-		return -ENOMEM;
-
-	g_hash_table_insert(proxy_hash, attr, g_dbus_proxy_ref(proxy));
-
-	return 0;
-}
-
-static int register_external_characteristics(GSList *proxies)
-
-{
-	GSList *list;
-
-	for (list = proxies; list; list = g_slist_next(list)) {
-		GDBusProxy *proxy = list->data;
-		DBusMessageIter iter;
-		bt_uuid_t uuid;
-		const char *path, *iface, *str;
-		int ret;
-
-		/* Mandatory property */
-		if (!g_dbus_proxy_get_property(proxy, "UUID", &iter))
-			return -EINVAL;
-
-		if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
-			return -EINVAL;
-
-		dbus_message_iter_get_basic(&iter, &str);
-
-		if (bt_string_to_uuid(&uuid, str) < 0)
-			return -EINVAL;
-
-		iface = g_dbus_proxy_get_interface(proxy);
-		path = g_dbus_proxy_get_path(proxy);
-
-		if (!strcmp(GATT_CHR_IFACE, iface))
-			ret = add_char(proxy, &uuid);
-		else
-			ret = add_char_desc(proxy, &uuid);
-
-		if (ret < 0)
-			return ret;
-
-		DBG("Added GATT: %s (%s)", path, str);
-	}
-
-	return 0;
-}
-
-static void client_ready(GDBusClient *client, void *user_data)
-{
-	struct external_service *esvc = user_data;
-	GDBusProxy *proxy;
-	DBusConnection *conn = btd_get_dbus_connection();
-	DBusMessage *reply;
-
-	if (!esvc->proxies)
-		goto fail;
-
-	proxy = esvc->proxies->data;
-	if (register_external_service(esvc, proxy) < 0)
-		goto fail;
-
-	if (register_external_characteristics(g_slist_next(esvc->proxies)) < 0)
-		goto fail;
-
-	DBG("Added GATT service %s", esvc->path);
-
-	reply = dbus_message_new_method_return(esvc->reg);
-	g_dbus_send_message(conn, reply);
-
-	dbus_message_unref(esvc->reg);
-	esvc->reg = NULL;
-
-	return;
-
-fail:
-	error("Could not register external service: %s", esvc->path);
-
-	/*
-	 * Set callback to NULL to avoid potential race condition
-	 * when calling remove_service and GDBusClient unref.
-	 */
-	g_dbus_client_set_disconnect_watch(esvc->client, NULL, NULL);
-
-	remove_service(conn, esvc);
-
-	reply = btd_error_invalid_args(esvc->reg);
-	g_dbus_send_message(conn, reply);
-}
-
-static struct external_service *external_service_new(DBusConnection *conn,
-					DBusMessage *msg, const char *path)
-{
-	struct external_service *esvc;
-	GDBusClient *client;
-	const char *sender = dbus_message_get_sender(msg);
-
-	client = g_dbus_client_new(conn, sender, "/");
-	if (!client)
-		return NULL;
-
-	esvc = g_new0(struct external_service, 1);
-	esvc->owner = g_strdup(sender);
-	esvc->reg = dbus_message_ref(msg);
-	esvc->client = client;
-	esvc->path = g_strdup(path);
-
-	g_dbus_client_set_disconnect_watch(client, remove_service, esvc);
-
-	g_dbus_client_set_proxy_handlers(client, proxy_added, proxy_removed,
-								NULL, esvc);
-
-	g_dbus_client_set_ready_watch(client, client_ready, esvc);
-
-	return esvc;
-}
-
-static DBusMessage *register_service(DBusConnection *conn,
-					DBusMessage *msg, void *user_data)
-{
-	struct external_service *esvc;
-	DBusMessageIter iter;
-	const char *path;
-
-	if (!dbus_message_iter_init(msg, &iter))
-		return btd_error_invalid_args(msg);
-
-	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_OBJECT_PATH)
-		return btd_error_invalid_args(msg);
-
-	dbus_message_iter_get_basic(&iter, &path);
-
-	if (g_slist_find_custom(external_services, path,
-						external_service_path_cmp))
-		return btd_error_already_exists(msg);
-
-	esvc = external_service_new(conn, msg, path);
-	if (!esvc)
-		return btd_error_failed(msg, "Not enough resources");
-
-	external_services = g_slist_prepend(external_services, esvc);
-
-	DBG("New service %p: %s", esvc, path);
-
-	return NULL;
-}
-
-static DBusMessage *unregister_service(DBusConnection *conn,
-					DBusMessage *msg, void *user_data)
-{
-	struct external_service *esvc;
-	DBusMessageIter iter;
-	const char *path;
-	GSList *list;
-
-	if (!dbus_message_iter_init(msg, &iter))
-		return btd_error_invalid_args(msg);
-
-	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_OBJECT_PATH)
-		return btd_error_invalid_args(msg);
-
-	dbus_message_iter_get_basic(&iter, &path);
-
-	list = g_slist_find_custom(external_services, path,
-						external_service_path_cmp);
-	if (!list)
-		return btd_error_does_not_exist(msg);
-
-	esvc = list->data;
-	if (!strcmp(dbus_message_get_sender(msg), esvc->owner))
-		return btd_error_does_not_exist(msg);
-
-	/*
-	 * Set callback to NULL to avoid potential race condition
-	 * when calling remove_service and GDBusClient unref.
-	 */
-	g_dbus_client_set_disconnect_watch(esvc->client, NULL, NULL);
-
-	remove_service(conn, esvc);
-
-	return dbus_message_new_method_return(msg);
-}
-
-static const GDBusMethodTable methods[] = {
-	{ GDBUS_EXPERIMENTAL_ASYNC_METHOD("RegisterService",
-				GDBUS_ARGS({ "service", "o"},
-						{ "options", "a{sv}"}),
-				NULL, register_service) },
-	{ GDBUS_EXPERIMENTAL_METHOD("UnregisterService",
-				GDBUS_ARGS({"service", "o"}),
-				NULL, unregister_service) },
-	{ }
-};
-
-gboolean gatt_dbus_manager_register(void)
-{
-	if (!g_dbus_register_interface(btd_get_dbus_connection(),
-				"/org/bluez", GATT_MGR_IFACE,
-				methods, NULL, NULL, NULL, NULL))
-		return FALSE;
-
-	proxy_hash = g_hash_table_new_full(g_direct_hash, g_direct_equal,
-				NULL, (GDestroyNotify) g_dbus_proxy_unref);
-
-	return TRUE;
-}
-
-void gatt_dbus_manager_unregister(void)
-{
-	/* We might not have initialized if experimental features are
-	 * not enabled.
-	 */
-	if (!proxy_hash)
-		return;
-
-	g_hash_table_destroy(proxy_hash);
-	proxy_hash = NULL;
-
-	g_slist_free_full(external_services, external_service_free);
-
-	g_dbus_unregister_interface(btd_get_dbus_connection(), "/org/bluez",
-							GATT_MGR_IFACE);
-}
diff --git a/src/gatt-dbus.h b/src/gatt-dbus.h
deleted file mode 100644
index 310cfa9..0000000
--- a/src/gatt-dbus.h
+++ /dev/null
@@ -1,25 +0,0 @@
-/*
- *
- *  BlueZ - Bluetooth protocol stack for Linux
- *
- *  Copyright (C) 2014  Instituto Nokia de Tecnologia - INdT
- *
- *
- *  This program is free software; you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation; either version 2 of the License, or
- *  (at your option) any later version.
- *
- *  This program is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *  GNU General Public License for more details.
- *
- *  You should have received a copy of the GNU General Public License
- *  along with this program; if not, write to the Free Software
- *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
- *
- */
-
-gboolean gatt_dbus_manager_register(void);
-void gatt_dbus_manager_unregister(void);
diff --git a/src/gatt-manager.c b/src/gatt-manager.c
new file mode 100644
index 0000000..296eabc
--- /dev/null
+++ b/src/gatt-manager.c
@@ -0,0 +1,121 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2014  Instituto Nokia de Tecnologia - INdT
+ *  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 <dbus/dbus.h>
+#include <gdbus/gdbus.h>
+
+#include "adapter.h"
+#include "gatt-manager.h"
+#include "dbus-common.h"
+#include "log.h"
+#include "error.h"
+#include "src/shared/queue.h"
+#include "src/shared/util.h"
+
+#define GATT_MANAGER_IFACE	"org.bluez.GattManager1"
+
+struct btd_gatt_manager {
+	struct btd_adapter *adapter;
+};
+
+static DBusMessage *manager_register_service(DBusConnection *conn,
+					DBusMessage *msg, void *user_data)
+{
+	DBG("RegisterService");
+
+	/* TODO */
+	return NULL;
+}
+
+static DBusMessage *manager_unregister_service(DBusConnection *conn,
+					DBusMessage *msg, void *user_data)
+{
+	DBG("UnregisterService");
+
+	/* TODO */
+	return NULL;
+}
+
+static const GDBusMethodTable manager_methods[] = {
+	{ GDBUS_EXPERIMENTAL_ASYNC_METHOD("RegisterService",
+			GDBUS_ARGS({ "service", "o" }, { "options", "a{sv}" }),
+			NULL, manager_register_service) },
+	{ GDBUS_EXPERIMENTAL_ASYNC_METHOD("UnregisterService",
+					GDBUS_ARGS({ "service", "o" }),
+					NULL, manager_unregister_service) },
+	{ }
+};
+
+static struct btd_gatt_manager *manager_create(struct btd_adapter *adapter)
+{
+	struct btd_gatt_manager *manager;
+
+	manager = new0(struct btd_gatt_manager, 1);
+	if (!manager)
+		return NULL;
+
+	manager->adapter = adapter;
+
+	if (!g_dbus_register_interface(btd_get_dbus_connection(),
+						adapter_get_path(adapter),
+						GATT_MANAGER_IFACE,
+						manager_methods, NULL, NULL,
+						manager, NULL)) {
+		error("Failed to register " GATT_MANAGER_IFACE);
+		free(manager);
+		return NULL;
+	}
+
+	return manager;
+}
+
+struct btd_gatt_manager *btd_gatt_manager_new(struct btd_adapter *adapter)
+{
+	struct btd_gatt_manager *manager;
+
+	if (!adapter)
+		return NULL;
+
+	manager = manager_create(adapter);
+	if (!manager)
+		return NULL;
+
+	DBG("GATT Manager registered for adapter: %s",
+						adapter_get_path(adapter));
+
+	return manager;
+}
+
+void btd_gatt_manager_destroy(struct btd_gatt_manager *manager)
+{
+	if (!manager)
+		return;
+
+	g_dbus_unregister_interface(btd_get_dbus_connection(),
+					adapter_get_path(manager->adapter),
+					GATT_MANAGER_IFACE);
+	free(manager);
+}
diff --git a/src/gatt-manager.h b/src/gatt-manager.h
new file mode 100644
index 0000000..9573341
--- /dev/null
+++ b/src/gatt-manager.h
@@ -0,0 +1,23 @@
+/*
+ *
+ *  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.
+ *
+ */
+
+struct btd_gatt_manager;
+
+struct btd_gatt_manager *btd_gatt_manager_new(struct btd_adapter *adapter);
+void btd_gatt_manager_destroy(struct btd_gatt_manager *manager);
diff --git a/src/gatt.c b/src/gatt.c
deleted file mode 100644
index df5ea1d..0000000
--- a/src/gatt.c
+++ /dev/null
@@ -1,321 +0,0 @@
-/*
- *
- *  BlueZ - Bluetooth protocol stack for Linux
- *
- *  Copyright (C) 2014  Instituto Nokia de Tecnologia - INdT
- *
- *
- *  This program is free software; you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation; either version 2 of the License, or
- *  (at your option) any later version.
- *
- *  This program is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *  GNU General Public License for more details.
- *
- *  You should have received a copy of the GNU General Public License
- *  along with this program; if not, write to the Free Software
- *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
- *
- */
-
-#ifdef HAVE_CONFIG_H
-#include <config.h>
-#endif
-
-#include <glib.h>
-#include <stdbool.h>
-
-#include "log.h"
-#include "lib/bluetooth.h"
-#include "lib/uuid.h"
-#include "attrib/att.h"
-#include "src/shared/util.h"
-
-#include "gatt-dbus.h"
-#include "gatt.h"
-
-/* Common GATT UUIDs */
-static const bt_uuid_t primary_uuid  = { .type = BT_UUID16,
-					.value.u16 = GATT_PRIM_SVC_UUID };
-
-static const bt_uuid_t chr_uuid = { .type = BT_UUID16,
-					.value.u16 = GATT_CHARAC_UUID };
-
-struct btd_attribute {
-	uint16_t handle;
-	bt_uuid_t type;
-	btd_attr_read_t read_cb;
-	btd_attr_write_t write_cb;
-	uint16_t value_len;
-	uint8_t value[0];
-};
-
-static GList *local_attribute_db;
-static uint16_t next_handle = 0x0001;
-
-static inline void put_uuid_le(const bt_uuid_t *src, void *dst)
-{
-	if (src->type == BT_UUID16)
-		put_le16(src->value.u16, dst);
-	else if (src->type == BT_UUID32)
-		put_le32(src->value.u32, dst);
-	else
-		/* Convert from 128-bit BE to LE */
-		bswap_128(&src->value.u128, dst);
-}
-
-/*
- * Helper function to create new attributes containing constant/static values.
- * eg: declaration of services/characteristics, and characteristics with
- * fixed values.
- */
-static struct btd_attribute *new_const_attribute(const bt_uuid_t *type,
-							const uint8_t *value,
-							uint16_t len)
-{
-	struct btd_attribute *attr;
-
-	attr = malloc0(sizeof(struct btd_attribute) + len);
-	if (!attr)
-		return NULL;
-
-	attr->type = *type;
-	memcpy(&attr->value, value, len);
-	attr->value_len = len;
-
-	return attr;
-}
-
-static struct btd_attribute *new_attribute(const bt_uuid_t *type,
-						btd_attr_read_t read_cb,
-						btd_attr_write_t write_cb)
-{
-	struct btd_attribute *attr;
-
-	attr = new0(struct btd_attribute, 1);
-	if (!attr)
-		return NULL;
-
-	attr->type = *type;
-	attr->read_cb = read_cb;
-	attr->write_cb = write_cb;
-
-	return attr;
-}
-
-static bool is_service(const struct btd_attribute *attr)
-{
-	if (attr->type.type != BT_UUID16)
-		return false;
-
-	if (attr->type.value.u16 == GATT_PRIM_SVC_UUID ||
-			attr->type.value.u16 == GATT_SND_SVC_UUID)
-		return true;
-
-	return false;
-}
-
-static int local_database_add(uint16_t handle, struct btd_attribute *attr)
-{
-	attr->handle = handle;
-
-	local_attribute_db = g_list_append(local_attribute_db, attr);
-
-	return 0;
-}
-
-struct btd_attribute *btd_gatt_add_service(const bt_uuid_t *uuid)
-{
-	struct btd_attribute *attr;
-	uint16_t len = bt_uuid_len(uuid);
-	uint8_t value[len];
-
-	/*
-	 * Service DECLARATION
-	 *
-	 *   TYPE         ATTRIBUTE VALUE
-	 * +-------+---------------------------------+
-	 * |0x2800 | 0xYYYY...                       |
-	 * | (1)   | (2)                             |
-	 * +------+----------------------------------+
-	 * (1) - 2 octets: Primary/Secondary Service UUID
-	 * (2) - 2 or 16 octets: Service UUID
-	 */
-
-	/* Set attribute value */
-	put_uuid_le(uuid, value);
-
-	attr = new_const_attribute(&primary_uuid, value, len);
-	if (!attr)
-		return NULL;
-
-	if (local_database_add(next_handle, attr) < 0) {
-		free(attr);
-		return NULL;
-	}
-
-	/* TODO: missing overflow checking */
-	next_handle = next_handle + 1;
-
-	return attr;
-}
-
-void btd_gatt_remove_service(struct btd_attribute *service)
-{
-	GList *list = g_list_find(local_attribute_db, service);
-	bool first_node;
-
-	if (!list)
-		return;
-
-	first_node = local_attribute_db == list;
-
-	/* Remove service declaration attribute */
-	free(list->data);
-	list = g_list_delete_link(list, list);
-
-	/* Remove all characteristics until next service declaration */
-	while (list && !is_service(list->data)) {
-		free(list->data);
-		list = g_list_delete_link(list, list);
-	}
-
-	/*
-	 * When removing the first node, local attribute database head
-	 * needs to be updated. Node removed from middle doesn't change
-	 * the list head address.
-	 */
-	if (first_node)
-		local_attribute_db = list;
-}
-
-struct btd_attribute *btd_gatt_add_char(const bt_uuid_t *uuid,
-						uint8_t properties,
-						btd_attr_read_t read_cb,
-						btd_attr_write_t write_cb)
-{
-	struct btd_attribute *char_decl, *char_value = NULL;
-
-	/* Attribute value length */
-	uint16_t len = 1 + 2 + bt_uuid_len(uuid);
-	uint8_t value[len];
-
-	/*
-	 * Characteristic DECLARATION
-	 *
-	 *   TYPE         ATTRIBUTE VALUE
-	 * +-------+---------------------------------+
-	 * |0x2803 | 0xXX 0xYYYY 0xZZZZ...           |
-	 * | (1)   |  (2)   (3)   (4)                |
-	 * +------+----------------------------------+
-	 * (1) - 2 octets: Characteristic declaration UUID
-	 * (2) - 1 octet : Properties
-	 * (3) - 2 octets: Handle of the characteristic Value
-	 * (4) - 2 or 16 octets: Characteristic UUID
-	 */
-
-	value[0] = properties;
-
-	/*
-	 * Since we don't know yet the characteristic value attribute
-	 * handle, we skip and set it later.
-	 */
-
-	put_uuid_le(uuid, &value[3]);
-
-	char_decl = new_const_attribute(&chr_uuid, value, len);
-	if (!char_decl)
-		goto fail;
-
-	char_value = new_attribute(uuid, read_cb, write_cb);
-	if (!char_value)
-		goto fail;
-
-	if (local_database_add(next_handle, char_decl) < 0)
-		goto fail;
-
-	next_handle = next_handle + 1;
-
-	/*
-	 * Characteristic VALUE
-	 *
-	 *   TYPE         ATTRIBUTE VALUE
-	 * +----------+---------------------------------+
-	 * |0xZZZZ... | 0x...                           |
-	 * |  (1)     |  (2)                            |
-	 * +----------+---------------------------------+
-	 * (1) - 2 or 16 octets: Characteristic UUID
-	 * (2) - N octets: Value is read dynamically from the service
-	 * implementation (external entity).
-	 */
-
-	if (local_database_add(next_handle, char_value) < 0)
-		/* TODO: remove declaration */
-		goto fail;
-
-	next_handle = next_handle + 1;
-
-	/*
-	 * Update characteristic value handle in characteristic declaration
-	 * attribute. For local attributes, we can assume that the handle
-	 * representing the characteristic value will get the next available
-	 * handle. However, for remote attribute this assumption is not valid.
-	 */
-	put_le16(char_value->handle, &char_decl->value[1]);
-
-	return char_value;
-
-fail:
-	free(char_decl);
-	free(char_value);
-
-	return NULL;
-}
-
-struct btd_attribute *btd_gatt_add_char_desc(const bt_uuid_t *uuid,
-						btd_attr_read_t read_cb,
-						btd_attr_write_t write_cb)
-{
-	struct btd_attribute *attr;
-
-	/*
-	 * From Core SPEC 4.1 page 2184:
-	 * "Characteristic descriptor declaration permissions are defined by a
-	 * higher layer profile or are implementation specific. A client shall
-	 * not assume all characteristic descriptor declarations are readable."
-	 *
-	 * The read/write callbacks presence will define the descriptor
-	 * permissions managed directly by the core. The upper layer can define
-	 * additional permissions constraints.
-	 */
-
-	attr = new_attribute(uuid, read_cb, write_cb);
-	if (!attr)
-		return NULL;
-
-	if (local_database_add(next_handle, attr) < 0) {
-		free(attr);
-		return NULL;
-	}
-
-	next_handle = next_handle + 1;
-
-	return attr;
-}
-
-void gatt_init(void)
-{
-	DBG("Starting GATT server");
-
-	gatt_dbus_manager_register();
-}
-
-void gatt_cleanup(void)
-{
-	DBG("Stopping GATT server");
-
-	gatt_dbus_manager_unregister();
-}
diff --git a/src/gatt.h b/src/gatt.h
deleted file mode 100644
index f16541e..0000000
--- a/src/gatt.h
+++ /dev/null
@@ -1,121 +0,0 @@
-/*
- *
- *  BlueZ - Bluetooth protocol stack for Linux
- *
- *  Copyright (C) 2014  Instituto Nokia de Tecnologia - INdT
- *
- *
- *  This program is free software; you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation; either version 2 of the License, or
- *  (at your option) any later version.
- *
- *  This program is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *  GNU General Public License for more details.
- *
- *  You should have received a copy of the GNU General Public License
- *  along with this program; if not, write to the Free Software
- *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
- *
- */
-
-struct btd_attribute;
-
-void gatt_init(void);
-
-void gatt_cleanup(void);
-
-/*
- * Read operation result callback. Called from the service implementation
- * informing the core (ATT layer) the result of read operation.
- * @err:	error in -errno format.
- * @value:	value of the attribute read.
- * @len:	length of value.
- * @user_data:	user_data passed in btd_attr_read_t callback.
- */
-typedef void (*btd_attr_read_result_t) (int err, uint8_t *value, size_t len,
-							void *user_data);
-/*
- * Service implementation callback passed to core (ATT layer). It manages read
- * operations received from remote devices.
- * @attr:	reference of the attribute to be read.
- * @result:	callback called from the service implementation informing the
- *		value of attribute read.
- * @user_data:	user_data passed in btd_attr_read_result_t callback.
- */
-typedef void (*btd_attr_read_t) (struct btd_attribute *attr,
-						btd_attr_read_result_t result,
-						void *user_data);
-
-/*
- * Write operation result callback. Called from the service implementation
- * informing the core (ATT layer) the result of the write operation. It is used
- * to manage Write Request operations.
- * @err:	error in -errno format.
- * @user_data:	user_data passed in btd_attr_write_t callback.
- */
-typedef void (*btd_attr_write_result_t) (int err, void *user_data);
-/*
- * Service implementation callback passed to core (ATT layer). It manages write
- * operations received from remote devices.
- * @attr:	reference of the attribute to be changed.
- * @value:	new attribute value.
- * @len:	length of value.
- * @result:	callback called from the service implementation informing the
- *		result of the write operation.
- * @user_data:	user_data passed in btd_attr_write_result_t callback.
- */
-typedef void (*btd_attr_write_t) (struct btd_attribute *attr,
-					const uint8_t *value, size_t len,
-					btd_attr_write_result_t result,
-					void *user_data);
-
-/* btd_gatt_add_service - Add a service declaration to local attribute database.
- * @uuid:	Service UUID.
- *
- * Returns a reference to service declaration attribute. In case of error,
- * NULL is returned.
- */
-struct btd_attribute *btd_gatt_add_service(const bt_uuid_t *uuid);
-
-/*
- * btd_gatt_remove_service - Remove a service (along with all its
- * characteristics) from the local attribute database.
- * @service:	Service declaration attribute.
- */
-void btd_gatt_remove_service(struct btd_attribute *service);
-
-/*
- * btd_gatt_add_char - Add a characteristic (declaration and value attributes)
- * to local attribute database.
- * @uuid:	Characteristic UUID (16-bits or 128-bits).
- * @properties:	Characteristic properties. See Core SPEC 4.1 page 2183.
- * @read_cb:	Callback used to provide the characteristic value.
- * @write_cb:	Callback called to notify the implementation that a new value
- *              is available.
- *
- * Returns a reference to characteristic value attribute. In case of error,
- * NULL is returned.
- */
-struct btd_attribute *btd_gatt_add_char(const bt_uuid_t *uuid,
-						uint8_t properties,
-						btd_attr_read_t read_cb,
-						btd_attr_write_t write_cb);
-
-/*
- * btd_gatt_add_char_desc - Add a characteristic descriptor to the local
- * attribute database.
- * @uuid:	Characteristic Descriptor UUID (16-bits or 128-bits).
- * @read_cb:	Callback that should be called once the characteristic
- *		descriptor attribute is read.
- * @write_cb:	Callback that should be called once the characteristic
- *		descriptor attribute is written.
- *
- * Returns a reference to characteristic descriptor attribute. In case of
- * error, NULL is returned.
- */
-struct btd_attribute *btd_gatt_add_char_desc(const bt_uuid_t *uuid,
-						btd_attr_read_t read_cb,
-						btd_attr_write_t write_cb);
diff --git a/src/main.c b/src/main.c
index 05eb13d..679ee78 100644
--- a/src/main.c
+++ b/src/main.c
@@ -56,7 +56,6 @@
 #include "dbus-common.h"
 #include "agent.h"
 #include "profile.h"
-#include "gatt.h"
 #include "systemd.h"
 
 #define BLUEZ_NAME "org.bluez"
@@ -600,8 +599,6 @@ int main(int argc, char *argv[])
 
 	g_dbus_set_flags(gdbus_flags);
 
-	gatt_init();
-
 	if (adapter_init() < 0) {
 		error("Adapter handling initialization failed");
 		exit(1);
@@ -667,8 +664,6 @@ int main(int argc, char *argv[])
 
 	adapter_cleanup();
 
-	gatt_cleanup();
-
 	rfkill_exit();
 
 	stop_sdp_server();
-- 
2.2.0.rc0.207.ga3a616c


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

* [PATCH BlueZ v1 06/17] core/gatt: Implement GattManager1.RegisterService
  2015-02-26  5:13 [PATCH BlueZ v1 00/17] Implement GATT server D-Bus API Arman Uguray
                   ` (4 preceding siblings ...)
  2015-02-26  5:13 ` [PATCH BlueZ v1 05/17] core/gatt: Add GattManager1 stubs Arman Uguray
@ 2015-02-26  5:13 ` Arman Uguray
  2015-02-26  5:13 ` [PATCH BlueZ v1 07/17] core/gatt: Register characteristics Arman Uguray
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Arman Uguray @ 2015-02-26  5:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Arman Uguray

This patch adds the initial implementation of the RegisterService
method. Currently only one attribute entry is created in the local
database for the GATT service declaration.
---
 src/gatt-manager.c | 334 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 333 insertions(+), 1 deletion(-)

diff --git a/src/gatt-manager.c b/src/gatt-manager.c
index 296eabc..0dc4626 100644
--- a/src/gatt-manager.c
+++ b/src/gatt-manager.c
@@ -26,27 +26,341 @@
 
 #include <dbus/dbus.h>
 #include <gdbus/gdbus.h>
+#include <glib.h>
 
+#include "lib/bluetooth.h"
+#include "lib/uuid.h"
 #include "adapter.h"
 #include "gatt-manager.h"
+#include "gatt-database.h"
 #include "dbus-common.h"
 #include "log.h"
 #include "error.h"
 #include "src/shared/queue.h"
 #include "src/shared/util.h"
+#include "src/shared/att.h"
+#include "src/shared/gatt-db.h"
 
 #define GATT_MANAGER_IFACE	"org.bluez.GattManager1"
+#define GATT_SERVICE_IFACE	"org.bluez.GattService1"
+
+#define UUID_GAP	0x1800
+#define UUID_GATT	0x1801
 
 struct btd_gatt_manager {
 	struct btd_adapter *adapter;
+	struct gatt_db *db;
+	struct queue *services;
+};
+
+struct external_service {
+	struct btd_gatt_manager *manager;
+	char *owner;
+	char *path;	/* Path to GattService1 */
+	DBusMessage *reg;
+	GDBusClient *client;
+	GDBusProxy *proxy;
+	struct gatt_db_attribute *attrib;
 };
 
+static bool match_service_path(const void *a, const void *b)
+{
+	const struct external_service *service = a;
+	const char *path = b;
+
+	return g_strcmp0(service->path, path) == 0;
+}
+
+static void service_free(void *data)
+{
+	struct external_service *service = data;
+
+	gatt_db_remove_service(service->manager->db, service->attrib);
+
+	if (service->client) {
+		g_dbus_client_set_disconnect_watch(service->client, NULL, NULL);
+		g_dbus_client_set_proxy_handlers(service->client, NULL, NULL,
+								NULL, NULL);
+		g_dbus_client_set_ready_watch(service->client, NULL, NULL);
+		g_dbus_client_unref(service->client);
+	}
+
+	if (service->proxy)
+		g_dbus_proxy_unref(service->proxy);
+
+	if (service->reg)
+		dbus_message_unref(service->reg);
+
+	if (service->owner)
+		g_free(service->owner);
+
+	if (service->path)
+		g_free(service->path);
+
+	free(service);
+}
+
+static gboolean service_free_idle_cb(void *data)
+{
+	service_free(data);
+
+	return FALSE;
+}
+
+static void service_remove_helper(void *data)
+{
+	struct external_service *service = data;
+
+	queue_remove(service->manager->services, service);
+
+	/*
+	 * Do not run in the same loop, this may be a disconnect
+	 * watch call and GDBusClient should not be destroyed.
+	 */
+	g_idle_add(service_free_idle_cb, service);
+}
+
+static void client_disconnect_cb(DBusConnection *conn, void *user_data)
+{
+	DBG("Client disconnected");
+
+	service_remove_helper(user_data);
+}
+
+static void service_remove(void *data)
+{
+	struct external_service *service = data;
+
+	/*
+	 * Set callback to NULL to avoid potential race condition
+	 * when calling remove_service and GDBusClient unref.
+	 */
+	g_dbus_client_set_disconnect_watch(service->client, NULL, NULL);
+
+	service_remove_helper(service);
+}
+
+static void proxy_added_cb(GDBusProxy *proxy, void *user_data)
+{
+	struct external_service *service = user_data;
+	const char *iface, *path;
+
+	iface = g_dbus_proxy_get_interface(proxy);
+	path = g_dbus_proxy_get_path(proxy);
+
+	if (!g_str_has_prefix(path, service->path))
+		return;
+
+	/* TODO: Handle characteristic and descriptors here */
+
+	if (g_strcmp0(iface, GATT_SERVICE_IFACE))
+		return;
+
+	DBG("Object added to service - path: %s, iface: %s", path, iface);
+
+	service->proxy = g_dbus_proxy_ref(proxy);
+}
+
+static void proxy_removed_cb(GDBusProxy *proxy, void *user_data)
+{
+	struct external_service *service = user_data;
+	const char *path;
+
+	path = g_dbus_proxy_get_path(proxy);
+
+	if (!g_str_has_prefix(path, service->path))
+		return;
+
+	DBG("Proxy removed - removing service: %s", service->path);
+
+	service_remove(service);
+}
+
+static bool parse_uuid(GDBusProxy *proxy, bt_uuid_t *uuid)
+{
+	DBusMessageIter iter;
+	bt_uuid_t tmp;
+	const char *uuidstr;
+
+	if (!g_dbus_proxy_get_property(proxy, "UUID", &iter))
+		return false;
+
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
+		return false;
+
+	dbus_message_iter_get_basic(&iter, &uuidstr);
+
+	if (bt_string_to_uuid(uuid, uuidstr) < 0)
+		return false;
+
+	/* GAP & GATT services are created and managed by BlueZ */
+	bt_uuid16_create(&tmp, UUID_GAP);
+	if (!bt_uuid_cmp(&tmp, uuid)) {
+		error("GAP service must be handled by BlueZ");
+		return false;
+	}
+
+	bt_uuid16_create(&tmp, UUID_GATT);
+	if (!bt_uuid_cmp(&tmp, uuid)) {
+		error("GATT service must be handled by BlueZ");
+		return false;
+	}
+
+	return true;
+}
+
+static bool parse_primary(GDBusProxy *proxy, bool *primary)
+{
+	DBusMessageIter iter;
+
+	if (!g_dbus_proxy_get_property(proxy, "Primary", &iter))
+		return false;
+
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_BOOLEAN)
+		return false;
+
+	dbus_message_iter_get_basic(&iter, primary);
+
+	return true;
+}
+
+static bool create_service_entry(struct external_service *service)
+{
+	bt_uuid_t uuid;
+	bool primary;
+
+	if (!parse_uuid(service->proxy, &uuid)) {
+		error("Failed to read \"UUID\" property of service");
+		return false;
+	}
+
+	if (!parse_primary(service->proxy, &primary)) {
+		error("Failed to read \"Primary\" property of service");
+		return false;
+	}
+
+	/* TODO: Determine the correct attribute count */
+	service->attrib = gatt_db_add_service(service->manager->db, &uuid,
+								primary, 1);
+	if (!service->attrib)
+		return false;
+
+	gatt_db_service_set_active(service->attrib, true);
+
+	return true;
+}
+
+static void client_ready_cb(GDBusClient *client, void *user_data)
+{
+	struct external_service *service = user_data;
+	DBusMessage *reply;
+	bool fail = false;
+
+	if (!service->proxy) {
+		error("No external GATT objects found");
+		fail = true;
+		reply = btd_error_failed(service->reg,
+						"No service object found");
+		goto reply;
+	}
+
+	if (!create_service_entry(service)) {
+		error("Failed to create GATT service entry in local database");
+		fail = true;
+		reply = btd_error_failed(service->reg,
+					"Failed to create entry in database");
+		goto reply;
+	}
+
+	DBG("GATT service registered: %s", service->path);
+
+	reply = dbus_message_new_method_return(service->reg);
+
+reply:
+	g_dbus_send_message(btd_get_dbus_connection(), reply);
+	dbus_message_unref(service->reg);
+	service->reg = NULL;
+
+	if (fail)
+		service_remove(service);
+}
+
+static struct external_service *service_create(DBusConnection *conn,
+					DBusMessage *msg, const char *path)
+{
+	struct external_service *service;
+	const char *sender = dbus_message_get_sender(msg);
+
+	if (!path || !g_str_has_prefix(path, "/"))
+		return NULL;
+
+	service = new0(struct external_service, 1);
+	if (!service)
+		return NULL;
+
+	service->client = g_dbus_client_new(conn, sender, path, path);
+	if (!service->client)
+		goto fail;
+
+	service->owner = g_strdup(sender);
+	if (!service->owner)
+		goto fail;
+
+	service->path = g_strdup(path);
+	if (!service->path)
+		goto fail;
+
+	service->reg = dbus_message_ref(msg);
+
+	g_dbus_client_set_disconnect_watch(service->client,
+						client_disconnect_cb, service);
+	g_dbus_client_set_proxy_handlers(service->client, proxy_added_cb,
+							proxy_removed_cb, NULL,
+							service);
+	g_dbus_client_set_ready_watch(service->client, client_ready_cb,
+								service);
+
+	return service;
+
+fail:
+	service_free(service);
+	return NULL;
+}
+
 static DBusMessage *manager_register_service(DBusConnection *conn,
 					DBusMessage *msg, void *user_data)
 {
+	struct btd_gatt_manager *manager = user_data;
+	DBusMessageIter args;
+	const char *path;
+	struct external_service *service;
+
 	DBG("RegisterService");
 
-	/* TODO */
+	if (!dbus_message_iter_init(msg, &args))
+		return btd_error_invalid_args(msg);
+
+	if (dbus_message_iter_get_arg_type(&args) != DBUS_TYPE_OBJECT_PATH)
+		return btd_error_invalid_args(msg);
+
+	dbus_message_iter_get_basic(&args, &path);
+
+	if (queue_find(manager->services, match_service_path, path))
+		return btd_error_already_exists(msg);
+
+	dbus_message_iter_next(&args);
+	if (dbus_message_iter_get_arg_type(&args) != DBUS_TYPE_ARRAY)
+		return btd_error_invalid_args(msg);
+
+	service = service_create(conn, msg, path);
+	if (!service)
+		return btd_error_failed(msg, "Failed to register service");
+
+	DBG("Registering service - path: %s", path);
+
+	service->manager = manager;
+	queue_push_tail(manager->services, service);
+
 	return NULL;
 }
 
@@ -72,11 +386,24 @@ static const GDBusMethodTable manager_methods[] = {
 static struct btd_gatt_manager *manager_create(struct btd_adapter *adapter)
 {
 	struct btd_gatt_manager *manager;
+	struct gatt_db *db;
+	struct btd_gatt_database *database;
+
+	database = btd_adapter_get_database(adapter);
+	db = btd_gatt_database_get_db(database);
+	if (!db)
+		return NULL;
 
 	manager = new0(struct btd_gatt_manager, 1);
 	if (!manager)
 		return NULL;
 
+	manager->services = queue_new();
+	if (!manager->services) {
+		free(manager);
+		return NULL;
+	}
+
 	manager->adapter = adapter;
 
 	if (!g_dbus_register_interface(btd_get_dbus_connection(),
@@ -85,10 +412,13 @@ static struct btd_gatt_manager *manager_create(struct btd_adapter *adapter)
 						manager_methods, NULL, NULL,
 						manager, NULL)) {
 		error("Failed to register " GATT_MANAGER_IFACE);
+		queue_destroy(manager->services, NULL);
 		free(manager);
 		return NULL;
 	}
 
+	manager->db = gatt_db_ref(db);
+
 	return manager;
 }
 
@@ -117,5 +447,7 @@ void btd_gatt_manager_destroy(struct btd_gatt_manager *manager)
 	g_dbus_unregister_interface(btd_get_dbus_connection(),
 					adapter_get_path(manager->adapter),
 					GATT_MANAGER_IFACE);
+	queue_destroy(manager->services, service_free);
+	gatt_db_unref(manager->db);
 	free(manager);
 }
-- 
2.2.0.rc0.207.ga3a616c


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

* [PATCH BlueZ v1 07/17] core/gatt: Register characteristics
  2015-02-26  5:13 [PATCH BlueZ v1 00/17] Implement GATT server D-Bus API Arman Uguray
                   ` (5 preceding siblings ...)
  2015-02-26  5:13 ` [PATCH BlueZ v1 06/17] core/gatt: Implement GattManager1.RegisterService Arman Uguray
@ 2015-02-26  5:13 ` Arman Uguray
  2015-02-26  5:13 ` [PATCH BlueZ v1 08/17] core/gatt: Support ReadValue for characteristics Arman Uguray
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Arman Uguray @ 2015-02-26  5:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Arman Uguray

This patch adds support for registering characteristic objects. This
adds internal plumbing for determining the final attribute count for
a service handle range and creates characteristic entries in the server
database.
---
 src/gatt-manager.c | 250 ++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 240 insertions(+), 10 deletions(-)

diff --git a/src/gatt-manager.c b/src/gatt-manager.c
index 0dc4626..cc52cd1 100644
--- a/src/gatt-manager.c
+++ b/src/gatt-manager.c
@@ -43,6 +43,7 @@
 
 #define GATT_MANAGER_IFACE	"org.bluez.GattManager1"
 #define GATT_SERVICE_IFACE	"org.bluez.GattService1"
+#define GATT_CHRC_IFACE		"org.bluez.GattCharacteristic1"
 
 #define UUID_GAP	0x1800
 #define UUID_GATT	0x1801
@@ -55,11 +56,21 @@ struct btd_gatt_manager {
 
 struct external_service {
 	struct btd_gatt_manager *manager;
+	bool failed;
 	char *owner;
 	char *path;	/* Path to GattService1 */
 	DBusMessage *reg;
 	GDBusClient *client;
 	GDBusProxy *proxy;
+	uint16_t attr_cnt;
+	struct gatt_db_attribute *attrib;
+	struct queue *chrcs;
+};
+
+struct external_chrc {
+	GDBusProxy *proxy;
+	uint8_t props;
+	uint8_t ext_props;
 	struct gatt_db_attribute *attrib;
 };
 
@@ -71,10 +82,23 @@ static bool match_service_path(const void *a, const void *b)
 	return g_strcmp0(service->path, path) == 0;
 }
 
+static void chrc_free(void *data)
+{
+	struct external_chrc *chrc = data;
+
+	if (chrc->proxy)
+		g_dbus_proxy_unref(chrc->proxy);
+
+	free(chrc);
+}
+
 static void service_free(void *data)
 {
 	struct external_service *service = data;
 
+	if (service->chrcs)
+		queue_destroy(service->chrcs, chrc_free);
+
 	gatt_db_remove_service(service->manager->db, service->attrib);
 
 	if (service->client) {
@@ -137,13 +161,114 @@ static void service_remove(void *data)
 	 */
 	g_dbus_client_set_disconnect_watch(service->client, NULL, NULL);
 
+	/*
+	 * Set proxy handlers to NULL, so that this gets called only once when
+	 * the first proxy that belongs to this service gets removed.
+	 */
+	g_dbus_client_set_proxy_handlers(service->client, NULL, NULL,
+								NULL, NULL);
+
 	service_remove_helper(service);
 }
 
+static struct external_chrc *chrc_create(GDBusProxy *proxy)
+{
+	struct external_chrc *chrc;
+
+	chrc = new0(struct external_chrc, 1);
+	if (!chrc)
+		return NULL;
+
+	chrc->proxy = g_dbus_proxy_ref(proxy);
+
+	return chrc;
+}
+
+static bool incr_attr_count(struct external_service *service, uint16_t incr)
+{
+	if (service->attr_cnt > UINT16_MAX - incr)
+		return false;
+
+	service->attr_cnt += incr;
+
+	return true;
+}
+
+static bool parse_service(GDBusProxy *proxy, struct external_service *service)
+{
+	DBusMessageIter iter;
+	const char *service_path;
+
+	if (!g_dbus_proxy_get_property(proxy, "Service", &iter))
+		return false;
+
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_OBJECT_PATH)
+		return false;
+
+	dbus_message_iter_get_basic(&iter, &service_path);
+
+	return g_strcmp0(service_path, service->path) == 0;
+}
+
+static bool parse_flags(GDBusProxy *proxy, uint8_t *props, uint8_t *ext_props)
+{
+	DBusMessageIter iter, array;
+	const char *flag;
+
+	*props = *ext_props = 0;
+
+	if (!g_dbus_proxy_get_property(proxy, "Flags", &iter))
+		return false;
+
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_ARRAY)
+		return false;
+
+	dbus_message_iter_recurse(&iter, &array);
+
+	do {
+		if (dbus_message_iter_get_arg_type(&array) != DBUS_TYPE_STRING)
+			return false;
+
+		dbus_message_iter_get_basic(&array, &flag);
+
+		if (!strcmp("broadcast", flag))
+			*props |= BT_GATT_CHRC_PROP_BROADCAST;
+		else if (!strcmp("read", flag))
+			*props |= BT_GATT_CHRC_PROP_READ;
+		else if (!strcmp("write-without-response", flag))
+			*props |= BT_GATT_CHRC_PROP_WRITE_WITHOUT_RESP;
+		else if (!strcmp("write", flag))
+			*props |= BT_GATT_CHRC_PROP_WRITE;
+		else if (!strcmp("notify", flag))
+			*props |= BT_GATT_CHRC_PROP_NOTIFY;
+		else if (!strcmp("indicate", flag))
+			*props |= BT_GATT_CHRC_PROP_INDICATE;
+		else if (!strcmp("authenticated-signed-writes", flag))
+			*props |= BT_GATT_CHRC_PROP_AUTH;
+		else if (!strcmp("reliable-write", flag))
+			*ext_props |= BT_GATT_CHRC_EXT_PROP_RELIABLE_WRITE;
+		else if (!strcmp("writable-auxiliaries", flag))
+			*ext_props |= BT_GATT_CHRC_EXT_PROP_WRITABLE_AUX;
+		else {
+			error("Invalid characteristic flag: %s", flag);
+			return false;
+		}
+	} while (dbus_message_iter_next(&array));
+
+	if (*ext_props)
+		*props |= BT_GATT_CHRC_PROP_EXT_PROP;
+
+	return true;
+}
+
 static void proxy_added_cb(GDBusProxy *proxy, void *user_data)
 {
 	struct external_service *service = user_data;
 	const char *iface, *path;
+	struct external_chrc *chrc;
+
+	if (service->failed || service->attrib)
+		return;
 
 	iface = g_dbus_proxy_get_interface(proxy);
 	path = g_dbus_proxy_get_path(proxy);
@@ -151,14 +276,74 @@ static void proxy_added_cb(GDBusProxy *proxy, void *user_data)
 	if (!g_str_has_prefix(path, service->path))
 		return;
 
-	/* TODO: Handle characteristic and descriptors here */
-
-	if (g_strcmp0(iface, GATT_SERVICE_IFACE))
+	/* TODO: Handle descriptors here */
+
+	if (g_strcmp0(iface, GATT_SERVICE_IFACE) == 0) {
+		if (service->proxy)
+			return;
+
+		/*
+		 * TODO: We may want to support adding included services in a
+		 * single hierarchy.
+		 */
+		if (g_strcmp0(path, service->path) != 0) {
+			error("Multiple services added within hierarchy");
+			service->failed = true;
+			return;
+		}
+
+		/* Add 1 for the service declaration */
+		if (!incr_attr_count(service, 1)) {
+			error("Failed to increment attribute count");
+			service->failed = true;
+			return;
+		}
+
+		service->proxy = g_dbus_proxy_ref(proxy);
+	} else if (g_strcmp0(iface, GATT_CHRC_IFACE) == 0) {
+		if (g_strcmp0(path, service->path) == 0) {
+			error("Characteristic path same as service path");
+			service->failed = true;
+			return;
+		}
+
+		chrc = chrc_create(proxy);
+		if (!chrc) {
+			service->failed = true;
+			return;
+		}
+
+		/*
+		 * Add 2 for the characteristic declaration and the value
+		 * attribute.
+		 */
+		if (!incr_attr_count(service, 2)) {
+			error("Failed to increment attribute count");
+			service->failed = true;
+			return;
+		}
+
+		/*
+		 * Parse characteristic flags (i.e. properties) here since they
+		 * are used to determine if any special descriptors should be
+		 * created.
+		 */
+		if (!parse_flags(proxy, &chrc->props, &chrc->ext_props)) {
+			error("Failed to parse characteristic properties");
+			service->failed = true;
+			return;
+		}
+
+		/*
+		 * TODO: Determine descriptors count to add based on special
+		 * characteristic properties (e.g. extended properties).
+		 */
+
+		queue_push_tail(service->chrcs, chrc);
+	} else
 		return;
 
 	DBG("Object added to service - path: %s, iface: %s", path, iface);
-
-	service->proxy = g_dbus_proxy_ref(proxy);
 }
 
 static void proxy_removed_cb(GDBusProxy *proxy, void *user_data)
@@ -224,10 +409,37 @@ static bool parse_primary(GDBusProxy *proxy, bool *primary)
 	return true;
 }
 
+static bool create_chrc_entry(struct external_service *service,
+						struct external_chrc *chrc)
+{
+	bt_uuid_t uuid;
+
+	if (!parse_uuid(chrc->proxy, &uuid)) {
+		error("Failed to read \"UUID\" property of characteristic");
+		return false;
+	}
+
+	if (!parse_service(chrc->proxy, service)) {
+		error("Invalid service path for characteristic");
+		service->failed = true;
+		return false;
+	}
+
+	/* TODO: Assign permissions and read/write callbacks */
+	chrc->attrib = gatt_db_service_add_characteristic(service->attrib,
+							&uuid, 0, chrc->props,
+							NULL, NULL, NULL);
+
+	/* TODO: Create descriptor entries */
+
+	return !!chrc->attrib;
+}
+
 static bool create_service_entry(struct external_service *service)
 {
 	bt_uuid_t uuid;
 	bool primary;
+	const struct queue_entry *entry;
 
 	if (!parse_uuid(service->proxy, &uuid)) {
 		error("Failed to read \"UUID\" property of service");
@@ -239,12 +451,26 @@ static bool create_service_entry(struct external_service *service)
 		return false;
 	}
 
-	/* TODO: Determine the correct attribute count */
 	service->attrib = gatt_db_add_service(service->manager->db, &uuid,
-								primary, 1);
+						primary, service->attr_cnt);
 	if (!service->attrib)
 		return false;
 
+	entry = queue_get_entries(service->chrcs);
+	while (entry) {
+		struct external_chrc *chrc = entry->data;
+
+		if (!create_chrc_entry(service, chrc)) {
+			error("Failed to create characteristic entry");
+			gatt_db_remove_service(service->manager->db,
+							service->attrib);
+			service->attrib = NULL;
+			return false;
+		}
+
+		entry = entry->next;
+	}
+
 	gatt_db_service_set_active(service->attrib, true);
 
 	return true;
@@ -256,11 +482,11 @@ static void client_ready_cb(GDBusClient *client, void *user_data)
 	DBusMessage *reply;
 	bool fail = false;
 
-	if (!service->proxy) {
-		error("No external GATT objects found");
+	if (!service->proxy || service->failed) {
+		error("No valid external GATT objects found");
 		fail = true;
 		reply = btd_error_failed(service->reg,
-						"No service object found");
+					"No valid service object found");
 		goto reply;
 	}
 
@@ -310,6 +536,10 @@ static struct external_service *service_create(DBusConnection *conn,
 	if (!service->path)
 		goto fail;
 
+	service->chrcs = queue_new();
+	if (!service->chrcs)
+		goto fail;
+
 	service->reg = dbus_message_ref(msg);
 
 	g_dbus_client_set_disconnect_watch(service->client,
-- 
2.2.0.rc0.207.ga3a616c


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

* [PATCH BlueZ v1 08/17] core/gatt: Support ReadValue for characteristics
  2015-02-26  5:13 [PATCH BlueZ v1 00/17] Implement GATT server D-Bus API Arman Uguray
                   ` (6 preceding siblings ...)
  2015-02-26  5:13 ` [PATCH BlueZ v1 07/17] core/gatt: Register characteristics Arman Uguray
@ 2015-02-26  5:13 ` Arman Uguray
  2015-02-26  5:13 ` [PATCH BlueZ v1 09/17] core/gatt: Support WriteValue " Arman Uguray
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Arman Uguray @ 2015-02-26  5:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Arman Uguray

This patch adds support for obtaining the characteristic value from an
external application during a read procedure.
---
 src/gatt-manager.c | 155 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 153 insertions(+), 2 deletions(-)

diff --git a/src/gatt-manager.c b/src/gatt-manager.c
index cc52cd1..e11d36d 100644
--- a/src/gatt-manager.c
+++ b/src/gatt-manager.c
@@ -48,6 +48,10 @@
 #define UUID_GAP	0x1800
 #define UUID_GATT	0x1801
 
+#ifndef MIN
+#define MIN(a, b) ((a) < (b) ? (a) : (b))
+#endif
+
 struct btd_gatt_manager {
 	struct btd_adapter *adapter;
 	struct gatt_db *db;
@@ -72,8 +76,35 @@ struct external_chrc {
 	uint8_t props;
 	uint8_t ext_props;
 	struct gatt_db_attribute *attrib;
+	struct queue *pending_ops;
+};
+
+struct pending_dbus_op {
+	struct external_chrc *chrc;
+	unsigned int id;
+	void *user_data;
 };
 
+static void cancel_pending_dbus_op(void *data, void *user_data)
+{
+	struct pending_dbus_op *op = data;
+
+	gatt_db_attribute_read_result(op->chrc->attrib, op->id,
+					BT_ATT_ERROR_REQUEST_NOT_SUPPORTED,
+					NULL, 0);
+	op->chrc = NULL;
+}
+
+static void pending_dbus_op_free(void *data)
+{
+	struct pending_dbus_op *op = data;
+
+	if (op->chrc)
+		queue_remove(op->chrc->pending_ops, op);
+
+	free(op);
+}
+
 static bool match_service_path(const void *a, const void *b)
 {
 	const struct external_service *service = a;
@@ -86,6 +117,8 @@ static void chrc_free(void *data)
 {
 	struct external_chrc *chrc = data;
 
+	queue_foreach(chrc->pending_ops, cancel_pending_dbus_op, NULL);
+
 	if (chrc->proxy)
 		g_dbus_proxy_unref(chrc->proxy);
 
@@ -179,6 +212,12 @@ static struct external_chrc *chrc_create(GDBusProxy *proxy)
 	if (!chrc)
 		return NULL;
 
+	chrc->pending_ops = queue_new();
+	if (!chrc->pending_ops) {
+		free(chrc);
+		return NULL;
+	}
+
 	chrc->proxy = g_dbus_proxy_ref(proxy);
 
 	return chrc;
@@ -409,6 +448,117 @@ static bool parse_primary(GDBusProxy *proxy, bool *primary)
 	return true;
 }
 
+static uint8_t dbus_error_to_att_ecode(const char *error_name)
+{
+	/* TODO: Parse error ATT ecode from error_message */
+
+	if (strcmp(error_name, "org.bluez.Error.Failed") == 0)
+		return 0x80;  /* For now return this "application error" */
+
+	if (strcmp(error_name, "org.bluez.Error.NotSupported") == 0)
+		return BT_ATT_ERROR_REQUEST_NOT_SUPPORTED;
+
+	if (strcmp(error_name, "org.bluez.Error.NotAuthorized") == 0)
+		return BT_ATT_ERROR_AUTHORIZATION;
+
+	if (strcmp(error_name, "org.bluez.Error.InvalidValueLength") == 0)
+		return BT_ATT_ERROR_INVALID_ATTRIBUTE_VALUE_LEN;
+
+	return 0;
+}
+
+static void read_reply_cb(DBusMessage *message, void *user_data)
+{
+	struct pending_dbus_op *op = user_data;
+	DBusError err;
+	DBusMessageIter iter, array;
+	uint8_t ecode = 0;
+	uint8_t *value = NULL;
+	int len = 0;
+
+	if (!op->chrc) {
+		DBG("Pending read was canceled when object got removed");
+		return;
+	}
+
+	dbus_error_init(&err);
+
+	if (dbus_set_error_from_message(&err, message) == TRUE) {
+		DBG("Failed to read value: %s: %s", err.name, err.message);
+		ecode = dbus_error_to_att_ecode(err.name);
+		ecode = ecode ? ecode : BT_ATT_ERROR_READ_NOT_PERMITTED;
+		dbus_error_free(&err);
+		goto done;
+	}
+
+	dbus_message_iter_init(message, &iter);
+
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_ARRAY) {
+		/*
+		 * Return not supported for this, as the external app basically
+		 * doesn't properly support reading from this characteristic.
+		 */
+		ecode = BT_ATT_ERROR_REQUEST_NOT_SUPPORTED;
+		error("Invalid return value received for \"ReadValue\"");
+		goto done;
+	}
+
+	dbus_message_iter_recurse(&iter, &array);
+	dbus_message_iter_get_fixed_array(&array, &value, &len);
+
+	if (len < 0) {
+		ecode = BT_ATT_ERROR_REQUEST_NOT_SUPPORTED;
+		value = NULL;
+		len = 0;
+		goto done;
+	}
+
+	/* Truncate the value if it's too large */
+	len = MIN(BT_ATT_MAX_VALUE_LEN, len);
+	value = len ? value : NULL;
+
+done:
+	gatt_db_attribute_read_result(op->chrc->attrib, op->id, ecode,
+								value, len);
+}
+
+static void chrc_read_cb(struct gatt_db_attribute *attrib,
+					unsigned int id, uint16_t offset,
+					uint8_t opcode, struct bt_att *att,
+					void *user_data)
+{
+	struct external_chrc *chrc = user_data;
+	struct pending_dbus_op *op;
+	uint8_t ecode = BT_ATT_ERROR_UNLIKELY;
+
+	if (chrc->attrib != attrib) {
+		error("Read callback called with incorrect attribute");
+		goto error;
+
+	}
+
+	op = new0(struct pending_dbus_op, 1);
+	if (!op) {
+		error("Failed to allocate memory for pending read call");
+		ecode = BT_ATT_ERROR_INSUFFICIENT_RESOURCES;
+		goto error;
+	}
+
+	op->chrc = chrc;
+	op->id = id;
+	queue_push_tail(chrc->pending_ops, op);
+
+	if (g_dbus_proxy_method_call(chrc->proxy, "ReadValue", NULL,
+						read_reply_cb, op,
+						pending_dbus_op_free) == TRUE)
+		return;
+
+	pending_dbus_op_free(op);
+
+error:
+	gatt_db_attribute_read_result(attrib, id, ecode, NULL, 0);
+}
+
 static bool create_chrc_entry(struct external_service *service,
 						struct external_chrc *chrc)
 {
@@ -425,10 +575,11 @@ static bool create_chrc_entry(struct external_service *service,
 		return false;
 	}
 
-	/* TODO: Assign permissions and read/write callbacks */
+	/* TODO: Assign permissions and write callback */
 	chrc->attrib = gatt_db_service_add_characteristic(service->attrib,
 							&uuid, 0, chrc->props,
-							NULL, NULL, NULL);
+							chrc_read_cb,
+							NULL, chrc);
 
 	/* TODO: Create descriptor entries */
 
-- 
2.2.0.rc0.207.ga3a616c


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

* [PATCH BlueZ v1 09/17] core/gatt: Support WriteValue for characteristics
  2015-02-26  5:13 [PATCH BlueZ v1 00/17] Implement GATT server D-Bus API Arman Uguray
                   ` (7 preceding siblings ...)
  2015-02-26  5:13 ` [PATCH BlueZ v1 08/17] core/gatt: Support ReadValue for characteristics Arman Uguray
@ 2015-02-26  5:13 ` Arman Uguray
  2015-02-26  5:13 ` [PATCH BlueZ v1 10/17] core/gatt: Make CCC addition API public Arman Uguray
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Arman Uguray @ 2015-02-26  5:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Arman Uguray

This patch adds support for writing to a characteristic from an external
application during a write procedure.
---
 src/gatt-manager.c | 120 ++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 115 insertions(+), 5 deletions(-)

diff --git a/src/gatt-manager.c b/src/gatt-manager.c
index e11d36d..9e09894 100644
--- a/src/gatt-manager.c
+++ b/src/gatt-manager.c
@@ -559,10 +559,116 @@ error:
 	gatt_db_attribute_read_result(attrib, id, ecode, NULL, 0);
 }
 
+static void write_setup_cb(DBusMessageIter *iter, void *user_data)
+{
+	struct pending_dbus_op *op = user_data;
+	struct iovec *iov = op->user_data;
+	DBusMessageIter array;
+
+	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, "y", &array);
+	dbus_message_iter_append_fixed_array(&array, DBUS_TYPE_BYTE,
+						&iov->iov_base, iov->iov_len);
+	dbus_message_iter_close_container(iter, &array);
+}
+
+static void write_reply_cb(DBusMessage *message, void *user_data)
+{
+	struct pending_dbus_op *op = user_data;
+	DBusError err;
+	DBusMessageIter iter;
+	uint8_t ecode = 0;
+
+	if (!op->chrc) {
+		DBG("Pending write was canceled when object got removed");
+		return;
+	}
+
+	dbus_error_init(&err);
+
+	if (dbus_set_error_from_message(&err, message) == TRUE) {
+		DBG("Failed to write value: %s: %s", err.name, err.message);
+		ecode = dbus_error_to_att_ecode(err.name);
+		ecode = ecode ? ecode : BT_ATT_ERROR_WRITE_NOT_PERMITTED;
+		dbus_error_free(&err);
+		goto done;
+	}
+
+	dbus_message_iter_init(message, &iter);
+	if (dbus_message_iter_has_next(&iter)) {
+		/*
+		 * Return not supported for this, as the external app basically
+		 * doesn't properly support the "WriteValue" API.
+		 */
+		ecode = BT_ATT_ERROR_REQUEST_NOT_SUPPORTED;
+		error("Invalid return value received for \"WriteValue\"");
+	}
+
+done:
+	gatt_db_attribute_write_result(op->chrc->attrib, op->id, ecode);
+}
+
+static void chrc_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 external_chrc *chrc = user_data;
+	struct pending_dbus_op *op;
+	uint8_t ecode = BT_ATT_ERROR_UNLIKELY;
+	struct iovec iov;
+
+	if (chrc->attrib != attrib) {
+		error("Write callback called with incorrect attribute");
+		goto error;
+	}
+
+	op = new0(struct pending_dbus_op, 1);
+	if (!op) {
+		error("Failed to allocate memory for pending read call");
+		ecode = BT_ATT_ERROR_INSUFFICIENT_RESOURCES;
+		goto error;
+	}
+
+	iov.iov_base = (uint8_t *) value;
+	iov.iov_len = len;
+
+	op->chrc = chrc;
+	op->id = id;
+	op->user_data = &iov;
+	queue_push_tail(chrc->pending_ops, op);
+
+	if (g_dbus_proxy_method_call(chrc->proxy, "WriteValue", write_setup_cb,
+						write_reply_cb, op,
+						pending_dbus_op_free) == TRUE)
+		return;
+
+	pending_dbus_op_free(op);
+
+error:
+	gatt_db_attribute_write_result(attrib, id, ecode);
+}
+
+static uint32_t permissions_from_props(uint8_t props, uint8_t ext_props)
+{
+	uint32_t perm = 0;
+
+	if (props & BT_GATT_CHRC_PROP_WRITE ||
+			props & BT_GATT_CHRC_PROP_WRITE_WITHOUT_RESP ||
+			ext_props & BT_GATT_CHRC_EXT_PROP_RELIABLE_WRITE)
+		perm |= BT_ATT_PERM_WRITE;
+
+	if (props & BT_GATT_CHRC_PROP_READ)
+		perm |= BT_ATT_PERM_READ;
+
+	return perm;
+}
+
 static bool create_chrc_entry(struct external_service *service,
 						struct external_chrc *chrc)
 {
 	bt_uuid_t uuid;
+	uint32_t perm;
 
 	if (!parse_uuid(chrc->proxy, &uuid)) {
 		error("Failed to read \"UUID\" property of characteristic");
@@ -571,15 +677,19 @@ static bool create_chrc_entry(struct external_service *service,
 
 	if (!parse_service(chrc->proxy, service)) {
 		error("Invalid service path for characteristic");
-		service->failed = true;
 		return false;
 	}
 
-	/* TODO: Assign permissions and write callback */
+	/*
+	 * TODO: Once shared/gatt-server properly supports permission checks,
+	 * set the permissions based on a D-Bus property of the external
+	 * characteristic.
+	 */
+	perm = permissions_from_props(chrc->props, chrc->ext_props);
 	chrc->attrib = gatt_db_service_add_characteristic(service->attrib,
-							&uuid, 0, chrc->props,
-							chrc_read_cb,
-							NULL, chrc);
+						&uuid, perm,
+						chrc->props, chrc_read_cb,
+						chrc_write_cb, chrc);
 
 	/* TODO: Create descriptor entries */
 
-- 
2.2.0.rc0.207.ga3a616c


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

* [PATCH BlueZ v1 10/17] core/gatt: Make CCC addition API public
  2015-02-26  5:13 [PATCH BlueZ v1 00/17] Implement GATT server D-Bus API Arman Uguray
                   ` (8 preceding siblings ...)
  2015-02-26  5:13 ` [PATCH BlueZ v1 09/17] core/gatt: Support WriteValue " Arman Uguray
@ 2015-02-26  5:13 ` Arman Uguray
  2015-02-26  5:13 ` [PATCH BlueZ v1 11/17] core/gatt: Create CCC for external characteristic Arman Uguray
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Arman Uguray @ 2015-02-26  5:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Arman Uguray

This patch adds the btd_gatt_database_add_ccc function to the
database's public API. The signature has been extended to accept
a callback that gets invoked to notify the upper layer when a CCC write
is performed. The result is cached by the database on a per-device basis
while the callback is invoked for all writes from all devices.
---
 src/gatt-database.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++------
 src/gatt-database.h |  11 ++++++
 2 files changed, 109 insertions(+), 12 deletions(-)

diff --git a/src/gatt-database.c b/src/gatt-database.c
index e876141..ebfcf2d 100644
--- a/src/gatt-database.c
+++ b/src/gatt-database.c
@@ -59,6 +59,7 @@ struct btd_gatt_database {
 	uint32_t gap_handle;
 	uint32_t gatt_handle;
 	struct queue *device_states;
+	struct queue *ccc_callbacks;
 	struct gatt_db_attribute *svc_chngd;
 	struct gatt_db_attribute *svc_chngd_ccc;
 };
@@ -74,11 +75,48 @@ struct ccc_state {
 	uint8_t value[2];
 };
 
+struct ccc_cb_data {
+	uint16_t handle;
+	btd_gatt_database_ccc_write_t callback;
+	btd_gatt_database_destroy_t destroy;
+	void *user_data;
+};
+
 struct device_info {
 	bdaddr_t bdaddr;
 	uint8_t bdaddr_type;
 };
 
+static void ccc_cb_free(void *data)
+{
+	struct ccc_cb_data *ccc_cb = data;
+
+	if (ccc_cb->destroy)
+		ccc_cb->destroy(ccc_cb->user_data);
+
+	free(ccc_cb);
+}
+
+static bool ccc_cb_match_service(const void *data, const void *match_data)
+{
+	const struct ccc_cb_data *ccc_cb = 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_cb->handle >= start && ccc_cb->handle <= end;
+}
+
+static bool ccc_cb_match_handle(const void *data, const void *match_data)
+{
+	const struct ccc_cb_data *ccc_cb = data;
+	uint16_t handle = PTR_TO_UINT(match_data);
+
+	return ccc_cb->handle == handle;
+}
+
 static bool dev_state_match(const void *a, const void *b)
 {
 	const struct device_state *dev_state = a;
@@ -218,6 +256,9 @@ static void gatt_database_free(void *data)
 
 	/* TODO: Persistently store CCC states before freeing them */
 	queue_destroy(database->device_states, device_state_free);
+	queue_destroy(database->ccc_callbacks, ccc_cb_free);
+	database->device_states = NULL;
+	database->ccc_callbacks = NULL;
 	gatt_db_unregister(database->db, database->db_id);
 	gatt_db_unref(database->db);
 	btd_adapter_unref(database->adapter);
@@ -510,6 +551,7 @@ static void gatt_ccc_write_cb(struct gatt_db_attribute *attrib,
 {
 	struct btd_gatt_database *database = user_data;
 	struct ccc_state *ccc;
+	struct ccc_cb_data *ccc_cb;
 	uint16_t handle;
 	uint8_t ecode = 0;
 	bdaddr_t bdaddr;
@@ -540,22 +582,38 @@ static void gatt_ccc_write_cb(struct gatt_db_attribute *attrib,
 		goto done;
 	}
 
-	/*
-	 * TODO: Perform this after checking with a callback to the upper
-	 * layer.
-	 */
-	ccc->value[0] = value[0];
-	ccc->value[1] = value[1];
+	ccc_cb = queue_find(database->ccc_callbacks, ccc_cb_match_handle,
+			UINT_TO_PTR(gatt_db_attribute_get_handle(attrib)));
+	if (!ccc_cb) {
+		ecode = BT_ATT_ERROR_UNLIKELY;
+		goto done;
+	}
+
+	/* If value is identical, then just succeed */
+	if (ccc->value[0] == value[0] && ccc->value[1] == value[1])
+		goto done;
+
+	if (ccc_cb->callback)
+		ecode = ccc_cb->callback(get_le16(value), ccc_cb->user_data);
+
+	if (!ecode) {
+		ccc->value[0] = value[0];
+		ccc->value[1] = value[1];
+	}
 
 done:
 	gatt_db_attribute_write_result(attrib, id, ecode);
 }
 
-static struct gatt_db_attribute *
-gatt_database_add_ccc(struct btd_gatt_database *database,
-							uint16_t service_handle)
+struct gatt_db_attribute *
+btd_gatt_database_add_ccc(struct btd_gatt_database *database,
+				uint16_t service_handle,
+				btd_gatt_database_ccc_write_t write_callback,
+				void *user_data,
+				btd_gatt_database_destroy_t destroy)
 {
-	struct gatt_db_attribute *service;
+	struct gatt_db_attribute *service, *ccc;
+	struct ccc_cb_data *ccc_cb;
 	bt_uuid_t uuid;
 
 	if (!database || !service_handle)
@@ -567,10 +625,30 @@ gatt_database_add_ccc(struct btd_gatt_database *database,
 		return NULL;
 	}
 
+	ccc_cb = new0(struct ccc_cb_data, 1);
+	if (!ccc_cb) {
+		error("Could not allocate memory for callback data");
+		return NULL;
+	}
+
 	bt_uuid16_create(&uuid, GATT_CLIENT_CHARAC_CFG_UUID);
-	return gatt_db_service_add_descriptor(service, &uuid,
+	ccc = gatt_db_service_add_descriptor(service, &uuid,
 				BT_ATT_PERM_READ | BT_ATT_PERM_WRITE,
 				gatt_ccc_read_cb, gatt_ccc_write_cb, database);
+	if (!ccc) {
+		error("Failed to create CCC entry in database");
+		free(ccc_cb);
+		return NULL;
+	}
+
+	ccc_cb->handle = gatt_db_attribute_get_handle(ccc);
+	ccc_cb->callback = write_callback;
+	ccc_cb->destroy = destroy;
+	ccc_cb->user_data = user_data;
+
+	queue_push_tail(database->ccc_callbacks, ccc_cb);
+
+	return ccc;
 }
 
 static void populate_gatt_service(struct btd_gatt_database *database)
@@ -592,7 +670,9 @@ static void populate_gatt_service(struct btd_gatt_database *database)
 				BT_ATT_PERM_READ, BT_GATT_CHRC_PROP_INDICATE,
 				NULL, NULL, database);
 
-	database->svc_chngd_ccc = gatt_database_add_ccc(database, start_handle);
+	database->svc_chngd_ccc = btd_gatt_database_add_ccc(database,
+							start_handle,
+							NULL, NULL, NULL);
 
 	gatt_db_service_set_active(service, true);
 }
@@ -744,6 +824,8 @@ static void gatt_db_service_removed(struct gatt_db_attribute *attrib,
 	send_service_changed(database, attrib);
 
 	queue_foreach(database->device_states, remove_device_ccc, attrib);
+	queue_remove_all(database->ccc_callbacks, ccc_cb_match_service, attrib,
+								ccc_cb_free);
 }
 
 struct btd_gatt_database *btd_gatt_database_new(struct btd_adapter *adapter)
@@ -768,6 +850,10 @@ struct btd_gatt_database *btd_gatt_database_new(struct btd_adapter *adapter)
 	if (!database->device_states)
 		goto fail;
 
+	database->ccc_callbacks = queue_new();
+	if (!database->ccc_callbacks)
+		goto fail;
+
 	database->db_id = gatt_db_register(database->db, gatt_db_service_added,
 							gatt_db_service_removed,
 							database, NULL);
diff --git a/src/gatt-database.h b/src/gatt-database.h
index 0d9106b..163b601 100644
--- a/src/gatt-database.h
+++ b/src/gatt-database.h
@@ -23,3 +23,14 @@ struct btd_gatt_database *btd_gatt_database_new(struct btd_adapter *adapter);
 void btd_gatt_database_destroy(struct btd_gatt_database *database);
 
 struct gatt_db *btd_gatt_database_get_db(struct btd_gatt_database *database);
+
+typedef uint8_t (*btd_gatt_database_ccc_write_t) (uint16_t value,
+							void *user_data);
+typedef void (*btd_gatt_database_destroy_t) (void *data);
+
+struct gatt_db_attribute *
+btd_gatt_database_add_ccc(struct btd_gatt_database *database,
+				uint16_t service_handle,
+				btd_gatt_database_ccc_write_t write_callback,
+				void *user_data,
+				btd_gatt_database_destroy_t destroy);
-- 
2.2.0.rc0.207.ga3a616c


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

* [PATCH BlueZ v1 11/17] core/gatt: Create CCC for external characteristic
  2015-02-26  5:13 [PATCH BlueZ v1 00/17] Implement GATT server D-Bus API Arman Uguray
                   ` (9 preceding siblings ...)
  2015-02-26  5:13 ` [PATCH BlueZ v1 10/17] core/gatt: Make CCC addition API public Arman Uguray
@ 2015-02-26  5:13 ` Arman Uguray
  2015-02-26  5:13 ` [PATCH BlueZ v1 12/17] core/gatt: Add btd_gatt_database_notify function Arman Uguray
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Arman Uguray @ 2015-02-26  5:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Arman Uguray

This patch adds support for adding a CCC descriptor entry for an
external characteristic, if it has the 'notify' or 'indicate' property.
When the CCC descriptor is written to, bluetoothd calls the
'StartNotify' and 'StopNotify' methods on the characteristic in a
reference counted manner.
---
 src/gatt-manager.c | 106 ++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 100 insertions(+), 6 deletions(-)

diff --git a/src/gatt-manager.c b/src/gatt-manager.c
index 9e09894..4353f38 100644
--- a/src/gatt-manager.c
+++ b/src/gatt-manager.c
@@ -76,7 +76,9 @@ struct external_chrc {
 	uint8_t props;
 	uint8_t ext_props;
 	struct gatt_db_attribute *attrib;
+	struct gatt_db_attribute *ccc;
 	struct queue *pending_ops;
+	unsigned int ntfy_cnt;
 };
 
 struct pending_dbus_op {
@@ -373,10 +375,14 @@ static void proxy_added_cb(GDBusProxy *proxy, void *user_data)
 			return;
 		}
 
-		/*
-		 * TODO: Determine descriptors count to add based on special
-		 * characteristic properties (e.g. extended properties).
-		 */
+		if ((chrc->props & BT_GATT_CHRC_PROP_NOTIFY ||
+				chrc->props & BT_GATT_CHRC_PROP_INDICATE) &&
+				!incr_attr_count(service, 1)) {
+			error("Failed to increment attribute count for CCC");
+			service->failed = true;
+			return;
+		}
+
 
 		queue_push_tail(service->chrcs, chrc);
 	} else
@@ -664,6 +670,87 @@ static uint32_t permissions_from_props(uint8_t props, uint8_t ext_props)
 	return perm;
 }
 
+static uint8_t ccc_write_cb(uint16_t value, void *user_data)
+{
+	struct external_chrc *chrc = user_data;
+
+	DBG("External CCC write received with value: 0x%04x", value);
+
+	/* Notifications/indications disabled */
+	if (!value) {
+		if (!chrc->ntfy_cnt)
+			return 0;
+
+		if (__sync_sub_and_fetch(&chrc->ntfy_cnt, 1))
+			return 0;
+
+		/*
+		 * Send request to stop notifying. This is best-effort
+		 * operation, so simply ignore the return the value.
+		 */
+		g_dbus_proxy_method_call(chrc->proxy, "StopNotify", NULL,
+							NULL, NULL, NULL);
+		return 0;
+	}
+
+	/*
+	 * TODO: All of the errors below should fall into the so called
+	 * "Application Error" range. Since there is no well defined error for
+	 * these, we return a generic ATT protocol error for now.
+	 */
+
+	if (chrc->ntfy_cnt == UINT_MAX) {
+		/* Maximum number of per-device CCC descriptors configured */
+		return BT_ATT_ERROR_REQUEST_NOT_SUPPORTED;
+	}
+
+	/* Don't support undefined CCC values yet */
+	if (value > 2 ||
+		(value == 1 && !(chrc->props & BT_GATT_CHRC_PROP_NOTIFY)) ||
+		(value == 2 && !(chrc->props & BT_GATT_CHRC_PROP_INDICATE)))
+		return BT_ATT_ERROR_REQUEST_NOT_SUPPORTED;
+
+	/*
+	 * Always call StartNotify for an incoming enable and ignore the return
+	 * value for now.
+	 */
+	if (g_dbus_proxy_method_call(chrc->proxy,
+						"StartNotify", NULL, NULL,
+						NULL, NULL) == FALSE)
+		return BT_ATT_ERROR_REQUEST_NOT_SUPPORTED;
+
+	__sync_fetch_and_add(&chrc->ntfy_cnt, 1);
+
+	return 0;
+}
+
+static bool create_ccc_entry(struct external_service *service,
+						struct external_chrc *chrc)
+{
+	uint16_t svc_start;
+	struct btd_gatt_database *database;
+
+	if (!gatt_db_attribute_get_service_handles(service->attrib, &svc_start,
+									NULL)) {
+		error("Failed to obtain service handle");
+		return false;
+	}
+
+	database = btd_adapter_get_database(service->manager->adapter);
+	if (!database)
+		return false;
+
+	chrc->ccc = btd_gatt_database_add_ccc(database, svc_start,
+								ccc_write_cb,
+								chrc, NULL);
+	if (!chrc->ccc) {
+		error("Failed to create CCC entry for characteristic");
+		return false;
+	}
+
+	return true;
+}
+
 static bool create_chrc_entry(struct external_service *service,
 						struct external_chrc *chrc)
 {
@@ -691,9 +778,16 @@ static bool create_chrc_entry(struct external_service *service,
 						chrc->props, chrc_read_cb,
 						chrc_write_cb, chrc);
 
-	/* TODO: Create descriptor entries */
+	if (!chrc->attrib) {
+		error("Failed to create characteristic entry in database");
+		return false;
+	}
+
+	if (chrc->props & BT_GATT_CHRC_PROP_NOTIFY ||
+				chrc->props & BT_GATT_CHRC_PROP_INDICATE)
+		return create_ccc_entry(service, chrc);
 
-	return !!chrc->attrib;
+	return true;
 }
 
 static bool create_service_entry(struct external_service *service)
-- 
2.2.0.rc0.207.ga3a616c


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

* [PATCH BlueZ v1 12/17] core/gatt: Add btd_gatt_database_notify function
  2015-02-26  5:13 [PATCH BlueZ v1 00/17] Implement GATT server D-Bus API Arman Uguray
                   ` (10 preceding siblings ...)
  2015-02-26  5:13 ` [PATCH BlueZ v1 11/17] core/gatt: Create CCC for external characteristic Arman Uguray
@ 2015-02-26  5:13 ` Arman Uguray
  2015-02-26  8:51   ` Luiz Augusto von Dentz
  2015-02-26  5:13 ` [PATCH BlueZ v1 13/17] core/gatt: Send not/ind for D-Bus characteristics Arman Uguray
                   ` (4 subsequent siblings)
  16 siblings, 1 reply; 28+ messages in thread
From: Arman Uguray @ 2015-02-26  5:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Arman Uguray

This patch add the btd_gatt_database_notify function to database's
public API. This allows upper layers to send out notifications
and indications to all connected devices that have configured the
specified CCC descriptor.
---
 src/gatt-database.c | 4 ++--
 src/gatt-database.h | 5 +++++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/gatt-database.c b/src/gatt-database.c
index ebfcf2d..d75b971 100644
--- a/src/gatt-database.c
+++ b/src/gatt-database.c
@@ -737,7 +737,7 @@ static void send_notification_to_device(void *data, void *user_data)
 							NULL, NULL);
 }
 
-static void send_notification_to_devices(struct btd_gatt_database *database,
+void btd_gatt_database_notify(struct btd_gatt_database *database,
 					uint16_t handle, const uint8_t *value,
 					uint16_t len, uint16_t ccc_handle,
 					bool indicate)
@@ -781,7 +781,7 @@ static void send_service_changed(struct btd_gatt_database *database,
 	put_le16(start, value);
 	put_le16(end, value + 2);
 
-	send_notification_to_devices(database, handle, value, sizeof(value),
+	btd_gatt_database_notify(database, handle, value, sizeof(value),
 							ccc_handle, true);
 }
 
diff --git a/src/gatt-database.h b/src/gatt-database.h
index 163b601..2a89ad6 100644
--- a/src/gatt-database.h
+++ b/src/gatt-database.h
@@ -34,3 +34,8 @@ btd_gatt_database_add_ccc(struct btd_gatt_database *database,
 				btd_gatt_database_ccc_write_t write_callback,
 				void *user_data,
 				btd_gatt_database_destroy_t destroy);
+
+void btd_gatt_database_notify(struct btd_gatt_database *database,
+					uint16_t handle, const uint8_t *value,
+					uint16_t len, uint16_t ccc_handle,
+					bool indicate);
-- 
2.2.0.rc0.207.ga3a616c


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

* [PATCH BlueZ v1 13/17] core/gatt: Send not/ind for D-Bus characteristics
  2015-02-26  5:13 [PATCH BlueZ v1 00/17] Implement GATT server D-Bus API Arman Uguray
                   ` (11 preceding siblings ...)
  2015-02-26  5:13 ` [PATCH BlueZ v1 12/17] core/gatt: Add btd_gatt_database_notify function Arman Uguray
@ 2015-02-26  5:13 ` Arman Uguray
  2015-02-26  9:02   ` Luiz Augusto von Dentz
  2015-02-26  5:13 ` [PATCH BlueZ v1 14/17] core/gatt: Create CEP for external characteristic Arman Uguray
                   ` (3 subsequent siblings)
  16 siblings, 1 reply; 28+ messages in thread
From: Arman Uguray @ 2015-02-26  5:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Arman Uguray

This patch adds support for sending out notification/indication packets
for external characteristics whenever a PropertiesChanged signal is
received for the "Value" property of an external characteristic that has
either the 'notify' or the 'indicate' property.
---
 src/gatt-manager.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 56 insertions(+), 3 deletions(-)

diff --git a/src/gatt-manager.c b/src/gatt-manager.c
index 4353f38..5e69495 100644
--- a/src/gatt-manager.c
+++ b/src/gatt-manager.c
@@ -72,6 +72,7 @@ struct external_service {
 };
 
 struct external_chrc {
+	struct external_service *service;
 	GDBusProxy *proxy;
 	uint8_t props;
 	uint8_t ext_props;
@@ -121,8 +122,10 @@ static void chrc_free(void *data)
 
 	queue_foreach(chrc->pending_ops, cancel_pending_dbus_op, NULL);
 
-	if (chrc->proxy)
+	if (chrc->proxy) {
+		g_dbus_proxy_set_property_watch(chrc->proxy, NULL, NULL);
 		g_dbus_proxy_unref(chrc->proxy);
+	}
 
 	free(chrc);
 }
@@ -206,7 +209,8 @@ static void service_remove(void *data)
 	service_remove_helper(service);
 }
 
-static struct external_chrc *chrc_create(GDBusProxy *proxy)
+static struct external_chrc *chrc_create(struct external_service *service,
+							GDBusProxy *proxy)
 {
 	struct external_chrc *chrc;
 
@@ -220,6 +224,7 @@ static struct external_chrc *chrc_create(GDBusProxy *proxy)
 		return NULL;
 	}
 
+	chrc->service = service;
 	chrc->proxy = g_dbus_proxy_ref(proxy);
 
 	return chrc;
@@ -348,7 +353,7 @@ static void proxy_added_cb(GDBusProxy *proxy, void *user_data)
 			return;
 		}
 
-		chrc = chrc_create(proxy);
+		chrc = chrc_create(service, proxy);
 		if (!chrc) {
 			service->failed = true;
 			return;
@@ -724,6 +729,48 @@ static uint8_t ccc_write_cb(uint16_t value, void *user_data)
 	return 0;
 }
 
+static void property_changed_cb(GDBusProxy *proxy, const char *name,
+					DBusMessageIter *iter, void *user_data)
+{
+	struct external_chrc *chrc = user_data;
+	struct btd_gatt_database *database;
+	DBusMessageIter array;
+	uint8_t *value = NULL;
+	int len = 0;
+
+	if (strcmp(name, "Value"))
+		return;
+
+	if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_ARRAY) {
+		DBG("Malformed \"Value\" property received");
+		return;
+	}
+
+	dbus_message_iter_recurse(iter, &array);
+	dbus_message_iter_get_fixed_array(&array, &value, &len);
+
+	if (len < 0) {
+		DBG("Malformed \"Value\" property received");
+		return;
+	}
+
+	/* Truncate the value if it's too large */
+	len = MIN(BT_ATT_MAX_VALUE_LEN, len);
+	value = len ? value : NULL;
+
+	database = btd_adapter_get_database(chrc->service->manager->adapter);
+	if (!database) {
+		error("Failed to obtain GATT database from adapter!");
+		return;
+	}
+
+	btd_gatt_database_notify(database,
+				gatt_db_attribute_get_handle(chrc->attrib),
+				value, len,
+				gatt_db_attribute_get_handle(chrc->ccc),
+				chrc->props & BT_GATT_CHRC_PROP_INDICATE);
+}
+
 static bool create_ccc_entry(struct external_service *service,
 						struct external_chrc *chrc)
 {
@@ -748,6 +795,12 @@ static bool create_ccc_entry(struct external_service *service,
 		return false;
 	}
 
+	if (g_dbus_proxy_set_property_watch(chrc->proxy, property_changed_cb,
+							chrc) == FALSE) {
+		error("Failed to set up property watch for characteristic");
+		return false;
+	}
+
 	return true;
 }
 
-- 
2.2.0.rc0.207.ga3a616c


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

* [PATCH BlueZ v1 14/17] core/gatt: Create CEP for external characteristic
  2015-02-26  5:13 [PATCH BlueZ v1 00/17] Implement GATT server D-Bus API Arman Uguray
                   ` (12 preceding siblings ...)
  2015-02-26  5:13 ` [PATCH BlueZ v1 13/17] core/gatt: Send not/ind for D-Bus characteristics Arman Uguray
@ 2015-02-26  5:13 ` Arman Uguray
  2015-02-26  5:13 ` [PATCH BlueZ v1 15/17] core/gatt: Register descriptors Arman Uguray
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Arman Uguray @ 2015-02-26  5:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Arman Uguray

This patch adds supporting for adding a Characteristic Extended
Properties descriptor entry for an external characteristic, if
it has the 'reliable-write' or 'writable-auxiliaries' property.
---
 src/gatt-manager.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 59 insertions(+), 3 deletions(-)

diff --git a/src/gatt-manager.c b/src/gatt-manager.c
index 5e69495..dd22071 100644
--- a/src/gatt-manager.c
+++ b/src/gatt-manager.c
@@ -388,6 +388,11 @@ static void proxy_added_cb(GDBusProxy *proxy, void *user_data)
 			return;
 		}
 
+		if (chrc->ext_props && !incr_attr_count(service, 1)) {
+			error("Failed to increment attribute count for CEP");
+			service->failed = true;
+			return;
+		}
 
 		queue_push_tail(service->chrcs, chrc);
 	} else
@@ -777,6 +782,13 @@ static bool create_ccc_entry(struct external_service *service,
 	uint16_t svc_start;
 	struct btd_gatt_database *database;
 
+
+	if (!(chrc->props & BT_GATT_CHRC_PROP_NOTIFY) &&
+				!(chrc->props & BT_GATT_CHRC_PROP_INDICATE)) {
+		DBG("No need to create CCC entry for characteristic");
+		return true;
+	}
+
 	if (!gatt_db_attribute_get_service_handles(service->attrib, &svc_start,
 									NULL)) {
 		error("Failed to obtain service handle");
@@ -804,6 +816,48 @@ static bool create_ccc_entry(struct external_service *service,
 	return true;
 }
 
+static void cep_write_cb(struct gatt_db_attribute *attrib, int err,
+								void *user_data)
+{
+	if (err)
+		DBG("Failed to store CEP value in the database");
+	else
+		DBG("Stored CEP value in the database");
+}
+
+static bool create_cep_entry(struct external_service *service,
+						struct external_chrc *chrc)
+{
+	struct gatt_db_attribute *cep;
+	bt_uuid_t uuid;
+	uint8_t value[2];
+
+	if (!chrc->ext_props) {
+		DBG("No need to create CEP entry for characteristic");
+		return true;
+	}
+
+	bt_uuid16_create(&uuid, GATT_CHARAC_EXT_PROPER_UUID);
+	cep = gatt_db_service_add_descriptor(service->attrib, &uuid,
+							BT_ATT_PERM_READ,
+							NULL, NULL, NULL);
+	if (!cep) {
+		error("Failed to create CEP entry for characteristic");
+		return false;
+	}
+
+	memset(value, 0, sizeof(value));
+	value[0] = chrc->ext_props;
+
+	if (!gatt_db_attribute_write(cep, 0, value, sizeof(value), 0, NULL,
+							cep_write_cb, NULL)) {
+		DBG("Failed to store CEP value in the database");
+		return false;
+	}
+
+	return true;
+}
+
 static bool create_chrc_entry(struct external_service *service,
 						struct external_chrc *chrc)
 {
@@ -836,9 +890,11 @@ static bool create_chrc_entry(struct external_service *service,
 		return false;
 	}
 
-	if (chrc->props & BT_GATT_CHRC_PROP_NOTIFY ||
-				chrc->props & BT_GATT_CHRC_PROP_INDICATE)
-		return create_ccc_entry(service, chrc);
+	if (!create_ccc_entry(service, chrc))
+		return false;
+
+	if (!create_cep_entry(service, chrc))
+		return false;
 
 	return true;
 }
-- 
2.2.0.rc0.207.ga3a616c


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

* [PATCH BlueZ v1 15/17] core/gatt: Register descriptors
  2015-02-26  5:13 [PATCH BlueZ v1 00/17] Implement GATT server D-Bus API Arman Uguray
                   ` (13 preceding siblings ...)
  2015-02-26  5:13 ` [PATCH BlueZ v1 14/17] core/gatt: Create CEP for external characteristic Arman Uguray
@ 2015-02-26  5:13 ` Arman Uguray
  2015-02-26  5:13 ` [PATCH BlueZ v1 16/17] core/gatt: Support descriptor reads/writes Arman Uguray
  2015-02-26  5:13 ` [PATCH BlueZ v1 17/17] tools: Added a Python example for GATT server API Arman Uguray
  16 siblings, 0 replies; 28+ messages in thread
From: Arman Uguray @ 2015-02-26  5:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Arman Uguray

This patch adds support for registering external descriptor objects.
---
 src/gatt-manager.c | 206 +++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 191 insertions(+), 15 deletions(-)

diff --git a/src/gatt-manager.c b/src/gatt-manager.c
index dd22071..d31d13f 100644
--- a/src/gatt-manager.c
+++ b/src/gatt-manager.c
@@ -44,6 +44,7 @@
 #define GATT_MANAGER_IFACE	"org.bluez.GattManager1"
 #define GATT_SERVICE_IFACE	"org.bluez.GattService1"
 #define GATT_CHRC_IFACE		"org.bluez.GattCharacteristic1"
+#define GATT_DESC_IFACE		"org.bluez.GattDescriptor1"
 
 #define UUID_GAP	0x1800
 #define UUID_GATT	0x1801
@@ -69,10 +70,12 @@ struct external_service {
 	uint16_t attr_cnt;
 	struct gatt_db_attribute *attrib;
 	struct queue *chrcs;
+	struct queue *descs;
 };
 
 struct external_chrc {
 	struct external_service *service;
+	char *path;
 	GDBusProxy *proxy;
 	uint8_t props;
 	uint8_t ext_props;
@@ -82,6 +85,14 @@ struct external_chrc {
 	unsigned int ntfy_cnt;
 };
 
+struct external_desc {
+	struct external_service *service;
+	char *chrc_path;
+	GDBusProxy *proxy;
+	struct gatt_db_attribute *attrib;
+	bool handled;
+};
+
 struct pending_dbus_op {
 	struct external_chrc *chrc;
 	unsigned int id;
@@ -127,9 +138,25 @@ static void chrc_free(void *data)
 		g_dbus_proxy_unref(chrc->proxy);
 	}
 
+	if (chrc->path)
+		g_free(chrc->path);
+
 	free(chrc);
 }
 
+static void desc_free(void *data)
+{
+	struct external_desc *desc = data;
+
+	if (desc->proxy)
+		g_dbus_proxy_unref(desc->proxy);
+
+	if (desc->chrc_path)
+		g_free(desc->chrc_path);
+
+	free(desc);
+}
+
 static void service_free(void *data)
 {
 	struct external_service *service = data;
@@ -137,6 +164,9 @@ static void service_free(void *data)
 	if (service->chrcs)
 		queue_destroy(service->chrcs, chrc_free);
 
+	if (service->descs)
+		queue_destroy(service->descs, desc_free);
+
 	gatt_db_remove_service(service->manager->db, service->attrib);
 
 	if (service->client) {
@@ -210,7 +240,8 @@ static void service_remove(void *data)
 }
 
 static struct external_chrc *chrc_create(struct external_service *service,
-							GDBusProxy *proxy)
+							GDBusProxy *proxy,
+							const char *path)
 {
 	struct external_chrc *chrc;
 
@@ -224,12 +255,41 @@ static struct external_chrc *chrc_create(struct external_service *service,
 		return NULL;
 	}
 
+	chrc->path = g_strdup(path);
+	if (!chrc->path) {
+		queue_destroy(chrc->pending_ops, NULL);
+		free(chrc);
+		return NULL;
+	}
+
 	chrc->service = service;
 	chrc->proxy = g_dbus_proxy_ref(proxy);
 
 	return chrc;
 }
 
+static struct external_desc *desc_create(struct external_service *service,
+							GDBusProxy *proxy,
+							const char *chrc_path)
+{
+	struct external_desc *desc;
+
+	desc = new0(struct external_desc, 1);
+	if (!desc)
+		return NULL;
+
+	desc->chrc_path = g_strdup(chrc_path);
+	if (!desc->chrc_path) {
+		free(desc);
+		return NULL;
+	}
+
+	desc->service = service;
+	desc->proxy = g_dbus_proxy_ref(proxy);
+
+	return desc;
+}
+
 static bool incr_attr_count(struct external_service *service, uint16_t incr)
 {
 	if (service->attr_cnt > UINT16_MAX - incr)
@@ -240,18 +300,27 @@ static bool incr_attr_count(struct external_service *service, uint16_t incr)
 	return true;
 }
 
-static bool parse_service(GDBusProxy *proxy, struct external_service *service)
+static bool parse_path(GDBusProxy *proxy, const char *name, const char **path)
 {
 	DBusMessageIter iter;
-	const char *service_path;
 
-	if (!g_dbus_proxy_get_property(proxy, "Service", &iter))
+	if (!g_dbus_proxy_get_property(proxy, name, &iter))
 		return false;
 
 	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_OBJECT_PATH)
 		return false;
 
-	dbus_message_iter_get_basic(&iter, &service_path);
+	dbus_message_iter_get_basic(&iter, path);
+
+	return true;
+}
+
+static bool check_service_path(GDBusProxy *proxy, struct external_service *service)
+{
+	const char *service_path;
+
+	if (!parse_path(proxy, "Service", &service_path))
+		return false;
 
 	return g_strcmp0(service_path, service->path) == 0;
 }
@@ -311,7 +380,6 @@ static void proxy_added_cb(GDBusProxy *proxy, void *user_data)
 {
 	struct external_service *service = user_data;
 	const char *iface, *path;
-	struct external_chrc *chrc;
 
 	if (service->failed || service->attrib)
 		return;
@@ -322,8 +390,6 @@ static void proxy_added_cb(GDBusProxy *proxy, void *user_data)
 	if (!g_str_has_prefix(path, service->path))
 		return;
 
-	/* TODO: Handle descriptors here */
-
 	if (g_strcmp0(iface, GATT_SERVICE_IFACE) == 0) {
 		if (service->proxy)
 			return;
@@ -347,13 +413,15 @@ static void proxy_added_cb(GDBusProxy *proxy, void *user_data)
 
 		service->proxy = g_dbus_proxy_ref(proxy);
 	} else if (g_strcmp0(iface, GATT_CHRC_IFACE) == 0) {
+		struct external_chrc *chrc;
+
 		if (g_strcmp0(path, service->path) == 0) {
 			error("Characteristic path same as service path");
 			service->failed = true;
 			return;
 		}
 
-		chrc = chrc_create(service, proxy);
+		chrc = chrc_create(service, proxy, path);
 		if (!chrc) {
 			service->failed = true;
 			return;
@@ -395,8 +463,35 @@ static void proxy_added_cb(GDBusProxy *proxy, void *user_data)
 		}
 
 		queue_push_tail(service->chrcs, chrc);
-	} else
+	} else if (g_strcmp0(iface, GATT_DESC_IFACE) == 0) {
+		struct external_desc *desc;
+		const char *chrc_path;
+
+		if (!parse_path(proxy, "Characteristic", &chrc_path)) {
+			error("Failed to obtain characteristic path for "
+								"descriptor");
+			service->failed = true;
+			return;
+		}
+
+		desc = desc_create(service, proxy, chrc_path);
+		if (!desc) {
+			service->failed = true;
+			return;
+		}
+
+		/* Add 1 for the descriptor attribute */
+		if (!incr_attr_count(service, 1)) {
+			error("Failed to increment attribute count");
+			service->failed = true;
+			return;
+		}
+
+		queue_push_tail(service->descs, desc);
+	} else {
+		DBG("Ignoring unrelated interface: %s", iface);
 		return;
+	}
 
 	DBG("Object added to service - path: %s, iface: %s", path, iface);
 }
@@ -446,6 +541,20 @@ static bool parse_uuid(GDBusProxy *proxy, bt_uuid_t *uuid)
 		return false;
 	}
 
+	/* The CCC & CEP descriptors are created and managed by BlueZ */
+	bt_uuid16_create(&tmp, GATT_CLIENT_CHARAC_CFG_UUID);
+	if (!bt_uuid_cmp(&tmp, uuid)) {
+		error("CCC descriptors must be handled by BlueZ");
+		return false;
+	}
+
+	/* The CCC & CEP descriptors are created and managed by BlueZ */
+	bt_uuid16_create(&tmp, GATT_CHARAC_EXT_PROPER_UUID);
+	if (!bt_uuid_cmp(&tmp, uuid)) {
+		error("CEP descriptors must be handled by BlueZ");
+		return false;
+	}
+
 	return true;
 }
 
@@ -858,18 +967,47 @@ static bool create_cep_entry(struct external_service *service,
 	return true;
 }
 
+static bool create_desc_entry(struct external_service *service,
+						struct external_desc *desc)
+{
+	bt_uuid_t uuid;
+
+	if (!parse_uuid(desc->proxy, &uuid)) {
+		error("Failed to read \"UUID\" property of descriptor");
+		return false;
+	}
+
+	/*
+	 * TODO: Set read/write callbacks and property set permissions based on
+	 * a D-Bus property of the external descriptor.
+	 */
+	desc->attrib = gatt_db_service_add_descriptor(service->attrib,
+								&uuid, 0, NULL,
+								NULL, NULL);
+
+	if (!desc->attrib) {
+		error("Failed to create descriptor entry in database");
+		return false;
+	}
+
+	desc->handled = true;
+
+	return true;
+}
+
 static bool create_chrc_entry(struct external_service *service,
 						struct external_chrc *chrc)
 {
 	bt_uuid_t uuid;
 	uint32_t perm;
+	const struct queue_entry *entry;
 
 	if (!parse_uuid(chrc->proxy, &uuid)) {
 		error("Failed to read \"UUID\" property of characteristic");
 		return false;
 	}
 
-	if (!parse_service(chrc->proxy, service)) {
+	if (!check_service_path(chrc->proxy, service)) {
 		error("Invalid service path for characteristic");
 		return false;
 	}
@@ -896,9 +1034,33 @@ static bool create_chrc_entry(struct external_service *service,
 	if (!create_cep_entry(service, chrc))
 		return false;
 
+	/* Handle the descriptors that belong to this characteristic. */
+	entry = queue_get_entries(service->descs);
+	while (entry) {
+		struct external_desc *desc = entry->data;
+
+		if (desc->handled || g_strcmp0(desc->chrc_path, chrc->path))
+			continue;
+
+		if (!create_desc_entry(service, desc)) {
+			chrc->attrib = NULL;
+			error("Failed to create descriptor entry");
+			return false;
+		}
+
+		entry = entry->next;
+	}
+
 	return true;
 }
 
+static bool match_desc_unhandled(const void *a, const void *b)
+{
+	const struct external_desc *desc = a;
+
+	return !desc->handled;
+}
+
 static bool create_service_entry(struct external_service *service)
 {
 	bt_uuid_t uuid;
@@ -926,18 +1088,28 @@ static bool create_service_entry(struct external_service *service)
 
 		if (!create_chrc_entry(service, chrc)) {
 			error("Failed to create characteristic entry");
-			gatt_db_remove_service(service->manager->db,
-							service->attrib);
-			service->attrib = NULL;
-			return false;
+			goto fail;
 		}
 
 		entry = entry->next;
 	}
 
+	/* If there are any unhandled descriptors, return an error */
+	if (queue_find(service->descs, match_desc_unhandled, NULL)) {
+		error("Found descriptor with no matching characteristic!");
+		goto fail;
+	}
+
 	gatt_db_service_set_active(service->attrib, true);
 
 	return true;
+
+fail:
+	gatt_db_remove_service(service->manager->db,
+					service->attrib);
+	service->attrib = NULL;
+
+	return false;
 }
 
 static void client_ready_cb(GDBusClient *client, void *user_data)
@@ -1004,6 +1176,10 @@ static struct external_service *service_create(DBusConnection *conn,
 	if (!service->chrcs)
 		goto fail;
 
+	service->descs = queue_new();
+	if (!service->descs)
+		goto fail;
+
 	service->reg = dbus_message_ref(msg);
 
 	g_dbus_client_set_disconnect_watch(service->client,
-- 
2.2.0.rc0.207.ga3a616c


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

* [PATCH BlueZ v1 16/17] core/gatt: Support descriptor reads/writes
  2015-02-26  5:13 [PATCH BlueZ v1 00/17] Implement GATT server D-Bus API Arman Uguray
                   ` (14 preceding siblings ...)
  2015-02-26  5:13 ` [PATCH BlueZ v1 15/17] core/gatt: Register descriptors Arman Uguray
@ 2015-02-26  5:13 ` Arman Uguray
  2015-02-26  5:13 ` [PATCH BlueZ v1 17/17] tools: Added a Python example for GATT server API Arman Uguray
  16 siblings, 0 replies; 28+ messages in thread
From: Arman Uguray @ 2015-02-26  5:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Arman Uguray

This patch adds support for reading and writing to a descriptor from an
external application during a read/write procedure. This patch unifies
the code paths for characteristic and descriptor read/write operations.
---
 src/gatt-manager.c | 148 +++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 104 insertions(+), 44 deletions(-)

diff --git a/src/gatt-manager.c b/src/gatt-manager.c
index d31d13f..81fe608 100644
--- a/src/gatt-manager.c
+++ b/src/gatt-manager.c
@@ -91,11 +91,13 @@ struct external_desc {
 	GDBusProxy *proxy;
 	struct gatt_db_attribute *attrib;
 	bool handled;
+	struct queue *pending_ops;
 };
 
 struct pending_dbus_op {
-	struct external_chrc *chrc;
 	unsigned int id;
+	struct gatt_db_attribute *attrib;
+	struct queue *owner_queue;
 	void *user_data;
 };
 
@@ -103,18 +105,18 @@ static void cancel_pending_dbus_op(void *data, void *user_data)
 {
 	struct pending_dbus_op *op = data;
 
-	gatt_db_attribute_read_result(op->chrc->attrib, op->id,
+	gatt_db_attribute_read_result(op->attrib, op->id,
 					BT_ATT_ERROR_REQUEST_NOT_SUPPORTED,
 					NULL, 0);
-	op->chrc = NULL;
+	op->owner_queue = NULL;
 }
 
 static void pending_dbus_op_free(void *data)
 {
 	struct pending_dbus_op *op = data;
 
-	if (op->chrc)
-		queue_remove(op->chrc->pending_ops, op);
+	if (op->owner_queue)
+		queue_remove(op->owner_queue, op);
 
 	free(op);
 }
@@ -148,6 +150,8 @@ static void desc_free(void *data)
 {
 	struct external_desc *desc = data;
 
+	queue_foreach(desc->pending_ops, cancel_pending_dbus_op, NULL);
+
 	if (desc->proxy)
 		g_dbus_proxy_unref(desc->proxy);
 
@@ -278,8 +282,16 @@ static struct external_desc *desc_create(struct external_service *service,
 	if (!desc)
 		return NULL;
 
+
+	desc->pending_ops = queue_new();
+	if (!desc->pending_ops) {
+		free(desc);
+		return NULL;
+	}
+
 	desc->chrc_path = g_strdup(chrc_path);
 	if (!desc->chrc_path) {
+		queue_destroy(desc->pending_ops, NULL);
 		free(desc);
 		return NULL;
 	}
@@ -601,7 +613,7 @@ static void read_reply_cb(DBusMessage *message, void *user_data)
 	uint8_t *value = NULL;
 	int len = 0;
 
-	if (!op->chrc) {
+	if (!op->owner_queue) {
 		DBG("Pending read was canceled when object got removed");
 		return;
 	}
@@ -643,25 +655,17 @@ static void read_reply_cb(DBusMessage *message, void *user_data)
 	value = len ? value : NULL;
 
 done:
-	gatt_db_attribute_read_result(op->chrc->attrib, op->id, ecode,
+	gatt_db_attribute_read_result(op->attrib, op->id, ecode,
 								value, len);
 }
 
-static void chrc_read_cb(struct gatt_db_attribute *attrib,
-					unsigned int id, uint16_t offset,
-					uint8_t opcode, struct bt_att *att,
-					void *user_data)
+static void send_read(struct gatt_db_attribute *attrib, GDBusProxy *proxy,
+					struct queue *owner_queue,
+					unsigned int id)
 {
-	struct external_chrc *chrc = user_data;
 	struct pending_dbus_op *op;
 	uint8_t ecode = BT_ATT_ERROR_UNLIKELY;
 
-	if (chrc->attrib != attrib) {
-		error("Read callback called with incorrect attribute");
-		goto error;
-
-	}
-
 	op = new0(struct pending_dbus_op, 1);
 	if (!op) {
 		error("Failed to allocate memory for pending read call");
@@ -669,11 +673,12 @@ static void chrc_read_cb(struct gatt_db_attribute *attrib,
 		goto error;
 	}
 
-	op->chrc = chrc;
+	op->owner_queue = owner_queue;
+	op->attrib = attrib;
 	op->id = id;
-	queue_push_tail(chrc->pending_ops, op);
+	queue_push_tail(owner_queue, op);
 
-	if (g_dbus_proxy_method_call(chrc->proxy, "ReadValue", NULL,
+	if (g_dbus_proxy_method_call(proxy, "ReadValue", NULL,
 						read_reply_cb, op,
 						pending_dbus_op_free) == TRUE)
 		return;
@@ -703,7 +708,7 @@ static void write_reply_cb(DBusMessage *message, void *user_data)
 	DBusMessageIter iter;
 	uint8_t ecode = 0;
 
-	if (!op->chrc) {
+	if (!op->owner_queue) {
 		DBG("Pending write was canceled when object got removed");
 		return;
 	}
@@ -729,28 +734,21 @@ static void write_reply_cb(DBusMessage *message, void *user_data)
 	}
 
 done:
-	gatt_db_attribute_write_result(op->chrc->attrib, op->id, ecode);
+	gatt_db_attribute_write_result(op->attrib, op->id, ecode);
 }
 
-static void chrc_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)
+static void send_write(struct gatt_db_attribute *attrib, GDBusProxy *proxy,
+					struct queue *owner_queue,
+					unsigned int id,
+					const uint8_t *value, size_t len)
 {
-	struct external_chrc *chrc = user_data;
 	struct pending_dbus_op *op;
 	uint8_t ecode = BT_ATT_ERROR_UNLIKELY;
 	struct iovec iov;
 
-	if (chrc->attrib != attrib) {
-		error("Write callback called with incorrect attribute");
-		goto error;
-	}
-
 	op = new0(struct pending_dbus_op, 1);
 	if (!op) {
-		error("Failed to allocate memory for pending read call");
+		error("Failed to allocate memory for pending write call");
 		ecode = BT_ATT_ERROR_INSUFFICIENT_RESOURCES;
 		goto error;
 	}
@@ -758,12 +756,13 @@ static void chrc_write_cb(struct gatt_db_attribute *attrib,
 	iov.iov_base = (uint8_t *) value;
 	iov.iov_len = len;
 
-	op->chrc = chrc;
+	op->owner_queue = owner_queue;
+	op->attrib = attrib;
 	op->id = id;
 	op->user_data = &iov;
-	queue_push_tail(chrc->pending_ops, op);
+	queue_push_tail(owner_queue, op);
 
-	if (g_dbus_proxy_method_call(chrc->proxy, "WriteValue", write_setup_cb,
+	if (g_dbus_proxy_method_call(proxy, "WriteValue", write_setup_cb,
 						write_reply_cb, op,
 						pending_dbus_op_free) == TRUE)
 		return;
@@ -967,6 +966,37 @@ static bool create_cep_entry(struct external_service *service,
 	return true;
 }
 
+static void desc_read_cb(struct gatt_db_attribute *attrib,
+					unsigned int id, uint16_t offset,
+					uint8_t opcode, struct bt_att *att,
+					void *user_data)
+{
+	struct external_desc *desc = user_data;
+
+	if (desc->attrib != attrib) {
+		error("Read callback called with incorrect attribute");
+		return;
+	}
+
+	send_read(attrib, desc->proxy, desc->pending_ops, id);
+}
+
+static void desc_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 external_desc *desc = user_data;
+
+	if (desc->attrib != attrib) {
+		error("Read callback called with incorrect attribute");
+		return;
+	}
+
+	send_write(attrib, desc->proxy, desc->pending_ops, id, value, len);
+}
+
 static bool create_desc_entry(struct external_service *service,
 						struct external_desc *desc)
 {
@@ -978,13 +1008,12 @@ static bool create_desc_entry(struct external_service *service,
 	}
 
 	/*
-	 * TODO: Set read/write callbacks and property set permissions based on
-	 * a D-Bus property of the external descriptor.
+	 * TODO: Set permissions based on a D-Bus property of the external
+	 * descriptor.
 	 */
-	desc->attrib = gatt_db_service_add_descriptor(service->attrib,
-								&uuid, 0, NULL,
-								NULL, NULL);
-
+	desc->attrib = gatt_db_service_add_descriptor(service->attrib, &uuid,
+					BT_ATT_PERM_READ | BT_ATT_PERM_WRITE,
+					desc_read_cb, desc_write_cb, desc);
 	if (!desc->attrib) {
 		error("Failed to create descriptor entry in database");
 		return false;
@@ -995,6 +1024,37 @@ static bool create_desc_entry(struct external_service *service,
 	return true;
 }
 
+static void chrc_read_cb(struct gatt_db_attribute *attrib,
+					unsigned int id, uint16_t offset,
+					uint8_t opcode, struct bt_att *att,
+					void *user_data)
+{
+	struct external_chrc *chrc = user_data;
+
+	if (chrc->attrib != attrib) {
+		error("Read callback called with incorrect attribute");
+		return;
+	}
+
+	send_read(attrib, chrc->proxy, chrc->pending_ops, id);
+}
+
+static void chrc_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 external_chrc *chrc = user_data;
+
+	if (chrc->attrib != attrib) {
+		error("Write callback called with incorrect attribute");
+		return;
+	}
+
+	send_write(attrib, chrc->proxy, chrc->pending_ops, id, value, len);
+}
+
 static bool create_chrc_entry(struct external_service *service,
 						struct external_chrc *chrc)
 {
-- 
2.2.0.rc0.207.ga3a616c


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

* [PATCH BlueZ v1 17/17] tools: Added a Python example for GATT server API
  2015-02-26  5:13 [PATCH BlueZ v1 00/17] Implement GATT server D-Bus API Arman Uguray
                   ` (15 preceding siblings ...)
  2015-02-26  5:13 ` [PATCH BlueZ v1 16/17] core/gatt: Support descriptor reads/writes Arman Uguray
@ 2015-02-26  5:13 ` Arman Uguray
  16 siblings, 0 replies; 28+ messages in thread
From: Arman Uguray @ 2015-02-26  5:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Arman Uguray

This patch introduces tools/gatt-example, which demonstrates how an
external application can use the GattManager1 API to register GATT
services. This example simulates a fake Heart Rate service while also
providing a test service with a non-SIG UUID that exercises various
API behavior.
---
 tools/gatt-example | 463 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 463 insertions(+)
 create mode 100755 tools/gatt-example

diff --git a/tools/gatt-example b/tools/gatt-example
new file mode 100755
index 0000000..8847e4e
--- /dev/null
+++ b/tools/gatt-example
@@ -0,0 +1,463 @@
+#!/usr/bin/python
+
+import dbus
+import dbus.exceptions
+import dbus.mainloop.glib
+import dbus.service
+
+import array
+import gobject
+
+from random import randint
+
+mainloop = None
+
+BLUEZ_SERVICE_NAME = 'org.bluez'
+GATT_MANAGER_IFACE = 'org.bluez.GattManager1'
+DBUS_OM_IFACE = 'org.freedesktop.DBus.ObjectManager'
+DBUS_PROP_IFACE = 'org.freedesktop.DBus.Properties'
+
+GATT_SERVICE_IFACE = 'org.bluez.GattService1'
+GATT_CHRC_IFACE = 'org.bluez.GattCharacteristic1'
+GATT_DESC_IFACE = 'org.bluez.GattDescriptor1'
+
+class InvalidArgsException(dbus.exceptions.DBusException):
+    _dbus_error_name = 'org.freedesktop.DBus.Error.InvalidArgs'
+
+class NotSupportedException(dbus.exceptions.DBusException):
+    _dbus_error_name = 'org.bluez.Error.NotSupported'
+
+class NotPermittedException(dbus.exceptions.DBusException):
+    _dbus_error_name = 'org.bluez.Error.NotPermitted'
+
+class InvalidValueLengthException(dbus.exceptions.DBusException):
+    _dbus_error_name = 'org.bluez.Error.InvalidValueLength'
+
+class FailedException(dbus.exceptions.DBusException):
+    _dbus_error_name = 'org.bluez.Error.Failed'
+
+
+class Service(dbus.service.Object):
+    PATH_BASE = '/org/bluez/example/service'
+
+    def __init__(self, bus, index, uuid, primary):
+        self.path = self.PATH_BASE + str(index)
+        self.bus = bus
+        self.uuid = uuid
+        self.primary = primary
+        self.characteristics = []
+        dbus.service.Object.__init__(self, bus, self.path)
+
+    def get_properties(self):
+        return {
+                GATT_SERVICE_IFACE: {
+                        'UUID': self.uuid,
+                        'Primary': self.primary,
+                        'Characteristics': dbus.Array(
+                                self.get_characteristic_paths(),
+                                signature='o')
+                }
+        }
+
+    def get_path(self):
+        return dbus.ObjectPath(self.path)
+
+    def add_characteristic(self, characteristic):
+        self.characteristics.append(characteristic)
+
+    def get_characteristic_paths(self):
+        result = []
+        for chrc in self.characteristics:
+            result.append(chrc.get_path())
+        return result
+
+    def get_characteristics(self):
+        return self.characteristics
+
+    @dbus.service.method(DBUS_PROP_IFACE,
+                         in_signature='s',
+                         out_signature='a{sv}')
+    def GetAll(self, interface):
+        if interface != GATT_SERVICE_IFACE:
+            raise InvalidArgsException()
+
+        return self.get_properties[GATT_SERVICE_IFACE]
+
+    @dbus.service.method(DBUS_OM_IFACE, out_signature='a{oa{sa{sv}}}')
+    def GetManagedObjects(self):
+        response = {}
+        print 'GetManagedObjects'
+
+        response[self.get_path()] = self.get_properties()
+        chrcs = self.get_characteristics()
+        for chrc in chrcs:
+            response[chrc.get_path()] = chrc.get_properties()
+            descs = chrc.get_descriptors()
+            for desc in descs:
+                response[desc.get_path()] = desc.get_properties()
+
+        return response
+
+
+class Characteristic(dbus.service.Object):
+    def __init__(self, bus, index, uuid, flags, service):
+        self.path = service.path + '/char' + str(index)
+        self.bus = bus
+        self.uuid = uuid
+        self.service = service
+        self.flags = flags
+        self.descriptors = []
+        dbus.service.Object.__init__(self, bus, self.path)
+
+    def get_properties(self):
+        return {
+                GATT_CHRC_IFACE: {
+                        'Service': self.service.get_path(),
+                        'UUID': self.uuid,
+                        'Flags': self.flags,
+                        'Descriptors': dbus.Array(
+                                self.get_descriptor_paths(),
+                                signature='o')
+                }
+        }
+
+    def get_path(self):
+        return dbus.ObjectPath(self.path)
+
+    def add_descriptor(self, descriptor):
+        self.descriptors.append(descriptor)
+
+    def get_descriptor_paths(self):
+        result = []
+        for desc in self.descriptors:
+            result.append(desc.get_path())
+        return result
+
+    def get_descriptors(self):
+        return self.descriptors
+
+    @dbus.service.method(DBUS_PROP_IFACE,
+                         in_signature='s',
+                         out_signature='a{sv}')
+    def GetAll(self, interface):
+        if interface != GATT_CHRC_IFACE:
+            raise InvalidArgsException()
+
+        return self.get_properties[GATT_CHRC_IFACE]
+
+    @dbus.service.method(GATT_CHRC_IFACE, out_signature='ay')
+    def ReadValue(self):
+        print 'Default ReadValue called, returning error'
+        raise NotSupportedException()
+
+    @dbus.service.method(GATT_CHRC_IFACE, in_signature='ay')
+    def WriteValue(self, value):
+        print 'Default WriteValue called, returning error'
+        raise NotSupportedException()
+
+    @dbus.service.method(GATT_CHRC_IFACE)
+    def StartNotify(self):
+        print 'Default StartNotify called, returning error'
+        raise NotSupportedException()
+
+    @dbus.service.method(GATT_CHRC_IFACE)
+    def StopNotify(self):
+        print 'Default StopNotify called, returning error'
+        raise NotSupportedException()
+
+    @dbus.service.signal(DBUS_PROP_IFACE,
+                         signature='sa{sv}as')
+    def PropertiesChanged(self, interface, changed, invalidated):
+        pass
+
+
+class Descriptor(dbus.service.Object):
+    def __init__(self, bus, index, uuid, characteristic):
+        self.path = characteristic.path + '/desc' + str(index)
+        self.bus = bus
+        self.uuid = uuid
+        self.chrc = characteristic
+        dbus.service.Object.__init__(self, bus, self.path)
+
+    def get_properties(self):
+        return {
+                GATT_DESC_IFACE: {
+                        'Characteristic': self.chrc.get_path(),
+                        'UUID': self.uuid,
+                }
+        }
+
+    def get_path(self):
+        return dbus.ObjectPath(self.path)
+
+    @dbus.service.method(DBUS_PROP_IFACE,
+                         in_signature='s',
+                         out_signature='a{sv}')
+    def GetAll(self, interface):
+        if interface != GATT_DESC_IFACE:
+            raise InvalidArgsException()
+
+        return self.get_properties[GATT_CHRC_IFACE]
+
+    @dbus.service.method(GATT_DESC_IFACE, out_signature='ay')
+    def ReadValue(self):
+        print 'Default ReadValue called, returning error'
+        raise NotSupportedException()
+
+    @dbus.service.method(GATT_DESC_IFACE, in_signature='ay')
+    def WriteValue(self, value):
+        print 'Default WriteValue called, returning error'
+        raise NotSupportedException()
+
+
+class HeartRateService(Service):
+    """
+    Fake Heart Rate Service that simulates a fake heart beat and control point
+    behavior.
+
+    """
+    HR_UUID = '0000180d-0000-1000-8000-00805f9b34fb'
+
+    def __init__(self, bus, index):
+        Service.__init__(self, bus, index, self.HR_UUID, True)
+        self.add_characteristic(HeartRateMeasurementChrc(bus, 0, self))
+        self.add_characteristic(BodySensorLocationChrc(bus, 1, self))
+        self.add_characteristic(HeartRateControlPointChrc(bus, 2, self))
+        self.energy_expended = 0
+
+
+class HeartRateMeasurementChrc(Characteristic):
+    HR_MSRMT_UUID = '00002a37-0000-1000-8000-00805f9b34fb'
+
+    def __init__(self, bus, index, service):
+        Characteristic.__init__(
+                self, bus, index,
+                self.HR_MSRMT_UUID,
+                ['notify'],
+                service)
+        self.notifying = False;
+        self.hr_ee_count = 0
+
+    def hr_msrmt_cb(self):
+        value = []
+        value.append(dbus.Byte(0x06))
+
+        value.append(dbus.Byte(randint(90, 130)))
+
+        if self.hr_ee_count % 10 == 0:
+            value[0] = dbus.Byte(value[0] | 0x08)
+            value.append(dbus.Byte(self.service.energy_expended & 0xff))
+            value.append(dbus.Byte((self.service.energy_expended >> 8) & 0xff))
+
+        self.service.energy_expended = \
+                min(0xffff, self.service.energy_expended + 1)
+        self.hr_ee_count += 1
+
+        print 'Updating value: ' + repr(value)
+
+        self.PropertiesChanged(GATT_CHRC_IFACE, { 'Value': value }, [])
+
+        return self.notifying
+
+    def _update_hr_msrmt_simulation(self):
+        print 'Update HR Measurement Simulation'
+
+        if not self.notifying:
+            return
+
+        gobject.timeout_add(1000, self.hr_msrmt_cb)
+
+    def StartNotify(self):
+        if self.notifying:
+            print 'Already notifying, nothing to do'
+            return
+
+        self.notifying = True
+        self._update_hr_msrmt_simulation()
+
+    def StopNotify(self):
+        if not self.notifying:
+            print 'Not notifying, nothing to do'
+            return
+
+        self.notifying = False
+        self._update_hr_msrmt_simulation()
+
+
+class BodySensorLocationChrc(Characteristic):
+    BODY_SNSR_LOC_UUID = '00002a38-0000-1000-8000-00805f9b34fb'
+
+    def __init__(self, bus, index, service):
+        Characteristic.__init__(
+                self, bus, index,
+                self.BODY_SNSR_LOC_UUID,
+                ['read'],
+                service)
+
+    def ReadValue(self):
+        # Return 'Chest' as the sensor location.
+        return [ 0x01 ]
+
+class HeartRateControlPointChrc(Characteristic):
+    HR_CTRL_PT_UUID = '00002a39-0000-1000-8000-00805f9b34fb'
+
+    def __init__(self, bus, index, service):
+        Characteristic.__init__(
+                self, bus, index,
+                self.HR_CTRL_PT_UUID,
+                ['write'],
+                service)
+
+    def WriteValue(self, value):
+        print 'Heart Rate Control Point WriteValue called'
+
+        if len(value) != 1:
+            raise InvalidValueLengthException()
+
+        byte = value[0]
+        print 'Control Point value: ' + repr(byte)
+
+        if byte != 1:
+            raise FailedException("0x80")
+
+        print 'Energy Expended field reset!'
+        self.service.energy_expended = 0
+
+
+class TestService(Service):
+    """
+    Dummy test service that provides characteristics and descriptors that
+    exercise various API functionality.
+
+    """
+    TEST_SVC_UUID = '12345678-1234-5678-1234-56789abcdef0'
+
+    def __init__(self, bus, index):
+        Service.__init__(self, bus, index, self.TEST_SVC_UUID, False)
+        self.add_characteristic(TestCharacteristic(bus, 0, self))
+
+
+class TestCharacteristic(Characteristic):
+    """
+    Dummy test characteristic. Allows writing arbitrary bytes to its value, and
+    contains "extended properties", as well as a test descriptor.
+
+    """
+    TEST_CHRC_UUID = '12345678-1234-5678-1234-56789abcdef1'
+
+    def __init__(self, bus, index, service):
+        Characteristic.__init__(
+                self, bus, index,
+                self.TEST_CHRC_UUID,
+                ['read', 'write', 'writable-auxiliaries'],
+                service)
+        self.value = []
+        self.add_descriptor(TestDescriptor(bus, 0, self))
+        self.add_descriptor(
+                CharacteristicUserDescriptionDescriptor(bus, 1, self))
+
+    def ReadValue(self):
+        print 'TestCharacteristic Read: ' + repr(self.value)
+        return self.value
+
+    def WriteValue(self, value):
+        print 'TestCharacteristic Write: ' + repr(value)
+        self.value = value
+
+
+class TestDescriptor(Descriptor):
+    """
+    Dummy test descriptor. Returns a static value.
+
+    """
+    TEST_DESC_UUID = '12345678-1234-5678-1234-56789abcdef2'
+
+    def __init__(self, bus, index, characteristic):
+        Descriptor.__init__(
+                self, bus, index,
+                self.TEST_DESC_UUID,
+                characteristic)
+
+    def ReadValue(self):
+        return [
+                dbus.Byte('T'), dbus.Byte('e'), dbus.Byte('s'), dbus.Byte('t')
+        ]
+
+
+class CharacteristicUserDescriptionDescriptor(Descriptor):
+    """
+    Writable CUD descriptor.
+
+    """
+    CUD_UUID = '2901'
+
+    def __init__(self, bus, index, characteristic):
+        self.writable = 'writable-auxiliaries' in characteristic.flags
+        self.value = array.array('B', 'This is characteristic is for testing')
+        self.value = self.value.tolist()
+        Descriptor.__init__(
+                self, bus, index,
+                self.CUD_UUID,
+                characteristic)
+
+    def ReadValue(self):
+        return self.value
+
+    def WriteValue(self, value):
+        if not self.writable:
+            raise NotPermittedException()
+        self.value = value
+
+
+def register_service_cb():
+    print 'GATT service registered'
+
+
+def register_service_error_cb(error):
+    print 'Failed to register service: ' + str(error)
+    mainloop.quit()
+
+
+def find_adapter(bus):
+    remote_om = dbus.Interface(bus.get_object(BLUEZ_SERVICE_NAME, '/'),
+                               DBUS_OM_IFACE)
+    objects = remote_om.GetManagedObjects()
+
+    for o, props in objects.iteritems():
+        if props.has_key(GATT_MANAGER_IFACE):
+            return o
+
+    return None
+
+def main():
+    global mainloop
+
+    dbus.mainloop.glib.DBusGMainLoop(set_as_default=True)
+
+    bus = dbus.SystemBus()
+
+    adapter = find_adapter(bus)
+    if not adapter:
+        print 'GattManager1 interface not found'
+        return
+
+    service_manager = dbus.Interface(
+            bus.get_object(BLUEZ_SERVICE_NAME, adapter),
+            GATT_MANAGER_IFACE)
+
+    hr_service = HeartRateService(bus, 0)
+    test_service = TestService(bus, 1)
+
+    mainloop = gobject.MainLoop()
+
+    service_manager.RegisterService(hr_service.get_path(), {},
+                                    reply_handler=register_service_cb,
+                                    error_handler=register_service_error_cb)
+    service_manager.RegisterService(test_service.get_path(), {},
+                                    reply_handler=register_service_cb,
+                                    error_handler=register_service_error_cb)
+
+    mainloop.run()
+
+if __name__ == '__main__':
+    main()
-- 
2.2.0.rc0.207.ga3a616c


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

* Re: [PATCH BlueZ v1 01/17] shared/gatt: Call bt_att_cancel_all in unref
  2015-02-26  5:13 ` [PATCH BlueZ v1 01/17] shared/gatt: Call bt_att_cancel_all in unref Arman Uguray
@ 2015-02-26  8:44   ` Luiz Augusto von Dentz
  2015-02-26 20:44     ` Arman Uguray
  0 siblings, 1 reply; 28+ messages in thread
From: Luiz Augusto von Dentz @ 2015-02-26  8:44 UTC (permalink / raw)
  To: Arman Uguray; +Cc: linux-bluetooth

Hi Arman,

On Thu, Feb 26, 2015 at 7:13 AM, Arman Uguray <armansito@chromium.org> wrote:
> This patch fixes a potential invalid access that can occur if bt_att
> outlives bt_gatt_client and if there are pending discovery requests
> when bt_gatt_client_unref is called.
>
> This patch fixes this by canceling all ATT operations that are handled
> by the bt_att in bt_gatt_client_unref. The proper fix, however, is to
> make the discovery procedures cancelable and to cancel those instead of
> canceling everything. A TODO has been added to fix this later.

Im not sure we should accept patches that are know to have corner
cases like this, bt_gatt_client is not the sole user of bt_att so by
cancelling all we may break other users so I think we should invest
more time in fixing the problem properly.

> ---
>  src/shared/gatt-client.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
> index cc972d6..92e72e2 100644
> --- a/src/shared/gatt-client.c
> +++ b/src/shared/gatt-client.c
> @@ -1529,6 +1529,15 @@ static void bt_gatt_client_free(struct bt_gatt_client *client)
>                 bt_att_unregister_disconnect(client->att, client->disc_id);
>                 bt_att_unregister(client->att, client->notify_id);
>                 bt_att_unregister(client->att, client->ind_id);
> +
> +               /*
> +                * TODO: If we free bt_gatt_client while there is an ongoing
> +                * discovery procedure, the discovery callback may cause an
> +                * invalid access. To avoid this, we cancel all ongoing ATT
> +                * operations but the proper fix here is to make discovery
> +                * procedures cancelable.
> +                */
> +               bt_att_cancel_all(client->att);
>                 bt_att_unref(client->att);
>         }
>
> --
> 2.2.0.rc0.207.ga3a616c
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Luiz Augusto von Dentz

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

* Re: [PATCH BlueZ v1 12/17] core/gatt: Add btd_gatt_database_notify function
  2015-02-26  5:13 ` [PATCH BlueZ v1 12/17] core/gatt: Add btd_gatt_database_notify function Arman Uguray
@ 2015-02-26  8:51   ` Luiz Augusto von Dentz
  2015-02-26 20:49     ` Arman Uguray
  0 siblings, 1 reply; 28+ messages in thread
From: Luiz Augusto von Dentz @ 2015-02-26  8:51 UTC (permalink / raw)
  To: Arman Uguray; +Cc: linux-bluetooth

Hi Arman,

On Thu, Feb 26, 2015 at 7:13 AM, Arman Uguray <armansito@chromium.org> wrote:
> This patch add the btd_gatt_database_notify function to database's
> public API. This allows upper layers to send out notifications
> and indications to all connected devices that have configured the
> specified CCC descriptor.
> ---
>  src/gatt-database.c | 4 ++--
>  src/gatt-database.h | 5 +++++
>  2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/src/gatt-database.c b/src/gatt-database.c
> index ebfcf2d..d75b971 100644
> --- a/src/gatt-database.c
> +++ b/src/gatt-database.c
> @@ -737,7 +737,7 @@ static void send_notification_to_device(void *data, void *user_data)
>                                                         NULL, NULL);
>  }
>
> -static void send_notification_to_devices(struct btd_gatt_database *database,
> +void btd_gatt_database_notify(struct btd_gatt_database *database,
>                                         uint16_t handle, const uint8_t *value,
>                                         uint16_t len, uint16_t ccc_handle,
>                                         bool indicate)
> @@ -781,7 +781,7 @@ static void send_service_changed(struct btd_gatt_database *database,
>         put_le16(start, value);
>         put_le16(end, value + 2);
>
> -       send_notification_to_devices(database, handle, value, sizeof(value),
> +       btd_gatt_database_notify(database, handle, value, sizeof(value),
>                                                         ccc_handle, true);
>  }
>
> diff --git a/src/gatt-database.h b/src/gatt-database.h
> index 163b601..2a89ad6 100644
> --- a/src/gatt-database.h
> +++ b/src/gatt-database.h
> @@ -34,3 +34,8 @@ btd_gatt_database_add_ccc(struct btd_gatt_database *database,
>                                 btd_gatt_database_ccc_write_t write_callback,
>                                 void *user_data,
>                                 btd_gatt_database_destroy_t destroy);
> +
> +void btd_gatt_database_notify(struct btd_gatt_database *database,
> +                                       uint16_t handle, const uint8_t *value,
> +                                       uint16_t len, uint16_t ccc_handle,
> +                                       bool indicate);
> --
> 2.2.0.rc0.207.ga3a616c

I think we should make gatt_db notify about database changes directly
so anyone interested in attribute value changes can subscribe to it.



-- 
Luiz Augusto von Dentz

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

* Re: [PATCH BlueZ v1 13/17] core/gatt: Send not/ind for D-Bus characteristics
  2015-02-26  5:13 ` [PATCH BlueZ v1 13/17] core/gatt: Send not/ind for D-Bus characteristics Arman Uguray
@ 2015-02-26  9:02   ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 28+ messages in thread
From: Luiz Augusto von Dentz @ 2015-02-26  9:02 UTC (permalink / raw)
  To: Arman Uguray; +Cc: linux-bluetooth

Hi Arman,

On Thu, Feb 26, 2015 at 7:13 AM, Arman Uguray <armansito@chromium.org> wrote:
> This patch adds support for sending out notification/indication packets
> for external characteristics whenever a PropertiesChanged signal is
> received for the "Value" property of an external characteristic that has
> either the 'notify' or the 'indicate' property.
> ---
>  src/gatt-manager.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 56 insertions(+), 3 deletions(-)
>
> diff --git a/src/gatt-manager.c b/src/gatt-manager.c
> index 4353f38..5e69495 100644
> --- a/src/gatt-manager.c
> +++ b/src/gatt-manager.c
> @@ -72,6 +72,7 @@ struct external_service {
>  };
>
>  struct external_chrc {
> +       struct external_service *service;
>         GDBusProxy *proxy;
>         uint8_t props;
>         uint8_t ext_props;
> @@ -121,8 +122,10 @@ static void chrc_free(void *data)
>
>         queue_foreach(chrc->pending_ops, cancel_pending_dbus_op, NULL);
>
> -       if (chrc->proxy)
> +       if (chrc->proxy) {
> +               g_dbus_proxy_set_property_watch(chrc->proxy, NULL, NULL);
>                 g_dbus_proxy_unref(chrc->proxy);
> +       }
>
>         free(chrc);
>  }
> @@ -206,7 +209,8 @@ static void service_remove(void *data)
>         service_remove_helper(service);
>  }
>
> -static struct external_chrc *chrc_create(GDBusProxy *proxy)
> +static struct external_chrc *chrc_create(struct external_service *service,
> +                                                       GDBusProxy *proxy)
>  {
>         struct external_chrc *chrc;
>
> @@ -220,6 +224,7 @@ static struct external_chrc *chrc_create(GDBusProxy *proxy)
>                 return NULL;
>         }
>
> +       chrc->service = service;
>         chrc->proxy = g_dbus_proxy_ref(proxy);
>
>         return chrc;
> @@ -348,7 +353,7 @@ static void proxy_added_cb(GDBusProxy *proxy, void *user_data)
>                         return;
>                 }
>
> -               chrc = chrc_create(proxy);
> +               chrc = chrc_create(service, proxy);
>                 if (!chrc) {
>                         service->failed = true;
>                         return;
> @@ -724,6 +729,48 @@ static uint8_t ccc_write_cb(uint16_t value, void *user_data)
>         return 0;
>  }
>
> +static void property_changed_cb(GDBusProxy *proxy, const char *name,
> +                                       DBusMessageIter *iter, void *user_data)
> +{
> +       struct external_chrc *chrc = user_data;
> +       struct btd_gatt_database *database;
> +       DBusMessageIter array;
> +       uint8_t *value = NULL;
> +       int len = 0;
> +
> +       if (strcmp(name, "Value"))
> +               return;
> +
> +       if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_ARRAY) {
> +               DBG("Malformed \"Value\" property received");
> +               return;
> +       }
> +
> +       dbus_message_iter_recurse(iter, &array);
> +       dbus_message_iter_get_fixed_array(&array, &value, &len);
> +
> +       if (len < 0) {
> +               DBG("Malformed \"Value\" property received");
> +               return;
> +       }
> +
> +       /* Truncate the value if it's too large */
> +       len = MIN(BT_ATT_MAX_VALUE_LEN, len);
> +       value = len ? value : NULL;
> +
> +       database = btd_adapter_get_database(chrc->service->manager->adapter);
> +       if (!database) {
> +               error("Failed to obtain GATT database from adapter!");
> +               return;
> +       }
> +
> +       btd_gatt_database_notify(database,
> +                               gatt_db_attribute_get_handle(chrc->attrib),
> +                               value, len,
> +                               gatt_db_attribute_get_handle(chrc->ccc),
> +                               chrc->props & BT_GATT_CHRC_PROP_INDICATE);

Here instead of call to btd_gatt_database_notify we should have
gatt_db_attribute_write, but perhaps it gets tricky because that is
also called for when the remote attempts to write, so perhaps we
should do gatt_db_attribute_notify which takes care of notifying via
gatt_db, anyway it is important to note that once we enable
notification the value can be cached.

> +}
> +
>  static bool create_ccc_entry(struct external_service *service,
>                                                 struct external_chrc *chrc)
>  {
> @@ -748,6 +795,12 @@ static bool create_ccc_entry(struct external_service *service,
>                 return false;
>         }
>
> +       if (g_dbus_proxy_set_property_watch(chrc->proxy, property_changed_cb,
> +                                                       chrc) == FALSE) {
> +               error("Failed to set up property watch for characteristic");
> +               return false;
> +       }
> +
>         return true;
>  }
>
> --
> 2.2.0.rc0.207.ga3a616c
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Luiz Augusto von Dentz

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

* Re: [PATCH BlueZ v1 03/17] gdbus/client: Allow specifying ObjectManager path
  2015-02-26  5:13 ` [PATCH BlueZ v1 03/17] gdbus/client: Allow specifying ObjectManager path Arman Uguray
@ 2015-02-26 15:38   ` Marcel Holtmann
  2015-02-26 20:32     ` Arman Uguray
  0 siblings, 1 reply; 28+ messages in thread
From: Marcel Holtmann @ 2015-02-26 15:38 UTC (permalink / raw)
  To: Arman Uguray; +Cc: linux-bluetooth

Hi Arman,

> GDBusClient currently hard-codes "/" as the remote ObjectManager path.
> This is generally incorrect, as an application can choose to expose an
> ObjectManager at any well-known path. This patch fixes this by allowing
> the user to pass in the ObjectManager path to g_dbus_client_new.
> ---
> client/main.c            |  2 +-
> gdbus/client.c           | 23 ++++++++++++++++-------
> gdbus/gdbus.h            |  5 +++--
> plugins/hostname.c       |  2 +-
> profiles/iap/main.c      |  2 +-
> tools/bluetooth-player.c |  2 +-
> tools/gap-tester.c       |  3 ++-
> tools/gatt-service.c     |  2 +-
> tools/mpris-proxy.c      |  2 +-
> tools/obexctl.c          |  2 +-
> unit/test-gdbus-client.c | 39 ++++++++++++++++++++++++++-------------
> 11 files changed, 54 insertions(+), 30 deletions(-)

please to not intermix gdbus/ changes with others. Everything in gdbus/ needs to be a separate patch. You are affecting more than one project here.

Regards

Marcel


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

* Re: [PATCH BlueZ v1 03/17] gdbus/client: Allow specifying ObjectManager path
  2015-02-26 15:38   ` Marcel Holtmann
@ 2015-02-26 20:32     ` Arman Uguray
  2015-02-27  6:47       ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 28+ messages in thread
From: Arman Uguray @ 2015-02-26 20:32 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: BlueZ development

Hi Marcel,

> On Thu, Feb 26, 2015 at 7:38 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Arman,
>
>> GDBusClient currently hard-codes "/" as the remote ObjectManager path.
>> This is generally incorrect, as an application can choose to expose an
>> ObjectManager at any well-known path. This patch fixes this by allowing
>> the user to pass in the ObjectManager path to g_dbus_client_new.
>> ---
>> client/main.c            |  2 +-
>> gdbus/client.c           | 23 ++++++++++++++++-------
>> gdbus/gdbus.h            |  5 +++--
>> plugins/hostname.c       |  2 +-
>> profiles/iap/main.c      |  2 +-
>> tools/bluetooth-player.c |  2 +-
>> tools/gap-tester.c       |  3 ++-
>> tools/gatt-service.c     |  2 +-
>> tools/mpris-proxy.c      |  2 +-
>> tools/obexctl.c          |  2 +-
>> unit/test-gdbus-client.c | 39 ++++++++++++++++++++++++++-------------
>> 11 files changed, 54 insertions(+), 30 deletions(-)
>
> please to not intermix gdbus/ changes with others. Everything in gdbus/ needs to be a separate patch. You are affecting more than one project here.
>

This is a gdbus/ only change but I it changes an API definition so all
of these other spots need to be updated otherwise the code won't
compile.

> Regards
>
> Marcel
>

Arman

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

* Re: [PATCH BlueZ v1 01/17] shared/gatt: Call bt_att_cancel_all in unref
  2015-02-26  8:44   ` Luiz Augusto von Dentz
@ 2015-02-26 20:44     ` Arman Uguray
  0 siblings, 0 replies; 28+ messages in thread
From: Arman Uguray @ 2015-02-26 20:44 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

Hi Luiz,

> On Thu, Feb 26, 2015 at 12:44 AM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
> Hi Arman,
>
> On Thu, Feb 26, 2015 at 7:13 AM, Arman Uguray <armansito@chromium.org> wrote:
>> This patch fixes a potential invalid access that can occur if bt_att
>> outlives bt_gatt_client and if there are pending discovery requests
>> when bt_gatt_client_unref is called.
>>
>> This patch fixes this by canceling all ATT operations that are handled
>> by the bt_att in bt_gatt_client_unref. The proper fix, however, is to
>> make the discovery procedures cancelable and to cancel those instead of
>> canceling everything. A TODO has been added to fix this later.
>
> Im not sure we should accept patches that are know to have corner
> cases like this, bt_gatt_client is not the sole user of bt_att so by
> cancelling all we may break other users so I think we should invest
> more time in fixing the problem properly.
>

OK, then I'll look at fixing this issue separately and send it to the
list in its own patch set. So this shouldn't need to block the GATT
server patches.

>> ---
>>  src/shared/gatt-client.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
>> index cc972d6..92e72e2 100644
>> --- a/src/shared/gatt-client.c
>> +++ b/src/shared/gatt-client.c
>> @@ -1529,6 +1529,15 @@ static void bt_gatt_client_free(struct bt_gatt_client *client)
>>                 bt_att_unregister_disconnect(client->att, client->disc_id);
>>                 bt_att_unregister(client->att, client->notify_id);
>>                 bt_att_unregister(client->att, client->ind_id);
>> +
>> +               /*
>> +                * TODO: If we free bt_gatt_client while there is an ongoing
>> +                * discovery procedure, the discovery callback may cause an
>> +                * invalid access. To avoid this, we cancel all ongoing ATT
>> +                * operations but the proper fix here is to make discovery
>> +                * procedures cancelable.
>> +                */
>> +               bt_att_cancel_all(client->att);
>>                 bt_att_unref(client->att);
>>         }
>>
>> --
>> 2.2.0.rc0.207.ga3a616c
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Luiz Augusto von Dentz

Arman

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

* Re: [PATCH BlueZ v1 12/17] core/gatt: Add btd_gatt_database_notify function
  2015-02-26  8:51   ` Luiz Augusto von Dentz
@ 2015-02-26 20:49     ` Arman Uguray
  2015-02-27 10:54       ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 28+ messages in thread
From: Arman Uguray @ 2015-02-26 20:49 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

Hi Luiz,

> On Thu, Feb 26, 2015 at 12:51 AM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
> Hi Arman,
>
> On Thu, Feb 26, 2015 at 7:13 AM, Arman Uguray <armansito@chromium.org> wrote:
>> This patch add the btd_gatt_database_notify function to database's
>> public API. This allows upper layers to send out notifications
>> and indications to all connected devices that have configured the
>> specified CCC descriptor.
>> ---
>>  src/gatt-database.c | 4 ++--
>>  src/gatt-database.h | 5 +++++
>>  2 files changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/gatt-database.c b/src/gatt-database.c
>> index ebfcf2d..d75b971 100644
>> --- a/src/gatt-database.c
>> +++ b/src/gatt-database.c
>> @@ -737,7 +737,7 @@ static void send_notification_to_device(void *data, void *user_data)
>>                                                         NULL, NULL);
>>  }
>>
>> -static void send_notification_to_devices(struct btd_gatt_database *database,
>> +void btd_gatt_database_notify(struct btd_gatt_database *database,
>>                                         uint16_t handle, const uint8_t *value,
>>                                         uint16_t len, uint16_t ccc_handle,
>>                                         bool indicate)
>> @@ -781,7 +781,7 @@ static void send_service_changed(struct btd_gatt_database *database,
>>         put_le16(start, value);
>>         put_le16(end, value + 2);
>>
>> -       send_notification_to_devices(database, handle, value, sizeof(value),
>> +       btd_gatt_database_notify(database, handle, value, sizeof(value),
>>                                                         ccc_handle, true);
>>  }
>>
>> diff --git a/src/gatt-database.h b/src/gatt-database.h
>> index 163b601..2a89ad6 100644
>> --- a/src/gatt-database.h
>> +++ b/src/gatt-database.h
>> @@ -34,3 +34,8 @@ btd_gatt_database_add_ccc(struct btd_gatt_database *database,
>>                                 btd_gatt_database_ccc_write_t write_callback,
>>                                 void *user_data,
>>                                 btd_gatt_database_destroy_t destroy);
>> +
>> +void btd_gatt_database_notify(struct btd_gatt_database *database,
>> +                                       uint16_t handle, const uint8_t *value,
>> +                                       uint16_t len, uint16_t ccc_handle,
>> +                                       bool indicate);
>> --
>> 2.2.0.rc0.207.ga3a616c
>
> I think we should make gatt_db notify about database changes directly
> so anyone interested in attribute value changes can subscribe to it.
>

So, after our discussion on IRC, will you be implementing this or
should I? If you give me a description of the kind of API you're
thinking about and how things should fit together, then I can have a
go at it, unless you've started working on it already.

>
>
> --
> Luiz Augusto von Dentz

Thanks!
Arman

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

* Re: [PATCH BlueZ v1 03/17] gdbus/client: Allow specifying ObjectManager path
  2015-02-26 20:32     ` Arman Uguray
@ 2015-02-27  6:47       ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 28+ messages in thread
From: Luiz Augusto von Dentz @ 2015-02-27  6:47 UTC (permalink / raw)
  To: Arman Uguray; +Cc: Marcel Holtmann, BlueZ development

Hi Arman,

On Thu, Feb 26, 2015 at 10:32 PM, Arman Uguray <armansito@chromium.org> wrote:
> Hi Marcel,
>
>> On Thu, Feb 26, 2015 at 7:38 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
>> Hi Arman,
>>
>>> GDBusClient currently hard-codes "/" as the remote ObjectManager path.
>>> This is generally incorrect, as an application can choose to expose an
>>> ObjectManager at any well-known path. This patch fixes this by allowing
>>> the user to pass in the ObjectManager path to g_dbus_client_new.
>>> ---
>>> client/main.c            |  2 +-
>>> gdbus/client.c           | 23 ++++++++++++++++-------
>>> gdbus/gdbus.h            |  5 +++--
>>> plugins/hostname.c       |  2 +-
>>> profiles/iap/main.c      |  2 +-
>>> tools/bluetooth-player.c |  2 +-
>>> tools/gap-tester.c       |  3 ++-
>>> tools/gatt-service.c     |  2 +-
>>> tools/mpris-proxy.c      |  2 +-
>>> tools/obexctl.c          |  2 +-
>>> unit/test-gdbus-client.c | 39 ++++++++++++++++++++++++++-------------
>>> 11 files changed, 54 insertions(+), 30 deletions(-)
>>
>> please to not intermix gdbus/ changes with others. Everything in gdbus/ needs to be a separate patch. You are affecting more than one project here.
>>
>
> This is a gdbus/ only change but I it changes an API definition so all
> of these other spots need to be updated otherwise the code won't
> compile.

I wonder if we really need this change or we should have a different
function. e.g. g_dbus_client_new_full where we can include the object
manager path without breaking backward compatibility.


-- 
Luiz Augusto von Dentz

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

* Re: [PATCH BlueZ v1 12/17] core/gatt: Add btd_gatt_database_notify function
  2015-02-26 20:49     ` Arman Uguray
@ 2015-02-27 10:54       ` Luiz Augusto von Dentz
  2015-02-27 14:33         ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 28+ messages in thread
From: Luiz Augusto von Dentz @ 2015-02-27 10:54 UTC (permalink / raw)
  To: Arman Uguray; +Cc: linux-bluetooth

Hi Arman,

On Thu, Feb 26, 2015 at 10:49 PM, Arman Uguray <armansito@chromium.org> wrote:
> Hi Luiz,
>
>> On Thu, Feb 26, 2015 at 12:51 AM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>> Hi Arman,
>>
>> On Thu, Feb 26, 2015 at 7:13 AM, Arman Uguray <armansito@chromium.org> wrote:
>>> This patch add the btd_gatt_database_notify function to database's
>>> public API. This allows upper layers to send out notifications
>>> and indications to all connected devices that have configured the
>>> specified CCC descriptor.
>>> ---
>>>  src/gatt-database.c | 4 ++--
>>>  src/gatt-database.h | 5 +++++
>>>  2 files changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/gatt-database.c b/src/gatt-database.c
>>> index ebfcf2d..d75b971 100644
>>> --- a/src/gatt-database.c
>>> +++ b/src/gatt-database.c
>>> @@ -737,7 +737,7 @@ static void send_notification_to_device(void *data, void *user_data)
>>>                                                         NULL, NULL);
>>>  }
>>>
>>> -static void send_notification_to_devices(struct btd_gatt_database *database,
>>> +void btd_gatt_database_notify(struct btd_gatt_database *database,
>>>                                         uint16_t handle, const uint8_t *value,
>>>                                         uint16_t len, uint16_t ccc_handle,
>>>                                         bool indicate)
>>> @@ -781,7 +781,7 @@ static void send_service_changed(struct btd_gatt_database *database,
>>>         put_le16(start, value);
>>>         put_le16(end, value + 2);
>>>
>>> -       send_notification_to_devices(database, handle, value, sizeof(value),
>>> +       btd_gatt_database_notify(database, handle, value, sizeof(value),
>>>                                                         ccc_handle, true);
>>>  }
>>>
>>> diff --git a/src/gatt-database.h b/src/gatt-database.h
>>> index 163b601..2a89ad6 100644
>>> --- a/src/gatt-database.h
>>> +++ b/src/gatt-database.h
>>> @@ -34,3 +34,8 @@ btd_gatt_database_add_ccc(struct btd_gatt_database *database,
>>>                                 btd_gatt_database_ccc_write_t write_callback,
>>>                                 void *user_data,
>>>                                 btd_gatt_database_destroy_t destroy);
>>> +
>>> +void btd_gatt_database_notify(struct btd_gatt_database *database,
>>> +                                       uint16_t handle, const uint8_t *value,
>>> +                                       uint16_t len, uint16_t ccc_handle,
>>> +                                       bool indicate);
>>> --
>>> 2.2.0.rc0.207.ga3a616c
>>
>> I think we should make gatt_db notify about database changes directly
>> so anyone interested in attribute value changes can subscribe to it.
>>
>
> So, after our discussion on IRC, will you be implementing this or
> should I? If you give me a description of the kind of API you're
> thinking about and how things should fit together, then I can have a
> go at it, unless you've started working on it already.

Im planning to continue this today, along with some other modification
like g_dbus_client_new_full so we don't break the API.

>
>>
>>
>> --
>> Luiz Augusto von Dentz
>
> Thanks!
> Arman



-- 
Luiz Augusto von Dentz

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

* Re: [PATCH BlueZ v1 12/17] core/gatt: Add btd_gatt_database_notify function
  2015-02-27 10:54       ` Luiz Augusto von Dentz
@ 2015-02-27 14:33         ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 28+ messages in thread
From: Luiz Augusto von Dentz @ 2015-02-27 14:33 UTC (permalink / raw)
  To: Arman Uguray; +Cc: linux-bluetooth

Hi Arman,

On Fri, Feb 27, 2015 at 12:54 PM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Arman,
>
> On Thu, Feb 26, 2015 at 10:49 PM, Arman Uguray <armansito@chromium.org> wrote:
>> Hi Luiz,
>>
>>> On Thu, Feb 26, 2015 at 12:51 AM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>>> Hi Arman,
>>>
>>> On Thu, Feb 26, 2015 at 7:13 AM, Arman Uguray <armansito@chromium.org> wrote:
>>>> This patch add the btd_gatt_database_notify function to database's
>>>> public API. This allows upper layers to send out notifications
>>>> and indications to all connected devices that have configured the
>>>> specified CCC descriptor.
>>>> ---
>>>>  src/gatt-database.c | 4 ++--
>>>>  src/gatt-database.h | 5 +++++
>>>>  2 files changed, 7 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/src/gatt-database.c b/src/gatt-database.c
>>>> index ebfcf2d..d75b971 100644
>>>> --- a/src/gatt-database.c
>>>> +++ b/src/gatt-database.c
>>>> @@ -737,7 +737,7 @@ static void send_notification_to_device(void *data, void *user_data)
>>>>                                                         NULL, NULL);
>>>>  }
>>>>
>>>> -static void send_notification_to_devices(struct btd_gatt_database *database,
>>>> +void btd_gatt_database_notify(struct btd_gatt_database *database,
>>>>                                         uint16_t handle, const uint8_t *value,
>>>>                                         uint16_t len, uint16_t ccc_handle,
>>>>                                         bool indicate)
>>>> @@ -781,7 +781,7 @@ static void send_service_changed(struct btd_gatt_database *database,
>>>>         put_le16(start, value);
>>>>         put_le16(end, value + 2);
>>>>
>>>> -       send_notification_to_devices(database, handle, value, sizeof(value),
>>>> +       btd_gatt_database_notify(database, handle, value, sizeof(value),
>>>>                                                         ccc_handle, true);
>>>>  }
>>>>
>>>> diff --git a/src/gatt-database.h b/src/gatt-database.h
>>>> index 163b601..2a89ad6 100644
>>>> --- a/src/gatt-database.h
>>>> +++ b/src/gatt-database.h
>>>> @@ -34,3 +34,8 @@ btd_gatt_database_add_ccc(struct btd_gatt_database *database,
>>>>                                 btd_gatt_database_ccc_write_t write_callback,
>>>>                                 void *user_data,
>>>>                                 btd_gatt_database_destroy_t destroy);
>>>> +
>>>> +void btd_gatt_database_notify(struct btd_gatt_database *database,
>>>> +                                       uint16_t handle, const uint8_t *value,
>>>> +                                       uint16_t len, uint16_t ccc_handle,
>>>> +                                       bool indicate);
>>>> --
>>>> 2.2.0.rc0.207.ga3a616c
>>>
>>> I think we should make gatt_db notify about database changes directly
>>> so anyone interested in attribute value changes can subscribe to it.
>>>
>>
>> So, after our discussion on IRC, will you be implementing this or
>> should I? If you give me a description of the kind of API you're
>> thinking about and how things should fit together, then I can have a
>> go at it, unless you've started working on it already.
>
> Im planning to continue this today, along with some other modification
> like g_dbus_client_new_full so we don't break the API.

I actually drop this since with the interface being in gatt-database.c
we don't actually need a notification system.


-- 
Luiz Augusto von Dentz

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

end of thread, other threads:[~2015-02-27 14:33 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-26  5:13 [PATCH BlueZ v1 00/17] Implement GATT server D-Bus API Arman Uguray
2015-02-26  5:13 ` [PATCH BlueZ v1 01/17] shared/gatt: Call bt_att_cancel_all in unref Arman Uguray
2015-02-26  8:44   ` Luiz Augusto von Dentz
2015-02-26 20:44     ` Arman Uguray
2015-02-26  5:13 ` [PATCH BlueZ v1 02/17] gdbus/client: Don't GetManagedObjects w/o handlers Arman Uguray
2015-02-26  5:13 ` [PATCH BlueZ v1 03/17] gdbus/client: Allow specifying ObjectManager path Arman Uguray
2015-02-26 15:38   ` Marcel Holtmann
2015-02-26 20:32     ` Arman Uguray
2015-02-27  6:47       ` Luiz Augusto von Dentz
2015-02-26  5:13 ` [PATCH BlueZ v1 04/17] doc/gatt-api.txt: New ObjectManager requirements Arman Uguray
2015-02-26  5:13 ` [PATCH BlueZ v1 05/17] core/gatt: Add GattManager1 stubs Arman Uguray
2015-02-26  5:13 ` [PATCH BlueZ v1 06/17] core/gatt: Implement GattManager1.RegisterService Arman Uguray
2015-02-26  5:13 ` [PATCH BlueZ v1 07/17] core/gatt: Register characteristics Arman Uguray
2015-02-26  5:13 ` [PATCH BlueZ v1 08/17] core/gatt: Support ReadValue for characteristics Arman Uguray
2015-02-26  5:13 ` [PATCH BlueZ v1 09/17] core/gatt: Support WriteValue " Arman Uguray
2015-02-26  5:13 ` [PATCH BlueZ v1 10/17] core/gatt: Make CCC addition API public Arman Uguray
2015-02-26  5:13 ` [PATCH BlueZ v1 11/17] core/gatt: Create CCC for external characteristic Arman Uguray
2015-02-26  5:13 ` [PATCH BlueZ v1 12/17] core/gatt: Add btd_gatt_database_notify function Arman Uguray
2015-02-26  8:51   ` Luiz Augusto von Dentz
2015-02-26 20:49     ` Arman Uguray
2015-02-27 10:54       ` Luiz Augusto von Dentz
2015-02-27 14:33         ` Luiz Augusto von Dentz
2015-02-26  5:13 ` [PATCH BlueZ v1 13/17] core/gatt: Send not/ind for D-Bus characteristics Arman Uguray
2015-02-26  9:02   ` Luiz Augusto von Dentz
2015-02-26  5:13 ` [PATCH BlueZ v1 14/17] core/gatt: Create CEP for external characteristic Arman Uguray
2015-02-26  5:13 ` [PATCH BlueZ v1 15/17] core/gatt: Register descriptors Arman Uguray
2015-02-26  5:13 ` [PATCH BlueZ v1 16/17] core/gatt: Support descriptor reads/writes Arman Uguray
2015-02-26  5:13 ` [PATCH BlueZ v1 17/17] tools: Added a Python example for GATT server API Arman Uguray

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.