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

Push notification on publisher/subscription match #264

Open
hidmic opened this issue Aug 24, 2020 · 3 comments
Open

Push notification on publisher/subscription match #264

hidmic opened this issue Aug 24, 2020 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@hidmic
Copy link
Contributor

hidmic commented Aug 24, 2020

Feature request

Feature description

Currently, the API does not provide (or documents) a generic mechanism to wait for publishers (subscriptions) to match subscriptions (publishers) -- only the current match count is available for query. Thus, code that monitors these counts has to sleep and retry. That's inefficient and potentially brittle. It'd be nice to enable such waits.

Implementation considerations

It seems to me that a new pub/sub event would be the simplest solution. All Tier 1 RMW implementations have a form of pub/sub matching listener that can be leveraged:

  • eProsima Fast-DDS has it, and both rmw_fastrtps_cpp and rmw_fastrtps_dynamic_cpp already use it (see here and here).
  • RTI Connext has it, and rmw_connext_cpp already uses it (see here and here).
  • ADLINK CycloneDDS has it, but rmw_cyclonedds_cpp doesn't use it yet (see here).

It's worth noting that some rcl tests appear to rely on pub/sub matching triggering the graph guard condition. Regardless of that not being true for any of the current Tier 1 RMW implementations, it seems to me that would create too many unwarranted awakes. Also, pub/sub matching does not imply a graph change.

@ivanpauno
Copy link
Member

ivanpauno commented Sep 15, 2020

I think that the graph guard condition should be triggered in all graph changes.
A summary of graph changes:

  • Anything that can change the output of the functions here.
  • subscription and publisher matching count: this and this.

About non-dds rmw implementations, FastRTPS didn't use to have wait sets (now FastDDS have).
They implemented rmw wait sets with the use of cv and listeners, so I think every non-DDS rmw implementation should be able to do that.

We could also have more fine grained guard conditions in the future.

About the difference between events and the graph guard condition, I think that in the future we could have a single event/condition type that can be added to the wait set instead of the wait set being able to wait in different kinds of objects.

@hidmic
Copy link
Contributor Author

hidmic commented Sep 23, 2020

Alright, as per offline discussion, we've agreed that the global guard condition should be able to detect all the aforementioned ROS graph events. #272 is a first step in this direction, updating the API spec.

@hidmic
Copy link
Contributor Author

hidmic commented Sep 24, 2020

Alright, since #272 publisher/subscription matching must trigger the graph guard condition. Implementations must now be adapted.

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

No branches or pull requests

2 participants