Re: [battery] Battery Editors Draft update needed to prepare for LC

On 09 Jun 2014, at 13:17, Mounir Lamouri <mounir@lamouri.fr> wrote:

> On Sat, 7 Jun 2014, at 8:04, cathy.chan@nokia.com wrote:
>> A few comments on the changes...
>> 
>> In Section 5 Navigator Interface, the second step says 
>> [[
>> If those steps were previously successfully run in the current browsing
>> context ...
>> ]]
>> It is not clear what "those steps" refer to. "these steps" would actually
>> be more accurate. Alternatively, consider something like "If an instance
>> of BatteryManager has previously been created in the current browsing
>> context..."
> 
> Done.

>> In Section 6 BatteryManager Interface, the following two statements are
>> conflicting with regard to what the value of chargingTime should be when
>> the implementation is unable to report it:
>> [[
>> When a BatteryManager object is created, if the implementation is unable
>> to report the battery's charging state, charging time, level or remaining
>> time, charging MUST be set to true, ***chargingTime to 0***, level to 1.0
>> and dischargingTime to the value positive Infinity respectively.
>> ]]

The defaults spec'd herein emulate fully charged battery. This means that with these defaults, the experience is the same as if the device had full battery.

>> [[
>> The chargingTime attribute MUST be set to 0, if the battery is full or
>> there is no battery attached to the system, and ***to the value positive
>> Infinity if the battery is discharging, the implementation is unable to
>> report the remaining charging time***, or otherwise.
>> ]]

This should align with the defaults described above assuming we want to still emulate the full battery, thus chargingTime must be 0 if the implementation is unable to report the remaining charging time.

Mounir - WDYT? How do the implementations comply with the above proposal?

>> I don't remember which one was the agreed behavior, but would note that
>> the latter one works well in the multiple batteries scenario as
>> described. If the former one is used, the multiple batteries description
>> might need more tweaks.

Cathy - what are the clarification needed to support multiple batteries?

> So, the confusion is that when the battery is charging but not full and
> the system can't report the charging time, it should report it as
> Infinity. I added that precision.

>> While we're looking at the attributes of BatteryManager, I would suggest
>> swapping the order of the sentences within each paragraph. Begin with
>> what the attribute represents, followed by how its value should be set.
>> For example:
>> [[
>> The charging attribute represents the charging state of the system's
>> battery. It MUST be set to false if the battery is discharging, and set
>> to true, if the battery is charging, the implementation is unable to
>> report the state, or there is no battery attached to the system, or
>> otherwise. When the battery charging state is updated, the user agent
>> MUST queue a task which sets the charging attribute's value and fires a
>> simple event named chargingchange at the BatteryManager object.
>> ]]
>> It reads more naturally.
> 
> Hmm, I am not sure what to do here. It is purely editorial and Anssi
> wrote that originally so I would prefer to let him decide how to change
> the wording.

I moved all the descriptive text into its own paragraph after the IDL block:

https://dvcs.w3.org/hg/dap/rev/65ff0151a2e5

>> In Examples 3 and 4:
>> 	navigator.getBattery(function(battery) {
>> should be 
>> 	navigator.getBattery().then(function(battery) {
> 
> Good catch ;)
> 
>> Finally, clearly just a nit...
>> In Example 1 & 2, clearTimeout(...) should be clearInterval(...).
> 
> Eh, that wasn't part of my changes but yes indeed :)

Fixed:

https://dvcs.w3.org/hg/dap/rev/fd30ea97e342

> Thank you for the great feedback Cathy!

Indeed, thanks! Please review the changes.

-Anssi

Received on Monday, 9 June 2014 13:29:01 UTC