Support ROS 2 Easy mode #139

Carlosespicur · 2025-03-13T11:04:59Z

Main Changes

This PR adds support for configuring easy mode in SimpleParticipants through the YAML configuration file using a new ros2-easy-mode tag.
The option is incompatible with the transports configuration, so easy mode is only configured if it is set as default (builtin).

Merge after:

[22872] Configure ROS 2 Easy Mode via C++ and XML Fast-DDS#5689

Signed-off-by: Carlosespicur <[email protected]>

juanlofer-eprosima

I believe we decided it would be a good idea to unset sensitive environment variables that may interfere with YAML configurations, which may be done in a generic method that could be implemented here.

Signed-off-by: Carlosespicur <[email protected]>

Carlosespicur · 2025-03-18T11:33:30Z

I believe we decided it would be a good idea to unset sensitive environment variables that may interfere with YAML configurations, which may be done in a generic method that could be implemented here.

Change applied on fee5466

codecov-commenter · 2025-03-19T07:12:34Z

Codecov Report

Attention: Patch coverage is 56.00000% with 11 lines in your changes missing coverage. Please review.

Project coverage is 38.00%. Comparing base (efc83a4) to head (56c14fb).

Files with missing lines	Patch %	Lines
...nts/src/cpp/participant/rtps/SimpleParticipant.cpp	40.00%	0 Missing and 3 partials ⚠️
...aml/include/ddspipe_yaml/testing/generate_yaml.hpp	0.00%	3 Missing ⚠️
ddspipe_participants/src/cpp/utils/utils.cpp	66.66%	1 Missing and 1 partial ⚠️
ddspipe_yaml/src/cpp/YamlReader_participants.cpp	0.00%	2 Missing ⚠️
...ants/src/cpp/participant/dds/CommonParticipant.cpp	0.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #139      +/-   ##
==========================================
+ Coverage   36.60%   38.00%   +1.40%     
==========================================
  Files         170      153      -17     
  Lines       11567     6986    -4581     
  Branches     5286     2780    -2506     
==========================================
- Hits         4234     2655    -1579     
+ Misses       4586     2938    -1648     
+ Partials     2747     1393    -1354

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

juanlofer-eprosima · 2025-03-20T13:37:33Z

ddspipe_participants/src/cpp/utils/utils.cpp

