[PATCH BlueZ 0/7] Proximity Monitor optimizations and checks

[PATCH BlueZ 0/7] Proximity Monitor optimizations and checks

[Claudio Takahasi]

Dependency: "[PATCH BlueZ 0/3] Add Proximity Monitor properties" needs
to be applied first.

The following patches contain optimizations and configuration checking
based on proximity config file values.

Bruna Moreira (1):
  Add enum for alert level values

Claudio Takahasi (6):
  Parse handles when probing on Proximity
  Request connection if Link or Path Loss is enabled
  Write Link Loss alert if the service is enabled
  Add utility function to convert alert levels
  Add function to check invalid alert level value
  Skip Link Loss handle discovery

 proximity/manager.c |   36 +++++++++-----
 proximity/monitor.c |  137 ++++++++++++++++++++++++++++++++++-----------------
 proximity/monitor.h |    3 +-
 3 files changed, 118 insertions(+), 58 deletions(-)

-- 
1.7.6

[PATCH BlueZ 1/7] Parse handles when probing on Proximity

[Claudio Takahasi]

Primary services start and end handles can now be obtained during
probing. This approach avoids primary service parsing on each
connection in the Proximity Monitor.
---
 proximity/manager.c |   36 ++++++++++++++++--------
 proximity/monitor.c |   74 +++++++++++++++++++++++++++++----------------------
 proximity/monitor.h |    3 +-
 3 files changed, 68 insertions(+), 45 deletions(-)

diff --git a/proximity/manager.c b/proximity/manager.c
index 30384d1..a767554 100644
--- a/proximity/manager.c
+++ b/proximity/manager.c
@@ -28,9 +28,11 @@
 
 #include <glib.h>
 #include <gdbus.h>
+#include <bluetooth/uuid.h>
 
 #include "adapter.h"
 #include "device.h"
+#include "att.h"
 #include "monitor.h"
 #include "reporter.h"
 #include "manager.h"
@@ -47,23 +49,33 @@ static struct enabled enabled  = {
 	.findme = TRUE,
 };
 
+static gint primary_uuid_cmp(gconstpointer a, gconstpointer b)
+{
+	const struct att_primary *prim = a;
+	const char *uuid = b;
+
+	return g_strcmp0(prim->uuid, uuid);
+}
+
 static int attio_device_probe(struct btd_device *device, GSList *uuids)
 {
-	gboolean linkloss = FALSE, pathloss = FALSE, findme = FALSE;
-
-	if (g_slist_find_custom(uuids, IMMEDIATE_ALERT_UUID,
-					(GCompareFunc) strcasecmp)) {
-		findme = enabled.findme;
-		if (g_slist_find_custom(uuids, TX_POWER_UUID,
-					(GCompareFunc) strcasecmp))
-			pathloss = enabled.pathloss;
-	}
+	struct att_primary *linkloss, *txpower, *immediate;
+	GSList *l, *primaries;
+
+	primaries = btd_device_get_primaries(device);
+
+	l = g_slist_find_custom(primaries, IMMEDIATE_ALERT_UUID,
+			primary_uuid_cmp);
+	immediate = (l ? l->data : NULL);
+
+	l = g_slist_find_custom(primaries, TX_POWER_UUID, primary_uuid_cmp);
+	txpower = (l ? l->data : NULL);
 
-	if (g_slist_find_custom(uuids, LINK_LOSS_UUID,
-				(GCompareFunc) strcasecmp))
-		linkloss = enabled.linkloss;
+	l = g_slist_find_custom(primaries, LINK_LOSS_UUID, primary_uuid_cmp);
+	linkloss = (l ? l->data : NULL);
 
-	return monitor_register(connection, device, linkloss, pathloss, findme);
+	return monitor_register(connection, device, linkloss, txpower,
+							immediate, &enabled);
 }
 
 static void attio_device_remove(struct btd_device *device)
