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

Changes to build against Fast DDS 3.0 #776

Open
wants to merge 10 commits into
base: rolling
Choose a base branch
from

Conversation

MiguelCompany
Copy link
Collaborator

@MiguelCompany MiguelCompany commented Sep 6, 2024

This PR brings in the necessary changes to build against Fast DDS 3.0.x

It has been reviewed by the eProsima Team in eProsima#32

Part of ros2/ros2#1598

Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

@MiguelCompany DCO is missing, can you check that?

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've left two minor questions.

Otherwise, this tentatively looks good to me, but I'd like to see full CI, including Windows Debug and a clang run.

@@ -38,10 +38,8 @@ find_package(rmw_dds_common REQUIRED)
find_package(rmw_fastrtps_shared_cpp REQUIRED)
find_package(tracetools REQUIRED)

find_package(fastrtps_cmake_module REQUIRED)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to keep fastrtps_cmake_module around at all, or should we look at removing that package entirely?

Copy link
Contributor

Choose a reason for hiding this comment

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

This might be removed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@@ -234,263 +250,234 @@ _create_type_name(const MembersType * members)
std::string message_namespace(members->message_namespace_);
std::string message_name(members->message_name_);
if (!message_namespace.empty()) {
// Find and replace C namespace separator with C++, in case this is using C typesupport
message_namespace = rcpputils::find_and_replace(message_namespace, "__", "::");
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain a little bit more of why this is needed now?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is needed to ensure same type coming from Python or C++ is the same type at DDS X-Types level. Without this a Python application will register a different DDS-XTypes TypeObject and maybe won't communicate with a C++ application.

This rename is already done in other places like MessageTypeSupport_impl.

Mario-DL and others added 7 commits October 16, 2024 08:10
* Fast DDS 3.0: First commit

Signed-off-by: Ricardo González Moreno <[email protected]>

* Fast DDS 3.0: continue

Signed-off-by: Ricardo González Moreno <[email protected]>

* Refs #21322: rmw_fastrtps_shared_cpp: update headers

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322: rmw_fastrtps_shared_cpp: move namespace from fastrtps to fastdds

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322: rmw_fastrtps_shared_cpp: Update export macro to FASTDDS

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322: rmw_fastrtps_shared_cpp: Revert header guard to the same name as the package name

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322: rmw_fastrtps_shared_cpp: Update ReturnCode_t

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322: rmw_fastrtps_shared_cpp: remove deprecated test

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322: rmw_fastrtps_shared_cpp: Update representation in test

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322: rmw_fastrtps_shared_cpp: Update is_plain() call

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322: rmw_fastrtps_shared_cpp: Update cmake extra dependencies to Fast 3.0.0 and doxygen predefied macro

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322: rmw_fastrtps_shared_cpp: TODO continue. Register type identifies if not registered

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322: rmw_fastrtps_dynamic_cpp: update headers

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322: rmw_fastrtps_dynamic_cpp: update to topicdatatype refactor

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322: rmw_fastrtps_dynamic_cpp: Update ReturnCode_t

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322: rmw_fastrtps_dynamic_cpp: move namespace from fastrtps to fastdds

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322: rmw_fastrtps_cpp: typo in CMakeLists.txt

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322: rmw_fastrtps_cpp: Typesupport receives rosidl_message_typesupport in construction

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322: rmw_fastrtps_cpp: update headers

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322: rmw_fastrtps_cpp: update to topicdatatype refactor

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322: rmw_fastrtps_cpp: Update ReturnCode_t

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322: rmw_fastrtps_cpp: move namespace from fastrtps to fastdds

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322: rmw_fastrtps_cpp: update rme_dynamic_message methods to fastdds

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322: rmw_fastrtps_cpp: use register_type_object_representation() method

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322: rmw_fastrtps_cpp: subscirption: cast to DynamicType::_ref_type

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322: rmw_fastrtps_cpp: required fastdds 3.0 in cmake extras

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322: rmw_fastrtps_shared_cpp: getter for ros msg typesupports

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322: rmw_fastrtps_shared_cpp: check return codes in write() operations

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322: rmw_fastrtps_shared_cpp: complete GetTypeIdentifiers() and solve minor warnings

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322: rmw_fastrtps_dynamic_cpp: required fastdds 3.0 in cmake extras

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322: rmw_fastrtps_dynamic_cpp: update typesupport

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322: rmw_fastrtps_dynamic_cpp: update headers and namespaces

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322. Fixes

Signed-off-by: Ricardo González Moreno <[email protected]>

* Refs #21322: Apply rev suggestions

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322: rmw_fastrtps_shared_cpp: linters

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322: rmw_fastrtps_dynamic_cpp: linters

Signed-off-by: Mario Dominguez <[email protected]>

* Refs #21322: rmw_fastrtps_cpp: linters

Signed-off-by: Mario Dominguez <[email protected]>

---------

Signed-off-by: Ricardo González Moreno <[email protected]>
Signed-off-by: Mario Dominguez <[email protected]>
Co-authored-by: Ricardo González Moreno <[email protected]>

Signed-off-by: Mario Domínguez López <[email protected]>
Signed-off-by: Miguel Company <[email protected]>
Signed-off-by: Miguel Company <[email protected]>
Signed-off-by: Ricardo González <[email protected]>
@bshantam97
Copy link

@MiguelCompany Hello, when will the FastDDS3 RMW Changes be merged with ROS2?

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.

6 participants