Skip to content

Conversation

@eitanme
Copy link
Contributor

@eitanme eitanme commented Jan 24, 2022

This PR fixes #87 by waiting on both a result and terminal status before marking a goal as completed.

The behavior is a bit tricky to simulate from a testing perspective as it's a race so I didn't add any new tests, but I ensured it passes the current suite and works for my case in practice.

I did a few things here:

  • Limited status updates to the status channel to give one source of truth for goal statuses rather than allowing feedback and result messages to also inject their own state.
  • Wait on both a result object and a terminal status to mark an action as completed.
  • Set the status of a goal to pending when it goes out over the wire so it is immediately marked as active.

Let me know if there are any questions and hope this helps!

What type of change is this?

  • Bug fix in a backwards-compatible manner.
  • New feature in a backwards-compatible manner.
  • Breaking change: bug fix or new feature that involve incompatible API changes.
  • Other (e.g. doc update, configuration, etc)

Checklist

  • I added a line to the CHANGELOG.rst file in the Unreleased section under the most fitting heading (e.g. Added, Changed, Removed).
  • I ran all tests on my computer and it's all green (i.e. invoke test).
  • I ran lint on my computer and there are no errors (i.e. invoke check).
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added necessary documentation (if appropriate)

Copy link
Collaborator

@gonzalocasas gonzalocasas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for this!!

@gonzalocasas gonzalocasas merged commit 715be2c into RobotWebTools:main Feb 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Actionlib Wrapper Should Wait for Terminal Status to Finish Goals

2 participants