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

zero copy api for cyclonedds #53

Merged
merged 2 commits into from
Oct 21, 2019
Merged

zero copy api for cyclonedds #53

merged 2 commits into from
Oct 21, 2019

Conversation

Karsten1987
Copy link
Contributor

addresses the missing functions for zero-copy API as noted in here: ros2/ros2#785 (comment)

Connects to ros2/rclcpp#896

Signed-off-by: Karsten Knese [email protected]

Karsten1987 and others added 2 commits October 20, 2019 22:15
@Karsten1987
zero copy api for cyclonedds
9414622 
Signed-off-by: Karsten Knese <[email protected]>
@eboasson
Add already obsoleted loaned message interfaces
fca78db 
The rmw_return_loaned_message and rmw_release_loaned_message functions
are still expected by rcl, even if they are scheduled to be replaced.
We need a working build, so add them for now.

Signed-off-by: Erik Boasson <[email protected]>
@eboasson
Copy link
Collaborator

@Karsten1987 @dirk-thomas This PR doesn't work for me locally because it provides the interface as amended by ros2/rmw#192, rather than the one currently implemented by rcl on master.

Adding the "old" interface (rmw_return_loaned_message and rmw_release_loaned_message) solves the problem. It is somewhat silly to add them only to remove them again in the very near future, but as @dirk-thomas mentioned on ros2/ros2#785, we really need to get this thing building again. Pushing this now should fix tonight's CI run.

Hopefully the two of you don't mind this temporary thing. If you do, too bad for me ...

@eboasson eboasson merged commit 7bfd709 into master Oct 21, 2019
@delete-merged-branch delete-merged-branch bot deleted the return_loaned_message branch October 21, 2019 10:38
@Karsten1987 Karsten1987 mentioned this pull request Oct 21, 2019
Merged
@Karsten1987
Copy link
Contributor Author

I think this is fine to make cyclone build with CI as long as the PRs for renaming the functions are not merged. I've come up with a follow up PR for removing the "legacy" functions then.
#55

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Karsten1987 @eboasson