diff --git a/proximity/monitor.c b/proximity/monitor.c
index f4ea0df..3e4f353 100644
--- a/proximity/monitor.c
+++ b/proximity/monitor.c
@@ -51,12 +51,14 @@
 
 #define PROXIMITY_INTERFACE "org.bluez.Proximity"
 
-#define LINK_LOSS_UUID "00001803-0000-1000-8000-00805f9b34fb"
 #define ALERT_LEVEL_CHR_UUID 0x2A06
 
 struct monitor {
 	struct btd_device *device;
 	GAttrib *attrib;
+	struct att_range *linkloss;
+	struct att_range *txpower;
+	struct att_range *immediate;
 	struct enabled enabled;
 	char *linklosslevel;		/* Link Loss Alert Level */
 	char *immediatelevel;		/* Immediate Alert Level */
@@ -139,38 +141,14 @@ static void char_discovered_cb(GSList *characteristics, guint8 status,
 
 static int write_alert_level(struct monitor *monitor)
 {
-	GSList *l, *primaries;
-	uint16_t start = 0, end = 0;
+	struct att_range *linkloss = monitor->linkloss;
 	bt_uuid_t uuid;
 
-	primaries = btd_device_get_primaries(monitor->device);
-	if (primaries == NULL) {
-		DBG("No primary services found");
-		return -1;
-	}
-
-	for (l = primaries; l; l = l->next) {
-		struct att_primary *primary = l->data;
-
-		if (strcmp(primary->uuid, LINK_LOSS_UUID) == 0) {
-			start = primary->start;
-			end = primary->end;
-			break;
-		}
-	}
-
-	if (!start) {
-		DBG("Link Loss service not found");
-		return -1;
-	}
-
-	DBG("Link Loss service found at range 0x%04x-0x%04x", start, end);
-
 	bt_uuid16_create(&uuid, ALERT_LEVEL_CHR_UUID);
 
 	/* FIXME: use cache (requires service changed support) ? */
-	gatt_discover_char(monitor->attrib, start, end, &uuid, char_discovered_cb,
-								monitor);
+	gatt_discover_char(monitor->attrib, linkloss->start, linkloss->end,
+			&uuid, char_discovered_cb, monitor);
 
 	return 0;
 }
@@ -341,6 +319,9 @@ static void monitor_destroy(gpointer user_data)
 	struct monitor *monitor = user_data;
 
 	btd_device_unref(monitor->device);
+	g_free(monitor->linkloss);
+	g_free(monitor->immediate);
+	g_free(monitor->txpower);
 	g_free(monitor->linklosslevel);
 	g_free(monitor->immediatelevel);
 	g_free(monitor->signallevel);
@@ -348,7 +329,8 @@ static void monitor_destroy(gpointer user_data)
 }
 
 int monitor_register(DBusConnection *conn, struct btd_device *device,
-			gboolean linkloss, gboolean pathloss, gboolean findme)
+		struct att_primary *linkloss, struct att_primary *txpower,
+		struct att_primary *immediate, struct enabled *enabled)
 {
 	const char *path = device_get_path(device);
 	struct monitor *monitor;
@@ -364,9 +346,6 @@ int monitor_register(DBusConnection *conn, struct btd_device *device,
 	monitor->device = btd_device_ref(device);
 	monitor->linklosslevel = (level ? : g_strdup("none"));
 	monitor->signallevel = g_strdup("unknown");
-	monitor->enabled.linkloss = linkloss;
-	monitor->enabled.pathloss = pathloss;
-	monitor->enabled.findme = findme;
 
 	if (g_dbus_register_interface(conn, path,
 				PROXIMITY_INTERFACE,
@@ -380,6 +359,37 @@ int monitor_register(DBusConnection *conn, struct btd_device *device,
 
 	DBG("Registered interface %s on path %s", PROXIMITY_INTERFACE, path);
 
+	if (linkloss && enabled->linkloss) {
+		monitor->linkloss = g_new0(struct att_range, 1);
+		monitor->linkloss->start = linkloss->start;
+		monitor->linkloss->end = linkloss->end;
+
+		monitor->enabled.linkloss = TRUE;
+	}
+
+	if (immediate) {
+		if (txpower && enabled->pathloss) {
+			monitor->txpower = g_new0(struct att_range, 1);
+			monitor->txpower->start = txpower->start;
+			monitor->txpower->end = txpower->end;
+
+			monitor->enabled.pathloss = TRUE;
+		}
+
+		if (enabled->pathloss || enabled->findme) {
+			monitor->immediate = g_new0(struct att_range, 1);
+			monitor->immediate->start = immediate->start;
+			monitor->immediate->end = immediate->end;
+		}
+
+		monitor->enabled.findme = enabled->findme;
+	}
+
+	DBG("Link Loss: %s, Path Loss: %s, FindMe: %s",
+				monitor->enabled.linkloss ? "TRUE" : "FALSE",
+				monitor->enabled.pathloss ? "TRUE" : "FALSE",
+				monitor->enabled.findme ? "TRUE" : "FALSE");
+
 	btd_device_add_attio_callback(device, attio_connected_cb,
 					attio_disconnected_cb, monitor);
 
diff --git a/proximity/monitor.h b/proximity/monitor.h
index ef47ee1..fb79e26 100644
--- a/proximity/monitor.h
+++ b/proximity/monitor.h
@@ -29,5 +29,6 @@ struct enabled {
 };
 
 int monitor_register(DBusConnection *conn, struct btd_device *device,
-			gboolean linkloss, gboolean pathloss, gboolean findme);
+		struct att_primary *linkloss, struct att_primary *txpower,
+		struct att_primary *immediate, struct enabled *enabled);
 void monitor_unregister(DBusConnection *conn, struct btd_device *device);
-- 
1.7.6

[PATCH BlueZ 2/7] Request connection if Link or Path Loss is enabled

[Claudio Takahasi]

Proximity Monitor needs to keep the link up if Link Loss and/or Path
Loss service is enabled.
---
 proximity/monitor.c |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/proximity/monitor.c b/proximity/monitor.c
index 3e4f353..ba1982e 100644
--- a/proximity/monitor.c
+++ b/proximity/monitor.c
@@ -63,6 +63,7 @@ struct monitor {
 	char *linklosslevel;		/* Link Loss Alert Level */
 	char *immediatelevel;		/* Immediate Alert Level */
 	char *signallevel;		/* Path Loss RSSI level */
+	guint attioid;
 };
 
 static inline int create_filename(char *buf, size_t size,
@@ -318,6 +319,12 @@ static void monitor_destroy(gpointer user_data)
 {
 	struct monitor *monitor = user_data;
 
+	if (monitor->attioid)
+		btd_device_remove_attio_callback(monitor->device,
+						monitor->attioid);
+	if (monitor->attrib)
+		g_attrib_unref(monitor->attrib);
+
 	btd_device_unref(monitor->device);
 	g_free(monitor->linkloss);
 	g_free(monitor->immediate);
@@ -390,8 +397,11 @@ int monitor_register(DBusConnection *conn, struct btd_device *device,
 				monitor->enabled.pathloss ? "TRUE" : "FALSE",
 				monitor->enabled.findme ? "TRUE" : "FALSE");
 
-	btd_device_add_attio_callback(device, attio_connected_cb,
-					attio_disconnected_cb, monitor);
+	if (monitor->enabled.linkloss || monitor->enabled.pathloss)
+		monitor->attioid = btd_device_add_attio_callback(device,
+							attio_connected_cb,
+							attio_disconnected_cb,
+							monitor);
 
 	return 0;
 }
-- 
1.7.6

[PATCH BlueZ 3/7] Write Link Loss alert if the service is enabled

[Claudio Takahasi]

Verifies if the Link Loss service is enabled before writting the Link
Loss Alert Level in the Proximity Reporter.
---
 proximity/monitor.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/proximity/monitor.c b/proximity/monitor.c
index ba1982e..1bba4cd 100644
--- a/proximity/monitor.c
+++ b/proximity/monitor.c
@@ -159,7 +159,9 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
 	struct monitor *monitor = user_data;
 
 	monitor->attrib = g_attrib_ref(attrib);
-	write_alert_level(monitor);
+
+	if (monitor->enabled.linkloss)
+		write_alert_level(monitor);
 }
 
 static void attio_disconnected_cb(gpointer user_data)
-- 
1.7.6

[PATCH BlueZ 4/7] Add utility function to convert alert levels

[Claudio Takahasi]

Convert Link Loss or Immediate Alert Level string to byte.
---
 proximity/monitor.c |   21 ++++++++++++---------
 1 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/proximity/monitor.c b/proximity/monitor.c
index 1bba4cd..6470db4 100644
--- a/proximity/monitor.c
+++ b/proximity/monitor.c
@@ -113,25 +113,28 @@ static char *read_proximity_config(bdaddr_t *sba, bdaddr_t *dba,
 	return strnew;
 }
 
+static uint8_t str2level(const char *level)
+{
+	if (g_strcmp0("high", level) == 0)
+		return 0x02;
+	else if (g_strcmp0("mild", level) == 0)
+		return 0x01;
+
+	return 0x00;
+}
+
 static void char_discovered_cb(GSList *characteristics, guint8 status,
 							gpointer user_data)
 {
 	struct monitor *monitor = user_data;
 	struct att_char *chr;
-	uint8_t value;
+	uint8_t value = str2level(monitor->linklosslevel);
 
 	if (status) {
 		error("Discover Link Loss handle: %s", att_ecode2str(status));
 		return;
 	}
 
-	if (strcmp(monitor->linklosslevel, "none") == 0)
-		value = 0x00;
-	else if (strcmp(monitor->linklosslevel, "mild") == 0)
-		value = 0x01;
-	else if (strcmp(monitor->linklosslevel, "high") == 0)
-		value = 0x02;
-
 	DBG("Setting alert level \"%s\" on Reporter", monitor->linklosslevel);
 
 	/* Assume there is a single Alert Level characteristic */
@@ -149,7 +152,7 @@ static int write_alert_level(struct monitor *monitor)
 
 	/* FIXME: use cache (requires service changed support) ? */
 	gatt_discover_char(monitor->attrib, linkloss->start, linkloss->end,
-			&uuid, char_discovered_cb, monitor);
+					&uuid, char_discovered_cb, monitor);
 
 	return 0;
 }
-- 
1.7.6

[PATCH BlueZ 5/7] Add enum for alert level values

[Claudio Takahasi]

From: Bruna Moreira <bruna.moreira@openbossa.org>

---
 proximity/monitor.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/proximity/monitor.c b/proximity/monitor.c
index 6470db4..ca19a8f 100644
--- a/proximity/monitor.c
+++ b/proximity/monitor.c
@@ -53,6 +53,12 @@
 
 #define ALERT_LEVEL_CHR_UUID 0x2A06
 
+enum {
+	ALERT_NONE = 0,
+	ALERT_MILD,
+	ALERT_HIGH,
+};
+
 struct monitor {
 	struct btd_device *device;
 	GAttrib *attrib;
@@ -116,11 +122,11 @@ static char *read_proximity_config(bdaddr_t *sba, bdaddr_t *dba,
 static uint8_t str2level(const char *level)
 {
 	if (g_strcmp0("high", level) == 0)
-		return 0x02;
+		return ALERT_HIGH;
 	else if (g_strcmp0("mild", level) == 0)
-		return 0x01;
+		return ALERT_MILD;
 
-	return 0x00;
+	return ALERT_NONE;;
 }
 
 static void char_discovered_cb(GSList *characteristics, guint8 status,
-- 
1.7.6

[PATCH BlueZ 6/7] Add function to check invalid alert level value

[Claudio Takahasi]

---
 proximity/monitor.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/proximity/monitor.c b/proximity/monitor.c
index ca19a8f..a6bdf34 100644
--- a/proximity/monitor.c
+++ b/proximity/monitor.c
@@ -181,6 +181,13 @@ static void attio_disconnected_cb(gpointer user_data)
 	monitor->attrib = NULL;
 }
 
+static gboolean level_is_valid(const char *level)
+{
+	return (g_str_equal("none", level) ||
+			g_str_equal("mild", level) ||
+			g_str_equal("high", level));
+}
+
 static DBusMessage *set_link_loss_alert(DBusConnection *conn, DBusMessage *msg,
 						const char *level, void *data)
 {
@@ -189,8 +196,7 @@ static DBusMessage *set_link_loss_alert(DBusConnection *conn, DBusMessage *msg,
 	const char *path = device_get_path(device);
 	bdaddr_t sba, dba;
 
-	if (!g_str_equal("none", level) && !g_str_equal("mild", level) &&
-			!g_str_equal("high", level))
+	if (!level_is_valid(level))
 		return btd_error_invalid_args(msg);
 
 	if (g_strcmp0(monitor->linklosslevel, level) == 0)
@@ -217,8 +223,7 @@ static DBusMessage *set_immediate_alert(DBusConnection *conn, DBusMessage *msg,
 	struct monitor *monitor = data;
 	const gchar *path = device_get_path(monitor->device);
 
-	if (!g_str_equal("none", level) && !g_str_equal("mild", level) &&
-			!g_str_equal("high", level))
+	if (!level_is_valid(level))
 		return btd_error_invalid_args(msg);
 
 	if (g_strcmp0(monitor->immediatelevel, level) == 0)
-- 
1.7.6

[PATCH BlueZ 7/7] Skip Link Loss handle discovery

[Claudio Takahasi]

Skip characteristic value handle discovery for Link Loss service if the
handle is already known.
---
 proximity/monitor.c |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/proximity/monitor.c b/proximity/monitor.c
index a6bdf34..ea5059f 100644
--- a/proximity/monitor.c
+++ b/proximity/monitor.c
@@ -69,6 +69,7 @@ struct monitor {
 	char *linklosslevel;		/* Link Loss Alert Level */
 	char *immediatelevel;		/* Immediate Alert Level */
 	char *signallevel;		/* Path Loss RSSI level */
+	uint16_t linklosshandle;	/* Link Loss Characteristic Value Handle */
 	guint attioid;
 };
 
@@ -145,8 +146,10 @@ static void char_discovered_cb(GSList *characteristics, guint8 status,
 
 	/* Assume there is a single Alert Level characteristic */
 	chr = characteristics->data;
+	monitor->linklosshandle = chr->value_handle;
 
-	gatt_write_cmd(monitor->attrib, chr->value_handle, &value, 1, NULL, NULL);
+	gatt_write_cmd(monitor->attrib, monitor->linklosshandle, &value, 1,
+								NULL, NULL);
 }
 
 static int write_alert_level(struct monitor *monitor)
@@ -154,6 +157,14 @@ static int write_alert_level(struct monitor *monitor)
 	struct att_range *linkloss = monitor->linkloss;
 	bt_uuid_t uuid;
 
+	if (monitor->linklosshandle) {
+		uint8_t value = str2level(monitor->linklosslevel);
+
+		gatt_write_cmd(monitor->attrib, monitor->linklosshandle,
+							&value, 1, NULL, NULL);
+		return 0;
+	}
+
 	bt_uuid16_create(&uuid, ALERT_LEVEL_CHR_UUID);
 
 	/* FIXME: use cache (requires service changed support) ? */
-- 
1.7.6

Re: [PATCH BlueZ 5/7] Add enum for alert level values

[Claudio Takahasi]

Hi Johan,

On Wed, Aug 3, 2011 at 3:41 PM, Claudio Takahasi
<claudio.takahasi@openbossa.org> wrote:
> From: Bruna Moreira <bruna.moreira@openbossa.org>
>
> ---
>  proximity/monitor.c |   12 +++++++++---
>  1 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/proximity/monitor.c b/proximity/monitor.c
> index 6470db4..ca19a8f 100644
> --- a/proximity/monitor.c
> +++ b/proximity/monitor.c
> @@ -53,6 +53,12 @@
>
>  #define ALERT_LEVEL_CHR_UUID 0x2A06
>
> +enum {
> +       ALERT_NONE = 0,
> +       ALERT_MILD,
> +       ALERT_HIGH,
> +};
> +
>  struct monitor {
>        struct btd_device *device;
>        GAttrib *attrib;
> @@ -116,11 +122,11 @@ static char *read_proximity_config(bdaddr_t *sba, bdaddr_t *dba,
>  static uint8_t str2level(const char *level)
>  {
>        if (g_strcmp0("high", level) == 0)
> -               return 0x02;
> +               return ALERT_HIGH;
>        else if (g_strcmp0("mild", level) == 0)
> -               return 0x01;
> +               return ALERT_MILD;
>
> -       return 0x00;
> +       return ALERT_NONE;;

I gonna send a -v2 patch fixing this extra semicolon.

BR,
Claudio

[PATCH BlueZ v2 5/7] Add enum for alert level values

[Claudio Takahasi]

From: Bruna Moreira <bruna.moreira@openbossa.org>

---
 proximity/monitor.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/proximity/monitor.c b/proximity/monitor.c
index 6470db4..60f91bd 100644
--- a/proximity/monitor.c
+++ b/proximity/monitor.c
@@ -53,6 +53,12 @@
 
 #define ALERT_LEVEL_CHR_UUID 0x2A06
 
+enum {
+	ALERT_NONE = 0,
+	ALERT_MILD,
+	ALERT_HIGH,
+};
+
 struct monitor {
 	struct btd_device *device;
 	GAttrib *attrib;
@@ -116,11 +122,11 @@ static char *read_proximity_config(bdaddr_t *sba, bdaddr_t *dba,
 static uint8_t str2level(const char *level)
 {
 	if (g_strcmp0("high", level) == 0)
-		return 0x02;
+		return ALERT_HIGH;
 	else if (g_strcmp0("mild", level) == 0)
-		return 0x01;
+		return ALERT_MILD;
 
-	return 0x00;
+	return ALERT_NONE;
 }
 
 static void char_discovered_cb(GSList *characteristics, guint8 status,
-- 
1.7.6

Re: [PATCH BlueZ 0/7] Proximity Monitor optimizations and checks

[Johan Hedberg]

Hi Claudio,

On Wed, Aug 03, 2011, Claudio Takahasi wrote:
> Dependency: "[PATCH BlueZ 0/3] Add Proximity Monitor properties" needs
> to be applied first.
> 
> The following patches contain optimizations and configuration checking
> based on proximity config file values.
> 
> Bruna Moreira (1):
>   Add enum for alert level values
> 
> Claudio Takahasi (6):
>   Parse handles when probing on Proximity
>   Request connection if Link or Path Loss is enabled
>   Write Link Loss alert if the service is enabled
>   Add utility function to convert alert levels
>   Add function to check invalid alert level value
>   Skip Link Loss handle discovery
> 
>  proximity/manager.c |   36 +++++++++-----
>  proximity/monitor.c |  137 ++++++++++++++++++++++++++++++++++-----------------
>  proximity/monitor.h |    3 +-
>  3 files changed, 118 insertions(+), 58 deletions(-)

Applied. Thanks.

Johan