[PATCH 00/14] Thermometer watchers API change + fixes

[PATCH 00/14] Thermometer watchers API change + fixes

[Andrzej Kaczmarek]

Hi,

Here's series of patches to move watchers from per-device to per-adapter
interface (basically the same as done for HRP, see other series of patches).
This applies to both final and intermediate temperature watchers - once
watcher is registered, all connected devices are setup to send measurement
notifications and the opposite when last watcher is unregistered.

This major change is done in patches 1-6.

Patches 7-11 contain some code and documentation refactoring, i.e. to
remove duplicated code.

Patches 12-14 are trival coding style fixes.

Unfortunately, I wasn't able to fully test this code due to lack of proper
thermometer device and sample device from CC2540 devkit does not quite
work as expected (e.g. I can see CCC is written properly when watcher is
registered but remote does not send any indications for unknown reason).



Andrzej Kaczmarek (14):
  thermometer: Store thermometer devices per-adapter
  thermometer: Register ThermometerManager interface on adapter path
  thermometer: Move watcher logic to adapter interface
  thermometer: Include remote device information in MeasurementReceived
  thermometer: Update API document
  thermometer: Update test script
  thermometer: Reformat MeasurementReceived description
  thermometer: Update driver naming style
  thermometer: Add constant definition for watcher interface name
  thermometer: Add common function to write characteristics CCC
  thermometer: Refactor processing of measurement characteristic value
  thermometer: Fix whitespace
  thermometer: Fix indentation
  thermometer: Fix missing braces

 doc/thermometer-api.txt            | 126 +++++-----
 profiles/thermometer/manager.c     |  24 +-
 profiles/thermometer/thermometer.c | 486 +++++++++++++++++++++----------------
 profiles/thermometer/thermometer.h |   2 +
 test/test-thermometer              |  16 +-
 5 files changed, 372 insertions(+), 282 deletions(-)

-- 
1.7.11.3

[PATCH 01/14] thermometer: Store thermometer devices per-adapter

[Andrzej Kaczmarek]

This patch replaces global list of thermometer devices with per-adapter
lists.
---
 profiles/thermometer/manager.c     |  16 +++++-
 profiles/thermometer/thermometer.c | 114 ++++++++++++++++++++++++++++++-------
 profiles/thermometer/thermometer.h |   2 +
 3 files changed, 110 insertions(+), 22 deletions(-)

diff --git a/profiles/thermometer/manager.c b/profiles/thermometer/manager.c
index d965976..86e3a0f 100644
--- a/profiles/thermometer/manager.c
+++ b/profiles/thermometer/manager.c
@@ -66,11 +66,25 @@ static void thermometer_driver_remove(struct btd_profile *p,
 	thermometer_unregister(device);
 }
 
+static int thermometer_adapter_probe(struct btd_profile *p,
+						struct btd_adapter *adapter)
+{
+	return thermometer_adapter_register(adapter);
+}
+
+static void thermometer_adapter_remove(struct btd_profile *p,
+						struct btd_adapter *adapter)
+{
+	thermometer_adapter_unregister(adapter);
+}
+
 static struct btd_profile thermometer_profile = {
 	.name		= "thermometer-device-driver",
 	.remote_uuids	= BTD_UUIDS(HEALTH_THERMOMETER_UUID),
 	.device_probe	= thermometer_driver_probe,
-	.device_remove	= thermometer_driver_remove
+	.device_remove	= thermometer_driver_remove,
+	.adapter_probe	= thermometer_adapter_probe,
+	.adapter_remove	= thermometer_adapter_remove
 };
 
 int thermometer_manager_init(void)
diff --git a/profiles/thermometer/thermometer.c b/profiles/thermometer/thermometer.c
index 77dcb26..c8a6826 100644
--- a/profiles/thermometer/thermometer.c
+++ b/profiles/thermometer/thermometer.c
@@ -55,23 +55,29 @@
 #define TEMPERATURE_TYPE_SIZE	1
 #define MEASUREMENT_INTERVAL_SIZE	2
 
