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.

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

Find a better name for 'unconnected' state #160

Closed
pozdnyakov opened this issue Jan 27, 2017 · 56 comments
Closed

Find a better name for 'unconnected' state #160

pozdnyakov opened this issue Jan 27, 2017 · 56 comments
Milestone

Comments

@pozdnyakov
Copy link

The 'unconnected' state is supposed to represent the initial sensor state before the 'start()' method is called for the first time, i.e. before we know if the sensor of the given type is actually present in the device.

The problem is that 'unconnected' looks confusing -- it could be treated as if sensor is physically unconnected from the device (for example a USB sensor hub). So renaming is advisable, possible options could be "unintialized" or "undefined".

@tobie
Copy link
Member

tobie commented Jan 27, 2017

Agree "unconnected" is bad.
"undefined" as a state is weird, though. "uninitialized," I originally rejected, because the object has been initialized already when you constructed it. But I'm warming up to it.

I guess the question is whether you think of the state attribute as describing the state of the sensor instance itself or the underlying sensor. I don't have a good answer to that. @rwaldron, @kenchris.

@tobie tobie modified the milestone: Level 1 Jan 27, 2017
@pozdnyakov
Copy link
Author

could be also "initial"

@tobie
Copy link
Member

tobie commented Jan 27, 2017

"pending" ?

@kenchris
Copy link
Contributor

kenchris commented Jan 27, 2017

"ready"? that is what is used elsewhere

@tobie
Copy link
Member

tobie commented Jan 27, 2017

@kenchris that's the state prior to knowing whether there's an available sensor. So I don't think "ready" is appropriate.

@pozdnyakov
Copy link
Author

IMO, "pending" more looks like we've already called smth. and wait for a feedback.
"constructed"?

@kenchris
Copy link
Contributor

But you are ready to call start right? and start will determine if there is a sensor?

@tobie
Copy link
Member

tobie commented Jan 27, 2017

But you are ready to call start right? and start will determine if there is a sensor?

Yes. Still, that's awkward. What are the existing examples of this?

@kenchris
Copy link
Contributor

It sounds like a very /uncertain/ state :-) Maybe that states should be changed instead. I guess promises currently solves this issue

@tobie
Copy link
Member

tobie commented Jan 27, 2017

You can't really use promises with an event driven interface, it's weird. We've gone through this conversation already. :)

@tobie
Copy link
Member

tobie commented Jan 27, 2017

What about we use "idle" at first and then "deactivated" after calling .stop()?

@kenchris
Copy link
Contributor

so idle | activating | active | deactivated ?

@kenchris
Copy link
Contributor

Then why isnt there a deactivating, or an inactive (idle)?

inactive | active | deactivated ?

@tobie
Copy link
Member

tobie commented Jan 27, 2017

Then why isnt there a deactivating

Because the process of deactivating is actually synchronous. You just flip a bit.

@kenchris
Copy link
Contributor

And that is the case on all systems? inactive | activating | active | deactivated |

@tobie
Copy link
Member

tobie commented Jan 27, 2017

And that is the case on all systems?

Yes. The system's irrelevant here, really.

@tobie
Copy link
Member

tobie commented Jan 27, 2017

Here are couple of proposals, feel free to play with the image src to come up with your preferred option.

Current

Connect all the things

Activate all the things

@pozdnyakov
Copy link
Author

+1 for "activate" semantics

@tobie
Copy link
Member

tobie commented Jan 30, 2017

@pozdnyakov should we also match the method names in that case?

s/start/activate/ and s/stop/deactivate/ ?

@pozdnyakov
Copy link
Author

@tobie yeah, think it's worth being consistent.

@tobie
Copy link
Member

tobie commented Jan 30, 2017

@rwaldron I know you have better things to do (much <3), but when you have time, thoughts on the above appreciated.

@pozdnyakov
Copy link
Author

@tobie can this issue be fixed now (after it was discussed at WG call)?

@tobie
Copy link
Member

tobie commented Feb 10, 2017

Yeah, we can fix the status now and fix the method names later.

pozdnyakov pushed a commit to pozdnyakov/sensors that referenced this issue Mar 28, 2017
Fixes w3c#160. Brings consistent names for Sensor's states and methods.
pozdnyakov pushed a commit to pozdnyakov/sensors that referenced this issue Mar 28, 2017
Fixes w3c#160. Brings consistent names for Sensor's states and methods.
@tobie
Copy link
Member

