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

[rmw] rmw_fastrtps_dds does not work well for C++ <-> Python pub/sub testing? #286

Open
EricCousineau-TRI opened this issue Jun 27, 2023 · 1 comment

Comments

@EricCousineau-TRI
Copy link
Collaborator

EricCousineau-TRI commented Jun 27, 2023

In #285, I had to workaround testing expectations based on the effective RMW implementation being used:

def assert_equal_for_cyclone_dds(actual, expected, name):
rmw_implementation = rclpy.utilities.get_rmw_implementation_identifier()
is_cyclone_dds = rmw_implementation == "rmw_cyclonedds_cpp"
if is_cyclone_dds:
assert actual == expected
else:
# TODO(eric.cousineau): At least for rmw_fastrtps_cpp, the behavior of
# publishing and receiving a message seems much less reliable than
# cyclone. Demote errors to warnings in this case.
if actual != expected:
warnings.warn(
f"WARNING! For RMW_IMPLEMENTATION={rmw_implementation}, "
f"incorrect value for {name}: {actual} != {expected}"
)
def test_cpp_node(drake_ros_fixture):
# Create a Python node.
node_py = rclpy.create_node("node_py")
sub_value_py = -1
def on_sub(message):
nonlocal sub_value_py
sub_value_py = message.data
node_py.create_subscription(Int32, "/cpp_pub", on_sub, 1)
pub_py = node_py.create_publisher(Int32, "/cpp_sub", 1)
# Create a C++ node.
cpp_node_options = CppNodeOptions(use_global_arguments=False)
assert not cpp_node_options.use_global_arguments
node_cpp = CppNode("direct_node_cpp", node_options=cpp_node_options)
assert node_cpp.get_name() == "direct_node_cpp"
# Create a "fixture" that will publish and subscribe to above topics.
fixture = CppPubAndSub(node=node_cpp)
timeout_sec = 0.0
# Show that we can publish from C++ to Python.
fixture.Publish(value=10)
rclpy.spin_once(node_py, timeout_sec=timeout_sec)
assert_equal_for_cyclone_dds(sub_value_py, 10, "sub_value_py")
# Show that we can publish from Python to C++.
message = Int32()
message.data = 100
pub_py.publish(message)
sub_value_cpp = fixture.SpinAndReturnLatest(timeout_sec=timeout_sec)
assert_equal_for_cyclone_dds(sub_value_cpp, 100, "sub_value_cpp")

This makes me concerned about using or even supporting rmw_fastrtps_cpp if something this simple is flaky.

Some options local to drake-ros:
(a) Keep this pattern, just warn when rmw_cyclonedds_cpp is not in use whenever testing diverges.
(b) Only ever allow rmw_cyclonedds_cpp, at least for our testing in drake-ros (and usage in Anzu).

\cc @adityapande-1995 @calderpg-tri @IanTheEngineer

@EricCousineau-TRI
Copy link
Collaborator Author

Originally found this as a difference between Bazel build and CMake build.

Repro steps based on above commit:

sudo apt install ros-humble-rmw-cyclonedds-cpp ros-humble-rmw-fastrtps-cpp

cd drake_ros_ws
colcon build --packages-up-to drake_ros --symlink-install
source ./install/setup.bash
cd build/drake_ros/

# Good
export RMW_IMPLEMENTATION=rmw_cyclonedds_cpp
ctest -V -R core_test_py

# Bad
export RMW_IMPLEMENTATION=rmw_fastrtps_cpp
ctest -V -R core_test_py

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

No branches or pull requests

1 participant