[PATCH BlueZ 0/5] DBus.Properties

[PATCH BlueZ 0/5] DBus.Properties

[Lucas De Marchi]

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 | 252 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 272 insertions(+), 2 deletions(-)

-- 
1.7.11.2

[PATCH BlueZ 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 | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/gdbus/object.c b/gdbus/object.c
index 900e7ab..72f77d4 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -491,6 +491,50 @@ 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_name", "s" },
+						{ "property_name", "s" }),
+			GDBUS_ARGS({ "value", "v" }),
+			properties_get) },
+	{ GDBUS_METHOD("Set", NULL,
+			GDBUS_ARGS({ "interface_name", "s" },
+					{ "property_name", "s" },
+					{ "value", "v" }),
+			properties_set) },
+	{ GDBUS_METHOD("GetAll",
+			GDBUS_ARGS({ "interface_name", "s" }),
+			GDBUS_ARGS({ "props", "a{sv}" }),
+			properties_get_all) },
+	{ }
+};
+
+static const GDBusSignalTable properties_signals[] = {
+	{ GDBUS_SIGNAL("PropertiesChanged",
+			GDBUS_ARGS({ "interface_name", "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 +585,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 +627,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 2/5] gdbus: Implement DBus.Properties.Get method

[Lucas De Marchi]

---
 gdbus/gdbus.h  | 13 ++++++++++--
 gdbus/object.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 74 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 72f77d4..543e61c 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -491,10 +491,72 @@ static const GDBusMethodTable introspect_methods[] = {
 	{ }
 };
 
+static inline const GDBusPropertyTable *find_property(const GDBusPropertyTable *properties,
+							const char *property_name)
+{
+	const GDBusPropertyTable *p;
+
+	for (p = properties; p && p->name; p++) {
+		if (strcmp(property_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, *property_name;
+	DBusMessageIter iter, value;
+	DBusMessage *reply;
+
+	if (!dbus_message_get_args(message, NULL,
+					DBUS_TYPE_STRING, &interface_name,
+					DBUS_TYPE_STRING, &property_name,
+					DBUS_TYPE_INVALID))
+		return NULL;
+
+	iface = find_interface(data->interfaces, interface_name);
+	if (iface == NULL)
+		return g_dbus_create_error(message, DBUS_ERROR_INVALID_ARGS,
+				"No such interface '%s'", interface_name);
+
+	property = find_property(iface->properties, property_name);
+	if (property == NULL)
+		return g_dbus_create_error(message, DBUS_ERROR_INVALID_ARGS,
+				"No such property '%s'", property_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'",
+						property_name);
+
+	if (property->get == NULL)
+		return g_dbus_create_error(message, DBUS_ERROR_INVALID_ARGS,
+					"Property '%s' is not readable",
+					property_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 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 543e61c..6daaa81 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -562,7 +562,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_name;
+	DBusMessageIter iter, dict;
+	DBusMessage *reply;
+
+	if (!dbus_message_get_args(message, NULL,
+					DBUS_TYPE_STRING, &interface_name,
+					DBUS_TYPE_INVALID))
+		return NULL;
+
+	iface = find_interface(data->interfaces, interface_name);
+	if (iface == NULL)
+		return g_dbus_create_error(message, DBUS_ERROR_INVALID_ARGS,
+				"No such interface '%s'", interface_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_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 4/5] gdbus: Implement DBus.Properties.Set method

[Lucas De Marchi]

---
 gdbus/gdbus.h  |  9 ++++++++
 gdbus/object.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 77 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 6daaa81..c19f1d7 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -622,7 +622,74 @@ 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 *property_name, *interface_name;
+
+	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, &property_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_name);
+	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_name);
+	if (iface == NULL)
+		return g_dbus_create_error(message, DBUS_ERROR_INVALID_ARGS,
+				"No such interface '%s'", interface_name);
+
+	property = find_property(iface->properties, property_name);
+	if (property == NULL)
+		return g_dbus_create_error(message,
+				DBUS_ERROR_UNKNOWN_PROPERTY,
+				"No such property '%s'", property_name);
+
+	if (property->set == NULL)
+		return g_dbus_create_error(message,
+					DBUS_ERROR_PROPERTY_READ_ONLY,
+					"Property '%s' is not writable",
+					property_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'",
+						property_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 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 c19f1d7..e2e8b12 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 1/5] gdbus: Add skeleton of DBus.Properties interface

[Marcel Holtmann]

Hi Lucas,

> 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 | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
> 
> diff --git a/gdbus/object.c b/gdbus/object.c
> index 900e7ab..72f77d4 100644
> --- a/gdbus/object.c
> +++ b/gdbus/object.c
> @@ -491,6 +491,50 @@ 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_name", "s" },
> +						{ "property_name", "s" }),

this should be "interface" and "name". Why are we so verbose here?

> +			GDBUS_ARGS({ "value", "v" }),
> +			properties_get) },
> +	{ GDBUS_METHOD("Set", NULL,
> +			GDBUS_ARGS({ "interface_name", "s" },
> +					{ "property_name", "s" },
> +					{ "value", "v" }),
> +			properties_set) },
> +	{ GDBUS_METHOD("GetAll",
> +			GDBUS_ARGS({ "interface_name", "s" }),
> +			GDBUS_ARGS({ "props", "a{sv}" }),

And this should be "properties". Here you are trying to be super short.

> +			properties_get_all) },
> +	{ }
> +};
> +
> +static const GDBusSignalTable properties_signals[] = {
> +	{ GDBUS_SIGNAL("PropertiesChanged",
> +			GDBUS_ARGS({ "interface_name", "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 +585,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 +627,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);
>  

Regards

Marcel

Re: [PATCH BlueZ 1/5] gdbus: Add skeleton of DBus.Properties interface

[Lucas De Marchi]

On Thu, Jul 19, 2012 at 5:03 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Lucas,
>
>> 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 | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 48 insertions(+)
>>
>> diff --git a/gdbus/object.c b/gdbus/object.c
>> index 900e7ab..72f77d4 100644
>> --- a/gdbus/object.c
>> +++ b/gdbus/object.c
>> @@ -491,6 +491,50 @@ 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_name", "s" },
>> +                                             { "property_name", "s" }),
>
> this should be "interface" and "name". Why are we so verbose here?

I put the same name as in spec:
http://dbus.freedesktop.org/doc/dbus-specification.html#standard-interfaces-properties

>
>> +                     GDBUS_ARGS({ "value", "v" }),
>> +                     properties_get) },
>> +     { GDBUS_METHOD("Set", NULL,
>> +                     GDBUS_ARGS({ "interface_name", "s" },
>> +                                     { "property_name", "s" },
>> +                                     { "value", "v" }),
>> +                     properties_set) },
>> +     { GDBUS_METHOD("GetAll",
>> +                     GDBUS_ARGS({ "interface_name", "s" }),
>> +                     GDBUS_ARGS({ "props", "a{sv}" }),
>
> And this should be "properties". Here you are trying to be super short.

same here.


should I change or let as is?

Lucas De  Marchi

Re: [PATCH BlueZ 1/5] gdbus: Add skeleton of DBus.Properties interface

[Marcel Holtmann]

Hi Lucas,

> >> 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 | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 48 insertions(+)
> >>
> >> diff --git a/gdbus/object.c b/gdbus/object.c
> >> index 900e7ab..72f77d4 100644
> >> --- a/gdbus/object.c
> >> +++ b/gdbus/object.c
> >> @@ -491,6 +491,50 @@ 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_name", "s" },
> >> +                                             { "property_name", "s" }),
> >
> > this should be "interface" and "name". Why are we so verbose here?
> 
> I put the same name as in spec:
> http://dbus.freedesktop.org/doc/dbus-specification.html#standard-interfaces-properties
> 
> >
> >> +                     GDBUS_ARGS({ "value", "v" }),
> >> +                     properties_get) },
> >> +     { GDBUS_METHOD("Set", NULL,
> >> +                     GDBUS_ARGS({ "interface_name", "s" },
> >> +                                     { "property_name", "s" },
> >> +                                     { "value", "v" }),
> >> +                     properties_set) },
> >> +     { GDBUS_METHOD("GetAll",
> >> +                     GDBUS_ARGS({ "interface_name", "s" }),
> >> +                     GDBUS_ARGS({ "props", "a{sv}" }),
> >
> > And this should be "properties". Here you are trying to be super short.
> 
> same here.
> 
> 
> should I change or let as is?

it is inconsistent at least. On one method you use props on the other it
is *_properties. No matter what the specification says, that is just
stupid. And the _name suffix is useless in this context. So yes, please
just change it.

Regards

Marcel
h