[RFC BlueZ 0/5] gattrib: Registering per-handle event listeners

[RFC BlueZ 0/5] gattrib: Registering per-handle event listeners

[Vinicius Costa Gomes]

Hi,

This RFC is mostly to gather some comments about the idea, and see if
anyone can see any problem with it.

As we are starting to have devices that support multiple profiles,
something like this is inevitable, if we want to avoid having every
profile getting notified for each notification/indication received.

Cheers,
--

Vinicius Costa Gomes (5):
  gattrib: Add support for listening for events for specific handles
  scan: Use the per handle GATT event notifier
  hog: Use the per handle GATT event notifier
  heartrate: Use the per handle GATT event notifier
  gas: Add the per handle GATT event notifier

 attrib/client.c                    |   4 +-
 attrib/gattrib.c                   |  33 ++++-
 attrib/gattrib.h                   |   7 +-
 attrib/gatttool.c                  |   8 +-
 attrib/interactive.c               |   8 +-
 profiles/gatt/gas.c                |  71 +++++----
 profiles/heartrate/heartrate.c     | 287 +++++++++++++++++++------------------
 profiles/input/hog_device.c        |  65 +++++----
 profiles/scanparam/scan.c          |  15 +-
 profiles/thermometer/thermometer.c |   2 +
 src/attrib-server.c                |   2 +-
 11 files changed, 262 insertions(+), 240 deletions(-)

-- 
1.7.12.2

[RFC BlueZ 1/5] gattrib: Add support for listening for events for specific handles

[Vinicius Costa Gomes]

We want only the profile that implements a service to be notified of
changes on that service. Before this patch, all the registered event
notifiers are being called.
---
 attrib/client.c                    |  4 ++--
 attrib/gattrib.c                   | 33 ++++++++++++++++++++++++++++-----
 attrib/gattrib.h                   |  7 ++++---
 attrib/gatttool.c                  |  8 ++++----
 attrib/interactive.c               |  8 ++++----
 profiles/gatt/gas.c                |  1 +
 profiles/heartrate/heartrate.c     |  2 +-
 profiles/input/hog_device.c        |  5 +++--
 profiles/scanparam/scan.c          |  4 ++--
 profiles/thermometer/thermometer.c |  2 ++
 src/attrib-server.c                |  2 +-
 11 files changed, 52 insertions(+), 24 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index 8b29cbb..cda5bc0 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -393,9 +393,9 @@ static void attio_connected(GAttrib *attrib, gpointer user_data)
 	gatt->attrib = g_attrib_ref(attrib);
 
 	g_attrib_register(gatt->attrib, ATT_OP_HANDLE_NOTIFY,
-					events_handler, gatt, NULL);
+			GATTRIB_ALL_HANDLES, events_handler, gatt, NULL);
 	g_attrib_register(gatt->attrib, ATT_OP_HANDLE_IND,
-					events_handler, gatt, NULL);
+			GATTRIB_ALL_HANDLES, events_handler, gatt, NULL);
 
 	g_slist_foreach(gatt->offline_chars, offline_char_write, attrib);
 
diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index 6f6942f..c928798 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -70,6 +70,7 @@ struct command {
 struct event {
 	guint id;
 	guint8 expected;
+	guint16 handle;
 	GAttribNotifyFunc func;
 	gpointer user_data;
 	GDestroyNotify notify;
@@ -351,6 +352,30 @@ static void wake_up_sender(struct _GAttrib *attrib)
 				can_write_data, attrib, destroy_sender);
 }
 
+static gboolean match_event(struct event *evt, const uint8_t *pdu, gsize len)
+{
+	guint16 handle;
+
+	if (evt->expected == GATTRIB_ALL_EVENTS)
+		return TRUE;
+
+	if (is_response(pdu[0]) == FALSE && evt->expected == GATTRIB_ALL_REQS)
+		return TRUE;
+
+	if (evt->expected == pdu[0] && evt->handle == GATTRIB_ALL_HANDLES)
+		return TRUE;
+
+	if (len < 3)
+		return FALSE;
+
+	handle = att_get_u16(&pdu[1]);
+
+	if (evt->expected == pdu[0] && evt->handle == handle)
+		return TRUE;
+
+	return FALSE;
+}
+
 static gboolean received_data(GIOChannel *io, GIOCondition cond, gpointer data)
 {
 	struct _GAttrib *attrib = data;
@@ -381,10 +406,7 @@ static gboolean received_data(GIOChannel *io, GIOCondition cond, gpointer data)
 	for (l = attrib->events; l; l = l->next) {
 		struct event *evt = l->data;
 
-		if (evt->expected == buf[0] ||
-				evt->expected == GATTRIB_ALL_EVENTS ||
-				(is_response(buf[0]) == FALSE &&
-						evt->expected == GATTRIB_ALL_REQS))
+		if (match_event(evt, buf, len))
 			evt->func(buf, len, evt->user_data);
 	}
 
@@ -639,7 +661,7 @@ gboolean g_attrib_set_mtu(GAttrib *attrib, int mtu)
 	return TRUE;
 }
 