tobie commented Mar 28, 2017

@rwaldron can you please look at this and share your thoughts? Thanks.

@rwaldron
Copy link
Contributor

Given:

When would a program write if (sensor.state === A) { ..., such that it would be distinct from if (sensor.state === D) { ...?

@tobie
Copy link
Member

tobie commented Apr 3, 2017

If it is "idle", shouldn't it be "active".

I don't understand this.

So "activating" is basically meaning "changing state",

"activating" just means changing from "idle" to "activated".

I am wondering whether some sensors will require time to shut down

They do, but since this is by design not observable (so the UA can shut the sensor down or keep it active if others are still relying on it), calling .stop() on a sensor simply tells the UA it no longer needs data from it, and prevents any further data from being exposed to the API's client. So it never will need to be async.

what will users use the "activating" state for?

That's a good question. I'll give you another one: which states do you think the API consumers would like to know, and which state changes do you think they be interested in?

@kenchris
Copy link
Contributor

kenchris commented Apr 3, 2017

'I am idle, I am active' vs 'I idled the day away, I activated the washing machine.'

That's a good question. I'll give you another one: which states do you think the API consumers would like to know, and which state changes do you think they be interested in?

I guess most want to know whether something is running or not running, whether to call start or stop.

You know when it is active as you will get your first reading :).

I had the same issues before like, when I call start() I don't care if it is already started, I just want to make sure it will start as soon as possible, because I need it. Same with stop, just stop as soon as you can, whether you are running or not. I don't want an API where I am kind of forced to look at states, especially if these states are not 100% trustworthy.

@tobie
Copy link
Member

tobie commented Apr 3, 2017

'I am idle, I am active' vs 'I idled the day away, I activated the washing machine.'

On the platform, change in state are reflected with an [INFINITIVE] event which changes the object to an [PAST-TENSE] state, e.g. "load" event -> "loaded" state, "activate" event -> "activated" state. That's why we're not using "active" as a state. Maybe "idle" isn't the right state name either, given that context.

I guess most want to know whether something is running or not running, whether to call start or stop.

You know when it is active as you will get your first reading :).

Well, you'll also get an "activated" event. That said, in the future, your first reading could be a cached reading, or it could also be a reading from your previous activation if we keep them around rather than replace them by null (there are arguments for both).

@kenchris
Copy link
Contributor

kenchris commented Apr 3, 2017

Yes, I think it should be "idled". Will there be an onidled event handler as well?

Hmm, old cached readings would probably be quite bad though, as many changes can have happened since I used the sensor last and mixing those with new readings will introduce errors. What is the use-case for that?

@tobie
Copy link
Member

tobie commented Apr 3, 2017

Yes, I think it should be "idled".

Well, "stopped" in that case. But then that's weird as first state. I don't want to bikeshed this into eternity, though.

Will there be an onidled event handler as well?

No, because it's sync.

Hmm, old cached readings would probably be quite bad though, as many changes can have happened since I used the sensor last and mixing those with new readings will introduce errors. What is the use-case for that?

Saving battery, e.g. for geolocation. Maybe this needs its own API, though. Tracked here: #89

@tobie
Copy link
Member

tobie commented Apr 3, 2017

Moving this forward, I'd like us to answer the following questions here:

  1. Are those three states a good model internally (for spec editing)? (y|n)
  2. Do we want to expose state at all? (y|n|maybe later)
  3. If we want to expose state now, do we expose it as an enum or a boolean "activated" attribute? (enum|boolean)
  4. If we expose those externally, what do we name those three states?
  5. What do we name the start/stop methods? (start/stop|activate/deactivate|something else)
  6. Do we also rename the onactivate handler? (y/n)

@tobie
Copy link
Member

tobie commented Apr 3, 2017