+struct thermometer_adapter {
+	struct btd_adapter	*adapter;
+	GSList			*devices;
+};
+
 struct thermometer {
-	struct btd_device	*dev;		/* Device reference */
-	GAttrib			*attrib;	/* GATT connection */
-	struct att_range	*svc_range;	/* Thermometer range */
-	guint			attioid;	/* Att watcher id */
-	guint			attindid;	/* Att incications id */
-	guint			attnotid;	/* Att notifications id */
-	GSList			*chars;		/* Characteristics */
-	GSList			*fwatchers;     /* Final measurements */
-	GSList			*iwatchers;     /* Intermediate measurements */
-	gboolean		intermediate;
-	uint8_t			type;
-	uint16_t		interval;
-	uint16_t		max;
-	uint16_t		min;
-	gboolean		has_type;
-	gboolean		has_interval;
+	struct btd_device		*dev;		/* Device reference */
+	struct thermometer_adapter	*tadapter;
+	GAttrib				*attrib;	/* GATT connection */
+	struct att_range		*svc_range;	/* Thermometer range */
+	guint				attioid;	/* Att watcher id */
+	guint				attindid;	/* Att incications id */
+	guint				attnotid;	/* Att notif id */
+	GSList				*chars;		/* Characteristics */
+	GSList				*fwatchers;     /* Final measurements */
+	GSList				*iwatchers;     /* Intermediate meas  */
+	gboolean			intermediate;
+	uint8_t				type;
+	uint16_t			interval;
+	uint16_t			max;
+	uint16_t			min;
+	gboolean			has_type;
+	gboolean			has_interval;
 };
 
 struct characteristic {
@@ -108,7 +114,7 @@ struct tmp_interval_data {
 	uint16_t		interval;
 };
 
-static GSList *thermometers = NULL;
+static GSList *thermometer_adapters = NULL;
 
 const char *temp_type[] = {
 	"<reserved>",
@@ -183,6 +189,17 @@ static void destroy_thermometer(gpointer user_data)
 	g_free(t);
 }
 
+static gint cmp_adapter(gconstpointer a, gconstpointer b)
+{
+	const struct thermometer_adapter *tadapter = a;
+	const struct btd_adapter *adapter = b;
+
+	if (adapter == tadapter->adapter)
+		return 0;
+
+	return -1;
+}
+
 static gint cmp_device(gconstpointer a, gconstpointer b)
 {
 	const struct thermometer *t = a;
@@ -231,6 +248,17 @@ static gint cmp_descriptor(gconstpointer a, gconstpointer b)
 	return bt_uuid_cmp(&desc->uuid, uuid);
 }
 
+static struct thermometer_adapter *
+find_thermometer_adapter(struct btd_adapter *adapter)
+{
+	GSList *l = g_slist_find_custom(thermometer_adapters, adapter,
+								cmp_adapter);
+	if (!l)
+		return NULL;
+
+	return l->data;
+}
+
 static struct characteristic *get_characteristic(struct thermometer *t,
 							const char *uuid)
 {
@@ -1233,13 +1261,25 @@ int thermometer_register(struct btd_device *device, struct gatt_primary *tattr)
 {
 	const gchar *path = device_get_path(device);
 	struct thermometer *t;
+	struct btd_adapter *adapter;
+	struct thermometer_adapter *tadapter;
+
+	adapter = device_get_adapter(device);
+
+	tadapter = find_thermometer_adapter(adapter);
+
+	if (tadapter == NULL)
+		return -1;
 
 	t = g_new0(struct thermometer, 1);
 	t->dev = btd_device_ref(device);
+	t->tadapter = tadapter;
 	t->svc_range = g_new0(struct att_range, 1);
 	t->svc_range->start = tattr->range.start;
 	t->svc_range->end = tattr->range.end;
 
+	tadapter->devices = g_slist_prepend(tadapter->devices, t);
+
 	if (!g_dbus_register_interface(btd_get_dbus_connection(),
 				path, THERMOMETER_INTERFACE,
 				thermometer_methods, thermometer_signals,
@@ -1250,8 +1290,6 @@ int thermometer_register(struct btd_device *device, struct gatt_primary *tattr)
 		return -EIO;
 	}
 
-	thermometers = g_slist_prepend(thermometers, t);
-
 	t->attioid = btd_device_add_attio_callback(device, attio_connected_cb,
 						attio_disconnected_cb, t);
 	return 0;
@@ -1260,14 +1298,48 @@ int thermometer_register(struct btd_device *device, struct gatt_primary *tattr)
 void thermometer_unregister(struct btd_device *device)
 {
 	struct thermometer *t;
+	struct btd_adapter *adapter;
+	struct thermometer_adapter *tadapter;
 	GSList *l;
 
-	l = g_slist_find_custom(thermometers, device, cmp_device);
+	adapter = device_get_adapter(device);
+
+	tadapter = find_thermometer_adapter(adapter);
+
+	if (tadapter == NULL)
+		return;
+
+	l = g_slist_find_custom(tadapter->devices, device, cmp_device);
 	if (l == NULL)
 		return;
 
 	t = l->data;
-	thermometers = g_slist_remove(thermometers, t);
+
+	tadapter->devices = g_slist_remove(tadapter->devices, t);
+
 	g_dbus_unregister_interface(btd_get_dbus_connection(),
 				device_get_path(t->dev), THERMOMETER_INTERFACE);
 }
+
+int thermometer_adapter_register(struct btd_adapter *adapter)
+{
+	struct thermometer_adapter *tadapter;
+
+	tadapter = g_new0(struct thermometer_adapter, 1);
+	tadapter->adapter = adapter;
+
+	thermometer_adapters = g_slist_prepend(thermometer_adapters, tadapter);
+
+	return 0;
+}
+
+void thermometer_adapter_unregister(struct btd_adapter *adapter)
+{
+	struct thermometer_adapter *tadapter;
+
+	tadapter = find_thermometer_adapter(adapter);
+	if (tadapter == NULL)
+		return;
+
+	thermometer_adapters = g_slist_remove(thermometer_adapters, tadapter);
+}
diff --git a/profiles/thermometer/thermometer.h b/profiles/thermometer/thermometer.h
index 2744ed6..ca83c31 100644
--- a/profiles/thermometer/thermometer.h
+++ b/profiles/thermometer/thermometer.h
@@ -22,3 +22,5 @@
 
 int thermometer_register(struct btd_device *device, struct gatt_primary *tattr);
 void thermometer_unregister(struct btd_device *device);
+int thermometer_adapter_register(struct btd_adapter *adapter);
+void thermometer_adapter_unregister(struct btd_adapter *adapter);
-- 
1.7.11.3

[PATCH 02/14] thermometer: Register ThermometerManager interface on adapter path

[Andrzej Kaczmarek]

---
 profiles/thermometer/thermometer.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/profiles/thermometer/thermometer.c b/profiles/thermometer/thermometer.c
index c8a6826..f2568c9 100644
--- a/profiles/thermometer/thermometer.c
+++ b/profiles/thermometer/thermometer.c
@@ -42,7 +42,8 @@
 #include "gatt.h"
 #include "thermometer.h"
 
-#define THERMOMETER_INTERFACE "org.bluez.Thermometer"
+#define THERMOMETER_INTERFACE		"org.bluez.Thermometer"
+#define THERMOMETER_MANAGER_INTERFACE	"org.bluez.ThermometerManager"
 
 /* Temperature measurement flag fields */
 #define TEMP_UNITS		0x01
@@ -189,6 +190,16 @@ static void destroy_thermometer(gpointer user_data)
 	g_free(t);
 }
 
+static void destroy_thermometer_adapter(gpointer user_data)
+{
+	struct thermometer_adapter *tadapter = user_data;
+
+	if (tadapter->devices != NULL)
+		g_slist_free_full(tadapter->devices, destroy_thermometer);
+
+	g_free(tadapter);
+}
+
 static gint cmp_adapter(gconstpointer a, gconstpointer b)
 {
 	const struct thermometer_adapter *tadapter = a;
@@ -1321,6 +1332,10 @@ void thermometer_unregister(struct btd_device *device)
 				device_get_path(t->dev), THERMOMETER_INTERFACE);
 }
 
+static const GDBusMethodTable thermometer_manager_methods[] = {
+	{ }
+};
+
 int thermometer_adapter_register(struct btd_adapter *adapter)
 {
 	struct thermometer_adapter *tadapter;
@@ -1328,6 +1343,18 @@ int thermometer_adapter_register(struct btd_adapter *adapter)
 	tadapter = g_new0(struct thermometer_adapter, 1);
 	tadapter->adapter = adapter;
 
+	if (!g_dbus_register_interface(btd_get_dbus_connection(),
+						adapter_get_path(adapter),
+						THERMOMETER_MANAGER_INTERFACE,
+						thermometer_manager_methods,
+						NULL, NULL, tadapter,
+						destroy_thermometer_adapter)) {
+		error("D-Bus failed to register %s interface",
+						THERMOMETER_MANAGER_INTERFACE);
+		destroy_thermometer_adapter(tadapter);
+		return -EIO;
+	}
+
 	thermometer_adapters = g_slist_prepend(thermometer_adapters, tadapter);
 
 	return 0;
@@ -1342,4 +1369,8 @@ void thermometer_adapter_unregister(struct btd_adapter *adapter)
 		return;
 
 	thermometer_adapters = g_slist_remove(thermometer_adapters, tadapter);
+
+	g_dbus_unregister_interface(btd_get_dbus_connection(),
+					adapter_get_path(tadapter->adapter),
+					THERMOMETER_MANAGER_INTERFACE);
 }
-- 
1.7.11.3

[PATCH 03/14] thermometer: Move watcher logic to adapter interface

[Andrzej Kaczmarek]

This patch moves watcher related methods to ThermometerManager interface
so watchers can be registered per-adapter instead of per-device.

Final measurement notifications are now enabled on all devices connected
to given adapter when first watcher is registered and disabled when last
watcher is unregistered.

Intermediate measurement notifications are enabled for all devices
connected to given adapter which support this feature.
---
 profiles/thermometer/thermometer.c | 134 +++++++++++++++++++------------------
 1 file changed, 70 insertions(+), 64 deletions(-)

diff --git a/profiles/thermometer/thermometer.c b/profiles/thermometer/thermometer.c
index f2568c9..4eb67a7 100644
--- a/profiles/thermometer/thermometer.c
+++ b/profiles/thermometer/thermometer.c
@@ -59,6 +59,8 @@
 struct thermometer_adapter {
 	struct btd_adapter	*adapter;
 	GSList			*devices;
+	GSList			*fwatchers;	/* Final measurements */
+	GSList			*iwatchers;	/* Intermediate measurements */
 };
 
 struct thermometer {
@@ -70,8 +72,6 @@ struct thermometer {
 	guint				attindid;	/* Att incications id */
 	guint				attnotid;	/* Att notif id */
 	GSList				*chars;		/* Characteristics */
-	GSList				*fwatchers;     /* Final measurements */
-	GSList				*iwatchers;     /* Intermediate meas  */
 	gboolean			intermediate;
 	uint8_t				type;
 	uint16_t			interval;
@@ -94,10 +94,10 @@ struct descriptor {
 };
 
 struct watcher {
-	struct thermometer	*t;
-	guint			id;
-	char			*srv;
-	char			*path;
+	struct thermometer_adapter	*tadapter;
+	guint				id;
+	char				*srv;
+	char				*path;
 };
 
 struct measurement {
@@ -182,9 +182,6 @@ static void destroy_thermometer(gpointer user_data)
 	if (t->chars != NULL)
 		g_slist_free_full(t->chars, destroy_char);
 
-	if (t->fwatchers != NULL)
-		g_slist_free_full(t->fwatchers, remove_watcher);
-
 	btd_device_unref(t->dev);
 	g_free(t->svc_range);
 	g_free(t);
@@ -197,6 +194,9 @@ static void destroy_thermometer_adapter(gpointer user_data)
 	if (tadapter->devices != NULL)
 		g_slist_free_full(tadapter->devices, destroy_thermometer);
 
+	if (tadapter->fwatchers != NULL)
+		g_slist_free_full(tadapter->fwatchers, remove_watcher);
+
 	g_free(tadapter);
 }
 
@@ -400,7 +400,7 @@ static void process_thermometer_desc(struct descriptor *desc)
 
 		if (g_strcmp0(ch->attr.uuid,
 					TEMPERATURE_MEASUREMENT_UUID) == 0) {
-			if (g_slist_length(ch->t->fwatchers) == 0)
+			if (g_slist_length(ch->t->tadapter->fwatchers) == 0)
 				return;
 
 			val = GATT_CLIENT_CHARAC_CFG_IND_BIT;
@@ -408,7 +408,7 @@ static void process_thermometer_desc(struct descriptor *desc)
 								"indication");
 		} else if (g_strcmp0(ch->attr.uuid,
 					INTERMEDIATE_TEMPERATURE_UUID) == 0) {
-			if (g_slist_length(ch->t->iwatchers) == 0)
+			if (g_slist_length(ch->t->tadapter->iwatchers) == 0)
 				return;
 
 			val = GATT_CLIENT_CHARAC_CFG_NOTIF_BIT;
@@ -718,8 +718,9 @@ static DBusMessage *set_property(DBusConnection *conn, DBusMessage *msg,
 	return write_attr_interval(t, msg, value);
 }
 
-static void enable_final_measurement(struct thermometer *t)
+static void enable_final_measurement(gpointer data, gpointer user_data)
 {
+	struct thermometer *t = data;
 	struct characteristic *ch;
 	struct descriptor *desc;
 	bt_uuid_t btuuid;
@@ -748,15 +749,16 @@ static void enable_final_measurement(struct thermometer *t)
 	gatt_write_char(t->attrib, desc->handle, atval, 2, measurement_cb, msg);
 }
 
-static void enable_intermediate_measurement(struct thermometer *t)
+static void enable_intermediate_measurement(gpointer data, gpointer user_data)
 {
+	struct thermometer *t = data;
 	struct characteristic *ch;
 	struct descriptor *desc;
 	bt_uuid_t btuuid;
 	uint8_t atval[2];
 	char *msg;
 
-	if (t->attrib == NULL)
+	if (t->attrib == NULL || !t->intermediate)
 		return;
 
 	ch = get_characteristic(t, INTERMEDIATE_TEMPERATURE_UUID);
@@ -778,8 +780,9 @@ static void enable_intermediate_measurement(struct thermometer *t)
 	gatt_write_char(t->attrib, desc->handle, atval, 2, measurement_cb, msg);
 }
 
-static void disable_final_measurement(struct thermometer *t)
+static void disable_final_measurement(gpointer data, gpointer user_data)
 {
+	struct thermometer *t = data;
 	struct characteristic *ch;
 	struct descriptor *desc;
 	bt_uuid_t btuuid;
@@ -808,8 +811,9 @@ static void disable_final_measurement(struct thermometer *t)
 	gatt_write_char(t->attrib, desc->handle, atval, 2, measurement_cb, msg);
 }
 
-static void disable_intermediate_measurement(struct thermometer *t)
+static void disable_intermediate_measurement(gpointer data, gpointer user_data)
 {
+	struct thermometer *t = data;
 	struct characteristic *ch;
 	struct descriptor *desc;
 	bt_uuid_t btuuid;
@@ -838,31 +842,34 @@ static void disable_intermediate_measurement(struct thermometer *t)
 	gatt_write_char(t->attrib, desc->handle, atval, 2, measurement_cb, msg);
 }
 
-static void remove_int_watcher(struct thermometer *t, struct watcher *w)
+static void remove_int_watcher(struct thermometer_adapter *tadapter,
+							struct watcher *w)
 {
-	if (!g_slist_find(t->iwatchers, w))
+	if (!g_slist_find(tadapter->iwatchers, w))
 		return;
 
-	t->iwatchers = g_slist_remove(t->iwatchers, w);
+	tadapter->iwatchers = g_slist_remove(tadapter->iwatchers, w);
 
-	if (g_slist_length(t->iwatchers) == 0)
-		disable_intermediate_measurement(t);
+	if (g_slist_length(tadapter->iwatchers) == 0)
+		g_slist_foreach(tadapter->devices,
+					disable_intermediate_measurement, 0);
 }
 
 static void watcher_exit(DBusConnection *conn, void *user_data)
 {
 	struct watcher *watcher = user_data;
-	struct thermometer *t = watcher->t;
+	struct thermometer_adapter *tadapter = watcher->tadapter;
 
 	DBG("Thermometer watcher %s disconnected", watcher->path);
 
-	remove_int_watcher(t, watcher);
+	remove_int_watcher(tadapter, watcher);
 
-	t->fwatchers = g_slist_remove(t->fwatchers, watcher);
+	tadapter->fwatchers = g_slist_remove(tadapter->fwatchers, watcher);
 	g_dbus_remove_watch(btd_get_dbus_connection(), watcher->id);
 
-	if (g_slist_length(t->fwatchers) == 0)
-		disable_final_measurement(t);
+	if (g_slist_length(tadapter->fwatchers) == 0)
+		g_slist_foreach(tadapter->devices,
+					disable_final_measurement, 0);
 }
 
 static struct watcher *find_watcher(GSList *list, const char *sender,
@@ -888,7 +895,7 @@ static DBusMessage *register_watcher(DBusConnection *conn, DBusMessage *msg,
 								void *data)
 {
 	const char *sender = dbus_message_get_sender(msg);
-	struct thermometer *t = data;
+	struct thermometer_adapter *tadapter = data;
 	struct watcher *watcher;
 	char *path;
 
@@ -896,7 +903,7 @@ static DBusMessage *register_watcher(DBusConnection *conn, DBusMessage *msg,
 							DBUS_TYPE_INVALID))
 		return btd_error_invalid_args(msg);
 
-	watcher = find_watcher(t->fwatchers, sender, path);
+	watcher = find_watcher(tadapter->fwatchers, sender, path);
 	if (watcher != NULL)
 		return btd_error_already_exists(msg);
 
@@ -905,14 +912,14 @@ static DBusMessage *register_watcher(DBusConnection *conn, DBusMessage *msg,
 	watcher = g_new0(struct watcher, 1);
 	watcher->srv = g_strdup(sender);
 	watcher->path = g_strdup(path);
-	watcher->t = t;
+	watcher->tadapter = tadapter;
 	watcher->id = g_dbus_add_disconnect_watch(conn, sender, watcher_exit,
 						watcher, destroy_watcher);
 
-	if (g_slist_length(t->fwatchers) == 0)
-		enable_final_measurement(t);
+	if (g_slist_length(tadapter->fwatchers) == 0)
+		g_slist_foreach(tadapter->devices, enable_final_measurement, 0);
 
-	t->fwatchers = g_slist_prepend(t->fwatchers, watcher);
+	tadapter->fwatchers = g_slist_prepend(tadapter->fwatchers, watcher);
 
 	return dbus_message_new_method_return(msg);
 }
@@ -921,7 +928,7 @@ static DBusMessage *unregister_watcher(DBusConnection *conn, DBusMessage *msg,
 								void *data)
 {
 	const char *sender = dbus_message_get_sender(msg);
-	struct thermometer *t = data;
+	struct thermometer_adapter *tadapter = data;
 	struct watcher *watcher;
 	char *path;
 
@@ -929,19 +936,20 @@ static DBusMessage *unregister_watcher(DBusConnection *conn, DBusMessage *msg,
 							DBUS_TYPE_INVALID))
 		return btd_error_invalid_args(msg);
 
-	watcher = find_watcher(t->fwatchers, sender, path);
+	watcher = find_watcher(tadapter->fwatchers, sender, path);
 	if (watcher == NULL)
 		return btd_error_does_not_exist(msg);
 
 	DBG("Thermometer watcher %s unregistered", path);
 
-	remove_int_watcher(t, watcher);
+	remove_int_watcher(tadapter, watcher);
 
-	t->fwatchers = g_slist_remove(t->fwatchers, watcher);
+	tadapter->fwatchers = g_slist_remove(tadapter->fwatchers, watcher);
 	g_dbus_remove_watch(btd_get_dbus_connection(), watcher->id);
 
-	if (g_slist_length(t->fwatchers) == 0)
-		disable_final_measurement(t);
+	if (g_slist_length(tadapter->fwatchers) == 0)
+		g_slist_foreach(tadapter->devices,
+					disable_final_measurement, 0);
 
 	return dbus_message_new_method_return(msg);
 }
@@ -950,30 +958,28 @@ static DBusMessage *enable_intermediate(DBusConnection *conn, DBusMessage *msg,
 								void *data)
 {
 	const char *sender = dbus_message_get_sender(msg);
-	struct thermometer *t = data;
+	struct thermometer_adapter *ta = data;
 	struct watcher *watcher;
 	char *path;
 
-	if (!t->intermediate)
-		return btd_error_not_supported(msg);
-
 	if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
 							DBUS_TYPE_INVALID))
 		return btd_error_invalid_args(msg);
 
-	watcher = find_watcher(t->fwatchers, sender, path);
+	watcher = find_watcher(ta->fwatchers, sender, path);
 	if (watcher == NULL)
 		return btd_error_does_not_exist(msg);
 
-	if (find_watcher(t->iwatchers, sender, path))
+	if (find_watcher(ta->iwatchers, sender, path))
 		return btd_error_already_exists(msg);
 
 	DBG("Intermediate measurement watcher %s registered", path);
 
-	if (g_slist_length(t->iwatchers) == 0)
-		enable_intermediate_measurement(t);
+	if (g_slist_length(ta->iwatchers) == 0)
+		g_slist_foreach(ta->devices,
+					enable_intermediate_measurement, 0);
 
-	t->iwatchers = g_slist_prepend(t->iwatchers, watcher);
+	ta->iwatchers = g_slist_prepend(ta->iwatchers, watcher);
 
 	return dbus_message_new_method_return(msg);
 }
@@ -982,7 +988,7 @@ static DBusMessage *disable_intermediate(DBusConnection *conn, DBusMessage *msg,
 								void *data)
 {
 	const char *sender = dbus_message_get_sender(msg);
-	struct thermometer *t = data;
+	struct thermometer_adapter *ta = data;
 	struct watcher *watcher;
 	char *path;
 
@@ -990,13 +996,13 @@ static DBusMessage *disable_intermediate(DBusConnection *conn, DBusMessage *msg,
 							DBUS_TYPE_INVALID))
 		return btd_error_invalid_args(msg);
 
-	watcher = find_watcher(t->iwatchers, sender, path);
+	watcher = find_watcher(ta->iwatchers, sender, path);
 	if (watcher == NULL)
 		return btd_error_does_not_exist(msg);
 
 	DBG("Intermediate measurement %s unregistered", path);
 
-	remove_int_watcher(t, watcher);
+	remove_int_watcher(ta, watcher);
 
 	return dbus_message_new_method_return(msg);
 }
@@ -1008,18 +1014,6 @@ static const GDBusMethodTable thermometer_methods[] = {
 	{ GDBUS_ASYNC_METHOD("SetProperty",
 			GDBUS_ARGS({ "name", "s" }, { "value", "v" }), NULL,
 			set_property) },
-	{ GDBUS_METHOD("RegisterWatcher",
-			GDBUS_ARGS({ "agent", "o" }), NULL,
-			register_watcher) },
-	{ GDBUS_METHOD("UnregisterWatcher",
-			GDBUS_ARGS({ "agent", "o" }), NULL,
-			unregister_watcher) },
-	{ GDBUS_METHOD("EnableIntermediateMeasurement",
-			GDBUS_ARGS({ "agent", "o" }), NULL,
-			enable_intermediate) },
-	{ GDBUS_METHOD("DisableIntermediateMeasurement",
-			GDBUS_ARGS({ "agent", "o" }), NULL,
-			disable_intermediate) },
 	{ }
 };
 
@@ -1071,9 +1065,9 @@ static void recv_measurement(struct thermometer *t, struct measurement *m)
 	GSList *wlist;
 
 	if (g_strcmp0(m->value, "Intermediate") == 0)
-		wlist = t->iwatchers;
+		wlist = t->tadapter->iwatchers;
 	else
-		wlist = t->fwatchers;
+		wlist = t->tadapter->fwatchers;
 
 	g_slist_foreach(wlist, update_watcher, m);
 }
@@ -1333,6 +1327,18 @@ void thermometer_unregister(struct btd_device *device)
 }
 
 static const GDBusMethodTable thermometer_manager_methods[] = {
+	{ GDBUS_METHOD("RegisterWatcher",
+			GDBUS_ARGS({ "agent", "o" }), NULL,
+			register_watcher) },
+	{ GDBUS_METHOD("UnregisterWatcher",
+			GDBUS_ARGS({ "agent", "o" }), NULL,
+			unregister_watcher) },
+	{ GDBUS_METHOD("EnableIntermediateMeasurement",
+			GDBUS_ARGS({ "agent", "o" }), NULL,
+			enable_intermediate) },
+	{ GDBUS_METHOD("DisableIntermediateMeasurement",
+			GDBUS_ARGS({ "agent", "o" }), NULL,
+			disable_intermediate) },
 	{ }
 };
 
-- 
1.7.11.3

[PATCH 04/14] thermometer: Include remote device information in MeasurementReceived

[Andrzej Kaczmarek]

Since watchers are now registered per-adapter it's necessary to include
remote device information in MeasurementReceived callback.

This patch adds parameter to MeasurementReceived method which is an
object path to remote device object.
---
 profiles/thermometer/thermometer.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/profiles/thermometer/thermometer.c b/profiles/thermometer/thermometer.c
