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

missing functionality in graph API #64

Closed
wjwwood opened this issue Oct 29, 2016 · 3 comments
Closed

missing functionality in graph API #64

wjwwood opened this issue Oct 29, 2016 · 3 comments
Labels
bug Something isn't working

Comments

@wjwwood
Copy link
Member

wjwwood commented Oct 29, 2016

While working on local graph changes for Connext I noticed two things about the FastRTPS implementation:

  • publishers and subscribers do not get removed from topic_names_and_types ever (even when destroyed)
  • count publisher and subscribers does not appear to count the publishers and subscribers but rather the number of different types on a topic

This can be shown using the graph tests in rcl, which are currently disabled for Fast RTPS since they don't pass:

ros2/rcl@596c0ec#diff-e4167ceebe2946426e927f55e4392af4R66

I didn't want to hold up the other PR's to fix this. I spent a few minutes looking at it, and I think the first issues could be addressed by storing the publisher/subscriber GUID's. The second issue will require being able to tell the difference in a publisher or subscriber coming up versus going down. I wasn't able to figure out how to do that and I wasn't even sure there was any notification that they went down at all.

You can see here that the topicNtypes map is inserted into:

https://github.com/eProsima/ROS-RMW-Fast-RTPS-cpp/blob/master/rmw_fastrtps_cpp/src/functions.cpp#L353

But as far as I can see nothing is ever removed. Maybe someone can give us a lead on how to accomplish this using the data coming into that function. You can see similar, but slightly different code for how we accomplish this in Connext:

https://github.com/ros2/rmw_connext/blob/master/rmw_connext_shared_cpp/src/shared_functions.cpp#L78-L82

@dirk-thomas
Copy link
Member

Can this be closed now?

@wjwwood
Copy link
Member Author

wjwwood commented Jun 2, 2017

Yup, thanks.

@wjwwood wjwwood closed this as completed Jun 2, 2017
@dhood
Copy link
Member

dhood commented Jun 5, 2017

for future reference the relevant PR is #111

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants