RE: [battery] Which events are to be fired when (un)plug in a charger?

Hi,

> From: Kostiainen, Anssi
> Sent: Wednesday, February 10, 2016 6:15 PM
> To: Zhang, Zhiqiang <zhiqiang.zhang@intel.com>
> Cc: W3C Device APIs WG <public-device-apis@w3.org>
> Subject: Re: [battery] Which events are to be fired when (un)plug in a
> charger?
> 
> Hi Zhiqiang,
> 
> [This is also a response to my DAP-ACTION-744]
> 
> > On 29 Jan 2016, at 01:01, Zhang, Zhiqiang <zhiqiang.zhang@intel.com>
> wrote:
> >
> > Hi,
> >
> > We have 2 test files to check the events fired when a device is (un)plugged
> in a charger at
> >
> > http://w3c-test.org/battery-status/battery-plugging-in-manual.html
> >
> > http://w3c-test.org/battery-status/battery-unplugging-manual.html
> >
> > Both of them have a precondition that the battery must not be full or reach
> full capacity during the time the test is run.
> >
> > To completely run the tests, however, we have to wait a long time, or even
> cannot run them completely on some devices. To check what happened, I
> created a demo to show all events fired and charging, (dis)chargingTime,
> level readouts at
> >
> > http://zqzhang.github.io/demo/battery/example.html
> 
> This is nice aid for debugging. You could integrate it into the manual tests to
> give a sense of progress to the tester.

Yes, I will.

> I ran battery-plugging-in-manual.html and example.html side by side on Ch48
> and FF44 on OS X (note: FF44 reports level only in 0.1 granularity which is
> compliant with the spec):
> 
> Ch48:
> 
> [[
> 
> not charging: chargingTime Infinity dischargingTime 9540 level 0.85
> [charger plugged in]
> Event dischargingtimechange: not charging: chargingTime Infinity
> dischargingTime 9480 level 0.85
> Event chargingchange: charging: chargingTime Infinity dischargingTime
> Infinity level 0.85
> Event dischargingtimechange: charging: chargingTime Infinity
> dischargingTime Infinity level 0.85
> Event chargingtimechange: charging: chargingTime 3660 dischargingTime
> Infinity level 0.86
> Event levelchange: charging: chargingTime 3660 dischargingTime Infinity level
> 0.86
> Event chargingtimechange: charging: chargingTime 3540 dischargingTime
> Infinity level 0.86
> 
> [battery-plugging-in-manual.html completes at this point, with 3 pass, 1 fail:
> ondischargingtimechange_test]
> 
> ]]
> 
> Note: dischargingTime is Infinity from the third event onwards, so I'm
> wondering if the ondischargingtimechange_test failure could be prevented
> by running that test only after the battery subsystem has had time to update
> its status after the charter is plugged in? Ch48 reports not charging once after
> being plugged in.

Yes, only make that assertion after battery subsystem reports it is in charging status.

> FF44:
> 
> [[
> 
> not charging: chargingTime Infinity dischargingTime 9540 level 0.9
> [charger plugged in]
> Event dischargingtimechange: not charging: chargingTime Infinity
> dischargingTime 9480 level 0.9
> Event dischargingtimechange: not charging: chargingTime Infinity
> dischargingTime Infinity level 0.9
> Event chargingchange: charging: chargingTime Infinity dischargingTime
> Infinity level 0.9
> Event chargingtimechange: charging: chargingTime 3660 dischargingTime
> Infinity level 0.9
> Event chargingtimechange: charging: chargingTime 3540 dischargingTime
> Infinity level 0.9
> Event chargingtimechange: charging: chargingTime 3420 dischargingTime
> Infinity level 0.9
> Event chargingtimechange: charging: chargingTime 3300 dischargingTime
> Infinity level 0.9
> Event chargingtimechange: charging: chargingTime 3060 dischargingTime
> Infinity level 0.9
> Event chargingtimechange: charging: chargingTime 2940 dischargingTime
> Infinity level 0.9
> Event chargingtimechange: charging: chargingTime 2820 dischargingTime
> Infinity level 0.9
> Event chargingtimechange: charging: chargingTime 2580 dischargingTime
> Infinity level 0.9
> Event chargingtimechange: charging: chargingTime 2520 dischargingTime
> Infinity level 0.9
> Event chargingtimechange: charging: chargingTime 2400 dischargingTime
> Infinity level 0.9
> Event chargingtimechange: charging: chargingTime 2220 dischargingTime
> Infinity level 0.9
> Event chargingtimechange: charging: chargingTime 2160 dischargingTime
> Infinity level 0.9
> Event levelchange: charging: chargingTime 0 dischargingTime Infinity level 1
> Event chargingtimechange: charging: chargingTime 0 dischargingTime Infinity
> level 1
> 
> [battery-plugging-in-manual.html completes at this point, with 3 pass, 1 fail:
> onchargingchange_test]
> 
> Event chargingchange: not charging: chargingTime Infinity dischargingTime
> Infinity level 1
> Event chargingtimechange: not charging: chargingTime Infinity
> dischargingTime Infinity level 1
> 
> ]]
> 
> onchargingchange_test fails in both Ch48 and FF44 with the following
> message (in FF44 failing takes longer since level and its change is reported in
> 0.1 granularity):
> 
> [[
> 
> FAIL
> 
> When the device is plugged in and its discharging time is updated, must set
> the dischargingTime attribute's value to Infinity and fire a
> dischargingtimechange event.
> 
> assert_equals: The value of the dischargingTime attribute must be set to
> Infinity. expected Infinity but got 9480
>     at Object.<anonymous> (http://www.w3c-test.org/battery-
> status/battery-plugging-in-manual.html:62:7)
>     at Object.Test.step (http://www.w3c-
> test.org/resources/testharness.js:1382:25)
>     at BatteryManager.<anonymous> (http://www.w3c-
> test.org/resources/testharness.js:1406:35)
> 
> ]]
> 
> > I ran the example.html and battery-plugging-in-manual.html  together with
> Chrome 47 and Firefox Nightly 47 on Windows desktop, and found that:
> >
> > - Both Chrome and Firefox were able to report correctly not charging status
> before plugging in a charger, e.g. not charging: chargingTime Infinity
> dischargingTime 4327 level 0.64 (0.6 by Firefox).
> 
> I can reproduce this (see the logs).
> 
> > - Firefox fired a 'dischargingtimechange' event in not charging status to
> report a dischargingTime change, while Chrome didn't.
> 
> In my tests both fired dischargingtimechange in not charging status (see line
> 2 in both the logs).
> 
> > But they both got a fail result for the 'ondischargingtimechange_test' which
> checks that dischargingTime attribute must be set to Infinity.
> 
> Is this ondischargingtimechange_test failure due to the fact that in the first
> two events fired dischargingTime != Infinity?

Yes, that is the root cause.

Thanks,
Zhiqiang

> > - Then both Chrome and Firefox fired 'levelchange' events which always
> reported (dis)chargingTime Infinity, until the battery reached full capacity.
> 
> I can reproduce this (see the logs).
> 
> > This shall be the root cause why we have to wait a long time to run the
> tests completely.
> > - Both of them fired 'chargingtimechange' which reported 'charging:
> chargingTime 0 dischargingTime Infinity level 1'. Aha the battery is full. And
> they got a summary result of '3 pass 1 fail' for the battery-plugging-in-
> manual.html  test file.
> 
> In my test runs it seems as if the root cause is that
> assert_equals(battery.dischargingTime, Infinity) is false in
> ondischargingtimechange_test since it receives the following input:
> 
> Ch48 (line 2):
> 
> Event dischargingtimechange: not charging: chargingTime Infinity
> dischargingTime 9480 level 0.85
> 
> FF44 (line 2):
> 
> Event dischargingtimechange: not charging: chargingTime Infinity
> dischargingTime 9480 level 0.9
> 
> ... and thus fails with:
> 
> assert_equals: The value of the dischargingTime attribute must be set to
> Infinity. expected Infinity but got 9480
> 
> > Re-read the specification, I think both the 2 implementations are compliant
> with specification, especially for the note "the definition of how often the
> chargingtimechange, dischargingtimechange, and levelchange events are
> fired is left to the implementation." Then we need to revise the tests.
> 
> Hopefully this input will help you revise the test cases. I think you're correct
> in that the implementations are compliant, and what is needed is to revise
> the tests.
> 
> > What do you think?
> 
> Thanks for your help in finalising the test suite, hoping we can advance to PR
> and Rec soonish :-)
> 
> > BTW, I haven't run battery-unplugging-manual.html completely with
> Chrome; once got 4 pass with Firefox.
> 
> Thanks,
> 
> -Anssi

Received on Tuesday, 16 February 2016 07:19:16 UTC