Log active use time between kernel crashes.

Log active use time between kernel crashes.

Also, initialize the network state from flimflam, just in case -- now
that the metrics daemon process starts a bit late,

BUG=none
TEST=unit tests, ran on the device, emerged arm-generic

[petkov, 2010-06-17 22:46:40]

... and the last in the series of CLs for logging time between kernel crashes.

[petkov, 2010-06-22 18:45:06]

ping...

[petkov, 2010-06-22 23:43:10]

Sam, could you please review the flimflam dbus stuff. Thanks!

[kmixter1, 2010-06-22 23:55:33]

LGTM with Sam's check on the connman interface stuff

otherwise a few nits.

http://codereview.chromium.org/2864009/diff/2001/3001
File Makefile (left):

http://codereview.chromium.org/2864009/diff/2001/3001#oldcode76
Makefile:76: metrics_daemon.o: \
Did you intentionally remove these extra dependencies?

http://codereview.chromium.org/2864009/diff/2001/3002
File metrics_daemon.cc (right):

http://codereview.chromium.org/2864009/diff/2001/3002#newcode130
metrics_daemon.cc:130: if (dbus_error_is_set (&error) || !reply) {
Does timeout fall into this case?  Also that function blocks all reception so
any other external notifications will be delayed until this RPC returns. You
might want to mention that in this function's comments to avoid widespread use
of it.  Unless 2s is always a tolerable amount of noise in measurements.

http://codereview.chromium.org/2864009/diff/2001/3002#newcode220
metrics_daemon.cc:220: string state_name = DBusGetString(connection,
"org.chromium.flimflam", "/",
deferring to sleffler on this one

http://codereview.chromium.org/2864009/diff/2001/3002#newcode405
metrics_daemon.cc:405: FilePath crash_detected(crash_file);
I'd probably do this check outside the function and then call the function
ProcessKernelCrash to be analogous to ProcessKernelCrash.

[Sam Leffler, 2010-06-23 18:20:43]

dbus stuff LGTM modulo nit/question

http://codereview.chromium.org/2864009/diff/2001/3002
File metrics_daemon.cc (right):

http://codereview.chromium.org/2864009/diff/2001/3002#newcode132
metrics_daemon.cc:132: return "";
Do you want to return something different like "offline"?  I see "" is handled
but if you cannot reach flimflam it might be best to assume you're offline.

[petkov, 2010-06-24 18:18:35]

Address comments, I think. Unless there're more comments in the next few
minutes, I'll push.

http://codereview.chromium.org/2864009/diff/2001/3001
File Makefile (left):

http://codereview.chromium.org/2864009/diff/2001/3001#oldcode76
Makefile:76: metrics_daemon.o: \
On 2010/06/22 23:55:33, kmixter1 wrote:
> Did you intentionally remove these extra dependencies?

Yes, they're not complete anyway -- and I don't think this is the right way to
do .h deps.

And when you ebuild/emerge, you usually build from scratch so it's irrelevant.

http://codereview.chromium.org/2864009/diff/2001/3002
File metrics_daemon.cc (right):

http://codereview.chromium.org/2864009/diff/2001/3002#newcode130
metrics_daemon.cc:130: if (dbus_error_is_set (&error) || !reply) {
On 2010/06/22 23:55:33, kmixter1 wrote:
> Does timeout fall into this case?  Also that function blocks all reception so
> any other external notifications will be delayed until this RPC returns. You
> might want to mention that in this function's comments to avoid widespread use
> of it.  Unless 2s is always a tolerable amount of noise in measurements.

I've added a comment. Currently this function is used once at startup. And,
normally, the response is returned right away. If Sam moves the TimeToDrop
metric to flimflam, we can also get rid of this function.

http://codereview.chromium.org/2864009/diff/2001/3002#newcode132
metrics_daemon.cc:132: return "";
On 2010/06/23 18:20:43, Sam Leffler wrote:
> Do you want to return something different like "offline"?  I see "" is handled
> but if you cannot reach flimflam it might be best to assume you're offline.

"" is marked as unknown network state which for all practical purposes is
equivalent to offline.

http://codereview.chromium.org/2864009/diff/2001/3002#newcode405
metrics_daemon.cc:405: FilePath crash_detected(crash_file);
On 2010/06/22 23:55:33, kmixter1 wrote:
> I'd probably do this check outside the function and then call the function
> ProcessKernelCrash to be analogous to ProcessKernelCrash.

Done.