Skip to content

Conversation

@jacobperron
Copy link
Member

Design doc: http://design.ros2.org/articles/qos_configurability.html
Implementation (C++): ros2/rclcpp#1408
Implementation (Python): ros2/rclpy#635

@mjcarroll mjcarroll temporarily deployed to ros2-documentation-pr-1033 January 21, 2021 00:45 Inactive
@jacobperron
Copy link
Member Author

cc/ @maryaB-osr Once it's ready, we can add a link to the tutorial here too

------------------------------------

It is now possible to externally configure the QoS settings for a node at start-up time.
QoS settings are **not** configurable during runtime and node authors must opt-in to enable this feature.
Copy link
Member

Choose a reason for hiding this comment

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

this seems a bit confusing to me, maybe:

Suggested change
QoS settings are **not** configurable during runtime and node authors must opt-in to enable this feature.
QoS settings are configurable during runtime only if the node authors have enabled this feature.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ivanpauno I think that rewrite completely changes the meaning, and suggests that QoS can be changed via runtime. So I don't think we want to say that.

Really, I think this sentence should be split into two because it is talk about two separate topics. Something like:

QoS settings are **not** configurable during runtime; they are only configurable at start-up time.
In order to change the QoS settings at runtime, node authors must opt-in to enable this feature.

Or something like that. The previous sentence may need to be tweaked as well.

Copy link
Member

Choose a reason for hiding this comment

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

ah ok, I got it now.
The distinction between "runtime" and "startup time" can be confusing (because "startup time" is also "runtime").

Really, I think this sentence should be split into two because it is talk about two separate topics

👍 your suggested wording looks fine!

Copy link
Member Author

Choose a reason for hiding this comment

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

I've updated the wording in 792ccbd

@ivanpauno ivanpauno added the enhancement New feature or request label Jan 21, 2021
Signed-off-by: Jacob Perron <[email protected]>
@clalancette clalancette merged commit 76c31dc into master Jan 21, 2021
@delete-merged-branch delete-merged-branch bot deleted the jacob/galactic_feat/configure_qos branch January 21, 2021 20:54
@clalancette
Copy link
Contributor

@Mergifyio backport rolling

mergify bot pushed a commit that referenced this pull request Jan 21, 2021
* Add Galactic feature note for externally configuring QoS

Signed-off-by: Jacob Perron <[email protected]>
(cherry picked from commit 76c31dc)
@mergify
Copy link
Contributor

mergify bot commented Jan 21, 2021

Command backport rolling: success

Backports have been created

clalancette pushed a commit that referenced this pull request Jan 21, 2021
* Add Galactic feature note for externally configuring QoS

Signed-off-by: Jacob Perron <[email protected]>
(cherry picked from commit 76c31dc)

Co-authored-by: Jacob Perron <[email protected]>
mergify bot added a commit that referenced this pull request Jan 21, 2021
* Add Galactic feature note for externally configuring QoS

Signed-off-by: Jacob Perron <[email protected]>
(cherry picked from commit 76c31dc)

Co-authored-by: Jacob Perron <[email protected]>
(cherry picked from commit 1d457fe)
mergify bot added a commit that referenced this pull request Jan 21, 2021
* Add Galactic feature note for externally configuring QoS

Signed-off-by: Jacob Perron <[email protected]>
(cherry picked from commit 76c31dc)

Co-authored-by: Jacob Perron <[email protected]>
(cherry picked from commit 1d457fe)
clalancette pushed a commit that referenced this pull request Jan 21, 2021
…) (#1040)

* Add Galactic feature note for externally configuring QoS

Signed-off-by: Jacob Perron <[email protected]>
(cherry picked from commit 76c31dc)

Co-authored-by: Jacob Perron <[email protected]>
(cherry picked from commit 1d457fe)

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
clalancette pushed a commit that referenced this pull request Jan 21, 2021
…) (#1041)

* Add Galactic feature note for externally configuring QoS

Signed-off-by: Jacob Perron <[email protected]>
(cherry picked from commit 76c31dc)

Co-authored-by: Jacob Perron <[email protected]>
(cherry picked from commit 1d457fe)

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
@ivanpauno
Copy link
Member

@Mergifyio backport rolling

wow, I want to use that!!

@mergify
Copy link
Contributor

mergify bot commented Jan 21, 2021

Command backport rolling: success

Backports have been created

@ivanpauno
Copy link
Member

we have checked that it handles a duplicated command correctly now hahaha

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants