fix(params): prevent parameter retrieval crashes in ROS2 #978
Conversation
- Add check for node's fully qualified name to prevent deadlocks - Add 2.0s timeout to spin_until_future_complete - Fix crashes when retrieving parameters via Roslibjs Fixes RobotWebTools#972
Mehsias
force-pushed
the
fix/issue-972-parameter-retrieval
branch
from
237d668 to
fbed931
Compare
|
@sea-bass I don't know what I did to break the whole CI. I force pushed a squashed commit because the linter check was failing, and now everything is failing. |
|
You somehow broke the |
| ready = client.wait_for_service(timeout_sec=_timeout_sec) | ||
| if not ready: | ||
| raise RuntimeError("Wait for list_parameters service timed out") |
There was a problem hiding this comment.
This function can crash if the wait_for_service call fails as the RuntimeError is not catched anywhere. The easiest way to make this call fail is to have a node which doesn't start it's parameter services. Here's an example:
import rclpy
rclpy.init();
node = rclpy.create_node('rosapi_test_node', start_parameter_services=False)
rclpy.spin(node)As this is still related to the issue you are trying to fix (crashes during parameter retrieval), could you also fix that?
IMO we shouldn't wait 5 seconds for service to become available as this will greatly increase the response time when we are running nodes without parameter servers. If we assume the rosapi node has enough time to spin and gather information about the ROS graph, we should be good with just returning an empty list if the list_parameters service is not available right away:
| ready = client.wait_for_service(timeout_sec=_timeout_sec) | |
| if not ready: | |
| raise RuntimeError("Wait for list_parameters service timed out") | |
| if not client.service_is_ready(): | |
| return [] |
Public API Changes
None
Description
Fixes #972