Re: api comments

Hi Allan,

Thanks a lot for the good comments! Please see my replies inline:

On Mon, Mar 30, 2009 at 6:32 PM, Allan Thomson (althomso)
<althomso@cisco.com> wrote:
> Comment #1:
>
>
>
> “The API is designed to enable both "one-shot" position requests and
> repeated position updates”,
>
>
>
> This suggests a periodic update rather than what I think it is intended to
> suggest is asynchronous updates.

The text says "repeated", as in "done again and again". I think
"periodic", as in "recurring at regular intervals of time", is
different to "repeated". The watchPosition() method indeed provides
repeated updates to the caller, so I think the text is correct.

>It is not clear from the document when and
> what causes an asynchronous location update to occur. It is movement? Time?
> Suggest that you change “repeated position updates” to “asynchronous
> updates” and clarify what triggers the updates to be sent.
>

I don't think the introduction is the place for that. We do clarify
what triggers the updates in this section:

http://dev.w3.org/geo/api/spec-source-v2.html#watch-position

>
>
> Comment #2:
>
>
>
> “…the ability to explicitly query the cached positions”
>
>
>
> Define cached? Where is it cached? Client or server side? What is the
> criteria for caching vs not caching.  Suggest the concept of caching is not
> something this API should address or should be a separate document
>

What Doug said: the position that was last acquired by (or last known
to) the implementation. The point of this feature of the API is to
allow a user to get a position cheaply. There is another thread about
this here:

http://lists.w3.org/Archives/Public/public-geolocation/2009Mar/0068.html

Note that we are not trying to specify how caching should be done.
Indeed, this is an implementation detail.

>
>
> Comment #3:
>
>
>
> “Location information is represented by latitude and longitude coordinates”.
>
>
>
> As we are including CIVIC address elements also this is no longer strictly
> the only way to represent location. Sentence needs to address both formats.
>

Yes, fixing...

>
>
> Comment #4:
>
>
>
> The examples don’t really highlight the high-level goals of the API which
> would be better for an introductory section. They seem to show examples of
> how the programmer would use each API call. Suggest they be moved to a later
> section where the API is described for each method or to an appendix.
>
>

These are meant to be examples of how a programmer would use each API.
Other W3C specs (e.g. XHR) do the same thing. I think examples like
these make it easier for a programmer to understand the API hence it
is a good practice to have them here instead of an appendix.

>
> Scope Section
>
>
>
> Comment #5:
>
>
>
> It would seem that the scope of this document is to not define the
> underlying mechanisms that the API provides such location data (whether
> locally or remotely determined). This should be explicitly stated so that no
> one thinks that the API requires a specific underlying transport mechanism
> or local determination capability.
>

But we say "This specification is limited to providing a scripting
APIs (...)". From this it follows that anything else (including the
underlying mechanisms...) is out of scope. But ok, I can add an
explicit sentence about this:

"The scope of the specification does not include the mechanisms and
methods used by the implementation to acquire location information".

>
>
> Comment #6:
>
>
>
> This API provides a mechanism to provide location of the calling agent ONLY.
> Might be obvious but I think it is worth stating due to security concerns
> that may arise if someone thought they can use this API to find out location
> of someone else.
>

The scope section says "This specification is limited to providing a
scripting APIs for retrieving geographic position information
associated with a hosting device".  Perhaps this is enough? Or say
"...associated only with the device hosting the implementation"?

