Skip to content
This repository has been archived by the owner on Jun 21, 2023. It is now read-only.

Groundwork for one participant per context refactor #382

Closed

Conversation

ivanpauno
Copy link
Member

Groundwork for #381.

Just moving code around, shouldn't change behavior.
This allows to:

  • Create a rmw_publisher_t from a participant.
  • Create a rmw_subscription_t from a participant.

This refactor can be merged right now, avoiding hard rebases later.

Signed-off-by: Ivan Santiago Paunovic <[email protected]>
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
@ivanpauno ivanpauno added enhancement New feature or request in review Waiting for review (Kanban column) labels Dec 20, 2019
@ivanpauno ivanpauno requested a review from hidmic December 20, 2019 19:09
@ivanpauno ivanpauno self-assigned this Dec 20, 2019
Copy link
Contributor

@hidmic hidmic left a 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 = "";
Copy link
Contributor

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.

Copy link
Member Author

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 4e2e037.

@@ -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);
Copy link
Contributor

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The struct is defined here.

And about renaming node_info to participant_info, I did that in 4e2e037.

Signed-off-by: Ivan Santiago Paunovic <[email protected]>
@ivanpauno ivanpauno requested a review from hidmic January 17, 2020 18:00
@ivanpauno
Copy link
Member Author

Closing as this is not going to be continued (considering that https://github.com/rticommunity/rmw_connextdds already implemented this new approach).

@ivanpauno ivanpauno closed this Feb 26, 2021
@ivanpauno ivanpauno deleted the ivanpauno/prepare-one-participant-per-context branch February 26, 2021 18:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request in review Waiting for review (Kanban column)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants