[PATCH BlueZ 00/17] Implement doc/gatt-api.txt for client

[PATCH BlueZ 00/17] Implement doc/gatt-api.txt for client

[Arman Uguray]

This patch set implements the D-Bus API outlined in doc/gatt-api.txt for
GATT client role. The main highlights are:
  - All exported objects are managed by a btd_gatt_client, each
    in turn owned by a btd_device. All D-Bus logic is isolated to
    src/gatt-client

  - A GattService1 hierarchy is exported for a GATT service only if no
    btd_service exists for it, i.e. no internal profile has claimed the
    service.

  - Objects are exported when the gatt-client is ready and are removed
    when the device disconnects, as there is nothing interesting that an
    external application can do with services without a connection and
    before the gatt-client becomes ready.

  - Objects are added/removed after "Service Changed" events.

Arman Uguray (17):
  core: gatt: Introduce gatt-client.
  core: gatt: Export GATT services on D-Bus
  core: gatt: Export GATT characteristics on D-Bus
  core: gatt: Export GATT descriptors on D-Bus
  core: gatt: Implement GattCharacteristic1.ReadValue
  core: gatt: Implement GattDescriptor1.ReadValue
  core: gatt: Implement GattCharacteristic1.StartNotify
  core: gatt: Implement GattCharacteristic1.StopNotify
  core: gatt: Expose charac. extended properties.
  shared/gatt-client: Expose MTU.
  core: gatt: Implement GattCharacteristic1.WriteValue.
  core: gatt: Don't always issue read-long for ReadValue
  core: gatt: Implement GattDescriptor1.WriteValue.
  core: gatt: Reference count chrcs and descs.
  core: gatt: Handle Service Changed.
  core: device: Add function to get GATT service
  core: gatt: Don't export claimed services

 Makefile.am              |    1 +
 src/device.c             |   51 +-
 src/device.h             |    4 +
 src/gatt-client.c        | 1943 ++++++++++++++++++++++++++++++++++++++++++++++
 src/gatt-client.h        |   30 +
 src/shared/gatt-client.c |   14 +-
 src/shared/gatt-client.h |    8 +-
 7 files changed, 2044 insertions(+), 7 deletions(-)
 create mode 100644 src/gatt-client.c
 create mode 100644 src/gatt-client.h

-- 
2.2.0.rc0.207.ga3a616c

[PATCH BlueZ 01/17] core: gatt: Introduce gatt-client.

[Arman Uguray]

This patch introduces src/gatt-client, which will implement the D-Bus
API outlined in doc/gatt-api.txt for the GATT client role.
---
 Makefile.am       |   1 +
 src/device.c      |  24 ++++++++-
 src/gatt-client.c | 142 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/gatt-client.h |  30 ++++++++++++
 4 files changed, 196 insertions(+), 1 deletion(-)
 create mode 100644 src/gatt-client.c
 create mode 100644 src/gatt-client.h

diff --git a/Makefile.am b/Makefile.am
index f2b22ae..02bfacc 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -181,6 +181,7 @@ src_bluetoothd_SOURCES = $(builtin_sources) \
 			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 \
 			src/eir.h src/eir.c
diff --git a/src/device.c b/src/device.c
index 2e55281..f13d9aa 100644
--- a/src/device.c
+++ b/src/device.c
@@ -59,6 +59,7 @@
 #include "attrib/gattrib.h"
 #include "attio.h"
 #include "device.h"
+#include "gatt-client.h"
 #include "profile.h"
 #include "service.h"
 #include "dbus-common.h"
@@ -223,6 +224,8 @@ struct btd_device {
 	struct gatt_db *db;			/* GATT db cache */
 	struct bt_gatt_client *client;		/* GATT client instance */
 
+	struct btd_gatt_client *client_dbus;
+
 	struct bearer_state bredr_state;
 	struct bearer_state le_state;
 
@@ -574,6 +577,9 @@ static void device_free(gpointer user_data)
 {
 	struct btd_device *device = user_data;
 
+	btd_gatt_client_destroy(device->client_dbus);
+	device->client_dbus = NULL;
+
 	g_slist_free_full(device->uuids, g_free);
 	g_slist_free_full(device->primaries, g_free);
 	g_slist_free_full(device->attios, g_free);
@@ -2695,6 +2701,8 @@ static void gatt_service_added(struct gatt_db_attribute *attr, void *user_data)
 		service_accept(l->data);
 
 	store_services(device);
+
+	btd_gatt_client_service_added(device->client_dbus, attr);
 }
 
 static gint prim_attr_cmp(gconstpointer a, gconstpointer b)
@@ -2765,6 +2773,8 @@ static void gatt_service_removed(struct gatt_db_attribute *attr,
 	g_free(prim);
 
 	store_services(device);
+
+	btd_gatt_client_service_removed(device->client_dbus, attr);
 }
 
 static struct btd_device *device_new(struct btd_adapter *adapter,
@@ -2791,6 +2801,15 @@ static struct btd_device *device_new(struct btd_adapter *adapter,
 	g_strdelimit(device->path, ":", '_');
 	g_free(address_up);
 
+	str2ba(address, &device->bdaddr);
+
+	device->client_dbus = btd_gatt_client_new(device);
+	if (!device->client_dbus) {
+		error("Failed to create btd_gatt_client");
+		device_free(device);
+		return NULL;
+	}
+
 	DBG("Creating device %s", device->path);
 
 	if (g_dbus_register_interface(dbus_conn,
@@ -2803,7 +2822,6 @@ static struct btd_device *device_new(struct btd_adapter *adapter,
 		return NULL;
 	}
 
-	str2ba(address, &device->bdaddr);
 	device->adapter = adapter;
 	device->temporary = TRUE;
 
@@ -3841,6 +3859,8 @@ static void att_disconnected_cb(int err, void *user_data)
 
 	g_slist_foreach(device->attios, attio_disconnected, NULL);
 
+	btd_gatt_client_disconnected(device->client_dbus);
+
 	if (!device_get_auto_connect(device)) {
 		DBG("Automatic connection disabled");
 		goto done;
@@ -3949,6 +3969,8 @@ static void gatt_client_ready_cb(bool success, uint8_t att_ecode,
 	device_accept_gatt_profiles(device);
 
 	g_slist_foreach(device->attios, attio_connected, device->attrib);
+
+	btd_gatt_client_ready(device->client_dbus);
 }
 
 static void gatt_client_service_changed(uint16_t start_handle,
diff --git a/src/gatt-client.c b/src/gatt-client.c
new file mode 100644
index 0000000..462753c
--- /dev/null
+++ b/src/gatt-client.c
@@ -0,0 +1,142 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2014  Google Inc.
+ *
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdbool.h>
+#include <stdint.h>
+
+#include <bluetooth/bluetooth.h>
+
+#include "log.h"
+#include "adapter.h"
+#include "device.h"
+#include "lib/uuid.h"
+#include "src/shared/queue.h"
+#include "src/shared/att.h"
+#include "src/shared/gatt-db.h"
+#include "src/shared/gatt-client.h"
+#include "src/shared/util.h"
+#include "gatt-client.h"
+
+struct btd_gatt_client {
+	struct btd_device *device;
+	char devaddr[18];
+	struct gatt_db *db;
+	struct bt_gatt_client *gatt;
+
+	struct queue *services;
+};
+
+static void service_free(void *data)
+{
+	/* TODO */
+}
+
+static void create_services(struct btd_gatt_client *client)
+{
+	DBG("Exporting objects for GATT services: %s", client->devaddr);
+
+	/* TODO */
+}
+
+struct btd_gatt_client *btd_gatt_client_new(struct btd_device *device)
+{
+	struct btd_gatt_client *client;
+	struct gatt_db *db;
+
+	if (!device)
+		return NULL;
+
+	db = btd_device_get_gatt_db(device);
+	if (!db)
+		return NULL;
+
+	client = new0(struct btd_gatt_client, 1);
+	if (!client)
+		return NULL;
+
+	client->services = queue_new();
+	if (!client->services) {
+		free(client);
+		return NULL;
+	}
+
+	client->device = device;
+	ba2str(device_get_address(device), client->devaddr);
+
+	client->db = gatt_db_ref(db);
+
+	return client;
+}
+
+void btd_gatt_client_destroy(struct btd_gatt_client *client)
+{
+	if (!client)
+		return;
+
+	queue_destroy(client->services, service_free);
+	bt_gatt_client_unref(client->gatt);
+	gatt_db_unref(client->db);
+	free(client);
+}
+
+void btd_gatt_client_ready(struct btd_gatt_client *client)
+{
+	struct bt_gatt_client *gatt;
+
+	if (!client)
+		return;
+
+	gatt = btd_device_get_gatt_client(client->device);
+	if (!gatt) {
+		error("GATT client not initialized");
+		return;
+	}
+
+	bt_gatt_client_unref(client->gatt);
+	client->gatt = bt_gatt_client_ref(gatt);
+
+	create_services(client);
+}
+
+void btd_gatt_client_service_added(struct btd_gatt_client *client,
+					struct gatt_db_attribute *attrib)
+{
+	/* TODO */
+}
+
+void btd_gatt_client_service_removed(struct btd_gatt_client *client,
+					struct gatt_db_attribute *attrib)
+{
+	/* TODO */
+}
+
+void btd_gatt_client_disconnected(struct btd_gatt_client *client)
+{
+	if (!client)
+		return;
+
+	DBG("Device disconnected. Cleaning up");
+
+	bt_gatt_client_unref(client->gatt);
+	client->gatt = NULL;
+}
diff --git a/src/gatt-client.h b/src/gatt-client.h
new file mode 100644
index 0000000..f6da3b0
--- /dev/null
+++ b/src/gatt-client.h
@@ -0,0 +1,30 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2014  Google Inc.
+ *
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ */
+
+struct btd_gatt_client;
+
+struct btd_gatt_client *btd_gatt_client_new(struct btd_device *device);
+void btd_gatt_client_destroy(struct btd_gatt_client *client);
+
+void btd_gatt_client_ready(struct btd_gatt_client *client);
+void btd_gatt_client_service_added(struct btd_gatt_client *client,
+					struct gatt_db_attribute *attrib);
+void btd_gatt_client_service_removed(struct btd_gatt_client *client,
+					struct gatt_db_attribute *attrib);
+void btd_gatt_client_disconnected(struct btd_gatt_client *client);
-- 
2.2.0.rc0.207.ga3a616c

[PATCH BlueZ 02/17] core: gatt: Export GATT services on D-Bus

[Arman Uguray]

This patch adds code that exports an object on D-Bus with the
org.bluez.GattService1 interface for each GATT service that is present
in bt_gatt_client.
---
 src/gatt-client.c | 153 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 150 insertions(+), 3 deletions(-)

diff --git a/src/gatt-client.c b/src/gatt-client.c
index 462753c..05782ab 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -24,6 +24,9 @@
 #include <stdbool.h>
 #include <stdint.h>
 
+#include <dbus/dbus.h>
+#include <gdbus/gdbus.h>
+
 #include <bluetooth/bluetooth.h>
 
 #include "log.h"
@@ -36,6 +39,9 @@
 #include "src/shared/gatt-client.h"
 #include "src/shared/util.h"
 #include "gatt-client.h"
+#include "dbus-common.h"
+
+#define GATT_SERVICE_IFACE "org.bluez.GattService1"
 
 struct btd_gatt_client {
 	struct btd_device *device;
@@ -46,16 +52,151 @@ struct btd_gatt_client {
 	struct queue *services;
 };
 
+struct service {
+	struct btd_gatt_client *client;
+	bool primary;
+	uint16_t start_handle;
+	uint16_t end_handle;
+	bt_uuid_t uuid;
+	char *path;
+};
+
+static gboolean service_property_get_uuid(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	char uuid[MAX_LEN_UUID_STR + 1];
+	const char *ptr = uuid;
+	struct service *service = data;
+
+	bt_uuid_to_string(&service->uuid, uuid, sizeof(uuid));
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &ptr);
+
+	return TRUE;
+}
+
+static gboolean service_property_get_device(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct service *service = data;
+	const char *str = device_get_path(service->client->device);
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_OBJECT_PATH, &str);
+
+	return TRUE;
+}
+
+static gboolean service_property_get_primary(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct service *service = data;
+	dbus_bool_t primary;
+
+	primary = service->primary ? TRUE : FALSE;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN, &primary);
+
+	return TRUE;
+}
+
+static gboolean service_property_get_characteristics(
+					const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	DBusMessageIter array;
+
+	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, "o", &array);
+
+	/* TODO: Implement this once characteristics are exported */
+
+	dbus_message_iter_close_container(iter, &array);
+
+	return TRUE;
+}
+
+static const GDBusPropertyTable service_properties[] = {
+	{ "UUID", "s", service_property_get_uuid },
+	{ "Device", "o", service_property_get_device },
+	{ "Primary", "b", service_property_get_primary },
+	{ "Characteristics", "ao", service_property_get_characteristics },
+	{ }
+};
+
 static void service_free(void *data)
 {
-	/* TODO */
+	struct service *service = data;
+
+	g_free(service->path);
+	free(service);
+}
+
+static struct service *service_create(struct gatt_db_attribute *attr,
+						struct btd_gatt_client *client)
+{
+	struct service *service;
+	const char *device_path = device_get_path(client->device);
+	bt_uuid_t uuid;
+
+	service = new0(struct service, 1);
+	if (!service)
+		return NULL;
+
+	service->client = client;
+
+	gatt_db_attribute_get_service_data(attr, &service->start_handle,
+							&service->end_handle,
+							&service->primary,
+							&uuid);
+	bt_uuid_to_uuid128(&uuid, &service->uuid);
+
+	service->path = g_strdup_printf("%s/service%04x", device_path,
+							service->start_handle);
+
+	if (!g_dbus_register_interface(btd_get_dbus_connection(), service->path,
+						GATT_SERVICE_IFACE,
+						NULL, NULL,
+						service_properties,
+						service, service_free)) {
+		error("Unable to register GATT service with handle 0x%04x for "
+							"device %s:",
+							service->start_handle,
+							client->devaddr);
+		service_free(service);
+
+		return NULL;
+	}
+
+	DBG("Exported GATT service: %s", service->path);
+
+	return service;
+}
+
+static void unregister_service(void *data)
+{
+	struct service *service = data;
+
+	DBG("Removing GATT service: %s", service->path);
+
+	g_dbus_unregister_interface(btd_get_dbus_connection(), service->path,
+							GATT_SERVICE_IFACE);
+}
+
+static void export_service(struct gatt_db_attribute *attr, void *user_data)
+{
+	struct btd_gatt_client *client = user_data;
+	struct service *service;
+
+	service = service_create(attr, client);
+	if (!service)
+		return;
+
+	queue_push_tail(client->services, service);
 }
 
 static void create_services(struct btd_gatt_client *client)
 {
 	DBG("Exporting objects for GATT services: %s", client->devaddr);
 
-	/* TODO */
+	gatt_db_foreach_service(client->db, NULL, export_service, client);
 }
 
 struct btd_gatt_client *btd_gatt_client_new(struct btd_device *device)
@@ -93,7 +234,7 @@ void btd_gatt_client_destroy(struct btd_gatt_client *client)
 	if (!client)
 		return;
 
-	queue_destroy(client->services, service_free);
+	queue_destroy(client->services, unregister_service);
 	bt_gatt_client_unref(client->gatt);
 	gatt_db_unref(client->db);
 	free(client);
@@ -137,6 +278,12 @@ void btd_gatt_client_disconnected(struct btd_gatt_client *client)
 
 	DBG("Device disconnected. Cleaning up");
 
+	/*
+	 * Remove all services. We'll recreate them when a new bt_gatt_client
+	 * becomes ready.
+	 */
+	queue_remove_all(client->services, NULL, NULL, unregister_service);
+
 	bt_gatt_client_unref(client->gatt);
 	client->gatt = NULL;
 }
-- 
2.2.0.rc0.207.ga3a616c

[PATCH BlueZ 03/17] core: gatt: Export GATT characteristics on D-Bus

[Arman Uguray]

This patch adds code that exports an object on D-Bus with the
org.bluez.GattCharacteristic1 interface for each GATT characteristic
that is present in bt_gatt_client.
---
 src/gatt-client.c | 332 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 330 insertions(+), 2 deletions(-)

diff --git a/src/gatt-client.c b/src/gatt-client.c
index 05782ab..1a0e1cf 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -30,6 +30,7 @@
 #include <bluetooth/bluetooth.h>
 
 #include "log.h"
+#include "error.h"
 #include "adapter.h"
 #include "device.h"
 #include "lib/uuid.h"
@@ -41,7 +42,8 @@
 #include "gatt-client.h"
 #include "dbus-common.h"
 
-#define GATT_SERVICE_IFACE "org.bluez.GattService1"
+#define GATT_SERVICE_IFACE		"org.bluez.GattService1"
+#define GATT_CHARACTERISTIC_IFACE	"org.bluez.GattCharacteristic1"
 
 struct btd_gatt_client {
 	struct btd_device *device;
@@ -59,8 +61,242 @@ struct service {
 	uint16_t end_handle;
 	bt_uuid_t uuid;
 	char *path;
+	struct queue *chrcs;
+	bool chrcs_ready;
 };
 
+struct characteristic {
+	struct service *service;
+	uint16_t handle;
+	uint16_t value_handle;
+	uint8_t props;
+	bt_uuid_t uuid;
+	char *path;
+};
+
+static gboolean characteristic_property_get_uuid(
+					const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	char uuid[MAX_LEN_UUID_STR + 1];
+	const char *ptr = uuid;
+	struct characteristic *chrc = data;
+
+	bt_uuid_to_string(&chrc->uuid, uuid, sizeof(uuid));
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &ptr);
+
+	return TRUE;
+}
+
+static gboolean characteristic_property_get_service(
+					const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct characteristic *chrc = data;
+	const char *str = chrc->service->path;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_OBJECT_PATH, &str);
+
+	return TRUE;
+}
+
+static gboolean characteristic_property_get_value(
+					const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	DBusMessageIter array;
+
+	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, "y", &array);
+
+	/* TODO: Implement this once the value is cached */
+
+	dbus_message_iter_close_container(iter, &array);
+
+	return TRUE;
+}
+
+static gboolean characteristic_property_get_notifying(
+					const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	dbus_bool_t notifying = FALSE;
+
+	/*
+	 * TODO: Return the correct value here once StartNotify and StopNotify
+	 * methods are implemented.
+	 */
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN, &notifying);
+
+	return TRUE;
+}
+
+static struct {
+	uint8_t prop;
+	char *str;
+} properties[] = {
+	/* Default Properties */
+	{ BT_GATT_CHRC_PROP_BROADCAST,		"broadcast" },
+	{ BT_GATT_CHRC_PROP_READ,		"read" },
+	{ BT_GATT_CHRC_PROP_WRITE_WITHOUT_RESP,	"write-without-response" },
+	{ BT_GATT_CHRC_PROP_WRITE,		"write" },
+	{ BT_GATT_CHRC_PROP_NOTIFY,		"notify" },
+	{ BT_GATT_CHRC_PROP_INDICATE,		"indicate" },
+	{ BT_GATT_CHRC_PROP_AUTH,		"authenticated-signed-writes" },
+	{ BT_GATT_CHRC_PROP_EXT_PROP,		"extended-properties" },
+	{ },
+	/* Extended Properties */
+	{ BT_GATT_CHRC_EXT_PROP_RELIABLE_WRITE,	"reliable-write" },
+	{ BT_GATT_CHRC_EXT_PROP_WRITABLE_AUX,	"writable-auxiliaries" },
+	{ }
+};
+
+static gboolean characteristic_property_get_flags(
+					const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct characteristic *chrc = data;
+	DBusMessageIter array;
+	int i;
+
+	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, "s", &array);
+
+	for (i = 0; properties[i].str; i++) {
+		if (chrc->props & properties[i].prop)
+			dbus_message_iter_append_basic(&array, DBUS_TYPE_STRING,
+							&properties[i].str);
+	}
+
+	/*
+	 * TODO: Handle extended properties if the descriptor is
+	 * present.
+	 */
+
+	dbus_message_iter_close_container(iter, &array);
+
+	return TRUE;
+}
+
+static DBusMessage *characteristic_read_value(DBusConnection *conn,
+					DBusMessage *msg, void *user_data)
+{
+	/* TODO: Implement */
+	return btd_error_failed(msg, "Not implemented");
+}
+
+static DBusMessage *characteristic_write_value(DBusConnection *conn,
+					DBusMessage *msg, void *user_data)
+{
+	/* TODO: Implement */
+	return btd_error_failed(msg, "Not implemented");
+}
+
+static DBusMessage *characteristic_start_notify(DBusConnection *conn,
+					DBusMessage *msg, void *user_data)
+{
+	/* TODO: Implement */
+	return btd_error_failed(msg, "Not implemented");
+}
+
+static DBusMessage *characteristic_stop_notify(DBusConnection *conn,
+					DBusMessage *msg, void *user_data)
+{
+	/* TODO: Implement */
+	return btd_error_failed(msg, "Not implemented");
+}
+
+static gboolean characteristic_property_get_descriptors(
+					const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	DBusMessageIter array;
+
+	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, "o", &array);
+
+	/* TODO: Implement this once descriptors are exported */
+
+	dbus_message_iter_close_container(iter, &array);
+
+	return TRUE;
+}
+
+static const GDBusPropertyTable characteristic_properties[] = {
+	{ "UUID", "s", characteristic_property_get_uuid },
+	{ "Service", "o", characteristic_property_get_service },
+	{ "Value", "ay", characteristic_property_get_value },
+	{ "Notifying", "b", characteristic_property_get_notifying },
+	{ "Flags", "as", characteristic_property_get_flags },
+	{ "Descriptors", "ao", characteristic_property_get_descriptors },
+	{ }
+};
+
+static const GDBusMethodTable characteristic_methods[] = {
+	{ GDBUS_ASYNC_METHOD("ReadValue", NULL, GDBUS_ARGS({ "value", "ay" }),
+						characteristic_read_value) },
+	{ GDBUS_ASYNC_METHOD("WriteValue", GDBUS_ARGS({ "value", "ay" }),
+					NULL, characteristic_write_value) },
+	{ GDBUS_ASYNC_METHOD("StartNotify", NULL, NULL,
+						characteristic_start_notify) },
+	{ GDBUS_METHOD("StopNotify", NULL, NULL, characteristic_stop_notify) },
+	{ }
+};
+
+static void characteristic_free(void *data)
+{
+	struct characteristic *chrc = data;
+
+	g_free(chrc->path);
+	free(chrc);
+}
+
+static struct characteristic *characteristic_create(
+						struct gatt_db_attribute *attr,
+						struct service *service)
+{
+	struct characteristic *chrc;
+	bt_uuid_t uuid;
+
+	chrc = new0(struct characteristic, 1);
+	if (!chrc)
+		return NULL;
+
+	chrc->service = service;
+
+	gatt_db_attribute_get_char_data(attr, &chrc->handle,
+							&chrc->value_handle,
+							&chrc->props, &uuid);
+	bt_uuid_to_uuid128(&uuid, &chrc->uuid);
+
+	chrc->path = g_strdup_printf("%s/char%04x", service->path,
+								chrc->handle);
+
+	if (!g_dbus_register_interface(btd_get_dbus_connection(), chrc->path,
+						GATT_CHARACTERISTIC_IFACE,
+						characteristic_methods, NULL,
+						characteristic_properties,
+						chrc, characteristic_free)) {
+		error("Unable to register GATT characteristic with handle "
+							"0x%04x", chrc->handle);
+		characteristic_free(chrc);
+
+		return NULL;
+	}
+
+	DBG("Exported GATT characteristic: %s", chrc->path);
+
+	return chrc;
+}
+
+static void unregister_characteristic(void *data)
+{
+	struct characteristic *chrc = data;
+
+	DBG("Removing GATT characteristic: %s", chrc->path);
+
+	g_dbus_unregister_interface(btd_get_dbus_connection(), chrc->path,
+						GATT_CHARACTERISTIC_IFACE);
+}
+
 static gboolean service_property_get_uuid(const GDBusPropertyTable *property,
 					DBusMessageIter *iter, void *data)
 {
@@ -98,15 +334,26 @@ static gboolean service_property_get_primary(const GDBusPropertyTable *property,
 	return TRUE;
 }
 
+static void append_chrc_path(void *data, void *user_data)
+{
+	struct characteristic *chrc = data;
+	DBusMessageIter *array = user_data;
+
+	dbus_message_iter_append_basic(array, DBUS_TYPE_OBJECT_PATH,
+								&chrc->path);
+}
+
 static gboolean service_property_get_characteristics(
 					const GDBusPropertyTable *property,
 					DBusMessageIter *iter, void *data)
 {
+	struct service *service = data;
 	DBusMessageIter array;
 
 	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, "o", &array);
 
-	/* TODO: Implement this once characteristics are exported */
+	if (service->chrcs_ready)
+		queue_foreach(service->chrcs, append_chrc_path, &array);
 
 	dbus_message_iter_close_container(iter, &array);
 
@@ -125,6 +372,7 @@ static void service_free(void *data)
 {
 	struct service *service = data;
 
+	queue_destroy(service->chrcs, NULL);  /* List should be empty here */
 	g_free(service->path);
 	free(service);
 }
@@ -140,6 +388,12 @@ static struct service *service_create(struct gatt_db_attribute *attr,
 	if (!service)
 		return NULL;
 
+	service->chrcs = queue_new();
+	if (!service->chrcs) {
+		free(service);
+		return NULL;
+	}
+
 	service->client = client;
 
 	gatt_db_attribute_get_service_data(attr, &service->start_handle,
@@ -176,10 +430,71 @@ static void unregister_service(void *data)
 
 	DBG("Removing GATT service: %s", service->path);
 
+	queue_remove_all(service->chrcs, NULL, NULL, unregister_characteristic);
+
 	g_dbus_unregister_interface(btd_get_dbus_connection(), service->path,
 							GATT_SERVICE_IFACE);
 }
 
+struct export_data {
+	void *root;
+	bool failed;
+};
+
+static void export_char(struct gatt_db_attribute *attr, void *user_data)
+{
+	struct characteristic *charac;
+	struct export_data *data = user_data;
+	struct service *service = data->root;
+
+	if (data->failed)
+		return;
+
+	charac = characteristic_create(attr, service);
+	if (!charac) {
+		data->failed = true;
+		return;
+	}
+
+	queue_push_tail(service->chrcs, charac);
+}
+
+static bool create_characteristics(struct gatt_db_attribute *attr,
+						struct service *service)
+{
+	struct export_data data;
+
+	data.root = service;
+	data.failed = false;
+
+	gatt_db_service_foreach_char(attr, export_char, &data);
+
+	return !data.failed;
+}
+
+static void notify_chrcs(void *data, void *user_data)
+{
+	struct service *service = data;
+
+	service->chrcs_ready = true;
+
+	g_dbus_emit_property_changed(btd_get_dbus_connection(), service->path,
+							GATT_SERVICE_IFACE,
+							"Characteristics");
+}
+
+static gboolean set_chrcs_ready(gpointer user_data)
+{
+	struct btd_gatt_client *client = user_data;
+
+	if (!client->gatt)
+		return FALSE;
+
+	queue_foreach(client->services, notify_chrcs, NULL);
+
+	return FALSE;
+}
+
 static void export_service(struct gatt_db_attribute *attr, void *user_data)
 {
 	struct btd_gatt_client *client = user_data;
@@ -189,6 +504,12 @@ static void export_service(struct gatt_db_attribute *attr, void *user_data)
 	if (!service)
 		return;
 
+	if (!create_characteristics(attr, service)) {
+		error("Exporting characteristics failed");
+		unregister_service(service);
+		return;
+	}
+
 	queue_push_tail(client->services, service);
 }
 
@@ -197,6 +518,13 @@ static void create_services(struct btd_gatt_client *client)
 	DBG("Exporting objects for GATT services: %s", client->devaddr);
 
 	gatt_db_foreach_service(client->db, NULL, export_service, client);
+
+	/*
+	 * Asynchronously update the "Characteristics" property of each service.
+	 * We do this so that users have a way to know that all characteristics
+	 * of a service have been exported.
+	 */
+	g_idle_add(set_chrcs_ready, client);
 }
 
 struct btd_gatt_client *btd_gatt_client_new(struct btd_device *device)
-- 
2.2.0.rc0.207.ga3a616c

[PATCH BlueZ 04/17] core: gatt: Export GATT descriptors on D-Bus

[Arman Uguray]

This patch adds code that exports an object on D-Bus with the
org.bluez.GattDescriptor1 interface for each GATT descriptor that is
present in bt_gatt_client.
---
 src/gatt-client.c | 194 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 190 insertions(+), 4 deletions(-)

diff --git a/src/gatt-client.c b/src/gatt-client.c
index 1a0e1cf..bd3711a 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -44,6 +44,7 @@
 
 #define GATT_SERVICE_IFACE		"org.bluez.GattService1"
 #define GATT_CHARACTERISTIC_IFACE	"org.bluez.GattCharacteristic1"
+#define GATT_DESCRIPTOR_IFACE		"org.bluez.GattDescriptor1"
 
 struct btd_gatt_client {
 	struct btd_device *device;
@@ -72,8 +73,135 @@ struct characteristic {
 	uint8_t props;
 	bt_uuid_t uuid;
 	char *path;
+	struct queue *descs;
 };
 
+struct descriptor {
+	struct characteristic *chrc;
+	uint16_t handle;
+	bt_uuid_t uuid;
+	char *path;
+};
+
+static gboolean descriptor_property_get_uuid(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	char uuid[MAX_LEN_UUID_STR + 1];
+	const char *ptr = uuid;
+	struct descriptor *desc = data;
+
+	bt_uuid_to_string(&desc->uuid, uuid, sizeof(uuid));
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &ptr);
+
+	return TRUE;
+}
+
+static gboolean descriptor_property_get_characteristic(
+					const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct descriptor *desc = data;
+	const char *str = desc->chrc->path;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_OBJECT_PATH, &str);
+
+	return TRUE;
+}
+
+static gboolean descriptor_property_get_value(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	DBusMessageIter array;
+
+	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, "y", &array);
+
+	/* TODO: Implement this once the value is cached */
+
+	dbus_message_iter_close_container(iter, &array);
+
+	return TRUE;
+}
+
+static DBusMessage *descriptor_read_value(DBusConnection *conn,
+					DBusMessage *msg, void *user_data)
+{
+	/* TODO: Implement */
+	return btd_error_failed(msg, "Not implemented");
+}
+
+static DBusMessage *descriptor_write_value(DBusConnection *conn,
+					DBusMessage *msg, void *user_data)
+{
+	/* TODO: Implement */
+	return btd_error_failed(msg, "Not implemented");
+}
+
+static const GDBusPropertyTable descriptor_properties[] = {
+	{ "UUID", "s", descriptor_property_get_uuid },
+	{ "Characteristic", "o", descriptor_property_get_characteristic },
+	{ "Value", "ay", descriptor_property_get_value },
+	{ }
+};
+
+static const GDBusMethodTable descriptor_methods[] = {
+	{ GDBUS_ASYNC_METHOD("ReadValue", NULL, GDBUS_ARGS({ "value", "ay" }),
+						descriptor_read_value) },
+	{ GDBUS_ASYNC_METHOD("WriteValue", GDBUS_ARGS({ "value", "ay" }),
+					NULL, descriptor_write_value) },
+	{ }
+};
+
+static void descriptor_free(void *data)
+{
+	struct descriptor *desc = data;
+
+	g_free(desc->path);
+	free(desc);
+}
+
+static struct descriptor *descriptor_create(struct gatt_db_attribute *attr,
+						struct characteristic *chrc)
+{
+	struct descriptor *desc;
+
+	desc = new0(struct descriptor, 1);
+	if (!desc)
+		return NULL;
+
+	desc->chrc = chrc;
+	desc->handle = gatt_db_attribute_get_handle(attr);
+
+	bt_uuid_to_uuid128(gatt_db_attribute_get_type(attr), &desc->uuid);
+
+	desc->path = g_strdup_printf("%s/desc%04x", chrc->path, desc->handle);
+
+	if (!g_dbus_register_interface(btd_get_dbus_connection(), desc->path,
+						GATT_DESCRIPTOR_IFACE,
+						descriptor_methods, NULL,
+						descriptor_properties,
+						desc, descriptor_free)) {
+		error("Unable to register GATT descriptor with handle 0x%04x",
+								desc->handle);
+		descriptor_free(desc);
+
+		return NULL;
+	}
+
+	DBG("Exported GATT characteristic descriptor: %s", desc->path);
+
+	return desc;
+}
+
+static void unregister_descriptor(void *data)
+{
+	struct descriptor *desc = data;
+
+	DBG("Removing GATT descriptor: %s", desc->path);
+
+	g_dbus_unregister_interface(btd_get_dbus_connection(), desc->path,
+							GATT_DESCRIPTOR_IFACE);
+}
+
 static gboolean characteristic_property_get_uuid(
 					const GDBusPropertyTable *property,
 					DBusMessageIter *iter, void *data)
@@ -205,15 +333,25 @@ static DBusMessage *characteristic_stop_notify(DBusConnection *conn,
 	return btd_error_failed(msg, "Not implemented");
 }
 
+static void append_desc_path(void *data, void *user_data)
+{
+	struct descriptor *desc = data;
+	DBusMessageIter *array = user_data;
+
+	dbus_message_iter_append_basic(array, DBUS_TYPE_OBJECT_PATH,
+								&desc->path);
+}
+
 static gboolean characteristic_property_get_descriptors(
 					const GDBusPropertyTable *property,
 					DBusMessageIter *iter, void *data)
 {
+	struct characteristic *chrc = data;
 	DBusMessageIter array;
 
 	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, "o", &array);
 
-	/* TODO: Implement this once descriptors are exported */
+	queue_foreach(chrc->descs, append_desc_path, &array);
 
 	dbus_message_iter_close_container(iter, &array);
 
@@ -245,6 +383,7 @@ static void characteristic_free(void *data)
 {
 	struct characteristic *chrc = data;
 
+	queue_destroy(chrc->descs, NULL);  /* List should be empty here */
 	g_free(chrc->path);
 	free(chrc);
 }
@@ -260,6 +399,12 @@ static struct characteristic *characteristic_create(
 	if (!chrc)
 		return NULL;
 
+	chrc->descs = queue_new();
+	if (!chrc->descs) {
+		free(chrc);
+		return NULL;
+	}
+
 	chrc->service = service;
 
 	gatt_db_attribute_get_char_data(attr, &chrc->handle,
@@ -293,6 +438,8 @@ static void unregister_characteristic(void *data)
 
 	DBG("Removing GATT characteristic: %s", chrc->path);
 
+	queue_remove_all(chrc->descs, NULL, NULL, unregister_descriptor);
+
 	g_dbus_unregister_interface(btd_get_dbus_connection(), chrc->path,
 						GATT_CHARACTERISTIC_IFACE);
 }
@@ -441,6 +588,37 @@ struct export_data {
 	bool failed;
 };
 
+static void export_desc(struct gatt_db_attribute *attr, void *user_data)
+{
+	struct descriptor *desc;
+	struct export_data *data = user_data;
+	struct characteristic *charac = data->root;
+
+	if (data->failed)
+		return;
+
+	desc = descriptor_create(attr, charac);
+	if (!desc) {
+		data->failed = true;
+		return;
+	}
+
+	queue_push_tail(charac->descs, desc);
+}
+
+static bool create_descriptors(struct gatt_db_attribute *attr,
+					struct characteristic *charac)
+{
+	struct export_data data;
+
+	data.root = charac;
+	data.failed = false;
+
+	gatt_db_service_foreach_desc(attr, export_desc, &data);
+
+	return !data.failed;
+}
+
 static void export_char(struct gatt_db_attribute *attr, void *user_data)
 {
 	struct characteristic *charac;
@@ -451,12 +629,20 @@ static void export_char(struct gatt_db_attribute *attr, void *user_data)
 		return;
 
 	charac = characteristic_create(attr, service);
-	if (!charac) {
-		data->failed = true;
-		return;
+	if (!charac)
+		goto fail;
+
+	if (!create_descriptors(attr, charac)) {
+		unregister_characteristic(charac);
+		goto fail;
 	}
 
 	queue_push_tail(service->chrcs, charac);
+
+	return;
+
+fail:
+	data->failed = true;
 }
 
 static bool create_characteristics(struct gatt_db_attribute *attr,
-- 
2.2.0.rc0.207.ga3a616c

[PATCH BlueZ 05/17] core: gatt: Implement GattCharacteristic1.ReadValue

[Arman Uguray]

This patch implements the ReadValue method of
org.bluez.GattCharacteristic1 and exposes the "Value" property based on
what was cached during the most recent read. The property is hidden if
the value is unknown.
---
 src/gatt-client.c | 217 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 213 insertions(+), 4 deletions(-)

diff --git a/src/gatt-client.c b/src/gatt-client.c
index bd3711a..1c4ea51 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -73,6 +73,12 @@ struct characteristic {
 	uint8_t props;
 	bt_uuid_t uuid;
 	char *path;
+
+	bool in_read;
+	bool value_known;
+	uint8_t *value;
+	size_t value_len;
+
 	struct queue *descs;
 };
 
@@ -83,6 +89,66 @@ struct descriptor {
 	char *path;
 };
 
+static DBusMessage *gatt_error_read_not_permitted(DBusMessage *msg)
+{
+	return g_dbus_create_error(msg, ERROR_INTERFACE ".ReadNotPermitted",
+				"Reading of this value is not allowed");
+}
+
+static DBusMessage *gatt_error_write_not_permitted(DBusMessage *msg)
+{
+	return g_dbus_create_error(msg, ERROR_INTERFACE ".WriteNotPermitted",
+				"Writing of this value is not allowed");
+}
+
+static DBusMessage *gatt_error_invalid_value_len(DBusMessage *msg)
+{
+	return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidValueLength",
+							"Invalid value length");
+}
+
+static DBusMessage *gatt_error_invalid_offset(DBusMessage *msg)
+{
+	return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidOffset",
+							"Invalid value offset");
+}
+
+static DBusMessage *gatt_error_not_paired(DBusMessage *msg)
+{
+	return g_dbus_create_error(msg, ERROR_INTERFACE ".NotPaired",
+								"Not Paired");
+}
+
+static DBusMessage *create_gatt_dbus_error(DBusMessage *msg, uint8_t att_ecode)
+{
+	switch (att_ecode) {
+	case BT_ATT_ERROR_READ_NOT_PERMITTED:
+		return gatt_error_read_not_permitted(msg);
+	case BT_ATT_ERROR_WRITE_NOT_PERMITTED:
+		return gatt_error_write_not_permitted(msg);
+	case BT_ATT_ERROR_AUTHENTICATION:
+	case BT_ATT_ERROR_INSUFFICIENT_ENCRYPTION:
+	case BT_ATT_ERROR_INSUFFICIENT_ENCRYPTION_KEY_SIZE:
+		return gatt_error_not_paired(msg);
+	case BT_ATT_ERROR_INVALID_OFFSET:
+		return gatt_error_invalid_offset(msg);
+	case BT_ATT_ERROR_INVALID_ATTRIBUTE_VALUE_LEN:
+		return gatt_error_invalid_value_len(msg);
+	case BT_ATT_ERROR_AUTHORIZATION:
+		return btd_error_not_authorized(msg);
+	case BT_ATT_ERROR_REQUEST_NOT_SUPPORTED:
+		return btd_error_not_supported(msg);
+	case 0:
+		return btd_error_failed(msg, "Operation failed");
+	default:
+		return g_dbus_create_error(msg, ERROR_INTERFACE,
+				"Operation failed with ATT error: 0x%02x",
+				att_ecode);
+	}
+
+	return NULL;
+}
+
 static gboolean descriptor_property_get_uuid(const GDBusPropertyTable *property,
 					DBusMessageIter *iter, void *data)
 {
@@ -232,17 +298,32 @@ static gboolean characteristic_property_get_value(
 					const GDBusPropertyTable *property,
 					DBusMessageIter *iter, void *data)
 {
+	struct characteristic *chrc = data;
 	DBusMessageIter array;
+	size_t i;
 
 	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, "y", &array);
 
-	/* TODO: Implement this once the value is cached */
+	if (chrc->value_known) {
+		for (i = 0; i < chrc->value_len; i++)
+			dbus_message_iter_append_basic(&array, DBUS_TYPE_BYTE,
+							chrc->value + i);
+	}
 
 	dbus_message_iter_close_container(iter, &array);
 
 	return TRUE;
 }
 
+static gboolean characteristic_property_value_exists(
+					const GDBusPropertyTable *property,
+					void *data)
+{
+	struct characteristic *chrc = data;
+
+	return chrc->value_known ? TRUE : FALSE;
+}
+
 static gboolean characteristic_property_get_notifying(
 					const GDBusPropertyTable *property,
 					DBusMessageIter *iter, void *data)
@@ -305,11 +386,137 @@ static gboolean characteristic_property_get_flags(
 	return TRUE;
 }
 
+struct chrc_dbus_op {
+	struct characteristic *chrc;
+	DBusMessage *msg;
+};
+
+static void chrc_dbus_op_free(void *data)
+{
+	struct chrc_dbus_op *op = data;
+
+	dbus_message_unref(op->msg);
+	free(op);
+}
+
+static bool chrc_resize_value(struct characteristic *chrc, size_t new_size)
+{
+	uint8_t *ptr;
+
+	if (chrc->value_len == new_size)
+		return true;
+
+	if (!new_size) {
+		free(chrc->value);
+		chrc->value = NULL;
+		chrc->value_len = 0;
+
+		return true;
+	}
+
+	ptr = realloc(chrc->value, sizeof(uint8_t) * new_size);
+	if (!ptr)
+		return false;
+
+	chrc->value = ptr;
+	chrc->value_len = new_size;
+
+	return true;
+}
+
+static void chrc_read_long_cb(bool success, uint8_t att_ecode,
+					const uint8_t *value, uint16_t length,
+					void *user_data)
+{
+	struct chrc_dbus_op *op = user_data;
+	struct characteristic *chrc = op->chrc;
+	DBusMessageIter iter, array;
+	DBusMessage *reply;
+
+	op->chrc->in_read = false;
+
+	if (!success) {
+		reply = create_gatt_dbus_error(op->msg, att_ecode);
+		goto done;
+	}
+
+	/*
+	 * If the value is the same, then only update it if it wasn't previously
+	 * known.
+	 */
+	if (chrc->value_known && chrc->value_len == length &&
+			!memcmp(chrc->value, value, sizeof(uint8_t) * length))
+		goto reply;
+
+	if (!chrc_resize_value(chrc, length)) {
+		/*
+		 * Failed to resize the buffer. Since we don't want to show a
+		 * stale value, if the value was previously known, then free and
+		 * hide it.
+		 */
+		free(chrc->value);
+		chrc->value = NULL;
+		chrc->value_len = 0;
+		chrc->value_known = false;
+
+		goto changed_signal;
+	}
+
+	chrc->value_known = true;
+	memcpy(chrc->value, value, sizeof(uint8_t) * length);
+
+changed_signal:
+	g_dbus_emit_property_changed(btd_get_dbus_connection(), chrc->path,
+						GATT_CHARACTERISTIC_IFACE,
+						"Value");
+
+reply:
+	reply = g_dbus_create_reply(op->msg, DBUS_TYPE_INVALID);
+	if (!reply) {
+		error("Failed to allocate D-Bus message reply");
+		return;
+	}
+
+	dbus_message_iter_init_append(reply, &iter);
+	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY, "y", &array);
+
+	for (i = 0; i < length; i++)
+		dbus_message_iter_append_basic(&array, DBUS_TYPE_BYTE,
+								value + i);
+
+	dbus_message_iter_close_container(&iter, &array);
+
+done:
+	g_dbus_send_message(btd_get_dbus_connection(), reply);
+}
+
 static DBusMessage *characteristic_read_value(DBusConnection *conn,
 					DBusMessage *msg, void *user_data)
 {
-	/* TODO: Implement */
-	return btd_error_failed(msg, "Not implemented");
+	struct characteristic *chrc = user_data;
+	struct bt_gatt_client *gatt = chrc->service->client->gatt;
+	struct chrc_dbus_op *op;
+
+	if (chrc->in_read)
+		return btd_error_in_progress(msg);
+
+	op = new0(struct chrc_dbus_op, 1);
+	if (!op)
+		return btd_error_failed(msg, "Failed to initialize request");
+
+	op->chrc = chrc;
+	op->msg = dbus_message_ref(msg);
+
+	if (bt_gatt_client_read_long_value(gatt, chrc->value_handle, 0,
+							chrc_read_long_cb, op,
+							chrc_dbus_op_free)) {
+		chrc->in_read = true;
+		return NULL;
+	}
+
+	chrc_dbus_op_free(op);
+
+	return btd_error_failed(msg, "Failed to send read request");
 }
 
 static DBusMessage *characteristic_write_value(DBusConnection *conn,
@@ -361,7 +568,8 @@ static gboolean characteristic_property_get_descriptors(
 static const GDBusPropertyTable characteristic_properties[] = {
 	{ "UUID", "s", characteristic_property_get_uuid },
 	{ "Service", "o", characteristic_property_get_service },
-	{ "Value", "ay", characteristic_property_get_value },
+	{ "Value", "ay", characteristic_property_get_value, NULL,
+					characteristic_property_value_exists },
 	{ "Notifying", "b", characteristic_property_get_notifying },
 	{ "Flags", "as", characteristic_property_get_flags },
 	{ "Descriptors", "ao", characteristic_property_get_descriptors },
@@ -384,6 +592,7 @@ static void characteristic_free(void *data)
 	struct characteristic *chrc = data;
 
 	queue_destroy(chrc->descs, NULL);  /* List should be empty here */
+	free(chrc->value);
 	g_free(chrc->path);
 	free(chrc);
 }
-- 
2.2.0.rc0.207.ga3a616c

[PATCH BlueZ 06/17] core: gatt: Implement GattDescriptor1.ReadValue

[Arman Uguray]

This patch implements the ReadValue method of org.bluez.GattDescriptor1
and exposes the "Value" property based on what was cached during the
most recent read. The property is hidden if the value is unknown.
---
 src/gatt-client.c | 266 ++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 177 insertions(+), 89 deletions(-)

diff --git a/src/gatt-client.c b/src/gatt-client.c
index 1c4ea51..9cbe32d 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -87,6 +87,11 @@ struct descriptor {
 	uint16_t handle;
 	bt_uuid_t uuid;
 	char *path;
+
+	bool in_read;
+	bool value_known;
+	uint8_t *value;
+	size_t value_len;
 };
 
 static DBusMessage *gatt_error_read_not_permitted(DBusMessage *msg)
@@ -177,22 +182,176 @@ static gboolean descriptor_property_get_characteristic(
 static gboolean descriptor_property_get_value(const GDBusPropertyTable *property,
 					DBusMessageIter *iter, void *data)
 {
+	struct descriptor *desc = data;
 	DBusMessageIter array;
+	size_t i;
 
 	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, "y", &array);
 
-	/* TODO: Implement this once the value is cached */
+	if (desc->value_known) {
+		for (i = 0; i < desc->value_len; i++)
+			dbus_message_iter_append_basic(&array, DBUS_TYPE_BYTE,
+							desc->value + i);
+	}
 
 	dbus_message_iter_close_container(iter, &array);
 
 	return TRUE;
 }
 
+static gboolean descriptor_property_value_exists(
+					const GDBusPropertyTable *property,
+					void *data)
+{
+	struct descriptor *desc = data;
+
+	return desc->value_known ? TRUE : FALSE;
+}
+
+static bool resize_value_buffer(size_t new_len, uint8_t **value, size_t *len)
+{
+	uint8_t *ptr;
+
+	if (*len == new_len)
+		return true;
+
+	if (!new_len) {
+		free(*value);
+		*value = NULL;
+		*len = 0;
+
+		return true;
+	}
+
+	ptr = realloc(*value, sizeof(uint8_t) * new_len);
+	if (!ptr)
+		return false;
+
+	*value = ptr;
+	*len = new_len;
+
+	return true;
+}
+
+static void update_value_property(const uint8_t *value, size_t len,
+					uint8_t **cur_value, size_t *cur_len,
+					bool *value_known,
+					const char *path, const char *iface)
+{
+	/*
+	 * If the value is the same, then only updated it if wasn't previously
+	 * known.
+	 */
+	if (*value_known && *cur_len == len &&
+			!memcmp(*cur_value, value, sizeof(uint8_t) * len))
+		return;
+
+	if (resize_value_buffer(len, cur_value, cur_len)) {
+		*value_known = true;
+		memcpy(*cur_value, value, sizeof(uint8_t) * len);
+	} else {
+		/*
+		 * Failed to resize the buffer. Since we don't want to show a
+		 * stale value, if the value was previously known then free and
+		 * hide it.
+		 */
+		free(*cur_value);
+		*cur_value = NULL;
+		*cur_len = 0;
+		*value_known = false;
+	}
+
+	g_dbus_emit_property_changed(btd_get_dbus_connection(), path, iface,
+								"Value");
+}
+
+struct async_dbus_op {
+	DBusMessage *msg;
+	void *data;
+};
+
+static void async_dbus_op_free(void *data)
+{
+	struct async_dbus_op *op = data;
+
+	dbus_message_unref(op->msg);
+	free(op);
+}
+
+static void message_append_byte_array(DBusMessage *msg, const uint8_t *bytes,
+								size_t len)
+{
+	DBusMessageIter iter, array;
+	size_t i;
+
+	dbus_message_iter_init_append(msg, &iter);
+	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY, "y", &array);
+
+	for (i = 0; i < len; i++)
+		dbus_message_iter_append_basic(&array, DBUS_TYPE_BYTE,
+								bytes + i);
+
+	dbus_message_iter_close_container(&iter, &array);
+}
+
+static void desc_read_long_cb(bool success, uint8_t att_ecode,
+					const uint8_t *value, uint16_t length,
+					void *user_data)
+{
+	struct async_dbus_op *op = user_data;
+	struct descriptor *desc = op->data;
+	DBusMessage *reply;
+
+	desc->in_read = false;
+
+	if (!success) {
+		reply = create_gatt_dbus_error(op->msg, att_ecode);
+		goto done;
+	}
+
+	update_value_property(value, length, &desc->value, &desc->value_len,
+						&desc->value_known, desc->path,
+						GATT_DESCRIPTOR_IFACE);
+
+	reply = g_dbus_create_reply(op->msg, DBUS_TYPE_INVALID);
+	if (!reply) {
+		error("Failed to allocate D-Bus message reply");
+		return;
+	}
+
+	message_append_byte_array(reply, value, length);
+
+done:
+	g_dbus_send_message(btd_get_dbus_connection(), reply);
+}
+
 static DBusMessage *descriptor_read_value(DBusConnection *conn,
 					DBusMessage *msg, void *user_data)
 {
-	/* TODO: Implement */
-	return btd_error_failed(msg, "Not implemented");
+	struct descriptor *desc = user_data;
+	struct bt_gatt_client *gatt = desc->chrc->service->client->gatt;
+	struct async_dbus_op *op;
+
+	if (desc->in_read)
+		return btd_error_in_progress(msg);
+
+	op = new0(struct async_dbus_op, 1);
+	if (!op)
+		return btd_error_failed(msg, "Failed to initialize request");
+
+	op->msg = dbus_message_ref(msg);
+	op->data = desc;
+
+	if (bt_gatt_client_read_long_value(gatt, desc->handle, 0,
+							desc_read_long_cb, op,
+							async_dbus_op_free)) {
+		desc->in_read = true;
+		return NULL;
+	}
+
+	async_dbus_op_free(op);
+
+	return btd_error_failed(msg, "Failed to send read request");
 }
 
 static DBusMessage *descriptor_write_value(DBusConnection *conn,
@@ -205,7 +364,8 @@ static DBusMessage *descriptor_write_value(DBusConnection *conn,
 static const GDBusPropertyTable descriptor_properties[] = {
 	{ "UUID", "s", descriptor_property_get_uuid },
 	{ "Characteristic", "o", descriptor_property_get_characteristic },
-	{ "Value", "ay", descriptor_property_get_value },
+	{ "Value", "ay", descriptor_property_get_value, NULL,
+					descriptor_property_value_exists },
 	{ }
 };
 
@@ -221,6 +381,7 @@ static void descriptor_free(void *data)
 {
 	struct descriptor *desc = data;
 
+	free(desc->value);
 	g_free(desc->path);
 	free(desc);
 }
@@ -386,105 +547,32 @@ static gboolean characteristic_property_get_flags(
 	return TRUE;
 }
 
-struct chrc_dbus_op {
-	struct characteristic *chrc;
-	DBusMessage *msg;
-};
-
-static void chrc_dbus_op_free(void *data)
-{
-	struct chrc_dbus_op *op = data;
-
-	dbus_message_unref(op->msg);
-	free(op);
-}
-
-static bool chrc_resize_value(struct characteristic *chrc, size_t new_size)
-{
-	uint8_t *ptr;
-
-	if (chrc->value_len == new_size)
-		return true;
-
-	if (!new_size) {
-		free(chrc->value);
-		chrc->value = NULL;
-		chrc->value_len = 0;
-
-		return true;
-	}
-
-	ptr = realloc(chrc->value, sizeof(uint8_t) * new_size);
-	if (!ptr)
-		return false;
-
-	chrc->value = ptr;
-	chrc->value_len = new_size;
-
-	return true;
-}
-
 static void chrc_read_long_cb(bool success, uint8_t att_ecode,
 					const uint8_t *value, uint16_t length,
 					void *user_data)
 {
-	struct chrc_dbus_op *op = user_data;
-	struct characteristic *chrc = op->chrc;
-	DBusMessageIter iter, array;
+	struct async_dbus_op *op = user_data;
+	struct characteristic *chrc = op->data;
 	DBusMessage *reply;
 
-	op->chrc->in_read = false;
+	chrc->in_read = false;
 
 	if (!success) {
 		reply = create_gatt_dbus_error(op->msg, att_ecode);
 		goto done;
 	}
 
-	/*
-	 * If the value is the same, then only update it if it wasn't previously
-	 * known.
-	 */
-	if (chrc->value_known && chrc->value_len == length &&
-			!memcmp(chrc->value, value, sizeof(uint8_t) * length))
-		goto reply;
-
-	if (!chrc_resize_value(chrc, length)) {
-		/*
-		 * Failed to resize the buffer. Since we don't want to show a
-		 * stale value, if the value was previously known, then free and
-		 * hide it.
-		 */
-		free(chrc->value);
-		chrc->value = NULL;
-		chrc->value_len = 0;
-		chrc->value_known = false;
-
-		goto changed_signal;
-	}
-
-	chrc->value_known = true;
-	memcpy(chrc->value, value, sizeof(uint8_t) * length);
-
-changed_signal:
-	g_dbus_emit_property_changed(btd_get_dbus_connection(), chrc->path,
-						GATT_CHARACTERISTIC_IFACE,
-						"Value");
+	update_value_property(value, length, &chrc->value, &chrc->value_len,
+						&chrc->value_known, chrc->path,
+						GATT_CHARACTERISTIC_IFACE);
 
-reply:
 	reply = g_dbus_create_reply(op->msg, DBUS_TYPE_INVALID);
 	if (!reply) {
 		error("Failed to allocate D-Bus message reply");
 		return;
 	}
 
-	dbus_message_iter_init_append(reply, &iter);
-	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY, "y", &array);
-
-	for (i = 0; i < length; i++)
-		dbus_message_iter_append_basic(&array, DBUS_TYPE_BYTE,
-								value + i);
-
-	dbus_message_iter_close_container(&iter, &array);
+	message_append_byte_array(reply, value, length);
 
 done:
 	g_dbus_send_message(btd_get_dbus_connection(), reply);
@@ -495,26 +583,26 @@ static DBusMessage *characteristic_read_value(DBusConnection *conn,
 {
 	struct characteristic *chrc = user_data;
 	struct bt_gatt_client *gatt = chrc->service->client->gatt;
-	struct chrc_dbus_op *op;
+	struct async_dbus_op *op;
 
 	if (chrc->in_read)
 		return btd_error_in_progress(msg);
 
-	op = new0(struct chrc_dbus_op, 1);
+	op = new0(struct async_dbus_op, 1);
 	if (!op)
 		return btd_error_failed(msg, "Failed to initialize request");
 
-	op->chrc = chrc;
 	op->msg = dbus_message_ref(msg);
+	op->data = chrc;
 
 	if (bt_gatt_client_read_long_value(gatt, chrc->value_handle, 0,
 							chrc_read_long_cb, op,
-							chrc_dbus_op_free)) {
+							async_dbus_op_free)) {
 		chrc->in_read = true;
 		return NULL;
 	}
 
-	chrc_dbus_op_free(op);
+	async_dbus_op_free(op);
 
 	return btd_error_failed(msg, "Failed to send read request");
 }
-- 
2.2.0.rc0.207.ga3a616c

[PATCH BlueZ 07/17] core: gatt: Implement GattCharacteristic1.StartNotify

[Arman Uguray]

This patch implements the StartNotify method of
org.bluez.GattCharacteristic1. Each call to StartNotify assigns a
session to the D-Bus sender which internally registers a unique
notify handler with the bt_gatt_client. Each received notification
or indication causes a PropertiesChanged signal to be emitted for the
"Value" property.

The notify handler gets automatically unregistered when sender
disconnects from D-Bus.
---
 src/gatt-client.c | 272 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 259 insertions(+), 13 deletions(-)

diff --git a/src/gatt-client.c b/src/gatt-client.c
index 9cbe32d..a29eea9 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -80,6 +80,9 @@ struct characteristic {
 	size_t value_len;
 
 	struct queue *descs;
+
+	bool notifying;
+	struct queue *notify_clients;
 };
 
 struct descriptor {
@@ -236,15 +239,20 @@ static bool resize_value_buffer(size_t new_len, uint8_t **value, size_t *len)
 static void update_value_property(const uint8_t *value, size_t len,
 					uint8_t **cur_value, size_t *cur_len,
 					bool *value_known,
-					const char *path, const char *iface)
+					const char *path, const char *iface,
+					bool notify_if_same)
 {
 	/*
 	 * If the value is the same, then only updated it if wasn't previously
 	 * known.
 	 */
 	if (*value_known && *cur_len == len &&
-			!memcmp(*cur_value, value, sizeof(uint8_t) * len))
+			!memcmp(*cur_value, value, sizeof(uint8_t) * len)) {
+		if (notify_if_same)
+			goto notify;
+
 		return;
+	}
 
 	if (resize_value_buffer(len, cur_value, cur_len)) {
 		*value_known = true;
@@ -261,6 +269,7 @@ static void update_value_property(const uint8_t *value, size_t len,
 		*value_known = false;
 	}
 
+notify:
 	g_dbus_emit_property_changed(btd_get_dbus_connection(), path, iface,
 								"Value");
 }
@@ -274,7 +283,9 @@ static void async_dbus_op_free(void *data)
 {
 	struct async_dbus_op *op = data;
 
-	dbus_message_unref(op->msg);
+	if (op->msg)
+		dbus_message_unref(op->msg);
+
 	free(op);
 }
 
@@ -311,7 +322,7 @@ static void desc_read_long_cb(bool success, uint8_t att_ecode,
 
 	update_value_property(value, length, &desc->value, &desc->value_len,
 						&desc->value_known, desc->path,
-						GATT_DESCRIPTOR_IFACE);
+						GATT_DESCRIPTOR_IFACE, false);
 
 	reply = g_dbus_create_reply(op->msg, DBUS_TYPE_INVALID);
 	if (!reply) {
@@ -489,12 +500,8 @@ static gboolean characteristic_property_get_notifying(
 					const GDBusPropertyTable *property,
 					DBusMessageIter *iter, void *data)
 {
-	dbus_bool_t notifying = FALSE;
-
-	/*
-	 * TODO: Return the correct value here once StartNotify and StopNotify
-	 * methods are implemented.
-	 */
+	struct characteristic *chrc = data;
+	dbus_bool_t notifying = chrc->notifying ? TRUE : FALSE;
 
 	dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN, &notifying);
 
@@ -564,7 +571,8 @@ static void chrc_read_long_cb(bool success, uint8_t att_ecode,
 
 	update_value_property(value, length, &chrc->value, &chrc->value_len,
 						&chrc->value_known, chrc->path,
-						GATT_CHARACTERISTIC_IFACE);
+						GATT_CHARACTERISTIC_IFACE,
+						false);
 
 	reply = g_dbus_create_reply(op->msg, DBUS_TYPE_INVALID);
 	if (!reply) {
@@ -614,11 +622,241 @@ static DBusMessage *characteristic_write_value(DBusConnection *conn,
 	return btd_error_failed(msg, "Not implemented");
 }
 
+struct notify_client {
+	struct characteristic *chrc;
+	int ref_count;
+	char *owner;
+	guint watch;
+	unsigned int notify_id;
+};
+
+static void notify_client_free(struct notify_client *client)
+{
+	DBG("owner %s", client->owner);
+
+	g_dbus_remove_watch(btd_get_dbus_connection(), client->watch);
+	free(client->owner);
+	free(client);
+}
+
+static void notify_client_unref(void *data)
+{
+	struct notify_client *client = data;
+
+	DBG("owner %s", client->owner);
+
+	if (__sync_sub_and_fetch(&client->ref_count, 1))
+		return;
+
+	notify_client_free(client);
+}
+
+static struct notify_client *notify_client_ref(struct notify_client *client)
+{
+	DBG("owner %s", client->owner);
+
+	__sync_fetch_and_add(&client->ref_count, 1);
+
+	return client;
+}
+
+static bool match_notifying(const void *a, const void *b)
+{
+	const struct notify_client *client = a;
+
+	return !!client->notify_id;
+}
+
+static void update_notifying(struct characteristic *chrc)
+{
+	if (!chrc->notifying)
+		return;
+
+	if (queue_find(chrc->notify_clients, match_notifying, NULL))
+		return;
+
+	chrc->notifying = false;
+
+	g_dbus_emit_property_changed(btd_get_dbus_connection(), chrc->path,
+						GATT_CHARACTERISTIC_IFACE,
+						"Notifying");
+}
+
+static void notify_client_disconnect(DBusConnection *conn, void *user_data)
+{
+	struct notify_client *client = user_data;
+	struct characteristic *chrc = client->chrc;
+	struct bt_gatt_client *gatt = chrc->service->client->gatt;
+
+	DBG("owner %s", client->owner);
+
+	queue_remove(chrc->notify_clients, client);
+	bt_gatt_client_unregister_notify(gatt, client->notify_id);
+
+	update_notifying(chrc);
+
+	notify_client_unref(client);
+}
+
+static struct notify_client *notify_client_create(struct characteristic *chrc,
+							const char *owner)
+{
+	struct notify_client *client;
+
+	client = new0(struct notify_client, 1);
+	if (!client)
+		return NULL;
+
+	client->chrc = chrc;
+	client->owner = strdup(owner);
+	if (!client->owner) {
+		free(client);
+		return NULL;
+	}
+
+	client->watch = g_dbus_add_disconnect_watch(btd_get_dbus_connection(),
+						owner, notify_client_disconnect,
+						client, NULL);
+	if (!client->watch) {
+		free(client->owner);
+		free(client);
+		return NULL;
+	}
+
+	return notify_client_ref(client);
+}
+
+static bool match_notify_sender(const void *a, const void *b)
+{
+	const struct notify_client *client = a;
+	const char *sender = b;
+
+	return strcmp(client->owner, sender) == 0;
+}
+
+static void notify_cb(uint16_t value_handle, const uint8_t *value,
+					uint16_t length, void *user_data)
+{
+	struct async_dbus_op *op = user_data;
+	struct notify_client *client = op->data;
+	struct characteristic *chrc = client->chrc;
+
+	/*
+	 * Even if the value didn't change, we want to send a PropertiesChanged
+	 * signal so that we propagate the notification/indication to
+	 * applications.
+	 */
+	update_value_property(value, length, &chrc->value, &chrc->value_len,
+						&chrc->value_known, chrc->path,
+						GATT_CHARACTERISTIC_IFACE,
+						true);
+}
+
+static void register_notify_cb(unsigned int id, uint16_t att_ecode,
+								void *user_data)
+{
+	struct async_dbus_op *op = user_data;
+	struct notify_client *client = op->data;
+	struct characteristic *chrc = client->chrc;
+	DBusMessage *reply;
+
+	/* Make sure that an interim disconnect did not remove the client */
+	if (!queue_find(chrc->notify_clients, NULL, client)) {
+		bt_gatt_client_unregister_notify(chrc->service->client->gatt,
+									id);
+		notify_client_unref(client);
+
+		reply = btd_error_failed(op->msg,
+						"Characteristic not available");
+		goto done;
+	}
+
+	/*
+	 * Drop the reference count that we added when registering the callback.
+	 */
+	notify_client_unref(client);
+
+	if (!id) {
+		queue_remove(chrc->notify_clients, client);
+		notify_client_free(client);
+
+		reply = create_gatt_dbus_error(op->msg, att_ecode);
+
+		goto done;
+	}
+
+	client->notify_id = id;
+
+	if (!chrc->notifying) {
+		chrc->notifying = true;
+		g_dbus_emit_property_changed(btd_get_dbus_connection(),
+					chrc->path, GATT_CHARACTERISTIC_IFACE,
+					"Notifying");
+	}
+
+	reply = g_dbus_create_reply(op->msg, DBUS_TYPE_INVALID);
+
+done:
+	if (reply)
+		g_dbus_send_message(btd_get_dbus_connection(), reply);
+	else
+		error("Failed to construct D-Bus message reply");
+
+	dbus_message_unref(op->msg);
+	op->msg = NULL;
+}
+
 static DBusMessage *characteristic_start_notify(DBusConnection *conn,
 					DBusMessage *msg, void *user_data)
 {
-	/* TODO: Implement */
-	return btd_error_failed(msg, "Not implemented");
+	struct characteristic *chrc = user_data;
+	struct bt_gatt_client *gatt = chrc->service->client->gatt;
+	const char *sender = dbus_message_get_sender(msg);
+	struct async_dbus_op *op;
+	struct notify_client *client;
+
+	if (!(chrc->props & BT_GATT_CHRC_PROP_NOTIFY ||
+				chrc->props & BT_GATT_CHRC_PROP_INDICATE))
+		return btd_error_not_supported(msg);
+
+	/* Each client can only have one active notify session. */
+	client = queue_find(chrc->notify_clients, match_notify_sender, sender);
+	if (client)
+		return client->notify_id ?
+				btd_error_failed(msg, "Already notifying") :
+				btd_error_in_progress(msg);
+
+	client = notify_client_create(chrc, sender);
+	if (!client)
+		return btd_error_failed(msg, "Failed allocate notify session");
+
+	op = new0(struct async_dbus_op, 1);
+	if (!op) {
+		notify_client_unref(client);
+		return btd_error_failed(msg, "Failed to initialize request");
+	}
+
+	/*
+	 * Add to the ref count so that a disconnect during register won't free
+	 * the client instance.
+	 */
+	op->data = notify_client_ref(client);
+	op->msg = dbus_message_ref(msg);
+
+	queue_push_tail(chrc->notify_clients, client);
+
+	if (bt_gatt_client_register_notify(gatt, chrc->value_handle,
+						register_notify_cb, notify_cb,
+						op, async_dbus_op_free))
+		return NULL;
+
+	queue_remove(chrc->notify_clients, client);
+	async_dbus_op_free(op);
+
+	/* Directly free the client */
+	notify_client_free(client);
+
+	return btd_error_failed(msg, "Failed to register notify session");
 }
 
 static DBusMessage *characteristic_stop_notify(DBusConnection *conn,
@@ -702,6 +940,13 @@ static struct characteristic *characteristic_create(
 		return NULL;
 	}
 
+	chrc->notify_clients = queue_new();
+	if (!chrc->notify_clients) {
+		queue_destroy(chrc->descs, NULL);
+		free(chrc);
+		return NULL;
+	}
+
 	chrc->service = service;
 
 	gatt_db_attribute_get_char_data(attr, &chrc->handle,
@@ -735,6 +980,7 @@ static void unregister_characteristic(void *data)
 
 	DBG("Removing GATT characteristic: %s", chrc->path);
 
+	queue_remove_all(chrc->notify_clients, NULL, NULL, notify_client_unref);
 	queue_remove_all(chrc->descs, NULL, NULL, unregister_descriptor);
 
 	g_dbus_unregister_interface(btd_get_dbus_connection(), chrc->path,
-- 
2.2.0.rc0.207.ga3a616c

[PATCH BlueZ 08/17] core: gatt: Implement GattCharacteristic1.StopNotify

[Arman Uguray]

This patch implements the StopNotify method of
org.bluez.GattCharacteristic1.
---
 src/gatt-client.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/src/gatt-client.c b/src/gatt-client.c
index a29eea9..e70248a 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -862,8 +862,25 @@ static DBusMessage *characteristic_start_notify(DBusConnection *conn,
 static DBusMessage *characteristic_stop_notify(DBusConnection *conn,
 					DBusMessage *msg, void *user_data)
 {
-	/* TODO: Implement */
-	return btd_error_failed(msg, "Not implemented");
+	struct characteristic *chrc = user_data;
+	struct bt_gatt_client *gatt = chrc->service->client->gatt;
+	const char *sender = dbus_message_get_sender(msg);
+	struct notify_client *client;
+
+	if (!chrc->notifying)
+		return btd_error_failed(msg, "Not notifying");
+
+	client = queue_remove_if(chrc->notify_clients, match_notify_sender,
+							(void *) sender);
+	if (!client)
+		return btd_error_failed(msg, "No notify session started");
+
+	bt_gatt_client_unregister_notify(gatt, client->notify_id);
+	update_notifying(chrc);
+
+	notify_client_unref(client);
+
+	return dbus_message_new_method_return(msg);
 }
 
 static void append_desc_path(void *data, void *user_data)
-- 
2.2.0.rc0.207.ga3a616c

[PATCH BlueZ 09/17] core: gatt: Expose charac. extended properties.

[Arman Uguray]

This patch reads the extended properties for characteristics that have
the necessary descriptor. Updating the "Characteristics" property is
delayed for services until the extended properties have been read for
all of their characteristics that have them.
---
 src/gatt-client.c | 127 +++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 98 insertions(+), 29 deletions(-)

diff --git a/src/gatt-client.c b/src/gatt-client.c
index e70248a..e00f432 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -64,6 +64,8 @@ struct service {
 	char *path;
 	struct queue *chrcs;
 	bool chrcs_ready;
+	struct queue *pending_ext_props;
+	guint idle_id;
 };
 
 struct characteristic {
@@ -71,6 +73,8 @@ struct characteristic {
 	uint16_t handle;
 	uint16_t value_handle;
 	uint8_t props;
+	uint16_t ext_props;
+	uint16_t ext_props_handle;
 	bt_uuid_t uuid;
 	char *path;
 
@@ -157,6 +161,15 @@ static DBusMessage *create_gatt_dbus_error(DBusMessage *msg, uint8_t att_ecode)
 	return NULL;
 }
 
+static bool uuid_cmp(const bt_uuid_t *uuid, uint16_t u16)
+{
+	bt_uuid_t uuid16;
+
+	bt_uuid16_create(&uuid16, u16);
+
+	return bt_uuid_cmp(uuid, &uuid16) == 0;
+}
+
 static gboolean descriptor_property_get_uuid(const GDBusPropertyTable *property,
 					DBusMessageIter *iter, void *data)
 {
@@ -427,6 +440,9 @@ static struct descriptor *descriptor_create(struct gatt_db_attribute *attr,
 
 	DBG("Exported GATT characteristic descriptor: %s", desc->path);
 
+	if (uuid_cmp(&desc->uuid, GATT_CHARAC_EXT_PROPER_UUID))
+		chrc->ext_props_handle = desc->handle;
+
 	return desc;
 }
 
@@ -521,12 +537,12 @@ static struct {
 	{ BT_GATT_CHRC_PROP_INDICATE,		"indicate" },
 	{ BT_GATT_CHRC_PROP_AUTH,		"authenticated-signed-writes" },
 	{ BT_GATT_CHRC_PROP_EXT_PROP,		"extended-properties" },
-	{ },
 	/* Extended Properties */
 	{ BT_GATT_CHRC_EXT_PROP_RELIABLE_WRITE,	"reliable-write" },
 	{ BT_GATT_CHRC_EXT_PROP_WRITABLE_AUX,	"writable-auxiliaries" },
 	{ }
 };
+static const int ext_props_index = 8;
 
 static gboolean characteristic_property_get_flags(
 					const GDBusPropertyTable *property,
@@ -534,21 +550,18 @@ static gboolean characteristic_property_get_flags(
 {
 	struct characteristic *chrc = data;
 	DBusMessageIter array;
+	uint16_t props;
 	int i;
 
 	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, "s", &array);
 
 	for (i = 0; properties[i].str; i++) {
-		if (chrc->props & properties[i].prop)
+		props = i < ext_props_index ? chrc->props : chrc->ext_props;
+		if (props & properties[i].prop)
 			dbus_message_iter_append_basic(&array, DBUS_TYPE_STRING,
 							&properties[i].str);
 	}
 
-	/*
-	 * TODO: Handle extended properties if the descriptor is
-	 * present.
-	 */
-
 	dbus_message_iter_close_container(iter, &array);
 
 	return TRUE;
@@ -1080,6 +1093,7 @@ static void service_free(void *data)
 	struct service *service = data;
 
 	queue_destroy(service->chrcs, NULL);  /* List should be empty here */
+	queue_destroy(service->pending_ext_props, NULL);
 	g_free(service->path);
 	free(service);
 }
@@ -1101,6 +1115,13 @@ static struct service *service_create(struct gatt_db_attribute *attr,
 		return NULL;
 	}
 
+	service->pending_ext_props = queue_new();
+	if (!service->pending_ext_props) {
+		queue_destroy(service->chrcs, NULL);
+		free(service);
+		return NULL;
+	}
+
 	service->client = client;
 
 	gatt_db_attribute_get_service_data(attr, &service->start_handle,
@@ -1137,12 +1158,24 @@ static void unregister_service(void *data)
 
 	DBG("Removing GATT service: %s", service->path);
 
+	if (service->idle_id)
+		g_source_remove(service->idle_id);
+
 	queue_remove_all(service->chrcs, NULL, NULL, unregister_characteristic);
 
 	g_dbus_unregister_interface(btd_get_dbus_connection(), service->path,
 							GATT_SERVICE_IFACE);
 }
 
+static void notify_chrcs(struct service *service)
+{
+	service->chrcs_ready = true;
+
+	g_dbus_emit_property_changed(btd_get_dbus_connection(), service->path,
+							GATT_SERVICE_IFACE,
+							"Characteristics");
+}
+
 struct export_data {
 	void *root;
 	bool failed;
@@ -1166,6 +1199,46 @@ static void export_desc(struct gatt_db_attribute *attr, void *user_data)
 	queue_push_tail(charac->descs, desc);
 }
 
+static void read_ext_props_cb(bool success, uint8_t att_ecode,
+					const uint8_t *value, uint16_t length,
+					void *user_data)
+{
+	struct characteristic *chrc = user_data;
+	struct service *service = chrc->service;
+
+	if (!success) {
+		error("Failed to obtain extended properties - error: 0x%02x",
+								att_ecode);
+		return;
+	}
+
+	if (!value || length != 2) {
+		error("Malformed extended properties value");
+		return;
+	}
+
+	chrc->ext_props = get_le16(value);
+	if (chrc->ext_props)
+		g_dbus_emit_property_changed(btd_get_dbus_connection(),
+						service->path,
+						GATT_SERVICE_IFACE, "Flags");
+
+	queue_remove(service->pending_ext_props, chrc);
+
+	if (queue_isempty(service->pending_ext_props))
+		notify_chrcs(service);
+}
+
+static void read_ext_props(void *data, void *user_data)
+{
+	struct characteristic *chrc = data;
+
+	bt_gatt_client_read_value(chrc->service->client->gatt,
+							chrc->ext_props_handle,
+							read_ext_props_cb,
+							chrc, NULL);
+}
+
 static bool create_descriptors(struct gatt_db_attribute *attr,
 					struct characteristic *charac)
 {
@@ -1199,6 +1272,9 @@ static void export_char(struct gatt_db_attribute *attr, void *user_data)
 
 	queue_push_tail(service->chrcs, charac);
 
+	if (charac->ext_props_handle)
+		queue_push_tail(service->pending_ext_props, charac);
+
 	return;
 
 fail:
@@ -1215,28 +1291,20 @@ static bool create_characteristics(struct gatt_db_attribute *attr,
 
 	gatt_db_service_foreach_char(attr, export_char, &data);
 
-	return !data.failed;
-}
-
-static void notify_chrcs(void *data, void *user_data)
-{
-	struct service *service = data;
+	if (data.failed)
+		return false;
 
-	service->chrcs_ready = true;
+	/* Obtain extended properties */
+	queue_foreach(service->pending_ext_props, read_ext_props, NULL);
 
-	g_dbus_emit_property_changed(btd_get_dbus_connection(), service->path,
-							GATT_SERVICE_IFACE,
-							"Characteristics");
+	return true;
 }
 
 static gboolean set_chrcs_ready(gpointer user_data)
 {
-	struct btd_gatt_client *client = user_data;
-
-	if (!client->gatt)
-		return FALSE;
+	struct service *service = user_data;
 
-	queue_foreach(client->services, notify_chrcs, NULL);
+	notify_chrcs(service);
 
 	return FALSE;
 }
@@ -1257,6 +1325,14 @@ static void export_service(struct gatt_db_attribute *attr, void *user_data)
 	}
 
 	queue_push_tail(client->services, service);
+
+	/*
+	 * Asynchronously update the "Characteristics" property of the service.
+	 * If there are any pending reads to obtain the value of the "Extended
+	 * Properties" descriptor then wait until they are complete.
+	 */
+	if (!service->chrcs_ready && queue_isempty(service->pending_ext_props))
+		service->idle_id = g_idle_add(set_chrcs_ready, service);
 }
 
 static void create_services(struct btd_gatt_client *client)
@@ -1264,13 +1340,6 @@ static void create_services(struct btd_gatt_client *client)
 	DBG("Exporting objects for GATT services: %s", client->devaddr);
 
 	gatt_db_foreach_service(client->db, NULL, export_service, client);
-
-	/*
-	 * Asynchronously update the "Characteristics" property of each service.
-	 * We do this so that users have a way to know that all characteristics
-	 * of a service have been exported.
-	 */
-	g_idle_add(set_chrcs_ready, client);
 }
 
 struct btd_gatt_client *btd_gatt_client_new(struct btd_device *device)
-- 
2.2.0.rc0.207.ga3a616c

[PATCH BlueZ 10/17] shared/gatt-client: Expose MTU.

[Arman Uguray]

This patch adds the bt_gatt_client_get_mtu function. It also includes a
minor fix in which the signature of the "value" argument of write
methods are changed from "uint8_t *" to "const uint8_t *".
---
 src/shared/gatt-client.c | 14 +++++++++++---
 src/shared/gatt-client.h |  8 +++++---
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index f7a90d1..6768892 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -1599,6 +1599,14 @@ bool bt_gatt_client_set_debug(struct bt_gatt_client *client,
 	return true;
 }
 
+uint16_t bt_gatt_client_get_mtu(struct bt_gatt_client *client)
+{
+	if (!client || !client->att)
+		return 0;
+
+	return bt_att_get_mtu(client->att);
+}
+
 struct read_op {
 	bt_gatt_client_read_callback_t callback;
 	void *user_data;
@@ -1955,7 +1963,7 @@ bool bt_gatt_client_read_long_value(struct bt_gatt_client *client,
 bool bt_gatt_client_write_without_response(struct bt_gatt_client *client,
 					uint16_t value_handle,
 					bool signed_write,
-					uint8_t *value, uint16_t length) {
+					const uint8_t *value, uint16_t length) {
 	uint8_t pdu[2 + length];
 
 	if (!client)
@@ -2011,7 +2019,7 @@ done:
 
 bool bt_gatt_client_write_value(struct bt_gatt_client *client,
 					uint16_t value_handle,
-					uint8_t *value, uint16_t length,
+					const uint8_t *value, uint16_t length,
 					bt_gatt_client_callback_t callback,
 					void *user_data,
 					bt_gatt_client_destroy_func_t destroy)
@@ -2260,7 +2268,7 @@ done:
 bool bt_gatt_client_write_long_value(struct bt_gatt_client *client,
 				bool reliable,
 				uint16_t value_handle, uint16_t offset,
-				uint8_t *value, uint16_t length,
+				const uint8_t *value, uint16_t length,
 				bt_gatt_client_write_long_callback_t callback,
 				void *user_data,
 				bt_gatt_client_destroy_func_t destroy)
diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
index 984c23f..42eeaec 100644
--- a/src/shared/gatt-client.h
+++ b/src/shared/gatt-client.h
@@ -70,6 +70,8 @@ bool bt_gatt_client_set_debug(struct bt_gatt_client *client,
 					void *user_data,
 					bt_gatt_client_destroy_func_t destroy);
 
+uint16_t bt_gatt_client_get_mtu(struct bt_gatt_client *client);
+
 bool bt_gatt_client_read_value(struct bt_gatt_client *client,
 					uint16_t value_handle,
 					bt_gatt_client_read_callback_t callback,
@@ -89,17 +91,17 @@ bool bt_gatt_client_read_multiple(struct bt_gatt_client *client,
 bool bt_gatt_client_write_without_response(struct bt_gatt_client *client,
 					uint16_t value_handle,
 					bool signed_write,
-					uint8_t *value, uint16_t length);
+					const uint8_t *value, uint16_t length);
 bool bt_gatt_client_write_value(struct bt_gatt_client *client,
 					uint16_t value_handle,
-					uint8_t *value, uint16_t length,
+					const uint8_t *value, uint16_t length,
 					bt_gatt_client_callback_t callback,
 					void *user_data,
 					bt_gatt_client_destroy_func_t destroy);
 bool bt_gatt_client_write_long_value(struct bt_gatt_client *client,
 				bool reliable,
 				uint16_t value_handle, uint16_t offset,
-				uint8_t *value, uint16_t length,
+				const uint8_t *value, uint16_t length,
 				bt_gatt_client_write_long_callback_t callback,
 				void *user_data,
 				bt_gatt_client_destroy_func_t destroy);
-- 
2.2.0.rc0.207.ga3a616c

[PATCH BlueZ 11/17] core: gatt: Implement GattCharacteristic1.WriteValue.

[Arman Uguray]

This patch implements the WriteValue method of
org.bluez.GattCharacteristic1. The GATT write procedure that should be
used is decided based on characteristic properties.
---
 src/gatt-client.c | 185 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 183 insertions(+), 2 deletions(-)

diff --git a/src/gatt-client.c b/src/gatt-client.c
index e00f432..4c0900b 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -79,6 +79,7 @@ struct characteristic {
 	char *path;
 
 	bool in_read;
+	bool in_write;
 	bool value_known;
 	uint8_t *value;
 	size_t value_len;
@@ -287,6 +288,35 @@ notify:
 								"Value");
 }
 
+static bool parse_value_arg(DBusMessage *msg, uint8_t **value,
+							size_t *value_len)
+{
+	DBusMessageIter iter, array;
+	uint8_t *val;
+	int len;
+
+	if (!dbus_message_iter_init(msg, &iter))
+		return false;
+
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_ARRAY)
+		return false;
+
+	dbus_message_iter_recurse(&iter, &array);
+	dbus_message_iter_get_fixed_array(&array, &val, &len);
+	dbus_message_iter_next(&iter);
+
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_INVALID)
+		return false;
+
+	if (len < 0)
+		return false;
+
+	*value = val;
+	*value_len = len;
+
+	return true;
+}
+
 struct async_dbus_op {
 	DBusMessage *msg;
 	void *data;
@@ -378,6 +408,95 @@ static DBusMessage *descriptor_read_value(DBusConnection *conn,
 	return btd_error_failed(msg, "Failed to send read request");
 }
 
+static void write_result_cb(bool success, bool reliable_error,
+					uint8_t att_ecode, void *user_data)
+{
+	struct async_dbus_op *op = user_data;
+	DBusMessage *reply;
+
+	if (op->data) {
+		struct characteristic *chrc;
+
+		chrc = op->data;
+		chrc->in_write = false;
+	}
+
+	if (!success) {
+		if (reliable_error)
+			reply = btd_error_failed(op->msg,
+						"Reliable write failed");
+		else
+			reply = create_gatt_dbus_error(op->msg, att_ecode);
+
+		goto done;
+	}
+
+	reply = g_dbus_create_reply(op->msg, DBUS_TYPE_INVALID);
+	if (!reply) {
+		error("Failed to allocate D-Bus message reply");
+		return;
+	}
+
+done:
+	g_dbus_send_message(btd_get_dbus_connection(), reply);
+}
+
+
+static void write_cb(bool success, uint8_t att_ecode, void *user_data)
+{
+	write_result_cb(success, false, att_ecode, user_data);
+}
+
+
+static bool start_long_write(DBusMessage *msg, uint16_t handle,
+					struct bt_gatt_client *gatt,
+					bool reliable, const uint8_t *value,
+					size_t value_len, void *data)
+{
+	struct async_dbus_op *op;
+
+	op = new0(struct async_dbus_op, 1);
+	if (!op)
+		return false;
+
+	op->msg = dbus_message_ref(msg);
+	op->data = data;
+
+	if (bt_gatt_client_write_long_value(gatt, reliable, handle,
+							0, value, value_len,
+							write_result_cb, op,
+							async_dbus_op_free))
+		return true;
+
+	async_dbus_op_free(op);
+
+	return false;
+}
+
+static bool start_write_request(DBusMessage *msg, uint16_t handle,
+					struct bt_gatt_client *gatt,
+					const uint8_t *value, size_t value_len,
+					void *data)
+{
+	struct async_dbus_op *op;
+
+	op = new0(struct async_dbus_op, 1);
+	if (!op)
+		return false;
+
+	op->msg = dbus_message_ref(msg);
+	op->data = data;
+
+	if (bt_gatt_client_write_value(gatt, handle, value, value_len,
+							write_cb, op,
+							async_dbus_op_free))
+		return true;
+
+	async_dbus_op_free(op);
+
+	return false;
+}
+
 static DBusMessage *descriptor_write_value(DBusConnection *conn,
 					DBusMessage *msg, void *user_data)
 {
@@ -631,8 +750,70 @@ static DBusMessage *characteristic_read_value(DBusConnection *conn,
 static DBusMessage *characteristic_write_value(DBusConnection *conn,
 					DBusMessage *msg, void *user_data)
 {
-	/* TODO: Implement */
-	return btd_error_failed(msg, "Not implemented");
+	struct characteristic *chrc = user_data;
+	struct bt_gatt_client *gatt = chrc->service->client->gatt;
+	uint8_t *value = NULL;
+	size_t value_len = 0;
+
+	if (chrc->in_write)
+		return btd_error_in_progress(msg);
+
+	if (!parse_value_arg(msg, &value, &value_len))
+		return btd_error_invalid_args(msg);
+
+	if (!(chrc->props & (BT_GATT_CHRC_PROP_WRITE |
+					BT_GATT_CHRC_PROP_WRITE_WITHOUT_RESP)))
+		if (!(chrc->ext_props & BT_GATT_CHRC_EXT_PROP_RELIABLE_WRITE))
+			return btd_error_not_supported(msg);
+
+	/*
+	 * Decide which write to use based on characteristic properties. For now
+	 * we don't perform signed writes since gatt-client doesn't support them
+	 * and the user can always encrypt the through pairing. The procedure to
+	 * use is determined based on the following priority:
+	 *
+	 *   * "reliable-write" property set -> reliable long-write.
+	 *   * "write" property set -> write request.
+	 *     - If value is larger than MTU - 3: long-write
+	 *   * "write-without-response" property set -> write command.
+	 */
+	if ((chrc->ext_props & BT_GATT_CHRC_EXT_PROP_RELIABLE_WRITE) &&
+			start_long_write(msg, chrc->value_handle, gatt, true,
+							value, value_len, chrc))
+		goto done_async;
+
+	if (chrc->props & BT_GATT_CHRC_PROP_WRITE) {
+		uint16_t mtu;
+		bool result;
+
+		mtu = bt_gatt_client_get_mtu(gatt);
+		if (!mtu)
+			return btd_error_failed(msg, "No ATT transport");
+
+		if (value_len <= (unsigned) mtu - 3)
+			result = start_write_request(msg, chrc->value_handle,
+							gatt, value,
+							value_len, chrc);
+		else
+			result = start_long_write(msg, chrc->value_handle, gatt,
+						false, value, value_len, chrc);
+
+		if (result)
+			goto done_async;
+	}
+
+	if ((chrc->props & BT_GATT_CHRC_PROP_WRITE_WITHOUT_RESP) &&
+			bt_gatt_client_write_without_response(gatt,
+							chrc->value_handle,
+							false, value,
+							value_len))
+		return dbus_message_new_method_return(msg);
+
+	return btd_error_failed(msg, "Failed to initiate write");
+
+done_async:
+	chrc->in_write = true;
+	return NULL;
 }
 
 struct notify_client {
-- 
2.2.0.rc0.207.ga3a616c

[PATCH BlueZ 12/17] core: gatt: Don't always issue read-long for ReadValue

[Arman Uguray]

A remote GATT server may not support the "long write" procedure for all
of its attributes (or not at all). This patch revises the implementation
of GattCharacteristic1.ReadValue and GattDescriptor1.ReadValue, so that
instead of always initiating a "long write" (i.e. ATT Read Blob Req),
they first send out a regular read request and then proceed to a long
write only if the value fills the current MTU and may potentially have
more bytes waiting.
---
 src/gatt-client.c | 215 +++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 172 insertions(+), 43 deletions(-)

diff --git a/src/gatt-client.c b/src/gatt-client.c
index 4c0900b..d350b1b 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -348,24 +348,51 @@ static void message_append_byte_array(DBusMessage *msg, const uint8_t *bytes,
 	dbus_message_iter_close_container(&iter, &array);
 }
 
-static void desc_read_long_cb(bool success, uint8_t att_ecode,
-					const uint8_t *value, uint16_t length,
-					void *user_data)
+struct async_read_op {
+	DBusMessage *msg;
+	int ref_count;
+	uint8_t *value;
+	size_t len;
+	struct bt_gatt_client *gatt;
+	uint16_t handle;
+	void *data;
+	bool (*complete)(const uint8_t *value, size_t len, void *data);
+};
+
+static struct async_read_op *async_read_op_ref(struct async_read_op *op)
 {
-	struct async_dbus_op *op = user_data;
-	struct descriptor *desc = op->data;
-	DBusMessage *reply;
+	__sync_fetch_and_add(&op->ref_count, 1);
 
-	desc->in_read = false;
+	return op;
+}
+
+static void async_read_op_unref(void *data)
+{
+	struct async_read_op *op = data;
+
+	if (__sync_sub_and_fetch(&op->ref_count, 1))
+		return;
+
+	dbus_message_unref(op->msg);
+	free(op->value);
+	free(op);
+}
+
+static void complete_read(struct async_read_op *op, bool success,
+					uint8_t att_ecode, const uint8_t *value,
+					uint16_t length)
+{
+	DBusMessage *reply;
 
 	if (!success) {
 		reply = create_gatt_dbus_error(op->msg, att_ecode);
 		goto done;
 	}
 
-	update_value_property(value, length, &desc->value, &desc->value_len,
-						&desc->value_known, desc->path,
-						GATT_DESCRIPTOR_IFACE, false);
+	if (!op->complete(value, length, op->data)) {
+		reply = btd_error_failed(op->msg, "Operation failed");
+		goto done;
+	}
 
 	reply = g_dbus_create_reply(op->msg, DBUS_TYPE_INVALID);
 	if (!reply) {
@@ -379,31 +406,133 @@ done:
 	g_dbus_send_message(btd_get_dbus_connection(), reply);
 }
 
+static void read_long_cb(bool success, uint8_t att_ecode,
+					const uint8_t *value, uint16_t length,
+					void *user_data)
+{
+	struct async_read_op *op = user_data;
+	uint8_t *final_value = NULL;
+	size_t final_len = 0;
+
+	if (!success) {
+		if (att_ecode != BT_ATT_ERROR_REQUEST_NOT_SUPPORTED &&
+				att_ecode != BT_ATT_ERROR_ATTRIBUTE_NOT_LONG &&
+				att_ecode != BT_ATT_ERROR_INVALID_OFFSET)
+			goto done;
+
+		success = true;
+		att_ecode = 0;
+	}
+
+	if (length == 0) {
+		final_len = op->len;
+		final_value = op->value;
+	} else {
+		final_len = op->len + length;
+		final_value = malloc(sizeof(uint8_t) * (op->len + length));
+		if (!final_value) {
+			success = false;
+			goto done;
+		}
+
+		memcpy(final_value, op->value, op->len);
+		memcpy(final_value + op->len, value, length);
+	}
+
+done:
+	complete_read(op, success, att_ecode, final_value, final_len);
+
+	if (final_value && final_value != op->value)
+		free(final_value);
+}
+
+static void read_cb(bool success, uint8_t att_ecode,
+					const uint8_t *value, uint16_t length,
+					void *user_data)
+{
+	struct async_read_op *op = user_data;
+
+	if (!success)
+		goto done;
+
+	/*
+	 * If the value length is exactly MTU-1, then we may not have read the
+	 * entire value. Perform a long read to obtain the rest, otherwise,
+	 * we're done.
+	 */
+	if (length < bt_gatt_client_get_mtu(op->gatt) - 1)
+		goto done;
+
+	op->value = malloc(sizeof(uint8_t) * length);
+	if (!op->value) {
+		success = false;
+		goto done;
+	}
+
+	memcpy(op->value, value, sizeof(uint8_t) * length);
+	op->len = length;
+
+	if (bt_gatt_client_read_long_value(op->gatt, op->handle, length,
+							read_long_cb,
+							async_read_op_ref(op),
+							async_read_op_unref))
+		return;
+
+	async_read_op_unref(op);
+	success = false;
+
+done:
+	complete_read(op, success, att_ecode, value, length);
+}
+
+static bool desc_read_complete(const uint8_t *value, size_t len, void *data)
+{
+	struct descriptor *desc = data;
+
+	desc->in_read = false;
+
+	/*
+	 * The descriptor might have been unregistered during the read. Return
+	 * failure.
+	 */
+	if (!desc->chrc)
+		return false;
+
+	update_value_property(value, len, &desc->value, &desc->value_len,
+						&desc->value_known, desc->path,
+						GATT_DESCRIPTOR_IFACE, false);
+
+	return true;
+}
+
 static DBusMessage *descriptor_read_value(DBusConnection *conn,
 					DBusMessage *msg, void *user_data)
 {
 	struct descriptor *desc = user_data;
 	struct bt_gatt_client *gatt = desc->chrc->service->client->gatt;
-	struct async_dbus_op *op;
+	struct async_read_op *op;
 
 	if (desc->in_read)
 		return btd_error_in_progress(msg);
 
-	op = new0(struct async_dbus_op, 1);
+	op = new0(struct async_read_op, 1);
 	if (!op)
 		return btd_error_failed(msg, "Failed to initialize request");
 
 	op->msg = dbus_message_ref(msg);
+	op->gatt = gatt;
+	op->handle = desc->handle;
 	op->data = desc;
+	op->complete = desc_read_complete;
 
-	if (bt_gatt_client_read_long_value(gatt, desc->handle, 0,
-							desc_read_long_cb, op,
-							async_dbus_op_free)) {
+	if (bt_gatt_client_read_value(gatt, desc->handle, read_cb,
+							async_read_op_ref(op),
+							async_read_op_unref)) {
 		desc->in_read = true;
 		return NULL;
 	}
 
-	async_dbus_op_free(op);
+	async_read_op_unref(op);
 
 	return btd_error_failed(msg, "Failed to send read request");
 }
@@ -686,36 +815,25 @@ static gboolean characteristic_property_get_flags(
 	return TRUE;
 }
 
-static void chrc_read_long_cb(bool success, uint8_t att_ecode,
-					const uint8_t *value, uint16_t length,
-					void *user_data)
+static bool chrc_read_complete(const uint8_t *value, size_t len, void *data)
 {
-	struct async_dbus_op *op = user_data;
-	struct characteristic *chrc = op->data;
-	DBusMessage *reply;
+	struct characteristic *chrc = data;
 
 	chrc->in_read = false;
 
-	if (!success) {
-		reply = create_gatt_dbus_error(op->msg, att_ecode);
-		goto done;
-	}
+	/*
+	 * The characteristic might have been unregistered during the read.
+	 * Return failure.
+	 */
+	if (!chrc->service)
+		return false;
 
-	update_value_property(value, length, &chrc->value, &chrc->value_len,
+	update_value_property(value, len, &chrc->value, &chrc->value_len,
 						&chrc->value_known, chrc->path,
 						GATT_CHARACTERISTIC_IFACE,
 						false);
 
-	reply = g_dbus_create_reply(op->msg, DBUS_TYPE_INVALID);
-	if (!reply) {
-		error("Failed to allocate D-Bus message reply");
-		return;
-	}
-
-	message_append_byte_array(reply, value, length);
-
-done:
-	g_dbus_send_message(btd_get_dbus_connection(), reply);
+	return true;
 }
 
 static DBusMessage *characteristic_read_value(DBusConnection *conn,
@@ -723,26 +841,37 @@ static DBusMessage *characteristic_read_value(DBusConnection *conn,
 {
 	struct characteristic *chrc = user_data;
 	struct bt_gatt_client *gatt = chrc->service->client->gatt;
-	struct async_dbus_op *op;
+	struct async_read_op *op;
 
 	if (chrc->in_read)
 		return btd_error_in_progress(msg);
 
-	op = new0(struct async_dbus_op, 1);
+	if (!(chrc->props & BT_GATT_CHRC_PROP_READ))
+		return gatt_error_read_not_permitted(msg);
+
+	op = new0(struct async_read_op, 1);
 	if (!op)
 		return btd_error_failed(msg, "Failed to initialize request");
 
 	op->msg = dbus_message_ref(msg);
+	op->gatt = gatt;
+	op->handle = chrc->value_handle;
 	op->data = chrc;
+	op->complete = chrc_read_complete;
 
-	if (bt_gatt_client_read_long_value(gatt, chrc->value_handle, 0,
-							chrc_read_long_cb, op,
-							async_dbus_op_free)) {
+	/*
+	 * A remote server may support the "read" procedure but may not support
+	 * a "long read". Hence, we start with the regular read procedure and
+	 * proceed to a long read only if necessary.
+	 */
+	if (bt_gatt_client_read_value(gatt, chrc->value_handle, read_cb,
+							async_read_op_ref(op),
+							async_read_op_unref)) {
 		chrc->in_read = true;
 		return NULL;
 	}
 
-	async_dbus_op_free(op);
+	async_read_op_unref(op);
 
 	return btd_error_failed(msg, "Failed to send read request");
 }
-- 
2.2.0.rc0.207.ga3a616c

[PATCH BlueZ 13/17] core: gatt: Implement GattDescriptor1.WriteValue.

[Arman Uguray]

This patch implements the WriteValue method of org.bluez.GattDescriptor1
---
 src/gatt-client.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 85 insertions(+), 13 deletions(-)

diff --git a/src/gatt-client.c b/src/gatt-client.c
index d350b1b..ee0a9c9 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -97,6 +97,7 @@ struct descriptor {
 	char *path;
 
 	bool in_read;
+	bool in_write;
 	bool value_known;
 	uint8_t *value;
 	size_t value_len;
@@ -317,9 +318,12 @@ static bool parse_value_arg(DBusMessage *msg, uint8_t **value,
 	return true;
 }
 
+typedef bool (*async_dbus_op_complete_t)(void *data);
+
 struct async_dbus_op {
 	DBusMessage *msg;
 	void *data;
+	async_dbus_op_complete_t complete;
 };
 
 static void async_dbus_op_free(void *data)
@@ -543,11 +547,9 @@ static void write_result_cb(bool success, bool reliable_error,
 	struct async_dbus_op *op = user_data;
 	DBusMessage *reply;
 
-	if (op->data) {
-		struct characteristic *chrc;
-
-		chrc = op->data;
-		chrc->in_write = false;
+	if (op->complete && !op->complete(op->data)) {
+		reply = btd_error_failed(op->msg, "Operation failed");
+		goto done;
 	}
 
 	if (!success) {
@@ -576,11 +578,11 @@ static void write_cb(bool success, uint8_t att_ecode, void *user_data)
 	write_result_cb(success, false, att_ecode, user_data);
 }
 
-
 static bool start_long_write(DBusMessage *msg, uint16_t handle,
 					struct bt_gatt_client *gatt,
 					bool reliable, const uint8_t *value,
-					size_t value_len, void *data)
+					size_t value_len, void *data,
+					async_dbus_op_complete_t complete)
 {
 	struct async_dbus_op *op;
 
@@ -590,6 +592,7 @@ static bool start_long_write(DBusMessage *msg, uint16_t handle,
 
 	op->msg = dbus_message_ref(msg);
 	op->data = data;
+	op->complete = complete;
 
 	if (bt_gatt_client_write_long_value(gatt, reliable, handle,
 							0, value, value_len,
@@ -605,7 +608,8 @@ static bool start_long_write(DBusMessage *msg, uint16_t handle,
 static bool start_write_request(DBusMessage *msg, uint16_t handle,
 					struct bt_gatt_client *gatt,
 					const uint8_t *value, size_t value_len,
-					void *data)
+					void *data,
+					async_dbus_op_complete_t complete)
 {
 	struct async_dbus_op *op;
 
@@ -615,6 +619,7 @@ static bool start_write_request(DBusMessage *msg, uint16_t handle,
 
 	op->msg = dbus_message_ref(msg);
 	op->data = data;
+	op->complete = complete;
 
 	if (bt_gatt_client_write_value(gatt, handle, value, value_len,
 							write_cb, op,
@@ -626,11 +631,61 @@ static bool start_write_request(DBusMessage *msg, uint16_t handle,
 	return false;
 }
 
+static bool desc_write_complete(void *data)
+{
+	struct descriptor *desc = data;
+
+	desc->in_write = false;
+
+	/*
+	 * The descriptor might have been unregistered during the read. Return
+	 * failure.
+	 */
+	return !!desc->chrc;
+}
+
 static DBusMessage *descriptor_write_value(DBusConnection *conn,
 					DBusMessage *msg, void *user_data)
 {
-	/* TODO: Implement */
-	return btd_error_failed(msg, "Not implemented");
+	struct descriptor *desc = user_data;
+	struct bt_gatt_client *gatt = desc->chrc->service->client->gatt;
+	uint8_t *value = NULL;
+	size_t value_len = 0;
+	bool result;
+
+	if (desc->in_write)
+		return btd_error_in_progress(msg);
+
+	if (!parse_value_arg(msg, &value, &value_len))
+		return btd_error_invalid_args(msg);
+
+	/*
+	 * Don't allow writing to Client Characteristic Configuration
+	 * descriptors. We achieve this through the StartNotify and StopNotify
+	 * methods on GattCharacteristic1.
+	 */
+	if (uuid_cmp(&desc->uuid, GATT_CLIENT_CHARAC_CFG_UUID))
+		return gatt_error_write_not_permitted(msg);
+
+	/*
+	 * Based on the value length and the MTU, either use a write or a long
+	 * write.
+	 */
+	if (value_len <= (unsigned) bt_gatt_client_get_mtu(gatt) - 3)
+		result = start_write_request(msg, desc->handle, gatt, value,
+							value_len, desc,
+							desc_write_complete);
+	else
+		result = start_long_write(msg, desc->handle, gatt, false, value,
+							value_len, desc,
+							desc_write_complete);
+
+	if (!result)
+		return btd_error_failed(msg, "Failed to initiate write");
+
+	desc->in_write = true;
+
+	return NULL;
 }
 
 static const GDBusPropertyTable descriptor_properties[] = {
@@ -876,6 +931,19 @@ static DBusMessage *characteristic_read_value(DBusConnection *conn,
 	return btd_error_failed(msg, "Failed to send read request");
 }
 
+static bool chrc_write_complete(void *data)
+{
+	struct characteristic *chrc = data;
+
+	chrc->in_write = false;
+
+	/*
+	 * The characteristic might have been unregistered during the read.
+	 * Return failure.
+	 */
+	return !!chrc->service;
+}
+
 static DBusMessage *characteristic_write_value(DBusConnection *conn,
 					DBusMessage *msg, void *user_data)
 {
@@ -908,7 +976,8 @@ static DBusMessage *characteristic_write_value(DBusConnection *conn,
 	 */
 	if ((chrc->ext_props & BT_GATT_CHRC_EXT_PROP_RELIABLE_WRITE) &&
 			start_long_write(msg, chrc->value_handle, gatt, true,
-							value, value_len, chrc))
+							value, value_len, chrc,
+							chrc_write_complete))
 		goto done_async;
 
 	if (chrc->props & BT_GATT_CHRC_PROP_WRITE) {
@@ -922,10 +991,12 @@ static DBusMessage *characteristic_write_value(DBusConnection *conn,
 		if (value_len <= (unsigned) mtu - 3)
 			result = start_write_request(msg, chrc->value_handle,
 							gatt, value,
-							value_len, chrc);
+							value_len, chrc,
+							chrc_write_complete);
 		else
 			result = start_long_write(msg, chrc->value_handle, gatt,
-						false, value, value_len, chrc);
+						false, value, value_len, chrc,
+						chrc_write_complete);
 
 		if (result)
 			goto done_async;
@@ -942,6 +1013,7 @@ static DBusMessage *characteristic_write_value(DBusConnection *conn,
 
 done_async:
 	chrc->in_write = true;
+
 	return NULL;
 }
 
-- 
2.2.0.rc0.207.ga3a616c

[PATCH BlueZ 14/17] core: gatt: Reference count chrcs and descs.

[Arman Uguray]

Many of the GATT D-Bus API methods are asynchronous and their callbacks
depend on characteristic and descriptor instances to be alive when they
execute. This patch makes characteristics and descriptors reference
counted so that an interim disconnect or "Service Changed" won't
immediately free up those instances.
---
 src/gatt-client.c | 193 ++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 135 insertions(+), 58 deletions(-)

diff --git a/src/gatt-client.c b/src/gatt-client.c
index ee0a9c9..a2e73ac 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -78,6 +78,8 @@ struct characteristic {
 	bt_uuid_t uuid;
 	char *path;
 
+	int ref_count;
+
 	bool in_read;
 	bool in_write;
 	bool value_known;
@@ -96,6 +98,8 @@ struct descriptor {
 	bt_uuid_t uuid;
 	char *path;
 
+	int ref_count;
+
 	bool in_read;
 	bool in_write;
 	bool value_known;
@@ -103,6 +107,45 @@ struct descriptor {
 	size_t value_len;
 };
 
+static struct characteristic *characteristic_ref(struct characteristic *chrc)
+{
+	__sync_fetch_and_add(&chrc->ref_count, 1);
+
+	return chrc;
+}
+
+static void characteristic_unref(void *data)
+{
+	struct characteristic *chrc = data;
+
+	if (__sync_sub_and_fetch(&chrc->ref_count, 1))
+		return;
+
+	queue_destroy(chrc->descs, NULL);  /* List should be empty here */
+	free(chrc->value);
+	g_free(chrc->path);
+	free(chrc);
+}
+
+static struct descriptor *descriptor_ref(struct descriptor *desc)
+{
+	__sync_fetch_and_add(&desc->ref_count, 1);
+
+	return desc;
+}
+
+static void descriptor_unref(void *data)
+{
+	struct descriptor *desc = data;
+
+	if (__sync_sub_and_fetch(&desc->ref_count, 1))
+		return;
+
+	free(desc->value);
+	g_free(desc->path);
+	free(desc);
+}
+
 static DBusMessage *gatt_error_read_not_permitted(DBusMessage *msg)
 {
 	return g_dbus_create_error(msg, ERROR_INTERFACE ".ReadNotPermitted",
@@ -319,17 +362,22 @@ static bool parse_value_arg(DBusMessage *msg, uint8_t **value,
 }
 
 typedef bool (*async_dbus_op_complete_t)(void *data);
+typedef void (*async_dbus_op_destroy_t)(void *data);
 
 struct async_dbus_op {
 	DBusMessage *msg;
 	void *data;
 	async_dbus_op_complete_t complete;
+	async_dbus_op_destroy_t destroy;
 };
 
 static void async_dbus_op_free(void *data)
 {
 	struct async_dbus_op *op = data;
 
+	if (op->destroy)
+		op->destroy(op->data);
+
 	if (op->msg)
 		dbus_message_unref(op->msg);
 
@@ -361,6 +409,7 @@ struct async_read_op {
 	uint16_t handle;
 	void *data;
 	bool (*complete)(const uint8_t *value, size_t len, void *data);
+	void (*destroy)(void *data);
 };
 
 static struct async_read_op *async_read_op_ref(struct async_read_op *op)
@@ -377,6 +426,9 @@ static void async_read_op_unref(void *data)
 	if (__sync_sub_and_fetch(&op->ref_count, 1))
 		return;
 
+	if (op->destroy)
+		op->destroy(op->data);
+
 	dbus_message_unref(op->msg);
 	free(op->value);
 	free(op);
@@ -526,8 +578,9 @@ static DBusMessage *descriptor_read_value(DBusConnection *conn,
 	op->msg = dbus_message_ref(msg);
 	op->gatt = gatt;
 	op->handle = desc->handle;
-	op->data = desc;
+	op->data = descriptor_ref(desc);
 	op->complete = desc_read_complete;
+	op->destroy = descriptor_unref;
 
 	if (bt_gatt_client_read_value(gatt, desc->handle, read_cb,
 							async_read_op_ref(op),
@@ -582,7 +635,8 @@ static bool start_long_write(DBusMessage *msg, uint16_t handle,
 					struct bt_gatt_client *gatt,
 					bool reliable, const uint8_t *value,
 					size_t value_len, void *data,
-					async_dbus_op_complete_t complete)
+					async_dbus_op_complete_t complete,
+					async_dbus_op_destroy_t destroy)
 {
 	struct async_dbus_op *op;
 
@@ -593,6 +647,7 @@ static bool start_long_write(DBusMessage *msg, uint16_t handle,
 	op->msg = dbus_message_ref(msg);
 	op->data = data;
 	op->complete = complete;
+	op->destroy = destroy;
 
 	if (bt_gatt_client_write_long_value(gatt, reliable, handle,
 							0, value, value_len,
@@ -609,7 +664,8 @@ static bool start_write_request(DBusMessage *msg, uint16_t handle,
 					struct bt_gatt_client *gatt,
 					const uint8_t *value, size_t value_len,
 					void *data,
-					async_dbus_op_complete_t complete)
+					async_dbus_op_complete_t complete,
+					async_dbus_op_destroy_t destroy)
 {
 	struct async_dbus_op *op;
 
@@ -620,6 +676,7 @@ static bool start_write_request(DBusMessage *msg, uint16_t handle,
 	op->msg = dbus_message_ref(msg);
 	op->data = data;
 	op->complete = complete;
+	op->destroy = destroy;
 
 	if (bt_gatt_client_write_value(gatt, handle, value, value_len,
 							write_cb, op,
@@ -674,11 +731,13 @@ static DBusMessage *descriptor_write_value(DBusConnection *conn,
 	if (value_len <= (unsigned) bt_gatt_client_get_mtu(gatt) - 3)
 		result = start_write_request(msg, desc->handle, gatt, value,
 							value_len, desc,
-							desc_write_complete);
+							desc_write_complete,
+							descriptor_unref);
 	else
 		result = start_long_write(msg, desc->handle, gatt, false, value,
 							value_len, desc,
-							desc_write_complete);
+							desc_write_complete,
+							descriptor_unref);
 
 	if (!result)
 		return btd_error_failed(msg, "Failed to initiate write");
@@ -704,15 +763,6 @@ static const GDBusMethodTable descriptor_methods[] = {
 	{ }
 };
 
-static void descriptor_free(void *data)
-{
-	struct descriptor *desc = data;
-
-	free(desc->value);
-	g_free(desc->path);
-	free(desc);
-}
-
 static struct descriptor *descriptor_create(struct gatt_db_attribute *attr,
 						struct characteristic *chrc)
 {
@@ -733,10 +783,11 @@ static struct descriptor *descriptor_create(struct gatt_db_attribute *attr,
 						GATT_DESCRIPTOR_IFACE,
 						descriptor_methods, NULL,
 						descriptor_properties,
-						desc, descriptor_free)) {
+						descriptor_ref(desc),
+						descriptor_unref)) {
 		error("Unable to register GATT descriptor with handle 0x%04x",
 								desc->handle);
-		descriptor_free(desc);
+		descriptor_unref(desc);
 
 		return NULL;
 	}
@@ -755,6 +806,8 @@ static void unregister_descriptor(void *data)
 
 	DBG("Removing GATT descriptor: %s", desc->path);
 
+	desc->chrc = NULL;
+
 	g_dbus_unregister_interface(btd_get_dbus_connection(), desc->path,
 							GATT_DESCRIPTOR_IFACE);
 }
@@ -911,8 +964,9 @@ static DBusMessage *characteristic_read_value(DBusConnection *conn,
 	op->msg = dbus_message_ref(msg);
 	op->gatt = gatt;
 	op->handle = chrc->value_handle;
-	op->data = chrc;
+	op->data = characteristic_ref(chrc);
 	op->complete = chrc_read_complete;
+	op->destroy = characteristic_unref;
 
 	/*
 	 * A remote server may support the "read" procedure but may not support
@@ -976,8 +1030,10 @@ static DBusMessage *characteristic_write_value(DBusConnection *conn,
 	 */
 	if ((chrc->ext_props & BT_GATT_CHRC_EXT_PROP_RELIABLE_WRITE) &&
 			start_long_write(msg, chrc->value_handle, gatt, true,
-							value, value_len, chrc,
-							chrc_write_complete))
+						value, value_len,
+						characteristic_ref(chrc),
+						chrc_write_complete,
+						characteristic_unref))
 		goto done_async;
 
 	if (chrc->props & BT_GATT_CHRC_PROP_WRITE) {
@@ -990,13 +1046,16 @@ static DBusMessage *characteristic_write_value(DBusConnection *conn,
 
 		if (value_len <= (unsigned) mtu - 3)
 			result = start_write_request(msg, chrc->value_handle,
-							gatt, value,
-							value_len, chrc,
-							chrc_write_complete);
+						gatt, value, value_len,
+						characteristic_ref(chrc),
+						chrc_write_complete,
+						characteristic_unref);
 		else
 			result = start_long_write(msg, chrc->value_handle, gatt,
-						false, value, value_len, chrc,
-						chrc_write_complete);
+						false, value, value_len,
+						characteristic_ref(chrc),
+						chrc_write_complete,
+						characteristic_unref);
 
 		if (result)
 			goto done_async;
@@ -1129,12 +1188,33 @@ static bool match_notify_sender(const void *a, const void *b)
 	return strcmp(client->owner, sender) == 0;
 }
 
+struct register_notify_op {
+	int ref_count;
+	struct notify_client *client;
+	struct characteristic *chrc;
+};
+
+static void register_notify_op_unref(void *data)
+{
+	struct register_notify_op *op = data;
+
+	DBG("Released register_notify_op");
+
+	if (__sync_sub_and_fetch(&op->ref_count, 1))
+		return;
+
+	notify_client_unref(op->client);
+	characteristic_unref(op->chrc);
+
+	free(op);
+}
+
 static void notify_cb(uint16_t value_handle, const uint8_t *value,
 					uint16_t length, void *user_data)
 {
 	struct async_dbus_op *op = user_data;
-	struct notify_client *client = op->data;
-	struct characteristic *chrc = client->chrc;
+	struct register_notify_op *notify_op = op->data;
+	struct characteristic *chrc = notify_op->chrc;
 
 	/*
 	 * Even if the value didn't change, we want to send a PropertiesChanged
@@ -1151,26 +1231,24 @@ static void register_notify_cb(unsigned int id, uint16_t att_ecode,
 								void *user_data)
 {
 	struct async_dbus_op *op = user_data;
-	struct notify_client *client = op->data;
-	struct characteristic *chrc = client->chrc;
+	struct register_notify_op *notify_op = op->data;
+	struct notify_client *client = notify_op->client;
+	struct characteristic *chrc = notify_op->chrc;
 	DBusMessage *reply;
 
-	/* Make sure that an interim disconnect did not remove the client */
-	if (!queue_find(chrc->notify_clients, NULL, client)) {
+	/*
+	 * Make sure that an interim disconnect or "Service Changed" did not
+	 * remove the client
+	 */
+	if (!chrc->service || !queue_find(chrc->notify_clients, NULL, client)) {
 		bt_gatt_client_unregister_notify(chrc->service->client->gatt,
 									id);
-		notify_client_unref(client);
 
 		reply = btd_error_failed(op->msg,
 						"Characteristic not available");
 		goto done;
 	}
 
-	/*
-	 * Drop the reference count that we added when registering the callback.
-	 */
-	notify_client_unref(client);
-
 	if (!id) {
 		queue_remove(chrc->notify_clients, client);
 		notify_client_free(client);
@@ -1209,6 +1287,7 @@ static DBusMessage *characteristic_start_notify(DBusConnection *conn,
 	const char *sender = dbus_message_get_sender(msg);
 	struct async_dbus_op *op;
 	struct notify_client *client;
+	struct register_notify_op *notify_op;
 
 	if (!(chrc->props & BT_GATT_CHRC_PROP_NOTIFY ||
 				chrc->props & BT_GATT_CHRC_PROP_INDICATE))
@@ -1225,20 +1304,28 @@ static DBusMessage *characteristic_start_notify(DBusConnection *conn,
 	if (!client)
 		return btd_error_failed(msg, "Failed allocate notify session");
 
+	notify_op = new0(struct register_notify_op, 1);
+	if (!notify_op) {
+		notify_client_unref(client);
+		return btd_error_failed(msg, "Failed to initialize request");
+	}
+
+	notify_op->ref_count = 1;
+	notify_op->client = client;
+	notify_op->chrc = characteristic_ref(chrc);
+
 	op = new0(struct async_dbus_op, 1);
 	if (!op) {
-		notify_client_unref(client);
+		register_notify_op_unref(notify_op);
 		return btd_error_failed(msg, "Failed to initialize request");
 	}
 
-	/*
-	 * Add to the ref count so that a disconnect during register won't free
-	 * the client instance.
-	 */
-	op->data = notify_client_ref(client);
+	op->data = notify_op;
 	op->msg = dbus_message_ref(msg);
+	op->destroy = register_notify_op_unref;
 
-	queue_push_tail(chrc->notify_clients, client);
+	/* The characteristic owns a reference to the client */
+	queue_push_tail(chrc->notify_clients, notify_client_ref(client));
 
 	if (bt_gatt_client_register_notify(gatt, chrc->value_handle,
 						register_notify_cb, notify_cb,
@@ -1248,9 +1335,6 @@ static DBusMessage *characteristic_start_notify(DBusConnection *conn,
 	queue_remove(chrc->notify_clients, client);
 	async_dbus_op_free(op);
 
-	/* Directly free the client */
-	notify_client_free(client);
-
 	return btd_error_failed(msg, "Failed to register notify session");
 }
 
@@ -1325,16 +1409,6 @@ static const GDBusMethodTable characteristic_methods[] = {
 	{ }
 };
 
-static void characteristic_free(void *data)
-{
-	struct characteristic *chrc = data;
-
-	queue_destroy(chrc->descs, NULL);  /* List should be empty here */
-	free(chrc->value);
-	g_free(chrc->path);
-	free(chrc);
-}
-
 static struct characteristic *characteristic_create(
 						struct gatt_db_attribute *attr,
 						struct service *service)
@@ -1373,10 +1447,11 @@ static struct characteristic *characteristic_create(
 						GATT_CHARACTERISTIC_IFACE,
 						characteristic_methods, NULL,
 						characteristic_properties,
-						chrc, characteristic_free)) {
+						characteristic_ref(chrc),
+						characteristic_unref)) {
 		error("Unable to register GATT characteristic with handle "
 							"0x%04x", chrc->handle);
-		characteristic_free(chrc);
+		characteristic_unref(chrc);
 
 		return NULL;
 	}
@@ -1395,6 +1470,8 @@ static void unregister_characteristic(void *data)
 	queue_remove_all(chrc->notify_clients, NULL, NULL, notify_client_unref);
 	queue_remove_all(chrc->descs, NULL, NULL, unregister_descriptor);
 
+	chrc->service = NULL;
+
 	g_dbus_unregister_interface(btd_get_dbus_connection(), chrc->path,
 						GATT_CHARACTERISTIC_IFACE);
 }
-- 
2.2.0.rc0.207.ga3a616c

[PATCH BlueZ 15/17] core: gatt: Handle Service Changed.

[Arman Uguray]

This patch adds handling for Service Changed events. All exported
objects that match attributes within the changed handle range are
unregistered and new ones get exported based on the newly discovered
services.
---
 src/gatt-client.c | 35 +++++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/src/gatt-client.c b/src/gatt-client.c
index a2e73ac..16a3b3b 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -1628,6 +1628,11 @@ static void unregister_service(void *data)
 
 static void notify_chrcs(struct service *service)
 {
+
+	if (service->chrcs_ready ||
+				!queue_isempty(service->pending_ext_props))
+		return;
+
 	service->chrcs_ready = true;
 
 	g_dbus_emit_property_changed(btd_get_dbus_connection(), service->path,
@@ -1684,8 +1689,7 @@ static void read_ext_props_cb(bool success, uint8_t att_ecode,
 
 	queue_remove(service->pending_ext_props, chrc);
 
-	if (queue_isempty(service->pending_ext_props))
-		notify_chrcs(service);
+	notify_chrcs(service);
 }
 
 static void read_ext_props(void *data, void *user_data)
@@ -1864,13 +1868,36 @@ void btd_gatt_client_ready(struct btd_gatt_client *client)
 void btd_gatt_client_service_added(struct btd_gatt_client *client,
 					struct gatt_db_attribute *attrib)
 {
-	/* TODO */
+	if (!client)
+		return;
+
+	export_service(attrib, client);
+}
+
+static bool match_service_handle(const void *a, const void *b)
+{
+	const struct service *service = a;
+	uint16_t start_handle = PTR_TO_UINT(b);
+
+	return service->start_handle == start_handle;
 }
 
 void btd_gatt_client_service_removed(struct btd_gatt_client *client,
 					struct gatt_db_attribute *attrib)
 {
-	/* TODO */
+	uint16_t start_handle, end_handle;
+
+	if (!client || !attrib)
+		return;
+
+	gatt_db_attribute_get_service_handles(attrib, &start_handle,
+								&end_handle);
+
+	DBG("GATT Services Removed - start: 0x%04x, end: 0x%04x", start_handle,
+								end_handle);
+	queue_remove_all(client->services, match_service_handle,
+						UINT_TO_PTR(start_handle),
+						unregister_service);
 }
 
 void btd_gatt_client_disconnected(struct btd_gatt_client *client)
-- 
2.2.0.rc0.207.ga3a616c

[PATCH BlueZ 16/17] core: device: Add function to get GATT service

[Arman Uguray]

Added btd_device_get_gatt_service which returns a btd_service by UUID
and handle-range.
---
 src/device.c | 27 +++++++++++++++++++++++++++
 src/device.h |  4 ++++
 2 files changed, 31 insertions(+)

diff --git a/src/device.c b/src/device.c
index f13d9aa..6488396 100644
--- a/src/device.c
+++ b/src/device.c
@@ -5432,6 +5432,33 @@ struct btd_service *btd_device_get_service(struct btd_device *dev,
 	return NULL;
 }
 
+struct btd_service *btd_device_get_gatt_service(struct btd_device *dev,
+							const char *remote_uuid,
+							uint16_t start_handle,
+							uint16_t end_handle)
+{
+	GSList *l;
+
+	if (!dev)
+		return NULL;
+
+	for (l = dev->services; l != NULL; l = g_slist_next(l)) {
+		struct btd_service *service = l->data;
+		struct btd_profile *p = btd_service_get_profile(service);
+		uint16_t start, end;
+
+		if (!btd_service_get_gatt_handles(service, &start, &end))
+			continue;
+
+		if (g_str_equal(p->remote_uuid, remote_uuid) &&
+							start == start_handle &&
+							end == end_handle)
+			return service;
+	}
+
+	return NULL;
+}
+
 void btd_device_init(void)
 {
 	dbus_conn = btd_get_dbus_connection();
diff --git a/src/device.h b/src/device.h
index a7fefee..f2d67ad 100644
--- a/src/device.h
+++ b/src/device.h
@@ -148,6 +148,10 @@ bool device_remove_svc_complete_callback(struct btd_device *dev,
 
 struct btd_service *btd_device_get_service(struct btd_device *dev,
 						const char *remote_uuid);
+struct btd_service *btd_device_get_gatt_service(struct btd_device *dev,
+							const char *remote_uuid,
+							uint16_t start_handle,
+							uint16_t end_handle);
 
 int device_discover_services(struct btd_device *device);
 int btd_device_connect_services(struct btd_device *dev, GSList *services);
-- 
2.2.0.rc0.207.ga3a616c

[PATCH BlueZ 17/17] core: gatt: Don't export claimed services

[Arman Uguray]

GattService1 objects for services which are claimed by an internal
profile are no longer exported. This is to prevent conflicts that
may arise between internal plugins and external applications that
wish to access the same GATT service.

This is at least until there is a better system for managing external
claims on GATT services and all remote profiles have been switched to
shared/gatt, which prevents some of the conflicts that may arise from
writes to CCC descriptors.
---
 src/gatt-client.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/src/gatt-client.c b/src/gatt-client.c
index 16a3b3b..4f44516 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -1772,11 +1772,36 @@ static gboolean set_chrcs_ready(gpointer user_data)
 	return FALSE;
 }
 
+static bool is_service_claimed(struct btd_device *dev,
+						struct gatt_db_attribute *attr)
+{
+	uint16_t start, end;
+	bt_uuid_t uuid;
+	char uuid_str[MAX_LEN_UUID_STR];
+
+	if (!gatt_db_attribute_get_service_data(attr, &start, &end, NULL,
+									&uuid))
+		return false;
+
+	bt_uuid_to_string(&uuid, uuid_str, sizeof(uuid_str));
+
+	return !!btd_device_get_gatt_service(dev, uuid_str, start, end);
+}
+
 static void export_service(struct gatt_db_attribute *attr, void *user_data)
 {
 	struct btd_gatt_client *client = user_data;
 	struct service *service;
 
+	/*
+	 * Check if a profile claimed this service and don't export it
+	 * otherwise.
+	 */
+	if (is_service_claimed(client->device, attr)) {
+		DBG("GATT service already claimed by an internal profile");
+		return;
+	}
+
 	service = service_create(attr, client);
 	if (!service)
 		return;
-- 
2.2.0.rc0.207.ga3a616c

Re: [PATCH BlueZ 09/17] core: gatt: Expose charac. extended properties.

[Johan Hedberg]

Hi Arman,

On Fri, Dec 19, 2014, Arman Uguray wrote:
> @@ -521,12 +537,12 @@ static struct {
>  	{ BT_GATT_CHRC_PROP_INDICATE,		"indicate" },
>  	{ BT_GATT_CHRC_PROP_AUTH,		"authenticated-signed-writes" },
>  	{ BT_GATT_CHRC_PROP_EXT_PROP,		"extended-properties" },
> -	{ },
>  	/* Extended Properties */
>  	{ BT_GATT_CHRC_EXT_PROP_RELIABLE_WRITE,	"reliable-write" },
>  	{ BT_GATT_CHRC_EXT_PROP_WRITABLE_AUX,	"writable-auxiliaries" },
>  	{ }
>  };
> +static const int ext_props_index = 8;
>  
>  static gboolean characteristic_property_get_flags(
>  					const GDBusPropertyTable *property,
> @@ -534,21 +550,18 @@ static gboolean characteristic_property_get_flags(
>  {
>  	struct characteristic *chrc = data;
>  	DBusMessageIter array;
> +	uint16_t props;
>  	int i;
>  
>  	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, "s", &array);
>  
>  	for (i = 0; properties[i].str; i++) {
> -		if (chrc->props & properties[i].prop)
> +		props = i < ext_props_index ? chrc->props : chrc->ext_props;
> +		if (props & properties[i].prop)
>  			dbus_message_iter_append_basic(&array, DBUS_TYPE_STRING,
>  							&properties[i].str);

This looks quite brittle to me with the hard-coded index and extra logic
in the for-loop. Couldn't you just put the extended properties in their
own array and have a separate for-loop for that?  Also, you could use
the NELEM() macro instead of checking for non-NULL string. You wouldn't
then need an empty element at the end of th array.

Johan

Re: [PATCH BlueZ 10/17] shared/gatt-client: Expose MTU.

[Luiz Augusto von Dentz]

Hi Arman,

On Fri, Dec 19, 2014 at 7:35 PM, Arman Uguray <armansito@chromium.org> wrote:
> This patch adds the bt_gatt_client_get_mtu function. It also includes a
> minor fix in which the signature of the "value" argument of write
> methods are changed from "uint8_t *" to "const uint8_t *".

We better split this changes.

> ---
>  src/shared/gatt-client.c | 14 +++++++++++---
>  src/shared/gatt-client.h |  8 +++++---
>  2 files changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
> index f7a90d1..6768892 100644
> --- a/src/shared/gatt-client.c
> +++ b/src/shared/gatt-client.c
> @@ -1599,6 +1599,14 @@ bool bt_gatt_client_set_debug(struct bt_gatt_client *client,
>         return true;
>  }
>
> +uint16_t bt_gatt_client_get_mtu(struct bt_gatt_client *client)
> +{
> +       if (!client || !client->att)
> +               return 0;
> +
> +       return bt_att_get_mtu(client->att);
> +}
> +
>  struct read_op {
>         bt_gatt_client_read_callback_t callback;
>         void *user_data;
> @@ -1955,7 +1963,7 @@ bool bt_gatt_client_read_long_value(struct bt_gatt_client *client,
>  bool bt_gatt_client_write_without_response(struct bt_gatt_client *client,
>                                         uint16_t value_handle,
>                                         bool signed_write,
> -                                       uint8_t *value, uint16_t length) {
> +                                       const uint8_t *value, uint16_t length) {
>         uint8_t pdu[2 + length];
>
>         if (!client)
> @@ -2011,7 +2019,7 @@ done:
>
>  bool bt_gatt_client_write_value(struct bt_gatt_client *client,
>                                         uint16_t value_handle,
> -                                       uint8_t *value, uint16_t length,
> +                                       const uint8_t *value, uint16_t length,
>                                         bt_gatt_client_callback_t callback,
>                                         void *user_data,
>                                         bt_gatt_client_destroy_func_t destroy)
> @@ -2260,7 +2268,7 @@ done:
>  bool bt_gatt_client_write_long_value(struct bt_gatt_client *client,
>                                 bool reliable,
>                                 uint16_t value_handle, uint16_t offset,
> -                               uint8_t *value, uint16_t length,
> +                               const uint8_t *value, uint16_t length,
>                                 bt_gatt_client_write_long_callback_t callback,
>                                 void *user_data,
>                                 bt_gatt_client_destroy_func_t destroy)
> diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
> index 984c23f..42eeaec 100644
> --- a/src/shared/gatt-client.h
> +++ b/src/shared/gatt-client.h
> @@ -70,6 +70,8 @@ bool bt_gatt_client_set_debug(struct bt_gatt_client *client,
>                                         void *user_data,
>                                         bt_gatt_client_destroy_func_t destroy);
>
> +uint16_t bt_gatt_client_get_mtu(struct bt_gatt_client *client);
> +
>  bool bt_gatt_client_read_value(struct bt_gatt_client *client,
>                                         uint16_t value_handle,
>                                         bt_gatt_client_read_callback_t callback,
> @@ -89,17 +91,17 @@ bool bt_gatt_client_read_multiple(struct bt_gatt_client *client,
>  bool bt_gatt_client_write_without_response(struct bt_gatt_client *client,
>                                         uint16_t value_handle,
>                                         bool signed_write,
> -                                       uint8_t *value, uint16_t length);
> +                                       const uint8_t *value, uint16_t length);
>  bool bt_gatt_client_write_value(struct bt_gatt_client *client,
>                                         uint16_t value_handle,
> -                                       uint8_t *value, uint16_t length,
> +                                       const uint8_t *value, uint16_t length,
>                                         bt_gatt_client_callback_t callback,
>                                         void *user_data,
>                                         bt_gatt_client_destroy_func_t destroy);
>  bool bt_gatt_client_write_long_value(struct bt_gatt_client *client,
>                                 bool reliable,
>                                 uint16_t value_handle, uint16_t offset,
> -                               uint8_t *value, uint16_t length,
> +                               const uint8_t *value, uint16_t length,
>                                 bt_gatt_client_write_long_callback_t callback,
>                                 void *user_data,
>                                 bt_gatt_client_destroy_func_t destroy);
> --
> 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

Re: [PATCH BlueZ 03/17] core: gatt: Export GATT characteristics on D-Bus

[Luiz Augusto von Dentz]

Hi Arman,

On Fri, Dec 19, 2014 at 7:35 PM, Arman Uguray <armansito@chromium.org> wrote:
> This patch adds code that exports an object on D-Bus with the
> org.bluez.GattCharacteristic1 interface for each GATT characteristic
> that is present in bt_gatt_client.
> ---
>  src/gatt-client.c | 332 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 330 insertions(+), 2 deletions(-)
>
> diff --git a/src/gatt-client.c b/src/gatt-client.c
> index 05782ab..1a0e1cf 100644
> --- a/src/gatt-client.c
> +++ b/src/gatt-client.c
> @@ -30,6 +30,7 @@
>  #include <bluetooth/bluetooth.h>
>
>  #include "log.h"
> +#include "error.h"
>  #include "adapter.h"
>  #include "device.h"
>  #include "lib/uuid.h"
> @@ -41,7 +42,8 @@
>  #include "gatt-client.h"
>  #include "dbus-common.h"
>
> -#define GATT_SERVICE_IFACE "org.bluez.GattService1"
> +#define GATT_SERVICE_IFACE             "org.bluez.GattService1"
> +#define GATT_CHARACTERISTIC_IFACE      "org.bluez.GattCharacteristic1"
>
>  struct btd_gatt_client {
>         struct btd_device *device;
> @@ -59,8 +61,242 @@ struct service {
>         uint16_t end_handle;
>         bt_uuid_t uuid;
>         char *path;
> +       struct queue *chrcs;
> +       bool chrcs_ready;
>  };
>
> +struct characteristic {
> +       struct service *service;
> +       uint16_t handle;
> +       uint16_t value_handle;
> +       uint8_t props;
> +       bt_uuid_t uuid;
> +       char *path;
> +};
> +
> +static gboolean characteristic_property_get_uuid(
> +                                       const GDBusPropertyTable *property,
> +                                       DBusMessageIter *iter, void *data)
> +{
> +       char uuid[MAX_LEN_UUID_STR + 1];
> +       const char *ptr = uuid;
> +       struct characteristic *chrc = data;
> +
> +       bt_uuid_to_string(&chrc->uuid, uuid, sizeof(uuid));
> +       dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &ptr);
> +
> +       return TRUE;
> +}
> +
> +static gboolean characteristic_property_get_service(
> +                                       const GDBusPropertyTable *property,
> +                                       DBusMessageIter *iter, void *data)
> +{
> +       struct characteristic *chrc = data;
> +       const char *str = chrc->service->path;
> +
> +       dbus_message_iter_append_basic(iter, DBUS_TYPE_OBJECT_PATH, &str);
> +
> +       return TRUE;
> +}
> +
> +static gboolean characteristic_property_get_value(
> +                                       const GDBusPropertyTable *property,
> +                                       DBusMessageIter *iter, void *data)
> +{
> +       DBusMessageIter array;
> +
> +       dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, "y", &array);
> +
> +       /* TODO: Implement this once the value is cached */
> +
> +       dbus_message_iter_close_container(iter, &array);
> +
> +       return TRUE;
> +}
> +
> +static gboolean characteristic_property_get_notifying(
> +                                       const GDBusPropertyTable *property,
> +                                       DBusMessageIter *iter, void *data)
> +{
> +       dbus_bool_t notifying = FALSE;
> +
> +       /*
> +        * TODO: Return the correct value here once StartNotify and StopNotify
> +        * methods are implemented.
> +        */
> +
> +       dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN, &notifying);
> +
> +       return TRUE;
> +}
> +
> +static struct {
> +       uint8_t prop;
> +       char *str;
> +} properties[] = {
> +       /* Default Properties */
> +       { BT_GATT_CHRC_PROP_BROADCAST,          "broadcast" },
> +       { BT_GATT_CHRC_PROP_READ,               "read" },
> +       { BT_GATT_CHRC_PROP_WRITE_WITHOUT_RESP, "write-without-response" },
> +       { BT_GATT_CHRC_PROP_WRITE,              "write" },
> +       { BT_GATT_CHRC_PROP_NOTIFY,             "notify" },
> +       { BT_GATT_CHRC_PROP_INDICATE,           "indicate" },
> +       { BT_GATT_CHRC_PROP_AUTH,               "authenticated-signed-writes" },
> +       { BT_GATT_CHRC_PROP_EXT_PROP,           "extended-properties" },
> +       { },
> +       /* Extended Properties */
> +       { BT_GATT_CHRC_EXT_PROP_RELIABLE_WRITE, "reliable-write" },
> +       { BT_GATT_CHRC_EXT_PROP_WRITABLE_AUX,   "writable-auxiliaries" },
> +       { }
> +};
> +
> +static gboolean characteristic_property_get_flags(
> +                                       const GDBusPropertyTable *property,
> +                                       DBusMessageIter *iter, void *data)
> +{
> +       struct characteristic *chrc = data;
> +       DBusMessageIter array;
> +       int i;
> +
> +       dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, "s", &array);
> +
> +       for (i = 0; properties[i].str; i++) {
> +               if (chrc->props & properties[i].prop)
> +                       dbus_message_iter_append_basic(&array, DBUS_TYPE_STRING,
> +                                                       &properties[i].str);
> +       }
> +
> +       /*
> +        * TODO: Handle extended properties if the descriptor is
> +        * present.
> +        */
> +
> +       dbus_message_iter_close_container(iter, &array);
> +
> +       return TRUE;
> +}
> +
> +static DBusMessage *characteristic_read_value(DBusConnection *conn,
> +                                       DBusMessage *msg, void *user_data)
> +{
> +       /* TODO: Implement */
> +       return btd_error_failed(msg, "Not implemented");
> +}
> +
> +static DBusMessage *characteristic_write_value(DBusConnection *conn,
> +                                       DBusMessage *msg, void *user_data)
> +{
> +       /* TODO: Implement */
> +       return btd_error_failed(msg, "Not implemented");
> +}
> +
> +static DBusMessage *characteristic_start_notify(DBusConnection *conn,
> +                                       DBusMessage *msg, void *user_data)
> +{
> +       /* TODO: Implement */
> +       return btd_error_failed(msg, "Not implemented");
> +}
> +
> +static DBusMessage *characteristic_stop_notify(DBusConnection *conn,
> +                                       DBusMessage *msg, void *user_data)
> +{
> +       /* TODO: Implement */
> +       return btd_error_failed(msg, "Not implemented");
> +}
> +
> +static gboolean characteristic_property_get_descriptors(
> +                                       const GDBusPropertyTable *property,
> +                                       DBusMessageIter *iter, void *data)
> +{
> +       DBusMessageIter array;
> +
> +       dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, "o", &array);
> +
> +       /* TODO: Implement this once descriptors are exported */
> +
> +       dbus_message_iter_close_container(iter, &array);
> +
> +       return TRUE;
> +}
> +
> +static const GDBusPropertyTable characteristic_properties[] = {
> +       { "UUID", "s", characteristic_property_get_uuid },
> +       { "Service", "o", characteristic_property_get_service },
> +       { "Value", "ay", characteristic_property_get_value },
> +       { "Notifying", "b", characteristic_property_get_notifying },
> +       { "Flags", "as", characteristic_property_get_flags },
> +       { "Descriptors", "ao", characteristic_property_get_descriptors },
> +       { }
> +};
> +
> +static const GDBusMethodTable characteristic_methods[] = {
> +       { GDBUS_ASYNC_METHOD("ReadValue", NULL, GDBUS_ARGS({ "value", "ay" }),
> +                                               characteristic_read_value) },
> +       { GDBUS_ASYNC_METHOD("WriteValue", GDBUS_ARGS({ "value", "ay" }),
> +                                       NULL, characteristic_write_value) },
> +       { GDBUS_ASYNC_METHOD("StartNotify", NULL, NULL,
> +                                               characteristic_start_notify) },
> +       { GDBUS_METHOD("StopNotify", NULL, NULL, characteristic_stop_notify) },

I don't think if StartNotify and StopNotify are a reliable API in case
of paired device, CCC is persistent thus on disconnect it would
remember if any client has enabled notification but upon reconnection
that client may no longer be available, IMO the best policy here is to
always enable notifications if possible so it is up to the application
to register for PropertiesChanged if it cares about them.

> +       { }
> +};
> +
> +static void characteristic_free(void *data)
> +{
> +       struct characteristic *chrc = data;
> +
> +       g_free(chrc->path);
> +       free(chrc);
> +}
> +
> +static struct characteristic *characteristic_create(
> +                                               struct gatt_db_attribute *attr,
> +                                               struct service *service)
> +{
> +       struct characteristic *chrc;
> +       bt_uuid_t uuid;
> +
> +       chrc = new0(struct characteristic, 1);
> +       if (!chrc)
> +               return NULL;
> +
> +       chrc->service = service;
> +
> +       gatt_db_attribute_get_char_data(attr, &chrc->handle,
> +                                                       &chrc->value_handle,
> +                                                       &chrc->props, &uuid);
> +       bt_uuid_to_uuid128(&uuid, &chrc->uuid);
> +
> +       chrc->path = g_strdup_printf("%s/char%04x", service->path,
> +                                                               chrc->handle);
> +
> +       if (!g_dbus_register_interface(btd_get_dbus_connection(), chrc->path,
> +                                               GATT_CHARACTERISTIC_IFACE,
> +                                               characteristic_methods, NULL,
> +                                               characteristic_properties,
> +                                               chrc, characteristic_free)) {
> +               error("Unable to register GATT characteristic with handle "
> +                                                       "0x%04x", chrc->handle);
> +               characteristic_free(chrc);
> +
> +               return NULL;
> +       }
> +
> +       DBG("Exported GATT characteristic: %s", chrc->path);
> +
> +       return chrc;
> +}
> +
> +static void unregister_characteristic(void *data)
> +{
> +       struct characteristic *chrc = data;
> +
> +       DBG("Removing GATT characteristic: %s", chrc->path);
> +
> +       g_dbus_unregister_interface(btd_get_dbus_connection(), chrc->path,
> +                                               GATT_CHARACTERISTIC_IFACE);
> +}
> +
>  static gboolean service_property_get_uuid(const GDBusPropertyTable *property,
>                                         DBusMessageIter *iter, void *data)
>  {
> @@ -98,15 +334,26 @@ static gboolean service_property_get_primary(const GDBusPropertyTable *property,
>         return TRUE;
>  }
>
> +static void append_chrc_path(void *data, void *user_data)
> +{
> +       struct characteristic *chrc = data;
> +       DBusMessageIter *array = user_data;
> +
> +       dbus_message_iter_append_basic(array, DBUS_TYPE_OBJECT_PATH,
> +                                                               &chrc->path);
> +}
> +
>  static gboolean service_property_get_characteristics(
>                                         const GDBusPropertyTable *property,
>                                         DBusMessageIter *iter, void *data)
>  {
> +       struct service *service = data;
>         DBusMessageIter array;
>
>         dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, "o", &array);
>
> -       /* TODO: Implement this once characteristics are exported */
> +       if (service->chrcs_ready)
> +               queue_foreach(service->chrcs, append_chrc_path, &array);
>
>         dbus_message_iter_close_container(iter, &array);
>
> @@ -125,6 +372,7 @@ static void service_free(void *data)
>  {
>         struct service *service = data;
>
> +       queue_destroy(service->chrcs, NULL);  /* List should be empty here */
>         g_free(service->path);
>         free(service);
>  }
> @@ -140,6 +388,12 @@ static struct service *service_create(struct gatt_db_attribute *attr,
>         if (!service)
>                 return NULL;
>
> +       service->chrcs = queue_new();
> +       if (!service->chrcs) {
> +               free(service);
> +               return NULL;
> +       }
> +
>         service->client = client;
>
>         gatt_db_attribute_get_service_data(attr, &service->start_handle,
> @@ -176,10 +430,71 @@ static void unregister_service(void *data)
>
>         DBG("Removing GATT service: %s", service->path);
>
> +       queue_remove_all(service->chrcs, NULL, NULL, unregister_characteristic);
> +
>         g_dbus_unregister_interface(btd_get_dbus_connection(), service->path,
>                                                         GATT_SERVICE_IFACE);
>  }
>
> +struct export_data {
> +       void *root;
> +       bool failed;
> +};
> +
> +static void export_char(struct gatt_db_attribute *attr, void *user_data)
> +{
> +       struct characteristic *charac;
> +       struct export_data *data = user_data;
> +       struct service *service = data->root;
> +
> +       if (data->failed)
> +               return;
> +
> +       charac = characteristic_create(attr, service);
> +       if (!charac) {
> +               data->failed = true;
> +               return;
> +       }
> +
> +       queue_push_tail(service->chrcs, charac);
> +}
> +
> +static bool create_characteristics(struct gatt_db_attribute *attr,
> +                                               struct service *service)
> +{
> +       struct export_data data;
> +
> +       data.root = service;
> +       data.failed = false;
> +
> +       gatt_db_service_foreach_char(attr, export_char, &data);
> +
> +       return !data.failed;
> +}
> +
> +static void notify_chrcs(void *data, void *user_data)
> +{
> +       struct service *service = data;
> +
> +       service->chrcs_ready = true;
> +
> +       g_dbus_emit_property_changed(btd_get_dbus_connection(), service->path,
> +                                                       GATT_SERVICE_IFACE,
> +                                                       "Characteristics");
> +}
> +
> +static gboolean set_chrcs_ready(gpointer user_data)
> +{
> +       struct btd_gatt_client *client = user_data;
> +
> +       if (!client->gatt)
> +               return FALSE;
> +
> +       queue_foreach(client->services, notify_chrcs, NULL);
> +
> +       return FALSE;
> +}
> +
>  static void export_service(struct gatt_db_attribute *attr, void *user_data)
>  {
>         struct btd_gatt_client *client = user_data;
> @@ -189,6 +504,12 @@ static void export_service(struct gatt_db_attribute *attr, void *user_data)
>         if (!service)
>                 return;
>
> +       if (!create_characteristics(attr, service)) {
> +               error("Exporting characteristics failed");
> +               unregister_service(service);
> +               return;
> +       }
> +
>         queue_push_tail(client->services, service);
>  }
>
> @@ -197,6 +518,13 @@ static void create_services(struct btd_gatt_client *client)
>         DBG("Exporting objects for GATT services: %s", client->devaddr);
>
>         gatt_db_foreach_service(client->db, NULL, export_service, client);
> +
> +       /*
> +        * Asynchronously update the "Characteristics" property of each service.
> +        * We do this so that users have a way to know that all characteristics
> +        * of a service have been exported.
> +        */
> +       g_idle_add(set_chrcs_ready, client);
>  }
>
>  struct btd_gatt_client *btd_gatt_client_new(struct btd_device *device)
> --
> 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

Re: [PATCH BlueZ 05/17] core: gatt: Implement GattCharacteristic1.ReadValue

[Luiz Augusto von Dentz]

Hi Arman,

On Fri, Dec 19, 2014 at 7:35 PM, Arman Uguray <armansito@chromium.org> wrote:
> This patch implements the ReadValue method of
> org.bluez.GattCharacteristic1 and exposes the "Value" property based on
> what was cached during the most recent read. The property is hidden if
> the value is unknown.
> ---
>  src/gatt-client.c | 217 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 213 insertions(+), 4 deletions(-)
>
> diff --git a/src/gatt-client.c b/src/gatt-client.c
> index bd3711a..1c4ea51 100644
> --- a/src/gatt-client.c
> +++ b/src/gatt-client.c
> @@ -73,6 +73,12 @@ struct characteristic {
>         uint8_t props;
>         bt_uuid_t uuid;
>         char *path;
> +
> +       bool in_read;
> +       bool value_known;
> +       uint8_t *value;
> +       size_t value_len;
> +
>         struct queue *descs;
>  };
>
> @@ -83,6 +89,66 @@ struct descriptor {
>         char *path;
>  };
>
> +static DBusMessage *gatt_error_read_not_permitted(DBusMessage *msg)
> +{
> +       return g_dbus_create_error(msg, ERROR_INTERFACE ".ReadNotPermitted",
> +                               "Reading of this value is not allowed");
> +}
> +
> +static DBusMessage *gatt_error_write_not_permitted(DBusMessage *msg)
> +{
> +       return g_dbus_create_error(msg, ERROR_INTERFACE ".WriteNotPermitted",
> +                               "Writing of this value is not allowed");
> +}
> +
> +static DBusMessage *gatt_error_invalid_value_len(DBusMessage *msg)
> +{
> +       return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidValueLength",
> +                                                       "Invalid value length");
> +}
> +
> +static DBusMessage *gatt_error_invalid_offset(DBusMessage *msg)
> +{
> +       return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidOffset",
> +                                                       "Invalid value offset");
> +}
> +
> +static DBusMessage *gatt_error_not_paired(DBusMessage *msg)
> +{
> +       return g_dbus_create_error(msg, ERROR_INTERFACE ".NotPaired",
> +                                                               "Not Paired");
> +}
> +
> +static DBusMessage *create_gatt_dbus_error(DBusMessage *msg, uint8_t att_ecode)
> +{
> +       switch (att_ecode) {
> +       case BT_ATT_ERROR_READ_NOT_PERMITTED:
> +               return gatt_error_read_not_permitted(msg);
> +       case BT_ATT_ERROR_WRITE_NOT_PERMITTED:
> +               return gatt_error_write_not_permitted(msg);
> +       case BT_ATT_ERROR_AUTHENTICATION:
> +       case BT_ATT_ERROR_INSUFFICIENT_ENCRYPTION:
> +       case BT_ATT_ERROR_INSUFFICIENT_ENCRYPTION_KEY_SIZE:
> +               return gatt_error_not_paired(msg);
> +       case BT_ATT_ERROR_INVALID_OFFSET:
> +               return gatt_error_invalid_offset(msg);
> +       case BT_ATT_ERROR_INVALID_ATTRIBUTE_VALUE_LEN:
> +               return gatt_error_invalid_value_len(msg);
> +       case BT_ATT_ERROR_AUTHORIZATION:
> +               return btd_error_not_authorized(msg);
> +       case BT_ATT_ERROR_REQUEST_NOT_SUPPORTED:
> +               return btd_error_not_supported(msg);
> +       case 0:
> +               return btd_error_failed(msg, "Operation failed");
> +       default:
> +               return g_dbus_create_error(msg, ERROR_INTERFACE,
> +                               "Operation failed with ATT error: 0x%02x",
> +                               att_ecode);
> +       }
> +
> +       return NULL;
> +}
> +
>  static gboolean descriptor_property_get_uuid(const GDBusPropertyTable *property,
>                                         DBusMessageIter *iter, void *data)
>  {
> @@ -232,17 +298,32 @@ static gboolean characteristic_property_get_value(
>                                         const GDBusPropertyTable *property,
>                                         DBusMessageIter *iter, void *data)
>  {
> +       struct characteristic *chrc = data;
>         DBusMessageIter array;
> +       size_t i;
>
>         dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, "y", &array);
>
> -       /* TODO: Implement this once the value is cached */
> +       if (chrc->value_known) {
> +               for (i = 0; i < chrc->value_len; i++)
> +                       dbus_message_iter_append_basic(&array, DBUS_TYPE_BYTE,
> +                                                       chrc->value + i);
> +       }
>
>         dbus_message_iter_close_container(iter, &array);
>
>         return TRUE;
>  }
>
> +static gboolean characteristic_property_value_exists(
> +                                       const GDBusPropertyTable *property,
> +                                       void *data)
> +{
> +       struct characteristic *chrc = data;
> +
> +       return chrc->value_known ? TRUE : FALSE;
> +}
> +
>  static gboolean characteristic_property_get_notifying(
>                                         const GDBusPropertyTable *property,
>                                         DBusMessageIter *iter, void *data)
> @@ -305,11 +386,137 @@ static gboolean characteristic_property_get_flags(
>         return TRUE;
>  }
>
> +struct chrc_dbus_op {
> +       struct characteristic *chrc;
> +       DBusMessage *msg;
> +};
> +
> +static void chrc_dbus_op_free(void *data)
> +{
> +       struct chrc_dbus_op *op = data;
> +
> +       dbus_message_unref(op->msg);
> +       free(op);
> +}
> +
> +static bool chrc_resize_value(struct characteristic *chrc, size_t new_size)
> +{
> +       uint8_t *ptr;
> +
> +       if (chrc->value_len == new_size)
> +               return true;
> +
> +       if (!new_size) {
> +               free(chrc->value);
> +               chrc->value = NULL;
> +               chrc->value_len = 0;
> +
> +               return true;
> +       }
> +
> +       ptr = realloc(chrc->value, sizeof(uint8_t) * new_size);
> +       if (!ptr)
> +               return false;
> +
> +       chrc->value = ptr;
> +       chrc->value_len = new_size;
> +
> +       return true;
> +}
> +
> +static void chrc_read_long_cb(bool success, uint8_t att_ecode,
> +                                       const uint8_t *value, uint16_t length,
> +                                       void *user_data)
> +{
> +       struct chrc_dbus_op *op = user_data;
> +       struct characteristic *chrc = op->chrc;
> +       DBusMessageIter iter, array;
> +       DBusMessage *reply;
> +
> +       op->chrc->in_read = false;
> +
> +       if (!success) {
> +               reply = create_gatt_dbus_error(op->msg, att_ecode);
> +               goto done;
> +       }
> +
> +       /*
> +        * If the value is the same, then only update it if it wasn't previously
> +        * known.
> +        */
> +       if (chrc->value_known && chrc->value_len == length &&
> +                       !memcmp(chrc->value, value, sizeof(uint8_t) * length))
> +               goto reply;
> +
> +       if (!chrc_resize_value(chrc, length)) {
> +               /*
> +                * Failed to resize the buffer. Since we don't want to show a
> +                * stale value, if the value was previously known, then free and
> +                * hide it.
> +                */
> +               free(chrc->value);
> +               chrc->value = NULL;
> +               chrc->value_len = 0;
> +               chrc->value_known = false;
> +
> +               goto changed_signal;
> +       }
> +
> +       chrc->value_known = true;
> +       memcpy(chrc->value, value, sizeof(uint8_t) * length);

We could store this in the db itself and remember across connections
since this is supposed to be used only when notifications are enabled.

> +changed_signal:
> +       g_dbus_emit_property_changed(btd_get_dbus_connection(), chrc->path,
> +                                               GATT_CHARACTERISTIC_IFACE,
> +                                               "Value");
> +
> +reply:
> +       reply = g_dbus_create_reply(op->msg, DBUS_TYPE_INVALID);
> +       if (!reply) {
> +               error("Failed to allocate D-Bus message reply");
> +               return;
> +       }
> +
> +       dbus_message_iter_init_append(reply, &iter);
> +       dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY, "y", &array);
> +
> +       for (i = 0; i < length; i++)
> +               dbus_message_iter_append_basic(&array, DBUS_TYPE_BYTE,
> +                                                               value + i);
> +
> +       dbus_message_iter_close_container(&iter, &array);
> +
> +done:
> +       g_dbus_send_message(btd_get_dbus_connection(), reply);
> +}
> +
>  static DBusMessage *characteristic_read_value(DBusConnection *conn,
>                                         DBusMessage *msg, void *user_data)
>  {
> -       /* TODO: Implement */
> -       return btd_error_failed(msg, "Not implemented");
> +       struct characteristic *chrc = user_data;
> +       struct bt_gatt_client *gatt = chrc->service->client->gatt;
> +       struct chrc_dbus_op *op;
> +
> +       if (chrc->in_read)
> +               return btd_error_in_progress(msg);
> +
> +       op = new0(struct chrc_dbus_op, 1);
> +       if (!op)
> +               return btd_error_failed(msg, "Failed to initialize request");
> +
> +       op->chrc = chrc;
> +       op->msg = dbus_message_ref(msg);
> +
> +       if (bt_gatt_client_read_long_value(gatt, chrc->value_handle, 0,
> +                                                       chrc_read_long_cb, op,
> +                                                       chrc_dbus_op_free)) {
> +               chrc->in_read = true;
> +               return NULL;
> +       }
> +
> +       chrc_dbus_op_free(op);
> +
> +       return btd_error_failed(msg, "Failed to send read request");
>  }
>
>  static DBusMessage *characteristic_write_value(DBusConnection *conn,
> @@ -361,7 +568,8 @@ static gboolean characteristic_property_get_descriptors(
>  static const GDBusPropertyTable characteristic_properties[] = {
>         { "UUID", "s", characteristic_property_get_uuid },
>         { "Service", "o", characteristic_property_get_service },
> -       { "Value", "ay", characteristic_property_get_value },
> +       { "Value", "ay", characteristic_property_get_value, NULL,
> +                                       characteristic_property_value_exists },
>         { "Notifying", "b", characteristic_property_get_notifying },
>         { "Flags", "as", characteristic_property_get_flags },
>         { "Descriptors", "ao", characteristic_property_get_descriptors },
> @@ -384,6 +592,7 @@ static void characteristic_free(void *data)
>         struct characteristic *chrc = data;
>
>         queue_destroy(chrc->descs, NULL);  /* List should be empty here */
> +       free(chrc->value);
>         g_free(chrc->path);
>         free(chrc);
>  }
> --
> 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

Re: [PATCH BlueZ 07/17] core: gatt: Implement GattCharacteristic1.StartNotify

[Michael Janssen]

Hi Arman,

On Fri, Dec 19, 2014 at 1:35 PM, Arman Uguray <armansito@chromium.org> wrote:
> This patch implements the StartNotify method of
> org.bluez.GattCharacteristic1. Each call to StartNotify assigns a
> session to the D-Bus sender which internally registers a unique
> notify handler with the bt_gatt_client. Each received notification
> or indication causes a PropertiesChanged signal to be emitted for the
> "Value" property.
>
> The notify handler gets automatically unregistered when sender
> disconnects from D-Bus.
> ---
>  src/gatt-client.c | 272 +++++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 259 insertions(+), 13 deletions(-)
>
> diff --git a/src/gatt-client.c b/src/gatt-client.c
> index 9cbe32d..a29eea9 100644
> --- a/src/gatt-client.c
> +++ b/src/gatt-client.c
> @@ -80,6 +80,9 @@ struct characteristic {
>         size_t value_len;
>
>         struct queue *descs;
> +
> +       bool notifying;
> +       struct queue *notify_clients;
>  };
>
>  struct descriptor {
> @@ -236,15 +239,20 @@ static bool resize_value_buffer(size_t new_len, uint8_t **value, size_t *len)
>  static void update_value_property(const uint8_t *value, size_t len,
>                                         uint8_t **cur_value, size_t *cur_len,
>                                         bool *value_known,
> -                                       const char *path, const char *iface)
> +                                       const char *path, const char *iface,
> +                                       bool notify_if_same)
>  {
>         /*
>          * If the value is the same, then only updated it if wasn't previously
>          * known.
>          */
>         if (*value_known && *cur_len == len &&
> -                       !memcmp(*cur_value, value, sizeof(uint8_t) * len))
> +                       !memcmp(*cur_value, value, sizeof(uint8_t) * len)) {
> +               if (notify_if_same)
> +                       goto notify;
> +
>                 return;
> +       }
>
>         if (resize_value_buffer(len, cur_value, cur_len)) {
>                 *value_known = true;
> @@ -261,6 +269,7 @@ static void update_value_property(const uint8_t *value, size_t len,
>                 *value_known = false;
>         }
>
> +notify:
>         g_dbus_emit_property_changed(btd_get_dbus_connection(), path, iface,
>                                                                 "Value");
>  }
> @@ -274,7 +283,9 @@ static void async_dbus_op_free(void *data)
>  {
>         struct async_dbus_op *op = data;
>
> -       dbus_message_unref(op->msg);
> +       if (op->msg)
> +               dbus_message_unref(op->msg);
> +
>         free(op);
>  }
>
> @@ -311,7 +322,7 @@ static void desc_read_long_cb(bool success, uint8_t att_ecode,
>
>         update_value_property(value, length, &desc->value, &desc->value_len,
>                                                 &desc->value_known, desc->path,
> -                                               GATT_DESCRIPTOR_IFACE);
> +                                               GATT_DESCRIPTOR_IFACE, false);
>
>         reply = g_dbus_create_reply(op->msg, DBUS_TYPE_INVALID);
>         if (!reply) {
> @@ -489,12 +500,8 @@ static gboolean characteristic_property_get_notifying(
>                                         const GDBusPropertyTable *property,
>                                         DBusMessageIter *iter, void *data)
>  {
> -       dbus_bool_t notifying = FALSE;
> -
> -       /*
> -        * TODO: Return the correct value here once StartNotify and StopNotify
> -        * methods are implemented.
> -        */
> +       struct characteristic *chrc = data;
> +       dbus_bool_t notifying = chrc->notifying ? TRUE : FALSE;
>
>         dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN, &notifying);
>
> @@ -564,7 +571,8 @@ static void chrc_read_long_cb(bool success, uint8_t att_ecode,
>
>         update_value_property(value, length, &chrc->value, &chrc->value_len,
>                                                 &chrc->value_known, chrc->path,
> -                                               GATT_CHARACTERISTIC_IFACE);
> +                                               GATT_CHARACTERISTIC_IFACE,
> +                                               false);
>
>         reply = g_dbus_create_reply(op->msg, DBUS_TYPE_INVALID);
>         if (!reply) {
> @@ -614,11 +622,241 @@ static DBusMessage *characteristic_write_value(DBusConnection *conn,
>         return btd_error_failed(msg, "Not implemented");
>  }
>
> +struct notify_client {
> +       struct characteristic *chrc;
> +       int ref_count;
> +       char *owner;
> +       guint watch;
> +       unsigned int notify_id;
> +};
> +
> +static void notify_client_free(struct notify_client *client)
> +{
> +       DBG("owner %s", client->owner);
> +
> +       g_dbus_remove_watch(btd_get_dbus_connection(), client->watch);
> +       free(client->owner);
> +       free(client);
> +}
> +
> +static void notify_client_unref(void *data)
> +{
> +       struct notify_client *client = data;
> +
> +       DBG("owner %s", client->owner);
> +
> +       if (__sync_sub_and_fetch(&client->ref_count, 1))
> +               return;
> +
> +       notify_client_free(client);
> +}

notify_client_free and notify_client_unref are both used in different
situations here.  It seems cleaner to do all management through
reference counting and you can inline notify_client_free into
notify_client_unref.

> +
> +static struct notify_client *notify_client_ref(struct notify_client *client)
> +{
> +       DBG("owner %s", client->owner);
> +
> +       __sync_fetch_and_add(&client->ref_count, 1);
> +
> +       return client;
> +}
> +
> +static bool match_notifying(const void *a, const void *b)
> +{
> +       const struct notify_client *client = a;
> +
> +       return !!client->notify_id;
> +}
> +
> +static void update_notifying(struct characteristic *chrc)
> +{
> +       if (!chrc->notifying)
> +               return;
> +
> +       if (queue_find(chrc->notify_clients, match_notifying, NULL))
> +               return;
> +
> +       chrc->notifying = false;
> +
> +       g_dbus_emit_property_changed(btd_get_dbus_connection(), chrc->path,
> +                                               GATT_CHARACTERISTIC_IFACE,
> +                                               "Notifying");
> +}
> +
> +static void notify_client_disconnect(DBusConnection *conn, void *user_data)
> +{
> +       struct notify_client *client = user_data;
> +       struct characteristic *chrc = client->chrc;
> +       struct bt_gatt_client *gatt = chrc->service->client->gatt;
> +
> +       DBG("owner %s", client->owner);
> +
> +       queue_remove(chrc->notify_clients, client);
> +       bt_gatt_client_unregister_notify(gatt, client->notify_id);
> +
> +       update_notifying(chrc);
> +
> +       notify_client_unref(client);
> +}
> +
> +static struct notify_client *notify_client_create(struct characteristic *chrc,
> +                                                       const char *owner)
> +{
> +       struct notify_client *client;
> +
> +       client = new0(struct notify_client, 1);
> +       if (!client)
> +               return NULL;
> +
> +       client->chrc = chrc;
> +       client->owner = strdup(owner);
> +       if (!client->owner) {
> +               free(client);
> +               return NULL;
> +       }
> +
> +       client->watch = g_dbus_add_disconnect_watch(btd_get_dbus_connection(),
> +                                               owner, notify_client_disconnect,
> +                                               client, NULL);
> +       if (!client->watch) {
> +               free(client->owner);
> +               free(client);
> +               return NULL;
> +       }
> +
> +       return notify_client_ref(client);
> +}
> +
> +static bool match_notify_sender(const void *a, const void *b)
> +{
> +       const struct notify_client *client = a;
> +       const char *sender = b;
> +
> +       return strcmp(client->owner, sender) == 0;
> +}
> +
> +static void notify_cb(uint16_t value_handle, const uint8_t *value,
> +                                       uint16_t length, void *user_data)
> +{
> +       struct async_dbus_op *op = user_data;
> +       struct notify_client *client = op->data;
> +       struct characteristic *chrc = client->chrc;
> +
> +       /*
> +        * Even if the value didn't change, we want to send a PropertiesChanged
> +        * signal so that we propagate the notification/indication to
> +        * applications.
> +        */
> +       update_value_property(value, length, &chrc->value, &chrc->value_len,
> +                                               &chrc->value_known, chrc->path,
> +                                               GATT_CHARACTERISTIC_IFACE,
> +                                               true);
> +}
> +
> +static void register_notify_cb(unsigned int id, uint16_t att_ecode,
> +                                                               void *user_data)
> +{
> +       struct async_dbus_op *op = user_data;
> +       struct notify_client *client = op->data;
> +       struct characteristic *chrc = client->chrc;
> +       DBusMessage *reply;
> +
> +       /* Make sure that an interim disconnect did not remove the client */
> +       if (!queue_find(chrc->notify_clients, NULL, client)) {
> +               bt_gatt_client_unregister_notify(chrc->service->client->gatt,
> +                                                                       id);
> +               notify_client_unref(client);
> +
> +               reply = btd_error_failed(op->msg,
> +                                               "Characteristic not available");
> +               goto done;
> +       }
> +
> +       /*
> +        * Drop the reference count that we added when registering the callback.
> +        */
> +       notify_client_unref(client);
> +
> +       if (!id) {
> +               queue_remove(chrc->notify_clients, client);
> +               notify_client_free(client);
> +
> +               reply = create_gatt_dbus_error(op->msg, att_ecode);
> +
> +               goto done;
> +       }
> +
> +       client->notify_id = id;
> +
> +       if (!chrc->notifying) {
> +               chrc->notifying = true;
> +               g_dbus_emit_property_changed(btd_get_dbus_connection(),
> +                                       chrc->path, GATT_CHARACTERISTIC_IFACE,
> +                                       "Notifying");
> +       }
> +
> +       reply = g_dbus_create_reply(op->msg, DBUS_TYPE_INVALID);
> +
> +done:
> +       if (reply)
> +               g_dbus_send_message(btd_get_dbus_connection(), reply);
> +       else
> +               error("Failed to construct D-Bus message reply");
> +
> +       dbus_message_unref(op->msg);
> +       op->msg = NULL;
> +}
> +
>  static DBusMessage *characteristic_start_notify(DBusConnection *conn,
>                                         DBusMessage *msg, void *user_data)
>  {
> -       /* TODO: Implement */
> -       return btd_error_failed(msg, "Not implemented");
> +       struct characteristic *chrc = user_data;
> +       struct bt_gatt_client *gatt = chrc->service->client->gatt;
> +       const char *sender = dbus_message_get_sender(msg);
> +       struct async_dbus_op *op;
> +       struct notify_client *client;
> +
> +       if (!(chrc->props & BT_GATT_CHRC_PROP_NOTIFY ||
> +                               chrc->props & BT_GATT_CHRC_PROP_INDICATE))
> +               return btd_error_not_supported(msg);
> +
> +       /* Each client can only have one active notify session. */
> +       client = queue_find(chrc->notify_clients, match_notify_sender, sender);
> +       if (client)
> +               return client->notify_id ?
> +                               btd_error_failed(msg, "Already notifying") :
> +                               btd_error_in_progress(msg);
> +
> +       client = notify_client_create(chrc, sender);
> +       if (!client)
> +               return btd_error_failed(msg, "Failed allocate notify session");
> +
> +       op = new0(struct async_dbus_op, 1);
> +       if (!op) {
> +               notify_client_unref(client);
> +               return btd_error_failed(msg, "Failed to initialize request");
> +       }
> +
> +       /*
> +        * Add to the ref count so that a disconnect during register won't free
> +        * the client instance.
> +        */
> +       op->data = notify_client_ref(client);
> +       op->msg = dbus_message_ref(msg);
> +
> +       queue_push_tail(chrc->notify_clients, client);
> +
> +       if (bt_gatt_client_register_notify(gatt, chrc->value_handle,
> +                                               register_notify_cb, notify_cb,
> +                                               op, async_dbus_op_free))
> +               return NULL;
> +
> +       queue_remove(chrc->notify_clients, client);
> +       async_dbus_op_free(op);
> +
> +       /* Directly free the client */
> +       notify_client_free(client);
> +
> +       return btd_error_failed(msg, "Failed to register notify session");
>  }
>
>  static DBusMessage *characteristic_stop_notify(DBusConnection *conn,
> @@ -702,6 +940,13 @@ static struct characteristic *characteristic_create(
>                 return NULL;
>         }
>
> +       chrc->notify_clients = queue_new();
> +       if (!chrc->notify_clients) {
> +               queue_destroy(chrc->descs, NULL);
> +               free(chrc);
> +               return NULL;
> +       }
> +
>         chrc->service = service;
>
>         gatt_db_attribute_get_char_data(attr, &chrc->handle,
> @@ -735,6 +980,7 @@ static void unregister_characteristic(void *data)
>
>         DBG("Removing GATT characteristic: %s", chrc->path);
>
> +       queue_remove_all(chrc->notify_clients, NULL, NULL, notify_client_unref);
>         queue_remove_all(chrc->descs, NULL, NULL, unregister_descriptor);
>
>         g_dbus_unregister_interface(btd_get_dbus_connection(), chrc->path,
> --
> 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



-- 
Michael Janssen

Re: [PATCH BlueZ 00/17] Implement doc/gatt-api.txt for client

[Luiz Augusto von Dentz]

Hi Arman,

On Fri, Dec 19, 2014 at 7:35 PM, Arman Uguray <armansito@chromium.org> wrote:
> This patch set implements the D-Bus API outlined in doc/gatt-api.txt for
> GATT client role. The main highlights are:
>   - All exported objects are managed by a btd_gatt_client, each
>     in turn owned by a btd_device. All D-Bus logic is isolated to
>     src/gatt-client
>
>   - A GattService1 hierarchy is exported for a GATT service only if no
>     btd_service exists for it, i.e. no internal profile has claimed the
>     service.
>
>   - Objects are exported when the gatt-client is ready and are removed
>     when the device disconnects, as there is nothing interesting that an
>     external application can do with services without a connection and
>     before the gatt-client becomes ready.
>
>   - Objects are added/removed after "Service Changed" events.
>
> Arman Uguray (17):
>   core: gatt: Introduce gatt-client.
>   core: gatt: Export GATT services on D-Bus
>   core: gatt: Export GATT characteristics on D-Bus
>   core: gatt: Export GATT descriptors on D-Bus
>   core: gatt: Implement GattCharacteristic1.ReadValue
>   core: gatt: Implement GattDescriptor1.ReadValue
>   core: gatt: Implement GattCharacteristic1.StartNotify
>   core: gatt: Implement GattCharacteristic1.StopNotify
>   core: gatt: Expose charac. extended properties.
>   shared/gatt-client: Expose MTU.
>   core: gatt: Implement GattCharacteristic1.WriteValue.
>   core: gatt: Don't always issue read-long for ReadValue
>   core: gatt: Implement GattDescriptor1.WriteValue.
>   core: gatt: Reference count chrcs and descs.
>   core: gatt: Handle Service Changed.
>   core: device: Add function to get GATT service
>   core: gatt: Don't export claimed services
>
>  Makefile.am              |    1 +
>  src/device.c             |   51 +-
>  src/device.h             |    4 +
>  src/gatt-client.c        | 1943 ++++++++++++++++++++++++++++++++++++++++++++++
>  src/gatt-client.h        |   30 +
>  src/shared/gatt-client.c |   14 +-
>  src/shared/gatt-client.h |    8 +-
>  7 files changed, 2044 insertions(+), 7 deletions(-)
>  create mode 100644 src/gatt-client.c
>  create mode 100644 src/gatt-client.h
>
> --
> 2.2.0.rc0.207.ga3a616c

Patches 1-6 are now applied, I might continue with WriteValue but due
to the heavy changes I had done this code does not apply cleanly
anymore.


-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ 17/17] core: gatt: Don't export claimed services

[Luiz Augusto von Dentz]

Hi Arman,

On Fri, Dec 19, 2014 at 7:36 PM, Arman Uguray <armansito@chromium.org> wrote:
> GattService1 objects for services which are claimed by an internal
> profile are no longer exported. This is to prevent conflicts that
> may arise between internal plugins and external applications that
> wish to access the same GATT service.
>
> This is at least until there is a better system for managing external
> claims on GATT services and all remote profiles have been switched to
> shared/gatt, which prevents some of the conflicts that may arise from
> writes to CCC descriptors.

I was thinking that we could use gatt_db_service_set_active if it
succeed on probing but for that to work we might have to add
gatt_db_service_get_active or make gatt_db_foreach_service ignore
services not active.

> ---
>  src/gatt-client.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/src/gatt-client.c b/src/gatt-client.c
> index 16a3b3b..4f44516 100644
> --- a/src/gatt-client.c
> +++ b/src/gatt-client.c
> @@ -1772,11 +1772,36 @@ static gboolean set_chrcs_ready(gpointer user_data)
>         return FALSE;
>  }
>
> +static bool is_service_claimed(struct btd_device *dev,
> +                                               struct gatt_db_attribute *attr)
> +{
> +       uint16_t start, end;
> +       bt_uuid_t uuid;
> +       char uuid_str[MAX_LEN_UUID_STR];
> +
> +       if (!gatt_db_attribute_get_service_data(attr, &start, &end, NULL,
> +                                                                       &uuid))
> +               return false;
> +
> +       bt_uuid_to_string(&uuid, uuid_str, sizeof(uuid_str));
> +
> +       return !!btd_device_get_gatt_service(dev, uuid_str, start, end);
> +}
> +
>  static void export_service(struct gatt_db_attribute *attr, void *user_data)
>  {
>         struct btd_gatt_client *client = user_data;
>         struct service *service;
>
> +       /*
> +        * Check if a profile claimed this service and don't export it
> +        * otherwise.
> +        */
> +       if (is_service_claimed(client->device, attr)) {
> +               DBG("GATT service already claimed by an internal profile");
> +               return;
> +       }
> +
>         service = service_create(attr, client);
>         if (!service)
>                 return;
> --
> 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