search

tocinc / rmw_coredx

CoreDX DDS integration layer for ROS2
Apache License 2.0
0 stars 6 forks source link

Interoperability issues #3

Open mikaelarguedas opened 7 years ago

mikaelarguedas commented 7 years ago

Hi, First of all congrats for the great work! nice to see a new rmw_implementation that works out of the box!

While playing around with this in my ros2 workspace I identified a few interoperability failures with other rmw_implementations.

CoreDX <=> Fast-RTPS: unable to transfer static arrays Can be reproduced by runnint in a sourced workspace:

CoreDX <=> RTI Connext static: it seems that any message are unable to be transfered and cause segmentation faults, even the talker/listener example.

ClarkTucker commented 7 years ago

Hi, Thanks for the feedback!

We'll take a look at these issues.

Just a couple questions to start with:

mikaelarguedas commented 7 years ago

Will do, very likely in a few days, thanks for being so repsonsive

mikaelarguedas commented 7 years ago

Hi @ClarkTucker

Hope this helps

ClarkTucker commented 7 years ago

Thanks @mikaelarguedas,

The RTI - TOC issue is known, I think. The bug had to do with RTI's handling of TypeObject 'module' constructs. A newer version of the RTI software should resolve it [I'm not sure which version includes the fix -- but I think it must be publicly available by now].

I'll take a look at the TOC - FastRTPS packet captures...

mikaelarguedas commented 7 years ago

@ClarkTucker I didn't see any newer version on the RTI website, is it possible for you to diclose which version number of Connext you're testing with? Also did you have a chance to look at the Fast-RTPS interoperability crash? Thanks!

nate-jackson commented 6 years ago

As a new datapoint, I tried running the fastrtps test with coredx compiled on linux with gcc5, with the newest rmw_coredx implementation as of a few days ago, and I don't see it crash, but I see this error come from the fastrtps side:

[RTPS_HISTORY Error] Change payload size of '5096' bytes is larger than the history payload size of '5000' bytes and cannot be resized. -> Function add_change

Coredx just continued publishing the message, no errors were evident on that side.

ClarkTucker commented 6 years ago

The new rmw_coredx implementation now uses the same topic name and partition generation scheme as the others implementations, which was the root of the initial interoperability hurdle. I believe that we are now discovering and matching the topics correctly, and have moved to the next level of interoperability.

I'm not sure how to address the error message from FastRTPS...

dirk-thomas commented 6 years ago

We are in the process of removing the usage of partition from the other rmw implementation (see ros2/ros2#476). Instead we are putting the whole ROS topic name including forward slashes into the DDS topic name (in anticipation of a future update of the spec to allow forward slashes).

ClarkTucker commented 6 years ago

Noted, thanks. Should I just monitor https://github.com/ros2/rmw_fastrtps/pull/192 for status? [btw, this should be easy for rmw_coredx to support...]

dirk-thomas commented 6 years ago

I sent Nina a short list of items we are in process to change about 9 days ago. For future changes / additions to the rmw interface we will create issues in the https://github.com/ros2/rmw repository and mention this team which developers of all rmw implementation should be a part of. Hopefuly that will help to have everyone on the same page and be able to discuss proposed changes in a bigger group.