After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 657702 - Add bindings for GDBusProxy in the Gio extension.
Add bindings for GDBusProxy in the Gio extension.
Status: RESOLVED OBSOLETE
Product: seed
Classification: Bindings
Component: libseed
git master
Other All
: Normal normal
: ---
Assigned To: seed-maint
seed-maint
Depends on:
Blocks:
 
 
Reported: 2011-08-30 13:14 UTC by Alexandre Mazari
Modified: 2021-05-25 17:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add bindings for GDBusProxy in the Gio extension. (6.39 KB, patch)
2011-08-30 13:14 UTC, Alexandre Mazari
none Details | Review
Add bindings for GDBusProxy in the Gio extension. (6.36 KB, patch)
2011-08-31 16:04 UTC, Alexandre Mazari
none Details | Review

Description Alexandre Mazari 2011-08-30 13:14:12 UTC
This introduces facilities to work with Gio.DBusProxy:
- a factory method DBusProxy.new
- methods generation using DBus Introspection
- de/marchalling of DBus messages (both arguments and return values) using the
 GLib extension found in #657699 
- naming conventions for signal handlers property change listeners and async methods returns

Usage:

Gio = imports.gi.Gio; // the extension augment the Gio namespace

// create a proxy for the given service
player = Gio.DBusProxy.new(
    Gio.BusType.SESSION,
    "org.mpris.MediaPlayer2.rhythmbox",
    "/org/mpris/MediaPlayer2",
    "org.mpris.MediaPlayer2.Player");

// declaration of Metadata and Volume properties change listeners
player.onMetadataChanged = function (new_value) {print("new title: "+new_value["xesam:title"])};
player.onVolumeChanged = function (new_value) {print("Volume: "+new_value)};

// declaration of Seeked signal handler
player.onSeeked = function (pos) {print("pos:"+pos);};

// declaration of the async callback for the Next method
player.onNextReturned = function(){ print("next"); }

// async invocation of the 'Next' DBus method
player.NextAsync();

// sync invocation of the 'Next' DBus method
player.Next();


Some advantages over the existing module:
 - doesn't depends and/or use native module, resulting in a lot less code to maintain
 - doesn't require the API user to declare a proxy interface, everything is 
 introspected on the bus
 - enforces declaring well-named methods as signal handlers and property change  listeners, avoiding hard-to-read nested async code.
 - installed in parallel with the existing module

Caveat:
 - no code for exporting object on the bus
 - need more testing
Comment 1 Alexandre Mazari 2011-08-30 13:14:24 UTC
Created attachment 195194 [details] [review]
Add bindings for GDBusProxy in the Gio extension.

Introduce facilities for client DBus using GDbus: methods generation, signals and properties binding.
/!\ This is not compatible with the existing native dbus module.
See documentation.
Comment 2 Alan Knowles 2011-08-31 04:51:19 UTC
This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.
Comment 3 Alexandre Mazari 2011-08-31 16:04:50 UTC
Created attachment 195314 [details] [review]
Add bindings for GDBusProxy in the Gio extension.

Introduce facilities for client DBus using GDbus: methods generation, signals and properties binding.
This is not compatible with the existing native dbus module.

update:
- comply with recent modification of GLib and libxml extensions.
Comment 4 Alexandre Mazari 2011-08-31 16:10:21 UTC
Alan,

I am not sure this patch is ready to be commited and released. At least I am not commited to maintain this API without feedback :)

Both the API and implementation would benefit from other eyeballs.
Comment 5 Alan Knowles 2011-09-01 03:47:37 UTC
Could you add a test to tests/javascript - showing how the API can be used/works.

That way it will be easy to see how the new API works, and we also end up with a test for it.

The code looks reasonably ok, I'm not sure why we are mixing to_js and toJS though. normally I would expect the 'to_js' to be the result of the C api export, hence all local language stuff is camelCase.
Comment 6 Alexandre Mazari 2011-09-02 07:13:43 UTC
Hi Alan,

Thanks for the review!

