1163245 - [Battery] Implementation of battery discharging remaining time

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Zibi Braniecki [:zbraniecki][:gandalf]]

+++ This bug was initially created as a clone of Bug #1116368 +++

This bug is to discuss the implementation of battery discharging remaining time.
Currently, we didn't have calculation of battery discharging remaining time, there are only two values, kUnknownRemainingTime and kDefaultRemainingTime.
https://dxr.mozilla.org/mozilla-central/source/hal/gonk/GonkHal.cpp?from=GonkHal.cpp&case=true#670-672

Comment 1

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: patch

Dave, this patch is parallel to patch from bug 1116368.

I tested it on Nexus 5 and it seems to work well. I'd like to use it for plotting battery chart over time.

Comment 2

[Zibi Braniecki [:zbraniecki][:gandalf]]

Just a reminder for myself: it seems that switching charging state uses previous state for calculation. We probably need to reset the previous state when charging changes.

Comment 3

[Dave Hylands [:dhylands]]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #2)
> Just a reminder for myself: it seems that switching charging state uses
> previous state for calculation. We probably need to reset the previous state
> when charging changes.

lastLevelChange is cleared whenever the state changes from charging to discharging (or vice-versa).

I don't see the static variable remainingTime being reset though.

Comment 4

[Dave Hylands [:dhylands]]

Comment on attachment 8603679 [details] [diff] [review]
patch

Review of attachment 8603679 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good except for the point about remainingTime.

::: hal/gonk/GonkHal.cpp
@@ +691,5 @@
> +
> +    } else {
> +      clock_gettime(CLOCK_MONOTONIC, &now);
> +      dtime = now.tv_sec - lastLevelChange.tv_sec;
> +      if (dtime < remainingTime) {

It seems like remainingTime might not be set properly if you change state from charging to dischanrging and aBatteryInfo->level() == previousLevel

remainingTime would contain the remaining time to reach full charge, not the remainingTime until discharged. This would autocorrect fairly quickly, but that's the only case I don't see handled properly.

Comment 5

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: patch v2

Updated the patch to reset remainingTime. Carrying r+

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/efbea75178d2

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/efbea75178d2