[PATCH v4 1/5] hfp_hf_bluez5: Rework code handling device changes

[PATCH v4 1/5] hfp_hf_bluez5: Rework code handling device changes

[Luiz Augusto von Dentz]

[-- Attachment #1: Type: text/plain, Size: 6981 bytes --]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This splits the handling of device changes and modem registration so
they can be uses separately.
---
 plugins/hfp_hf_bluez5.c | 150 ++++++++++++++++++++++++++----------------------
 1 file changed, 80 insertions(+), 70 deletions(-)

diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index 6ae98b3..5f23b7f 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -176,11 +176,42 @@ static int service_level_connection(struct ofono_modem *modem,
 	return -EINPROGRESS;
 }
 
-static struct ofono_modem *modem_register(const char *device,
-				const char *device_address, const char *alias)
+static void modem_removed(GDBusProxy *proxy, void *user_data)
+{
+	struct ofono_modem *modem = user_data;
+
+	ofono_modem_remove(modem);
+}
+
+static void alias_changed(GDBusProxy *proxy, const char *name,
+					DBusMessageIter *iter, void *user_data)
+{
+	const char *alias;
+	struct ofono_modem *modem = user_data;
+
+	if (g_str_equal("Alias", name) == FALSE)
+		return;
+
+	dbus_message_iter_get_basic(iter, &alias);
+	ofono_modem_set_name(modem, alias);
+}
+
+static struct ofono_modem *modem_register(const char *device, GDBusProxy *proxy)
 {
 	struct ofono_modem *modem;
 	char *path;
+	DBusMessageIter iter;
+	const char *alias, *remote;
+
+	if (g_dbus_proxy_get_property(proxy, "Alias", &iter) == FALSE)
+		return NULL;
+
+	dbus_message_iter_get_basic(&iter, &alias);
+
+	if (g_dbus_proxy_get_property(proxy, "Address", &iter) == FALSE)
+		return NULL;
+
+	dbus_message_iter_get_basic(&iter, &remote);
 
 	path = g_strconcat("hfp", device, NULL);
 
@@ -191,12 +222,15 @@ static struct ofono_modem *modem_register(const char *device,
 	if (modem == NULL)
 		return NULL;
 
-	ofono_modem_set_string(modem, "Remote", device_address);
+	ofono_modem_set_string(modem, "Remote", remote);
 	ofono_modem_set_string(modem, "DevicePath", device);
 
 	ofono_modem_set_name(modem, alias);
 	ofono_modem_register(modem);
 
+	g_dbus_proxy_set_property_watch(proxy, alias_changed, modem);
+	g_dbus_proxy_set_removed_watch(proxy, modem_removed, modem);
+
 	return modem;
 }
 
@@ -499,6 +533,29 @@ static int get_version(DBusMessageIter *iter, uint16_t *version)
 	return -ENOENT;
 }
 
+static gboolean has_hfp_ag_uuid(DBusMessageIter *array)
+{
+	DBusMessageIter value;
+
+	if (dbus_message_iter_get_arg_type(array) != DBUS_TYPE_ARRAY)
+		return FALSE;
+
+	dbus_message_iter_recurse(array, &value);
+
+	while (dbus_message_iter_get_arg_type(&value) == DBUS_TYPE_STRING) {
+		const char *uuid;
+
+		dbus_message_iter_get_basic(&value, &uuid);
+
+		if (g_str_equal(uuid, HFP_AG_UUID) == TRUE)
+			return TRUE;
+
+		dbus_message_iter_next(&value);
+	}
+
+	return FALSE;
+}
+
 static DBusMessage *profile_new_connection(DBusConnection *conn,
 					DBusMessage *msg, void *user_data)
 {
@@ -543,8 +600,8 @@ static DBusMessage *profile_new_connection(DBusConnection *conn,
 	if (modem == NULL) {
 		close(fd);
 		return g_dbus_create_error(msg, BLUEZ_ERROR_INTERFACE
-					".Rejected",
-					"Unknown Bluetooth device");
+						".Rejected",
+						"Unknown Bluetooth device");
 	}
 
 	err = service_level_connection(modem, fd, version);
@@ -686,52 +743,15 @@ static void connect_handler(DBusConnection *conn, void *user_data)
 					HFP_EXT_PROFILE_PATH, NULL, features);
 }
 
