read() will resolve geolocation promise only if notify new reading is invoked

[kenchris]

The following fails

    promise_test(async t => {
      const sensor = new sensorType();
      const sensorWatcher = new EventWatcher(t, sensor, ["reading", "error"]);
      sensor.start();
  
      await sensorWatcher.wait_for("reading");
      const cachedTimeStamp1 = sensor.timestamp;
  
      await sensorWatcher.wait_for("reading");
      const cachedTimeStamp2 = sensor.timestamp;
  
      assert_greater_than(cachedTimeStamp2, cachedTimeStamp1);
      sensor.stop();
    }, `${sensorType.name}: sensor timestamp is updated when time passes`);

As geolocation updates are slow and it will get the same reading with the same timestamp, making this test fail.

I guess either the test is wrong, or timestamp should be when the event fires

[anssiko]

Such a test should use assert_greater_than_equal(actual, expected, description) per the spec:

A latest geolocation reading is a latest reading for a Sensor of Geolocation Sensor sensor type.
https://wicg.github.io/geolocation-sensor/#latest-geolocation-reading

When you follow the definitions: latest reading -> sensor readings -> raw sensor readings:

Each reading is composed of the values of the different physical quantities measured by the sensor at time t_n which is called the reading timestamp.
https://w3c.github.io/sensors/#reading-timestamp

Most if not all use cases require the timestamp to indicate the actual time when the reading was obtained, and not when the event fires. Web developers can get a timestamp that is correlated with event firing via Event.timeStamp if using onreading or via performance.now() if using read().

@Honry to check if we should add a test case for this. I don't see such a test case in https://github.com/w3c/web-platform-tests/labels/geolocation-sensor or https://github.com/w3c/web-platform-tests/tree/master/geolocation-sensor yet. Maybe @kenchris can review it.

I submitted a PR to clarify related concepts in w3c/sensors#348

[Honry]

@anssiko, we have the test at https://github.com/w3c/web-platform-tests/blob/master/generic-sensor/generic-sensor-tests.js#L76

But I have one concern, if the senor reading is the same with previous one(with same timestamp),
does it mean there's no new sensor reading exposed? Thus the reading event will not be triggered.

[kenchris]

Yes, the test above is from WPT :-)

@Honry, if you do a new sensor, it might very well return an existing value and that will then of course trigger "reading" events or resolve read().

This might change when we add options to the new spec, like maxAge or similar

[anssiko]

@Honry good catch, read() will resolve geolocation promise only if notify new reading is invoked. The following step in https://wicg.github.io/geolocation-sensor/#geolocationsensor-read:

If notify new reading is invoked with geo, then resolve geolocation promise with geo and p.

What would be a good default behavior for read()? Always return as fresh reading as possible or prefer cached to be able to resolve sooner? I'd like to figure out the default behavior first before we get to configuration options maxAge and friends.

When we have an agreement on the above, need to figure out how to spec this preferably reusing the abstract operations from the Generic Sensor API.

[kenchris]

I would be fine with a cached reading... if it's within a reasonable timeframe, especially with this one-shot...

This is mostly useful for rough estimates on the location of the user, like country/neighborhood. When we add options, people can specify a different "maxAge" or similar. But I think for default, 15-60 minutes might not be bad

[tomayac]

Can this be merged into #25? The failing test apart, the core of this seems to boil down to the questions discussed over there…

FWIW, the current legacy default behavior seems to favor low latency over high accuracy; personally I am fine with this, but I propose we discuss in #25.

[kenchris]

I am fine with it

[tomayac]

Closing in favor of #25.