Bug 32844 - Allow Gabble plugins to provide additional TpChannelManager implementations
Summary: Allow Gabble plugins to provide additional TpChannelManager implementations
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: gabble (show other bugs)
Version: git master
Hardware: Other All
: medium enhancement
Assignee: Telepathy bugs list
QA Contact: Telepathy bugs list
URL: http://cgit.freedesktop.org/~jonny/te...
Whiteboard:
Keywords: patch
: 34247 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-01-05 03:23 UTC by Will Thompson
Modified: 2011-04-04 08:07 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments

Description Will Thompson 2011-01-05 03:23:22 UTC
Currently Gabble plugins can provide sidecars, and new statuses (using privacy lists). They can't implement additional channel managers; nor are they notified of connections' existence unless someone requests one of their sidecars.

Gabble hands a GPtrArray of implementations of TpChannelManager from its _gabble_connection_create_channel_managers() function (which implements create_channel_managers in TpBaseConnectionClass). A corresponding method could be added to GabblePluginInterface:

    typedef GPtrArray * (*GabblePluginCreateChannelManagersImpl) (
        GabblePlugin *self,
        GabbleConnection *connection);

    struct _GabblePluginInterface {
        // ...

        GabblePluginCreateChannelManagersImpl create_channel_managers;
    };

and a method would be added to GabblePluginLoader:

    GPtrArray *gabble_plugin_loader_create_channel_managers (
        GabblePluginLoader *self,
        GabbleConnection *connection);

This function, rather like gabble_plugin_loader_create_sidecar, would iterate across all plugins and, if present, call their create_channel_managers implementation. It would concatenate all the results and return them. Then _gabble_connection_create_channel_managers() could call this function and add its result to the GPtrArray that it gives to tp-glib.

It'd be convenient to allow plugins to return NULL from create_channel_managers, so plugins that wanted to piggy-back onto connections without actually implementing any channel classes could implement this method, get called for each new connection, and just return NULL.

(I would normally think that this method should return GList *, because concatenating GPtrArrays is tedious and clumsy. Plus this gives us the NULL-return thing for free, since the empty GList is just a NULL pointer. But the telepathy-glib method at the top of it all wants a GPtrArray, so … I could be convinced either way.)
Comment 1 Will Thompson 2011-02-14 03:52:17 UTC
*** Bug 34247 has been marked as a duplicate of this bug. ***
Comment 2 Will Thompson 2011-02-14 04:32:11 UTC
Review for Jonny's implementation of this:

In gabble_plugin_loader_create_channel_managers:

+
+static void
+copy_to_other_array (gpointer data,
+    gpointer user_data)
+{
+  g_ptr_array_add (user_data, data);
+}
...
+      g_ptr_array_foreach (managers, copy_to_other_array, out);
+      g_ptr_array_free (managers, TRUE);
                                   ^^^^

In _gabble_connection_create_channel_managers:

+static void
+add_to_array (gpointer data,
+    gpointer user_data)
+{
+  g_ptr_array_add (user_data, data);
+}
+
...
+  g_ptr_array_foreach (tmp, add_to_array, channel_managers);
+  g_ptr_array_free (tmp, FALSE);
                          ^^^^^

So you wrote this function twice, and one of your two implementations is buggy. This is a pretty compelling argument for writing gabble_ptr_array_append() or similar.

+  const gchar * const empty[] = { "omg", "hi mum!", NULL };

You have a new and interesting definition of empty.

You didn't add checking for this stuff to tests/twisted/sidecars.py (or I guess to a new plugin — this isn't really to do with sidecars).

> Question[1]: I've removed the assertion that all channel managers are also
> GabbleCapsChannelManagers. Is this alright or shall we add this to the plugin
> API too? I'd say it's fine for now.

Sure, I think that's fine. (But yeah, we'll probably need this later.)

I suppose theoretically we should probably have a way to add this stuff to <http://telepathy.freedesktop.org/spec/Protocol.html#Property:RequestableChannelClasses>, too...
Comment 3 Jonny Lamb 2011-03-02 09:08:12 UTC
(In reply to comment #2)
> So you wrote this function twice, and one of your two implementations is buggy.
> This is a pretty compelling argument for writing gabble_ptr_array_append() or
> similar.

As I said in the salut bug, having such a helper function wouldn't actually help in this case (although see bug #34249). I've fixed the wrong one.

> You have a new and interesting definition of empty.

I do. Fixed.

> You didn't add checking for this stuff to tests/twisted/sidecars.py (or I guess
> to a new plugin — this isn't really to do with sidecars).

Done.

All pushed to my branch. CZECH IT OWWWWT.
Comment 4 Jonny Lamb 2011-04-04 00:35:06 UTC
I added another patch to the branch to please Andre who was complaining that there was no way to get the WockySession.
Comment 5 Jonny Lamb 2011-04-04 08:07:49 UTC
le finally done.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.