First implementation by ivanpauno · Pull Request #4 · ros2/rmw_dds_common

ivanpauno · 2019-10-10T19:29:09Z

Adds:

Generation of common messages.
- A package called rmw_dds_common_generators. It basically depends on rosidl_generator_c, rosidl_generator_cpp, rosidl_typesupport_introspection_c, rosidl_typesupport_introspection_cpp, and a group depend in rosidl_typesupport_cpp_packages.
  This is used to avoid depending in the default generators, which also depend in rmw_typesupport_c/cpp creating a circular dependency.
  This metapackage is needed, as there is no build_tool_group_depend equivalent.
- rmw_dds_common generates three messages Gid.idl, NodeCustomInfo.idl, ParticipantCustomInfo.idl.
- Each dds implementation will generate a *__type_support.cpp for the messages manually (example here).
GraphCache class, that could be used in all the vendors.
A common Context class, that will be an implementation detail in all the rmw_context_t of rmw implementations based on DDS, that uses one Participant per Context.

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

* Add gid_utils Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com> * Add NodeCache class Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com> Add testing of TopicCache class Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

* Please linter. Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

hidmic

First pass! Man this one's big.

rmw_dds_common/CMakeLists.txt

rmw_dds_common/include/rmw_dds_common/graph_cache.hpp

rmw_dds_common/src/graph_cache.cpp

rmw_dds_common/include/rmw_dds_common/graph_cache.hpp

rmw_dds_common/src/graph_cache.cpp

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

rmw_dds_common/src/graph_cache.cpp

…ntation Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

…icipant Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

ivanpauno · 2020-02-10T21:20:53Z

@wjwwood I realized recently that I'm not triggering the graph guard condition in many cases I should.
That isn't causing failures, just because lack of tests.
I will pass a rmw_guard_condition_t * to GraphCache, and trigger it in all the setters.

wjwwood · 2020-02-10T21:37:47Z

Ok, yeah we do need to be triggering the node guard condition anytime the graph changes. That should have been the case before.

It would be nice to have new tests in rcl or something to ensure that this is happening. Unfortunately these tests are notoriously complicated, due to their nature of being asynchronous.

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

ivanpauno · 2020-02-11T19:18:02Z

Ok, yeah we do need to be triggering the node guard condition anytime the graph changes. That should have been the case before.

Done in
ros2/rmw_fastrtps@bffc25c and 8d6e757.

It would be nice to have new tests in rcl or something to ensure that this is happening. Unfortunately these tests are notoriously complicated, due to their nature of being asynchronous.

Opened issue.

wjwwood · 2020-02-13T19:50:52Z

@ivanpauno can you update the original description before you merge this? It looks out of date, and it's most likely what someone will look at when coming back to this in the future.

Also, please go ahead and move this to the ROS 2 organization.

rmw_dds_common/include/rmw_dds_common/gid_utils.hpp

rmw_dds_common/include/rmw_dds_common/graph_cache.hpp

rmw_dds_common/src/graph_cache.cpp

hidmic · 2020-02-28T14:18:02Z

rmw_dds_common/src/graph_cache.cpp

+        break;
+      case EndpointCreator::BARE_DDS_PARTICIPANT:
+        node_name = "_CREATED_BY_BARE_DDS_APP_";
+        node_namespace = "_CREATED_BY_BARE_DDS_APP_";


@ivanpauno why not using these string literals when first constructing the tuple? Then you can optimize this switch away.

I could optimize EndpointCreator::BARE_DDS_PARTICIPANT case, but not also EndpointCreator::UNDISCOVERED_ROS_NODE. I find clearer if all the cases are together.

rmw_dds_common/src/graph_cache.cpp

rmw_dds_common/test/test_graph_cache.cpp

hidmic · 2020-02-28T14:35:21Z

rmw_dds_common/test/test_graph_cache.cpp

