Should the enum SensorState have nosensor ?

[riju]

@timvolodine asked : should there be something corresponding to 'there is no relevant hardware
sensor'?

See #104 (comment) for resolutions.

[riju]

I think "errored" state suffices as of now, till we work on the Discovery part (with remote sensors) in more detail in v2.
irrc @tobie said that if the api-consumers want to know more, they should listen to the onerror event and get additional information from it.

So, idle -> errored, when there is no relevant hardware sensor.

[tobie]

Given we're focusing on local sensors only at this point, the absence or presence of sensors should be know upfront. In this case, the constructor would throw (this is already specified in the spec). If there are concrete cases where sensors go MIA, then we can report this with an error event. Is this something that's already showing up during implementation, @riju?

[anssiko]

It might not be possible to implement the constructor in a non-blocking manner if we require implementations to throw a TypeError if there's no sensor, as spec'd in the Construct Sensor Object:

5. Otherwise, throw a TypeError.

Figuring out whether there's underlying sensor hardware requires crossing process boundary (at least in Blink, and I guess similarly in other modern engines) and sync IPC should be avoided.

That said, @riju and @alexshalamov will experiment with Blink to see if this implementation feedback should motivate spec changes and will update this issue accordingly.

[timvolodine]

Correct, in chrome it would be impossible to do the "sensor present" check upon construction in a non-blocking way, and we should not make any blocking calls for this anyway. Pre-checking sensor upon startup seems too much overhead. So I think "errored" state kind of makes sense. Maybe there should be enums for "errors" though.. "relevant sensor(s) not available" is a pretty common case and it would probably make sense for the developers to test that quickly.

[tobie]

OK, I somehow assumed that a startup pre-check would be standard and cheap.

I'll revamp [[Construct Sensor Object]] accordingly and see what incidences, if any, this has on requesting user permission.

I was planning on using the NotReadableError for cases where sensors went MIA. Do you think there should be something more specific here?

[timvolodine]

I guess the first start() invocation will trigger user permission when necessary.
Would it be reasonable in the MIA case: to invoke the onerror callback and implicitly stop the sensors?

[tobie]

Oh, that's interesting. I had imagined user permission would be triggered during construction (and would fire an "error" event should permission not be granted).

Might make sense to tie it to .start() instead. Especially if .start() return promises (pending #94).

The other option is for construction to trigger a sensor presence check and fire an error if it's missing.

[tobie]

We're converging on:

• dropping usage of promise for start() and stop() methods,
• removing the onstatechange event handler,
• turning the SensorState enum into slots (or similar unexposed thingies),
• renaming the "active" state into "activated",
• adding an onactivate handler which is called when the state transitions from "activating" to "activated".

interface Sensor : EventTarget {
  readonly attribute SensorReading? reading;
  void start();
  void stop();
  attribute EventHandler onchange;
  attribute EventHandler onactivate;
  attribute EventHandler onerror;
};

In order to come to the above conclusions, we experimented with the following code examples:

Promise based

The key issue here is you still need to register an error handler here to catch errors after the start promise gets resolved, but you must either remove duplicate error messages, or only setup the handler after promise completion.

let g = new Gyroscope({frequency: 60});
function loop() {
    console.log(g.reading);
    requestAnimationFrame(loop)
}
g.start().then(_ => requestAnimationFrame(loop), err => {
    if (err.name == "NotReadableError")
        alert("you don't have a gyro!");
    else if (err.name == "NotAllowedError")
        alert("Give me the gyro!");
});

With await

Same lack of post promise resolve error handling.

async function run() {
  let g = new Gyroscope({frequency: 60});
  function loop() {
    console.log(g.reading);
    requestAnimationFrame(loop)
  }
  try {
    await g.start();
    requestAnimationFrame(loop);
  } catch(err) {
    if (err.name == "NotReadableError")
        alert("you don't have a gyro!");
    else if (err.name == "NotAllowedError")
        alert("Give me the gyro!");
  }
}

Event based (current solution)

let g = new Gyroscope({frequency: 60});
function loop() {
    console.log(g.reading);
    requestAnimationFrame(loop)
}

g.onstatechange = _ => {
    if (g.state == "active")
        requestAnimationFrame(loop);
}

g.onerror = err => {
    if (err.name == "NotReadableError")
        alert("you don't have a gyro!");
    else if (err.name == "NotAllowedError")
        alert("Give me the gyro!");
}

g.start();

With a single "activate" event (preferred solution).

let g = new Gyroscope({frequency: 60});
function loop() {
    console.log(g.reading);
    requestAnimationFrame(loop)
}

g.onactivate = _ => requestAnimationFrame(loop);
// g.state => activated

g.onerror = err => {
    if (err.name == "NotReadableError")
        alert("you don't have a gyro!");
    else if (err.name == "NotAllowedError")
        alert("Give me the gyro!");
}

g.start();