-
Notifications
You must be signed in to change notification settings - Fork 28
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
Support Fast CDR v2 #114
Support Fast CDR v2 #114
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this one needs to be rebased on top #113 (at least, it seems to me that includes some of the same changes).
Once that is done I'll do another review.
Signed-off-by: Miguel Company <[email protected]>
Signed-off-by: Miguel Company <[email protected]>
Signed-off-by: Miguel Company <[email protected]>
925df83
to
ca20c7a
Compare
Signed-off-by: Miguel Company <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of small changes here.
@@ -30,7 +30,7 @@ | |||
<build_export_depend>rosidl_typesupport_interface</build_export_depend> | |||
|
|||
<!-- Needed for headers in this package, or the headers it generates, and also needs to be linked in downstream packages --> | |||
<depend>fastcdr</depend> | |||
<depend version_gte="2">fastcdr</depend> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as in ros2/rmw_fastrtps#746 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did it, but didn't push: a725fbb
@@ -32,7 +32,7 @@ | |||
<build_export_depend>rosidl_typesupport_interface</build_export_depend> | |||
|
|||
<!-- Needed for headers in this package, or the headers it generates, and also needs to be linked in downstream packages --> | |||
<depend>fastcdr</depend> | |||
<depend version_gte="2">fastcdr</depend> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as in ros2/rmw_fastrtps#746 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did it, but didn't push: a725fbb
Signed-off-by: Miguel Company <[email protected]>
@clalancette The tests that were updated in #113 were not updated here. I've fixed that in 7fb44ae |
This makes the necessary changes to allow building with Fast CDR v2
Part of ros2/ros2#1530
It is built on top of #113