search

gramaziokohler / roslibpy

Python ROS Bridge library
MIT License
273 stars 56 forks source link

Actionlib Wrapper Should Wait for Terminal Status to Finish Goals #87

Closed eitanme closed 2 years ago

eitanme commented 2 years ago

Description I think there is a bug in the Goal class within actionlib.py where a goal can be marked as finished before a terminal status is received because it only checks whether a result is received over the wire and makes no checks on status.

The issue is that status and result come through on different topics so there is a race where the last status received could be ACTIVE even after a result comes in on the wire. The goal will eventually transition to SUCCEDED, but it may do so after user checks on the goal status leading to unexpected results.

To Reproduce Steps to reproduce the behavior:

I've seen this intermittently in testing when checking for a SUCCEEDED goal status and unexpectedly seeing an ACTIVE status even when a result is received. It's a race so it doesn't always happen, but writing my own check to wait until status turns from ACTIVE to SUCCEEDED on the Goal fixes the behavior for me.

Expected behavior A Goal should not be considered finished until both a result message is received AND a terminal status message is received.

System (please complete the following information): Name: roslibpy Version: 1.2.1 Summary: Python ROS Bridge library. Home-page: https://github.com/gramaziokohler/roslibpy Author: Gramazio Kohler Research Author-email: gramaziokohler@arch.ethz.ch License: MIT license Location: /usr/local/lib/python3.8/dist-packages Requires: autobahn, twisted

Additional context I think this should be a pretty straightforward fix in is_finished and wait in the Goal class. I'm sorry I haven't submitted a patch as I've installed from pip and don't have the source to test with at the moment, but let me know if it'd be useful and I can try to find time to take a stab at it.

gonzalocasas commented 2 years ago

Thanks for the report @eitanme ! If you can/have the time to try and send us a patch, it would be more than welcome! If not, we'll get to it soon.

eitanme commented 2 years ago

I had some time, so I put together a fix in #88.

Hopefully, this helps, @gonzalocasas let me know what you think and if you have questions.