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

Add function to call through to dds layer to notify about new network interfaces #2086

Draft
wants to merge 2 commits into
base: rolling
Choose a base branch
from

Conversation

jakymiws
Copy link

Directly related to irobot/add-dynamic-network-notify-function in jakymiws/rcl

Signed-off-by: Sebastian Jakymiw <sjakymiw@irobot.com>
@jakymiws
Copy link
Author

@@ -330,6 +330,14 @@ RCLCPP_PUBLIC
std::vector<const char *>
get_c_vector_string(const std::vector<std::string> & strings_in);

/// Call dds to notify about new network interfaces.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This layer shouldn't assume that the underlying middleware is a DDS.
Maybe the comment should say something like "Notify middleware to refresh available network interfaces?"

Copy link
Author

Choose a reason for hiding this comment

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

@alsora good point - will change that.

@alsora
Copy link
Collaborator

alsora commented Jan 19, 2023

@jakymiws we should also add a function implementation in the other TIER 1 RMW middelwares.
In this case we can return a NOT IMPLEMENTED error if the implementation does not support it yet

Signed-off-by: Sebastian Jakymiw <sjakymiw@irobot.com>
@jakymiws
Copy link
Author

jakymiws commented Jan 23, 2023

@alsora
Done with cyclone-dds here: ros2/rmw_cyclonedds#433 and connext-dds here: ros2/rmw_connextdds#99

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.

None yet

2 participants