Re: Feedback about Battery Status Events

On Wed, Oct 5, 2011 at 5:04 AM, Anssi Kostiainen
<anssi.kostiainen@nokia.com> wrote:
> Hi,
>
> On 4.10.2011, at 17.55, ext Mounir Lamouri wrote:
>
>> As part of the WebAPI effort at Mozilla [1], we did write a draft implementation of something we are calling Battery API [2] (inspired by the Battery Status Events). Those two specifications are close and we would like to give some feedback on the changes we did and the changes we might want to discuss.
>>
>> The major change we did is actually making the API synchronous: the BatteryManager object has a few attributes that give the current state of the device's battery. The DAP specification requires to use an event handler to get those information but we believe there are use cases where a developer might just want to get some information at a given moment without caring about changes and this shouldn't be harder than reading an attribute. For example, a game might disable some features if the battery is low but these features will not magically appear during the game if the device is plugged, it might change between two levels instead.
>
> The Battery Status Events was inspired by DeviceOrientation, so most of the asynchronous model was inherited from it. I agree that there are high value use cases for a synchronous API. The only concern I have with the synchronous API is potential for race conditions due to battery-related subsystems being slow to initialize, or blocking. However, your proposal seems to be fairly similar to document.readyState (and document.onreadystatechange) in its design so I believe we could handle the above concern similarly by defining default values to all the attributes. For example, the attributes could be nullable, and null could indicate uninitialized state (cf. readyState defaults to 'loading'):
>
> var b = window.navigator.mozBattery;
> console.log(b.level); // null if uninitialized
>
> b.addEventListener('levelchange'), function () {
>   console.log(b.level); // always non-null
> });

I'm not terribly worried about the case when the battery subsystem
hasn't started yet. It seems unlikely that you'll have started
applications (including browsers) quickly enough during device startup
that the battery subsystem isn't up and running yet. Possibly I could
see it happening if you are implementing the battery status UI which
is rendered as part of the default device UI since that starts
immediately on startup.

So while it can happen, it seems rare enough that we shouldn't worry
too much about it. I think I prefer Mounir's suggestion to return 100%
in that case. Any UI could be updated quickly enough anyway as soon as
the battery subsystem is up and running, by simply firing the
appropriate events and updating the status properties.

The advantage of returning 100% rather than null is that it seems that
in most cases that'll make applications how you'd like them to in many
cases.

The main use cases I've had in mind for battery are:

* An application wanting to go into low-power-usage mode when battery
is getting close to running out. For example turning off smooth
animations, or doing less frequent status updates.
* A utility application which automatically puts the phone in
low-power-usage mode when battery is getting close to running out. For
example by turning off 3G data or decreasing back-light intensity.
* A battery application displaying the current battery charge,
possibly in combination with other status information about the
device.

For the first two it seems better to default to 100% rather than null
if battery isn't accessible yet. For the last one it would be wrong,
but likely only for a few fractions of a second until the battery
subservice is up and running so I don't know that it's worth worrying
about.

>> In addition, we changed the complex set of events to something more simple: every time an attribute is changed, an event is fired. That means, there are 'levelchange', 'chargingchange' and 'statuschange' events instead of the other four events that seem a bit odd to us. The event handler can then access to BatteryManager to get the new values.
>
> Doug Turner initially proposed the Battery{Low|CriticallyLow} events which were then folded into battery{status|low|critical|ok} events. The discussion started at [1]. Your alternative proposal seems to handle all the use cases we've discussed. Only in some corner cases, such as if the app is only interested in the critically low state, some unneeded events may get fired (each time a status attribute changes its value compared to only when status changes its value to 'critical'). On the other hand, your proposal handles situations better where the app is only interested in a single attribute's value change.
>
> So whereas in the DAP proposal to get the current level:
>
> var b = new BatteryStatusEventSource();
> b.addEventListener('batterycritical', function (ev) {
>  // data in the ev of type BatteryStatusEvent
>  console.log(ev.level);
> });
>
> In your proposal:
>
> var b = window.navigator.mozBattery;
> b.addEventListener('statuschange', function () {
>  // data in the BatteryManager exposed on window.navigator.mozBattery
>  if (b.status === 'critical') {
>    console.log(b.level);
>  }
> });

Most likely you also want to be notified when you are transitioning
out of critical mode, right? So I think statuschange is better than
batterycritical. But see also below regarding if we should keep this
at all.

