[Bug 24385] Implement the Capabilities interface

[bugzilla-daemon at freedesktop.org]

http://bugs.freedesktop.org/show_bug.cgi?id=24385





--- Comment #1 from Will Thompson <will.thompson at collabora.co.uk>  2009-10-11 09:05:02 PST ---
+    haze_connection_capabilities_class_init(object_class);

Coding style: space before (. By the way, thanks for following the
Telepathy style in new files, and the Haze style (which I regret being
different ;-)) in existing files.

tp_flags_to_purple_caps and purple_caps_to_tp_flags:

+  caps |= (flags & TP_CHANNEL_MEDIA_CAPABILITY_AUDIO) != 0 ?
+      PURPLE_MEDIA_CAPS_AUDIO : 0;

What's wrong with:

    if (flags & TP_CHANNEL_MEDIA_CAPABILITY_AUDIO)
      caps |= PURPLE_MEDIA_CAPS_AUDIO;

?

haze_connection_advertise_capabilities: please don't copy-paste
incorrect docstrings. :)

+    }
+  for (i = 0; NULL != del[i]; i++)

Style pedantry: please leave a blank line after the end of a block
before the next statement (if it's not an 'else'). I think it makes the
code easier to follow.

+  g_timeout_add (10000, caps_received_cb_cb, crd);

As a general point, it's better to use g_timeout_add_seconds() where
possible to avoid wakeups. But this is just a hack so I'm not too
bothered. :)

But: if the PurpleConnection is disconnected before the 10 seconds are
up, we'll crash when the callback is called.

You never disconnect from PurpleMediaManager:init-media.

I'm inclined to say that haze_media_manager_channel_class (); should
have a static hash table containing the channel class, but I think this
might just be a note from the department of premature optimization. :-)

+  value = tp_g_value_slice_new (G_TYPE_STRING);
+  g_value_set_static_string (value, TP_IFACE_CHANNEL_TYPE_STREAMED_MEDIA);
+  g_hash_table_insert (table, TP_IFACE_CHANNEL ".ChannelType", value);

could just become

    tp_asv_set_static_string (table, TP_IFACE_CHANNEL ".ChannelType",
        TP_IFACE_CHANNEL_TYPE_STREAMED_MEDIA);

In fact, the whole function could become:

    return tp_asv_new (
        TP_IFACE_CHANNEL ".ChannelType", G_TYPE_STRING,
            TP_IFACE_CHANNEL_TYPE_STREAMED_MEDIA,
        TP_IFACE_CHANNEL ".TargetHandleType", G_TYPE_UINT,
            TP_HANDLE_TYPE_CONTACT,
        NULL);

:-)

(The code in Gabble's channel managers pre-dates tp_asv_new() and
tp_asv_set_*().)

TBH. it's probably worth copying gabble_g_ptr_array_copy () into a new
util.c in Haze (or indeed putting it in tp-glib, which already contains
tp_g_ptr_array_contains ()).


-- 
Configure bugmail: http://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.