That said, I think some of those questions tie into how we plan to expose data for high-frequency sensors (e.g. will data be exposed on attributes? When a sensor has been stopped, can you still query it's last state? etc.). I think we need to make progress there before we can settle on a decision here.

@pozdnyakov
Copy link
Author

As far as I remember, initially states were considered at start & stop calls, so that these methods send exceptions if were called in inappropriate state. Now such calls are simply ignored so boolean activated should be enough.

@pozdnyakov
Copy link
Author

as for data in idle state I think we found a good solution already #168. There is no problem for user to cache readings at any moment, if needed.

@tobie
Copy link
Member

tobie commented Apr 3, 2017

There is no problem for user to cache readings at any moment, if needed.

This was a requirement for geolocation. Providing pre-activation readings.

@tobie
Copy link
Member

tobie commented Apr 3, 2017

as for data in idle state I think we found a good solution already #168.

You're right. :-/

I was wondering how that would work once we look at HF sensors, e.g.:

sensor.stop();
sensor.getQuaternion(buffer); // or however we'll call that

Would that throw?

Return the latest reading? Etc.

@pozdnyakov
Copy link
Author

pozdnyakov commented Apr 3, 2017

for getters like this it's enough to have binary state: if activated - populate; otherwise throw.

The difference with start()/stop() is that such methods (like getQuaternion) do not modify sensor's state.

@pozdnyakov
Copy link
Author

This was a requirement for geolocation. Providing pre-activation readings.

Could you please give some more details? What are these pre-activation readings and how they are supposed to be used?

@kenchris
Copy link
Contributor

kenchris commented Apr 3, 2017

GPS is slow at making a connection, so often last position serves the purpose until a new reading becomes available (after calibrating and getting all satellite connections)

@tobie
Copy link
Member

tobie commented Apr 3, 2017

Yeah, the use case are described with examples in the geolocation spec's intro. I think having specific methods for this is what makes the most sense, so you can disregard my pushback here.

Following @pozdnyakov's input here, it seems we can try to give an answer to at least some of the questions above. Here are mine with some rationale to them:

  1. Are those three states a good model internally (for spec editing)?
    Yes. I looked at the spec and we don't need more to spec things.
  2. Do we want to expose state at all?
    I think we can move that decision to later. It doesn't seem to be critical to developers right now. Let's expose it once we have use cases.
  3. If we want to expose state now, do we expose it as an enum or a boolean "activated" attribute?
    We can avoid taking this decision right now and wait for developer feedback before we do.
  4. If we expose those externally, what do we name those three states?
    Same as above: we can wait.
  5. What do we name the start/stop methods?
    I don't have a strong preference, here, but I think we should keep start/stop, because they're short and explicit. Moving to activate/deactivate has other problems. Authors would expect the onactivate handler to be called in the same event turn as activate is called. Not asynchronously, when the activation has been carried out.
  6. Do we also rename the onactivate handler?
    I'd leave it as such unless someone has a better proposition.

@rwaldron
Copy link
Contributor

rwaldron commented Apr 3, 2017

@tobie I just want to tick off your list:

  1. Agreed
  2. Agreed
  3. Agree, but I think boolean "activated" attribute is ideal for end programs (simple usage: if (sensor.activated) { ...)
  4. See 3
  5. Agreed
  6. Agreed

@pozdnyakov
Copy link
Author

+1 to #160 (comment)

@tobie Did I get it right that we agreed to drop errored and unconnected states? : )

pozdnyakov pushed a commit to pozdnyakov/sensors that referenced this issue Apr 4, 2017
Fixes w3c#160. Embarking on a three sensor states set up. States are 'idle', 'activating', 'activated'.
@tobie
Copy link
Member

tobie commented Apr 4, 2017

Did I get it right that we agreed to drop errored and unconnected states? : )

Yes.

@tobie tobie closed this as completed in #179 Apr 4, 2017
tobie pushed a commit that referenced this issue Apr 4, 2017
Addresses #160.

States are now 'idle', 'activating', 'activated'.
@tobie
Copy link
Member

tobie commented Apr 4, 2017

Reopening so we can:

  • remember to remove the SensorState enum in favor of infra (non-exposed states). See Stop exposing Sensor state. #180.
  • lift off the relevant SVG state diagram and add it to the spec.

@tobie tobie reopened this Apr 4, 2017
tobie added a commit to tobie/sensors that referenced this issue Apr 4, 2017
tobie added a commit to tobie/sensors that referenced this issue Apr 27, 2017
tobie added a commit that referenced this issue Apr 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants