-
Notifications
You must be signed in to change notification settings - Fork 33
Groundwork for one participant per context refactor #382
Conversation
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
…://github.com/ros2/rmw_connext into ivanpauno/prepare-one-participant-per-context
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM as a first step.
ConnextPublisherListener * publisher_listener = nullptr; | ||
ConnextStaticPublisherInfo * publisher_info = nullptr; | ||
rmw_publisher_t * publisher = nullptr; | ||
std::string mangled_name = ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ivanpauno nit: no need to initialize with an empty string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually copy-pasted that, but ok.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in 4e2e037.
rmw_connext_cpp/src/rmw_service.cpp
Outdated
@@ -56,7 +56,7 @@ rmw_create_service( | |||
return nullptr; | |||
} | |||
|
|||
auto node_info = static_cast<ConnextNodeInfo *>(node->data); | |||
auto node_info = static_cast<ConnextParticipantInfo *>(node->data); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ivanpauno here and elsewhere, ConnextNodeInfo
became ConnextParticipantInfo
, but I don't see the struct
defined and node_info
is probably not the right name anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
Closing as this is not going to be continued (considering that https://github.com/rticommunity/rmw_connextdds already implemented this new approach). |
Groundwork for #381.
Just moving code around, shouldn't change behavior.
This allows to:
This refactor can be merged right now, avoiding hard rebases later.