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

Use FastDDS Sync as default DDS #315

Merged
merged 1 commit into from
Jan 25, 2022
Merged

Conversation

audrow
Copy link
Member

@audrow audrow commented Nov 29, 2021

Switching the default DDS to FastDDS Sync after the Technical Steering Committee's recent vote (results). You can find the full meeting minutes here: https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-2021-11-18/23209.

This PR relies on ros2/rmw_fastrtps#571.

FYI @JaimeMartin and @MiguelCompany.

Signed-off-by: Audrow Nash <audrow@hey.com>
@audrow audrow changed the title Use FastDDS as default DDS Use FastDDS Sync as default DDS Nov 29, 2021
@audrow
Copy link
Member Author

audrow commented Nov 29, 2021

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status
  • Windows Debug Build Status
  • Linux clang Build Status

@emersonknapp
Copy link
Contributor

emersonknapp commented Nov 29, 2021

EDIT: def not related - sorry for the noise

Is this change related at all to ros2/ci#601 issue? The Windows CI started failing, and the other activity I see there is related to this. Not sure if that's just a coincidence though

@emersonknapp
Copy link
Contributor

@audrow if you're overriding builds when making newer changes, might make sense to cancel the older ones still running in the queue/runners (unless their results are still relevant). There's a pretty big backup right now since a full windows build takes 3+ hrs, it's hard to tell from this context here if all the builds are needed.

@audrow
Copy link
Member Author

audrow commented Nov 30, 2021

@audrow if you're overriding builds when making newer changes, might make sense to cancel the older ones still running in the queue/runners (unless their results are still relevant). There's a pretty big backup right now since a full windows build takes 3+ hrs, it's hard to tell from this context here if all the builds are needed.

@emersonknapp, it's a good suggestion. I don't think I've left any jobs running. I restarted the Windows jobs, since the previous runs already failed and it looks like it was a connectivity issue.

@emersonknapp
Copy link
Contributor

emersonknapp commented Nov 30, 2021

EDIT: sorry I was slightly misreading the situation, please ignore the noise i am making today

@ros-discourse
Copy link

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-12-16-2021/23541/1

@audrow
Copy link
Member Author

audrow commented Dec 30, 2021

Here's another run:

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

@audrow
Copy link
Member Author

audrow commented Jan 14, 2022

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

@MiguelCompany
Copy link

@audrow I just checked locally that merging eProsima/Fast-DDS#2358 fixes the crash on Windows when the tests of tf2_ros_py exit.

@MiguelCompany
Copy link

@audrow eProsima/Fast-DDS#2358 has just been merged. Please run CI again, thank you

@clalancette
Copy link
Contributor

Another try at CI:

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

@clalancette
Copy link
Contributor

clalancette commented Jan 21, 2022

@MiguelCompany It works! Thanks for the fix.

Here's a few more jobs now to see where we are at:

  • Windows Debug Build Status
    • Rerun Build Status
  • Linux Clang Build Status
  • RHEL Build Status
  • Linux packaging Build Status

@ros-discourse
Copy link

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-january-20th-2022/23986/1

@clalancette
Copy link
Contributor

All right, with CI mostly green (RHEL is a known problem, and Windows Debug looks a bit flaky), I think we are good to merge this. I'm going to do that and also merge in ros2/rmw_fastrtps#571 at the same time.

@clalancette clalancette merged commit de38e79 into master Jan 25, 2022
@delete-merged-branch delete-merged-branch bot deleted the audrow/update-default-dds branch January 25, 2022 13:22
Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

Post-hoc approving.

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.

6 participants