Last Comment Bug 699743 - Implement battery.chargingTime
: Implement battery.chargingTime
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla11
Assigned To: Mounir Lamouri (:mounir)
:
:
Mentors:
Depends on:
Blocks: 678694 700261 702511
  Show dependency treegraph
 
Reported: 2011-11-04 03:15 PDT by Mounir Lamouri (:mounir)
Modified: 2011-11-15 14:45 PST (History)
5 users (show)
mounir: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Add .chargingTime in the DOM (8.09 KB, patch)
2011-11-07 04:05 PST, Mounir Lamouri (:mounir)
jonas: review+
asa: approval‑mozilla‑aurora-
Details | Diff | Splinter Review
Updated test page with new functionality (1.27 KB, text/plain)
2011-11-10 19:18 PST, John Hammink
no flags Details

Description Mounir Lamouri (:mounir) 2011-11-04 03:15:52 PDT
This has been added to the specs while implementing the first version of Battery API. The only issue I see with this new property is that it will tell the author whether or not the device has a battery coupled to battery.dischargingTime (both will return Infinity if there is no battery).
Comment 1 Mounir Lamouri (:mounir) 2011-11-07 04:05:28 PST
Created attachment 572426 [details] [diff] [review]
Add .chargingTime in the DOM
Comment 2 Jonas Sicking (:sicking) No longer reading bugmail consistently 2011-11-07 12:24:35 PST
Comment on attachment 572426 [details] [diff] [review]
Add .chargingTime in the DOM

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

r=me
Comment 3 Mounir Lamouri (:mounir) 2011-11-09 06:23:21 PST
Comment on attachment 572426 [details] [diff] [review]
Add .chargingTime in the DOM

This has just landed in mozilla-inbound. Requesting aurora approval for API completeness in Firefox 10. It would be better to ship Firefox 10 with the entire API if we say it is present. The risk is very low given that there is no use of the API for the moment.
Comment 4 Marco Bonardo [::mak] 2011-11-10 03:10:18 PST
https://hg.mozilla.org/mozilla-central/rev/42f490a7566a
Comment 5 John Hammink 2011-11-10 19:18:06 PST
Created attachment 573723 [details]
Updated test page with new functionality

This is the test page I'm presently using (and planning to do so with our community also) to test battery api in it's most recent state.     

Implements .chargingTime and .dischargingTime
Comment 6 John Hammink 2011-11-11 14:10:36 PST
"This has been added to the specs while implementing the first version of Battery API. The only issue I see with this new property is that it will tell the author whether or not the device has a battery coupled to battery.dischargingTime (both will return Infinity if there is no battery)."

I'm not sure this is working as it should.  In my battery app (shown above) both .dischargingTime and .chargingTime show Infinity. Even though there is a battery and it's reading other values from it.  

Is there something else I must do explicitly in my JS code to couple the battery to b.dischargingTime/.chargingTime ?
Comment 7 Mounir Lamouri (:mounir) 2011-11-12 00:51:32 PST
Please, do not reopen a bug if you see something isn't working, open a new bug instead.

However, I believe this is working (even if not perfect). The issue is that on Android, we get a battery update every 1% change which means we have to get 2% change before being able to estimate the charging/discharging time. This can be a bit long. In addition, we do not listen to battery events when we are on background which makes thing a bit more long (and wrong sometimes).

So please, check that your level correctly increase/decrease and when you have at least 2% change, one of the dischargingTime/chargingTime values should change from Infinity to something. At least, I can confirm it's working on my phone.
Comment 8 Asa Dotzler [:asa] 2011-11-15 14:45:22 PST
Comment on attachment 572426 [details] [diff] [review]
Add .chargingTime in the DOM

It looks like this feature wasn't ready for 10. Please re-nominate if you have a good justification for trying to get this into 10 rather than just waiting for 11.

Note You need to log in before you can comment on or make changes to this bug.