ros / actionlib

Provides a standardized interface for interfacing with preemptable tasks. Examples of this include moving the base to a target location, performing a laser scan and returning the resulting point cloud, detecting the handle of a door, etc.
http://www.ros.org/wiki/actionlib
96 stars 155 forks source link

Lost feedback on python actionclient #213

Open Doomerdinger opened 1 year ago

Doomerdinger commented 1 year ago

If using the python action client, sometimes the last (1 or more) feedback message(s) can get lost if the goal is marked as completed before the feedback callback is triggered.

This appears to be an intentional decision as seen here, where the feedback callback is skipped if the commstate is done.

However, I do not see this issue when using the c++ client. The same code on the c++ side can be seen here, which does not appear to check if the goal is completed or not.

This causes issues on the python side if the feedback is significant and we want to ensure we receive all feedback the server sends.

If my understanding of the c++ functionally is correct, this seems to be a mistake on the python side. If I can get some consensus here I will happily open a PR.

Timple commented 1 year ago

Interesting that there is a discrepancy between c++ and python...

I do want to add a concern here. We send a lot of consecutive action goals. What I certainly would see a regression if feedback from a previous goal ended up while a new goal was send out.

Our feedback messages contain a progress. Now imagine we send out a new goal, but a 99.9% feedback message arrives from a previous goal. We would draw wrong conclusions from this...

On the other hand, if no new goal is active we don't mind receiving it.

Personally I always saw action feedback like I see (non-latched) topics. Information might be missed. Therefor we put all important info in the result of the action. But of course this does not have to hold for everyone.

Doomerdinger commented 1 year ago

That's a good point.

Could potentially account for that using the goal_id.

Either way I'd just like it to be consistent across the two platforms which I do not think it is at the moment.