[PATCH BlueZ v2 0/5] DBus.Properties

[PATCH BlueZ v2 0/5] DBus.Properties

[Lucas De Marchi]

v2:
	s/property_name/name/
	s/interface_name/interface/

Original text:

This is the first part of the patches for implementing the DBus.Properties as
demonstrated during last BlueZ meeting. I changed the Properties.Set
implementation a little bit from what was demonstrated - the goal here is to
facilitate the use of the new API.

Luiz will send the ObjectManager implementation on top of these ones.


Lucas De Marchi (5):
  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/gdbus.h  |  22 +++++-
 gdbus/object.c | 246 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 266 insertions(+), 2 deletions(-)

-- 
1.7.11.2

[PATCH BlueZ v2 1/5] 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 900e7ab..91f7e97 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -491,6 +491,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,
@@ -541,6 +583,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;
 }
 
@@ -580,6 +625,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.2

[PATCH BlueZ v2 2/5] gdbus: Implement DBus.Properties.Get method

[Lucas De Marchi]

---
 gdbus/gdbus.h  | 13 ++++++++++--
 gdbus/object.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 72 insertions(+), 3 deletions(-)

diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
index 0a8a27c..e2a8460 100644
--- a/gdbus/gdbus.h
+++ b/gdbus/gdbus.h
@@ -55,6 +55,13 @@ typedef void (* GDBusDestroyFunction) (void *user_data);
 typedef DBusMessage * (* GDBusMethodFunction) (DBusConnection *connection,
 					DBusMessage *message, void *user_data);
 
+typedef struct GDBusPropertyTable GDBusPropertyTable;
+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,
@@ -102,11 +109,13 @@ typedef struct {
 	const GDBusArgInfo *args;
 } GDBusSignalTable;
 
-typedef struct {
+struct GDBusPropertyTable {
 	const char *name;
 	const char *type;
+	GDBusPropertyGetter get;
+	GDBusPropertyExists exists;
 	GDBusPropertyFlags flags;
-} GDBusPropertyTable;
+};
 
 typedef struct {
 	unsigned int privilege;
diff --git a/gdbus/object.c b/gdbus/object.c
index 91f7e97..30b5712 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -491,10 +491,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.2

[PATCH BlueZ v2 3/5] 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 30b5712..a34039f 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -560,7 +560,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.2

[PATCH BlueZ v2 4/5] gdbus: Implement DBus.Properties.Set method

[Lucas De Marchi]

---
 gdbus/gdbus.h  |  9 ++++++++
 gdbus/object.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
index e2a8460..3e0c643 100644
--- a/gdbus/gdbus.h
+++ b/gdbus/gdbus.h
@@ -59,6 +59,14 @@ typedef struct GDBusPropertyTable GDBusPropertyTable;
 typedef gboolean (*GDBusPropertyGetter)(const GDBusPropertyTable *property,
 					DBusMessageIter *iter, void *data);
 
+typedef enum {
+	G_DBUS_PROPERTY_SET_SUCCESS = 0,
+	G_DBUS_PROPERTY_SET_INVALID_ARGUMENTS,
+} GDBusPropertySetReturn;
+
+typedef GDBusPropertySetReturn (*GDBusPropertySetter)(const GDBusPropertyTable *property,
+					DBusMessageIter *value, void *data);
+
 typedef gboolean (*GDBusPropertyExists)(const GDBusPropertyTable *property,
 								void *data);
 
@@ -113,6 +121,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 a34039f..1053486 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -620,7 +620,72 @@ static DBusMessage *properties_get_all(DBusConnection *connection,
 static DBusMessage *properties_set(DBusConnection *connection,
 					DBusMessage *message, void *user_data)
 {
-	return NULL;
+	GDBusPropertySetReturn ret;
+	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 NULL;
+
+	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);
+
+	ret = property->set(property, &sub, iface->user_data);
+
+	switch (ret) {
+	case G_DBUS_PROPERTY_SET_SUCCESS:
+		return dbus_message_new_method_return(message);
+	case G_DBUS_PROPERTY_SET_INVALID_ARGUMENTS:
+		return g_dbus_create_error(message, DBUS_ERROR_INVALID_ARGS,
+						"Invalid arguments");
+	}
+
+	return g_dbus_create_error(message, DBUS_ERROR_FAILED, "Failed");
 }
 
 static const GDBusMethodTable properties_methods[] = {
-- 
1.7.11.2

[PATCH BlueZ v2 5/5] gdbus: Add properties into Introspectable interface

[Lucas De Marchi]

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

diff --git a/gdbus/object.c b/gdbus/object.c
index 1053486..fc2b5e2 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -80,6 +80,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 &
@@ -126,6 +127,26 @@ 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,
+				">\n\t\t\t<annotation"
+				" name=\"org.freedesktop.DBus.Deprecated\""
+				" value=\"true\"/>\n\t\t</property>\n");
+	}
 }
 
 static void generate_introspection_xml(DBusConnection *conn,
-- 
1.7.11.2

Re: [PATCH BlueZ v2 2/5] gdbus: Implement DBus.Properties.Get method

[Marcel Holtmann]

Hi Lucas,

>  gdbus/gdbus.h  | 13 ++++++++++--
>  gdbus/object.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 72 insertions(+), 3 deletions(-)
> 
> diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
> index 0a8a27c..e2a8460 100644
> --- a/gdbus/gdbus.h
> +++ b/gdbus/gdbus.h
> @@ -55,6 +55,13 @@ typedef void (* GDBusDestroyFunction) (void *user_data);
>  typedef DBusMessage * (* GDBusMethodFunction) (DBusConnection *connection,
>  					DBusMessage *message, void *user_data);
>  
> +typedef struct GDBusPropertyTable GDBusPropertyTable;
> +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,
> @@ -102,11 +109,13 @@ typedef struct {
>  	const GDBusArgInfo *args;
>  } GDBusSignalTable;
>  
> -typedef struct {
> +struct GDBusPropertyTable {
>  	const char *name;
>  	const char *type;
> +	GDBusPropertyGetter get;
> +	GDBusPropertyExists exists;
>  	GDBusPropertyFlags flags;
> -} GDBusPropertyTable;
> +};

can we make this a bit cleaner and do it similar for all structs +
typedefs and not just make GDBusPropertyTable special.

Regards

Marcel