Re: [sysinfo] draft ready for review

Hi Doug,
Thanks for the great feedback. Answers below.

On 18/12/2009 19:13, Doug Turner wrote:

> 1) Have you considered just using dom events?

Yes, but indeed they don't allow you to pass options. I find the 
threshold options important somehow. Sometimes you want to just be 
notified of the battery level being only if less than some value, not 
every time it changes.

Also I find a get() / DOMEvent / set() less homogenous than 
get/watch/set. Another reason what that I was trying to remain close to 
how geolocation does it.

> Then any changes in PowerState would call callback.  Also, when
> addEventListener is called, callback is called schedule to be called
> as soon as possible to provide the current PowerState (to simulate
> .get()).

So if you just want to do a get() you'd have to do

callback(info) {
   alert("Low battery level: " + info.level);
}
addEventListener("PowerState", callback);
removeEventListener("PowerState", callback);

right?

> 2) Maybe we can roll get and watch together into one api.  I am not
> sure the value of having two different methods are -- one for one
> time shot, and one for watching changes.

like

get("PowerLevel", success, error)

and

get("PowerLevel", success, error, {watch: true});

for instance? Why not. But I think that the current way makes it clearer 
that both types of operations are possible, rather than the watch 
feature being buried inside the options parameter. What was 
geolocation's reason for having both get and watch?

> 3) I suggest removing:
>
> Error.message for similar reasons to why it was removed from the geo
> spec.

Good point. done.

> 4) I suggest removing:
>
> const unsigned short UNKNOWN_ERROR = 0;
>
> I am not convinced that this error code is any better than
> INFORMATION_UNAVAILABLE

True. I'd forgotten geolocation's resolution on this. Fixed.

> 5) I really don't like the Threshold and id values on Options.  For
> example, given if I want to watch() PowerState, what do these values
> mean.

They mean: only callback if the values are either below or above the 
thresholds. As I wrote above, I think it would be tedious to have the 
power-change event fired all the time if you're only interested in 
critical states. And I've claimed that in most applications you are.
How many apps display the CPU load in real-time, for instance? Many more 
would only want to adapt to uncommon occurrences of CPU load.

  Maybe we define a PowerStateOption (and a *StateOption) that
> we can clearly spec what these threshold values mean.

I thought that at the beginning and later realised that thresholds apply 
almost everywhere: temperature, sensors, CPU load, free memory, signal 
strength, etc. If we find more options that only applied to specific 
properties, then indeed it makes sense to have *StateOption objects, but 
I've not found any yet.


> 6) timeRemaining in milliseconds?  i would have thought seconds would
> be good enough.

This is just for compatibility with other specification, geolocation 
again, which has its timeout in milliseconds. But also ECMAScript's 
setTimeOut, Date() etc. It just makes calculations slightly easier.

> 7) CpuState - have you considered moving 'load' into cpu?

*having* load into cpu is fine by me. You can already get it with 
currentFrequency/maxFrequency, but it's nevertheless convenient.

*moving* load into Cpu (i.e. removing the global load) I'm not keen on. 
I think it's better to provide a single value, like UNIX's system load, 
that reflects something general. Most webapp authors will only care 
about one value, to check if it's too high, and won't want to bother 
checking how many CPUs there are, what are their respective frequency, 
what combination of those frequencies should the webapp react to. That's 
too low-level for the webapp developer to care about, so it should be 
left to the API developer, who'll just hand off a single value, computed 
in an hardware-dependent fashion.

> 8) Cpu id - Do we want to spec this string out a bit more so that web
> developers don't have to struggle parsing it?

Why parse it? It's only meant to be reused as is in the id parameter of 
get, watch or set. In fact we should specify that the string is opaque 
and should not be inspected.

> 9) maxFrequency, minFrequency - optional?

If they are made optional, then currentFrequency doesn't provide 
information anymore. Or, you add load inside each Cpu, as you suggested 
above. I don't have a strong feeling, but I'd rather have group 
consensus on it.


> 10) nit:  the white space of the ThermalState declaration is off.

Fixed. There was an error in the declaration, too.


> 11) the attribute 'info' on some of the sensors that describe the
> sensor -- I am not sure that is really needed; could we make these
> optional.

Absolutely, fixed.

> 12) Connection ipAddress - An implementation must support both IPv4
> and IPv6.  What is the format of this string; is it either a IP4 or a
> ip6?

I guess that the consequence of supporting any version (including 
obscure ones, like IPv9 apparently) of IP is that the string would be 
free-form. Or we specifically go for 4 and 6 and define the format as 
the union of each version's format. What do you think?


>
> 13) Have you considered moving:
>
> readonly attribute float?       currentSignalStrength; readonly
> attribute unsigned int currentDownloadBandwidth; readonly attribute
> unsigned int currentUploadBandwidth; readonly attribute DOMString
> ipAddress;
>
> Into the Connection object?

Yes. It was that way originally, and then I started writing an example 
monitoring the signal level and it went:

get("Network",success)
function success(connections) {
   watch("Network",success2,error,{id: connection.activeConnection.id});
}

function success2(activeConnection) {
   if (activeConnection.signalLevel) {
     ... continue execution flow ...
   }
}

2 nested callbacks is getting a little hairy, so I thought it would be 
better to write:

watch("Network", success);

function success(connection) {
   if (connection.signalLevel) {
     ... continue execution flow ...
   }
}

> 14) Have you considered adding the MAC address of the router to
> Connection?  (use case: WiFi positioning).

No. To be honest I didn't add too much there because I expect expert to 
tell me what's best to have. So I've added it. But just to make sure I'm 
understanding you right, you are suggesting adding something like
"readonly DOMString MacAddress" to Connection, right?


> 15) Given that this is a pretty long list of sensors, maybe we should
> add orientation, acceleration, and compass.  Theses are suppose to be
> done in the W3C Geolocation WG, but I do not see why they can't be
> added here... they are pretty small additions and do not have exactly
> the same privacy concerns.

I was hoping geolocation to do the work for us, but fair enough ;) I'd 
like a decision from the WG before starting, though.

> 16) I am sure that the "Privacy considerations for implementors" is
> going to be discussed a bunch.  One of the ideas that we might want
> to consider is allowing some property ids not have to prompt. For
> example, maybe PowerSource is something that we would like to
> generally expose without having any permission being granted.

Yes, I have in fact been wondering if any of the information exposed by 
this API has any real privacy concerns. Maybe someone will convince that 
some do and I'm happy to discriminate between properties that are or 
aren't private.

Max.

Received on Monday, 21 December 2009 12:29:00 UTC