Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Stop exposing Sensor state. #180

Merged
merged 4 commits into from
Apr 5, 2017
Merged

Stop exposing Sensor state. #180

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
369c871
Stop exposing Sensor state.
tobie Apr 4, 2017
4d9008f
Fix typo.
tobie Apr 5, 2017
04ebcfb
Add activated attribute.
tobie Apr 5, 2017
5cedf89
Use the right state.
tobie Apr 5, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 20 additions & 30 deletions index.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -592,7 +592,6 @@ A [=sensor=] has an associated <dfn>current polling frequency</dfn> which is ini
<pre class="idl">
[SecureContext]
interface Sensor : EventTarget {
readonly attribute SensorState state;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will it be modified into boolean activated, like you and @rwaldron discussed at #160 (comment) or we'll leave it not exposed for now?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should just avoid exposing it for now. That's the kind of precise feedback I'd like to see from origin trials, though (maybe we could even expose both there and see which ones devs end up using?)

Copy link

@pozdnyakov pozdnyakov Apr 5, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In origin trials the API call statistics is collected, so IMO it is preferable to leave something and see whether it is called or not. I would personally leave the activated attribute: it is minimalistic and perfectly aligned with the rest of API. Exposing both is also possible, so I'm fine with any decision regarding this :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, my worry about basing this on stats is that actual usage will depend highly on what documentation we release before we go to origin trial. @kenchris any thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am fine with exposing the activated attribute. Then we can also see whether people use it or not or need something more powerful

readonly attribute DOMHighResTimeStamp? timestamp;
void start();
void stop();
Expand All @@ -604,12 +603,6 @@ interface Sensor : EventTarget {
dictionary SensorOptions {
double? frequency;
};

enum SensorState {
"idle",
"activating",
"activated"
};
</pre>

A {{Sensor}} object has an associated [=sensor=].
Expand Down Expand Up @@ -648,10 +641,10 @@ with the internal slots described in the following table:
<tr>
<td><dfn attribute for=Sensor>\[[state]]</dfn></td>
<td>The current state of {{Sensor}} object which is one of
<a enum-value>"idle"</a>,
<a enum-value>"activating"</a>,
<a enum-value>"activated"</a>,
It is initially <a enum-value>"idle"</a>.
"idle",
"activating", or
"activated".
It is initially "idle".
</td>
</tr>
<tr>
Expand Down Expand Up @@ -683,11 +676,6 @@ with the internal slots described in the following table:
</tbody>
</table>


### Sensor.state ### {#sensor-state}

The getter of the {{Sensor/state!!attribute}} attribute returns <emu-val>this</emu-val>.\[[state]].

### Sensor.timestamp ### {#sensor-timestamp}

The getter of the {{Sensor/timestamp!!attribute}} attribute returns
Expand All @@ -698,9 +686,9 @@ The getter of the {{Sensor/timestamp!!attribute}} attribute returns
<div algorithm="to start a sensor">
The {{Sensor/start()}} method must run these steps or their [=equivalent=]:
1. Let |sensor_state| be the value of sensor_instance|.{{[[state]]}}.
1. If |sensor_state| is either <a enum-value>"activating"</a>
or <a enum-value>"activated"</a>, then return.
1. Set |sensor_instance|.{{[[state]]}} to <a enum-value>"activating"</a>.
1. If |sensor_state| is either "activating"
or "activated", then return.
1. Set |sensor_instance|.{{[[state]]}} to "activating".
1. Run these sub-steps [=in parallel=]:
1. let |connected| be the result of invoking
the [=Connect to Sensor=] abstract operation.
Expand All @@ -722,28 +710,30 @@ The {{Sensor/start()}} method must run these steps or their [=equivalent=]:
<div algorithm="to stop a sensor">
The {{Sensor/stop()}} method must run these steps or their [=equivalent=]:

1. If |sensor_instance|.{{[[state]]}} is neither <a enum-value>"activating"</a>
nor <a enum-value>"activated"</a>, then return.
1. Set |sensor_instance|.{{[[state]]}} to <a enum-value>"idle"</a>.
1. If |sensor_instance|.{{[[state]]}} is "idle", then return.
1. Set |sensor_instance|.{{[[state]]}} to "idle".
1. Run these sub-steps [=in parallel=]:
1. Invoke [=Unregister a Sensor=] passing it |sensor_instance| as argument.
</div>

### Sensor.onchange ### {#sensor-onchange}

{{Sensor/onchange}} is an {{EventHandler}} which is called whenever a new [=sensor reading|reading=] is available.
{{Sensor/onchange}} is an {{EventHandler}} which is called
whenever a new [=sensor reading|reading=] is available.

Issue: Should this be renamed `onreading`?
Should we instead add an `ondata` {{EventHandler}} for continuous data
and use `onchange` when the data changed?

### Sensor.onactivate ### {#sensor-onactivate}

{{Sensor/onactivate}} is an {{EventHandler}} which is called when <emu-val>this</emu-val>.{{[[state]]}} transitions from <a enum-value>"activating"</a> to <a enum-value>"activated"</a>.
{{Sensor/onactivate}} is an {{EventHandler}} which is called when
<emu-val>this</emu-val>.{{[[state]]}} transitions from "activating" to "activated".

### Sensor.onerror ### {#sensor-onerror}

{{Sensor/onerror}} is an {{EventHandler}} which is called whenever an [=exception type|exception=] cannot be handled synchronously.
{{Sensor/onerror}} is an {{EventHandler}} which is called whenever
an [=exception type|exception=] cannot be handled synchronously.

### Event handlers ### {#event-handlers}

Expand Down Expand Up @@ -817,7 +807,7 @@ Gets the {{Error}} object passed to {{SensorErrorEventInit}}.
{{Sensor}} {{Sensor/timestamp!!attribute}} attributes.
1. If [=identifying parameters=] in |options| are set, then:
1. Set |sensor_instance|.{{[[identifyingParameters]]}} to [=identifying parameters=].
1. Set |sensor_instance|.{{[[state]]}} to <a enum-value>"idle"</a>.
1. Set |sensor_instance|.{{[[state]]}} to "idle".
1. Return |sensor_instance|.
</div>

Expand Down Expand Up @@ -873,7 +863,7 @@ Gets the {{Error}} object passed to {{SensorErrorEventInit}}.
1. Invoke the [=Set Sensor Settings=] abstract operation,
passing it |sensor| as argument.
<!-- 1. Let |latest_reading| be |sensor|'s associated [=latest reading=] [ordered map|map].
1. If |current_reading|["timestamp"] is not `null` and |sensor_instance|'s state is still <a enum-value>"activating"</a>, then
1. If |current_reading|["timestamp"] is not `null` and |sensor_instance|'s state is still "activating", then
1. invoke the [=Update Observers=] operation, passing it
|sensor_instance| and |current_reading| as arguments. -->
</div>
Expand Down Expand Up @@ -1090,8 +1080,8 @@ Gets the {{Error}} object passed to {{SensorErrorEventInit}}.
: output
:: None

1. If |sensor_instance|.{{[[state]]}} is <a enum-value>"activating"</a>:
1. Set |sensor_instance|.{{[[state]]}} <a enum-value>"activated"</a>.
1. If |sensor_instance|.{{[[state]]}} is "activating":
1. Set |sensor_instance|.{{[[state]]}} to "activated".
1. [=Fire an event=] named "activate" at |sensor_instance|.
1. If |sensor_instance|.{{[[waitingForUpdate]]}} is true, then

Expand All @@ -1115,7 +1105,7 @@ Gets the {{Error}} object passed to {{SensorErrorEventInit}}.
: output
:: None

1. Set |sensor_instance|.{{[[state]]}} to <a enum-value>"idle"</a>.
1. Set |sensor_instance|.{{[[state]]}} to "idle".
1. [=Fire an event=] named "error" at |sensor_instance| using {{SensorErrorEvent}}
with its {{SensorErrorEvent/error!!attribute}} attribute initialized to |error|.
</div>
Expand Down