[PATCH v2 00/13] Add Heart Rate Service

[PATCH v2 00/13] Add Heart Rate Service

[Rafal Garbat]

Add support for GATT Client Heart Rate Service.

This patchset adds Heart Rate client service, exposed on Heart Rate 
DBus API by Santiago Carot-Nemesio <sancane@gmail.com>. It allows 
registering and unregistering watchers, handling measurement notifications
and reseting Control Point. It bases on Thermometer service. Python 
test script is included. Tested against Polar H7 Heart Rate Sensor.

Patchset v2 fixes some bugs in parsing measurement notification
and checking pdu length. Test script has been extended by 
Reset cmd support. Enabling measurement has been disabled by default
at the new connection.

Rafal Garbat (12):
  heartrate: Add Heart Rate Service GATT client skeleton
  heartrate: Add conn logic and attio callbacks
  heartrate: Discover Characteristic Descriptors
  heartrate: Process Heart Rate Descriptors
  heartrate: Add DBus connection logic
  heartrate: Process Heart Rate Characteristics
  heartrate: Add notification support
  heartrate: Process measurements
  heartrate: Add support for Control Point Reset
  heartrate: Add GetProperties method handle
  heartrate: Add org.bluez.HeartRateWatcher iface to default policy
  heartrate: Add Heart Rate test script

Santiago Carot-Nemesio (1):
  Heart Rate Profile API

 Makefile.am                    |   10 +-
 Makefile.tools                 |    4 +-
 doc/heartrate-api.txt          |   74 ++++
 lib/uuid.h                     |    5 +
 profiles/heartrate/heartrate.c |  868 ++++++++++++++++++++++++++++++++++++++++
 profiles/heartrate/heartrate.h |   25 ++
 profiles/heartrate/main.c      |   68 ++++
 profiles/heartrate/manager.c   |   93 +++++
 profiles/heartrate/manager.h   |   24 ++
 src/bluetooth.conf             |    1 +
 test/test-heartrate            |   91 +++++
 11 files changed, 1259 insertions(+), 4 deletions(-)
 create mode 100644 doc/heartrate-api.txt
 create mode 100644 profiles/heartrate/heartrate.c
 create mode 100644 profiles/heartrate/heartrate.h
 create mode 100644 profiles/heartrate/main.c
 create mode 100644 profiles/heartrate/manager.c
 create mode 100644 profiles/heartrate/manager.h
 create mode 100755 test/test-heartrate

-- 
1.7.9.5

[PATCH v2 01/13] Heart Rate Profile API

[Rafal Garbat]

From: Santiago Carot-Nemesio <sancane@gmail.com>

---
 doc/heartrate-api.txt |   74 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)
 create mode 100644 doc/heartrate-api.txt

