search

ros-controls / ros2_controllers

Generic robotic controllers to accompany ros2_control
https://control.ros.org
Apache License 2.0
355 stars 318 forks source link

Test failures: subscription already associated with a wait set #1101

Closed christophfroehlich closed 2 months ago

christophfroehlich commented 5 months ago 
[INFO] [1713388339.950896541] [test_joint_group_velocity_controller]: activate successful
unknown file: Failure
C++ exception with description "subscription already associated with a wait set" thrown in the test body.

in rolling builds https://github.com/ros-controls/ros2_controllers/actions/runs/8728644425/job/23948909083?pr=1100 and in debian source build https://github.com/ros-controls/ros2_controllers/actions/runs/8728644426/job/23948908730

might be related to https://github.com/ros2/rclcpp/pull/2142

christophfroehlich commented 5 months ago

:eyes: @bmagyar they confirmed in the linked PR that this is not possible any more. You said you have a different approach for the test structure?

christophfroehlich commented 5 months ago

@dhood as you brought yourself into the topic, do you maybe have an idea how to fix that temporarily?

dhood commented 5 months ago

yes, curiosity got the better of me, I'm a sucker for CI that needs troubleshooting :innocent:

Perhaps explicitly removing the subscription from the waitset after it's been waited on, in wait_for? might be hanging onto a reference by the time the spin_some gets called. I opened https://github.com/ros-controls/ros2_controllers/pull/1106 hoping it would run CI for me but no such luck.

Otherwise an alternate approach to avoid waitsets could be an async wait on the underlying rt_commandptr to be set away from null (in combination with spin_until_future_complete), but not as nice. Maybe something else is possible with the default callback group of the node not having that parameter true that William mentioned, but I imagine it would affect something else on those nodes accidentally..?

christophfroehlich commented 2 months ago

fixed with #1206 (kudos to @saikishor)