2012-07-24 10:46:21

by Lucas De Marchi

[permalink] [raw]
Subject: [PATCH BlueZ v2 0/5] DBus.Properties

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



2012-07-24 13:36:18

by Marcel Holtmann

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

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



2012-07-24 10:46:22

by Lucas De Marchi

[permalink] [raw]
Subject: [PATCH BlueZ v2 1/5] gdbus: Add skeleton of DBus.Properties interface

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


2012-07-24 10:46:26

by Lucas De Marchi

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

---
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


2012-07-24 10:46:25

by Lucas De Marchi

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

---
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


2012-07-24 10:46:24

by Lucas De Marchi

[permalink] [raw]
Subject: [PATCH BlueZ v2 3/5] gdbus: Implement DBus.Properties.GetAll method

---
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


2012-07-24 10:46:23

by Lucas De Marchi

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

---
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