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

Drop workaround for Fast-RTPS typesupport issue #233

Merged
merged 1 commit into from
Feb 28, 2020
Merged

Drop workaround for Fast-RTPS typesupport issue #233

merged 1 commit into from
Feb 28, 2020

Conversation

hidmic
Copy link
Contributor

@hidmic hidmic commented Jan 9, 2020

Precisely what the title says. To be tested and merged along with ros2/rmw_fastrtps#342.

@hidmic
Drop workaround for ros2/rmw_fastrtps#265.
376352c 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic hidmic changed the title Drop workaround for https://github.com/ros2/rmw_fastrtps/issues/265. Drop workaround for Fast-RTPS typesupport issue Jan 9, 2020
@hidmic
Copy link
Contributor Author

hidmic commented Jan 9, 2020 •
edited
Loading

CI up to ros1_bridge:

  • Packaging Linux Build Status

@dirk-thomas
Copy link
Member

Depending on the answer of this question ros2/rmw_fastrtps#342 (comment) this change should be considered for backporting.

@hidmic hidmic mentioned this pull request Jan 15, 2020
Merged
@hidmic
Copy link
Contributor Author

hidmic commented Jan 15, 2020

Hmm, ros1_bridge seems to exhibit segfaults when using rmw_fastrtps_dynamic_cpp only. I'll investigate.

This was referenced Jan 15, 2020
Closed
Closed
@hidmic hidmic mentioned this pull request Jan 27, 2020
Closed
@hidmic
Copy link
Contributor Author

hidmic commented Jan 27, 2020

This is blocked until eclipse-cyclonedds/cyclonedds#339 and ros2/rmw_fastrtps#265 are finally addressed. Which leaves us in the same place we were at when this workaround was first introduced. I pinged eProsima and ADLINK folks, but we may as well close this until those fixes land.

@MiguelCompany
Copy link

This is blocked until eclipse-cyclonedds/cyclonedds#339 and ros2/rmw_fastrtps#265

@hidmic I then suggest to change the title of this PR.

@hidmic hidmic changed the title Drop workaround for Fast-RTPS typesupport issue Drop workaround for Fast-RTPS and CycloneDDS typesupport issue Jan 28, 2020
@hidmic
Copy link
Contributor Author

hidmic commented Jan 28, 2020

I then suggest to change the title of this PR.

Fair enough :)

@dirk-thomas dirk-thomas changed the title Drop workaround for Fast-RTPS and CycloneDDS typesupport issue Drop workaround for Fast-RTPS typesupport issue Jan 28, 2020
@dirk-thomas
Copy link
Member

I changed the title back since the patch knows nothing about CycloneDDS but only checks for FastRTPS.

@hidmic
Copy link
Contributor Author

hidmic commented Jan 28, 2020

Oh, that's true. Silly me. I wonder why no one has run into this issue using CycloneDDS.

@dirk-thomas
Copy link
Member

I wonder why no one has run into this issue using CycloneDDS.

The test in the packaging job is failing with CycloneDDS for that reason.

@dirk-thomas dirk-thomas mentioned this pull request Feb 25, 2020
Merged
dirk-thomas
dirk-thomas approved these changes Feb 25, 2020
View reviewed changes
Copy link
Member

@dirk-thomas dirk-thomas left a comment

Choose a reason for hiding this comment

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

Passes with ros2/rmw_fastrtps#350.

@dirk-thomas dirk-thomas merged commit d0cd1a7 into master Feb 28, 2020
@delete-merged-branch delete-merged-branch bot deleted the hidmic/drop-rmw-fastrtps-265-workaround branch February 28, 2020 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dirk-thomas dirk-thomas dirk-thomas approved these changes

Assignees

@hidmic hidmic

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hidmic @dirk-thomas @MiguelCompany