Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create PublisherData within NodeData #278

Draft
wants to merge 1 commit into
base: yadu/raii-node
Choose a base branch
from

Conversation

Yadunund
Copy link
Member

@Yadunund Yadunund commented Sep 9, 2024

This PR replaces class rmw_publisher_data_t with class PublisherData which provides thread-safe access to publisher functions. An instance of PublisherData is created via the NodeData class. rmw_publisher->data is now set to rmw_node_t. The assumption here is that the node outlives the publisher similar to the assumption that the context outlives the node. From this rmw_node_t raw_ptr, we are able to obtain it's NodeData and then lookup the PublisherData for the publisher.

rmw_publisher->data = publisher_data;
// Store type erased node in rmw_publisher->data so that the
// PublisherData can be safely accessed.
rmw_publisher->data = reinterpret_cast<void *>(const_cast<rmw_node_t *>(node));
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 had to rely on a reinterpret_cast<const_cast<>> to be able to store the const rmw_node_t * node available to rmw_create_publisher() within rmw_publisher->data as the data field is void * and not const void *.

This is a bit messy. Another alternative I considered was storing the rmw_context_impl_s * ptr here and then including an unordered_map in this class to obtain the NodeData for a given rmw_publisher_t but this involves extra bookkeeping that we need to do when we create and destroy a publisher. Open to other suggestions!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant