ros2 / rmw_dps

Implementation of the ROS Middleware (rmw) Interface using Intel's Distributed Publish & Subscribe.
Apache License 2.0
23 stars 8 forks source link

rmw_dps support for RMW 0.7.1 #6

Closed alsora closed 5 years ago

alsora commented 5 years ago

The RMW has been updated few days ago to version 0.7.1 and this is not supported yet from rmw_dps.

First of all there are build errors, due to the new APIs (allocation and liveliness mostly).

Moreover there is a runtime error when a subscriber try to deserialize a message of type ParameterEvent.

I saw a similar problem mentioned for Fast-RTPS: https://github.com/ros2/rmw_fastrtps/issues/36#issuecomment-224062166

I'm working on a PR: https://github.com/ros2/rmw_dps/pull/7

It allows to build without errors and, if you remove the ParameterEvents publisher and subscribers, it allows nodes to communicate.

Do you have any ideas on how to fix this deserialization issue?

@AAlon

malsbat commented 5 years ago

I'll take a look at the deserialization issue. Can you point to me the failing test?

alsora commented 5 years ago

There are a bunch of packages with failed tests.

  8 packages had test failures: message_filters rcl rcl_action rclcpp rclcpp_action rclcpp_components rclpy tlsf_cpp

The main things I can see are:

The error I was referring to is probably happening in the last two tests mentioned above. However, it's sufficient to run any 2 nodes to notice it

ros2 run examples_rclcpp_minimal_publisher publisher_lamda
ros2 run examples_rclcpp_minimal_subscriber subscriber_lambda

In any case, the segfault occurs when calling the resize_function of the MessageMember https://github.com/ros2/rmw_dps/blob/ab6f8d0257d6cf61fc499fee6ab3936e0b3234f6/rmw_dps_cpp/include/rmw_dps_cpp/TypeSupport_impl.hpp#L403

This happens only for ParameterEvents messages, while it works for custom and other standard messages. Moreover, note that the error does not occur when using ROS2 Crystal

malsbat commented 5 years ago

Thanks for the test pointer. I will reproduce on my end and fix it.

Implementation of the discovery/introspection APIs is next.

malsbat commented 5 years ago

@alsora the issue appears to be that the resize_function is not hooked into the introspection structure in the latest code while it was in Crystal. I'm still tracking down exactly when and why this occurred.

Some background: in the deserialization code you highlighted above, field points to a std::vector of some type inside a ROS message. The code does a placement new to initialize the memory inside the message then calls the resize_function of the actual type to allocate the memory for the elements. With no resize_function in place, we crash.

malsbat commented 5 years ago

https://github.com/ros2/rosidl/issues/378

alsora commented 5 years ago

With the proposed PR https://github.com/ros2/rosidl/pull/379 The two rclcpp tests that were having a segfault now succed. Moreover, I'm able to run nodes with parameter events

alsora commented 5 years ago

The previously mentioned PR has been merged, so it's possible to close this issue.