>
>
> API Description – Geolocation Interface
>
>
>
> Comment #7:
>
>
>
> interface Geolocation {
>
>     void getCurrentPosition(in PositionCallback successCallback);
>
>     void getCurrentPosition(in PositionCallback successCallback, in
> PositionErrorCallback errorCallback);
>
>     void getCurrentPosition(in PositionCallback successCallback, in
> PositionErrorCallback errorCallback, in PositionOptions options);
>
>
>
>     int watchPosition(in PositionCallback successCallback);
>
>     int watchPosition(in PositionCallback successCallback, in
> PositionErrorCallback errorCallback);
>
>     int watchPosition(in PositionCallback successCallback, in
> PositionErrorCallback errorCallback, in PositionOptions options);
>
>
>
>
>
> Are these methods completely orthogonal (i.e. thread safe)?
>

Thread-safety should be handled by the implementation, not the API
specification. Also note that this is a Web API so thread safety isn't
really an issue anyway. However, even in a single-threaded application
environment (e.g. JS),  since the API is async, you can indeed have
several calls to API methods running concurrently. But it certainly is
possible to differentiate between these call instances if you really
need to.

>
> Comment #8:
>
>
>
>   interface PositionCallback {
>
>     void handleEvent(in Position position);
>
>   };
>
>
>
>   interface PositionErrorCallback {
>
>     void handleEvent(in PositionError error);
>
>   };
>
>
>
>
>
> Why are there two functions rather than just one function with a status
> code? For example there may be situations where a location object will be
> returned but some additional status still needs to be conveyed back to the
> caller also.

This is false. The API does not allow this.

> In that case there is no way to do that right now. Also, having
> two methods will create code bloat unnecessarily.
>
>
>
> Suggest make a single method for callback with a status object (code + other
> attributes) and a location position object.
>

So then all users of the API will have to provide a callback function
that has an "if" statement at its top level. We decided it is cleaner
to have two separate callbacks.


>
> Comment #9:
>
>
>
> “The getCurrentPosition() takes one, two or three arguments. When called, it
> must immediately return and then asynchronously acquire a new Position
> object. If successful, this method must invoke its associated
> successCallback argument with a Position object as an argument. If the
> attempt fails, and the method was invoked with a non-null errorCallback
> argument, this method must invoke the errorCallback with a PositionError
> object as an argument”
>
>
>
> The getCurrentPosition method needs to return a success or failed result
> code so that the caller knows the callback has been registered successfully.
> Or define an exception but there needs to be a way for an implementation to
> return “callback not registered” or other unusual programmatic errors.
>

Implementations should raise an exceptions for programmer errors. We
could add a catch-all (i.e.a applying to all methods in the API)
sentence explaining this.

>
> Comment #10:
>
>
>
> “The enableHighAccuracy attribute provides a hint that the application would
> like to receive the best possible results. This may result in slower
> response times or increased power consumption. The user might also deny this
> capability, or the device might not be able to provide more accurate results
> than if the flag wasn't specified”
>
>
>
> The definition of “high” accuracy is dependent on many things outside the
> control of the user and they have no way of understanding why they are or
> are not getting high accuracy.  99% clients will always want the best
> accuracy you can give. Suggest you remove this attribute as it is not useful
> and will cause many comments in the future on what is the definition of high
> accuracy. Another possible option is to define a polygon and error estimates
> on the location result so that you can represent accuracy more correctly. To
> do this see comment regarding X,Y positional information in civic
> attributes.
>
>

I know this is controversial as probably the name of the attribute is
wrong. In any case, it is certainly false that 99% of the clients will
always want the best accuracy. Indeed, many may live with a lower
accuracy at the benefit of increased batter life (e.g. on mobile
devices). Google has today such products (e.g. google.com on mobile).
Anyway, this has been discussed extensively in the two threads
mentioned in the spec. We are certainly open to better alternatives,
if anyone can provide any.

>
>
>
> API Description – Coordinates Interface
>
>
>
> Comment #11:
>
>
>
>   interface Coordinates {
>
>     readonly attribute double latitude;
>
>     readonly attribute double longitude;
>
>     readonly attribute double altitude;
>
>     readonly attribute double accuracy;
>
>     readonly attribute double altitudeAccuracy;
>
>     readonly attribute double heading;
>
>     readonly attribute double speed;
>
>   };
>
>
>
> Heading/Direction (less so) and speed (more so) are not specific to
> geospatial information AND are common to both civic as well as geospatial
> location objects. Suggest heading/direction and speed should be a separate
> object that can be referenced by both civic and geospatial positions.
>

Strictly speaking, you are right: it should be decoupled from the
Coordinates interface. How would you name this new interface?
Velocity? In practice, I suspect that velocity can only be provided by
implementations that can also provide geospatial coords.

>
>
> API Description – Address Interface
>
>
>
> Comment #12:
>
>
>
>   interface Address {
>
>     readonly attribute DOMString country;
>
>     readonly attribute DOMString region;
>
>     readonly attribute DOMString county;
>
>     readonly attribute DOMString city;
>
>     readonly attribute DOMString street;
>
>     readonly attribute DOMString streetNumber;
>
>     readonly attribute DOMString premises;
>
>     readonly attribute DOMString additionalInformation;
>
>     readonly attribute DOMString postalCode;
>
>   };
>
>
>
> This object is missing attributes for
>
>
>
> 1)       Building

We have "premises: The premises denotes the details of the premises,
such as a building name, block of flats, etc.". Isn't this the same?

>
> 2)       Floor

Could this, X and Y go to "additionalLocationInfo"  for those apps
that really need them?

>
> Comment #13:
>
>
>
> AdditionalInformation is inadequate for the “rest” of the information for
> buildings, floors…etc as the spec currently states.
>
>

This was always going to be a trade-off between the API complexity and
the level of detail that is possible to encode in separate fields.
Again, why is it inadequate to put floor, X and Y in the
"additionalLocationInformation" field?

>
>
>
> API Description – PositionError Interface
>
>
>
> Comment #14:
>
>
>
> Need to add “format unavailable” for times when lat/long or civic formats
> are not available due to lack of support of a particular format either from
> the underlying location determination capability or environment you are
> currently in. This is different from a position being unavailable due to
> lack of gps sync or civic information available from a location server due
> to network problems.
>
>

Yes, we have a separate thread about this and the PositionOptions
extension that Doug mentioned. I'll start and ISSUE to track this.

>
>
>
> Use Cases and Requirements
>
>
>
> Comment #15:
>
>
>
> The use case examples are all outdoor/consumer related use cases. We need to
> add indoor use cases that can also help explain how location is used for
> business like environments. Can provide information in a separate
> submission.
>

Sounds good, thanks.

>
>
> Comment #16:
>
>
>
> “The Geolocation API must allow an application to register to receive
> repeated position updates.”
>
>
>
> Again what does “repeated” really mean. Time or movement based. Suggest this
> is not a well defined requirement.
>

It's meant to be movement based. How about "“The Geolocation API must
allow an application to register to receive updates when the position
of the hosting device changes”?

>
>
> Comment #17:
>
>
>
> “The Geolocation API must allow an application to cheaply query the last
> known position.”
>
> As opposed to the current location? Define cheap? This is not a well-defined
> requirement. Please revise.
>

Return a position that was previously acquired and, hence, not doing
the work normally necessary to acquire a new position. Would that be
more clear?

>
>
> Comment #18:
>
> “The Geolocation API must allow an application to specify a desired accuracy
> level of the location information.”
>
> Add “even if that accuracy level is not possible or achievable by the
> location determination function”
>

Sounds reasonable.

>
>
> Comment #19:
>
>
>
> “The Geolocation API must be agnostic to the underlying sources of location
> information”.
>
>
>
> This can never be STRICTLY enforced. A lat/long value can only be provided
> by a system that is lat/long aware. Therefore there has to be some
> dependence on the underlying function to provide this information. I would
> suggest the API “may” be agnostic. Not “must be agnostic”.
>

But then it isn't a requirement anymore. Actually, we could perhaps
remove this as I am not sure it adds any value.

Thanks,
Andrei

Received on Tuesday, 31 March 2009 12:32:16 UTC