What illuminance value should be reported for stopped sensor?

[alexshalamov]

Consider the case when sensor is started and then developer caches reading object.

let als = new AmbientLightSensor();
als.start();
let cachedReading = als.reading;
als.stop();
// ?
console.log(cachedReading.illuminance);

At the moment I've implemented it to return 0.0, is this expected behaviour?

[anssiko]

SensorReading values are frozen and cannot and should not be modified. They have a timestamp, so that'd be wrong if the value of the illuminance attribute would be modified at any point in time.

We should probably clarify this in the spec, so I leave this issue open for that.

[tobie]

That's actually a generic sensor issue.

As @anssiko pointed out, the answer is that a sensor reading object should be immutable.

[rwaldron]

let als = new AmbientLightSensor();
als.start();
let cachedReading = als.reading;
als.stop();
// ?
console.log(cachedReading.illuminance);

At the moment I've implemented it to return 0.0, is this expected behaviour?

My understanding is that als.reading is actually still null, because it won't be updated in the same turn. @tobie unless that's no longer the case?

[alexshalamov]

@tobie, sure, readings are immutable. My question was about expected behaviour for cached readings. As Rick mentioned, als.reading would be null, that is fine, we've implemented as spec says. However, spec doesn't specify what happens if developer caches reading and accesses it after stop is called.

Should we return data from the last reading before stop was called? Or maybe we should return initial values that are specified for SensorReadingInit dictionary?

[tobie]

Well, the SensorReading object is immutable, so the value of its properties stay the same as when they were set.

[alexshalamov]

So, you mean that Sensor.reading getter would always make a copy of a current reading when sensor is active?

[tobie]

No. I mean that for each new sensor reading you must construct a new SensorReading object whose property values represent the state of the sensor at time t. Each prop is read only and its value won't ever change. Sensor.reading is just a getter for that object which corresponds to the latest sensor reading.

[rwaldron]

However, spec doesn't specify what happens if developer caches reading and accesses it after stop is called.

Why would it? I would expect JavaScript semantics:

let raw = 1;
let reading = {
  get illuminance() {
    return raw;
  }
};

let als = {
  reading
};

let cached = als.reading;

console.log(cached.illuminance); // 1

als.reading = null;

console.log(cached.illuminance); // 1

[alexshalamov]

@rwaldron you are correct, problem is that Sensor.reading points to 'current' reading and mutated from native side, after stop is called, there is no 'current' there can be only 'last' reading. @tobie, we will need to fix implementation, should be easy.

[tobie]

@alexshalamov, yeah, the UA must generate a new immutable SensorReading object for each new sensor reading and have Sensor.reading point to it.

let als = new AmbientLightSensor();
als.start({frequency: 60 });
let als2 = new AmbientLightSensor();
als2.start({ frequency: 25 });

let cachedReading1 = als.reading; // null

// time passes
let cachedReading2 = als.reading; // { illuminance: 23, timestamp: 87249 }
let cachedReading3 = als.reading; // { illuminance: 23, timestamp: 87249 }
let als2cachedReading = als2.reading; // { illuminance: 23, timestamp: 87249 }

// more time passes
let cachedReading4 = als.reading; // { illuminance: 344, timestamp: 3497979 }

als.stop();
als2.stop();

console.log(cachedReading1.illuminance);
// Uncaught TypeError: Cannot read property 'illuminance' of null(…)
console.log(cachedReading1 === cachedReading2);
// false

console.log(cachedReading2.illuminance);
// 23
console.log(cachedReading3.illuminance);
// 23
console.log(cachedReading2 === cachedReading3);
// true
console.log(als2cachedReading.illuminance);
// 23
console.log(cachedReading2 === als2cachedReading);
// true

console.log(cachedReading4.illuminance);
// 344
console.log(cachedReading3 === cachedReading4);
// false

[alexshalamov]

@tobie, I understand how you want API to behave, it is just bit confusing, maybe Sensor.reading section needs to be reformulated somehow. Spec says that Sensor.reading points to current reading. Reading is an object, therefore if I write let cached = Sensor.reading; by default JS engines will make reference, as a web developer I might think that cached is pointing to current reading. While in your example, Sensor.reading getter makes a immutable clone of a current reading.

[tobie]

I understand how you want API to behave, it is just bit confusing, maybe Sensor.reading section needs to be reformulated somehow.

Heh. yeah. :) There's a lot of reformulation needed. That said, the update current reading algorithm is quite clear about this.

Spec says that Sensor.reading points to current reading. Reading is an object, therefore if I write let cached = Sensor.reading; by default JS engines will make reference, as a web developer I might think that cached is pointing to current reading.

That's a good point in terms of meeting developer expectations. @rwaldron, WDYT?

Should we use method instead, e.g.:

sensor.read();

While in your example, Sensor.reading getter makes a immutable clone of a current reading.

That's not what happens, because if it did: als.reading === als.reading; would return false and we want it to return true. Instead what needs to happen is that you create a new SensorReading for each reading, and have Sensor.reading getter return it. In JS:

let current_reading = null;
let als = {
  get reading() {
    return current_reading;
  }
};

// poll sensor
current_reading = {
  get illuminance() {
    return 23;
  },
  get timestamp() {
    return 87249;
  }
}

// poll sensor later
current_reading = {
  get illuminance() {
    return 344;
  },
  get timestamp() {
    return 3497979;
  }
}

[alexshalamov]

@tobie If als.reading === als.reading; then let cached = als.reading; cached === als.reading; When cached !== als.reading; on the next update?

[tobie]

When cached !== als.reading; on the next update?

Yes.

[rwaldron]

Spec says that Sensor.reading points to current reading. Reading is an object, therefore if I write let cached = Sensor.reading; by default JS engines will make reference, as a web developer I might think that cached is pointing to current reading.

That's a good point in terms of meeting developer expectations. @rwaldron, WDYT?

I think the language shouldn't use the word "current", for two reasons:

1. Since we're in the realm of discussing electrical things, the word "current" is already reserved for the thing measured in terms of amps. Is "current reading" a measurement of a sensor's current consumption, or the most recent measurement of some readable thing?
2. As @alexshalamov has identified, "current reading" doesn't make sense if the sensor is presently stopped; "last reading", "latest reading", "most recent reading"... these might be better for spec language.

W/r to sensor.read/start(): sensor.read() would have its own baggage, specifically that devs might expect it to return a value, which it wouldn't (and definitely shouldn't).

[tobie]

I think the language shouldn't use the word "current", for two reasons:

That's not a term that's exposed to developers, and it's defined in the spec. But I'm happy to change if you find it confusing.

W/r to sensor.read/start(): sensor.read() would have its own baggage, specifically that devs might expect it to return a value, which it wouldn't (and definitely shouldn't).

No, I meant sensor.read() as a way to access the latest reading instead of Sensor.reading as @alexshalamov brought up some potential developer confusion about the behavior of Sensor.reading. Note I'm not advocating this particular change, or any change at all, just using .read() as a way to start the conversation, like I could have used: sensor.getLatestReading();

[alexshalamov]

+1 for sensor.getLatestReading(); that will always return clone of an 'internal slot latestReading' associated with sensor instance. Making references to objects that are owned by Sensor might bring side effects, for example, if sensor doesn't have event listeners, goes out of scope and garbage collected by JS engine, it is unclear what to do with objects that refer to Sensor.reading.

[tobie]

@alexshalamov Why would you want to make it return a clone, though? Imho, SensorReading is a similar pattern as events. You want the same reading to be ===.

[alexshalamov]

@tobie As I mentioned, I would like to avoid to have references to internal data of a sensor object. But either way is fine for me (cloning or making reference). I have no objections if getter will return latest reading and in next update local reference would be !== sensor.getLatestReading(). Maybe someone will find it even useful, e.g. cached value can be invalidated and updated with latest.

If you are planning to change Sensor.reading to sensor.getLatestReading(), semantics will be slightly different, for example, after stop() is called, should sensor.getLatestReading() return null? If not, what happens if permission is revoked.

Anyways, your explanation about how you want API to behave was helpful and few fixes need to be made to implementation to align with the spec. Thanks.

[tobie]

If you are planning to change Sensor.reading to sensor.getLatestReading(), semantics will be slightly different, for example, after stop() is called, should sensor.getLatestReading() return null? If not, what happens if permission is revoked.

I'm waiting for feedback from @rwaldron on having this be a method rather than an attribute getter.

[pozdnyakov]

Attributes should not return a new object every time (pls. see https://w3ctag.github.io/design-principles/#attributes-like-data ) so method looks more preferable.

[tobie]

@pozdnyakov third list item of the linked section of the API design doc says the following (emphasis mine):

Ensure that your attribute’s getter returns the same object each time it is called, until some occurrence in another part of the system causes a logical "reset" of the property’s value. In particular, obj.attribute === obj.attribute must always hold, and so returning a new value from the getter each time is not allowed.

This describes pretty much exactly the wanted behavior, where the 'logical "reset"' corresponds to a new sensor reading.

[rwaldron]

No, I meant sensor.read() as a way to access the latest reading instead of Sensor.reading
...
sensor.getLatestReading();

Using a method of the sensor instance to get a data value that already exists (it must, because the value was delivered by whatever subsystem provides it and an event was emitted to signal that delivery), then the API becomes misleading: for sensor.read() you could imagine someone seeing that call and thinking that it was actually making a new "read" measurement and returning that value. Subjectively, sensor.getLatestReading() is an eyesore—one that doesn't pay for itself if you consider the context.

J5 avoids creating this accessor reference "problem" by defining the accessor property that will expose the latest reading value directly on the instance, eg. if applied here: sensor.illuminance. The event objects contain a property whose value is just a copy of the measurement value (no accessor). J5 attempts to clearly separate data and behavior, as properties and methods which are defined directly on the instance (not on a "nested" property). For instances of Multi class, which represent chips that provide a "collection" of sensors, we expose the component sensor instances as values of getter accessor properties, ie:

// The BME280 provides barometer, hygrometer and thermometer sensors
var multi = new Multi({ controller: "BME280" });

// In the following, the properties: barometer, hygrometer and thermometer, 
// are all getter accessors that return an instance of their respective sensor class: 
multi.barometer instanceof Barometer;
multi.hygrometer instanceof Hygrometer;
multi.thermometer instanceof Thermometer;

// And the data properties of each of those instances are also getter accessors, 
// which return the relevant data value:
multi.barometer.pressure; 
multi.hygrometer.relativeHumidity;
multi.thermometer.celsius;

I'm not offering this as a suggestion to solve the issue at hand, but instead to lay the groundwork for my next statement: The IMU and Multi class (which are designed as shown above, were introduced in December 2014, and were rolled out at a workshop (CodeMash 2015) attended by ~90 people (a recap: http://briangenisio.com/software/2015/01/12/codemash-nodebots-recap.html). Code that uses instances of these classes has appeared in tutorials and articles, including several on Sparkfun.com. In that almost-two-years time, no issues have been filed, no support questions in gitter, no twitter critics, no colleagues, no core (or fringe) committers, not one single mention of the accessor properties being somehow unexpected behavior, or creating surprising references.

Empirically speaking, I doubt that devs will write code like let cache = sensor.reading, when the data they actually want to cache is at let cache = sensor.reading.theActualThingICareAbout.

If none of that is compelling enough to "leaving it as is", another approach might be considered: implementations don't need to use traditional attribute getter accessor properties (which can used to produce a reference) to expose a value that's asynchronously updated. If you're willing to make a minor break from the dogmatic "API Design Principles", and allow the sensor.reading property to return a new SensorReading object when there is a new measurement, then sensors can be specified and implemented in terms of Proxy objects:

(Warning: I'm only presenting this as a thought exercise, not as a proposed solution)

// Start: Imaginary data source mechanism
const timeOfOrigin = Date.now();
const imaginaryDataSource = new Map();
const simulateMeasurementCreateSensorReading = (illuminance) => {
  imaginaryDataSource.set("reading", new SensorReading({ illuminance }));
};
class SensorReading {
  constructor(data) {
    this.timeStamp = Date.now() - timeOfOrigin;
    Object.assign(this, data);
  }
}
// End: Imaginary data source mechanism

// Will be used for some private state
const priv = new WeakMap();
// Partial simulator/host implementation
class Sensor {
  constructor(settings) {
    let reading = null;
    const state = {
      isActive: false,
    };
    const proxy = new Proxy(this, {
      get: (target, name) => {
        if (target === this && name === "reading") {
          // Only updates when active...
          if (state.isActive) {
            reading = imaginaryDataSource.get("reading");
          }
          return reading;
        }
        // Proxy trap handlers for prototype methods
        if (typeof target[name] === "function") {
          return target[name];
        }
        // console.log(target);
      },
    });

    // Register private state in weakmap side table
    priv.set(proxy, state);

    return proxy;
  }
  start() {
    priv.get(this).isActive = true;
  }
  stop() {
    priv.get(this).isActive = false;
  }
}

let cached1 = null;
let cached2 = null;
let cached3 = null;
let sensor = new Sensor();
sensor.start();
// initial reading is null
simulateMeasurementCreateSensorReading(null);

// Proxy objects "present" themselves as their target would:
console.log(sensor instanceof Sensor);
console.log(Object.getPrototypeOf(sensor) === Sensor.prototype);
console.log(typeof sensor.reading === "object");
console.log(sensor.reading instanceof SensorReading);

// Even though there's no actual reading data, these are still the
// same object for the life of the initial execution turn.
console.log(sensor.reading === sensor.reading);

// And the value of "illuminance" is null when first initialized...
console.log(typeof sensor.reading.illuminance === "object");
console.log(sensor.reading.illuminance === null);

// Some turn later, an illuminance reading has been taken and
// delivered by the host...
/* ---- Execution turn boundary ----- */
simulateMeasurementCreateSensorReading(1);

// ...And now, this value is a number...
console.log(sensor.reading.illuminance === 1);
console.log(typeof sensor.reading.illuminance === "number");

// So a program could cache this attribute's value, which
// is an object representing "reading"; a "reading" is
// a SensorReading object containing the latest measurement
// data delivered by the host.
cached1 = sensor.reading;

// And it would match throughout _this_ turn.
console.log(cached1.illuminance === sensor.reading.illuminance);

// In this turn, sensor.reading attribute's value will always be the same SensorReading object
console.log(sensor.reading === sensor.reading);

// Some execution turn later, the data source is updated with data from a new measurement.
// A new measurement should intuitively create a new SensorReading object...
// Which opposes the design rules stated here:
// https://w3ctag.github.io/design-principles/#attributes-like-data
/* ---- Execution turn boundary ----- */
simulateMeasurementCreateSensorReading(2);

// The previously cached SensorReading object does not contain a copy of any
// get accessor, so the value of that cached SensorReading object's illuminance property
// is the value that was present when the cache assignment occurred.
console.log(cached1.illuminance !== sensor.reading.illuminance);
// cached1.illuminance is still 1
console.log(cached1.illuminance === 1);
// And the new SensorReading object's illuminance property has a value of 2
console.log(sensor.reading.illuminance === 2);

// I would also expect that a new measurement would produce a new SensorReading object,
// so intuitively, cached !== sensor.reading
console.log(cached1 !== sensor.reading);

// But until a new reading In this turn, sensor.reading attribute's value will always be the same SensorReading object
console.log(sensor.reading === sensor.reading);

// Since there is no set() trap for "reading",
// this is meaningless:
sensor.reading = 1;

// Cache this SensorReading object...
cached2 = sensor.reading;

// Is it possible for more then one measurement to occur in a
// single execution?
simulateMeasurementCreateSensorReading(3);

// Since a _new_ measurement was delivered by the host, sensor.reading
// produces a _new_ SensorReading object.
console.log(cached2 !== sensor.reading);

// Of course that will remain equal to itself until a new measurement
// is delivered...
console.log(sensor.reading === sensor.reading);

// cached2.illuminance is still 2
console.log(cached2.illuminance === 2);
// And the new SensorReading object's illuminance property has a value of 3
console.log(sensor.reading.illuminance === 3);

// So, let's cache this last reading before we proceed...
cached3 = sensor.reading;

// But then, _this_ sensor is stopped...
sensor.stop();

// ...And also a new execution turn has been entered...
/* ---- Execution turn boundary ----- */
simulateMeasurementCreateSensorReading(4);

// But sensor.reading is no longer returning new
// SensorReading objects, even though the host
// just delivered a new measurement...
console.log(cached3 === sensor.reading);

// cached3.illuminance is still 3
console.log(cached3.illuminance === 3);
// And sensor.reading still returns the SensorReading object
// from before the sensor was stopped.
console.log(sensor.reading.illuminance === 3);

Notes about the above code:

• Execute in a JS runtime that supports the thing necessary modern ES things.
• All of the expressions will evaluate to true.

[tobie]

If you're willing to make a minor break from the dogmatic "API Design Principles",

The design principles you mention actually describe precisely what it is you're suggesting below. Incidentally, that's also how it's already spec'ed. :)

and allow the sensor.reading property to return a new SensorReading object when there is a new measurement.

Yup. There's a new SensorReading per measurement.

I'm glad we're all on the same page. I'll edit the spec to clarify what's already there.

[rwaldron]

Incidentally, that's also how it's already spec'ed. :)

Ok, that's what I thought forever... I must've turned myself around while I was reading this thread, the spec text and writing my way-too-long response. 0_o

[alexshalamov]

Issue fixed by w3c/sensors#197 https://w3c.github.io/sensors/#set-sensor-settings