> I like your proposal's clearly named *change events (should they be prefixed with battery, e.g. statuschange is a rather generic name for an event?).

I agree with Mounir here. No need to make the names longer than
needed. It's clear from context that we're talking about battery.

>> Furthermore, we are adding a 'mozBattery' attribute in window.navigator [3] instead of using a constructor [4]. This attribute could be 'null' if the device has no battery. It prevents returning some specific values for the attributes when there is actually no battery. One disadvantage is that there is no way to know if a battery is hot plugged.
>
> The constructor was introduced to sidestep addEventListener() side-effects, discussion at [2] and solution at [3]. But if the event data is exposed on window.navigator.mozBattery or similar instead of on the event object I guess this is a non-issue. I.e. when the window.navigator.mozBattery.foo value is set, the UA fires a simple event named {charging|level|status}change at the BatteryManager, instead of addEventListener() triggering the event, which is considered bad.
>
> Returning null was discussed starting at [4]. Hot plugging was the main argument against returning null, it was also argued that some information about the battery should be always returned even if there's no battery (Robin may want to elaborate on that one?).

I think the idea was to return null on devices which don't use battery
at all. On my desktop machine there is no way I'm going to hot plug a
battery and start carrying it away (I probably wouldn't get past the
downstairs security guards anyway, despite 5 years of ice hockey).

On a device which has battery, but where one isn't currently plugged
in it seems appropriate to return 0 for level of charge and true for
charging. If a battery which is charged to 50% is plugged in simply
change the charge level to 50% and fire the appropriate events.

>> Finally, the Battery Status Events specification introduces a 'status' attribute that can be 'ok', 'low' or 'critical' our implementation keeps that but we might be tempted to remove it. Indeed, from where the thresholds should come from? The Battery Status Events specifications say it should be left to the implementation but how can the implementation set sensitive values? Do battery internally have a 'low' indicator? In addition, even if the UA set the thresholds, they might not fit what applications are expecting. In that case, we should probably allow those applications to set them. However, that would be very similar than just listening to level changes and put the very simple logic in the event handler.
>
> The rationale for statuses was that not all batteries are equal and that the underlying platforms typically know better when the battery's level is low or critical, depending on the decay rate and other parameters. Mapping statuses to certain level thresholds may not be an optimal approach, but is a viable implementation strategy if the platform does not expose more than that. This issue was discussed at [1] and [5].

I'm still not sure that I buy the value of ok/low/critical. If the UA
(or OS) knows that a battery decays faster towards the end, then it
should take that into account when exposing the battery level. I.e.
the battery level should represent how much battery time (mAh) is
left. Not the electrical potential difference or anything like that,
which would be affected by battery type etc.

>> One solution would be to replace the 'plugged' ('charging' in our implementation) and 'status' attribute with a 'state' attribute that could be 'charging' or 'discharging' for the moment.
>
> It is possible that the battery depletes even if it is plugged in under heavy load if the charging currency is low enough. Should the design support such a corner case? If not, your proposal is simpler, and simpler is typically better, but we should make sure it is unambiguous.

I think by having the 'level' property still available even when
.charging is true we're covering that case.

>> So, the idea of this email is to give our feedback about the Battery Status Events specification and why there are some parts of it we don't like but we also want to know why those parts are like that. We believe those were the results of some discussions and we might be missing a lot. We are eagerly waiting for your feedbacks!
>
> Thanks for your feedback and implementation, very helpful! Hopefully I was able to shed some light on the past discussions and design decisions made. If something is missing or unclear, let me know. Generally, I like your simpler proposal and would like to align the spec to your proposal once the minor issues discussed above are resolved, and no further issues or concerns are raised by other members. There's a proposed WebKit patch [6] by Kihong, so feedback from WebKittens regarding Mozilla's proposal would be helpful to get too.
>
> So let's iron out the kinks together and come up with an updated proposal soon.

I'd like to make to the original proposal in this thread:

* Remove the .status property (and the statuschange event) for the
reason stated above
* Make .level be a float between 0 and 1 instead. That tends to make
using it in math easier (the <progress> and <meter> elements have
similar APIs which go from 0 to 1).

But otherwise I agree with where this is heading :-)

/ Jonas

Received on Monday, 17 October 2011 23:17:00 UTC