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.

Sign up for GitHub

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

Jump to bottom

rmw_fastrtps supports service event gid uniqueness test. #2638

Merged
merged 1 commit into from
Oct 14, 2024
Merged

rmw_fastrtps supports service event gid uniqueness test. #2638

merged 1 commit into from
Oct 14, 2024

Conversation

fujitatomoya
Copy link
Collaborator

part of ros2/rmw#357

this depends on ros2/rmw_fastrtps#781

@fujitatomoya
Copy link
Collaborator Author

CI:

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

@fujitatomoya
Copy link
Collaborator Author

@clalancette @MiguelCompany can you review this?

@MiguelCompany
Copy link
Contributor

@fujitatomoya the rpr job failed because ros2/rmw_fastrtps#781 was not yet merged.
I don't know whether that matters given the CI run in #2638 (comment), but I suppose that just pushing the changes again would retrigger it.

@fujitatomoya
Copy link
Collaborator Author

@MiguelCompany thanks for the review.

the rpr job failed because ros2/rmw_fastrtps#781 was not yet merged.

i believe as long as CI is green, that is fine. I am not sure how often rpr system is rebased...

i would like to check with rmw_zenoh before pushing this. @Yadunund is working on that, so do not want to bring another breaking trouble for them with this.

@fujitatomoya
Copy link
Collaborator Author

@clalancette can you review and merge this?

CC: @Yadunund i did confirm this test is passing with rmw_zenoh, see https://github.com/ros2/rclcpp/pull/2638/files#r1792386899

Yadunund
Yadunund approved these changes Oct 9, 2024
View reviewed changes
Copy link
Member

@Yadunund Yadunund left a comment

Choose a reason for hiding this comment

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

LGTM

@fujitatomoya
Copy link
Collaborator Author

Pulls: #2638
Gist: https://gist.githubusercontent.com/fujitatomoya/6cc38769c5a97b7487be22039852e314/raw/657a433d4394c661f09019fb81e4e418a8b00ab1/ros2.repos
BUILD args: --packages-above-and-dependencies rclcpp
TEST args: --packages-above rclcpp
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/14685

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

@clalancette
Copy link
Contributor

@ros-pull-request-builder retest this please

@clalancette
Copy link
Contributor

This is only a change in the rclcpp tests, so the Windows test failure must be unrelated. Going ahead and merging this one.

@clalancette clalancette merged commit 0be8aa0 into rolling Oct 14, 2024
3 checks passed
@clalancette clalancette deleted the fujitatomoya/service-event-gid-uniqueness-fastrtps branch October 14, 2024 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clalancette clalancette clalancette approved these changes

@Yadunund Yadunund Yadunund approved these changes

@MiguelCompany MiguelCompany MiguelCompany approved these changes

@ivanpauno ivanpauno Awaiting requested review from ivanpauno ivanpauno is a code owner

@hidmic hidmic Awaiting requested review from hidmic hidmic is a code owner

@wjwwood wjwwood Awaiting requested review from wjwwood wjwwood is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@fujitatomoya @MiguelCompany @clalancette @Yadunund