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

Implement the rmw_get_publishers_info_by_topic and rmw_get_subscriptions_info_by_topic methods #82

Closed
mm318 opened this issue Dec 11, 2019 · 5 comments

Comments

@mm318
Copy link
Member

mm318 commented Dec 11, 2019

As a follow up to #81, which currently returns RMW_RET_UNSUPPORTED for the rmw_get_publishers_info_by_topic() and rmw_get_subscriptions_info_by_topic() functions, ensure that these methods are implemented before the ROS2 Foxy release.

These methods are implemented for rmw_fastrtps in ros2/rmw_fastrtps#336, which can be used as a reference.

@dpotman
Copy link
Contributor

dpotman commented Jan 30, 2020

@ivanpauno have you already started implementing this? If not: I can make some time next week to implement these 2 methods.

@ivanpauno
Copy link
Member

@ivanpauno have you already started implementing this? If not: I can make some time next week to implement these 2 methods.

I didn't start. I assigned myself because after moving the Participant mapping to a Context and not a node, the discovery info is communicated in a completly different way, and the function will have to be rewritten.
If you want to have the function available just now, please go ahead an implement it.

Also, I'm not sure if we're going to do the refactor I previously mention in cyclonedds or not, as cyclone doesn't have the performance problems the other vendors have when creating multiple participants. That's another discussion ... but in the case the refactor is not applied to rmw_cyclonedds, I will just unassing me.

@dpotman
Copy link
Contributor

dpotman commented Feb 6, 2020

@ivanpauno I've implemented these two functions based on the current state wrt participant mapping. In case an update of these functions is required after the refactoring you mentioned, I'm happy to do that. In ros2/rcl#571 I've enabled the tests for Cyclone

@ivanpauno
Copy link
Member

@dennis-adlink Thanks for opening this!
I will run CI for #97 together with ros2/rcl#571, when the comments in the later are resolved.

It would be good if somebody from adlink can review #97, as I don't have much time now for doing a thorough review right (I would be able to do it in a few weeks).

@ivanpauno ivanpauno removed their assignment Feb 6, 2020
@mm318
Copy link
Member Author

mm318 commented Feb 18, 2020

This issue has been resolved with the completion of #97.

@mm318 mm318 closed this as completed Feb 18, 2020
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

No branches or pull requests

3 participants