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 QoS overrides to stereo_image_proc #651

Merged
merged 6 commits into from
Jul 22, 2021
Merged

Add QoS overrides to stereo_image_proc #651

merged 6 commits into from
Jul 22, 2021

Conversation

audrow
Copy link
Collaborator

@audrow audrow commented Apr 14, 2021

@audrow audrow requested a review from jacobperron April 28, 2021 22:32
@audrow audrow marked this pull request as ready for review April 28, 2021 22:32
@jacobperron jacobperron changed the title Add QoS overrides Add QoS overrides to stereo_image_proc Apr 29, 2021
Copy link
Contributor

@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.

LGTM, one nitpick

@jacobperron
Copy link
Contributor

We should make sure the CI jobs are green before merging. Since this change is not compatible with Foxy, we should probably branch (FYI @JWhitleyWork). For Rolling, we should make releases for the changes to image_common and message_filters (I can help with that); for Galactic it may require backporting those changes (I'll check when releasing for Rolling).

@audrow
Copy link
Collaborator Author

audrow commented May 19, 2021

I think I'm going to make a PR to rclcpp to create a default QoS override options object to remove some redundancy.

@jacobperron
Copy link
Contributor

@JWhitleyWork Friendly ping for review. It sounds like we may also want to re-target this to a new rolling branch once it exists (#659 (comment))

@jacobperron
Copy link
Contributor

@ros-pull-request-builder retest this please

@pmusau17
Copy link
Contributor

Tested this on ros2 galactic and ros2 rolling. It builds and passes all tests. I made two small edits to the package.xml files in this PR.

@jacobperron jacobperron changed the base branch from ros2 to rolling June 28, 2021 23:28
@jacobperron
Copy link
Contributor

Re-targeted rolling.

@clalancette
Copy link
Contributor

@ros-pull-request-builder retest this please

1 similar comment
@clalancette
Copy link
Contributor

@ros-pull-request-builder retest this please

@JWhitleyWork JWhitleyWork merged commit c989092 into ros-perception:rolling Jul 22, 2021
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.

6 participants