-guint g_attrib_register(GAttrib *attrib, guint8 opcode,
+guint g_attrib_register(GAttrib *attrib, guint8 opcode, guint16 handle,
 				GAttribNotifyFunc func, gpointer user_data,
 				GDestroyNotify notify)
 {
@@ -651,6 +673,7 @@ guint g_attrib_register(GAttrib *attrib, guint8 opcode,
 		return 0;
 
 	event->expected = opcode;
+	event->handle = handle;
 	event->func = func;
 	event->user_data = user_data;
 	event->notify = notify;
diff --git a/attrib/gattrib.h b/attrib/gattrib.h
index bca966f..3fe92c7 100644
--- a/attrib/gattrib.h
+++ b/attrib/gattrib.h
@@ -30,6 +30,7 @@ extern "C" {
 
 #define GATTRIB_ALL_EVENTS 0xFF
 #define GATTRIB_ALL_REQS 0xFE
+#define GATTRIB_ALL_HANDLES 0x0000
 
 struct _GAttrib;
 typedef struct _GAttrib GAttrib;
@@ -60,9 +61,9 @@ gboolean g_attrib_cancel_all(GAttrib *attrib);
 gboolean g_attrib_set_debug(GAttrib *attrib,
 		GAttribDebugFunc func, gpointer user_data);
 
-guint g_attrib_register(GAttrib *attrib, guint8 opcode,
-		GAttribNotifyFunc func, gpointer user_data,
-					GDestroyNotify notify);
+guint g_attrib_register(GAttrib *attrib, guint8 opcode, guint16 handle,
+				GAttribNotifyFunc func, gpointer user_data,
+				GDestroyNotify notify);
 
 gboolean g_attrib_is_encrypted(GAttrib *attrib);
 
diff --git a/attrib/gatttool.c b/attrib/gatttool.c
index 16cce0c..5517408 100644
--- a/attrib/gatttool.c
+++ b/attrib/gatttool.c
@@ -111,10 +111,10 @@ static gboolean listen_start(gpointer user_data)
 {
 	GAttrib *attrib = user_data;
 
-	g_attrib_register(attrib, ATT_OP_HANDLE_NOTIFY, events_handler,
-							attrib, NULL);
-	g_attrib_register(attrib, ATT_OP_HANDLE_IND, events_handler,
-							attrib, NULL);
+	g_attrib_register(attrib, ATT_OP_HANDLE_NOTIFY, GATTRIB_ALL_HANDLES,
+						events_handler, attrib, NULL);
+	g_attrib_register(attrib, ATT_OP_HANDLE_IND, GATTRIB_ALL_HANDLES,
+						events_handler, attrib, NULL);
 
 	return FALSE;
 }
diff --git a/attrib/interactive.c b/attrib/interactive.c
index b41a7bb..df0bb86 100644
--- a/attrib/interactive.c
+++ b/attrib/interactive.c
@@ -147,10 +147,10 @@ static void connect_cb(GIOChannel *io, GError *err, gpointer user_data)
 	}
 
 	attrib = g_attrib_new(iochannel);
-	g_attrib_register(attrib, ATT_OP_HANDLE_NOTIFY, events_handler,
-							attrib, NULL);
-	g_attrib_register(attrib, ATT_OP_HANDLE_IND, events_handler,
-							attrib, NULL);
+	g_attrib_register(attrib, ATT_OP_HANDLE_NOTIFY, GATTRIB_ALL_HANDLES,
+						events_handler, attrib, NULL);
+	g_attrib_register(attrib, ATT_OP_HANDLE_IND, GATTRIB_ALL_HANDLES,
+						events_handler, attrib, NULL);
 	set_state(STATE_CONNECTED);
 }
 
diff --git a/profiles/gatt/gas.c b/profiles/gatt/gas.c
index 74ca9ce..35a9152 100644
--- a/profiles/gatt/gas.c
+++ b/profiles/gatt/gas.c
@@ -333,6 +333,7 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
 	}
 
 	gas->changed_ind = g_attrib_register(gas->attrib, ATT_OP_HANDLE_IND,
+						GATTRIB_ALL_HANDLES,
 						indication_cb, gas, NULL);
 
 	if (device_get_appearance(gas->device, &app) < 0) {
diff --git a/profiles/heartrate/heartrate.c b/profiles/heartrate/heartrate.c
index 94d4b8d..d9d6c03 100644
--- a/profiles/heartrate/heartrate.c
+++ b/profiles/heartrate/heartrate.c
@@ -544,7 +544,7 @@ 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);
+				GATTRIB_ALL_HANDLES, notify_handler, hr, NULL);
 
 	gatt_discover_char(hr->attrib, hr->svc_range->start, hr->svc_range->end,
 						NULL, discover_char_cb, hr);
diff --git a/profiles/input/hog_device.c b/profiles/input/hog_device.c
index a8cc568..52ebd95 100644
--- a/profiles/input/hog_device.c
+++ b/profiles/input/hog_device.c
@@ -612,8 +612,9 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
 	hogdev->attrib = g_attrib_ref(attrib);
 
 	hogdev->report_cb_id = g_attrib_register(hogdev->attrib,
-					ATT_OP_HANDLE_NOTIFY, report_value_cb,
-					hogdev, NULL);
+					ATT_OP_HANDLE_NOTIFY,
+					GATTRIB_ALL_HANDLES,
+					report_value_cb, hogdev, NULL);
 
 	if (hogdev->reports == NULL) {
 		gatt_discover_char(hogdev->attrib, prim->range.start,
diff --git a/profiles/scanparam/scan.c b/profiles/scanparam/scan.c
index e523df5..bbf646c 100644
--- a/profiles/scanparam/scan.c
+++ b/profiles/scanparam/scan.c
@@ -115,8 +115,8 @@ static void ccc_written_cb(guint8 status, const guint8 *pdu,
 	DBG("Scan Refresh: notification enabled");
 
 	scan->refresh_cb_id = g_attrib_register(scan->attrib,
-					ATT_OP_HANDLE_NOTIFY, refresh_value_cb,
-					user_data, NULL);
+				ATT_OP_HANDLE_NOTIFY, GATTRIB_ALL_HANDLES,
+				refresh_value_cb, user_data, NULL);
 }
 
 static void discover_descriptor_cb(guint8 status, const guint8 *pdu,
diff --git a/profiles/thermometer/thermometer.c b/profiles/thermometer/thermometer.c
index 98cfb34..de5a5bb 100644
--- a/profiles/thermometer/thermometer.c
+++ b/profiles/thermometer/thermometer.c
@@ -1201,8 +1201,10 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
 	t->attrib = g_attrib_ref(attrib);
 
 	t->attindid = g_attrib_register(t->attrib, ATT_OP_HANDLE_IND,
+							GATTRIB_ALL_HANDLES,
 							ind_handler, t, NULL);
 	t->attnotid = g_attrib_register(t->attrib, ATT_OP_HANDLE_NOTIFY,
+							GATTRIB_ALL_HANDLES,
 							notif_handler, t, NULL);
 	gatt_discover_char(t->attrib, t->svc_range->start, t->svc_range->end,
 					NULL, configure_thermometer_cb, t);
diff --git a/src/attrib-server.c b/src/attrib-server.c
index 76a32af..d174301 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -1086,7 +1086,7 @@ guint attrib_channel_attach(GAttrib *attrib)
 
 	channel->attrib = g_attrib_ref(attrib);
 	channel->id = g_attrib_register(channel->attrib, GATTRIB_ALL_REQS,
-					channel_handler, channel, NULL);
+			GATTRIB_ALL_HANDLES, channel_handler, channel, NULL);
 
 	channel->cleanup_id = g_io_add_watch(io, G_IO_HUP, channel_watch_cb,
 								channel);
-- 
1.7.12.2

[RFC BlueZ 2/5] scan: Use the per handle GATT event notifier

[Vinicius Costa Gomes]

---
 profiles/scanparam/scan.c | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/profiles/scanparam/scan.c b/profiles/scanparam/scan.c
index bbf646c..09fbe1f 100644
--- a/profiles/scanparam/scan.c
+++ b/profiles/scanparam/scan.c
@@ -83,17 +83,6 @@ static void refresh_value_cb(const uint8_t *pdu, uint16_t len,
 						gpointer user_data)
 {
 	struct scan *scan = user_data;
-	uint16_t handle;
-
-	if (len < 4) { /* 1-byte opcode + 2-byte handle + refresh */
-		error("Malformed ATT notification");
-		return;
-	}
-
-	handle = att_get_u16(&pdu[1]);
-
-	if (handle != scan->refresh_handle)
-		return;
 
 	DBG("Server requires refresh: %d", pdu[3]);
 
@@ -115,8 +104,8 @@ static void ccc_written_cb(guint8 status, const guint8 *pdu,
 	DBG("Scan Refresh: notification enabled");
 
 	scan->refresh_cb_id = g_attrib_register(scan->attrib,
-				ATT_OP_HANDLE_NOTIFY, GATTRIB_ALL_HANDLES,
-				refresh_value_cb, user_data, NULL);
+				ATT_OP_HANDLE_NOTIFY, scan->refresh_handle,
+				refresh_value_cb, scan, NULL);
 }
 
 static void discover_descriptor_cb(guint8 status, const guint8 *pdu,
-- 
1.7.12.2

[RFC BlueZ 3/5] hog: Use the per handle GATT event notifier

[Vinicius Costa Gomes]

---
 profiles/input/hog_device.c | 66 ++++++++++++++++++++++++---------------------
 1 file changed, 36 insertions(+), 30 deletions(-)

diff --git a/profiles/input/hog_device.c b/profiles/input/hog_device.c
index 52ebd95..34f4a41 100644
--- a/profiles/input/hog_device.c
+++ b/profiles/input/hog_device.c
@@ -92,27 +92,18 @@ struct hog_device {
 struct report {
 	uint8_t			id;
 	uint8_t			type;
+	guint			notifyid;
 	struct gatt_char	*decl;
 	struct hog_device	*hogdev;
 };
 
-static gint report_handle_cmp(gconstpointer a, gconstpointer b)
-{
-	const struct report *report = a;
-	uint16_t handle = GPOINTER_TO_UINT(b);
-
-	return report->decl->value_handle - handle;
-}
-
 static void report_value_cb(const uint8_t *pdu, uint16_t len,
 							gpointer user_data)
 {
-	struct hog_device *hogdev = user_data;
+	struct report *report = user_data;
+	struct hog_device *hogdev = report->hogdev;
 	struct uhid_event ev;
 	uint16_t report_size = len - 3;
-	guint handle;
-	GSList *l;
-	struct report *report;
 	uint8_t *buf;
 
 	if (len < 3) { /* 1-byte opcode + 2-byte handle */
@@ -120,17 +111,6 @@ static void report_value_cb(const uint8_t *pdu, uint16_t len,
 		return;
 	}
 
-	handle = att_get_u16(&pdu[1]);
-
-	l = g_slist_find_custom(hogdev->reports, GUINT_TO_POINTER(handle),
-							report_handle_cmp);
-	if (!l) {
-		error("Invalid report");
-		return;
-	}
-
-	report = l->data;
-
 	memset(&ev, 0, sizeof(ev));
 	ev.type = UHID_INPUT;
 	ev.u.input.size = MIN(report_size, UHID_DATA_MAX);
@@ -154,22 +134,31 @@ static void report_value_cb(const uint8_t *pdu, uint16_t len,
 static void report_ccc_written_cb(guint8 status, const guint8 *pdu,
 					guint16 plen, gpointer user_data)
 {
+	struct report *report = user_data;
+	struct hog_device *hogdev = report->hogdev;
+
 	if (status != 0) {
 		error("Write report characteristic descriptor failed: %s",
 							att_ecode2str(status));
 		return;
 	}
 
+	report->notifyid = g_attrib_register(hogdev->attrib,
+					ATT_OP_HANDLE_NOTIFY,
+					report->decl->value_handle,
+					report_value_cb, report, NULL);
+
 	DBG("Report characteristic descriptor written: notifications enabled");
 }
 
 static void write_ccc(uint16_t handle, gpointer user_data)
 {
-	struct hog_device *hogdev = user_data;
+	struct report *report = user_data;
+	struct hog_device *hogdev = report->hogdev;
 	uint8_t value[] = { 0x01, 0x00 };
 
 	gatt_write_char(hogdev->attrib, handle, value, sizeof(value),
-					report_ccc_written_cb, hogdev);
+					report_ccc_written_cb, report);
 }
 
 static void report_reference_cb(guint8 status, const guint8 *pdu,
@@ -196,6 +185,7 @@ static void report_reference_cb(guint8 status, const guint8 *pdu,
 static void external_report_reference_cb(guint8 status, const guint8 *pdu,
 					guint16 plen, gpointer user_data);
 
+
 static void discover_descriptor_cb(guint8 status, const guint8 *pdu,
 					guint16 len, gpointer user_data)
 {
@@ -608,24 +598,37 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
 {
 	struct hog_device *hogdev = user_data;
 	struct gatt_primary *prim = hogdev->hog_primary;
+	GSList *l;
 
 	hogdev->attrib = g_attrib_ref(attrib);
 
-	hogdev->report_cb_id = g_attrib_register(hogdev->attrib,
-					ATT_OP_HANDLE_NOTIFY,
-					GATTRIB_ALL_HANDLES,
-					report_value_cb, hogdev, NULL);
-
 	if (hogdev->reports == NULL) {
 		gatt_discover_char(hogdev->attrib, prim->range.start,
 						prim->range.end, NULL,
 						char_discovered_cb, hogdev);
+		return;
+	}
+
+	for (l = hogdev->reports; l; l = l->next) {
+		struct report *r = l->data;
+
+		r->notifyid = g_attrib_register(hogdev->attrib,
+					ATT_OP_HANDLE_NOTIFY,
+					r->decl->value_handle,
+					report_value_cb, r, NULL);
 	}
 }
 
 static void attio_disconnected_cb(gpointer user_data)
 {
 	struct hog_device *hogdev = user_data;
+	GSList *l;
+
+	for (l = hogdev->reports; l; l = l->next) {
+		struct report *r = l->data;
+
+		g_attrib_unregister(hogdev->attrib, r->notifyid);
+	}
 
 	g_attrib_unregister(hogdev->attrib, hogdev->report_cb_id);
 	hogdev->report_cb_id = 0;
@@ -652,6 +655,9 @@ static struct hog_device *hog_device_new(struct btd_device *device,
 static void report_free(void *data)
 {
 	struct report *report = data;
+	struct hog_device *hogdev = report->hogdev;
+
+	g_attrib_unregister(hogdev->attrib, report->notifyid);
 	g_free(report->decl);
 	g_free(report);
 }
-- 
1.7.12.2

[RFC BlueZ 4/5] heartrate: Use the per handle GATT event notifier

[Vinicius Costa Gomes]

---
 profiles/heartrate/heartrate.c | 287 +++++++++++++++++++++--------------------
 1 file changed, 145 insertions(+), 142 deletions(-)

diff --git a/profiles/heartrate/heartrate.c b/profiles/heartrate/heartrate.c
index d9d6c03..871b74e 100644
--- a/profiles/heartrate/heartrate.c
+++ b/profiles/heartrate/heartrate.c
@@ -260,6 +260,150 @@ static void char_write_cb(guint8 status, const guint8 *pdu, guint16 len,
 	g_free(msg);
 }
 
+static void update_watcher(gpointer data, gpointer user_data)
+{
+	struct watcher *w = data;
+	struct measurement *m = user_data;
+	struct heartrate *hr = m->hr;
+	const gchar *path = device_get_path(hr->dev);
+	DBusMessageIter iter;
+	DBusMessageIter dict;
+	DBusMessage *msg;
+
+	msg = dbus_message_new_method_call(w->srv, w->path,
+			HEART_RATE_WATCHER_INTERFACE, "MeasurementReceived");
+	if (msg == NULL)
+		return;
+
+	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
+			DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
+
+	dict_append_entry(&dict, "Value", DBUS_TYPE_UINT16, &m->value);
+
+	if (m->has_energy)
+		dict_append_entry(&dict, "Energy", DBUS_TYPE_UINT16,
+								&m->energy);
+
+	if (m->has_contact)
+		dict_append_entry(&dict, "Contact", DBUS_TYPE_BOOLEAN,
+								&m->contact);
+
+	if (m->num_interval > 0)
+		dict_append_array(&dict, "Interval", DBUS_TYPE_UINT16,
+						&m->interval, m->num_interval);
+
+	dbus_message_iter_close_container(&iter, &dict);
+
+	dbus_message_set_no_reply(msg, TRUE);
+	g_dbus_send_message(btd_get_dbus_connection(), msg);
+}
+
+static void process_measurement(struct heartrate *hr, const uint8_t *pdu,
+								uint16_t len)
+{
+	struct measurement m;
+	uint8_t flags;
+
+	flags = *pdu;
+
+	pdu++;
+	len--;
+
+	memset(&m, 0, sizeof(m));
+
+	if (flags & HR_VALUE_FORMAT) {
+		if (len < 2) {
+			error("Heart Rate Measurement field missing");
+			return;
+		}
+
+		m.value = att_get_u16(pdu);
+		pdu += 2;
+		len -= 2;
+	} else {
+		if (len < 1) {
+			error("Heart Rate Measurement field missing");
+			return;
+		}
+
+		m.value = *pdu;
+		pdu++;
+		len--;
+	}
+
+	if (flags & ENERGY_EXP_STATUS) {
+		if (len < 2) {
+			error("Energy Expended field missing");
+			return;
+		}
+
+		m.has_energy = TRUE;
+		m.energy = att_get_u16(pdu);
+		pdu += 2;
+		len -= 2;
+	}
+
+	if (flags & RR_INTERVAL) {
+		int i;
+
+		if (len == 0 || (len % 2 != 0)) {
+			error("RR-Interval field malformed");
+			return;
+		}
+
+		m.num_interval = len / 2;
+		m.interval = g_new(uint16_t, m.num_interval);
+
+		for (i = 0; i < m.num_interval; pdu += 2, i++)
+			m.interval[i] = att_get_u16(pdu);
+	}
+
+	if (flags & SENSOR_CONTACT_SUPPORT) {
+		m.has_contact = TRUE;
+		m.contact = !!(flags & SENSOR_CONTACT_DETECTED);
+	}
+
+	/* Notify all registered watchers */
+	m.hr = hr;
+	g_slist_foreach(hr->hradapter->watchers, update_watcher, &m);
+
+	g_free(m.interval);
+}
+
+static void notify_handler(const uint8_t *pdu, uint16_t len, gpointer user_data)
+{
+	struct heartrate *hr = user_data;
+
+	/* should be at least opcode (1b) + handle (2b) */
+	if (len < 3) {
+		error("Invalid PDU received");
+		return;
+	}
+
+	process_measurement(hr, pdu + 3, len - 3);
+}
+
+static void ccc_write_cb(guint8 status, const guint8 *pdu, guint16 len,
+							gpointer user_data)
+{
+	struct heartrate *hr = user_data;
+
+	if (status != 0) {
+		error("Enable measurement failed");
+		return;
+	}
+
+	hr->attionotid = g_attrib_register(hr->attrib, ATT_OP_HANDLE_NOTIFY,
+						hr->measurement_val_handle,
+						notify_handler, hr, NULL);
+}
+
 static void discover_ccc_cb(guint8 status, const guint8 *pdu,
 						guint16 len, gpointer user_data)
 {
@@ -291,7 +435,6 @@ static void discover_ccc_cb(guint8 status, const guint8 *pdu,
 
 		if (uuid == GATT_CLIENT_CHARAC_CFG_UUID) {
 			uint8_t value[2];
-			char *msg;
 
 			hr->measurement_ccc_handle = handle;
 
@@ -299,10 +442,9 @@ static void discover_ccc_cb(guint8 status, const guint8 *pdu,
 				break;
 
 			att_put_u16(GATT_CLIENT_CHARAC_CFG_NOTIF_BIT, value);
-			msg = g_strdup("Enable measurement");
 
 			gatt_write_char(hr->attrib, handle, value,
-					sizeof(value), char_write_cb, msg);
+					sizeof(value), ccc_write_cb, hr);
 
 			break;
 		}
@@ -399,142 +541,6 @@ static void disable_measurement(gpointer data, gpointer user_data)
 							char_write_cb, msg);
 }
 
-static void update_watcher(gpointer data, gpointer user_data)
-{
-	struct watcher *w = data;
-	struct measurement *m = user_data;
-	struct heartrate *hr = m->hr;
-	const gchar *path = device_get_path(hr->dev);
-	DBusMessageIter iter;
-	DBusMessageIter dict;
-	DBusMessage *msg;
-
-	msg = dbus_message_new_method_call(w->srv, w->path,
-			HEART_RATE_WATCHER_INTERFACE, "MeasurementReceived");
-	if (msg == NULL)
-		return;
-
-	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
-			DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
-
-	dict_append_entry(&dict, "Value", DBUS_TYPE_UINT16, &m->value);
-
-	if (m->has_energy)
-		dict_append_entry(&dict, "Energy", DBUS_TYPE_UINT16,
-								&m->energy);
-
-	if (m->has_contact)
-		dict_append_entry(&dict, "Contact", DBUS_TYPE_BOOLEAN,
-								&m->contact);
-
-	if (m->num_interval > 0)
-		dict_append_array(&dict, "Interval", DBUS_TYPE_UINT16,
-						&m->interval, m->num_interval);
-
-	dbus_message_iter_close_container(&iter, &dict);
-
-	dbus_message_set_no_reply(msg, TRUE);
-	g_dbus_send_message(btd_get_dbus_connection(), msg);
-}
-
-static void process_measurement(struct heartrate *hr, const uint8_t *pdu,
-								uint16_t len)
-{
-	struct measurement m;
-	uint8_t flags;
-
-	flags = *pdu;
-
-	pdu++;
-	len--;
-
-	memset(&m, 0, sizeof(m));
-
-	if (flags & HR_VALUE_FORMAT) {
-		if (len < 2) {
-			error("Heart Rate Measurement field missing");
-			return;
-		}
-
-		m.value = att_get_u16(pdu);
-		pdu += 2;
-		len -= 2;
-	} else {
-		if (len < 1) {
-			error("Heart Rate Measurement field missing");
-			return;
-		}
-
-		m.value = *pdu;
-		pdu++;
-		len--;
-	}
-
-	if (flags & ENERGY_EXP_STATUS) {
-		if (len < 2) {
-			error("Energy Expended field missing");
-			return;
-		}
-
-		m.has_energy = TRUE;
-		m.energy = att_get_u16(pdu);
-		pdu += 2;
-		len -= 2;
-	}
-
-	if (flags & RR_INTERVAL) {
-		int i;
-
-		if (len == 0 || (len % 2 != 0)) {
-			error("RR-Interval field malformed");
-			return;
-		}
-
-		m.num_interval = len / 2;
-		m.interval = g_new(uint16_t, m.num_interval);
-
-		for (i = 0; i < m.num_interval; pdu += 2, i++)
-			m.interval[i] = att_get_u16(pdu);
-	}
-
-	if (flags & SENSOR_CONTACT_SUPPORT) {
-		m.has_contact = TRUE;
-		m.contact = !!(flags & SENSOR_CONTACT_DETECTED);
-	}
-
-	/* Notify all registered watchers */
-	m.hr = hr;
-	g_slist_foreach(hr->hradapter->watchers, update_watcher, &m);
-
-	g_free(m.interval);
-}
-
-static void notify_handler(const uint8_t *pdu, uint16_t len, gpointer user_data)
-{
-	struct heartrate *hr = user_data;
-	uint16_t handle;
-
-	/* should be at least opcode (1b) + handle (2b) */
-	if (len < 3) {
-		error("Invalid PDU received");
-		return;
-	}
-
-	handle = att_get_u16(pdu + 1);
-	if (handle != hr->measurement_val_handle) {
-		error("Unexpected handle: 0x%04x", handle);
-		return;
-	}
-
-	process_measurement(hr, pdu + 3, len - 3);
-}
-
 static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
 {
 	struct heartrate *hr = user_data;
@@ -543,9 +549,6 @@ 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,
-				GATTRIB_ALL_HANDLES, notify_handler, hr, NULL);
-
 	gatt_discover_char(hr->attrib, hr->svc_range->start, hr->svc_range->end,
 						NULL, discover_char_cb, hr);
 }
-- 
1.7.12.2

[RFC BlueZ 5/5] gas: Add the per handle GATT event notifier

[Vinicius Costa Gomes]

---
 profiles/gatt/gas.c | 72 +++++++++++++++++++++++++----------------------------
 1 file changed, 34 insertions(+), 38 deletions(-)

diff --git a/profiles/gatt/gas.c b/profiles/gatt/gas.c
index 35a9152..d913df5 100644
--- a/profiles/gatt/gas.c
+++ b/profiles/gatt/gas.c
@@ -166,39 +166,10 @@ done:
 	att_data_list_free(list);
 }
 
-static void ccc_written_cb(guint8 status, const guint8 *pdu, guint16 plen,
-							gpointer user_data)
-{
-	struct gas *gas = user_data;
-
-	if (status) {
-		error("Write Service Changed CCC failed: %s",
-						att_ecode2str(status));
-		return;
-	}
-
-	DBG("Service Changed indications enabled");
-
-	write_ctp_handle(adapter_get_address(device_get_adapter(gas->device)),
-					device_get_address(gas->device),
-					device_get_addr_type(gas->device),
-					GATT_CHARAC_SERVICE_CHANGED,
-					gas->changed_handle);
-}
-
-static void write_ccc(GAttrib *attrib, uint16_t handle, gpointer user_data)
-{
-	uint8_t value[2];
-
-	att_put_u16(GATT_CLIENT_CHARAC_CFG_IND_BIT, value);
-	gatt_write_char(attrib, handle, value, sizeof(value), ccc_written_cb,
-								user_data);
-}
-
 static void indication_cb(const uint8_t *pdu, uint16_t len, gpointer user_data)
 {
 	struct gas *gas = user_data;
-	uint16_t handle, start, end, olen;
+	uint16_t start, end, olen;
 	size_t plen;
 	uint8_t *opdu;
 
@@ -207,13 +178,9 @@ static void indication_cb(const uint8_t *pdu, uint16_t len, gpointer user_data)
 		return;
 	}
 
-	handle = att_get_u16(&pdu[1]);
 	start = att_get_u16(&pdu[3]);
 	end = att_get_u16(&pdu[5]);
 
-	if (handle != gas->changed_handle)
-		return;
-
 	DBG("Service Changed start: 0x%04X end: 0x%04X", start, end);
 
 	if (device_is_bonded(gas->device) == FALSE) {
@@ -229,6 +196,39 @@ static void indication_cb(const uint8_t *pdu, uint16_t len, gpointer user_data)
 	btd_device_gatt_set_service_changed(gas->device, start, end);
 }
 
+static void ccc_written_cb(guint8 status, const guint8 *pdu, guint16 plen,
+							gpointer user_data)
+{
+	struct gas *gas = user_data;
+
+	if (status) {
+		error("Write Service Changed CCC failed: %s",
+						att_ecode2str(status));
+		return;
+	}
+
+	DBG("Service Changed indications enabled");
+
+	gas->changed_ind = g_attrib_register(gas->attrib, ATT_OP_HANDLE_IND,
+						gas->changed_handle,
+						indication_cb, gas, NULL);
+
+	write_ctp_handle(adapter_get_address(device_get_adapter(gas->device)),
+					device_get_address(gas->device),
+					device_get_addr_type(gas->device),
+					GATT_CHARAC_SERVICE_CHANGED,
+					gas->changed_handle);
+}
+
+static void write_ccc(GAttrib *attrib, uint16_t handle, gpointer user_data)
+{
+	uint8_t value[2];
+
+	att_put_u16(GATT_CLIENT_CHARAC_CFG_IND_BIT, value);
+	gatt_write_char(attrib, handle, value, sizeof(value), ccc_written_cb,
+								user_data);
+}
+
 static void gatt_descriptors_cb(guint8 status, const guint8 *pdu, guint16 len,
 							gpointer user_data)
 {
@@ -332,10 +332,6 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
 		DBG("MTU Exchange: Requesting %d", imtu);
 	}
 
-	gas->changed_ind = g_attrib_register(gas->attrib, ATT_OP_HANDLE_IND,
-						GATTRIB_ALL_HANDLES,
-						indication_cb, gas, NULL);
-
 	if (device_get_appearance(gas->device, &app) < 0) {
 		bt_uuid_t uuid;
 
-- 
1.7.12.2

Re: [RFC BlueZ 3/5] hog: Use the per handle GATT event notifier

[Vinicius Costa Gomes]

Hi,

On 21:26 Mon 08 Oct, Vinicius Costa Gomes wrote:
> ---
>  profiles/input/hog_device.c | 66 ++++++++++++++++++++++++---------------------
>  1 file changed, 36 insertions(+), 30 deletions(-)
> 
> diff --git a/profiles/input/hog_device.c b/profiles/input/hog_device.c
> index 52ebd95..34f4a41 100644
> --- a/profiles/input/hog_device.c
> +++ b/profiles/input/hog_device.c
> @@ -154,22 +134,31 @@ static void report_value_cb(const uint8_t *pdu, uint16_t len,
>  static void report_ccc_written_cb(guint8 status, const guint8 *pdu,
>  					guint16 plen, gpointer user_data)
>  {
> +	struct report *report = user_data;
> +	struct hog_device *hogdev = report->hogdev;
> +
>  	if (status != 0) {
>  		error("Write report characteristic descriptor failed: %s",
>  							att_ecode2str(status));
>  		return;
>  	}
>  
> +	report->notifyid = g_attrib_register(hogdev->attrib,
> +					ATT_OP_HANDLE_NOTIFY,
> +					report->decl->value_handle,
> +					report_value_cb, report, NULL);
> +
>  	DBG("Report characteristic descriptor written: notifications enabled");
>  }
>  
>  static void write_ccc(uint16_t handle, gpointer user_data)
>  {
> -	struct hog_device *hogdev = user_data;
> +	struct report *report = user_data;
> +	struct hog_device *hogdev = report->hogdev;
>  	uint8_t value[] = { 0x01, 0x00 };
>  
>  	gatt_write_char(hogdev->attrib, handle, value, sizeof(value),
> -					report_ccc_written_cb, hogdev);
> +					report_ccc_written_cb, report);
>  }

There's a bug in this patch. So please consider it just as a proof of
concept.


Cheers,
-- 
Vinicius