index 4eb67a7..f0c1d67 100644
--- a/profiles/thermometer/thermometer.c
+++ b/profiles/thermometer/thermometer.c
@@ -101,13 +101,14 @@ struct watcher {
 };
 
 struct measurement {
-	int16_t		exp;
-	int32_t		mant;
-	uint64_t	time;
-	gboolean	suptime;
-	char		*unit;
-	char		*type;
-	char		*value;
+	struct thermometer	*t;
+	int16_t			exp;
+	int32_t			mant;
+	uint64_t		time;
+	gboolean		suptime;
+	char			*unit;
+	char			*type;
+	char			*value;
 };
 
 struct tmp_interval_data {
@@ -1027,6 +1028,7 @@ static void update_watcher(gpointer data, gpointer user_data)
 {
 	struct watcher *w = data;
 	struct measurement *m = user_data;
+	const gchar *path = device_get_path(m->t->dev);
 	DBusMessageIter iter;
 	DBusMessageIter dict;
 	DBusMessage *msg;
@@ -1039,6 +1041,8 @@ static void update_watcher(gpointer data, gpointer user_data)
 
 	dbus_message_iter_init_append(msg, &iter);
 
+	dbus_message_iter_append_basic(&iter, DBUS_TYPE_OBJECT_PATH , &path);
+
 	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
@@ -1064,6 +1068,8 @@ static void recv_measurement(struct thermometer *t, struct measurement *m)
 {
 	GSList *wlist;
 
+	m->t = t;
+
 	if (g_strcmp0(m->value, "Intermediate") == 0)
 		wlist = t->tadapter->iwatchers;
 	else
-- 
1.7.11.3

[PATCH 05/14] thermometer: Update API document

[Andrzej Kaczmarek]

This patch updates Thermometer API document to reflect changes in
Thermometer interface and introduction of ThermometerManager
interface.
---
 doc/thermometer-api.txt | 59 +++++++++++++++++++++++++------------------------
 1 file changed, 30 insertions(+), 29 deletions(-)

diff --git a/doc/thermometer-api.txt b/doc/thermometer-api.txt
index 2271270..b0b0fe2 100644
--- a/doc/thermometer-api.txt
+++ b/doc/thermometer-api.txt
@@ -3,28 +3,14 @@ BlueZ D-Bus Thermometer API description
 
 	Santiago Carot-Nemesio <sancane@gmail.com>
 
-Health Thermometer Profile hierarchy
-=====================================
-
-Service		org.bluez
-Interface	org.bluez.Thermometer
-Object path	[variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
-
-
-Methods		void SetProperty(string name, variant value)
-
-			Changes the value of the specified property. Only
-			read-write properties can be changed. On success
-			this will emit a PropertyChanged signal.
+Health Thermometer Manager hierarchy
+============================
 
-			Possible Errors: org.bluez.Error.InvalidArguments
+Service 	org.bluez
+Interface	org.bluez.ThermometerManager
+Object path	[variable prefix]/{hci0,hci1,...}
 
-		dict GetProperties()
-
-			Returns all properties for the interface. See the
-			Properties section for the available properties.
-
-		RegisterWatcher(object agent)
+Methods 	RegisterWatcher(object agent)
 
 			Registers a watcher to monitor scanned measurements.
 			This agent will be notified about final temperature
@@ -36,30 +22,45 @@ Methods		void SetProperty(string name, variant value)
 
 			Unregisters a watcher.
 
-			Final and intermediate temperatures won't be notified to
-			this agent any more.
-
-			Possible Errors: org.bluez.Error.InvalidArguments
-					org.bluez.Error.NotFound
-
 		EnableIntermediateMeasurement(object agent)
 
 			Enables intermediate measurement notifications for this
-			agent if the thermometer supports it.
+			agent. Intermediate measurements will be enabled only
+			for thermometers which support it.
 
 			Possible Errors: org.bluez.Error.InvalidArguments
-					org.bluez.Error.NotSupported
 
 		DisableIntermediateMeasurement(object agent)
 
 			Disables intermediate measurement notifications for this
-			agent. It will disable notifications in the thermometer
+			agent. It will disable notifications in thermometers
 			when the last agent removes the watcher for intermediate
 			measurements.
 
 			Possible Errors: org.bluez.Error.InvalidArguments
 					org.bluez.Error.NotFound
 
+Health Thermometer Profile hierarchy
+=====================================
+
+Service		org.bluez
+Interface	org.bluez.Thermometer
+Object path	[variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
+
+
+Methods		void SetProperty(string name, variant value)
+
+			Changes the value of the specified property. Only
+			read-write properties can be changed. On success
+			this will emit a PropertyChanged signal.
+
+			Possible Errors: org.bluez.Error.InvalidArguments
+
+		dict GetProperties()
+
+			Returns all properties for the interface. See the
+			Properties section for the available properties.
+
 Signals		PropertyChanged(string name, variant value)
 
 			This signal indicates a changed value of the given
-- 
1.7.11.3

Re: [PATCH 05/14] thermometer: Update API document

[Johan Hedberg]

Hi Andrzej,

On Tue, Sep 25, 2012, Andrzej Kaczmarek wrote:
> This patch updates Thermometer API document to reflect changes in
> Thermometer interface and introduction of ThermometerManager
> interface.
> ---
>  doc/thermometer-api.txt | 59 +++++++++++++++++++++++++------------------------
>  1 file changed, 30 insertions(+), 29 deletions(-)
> 
> diff --git a/doc/thermometer-api.txt b/doc/thermometer-api.txt
> index 2271270..b0b0fe2 100644
> --- a/doc/thermometer-api.txt
> +++ b/doc/thermometer-api.txt
> @@ -3,28 +3,14 @@ BlueZ D-Bus Thermometer API description
>  
>  	Santiago Carot-Nemesio <sancane@gmail.com>
>  
> -Health Thermometer Profile hierarchy
> -=====================================

Could you please fix minor formatting inconsistencies like the
underlines (====) being different length than the text above it.

> -Service		org.bluez
> -Interface	org.bluez.Thermometer
> -Object path	[variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> -
> -
> -Methods		void SetProperty(string name, variant value)
> -
> -			Changes the value of the specified property. Only
> -			read-write properties can be changed. On success
> -			this will emit a PropertyChanged signal.
> +Health Thermometer Manager hierarchy
> +============================
>  
> -			Possible Errors: org.bluez.Error.InvalidArguments
> +Service 	org.bluez

Theres some weird space-tab stuff going on with the line above please
fix.

Johan

[PATCH 06/14] thermometer: Update test script

[Andrzej Kaczmarek]

---
 test/test-thermometer | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/test/test-thermometer b/test/test-thermometer
index 9216264..2ca260f 100755
--- a/test/test-thermometer
+++ b/test/test-thermometer
@@ -16,9 +16,9 @@ from optparse import OptionParser, make_option
 
 class Watcher(dbus.service.Object):
 	@dbus.service.method("org.bluez.ThermometerWatcher",
-					in_signature="a{sv}", out_signature="")
-	def MeasurementReceived(self, measure):
-		print(measure["Measurement"], " measurement received")
+					in_signature="oa{sv}", out_signature="")
+	def MeasurementReceived(self, device, measure):
+		print("%s measurement received from %s" % (measure["Measurement"], device))
 		print("Exponent: ", measure["Exponent"])
 		print("Mantissa: ", measure["Mantissa"])
 		print("Unit: ", measure["Unit"])
@@ -66,23 +66,23 @@ if __name__ == "__main__":
 	adapter = dbus.Interface(bus.get_object("org.bluez", adapter_path),
 							"org.bluez.Adapter")
 
+	thermometer_manager = dbus.Interface(bus.get_object("org.bluez",
+				adapter_path), "org.bluez.ThermometerManager")
+
 	device_path = adapter.FindDevice(options.address)
 
 	bus.add_signal_receiver(property_changed, bus_name="org.bluez",
 				dbus_interface="org.bluez.Thermometer",
 				signal_name="PropertyChanged")
 
-	thermometer = dbus.Interface(bus.get_object("org.bluez",
-					device_path), "org.bluez.Thermometer")
-
 	path = "/test/watcher"
 	watcher = Watcher(bus, path)
 
-	thermometer.RegisterWatcher(path)
+	thermometer_manager.RegisterWatcher(path)
 
 	if len(args) > 0:
 		if args[0] == "EnableIntermediateMeasurement":
-			thermometer.EnableIntermediateMeasurement(path)
+			thermometer_manager.EnableIntermediateMeasurement(path)
 		else:
 			print("unknown command")
 			sys.exit(1)
-- 
1.7.11.3

[PATCH 07/14] thermometer: Reformat MeasurementReceived description

[Andrzej Kaczmarek]

MeasurementReceived method description is reformatted to be more
readable and consistent with other API documents.

Special values for Exponent and Mantissa fields specify now only NaN
value as this is only special value defined by HTS specification for
measurement.
---
 doc/thermometer-api.txt | 67 +++++++++++++++++++++++++++----------------------
 1 file changed, 37 insertions(+), 30 deletions(-)

diff --git a/doc/thermometer-api.txt b/doc/thermometer-api.txt
index b0b0fe2..1665aa1 100644
--- a/doc/thermometer-api.txt
+++ b/doc/thermometer-api.txt
@@ -96,33 +96,40 @@ Service		unique name
 Interface	org.bluez.ThermometerWatcher
 Object path	freely definable
 
-Methods		void MeasurementReceived(dict measure)
-
-			This callback gets called when a measure has been
-			scanned	in the thermometer. The Time entry in the dict
-			will be only present if the device supports storing of
-			data. The time value is expressed in seconds since epoch.
-			The value represented is (mantissa) x (10**exponent)
-			See foot note for special values when treating with
-			health devices. The Type entry is only present if the
-			measurement type is known. Otherwise, it is undefined.
-
-			Dict is defined as below:
-			{
-				"Exponent" : int8,
-				"Mantissa" : int32,
-				"Unit" : ("Celsius" or "Fahrenheit"),
-				"Time" : uint64,
-				"Type" : ("Armpit", "Body", "Ear", "Finger",
-					"Intestines", "Mouth", "Rectum", "Toe",
-					"Tympanum"),
-				"Measurement" : ("Final" or "Intermediate"),
-			}
-
-			For special cases, the exponent shall always be zero and
-			the mantissa should be one of following values:
-
-			NRes = -(2**23)
-			NaN = +(2**23-1)
-			INFINITY = (2**23-2)
-			-INFINITY = -(2**23-2)
+Methods		void MeasurementReceived(dict measurement)
+
+			This callback gets called when a measurement has been
+			scanned in the thermometer.
+
+			Measurement:
+
+				int16 Exponent:
+				int32 Mantissa:
+
+					Measurement value is calculated as
+					(Mantissa) x (10^Exponent)
+
+					In case of invalid or missing data,
+					Exponent is set to 0 and Mantissa is
+					set to 2^23-1 (0x7FFFFF hexadecimal).
+
+				string Unit:
+
+					Possible values: "Celsius" or "Fahrenheit"
+
+				uint64 Time (optional):
+
+					Time of measurement, if supported by device.
+					Expressed in seconds since epoch.
+
+				string Type (optional):
+
+					Only present if measurement type is known.
+
+					Possible values: "Armpit", "Body", "Ear",
+						"Finger", "Intestines", "Mouth",
+						"Rectum", "Toe", "Tympanum"
+
+				string Measurement:
+
+					Possible values: "Final" or "Intermediate"
-- 
1.7.11.3

Re: [PATCH 07/14] thermometer: Reformat MeasurementReceived description

[Johan Hedberg]

Hi Andrzej,

On Tue, Sep 25, 2012, Andrzej Kaczmarek wrote:
> +			Measurement:
> +
> +				int16 Exponent:
> +				int32 Mantissa:
> +
> +					Measurement value is calculated as
> +					(Mantissa) x (10^Exponent)
> +
> +					In case of invalid or missing data,
> +					Exponent is set to 0 and Mantissa is
> +					set to 2^23-1 (0x7FFFFF hexadecimal).

Why aren't we using the D-Bus DOUBLE type for this? For invalid/missing
data just leave out these values from the dict. No need to try to
brute-force a 1:1 mapping from the protocol to D-Bus.

> +
> +				string Unit:
> +
> +					Possible values: "Celsius" or "Fahrenheit"

String properties should be lower-case. Please fix (both doc and
implementation).

> +				string Type (optional):
> +
> +					Only present if measurement type is known.
> +
> +					Possible values: "Armpit", "Body", "Ear",
> +						"Finger", "Intestines", "Mouth",
> +						"Rectum", "Toe", "Tympanum"

Same here.

> +				string Measurement:
> +
> +					Possible values: "Final" or "Intermediate"

And here.

Johan

Re: [PATCH 07/14] thermometer: Reformat MeasurementReceived description

[Santiago Carot]

Hi johan,

2012/9/28 Johan Hedberg <johan.hedberg@gmail.com>:
> Hi Andrzej,
>
> On Tue, Sep 25, 2012, Andrzej Kaczmarek wrote:
>> +                     Measurement:
>> +
>> +                             int16 Exponent:
>> +                             int32 Mantissa:
>> +
>> +                                     Measurement value is calculated as
>> +                                     (Mantissa) x (10^Exponent)
>> +
>> +                                     In case of invalid or missing data,
>> +                                     Exponent is set to 0 and Mantissa is
>> +                                     set to 2^23-1 (0x7FFFFF hexadecimal).
>
> Why aren't we using the D-Bus DOUBLE type for this? For invalid/missing
> data just leave out these values from the dict. No need to try to
> brute-force a 1:1 mapping from the protocol to D-Bus.
>

We don't use double type here because these information is valuable
for upper profiles implementing a IEE11073-20601 protocol layer. This
fact is scarcely commented in HTP and better explained in former
specification where there is a need to pack both values (mantissa and
exponent) in special kind of APDUs. Of course we could provide the
double value here, but there will remain the need to know when an
special case occurs: NRes, NaN,INFINITY, -INFINITY.

Furthermore, providing a double value here will force to upper
profiles to do extra stuff to calculate the mantissa and the exponent
values in order to create APDUs, beside they will have to deal with
impreccisions derivated to comparate float point numbers and so it
might make difficult to check when a special case has happened.

these are, as far as I remember, the reasons which we did it in this
way. We were already talking about this fact in the mailing list in
the past but I don't find the thread right now. sorry

Re: [PATCH 07/14] thermometer: Reformat MeasurementReceived description

[Johan Hedberg]

Hi Santiago,

On Fri, Sep 28, 2012, Santiago Carot wrote:
> 2012/9/28 Johan Hedberg <johan.hedberg@gmail.com>:
> > Hi Andrzej,
> >
> > On Tue, Sep 25, 2012, Andrzej Kaczmarek wrote:
> >> +                     Measurement:
> >> +
> >> +                             int16 Exponent:
> >> +                             int32 Mantissa:
> >> +
> >> +                                     Measurement value is calculated as
> >> +                                     (Mantissa) x (10^Exponent)
> >> +
> >> +                                     In case of invalid or missing data,
> >> +                                     Exponent is set to 0 and Mantissa is
> >> +                                     set to 2^23-1 (0x7FFFFF hexadecimal).
> >
> > Why aren't we using the D-Bus DOUBLE type for this? For invalid/missing
> > data just leave out these values from the dict. No need to try to
> > brute-force a 1:1 mapping from the protocol to D-Bus.
> 
> We don't use double type here because these information is valuable
> for upper profiles implementing a IEE11073-20601 protocol layer. This
> fact is scarcely commented in HTP and better explained in former
> specification where there is a need to pack both values (mantissa and
> exponent) in special kind of APDUs. Of course we could provide the
> double value here, but there will remain the need to know when an
> special case occurs: NRes, NaN,INFINITY, -INFINITY.
> 
> Furthermore, providing a double value here will force to upper
> profiles to do extra stuff to calculate the mantissa and the exponent
> values in order to create APDUs, beside they will have to deal with
> impreccisions derivated to comparate float point numbers and so it
> might make difficult to check when a special case has happened.
> 
> these are, as far as I remember, the reasons which we did it in this
> way. We were already talking about this fact in the mailing list in
> the past but I don't find the thread right now. sorry

This is good enough as a justification. I remembered we had discussed
this before but for the details I needed to get my memory refreshed.
Thanks.

Johan

[PATCH 08/14] thermometer: Update driver naming style

[Andrzej Kaczmarek]

This patch changes device probe and remove functions name to include
'device' rather than 'driver' name as it better describes what both
do.

Also profile driver name is changed to better describe that it's now
profile driver rather than device driver only.
---
 profiles/thermometer/manager.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/profiles/thermometer/manager.c b/profiles/thermometer/manager.c
index 86e3a0f..6057633 100644
--- a/profiles/thermometer/manager.c
+++ b/profiles/thermometer/manager.c
@@ -41,7 +41,7 @@ static gint primary_uuid_cmp(gconstpointer a, gconstpointer b)
 	return g_strcmp0(prim->uuid, uuid);
 }
 
-static int thermometer_driver_probe(struct btd_profile *p,
+static int thermometer_device_probe(struct btd_profile *p,
 					struct btd_device *device,
 					GSList *uuids)
 {
@@ -60,7 +60,7 @@ static int thermometer_driver_probe(struct btd_profile *p,
 	return thermometer_register(device, tattr);
 }
 
-static void thermometer_driver_remove(struct btd_profile *p,
+static void thermometer_device_remove(struct btd_profile *p,
 						struct btd_device *device)
 {
 	thermometer_unregister(device);
@@ -79,10 +79,10 @@ static void thermometer_adapter_remove(struct btd_profile *p,
 }
 
 static struct btd_profile thermometer_profile = {
-	.name		= "thermometer-device-driver",
+	.name		= "Health Thermometer GATT driver",
 	.remote_uuids	= BTD_UUIDS(HEALTH_THERMOMETER_UUID),
-	.device_probe	= thermometer_driver_probe,
-	.device_remove	= thermometer_driver_remove,
+	.device_probe	= thermometer_device_probe,
+	.device_remove	= thermometer_device_remove,
 	.adapter_probe	= thermometer_adapter_probe,
 	.adapter_remove	= thermometer_adapter_remove
 };
-- 
1.7.11.3

[PATCH 09/14] thermometer: Add constant definition for watcher interface name

[Andrzej Kaczmarek]

---
 profiles/thermometer/thermometer.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/profiles/thermometer/thermometer.c b/profiles/thermometer/thermometer.c
index f0c1d67..6f3314a 100644
--- a/profiles/thermometer/thermometer.c
+++ b/profiles/thermometer/thermometer.c
@@ -44,6 +44,7 @@
 
 #define THERMOMETER_INTERFACE		"org.bluez.Thermometer"
 #define THERMOMETER_MANAGER_INTERFACE	"org.bluez.ThermometerManager"
+#define THERMOMETER_WATCHER_INTERFACE	"org.bluez.ThermometerWatcher"
 
 /* Temperature measurement flag fields */
 #define TEMP_UNITS		0x01
@@ -1034,7 +1035,7 @@ static void update_watcher(gpointer data, gpointer user_data)
 	DBusMessage *msg;
 
 	msg = dbus_message_new_method_call(w->srv, w->path,
-				"org.bluez.ThermometerWatcher",
+				THERMOMETER_WATCHER_INTERFACE,
 				"MeasurementReceived");
 	if (msg == NULL)
 		return;
-- 
1.7.11.3

[PATCH 10/14] thermometer: Add common function to write characteristics CCC

[Andrzej Kaczmarek]

There are now few separate functions to write CCC for measurement
characteristics which looks similar. This patch adds common function
to write given value into CCC for given characteristics and leaves
dedicated functions to act only as simple wrappers.
---
 profiles/thermometer/thermometer.c | 108 ++++++++-----------------------------
 1 file changed, 23 insertions(+), 85 deletions(-)

diff --git a/profiles/thermometer/thermometer.c b/profiles/thermometer/thermometer.c
index 6f3314a..29d5e02 100644
--- a/profiles/thermometer/thermometer.c
+++ b/profiles/thermometer/thermometer.c
@@ -376,7 +376,7 @@ static void valid_range_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
 	change_property(desc->ch->t, "Minimum", &min);
 }
 
-static void measurement_cb(guint8 status, const guint8 *pdu,
+static void write_ccc_cb(guint8 status, const guint8 *pdu,
 						guint16 len, gpointer user_data)
 {
 	char *msg = user_data;
@@ -426,7 +426,7 @@ static void process_thermometer_desc(struct descriptor *desc)
 
 		att_put_u16(val, atval);
 		gatt_write_char(ch->t->attrib, desc->handle, atval, 2,
-							measurement_cb, msg);
+							write_ccc_cb, msg);
 		return;
 	}
 
@@ -720,9 +720,8 @@ static DBusMessage *set_property(DBusConnection *conn, DBusMessage *msg,
 	return write_attr_interval(t, msg, value);
 }
 
-static void enable_final_measurement(gpointer data, gpointer user_data)
+static void write_ccc(struct thermometer *t, const char *uuid, uint16_t value)
 {
-	struct thermometer *t = data;
 	struct characteristic *ch;
 	struct descriptor *desc;
 	bt_uuid_t btuuid;
@@ -732,116 +731,55 @@ static void enable_final_measurement(gpointer data, gpointer user_data)
 	if (t->attrib == NULL)
 		return;
 
-	ch = get_characteristic(t, TEMPERATURE_MEASUREMENT_UUID);
+	ch = get_characteristic(t, uuid);
 	if (ch == NULL) {
-		DBG("Temperature measurement characteristic not found");
+		DBG("Characteristic %s not found", uuid);
 		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");
+		DBG("CCC descriptor for %s not found", uuid);
 		return;
 	}
 
-	atval[0] = 0x02;
-	atval[1] = 0x00;
-	msg = g_strdup("Enable final measurement");
-	gatt_write_char(t->attrib, desc->handle, atval, 2, measurement_cb, msg);
+	att_put_u16(value, atval);
+
+	msg = g_strdup_printf("Write CCC: %04x for %s", value, uuid);
+
+	gatt_write_char(t->attrib, desc->handle, atval, sizeof(atval),
+							write_ccc_cb, msg);
 }
 
-static void enable_intermediate_measurement(gpointer data, gpointer user_data)
+static void enable_final_measurement(gpointer data, gpointer user_data)
 {
 	struct thermometer *t = data;
-	struct characteristic *ch;
-	struct descriptor *desc;
-	bt_uuid_t btuuid;
-	uint8_t atval[2];
-	char *msg;
-
-	if (t->attrib == NULL || !t->intermediate)
-		return;
 
-	ch = get_characteristic(t, INTERMEDIATE_TEMPERATURE_UUID);
-	if (ch == NULL) {
-		DBG("Intermediate measurement characteristic not found");
-		return;
-	}
+	write_ccc(t, TEMPERATURE_MEASUREMENT_UUID,
+					GATT_CLIENT_CHARAC_CFG_IND_BIT);
+}
 
-	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;
-	}
+static void enable_intermediate_measurement(gpointer data, gpointer user_data)
+{
+	struct thermometer *t = data;
 
-	atval[0] = 0x01;
-	atval[1] = 0x00;
-	msg = g_strdup("Enable intermediate measurement");
-	gatt_write_char(t->attrib, desc->handle, atval, 2, measurement_cb, msg);
+	write_ccc(t, INTERMEDIATE_TEMPERATURE_UUID,
+					GATT_CLIENT_CHARAC_CFG_NOTIF_BIT);
 }
 
 static void disable_final_measurement(gpointer data, gpointer user_data)
 {
 	struct thermometer *t = data;
-	struct characteristic *ch;
-	struct descriptor *desc;
-	bt_uuid_t btuuid;
-	uint8_t atval[2];
-	char *msg;
 
-	if (t->attrib == NULL)
-		return;
-
-	ch = get_characteristic(t, TEMPERATURE_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;
-	}
-
-	atval[0] = 0x00;
-	atval[1] = 0x00;
-	msg = g_strdup("Disable final measurement");
-	gatt_write_char(t->attrib, desc->handle, atval, 2, measurement_cb, msg);
+	write_ccc(t, TEMPERATURE_MEASUREMENT_UUID, 0x0000);
 }
 
 static void disable_intermediate_measurement(gpointer data, gpointer user_data)
 {
 	struct thermometer *t = data;
-	struct characteristic *ch;
-	struct descriptor *desc;
-	bt_uuid_t btuuid;
-	uint8_t atval[2];
-	char *msg;
-
-	if (t->attrib == NULL)
-		return;
-
-	ch = get_characteristic(t, INTERMEDIATE_TEMPERATURE_UUID);
-	if (ch == NULL) {
-		DBG("Intermediate 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;
-	}
 
-	atval[0] = 0x00;
-	atval[1] = 0x00;
-	msg = g_strdup("Disable intermediate measurement");
-	gatt_write_char(t->attrib, desc->handle, atval, 2, measurement_cb, msg);
+	write_ccc(t, INTERMEDIATE_TEMPERATURE_UUID, 0x0000);
 }
 
 static void remove_int_watcher(struct thermometer_adapter *tadapter,
-- 
1.7.11.3

[PATCH 11/14] thermometer: Refactor processing of measurement characteristic value

[Andrzej Kaczmarek]

Data buffer is read by simply moving buffer pointer instead of calculating
indexes of consecutive fields. This makes function flow easier to follow
as there's no need to care about presence of fields prior to current when
reading data.
---
 profiles/thermometer/thermometer.c | 66 +++++++++++++++++++++-----------------
 1 file changed, 37 insertions(+), 29 deletions(-)

diff --git a/profiles/thermometer/thermometer.c b/profiles/thermometer/thermometer.c
index 29d5e02..f42ab7d 100644
--- a/profiles/thermometer/thermometer.c
+++ b/profiles/thermometer/thermometer.c
@@ -1021,27 +1021,37 @@ static void proc_measurement(struct thermometer *t, const uint8_t *pdu,
 						uint16_t len, gboolean final)
 {
 	struct measurement m;
-	const char *type;
+	const char *type = NULL;
 	uint8_t flags;
 	uint32_t raw;
 
-	if (len < 4) {
+	/* skip opcode and handle */
+	pdu += 3;
+	len -= 3;
+
+	if (len < 1) {
 		DBG("Mandatory flags are not provided");
 		return;
 	}
 
-	flags = pdu[3];
+	flags = *pdu;
+
 	if (flags & TEMP_UNITS)
 		m.unit = "Fahrenheit";
 	else
 		m.unit = "Celsius";
 
-	if (len < 8) {
-		DBG("Temperature measurement value is not provided");
+	pdu++;
+	len--;
+
+	if (len < 4) {
+		DBG("Mandatory temperature measurement value is not provided");
 		return;
 	}
 
-	raw = att_get_u32(&pdu[4]);
+	memset(&m, 0, sizeof(m));
+
+	raw = att_get_u32(pdu);
 	m.mant = raw & 0x00FFFFFF;
 	m.exp = ((int32_t) raw) >> 24;
 
@@ -1050,48 +1060,46 @@ static void proc_measurement(struct thermometer *t, const uint8_t *pdu,
 		m.mant = m.mant - FLOAT_MAX_MANTISSA;
 	}
 
+	pdu += 4;
+	len -= 4;
+
 	if (flags & TEMP_TIME_STAMP) {
 		struct tm ts;
 		time_t time;
 
-		if (len < 15) {
-			DBG("Can't get time stamp value");
+		if (len < 7) {
+			DBG("Time stamp is not provided");
 			return;
 		}
 
-		ts.tm_year = att_get_u16(&pdu[8]) - 1900;
-		ts.tm_mon = pdu[10] - 1;
-		ts.tm_mday = pdu[11];
-		ts.tm_hour = pdu[12];
-		ts.tm_min = pdu[13];
-		ts.tm_sec = pdu[14];
+		ts.tm_year = att_get_u16(pdu) - 1900;
+		ts.tm_mon = *(pdu + 2) - 1;
+		ts.tm_mday = *(pdu + 3);
+		ts.tm_hour = *(pdu + 4);
+		ts.tm_min = *(pdu + 5);
+		ts.tm_sec = *(pdu + 6);
 		ts.tm_isdst = -1;
 
 		time = mktime(&ts);
 		m.time = (uint64_t) time;
 		m.suptime = TRUE;
-	} else
-		m.suptime = FALSE;
+
+		pdu += 7;
+		len -= 7;
+	}
 
 	if (flags & TEMP_TYPE) {
-		uint8_t index;
-
-		if (m.suptime && len >= 16)
-			index = 15;
-		else if (!m.suptime && len >= 9)
-			index = 9;
-		else {
-			DBG("Can't get temperature type");
+		if (len < 1) {
+			DBG("Temperature type is not provided");
 			return;
 		}
 
-		type = temptype2str(pdu[index]);
-	} else if (t->has_type)
+		type = temptype2str(*pdu);
+	} else if (t->has_type) {
 		type = temptype2str(t->type);
-	else
-		type = NULL;
+	}
 
-	m.type = type ? g_strdup(type) : NULL;
+	m.type = g_strdup(type);
 	m.value = final ? "Final" : "Intermediate";
 
 	recv_measurement(t, &m);
-- 
1.7.11.3

[PATCH 12/14] thermometer: Fix whitespace

[Andrzej Kaczmarek]

---
 profiles/thermometer/thermometer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/profiles/thermometer/thermometer.c b/profiles/thermometer/thermometer.c
index f42ab7d..7c9cd0e 100644
--- a/profiles/thermometer/thermometer.c
+++ b/profiles/thermometer/thermometer.c
@@ -634,7 +634,7 @@ static DBusMessage *get_properties(DBusConnection *conn, DBusMessage *msg,
 	return reply;
 }
 
-static void write_interval_cb (guint8 status, const guint8 *pdu, guint16 len,
+static void write_interval_cb(guint8 status, const guint8 *pdu, guint16 len,
 							gpointer user_data)
 {
 	struct tmp_interval_data *data = user_data;
-- 
1.7.11.3

[PATCH 13/14] thermometer: Fix indentation

[Andrzej Kaczmarek]

---
 profiles/thermometer/thermometer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/profiles/thermometer/thermometer.c b/profiles/thermometer/thermometer.c
index 7c9cd0e..8733843 100644
--- a/profiles/thermometer/thermometer.c
+++ b/profiles/thermometer/thermometer.c
@@ -83,7 +83,7 @@ struct thermometer {
 };
 
 struct characteristic {
-	struct gatt_char		attr;	/* Characteristic */
+	struct gatt_char	attr;	/* Characteristic */
 	GSList			*desc;	/* Descriptors */
 	struct thermometer	*t;	/* Thermometer where the char belongs */
 };
-- 
1.7.11.3

[PATCH 14/14] thermometer: Fix missing braces

[Andrzej Kaczmarek]

---
 profiles/thermometer/thermometer.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/profiles/thermometer/thermometer.c b/profiles/thermometer/thermometer.c
index 8733843..0e2bf0a 100644
--- a/profiles/thermometer/thermometer.c
+++ b/profiles/thermometer/thermometer.c
@@ -335,8 +335,9 @@ static void change_property(struct thermometer *t, const char *name,
 		emit_property_changed(device_get_path(t->dev),
 					THERMOMETER_INTERFACE, name,
 					DBUS_TYPE_UINT16, &t->min);
-	} else
+	} else {
 		DBG("%s is not a thermometer property", name);
+	}
 }
 
 static void valid_range_desc_cb(guint8 status, const guint8 *pdu, guint16 len,
@@ -421,8 +422,9 @@ static void process_thermometer_desc(struct descriptor *desc)
 			val = GATT_CLIENT_CHARAC_CFG_IND_BIT;
 			msg = g_strdup("Enable Measurement Interval "
 								"indication");
-		} else
+		} else {
 			goto done;
+		}
 
 		att_put_u16(val, atval);
 		gatt_write_char(ch->t->attrib, desc->handle, atval, 2,
@@ -548,12 +550,13 @@ static void process_thermometer_char(struct characteristic *ch)
 		gboolean intermediate = TRUE;
 		change_property(ch->t, "Intermediate", &intermediate);
 		return;
-	} else if (g_strcmp0(ch->attr.uuid, TEMPERATURE_TYPE_UUID) == 0)
+	} else if (g_strcmp0(ch->attr.uuid, TEMPERATURE_TYPE_UUID) == 0) {
 		gatt_read_char(ch->t->attrib, ch->attr.value_handle, 0,
 							read_temp_type_cb, ch);
-	else if (g_strcmp0(ch->attr.uuid, MEASUREMENT_INTERVAL_UUID) == 0)
+	} else if (g_strcmp0(ch->attr.uuid, MEASUREMENT_INTERVAL_UUID) == 0) {
 		gatt_read_char(ch->t->attrib, ch->attr.value_handle, 0,
 							read_interval_cb, ch);
+	}
 }
 
 static void configure_thermometer_cb(GSList *characteristics, guint8 status,
@@ -591,10 +594,11 @@ static void configure_thermometer_cb(GSList *characteristics, guint8 status,
 			if (start == c->handle)
 				continue;
 			end = c->handle - 1;
-		} else if (c->value_handle != t->svc_range->end)
+		} else if (c->value_handle != t->svc_range->end) {
 			end = t->svc_range->end;
-		else
+		} else {
 			continue;
+		}
 
 		gatt_find_info(t->attrib, start, end, discover_desc_cb, ch);
 	}
-- 
1.7.11.3

Re: [PATCH 00/14] Thermometer watchers API change + fixes

[Anderson Lizardo]

Hi Andrzej,

On Tue, Sep 25, 2012 at 10:52 AM, Andrzej Kaczmarek
<andrzej.kaczmarek@tieto.com> wrote:
> Hi,
>
> Here's series of patches to move watchers from per-device to per-adapter
> interface (basically the same as done for HRP, see other series of patches).
> This applies to both final and intermediate temperature watchers - once
> watcher is registered, all connected devices are setup to send measurement
> notifications and the opposite when last watcher is unregistered.
>
> This major change is done in patches 1-6.
>
> Patches 7-11 contain some code and documentation refactoring, i.e. to
> remove duplicated code.
>
> Patches 12-14 are trival coding style fixes.
>
> Unfortunately, I wasn't able to fully test this code due to lack of proper
> thermometer device and sample device from CC2540 devkit does not quite
> work as expected (e.g. I can see CCC is written properly when watcher is
> registered but remote does not send any indications for unknown reason).

Unfortunately, the thermometer demo app (at least on TI's BLE stack
v1.1 installation) has several bugs. I haven't tested yet the latest
release (1.2.1), which version are you using?

Do you have access to PTS? You could test with it as well.

Maybe we should consider implementing a simple Thermometer role in
BlueZ for testing purposes. It is a relatively simple profile.

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

Re: [PATCH 00/14] Thermometer watchers API change + fixes

[Andrzej Kaczmarek]

Hi Anderson,

On 09/25/2012 05:21 PM, Anderson Lizardo wrote:
> Hi Andrzej,
>
> On Tue, Sep 25, 2012 at 10:52 AM, Andrzej Kaczmarek
> <andrzej.kaczmarek@tieto.com> wrote:
>> Hi,
>>
>> Here's series of patches to move watchers from per-device to per-adapter
>> interface (basically the same as done for HRP, see other series of patches).
>> This applies to both final and intermediate temperature watchers - once
>> watcher is registered, all connected devices are setup to send measurement
>> notifications and the opposite when last watcher is unregistered.
>>
>> This major change is done in patches 1-6.
>>
>> Patches 7-11 contain some code and documentation refactoring, i.e. to
>> remove duplicated code.
>>
>> Patches 12-14 are trival coding style fixes.
>>
>> Unfortunately, I wasn't able to fully test this code due to lack of proper
>> thermometer device and sample device from CC2540 devkit does not quite
>> work as expected (e.g. I can see CCC is written properly when watcher is
>> registered but remote does not send any indications for unknown reason).
>
> Unfortunately, the thermometer demo app (at least on TI's BLE stack
> v1.1 installation) has several bugs. I haven't tested yet the latest
> release (1.2.1), which version are you using?

1.2 but downloaded 1.2.1 and thermometer demo app didn't change so it 
still does not work.

> Do you have access to PTS? You could test with it as well.

I currently don't have BLE dongle for PTS but will test as soon as I 
have one.

> Maybe we should consider implementing a simple Thermometer role in
> BlueZ for testing purposes. It is a relatively simple profile.

So you mean something like optional org.bluez.ThermometerSensor 
interface on adapter, similar to what is done for reporter role for 
Proximity?

BR,
Andrzej

Re: [PATCH 00/14] Thermometer watchers API change + fixes

[Anderson Lizardo]

Hi Andrzej,

On Thu, Sep 27, 2012 at 6:47 AM, Andrzej Kaczmarek
<andrzej.kaczmarek@tieto.com> wrote:
>> Maybe we should consider implementing a simple Thermometer role in
>> BlueZ for testing purposes. It is a relatively simple profile.
>
>
> So you mean something like optional org.bluez.ThermometerSensor interface on
> adapter, similar to what is done for reporter role for Proximity?

Yes. There has been some interest on the past on the mailing list from
people willing to implement this role on BlueZ (although I have not
heard anymore from them), so it would become not just a test plugin,
but have an API that allows for BlueZ to expose some external
thermometer measurements over LE. But for now a "dummy" thermometer
which sends dummy measurements in various formats should be enough
(probably enabled only with --enable-test so it is not installed in
production).

Do you have interest/time for working on something like this?

BTW, I definitely plan to test both your HRP implementation and the
HTP changes, but currently I'm working in other things... I'll let you
know once I have these tested (probably with patches if I find any
issues).

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

Re: [PATCH 00/14] Thermometer watchers API change + fixes

[Andrzej Kaczmarek]

Hi Anderson,

On 09/27/2012 02:03 PM, Anderson Lizardo wrote:
> Hi Andrzej,
>
> On Thu, Sep 27, 2012 at 6:47 AM, Andrzej Kaczmarek
> <andrzej.kaczmarek@tieto.com> wrote:
>>> Maybe we should consider implementing a simple Thermometer role in
>>> BlueZ for testing purposes. It is a relatively simple profile.
>>
>>
>> So you mean something like optional org.bluez.ThermometerSensor interface on
>> adapter, similar to what is done for reporter role for Proximity?
>
> Yes. There has been some interest on the past on the mailing list from
> people willing to implement this role on BlueZ (although I have not
> heard anymore from them), so it would become not just a test plugin,
> but have an API that allows for BlueZ to expose some external
> thermometer measurements over LE. But for now a "dummy" thermometer
> which sends dummy measurements in various formats should be enough
> (probably enabled only with --enable-test so it is not installed in
> production).
>
> Do you have interest/time for working on something like this?

I could take a look on this in spare time, sure.

By "dummy" thermometer you mean separate plugin (dummythermometer or 
sth) which, once enabled, will register service in GATT and send dummy 
measurements when configured by remote? Or perhaps develop it inside 
thermometer profile (implemented as described above) so it can be 
changed into full blown thermometer role with API etc. later?

> BTW, I definitely plan to test both your HRP implementation and the
> HTP changes, but currently I'm working in other things... I'll let you
> know once I have these tested (probably with patches if I find any
> issues).

I should have BLE PTS dongle this week so will test both implementations 
as well in case you won't manage to do this until then.

BR,
Andrzej

Re: [PATCH 00/14] Thermometer watchers API change + fixes

[Anderson Lizardo]

Hi Andrzej,

On Mon, Oct 1, 2012 at 7:44 AM, Andrzej Kaczmarek
<andrzej.kaczmarek@tieto.com> wrote:
> By "dummy" thermometer you mean separate plugin (dummythermometer or sth)
> which, once enabled, will register service in GATT and send dummy
> measurements when configured by remote? Or perhaps develop it inside
> thermometer profile (implemented as described above) so it can be changed
> into full blown thermometer role with API etc. later?

I was thinking having it under profiles/thermometer (just like
proximity reporter is currently in profiles/proximity). The "dummy"
code could then be replaced later with the API. As long as nothing is
enabled until the CCC is written, I don't see any problem having it
running along HTP collector.

>> BTW, I definitely plan to test both your HRP implementation and the
>> HTP changes, but currently I'm working in other things... I'll let you
>> know once I have these tested (probably with patches if I find any
>> issues).
>
>
> I should have BLE PTS dongle this week so will test both implementations as
> well in case you won't manage to do this until then.

Great!

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