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

Support Fast CDR v2 #114

Merged
merged 5 commits into from
Mar 26, 2024
Merged

Support Fast CDR v2 #114

merged 5 commits into from
Mar 26, 2024

Conversation

MiguelCompany
Copy link
Contributor

@MiguelCompany MiguelCompany commented Feb 29, 2024

This makes the necessary changes to allow building with Fast CDR v2

Part of ros2/ros2#1530

It is built on top of #113

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.

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 <miguelcompany@eprosima.com>
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
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.

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>
Copy link
Contributor

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)

Copy link
Contributor Author

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>
Copy link
Contributor

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)

Copy link
Contributor Author

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 <miguelcompany@eprosima.com>
@MiguelCompany
Copy link
Contributor Author

@clalancette The tests that were updated in #113 were not updated here. I've fixed that in 7fb44ae

@clalancette clalancette merged commit edf7835 into ros2:rolling Mar 26, 2024
2 of 3 checks passed
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.

None yet

2 participants