Bug 46358 - In python environment, list_connection_managers passes an empty list as cms.
Summary: In python environment, list_connection_managers passes an empty list as cms.
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: tp-glib (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Simon McVittie
QA Contact: Telepathy bugs list
URL: http://cgit.freedesktop.org/~smcv/tel...
Whiteboard: review+
Keywords: patch
Depends on:
Blocks: tp-glib-1.0
  Show dependency treegraph
 
Reported: 2012-02-20 10:11 UTC by Kai Huuhko
Modified: 2012-04-11 03:23 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
[01/10] Expect to find service files in the srcdir, not the builddir (1.79 KB, patch)
2012-03-08 03:20 UTC, Simon McVittie
Details | Splinter Review
[02/10] tests/dbus/cm: actually run the test that lists connection managers (838 bytes, patch)
2012-03-08 03:20 UTC, Simon McVittie
Details | Splinter Review
[03/10] cm test: free things in a less verbose way (3.17 KB, patch)
2012-03-08 03:21 UTC, Simon McVittie
Details | Splinter Review
[04/10] CM test: after listing CMs, inspect them in the test, not the callback (3.92 KB, patch)
2012-03-08 03:22 UTC, Simon McVittie
Details | Splinter Review
[05/10] _tp_object_list_copy, _tp_object_list_free: add (4.43 KB, patch)
2012-03-08 03:22 UTC, Simon McVittie
Details | Splinter Review
[06/10] tp_list_connection_managers_async: add (5.54 KB, patch)
2012-03-08 03:23 UTC, Simon McVittie
Details | Splinter Review
[07/10] Test listing CMs the new way (2.59 KB, patch)
2012-03-08 03:23 UTC, Simon McVittie
Details | Splinter Review
[08/10] tp_protocol_dup_params, tp_protocol_dup_param, tp_protocol_borrow_params: add (6.71 KB, patch)
2012-03-08 03:23 UTC, Simon McVittie
Details | Splinter Review
[09/10] tp_connection_manager_dup_protocols: add (5.95 KB, patch)
2012-03-08 03:24 UTC, Simon McVittie
Details | Splinter Review
[10/10] Add a Python example which lists and inspects protocols with g-i (2.86 KB, patch)
2012-03-08 03:25 UTC, Simon McVittie
Details | Splinter Review

Description Kai Huuhko 2012-02-20 10:11:20 UTC
In tp-glib, python environment, calling list_connection_managers; it passes to the callback cms, n_cms and others. (see URL for spec).

n_cms is passed with the correct value, however cms is passed as an empty list.
Comment 1 Simon McVittie 2012-02-22 11:46:42 UTC
(In reply to comment #0)
> n_cms is passed with the correct value, however cms is passed as an empty list.

I think the right solution for this is to add a tp_list_connection_managers_async()/tp_list_connection_managers_finish() pair which is nicely introspectable and does sensible things (use GAsyncResult, etc.), and eventually deprecate/remove tp_list_connection_managers.

(Also, we can take this opportunity to make it always-async, i.e. guarantee to call the callback from the main loop, and never be re-entrant.)

I've written a simple tp_list_connection_managers_async() in terms of tp_list_connection_managers(), but they should probably have common code rather than one calling the other.

Also, we need to decide what _finish() returns in C - GList? GPtrArray? (transfer container)? (transfer full)? See Bug #39189.
Comment 2 Simon McVittie 2012-03-08 03:20:29 UTC
Created attachment 58163 [details] [review]
[01/10] Expect to find service files in the srcdir, not the  builddir

This fixes the /cm/list test in out-of-tree builds, or rather, would
fix it if that test had ever worked (which is fixed by the next commit).
Comment 3 Simon McVittie 2012-03-08 03:20:49 UTC
Created attachment 58164 [details] [review]
[02/10] tests/dbus/cm: actually run the test that lists   connection managers

It turns out this was my fault, back in 2009. Oops.
Comment 4 Simon McVittie 2012-03-08 03:21:04 UTC
Created attachment 58165 [details] [review]
[03/10] cm test: free things in a less verbose way
Comment 5 Simon McVittie 2012-03-08 03:22:25 UTC
Created attachment 58166 [details] [review]
[04/10] CM test: after listing CMs, inspect them in the test,  not the callback

This will make it easier to re-use the same test for different ways to
list CMs.
Comment 6 Simon McVittie 2012-03-08 03:22:52 UTC
Created attachment 58167 [details] [review]
[05/10] _tp_object_list_copy, _tp_object_list_free: add
Comment 7 Simon McVittie 2012-03-08 03:23:14 UTC
Created attachment 58168 [details] [review]
[06/10] tp_list_connection_managers_async: add

This is a modern, bindable version of tp_list_connection_managers(),
which has been noted not to work in Python.
Comment 8 Simon McVittie 2012-03-08 03:23:29 UTC
Created attachment 58169 [details] [review]
[07/10] Test listing CMs the new way
Comment 9 Simon McVittie 2012-03-08 03:23:56 UTC
Created attachment 58170 [details] [review]
[08/10] tp_protocol_dup_params, tp_protocol_dup_param,  tp_protocol_borrow_params: add

tp_protocol_dup_params might be less nice for C (you have to free the
list and the items), but is definitely nicer for Python and other g-i
bindings. It will probably be renamed to ...get_params in Telepathy 1.0.
 
tp_protocol_borrow_params is an efficient "C binding", but not as GObject'y.

tp_protocol_dup_param will replace get_param in Telepathy 1.0.
Comment 10 Simon McVittie 2012-03-08 03:24:25 UTC
Created attachment 58171 [details] [review]
[09/10] tp_connection_manager_dup_protocols: add

Again, this provides a more introspectable way to get the protocols.
Combining dup_protocol_names() and get_protocol() is more type-safe,
but less obvious.
Comment 11 Simon McVittie 2012-03-08 03:25:22 UTC
Created attachment 58172 [details] [review]
[10/10] Add a Python example which lists and inspects  protocols with g-i

I could get used to this "rapid prototyping in Python" thing.
Comment 12 Jonny Lamb 2012-03-08 12:42:03 UTC
This all looks good.
Comment 13 Simon McVittie 2012-04-11 03:23:57 UTC
Was fixed in 0.17.6


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.