-
Notifications
You must be signed in to change notification settings - Fork 10
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
Make PressureObserver.observe() and data delivery algorithm less vague #283
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Related to #282: we need these algorithms to be properly defined in order to be able to support WebDriver and fake pressure states. This somewhat big change intends to clarify what "activate" and "deactivate" data delivery actually mean, as there used to be just a "data delivery" algorithm and no accompanying definitions for those two verbs. Furthermore, the data delivery algorithm itself was confusing: - It referenced a `data` variable in its declaration that was never passed by any callers. - `data` was of an implementation-defined type and format, but the steps assumed it had some associated information like source type that was not set anywhere. Fixing the above has required changes in different layers: - The "platform collector" concept, which used to be an abstract entity with which all globals interacted to retrieve telemetry data for all source types, is now a per-global and per-source type concept. The lower-level concept that represents a cross-global interface for the hardware or OS is now a "pressure source", which contains a snapshot of the latest reading it has retrieved along with a timestamp. - "Data delivery" is now called "data collection". It uses a platform collector and its associated pressure source to retrieve a telemetry sample that is transformed into a pressure state. - There are algorithms for activating and deactivating data collection. Both ensure they data collection cannot be started/stopped if they have already been. - `PressureObserver.observe()`'s had a "is not a valid source type" check that was too vague, as this step determined whether a given source type is supported by the platform or not, but the definition of "valid source type" was something else entirely. This step has been replaced by a sequence of steps that attempts to retrieve an existing platform collector for a source type and, if one does not exist, tries to connect to a corresponding pressure source. This change makes the same platform collector be used for all observers of a given source type and lays out in more detail what it means to check whether a source type is valid or not in this context. Co-authored with @kenchris in #265. It was split off as a separate pull request to make it easier to review and understand.
kenchris
approved these changes
Jun 14, 2024
arskama
reviewed
Jun 14, 2024
arskama
approved these changes
Jun 14, 2024
This was referenced Jun 14, 2024
anssiko
pushed a commit
that referenced
this pull request
Jun 17, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related to #282: we need these algorithms to be properly defined in order to
be able to support WebDriver and fake pressure states.
This somewhat big change intends to clarify what "activate" and "deactivate"
data delivery actually mean, as there used to be just a "data delivery"
algorithm and no accompanying definitions for those two verbs. Furthermore,
the data delivery algorithm itself was confusing:
data
variable in its declaration that was never passedby any callers.
data
was of an implementation-defined type and format, but the stepsassumed it had some associated information like source type that was not
set anywhere.
Fixing the above has required changes in different layers:
with which all globals interacted to retrieve telemetry data for all
source types, is now a per-global and per-source type concept.
The lower-level concept that represents a cross-global interface for the
hardware or OS is now a "pressure source", which contains a snapshot of
the latest reading it has retrieved along with a timestamp.
collector and its associated pressure source to retrieve a telemetry
sample that is transformed into a pressure state.
ensure they data collection cannot be started/stopped if they have already
been.
PressureObserver.observe()
's had a "is not a valid source type" checkthat was too vague, as this step determined whether a given source type
is supported by the platform or not, but the definition of "valid source
type" was something else entirely.
This step has been replaced by a sequence of steps that attempts to
retrieve an existing platform collector for a source type and, if one does
not exist, tries to connect to a corresponding pressure source. This
change makes the same platform collector be used for all observers of a
given source type and lays out in more detail what it means to check
whether a source type is valid or not in this context.
Co-authored with @kenchris in #265. It was split off as a separate pull
request to make it easier to review and understand.
Preview | Diff