ros-controls / ros2_control

Generic and simple controls framework for ROS 2
https://control.ros.org
Apache License 2.0
469 stars 284 forks source link

spawner does not work with discovery-server #1200

Closed Timple closed 2 weeks ago

Timple commented 8 months ago

Describe the bug The fastdds discovery server is optimized in the sense that one cannot (by default) see all nodes/topics/services.

The spawner therefor observes wrongfully that the controller_manager is not present and waits until a timeout occurs.

To Reproduce Steps to reproduce the behavior:

  1. Terminal 1: fastdds discovery -i 0
  2. Terminal 2: export ROS_DISCOVERY_SERVER=localhost:11811 && ros2 launch ros2_control_demo_example_2 diffbot.launch.py

Expected behavior Everything works the same as without discovery server.

Screenshots Not helpful

Environment (please complete the following information):

Proposed solution I think crawling nodes and services to find your other half is an anti-pattern.

We could use a pythonic "Ask forgiveness not permission" method. Try to call the service until is succeeds or a timeout is reached.

Can you agree on such an implementation? Than we could implement this.

fmauch commented 8 months ago

I was just about to start implementing a fix for #1182 when stumbling upon this. I would be nice to fix both issues with a change.

@Timple crawling for nodes is one thing. From my understanding waiting for services is not an anti-pattern but the usual way to make sure your target service is actually there before calling it.

Removing the wait_for_controller_manager functionality makes use of wait_for_service internally which seems to work fine with the discovery server. If this also solves my use case from #1182 I'll provide a PR implementing this.

Timple commented 8 months ago

I didn't mean to suggest waiting for a service is an anti-pattern. I was refering to the crawling.

Although I don't really see the benefit of waiting for the service. Why not do the pythonic optimistic way: try the service and re-try if it failed. It doesn't introduce more overhead I think. It even reduces overhead in the happy-flow.

But great that you are looking into this 🙂

destogl commented 2 weeks ago

resolved in #1562