diff --git a/doc/heartrate-api.txt b/doc/heartrate-api.txt
new file mode 100644
index 0000000..2738e20
--- /dev/null
+++ b/doc/heartrate-api.txt
@@ -0,0 +1,74 @@
+BlueZ D-Bus Heart Rate API description
+****************************************
+
+Copyright (C) 2012	Santiago Carot-Nemesio <sancane@gmail.com>
+
+Heart Rate Profile hierarchy
+============================
+
+Service		org.bluez
+Interface	org.bluez.HeartRate
+Object path	[variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
+
+Methods		dict GetProperties()
+
+			Returns all properties for the interface. See the
+			Properties section for the available properties.
+
+		RegisterWatcher(object agent)
+
+			Registers a watcher to monitor heart rate measurements.
+
+			Possible Errors: org.bluez.Error.InvalidArguments
+
+		UnregisterWatcher(object agent)
+
+			Unregisters a watcher.
+
+			Possible Errors: org.bluez.Error.InvalidArguments
+					org.bluez.Error.NotFound
+
+		Reset()
+
+			Restart the accumulation of energy expended from zero.
+
+			Possible Errors: org.bluez.Error.NotSupported
+
+Properties	boolean ResetSupported [readonly]
+
+			True if energy expended is supported.
+
+Heart Rate Watcher hierarchy
+
+============================
+Service		unique name
+Interface	org.bluez.HeartRateWatcher
+Object path	freely definable
+
+Methods		void MeasurementReceived(dict measure)
+
+			This callback is called whenever a heart rate
+			measurement is received from the heart rate device.
+			The unit for the Value is expressed in beats per
+			minute (bpm). The energy field is optional and
+			represents the accumulated energy expended in
+			kilo Joules since last time it was reset. Furthermore,
+			the device will be automatically reset when it
+			is needed.
+			The Contact field, if present, indicates
+			that the device supports contact sensor, besides it
+			will be true if skin contact is detected. The optional
+			interval field is an array containing RR-Interval
+			values which represent the time between two R-Wave
+			detections, where the RR-Interval value 0 is older
+			than the value 1 and so on.
+
+			Dict is defined as below:
+			{
+				"Value" : uint16,
+				"Energy" : uint16,
+				"Contact" : boolean,
+				"Location" : ("Other", "Chest", "Wrist","Finger",
+					"Hand", "Earlobe", "Foot"),
+				"Interval" : array{uint16}
+			}
-- 
1.7.9.5

[PATCH v2 02/13] heartrate: Add Heart Rate Service GATT client skeleton

[Rafal Garbat]

Add initial support for the Heart Rate Service GATT
Client side.

---
 Makefile.am                    |   10 ++++-
 lib/uuid.h                     |    2 +
 profiles/heartrate/heartrate.c |   84 ++++++++++++++++++++++++++++++++++++++++
 profiles/heartrate/heartrate.h |   24 ++++++++++++
 profiles/heartrate/main.c      |   52 +++++++++++++++++++++++++
 profiles/heartrate/manager.c   |   60 ++++++++++++++++++++++++++++
 profiles/heartrate/manager.h   |   24 ++++++++++++
 7 files changed, 254 insertions(+), 2 deletions(-)
 create mode 100644 profiles/heartrate/heartrate.c
 create mode 100644 profiles/heartrate/heartrate.h
 create mode 100644 profiles/heartrate/main.c
 create mode 100644 profiles/heartrate/manager.c
 create mode 100644 profiles/heartrate/manager.h

diff --git a/Makefile.am b/Makefile.am
index 45a811c..594ea56 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -211,7 +211,8 @@ builtin_sources += profiles/health/hdp_main.c profiles/health/hdp_types.h \
 endif
 
 if GATTMODULES
-builtin_modules += thermometer alert time gatt_example proximity deviceinfo
+builtin_modules += thermometer alert time gatt_example proximity deviceinfo \
+	heartrate
 builtin_sources += profiles/thermometer/main.c \
 			profiles/thermometer/manager.h \
 			profiles/thermometer/manager.c \
@@ -237,7 +238,12 @@ builtin_sources += profiles/thermometer/main.c \
 			profiles/deviceinfo/manager.h \
 			profiles/deviceinfo/manager.c \
 			profiles/deviceinfo/deviceinfo.h \
-			profiles/deviceinfo/deviceinfo.c
+			profiles/deviceinfo/deviceinfo.c \
+			profiles/heartrate/main.c \
+			profiles/heartrate/manager.c \
+			profiles/heartrate/manager.h \
+			profiles/heartrate/heartrate.c \
+			profiles/heartrate/heartrate.h
 endif
 
 builtin_modules += formfactor
diff --git a/lib/uuid.h b/lib/uuid.h
index 2c2b351..99b88cc 100644
--- a/lib/uuid.h
+++ b/lib/uuid.h
@@ -63,6 +63,8 @@ extern "C" {
 
 #define SAP_UUID		"0000112D-0000-1000-8000-00805f9b34fb"
 
+#define HEART_RATE_UUID		"0000180d-0000-1000-8000-00805f9b34fb"
+
 #define HEALTH_THERMOMETER_UUID		"00001809-0000-1000-8000-00805f9b34fb"
 #define TEMPERATURE_MEASUREMENT_UUID	"00002a1c-0000-1000-8000-00805f9b34fb"
 #define TEMPERATURE_TYPE_UUID		"00002a1d-0000-1000-8000-00805f9b34fb"
diff --git a/profiles/heartrate/heartrate.c b/profiles/heartrate/heartrate.c
new file mode 100644
index 0000000..9bd93c5
--- /dev/null
+++ b/profiles/heartrate/heartrate.c
@@ -0,0 +1,84 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2012 Tieto Poland
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <errno.h>
+#include <glib.h>
+#include <bluetooth/uuid.h>
+
+#include "adapter.h"
+#include "device.h"
+#include "heartrate.h"
+
+struct heartrate {
+	struct btd_device	*dev;  /*Device reference*/
+};
+
+static GSList *hr_servers = NULL;
+
+static gint cmp_device(gconstpointer a, gconstpointer b)
+{
+	const struct heartrate *hr = a;
+	const struct btd_device *dev = b;
+
+	if (dev == hr->dev)
+		return 0;
+
+	return -1;
+}
+
+static void heartrate_destroy(gpointer user_data)
+{
+	struct heartrate *hr = user_data;
+
+	btd_device_unref(hr->dev);
+	g_free(hr);
+}
+
+int heartrate_register(struct btd_device *device)
+{
+	struct heartrate *hr;
+
+	hr = g_new0(struct heartrate, 1);
+	hr->dev = btd_device_ref(device);
+
+	hr_servers = g_slist_prepend(hr_servers, hr);
+
+	return 0;
+}
+void heartrate_unregister(struct btd_device *device)
+{
+	struct heartrate *hr;
+	GSList *l;
+
+	l = g_slist_find_custom(hr_servers, device, cmp_device);
+	if (l == NULL)
+		return;
+
+	hr = l->data;
+	hr_servers = g_slist_remove(hr_servers, hr);
+
+	heartrate_destroy(hr);
+}
diff --git a/profiles/heartrate/heartrate.h b/profiles/heartrate/heartrate.h
new file mode 100644
index 0000000..8d4271c
--- /dev/null
+++ b/profiles/heartrate/heartrate.h
@@ -0,0 +1,24 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2012 Tieto Poland
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+int heartrate_register(struct btd_device *device);
+void heartrate_unregister(struct btd_device *device);
diff --git a/profiles/heartrate/main.c b/profiles/heartrate/main.c
new file mode 100644
index 0000000..40f34bc
--- /dev/null
+++ b/profiles/heartrate/main.c
@@ -0,0 +1,52 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2012 Tieto Poland
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdint.h>
+#include <glib.h>
+#include <errno.h>
+
+#include "plugin.h"
+#include "manager.h"
+#include "hcid.h"
+#include "log.h"
+
+static int heartrate_init(void)
+{
+	if (!main_opts.gatt_enabled) {
+		DBG("GATT is disabled");
+		return -ENOTSUP;
+	}
+
+	return heartrate_manager_init();
+}
+
+static void heartrate_exit(void)
+{
+	heartrate_manager_exit();
+}
+
+BLUETOOTH_PLUGIN_DEFINE(heartrate, VERSION, BLUETOOTH_PLUGIN_PRIORITY_DEFAULT,
+					heartrate_init, heartrate_exit)
diff --git a/profiles/heartrate/manager.c b/profiles/heartrate/manager.c
new file mode 100644
index 0000000..da6c7ef
--- /dev/null
+++ b/profiles/heartrate/manager.c
@@ -0,0 +1,60 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2012 Tieto Poland
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#include <gdbus.h>
+#include <errno.h>
+#include <bluetooth/uuid.h>
+
+#include "adapter.h"
+#include "device.h"
+#include "att.h"
+#include "gattrib.h"
+#include "gatt.h"
+#include "heartrate.h"
+#include "manager.h"
+
+static int heartrate_driver_probe(struct btd_device *device, GSList *uuids)
+{
+	return heartrate_register(device);
+}
+
+static void heartrate_driver_remove(struct btd_device *device)
+{
+	heartrate_unregister(device);
+}
+
+static struct btd_device_driver heartrate_device_driver = {
+	.name	= "heart-rate-driver",
+	.uuids	= BTD_UUIDS(HEART_RATE_UUID),
+	.probe	= heartrate_driver_probe,
+	.remove	= heartrate_driver_remove
+};
+
+int heartrate_manager_init(void)
+{
+	return btd_register_device_driver(&heartrate_device_driver);
+}
+
+void heartrate_manager_exit(void)
+{
+	btd_unregister_device_driver(&heartrate_device_driver);
+}
diff --git a/profiles/heartrate/manager.h b/profiles/heartrate/manager.h
new file mode 100644
index 0000000..de799f6
--- /dev/null
+++ b/profiles/heartrate/manager.h
@@ -0,0 +1,24 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2012 Tieto Poland
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+int heartrate_manager_init(void);
+void heartrate_manager_exit(void);
-- 
1.7.9.5

[PATCH v2 03/13] heartrate: Add conn logic and attio callbacks

[Rafal Garbat]

Handle connection to the remote device when the
Heart Rate plugin is loaded.

---
 profiles/heartrate/heartrate.c |   86 +++++++++++++++++++++++++++++++++++++++-
 profiles/heartrate/heartrate.h |    2 +-
 profiles/heartrate/manager.c   |   22 +++++++++-
 3 files changed, 107 insertions(+), 3 deletions(-)

diff --git a/profiles/heartrate/heartrate.c b/profiles/heartrate/heartrate.c
index 9bd93c5..1114cff 100644
--- a/profiles/heartrate/heartrate.c
+++ b/profiles/heartrate/heartrate.c
@@ -30,10 +30,24 @@
 
 #include "adapter.h"
 #include "device.h"
+#include "gattrib.h"
+#include "attio.h"
+#include "att.h"
+#include "gatt.h"
 #include "heartrate.h"
+#include "log.h"
 
 struct heartrate {
 	struct btd_device	*dev;  /*Device reference*/
+	GAttrib			*attrib; /*GATT connection*/
+	guint			attioid; /*ATT watcher id*/
+	struct att_range	*svc_range; /*Heart Rate range*/
+	GSList			*chars; /*Characteristics*/
+};
+
+struct characteristic {
+	struct gatt_char	attr; /*Characteristic*/
+	struct heartrate	*hr; /*Parent Heart Rate Service*/
 };
 
 static GSList *hr_servers = NULL;
@@ -53,19 +67,89 @@ static void heartrate_destroy(gpointer user_data)
 {
 	struct heartrate *hr = user_data;
 
+	if (hr->chars != NULL)
+		g_slist_free_full(hr->chars, g_free);
+
+	if (hr->attioid > 0)
+		btd_device_remove_attio_callback(hr->dev, hr->attioid);
+
+	if (hr->attrib != NULL)
+		g_attrib_unref(hr->attrib);
+
 	btd_device_unref(hr->dev);
+	g_free(hr->svc_range);
 	g_free(hr);
+
+}
+
+static void configure_heartrate_cb(GSList *characteristics, guint8 status,
+							gpointer user_data)
+{
+	struct heartrate *hr = user_data;
+	GSList *l;
+
+	if (status != 0) {
+		error("Discover heartrate characteristics: %s",
+							att_ecode2str(status));
+		return;
+	}
+
+	for (l = characteristics; l; l = l->next) {
+		struct gatt_char *c = l->data;
+		struct characteristic *ch;
+
+		ch = g_new0(struct characteristic, 1);
+		ch->attr.handle = c->handle;
+		ch->attr.properties = c->properties;
+		ch->attr.value_handle = c->value_handle;
+		memcpy(ch->attr.uuid, c->uuid, MAX_LEN_UUID_STR + 1);
+		ch->hr = hr;
+
+		hr->chars = g_slist_append(hr->chars, ch);
+	}
+}
+
+static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
+{
+	struct heartrate *hr = user_data;
+
+	DBG("GATT Connected");
+
+	hr->attrib = g_attrib_ref(attrib);
+
+	if (hr->chars == NULL)
+		gatt_discover_char(hr->attrib, hr->svc_range->start,
+						hr->svc_range->end, NULL,
+						configure_heartrate_cb, hr);
 }
 
-int heartrate_register(struct btd_device *device)
+static void attio_disconnected_cb(gpointer user_data)
+{
+	struct heartrate *hr = user_data;
+
+	DBG("GATT Disconnected");
+
+	g_attrib_unref(hr->attrib);
+	hr->attrib = NULL;
+}
+
+int heartrate_register(struct btd_device *device, struct gatt_primary *pattr)
 {
 	struct heartrate *hr;
 
 	hr = g_new0(struct heartrate, 1);
 	hr->dev = btd_device_ref(device);
 
+	hr->svc_range = g_new0(struct att_range, 1);
+	hr->svc_range->start = pattr->range.start;
+	hr->svc_range->end = pattr->range.end;
+
 	hr_servers = g_slist_prepend(hr_servers, hr);
 
+	hr->attioid = btd_device_add_attio_callback(device,
+						attio_connected_cb,
+						attio_disconnected_cb ,
+						hr);
 	return 0;
 }
 void heartrate_unregister(struct btd_device *device)
diff --git a/profiles/heartrate/heartrate.h b/profiles/heartrate/heartrate.h
index 8d4271c..1d2ba89 100644
--- a/profiles/heartrate/heartrate.h
+++ b/profiles/heartrate/heartrate.h
@@ -20,5 +20,5 @@
  *
  */
 
-int heartrate_register(struct btd_device *device);
+int heartrate_register(struct btd_device *device, struct gatt_primary *pattr);
 void heartrate_unregister(struct btd_device *device);
diff --git a/profiles/heartrate/manager.c b/profiles/heartrate/manager.c
index da6c7ef..6ba059d 100644
--- a/profiles/heartrate/manager.c
+++ b/profiles/heartrate/manager.c
@@ -32,9 +32,29 @@
 #include "heartrate.h"
 #include "manager.h"
 
+static gint primary_uuid_cmp(gconstpointer a, gconstpointer b)
+{
+	const struct gatt_primary *prim = a;
+	const char *uuid = b;
+
+	return g_strcmp0(prim->uuid, uuid);
+}
+
 static int heartrate_driver_probe(struct btd_device *device, GSList *uuids)
 {
-	return heartrate_register(device);
+	struct gatt_primary *pattr;
+	GSList *primaries, *l;
+
+	primaries = btd_device_get_primaries(device);
+
+	l = g_slist_find_custom(primaries, HEART_RATE_UUID,
+							primary_uuid_cmp);
+	if (l == NULL)
+		return -EINVAL;
+
+	pattr = l->data;
+
+	return heartrate_register(device, pattr);
 }
 
 static void heartrate_driver_remove(struct btd_device *device)
-- 
1.7.9.5

[PATCH v2 04/13] heartrate: Discover Characteristic Descriptors

[Rafal Garbat]

Discover and store each of the Heart Rate Service
characteristic descriptors.

---
 profiles/heartrate/heartrate.c |   61 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/profiles/heartrate/heartrate.c b/profiles/heartrate/heartrate.c
index 1114cff..e917f27 100644
--- a/profiles/heartrate/heartrate.c
+++ b/profiles/heartrate/heartrate.c
@@ -47,9 +47,16 @@ struct heartrate {
 
 struct characteristic {
 	struct gatt_char	attr; /*Characteristic*/
+	GSList			*desc; /*Descriptors*/
 	struct heartrate	*hr; /*Parent Heart Rate Service*/
 };
 
+struct descriptor {
+	struct characteristic	*ch;
+	uint16_t		handle;
+	bt_uuid_t		uuid;
+};
+
 static GSList *hr_servers = NULL;
 
 static gint cmp_device(gconstpointer a, gconstpointer b)
@@ -82,6 +89,44 @@ static void heartrate_destroy(gpointer user_data)
 
 }
 
+static void discover_desc_cb(guint8 status, const guint8 *pdu,
+					guint16 len, gpointer user_data)
+{
+	struct characteristic *ch = user_data;
+	struct att_data_list *list;
+	uint8_t format;
+	int i;
+
+	if (status != 0) {
+		error("Discover all characteristic descriptors failed [%s]: %s",
+					ch->attr.uuid, att_ecode2str(status));
+		return;
+	}
+
+	list = dec_find_info_resp(pdu, len, &format);
+	if (list == NULL)
+		return;
+
+	for (i = 0; i < list->num; i++) {
+		struct descriptor *desc;
+		uint8_t *value;
+
+		value = list->data[i];
+		desc = g_new0(struct descriptor, 1);
+		desc->handle = att_get_u16(value);
+		desc->ch = ch;
+
+		if (format == 0x01)
+			desc->uuid = att_get_uuid16(&value[2]);
+		else
+			desc->uuid = att_get_uuid128(&value[2]);
+
+		ch->desc = g_slist_append(ch->desc, desc);
+	}
+
+	att_data_list_free(list);
+}
+
 static void configure_heartrate_cb(GSList *characteristics, guint8 status,
 							gpointer user_data)
 {
@@ -97,6 +142,7 @@ static void configure_heartrate_cb(GSList *characteristics, guint8 status,
 	for (l = characteristics; l; l = l->next) {
 		struct gatt_char *c = l->data;
 		struct characteristic *ch;
+		uint16_t start, end;
 
 		ch = g_new0(struct characteristic, 1);
 		ch->attr.handle = c->handle;
@@ -106,6 +152,21 @@ static void configure_heartrate_cb(GSList *characteristics, guint8 status,
 		ch->hr = hr;
 
 		hr->chars = g_slist_append(hr->chars, ch);
+
+		start = c->value_handle + 1;
+
+		if (l->next != NULL) {
+			struct gatt_char *c = l->next->data;
+			if (start == c->handle)
+				continue;
+			end = c->handle - 1;
+		} else if (c->value_handle != hr->svc_range->end) {
+			end = hr->svc_range->end;
+		} else {
+			continue;
+		}
+
+		gatt_find_info(hr->attrib, start, end, discover_desc_cb, ch);
 	}
 }
 
-- 
1.7.9.5

[PATCH v2 05/13] heartrate: Process Heart Rate Descriptors

[Rafal Garbat]

Process Heart Rate service descriptors and write
client specific characteristic configuration.

---
 lib/uuid.h                     |    1 +
 profiles/heartrate/heartrate.c |   34 +++++++++++++++++++++++++++++++++-
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/lib/uuid.h b/lib/uuid.h
index 99b88cc..5d81856 100644
--- a/lib/uuid.h
+++ b/lib/uuid.h
@@ -64,6 +64,7 @@ extern "C" {
 #define SAP_UUID		"0000112D-0000-1000-8000-00805f9b34fb"
 
 #define HEART_RATE_UUID		"0000180d-0000-1000-8000-00805f9b34fb"
+#define HEART_RATE_MEASUREMENT_UUID	"00002a37-0000-1000-8000-00805f9b34fb"
 
 #define HEALTH_THERMOMETER_UUID		"00001809-0000-1000-8000-00805f9b34fb"
 #define TEMPERATURE_MEASUREMENT_UUID	"00002a1c-0000-1000-8000-00805f9b34fb"
diff --git a/profiles/heartrate/heartrate.c b/profiles/heartrate/heartrate.c
index e917f27..4c313b2 100644
--- a/profiles/heartrate/heartrate.c
+++ b/profiles/heartrate/heartrate.c
@@ -70,12 +70,21 @@ static gint cmp_device(gconstpointer a, gconstpointer b)
 	return -1;
 }
 
+static void char_destroy(gpointer user_data)
+{
+	struct characteristic *c = user_data;
+
+	g_slist_free_full(c->desc, g_free);
+
+	g_free(c);
+}
+
 static void heartrate_destroy(gpointer user_data)
 {
 	struct heartrate *hr = user_data;
 
 	if (hr->chars != NULL)
-		g_slist_free_full(hr->chars, g_free);
+		g_slist_free_full(hr->chars, char_destroy);
 
 	if (hr->attioid > 0)
 		btd_device_remove_attio_callback(hr->dev, hr->attioid);
@@ -89,6 +98,28 @@ static void heartrate_destroy(gpointer user_data)
 
 }
 
+static void process_heartrate_desc(struct descriptor *desc)
+{
+	struct characteristic *ch = desc->ch;
+	char uuidstr[MAX_LEN_UUID_STR];
+	bt_uuid_t btuuid;
+
+	bt_uuid16_create(&btuuid, GATT_CLIENT_CHARAC_CFG_UUID);
+
+	if (bt_uuid_cmp(&desc->uuid, &btuuid) == 0 &&
+		g_strcmp0(ch->attr.uuid,
+				HEART_RATE_MEASUREMENT_UUID) == 0) {
+
+		DBG("Heart Rate Measurement notification" \
+						" supported");
+		return;
+	}
+
+	bt_uuid_to_string(&desc->uuid, uuidstr, MAX_LEN_UUID_STR);
+	DBG("Ignored descriptor %s in characteristic %s", uuidstr,
+							ch->attr.uuid);
+}
+
 static void discover_desc_cb(guint8 status, const guint8 *pdu,
 					guint16 len, gpointer user_data)
 {
@@ -122,6 +153,7 @@ static void discover_desc_cb(guint8 status, const guint8 *pdu,
 			desc->uuid = att_get_uuid128(&value[2]);
 
 		ch->desc = g_slist_append(ch->desc, desc);
+		process_heartrate_desc(desc);
 	}
 
 	att_data_list_free(list);
-- 
1.7.9.5

[PATCH v2 06/13] heartrate: Add DBus connection logic

[Rafal Garbat]

Add DBus connection logic and watcher methods.

---
 profiles/heartrate/heartrate.c |  157 +++++++++++++++++++++++++++++++++++++++-
 profiles/heartrate/heartrate.h |    3 +-
 profiles/heartrate/main.c      |   18 ++++-
 profiles/heartrate/manager.c   |   19 ++++-
 profiles/heartrate/manager.h   |    2 +-
 5 files changed, 191 insertions(+), 8 deletions(-)

diff --git a/profiles/heartrate/heartrate.c b/profiles/heartrate/heartrate.c
index 4c313b2..31b4232 100644
--- a/profiles/heartrate/heartrate.c
+++ b/profiles/heartrate/heartrate.c
@@ -24,12 +24,15 @@
 #include <config.h>
 #endif
 
+#include <gdbus.h>
 #include <errno.h>
 #include <glib.h>
 #include <bluetooth/uuid.h>
 
 #include "adapter.h"
+#include "dbus-common.h"
 #include "device.h"
+#include "error.h"
 #include "gattrib.h"
 #include "attio.h"
 #include "att.h"
@@ -37,12 +40,16 @@
 #include "heartrate.h"
 #include "log.h"
 
+#define HEART_RATE_INTERFACE "org.bluez.HeartRate"
+
 struct heartrate {
 	struct btd_device	*dev;  /*Device reference*/
+	DBusConnection		*conn; /*DBus conn*/
 	GAttrib			*attrib; /*GATT connection*/
 	guint			attioid; /*ATT watcher id*/
 	struct att_range	*svc_range; /*Heart Rate range*/
 	GSList			*chars; /*Characteristics*/
+	GSList			*watchers; /*Watchers*/
 };
 
 struct characteristic {
@@ -57,6 +64,13 @@ struct descriptor {
 	bt_uuid_t		uuid;
 };
 
+struct watcher {
+	struct heartrate	*hr;
+	guint			id;
+	char			*srv;
+	char			*path;
+};
+
 static GSList *hr_servers = NULL;
 
 static gint cmp_device(gconstpointer a, gconstpointer b)
@@ -70,6 +84,35 @@ static gint cmp_device(gconstpointer a, gconstpointer b)
 	return -1;
 }
 
+static gint cmp_watcher(gconstpointer a, gconstpointer b)
+{
+	const struct watcher *watcher = a;
+	const struct watcher *match = b;
+	int ret;
+
+	ret = g_strcmp0(watcher->srv, match->srv);
+	if (ret != 0)
+		return ret;
+
+	return g_strcmp0(watcher->path, match->path);
+}
+
+static void watcher_remove(gpointer user_data)
+{
+	struct watcher *watcher = user_data;
+
+	g_dbus_remove_watch(watcher->hr->conn, watcher->id);
+}
+
+static void watcher_destroy(gpointer user_data)
+{
+	struct watcher *watcher = user_data;
+
+	g_free(watcher->path);
+	g_free(watcher->srv);
+	g_free(watcher);
+}
+
 static void char_destroy(gpointer user_data)
 {
 	struct characteristic *c = user_data;
@@ -92,12 +135,109 @@ static void heartrate_destroy(gpointer user_data)
 	if (hr->attrib != NULL)
 		g_attrib_unref(hr->attrib);
 
+	if (hr->watchers != NULL)
+		g_slist_free_full(hr->watchers, watcher_remove);
+
 	btd_device_unref(hr->dev);
+	dbus_connection_unref(hr->conn);
 	g_free(hr->svc_range);
 	g_free(hr);
+}
+
+static void watcher_exit(DBusConnection *conn, void *user_data)
+{
+	struct watcher *watcher = user_data;
+	struct heartrate *hr = watcher->hr;
+
+	DBG("Heart Rate watcher %s disconnected", watcher->path);
+
+	hr->watchers = g_slist_remove(hr->watchers, watcher);
+	g_dbus_remove_watch(watcher->hr->conn, watcher->id);
+}
+
+static struct watcher *watcher_find(GSList *list, const char *sender,
+							const char *path)
+{
+	struct watcher *match;
+	GSList *l;
+
+	match = g_new0(struct watcher, 1);
+	match->srv = g_strdup(sender);
+	match->path = g_strdup(path);
+
+	l = g_slist_find_custom(list, match, cmp_watcher);
+	watcher_destroy(match);
+
+	if (l != NULL)
+		return l->data;
+
+	return NULL;
+}
+
+static DBusMessage *watcher_register(DBusConnection *conn, DBusMessage *msg,
+								void *data)
+{
+	const char *sender = dbus_message_get_sender(msg);
+	struct heartrate *hr = data;
+	struct watcher *watcher;
+	char *path;
+
+	if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
+							DBUS_TYPE_INVALID))
+		return btd_error_invalid_args(msg);
+
+	watcher = watcher_find(hr->watchers, sender, path);
+	if (watcher != NULL)
+		return btd_error_already_exists(msg);
+
+	DBG("Heart Rate watcher %s registered", path);
+
+	watcher = g_new0(struct watcher, 1);
+	watcher->srv = g_strdup(sender);
+	watcher->path = g_strdup(path);
+	watcher->hr = hr;
+	watcher->id = g_dbus_add_disconnect_watch(conn, sender, watcher_exit,
+						watcher, watcher_destroy);
+
+	hr->watchers = g_slist_prepend(hr->watchers, watcher);
 
+	return dbus_message_new_method_return(msg);
 }
 
+static DBusMessage *watcher_unregister(DBusConnection *conn, DBusMessage *msg,
+								void *data)
+{
+	const char *sender = dbus_message_get_sender(msg);
+	struct heartrate *hr = data;
+	struct watcher *watcher;
+	char *path;
+
+	if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
+							DBUS_TYPE_INVALID))
+		return btd_error_invalid_args(msg);
+
+	watcher = watcher_find(hr->watchers, sender, path);
+	if (watcher == NULL)
+		return btd_error_does_not_exist(msg);
+
+	DBG("Heart Rate watcher %s unregistered", path);
+
+	hr->watchers = g_slist_remove(hr->watchers, watcher);
+	g_dbus_remove_watch(watcher->hr->conn, watcher->id);
+
+	return dbus_message_new_method_return(msg);
+}
+
+static const GDBusMethodTable heartrate_methods[] = {
+	{ GDBUS_METHOD("RegisterWatcher",
+			GDBUS_ARGS({ "agent", "o" }), NULL,
+			watcher_register) },
+	{ GDBUS_METHOD("UnregisterWatcher",
+			GDBUS_ARGS({ "agent", "o" }), NULL,
+			watcher_unregister) },
+	{ }
+};
+
 static void process_heartrate_desc(struct descriptor *desc)
 {
 	struct characteristic *ch = desc->ch;
@@ -226,17 +366,29 @@ static void attio_disconnected_cb(gpointer user_data)
 	hr->attrib = NULL;
 }
 
-int heartrate_register(struct btd_device *device, struct gatt_primary *pattr)
+int heartrate_register(DBusConnection *connection, struct btd_device *device,
+						struct gatt_primary *pattr)
 {
+	const gchar *path = device_get_path(device);
 	struct heartrate *hr;
 
 	hr = g_new0(struct heartrate, 1);
 	hr->dev = btd_device_ref(device);
 
+	hr->conn = dbus_connection_ref(connection);
 	hr->svc_range = g_new0(struct att_range, 1);
 	hr->svc_range->start = pattr->range.start;
 	hr->svc_range->end = pattr->range.end;
 
+	if (!g_dbus_register_interface(hr->conn, path, HEART_RATE_INTERFACE,
+						heartrate_methods, NULL, NULL,
+						hr, heartrate_destroy)) {
+		error("D-Bus failed to register %s interface",
+							HEART_RATE_INTERFACE);
+		heartrate_destroy(hr);
+		return -EIO;
+	}
+
 	hr_servers = g_slist_prepend(hr_servers, hr);
 
 	hr->attioid = btd_device_add_attio_callback(device,
@@ -257,5 +409,6 @@ void heartrate_unregister(struct btd_device *device)
 	hr = l->data;
 	hr_servers = g_slist_remove(hr_servers, hr);
 
-	heartrate_destroy(hr);
+	g_dbus_unregister_interface(hr->conn, device_get_path(hr->dev),
+							HEART_RATE_INTERFACE);
 }
diff --git a/profiles/heartrate/heartrate.h b/profiles/heartrate/heartrate.h
index 1d2ba89..08e2c92 100644
--- a/profiles/heartrate/heartrate.h
+++ b/profiles/heartrate/heartrate.h
@@ -20,5 +20,6 @@
  *
  */
 
-int heartrate_register(struct btd_device *device, struct gatt_primary *pattr);
+int heartrate_register(DBusConnection *connection, struct btd_device *device,
+						struct gatt_primary *tattr);
 void heartrate_unregister(struct btd_device *device);
diff --git a/profiles/heartrate/main.c b/profiles/heartrate/main.c
index 40f34bc..8e8fde9 100644
--- a/profiles/heartrate/main.c
+++ b/profiles/heartrate/main.c
@@ -27,12 +27,15 @@
 #include <stdint.h>
 #include <glib.h>
 #include <errno.h>
+#include <gdbus.h>
 
 #include "plugin.h"
 #include "manager.h"
 #include "hcid.h"
 #include "log.h"
 
+static DBusConnection *connection = NULL;
+
 static int heartrate_init(void)
 {
 	if (!main_opts.gatt_enabled) {
@@ -40,12 +43,25 @@ static int heartrate_init(void)
 		return -ENOTSUP;
 	}
 
-	return heartrate_manager_init();
+	connection = dbus_bus_get(DBUS_BUS_SYSTEM, NULL);
+	if (connection == NULL)
+		return -EIO;
+
+	if (heartrate_manager_init(connection) < 0) {
+		dbus_connection_unref(connection);
+		connection = NULL;
+		return -EIO;
+	}
+
+	return 0;
 }
 
 static void heartrate_exit(void)
 {
 	heartrate_manager_exit();
+
+	dbus_connection_unref(connection);
+	connection = NULL;
 }
 
 BLUETOOTH_PLUGIN_DEFINE(heartrate, VERSION, BLUETOOTH_PLUGIN_PRIORITY_DEFAULT,
diff --git a/profiles/heartrate/manager.c b/profiles/heartrate/manager.c
index 6ba059d..c373f86 100644
--- a/profiles/heartrate/manager.c
+++ b/profiles/heartrate/manager.c
@@ -32,6 +32,8 @@
 #include "heartrate.h"
 #include "manager.h"
 
+static DBusConnection *connection = NULL;
+
 static gint primary_uuid_cmp(gconstpointer a, gconstpointer b)
 {
 	const struct gatt_primary *prim = a;
@@ -54,7 +56,7 @@ static int heartrate_driver_probe(struct btd_device *device, GSList *uuids)
 
 	pattr = l->data;
 
-	return heartrate_register(device, pattr);
+	return heartrate_register(connection, device, pattr);
 }
 
 static void heartrate_driver_remove(struct btd_device *device)
@@ -69,12 +71,23 @@ static struct btd_device_driver heartrate_device_driver = {
 	.remove	= heartrate_driver_remove
 };
 
-int heartrate_manager_init(void)
+int heartrate_manager_init(DBusConnection *conn)
+
 {
-	return btd_register_device_driver(&heartrate_device_driver);
+	int ret;
+
+	ret = btd_register_device_driver(&heartrate_device_driver);
+	if (ret < 0)
+		return ret;
+
+	connection = dbus_connection_ref(conn);
+	return 0;
 }
 
 void heartrate_manager_exit(void)
 {
 	btd_unregister_device_driver(&heartrate_device_driver);
+
+	dbus_connection_unref(connection);
+	connection = NULL;
 }
diff --git a/profiles/heartrate/manager.h b/profiles/heartrate/manager.h
index de799f6..5e9c8b2 100644
--- a/profiles/heartrate/manager.h
+++ b/profiles/heartrate/manager.h
@@ -20,5 +20,5 @@
  *
  */
 
-int heartrate_manager_init(void);
+int heartrate_manager_init(DBusConnection *conn);
 void heartrate_manager_exit(void);
-- 
1.7.9.5

[PATCH v2 07/13] heartrate: Process Heart Rate Characteristics

[Rafal Garbat]

Read and store Sensor Location characteristic.

---
 lib/uuid.h                     |    2 ++
 profiles/heartrate/heartrate.c |   46 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/lib/uuid.h b/lib/uuid.h
index 5d81856..5d1e091 100644
--- a/lib/uuid.h
+++ b/lib/uuid.h
@@ -65,6 +65,8 @@ extern "C" {
 
 #define HEART_RATE_UUID		"0000180d-0000-1000-8000-00805f9b34fb"
 #define HEART_RATE_MEASUREMENT_UUID	"00002a37-0000-1000-8000-00805f9b34fb"
+#define HEART_RATE_CONTROL_POINT_UUID	"00002a39-0000-1000-8000-00805f9b34fb"
+#define BODY_SENSOR_LOCATION_UUID	"00002a38-0000-1000-8000-00805f9b34fb"
 
 #define HEALTH_THERMOMETER_UUID		"00001809-0000-1000-8000-00805f9b34fb"
 #define TEMPERATURE_MEASUREMENT_UUID	"00002a1c-0000-1000-8000-00805f9b34fb"
diff --git a/profiles/heartrate/heartrate.c b/profiles/heartrate/heartrate.c
index 31b4232..d0bcd57 100644
--- a/profiles/heartrate/heartrate.c
+++ b/profiles/heartrate/heartrate.c
@@ -42,6 +42,8 @@
 
 #define HEART_RATE_INTERFACE "org.bluez.HeartRate"
 
+#define SENSOR_LOCATION_SIZE 1
+
 struct heartrate {
 	struct btd_device	*dev;  /*Device reference*/
 	DBusConnection		*conn; /*DBus conn*/
@@ -50,6 +52,8 @@ struct heartrate {
 	struct att_range	*svc_range; /*Heart Rate range*/
 	GSList			*chars; /*Characteristics*/
 	GSList			*watchers; /*Watchers*/
+	gboolean		has_location;
+	uint8_t		location; /*Body Sensor location*/
 };
 
 struct characteristic {
@@ -238,6 +242,46 @@ static const GDBusMethodTable heartrate_methods[] = {
 	{ }
 };
 
+static void read_sensor_location_cb(guint8 status, const guint8 *pdu,
+						guint16 len, gpointer user_data)
+{
+	struct characteristic *ch = user_data;
+	struct heartrate *hr = ch->hr;
+	uint8_t value[SENSOR_LOCATION_SIZE];
+	ssize_t vlen;
+
+	if (status != 0) {
+		error("Body Sensor Location value read failed: %s",
+							att_ecode2str(status));
+		return;
+	}
+
+	vlen = dec_read_resp(pdu, len, value, sizeof(value));
+	if (vlen < 0) {
+		error("Protocol error.");
+		return;
+	}
+
+	if (vlen != 1) {
+		error("Invalid length for Body Sensor Location");
+		return;
+	}
+
+	hr->has_location = TRUE;
+	hr->location = value[0];
+}
+
+static void process_heartrate_char(struct characteristic *ch)
+{
+	if (g_strcmp0(ch->attr.uuid,
+				HEART_RATE_CONTROL_POINT_UUID) == 0)
+		DBG("Heart Rate Control Point reset not supported by client");
+	 else if (g_strcmp0(ch->attr.uuid,
+					BODY_SENSOR_LOCATION_UUID) == 0)
+		gatt_read_char(ch->hr->attrib, ch->attr.value_handle, 0,
+						read_sensor_location_cb, ch);
+}
+
 static void process_heartrate_desc(struct descriptor *desc)
 {
 	struct characteristic *ch = desc->ch;
@@ -325,6 +369,8 @@ static void configure_heartrate_cb(GSList *characteristics, guint8 status,
 
 		hr->chars = g_slist_append(hr->chars, ch);
 
+		process_heartrate_char(ch);
+
 		start = c->value_handle + 1;
 
 		if (l->next != NULL) {
-- 
1.7.9.5

[PATCH v2 08/13] heartrate: Add notification support

[Rafal Garbat]

Add handling notification event.

---
 profiles/heartrate/heartrate.c |   20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/profiles/heartrate/heartrate.c b/profiles/heartrate/heartrate.c
index d0bcd57..e88d4bc 100644
--- a/profiles/heartrate/heartrate.c
+++ b/profiles/heartrate/heartrate.c
@@ -42,6 +42,8 @@
 
 #define HEART_RATE_INTERFACE "org.bluez.HeartRate"
 
+#define MIN_NOTIFICATION_LEN 3  /* 1-byte opcode + 2-byte handle */
+
 #define SENSOR_LOCATION_SIZE 1
 
 struct heartrate {
@@ -49,6 +51,7 @@ struct heartrate {
 	DBusConnection		*conn; /*DBus conn*/
 	GAttrib			*attrib; /*GATT connection*/
 	guint			attioid; /*ATT watcher id*/
+	guint			attionotid; /*ATT notification watcher id*/
 	struct att_range	*svc_range; /*Heart Rate range*/
 	GSList			*chars; /*Characteristics*/
 	GSList			*watchers; /*Watchers*/
@@ -136,8 +139,10 @@ static void heartrate_destroy(gpointer user_data)
 	if (hr->attioid > 0)
 		btd_device_remove_attio_callback(hr->dev, hr->attioid);
 
-	if (hr->attrib != NULL)
+	if (hr->attrib != NULL) {
+		g_attrib_unregister(hr->attrib, hr->attionotid);
 		g_attrib_unref(hr->attrib);
+	}
 
 	if (hr->watchers != NULL)
 		g_slist_free_full(hr->watchers, watcher_remove);
@@ -388,6 +393,14 @@ static void configure_heartrate_cb(GSList *characteristics, guint8 status,
 	}
 }
 
+static void notify_handler(const uint8_t *pdu, uint16_t len, gpointer user_data)
+{
+	if (len < MIN_NOTIFICATION_LEN) {
+		error("Bad pdu received");
+		return;
+	}
+}
+
 static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
 {
 	struct heartrate *hr = user_data;
@@ -396,6 +409,9 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
 
 	hr->attrib = g_attrib_ref(attrib);
 
+	hr->attionotid = g_attrib_register(hr->attrib,
+			ATT_OP_HANDLE_NOTIFY, notify_handler, hr , NULL);
+
 	if (hr->chars == NULL)
 		gatt_discover_char(hr->attrib, hr->svc_range->start,
 						hr->svc_range->end, NULL,
@@ -408,6 +424,8 @@ static void attio_disconnected_cb(gpointer user_data)
 
 	DBG("GATT Disconnected");
 
+	g_attrib_unregister(hr->attrib, hr->attionotid);
+	hr->attionotid = 0;
 	g_attrib_unref(hr->attrib);
 	hr->attrib = NULL;
 }
-- 
1.7.9.5

[PATCH v2 09/13] heartrate: Process measurements

[Rafal Garbat]

Enable or disable measurement while watcher is registered
or unregistered. Process measurement data when notification
is received and update watcher.

---
 profiles/heartrate/heartrate.c |  318 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 318 insertions(+)

diff --git a/profiles/heartrate/heartrate.c b/profiles/heartrate/heartrate.c
index e88d4bc..587438b 100644
--- a/profiles/heartrate/heartrate.c
+++ b/profiles/heartrate/heartrate.c
@@ -46,6 +46,12 @@
 
 #define SENSOR_LOCATION_SIZE 1
 
+#define HR_VALUE_FORMAT	0x01
+#define SENSOR_SUPPORTED	0x02
+#define CONTACT_DETECTED	0x04
+#define ENERGY_EXP_STATUS	0x08
+#define RR_INTERVAL		0x10
+
 struct heartrate {
 	struct btd_device	*dev;  /*Device reference*/
 	DBusConnection		*conn; /*DBus conn*/
@@ -78,8 +84,38 @@ struct watcher {
 	char			*path;
 };
 
+struct measurement {
+	gboolean	u16_val_fmt; /*Heart Rate Value Format*/
+	uint16_t	value;
+	uint16_t	energy;
+	gboolean	energy_sup; /*Energy Status Supported*/
+	gboolean	contact;
+	char		*location;
+	gboolean	interval_sup; /*RR-Interval Supported*/
+	GSList		*interval;
+};
+
+const char *location_type[] = {
+	"Other",
+	"Chest",
+	"Wrist",
+	"Finger",
+	"Hand",
+	"Earlobe",
+	"Foot"
+};
+
 static GSList *hr_servers = NULL;
 
+static const gchar *location2str(uint8_t value)
+{
+	 if (value < G_N_ELEMENTS(location_type))
+		return location_type[value];
+
+	error("Location type %d reserved for future use", value);
+	return NULL;
+}
+
 static gint cmp_device(gconstpointer a, gconstpointer b)
 {
 	const struct heartrate *hr = a;
@@ -91,6 +127,30 @@ static gint cmp_device(gconstpointer a, gconstpointer b)
 	return -1;
 }
 
+static gint cmp_char_uuid(gconstpointer a, gconstpointer b)
+{
+	const struct characteristic *ch = a;
+	const char *uuid = b;
+
+	return g_strcmp0(ch->attr.uuid, uuid);
+}
+
+static gint cmp_char_val_handle(gconstpointer a, gconstpointer b)
+{
+	const struct characteristic *ch = a;
+	const uint16_t *handle = b;
+
+	return ch->attr.value_handle - *handle;
+}
+
+static gint cmp_descriptor(gconstpointer a, gconstpointer b)
+{
+	const struct descriptor *desc = a;
+	const bt_uuid_t *uuid = b;
+
+	return bt_uuid_cmp(&desc->uuid, uuid);
+}
+
 static gint cmp_watcher(gconstpointer a, gconstpointer b)
 {
 	const struct watcher *watcher = a;
@@ -153,6 +213,78 @@ static void heartrate_destroy(gpointer user_data)
 	g_free(hr);
 }
 
+static struct characteristic *get_characteristic(struct heartrate *hr,
+							const char *uuid)
+{
+	GSList *l;
+
+	l = g_slist_find_custom(hr->chars, uuid, cmp_char_uuid);
+	if (l == NULL)
+		return NULL;
+
+	return l->data;
+}
+
+static struct descriptor *get_descriptor(struct characteristic *ch,
+							const bt_uuid_t *uuid)
+{
+	GSList *l;
+
+	l = g_slist_find_custom(ch->desc, uuid, cmp_descriptor);
+	if (l == NULL)
+		return NULL;
+
+	return l->data;
+}
+
+static void measurement_cb(guint8 status, const guint8 *pdu,
+						guint16 len, gpointer user_data)
+{
+	char *msg = user_data;
+
+	if (status != 0)
+		error("%s failed", msg);
+
+	g_free(msg);
+}
+
+static void measurement_toggle(struct heartrate *hr, gboolean enable)
+{
+	struct characteristic *ch;
+	struct descriptor *desc;
+	bt_uuid_t btuuid;
+	uint8_t atval[2];
+	uint16_t val;
+	char *msg;
+
+	if (hr->attrib == NULL)
+		return;
+
+	ch = get_characteristic(hr, HEART_RATE_MEASUREMENT_UUID);
+	if (ch == NULL) {
+		DBG("Temperature measurement characteristic not found");
+		return;
+	}
+
+	bt_uuid16_create(&btuuid, GATT_CLIENT_CHARAC_CFG_UUID);
+	desc = get_descriptor(ch, &btuuid);
+	if (desc == NULL) {
+		DBG("Client characteristic configuration descriptor not found");
+		return;
+	}
+
+	val = enable ? GATT_CLIENT_CHARAC_CFG_NOTIF_BIT : 0x0000;
+
+	msg = enable ? g_strdup("Enable measurement") :
+				g_strdup("Disable measurement");
+
+	att_put_u16(val, atval);
+
+	gatt_write_char(hr->attrib, desc->handle, atval, 2,
+							measurement_cb, msg);
+
+}
+
 static void watcher_exit(DBusConnection *conn, void *user_data)
 {
 	struct watcher *watcher = user_data;
@@ -162,6 +294,9 @@ static void watcher_exit(DBusConnection *conn, void *user_data)
 
 	hr->watchers = g_slist_remove(hr->watchers, watcher);
 	g_dbus_remove_watch(watcher->hr->conn, watcher->id);
+
+	if (g_slist_length(hr->watchers) == 0)
+		measurement_toggle(hr, FALSE);
 }
 
 static struct watcher *watcher_find(GSList *list, const char *sender,
@@ -208,6 +343,9 @@ static DBusMessage *watcher_register(DBusConnection *conn, DBusMessage *msg,
 	watcher->id = g_dbus_add_disconnect_watch(conn, sender, watcher_exit,
 						watcher, watcher_destroy);
 
+	if (g_slist_length(hr->watchers) == 0)
+		measurement_toggle(hr, TRUE);
+
 	hr->watchers = g_slist_prepend(hr->watchers, watcher);
 
 	return dbus_message_new_method_return(msg);
@@ -234,6 +372,9 @@ static DBusMessage *watcher_unregister(DBusConnection *conn, DBusMessage *msg,
 	hr->watchers = g_slist_remove(hr->watchers, watcher);
 	g_dbus_remove_watch(watcher->hr->conn, watcher->id);
 
+	if (g_slist_length(hr->watchers) == 0)
+		measurement_toggle(hr, FALSE);
+
 	return dbus_message_new_method_return(msg);
 }
 
@@ -393,12 +534,189 @@ static void configure_heartrate_cb(GSList *characteristics, guint8 status,
 	}
 }
 
+static void update_watcher(gpointer data, gpointer user_data)
+{
+	struct watcher *w = data;
+	struct measurement *m = user_data;
+	DBusConnection *conn = w->hr->conn;
+	DBusMessageIter iter;
+	DBusMessageIter dict;
+	DBusMessage *msg;
+	GSList *l;
+	uint16_t *interval;
+	guint n_elem;
+	guint i;
+
+	msg = dbus_message_new_method_call(w->srv, w->path,
+				"org.bluez.HeartRateWatcher",
+				"MeasurementReceived");
+	if (msg == NULL)
+		return;
+
+	dbus_message_iter_init_append(msg, &iter);
+
+	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
+			DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
+			DBUS_TYPE_STRING_AS_STRING DBUS_TYPE_VARIANT_AS_STRING
+			DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
+
+	dict_append_entry(&dict, "Value", DBUS_TYPE_UINT16, &m->value);
+	dict_append_entry(&dict, "Energy", DBUS_TYPE_UINT16, &m->energy);
+	dict_append_entry(&dict, "Contact", DBUS_TYPE_BOOLEAN, &m->contact);
+	dict_append_entry(&dict, "Location", DBUS_TYPE_STRING, &m->location);
+
+	n_elem = g_slist_length(m->interval);
+
+	interval = g_new(uint16_t, n_elem);
+
+	if (interval != NULL) {
+
+		for (i = 0, l = m->interval; l; l = l->next, i++)
+			interval[i] =  GPOINTER_TO_UINT(l->data);
+
+		dict_append_array(&dict, "Interval", DBUS_TYPE_UINT16,
+							&interval, n_elem);
+	}
+
+	dbus_message_iter_close_container(&iter, &dict);
+
+	dbus_message_set_no_reply(msg, TRUE);
+	g_dbus_send_message(conn, msg);
+
+	if (interval != NULL)
+		g_free(interval);
+}
+
+static void recv_measurement(struct heartrate *hr, struct measurement *m)
+{
+	GSList *wlist = hr->watchers;
+
+	g_slist_foreach(wlist, update_watcher, m);
+}
+
+static void get_flags(struct measurement *m, uint8_t pdu)
+{
+	uint8_t flags = pdu;
+
+	if (flags & HR_VALUE_FORMAT)
+		m->u16_val_fmt = TRUE;  /*value format set to uint16*/
+
+	if ((flags & SENSOR_SUPPORTED) && (flags & CONTACT_DETECTED))
+		m->contact = TRUE;
+
+	if (flags & ENERGY_EXP_STATUS)
+		m->energy_sup = TRUE;
+
+	if (flags & RR_INTERVAL)
+		m->interval_sup = TRUE;
+}
+
+static void proc_measurement(struct heartrate *hr, const uint8_t *pdu,
+						uint16_t len)
+{
+	struct measurement m;
+	const char *loc;
+	gint pdu_idx;
+	gint min_pdu_len;
+
+	if (len < 4) {
+		error("Mandatory flags are not provided");
+		return;
+	}
+
+	memset(&m, 0, sizeof(struct measurement));
+
+	get_flags(&m, pdu[3]);
+
+	pdu_idx = 4;
+	min_pdu_len = len - 4;
+
+	if (m.u16_val_fmt) {
+
+		if (min_pdu_len < 2) {
+			error("Heart Rate measurement value is not provided");
+			return;
+		}
+
+		m.value = att_get_u16(&pdu[pdu_idx]);
+		pdu_idx += 2;
+		min_pdu_len -= 2;
+	} else {
+
+		if (min_pdu_len < 1) {
+			error("Heart Rate measurement value is not provided");
+			return;
+		}
+
+		m.value = pdu[pdu_idx];
+		pdu_idx++;
+		min_pdu_len--;
+	}
+
+	if (m.energy_sup) {
+
+		if (min_pdu_len < 2) {
+			error("Can't get Energy Expended field");
+			return;
+		}
+
+		m.energy = att_get_u16(&pdu[pdu_idx]);
+		pdu_idx += 2;
+		min_pdu_len -= 2;
+	}
+
+	if (m.interval_sup) {
+		gint i;
+
+		if (min_pdu_len < 0) {
+			error("Can't get RR-interval");
+			return;
+		}
+
+		for (i = len - pdu_idx; i > 0; i--) {
+			uint16_t val = att_get_u16(&pdu[pdu_idx++]);
+			m.interval = g_slist_append(m.interval,
+						GUINT_TO_POINTER(val));
+		}
+	}
+
+	if (hr->has_location) {
+		loc = location2str(hr->location);
+		m.location =  g_strdup(loc);
+	} else {
+		m.location = NULL;
+	}
+
+	recv_measurement(hr, &m);
+
+	g_free(m.location);
+
+	if (m.interval != NULL)
+		g_slist_free(m.interval);
+}
+
 static void notify_handler(const uint8_t *pdu, uint16_t len, gpointer user_data)
 {
+	struct heartrate *hr = user_data;
+	const struct characteristic *ch;
+	uint16_t handle;
+	GSList *l;
+
 	if (len < MIN_NOTIFICATION_LEN) {
 		error("Bad pdu received");
 		return;
 	}
+
+	handle = att_get_u16(&pdu[1]);
+	l = g_slist_find_custom(hr->chars, &handle, cmp_char_val_handle);
+	if (l == NULL) {
+		error("Unexpected handle: 0x%04x", handle);
+		return;
+	}
+
+	ch = l->data;
+	if (g_strcmp0(ch->attr.uuid, HEART_RATE_MEASUREMENT_UUID) == 0)
+		proc_measurement(hr, pdu, len);
 }
 
 static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
-- 
1.7.9.5

[PATCH v2 10/13] heartrate: Add support for Control Point Reset

[Rafal Garbat]

Handle Control Point reset if server supports it.

---
 profiles/heartrate/heartrate.c |   50 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 46 insertions(+), 4 deletions(-)

diff --git a/profiles/heartrate/heartrate.c b/profiles/heartrate/heartrate.c
index 587438b..267ad54 100644
--- a/profiles/heartrate/heartrate.c
+++ b/profiles/heartrate/heartrate.c
@@ -63,6 +63,7 @@ struct heartrate {
 	GSList			*watchers; /*Watchers*/
 	gboolean		has_location;
 	uint8_t		location; /*Body Sensor location*/
+	gboolean		cp_reset; /*Control Point Reset support*/
 };
 
 struct characteristic {
@@ -378,6 +379,43 @@ static DBusMessage *watcher_unregister(DBusConnection *conn, DBusMessage *msg,
 	return dbus_message_new_method_return(msg);
 }
 
+static gint process_att_reset(struct heartrate *hr)
+{
+	struct characteristic *ch;
+	char *msg;
+	uint8_t atval;
+
+	if (hr->attrib == NULL)
+		return -1;
+
+	ch = get_characteristic(hr, HEART_RATE_CONTROL_POINT_UUID);
+	if (ch == NULL)
+		return -1;
+
+	atval = 0x01;
+	msg = g_strdup("Reset Control Point");
+
+	gatt_write_char(hr->attrib, ch->attr.value_handle, &atval, 1,
+						measurement_cb, msg);
+	return 0;
+}
+
+static DBusMessage *control_point_reset(DBusConnection *conn, DBusMessage *msg,
+								void *data)
+{
+	struct heartrate *hr = data;
+
+	if (!hr->cp_reset)
+		return btd_error_not_supported(msg);
+
+	if (process_att_reset(hr) < 0)
+		return btd_error_not_available(msg);
+
+	DBG("Energy Expended Value has been reset");
+
+	return dbus_message_new_method_return(msg);
+}
+
 static const GDBusMethodTable heartrate_methods[] = {
 	{ GDBUS_METHOD("RegisterWatcher",
 			GDBUS_ARGS({ "agent", "o" }), NULL,
@@ -385,6 +423,8 @@ static const GDBusMethodTable heartrate_methods[] = {
 	{ GDBUS_METHOD("UnregisterWatcher",
 			GDBUS_ARGS({ "agent", "o" }), NULL,
 			watcher_unregister) },
+	{ GDBUS_METHOD("Reset", NULL, NULL,
+			control_point_reset) },
 	{ }
 };
 
@@ -420,12 +460,14 @@ static void read_sensor_location_cb(guint8 status, const guint8 *pdu,
 static void process_heartrate_char(struct characteristic *ch)
 {
 	if (g_strcmp0(ch->attr.uuid,
-				HEART_RATE_CONTROL_POINT_UUID) == 0)
-		DBG("Heart Rate Control Point reset not supported by client");
-	 else if (g_strcmp0(ch->attr.uuid,
-					BODY_SENSOR_LOCATION_UUID) == 0)
+				HEART_RATE_CONTROL_POINT_UUID) == 0) {
+		ch->hr->cp_reset = TRUE;
+		DBG("Heart Rate Control Point reset supported by server");
+	 } else if (g_strcmp0(ch->attr.uuid,
+					BODY_SENSOR_LOCATION_UUID) == 0) {
 		gatt_read_char(ch->hr->attrib, ch->attr.value_handle, 0,
 						read_sensor_location_cb, ch);
+	 }
 }
 
 static void process_heartrate_desc(struct descriptor *desc)
-- 
1.7.9.5

[PATCH v2 11/13] heartrate: Add GetProperties method handle

[Rafal Garbat]

Add method to return Heart Rate Service properties.

---
 profiles/heartrate/heartrate.c |   30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/profiles/heartrate/heartrate.c b/profiles/heartrate/heartrate.c
index 267ad54..0eb6ee8 100644
--- a/profiles/heartrate/heartrate.c
+++ b/profiles/heartrate/heartrate.c
@@ -238,6 +238,33 @@ static struct descriptor *get_descriptor(struct characteristic *ch,
 	return l->data;
 }
 
+static DBusMessage *get_properties(DBusConnection *conn, DBusMessage *msg,
+								void *data)
+{
+	struct heartrate *hr = data;
+	DBusMessageIter iter;
+	DBusMessageIter dict;
+	DBusMessage *reply;
+
+	reply = dbus_message_new_method_return(msg);
+	if (reply == NULL)
+		return NULL;
+
+	dbus_message_iter_init_append(reply, &iter);
+
+	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
+			DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
+			DBUS_TYPE_STRING_AS_STRING DBUS_TYPE_VARIANT_AS_STRING
+			DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
+
+	dict_append_entry(&dict, "ResetSupported", DBUS_TYPE_BOOLEAN,
+							&hr->cp_reset);
+
+	dbus_message_iter_close_container(&iter, &dict);
+
+	return reply;
+}
+
 static void measurement_cb(guint8 status, const guint8 *pdu,
 						guint16 len, gpointer user_data)
 {
@@ -417,6 +444,9 @@ static DBusMessage *control_point_reset(DBusConnection *conn, DBusMessage *msg,
 }
 
 static const GDBusMethodTable heartrate_methods[] = {
+	{ GDBUS_METHOD("GetProperties",
+			NULL, GDBUS_ARGS({ "properties", "a{sv}" }),
+			get_properties) },
 	{ GDBUS_METHOD("RegisterWatcher",
 			GDBUS_ARGS({ "agent", "o" }), NULL,
 			watcher_register) },
-- 
1.7.9.5

[PATCH v2 12/13] heartrate: Add org.bluez.HeartRateWatcher iface to default policy

[Rafal Garbat]

---
 src/bluetooth.conf |    1 +
 1 file changed, 1 insertion(+)

diff --git a/src/bluetooth.conf b/src/bluetooth.conf
index 664dbd9..77a9371 100644
--- a/src/bluetooth.conf
+++ b/src/bluetooth.conf
@@ -16,6 +16,7 @@
     <allow send_interface="org.bluez.MediaPlayer"/>
     <allow send_interface="org.bluez.Watcher"/>
     <allow send_interface="org.bluez.ThermometerWatcher"/>
+    <allow send_interface="org.bluez.HeartRateWatcher"/>
   </policy>
 
   <policy at_console="true">
-- 
1.7.9.5

[PATCH v2 13/13] heartrate: Add Heart Rate test script

[Rafal Garbat]

---
 Makefile.tools      |    4 +--
 test/test-heartrate |   91 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+), 2 deletions(-)
 create mode 100755 test/test-heartrate

diff --git a/Makefile.tools b/Makefile.tools
index 5579b86..77b0a3f 100644
--- a/Makefile.tools
+++ b/Makefile.tools
@@ -222,7 +222,7 @@ EXTRA_DIST += test/sap_client.py test/hsplay test/hsmicro \
 		test/test-network test/simple-agent test/simple-service \
 		test/simple-endpoint test/test-audio test/test-input \
 		test/test-sap-server test/test-oob test/test-attrib \
-		test/test-proximity test/test-thermometer test/test-health \
-		test/test-health-sink test/service-record.dtd \
+		test/test-proximity test/test-thermometer test/test-heartrate \
+		test/test-health test/test-health-sink test/service-record.dtd \
 		test/service-did.xml test/service-spp.xml test/service-opp.xml \
 		test/service-ftp.xml test/simple-player test/test-nap
diff --git a/test/test-heartrate b/test/test-heartrate
new file mode 100755
index 0000000..4945583
--- /dev/null
+++ b/test/test-heartrate
@@ -0,0 +1,91 @@
+#!/usr/bin/python
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+'''
+Heart Rate Monitor test script
+'''
+
+import gobject
+
+import sys
+import dbus
+import dbus.service
+import dbus.mainloop.glib
+from optparse import OptionParser, make_option
+
+class Watcher(dbus.service.Object):
+	@dbus.service.method("org.bluez.HeartRateWatcher",
+					in_signature="a{sv}", out_signature="")
+	def MeasurementReceived(self, measure):
+		print("Measurement received")
+		print("Value", measure["Value"])
+		print("Energy", measure["Energy"])
+		print("Contact", measure["Contact"])
+		print("Location", measure["Location"])
+
+		for i in measure["Interval"]:
+			print("Interval", i)
+
+if __name__ == "__main__":
+	dbus.mainloop.glib.DBusGMainLoop(set_as_default=True)
+
+	bus = dbus.SystemBus()
+
+	manager = dbus.Interface(bus.get_object("org.bluez", "/"),
+					"org.bluez.Manager")
+
+	option_list = [
+		make_option("-i", "--adapter", action="store",
+			type="string", dest="adapter"),
+		make_option("-b", "--device", action="store",
+			type="string", dest="address"),
+		]
+
+	parser = OptionParser(option_list=option_list)
+
+	(options, args) = parser.parse_args()
+
+	if not options.address:
+		print("Usage: %s [-i <adapter>] -b <bdaddr> [cmd]" % (sys.argv[0]))
+		print("Possible commands:")
+		print("\tReset")
+		sys.exit(1)
+
+	if options.adapter:
+		adapter_path = manager.FindAdapter(options.adapter)
+	else:
+		adapter_path = manager.DefaultAdapter()
+
+	adapter = dbus.Interface(bus.get_object("org.bluez", adapter_path),
+							"org.bluez.Adapter")
+
+	device_path = adapter.FindDevice(options.address)
+
+	device = dbus.Interface(bus.get_object("org.bluez", device_path),
+							"org.bluez.Device")
+
+	heartrate = dbus.Interface(bus.get_object("org.bluez",
+					device_path), "org.bluez.HeartRate")
+
+	path = "/test/watcher"
+	watcher = Watcher(bus, path)
+
+	heartrate.RegisterWatcher(path)
+
+	properties = heartrate.GetProperties()
+	reset_sup = properties["ResetSupported"]
+
+	if len(args) > 0:
+		if args[0] == "Reset":
+			if reset_sup:
+				heartrate.Reset()
+			else:
+				print("Reset not supported")
+				sys.exit(1)
+		else:
+			print("unknown command")
+			sys.exit(1)
+
+	mainloop = gobject.MainLoop()
+	mainloop.run()
-- 
1.7.9.5

Re: [PATCH v2 01/13] Heart Rate Profile API

[Johan Hedberg]

Hi,

On Mon, Aug 13, 2012, Rafal Garbat wrote:
> +Heart Rate Profile hierarchy
> +============================
> +
> +Service		org.bluez
> +Interface	org.bluez.HeartRate
> +Object path	[variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> +
> +Methods		dict GetProperties()
> +
> +			Returns all properties for the interface. See the
> +			Properties section for the available properties.
> +
> +		RegisterWatcher(object agent)
> +
> +			Registers a watcher to monitor heart rate measurements.
> +
> +			Possible Errors: org.bluez.Error.InvalidArguments

I'm wondering if it wouldn't make more sense to have these
RegisterWatcher APIs (thermometer, heart rate, others?) per-adapter
instead of per-device. That would be much friendlier to applications in
that they wouldn't need to separately search for paired/configured
devices supporting a specific service. Moving this to be per-adapter
would also mean that the first parameter of the Watcher methods would be
the object path of which device is in question.

Johan

Re: [PATCH v2 01/13] Heart Rate Profile API

[Johan Hedberg]

Hi,

On Tue, Aug 14, 2012, Johan Hedberg wrote:
> Hi,
> 
> On Mon, Aug 13, 2012, Rafal Garbat wrote:
> > +Heart Rate Profile hierarchy
> > +============================
> > +
> > +Service		org.bluez
> > +Interface	org.bluez.HeartRate
> > +Object path	[variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> > +
> > +Methods		dict GetProperties()
> > +
> > +			Returns all properties for the interface. See the
> > +			Properties section for the available properties.
> > +
> > +		RegisterWatcher(object agent)
> > +
> > +			Registers a watcher to monitor heart rate measurements.
> > +
> > +			Possible Errors: org.bluez.Error.InvalidArguments
> 
> I'm wondering if it wouldn't make more sense to have these
> RegisterWatcher APIs (thermometer, heart rate, others?) per-adapter
> instead of per-device. That would be much friendlier to applications in
> that they wouldn't need to separately search for paired/configured
> devices supporting a specific service. Moving this to be per-adapter
> would also mean that the first parameter of the Watcher methods would be
> the object path of which device is in question.

So any comments on this? I'd like to get this moving forward and finally
merged upstream.

Johan

Re: [PATCH v2 01/13] Heart Rate Profile API

[Garbat Rafal]

Hi,

On 08/28/2012 05:20 PM, Johan Hedberg wrote:
> Hi,
>
> On Tue, Aug 14, 2012, Johan Hedberg wrote:
>> Hi,
>>
>> On Mon, Aug 13, 2012, Rafal Garbat wrote:
>>> +Heart Rate Profile hierarchy
>>> +============================
>>> +
>>> +Service		org.bluez
>>> +Interface	org.bluez.HeartRate
>>> +Object path	[variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
>>> +
>>> +Methods		dict GetProperties()
>>> +
>>> +			Returns all properties for the interface. See the
>>> +			Properties section for the available properties.
>>> +
>>> +		RegisterWatcher(object agent)
>>> +
>>> +			Registers a watcher to monitor heart rate measurements.
>>> +
>>> +			Possible Errors: org.bluez.Error.InvalidArguments
>> I'm wondering if it wouldn't make more sense to have these
>> RegisterWatcher APIs (thermometer, heart rate, others?) per-adapter
>> instead of per-device. That would be much friendlier to applications in
>> that they wouldn't need to separately search for paired/configured
>> devices supporting a specific service. Moving this to be per-adapter
>> would also mean that the first parameter of the Watcher methods would be
>> the object path of which device is in question.
> So any comments on this? I'd like to get this moving forward and finally
> merged upstream.
>
> Johan
> --
> 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
Sorry for a late reply.
I guess that moving RegisterWatcher methods to the adapter iface sounds 
reasonable, but we need to think how to do it i.e. to properly handle 
devices that support several profiles based on registering watchers (do 
we want to register watcher for all the profiles or have a parameter for 
Watcher methods to specify the target), etc.
Correct me if I'm wrong or missing something.
I'd suggest merging heartrate as this profile is quite similar to the 
thermometer and it works (and no one have any objections to the code) 
and re-factor this later on.
Unfortunately I'll be off for the next three weeks, but I can get back 
to this when I'm back.

BR,
Rafal

Re: [PATCH v2 01/13] Heart Rate Profile API

[Johan Hedberg]

Hi Rafal,

On Tue, Aug 28, 2012, Garbat Rafal wrote:
> >>I'm wondering if it wouldn't make more sense to have these
> >>RegisterWatcher APIs (thermometer, heart rate, others?) per-adapter
> >>instead of per-device. That would be much friendlier to applications in
> >>that they wouldn't need to separately search for paired/configured
> >>devices supporting a specific service. Moving this to be per-adapter
> >>would also mean that the first parameter of the Watcher methods would be
> >>the object path of which device is in question.
> >So any comments on this? I'd like to get this moving forward and finally
> >merged upstream.
> 
> Sorry for a late reply.
> I guess that moving RegisterWatcher methods to the adapter iface
> sounds reasonable, but we need to think how to do it i.e. to
> properly handle devices that support several profiles based on
> registering watchers (do we want to register watcher for all the
> profiles or have a parameter for Watcher methods to specify the
> target), etc.
> Correct me if I'm wrong or missing something.
> I'd suggest merging heartrate as this profile is quite similar to
> the thermometer and it works (and no one have any objections to the
> code) and re-factor this later on.
> Unfortunately I'll be off for the next three weeks, but I can get
> back to this when I'm back.

Since it's not just a refactoring but an API change/break I'd rather get
this right from the start. The thermometer API should also be updated to
be per-adapter for our next release (BlueZ 5).

Johan

Re: [PATCH v2 01/13] Heart Rate Profile API

[Anderson Lizardo]

Hi Johan,

On Tue, Aug 28, 2012 at 12:56 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> On Tue, Aug 28, 2012, Garbat Rafal wrote:
>> I guess that moving RegisterWatcher methods to the adapter iface
>> sounds reasonable, but we need to think how to do it i.e. to
>> properly handle devices that support several profiles based on
>> registering watchers (do we want to register watcher for all the
>> profiles or have a parameter for Watcher methods to specify the
>> target), etc.
>> Correct me if I'm wrong or missing something.
>> I'd suggest merging heartrate as this profile is quite similar to
>> the thermometer and it works (and no one have any objections to the
>> code) and re-factor this later on.
>> Unfortunately I'll be off for the next three weeks, but I can get
>> back to this when I'm back.
>
> Since it's not just a refactoring but an API change/break I'd rather get
> this right from the start. The thermometer API should also be updated to
> be per-adapter for our next release (BlueZ 5).

The registered "watcher" object will have different interface
depending on the profile. The application which will call
RegisterWatcher() needs to check that the device supports the expected
GATT service (e.g. by checking the "UUIDs" property of that device
object) before using the  org.bluez.HeartRate interface, therefore
IMHO it makes sense to have RegisterWatcher() on the device object
path.

Unless you are proposing a generic Watcher API that could somehow be
shared by all profiles (including a single shared interface for the
watcher object)? How to represent data from profiles which are not
"measurement" based?

Best Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

Re: [PATCH v2 01/13] Heart Rate Profile API

[Johan Hedberg]

Hi Lizardo,

On Tue, Aug 28, 2012, Anderson Lizardo wrote:
> On Tue, Aug 28, 2012 at 12:56 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> > On Tue, Aug 28, 2012, Garbat Rafal wrote:
> >> I guess that moving RegisterWatcher methods to the adapter iface
> >> sounds reasonable, but we need to think how to do it i.e. to
> >> properly handle devices that support several profiles based on
> >> registering watchers (do we want to register watcher for all the
> >> profiles or have a parameter for Watcher methods to specify the
> >> target), etc.
> >> Correct me if I'm wrong or missing something.
> >> I'd suggest merging heartrate as this profile is quite similar to
> >> the thermometer and it works (and no one have any objections to the
> >> code) and re-factor this later on.
> >> Unfortunately I'll be off for the next three weeks, but I can get
> >> back to this when I'm back.
> >
> > Since it's not just a refactoring but an API change/break I'd rather get
> > this right from the start. The thermometer API should also be updated to
> > be per-adapter for our next release (BlueZ 5).
> 
> The registered "watcher" object will have different interface
> depending on the profile. The application which will call
> RegisterWatcher() needs to check that the device supports the expected
> GATT service (e.g. by checking the "UUIDs" property of that device
> object) before using the  org.bluez.HeartRate interface, therefore
> IMHO it makes sense to have RegisterWatcher() on the device object
> path.

I'm not quite following. You'd still have per-profile interfaces on the
adapter path.

> Unless you are proposing a generic Watcher API that could somehow be
> shared by all profiles (including a single shared interface for the
> watcher object)? How to represent data from profiles which are not
> "measurement" based?

I suppose you were directing this at Rafal and not me? I don't think it
makes sense to merge these. I was only saying that the interfaces should
go from Device to Adapter.

Johan

Re: [PATCH v2 01/13] Heart Rate Profile API

[Anderson Lizardo]

Hi Johan,

On Tue, Aug 28, 2012 at 2:10 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
>> The registered "watcher" object will have different interface
>> depending on the profile. The application which will call
>> RegisterWatcher() needs to check that the device supports the expected
>> GATT service (e.g. by checking the "UUIDs" property of that device
>> object) before using the  org.bluez.HeartRate interface, therefore
>> IMHO it makes sense to have RegisterWatcher() on the device object
>> path.
>
> I'm not quite following. You'd still have per-profile interfaces on the
> adapter path.

What I meant, is that, before using RegisterWatcher(), you need to
check the "UUIDs" property on the device path. So you still need to
either enumerate all device objects and read their "UUIDs" property,
or monitor the "PropertiesChanged" signal (once the new Property API
is upstream).

So IMHO ff you need to access the device object, there is no gain in
moving the method to adapter object.

>> Unless you are proposing a generic Watcher API that could somehow be
>> shared by all profiles (including a single shared interface for the
>> watcher object)? How to represent data from profiles which are not
>> "measurement" based?
>
> I suppose you were directing this at Rafal and not me? I don't think it
> makes sense to merge these. I was only saying that the interfaces should
> go from Device to Adapter.

No, I was commenting specifically on your suggestion to move
RegisterWatcher() to the adapter path and have a "device" parameter to
select the device. The user of this API would still need to check if
the device actually supports that wanted GATT service (using the UUIDs
device property) before registering any watcher.

Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

Re: [PATCH v2 01/13] Heart Rate Profile API

[Anderson Lizardo]

Hi,

After some discussion with Johan on IRC, I think I understand what he
is proposing, and it makes sense IMHO. The idea is to simply replace
the object path for RegisterWatcher/UnregisterWatcher to [variable
prefix]/{hci0,hci1,...} (adapter path) and have the watcher object
methods accept a "object device" as first argument.

This way, the application that wants to register wacthers will monitor
only for new adapters (and not new devices) and register watchers on
them. The D-Bus interface for RegisterWatcher() will still be profile
specific (e.g. org.bluez.HeartRate in this case).

One issue with this idea (and which still needs to be sorted out if we
proceed with this), is that other methods specific to the profile
(like Reset() for Heart Rate and EnableIntermediateMeasurement() for
Thermometer) need to be in different interfaces.

Johan, do you have any ideas for this?

Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

Re: [PATCH v2 01/13] Heart Rate Profile API

[Garbat Rafal]

Hi,

On 08/28/2012 09:21 PM, Anderson Lizardo wrote:
> Hi,
>
> After some discussion with Johan on IRC, I think I understand what he
> is proposing, and it makes sense IMHO. The idea is to simply replace
> the object path for RegisterWatcher/UnregisterWatcher to [variable
> prefix]/{hci0,hci1,...} (adapter path) and have the watcher object
> methods accept a "object device" as first argument.
>
> This way, the application that wants to register wacthers will monitor
> only for new adapters (and not new devices) and register watchers on
> them. The D-Bus interface for RegisterWatcher() will still be profile
> specific (e.g. org.bluez.HeartRate in this case).
>
> One issue with this idea (and which still needs to be sorted out if we
> proceed with this), is that other methods specific to the profile
> (like Reset() for Heart Rate and EnableIntermediateMeasurement() for
> Thermometer) need to be in different interfaces.
>
> Johan, do you have any ideas for this?
>
> Regards,
I guess, that with such approach, the HeartRate API could look something 
like this :

BlueZ D-Bus Heart Rate API description
****************************************

Copyright (C) 2012    Santiago Carot-Nemesio <sancane@gmail.com>

Heart Rate Manager hierarchy
============================

Service        org.bluez
Interface    org.bluez.HeartRateManager
Object path    [variable prefix]/{hci0,hci1,...}

Methods        RegisterWatcher(object agent)

                           Registers a watcher to monitor heart rate 
measurements.

                           Possible Errors: org.bluez.Error.InvalidArguments

                       UnregisterWatcher(object agent)

                           Unregisters a watcher.

Heart Rate Profile hierarchy
============================

Service        org.bluez
Interface    org.bluez.HeartRate
Object path    [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX

Methods        dict GetProperties()

                         Returns all properties for the interface. See the
                         Properties section for the available properties.

                       Reset()

                         Restart the accumulation of energy expended 
from zero.

                         Possible Errors: org.bluez.Error.NotSupported

Properties    boolean ResetSupported [readonly]

                         True if energy expended is supported.

Heart Rate Watcher hierarchy

============================
Service        unique name
Interface    org.bluez.HeartRateWatcher
Object path    freely definable

Methods        void MeasurementReceived(object device, dict measure)

             This callback is called whenever a heart rate
             measurement is received from the heart rate device.
             The unit for the Value is expressed in beats per
             minute (bpm). The energy field is optional and
             represents the accumulated energy expended in
             kilo Joules since last time it was reset. Furthermore,
             the device will be automatically reset when it
             is needed.
             The Contact field, if present, indicates
             that the device supports contact sensor, besides it
             will be true if skin contact is detected. The optional
             interval field is an array containing RR-Interval
             values which represent the time between two R-Wave
             detections, where the RR-Interval value 0 is older
             than the value 1 and so on.

             Dict is defined as below:
             {
                 "Value" : uint16,
                 "Energy" : uint16,
                 "Contact" : boolean,
                 "Location" : ("Other", "Chest", "Wrist","Finger",
                     "Hand", "Earlobe", "Foot"),
                 "Interval" : array{uint16}
             }


Just a proposal, please comment.

Rafal

Re: [PATCH v2 01/13] Heart Rate Profile API

[Anderson Lizardo]

Hi Rafal,

> I guess, that with such approach, the HeartRate API could look something
> like this :
> [snip]
> Just a proposal, please comment.

This is my understanding as well. But I'll wait for Johan to comment
to make sure we fully understood the idea.

Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

Re: [PATCH v2 01/13] Heart Rate Profile API

[Johan Hedberg]

Hi Lizardo,

On Wed, Aug 29, 2012, Anderson Lizardo wrote:
> Hi Rafal,
> 
> > I guess, that with such approach, the HeartRate API could look something
> > like this :
> > [snip]
> > Just a proposal, please comment.
> 
> This is my understanding as well. But I'll wait for Johan to comment
> to make sure we fully understood the idea.

Looks more or less good to me.

Johan