w3c / compute-pressure

A web API proposal that provides information about available compute capacity
https://www.w3.org/TR/compute-pressure/
Other
69 stars 10 forks source link

Automation section and WebDriver support #265

Closed kenchris closed 3 months ago

kenchris commented 5 months ago

Preview | Diff

arskama commented 5 months ago

1) I find it a bit strange that the creation of the virtual source also defines the sample array. Should we restrict the creation to only create the virtual source? That would allow the user to control when the updates actually happen.

2) Also we don't have a getter version, so we cannot check what was the set samplingInterval at creation. Should we have one Getter returning the existing virtual sources and their set samplingInterval?

3) Should we have an example to show what needs to be done to for example set a new samplingInterval to a virtual source? Since apparently the virtual source needs to be deleted first? (right?)

rakuco commented 4 months ago
  1. I find it a bit strange that the creation of the virtual source also defines the sample array. Should we restrict the creation to only create the virtual source? That would allow the user to control when the updates actually happen.

This ended up being fixed a while ago.

  1. Also we don't have a getter version, so we cannot check what was the set samplingInterval at creation. Should we have one Getter returning the existing virtual sources and their set samplingInterval?

Getters are tricky because the information they return might be outdated by the time it reaches users. Unless really needed, I don't think we need to provide ways to access this information (which should be available to users anyway given that they made the WebDriver calls).

  1. Should we have an example to show what needs to be done to for example set a new samplingInterval to a virtual source? Since apparently the virtual source needs to be deleted first? (right?)

If we take the "let users handle the sampling rate" approach I mentioned in one of the inline comments, this might end up becoming a non-issue as a side-effect.

rakuco commented 4 months ago

@kenchris @arskama @JuhaVainio I've just pushed a new big commit, c01a9a913d9cc0d85b9043f5eefe5924b67fd40e, that implements the remaining feedback from a comment I posted a while ago. The commit message links to the specific comment I'm referring to and describes what's being done.

In short:

The latter behavior is not set in stone, but it is the easiest/most conservative option, so we can use it as a starting point (the previous behavior was just unspecified, which is worse IMO).

Please take a look at the recent changes; if all goes well the only thing left is properly handling a sample's timestamp, which also depends on #274.

rakuco commented 3 months ago

Please take a look at the recent changes; if all goes well the only thing left is properly handling a sample's timestamp, which also depends on #274.

I've pushed a couple of commits now that #274 has been merged (although #279 and #280 are still pending). I'm basically adding more <dfn>s so that real pressure sources provide data as "pressure source samples" (which have platform-provided data + a timestamp) and virtual pressure sources' pending samples now have both a PressureState and a timestamp.

The "data collection" steps take care of transforming both timestamps into something consumable (i.e. the existing call to "relative high resolution time").

rakuco commented 3 months ago

If pressure source sample is now describing also "virtual pressure source" samples.

Would the definition of data require change?

Good point, I think allowing a PressureState there was too much of a stretch. I've made the definition clearer in commit d11d169 by saying data is a PressureState when the pressure source is a virtual one.

rakuco commented 3 months ago

Alright, everything's been taken care of! The last bigger change is 93beaa05487f42021aaf9abf106bc36db1c8e378, which aligns the pressure type checks in the WebDriver endpoints with the same list of types used by PressureObserver.knownSources.

It does mean that one cannot add a virtual pressure source for a type that is not in knownSources. Whether that is really desirable can be changed by the editors later.

rakuco commented 3 months ago

There are a lot of changes in this PR as everyone can see. A lot of them are not in the newly-added Automation section, but in the rest where we had to provide less vague definitions for some concepts and make some of the algorithms less abstract.

I will file a tracking bug for the WebDriver work and split this PR into multiple ones to make the changes easier to understand in the future.

arskama commented 3 months ago

One major change we forgot in this PR: Raphael as an Editor. :)

kenchris commented 3 months ago

Indeed, I wanted to mention that in the meeting yesterday and totally forgot!

rakuco commented 3 months ago

The changes here have landed in #283 and #284. Closing.