[PATCH v0 1/4] bluetooth: Fix reading Bluetooth Adapters

[PATCH v0 1/4] bluetooth: Fix reading Bluetooth Adapters

[Claudio Takahasi]

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

This patch replaces GetProperties method by the Get method of the
object manager to read the available Bluetooth adapters.
---
 plugins/bluetooth.c | 30 ++++++++++++++++++++++--------
 plugins/bluetooth.h |  2 ++
 2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/plugins/bluetooth.c b/plugins/bluetooth.c
index 5d28530..f35c755 100644
--- a/plugins/bluetooth.c
+++ b/plugins/bluetooth.c
@@ -749,7 +749,7 @@ static gboolean device_removed(DBusConnection *conn,
 	return TRUE;
 }
 
-static void parse_adapters(DBusMessageIter *array, gpointer user_data)
+static void parse_adapters(DBusMessageIter *array)
 {
 	DBusMessageIter value;
 
@@ -776,26 +776,35 @@ static void parse_adapters(DBusMessageIter *array, gpointer user_data)
 	}
 }
 
-static void manager_properties_cb(DBusPendingCall *call, gpointer user_data)
+static void adapters_objects_cb(DBusPendingCall *call, gpointer user_data)
 {
 	DBusMessage *reply;
 	DBusError derr;
+	DBusMessageIter array, variant;
+
+	DBG("");
 
 	reply = dbus_pending_call_steal_reply(call);
 
 	dbus_error_init(&derr);
 
 	if (dbus_set_error_from_message(&derr, reply)) {
-		ofono_error("Manager.GetProperties() replied an error: %s, %s",
+		ofono_error("Get(\"%s\", \"Adapters\") replied an error: %s, "
+					"%s", FREEDESKTOP_PROPERTIES_INTERFACE,
 					derr.name, derr.message);
 		dbus_error_free(&derr);
 		goto done;
 	}
 
-	DBG("");
+	if (dbus_message_iter_init(reply, &variant) == FALSE)
+		goto done;
 
-	bluetooth_parse_properties(reply, "Adapters", parse_adapters, NULL,
-						NULL);
+	if (dbus_message_iter_get_arg_type(&variant) != DBUS_TYPE_VARIANT)
+		goto done;
+
+	dbus_message_iter_recurse(&variant, &array);
+
+	parse_adapters(&array);
 
 done:
 	dbus_message_unref(reply);
@@ -803,8 +812,13 @@ done:
 
 static void bluetooth_connect(DBusConnection *conn, void *user_data)
 {
-	bluetooth_send_with_reply("/", BLUEZ_MANAGER_INTERFACE, "GetProperties",
-				NULL, manager_properties_cb, NULL, NULL, -1,
+	const char *interface = BLUEZ_MANAGER_INTERFACE;
+	const char *property = "Adapters";
+
+	bluetooth_send_with_reply("/", FREEDESKTOP_PROPERTIES_INTERFACE, "Get",
+				NULL, adapters_objects_cb, NULL, NULL, -1,
+				DBUS_TYPE_STRING, &interface,
+				DBUS_TYPE_STRING, &property,
 				DBUS_TYPE_INVALID);
 
 	bluetooth_send_with_reply("/", BLUEZ_MANAGER_INTERFACE, "FindAdapter",
diff --git a/plugins/bluetooth.h b/plugins/bluetooth.h
index 4fc16ad..af59d3d 100644
--- a/plugins/bluetooth.h
+++ b/plugins/bluetooth.h
@@ -29,6 +29,8 @@
 #define	BLUEZ_SERVICE_INTERFACE		BLUEZ_SERVICE ".Service"
 #define BLUEZ_SERIAL_INTERFACE		BLUEZ_SERVICE ".Serial"
 
+#define	FREEDESKTOP_PROPERTIES_INTERFACE	"org.freedesktop.DBus.Properties"
+
 #define DBUS_TIMEOUT 15
 
 #define DUN_GW_UUID	"00001103-0000-1000-8000-00805f9b34fb"
-- 
1.7.11.7

[PATCH v0 2/4] bluetooth: Fix reading adapter properties

[Claudio Takahasi]

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

This patch replaces GetProperties method by the GetAll method of the
object manager to read the adapter properties.
---
 plugins/bluetooth.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/plugins/bluetooth.c b/plugins/bluetooth.c
index f35c755..1ab3214 100644
--- a/plugins/bluetooth.c
+++ b/plugins/bluetooth.c
@@ -429,7 +429,8 @@ static void adapter_properties_cb(DBusPendingCall *call, gpointer user_data)
 	dbus_error_init(&derr);
 
 	if (dbus_set_error_from_message(&derr, reply)) {
-		ofono_error("Adapter.GetProperties replied an error: %s, %s",
+		ofono_error("%s GetAll(\"%s\") replied an error: %s, %s",
+					path, FREEDESKTOP_PROPERTIES_INTERFACE,
 					derr.name, derr.message);
 		dbus_error_free(&derr);
 		goto done;
@@ -463,9 +464,15 @@ done:
 static void get_adapter_properties(const char *path, const char *handle,
 						gpointer user_data)
 {
-	bluetooth_send_with_reply(path, BLUEZ_ADAPTER_INTERFACE,
-			"GetProperties", NULL, adapter_properties_cb,
-			g_strdup(path), g_free, -1, DBUS_TYPE_INVALID);
+	const char *interface = BLUEZ_ADAPTER_INTERFACE;
+
+	DBG("Calling %s GetAll(%s)", path, interface);
+
+	bluetooth_send_with_reply(path, FREEDESKTOP_PROPERTIES_INTERFACE,
+				"GetAll", NULL, adapter_properties_cb,
+				g_strdup(path), g_free, -1,
+				DBUS_TYPE_STRING, &interface,
+				DBUS_TYPE_INVALID);
 }
 
 static void remove_record(struct server *server)
@@ -706,9 +713,7 @@ static gboolean adapter_added(DBusConnection *conn, DBusMessage *message,
 	dbus_message_get_args(message, NULL, DBUS_TYPE_OBJECT_PATH, &path,
 				DBUS_TYPE_INVALID);
 
-	bluetooth_send_with_reply(path, BLUEZ_ADAPTER_INTERFACE,
-			"GetProperties", NULL, adapter_properties_cb,
-			g_strdup(path), g_free, -1, DBUS_TYPE_INVALID);
+	get_adapter_properties(path, NULL, NULL);
 
 	return TRUE;
 }
@@ -766,11 +771,7 @@ static void parse_adapters(DBusMessageIter *array)
 
 		dbus_message_iter_get_basic(&value, &path);
 
-		DBG("Calling GetProperties on %s", path);
-
-		bluetooth_send_with_reply(path, BLUEZ_ADAPTER_INTERFACE,
-				"GetProperties", NULL, adapter_properties_cb,
-				g_strdup(path), g_free, -1, DBUS_TYPE_INVALID);
+		get_adapter_properties(path, NULL, NULL);
 
 		dbus_message_iter_next(&value);
 	}
-- 
1.7.11.7

[PATCH v0 3/4] bluetooth: Fix read device properties

[Claudio Takahasi]

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

This patch replaces GetProperties method by the GetAll method of the
object manager to read the device properties.
---
 plugins/bluetooth.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/plugins/bluetooth.c b/plugins/bluetooth.c
index 1ab3214..675e53d 100644
--- a/plugins/bluetooth.c
+++ b/plugins/bluetooth.c
@@ -299,7 +299,8 @@ static void device_properties_cb(DBusPendingCall *call, gpointer user_data)
 	dbus_error_init(&derr);
 
 	if (dbus_set_error_from_message(&derr, reply)) {
-		ofono_error("Device.GetProperties replied an error: %s, %s",
+		ofono_error("%s GetAll(\"%s\") replied an error: %s, %s",
+					path, FREEDESKTOP_PROPERTIES_INTERFACE,
 					derr.name, derr.message);
 		dbus_error_free(&derr);
 		goto done;
@@ -350,6 +351,19 @@ static void parse_devices(DBusMessageIter *array, gpointer user_data)
 	}
 }
 
+static void get_device_properties(const char *path)
+{
+	const char *interface = BLUEZ_DEVICE_INTERFACE;
+
+	DBG("Calling %s GetAll(%s)", path, interface);
+
+	bluetooth_send_with_reply(path, FREEDESKTOP_PROPERTIES_INTERFACE,
+				"GetAll", NULL, device_properties_cb,
+				g_strdup(path), g_free, -1,
+				DBUS_TYPE_STRING, &interface,
+				DBUS_TYPE_INVALID);
+}
+
 static gboolean property_changed(DBusConnection *conn, DBusMessage *msg,
 				void *user_data)
 {
@@ -382,10 +396,7 @@ static gboolean property_changed(DBusConnection *conn, DBusMessage *msg,
 		 * refetch everything again
 		 */
 		if (uuids)
-			bluetooth_send_with_reply(path, BLUEZ_DEVICE_INTERFACE,
-					"GetProperties", NULL,
-					device_properties_cb, g_strdup(path),
-					g_free, -1, DBUS_TYPE_INVALID);
+			get_device_properties(path);
 	} else if (g_str_equal(property, "Alias") == TRUE) {
 		const char *path = dbus_message_get_path(msg);
 		struct bluetooth_profile *profile;
@@ -450,10 +461,7 @@ static void adapter_properties_cb(DBusPendingCall *call, gpointer user_data)
 	for (l = device_list; l; l = l->next) {
 		const char *device = l->data;
 
-		bluetooth_send_with_reply(device, BLUEZ_DEVICE_INTERFACE,
-					"GetProperties", NULL,
-					device_properties_cb, g_strdup(device),
-					g_free, -1, DBUS_TYPE_INVALID);
+		get_device_properties(device);
 	}
 
 done:
-- 
1.7.11.7

[PATCH v0 4/4] bluetooth: Fix Device PropertyChanged

[Claudio Takahasi]

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

BlueZ PropertyChanged signal for Device objects was replaced by
PropertiesChanged emitted on org.freedesktop.DBus.Properties interface.
---
 plugins/bluetooth.c | 105 ++++++++++++++++++++++++++++++----------------------
 1 file changed, 61 insertions(+), 44 deletions(-)

diff --git a/plugins/bluetooth.c b/plugins/bluetooth.c
index 675e53d..09c9870 100644
--- a/plugins/bluetooth.c
+++ b/plugins/bluetooth.c
@@ -364,64 +364,81 @@ static void get_device_properties(const char *path)
 				DBUS_TYPE_INVALID);
 }
 
-static gboolean property_changed(DBusConnection *conn, DBusMessage *msg,
+static gboolean properties_changed(DBusConnection *conn, DBusMessage *msg,
 				void *user_data)
 {
-	const char *property;
-	DBusMessageIter iter;
+	const char *property, *interface;
+	DBusMessageIter iter, dict, entry, variant;
+
+	DBG("");
 
-	dbus_message_iter_init(msg, &iter);
+	if (dbus_message_iter_init(msg, &iter) == FALSE)
+		return FALSE;
 
+	/* Reading the interface */
 	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
+		return TRUE;
+
+	dbus_message_iter_get_basic(&iter, &interface);
+	if (g_strcmp0(BLUEZ_DEVICE_INTERFACE, interface) != 0)
+		return TRUE;
+
+	if (!dbus_message_iter_next(&iter))
 		return FALSE;
 
-	dbus_message_iter_get_basic(&iter, &property);
-	if (g_str_equal(property, "UUIDs") == TRUE) {
-		GSList *uuids = NULL;
-		const char *path = dbus_message_get_path(msg);
-		DBusMessageIter variant;
+	/* Reading Dict entries containing properties */
+	dbus_message_iter_recurse(&iter, &dict);
 
-		if (!dbus_message_iter_next(&iter))
+	do {
+		if (dbus_message_iter_get_arg_type(&dict) !=
+						DBUS_TYPE_DICT_ENTRY)
 			return FALSE;
 
-		if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
-			return FALSE;
+		dbus_message_iter_recurse(&dict, &entry);
 
-		dbus_message_iter_recurse(&iter, &variant);
-
-		parse_uuids(&variant, &uuids);
-
-		/* We need the full set of properties to be able to create
-		 * the modem properly, including Adapter and Alias, so
-		 * refetch everything again
-		 */
-		if (uuids)
-			get_device_properties(path);
-	} else if (g_str_equal(property, "Alias") == TRUE) {
-		const char *path = dbus_message_get_path(msg);
-		struct bluetooth_profile *profile;
-		const char *alias = NULL;
-		DBusMessageIter variant;
-		GHashTableIter hash_iter;
-		gpointer key, value;
-
-		if (!dbus_message_iter_next(&iter))
+		if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_STRING)
 			return FALSE;
 
-		if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
-			return FALSE;
+		dbus_message_iter_get_basic(&entry, &property);
 
-		dbus_message_iter_recurse(&iter, &variant);
+		if (!dbus_message_iter_next(&entry))
+			return FALSE;
 
-		parse_string(&variant, &alias);
+		if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_VARIANT)
+			return FALSE;
 
-		g_hash_table_iter_init(&hash_iter, uuid_hash);
-		while (g_hash_table_iter_next(&hash_iter, &key, &value)) {
-			profile = value;
-			if (profile->set_alias)
-				profile->set_alias(path, alias);
+		dbus_message_iter_recurse(&entry, &variant);
+		if (g_str_equal(property, "UUIDs") == TRUE) {
+			GSList *uuids = NULL;
+			const char *path = dbus_message_get_path(msg);
+
+			parse_uuids(&variant, &uuids);
+
+			/* We need the full set of properties to be able to
+			 * create the modem properly, including Adapter and
+			 * Alias, so refetch everything again
+			 */
+			if (uuids)
+				get_device_properties(path);
+		} else if (g_str_equal(property, "Alias") == TRUE) {
+			const char *path = dbus_message_get_path(msg);
+			struct bluetooth_profile *profile;
+			const char *alias = NULL;
+			GHashTableIter hash_iter;
+			gpointer key, value;
+
+			parse_string(&variant, &alias);
+
+			g_hash_table_iter_init(&hash_iter, uuid_hash);
+			while (g_hash_table_iter_next(&hash_iter, &key,
+								&value)) {
+				profile = value;
+				if (profile->set_alias)
+					profile->set_alias(path, alias);
+			}
 		}
-	}
+
+	} while (dbus_message_iter_next(&dict));
 
 	return TRUE;
 }
@@ -882,9 +899,9 @@ static void bluetooth_ref(void)
 
 	property_watch = g_dbus_add_signal_watch(connection,
 						BLUEZ_SERVICE, NULL,
-						BLUEZ_DEVICE_INTERFACE,
-						"PropertyChanged",
-						property_changed, NULL, NULL);
+						FREEDESKTOP_PROPERTIES_INTERFACE,
+						"PropertiesChanged",
+						properties_changed, NULL, NULL);
 
 	if (bluetooth_watch == 0 || adapter_added_watch == 0 ||
 			adapter_removed_watch == 0 || property_watch == 0) {
-- 
1.7.11.7

Re: [PATCH v0 4/4] bluetooth: Fix Device PropertyChanged

[Daniel Wagner]

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

Hi Claudio,

On 20.11.2012 13:12, Claudio Takahasi wrote:
> BlueZ PropertyChanged signal for Device objects was replaced by
> PropertiesChanged emitted on org.freedesktop.DBus.Properties interface.

Do I understand this correctly, we will not support older BlueZ after 
this change? I would expect that we support 4.x and 5.x at the same time 
for a while.

cheers,
daniel

Re: [PATCH v0 4/4] bluetooth: Fix Device PropertyChanged

[Claudio Takahasi]

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

Hi Daniel,

On Tue, Nov 20, 2012 at 9:42 AM, Daniel Wagner <wagi@monom.org> wrote:
> Hi Claudio,
>
>
> On 20.11.2012 13:12, Claudio Takahasi wrote:
>>
>> BlueZ PropertyChanged signal for Device objects was replaced by
>> PropertiesChanged emitted on org.freedesktop.DBus.Properties interface.
>
>
> Do I understand this correctly, we will not support older BlueZ after this
> change? I would expect that we support 4.x and 5.x at the same time for a
> while.
>
> cheers,
> daniel
>

The patch series that I sent is not BlueZ backward compatible. AFAIK,
there isn't a clean way to detect which BlueZ version is running.
One alternative(hackish) is to fallback if a given method call fails
and add two different signal watchers for PropertiesChanged and
PropertyChanged.

Marcel/Denis: Do you want me to add code keep oFono compatible with
older BlueZ versions?
IMO, I don't think the effort worst, it will become a mess to
implement external hfp profile on oFono. Another point is: if the plan
is to create the modem when the new connection is notified instead of
when the Bluetooth device is created/paired it may introduce problems
to keep the code compatible.

Regards,
Claudio

Re: [PATCH v0 1/4] bluetooth: Fix reading Bluetooth Adapters

[Claudio Takahasi]

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

Hi Marcel:

On Tue, Nov 20, 2012 at 9:12 AM, Claudio Takahasi
<claudio.takahasi@openbossa.org> wrote:
> This patch replaces GetProperties method by the Get method of the
> object manager to read the available Bluetooth adapters.
> ---
>  plugins/bluetooth.c | 30 ++++++++++++++++++++++--------
>  plugins/bluetooth.h |  2 ++
>  2 files changed, 24 insertions(+), 8 deletions(-)

Please ignore this patch series.

As discussed in the IRC, we will split the Bluetooth/HFP plugins to
keep the compatibility with BlueZ 4 and BlueZ 5. I will send a new
patch series as soon as possible.

Regards,
Claudio.