[PATCH BlueZ v3 00/15] gdbus: DBus.Properties + ObjectManager

[PATCH BlueZ v3 00/15] gdbus: DBus.Properties + ObjectManager

[Lucas De Marchi]

Hey,
	
here is a third version of the changes planned to gdbus. In contrary to
previous versions, this patch set includes also the Object Manager, since the
PropertiesChanged signal was made on top of it.

The first two patches are cosmetics to the other ones and somehow independent.

The last two patches are examples on how we would use the API after the conversion.

DBus.Properties.Set() is async now. We pass the message on so user can use it
to create the reply or error message.

Lucas De Marchi (10):
  gdbus: Move typedefs up
  gdbus: Define macros to add annotations
  gdbus: Add skeleton of DBus.Properties interface
  gdbus: Implement DBus.Properties.Get method
  gdbus: Implement DBus.Properties.GetAll method
  gdbus: Implement DBus.Properties.Set method
  gdbus: Add properties into Introspectable interface
  gdbus: Implement PropertiesChanged signal
  control: Use DBus.Properties
  audio: device: Use DBus.Properties

Luiz Augusto von Dentz (5):
  gdbus: Add support for org.freedesktop.DBus.ObjectManager interface
  gdbus: Group interface changes to reduce the amount of signals
    emitted
  gdbus: Only export ObjectManager interface on root path
  gdbus: Integrates ObjectManager with Properties interface
  gdbus: Simplify code for appending properties

 audio/control.c |  56 ++--
 audio/device.c  |  48 ++--
 gdbus/gdbus.h   |  66 +++--
 gdbus/object.c  | 783 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
 4 files changed, 797 insertions(+), 156 deletions(-)

-- 
1.7.11.5

[PATCH BlueZ v3 01/15] gdbus: Move typedefs up

[Lucas De Marchi]

Move the typedefs up so they can be used by functions and callbacks.
---
 gdbus/gdbus.h | 47 +++++++++++++++++++++++++++++------------------
 1 file changed, 29 insertions(+), 18 deletions(-)

diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
index 0a8a27c..34e3cb3 100644
--- a/gdbus/gdbus.h
+++ b/gdbus/gdbus.h
@@ -31,6 +31,17 @@ extern "C" {
 #include <dbus/dbus.h>
 #include <glib.h>
 
+typedef enum GDBusMethodFlags GDBusMethodFlags;
+typedef enum GDBusSignalFlags GDBusSignalFlags;
+typedef enum GDBusPropertyFlags GDBusPropertyFlags;
+typedef enum GDBusSecurityFlags GDBusSecurityFlags;
+
+typedef struct GDBusArgInfo GDBusArgInfo;
+typedef struct GDBusMethodTable GDBusMethodTable;
+typedef struct GDBusSignalTable GDBusSignalTable;
+typedef struct GDBusPropertyTable GDBusPropertyTable;
+typedef struct GDBusSecurityTable GDBusSecurityTable;
+
 typedef void (* GDBusWatchFunction) (DBusConnection *connection,
 							void *user_data);
 
@@ -62,58 +73,58 @@ typedef void (* GDBusSecurityFunction) (DBusConnection *connection,
 						gboolean interaction,
 						GDBusPendingReply pending);
 
-typedef enum {
+enum GDBusMethodFlags {
 	G_DBUS_METHOD_FLAG_DEPRECATED = (1 << 0),
 	G_DBUS_METHOD_FLAG_NOREPLY    = (1 << 1),
 	G_DBUS_METHOD_FLAG_ASYNC      = (1 << 2),
-} GDBusMethodFlags;
+};
 
-typedef enum {
+enum GDBusSignalFlags {
 	G_DBUS_SIGNAL_FLAG_DEPRECATED = (1 << 0),
-} GDBusSignalFlags;
+};
 
-typedef enum {
+enum GDBusPropertyFlags {
 	G_DBUS_PROPERTY_FLAG_DEPRECATED = (1 << 0),
-} GDBusPropertyFlags;
+};
 
-typedef enum {
+enum GDBusSecurityFlags {
 	G_DBUS_SECURITY_FLAG_DEPRECATED        = (1 << 0),
 	G_DBUS_SECURITY_FLAG_BUILTIN           = (1 << 1),
 	G_DBUS_SECURITY_FLAG_ALLOW_INTERACTION = (1 << 2),
-} GDBusSecurityFlags;
+};
 
-typedef struct {
+struct GDBusArgInfo {
 	const char *name;
 	const char *signature;
-} GDBusArgInfo;
+};
 
-typedef struct {
+struct GDBusMethodTable {
 	const char *name;
 	GDBusMethodFunction function;
 	GDBusMethodFlags flags;
 	unsigned int privilege;
 	const GDBusArgInfo *in_args;
 	const GDBusArgInfo *out_args;
-} GDBusMethodTable;
+};
 
-typedef struct {
+struct GDBusSignalTable {
 	const char *name;
 	GDBusSignalFlags flags;
 	const GDBusArgInfo *args;
-} GDBusSignalTable;
+};
 
-typedef struct {
+struct GDBusPropertyTable {
 	const char *name;
 	const char *type;
 	GDBusPropertyFlags flags;
-} GDBusPropertyTable;
+};
 
-typedef struct {
+struct GDBusSecurityTable {
 	unsigned int privilege;
 	const char *action;
 	GDBusSecurityFlags flags;
 	GDBusSecurityFunction function;
-} GDBusSecurityTable;
+};
 
 #define GDBUS_ARGS(args...) (const GDBusArgInfo[]) { args, { } }
 
-- 
1.7.11.5

[PATCH BlueZ v3 02/15] gdbus: Define macros to add annotations

[Lucas De Marchi]

Besides being more readable this way it avoids going over 80 chars.
---
 gdbus/object.c | 40 ++++++++++++++++++++++++++++------------
 1 file changed, 28 insertions(+), 12 deletions(-)

diff --git a/gdbus/object.c b/gdbus/object.c
index 9689006..24e8285 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -76,6 +76,16 @@ static void print_arguments(GString *gstr, const GDBusArgInfo *args,
 	}
 }
 
+#define G_DBUS_ANNOTATE(prefix_, name_, value_)				\
+	prefix_ "<annotation name=\"org.freedesktop.DBus." name_ "\" "	\
+	"value=\"" value_ "\"/>\n"
+
+#define G_DBUS_ANNOTATE_DEPRECATED(prefix_) \
+	G_DBUS_ANNOTATE(prefix_, "Deprecated", "true")
+
+#define G_DBUS_ANNOTATE_NOREPLY(prefix_) \
+	G_DBUS_ANNOTATE(prefix_, "Method.NoReply", "true")
+
 static void generate_interface_xml(GString *gstr, struct interface_data *iface)
 {
 	const GDBusMethodTable *method;
@@ -90,19 +100,22 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
 		if (!deprecated && !noreply &&
 				!(method->in_args && method->in_args->name) &&
 				!(method->out_args && method->out_args->name))
-			g_string_append_printf(gstr, "\t\t<method name=\"%s\"/>\n",
-								method->name);
+			g_string_append_printf(gstr,
+						"\t\t<method name=\"%s\"/>\n",
+						method->name);
 		else {
-			g_string_append_printf(gstr, "\t\t<method name=\"%s\">\n",
-								method->name);
+			g_string_append_printf(gstr,
+						"\t\t<method name=\"%s\">\n",
+						method->name);
 			print_arguments(gstr, method->in_args, "in");
 			print_arguments(gstr, method->out_args, "out");
 
 			if (deprecated)
-				g_string_append_printf(gstr, "\t\t\t<annotation name=\"org.freedesktop.DBus.Deprecated\" value=\"true\"/>\n");
-
+				g_string_append_printf(gstr,
+					G_DBUS_ANNOTATE_DEPRECATED("\t\t\t"));
 			if (noreply)
-				g_string_append_printf(gstr, "\t\t\t<annotation name=\"org.freedesktop.DBus.Method.NoReply\" value=\"true\"/>\n");
+				g_string_append_printf(gstr,
+					G_DBUS_ANNOTATE_NOREPLY("\t\t\t"));
 
 			g_string_append_printf(gstr, "\t\t</method>\n");
 		}
@@ -113,15 +126,18 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
 						G_DBUS_SIGNAL_FLAG_DEPRECATED;
 
 		if (!deprecated && !(signal->args && signal->args->name))
-			g_string_append_printf(gstr, "\t\t<signal name=\"%s\"/>\n",
-								signal->name);
+			g_string_append_printf(gstr,
+						"\t\t<signal name=\"%s\"/>\n",
+						signal->name);
 		else {
-			g_string_append_printf(gstr, "\t\t<signal name=\"%s\">\n",
-								signal->name);
+			g_string_append_printf(gstr,
+						"\t\t<signal name=\"%s\">\n",
+						signal->name);
 			print_arguments(gstr, signal->args, NULL);
 
 			if (deprecated)
-				g_string_append_printf(gstr, "\t\t\t<annotation name=\"org.freedesktop.DBus.Deprecated\" value=\"true\"/>\n");
+				g_string_append_printf(gstr,
+					G_DBUS_ANNOTATE_DEPRECATED("\t\t\t"));
 
 			g_string_append_printf(gstr, "\t\t</signal>\n");
 		}
-- 
1.7.11.5

[PATCH BlueZ v3 03/15] gdbus: Add skeleton of DBus.Properties interface

[Lucas De Marchi]

This interface is responsible for handling properties of all objects in
a given path. Right now it only registers itself, doing nothing useful.
A conversion to this new layout will be done by subsequent patches.

org.freedesktop.org.DBus.Properties spec can be found at
http://dbus.freedesktop.org/doc/dbus-specification.html#standard-interfaces-properties
---
 gdbus/object.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/gdbus/object.c b/gdbus/object.c
index 24e8285..a814e10 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -507,6 +507,48 @@ static const GDBusMethodTable introspect_methods[] = {
 	{ }
 };
 
+static DBusMessage *properties_get(DBusConnection *connection,
+					DBusMessage *message, void *user_data)
+{
+	return NULL;
+}
+
+static DBusMessage *properties_get_all(DBusConnection *connection,
+					DBusMessage *message, void *user_data)
+{
+	return NULL;
+}
+
+static DBusMessage *properties_set(DBusConnection *connection,
+					DBusMessage *message, void *user_data)
+{
+	return NULL;
+}
+
+static const GDBusMethodTable properties_methods[] = {
+	{ GDBUS_METHOD("Get",
+			GDBUS_ARGS({ "interface", "s" }, { "name", "s" }),
+			GDBUS_ARGS({ "value", "v" }),
+			properties_get) },
+	{ GDBUS_METHOD("Set", NULL,
+			GDBUS_ARGS({ "interface", "s" }, { "name", "s" },
+							{ "value", "v" }),
+			properties_set) },
+	{ GDBUS_METHOD("GetAll",
+			GDBUS_ARGS({ "interface", "s" }),
+			GDBUS_ARGS({ "properties", "a{sv}" }),
+			properties_get_all) },
+	{ }
+};
+
+static const GDBusSignalTable properties_signals[] = {
+	{ GDBUS_SIGNAL("PropertiesChanged",
+			GDBUS_ARGS({ "interface", "s" },
+					{ "changed_properties", "a{sv}" },
+					{ "invalidated_properties", "as"})) },
+	{ }
+};
+
 static void add_interface(struct generic_data *data, const char *name,
 				const GDBusMethodTable *methods,
 				const GDBusSignalTable *signals,
@@ -557,6 +599,9 @@ static struct generic_data *object_path_ref(DBusConnection *connection,
 	add_interface(data, DBUS_INTERFACE_INTROSPECTABLE,
 			introspect_methods, NULL, NULL, data, NULL);
 
+	add_interface(data, DBUS_INTERFACE_PROPERTIES, properties_methods,
+					properties_signals, NULL, data, NULL);
+
 	return data;
 }
 
@@ -596,6 +641,7 @@ static void object_path_unref(DBusConnection *connection, const char *path)
 		return;
 
 	remove_interface(data, DBUS_INTERFACE_INTROSPECTABLE);
+	remove_interface(data, DBUS_INTERFACE_PROPERTIES);
 
 	invalidate_parent_data(connection, path);
 
-- 
1.7.11.5

[PATCH BlueZ v3 04/15] gdbus: Implement DBus.Properties.Get method

[Lucas De Marchi]

---
 gdbus/gdbus.h  |  8 ++++++++
 gdbus/object.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
index 34e3cb3..b2e78c4 100644
--- a/gdbus/gdbus.h
+++ b/gdbus/gdbus.h
@@ -66,6 +66,12 @@ typedef void (* GDBusDestroyFunction) (void *user_data);
 typedef DBusMessage * (* GDBusMethodFunction) (DBusConnection *connection,
 					DBusMessage *message, void *user_data);
 
+typedef gboolean (*GDBusPropertyGetter)(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data);
+
+typedef gboolean (*GDBusPropertyExists)(const GDBusPropertyTable *property,
+								void *data);
+
 typedef guint32 GDBusPendingReply;
 
 typedef void (* GDBusSecurityFunction) (DBusConnection *connection,
@@ -116,6 +122,8 @@ struct GDBusSignalTable {
 struct GDBusPropertyTable {
 	const char *name;
 	const char *type;
+	GDBusPropertyGetter get;
+	GDBusPropertyExists exists;
 	GDBusPropertyFlags flags;
 };
 
diff --git a/gdbus/object.c b/gdbus/object.c
index a814e10..2ad32c0 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -507,10 +507,70 @@ static const GDBusMethodTable introspect_methods[] = {
 	{ }
 };
 
+static inline const GDBusPropertyTable *find_property(const GDBusPropertyTable *properties,
+							const char *name)
+{
+	const GDBusPropertyTable *p;
+
+	for (p = properties; p && p->name; p++) {
+		if (strcmp(name, p->name) == 0)
+			return p;
+	}
+
+	return NULL;
+}
+
 static DBusMessage *properties_get(DBusConnection *connection,
 					DBusMessage *message, void *user_data)
 {
-	return NULL;
+	struct generic_data *data = user_data;
+	struct interface_data *iface;
+	const GDBusPropertyTable *property;
+	const char *interface, *name;
+	DBusMessageIter iter, value;
+	DBusMessage *reply;
+
+	if (!dbus_message_get_args(message, NULL,
+					DBUS_TYPE_STRING, &interface,
+					DBUS_TYPE_STRING, &name,
+					DBUS_TYPE_INVALID))
+		return NULL;
+
+	iface = find_interface(data->interfaces, interface);
+	if (iface == NULL)
+		return g_dbus_create_error(message, DBUS_ERROR_INVALID_ARGS,
+				"No such interface '%s'", interface);
+
+	property = find_property(iface->properties, name);
+	if (property == NULL)
+		return g_dbus_create_error(message, DBUS_ERROR_INVALID_ARGS,
+				"No such property '%s'", name);
+
+	if (property->exists != NULL &&
+			!property->exists(property, iface->user_data))
+		return g_dbus_create_error(message, DBUS_ERROR_INVALID_ARGS,
+					"No such property '%s'", name);
+
+	if (property->get == NULL)
+		return g_dbus_create_error(message, DBUS_ERROR_INVALID_ARGS,
+				"Property '%s' is not readable", name);
+
+	reply = dbus_message_new_method_return(message);
+	if (reply == NULL)
+		return NULL;
+
+	dbus_message_iter_init_append(reply, &iter);
+	dbus_message_iter_open_container(&iter, DBUS_TYPE_VARIANT,
+						property->type, &value);
+
+	if (!property->get(property, &value, iface->user_data)) {
+		dbus_message_unref(reply);
+		return NULL;
+	}
+
+	dbus_message_iter_close_container(&iter, &value);
+
+	return reply;
 }
 
 static DBusMessage *properties_get_all(DBusConnection *connection,
-- 
1.7.11.5

[PATCH BlueZ v3 05/15] gdbus: Implement DBus.Properties.GetAll method

[Lucas De Marchi]

---
 gdbus/object.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 55 insertions(+), 1 deletion(-)

diff --git a/gdbus/object.c b/gdbus/object.c
index 2ad32c0..50b6dc2 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -576,7 +576,61 @@ static DBusMessage *properties_get(DBusConnection *connection,
 static DBusMessage *properties_get_all(DBusConnection *connection,
 					DBusMessage *message, void *user_data)
 {
-	return NULL;
+	struct generic_data *data = user_data;
+	struct interface_data *iface;
+	const GDBusPropertyTable *p;
+	const char *interface;
+	DBusMessageIter iter, dict;
+	DBusMessage *reply;
+
+	if (!dbus_message_get_args(message, NULL,
+					DBUS_TYPE_STRING, &interface,
+					DBUS_TYPE_INVALID))
+		return NULL;
+
+	iface = find_interface(data->interfaces, interface);
+	if (iface == NULL)
+		return g_dbus_create_error(message, DBUS_ERROR_INVALID_ARGS,
+					"No such interface '%s'", interface);
+
+	reply = dbus_message_new_method_return(message);
+	if (reply == NULL)
+		return NULL;
+
+	dbus_message_iter_init_append(reply, &iter);
+	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
+			DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
+			DBUS_TYPE_STRING_AS_STRING DBUS_TYPE_VARIANT_AS_STRING
+			DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
+
+	for (p = iface->properties; p && p->name; p++) {
+		DBusMessageIter entry, value;
+
+		if (p->get == NULL)
+			continue;
+
+		if (p->exists != NULL && !p->exists(p, iface->user_data))
+			continue;
+
+		dbus_message_iter_open_container(&dict, DBUS_TYPE_DICT_ENTRY,
+								NULL, &entry);
+		dbus_message_iter_append_basic(&entry, DBUS_TYPE_STRING,
+								p->name);
+		dbus_message_iter_open_container(&entry, DBUS_TYPE_VARIANT,
+							p->type, &value);
+
+		if (!p->get(p, &value, iface->user_data)) {
+			dbus_message_unref(reply);
+			return NULL;
+		}
+
+		dbus_message_iter_close_container(&entry, &value);
+		dbus_message_iter_close_container(&dict, &entry);
+	}
+
+	dbus_message_iter_close_container(&iter, &dict);
+
+	return reply;
 }
 
 static DBusMessage *properties_set(DBusConnection *connection,
-- 
1.7.11.5

[PATCH BlueZ v3 06/15] gdbus: Implement DBus.Properties.Set method

[Lucas De Marchi]

Contrary to Get() and GetAll(), Set is asynchronous so we pass on the
DBusMessage so user is able to create the response. It's the only use of
this parameter.
---
 gdbus/gdbus.h  |  7 +++++++
 gdbus/object.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
index b2e78c4..3e4aa16 100644
--- a/gdbus/gdbus.h
+++ b/gdbus/gdbus.h
@@ -31,6 +31,8 @@ extern "C" {
 #include <dbus/dbus.h>
 #include <glib.h>
 
+typedef enum GDBusPropertySetReturn GDBusPropertySetReturn;
+
 typedef enum GDBusMethodFlags GDBusMethodFlags;
 typedef enum GDBusSignalFlags GDBusSignalFlags;
 typedef enum GDBusPropertyFlags GDBusPropertyFlags;
@@ -69,6 +71,10 @@ typedef DBusMessage * (* GDBusMethodFunction) (DBusConnection *connection,
 typedef gboolean (*GDBusPropertyGetter)(const GDBusPropertyTable *property,
 					DBusMessageIter *iter, void *data);
 
+typedef DBusMessage *(*GDBusPropertySetter)(const GDBusPropertyTable *property,
+					DBusMessageIter *value,
+					DBusMessage *msg, void *data);
+
 typedef gboolean (*GDBusPropertyExists)(const GDBusPropertyTable *property,
 								void *data);
 
@@ -123,6 +129,7 @@ struct GDBusPropertyTable {
 	const char *name;
 	const char *type;
 	GDBusPropertyGetter get;
+	GDBusPropertySetter set;
 	GDBusPropertyExists exists;
 	GDBusPropertyFlags flags;
 };
diff --git a/gdbus/object.c b/gdbus/object.c
index 50b6dc2..22b8ebb 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -636,7 +636,62 @@ static DBusMessage *properties_get_all(DBusConnection *connection,
 static DBusMessage *properties_set(DBusConnection *connection,
 					DBusMessage *message, void *user_data)
 {
-	return NULL;
+	struct generic_data *data = user_data;
+	DBusMessageIter iter, sub;
+	struct interface_data *iface;
+	const GDBusPropertyTable *property;
+	const char *name, *interface;
+
+	if (!dbus_message_iter_init(message, &iter))
+		return g_dbus_create_error(message, DBUS_ERROR_INVALID_ARGS,
+							"No arguments given");
+
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
+		return g_dbus_create_error(message, DBUS_ERROR_INVALID_ARGS,
+					"Invalid argument type: '%c'",
+					dbus_message_iter_get_arg_type(&iter));
+
+	dbus_message_iter_get_basic(&iter, &name);
+	dbus_message_iter_next(&iter);
+
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
+		return g_dbus_create_error(message, DBUS_ERROR_INVALID_ARGS,
+					"Invalid argument type: '%c'",
+					dbus_message_iter_get_arg_type(&iter));
+
+	dbus_message_iter_get_basic(&iter, &interface);
+	dbus_message_iter_next(&iter);
+
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
+		return g_dbus_create_error(message, DBUS_ERROR_INVALID_ARGS,
+					"Invalid argument type: '%c'",
+					dbus_message_iter_get_arg_type(&iter));
+
+	dbus_message_iter_recurse(&iter, &sub);
+
+	iface = find_interface(data->interfaces, interface);
+	if (iface == NULL)
+		return g_dbus_create_error(message, DBUS_ERROR_INVALID_ARGS,
+					"No such interface '%s'", interface);
+
+	property = find_property(iface->properties, name);
+	if (property == NULL)
+		return g_dbus_create_error(message,
+						DBUS_ERROR_UNKNOWN_PROPERTY,
+						"No such property '%s'", name);
+
+	if (property->set == NULL)
+		return g_dbus_create_error(message,
+					DBUS_ERROR_PROPERTY_READ_ONLY,
+					"Property '%s' is not writable", name);
+
+	if (property->exists != NULL &&
+			!property->exists(property, iface->user_data))
+		return g_dbus_create_error(message,
+						DBUS_ERROR_UNKNOWN_PROPERTY,
+						"No such property '%s'", name);
+
+	return property->set(property, &sub, message, iface->user_data);
 }
 
 static const GDBusMethodTable properties_methods[] = {
@@ -644,7 +699,7 @@ static const GDBusMethodTable properties_methods[] = {
 			GDBUS_ARGS({ "interface", "s" }, { "name", "s" }),
 			GDBUS_ARGS({ "value", "v" }),
 			properties_get) },
-	{ GDBUS_METHOD("Set", NULL,
+	{ GDBUS_ASYNC_METHOD("Set", NULL,
 			GDBUS_ARGS({ "interface", "s" }, { "name", "s" },
 							{ "value", "v" }),
 			properties_set) },
-- 
1.7.11.5

Re: [PATCH BlueZ v3 06/15] gdbus: Implement DBus.Properties.Set method

[Marcel Holtmann]

Hi Lucas,

> Contrary to Get() and GetAll(), Set is asynchronous so we pass on the
> DBusMessage so user is able to create the response. It's the only use of
> this parameter.
> ---
>  gdbus/gdbus.h  |  7 +++++++
>  gdbus/object.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 64 insertions(+), 2 deletions(-)
> 
> diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
> index b2e78c4..3e4aa16 100644
> --- a/gdbus/gdbus.h
> +++ b/gdbus/gdbus.h
> @@ -31,6 +31,8 @@ extern "C" {
>  #include <dbus/dbus.h>
>  #include <glib.h>
>  
> +typedef enum GDBusPropertySetReturn GDBusPropertySetReturn;
> +
>  typedef enum GDBusMethodFlags GDBusMethodFlags;
>  typedef enum GDBusSignalFlags GDBusSignalFlags;
>  typedef enum GDBusPropertyFlags GDBusPropertyFlags;
> @@ -69,6 +71,10 @@ typedef DBusMessage * (* GDBusMethodFunction) (DBusConnection *connection,
>  typedef gboolean (*GDBusPropertyGetter)(const GDBusPropertyTable *property,
>  					DBusMessageIter *iter, void *data);
>  
> +typedef DBusMessage *(*GDBusPropertySetter)(const GDBusPropertyTable *property,
> +					DBusMessageIter *value,
> +					DBusMessage *msg, void *data);
> +

I am not really happy with this. We just need a unique handle here since
the return value is either success or an error. I rather don't send
messages around for no other reason to create the error.

Inside the authorization code I am using GDBusPendingReply as unique
token. What about using the same approach?

Regards

Marcel

Re: [PATCH BlueZ v3 06/15] gdbus: Implement DBus.Properties.Set method

[Luiz Augusto von Dentz]

Hi Marcel,

On Thu, Sep 6, 2012 at 11:01 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Lucas,
>
>> Contrary to Get() and GetAll(), Set is asynchronous so we pass on the
>> DBusMessage so user is able to create the response. It's the only use of
>> this parameter.
>> ---
>>  gdbus/gdbus.h  |  7 +++++++
>>  gdbus/object.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>>  2 files changed, 64 insertions(+), 2 deletions(-)
>>
>> diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
>> index b2e78c4..3e4aa16 100644
>> --- a/gdbus/gdbus.h
>> +++ b/gdbus/gdbus.h
>> @@ -31,6 +31,8 @@ extern "C" {
>>  #include <dbus/dbus.h>
>>  #include <glib.h>
>>
>> +typedef enum GDBusPropertySetReturn GDBusPropertySetReturn;
>> +
>>  typedef enum GDBusMethodFlags GDBusMethodFlags;
>>  typedef enum GDBusSignalFlags GDBusSignalFlags;
>>  typedef enum GDBusPropertyFlags GDBusPropertyFlags;
>> @@ -69,6 +71,10 @@ typedef DBusMessage * (* GDBusMethodFunction) (DBusConnection *connection,
>>  typedef gboolean (*GDBusPropertyGetter)(const GDBusPropertyTable *property,
>>                                       DBusMessageIter *iter, void *data);
>>
>> +typedef DBusMessage *(*GDBusPropertySetter)(const GDBusPropertyTable *property,
>> +                                     DBusMessageIter *value,
>> +                                     DBusMessage *msg, void *data);
>> +
>
> I am not really happy with this. We just need a unique handle here since
> the return value is either success or an error. I rather don't send
> messages around for no other reason to create the error.
>
> Inside the authorization code I am using GDBusPendingReply as unique
> token. What about using the same approach?

It would be perfect, but Im afraid sometimes the properties can have
restriction on which sender can set them, so perhaps we need to add
the name of the sender as parameter in addiction to just the handle?

-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ v3 06/15] gdbus: Implement DBus.Properties.Set method

[Marcel Holtmann]

Hi Luiz,

> >> Contrary to Get() and GetAll(), Set is asynchronous so we pass on the
> >> DBusMessage so user is able to create the response. It's the only use of
> >> this parameter.
> >> ---
> >>  gdbus/gdbus.h  |  7 +++++++
> >>  gdbus/object.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> >>  2 files changed, 64 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
> >> index b2e78c4..3e4aa16 100644
> >> --- a/gdbus/gdbus.h
> >> +++ b/gdbus/gdbus.h
> >> @@ -31,6 +31,8 @@ extern "C" {
> >>  #include <dbus/dbus.h>
> >>  #include <glib.h>
> >>
> >> +typedef enum GDBusPropertySetReturn GDBusPropertySetReturn;
> >> +
> >>  typedef enum GDBusMethodFlags GDBusMethodFlags;
> >>  typedef enum GDBusSignalFlags GDBusSignalFlags;
> >>  typedef enum GDBusPropertyFlags GDBusPropertyFlags;
> >> @@ -69,6 +71,10 @@ typedef DBusMessage * (* GDBusMethodFunction) (DBusConnection *connection,
> >>  typedef gboolean (*GDBusPropertyGetter)(const GDBusPropertyTable *property,
> >>                                       DBusMessageIter *iter, void *data);
> >>
> >> +typedef DBusMessage *(*GDBusPropertySetter)(const GDBusPropertyTable *property,
> >> +                                     DBusMessageIter *value,
> >> +                                     DBusMessage *msg, void *data);
> >> +
> >
> > I am not really happy with this. We just need a unique handle here since
> > the return value is either success or an error. I rather don't send
> > messages around for no other reason to create the error.
> >
> > Inside the authorization code I am using GDBusPendingReply as unique
> > token. What about using the same approach?
> 
> It would be perfect, but Im afraid sometimes the properties can have
> restriction on which sender can set them, so perhaps we need to add
> the name of the sender as parameter in addiction to just the handle?

that should be done through the same security handling we have for the
method calls. Don't try to invent something new.

Regards

Marcel

Re: [PATCH BlueZ v3 06/15] gdbus: Implement DBus.Properties.Set method

[Luiz Augusto von Dentz]

Hi Marcel,

On Fri, Sep 7, 2012 at 8:49 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Luiz,
>
>> >> Contrary to Get() and GetAll(), Set is asynchronous so we pass on the
>> >> DBusMessage so user is able to create the response. It's the only use of
>> >> this parameter.
>> >> ---
>> >>  gdbus/gdbus.h  |  7 +++++++
>> >>  gdbus/object.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>> >>  2 files changed, 64 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
>> >> index b2e78c4..3e4aa16 100644
>> >> --- a/gdbus/gdbus.h
>> >> +++ b/gdbus/gdbus.h
>> >> @@ -31,6 +31,8 @@ extern "C" {
>> >>  #include <dbus/dbus.h>
>> >>  #include <glib.h>
>> >>
>> >> +typedef enum GDBusPropertySetReturn GDBusPropertySetReturn;
>> >> +
>> >>  typedef enum GDBusMethodFlags GDBusMethodFlags;
>> >>  typedef enum GDBusSignalFlags GDBusSignalFlags;
>> >>  typedef enum GDBusPropertyFlags GDBusPropertyFlags;
>> >> @@ -69,6 +71,10 @@ typedef DBusMessage * (* GDBusMethodFunction) (DBusConnection *connection,
>> >>  typedef gboolean (*GDBusPropertyGetter)(const GDBusPropertyTable *property,
>> >>                                       DBusMessageIter *iter, void *data);
>> >>
>> >> +typedef DBusMessage *(*GDBusPropertySetter)(const GDBusPropertyTable *property,
>> >> +                                     DBusMessageIter *value,
>> >> +                                     DBusMessage *msg, void *data);
>> >> +
>> >
>> > I am not really happy with this. We just need a unique handle here since
>> > the return value is either success or an error. I rather don't send
>> > messages around for no other reason to create the error.
>> >
>> > Inside the authorization code I am using GDBusPendingReply as unique
>> > token. What about using the same approach?
>>
>> It would be perfect, but Im afraid sometimes the properties can have
>> restriction on which sender can set them, so perhaps we need to add
>> the name of the sender as parameter in addiction to just the handle?
>
> that should be done through the same security handling we have for the
> method calls. Don't try to invent something new.

Im afraid you will have to go in detail what you want here, the
security table seems to be meant for checking privileges in a method
level while we can probably extend it for properties and have
privileges also in the properties table, but the security table seems
to be global not per interface.


-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ v3 06/15] gdbus: Implement DBus.Properties.Set method

[Marcel Holtmann]

Hi Luiz,

> >> >> Contrary to Get() and GetAll(), Set is asynchronous so we pass on the
> >> >> DBusMessage so user is able to create the response. It's the only use of
> >> >> this parameter.
> >> >> ---
> >> >>  gdbus/gdbus.h  |  7 +++++++
> >> >>  gdbus/object.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> >> >>  2 files changed, 64 insertions(+), 2 deletions(-)
> >> >>
> >> >> diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
> >> >> index b2e78c4..3e4aa16 100644
> >> >> --- a/gdbus/gdbus.h
> >> >> +++ b/gdbus/gdbus.h
> >> >> @@ -31,6 +31,8 @@ extern "C" {
> >> >>  #include <dbus/dbus.h>
> >> >>  #include <glib.h>
> >> >>
> >> >> +typedef enum GDBusPropertySetReturn GDBusPropertySetReturn;
> >> >> +
> >> >>  typedef enum GDBusMethodFlags GDBusMethodFlags;
> >> >>  typedef enum GDBusSignalFlags GDBusSignalFlags;
> >> >>  typedef enum GDBusPropertyFlags GDBusPropertyFlags;
> >> >> @@ -69,6 +71,10 @@ typedef DBusMessage * (* GDBusMethodFunction) (DBusConnection *connection,
> >> >>  typedef gboolean (*GDBusPropertyGetter)(const GDBusPropertyTable *property,
> >> >>                                       DBusMessageIter *iter, void *data);
> >> >>
> >> >> +typedef DBusMessage *(*GDBusPropertySetter)(const GDBusPropertyTable *property,
> >> >> +                                     DBusMessageIter *value,
> >> >> +                                     DBusMessage *msg, void *data);
> >> >> +
> >> >
> >> > I am not really happy with this. We just need a unique handle here since
> >> > the return value is either success or an error. I rather don't send
> >> > messages around for no other reason to create the error.
> >> >
> >> > Inside the authorization code I am using GDBusPendingReply as unique
> >> > token. What about using the same approach?
> >>
> >> It would be perfect, but Im afraid sometimes the properties can have
> >> restriction on which sender can set them, so perhaps we need to add
> >> the name of the sender as parameter in addiction to just the handle?
> >
> > that should be done through the same security handling we have for the
> > method calls. Don't try to invent something new.
> 
> Im afraid you will have to go in detail what you want here, the
> security table seems to be meant for checking privileges in a method
> level while we can probably extend it for properties and have
> privileges also in the properties table, but the security table seems
> to be global not per interface.

you can specify unique integer identifiers as security label to each
method. We could extend that to each property. The security label is
used to pick which security callback to call.

Regards

Marcel

Re: [PATCH BlueZ v3 06/15] gdbus: Implement DBus.Properties.Set method

[Luiz Augusto von Dentz]

Hi Marcel,

On Fri, Sep 7, 2012 at 11:19 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
>> Im afraid you will have to go in detail what you want here, the
>> security table seems to be meant for checking privileges in a method
>> level while we can probably extend it for properties and have
>> privileges also in the properties table, but the security table seems
>> to be global not per interface.
>
> you can specify unique integer identifiers as security label to each
> method. We could extend that to each property. The security label is
> used to pick which security callback to call.

Fair enough, but one thing that we might have to consider is that the
label could be dynamic e.g. when a method call has a special security
policy regarding senders (common when we track senders), so perhaps
instead of having the privileges as it is now we need a callback e.g.
check_privileges which takes the sender and returns the
label/privileges needed.

-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ v3 06/15] gdbus: Implement DBus.Properties.Set method

[Lucas De Marchi]

Hi Marcel,

On Thu, Sep 6, 2012 at 5:01 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Lucas,
>
>> Contrary to Get() and GetAll(), Set is asynchronous so we pass on the
>> DBusMessage so user is able to create the response. It's the only use of
>> this parameter.
>> ---
>>  gdbus/gdbus.h  |  7 +++++++
>>  gdbus/object.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>>  2 files changed, 64 insertions(+), 2 deletions(-)
>>
>> diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
>> index b2e78c4..3e4aa16 100644
>> --- a/gdbus/gdbus.h
>> +++ b/gdbus/gdbus.h
>> @@ -31,6 +31,8 @@ extern "C" {
>>  #include <dbus/dbus.h>
>>  #include <glib.h>
>>
>> +typedef enum GDBusPropertySetReturn GDBusPropertySetReturn;
>> +
>>  typedef enum GDBusMethodFlags GDBusMethodFlags;
>>  typedef enum GDBusSignalFlags GDBusSignalFlags;
>>  typedef enum GDBusPropertyFlags GDBusPropertyFlags;
>> @@ -69,6 +71,10 @@ typedef DBusMessage * (* GDBusMethodFunction) (DBusConnection *connection,
>>  typedef gboolean (*GDBusPropertyGetter)(const GDBusPropertyTable *property,
>>                                       DBusMessageIter *iter, void *data);
>>
>> +typedef DBusMessage *(*GDBusPropertySetter)(const GDBusPropertyTable *property,
>> +                                     DBusMessageIter *value,
>> +                                     DBusMessage *msg, void *data);
>> +
>
> I am not really happy with this. We just need a unique handle here since
> the return value is either success or an error. I rather don't send
> messages around for no other reason to create the error.
>
> Inside the authorization code I am using GDBusPendingReply as unique
> token. What about using the same approach?

Sounds reasonable... I'll send an update on the next days.

Thanks
Lucas De Marchi

[PATCH BlueZ v3 07/15] gdbus: Add properties into Introspectable interface

[Lucas De Marchi]

---
 gdbus/object.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/gdbus/object.c b/gdbus/object.c
index 22b8ebb..df75116 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -90,6 +90,7 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
 {
 	const GDBusMethodTable *method;
 	const GDBusSignalTable *signal;
+	const GDBusPropertyTable *property;
 
 	for (method = iface->methods; method && method->name; method++) {
 		gboolean deprecated = method->flags &
@@ -142,6 +143,24 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
 			g_string_append_printf(gstr, "\t\t</signal>\n");
 		}
 	}
+
+	for (property = iface->properties; property && property->name;
+								property++) {
+		gboolean deprecated = property->flags &
+					G_DBUS_PROPERTY_FLAG_DEPRECATED;
+
+		g_string_append_printf(gstr, "\t\t<property name=\"%s\""
+					" type=\"%s\" access=\"%s%s\"",
+					property->name,	property->type,
+					property->get ? "read" : "",
+					property->set ? "write" : "");
+
+		if (!deprecated)
+			g_string_append_printf(gstr, "/>\n");
+		else
+			g_string_append_printf(gstr,
+				G_DBUS_ANNOTATE_DEPRECATED(">\n\t\t\t"));
+	}
 }
 
 static void generate_introspection_xml(DBusConnection *conn,
-- 
1.7.11.5

[PATCH BlueZ v3 08/15] gdbus: Add support for org.freedesktop.DBus.ObjectManager interface

[Lucas De Marchi]

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

This implements initial support for ObjectManager, it automatically adds
objects to its parents so no action is needed by daemons to get their
objects managed by this interface.

ObjectManager is part of D-Bus spec since revision 0.17:
http://dbus.freedesktop.org/doc/dbus-specification.html#standard-interfaces-objectmanager
---
 gdbus/object.c | 248 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 238 insertions(+), 10 deletions(-)

diff --git a/gdbus/object.c b/gdbus/object.c
index df75116..a14d93e 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -37,10 +37,16 @@
 #define error(fmt...)
 #define debug(fmt...)
 
+#define DBUS_INTERFACE_OBJECT_MANAGER "org.freedesktop.DBus.ObjectManager"
+
 struct generic_data {
 	unsigned int refcount;
+	DBusConnection *conn;
+	char *path;
 	GSList *interfaces;
+	GSList *objects;
 	char *introspect;
+	struct generic_data *parent;
 };
 
 struct interface_data {
@@ -399,11 +405,28 @@ static gboolean check_privilege(DBusConnection *conn, DBusMessage *msg,
 	return FALSE;
 }
 
+static void reset_parent(gpointer data, gpointer user_data)
+{
+	struct generic_data *child = data;
+	struct generic_data *parent = user_data;
+
+	child->parent = parent;
+}
+
 static void generic_unregister(DBusConnection *connection, void *user_data)
 {
 	struct generic_data *data = user_data;
+	struct generic_data *parent = data->parent;
 
+	if (parent != NULL)
+		parent->objects = g_slist_remove(parent->objects, data);
+
+	g_slist_foreach(data->objects, reset_parent, data->parent);
+	g_slist_free(data->objects);
+
+	dbus_connection_unref(data->conn);
 	g_free(data->introspect);
+	g_free(data->path);
 	g_free(data);
 }
 
@@ -485,9 +508,10 @@ static DBusObjectPathVTable generic_table = {
 	.message_function	= generic_message,
 };
 
-static void invalidate_parent_data(DBusConnection *conn, const char *child_path)
+static struct generic_data *invalidate_parent_data(DBusConnection *conn,
+						const char *child_path)
 {
-	struct generic_data *data = NULL;
+	struct generic_data *data = NULL, *child = NULL, *parent = NULL;
 	char *parent_path, *slash;
 
 	parent_path = g_strdup(child_path);
@@ -508,16 +532,30 @@ static void invalidate_parent_data(DBusConnection *conn, const char *child_path)
 		goto done;
 	}
 
-	invalidate_parent_data(conn, parent_path);
+	parent = invalidate_parent_data(conn, parent_path);
 
-	if (data == NULL)
-		goto done;
+	if (data == NULL) {
+		data = parent;
+		if (data == NULL)
+			goto done;
+	}
 
 	g_free(data->introspect);
 	data->introspect = NULL;
 
+	if (!dbus_connection_get_object_path_data(conn, child_path,
+							(void *) &child))
+		goto done;
+
+	if (child == NULL || g_slist_find(data->objects, child) != NULL)
+		goto done;
+
+	data->objects = g_slist_prepend(data->objects, child);
+	child->parent = data;
+
 done:
 	g_free(parent_path);
+	return data;
 }
 
 static const GDBusMethodTable introspect_methods[] = {
@@ -617,6 +655,7 @@ static DBusMessage *properties_get_all(DBusConnection *connection,
 		return NULL;
 
 	dbus_message_iter_init_append(reply, &iter);
+
 	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
 			DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
 			DBUS_TYPE_STRING_AS_STRING DBUS_TYPE_VARIANT_AS_STRING
@@ -737,7 +776,158 @@ static const GDBusSignalTable properties_signals[] = {
 	{ }
 };
 
-static void add_interface(struct generic_data *data, const char *name,
+static void append_properties(struct interface_data *data,
+							DBusMessageIter *iter)
+{
+	DBusMessageIter dict;
+
+	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
+				DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
+				DBUS_TYPE_STRING_AS_STRING
+				DBUS_TYPE_VARIANT_AS_STRING
+				DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
+
+	/* TODO: list properties */
+
+	dbus_message_iter_close_container(iter, &dict);
+}
+
+static void append_interface(gpointer data, gpointer user_data)
+{
+	struct interface_data *iface = data;
+	DBusMessageIter *array = user_data;
+	DBusMessageIter entry;
+
+	dbus_message_iter_open_container(array, DBUS_TYPE_DICT_ENTRY, NULL,
+								&entry);
+	dbus_message_iter_append_basic(&entry, DBUS_TYPE_STRING, &iface->name);
+	append_properties(data, &entry);
+	dbus_message_iter_close_container(array, &entry);
+}
+
+static void append_interfaces(struct generic_data *data, DBusMessageIter *iter)
+{
+	DBusMessageIter array;
+
+	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
+				DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
+				DBUS_TYPE_STRING_AS_STRING
+				DBUS_TYPE_ARRAY_AS_STRING
+				DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
+				DBUS_TYPE_STRING_AS_STRING
+				DBUS_TYPE_VARIANT_AS_STRING
+				DBUS_DICT_ENTRY_END_CHAR_AS_STRING
+				DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &array);
+
+	g_slist_foreach(data->interfaces, append_interface, &array);
+
+	dbus_message_iter_close_container(iter, &array);
+}
+
+static void append_object(gpointer data, gpointer user_data)
+{
+	struct generic_data *child = data;
+	DBusMessageIter *array = user_data;
+	DBusMessageIter entry;
+
+	dbus_message_iter_open_container(array, DBUS_TYPE_DICT_ENTRY, NULL,
+								&entry);
+	dbus_message_iter_append_basic(&entry, DBUS_TYPE_OBJECT_PATH,
+								&child->path);
+	append_interfaces(child, &entry);
+	dbus_message_iter_close_container(array, &entry);
+}
+
+static DBusMessage *get_objects(DBusConnection *connection,
+				DBusMessage *message, void *user_data)
+{
+	struct generic_data *data = user_data;
+	DBusMessage *reply;
+	DBusMessageIter iter;
+	DBusMessageIter array;
+
+	reply = dbus_message_new_method_return(message);
+	if (reply == NULL)
+		return NULL;
+
+	dbus_message_iter_init_append(reply, &iter);
+
+	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
+					DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
+					DBUS_TYPE_OBJECT_PATH_AS_STRING
+					DBUS_TYPE_ARRAY_AS_STRING
+					DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
+					DBUS_TYPE_STRING_AS_STRING
+					DBUS_TYPE_ARRAY_AS_STRING
+					DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
+					DBUS_TYPE_STRING_AS_STRING
+					DBUS_TYPE_VARIANT_AS_STRING
+					DBUS_DICT_ENTRY_END_CHAR_AS_STRING
+					DBUS_DICT_ENTRY_END_CHAR_AS_STRING
+					DBUS_DICT_ENTRY_END_CHAR_AS_STRING,
+					&array);
+
+	g_slist_foreach(data->objects, append_object, &array);
+
+	dbus_message_iter_close_container(&iter, &array);
+
+	return reply;
+}
+
+static const GDBusMethodTable manager_methods[] = {
+	{ GDBUS_METHOD("GetManagedObjects", NULL,
+		GDBUS_ARGS({ "objects", "a{oa{sa{sv}}}" }), get_objects) },
+	{ }
+};
+
+static const GDBusSignalTable manager_signals[] = {
+	{ GDBUS_SIGNAL("InterfacesAdded",
+		GDBUS_ARGS({ "object", "o" },
+				{ "interfaces", "a{sa{sv}}" })) },
+	{ GDBUS_SIGNAL("InterfacesRemoved",
+		GDBUS_ARGS({ "object", "o" }, { "interfaces", "as" })) },
+	{ }
+};
+
+static void emit_interface_added(struct generic_data *data,
+						struct interface_data *iface)
+{
+	DBusMessage *signal;
+	DBusMessageIter iter, array;
+	struct generic_data *parent = data->parent;
+
+	if (parent == NULL)
+		return;
+
+	signal = dbus_message_new_signal(parent->path,
+					DBUS_INTERFACE_OBJECT_MANAGER,
+					"InterfacesAdded");
+	if (signal == NULL)
+		return;
+
+	dbus_message_iter_init_append(signal, &iter);
+	dbus_message_iter_append_basic(&iter, DBUS_TYPE_OBJECT_PATH,
+								&data->path);
+
+	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
+				DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
+				DBUS_TYPE_STRING_AS_STRING
+				DBUS_TYPE_ARRAY_AS_STRING
+				DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
+				DBUS_TYPE_STRING_AS_STRING
+				DBUS_TYPE_VARIANT_AS_STRING
+				DBUS_DICT_ENTRY_END_CHAR_AS_STRING
+				DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &array);
+
+	append_interface(iface, &array);
+
+	dbus_message_iter_close_container(&iter, &array);
+
+	g_dbus_send_message(data->conn, signal);
+}
+
+static void add_interface(struct generic_data *data,
+				const char *name,
 				const GDBusMethodTable *methods,
 				const GDBusSignalTable *signals,
 				const GDBusPropertyTable *properties,
@@ -755,6 +945,8 @@ static void add_interface(struct generic_data *data, const char *name,
 	iface->destroy = destroy;
 
 	data->interfaces = g_slist_append(data->interfaces, iface);
+
+	emit_interface_added(data, iface);
 }
 
 static struct generic_data *object_path_ref(DBusConnection *connection,
@@ -771,6 +963,8 @@ static struct generic_data *object_path_ref(DBusConnection *connection,
 	}
 
 	data = g_new0(struct generic_data, 1);
+	data->conn = dbus_connection_ref(connection);
+	data->path = g_strdup(path);
 	data->refcount = 1;
 
 	data->introspect = g_strdup(DBUS_INTROSPECT_1_0_XML_DOCTYPE_DECL_NODE "<node></node>");
@@ -784,8 +978,11 @@ static struct generic_data *object_path_ref(DBusConnection *connection,
 
 	invalidate_parent_data(connection, path);
 
-	add_interface(data, DBUS_INTERFACE_INTROSPECTABLE,
-			introspect_methods, NULL, NULL, data, NULL);
+	add_interface(data, DBUS_INTERFACE_INTROSPECTABLE, introspect_methods,
+						NULL, NULL, data, NULL);
+
+	add_interface(data, DBUS_INTERFACE_OBJECT_MANAGER, manager_methods,
+					manager_signals, NULL, data, NULL);
 
 	add_interface(data, DBUS_INTERFACE_PROPERTIES, properties_methods,
 					properties_signals, NULL, data, NULL);
@@ -793,6 +990,34 @@ static struct generic_data *object_path_ref(DBusConnection *connection,
 	return data;
 }
 
+static void emit_interface_remove(struct generic_data *data,
+						struct interface_data *iface)
+{
+	DBusMessage *signal;
+	DBusMessageIter iter, array;
+	struct generic_data *parent = data->parent;
+
+	if (parent == NULL)
+		return;
+
+	signal = dbus_message_new_signal(parent->path,
+					DBUS_INTERFACE_OBJECT_MANAGER,
+					"InterfacesRemoved");
+	if (signal == NULL)
+		return;
+
+	dbus_message_iter_init_append(signal, &iter);
+	dbus_message_iter_append_basic(&iter, DBUS_TYPE_OBJECT_PATH,
+								&data->path);
+
+	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
+					DBUS_TYPE_STRING_AS_STRING, &array);
+	dbus_message_iter_append_basic(&array, DBUS_TYPE_STRING, &iface->name);
+	dbus_message_iter_close_container(&iter, &array);
+
+	g_dbus_send_message(data->conn, signal);
+}
+
 static gboolean remove_interface(struct generic_data *data, const char *name)
 {
 	struct interface_data *iface;
@@ -801,6 +1026,8 @@ static gboolean remove_interface(struct generic_data *data, const char *name)
 	if (iface == NULL)
 		return FALSE;
 
+	emit_interface_remove(data, iface);
+
 	data->interfaces = g_slist_remove(data->interfaces, iface);
 
 	if (iface->destroy)
@@ -830,6 +1057,7 @@ static void object_path_unref(DBusConnection *connection, const char *path)
 
 	remove_interface(data, DBUS_INTERFACE_INTROSPECTABLE);
 	remove_interface(data, DBUS_INTERFACE_PROPERTIES);
+	remove_interface(data, DBUS_INTERFACE_OBJECT_MANAGER);
 
 	invalidate_parent_data(connection, path);
 
@@ -928,8 +1156,8 @@ gboolean g_dbus_register_interface(DBusConnection *connection,
 		return FALSE;
 	}
 
-	add_interface(data, name, methods, signals,
-			properties, user_data, destroy);
+	add_interface(data, name, methods, signals, properties, user_data,
+								destroy);
 
 	g_free(data->introspect);
 	data->introspect = NULL;
-- 
1.7.11.5

Re: [PATCH BlueZ v3 08/15] gdbus: Add support for org.freedesktop.DBus.ObjectManager interface

[Vinicius Costa Gomes]

Hi Luiz,

On 03:50 Sat 18 Aug, Lucas De Marchi wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> 
> This implements initial support for ObjectManager, it automatically adds
> objects to its parents so no action is needed by daemons to get their
> objects managed by this interface.
> 
> ObjectManager is part of D-Bus spec since revision 0.17:
> http://dbus.freedesktop.org/doc/dbus-specification.html#standard-interfaces-objectmanager
> ---
>  gdbus/object.c | 248 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 238 insertions(+), 10 deletions(-)
> 

[snip]

>  
>  static const GDBusMethodTable introspect_methods[] = {
> @@ -617,6 +655,7 @@ static DBusMessage *properties_get_all(DBusConnection *connection,
>  		return NULL;
>  
>  	dbus_message_iter_init_append(reply, &iter);
> +

The code looks a little better without this extra line.

>  	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
>  			DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
>  			DBUS_TYPE_STRING_AS_STRING DBUS_TYPE_VARIANT_AS_STRING
> @@ -737,7 +776,158 @@ static const GDBusSignalTable properties_signals[] = {
>  	{ }
>  };
>  

Cheers,
-- 
Vinicius

Re: [PATCH BlueZ v3 08/15] gdbus: Add support for org.freedesktop.DBus.ObjectManager interface

[Luiz Augusto von Dentz]

Hi Vinicius,

On Sat, Aug 18, 2012 at 9:25 PM, Vinicius Costa Gomes
<vinicius.gomes@openbossa.org> wrote:

>>  static const GDBusMethodTable introspect_methods[] = {
>> @@ -617,6 +655,7 @@ static DBusMessage *properties_get_all(DBusConnection *connection,
>>               return NULL;
>>
>>       dbus_message_iter_init_append(reply, &iter);
>> +
>
> The code looks a little better without this extra line.

It is now fixed in my repository:
https://gitorious.org/~vudentz/bluez/vudentzs-clone/commit/266200c548467c8eac785b3eba91afcd2bc244f9?format=patch

@Lucas: Please rebase

-- 
Luiz Augusto von Dentz

[PATCH BlueZ v3 09/15] gdbus: Group interface changes to reduce the amount of signals emitted

[Lucas De Marchi]

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

InterfacesAdded and InterfacesRemoved can group all the interfaces
changes together in one message.
---
 gdbus/object.c | 363 +++++++++++++++++++++++++++++++++------------------------
 1 file changed, 209 insertions(+), 154 deletions(-)

diff --git a/gdbus/object.c b/gdbus/object.c
index a14d93e..11c317f 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -45,6 +45,9 @@ struct generic_data {
 	char *path;
 	GSList *interfaces;
 	GSList *objects;
+	GSList *added;
+	GSList *removed;
+	guint process_id;
 	char *introspect;
 	struct generic_data *parent;
 };
@@ -65,6 +68,8 @@ struct security_data {
 	void *iface_user_data;
 };
 
+static gboolean process_changes(gpointer user_data);
+
 static void print_arguments(GString *gstr, const GDBusArgInfo *args,
 						const char *direction)
 {
@@ -413,21 +418,71 @@ static void reset_parent(gpointer data, gpointer user_data)
 	child->parent = parent;
 }
 
-static void generic_unregister(DBusConnection *connection, void *user_data)
+static void append_properties(struct interface_data *data,
+							DBusMessageIter *iter)
 {
-	struct generic_data *data = user_data;
+	DBusMessageIter dict;
+
+	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
+				DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
+				DBUS_TYPE_STRING_AS_STRING
+				DBUS_TYPE_VARIANT_AS_STRING
+				DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
+
+	/* TODO: list properties */
+
+	dbus_message_iter_close_container(iter, &dict);
+}
+
+static void append_interface(gpointer data, gpointer user_data)
+{
+	struct interface_data *iface = data;
+	DBusMessageIter *array = user_data;
+	DBusMessageIter entry;
+
+	dbus_message_iter_open_container(array, DBUS_TYPE_DICT_ENTRY, NULL,
+								&entry);
+	dbus_message_iter_append_basic(&entry, DBUS_TYPE_STRING, &iface->name);
+	append_properties(data, &entry);
+	dbus_message_iter_close_container(array, &entry);
+}
+
+static void emit_interfaces_added(struct generic_data *data)
+{
+	DBusMessage *signal;
+	DBusMessageIter iter, array;
 	struct generic_data *parent = data->parent;
 
-	if (parent != NULL)
-		parent->objects = g_slist_remove(parent->objects, data);
+	if (parent == NULL)
+		return;
 
-	g_slist_foreach(data->objects, reset_parent, data->parent);
-	g_slist_free(data->objects);
+	signal = dbus_message_new_signal(parent->path,
+					DBUS_INTERFACE_OBJECT_MANAGER,
+					"InterfacesAdded");
+	if (signal == NULL)
+		return;
 
-	dbus_connection_unref(data->conn);
-	g_free(data->introspect);
-	g_free(data->path);
-	g_free(data);
+	dbus_message_iter_init_append(signal, &iter);
+	dbus_message_iter_append_basic(&iter, DBUS_TYPE_OBJECT_PATH,
+								&data->path);
+
+	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
+				DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
+				DBUS_TYPE_STRING_AS_STRING
+				DBUS_TYPE_ARRAY_AS_STRING
+				DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
+				DBUS_TYPE_STRING_AS_STRING
+				DBUS_TYPE_VARIANT_AS_STRING
+				DBUS_DICT_ENTRY_END_CHAR_AS_STRING
+				DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &array);
+
+	g_slist_foreach(data->added, append_interface, &array);
+	g_slist_free(data->added);
+	data->added = NULL;
+
+	dbus_message_iter_close_container(&iter, &array);
+
+	g_dbus_send_message(data->conn, signal);
 }
 
 static struct interface_data *find_interface(GSList *interfaces,
@@ -440,6 +495,7 @@ static struct interface_data *find_interface(GSList *interfaces,
 
 	for (list = interfaces; list; list = list->next) {
 		struct interface_data *iface = list->data;
+
 		if (!strcmp(name, iface->name))
 			return iface;
 	}
@@ -468,45 +524,37 @@ static gboolean g_dbus_args_have_signature(const GDBusArgInfo *args,
 	return TRUE;
 }
 
-static DBusHandlerResult generic_message(DBusConnection *connection,
-					DBusMessage *message, void *user_data)
+static gboolean remove_interface(struct generic_data *data, const char *name)
 {
-	struct generic_data *data = user_data;
 	struct interface_data *iface;
-	const GDBusMethodTable *method;
-	const char *interface;
 
-	interface = dbus_message_get_interface(message);
-
-	iface = find_interface(data->interfaces, interface);
+	iface = find_interface(data->interfaces, name);
 	if (iface == NULL)
-		return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
-
-	for (method = iface->methods; method &&
-			method->name && method->function; method++) {
-		if (dbus_message_is_method_call(message, iface->name,
-							method->name) == FALSE)
-			continue;
+		return FALSE;
 
-		if (g_dbus_args_have_signature(method->in_args,
-							message) == FALSE)
-			continue;
+	data->interfaces = g_slist_remove(data->interfaces, iface);
 
-		if (check_privilege(connection, message, method,
-						iface->user_data) == TRUE)
-			return DBUS_HANDLER_RESULT_HANDLED;
+	if (iface->destroy) {
+		iface->destroy(iface->user_data);
+		iface->user_data = NULL;
+	}
 
-		return process_message(connection, message, method,
-							iface->user_data);
+	if (data->parent == NULL) {
+		g_free(iface->name);
+		g_free(iface);
+		return TRUE;
 	}
 
-	return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
-}
+	data->removed = g_slist_prepend(data->removed, iface->name);
+	g_free(iface);
 
-static DBusObjectPathVTable generic_table = {
-	.unregister_function	= generic_unregister,
-	.message_function	= generic_message,
-};
+	if (data->process_id > 0)
+		return TRUE;
+
+	data->process_id = g_idle_add(process_changes, data);
+
+	return TRUE;
+}
 
 static struct generic_data *invalidate_parent_data(DBusConnection *conn,
 						const char *child_path)
@@ -558,12 +606,6 @@ done:
 	return data;
 }
 
-static const GDBusMethodTable introspect_methods[] = {
-	{ GDBUS_METHOD("Introspect", NULL,
-			GDBUS_ARGS({ "xml", "s" }), introspect) },
-	{ }
-};
-
 static inline const GDBusPropertyTable *find_property(const GDBusPropertyTable *properties,
 							const char *name)
 {
@@ -776,35 +818,128 @@ static const GDBusSignalTable properties_signals[] = {
 	{ }
 };
 
-static void append_properties(struct interface_data *data,
-							DBusMessageIter *iter)
+static void append_name(gpointer data, gpointer user_data)
 {
-	DBusMessageIter dict;
+	char *name = data;
+	DBusMessageIter *iter = user_data;
 
-	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
-				DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
-				DBUS_TYPE_STRING_AS_STRING
-				DBUS_TYPE_VARIANT_AS_STRING
-				DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &name);
+}
 
-	/* TODO: list properties */
+static void emit_interfaces_removed(struct generic_data *data)
+{
+	DBusMessage *signal;
+	DBusMessageIter iter, array;
+	struct generic_data *parent = data->parent;
 
-	dbus_message_iter_close_container(iter, &dict);
+	if (parent == NULL)
+		return;
+
+	signal = dbus_message_new_signal(parent->path,
+					DBUS_INTERFACE_OBJECT_MANAGER,
+					"InterfacesRemoved");
+	if (signal == NULL)
+		return;
+
+	dbus_message_iter_init_append(signal, &iter);
+	dbus_message_iter_append_basic(&iter, DBUS_TYPE_OBJECT_PATH,
+								&data->path);
+	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
+					DBUS_TYPE_STRING_AS_STRING, &array);
+
+	g_slist_foreach(data->removed, append_name, &array);
+	g_slist_free_full(data->removed, g_free);
+	data->removed = NULL;
+
+	dbus_message_iter_close_container(&iter, &array);
+
+	g_dbus_send_message(data->conn, signal);
 }
 
-static void append_interface(gpointer data, gpointer user_data)
+static gboolean process_changes(gpointer user_data)
 {
-	struct interface_data *iface = data;
-	DBusMessageIter *array = user_data;
-	DBusMessageIter entry;
+	struct generic_data *data = user_data;
 
-	dbus_message_iter_open_container(array, DBUS_TYPE_DICT_ENTRY, NULL,
-								&entry);
-	dbus_message_iter_append_basic(&entry, DBUS_TYPE_STRING, &iface->name);
-	append_properties(data, &entry);
-	dbus_message_iter_close_container(array, &entry);
+	data->process_id = 0;
+
+	if (data->added != NULL)
+		emit_interfaces_added(data);
+
+	if (data->removed != NULL)
+		emit_interfaces_removed(data);
+
+	return FALSE;
+}
+
+static void generic_unregister(DBusConnection *connection, void *user_data)
+{
+	struct generic_data *data = user_data;
+	struct generic_data *parent = data->parent;
+
+	if (parent != NULL)
+		parent->objects = g_slist_remove(parent->objects, data);
+
+	if (data->process_id > 0) {
+		g_source_remove(data->process_id);
+		if (data->removed != NULL)
+			emit_interfaces_removed(data);
+	}
+
+	g_slist_foreach(data->objects, reset_parent, data->parent);
+	g_slist_free(data->objects);
+
+	dbus_connection_unref(data->conn);
+	g_free(data->introspect);
+	g_free(data->path);
+	g_free(data);
+}
+
+static DBusHandlerResult generic_message(DBusConnection *connection,
+					DBusMessage *message, void *user_data)
+{
+	struct generic_data *data = user_data;
+	struct interface_data *iface;
+	const GDBusMethodTable *method;
+	const char *interface;
+
+	interface = dbus_message_get_interface(message);
+
+	iface = find_interface(data->interfaces, interface);
+	if (iface == NULL)
+		return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+
+	for (method = iface->methods; method &&
+			method->name && method->function; method++) {
+		if (dbus_message_is_method_call(message, iface->name,
+							method->name) == FALSE)
+			continue;
+
+		if (g_dbus_args_have_signature(method->in_args,
+							message) == FALSE)
+			continue;
+
+		if (check_privilege(connection, message, method,
+						iface->user_data) == TRUE)
+			return DBUS_HANDLER_RESULT_HANDLED;
+
+		return process_message(connection, message, method,
+							iface->user_data);
+	}
+
+	return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 }
 
+static DBusObjectPathVTable generic_table = {
+	.unregister_function	= generic_unregister,
+	.message_function	= generic_message,
+};
+
+static const GDBusMethodTable introspect_methods[] = {
+	{ GDBUS_METHOD("Introspect", NULL,
+			GDBUS_ARGS({ "xml", "s" }), introspect) },
+	{ }
+};
+
 static void append_interfaces(struct generic_data *data, DBusMessageIter *iter)
 {
 	DBusMessageIter array;
@@ -889,43 +1024,6 @@ static const GDBusSignalTable manager_signals[] = {
 	{ }
 };
 
-static void emit_interface_added(struct generic_data *data,
-						struct interface_data *iface)
-{
-	DBusMessage *signal;
-	DBusMessageIter iter, array;
-	struct generic_data *parent = data->parent;
-
-	if (parent == NULL)
-		return;
-
-	signal = dbus_message_new_signal(parent->path,
-					DBUS_INTERFACE_OBJECT_MANAGER,
-					"InterfacesAdded");
-	if (signal == NULL)
-		return;
-
-	dbus_message_iter_init_append(signal, &iter);
-	dbus_message_iter_append_basic(&iter, DBUS_TYPE_OBJECT_PATH,
-								&data->path);
-
-	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
-				DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
-				DBUS_TYPE_STRING_AS_STRING
-				DBUS_TYPE_ARRAY_AS_STRING
-				DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
-				DBUS_TYPE_STRING_AS_STRING
-				DBUS_TYPE_VARIANT_AS_STRING
-				DBUS_DICT_ENTRY_END_CHAR_AS_STRING
-				DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &array);
-
-	append_interface(iface, &array);
-
-	dbus_message_iter_close_container(&iter, &array);
-
-	g_dbus_send_message(data->conn, signal);
-}
-
 static void add_interface(struct generic_data *data,
 				const char *name,
 				const GDBusMethodTable *methods,
@@ -945,8 +1043,14 @@ static void add_interface(struct generic_data *data,
 	iface->destroy = destroy;
 
 	data->interfaces = g_slist_append(data->interfaces, iface);
+	if (data->parent == NULL)
+		return;
+
+	data->added = g_slist_append(data->added, iface);
+	if (data->process_id > 0)
+		return;
 
-	emit_interface_added(data, iface);
+	data->process_id = g_idle_add(process_changes, data);
 }
 
 static struct generic_data *object_path_ref(DBusConnection *connection,
@@ -990,55 +1094,6 @@ static struct generic_data *object_path_ref(DBusConnection *connection,
 	return data;
 }
 
-static void emit_interface_remove(struct generic_data *data,
-						struct interface_data *iface)
-{
-	DBusMessage *signal;
-	DBusMessageIter iter, array;
-	struct generic_data *parent = data->parent;
-
-	if (parent == NULL)
-		return;
-
-	signal = dbus_message_new_signal(parent->path,
-					DBUS_INTERFACE_OBJECT_MANAGER,
-					"InterfacesRemoved");
-	if (signal == NULL)
-		return;
-
-	dbus_message_iter_init_append(signal, &iter);
-	dbus_message_iter_append_basic(&iter, DBUS_TYPE_OBJECT_PATH,
-								&data->path);
-
-	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
-					DBUS_TYPE_STRING_AS_STRING, &array);
-	dbus_message_iter_append_basic(&array, DBUS_TYPE_STRING, &iface->name);
-	dbus_message_iter_close_container(&iter, &array);
-
-	g_dbus_send_message(data->conn, signal);
-}
-
-static gboolean remove_interface(struct generic_data *data, const char *name)
-{
-	struct interface_data *iface;
-
-	iface = find_interface(data->interfaces, name);
-	if (iface == NULL)
-		return FALSE;
-
-	emit_interface_remove(data, iface);
-
-	data->interfaces = g_slist_remove(data->interfaces, iface);
-
-	if (iface->destroy)
-		iface->destroy(iface->user_data);
-
-	g_free(iface->name);
-	g_free(iface);
-
-	return TRUE;
-}
-
 static void object_path_unref(DBusConnection *connection, const char *path)
 {
 	struct generic_data *data = NULL;
@@ -1059,9 +1114,9 @@ static void object_path_unref(DBusConnection *connection, const char *path)
 	remove_interface(data, DBUS_INTERFACE_PROPERTIES);
 	remove_interface(data, DBUS_INTERFACE_OBJECT_MANAGER);
 
-	invalidate_parent_data(connection, path);
+	invalidate_parent_data(data->conn, data->path);
 
-	dbus_connection_unregister_object_path(connection, path);
+	dbus_connection_unregister_object_path(data->conn, data->path);
 }
 
 static gboolean check_signal(DBusConnection *conn, const char *path,
@@ -1186,7 +1241,7 @@ gboolean g_dbus_unregister_interface(DBusConnection *connection,
 	g_free(data->introspect);
 	data->introspect = NULL;
 
-	object_path_unref(connection, path);
+	object_path_unref(connection, data->path);
 
 	return TRUE;
 }
-- 
1.7.11.5

Re: [PATCH BlueZ v3 09/15] gdbus: Group interface changes to reduce the amount of signals emitted

[Vinicius Costa Gomes]

Hi Luiz,

On 03:50 Sat 18 Aug, Lucas De Marchi wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> 
> InterfacesAdded and InterfacesRemoved can group all the interfaces
> changes together in one message.
> ---
>  gdbus/object.c | 363 +++++++++++++++++++++++++++++++++------------------------
>  1 file changed, 209 insertions(+), 154 deletions(-)
> 
>  

[snip]

>  static struct interface_data *find_interface(GSList *interfaces,
> @@ -440,6 +495,7 @@ static struct interface_data *find_interface(GSList *interfaces,
>  
>  	for (list = interfaces; list; list = list->next) {
>  		struct interface_data *iface = list->data;
> +

Seems I have a special talent for spotting these things (on others patches
;-)) 

>  		if (!strcmp(name, iface->name))
>  			return iface;
>  	}
> @@ -468,45 +524,37 @@ static gboolean g_dbus_args_have_signature(const GDBusArgInfo *args,
>  	return TRUE;
>  }


Cheers,
-- 
Vinicius

Re: [PATCH BlueZ v3 09/15] gdbus: Group interface changes to reduce the amount of signals emitted

[Luiz Augusto von Dentz]

Hi Vinicius,

On Sat, Aug 18, 2012 at 9:28 PM, Vinicius Costa Gomes
<vinicius.gomes@openbossa.org> wrote:
>>  static struct interface_data *find_interface(GSList *interfaces,
>> @@ -440,6 +495,7 @@ static struct interface_data *find_interface(GSList *interfaces,
>>
>>       for (list = interfaces; list; list = list->next) {
>>               struct interface_data *iface = list->data;
>> +
>
> Seems I have a special talent for spotting these things (on others patches
> ;-))
>

Also fixed now, thanks

-- 
Luiz Augusto von Dentz

[PATCH BlueZ v3 10/15] gdbus: Only export ObjectManager interface on root path

[Lucas De Marchi]

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

ObjectManager should be exported only in the root path and list all
the children paths.
---
 gdbus/object.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/gdbus/object.c b/gdbus/object.c
index 11c317f..69bad68 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -456,6 +456,10 @@ static void emit_interfaces_added(struct generic_data *data)
 	if (parent == NULL)
 		return;
 
+	/* Find root data */
+	while (parent->parent)
+		parent = parent->parent;
+
 	signal = dbus_message_new_signal(parent->path,
 					DBUS_INTERFACE_OBJECT_MANAGER,
 					"InterfacesAdded");
@@ -835,6 +839,10 @@ static void emit_interfaces_removed(struct generic_data *data)
 	if (parent == NULL)
 		return;
 
+	/* Find root data */
+	while (parent->parent)
+		parent = parent->parent;
+
 	signal = dbus_message_new_signal(parent->path,
 					DBUS_INTERFACE_OBJECT_MANAGER,
 					"InterfacesRemoved");
@@ -971,6 +979,8 @@ static void append_object(gpointer data, gpointer user_data)
 								&child->path);
 	append_interfaces(child, &entry);
 	dbus_message_iter_close_container(array, &entry);
+
+	g_slist_foreach(child->objects, append_object, user_data);
 }
 
 static DBusMessage *get_objects(DBusConnection *connection,
@@ -1085,8 +1095,11 @@ static struct generic_data *object_path_ref(DBusConnection *connection,
 	add_interface(data, DBUS_INTERFACE_INTROSPECTABLE, introspect_methods,
 						NULL, NULL, data, NULL);
 
-	add_interface(data, DBUS_INTERFACE_OBJECT_MANAGER, manager_methods,
-					manager_signals, NULL, data, NULL);
+	/* Only root path export ObjectManager interface */
+	if (data->parent == NULL)
+		add_interface(data, DBUS_INTERFACE_OBJECT_MANAGER,
+					manager_methods, manager_signals,
+					NULL, data, NULL);
 
 	add_interface(data, DBUS_INTERFACE_PROPERTIES, properties_methods,
 					properties_signals, NULL, data, NULL);
-- 
1.7.11.5

[PATCH BlueZ v3 11/15] gdbus: Integrates ObjectManager with Properties interface

[Lucas De Marchi]

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

This appends the properties and its values when using ObjectManager.
---
 gdbus/object.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/gdbus/object.c b/gdbus/object.c
index 69bad68..b9c5db0 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -418,10 +418,28 @@ static void reset_parent(gpointer data, gpointer user_data)
 	child->parent = parent;
 }
 
+static void append_property(struct interface_data *iface,
+			const GDBusPropertyTable *p, DBusMessageIter *dict)
+{
+	DBusMessageIter entry, value;
+
+	dbus_message_iter_open_container(dict, DBUS_TYPE_DICT_ENTRY, NULL,
+								&entry);
+	dbus_message_iter_append_basic(&entry, DBUS_TYPE_STRING, &p->name);
+	dbus_message_iter_open_container(&entry, DBUS_TYPE_VARIANT, p->type,
+								&value);
+
+	p->get(p, &value, iface->user_data);
+
+	dbus_message_iter_close_container(&entry, &value);
+	dbus_message_iter_close_container(dict, &entry);
+}
+
 static void append_properties(struct interface_data *data,
 							DBusMessageIter *iter)
 {
 	DBusMessageIter dict;
+	const GDBusPropertyTable *p;
 
 	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
 				DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
@@ -429,7 +447,15 @@ static void append_properties(struct interface_data *data,
 				DBUS_TYPE_VARIANT_AS_STRING
 				DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
 
-	/* TODO: list properties */
+	for (p = data->properties; p && p->name; p++) {
+		if (p->get == NULL)
+			continue;
+
+		if (p->exists != NULL && !p->exists(p, data->user_data))
+			continue;
+
+		append_property(data, p, &dict);
+	}
 
 	dbus_message_iter_close_container(iter, &dict);
 }
-- 
1.7.11.5

[PATCH BlueZ v3 12/15] gdbus: Simplify code for appending properties

[Lucas De Marchi]

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

This reuse append_properties for GetAll and GetManagedObjects
---
 gdbus/object.c | 35 ++---------------------------------
 1 file changed, 2 insertions(+), 33 deletions(-)

diff --git a/gdbus/object.c b/gdbus/object.c
index b9c5db0..78f7aa1 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -707,9 +707,8 @@ static DBusMessage *properties_get_all(DBusConnection *connection,
 {
 	struct generic_data *data = user_data;
 	struct interface_data *iface;
-	const GDBusPropertyTable *p;
 	const char *interface;
-	DBusMessageIter iter, dict;
+	DBusMessageIter iter;
 	DBusMessage *reply;
 
 	if (!dbus_message_get_args(message, NULL,
@@ -728,37 +727,7 @@ static DBusMessage *properties_get_all(DBusConnection *connection,
 
 	dbus_message_iter_init_append(reply, &iter);
 
-	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
-			DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
-			DBUS_TYPE_STRING_AS_STRING DBUS_TYPE_VARIANT_AS_STRING
-			DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
-
-	for (p = iface->properties; p && p->name; p++) {
-		DBusMessageIter entry, value;
-
-		if (p->get == NULL)
-			continue;
-
-		if (p->exists != NULL && !p->exists(p, iface->user_data))
-			continue;
-
-		dbus_message_iter_open_container(&dict, DBUS_TYPE_DICT_ENTRY,
-								NULL, &entry);
-		dbus_message_iter_append_basic(&entry, DBUS_TYPE_STRING,
-								p->name);
-		dbus_message_iter_open_container(&entry, DBUS_TYPE_VARIANT,
-							p->type, &value);
-
-		if (!p->get(p, &value, iface->user_data)) {
-			dbus_message_unref(reply);
-			return NULL;
-		}
-
-		dbus_message_iter_close_container(&entry, &value);
-		dbus_message_iter_close_container(&dict, &entry);
-	}
-
-	dbus_message_iter_close_container(&iter, &dict);
+	append_properties(iface, &iter);
 
 	return reply;
 }
-- 
1.7.11.5

[PATCH BlueZ v3 13/15] gdbus: Implement PropertiesChanged signal

[Lucas De Marchi]

---
 gdbus/gdbus.h  |   4 +++
 gdbus/object.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 110 insertions(+), 2 deletions(-)

diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
index 3e4aa16..e1b2533 100644
--- a/gdbus/gdbus.h
+++ b/gdbus/gdbus.h
@@ -246,6 +246,10 @@ guint g_dbus_add_signal_watch(DBusConnection *connection,
 gboolean g_dbus_remove_watch(DBusConnection *connection, guint tag);
 void g_dbus_remove_all_watches(DBusConnection *connection);
 
+void g_dbus_emit_property_changed(DBusConnection *connection,
+				const char *path, const char *interface,
+				const char *name);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/gdbus/object.c b/gdbus/object.c
index 78f7aa1..2b71e73 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -48,6 +48,7 @@ struct generic_data {
 	GSList *added;
 	GSList *removed;
 	guint process_id;
+	gboolean pending_prop;
 	char *introspect;
 	struct generic_data *parent;
 };
@@ -57,6 +58,7 @@ struct interface_data {
 	const GDBusMethodTable *methods;
 	const GDBusSignalTable *signals;
 	const GDBusPropertyTable *properties;
+	GSList *pending_prop;
 	void *user_data;
 	GDBusDestroyFunction destroy;
 };
@@ -69,6 +71,9 @@ struct security_data {
 };
 
 static gboolean process_changes(gpointer user_data);
+static void process_properties_from_interface(struct generic_data *data,
+						struct interface_data *iface);
+static void process_property_changes(struct generic_data *data);
 
 static void print_arguments(GString *gstr, const GDBusArgInfo *args,
 						const char *direction)
@@ -868,6 +873,10 @@ static gboolean process_changes(gpointer user_data)
 	if (data->added != NULL)
 		emit_interfaces_added(data);
 
+	/* Flush pending properties */
+	if (data->pending_prop == TRUE)
+		process_property_changes(data);
+
 	if (data->removed != NULL)
 		emit_interfaces_removed(data);
 
@@ -884,8 +893,7 @@ static void generic_unregister(DBusConnection *connection, void *user_data)
 
 	if (data->process_id > 0) {
 		g_source_remove(data->process_id);
-		if (data->removed != NULL)
-			emit_interfaces_removed(data);
+		process_changes(data);
 	}
 
 	g_slist_foreach(data->objects, reset_parent, data->parent);
@@ -1397,3 +1405,99 @@ gboolean g_dbus_emit_signal_valist(DBusConnection *connection,
 	return emit_signal_valist(connection, path, interface,
 							name, type, args);
 }
+
+static void process_properties_from_interface(struct generic_data *data,
+						struct interface_data *iface)
+{
+	GSList *l;
+	DBusMessage *signal;
+	DBusMessageIter iter, dict, array;
+
+	if (iface->pending_prop == NULL)
+		return;
+
+	signal = dbus_message_new_signal(data->path,
+			DBUS_INTERFACE_PROPERTIES, "PropertiesChanged");
+	if (signal == NULL) {
+		error("Unable to allocate new " DBUS_INTERFACE_PROPERTIES
+						".PropertiesChanged signal");
+		return;
+	}
+
+	iface->pending_prop = g_slist_reverse(iface->pending_prop);
+
+	dbus_message_iter_init_append(signal, &iter);
+	dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING,	&iface->name);
+	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
+			DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
+			DBUS_TYPE_STRING_AS_STRING DBUS_TYPE_VARIANT_AS_STRING
+			DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
+
+	for (l = iface->pending_prop; l != NULL; l = l->next) {
+		GDBusPropertyTable *p = l->data;
+
+		if (p->get == NULL)
+			continue;
+
+		if (p->exists != NULL && !p->exists(p, iface->user_data))
+			continue;
+
+		append_property(iface, p, &dict);
+	}
+
+	dbus_message_iter_close_container(&iter, &dict);
+	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
+				DBUS_TYPE_STRING_AS_STRING, &array);
+	dbus_message_iter_close_container(&iter, &array);
+
+	g_dbus_send_message(data->conn, signal);
+
+	g_slist_free(iface->pending_prop);
+	iface->pending_prop = NULL;
+}
+
+static void process_property_changes(struct generic_data *data)
+{
+	GSList *l;
+
+	for (l = data->interfaces; l != NULL; l = l->next) {
+		struct interface_data *iface = l->data;
+
+		process_properties_from_interface(data, iface);
+	}
+
+	data->pending_prop = FALSE;
+}
+
+void g_dbus_emit_property_changed(DBusConnection *connection,
+				const char *path, const char *interface,
+				const char *name)
+{
+	const GDBusPropertyTable *property;
+	struct generic_data *data;
+	struct interface_data *iface;
+
+	if (!dbus_connection_get_object_path_data(connection, path,
+					(void **) &data) || data == NULL)
+		return;
+
+	iface = find_interface(data->interfaces, interface);
+	if (iface == NULL)
+		return;
+
+	property = find_property(iface->properties, name);
+	if (property == NULL) {
+		error("Could not find property %s in %p", name,
+							iface->properties);
+		return;
+	}
+
+	data->pending_prop = TRUE;
+	iface->pending_prop = g_slist_prepend(iface->pending_prop,
+						(void *) property);
+
+	if (!data->process_id) {
+		data->process_id = g_idle_add(process_changes, data);
+		return;
+	}
+}
-- 
1.7.11.5

Re: [PATCH BlueZ v3 13/15] gdbus: Implement PropertiesChanged signal

[Vinicius Costa Gomes]

Hi Lucas,

On 03:51 Sat 18 Aug, Lucas De Marchi wrote:
> ---
>  gdbus/gdbus.h  |   4 +++
>  gdbus/object.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 110 insertions(+), 2 deletions(-)

[snip]

> @@ -1397,3 +1405,99 @@ gboolean g_dbus_emit_signal_valist(DBusConnection *connection,
>  	return emit_signal_valist(connection, path, interface,
>  							name, type, args);
>  }
> +
> +static void process_properties_from_interface(struct generic_data *data,
> +						struct interface_data *iface)
> +{
> +	GSList *l;
> +	DBusMessage *signal;
> +	DBusMessageIter iter, dict, array;
> +
> +	if (iface->pending_prop == NULL)
> +		return;
> +
> +	signal = dbus_message_new_signal(data->path,
> +			DBUS_INTERFACE_PROPERTIES, "PropertiesChanged");
> +	if (signal == NULL) {
> +		error("Unable to allocate new " DBUS_INTERFACE_PROPERTIES
> +						".PropertiesChanged signal");
> +		return;
> +	}
> +
> +	iface->pending_prop = g_slist_reverse(iface->pending_prop);
> +
> +	dbus_message_iter_init_append(signal, &iter);
> +	dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING,	&iface->name);
> +	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
> +			DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
> +			DBUS_TYPE_STRING_AS_STRING DBUS_TYPE_VARIANT_AS_STRING
> +			DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
> +
> +	for (l = iface->pending_prop; l != NULL; l = l->next) {
> +		GDBusPropertyTable *p = l->data;
> +
> +		if (p->get == NULL)
> +			continue;
> +
> +		if (p->exists != NULL && !p->exists(p, iface->user_data))
> +			continue;

I was thinking about if the following scenario where I want a property that
only gets notified when it changes, but I can't do Get() on it is something
that we want to support. (Yeah, using signals makes more sense for this, but
the question remains ;-))

I was with the impression that that was one of the purposes of the exists()
method.

Anyway, looks good to me.

> +
> +		append_property(iface, p, &dict);
> +	}
> +
> +	dbus_message_iter_close_container(&iter, &dict);
> +	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
> +				DBUS_TYPE_STRING_AS_STRING, &array);
> +	dbus_message_iter_close_container(&iter, &array);
> +
> +	g_dbus_send_message(data->conn, signal);
> +
> +	g_slist_free(iface->pending_prop);
> +	iface->pending_prop = NULL;
> +}
> +
> +static void process_property_changes(struct generic_data *data)
> +{
> +	GSList *l;
> +
> +	for (l = data->interfaces; l != NULL; l = l->next) {
> +		struct interface_data *iface = l->data;
> +
> +		process_properties_from_interface(data, iface);
> +	}
> +
> +	data->pending_prop = FALSE;
> +}
> +
> +void g_dbus_emit_property_changed(DBusConnection *connection,
> +				const char *path, const char *interface,
> +				const char *name)
> +{
> +	const GDBusPropertyTable *property;
> +	struct generic_data *data;
> +	struct interface_data *iface;
> +
> +	if (!dbus_connection_get_object_path_data(connection, path,
> +					(void **) &data) || data == NULL)
> +		return;
> +
> +	iface = find_interface(data->interfaces, interface);
> +	if (iface == NULL)
> +		return;
> +
> +	property = find_property(iface->properties, name);
> +	if (property == NULL) {
> +		error("Could not find property %s in %p", name,
> +							iface->properties);
> +		return;
> +	}
> +
> +	data->pending_prop = TRUE;
> +	iface->pending_prop = g_slist_prepend(iface->pending_prop,
> +						(void *) property);
> +
> +	if (!data->process_id) {
> +		data->process_id = g_idle_add(process_changes, data);
> +		return;
> +	}
> +}
> -- 
> 1.7.11.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Cheers,
-- 
Vinicius

Re: [PATCH BlueZ v3 13/15] gdbus: Implement PropertiesChanged signal

[Lucas De Marchi]

On Sat, Aug 18, 2012 at 3:14 PM, Vinicius Costa Gomes
<vinicius.gomes@openbossa.org> wrote:
> Hi Lucas,
>
> On 03:51 Sat 18 Aug, Lucas De Marchi wrote:
>> ---
>>  gdbus/gdbus.h  |   4 +++
>>  gdbus/object.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>>  2 files changed, 110 insertions(+), 2 deletions(-)
>
> [snip]
>
>> @@ -1397,3 +1405,99 @@ gboolean g_dbus_emit_signal_valist(DBusConnection *connection,
>>       return emit_signal_valist(connection, path, interface,
>>                                                       name, type, args);
>>  }
>> +
>> +static void process_properties_from_interface(struct generic_data *data,
>> +                                             struct interface_data *iface)
>> +{
>> +     GSList *l;
>> +     DBusMessage *signal;
>> +     DBusMessageIter iter, dict, array;
>> +
>> +     if (iface->pending_prop == NULL)
>> +             return;
>> +
>> +     signal = dbus_message_new_signal(data->path,
>> +                     DBUS_INTERFACE_PROPERTIES, "PropertiesChanged");
>> +     if (signal == NULL) {
>> +             error("Unable to allocate new " DBUS_INTERFACE_PROPERTIES
>> +                                             ".PropertiesChanged signal");
>> +             return;
>> +     }
>> +
>> +     iface->pending_prop = g_slist_reverse(iface->pending_prop);
>> +
>> +     dbus_message_iter_init_append(signal, &iter);
>> +     dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING, &iface->name);
>> +     dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
>> +                     DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
>> +                     DBUS_TYPE_STRING_AS_STRING DBUS_TYPE_VARIANT_AS_STRING
>> +                     DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
>> +
>> +     for (l = iface->pending_prop; l != NULL; l = l->next) {
>> +             GDBusPropertyTable *p = l->data;
>> +
>> +             if (p->get == NULL)
>> +                     continue;
>> +
>> +             if (p->exists != NULL && !p->exists(p, iface->user_data))
>> +                     continue;
>
> I was thinking about if the following scenario where I want a property that
> only gets notified when it changes, but I can't do Get() on it is something
> that we want to support. (Yeah, using signals makes more sense for this, but
> the question remains ;-))
>
> I was with the impression that that was one of the purposes of the exists()
> method.

Yeah... I'm not sure we're going to support these. I need  to convert
the other places to the new DBus.Properties to see if this is needed.
If it indeed is, then I'll write a separate patch to put this property
in the list of invalidated properties.


>
> Anyway, looks good to me.
>
>> +
>> +             append_property(iface, p, &dict);
>> +     }
>> +
>> +     dbus_message_iter_close_container(&iter, &dict);
>> +     dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
>> +                             DBUS_TYPE_STRING_AS_STRING, &array);
>> +     dbus_message_iter_close_container(&iter, &array);

Here... this is the empty array that we are sending now. If we need
the case above I'll fill it here.

Thanks
Lucas De Marchi

>> +
>> +     g_dbus_send_message(data->conn, signal);
>> +
>> +     g_slist_free(iface->pending_prop);
>> +     iface->pending_prop = NULL;
>> +}
>> +
>> +static void process_property_changes(struct generic_data *data)
>> +{
>> +     GSList *l;
>> +
>> +     for (l = data->interfaces; l != NULL; l = l->next) {
>> +             struct interface_data *iface = l->data;
>> +
>> +             process_properties_from_interface(data, iface);
>> +     }
>> +
>> +     data->pending_prop = FALSE;
>> +}
>> +
>> +void g_dbus_emit_property_changed(DBusConnection *connection,
>> +                             const char *path, const char *interface,
>> +                             const char *name)
>> +{
>> +     const GDBusPropertyTable *property;
>> +     struct generic_data *data;
>> +     struct interface_data *iface;
>> +
>> +     if (!dbus_connection_get_object_path_data(connection, path,
>> +                                     (void **) &data) || data == NULL)
>> +             return;
>> +
>> +     iface = find_interface(data->interfaces, interface);
>> +     if (iface == NULL)
>> +             return;
>> +
>> +     property = find_property(iface->properties, name);
>> +     if (property == NULL) {
>> +             error("Could not find property %s in %p", name,
>> +                                                     iface->properties);
>> +             return;
>> +     }
>> +
>> +     data->pending_prop = TRUE;
>> +     iface->pending_prop = g_slist_prepend(iface->pending_prop,
>> +                                             (void *) property);
>> +
>> +     if (!data->process_id) {
>> +             data->process_id = g_idle_add(process_changes, data);
>> +             return;
>> +     }
>> +}
>> --
>> 1.7.11.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
> Cheers,
> --
> Vinicius

[PATCH BlueZ v3 14/15] control: Use DBus.Properties

[Lucas De Marchi]

---
 audio/control.c | 56 +++++++++++++++++---------------------------------------
 1 file changed, 17 insertions(+), 39 deletions(-)

diff --git a/audio/control.c b/audio/control.c
index 187f838..dc15ca4 100644
--- a/audio/control.c
+++ b/audio/control.c
@@ -68,7 +68,6 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
 				avctp_state_t new_state, void *user_data)
 {
 	struct control *control = dev->control;
-	gboolean value;
 
 	switch (new_state) {
 	case AVCTP_STATE_DISCONNECTED:
@@ -77,13 +76,11 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
 		if (old_state != AVCTP_STATE_CONNECTED)
 			break;
 
-		value = FALSE;
 		g_dbus_emit_signal(dev->conn, dev->path,
 					AUDIO_CONTROL_INTERFACE,
 					"Disconnected", DBUS_TYPE_INVALID);
-		emit_property_changed(dev->conn, dev->path,
-					AUDIO_CONTROL_INTERFACE, "Connected",
-					DBUS_TYPE_BOOLEAN, &value);
+		g_dbus_emit_property_changed(dev->conn, dev->path,
+					AUDIO_CONTROL_INTERFACE, "Connected");
 
 		break;
 	case AVCTP_STATE_CONNECTING:
@@ -94,13 +91,11 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
 
 		break;
 	case AVCTP_STATE_CONNECTED:
-		value = TRUE;
 		g_dbus_emit_signal(dev->conn, dev->path,
 				AUDIO_CONTROL_INTERFACE, "Connected",
 				DBUS_TYPE_INVALID);
-		emit_property_changed(dev->conn, dev->path,
-				AUDIO_CONTROL_INTERFACE, "Connected",
-				DBUS_TYPE_BOOLEAN, &value);
+		g_dbus_emit_property_changed(dev->conn, dev->path,
+				AUDIO_CONTROL_INTERFACE, "Connected");
 		break;
 	default:
 		return;
@@ -168,42 +163,21 @@ static DBusMessage *volume_down(DBusConnection *conn, DBusMessage *msg,
 	return dbus_message_new_method_return(msg);
 }
 
-static DBusMessage *control_get_properties(DBusConnection *conn,
-					DBusMessage *msg, void *data)
+static gboolean control_prop_get_connected(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
 {
 	struct audio_device *device = data;
-	DBusMessage *reply;
-	DBusMessageIter iter;
-	DBusMessageIter dict;
-	gboolean value;
-
-	reply = dbus_message_new_method_return(msg);
-	if (!reply)
-		return NULL;
+	dbus_bool_t value =  (device->control->session != NULL);
 
-	dbus_message_iter_init_append(reply, &iter);
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN, &value);
 
-	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
-			DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
-			DBUS_TYPE_STRING_AS_STRING DBUS_TYPE_VARIANT_AS_STRING
-			DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
-
-	/* Connected */
-	value = (device->control->session != NULL);
-	dict_append_entry(&dict, "Connected", DBUS_TYPE_BOOLEAN, &value);
-
-	dbus_message_iter_close_container(&iter, &dict);
-
-	return reply;
+	return TRUE;
 }
 
 static const GDBusMethodTable control_methods[] = {
 	{ GDBUS_DEPRECATED_METHOD("IsConnected",
 				NULL, GDBUS_ARGS({ "connected", "b" }),
 				control_is_connected) },
-	{ GDBUS_METHOD("GetProperties",
-				NULL, GDBUS_ARGS({ "properties", "a{sv}" }),
-				control_get_properties) },
 	{ GDBUS_METHOD("VolumeUp", NULL, NULL, volume_up) },
 	{ GDBUS_METHOD("VolumeDown", NULL, NULL, volume_down) },
 	{ }
@@ -212,11 +186,14 @@ static const GDBusMethodTable control_methods[] = {
 static const GDBusSignalTable control_signals[] = {
 	{ GDBUS_DEPRECATED_SIGNAL("Connected", NULL) },
 	{ GDBUS_DEPRECATED_SIGNAL("Disconnected", NULL) },
-	{ GDBUS_SIGNAL("PropertyChanged",
-			GDBUS_ARGS({ "name", "s" }, { "value", "v" })) },
 	{ }
 };
 
+static const GDBusPropertyTable control_properties[] = {
+	{ "Connected", "b", control_prop_get_connected },
+	{}
+};
+
 static void path_unregister(void *data)
 {
 	struct audio_device *dev = data;
@@ -250,8 +227,9 @@ struct control *control_init(struct audio_device *dev, uint16_t uuid16)
 
 	if (!g_dbus_register_interface(dev->conn, dev->path,
 					AUDIO_CONTROL_INTERFACE,
-					control_methods, control_signals, NULL,
-					dev, path_unregister))
+					control_methods, control_signals,
+					control_properties, dev,
+					path_unregister))
 		return NULL;
 
 	DBG("Registered interface %s on path %s",
-- 
1.7.11.5

[PATCH BlueZ v3 15/15] audio: device: Use DBus.Properties

[Lucas De Marchi]

---
 audio/device.c | 48 ++++++++++++++++++------------------------------
 1 file changed, 18 insertions(+), 30 deletions(-)

diff --git a/audio/device.c b/audio/device.c
index 9507eac..ec6dc43 100644
--- a/audio/device.c
+++ b/audio/device.c
@@ -279,9 +279,8 @@ static void device_set_state(struct audio_device *dev, audio_state_t new_state)
 		g_dbus_send_message(dev->conn, reply);
 	}
 
-	emit_property_changed(dev->conn, dev->path,
-				AUDIO_INTERFACE, "State",
-				DBUS_TYPE_STRING, &state_str);
+	g_dbus_emit_property_changed(dev->conn, dev->path,
+						AUDIO_INTERFACE, "State");
 }
 
 static gboolean avdtp_connect_timeout(gpointer user_data)
@@ -586,48 +585,37 @@ static DBusMessage *dev_disconnect(DBusConnection *conn, DBusMessage *msg,
 	return NULL;
 }
 
-static DBusMessage *dev_get_properties(DBusConnection *conn, DBusMessage *msg,
+static gboolean dev_prop_exists_state(const GDBusPropertyTable *property,
 								void *data)
 {
 	struct audio_device *device = data;
-	DBusMessage *reply;
-	DBusMessageIter iter;
-	DBusMessageIter dict;
-	const char *state;
+	const char *state = state2str(device->priv->state);
 
-	reply = dbus_message_new_method_return(msg);
-	if (!reply)
-		return NULL;
-
-	dbus_message_iter_init_append(reply, &iter);
+	return state != NULL;
+}
 
-	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
-			DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
-			DBUS_TYPE_STRING_AS_STRING DBUS_TYPE_VARIANT_AS_STRING
-			DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
+static gboolean dev_prop_get_state(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct audio_device *device = data;
+	const char *state = state2str(device->priv->state);
 
-	/* State */
-	state = state2str(device->priv->state);
-	if (state)
-		dict_append_entry(&dict, "State", DBUS_TYPE_STRING, &state);
+	if (state == NULL)
+		return FALSE;
 
-	dbus_message_iter_close_container(&iter, &dict);
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &state);
 
-	return reply;
+	return TRUE;
 }
 
 static const GDBusMethodTable dev_methods[] = {
 	{ GDBUS_ASYNC_METHOD("Connect", NULL, NULL, dev_connect) },
 	{ GDBUS_METHOD("Disconnect", NULL, NULL, dev_disconnect) },
-	{ GDBUS_METHOD("GetProperties",
-		NULL, GDBUS_ARGS({ "properties", "a{sv}" }),
-		dev_get_properties) },
 	{ }
 };
 
-static const GDBusSignalTable dev_signals[] = {
-	{ GDBUS_SIGNAL("PropertyChanged",
-			GDBUS_ARGS({ "name", "s" }, { "value", "v" })) },
+static const GDBusPropertyTable dev_properties[] = {
+	{ "State", "s", dev_prop_get_state, NULL, dev_prop_exists_state },
 	{ }
 };
 
@@ -653,7 +641,7 @@ struct audio_device *audio_device_register(DBusConnection *conn,
 
 	if (!g_dbus_register_interface(dev->conn, dev->path,
 					AUDIO_INTERFACE,
-					dev_methods, dev_signals, NULL,
+					dev_methods, NULL, dev_properties,
 					dev, NULL)) {
 		error("Unable to register %s on %s", AUDIO_INTERFACE,
 								dev->path);
-- 
1.7.11.5

Re: [PATCH BlueZ v3 00/15] gdbus: DBus.Properties + ObjectManager

[Luiz Augusto von Dentz]

Hi Lucas,

On Sat, Aug 18, 2012 at 9:50 AM, Lucas De Marchi
<lucas.demarchi@profusion.mobi> wrote:
> Hey,
>
> here is a third version of the changes planned to gdbus. In contrary to
> previous versions, this patch set includes also the Object Manager, since the
> PropertiesChanged signal was made on top of it.
>
> The first two patches are cosmetics to the other ones and somehow independent.
>
> The last two patches are examples on how we would use the API after the conversion.
>
> DBus.Properties.Set() is async now. We pass the message on so user can use it
> to create the reply or error message.

Another thing we discussed was getter to be async as well, but this
had several implications:

1. Properties should be cacheable and by that it means the client can
listen to PropertiesChanged after it knows about the initial value.

2. In case of GetAll and GetManagedObjects where all properties are
fetched it would be extremely difficult to deal with getter being
async, each property could cause a delay in the response and in the
meantime the properties already fetched could have changed.

3. The error cases, when the getter fails, are complicated even more
by libdbus not supporting changes to its items already in the
iterator, so we would have to workaround this.

So a property which the value is initially unknown by server should
either be omitted or don't be exported as a property if the value
cannot be cached consistently.

-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ v3 00/15] gdbus: DBus.Properties + ObjectManager

[Lucas De Marchi]

On Mon, Aug 20, 2012 at 4:56 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Lucas,
>
> On Sat, Aug 18, 2012 at 9:50 AM, Lucas De Marchi
> <lucas.demarchi@profusion.mobi> wrote:
>> Hey,
>>
>> here is a third version of the changes planned to gdbus. In contrary to
>> previous versions, this patch set includes also the Object Manager, since the
>> PropertiesChanged signal was made on top of it.
>>
>> The first two patches are cosmetics to the other ones and somehow independent.
>>
>> The last two patches are examples on how we would use the API after the conversion.
>>
>> DBus.Properties.Set() is async now. We pass the message on so user can use it
>> to create the reply or error message.
>
> Another thing we discussed was getter to be async as well, but this
> had several implications:
>
> 1. Properties should be cacheable and by that it means the client can
> listen to PropertiesChanged after it knows about the initial value.
>
> 2. In case of GetAll and GetManagedObjects where all properties are
> fetched it would be extremely difficult to deal with getter being
> async, each property could cause a delay in the response and in the
> meantime the properties already fetched could have changed.
>
> 3. The error cases, when the getter fails, are complicated even more
> by libdbus not supporting changes to its items already in the
> iterator, so we would have to workaround this.
>
> So a property which the value is initially unknown by server should
> either be omitted or don't be exported as a property if the value
> cannot be cached consistently.

Additionally, we don't have async GetProperties in Bluez right now
("git grep METHOD | grep GetProperties"), so there's no urge need for
them. The one project that uses async GetProperties is oFono. I
propose we go with the simpler approach, letting the getter be sync
and when time comes to convert oFono we can think about how to handle
it (if we are not already using ell).



Lucas De Marchi