+    const char* env_vars[] = {
+        "ROS2_EASY_MODE",
+        "FASTDDS_ENVIRONMENT_FILE",
+        "ROS_DISCOVERY_SERVER"


What about "ROS_SUPER_CLIENT" and "FASTDDS_BUILTIN_TRANSPORTS"? The first one should be in the list for sure, the other one could be useful though.

After discussion we decided not to block "FASTDDS_BUILTIN_TRANSPORTS" just in case we need it in the future.

Changes applied on f0d98af

juanlofer-eprosima · 2025-03-20T14:05:51Z

ddspipe_participants/src/cpp/participant/dds/CommonParticipant.cpp

@@ -395,6 +395,9 @@ fastdds::dds::DomainParticipantQos CommonParticipant::reckon_participant_qos_()

 fastdds::dds::DomainParticipant* CommonParticipant::create_dds_participant_()
 {
+    // Unset environment variables that conflict with YAML configuration


Suggested change

// Unset environment variables that conflict with YAML configuration

// Unset environment variables that conflict with configuration

Please also add this to the RTPS common participant.

Changes applied on f0d98af

juanlofer-eprosima · 2025-03-20T14:07:51Z

ddspipe_participants/src/cpp/utils/utils.cpp

+
+    for (const char* var : env_vars)
+    {
+    #ifdef _WIN32


Please double check this is the right macro or there is a more generic one.

After discussing this, we concluded that it should be the most suitable one. It is coherent with https://stackoverflow.com/questions/2989810/which-cross-platform-preprocessor-defines-win32-or-win32-or-win32

juanlofer-eprosima · 2025-03-20T14:16:21Z

ddspipe_participants/src/cpp/participant/rtps/SimpleParticipant.cpp

+    if (!simple_configuration->easy_mode_ip.empty())
+    {
+        // This option is incompatible with transport tag configurations,
+        // so check that transport is set to the default value (builtin)
+        if (simple_configuration->transport != core::types::TransportDescriptors::builtin)
+        {
+            EPROSIMA_LOG_WARNING(SIMPLEPARTICIPANT,
+                    "Ignoring IP value: Easy mode configuration is incompatible with transport tag configurations.");
+        }
+        else
+        {
+            // Check if the IP is a valid IPv4 address
+            if (!fastdds::rtps::IPLocator::isIPv4(simple_configuration->easy_mode_ip))
+            {
+                EPROSIMA_LOG_WARNING(SIMPLEPARTICIPANT,
+                        "Ignoring Easy Mode IP value. It must be a valid IPv4 address.");
+            }
+            else
+            {
+                params.easy_mode_ip = simple_configuration->easy_mode_ip;
+            }
+        }
+    }


I would move this logic to the is_valid method, and perhaps it would be better to fail instead of just printing a warning.

Changes applied on f0d98af

Signed-off-by: Carlosespicur <[email protected]>

Carlosespicur requested a review from juanlofer-eprosima March 13, 2025 11:05

Carlosespicur had a problem deploying to codecov March 13, 2025 11:05 — with GitHub Actions Failure

Refs #22870: Configure easy mode IP from YAML

c5b6575

Signed-off-by: Carlosespicur <[email protected]>

Carlosespicur force-pushed the feature/ros2-easy-mode-config branch from 6b398aa to c5b6575 Compare March 13, 2025 11:07

Carlosespicur had a problem deploying to codecov March 13, 2025 11:07 — with GitHub Actions Failure

Refs #22870: Uncrustify

13ec284

Signed-off-by: Carlosespicur <[email protected]>

Carlosespicur had a problem deploying to codecov March 13, 2025 11:09 — with GitHub Actions Failure

Refs #22870: Add methods for testing yaml in other tools

7c0a3f4

Signed-off-by: Carlosespicur <[email protected]>

Carlosespicur had a problem deploying to codecov March 14, 2025 08:03 — with GitHub Actions Failure

This was referenced Mar 14, 2025

[22870] Support ROS 2 Easy Mode eProsima/DDS-Router#495

Open

[22870] Support ROS 2 Easy Mode eProsima/Fast-DDS-spy#116

Open

[22870] Support ROS 2 Easy Mode eProsima/DDS-Record-Replay#180

Open

juanlofer-eprosima requested changes Mar 17, 2025

View reviewed changes

Carlosespicur had a problem deploying to codecov March 18, 2025 11:30 — with GitHub Actions Failure

Refs #22870: Unset configuration environment variables

fee5466

Signed-off-by: Carlosespicur <[email protected]>

Carlosespicur force-pushed the feature/ros2-easy-mode-config branch from d97b6a7 to fee5466 Compare March 18, 2025 11:32

Carlosespicur had a problem deploying to codecov March 18, 2025 11:32 — with GitHub Actions Failure

Carlosespicur requested a review from juanlofer-eprosima March 19, 2025 06:59

Carlosespicur temporarily deployed to codecov March 19, 2025 06:59 — with GitHub Actions Inactive

juanlofer-eprosima requested changes Mar 20, 2025

View reviewed changes

Carlosespicur temporarily deployed to codecov March 21, 2025 11:55 — with GitHub Actions Inactive

Refs #22870: Apply suggested changes

f0d98af

Signed-off-by: Carlosespicur <[email protected]>

Carlosespicur force-pushed the feature/ros2-easy-mode-config branch from 7488a80 to f0d98af Compare March 21, 2025 11:57

Carlosespicur temporarily deployed to codecov March 21, 2025 11:57 — with GitHub Actions Inactive

WIP

56c14fb

Signed-off-by: Carlosespicur <[email protected]>

Carlosespicur deployed to codecov March 25, 2025 13:41 — with GitHub Actions Active

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support ROS 2 Easy mode #139

Support ROS 2 Easy mode #139

Carlosespicur commented Mar 13, 2025 •

edited

Loading

juanlofer-eprosima left a comment

Carlosespicur commented Mar 18, 2025

codecov-commenter commented Mar 19, 2025 •

edited

Loading

juanlofer-eprosima Mar 20, 2025

juanlofer-eprosima Mar 20, 2025

Carlosespicur Mar 21, 2025

juanlofer-eprosima Mar 20, 2025

juanlofer-eprosima Mar 20, 2025

Carlosespicur Mar 21, 2025

juanlofer-eprosima Mar 20, 2025

Carlosespicur Mar 21, 2025

juanlofer-eprosima Mar 20, 2025

Carlosespicur Mar 21, 2025

	// Unset environment variables that conflict with YAML configuration
	// Unset environment variables that conflict with configuration

Support ROS 2 Easy mode #139

Are you sure you want to change the base?

Support ROS 2 Easy mode #139

Conversation

Carlosespicur commented Mar 13, 2025 • edited Loading

Main Changes

juanlofer-eprosima left a comment

Choose a reason for hiding this comment

Carlosespicur commented Mar 18, 2025

codecov-commenter commented Mar 19, 2025 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Carlosespicur commented Mar 13, 2025 •

edited

Loading

codecov-commenter commented Mar 19, 2025 •

edited

Loading