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

Change sampleRate to sampleInterval #254

Merged
merged 4 commits into from
Mar 19, 2024
Merged

Change sampleRate to sampleInterval #254

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2d9d134
Change samplerate to sampleInterval
kenchris Mar 18, 2024
f469c0a
Fix comments from Raphael
kenchris Mar 19, 2024
ebc1921
Apply suggestions from code review
kenchris Mar 19, 2024
b4b254b
Add part that was removed
kenchris Mar 19, 2024
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
46 changes: 22 additions & 24 deletions index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,14 +197,25 @@ <h3>Supported sources</h3>
</section>
<section>
<h3>Sampling and Reporting Rate</h3>
<p>
The <dfn>requested sampling interval</dfn> represents the desired
interval between samples, expressed in milliseconds.
kenchris marked this conversation as resolved.
Show resolved Hide resolved
</p>
<p>
Interval and frequency are inverses of each other, so the
[=requested sampling interval=] can also be expressed as a
<dfn>requested sampling rate</dfn> in Hertz (cycles per second) by
dividing 1000 by the [=requested sampling interval=] value.
</p>
<p>
The <dfn>sampling rate</dfn> for a [=platform collector=] is defined as a rate
at which the [=user agent=] obtains telemetry readings from the underlying platform,
and it might differ from the pressure observers' [=requested sampling rates=].
rakuco marked this conversation as resolved.
Show resolved Hide resolved
The rate is measured in Hertz (cycles per second).
</p>
<p>
The <dfn>reporting rate</dfn> for a pressure observer is the rate at which it runs
the [=data delivery=] steps.
the [=data delivery=] steps, and it will never exceed the [=sampling rate=].
</p>
<p>
The [=sampling rate=] differs from the [=requested sampling rate=] when the
Expand All @@ -221,7 +232,7 @@ <h3>Sampling and Reporting Rate</h3>
</p>
<aside class="note">
In case there are multiple instances of {{PressureObserver}} active with different
[=requested sampling rates=], it is up to the [=user agent=] to set a
[=requested sampling intervals=], it is up to the [=user agent=] to set a
[=platform collector=] level [=sampling rate=] that best fulfills these requests,
while making sure that the [=reporting rate=] of all {{PressureObserver}}s does
not exceed their respective [=requested sampling rates=].
Expand Down Expand Up @@ -366,7 +377,7 @@ <h3>
a <dfn>[[\Callback]]</dfn> of type {{PressureUpdateCallback}} set on creation.
</li>
<li>
a <dfn>[[\SampleRate]]</dfn> double set on creation.
a <dfn>[[\SampleInterval]]</dfn> unsigned long set on creation.
</li>
<li>
a <dfn>[[\PendingObservePromises]]</dfn> [=list=] of zero or more source-promise [=tuples=], initially empty,
Expand Down Expand Up @@ -531,10 +542,7 @@ <h3>The <dfn>constructor()</dfn> method</h3>
Set |this|.{{PressureObserver/[[Callback]]}} to |callback:PressureUpdateCallback|.
</li>
<li>
If |options|["sampleRate"] is less than or equal to 0, throw a {{RangeError}}.
Copy link
Member

Choose a reason for hiding this comment

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

0 is a valid value for unsigned long and unsigned long long. If is allowed here, it means users can make the "passes rate test" check always return true. It's not exactly wrong, so it's more like what you'd like to communicate here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is a bit weird that it becomes a RangeError, so I more consider it the fastest frequency possible, which the implementation can limit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Basically 0 means, give me events as soon/fast as possible

</li>
<li>
Set |this:PressureObserver|.{{PressureObserver/[[SampleRate]]}} to |options|["sampleRate"].
Set |this:PressureObserver|.{{PressureObserver/[[SampleInterval]]}} to |options|["sampleInterval"].
</li>
</ol>
</p>
Expand Down Expand Up @@ -811,25 +819,18 @@ <h3>The <dfn>toJSON</dfn> member</h3>
<h3>The <dfn>PressureObserverOptions</dfn> dictionary</h3>
<pre class="idl">
dictionary PressureObserverOptions {
double sampleRate = 1.0;
[EnforceRange] unsigned long sampleInterval = 0;
};
</pre>
<section>
<h3>The <dfn>sampleRate</dfn> member</h3>
<h3>The <dfn>sampleInterval</dfn> member</h3>
<p>
The {{PressureObserverOptions/sampleRate}} member represents the <dfn>requested sampling
rate</dfn> expressed in Hz, ie. it represents the number of samples requested to be obtained
from the hardware per second. The [=reporting rate=] will never exceed the [=requested sampling rate=].
The {{PressureObserverOptions/sampleInterval}} member represents the [=requested sampling
interval=] expressed in milliseconds.
</p>
<aside class="note">
<p>
For slow sampling rates (less than 1hz), it is common to use seconds and talk about
period instead of rate. Fortunately, it is easy to convert between the two, as hertz
is rotations per second. If you want to request a sampling rate of 10 seconds, then
just use the value of 0.1.
</p>
<p>
A [=user agent=] might not be able to respect the requested sampling rate. For more information
A [=user agent=] might not be able to respect the requested sampling interval. For more information
consult [[[#sampling-and-reporting-rate]]].
</p>
</aside>
Expand Down Expand Up @@ -1039,16 +1040,13 @@ <h3>Supporting algorithms</h3>
Let |record:PressureRecord| be |observer|.{{PressureObserver/[[LastRecordMap]]}}[|source|].
</li>
<li>
Let |sampleRate| be |observer|.{{PressureObserver/[[SampleRate]]}}.
Let |sampleInterval| be |observer|.{{PressureObserver/[[SampleInterval]]}}.
</li>
<li>
Let |timeDeltaMilliseconds:DOMHighResTimeStamp| = |timestamp| - |record|.{{PressureRecord/[[Time]]}}.
</li>
<li>
Let |intervalSeconds| = 1 / |sampleRate|.
</li>
<li>
If (|timeDeltaMilliseconds| / 1000) &ge; |intervalSeconds|, return true, otherwise return false.
If |timeDeltaMilliseconds| &ge; |sampleInterval|, return true, otherwise return false.
</li>
</ol>
The <dfn>has change in data</dfn> steps given the argument |observer:PressureObserver|, |source:PressureSource|,
Expand Down
Loading