[PATCH BlueZ v0 0/5] Add basic GATT descriptors

[PATCH BlueZ v0 0/5] Add basic GATT descriptors

[Claudio Takahasi]

This patchset adds the basic support for GATT descriptors (local GATT
services) implemented according to doc/gatt-api.txt

"UUID", and "Value" properties are implemented by this patchset.
"Characteristic" (parent) is still pending. "Permissions" support is
not defined yet.

Andre Guedes (1):
  gatt: Introduce helper for adding descriptors

Claudio Takahasi (4):
  gatt: Add descriptor to the database
  tools: Add testing descriptor for IAS Alert Level
  tools: Add Get/Set descriptor
  tools: Add emitting descriptor PropertiesChanged

 src/gatt-dbus.c      | 100 ++++++++++++++++++++++++++---------------
 src/gatt.c           |  54 +++++++++++++++++++---
 src/gatt.h           |  16 +++++++
 tools/gatt-service.c | 124 +++++++++++++++++++++++++++++++++++++++++++++++----
 4 files changed, 245 insertions(+), 49 deletions(-)

-- 
1.8.3.1

[PATCH BlueZ v0 1/5] gatt: Introduce helper for adding descriptors

[Claudio Takahasi]

From: Andre Guedes <andre.guedes@openbossa.org>

This patch adds btd_gatt_add_char_desc() helper which adds a
characteristic descriptor to the local attribute database.
---
 src/gatt.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++-----
 src/gatt.h | 16 ++++++++++++++++
 2 files changed, 65 insertions(+), 5 deletions(-)

diff --git a/src/gatt.c b/src/gatt.c
index 92092d3..f07effa 100644
--- a/src/gatt.c
+++ b/src/gatt.c
@@ -87,6 +87,23 @@ static struct btd_attribute *new_const_attribute(const bt_uuid_t *type,
 	return attr;
 }
 
