[PATCH v6 1/3] hfp_hf_bluez5: Rework code handling device changes

[PATCH v6 1/3] hfp_hf_bluez5: Rework code handling device changes

[Luiz Augusto von Dentz]

[-- Attachment #1: Type: text/plain, Size: 5611 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.
---
v6: Now properly rebased

 plugins/hfp_hf_bluez5.c | 104 +++++++++++++++++++++++++-----------------------
 1 file changed, 55 insertions(+), 49 deletions(-)

diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index f7d2450..bdeb3ca 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;
 }
 
@@ -522,30 +556,15 @@ static gboolean has_hfp_ag_uuid(DBusMessageIter *array)
 	return FALSE;
 }
 
-static void modem_removed(GDBusProxy *proxy, void *user_data)
+static void modem_unregister(struct ofono_modem *modem, GDBusProxy *proxy)
 {
-	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)
-{
-	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_from_proxy(GDBusProxy *proxy,
-							const char *path)
+static void *device_changed(GDBusProxy *proxy, const char *path)
 {
-	const char *alias, *remote;
 	DBusMessageIter iter;
 	dbus_bool_t paired;
 	struct ofono_modem *modem;
@@ -558,36 +577,23 @@ static struct ofono_modem *modem_register_from_proxy(GDBusProxy *proxy,
 	modem = ofono_modem_find(device_path_compare, (void *) path);
 
 	if (paired == FALSE) {
-		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 (modem != NULL)
+			modem_unregister(modem, proxy);
 		return NULL;
 	}
 
-	if (modem)
-		return modem;
-
-	if (g_dbus_proxy_get_property(proxy, "UUIDs", &iter) == FALSE)
-		return NULL;
-
-	if (has_hfp_ag_uuid(&iter) == FALSE)
-		return NULL;
-
-	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)
+	if (g_dbus_proxy_get_property(proxy, "UUIDs", &iter) == FALSE ||
+					has_hfp_ag_uuid(&iter) == FALSE) {
+		if (modem != NULL)
+			modem_unregister(modem, proxy);
 		return NULL;
+	}
 
-	dbus_message_iter_get_basic(&iter, &remote);
+	/* Skip if modem already registered */
+	if (modem)
+		return modem;
 
-	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 = modem_register(path, proxy);
 
 	return modem;
 }
@@ -637,7 +643,7 @@ static DBusMessage *profile_new_connection(DBusConnection *conn,
 		GDBusProxy *proxy;
 
 		proxy = g_dbus_proxy_new(bluez, device, BLUEZ_DEVICE_INTERFACE);
-		modem = modem_register_from_proxy(proxy, device);
+		modem = modem_register(device, proxy);
 		g_dbus_proxy_unref(proxy);
 
 		if (!modem) {
@@ -797,7 +803,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,
@@ -815,7 +821,7 @@ static void property_changed(GDBusProxy *proxy, const char *name,
 			g_str_equal("ServicesResolved", name) != TRUE)
 		return;
 
-	modem_register_from_proxy(proxy, path);
+	device_changed(proxy, path);
 }
 
 static int hfp_init(void)
-- 
2.9.3

[PATCH v6 2/3] 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 v6 3/3] handsfree-audio: Add Acquire implementation

[Luiz Augusto von Dentz]

[-- Attachment #1: Type: text/plain, Size: 1213 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 | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/src/handsfree-audio.c b/src/handsfree-audio.c
index dab16da..5ca8702 100644
--- a/src/handsfree-audio.c
+++ b/src/handsfree-audio.c
@@ -326,6 +326,13 @@ 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,
+					DBUS_TYPE_INVALID);
+		goto done;
+	}
+
 	send_new_connection(card->path, sk, card->selected_codec);
 
 	close(sk);
@@ -403,6 +410,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

Re: [PATCH v6 1/3] hfp_hf_bluez5: Rework code handling device changes

[Denis Kenzior]

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

Hi Luiz,

On 05/04/2017 03:22 AM, Luiz Augusto von Dentz wrote:
> 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.
> ---
> v6: Now properly rebased
>
>  plugins/hfp_hf_bluez5.c | 104 +++++++++++++++++++++++++-----------------------
>  1 file changed, 55 insertions(+), 49 deletions(-)
>

All applied, thanks.

Regards,
-Denis