Skip to content

Drop workaround for Fast-RTPS typesupport issue #233

Merged
dirk-thomas merged 1 commit intomasterfrom
hidmic/drop-rmw-fastrtps-265-workaround
Feb 28, 2020
Merged

Drop workaround for Fast-RTPS typesupport issue #233
dirk-thomas merged 1 commit intomasterfrom
hidmic/drop-rmw-fastrtps-265-workaround

Conversation

@hidmic
Copy link
Copy Markdown

@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
Copy Markdown
Author

hidmic commented Jan 9, 2020 •
edited
Loading

CI up to ros1_bridge:

  • Packaging Linux Build Status

@dirk-thomas
Copy link
Copy Markdown
Member

dirk-thomas commented Jan 15, 2020

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown

MiguelCompany commented Jan 28, 2020

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
Copy Markdown
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
Copy Markdown
Member

dirk-thomas commented Jan 28, 2020

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

@hidmic
Copy link
Copy Markdown
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
Copy Markdown
Member

dirk-thomas commented Jan 28, 2020

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
Copy Markdown
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

1 more reviewer

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

Reviewers whose approvals may not affect merge requirements

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