Re: Need approval to submit a patch

[David Zeuthen <zeuthen gmail com>]

Hey,

On Wed, May 18, 2011 at 7:09 AM, marcin saepia net <marcin saepia net> wrote:
>> First of all the main reason there isn't a function for this is that
>> with the new gdbus-codegen(1) tool, this is automatically handled.
>> That was always the plan and it's why pre-gdbus-codegen(1) is so
>> low-level.
>
> I've looked at gdbus-codegen(1). I also extensively use GStreamer.
> When my pipeline changes, I have to update several variables at one
> time, as they should be considered atomic. It means, that I want to
> send notification signal with all of them, at one time. I see no way
> how it could be automatically handled by gdbus-codegen(1).
>
> In more complex applications there always would be situation like one
> described above. It would be nice to have convenient function to
> *send* such custom signals. Example shown here:
> http://people.freedesktop.org/~david/gio-gdbus-codegen-20110412/gdbus-example-gdbus-codegen.html
> currently covers only automatic *receiving* PropertyChanged signals.

The gdbus-codegen(1) man page states

 To facilitate atomic changesets (multiple properties changing at the
 same time), "notify" signals are queued up when received. The queue
 is drained in an idle handler (which is called from the thread-default
 main loop of the thread where the skeleton object was contructed) and
 will cause emissions of the org.freedesktop.DBus.Properties::PropertiesChanged
 signal with all the properties that have changed. Use
 g_dbus_interface_skeleton_flush() or g_dbus_object_skeleton_flush() to empty
 the queue immediately. Use g_object_freeze_notify() and
 g_object_thaw_notify() for atomic changesets if on a different thread.

 (the freeze/thaw comment was added in this commit on Sunday
  http://git.gnome.org/browse/glib/commit/?id=11e01802abcf006caa8e3c2ecf6cfd195db98fd9

and this works just fine - we even have unit-tests to ensure that
multiple properties are emitted together and that things like

 g_object_set (some_dbus_iface, "its-dbus-property", 4, NULL);
 g_object_set (some_dbus_iface, "its-dbus-property", 0, NULL);
 g_object_set (some_dbus_iface, "its-dbus-property", 4, NULL);
 g_object_set (some_dbus_iface, "its-dbus-property", 0, NULL);

does not send out any PropertiesChanged D-Bus signal if
:its-dbus-property is 0 and only one signal if it's not NULL. This is
really how you want it to work - not having to worry about low-level
details like the PropertiesChanged D-Bus signal

>> For starters, I don't really like varargs functions because they are
>> not available to language bindings (GVariant is an exception and we
>> can assume each language will bind it separately). And I just don't
>> think it's that much shorter than the code I pasted above... and with
>> gdbus-codegen(1) it's kinda a non-issue because it happens
>> automatically.
>
> As I wrote above, length of the code is not the case. DRY is the case.
> Do-Not-Reinvent-The-Wheel is also the case.
>
> And as far as I understand, gdbus-codegen provides only tool to handle
> receiving of such signals.
>
>> g_dbus_connection_emit_properties_changed_signal() method that takes
>> two GVariant* (GVariantBuilder is probably not bindable) instances,
>> then you can insert g_variant_builder_end() calls at that place? That
>> should be bindable...
>
> To be honest, I do not understand what "bindable" means  in this context ;)

Bindable means automatically being able to use the function from
languages that bind the library, e.g. Javascript (through e.g. gjs or
seed) or Python (through pygobject). This isn't possible for the
varadic function that you proposed. That's why I suggested to do it in
another way.

Either way, as I said, this mailing list is the wrong place for this
kind of discussion. Please file a bug in bugzilla and we can keep
discussing it there. As I said, I'm not opposed to porcelain like
this, it just needs to be done right (e.g. bindable), that's all!

    David