Bug 38978

MC has a GInterface for network states, McdTransport.

On the Nokia N900, there's an implementation of this for Nokia's proprietary ConIc service (which is analogous to NM and ConnMan). Sadly, there are no open source implementations of the interface, but the code to make use of the connectivity information is mainly in MC itself (which of course is Free). So Empathy's code could potentially be transplanted into an McdTransport implementation within the MC tree.

Looks good, it will use a bit more CPU, but I guess it's not dramatic. And we have to remember to remove it if/when we move the UI to something like clutter (it's cheaper to have the CPU do the flippage)

arg, wrong bug.. 
What I mean to say here is: don't forget my pet bug: Empathy goes back online even if it was disconnected by connecting somewhere else and then NM goes online->offline->online..

Created attachment 48825 [details]
generic example of how an mcd transport plugin is formed

Assigning to Will as he volunteer to fix during the BoF so he can't step back now. :p

Here are some preliminary uncontroversial tweaks to MC's tests and build system: http://cgit.collabora.com/git/user/wjt/telepathy-mission-control-wjt.git/log/?h=uncontroversial

I have functioning ConnMan and Network Manager support, and functioning tests for it, except that one of my test changes seems to expose a race in a completely unrelated test.

uncontroversial branch is uncontroversial. ++

It is done.

There are still verrrrry occasional bugs in the test suite, due to the tests taking a match-all match rule plus the issue I keep hitting where libdbus doesn't check the sender of method replies. The symptom I hit a few times is that one head of the test suite calls RequestName; another head replies to a ConnMan connectivity query with "offline"; and libdbus misconstrues the "offline" reply as a reply from the bus daemon to the call to RequestName! This is unfixable without removing the match-all rule and figuring out how to avoid races and keep things coherent in the tests with >1 bus connection, which I can't bring myself to do right now.

A word on the design of the branch: I chose to keep the NM/ConnMan-tracking code separate from the McdTransportPlugin implementation, with a view to making it easier, in the future, to move to a better internal API for this stuff without having to make invasive changes to the actual tracking code. McdKludgeTransport is basically boilerplate glue between McdConnectivityMonitor and the guts of MC.

(In reply to comment #7)
> uncontroversial branch is uncontroversial. ++

Merged it, ta.

Looks great; you owned with those tests!


+ * Copyright (C) 2009 Collabora Ltd.
should be updated

Would be good to open a nm-glib bug report as in a perfect world MC shouldn't do blocking calls.

McdKludgeTransport: why "kludge"?


+    GList *one;
I'd add a comment explaining what's this; it's not that clear to me.


  return "i love the internet";
rly? :)

Did you test it with a NM enabled Empathy? It's a bit late to remove this code for 3.2 (release on Monday...).

(In reply to comment #11)
> Looks great; you owned with those tests!

I aim to please.

> + * Copyright (C) 2009 Collabora Ltd.
> should be updated

Done. I'll squash this back. I updated the .h but forgot to update the .c.

> Would be good to open a nm-glib bug report as in a perfect world MC shouldn't
> do blocking calls.

I guess the counter-argument will be that one blocking call during initialization is not bad enough to warrant async-ifying the entire API. This is an unusual case. I actually tried making the fake NM and ConnMan services run in a separate process from the test case, which would (I think) be better in any case, but that fails due to heinous crimes committed by the MC test suite.

> McdKludgeTransport: why "kludge"?

For want of a better name, really.

> +    GList *one;
> I'd add a comment explaining what's this; it's not that clear to me.

I have done so—I've renamed the variable too.

>   return "i love the internet";
> rly? :)

nyanyanyanyanyanyanyanyanyanyan

(In reply to comment #13)
> > + * Copyright (C) 2009 Collabora Ltd.
> > should be updated
> 
> Done. I'll squash this back. I updated the .h but forgot to update the .c.

yeah fixup the fixup. :)


Looks good to me but you mentionned some issues when not using
ConnectAutomatically?

(In reply to comment #14)
> (In reply to comment #13)
> > > + * Copyright (C) 2009 Collabora Ltd.
> > > should be updated
> > 
> > Done. I'll squash this back. I updated the .h but forgot to update the .c.
> 
> yeah fixup the fixup. :)

I'll fix it up before merging; I just wanted to make it clear what I'd changed.

> Looks good to me but you mentionned some issues when not using
> ConnectAutomatically?

Yes: accounts didn't actually get reconnected if ¬ConnectAutomatically. So I've pushed a pile more patches that make that part work.

I've tested it with a version of Empathy that listens to Network Manager, and they seem to work fine together. It also works if Empathy is not running.

Looks pretty good. Thanks for these very nicely separated and clear commits, there were a joy to review. :)

Merged! This will be in 5.9.3. :D