(In reply to comment #5)
> Could you add a test to tests/javascript - showing how the API can be
> used/works.

I will.

> 
> That way it will be easy to see how the new API works, and we also end up with
> a test for it.
> 
> The code looks reasonably ok, I'm not sure why we are mixing to_js and toJS
> though. normally I would expect the 'to_js' to be the result of the C api
> export, hence all local language stuff is camelCase.
Agreed. The reasoning here is that imported symbols from the libxml modules are camelCased where the GLib imports are underscored. For coherency reason, i made the extentions matches the style of their respective namespaces.

So should I make them all camel ?
Comment 7 Alexandre Mazari 2011-09-02 07:30:44 UTC
Here are open questions I need some advices/enlightenment about. Feel free to comment!

First, the API:
- lower-case the first letter of method names ? Would look definitely better and feel more js.
- allow several dbus interface for one object ?
- allow several handlers for each signal, property change and asyn method return ?
- add a compatibility layer with current DBus module ?
- instead of providing a factory method returning a vanilia dbusproxy that is then augmented with handlers, make it take an object ?
ie the previous example would look something like :

player = Gio.DBusProxy.new({
    bus:                Gio.BusType.SESSION,
    name:               "org.mpris.MediaPlayer2.rhythmbox",
    object_path:        "/org/mpris/MediaPlayer2",
    interface:          "org.mpris.MediaPlayer2.Player",
    onMetadataChanged:  function (new_value) {print("title:"+new_value["xesam:title"])},
    onSeeked:           function (pos) {print("pos:"+pos);},
    onNextReturned:     function(){ print("next"); }

);

- provide an API to add afterwards an interface to an object .
(player.interfaces += "org.mpris.MediaPlayer2.Playlist" )
Comment 8 Alexandre Mazari 2011-09-02 07:34:23 UTC
Regarding the implementation:
- somehow cache/share the introspection proxy
- parse GDBusInterfaceInfo instead using the xml payload for introspection ?
- make the constructor async ?
Comment 9 Alan Knowles 2011-09-02 09:31:09 UTC
(In reply to comment #7)
> Here are open questions I need some advices/enlightenment about. Feel free to
> comment!
> 
> First, the API:
> - lower-case the first letter of method names ? Would look definitely better
> and feel more js.
do not mind, although I wonderif it adds a bit of a gotcha if somebody knows the original dbus interface.


> - allow several dbus interface for one object ?
Is this only really for the event handlers - I'd be tempted not to do that as it may get confusing to follow the code. (see the 'listeners' idea below though)

> - allow several handlers for each signal, property change and asyn method
> return ?
(see listeners idea)

> - add a compatibility layer with current DBus module ?
Probably a good idea, although i'm not sure how much dbus code is already out there, as the previous code was very unstable.


> - instead of providing a factory method returning a vanilia dbusproxy that is
> then augmented with handlers, make it take an object ?
Possibly confusing, and complex to maintain, but if you think it's feasible.


> ie the previous example would look something like :
> 
> player = Gio.DBusProxy.new({
>     bus:                Gio.BusType.SESSION,
>     name:               "org.mpris.MediaPlayer2.rhythmbox",
>     object_path:        "/org/mpris/MediaPlayer2",
>     interface:          "org.mpris.MediaPlayer2.Player",
>     onMetadataChanged:  function (new_value)
> {print("title:"+new_value["xesam:title"])},
>     onSeeked:           function (pos) {print("pos:"+pos);},
>     onNextReturned:     function(){ print("next"); }
> 
> );

Most of the code I work with in JS  uses listeners like this.

player = Gio.DBusProxy.new({
     bus:                Gio.BusType.SESSION,
     ....
     listeners : { 
        MetadataChanged : function () { .... } ,
        Seeked:           function (pos) {print("pos:"+pos);},
        NextReturned:     function(){ print("next"); }
     }
);

you can manually connect a listener afterwards using:
    player.on('Seeked', function() {...})

one thing to note, is that it always passes the wrapped object scope as 'this', and also always passes that as the first argument to each method.

obviously seed uses (which is a bit JQuery'y)
player.signals.Seeked.connect(function() {});

That listeners method allows you to provide a single Object to handle all the events from multiple dbus proxies. 

Just some food for thought there.

> - provide an API to add afterwards an interface to an object .
> (player.interfaces += "org.mpris.MediaPlayer2.Playlist" )

Probably needs to push(), rather than +=.. C# getting the better of you ;)
Comment 10 André Klapper 2021-05-25 17:32:13 UTC
GNOME is going to shut down bugzilla.gnome.org in favor of gitlab.gnome.org.
As part of that, we are mass-closing older open tickets in bugzilla.gnome.org
which have not seen updates for a longer time (resources are unfortunately
quite limited so not every ticket can get handled).

If you can still reproduce the situation described in this ticket in a recent
and supported software version, then please follow
  https://wiki.gnome.org/GettingInTouch/BugReportingGuidelines
and create a new enhancement request ticket at
  https://gitlab.gnome.org/GNOME/seed/-/issues/

Thank you for your understanding and your help.