Re: Feedback about Battery Status Events

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
});

> 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);
  }
});

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?).

> 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?).

> 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].

> 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.

> 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.

-Anssi

> [1] https://wiki.mozilla.org/WebAPI
> [2] https://wiki.mozilla.org/WebAPI/BatteryAPI
> [3] Ideally, that should be window.navigator.devices
> [4] We are actually looking for large feedback about which pattern should be usually preferred. There are a few new APIs we are working on that use an object in navigator and some have a constructor.


[1] http://lists.w3.org/Archives/Public/public-device-apis/2011Jun/0058.html
[2] http://lists.w3.org/Archives/Public/public-device-apis/2011Sep/0025.html
[3] http://lists.w3.org/Archives/Public/public-device-apis/2011Sep/0061.html
[4] http://lists.w3.org/Archives/Public/public-device-apis/2011Sep/0063.html
[5] http://lists.w3.org/Archives/Public/public-device-apis/2011Sep/0003.html
[6] https://bugs.webkit.org/show_bug.cgi?id=62698

Received on Wednesday, 5 October 2011 12:05:32 UTC