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

publish with QoS reliable as default #1224

Merged
merged 2 commits into from
Feb 18, 2021
Merged

publish with QoS reliable as default #1224

merged 2 commits into from
Feb 18, 2021

Conversation

BrettRD
Copy link

@BrettRD BrettRD commented Feb 9, 2021

This pull request is a minimal fix to ensure compatibility with rqt and other default (reliable) subscribers, while still allowing production systems to take advantage of best-effort transport, and still demonstrating the use of the recommended QoS profiles.

This is a revision of #1219

This applies only to publishers defaulting to rclcpp::SensorDataQoS() and sets defaults to rclcpp::SensorDataQoS().reliable()

Copy link
Collaborator

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

Thanks, this change seems reasonable to me.

Linters are complaining about the line length being over 100 characters in some places now. After fixing that, I think this change is good.

@BrettRD
Copy link
Author

BrettRD commented Feb 17, 2021

@jacobperron I think the failing tests are unrelated to the patch now

@jacobperron
Copy link
Collaborator

jacobperron commented Feb 17, 2021

I think the failing tests are unrelated to the patch now

I think so too, I'll retrigger anyways to see if there are some flakes.

@jacobperron
Copy link
Collaborator

@ros-pull-request-builder retest this please

@jacobperron jacobperron merged commit 8a32066 into ros-simulation:foxy Feb 18, 2021
jacobperron pushed a commit that referenced this pull request Feb 18, 2021
* default to reliable publishers
@jacobperron
Copy link
Collaborator

Forward port to Rolling: #1235

@BrettRD BrettRD mentioned this pull request Feb 23, 2021
jacobperron added a commit that referenced this pull request Mar 18, 2021
* default to reliable publishers

Co-authored-by: Brett Downing <BrettRD@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants