GUID fix for node graph implementation

[mm318]

There were some hacks left in the ros2/rcl unit test (specifically in test_graph.cpp), because the unique identifier for participants and topics used in rmw_opensplice didn't seem to be correct. This pull request fixes that issue, and the hacks in the unit tests can now be removed.

[mm318]

Justification of this change:

The InstanceHandle_t type does uniquely identify entities, such as DomainParticipants, Subscribers, Publishers, and Topics: https://github.com/ADLINK-IST/opensplice/blob/master/src/api/dcps/isocpp2/include/dds/core/TEntity.hpp#L203

So we intend to use InstanceHandle_t as the unique identifier of participants and topics.

Then one question is how to translate from BuiltinTopicKey_t to InstanceHandle_t.

We can first see that BuiltinTopicKey/BuiltinTopicKey_t can be translated to v_builtinTopicKey/v_gid: https://github.com/ADLINK-IST/opensplice/blob/master/src/api/dcps/isocpp2/code/org/opensplice/topic/BuiltinTopicCopy.cpp#L89

Then v_builtinTopicKey/v_gid can be translated to an InstanceHandle_t as done here: https://github.com/ADLINK-IST/opensplice/blob/master/src/api/dcps/c%2B%2B/common/code/DataReader.cpp#L1384 and here: https://github.com/ADLINK-IST/opensplice/blob/master/src/api/dcps/c%2B%2B/common/code/DataWriter.cpp#L552

This is the approach taken by the implementation of DDS_BuiltinTopicKey_to_InstanceHandle() in this pull request.

[mm318]

The hacks in the ros2/rcl unit test is being removed with ros2/rcl#368.

[clalancette]

This looks generally fine to me; I've got a few comments for improvement, but it is nothing major. I'm going to run CI on the whole thing right now. Once the comments are fixed and CI is green, I'm happy to approve.

[clalancette]

CI:

• Linux
• Linux-aarch64
• macOS
• Windows

[clalancette]

The one test failure on aarch64 is a known flakey test: ros2/build_farmer#133 . Thus, I'll approve and merge this. Thank you!

[chapulina]

@mm318, this and ros2/rcl#368 will need to be backported to the crystal branches in order to get into patch release 2. Would you or someone else from your team be willing to open a PR with the backport? We can take care of running CI. cc @ross-desmond

[mm318]

I will be travelling. @ross-desmond, can you do the backport, please? Thanks!

[nuclearsandwich]

Would you or someone else from your team be willing to open a PR with the backport?

I can take care of opening the backport PR itself as long as someone from the team is able to field feedback if CI comes back with issues.

[ross-desmond]

Perfect, i will be available to help should there be issues

…

On Tue, Feb 5, 2019, 4:28 PM Steven! Ragnarök ***@***.*** wrote: Would you or someone else from your team be willing to open a PR with the backport? I can take care of opening the backport PR itself as long as someone from the team is able to field feedback if CI comes back with issues. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#255 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AqOeTOz9UbqViBU9eAEz9vkNMD5QTA1bks5vKiG6gaJpZM4ZfR7C> .

[ross-desmond]

"Prior to this fix in rmw-opensplice, a node was unable to lookup it's own topics and services discovered through the Simple Discovery Protocol. This enables rmw-opensplice to behave the way rmw-connext and rmw-fastrtps in regards to the node graph API."
Description from ros2/ros2#647 discussion