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 723766 - Use GDBus to implement and use ChatManager API
Use GDBus to implement and use ChatManager API
Status: RESOLVED FIXED
Product: empathy
Classification: Core
Component: General
unspecified
Other Linux
: Normal normal
: ---
Assigned To: empathy-maint
empathy-maint
Depends on:
Blocks:
 
 
Reported: 2014-02-06 14:32 UTC by Guillaume Desmottes
Modified: 2014-02-12 07:55 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
chat-manager: define CHAT_MANAGER_PATH (1.42 KB, patch)
2014-02-06 14:33 UTC, Guillaume Desmottes
none Details | Review
prefix Telepathy bus names with TP_ (6.93 KB, patch)
2014-02-06 14:33 UTC, Guillaume Desmottes
none Details | Review
add empathy-bus-names.h (7.39 KB, patch)
2014-02-06 14:33 UTC, Guillaume Desmottes
none Details | Review
generate GDBus API for the chat manager (3.73 KB, patch)
2014-02-06 14:33 UTC, Guillaume Desmottes
none Details | Review
chat-mgr: Use GDBus client side API (3.46 KB, patch)
2014-02-06 14:33 UTC, Guillaume Desmottes
none Details | Review
chat-mgr: use GDBus service API (5.42 KB, patch)
2014-02-06 14:33 UTC, Guillaume Desmottes
none Details | Review
chat-mgr: Use GDBus client side API (3.68 KB, patch)
2014-02-07 08:53 UTC, Guillaume Desmottes
none Details | Review
chat-mgr: use GDBus service API (4.87 KB, patch)
2014-02-07 08:53 UTC, Guillaume Desmottes
none Details | Review
extensions: remove Chat_Manager.xml (2.90 KB, patch)
2014-02-07 09:11 UTC, Guillaume Desmottes
none Details | Review

Description Guillaume Desmottes 2014-02-06 14:32:33 UTC
We are getting rid of extensions in next and I'd like to remove it completely. But we also have non Telepathy API using its code gen. Let's use GDBus instead.
Comment 1 Guillaume Desmottes 2014-02-06 14:33:00 UTC
Created attachment 268293 [details] [review]
chat-manager: define CHAT_MANAGER_PATH
Comment 2 Guillaume Desmottes 2014-02-06 14:33:07 UTC
Created attachment 268294 [details] [review]
prefix Telepathy bus names with TP_

So it's clearer that we are referring the Telepathy bus name and not the
GtkApplication one.
Comment 3 Guillaume Desmottes 2014-02-06 14:33:11 UTC
Created attachment 268295 [details] [review]
add empathy-bus-names.h

It's convenient to have a single file containing all the D-Bus names so a
component can easily call a method on another one.

Also rename from DBUS_NAME to BUS_NAME to stay coherent with the TP_BUS_NAME.
Comment 4 Guillaume Desmottes 2014-02-06 14:33:14 UTC
Created attachment 268296 [details] [review]
generate GDBus API for the chat manager

Plan is to get rid of our own code gen system in next.
Comment 5 Guillaume Desmottes 2014-02-06 14:33:17 UTC
Created attachment 268297 [details] [review]
chat-mgr: Use GDBus client side API
Comment 6 Guillaume Desmottes 2014-02-06 14:33:21 UTC
Created attachment 268298 [details] [review]
chat-mgr: use GDBus service API

We have now to use the GtkApplication bus name when calling the method as GBus
uses this bus name and not the Telepathy one which is used by dbus-glib.
Comment 7 Xavier Claessens 2014-02-06 16:36:50 UTC
Review of attachment 268296 [details] [review]:

::: src/empathy-audio-src.c
@@ +320,3 @@
      * 0.11/1.0, this should change so that we actually request what the encoder
      * wants downstream. */
+    caps = gst_caps_new_simple ("audio/x-raw-int",

This seems unrelated change.
Comment 8 Xavier Claessens 2014-02-06 16:42:20 UTC
Review of attachment 268297 [details] [review]:

::: src/empathy-chat-manager.c
@@ +586,3 @@
+      G_DBUS_PROXY_FLAGS_NONE, EMPATHY_CHAT_TP_BUS_NAME, CHAT_MANAGER_PATH,
+      NULL, chat_mgr_proxy_cb,
+      GINT_TO_POINTER (action_time));

