Bug 56600

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.

(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).

Created attachment 69379 [details] [review]
Patch to update ConnMan API usage

Updated the patch per your comments.

Comment on attachment 69379 [details] [review]
Patch to update ConnMan API usage

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

Looks good!

Pushed. Thanks :)