-static gboolean has_hfp_ag_uuid(DBusMessageIter *array)
+static void modem_unregister(struct ofono_modem *modem, GDBusProxy *proxy)
 {
-	DBusMessageIter value;
-
-	if (dbus_message_iter_get_arg_type(array) != DBUS_TYPE_ARRAY)
-		return FALSE;
-
-	dbus_message_iter_recurse(array, &value);
-
-	while (dbus_message_iter_get_arg_type(&value) == DBUS_TYPE_STRING) {
-		const char *uuid;
-
-		dbus_message_iter_get_basic(&value, &uuid);
-
-		if (g_str_equal(uuid, HFP_AG_UUID) == TRUE)
-			return TRUE;
-
-		dbus_message_iter_next(&value);
-	}
-
-	return FALSE;
-}
-
-static void modem_removed(GDBusProxy *proxy, void *user_data)
-{
-	struct ofono_modem *modem = user_data;
-
 	ofono_modem_remove(modem);
+	g_dbus_proxy_set_removed_watch(proxy, NULL, NULL);
+	g_dbus_proxy_set_property_watch(proxy, NULL, NULL);
 }
 
-static void alias_changed(GDBusProxy *proxy, const char *name,
-					DBusMessageIter *iter, void *user_data)
+static void device_changed(GDBusProxy *proxy, const char *path)
 {
-	const char *alias;
-	struct ofono_modem *modem = user_data;
-
-	if (g_str_equal("Alias", name) == FALSE)
-		return;
-
-	dbus_message_iter_get_basic(iter, &alias);
-	ofono_modem_set_name(modem, alias);
-}
-
-static void modem_register_from_proxy(GDBusProxy *proxy, const char *path)
-{
-	const char *alias, *remote;
 	DBusMessageIter iter;
 	dbus_bool_t paired;
 	struct ofono_modem *modem;
@@ -741,36 +761,26 @@ static void modem_register_from_proxy(GDBusProxy *proxy, const char *path)
 
 	dbus_message_iter_get_basic(&iter, &paired);
 
-	if (paired == FALSE) {
-		modem = ofono_modem_find(device_path_compare, (void *) path);
+	modem = ofono_modem_find(device_path_compare, (void *) path);
 
-		if (modem != NULL) {
-			ofono_modem_remove(modem);
-			g_dbus_proxy_set_removed_watch(proxy, NULL, NULL);
-			g_dbus_proxy_set_property_watch(proxy, NULL, NULL);
-		}
+	if (paired == FALSE) {
+		if (modem != NULL)
+			modem_unregister(modem, proxy);
 		return;
 	}
 
-	if (g_dbus_proxy_get_property(proxy, "UUIDs", &iter) == FALSE)
-		return;
-
-	if (has_hfp_ag_uuid(&iter) == FALSE)
+	if (g_dbus_proxy_get_property(proxy, "UUIDs", &iter) == FALSE ||
+				has_hfp_ag_uuid(&iter) == FALSE) {
+		if (modem != NULL)
+			modem_unregister(modem, proxy);
 		return;
+	}
 
-	if (g_dbus_proxy_get_property(proxy, "Alias", &iter) == FALSE)
-		return;
-
-	dbus_message_iter_get_basic(&iter, &alias);
-
-	if (g_dbus_proxy_get_property(proxy, "Address", &iter) == FALSE)
+	/* Skip if modem already registered */
+	if (modem != NULL)
 		return;
 
-	dbus_message_iter_get_basic(&iter, &remote);
-
-	modem = modem_register(path, remote, alias);
-	g_dbus_proxy_set_property_watch(proxy, alias_changed, modem);
-	g_dbus_proxy_set_removed_watch(proxy, modem_removed, modem);
+	modem_register(path, proxy);
 }
 
 static void proxy_added(GDBusProxy *proxy, void *user_data)
@@ -783,7 +793,7 @@ static void proxy_added(GDBusProxy *proxy, void *user_data)
 	if (g_str_equal(BLUEZ_DEVICE_INTERFACE, interface) == FALSE)
 		return;
 
-	modem_register_from_proxy(proxy, path);
+	device_changed(proxy, path);
 }
 
 static void property_changed(GDBusProxy *proxy, const char *name,
@@ -800,7 +810,7 @@ static void property_changed(GDBusProxy *proxy, const char *name,
 	if (g_str_equal("Paired", name) != TRUE)
 		return;
 
-	modem_register_from_proxy(proxy, path);
+	device_changed(proxy, path);
 }
 
 static int hfp_init(void)
-- 
2.9.3

[PATCH v4 2/5] hfp_hf_bluez5: Fix not creating modem during NewConnection

[Luiz Augusto von Dentz]

[-- Attachment #1: Type: text/plain, Size: 1207 bytes --]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

In case the UUIDs are not updated, as they are still being resolved, when
Paired property changes a modem will never be registered.

In order to fix this problem allow modems to be registered directly
during NewConnection.
---
 plugins/hfp_hf_bluez5.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index 5f23b7f..6e4e499 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -598,10 +598,18 @@ static DBusMessage *profile_new_connection(DBusConnection *conn,
 
 	modem = ofono_modem_find(device_path_compare, (void *) device);
 	if (modem == NULL) {
-		close(fd);
-		return g_dbus_create_error(msg, BLUEZ_ERROR_INTERFACE
+		GDBusProxy *proxy;
+
+		proxy = g_dbus_proxy_new(bluez, device, BLUEZ_DEVICE_INTERFACE);
+		modem = modem_register(device, proxy);
+		g_dbus_proxy_unref(proxy);
+
+		if (!modem) {
+			close(fd);
+			return g_dbus_create_error(msg, BLUEZ_ERROR_INTERFACE
 						".Rejected",
 						"Unknown Bluetooth device");
+		}
 	}
 
 	err = service_level_connection(modem, fd, version);
-- 
2.9.3

[PATCH v4 3/5] hfp_hf_bluez5: Handle ServicesResolved signal

[Luiz Augusto von Dentz]

[-- Attachment #1: Type: text/plain, Size: 880 bytes --]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This adds handling for ServicesResolved signal which tells when BlueZ
is done resolving the device services so the code will no longer ignore
devices that got its services resolved after Paired signal.
---
 plugins/hfp_hf_bluez5.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index 6e4e499..c04d740 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -815,7 +815,8 @@ static void property_changed(GDBusProxy *proxy, const char *name,
 	if (g_str_equal(BLUEZ_DEVICE_INTERFACE, interface) == FALSE)
 		return;
 
-	if (g_str_equal("Paired", name) != TRUE)
+	if (g_str_equal("Paired", name) != TRUE &&
+			g_str_equal("ServicesResolved", name) != TRUE)
 		return;
 
 	device_changed(proxy, path);
-- 
2.9.3

[PATCH v4 4/5] doc/handsfree-audio-api: Add Acquire method

[Luiz Augusto von Dentz]

[-- Attachment #1: Type: text/plain, Size: 1767 bytes --]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This adds Acquire method which can be used by agents that require
pulling the fd directly instead of waiting a NewConnection.

Note: sounds servers like PulseAudio do auto suspend streams when idle
for a certain amount of time and once anything happens it will resume
the stream, though this all happens in the so called IO thread in a
blocking fashion making it impossible to receive the fd via NewConnetion
causing the stream to fail to resume. There are other forms to work
around but this seems to be most convenient as we do want the auto
suspend feature to work properly but letting the stream to fail to
resume may create unexpected artifacts while the NewConnection is
handled in main thread.
---
 doc/handsfree-audio-api.txt | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/doc/handsfree-audio-api.txt b/doc/handsfree-audio-api.txt
index d82035c..87d51cf 100644
--- a/doc/handsfree-audio-api.txt
+++ b/doc/handsfree-audio-api.txt
@@ -76,6 +76,22 @@ Methods		dict GetProperties()
 					 [service].Error.NotImplemented
 					 [service].Error.NotAllowed
 
+		fd, byte Acquire()
+
+			Attempts to establish the SCO audio connection
+			returning the filedescriptor of the connection and the
+			codec in use.
+
+			Note: Contrary to Connect this does not call
+			NewConnection so it can be called in a blocking
+			manner.
+
+			Possible Errors: [service].Error.InProgress
+					 [service].Error.Failed
+					 [service].Error.NotAvailable
+					 [service].Error.NotImplemented
+					 [service].Error.NotAllowed
+
 Signals		PropertyChanged(string name, variant value)
 
 			This signal indicates a changed value of the given
-- 
2.9.3

[PATCH v4 5/5] handsfree-audio: Add Acquire implementation

[Luiz Augusto von Dentz]

[-- Attachment #1: Type: text/plain, Size: 1184 bytes --]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This adds Acquire method which can be used by agents that require
pulling the fd directly instead of waiting a NewConnection.
---
 src/handsfree-audio.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/handsfree-audio.c b/src/handsfree-audio.c
index dab16da..f2c323d 100644
--- a/src/handsfree-audio.c
+++ b/src/handsfree-audio.c
@@ -326,6 +326,12 @@ static gboolean sco_connect_cb(GIOChannel *io, GIOCondition cond,
 
 	sk = g_io_channel_unix_get_fd(io);
 
+	if (card->msg && dbus_message_has_member(card->msg, "Acquire")) {
+		reply = g_dbus_create_reply(card->msg, DBUS_TYPE_UNIX_FD, &sk,
+					DBUS_TYPE_BYTE, &card->selected_codec);
+		goto done;
+	}
+
 	send_new_connection(card->path, sk, card->selected_codec);
 
 	close(sk);
@@ -403,6 +409,9 @@ static const GDBusMethodTable card_methods[] = {
 			NULL, GDBUS_ARGS({ "properties", "a{sv}" }),
 			card_get_properties) },
 	{ GDBUS_ASYNC_METHOD("Connect", NULL, NULL, card_connect) },
+	{ GDBUS_ASYNC_METHOD("Acquire", NULL,
+			GDBUS_ARGS({"sco", "h"}, {"codec", "y"}),
+			card_connect) },
 	{ }
 };
 
-- 
2.9.3