You can't cast gint64 to a pointer on 32bits plateforms AFAIK. You'll probably have to g_malloc(sizeof(gint64)) or something like that.

empathy_chat_manager_call_undo_closed_chat (void) isn't supposed to be called a lot, I guess? Otherwise you would have to cache the proxy.
Comment 9 Xavier Claessens 2014-02-06 16:48:48 UTC
Review of attachment 268298 [details] [review]:

::: src/empathy-chat-manager.c
@@ +323,3 @@
   EmpathyChatManagerPriv *priv = GET_PRIV (self);
 
+  g_dbus_interface_skeleton_unexport (G_DBUS_INTERFACE_SKELETON (object));

GDBusInterfaceSkeleton's finalize does it for you already.

@@ +397,3 @@
+empathy_chat_manager_constructed (GObject *obj)
+{
+  g_bus_get (G_BUS_TYPE_SESSION, NULL, get_bus_cb, g_object_ref (obj));

You can do it sync here, GApplication will have created it already.
Comment 10 Guillaume Desmottes 2014-02-07 08:48:20 UTC
(In reply to comment #7)
> Review of attachment 268296 [details] [review]:
> 
> ::: src/empathy-audio-src.c
> @@ +320,3 @@
>       * 0.11/1.0, this should change so that we actually request what the
> encoder
>       * wants downstream. */
> +    caps = gst_caps_new_simple ("audio/x-raw-int",
> 
> This seems unrelated change.

Indeed; reverted.

(In reply to comment #8)
> Review of attachment 268297 [details] [review]:
> 
> ::: src/empathy-chat-manager.c
> @@ +586,3 @@
> +      G_DBUS_PROXY_FLAGS_NONE, EMPATHY_CHAT_TP_BUS_NAME, CHAT_MANAGER_PATH,
> +      NULL, chat_mgr_proxy_cb,
> +      GINT_TO_POINTER (action_time));
> 
> You can't cast gint64 to a pointer on 32bits plateforms AFAIK. You'll probably
> have to g_malloc(sizeof(gint64)) or something like that.

Oh yeah good point. I used a GVariant because this is 2014 and mallocing integer is so 70s.

> empathy_chat_manager_call_undo_closed_chat (void) isn't supposed to be called a
> lot, I guess? Otherwise you would have to cache the proxy.

Not really. We never cached it and it was fine so far so...
Comment 11 Guillaume Desmottes 2014-02-07 08:52:57 UTC
(In reply to comment #9)
> Review of attachment 268298 [details] [review]:
> 
> ::: src/empathy-chat-manager.c
> @@ +323,3 @@
>    EmpathyChatManagerPriv *priv = GET_PRIV (self);
> 
> +  g_dbus_interface_skeleton_unexport (G_DBUS_INTERFACE_SKELETON (object));
> 
> GDBusInterfaceSkeleton's finalize does it for you already.

Oh cool; removed.

> @@ +397,3 @@
> +empathy_chat_manager_constructed (GObject *obj)
> +{
> +  g_bus_get (G_BUS_TYPE_SESSION, NULL, get_bus_cb, g_object_ref (obj));
> 
> You can do it sync here, GApplication will have created it already.

Oh yeah good idea; thanks!
Comment 12 Guillaume Desmottes 2014-02-07 08:53:02 UTC
Created attachment 268383 [details] [review]
chat-mgr: Use GDBus client side API
Comment 13 Guillaume Desmottes 2014-02-07 08:53:06 UTC
Created attachment 268384 [details] [review]
chat-mgr: use GDBus service API

We have now to use the GtkApplication bus name when calling the method as GBus
uses this bus name and not the Telepathy one which is used by dbus-glib.
Comment 14 Guillaume Desmottes 2014-02-07 09:11:24 UTC
Created attachment 268388 [details] [review]
extensions: remove Chat_Manager.xml

We use GDBus now.
Comment 15 Xavier Claessens 2014-02-11 19:53:03 UTC
+1
Comment 16 Guillaume Desmottes 2014-02-12 07:55:52 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.