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.

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

Add content to "multiple-sensors" section #308

Merged
merged 1 commit into from
Oct 12, 2017

Conversation

alexshalamov
Copy link

@alexshalamov alexshalamov commented Oct 11, 2017

index.bs Outdated

Conversely, multiple {{Sensor|Sensors}} of the same [=sensor type=] may need to be created, when

Choose a reason for hiding this comment

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

s/Conversely/On the other hand ?

Choose a reason for hiding this comment

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

s/may need to be created/may be created

index.bs Outdated
@@ -1503,12 +1503,17 @@ exposing [=low-level=] sensor APIs, but should also expose

<h3 id="multiple-sensors">When is Enabling Multiple Sensors of the Same Type Not the Right Choice?</h3>

TODO: provide guidance on when to:
It is possible to construct multiple {{Sensor}} instances of the same [=sensor type=], however,
it is not advisable to do that when these instances are constructed using equal {{SensorOptions}},

Choose a reason for hiding this comment

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

"it is not advisable to have multiple {{Sensor}} objects that were constructed with the same construction parameters as it can unnecessarily increase CPU/power load. Instead, multiple observers should add event listeners to a single {{Sensor}} object. " wdyt?

index.bs Outdated

Conversely, multiple {{Sensor|Sensors}} of the same [=sensor type=] may need to be created, when
they are intended to be used with different settings, such as: [=requested sampling frequency=],

Choose a reason for hiding this comment

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

if they have different properties

index.bs Outdated
- allow multiple sensors of the same type to be instantiated,
- create different interfaces that inherit from {{Sensor}},
- add constructor parameters to tweak sensors settings (e.g. setting required accuracy).
In cases when multiple observers are interested of notifications for a newly available
Copy link
Member

Choose a reason for hiding this comment

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

interested in notifications of

Copy link
Author

Choose a reason for hiding this comment

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

done

index.bs Outdated
- create different interfaces that inherit from {{Sensor}},
- add constructor parameters to tweak sensors settings (e.g. setting required accuracy).
In cases when multiple observers are interested of notifications for a newly available
[=sensor reading=], [=event listener=] should be added for a {{Sensor}} instead of creating
Copy link
Member

Choose a reason for hiding this comment

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

IIUC, perhaps:

an event listener can be added on a single Sensor instance instead of creating

We try to generally avoid using RFC 2119 terms in informative sections, thus s/should/can/.

Copy link
Author

Choose a reason for hiding this comment

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

done

index.bs Outdated

Conversely, multiple {{Sensor|Sensors}} of the same [=sensor type=] may need to be created, when
Copy link
Member

Choose a reason for hiding this comment

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

s/may/can/

Copy link
Author

Choose a reason for hiding this comment

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

done

@alexshalamov alexshalamov force-pushed the multiple_sensors_todo branch 3 times, most recently from f67342a to f46c9ae Compare October 12, 2017 12:15
Copy link

@pozdnyakov pozdnyakov left a comment

Choose a reason for hiding this comment

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

lgtm

@alexshalamov alexshalamov merged commit 3bcde1a into w3c:master Oct 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants