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

Change the system default depth to 1. #342

Closed
wants to merge 1 commit into from

Conversation

clalancette
Copy link
Contributor

This is what the RMWs were doing anyway, and setting it to a number here (and not an unknown of 0) means that we won't get warnings from the rclcpp and rclpy layers.

Signed-off-by: Chris Lalancette clalancette@openrobotics.org

Fixes #341

This is what the RMWs were doing anyway, and setting it to
a number here (and not an unknown of 0) means that we won't
get warnings from the rclcpp and rclpy layers.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette
Copy link
Contributor Author

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

@clalancette it looks good for now, but according to https://docs.ros.org/en/rolling/Concepts/About-Quality-of-Service-Settings.html

Different RMW implementations may have different defaults.

that means, we should ask RMW implementation what the default value is?

@clalancette
Copy link
Contributor Author

that means, we should ask RMW implementation what the default value is?

Hm, that's a good point. Maybe this isn't the correct thing to do, then. Maybe the correct thing to do is to special-case SystemDefaultsQoS to avoid the warning message, since in that case the user very specifically asked to use whatever the underlying middleware defaults are.

What do you think?

@fujitatomoya
Copy link
Collaborator

Maybe the correct thing to do is to special-case SystemDefaultsQoS to avoid the warning message, since in that case the user very specifically asked to use whatever the underlying middleware defaults are.

this sounds right to me. and then rmw implementation sets the implementation specific QoS depth accordingly. (e.g rmw_fastftps case, it will set 1 for depth.)

@clalancette
Copy link
Contributor Author

See ros2/rclcpp#2082 instead. I'm going to close this one out.

@clalancette clalancette deleted the clalancette/fix-system-default-depth branch January 13, 2023 16:37
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.

Warning about history depth of 0 even while using system default QoS
2 participants