Bug 56600 - Connman API changed
Summary: Connman API changed
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: mission-control (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Telepathy bugs list
QA Contact: Telepathy bugs list
URL:
Whiteboard:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2012-10-31 04:38 UTC by Mike Ruprecht
Modified: 2012-11-01 22:14 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Update Connman API usage (4.10 KB, patch)
2012-10-31 04:38 UTC, Mike Ruprecht
Details | Splinter Review
Patch to update ConnMan API usage (6.34 KB, patch)
2012-11-01 06:16 UTC, Mike Ruprecht
Details | Splinter Review

Description Mike Ruprecht 2012-10-31 04:38:26 UTC
Created attachment 69339 [details] [review]
Update Connman API usage

Connman no longer has the GetState method nor StateChanged signal. The attached patch updates it to use the State property and PropertyChanged signal.
Comment 1 Guillaume Desmottes 2012-10-31 08:40:05 UTC
Comment on attachment 69339 [details] [review]
Update Connman API usage

Review of attachment 69339 [details] [review]:
-----------------------------------------------------------------

Please attach a proper git commit (git format-patch) so I can review the commit message as well.

The fact that you didn't have to change fakeconnectivity.py indicates that this code is poorly tested. Any chance to extend the test?

::: src/connectivity-monitor.c
@@ +194,2 @@
>      {
> +      if (g_hash_table_lookup_extended (props, "State", NULL, &val))

Couldn't you use tp_asv_get_string().

@@ +301,4 @@
>            "net.connman.Manager");
>  
>        dbus_g_object_register_marshaller (
> +          g_cclosure_marshal_generic,

Smcv: is it ok to use the generic marshaller here? The _mcd_ext_marshal_* don't seem to be used any more.
Comment 2 Simon McVittie 2012-10-31 12:20:32 UTC
(In reply to comment #1)
> >        dbus_g_object_register_marshaller (
> > +          g_cclosure_marshal_generic,
> 
> Smcv: is it ok to use the generic marshaller here? The _mcd_ext_marshal_*
> don't seem to be used any more.

Yes, this is fine (see bug #46523).
Comment 3 Mike Ruprecht 2012-11-01 06:16:34 UTC
Created attachment 69379 [details] [review]
Patch to update ConnMan API usage

Updated the patch per your comments.
Comment 4 Guillaume Desmottes 2012-11-01 09:01:05 UTC
Comment on attachment 69379 [details] [review]
Patch to update ConnMan API usage

Review of attachment 69379 [details] [review]:
-----------------------------------------------------------------

Looks good!
Comment 5 Mike Ruprecht 2012-11-01 22:14:45 UTC
Pushed. Thanks :)


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.