+using DemangleFunctionT = GraphCache::DemangleFunctionT;
+
+void
+check_results(


@ivanpauno nit: for all check_* functions, consider not using default parameters for empty sets or zero counts and provide them explicitly instead. IMO it'll make the tests much easier to read.

I'd also be a bit more verbose on the naming e.g. check_results -> check_graph_cache_state. Same for the rest.

Still applies, but won't block on it.

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

rmw_dds_common/include/rmw_dds_common/graph_cache.hpp

rmw_dds_common/src/graph_cache.cpp

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Signed-off-by: Ivan S Paunovic <ivanpauno@gmail.com>

hidmic

LGTM but for a few minor comments here and there.

rmw_dds_common/msg/ParticipantEntitiesInfo.idl

rmw_dds_common/src/graph_cache.cpp

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

hidmic

LGTM

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

hidmic

LGTM

rmw_dds_common_generators/rmw_dds_common_generators-extras.cmake.in

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

ivanpauno added 12 commits September 4, 2019 16:31

Add message interfaces generation

6fa5758

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Add NodeCache class (#1)

d80810a

* Add gid_utils Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com> * Add NodeCache class Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Minimal improvements in signatures

6663ae9

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Add operator<< to rmw_gid_t

53ca840

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Add rmw_gid_t operator==

248b48a

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Add TopicCache class

58043c1

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com> Add testing of TopicCache class Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Add rmw_context_impl_t and related functions

b913878

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Correct rmw_context_impl

0cc19c5

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Minor update

79325e0

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Many corrections after iterating with Fast-RTPS implementation

d8ec3a1

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Refactor context impl. Deleted locked object.

58c1bf5

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Clean rmw_dds_common_generators package.xml

b0b53e4

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

ivanpauno mentioned this pull request Oct 10, 2019

Switch to one Participant per Context ros2/rmw_fastrtps#312

Merged

ivanpauno added 8 commits October 14, 2019 15:49

* Add graph instronspection methods to TopicCache class.

52f58f3

* Please linter. Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Make TopicCache thread safe

97fa572

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Avoid const_cast

db22807

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Please linter

4e74da3

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Corrected get_names_and_types

3191067

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Added guard condition to context

33f30b5

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Refactor TopicCache and NodeCache in GraphCache

2847f8d

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Pleased linter

11d34b2

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

hidmic requested changes Nov 21, 2019

View reviewed changes

ivanpauno added 2 commits November 26, 2019 10:52

Address some review comments

f7471c9

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Implement GraphCache operator<<

f55724a

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

hidmic reviewed Nov 28, 2019

View reviewed changes

rmw_dds_common/src/graph_cache.cpp Outdated Show resolved Hide resolved

ivanpauno added 5 commits November 29, 2019 11:24

Add missing locks. Enforce alpha order of topics, as previous impleme…

100c1df

…ntation Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Avoid duplicated service names

2db889d

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Corrected package.xml format

1258d08

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Fix possible race between update_participant_entities and remove_part…

dce33b7

…icipant Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Add test

29827d8

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Trigger guard condition in any graph change

8d6e757

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

wjwwood approved these changes Feb 13, 2020

View reviewed changes

hidmic requested changes Feb 28, 2020

View reviewed changes

Address peer review comments

6247fc7

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

hidmic approved these changes Mar 2, 2020

View reviewed changes

ivanpauno added 3 commits March 12, 2020 16:48

Add function to retrive list of context names of nodes

cd0037a

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Naming: replace context_name with security_context

651a0b8

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Remove just one element when removing a node

28442ef

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

hidmic reviewed Mar 19, 2020

View reviewed changes

rmw_dds_common/include/rmw_dds_common/graph_cache.hpp Outdated Show resolved Hide resolved

rmw_dds_common/src/graph_cache.cpp Show resolved Hide resolved

ivanpauno and others added 3 commits March 20, 2020 16:03

Address peer review comments

b7f8ce4

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Correct linkage error on Windows

9b9c671

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Fix build failures on Windows

dc989e4

Signed-off-by: Ivan S Paunovic <ivanpauno@gmail.com>

ivanpauno requested a review from hidmic March 25, 2020 17:53

ivanpauno mentioned this pull request Mar 25, 2020

Add an EXCLUDE parameter to rosidl_generate_interfaces ros2/rosidl#449

Closed

hidmic approved these changes Mar 25, 2020

View reviewed changes

rmw_dds_common/msg/ParticipantEntitiesInfo.idl Outdated Show resolved Hide resolved

rmw_dds_common/src/graph_cache.cpp Outdated Show resolved Hide resolved

rmw_dds_common/src/graph_cache.cpp Outdated Show resolved Hide resolved

Address peer review comments

7579823

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

ivanpauno requested a review from hidmic March 26, 2020 16:07

hidmic approved these changes Mar 26, 2020

View reviewed changes

ivanpauno added 2 commits March 27, 2020 16:07

Please linters

0289ff0

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

Fix problem when rebuilding a workspace using

1806af1

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

ivanpauno requested a review from hidmic March 27, 2020 19:09

hidmic approved these changes Mar 27, 2020

View reviewed changes

rmw_dds_common_generators/rmw_dds_common_generators-extras.cmake.in Outdated Show resolved Hide resolved

Avoid warning on Mac

4b3d4ba

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>

ivanpauno merged commit dc92e86 into master Apr 2, 2020

delete-merged-branch bot deleted the ivanpauno/first-implementation branch April 2, 2020 17:25

ivanpauno restored the ivanpauno/first-implementation branch April 2, 2020 17:44

Conversation

ivanpauno commented Oct 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hidmic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ivanpauno commented Feb 10, 2020

Uh oh!

wjwwood commented Feb 10, 2020

Uh oh!

ivanpauno commented Feb 11, 2020

Uh oh!

wjwwood commented Feb 13, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hidmic Feb 28, 2020

Choose a reason for hiding this comment

Uh oh!

ivanpauno Feb 28, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hidmic Feb 28, 2020

Choose a reason for hiding this comment

Uh oh!

hidmic Feb 28, 2020

Choose a reason for hiding this comment

Uh oh!

hidmic Mar 2, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

hidmic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hidmic left a comment

Choose a reason for hiding this comment

Uh oh!

hidmic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ivanpauno commented Oct 10, 2019 •

edited

Loading