+static struct btd_attribute *new_attribute(const bt_uuid_t *type,
+						btd_attr_read_t read_cb,
+						btd_attr_write_t write_cb)
+{
+	struct btd_attribute *attr;
+
+	attr = new0(struct btd_attribute, 1);
+	if (!attr)
+		return NULL;
+
+	attr->type = *type;
+	attr->read_cb = read_cb;
+	attr->write_cb = write_cb;
+
+	return attr;
+}
+
 static int local_database_add(uint16_t handle, struct btd_attribute *attr)
 {
 	attr->handle = handle;
@@ -170,7 +187,7 @@ struct btd_attribute *btd_gatt_add_char(const bt_uuid_t *uuid,
 	if (!char_decl)
 		goto fail;
 
-	char_value = new0(struct btd_attribute, 1);
+	char_value = new_attribute(uuid, read_cb, write_cb);
 	if (!char_value)
 		goto fail;
 
@@ -192,10 +209,6 @@ struct btd_attribute *btd_gatt_add_char(const bt_uuid_t *uuid,
 	 * implementation (external entity).
 	 */
 
-	char_value->type = *uuid;
-	char_value->read_cb = read_cb;
-	char_value->write_cb = write_cb;
-
 	if (local_database_add(next_handle, char_value) < 0)
 		/* TODO: remove declaration */
 		goto fail;
@@ -219,6 +232,37 @@ fail:
 	return NULL;
 }
 
+struct btd_attribute *btd_gatt_add_char_desc(const bt_uuid_t *uuid,
+						btd_attr_read_t read_cb,
+						btd_attr_write_t write_cb)
+{
+	struct btd_attribute *attr;
+
+	/*
+	 * From Core SPEC 4.1 page 2184:
+	 * "Characteristic descriptor declaration permissions are defined by a
+	 * higher layer profile or are implementation specific. A client shall
+	 * not assume all characteristic descriptor declarations are readable."
+	 *
+	 * The read/write callbacks presence will define the descriptor
+	 * permissions managed directly by the core. The upper layer can define
+	 * additional permissions constraints.
+	 */
+
+	attr = new_attribute(uuid, read_cb, write_cb);
+	if (!attr)
+		return NULL;
+
+	if (local_database_add(next_handle, attr) < 0) {
+		free(attr);
+		return NULL;
+	}
+
+	next_handle = next_handle + 1;
+
+	return attr;
+}
+
 void gatt_init(void)
 {
 	DBG("Starting GATT server");
diff --git a/src/gatt.h b/src/gatt.h
index 6c7f57e..da7af92 100644
--- a/src/gatt.h
+++ b/src/gatt.h
@@ -96,3 +96,19 @@ struct btd_attribute *btd_gatt_add_char(const bt_uuid_t *uuid,
 						uint8_t properties,
 						btd_attr_read_t read_cb,
 						btd_attr_write_t write_cb);
+
+/*
+ * btd_gatt_add_char_desc - Add a characteristic descriptor to the local
+ * attribute database.
+ * @uuid:	Characteristic Descriptor UUID (16-bits or 128-bits).
+ * @read_cb:	Callback that should be called once the characteristic
+ *		descriptor attribute is read.
+ * @write_cb:	Callback that should be called once the characteristic
+ *		descriptor attribute is written.
+ *
+ * Returns a reference to characteristic descriptor attribute. In case of
+ * error, NULL is returned.
+ */
+struct btd_attribute *btd_gatt_add_char_desc(const bt_uuid_t *uuid,
+						btd_attr_read_t read_cb,
+						btd_attr_write_t write_cb);
-- 
1.8.3.1

[PATCH BlueZ v0 2/5] gatt: Add descriptor to the database

[Claudio Takahasi]

Initial support for GATT characteristic descriptors. This patch adds the
characteristic descriptor attribute to the GATT local database based
on the fetched GDBusProxy objects.
---
 src/gatt-dbus.c | 100 ++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 65 insertions(+), 35 deletions(-)

diff --git a/src/gatt-dbus.c b/src/gatt-dbus.c
index 3c9dc05..1bfa21f 100644
--- a/src/gatt-dbus.c
+++ b/src/gatt-dbus.c
@@ -353,20 +353,71 @@ static int register_external_service(const struct external_app *eapp,
 	return 0;
 }
 
+static int add_char(GDBusProxy *proxy, const bt_uuid_t *uuid)
+{
+	DBusMessageIter iter;
+	struct btd_attribute *attr;
+	btd_attr_write_t write_cb;
+	btd_attr_read_t read_cb;
+	uint8_t propmask = 0;
+
+	/*
+	 * Optional property. If is not informed, read and write
+	 * procedures will be allowed. Upper-layer should handle
+	 * characteristic requirements.
+	 */
+	if (g_dbus_proxy_get_property(proxy, "Flags", &iter))
+		propmask = flags_get_bitmask(&iter);
+	else
+		propmask = GATT_CHR_PROP_WRITE_WITHOUT_RESP
+						| GATT_CHR_PROP_WRITE
+						| GATT_CHR_PROP_READ;
+	if (!propmask)
+		return -EINVAL;
+
+	if (propmask & GATT_CHR_PROP_READ)
+		read_cb = proxy_read_cb;
+	else
+		read_cb = NULL;
+
+	if (propmask & (GATT_CHR_PROP_WRITE | GATT_CHR_PROP_WRITE_WITHOUT_RESP))
+		write_cb = proxy_write_cb;
+	else
+		write_cb = NULL;
+
+	attr = btd_gatt_add_char(uuid, propmask, read_cb, write_cb);
+	if (!attr)
+		return -ENOMEM;
+
+	g_hash_table_insert(proxy_hash, attr, g_dbus_proxy_ref(proxy));
+
+	return 0;
+}
+
+static int add_char_desc(GDBusProxy *proxy, const bt_uuid_t *uuid)
+{
+	struct btd_attribute *attr;
+
+	attr = btd_gatt_add_char_desc(uuid, proxy_read_cb, proxy_write_cb);
+	if (!attr)
+		return -ENOMEM;
+
+	g_hash_table_insert(proxy_hash, attr, g_dbus_proxy_ref(proxy));
+
+	return 0;
+}
+
 static int register_external_characteristics(GSList *proxies)
 
 {
 	GSList *list;
 
 	for (list = proxies; list; list = g_slist_next(list)) {
+		GDBusProxy *proxy = list->data;
 		DBusMessageIter iter;
-		const char *str, *path;
 		bt_uuid_t uuid;
-		struct btd_attribute *attr;
-		GDBusProxy *proxy = list->data;
-		btd_attr_write_t write_cb;
-		btd_attr_read_t read_cb;
-		uint8_t propmask = 0;
+		const char *path, *iface, *str;
+		int ret;
 
 		/* Mandatory property */
 		if (!g_dbus_proxy_get_property(proxy, "UUID", &iter))
@@ -380,39 +431,18 @@ static int register_external_characteristics(GSList *proxies)
 		if (bt_string_to_uuid(&uuid, str) < 0)
 			return -EINVAL;
 
-		/*
-		 * Optional property. If is not informed, read and write
-		 * procedures will be allowed. Upper-layer should handle
-		 * characteristic requirements.
-		 */
-		if (g_dbus_proxy_get_property(proxy, "Flags", &iter))
-			propmask = flags_get_bitmask(&iter);
-		else
-			propmask = GATT_CHR_PROP_WRITE_WITHOUT_RESP
-						| GATT_CHR_PROP_WRITE
-						| GATT_CHR_PROP_READ;
-		if (!propmask)
-			return -EINVAL;
-
-		if (propmask & GATT_CHR_PROP_READ)
-			read_cb = proxy_read_cb;
-		else
-			read_cb = NULL;
+		iface = g_dbus_proxy_get_interface(proxy);
+		path = g_dbus_proxy_get_path(proxy);
 
-		if (propmask & (GATT_CHR_PROP_WRITE |
-					GATT_CHR_PROP_WRITE_WITHOUT_RESP))
-			write_cb = proxy_write_cb;
+		if (!strcmp(GATT_CHR_IFACE, iface))
+			ret = add_char(proxy, &uuid);
 		else
-			write_cb = NULL;
+			ret = add_char_desc(proxy, &uuid);
 
-		attr = btd_gatt_add_char(&uuid, propmask, read_cb, write_cb);
-		if (!attr)
-			return -EINVAL;
-
-		path = g_dbus_proxy_get_path(proxy);
-		DBG("Added GATT CHR: %s (%s)", path, str);
+		if (ret < 0)
+			return ret;
 
-		g_hash_table_insert(proxy_hash, attr, g_dbus_proxy_ref(proxy));
+		DBG("Added GATT: %s (%s)", path, str);
 	}
 
 	return 0;
-- 
1.8.3.1

[PATCH BlueZ v0 3/5] tools: Add testing descriptor for IAS Alert Level

[Claudio Takahasi]

This patch adds a testing purpose only characteristic descriptor to
allow reading and writing descriptors "Value" property.
---
 tools/gatt-service.c | 50 +++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 43 insertions(+), 7 deletions(-)

diff --git a/tools/gatt-service.c b/tools/gatt-service.c
index e319163..707b659 100644
--- a/tools/gatt-service.c
+++ b/tools/gatt-service.c
@@ -40,11 +40,15 @@
 #define GATT_MGR_IFACE			"org.bluez.GattManager1"
 #define GATT_SERVICE_IFACE		"org.bluez.GattService1"
 #define GATT_CHR_IFACE			"org.bluez.GattCharacteristic1"
+#define GATT_DESCRIPTOR_IFACE		"org.bluez.GattDescriptor1"
 
 /* Immediate Alert Service UUID */
 #define IAS_UUID			"00001802-0000-1000-8000-00805f9b34fb"
 #define ALERT_LEVEL_CHR_UUID		"00002a06-0000-1000-8000-00805f9b34fb"
 
+/* Random UUID for testing purpose */
+#define READ_WRITE_DESCRIPTOR_UUID	"8260c653-1a54-426b-9e36-e84c238bc669"
+
 static GMainLoop *main_loop;
 static GSList *services;
 static DBusConnection *connection;
@@ -66,6 +70,21 @@ static const char const *ias_alert_level_props[] = {
 						"write-without-response",
 						NULL };
 
+static gboolean desc_get_uuid(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *user_data)
+{
+	const char *uuid = user_data;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &uuid);
+
+	return TRUE;
+}
+
+static const GDBusPropertyTable desc_properties[] = {
+	{ "UUID",		"s",	desc_get_uuid },
+	{ }
+};
+
 static gboolean chr_get_uuid(const GDBusPropertyTable *property,
 					DBusMessageIter *iter, void *user_data)
 {
@@ -201,30 +220,46 @@ static void chr_iface_destroy(gpointer user_data)
 	g_free(chr);
 }
 
-static gboolean register_characteristic(DBusConnection *conn, const char *uuid,
+static gboolean register_characteristic(const char *chr_uuid,
 						const uint8_t *value, int vlen,
 						const char **props,
+						const char *desc_uuid,
 						const char *service_path)
 {
 	struct characteristic *chr;
 	static int id = 1;
-	char *path;
+	char *chr_path, *desc_path;
 	gboolean ret = TRUE;
 
-	path = g_strdup_printf("%s/characteristic%d", service_path, id++);
+	chr_path = g_strdup_printf("%s/characteristic%d", service_path, id++);
 
 	chr = g_new0(struct characteristic, 1);
 
-	chr->uuid = g_strdup(uuid);
+	chr->uuid = g_strdup(chr_uuid);
 	chr->value = g_memdup(value, vlen);
 	chr->vlen = vlen;
-	chr->path = path;
+	chr->path = chr_path;
 	chr->props = props;
 
-	if (!g_dbus_register_interface(conn, path, GATT_CHR_IFACE,
+	if (!g_dbus_register_interface(connection, chr_path, GATT_CHR_IFACE,
 					NULL, NULL, chr_properties,
 					chr, chr_iface_destroy)) {
 		printf("Couldn't register characteristic interface\n");
+		return FALSE;
+	}
+
+	if (!desc_uuid)
+		return ret;
+
+	desc_path = g_strdup_printf("%s/descriptor%d", chr_path, id++);
+	if (!g_dbus_register_interface(connection, desc_path,
+					GATT_DESCRIPTOR_IFACE,
+					NULL, NULL, desc_properties,
+					g_strdup(desc_uuid), g_free)) {
+		printf("Couldn't register descriptor interface\n");
+		g_dbus_unregister_interface(connection, chr_path,
+							GATT_CHR_IFACE);
+		g_free(desc_path);
 		ret = FALSE;
 	}
 
@@ -258,9 +293,10 @@ static void create_services()
 		return;
 
 	/* Add Alert Level Characteristic to Immediate Alert Service */
-	if (!register_characteristic(connection, ALERT_LEVEL_CHR_UUID,
+	if (!register_characteristic(ALERT_LEVEL_CHR_UUID,
 						&level, sizeof(level),
 						ias_alert_level_props,
+						READ_WRITE_DESCRIPTOR_UUID,
 						service_path)) {
 		printf("Couldn't register Alert Level characteristic (IAS)\n");
 		g_dbus_unregister_interface(connection, service_path,
-- 
1.8.3.1

[PATCH BlueZ v0 4/5] tools: Add Get/Set descriptor

[Claudio Takahasi]

This patch adds get and set callbacks to allow managing "Value" property
of the user defined descriptor related to IAS Alert Level.
---
 tools/gatt-service.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 68 insertions(+), 5 deletions(-)

diff --git a/tools/gatt-service.c b/tools/gatt-service.c
index 707b659..c304b60 100644
--- a/tools/gatt-service.c
+++ b/tools/gatt-service.c
@@ -61,6 +61,12 @@ struct characteristic {
 	const char **props;
 };
 
+struct descriptor {
+	char *uuid;
+	uint8_t *value;
+	int vlen;
+};
+
 /*
  * Alert Level support Write Without Response only. Supported
  * properties are defined at doc/gatt-api.txt. See "Flags"
@@ -73,15 +79,57 @@ static const char const *ias_alert_level_props[] = {
 static gboolean desc_get_uuid(const GDBusPropertyTable *property,
 					DBusMessageIter *iter, void *user_data)
 {
-	const char *uuid = user_data;
+	struct descriptor *desc = user_data;
 
-	dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &uuid);
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &desc->uuid);
+
+	return TRUE;
+}
+
+static gboolean desc_get_value(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *user_data)
+{
+	struct descriptor *desc = user_data;
+	DBusMessageIter array;
+
+	printf("Descriptor(%s): Get(\"Value\")\n", desc->uuid);
+
+	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
+					DBUS_TYPE_BYTE_AS_STRING, &array);
+
+	if (desc->vlen && desc->value)
+		dbus_message_iter_append_fixed_array(&array, DBUS_TYPE_BYTE,
+						&desc->value, desc->vlen);
+
+	dbus_message_iter_close_container(iter, &array);
 
 	return TRUE;
 }
 
+static void desc_set_value(const GDBusPropertyTable *property,
+				DBusMessageIter *iter,
+				GDBusPendingPropertySet id, void *user_data)
+{
+	struct descriptor *desc = user_data;
+	DBusMessageIter array;
+	const uint8_t *value;
+	int vlen;
+
+	printf("Descriptor(%s): Set(\"Value\", ...)\n", desc->uuid);
+
+	dbus_message_iter_recurse(iter, &array);
+	dbus_message_iter_get_fixed_array(&array, &value, &vlen);
+
+	g_free(desc->value);
+	desc->value = g_memdup(value, vlen);
+	desc->vlen = vlen;
+
+	g_dbus_pending_property_success(id);
+}
+
 static const GDBusPropertyTable desc_properties[] = {
 	{ "UUID",		"s",	desc_get_uuid },
+	{ "Value",		"ay",	desc_get_value, desc_set_value, NULL },
 	{ }
 };
 
@@ -101,7 +149,7 @@ static gboolean chr_get_value(const GDBusPropertyTable *property,
 	struct characteristic *chr = user_data;
 	DBusMessageIter array;
 
-	printf("Get(\"Value\")\n");
+	printf("Characteristic(%s): Get(\"Value\")\n", chr->uuid);
 
 	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
 					DBUS_TYPE_BYTE_AS_STRING, &array);
@@ -142,7 +190,7 @@ static void chr_set_value(const GDBusPropertyTable *property,
 	uint8_t *value;
 	int len;
 
-	printf("Set('Value', ...)\n");
+	printf("Characteristic(%s): Set('Value', ...)\n", chr->uuid);
 
 	if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_ARRAY) {
 		printf("Invalid value for Set('Value'...)\n");
@@ -220,6 +268,15 @@ static void chr_iface_destroy(gpointer user_data)
 	g_free(chr);
 }
 
+static void desc_iface_destroy(gpointer user_data)
+{
+	struct descriptor *desc = user_data;
+
+	g_free(desc->uuid);
+	g_free(desc->value);
+	g_free(desc);
+}
+
 static gboolean register_characteristic(const char *chr_uuid,
 						const uint8_t *value, int vlen,
 						const char **props,
@@ -227,6 +284,7 @@ static gboolean register_characteristic(const char *chr_uuid,
 						const char *service_path)
 {
 	struct characteristic *chr;
+	struct descriptor *desc;
 	static int id = 1;
 	char *chr_path, *desc_path;
 	gboolean ret = TRUE;
@@ -251,15 +309,20 @@ static gboolean register_characteristic(const char *chr_uuid,
 	if (!desc_uuid)
 		return ret;
 
+	desc = g_new0(struct descriptor, 1);
+	desc->uuid = g_strdup(desc_uuid);
+
 	desc_path = g_strdup_printf("%s/descriptor%d", chr_path, id++);
 	if (!g_dbus_register_interface(connection, desc_path,
 					GATT_DESCRIPTOR_IFACE,
 					NULL, NULL, desc_properties,
-					g_strdup(desc_uuid), g_free)) {
+					desc, desc_iface_destroy)) {
 		printf("Couldn't register descriptor interface\n");
 		g_dbus_unregister_interface(connection, chr_path,
 							GATT_CHR_IFACE);
 		g_free(desc_path);
+		desc_iface_destroy(desc);
+
 		ret = FALSE;
 	}
 
-- 
1.8.3.1

[PATCH BlueZ v0 5/5] tools: Add emitting descriptor PropertiesChanged

[Claudio Takahasi]

This patch forces emitting PropertiesChanged signal when the descriptor
Value changes. Internally, BlueZ tracks the signal emitted to update
the GDBusProxy properties.
---
 tools/gatt-service.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/tools/gatt-service.c b/tools/gatt-service.c
index c304b60..c4c358c 100644
--- a/tools/gatt-service.c
+++ b/tools/gatt-service.c
@@ -63,6 +63,7 @@ struct characteristic {
 
 struct descriptor {
 	char *uuid;
+	char *path;
 	uint8_t *value;
 	int vlen;
 };
@@ -125,6 +126,10 @@ static void desc_set_value(const GDBusPropertyTable *property,
 	desc->vlen = vlen;
 
 	g_dbus_pending_property_success(id);
+
+	g_dbus_emit_property_changed(connection, desc->path,
+					GATT_DESCRIPTOR_IFACE, "Value");
+
 }
 
 static const GDBusPropertyTable desc_properties[] = {
@@ -274,6 +279,7 @@ static void desc_iface_destroy(gpointer user_data)
 
 	g_free(desc->uuid);
 	g_free(desc->value);
+	g_free(desc->path);
 	g_free(desc);
 }
 
@@ -309,10 +315,12 @@ static gboolean register_characteristic(const char *chr_uuid,
 	if (!desc_uuid)
 		return ret;
 
+	desc_path = g_strdup_printf("%s/descriptor%d", chr_path, id++);
+
 	desc = g_new0(struct descriptor, 1);
 	desc->uuid = g_strdup(desc_uuid);
+	desc->path = desc_path;
 
-	desc_path = g_strdup_printf("%s/descriptor%d", chr_path, id++);
 	if (!g_dbus_register_interface(connection, desc_path,
 					GATT_DESCRIPTOR_IFACE,
 					NULL, NULL, desc_properties,
@@ -320,7 +328,6 @@ static gboolean register_characteristic(const char *chr_uuid,
 		printf("Couldn't register descriptor interface\n");
 		g_dbus_unregister_interface(connection, chr_path,
 							GATT_CHR_IFACE);
-		g_free(desc_path);
 		desc_iface_destroy(desc);
 
 		ret = FALSE;
-- 
1.8.3.1

Re: [PATCH BlueZ v0 3/5] tools: Add testing descriptor for IAS Alert Level

[Johan Hedberg]

Hi Claudio,

On Fri, Mar 28, 2014, Claudio Takahasi wrote:
> This patch adds a testing purpose only characteristic descriptor to
> allow reading and writing descriptors "Value" property.
> ---
>  tools/gatt-service.c | 50 +++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 43 insertions(+), 7 deletions(-)

I've applied the first two patches but noticed something with this one:

> +static gboolean register_characteristic(const char *chr_uuid,
>  						const uint8_t *value, int vlen,
>  						const char **props,
> +						const char *desc_uuid,
>  						const char *service_path)
>  {
>  	struct characteristic *chr;
>  	static int id = 1;
> -	char *path;
> +	char *chr_path, *desc_path;
>  	gboolean ret = TRUE;
>  
> -	path = g_strdup_printf("%s/characteristic%d", service_path, id++);
> +	chr_path = g_strdup_printf("%s/characteristic%d", service_path, id++);
>  
>  	chr = g_new0(struct characteristic, 1);
>  
> -	chr->uuid = g_strdup(uuid);
> +	chr->uuid = g_strdup(chr_uuid);
>  	chr->value = g_memdup(value, vlen);
>  	chr->vlen = vlen;
> -	chr->path = path;
> +	chr->path = chr_path;
>  	chr->props = props;
>  
> -	if (!g_dbus_register_interface(conn, path, GATT_CHR_IFACE,
> +	if (!g_dbus_register_interface(connection, chr_path, GATT_CHR_IFACE,
>  					NULL, NULL, chr_properties,
>  					chr, chr_iface_destroy)) {
>  		printf("Couldn't register characteristic interface\n");
> +		return FALSE;
> +	}
> +
> +	if (!desc_uuid)
> +		return ret;
> +
> +	desc_path = g_strdup_printf("%s/descriptor%d", chr_path, id++);
> +	if (!g_dbus_register_interface(connection, desc_path,
> +					GATT_DESCRIPTOR_IFACE,
> +					NULL, NULL, desc_properties,
> +					g_strdup(desc_uuid), g_free)) {
> +		printf("Couldn't register descriptor interface\n");
> +		g_dbus_unregister_interface(connection, chr_path,
> +							GATT_CHR_IFACE);
> +		g_free(desc_path);
>  		ret = FALSE;
>  	}
>  

Looks like you're leaking desc_path here if register_interface succeeds.
In general it seems a bit unnecessary to go for a dynamically allocated
approach since you only need the variable in the same function. Probably
a stack variable and snprintf would do an equally good or better job.

Johan

Re: [PATCH BlueZ v0 3/5] tools: Add testing descriptor for IAS Alert Level

[Claudio Takahasi]

Hi Johan:

On Fri, Mar 28, 2014 at 2:51 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Claudio,
>
> On Fri, Mar 28, 2014, Claudio Takahasi wrote:
>> This patch adds a testing purpose only characteristic descriptor to
>> allow reading and writing descriptors "Value" property.
>> ---
>>  tools/gatt-service.c | 50 +++++++++++++++++++++++++++++++++++++++++++-------
>>  1 file changed, 43 insertions(+), 7 deletions(-)
>
> I've applied the first two patches but noticed something with this one:
>
>> +static gboolean register_characteristic(const char *chr_uuid,
>>                                               const uint8_t *value, int vlen,
>>                                               const char **props,
>> +                                             const char *desc_uuid,
>>                                               const char *service_path)
>>  {
>>       struct characteristic *chr;
>>       static int id = 1;
>> -     char *path;
>> +     char *chr_path, *desc_path;
>>       gboolean ret = TRUE;
>>
>> -     path = g_strdup_printf("%s/characteristic%d", service_path, id++);
>> +     chr_path = g_strdup_printf("%s/characteristic%d", service_path, id++);
>>
>>       chr = g_new0(struct characteristic, 1);
>>
>> -     chr->uuid = g_strdup(uuid);
>> +     chr->uuid = g_strdup(chr_uuid);
>>       chr->value = g_memdup(value, vlen);
>>       chr->vlen = vlen;
>> -     chr->path = path;
>> +     chr->path = chr_path;
>>       chr->props = props;
>>
>> -     if (!g_dbus_register_interface(conn, path, GATT_CHR_IFACE,
>> +     if (!g_dbus_register_interface(connection, chr_path, GATT_CHR_IFACE,
>>                                       NULL, NULL, chr_properties,
>>                                       chr, chr_iface_destroy)) {
>>               printf("Couldn't register characteristic interface\n");
>> +             return FALSE;
>> +     }
>> +
>> +     if (!desc_uuid)
>> +             return ret;
>> +
>> +     desc_path = g_strdup_printf("%s/descriptor%d", chr_path, id++);
>> +     if (!g_dbus_register_interface(connection, desc_path,
>> +                                     GATT_DESCRIPTOR_IFACE,
>> +                                     NULL, NULL, desc_properties,
>> +                                     g_strdup(desc_uuid), g_free)) {
>> +             printf("Couldn't register descriptor interface\n");
>> +             g_dbus_unregister_interface(connection, chr_path,
>> +                                                     GATT_CHR_IFACE);
>> +             g_free(desc_path);
>>               ret = FALSE;
>>       }
>>
>
> Looks like you're leaking desc_path here if register_interface succeeds.
> In general it seems a bit unnecessary to go for a dynamically allocated
> approach since you only need the variable in the same function. Probably
> a stack variable and snprintf would do an equally good or better job.
>
> Johan

OK. I will send a new patch fixing this memory leak.

In this case specifically, I prefer to use dynamically allocated
memory because in the next patch, the object path will be necessary to
emit the PropertiesChanged signal.

Regards,
Claudio

[PATCH BlueZ v1 0/3] Add basic GATT descriptors

[Claudio Takahasi]

Remaining patches of "[PATCH BlueZ v0 0/5] Add basic GATT descriptors"

This patchset adds the basic support for GATT descriptors (local GATT
services) implemented according to doc/gatt-api.txt

"UUID", and "Value" properties are implemented by this patchset.
"Characteristic" (parent) is still pending. "Permissions" support is
not defined yet.

Changes v0-v1:
  * Fixed memory leak of descriptor's object path

Claudio Takahasi (3):
  tools: Add testing descriptor for IAS Alert Level
  tools: Add Get/Set descriptor
  tools: Add emitting descriptor PropertiesChanged

 tools/gatt-service.c | 128 +++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 114 insertions(+), 14 deletions(-)

-- 
1.8.3.1

[PATCH BlueZ v1 1/3] tools: Add testing descriptor for IAS Alert Level

[Claudio Takahasi]

This patch adds a testing purpose only characteristic descriptor to
allow reading and writing descriptor's "Value" property.
---
 tools/gatt-service.c | 53 +++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 44 insertions(+), 9 deletions(-)

diff --git a/tools/gatt-service.c b/tools/gatt-service.c
index e319163..d2859f8 100644
--- a/tools/gatt-service.c
+++ b/tools/gatt-service.c
@@ -40,11 +40,15 @@
 #define GATT_MGR_IFACE			"org.bluez.GattManager1"
 #define GATT_SERVICE_IFACE		"org.bluez.GattService1"
 #define GATT_CHR_IFACE			"org.bluez.GattCharacteristic1"
+#define GATT_DESCRIPTOR_IFACE		"org.bluez.GattDescriptor1"
 
 /* Immediate Alert Service UUID */
 #define IAS_UUID			"00001802-0000-1000-8000-00805f9b34fb"
 #define ALERT_LEVEL_CHR_UUID		"00002a06-0000-1000-8000-00805f9b34fb"
 
+/* Random UUID for testing purpose */
+#define READ_WRITE_DESCRIPTOR_UUID	"8260c653-1a54-426b-9e36-e84c238bc669"
+
 static GMainLoop *main_loop;
 static GSList *services;
 static DBusConnection *connection;
@@ -66,6 +70,21 @@ static const char const *ias_alert_level_props[] = {
 						"write-without-response",
 						NULL };
 
+static gboolean desc_get_uuid(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *user_data)
+{
+	const char *uuid = user_data;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &uuid);
+
+	return TRUE;
+}
+
+static const GDBusPropertyTable desc_properties[] = {
+	{ "UUID",		"s",	desc_get_uuid },
+	{ }
+};
+
 static gboolean chr_get_uuid(const GDBusPropertyTable *property,
 					DBusMessageIter *iter, void *user_data)
 {
@@ -201,33 +220,48 @@ static void chr_iface_destroy(gpointer user_data)
 	g_free(chr);
 }
 
-static gboolean register_characteristic(DBusConnection *conn, const char *uuid,
+static gboolean register_characteristic(const char *chr_uuid,
 						const uint8_t *value, int vlen,
 						const char **props,
+						const char *desc_uuid,
 						const char *service_path)
 {
 	struct characteristic *chr;
 	static int id = 1;
-	char *path;
+	char *desc_path;
 	gboolean ret = TRUE;
 
-	path = g_strdup_printf("%s/characteristic%d", service_path, id++);
-
 	chr = g_new0(struct characteristic, 1);
-
-	chr->uuid = g_strdup(uuid);
+	chr->uuid = g_strdup(chr_uuid);
 	chr->value = g_memdup(value, vlen);
 	chr->vlen = vlen;
-	chr->path = path;
 	chr->props = props;
+	chr->path = g_strdup_printf("%s/characteristic%d", service_path, id++);
 
-	if (!g_dbus_register_interface(conn, path, GATT_CHR_IFACE,
+	if (!g_dbus_register_interface(connection, chr->path, GATT_CHR_IFACE,
 					NULL, NULL, chr_properties,
 					chr, chr_iface_destroy)) {
 		printf("Couldn't register characteristic interface\n");
+		chr_iface_destroy(chr);
+		return FALSE;
+	}
+
+	if (!desc_uuid)
+		return TRUE;
+
+	desc_path = g_strdup_printf("%s/descriptor%d", chr->path, id++);
+	if (!g_dbus_register_interface(connection, desc_path,
+					GATT_DESCRIPTOR_IFACE,
+					NULL, NULL, desc_properties,
+					g_strdup(desc_uuid), g_free)) {
+		printf("Couldn't register descriptor interface\n");
+		g_dbus_unregister_interface(connection, chr->path,
+							GATT_CHR_IFACE);
 		ret = FALSE;
 	}
 
+	g_free(desc_path);
+
 	return ret;
 }
 
@@ -258,9 +292,10 @@ static void create_services()
 		return;
 
 	/* Add Alert Level Characteristic to Immediate Alert Service */
-	if (!register_characteristic(connection, ALERT_LEVEL_CHR_UUID,
+	if (!register_characteristic(ALERT_LEVEL_CHR_UUID,
 						&level, sizeof(level),
 						ias_alert_level_props,
+						READ_WRITE_DESCRIPTOR_UUID,
 						service_path)) {
 		printf("Couldn't register Alert Level characteristic (IAS)\n");
 		g_dbus_unregister_interface(connection, service_path,
-- 
1.8.3.1

[PATCH BlueZ v1 2/3] tools: Add Get/Set descriptor

[Claudio Takahasi]

This patch adds get and set callbacks to allow managing "Value" property
of the user defined descriptor related to IAS Alert Level.
---
 tools/gatt-service.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 68 insertions(+), 5 deletions(-)

diff --git a/tools/gatt-service.c b/tools/gatt-service.c
index d2859f8..ee37ef3 100644
--- a/tools/gatt-service.c
+++ b/tools/gatt-service.c
@@ -61,6 +61,12 @@ struct characteristic {
 	const char **props;
 };
 
+struct descriptor {
+	char *uuid;
+	uint8_t *value;
+	int vlen;
+};
+
 /*
  * Alert Level support Write Without Response only. Supported
  * properties are defined at doc/gatt-api.txt. See "Flags"
@@ -73,15 +79,57 @@ static const char const *ias_alert_level_props[] = {
 static gboolean desc_get_uuid(const GDBusPropertyTable *property,
 					DBusMessageIter *iter, void *user_data)
 {
-	const char *uuid = user_data;
+	struct descriptor *desc = user_data;
 
-	dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &uuid);
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &desc->uuid);
+
+	return TRUE;
+}
+
+static gboolean desc_get_value(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *user_data)
+{
+	struct descriptor *desc = user_data;
+	DBusMessageIter array;
+
+	printf("Descriptor(%s): Get(\"Value\")\n", desc->uuid);
+
+	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
+					DBUS_TYPE_BYTE_AS_STRING, &array);
+
+	if (desc->vlen && desc->value)
+		dbus_message_iter_append_fixed_array(&array, DBUS_TYPE_BYTE,
+						&desc->value, desc->vlen);
+
+	dbus_message_iter_close_container(iter, &array);
 
 	return TRUE;
 }
 
+static void desc_set_value(const GDBusPropertyTable *property,
+				DBusMessageIter *iter,
+				GDBusPendingPropertySet id, void *user_data)
+{
+	struct descriptor *desc = user_data;
+	DBusMessageIter array;
+	const uint8_t *value;
+	int vlen;
+
+	printf("Descriptor(%s): Set(\"Value\", ...)\n", desc->uuid);
+
+	dbus_message_iter_recurse(iter, &array);
+	dbus_message_iter_get_fixed_array(&array, &value, &vlen);
+
+	g_free(desc->value);
+	desc->value = g_memdup(value, vlen);
+	desc->vlen = vlen;
+
+	g_dbus_pending_property_success(id);
+}
+
 static const GDBusPropertyTable desc_properties[] = {
 	{ "UUID",		"s",	desc_get_uuid },
+	{ "Value",		"ay",	desc_get_value, desc_set_value, NULL },
 	{ }
 };
 
@@ -101,7 +149,7 @@ static gboolean chr_get_value(const GDBusPropertyTable *property,
 	struct characteristic *chr = user_data;
 	DBusMessageIter array;
 
-	printf("Get(\"Value\")\n");
+	printf("Characteristic(%s): Get(\"Value\")\n", chr->uuid);
 
 	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
 					DBUS_TYPE_BYTE_AS_STRING, &array);
@@ -142,7 +190,7 @@ static void chr_set_value(const GDBusPropertyTable *property,
 	uint8_t *value;
 	int len;
 
-	printf("Set('Value', ...)\n");
+	printf("Characteristic(%s): Set('Value', ...)\n", chr->uuid);
 
 	if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_ARRAY) {
 		printf("Invalid value for Set('Value'...)\n");
@@ -220,6 +268,15 @@ static void chr_iface_destroy(gpointer user_data)
 	g_free(chr);
 }
 
+static void desc_iface_destroy(gpointer user_data)
+{
+	struct descriptor *desc = user_data;
+
+	g_free(desc->uuid);
+	g_free(desc->value);
+	g_free(desc);
+}
+
 static gboolean register_characteristic(const char *chr_uuid,
 						const uint8_t *value, int vlen,
 						const char **props,
@@ -227,6 +284,7 @@ static gboolean register_characteristic(const char *chr_uuid,
 						const char *service_path)
 {
 	struct characteristic *chr;
+	struct descriptor *desc;
 	static int id = 1;
 	char *desc_path;
 	gboolean ret = TRUE;
@@ -249,14 +307,19 @@ static gboolean register_characteristic(const char *chr_uuid,
 	if (!desc_uuid)
 		return TRUE;
 
+	desc = g_new0(struct descriptor, 1);
+	desc->uuid = g_strdup(desc_uuid);
+
 	desc_path = g_strdup_printf("%s/descriptor%d", chr->path, id++);
 	if (!g_dbus_register_interface(connection, desc_path,
 					GATT_DESCRIPTOR_IFACE,
 					NULL, NULL, desc_properties,
-					g_strdup(desc_uuid), g_free)) {
+					desc, desc_iface_destroy)) {
 		printf("Couldn't register descriptor interface\n");
 		g_dbus_unregister_interface(connection, chr->path,
 							GATT_CHR_IFACE);
+
+		desc_iface_destroy(desc);
 		ret = FALSE;
 	}
 
-- 
1.8.3.1

[PATCH BlueZ v1 3/3] tools: Add emitting descriptor PropertiesChanged

[Claudio Takahasi]

This patch forces emitting PropertiesChanged signal when the descriptor
Value changes. Internally, BlueZ tracks the signal emitted to update
the GDBusProxy properties.
---
 tools/gatt-service.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/tools/gatt-service.c b/tools/gatt-service.c
index ee37ef3..6bca404 100644
--- a/tools/gatt-service.c
+++ b/tools/gatt-service.c
@@ -63,6 +63,7 @@ struct characteristic {
 
 struct descriptor {
 	char *uuid;
+	char *path;
 	uint8_t *value;
 	int vlen;
 };
@@ -125,6 +126,10 @@ static void desc_set_value(const GDBusPropertyTable *property,
 	desc->vlen = vlen;
 
 	g_dbus_pending_property_success(id);
+
+	g_dbus_emit_property_changed(connection, desc->path,
+					GATT_DESCRIPTOR_IFACE, "Value");
+
 }
 
 static const GDBusPropertyTable desc_properties[] = {
@@ -274,6 +279,7 @@ static void desc_iface_destroy(gpointer user_data)
 
 	g_free(desc->uuid);
 	g_free(desc->value);
+	g_free(desc->path);
 	g_free(desc);
 }
 
@@ -286,8 +292,6 @@ static gboolean register_characteristic(const char *chr_uuid,
 	struct characteristic *chr;
 	struct descriptor *desc;
 	static int id = 1;
-	char *desc_path;
-	gboolean ret = TRUE;
 
 	chr = g_new0(struct characteristic, 1);
 	chr->uuid = g_strdup(chr_uuid);
@@ -309,9 +313,9 @@ static gboolean register_characteristic(const char *chr_uuid,
 
 	desc = g_new0(struct descriptor, 1);
 	desc->uuid = g_strdup(desc_uuid);
+	desc->path = g_strdup_printf("%s/descriptor%d", chr->path, id++);
 
-	desc_path = g_strdup_printf("%s/descriptor%d", chr->path, id++);
-	if (!g_dbus_register_interface(connection, desc_path,
+	if (!g_dbus_register_interface(connection, desc->path,
 					GATT_DESCRIPTOR_IFACE,
 					NULL, NULL, desc_properties,
 					desc, desc_iface_destroy)) {
@@ -320,12 +324,10 @@ static gboolean register_characteristic(const char *chr_uuid,
 							GATT_CHR_IFACE);
 
 		desc_iface_destroy(desc);
-		ret = FALSE;
+		return FALSE;
 	}
 
-	g_free(desc_path);
-
-	return ret;
+	return TRUE;
 }
 
 static char *register_service(const char *uuid)
-- 
1.8.3.1

Re: [PATCH BlueZ v1 0/3] Add basic GATT descriptors

[Johan Hedberg]

Hi Claudio,

On Mon, Mar 31, 2014, Claudio Takahasi wrote:
> Remaining patches of "[PATCH BlueZ v0 0/5] Add basic GATT descriptors"
> 
> This patchset adds the basic support for GATT descriptors (local GATT
> services) implemented according to doc/gatt-api.txt
> 
> "UUID", and "Value" properties are implemented by this patchset.
> "Characteristic" (parent) is still pending. "Permissions" support is
> not defined yet.
> 
> Changes v0-v1:
>   * Fixed memory leak of descriptor's object path
> 
> Claudio Takahasi (3):
>   tools: Add testing descriptor for IAS Alert Level
>   tools: Add Get/Set descriptor
>   tools: Add emitting descriptor PropertiesChanged
> 
>  tools/gatt-service.c | 128 +++++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 114 insertions(+), 14 deletions(-)

All three patches have been applied. Thanks.

Johan