[PATCH v0 1/2] hfp_hf_bluez5: Improve Proxy property changed handling

[PATCH v0 1/2] hfp_hf_bluez5: Improve Proxy property changed handling

[Claudio Takahasi]

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

This patch removes unnecessary GDBusProxy object access when property
changed is triggered. The property name and the argument iterator is
informed in the function callback.
---
 plugins/hfp_hf_bluez5.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index 2aabdd2..653829b 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -540,7 +540,6 @@ static void property_changed(GDBusProxy *proxy, const char *name,
 {
 	const char *interface, *path, *alias;
 	struct ofono_modem *modem;
-	DBusMessageIter alias_iter;
 
 	interface = g_dbus_proxy_get_interface(proxy);
 	path = g_dbus_proxy_get_path(proxy);
@@ -550,10 +549,10 @@ static void property_changed(GDBusProxy *proxy, const char *name,
 	if (g_str_equal(BLUEZ_DEVICE_INTERFACE, interface) == FALSE)
 		return;
 
-	if (g_dbus_proxy_get_property(proxy, "Alias", &alias_iter) == FALSE)
+	if (g_str_equal("Alias", name) == FALSE)
 		return;
 
-	dbus_message_iter_get_basic(&alias_iter, &alias);
+	dbus_message_iter_get_basic(iter, &alias);
 
 	modem = g_hash_table_lookup(modem_hash, path);
 	if (modem == NULL)
-- 
1.7.11.7

[PATCH v0 2/2] hfp_hf_bluez5: Only register modems for Paired devices

[Claudio Takahasi]

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

From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>

When there are many devices around that support the HFP AG profile,
we may have a lot of modems that the user will never use.
---
 plugins/hfp_hf_bluez5.c | 61 +++++++++++++++++++++++++++++++++----------------
 1 file changed, 41 insertions(+), 20 deletions(-)

diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index 653829b..8cac945 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -479,20 +479,18 @@ static gboolean has_hfp_ag_uuid(DBusMessageIter *array)
 	return FALSE;
 }
 
-static void proxy_added(GDBusProxy *proxy, void *user_data)
+static void modem_register_from_proxy(GDBusProxy *proxy, const char *path)
 {
-	const char *interface, *path, *alias, *address;
+	const char *alias, *remote;
 	DBusMessageIter iter;
+	dbus_bool_t paired;
 
-	interface = g_dbus_proxy_get_interface(proxy);
-	path = g_dbus_proxy_get_path(proxy);
-
-	if (g_str_equal(BLUEZ_DEVICE_INTERFACE, interface) == FALSE)
+	if (g_dbus_proxy_get_property(proxy, "Paired", &iter) == FALSE)
 		return;
 
-	g_hash_table_insert(devices_proxies, g_strdup(path),
-						g_dbus_proxy_ref(proxy));
-	DBG("Device proxy: %s(%p)", path, proxy);
+	dbus_message_iter_get_basic(&iter, &paired);
+	if (paired == FALSE)
+		return;
 
 	if (g_dbus_proxy_get_property(proxy, "UUIDs", &iter) == FALSE)
 		return;
@@ -509,9 +507,26 @@ static void proxy_added(GDBusProxy *proxy, void *user_data)
 	if (g_dbus_proxy_get_property(proxy, "Address", &iter) == FALSE)
 		return;
 
-	dbus_message_iter_get_basic(&iter, &address);
+	dbus_message_iter_get_basic(&iter, &remote);
+
+	modem_register(path, remote, alias);
+}
+
+static void proxy_added(GDBusProxy *proxy, void *user_data)
+{
+	const char *interface, *path;
+
+	interface = g_dbus_proxy_get_interface(proxy);
+	path = g_dbus_proxy_get_path(proxy);
 
-	modem_register(path, address, alias);
+	if (g_str_equal(BLUEZ_DEVICE_INTERFACE, interface) == FALSE)
+		return;
+
+	g_hash_table_insert(devices_proxies, g_strdup(path),
+						g_dbus_proxy_ref(proxy));
+	DBG("Device proxy: %s(%p)", path, proxy);
+
+	modem_register_from_proxy(proxy, path);
 }
 
 static void proxy_removed(GDBusProxy *proxy, void *user_data)
@@ -538,27 +553,33 @@ static void proxy_removed(GDBusProxy *proxy, void *user_data)
 static void property_changed(GDBusProxy *proxy, const char *name,
 					DBusMessageIter *iter, void *user_data)
 {
-	const char *interface, *path, *alias;
+	const char *interface, *path;
 	struct ofono_modem *modem;
 
 	interface = g_dbus_proxy_get_interface(proxy);
 	path = g_dbus_proxy_get_path(proxy);
 
-	DBG("path: %s interface: %s", path, interface);
-
 	if (g_str_equal(BLUEZ_DEVICE_INTERFACE, interface) == FALSE)
 		return;
 
-	if (g_str_equal("Alias", name) == FALSE)
+	if (g_str_equal("Paired", name) == TRUE) {
+		modem_register_from_proxy(proxy, path);
 		return;
+	}
 
-	dbus_message_iter_get_basic(iter, &alias);
+	if (g_str_equal("Alias", name) == TRUE) {
+		const char *alias;
 
-	modem = g_hash_table_lookup(modem_hash, path);
-	if (modem == NULL)
-		return;
+		dbus_message_iter_get_basic(iter, &alias);
 
-	ofono_modem_set_name(modem, alias);
+		modem = g_hash_table_lookup(modem_hash, path);
+		if (modem == NULL)
+			return;
+
+		DBG("path: %s Alias: %s", path, alias);
+
+		ofono_modem_set_name(modem, alias);
+	}
 }
 
 static int hfp_init(void)
-- 
1.7.11.7

Re: [PATCH v0 1/2] hfp_hf_bluez5: Improve Proxy property changed handling

[Denis Kenzior]

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

Hi Claudio,

On 01/30/2013 10:24 AM, Claudio Takahasi wrote:
> This patch removes unnecessary GDBusProxy object access when property
> changed is triggered. The property name and the argument iterator is
> informed in the function callback.
> ---
>   plugins/hfp_hf_bluez5.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
>

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH v0 2/2] hfp_hf_bluez5: Only register modems for Paired devices

[Denis Kenzior]

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

Hi Vinicius,

On 01/30/2013 10:24 AM, Claudio Takahasi wrote:
> From: Vinicius Costa Gomes<vinicius.gomes@openbossa.org>
>
> When there are many devices around that support the HFP AG profile,
> we may have a lot of modems that the user will never use.
> ---
>   plugins/hfp_hf_bluez5.c | 61 +++++++++++++++++++++++++++++++++----------------
>   1 file changed, 41 insertions(+), 20 deletions(-)
>

Patch has been applied, however:

> -	if (g_str_equal("Alias", name) == FALSE)
> +	if (g_str_equal("Paired", name) == TRUE) {
> +		modem_register_from_proxy(proxy, path);
>   		return;
> +	}

This part looks suspicious in the case where Paired goes to false.  If 
BlueZ behaves in a certain way when the device is unpaired then I'd like 
to see a comment here explaining that.

Regards,
-Denis

Re: [PATCH v0 2/2] hfp_hf_bluez5: Only register modems for Paired devices

[Claudio Takahasi]

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

Hi Denis:

On Thu, Jan 31, 2013 at 12:44 AM, Denis Kenzior <denkenz@gmail.com> wrote:
> Hi Vinicius,
>
>
> On 01/30/2013 10:24 AM, Claudio Takahasi wrote:
>>
>> From: Vinicius Costa Gomes<vinicius.gomes@openbossa.org>
>>
>> When there are many devices around that support the HFP AG profile,
>> we may have a lot of modems that the user will never use.
>> ---
>>   plugins/hfp_hf_bluez5.c | 61
>> +++++++++++++++++++++++++++++++++----------------
>>   1 file changed, 41 insertions(+), 20 deletions(-)
>>
>
> Patch has been applied, however:
>
>
>> -       if (g_str_equal("Alias", name) == FALSE)
>> +       if (g_str_equal("Paired", name) == TRUE) {
>> +               modem_register_from_proxy(proxy, path);
>>                 return;
>> +       }
>
>
> This part looks suspicious in the case where Paired goes to false.  If BlueZ
> behaves in a certain way when the device is unpaired then I'd like to see a
> comment here explaining that.

It is possible to unpair a device using the Bluetooth Management
interface, but there isn't D-Bus API to unpair a device. The user
needs to call RemoveDevice(), in this case
hfp_hf_bluez5:proxy_removed() will be called and the modem will be
removed.

Even for MGMT Unpair command the device is being remove, I am not sure
if this is a feature or a bug ;-)

Regards,
Claudio.