-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Check error flags when canceling an action task #19
Conversation
Signed-off-by: Joni Pöllänen <joni.pollanen@karelics.fi>
Signed-off-by: Joni Pöllänen <joni.pollanen@karelics.fi>
Signed-off-by: Joni Pöllänen <joni.pollanen@karelics.fi>
Signed-off-by: Joni Pöllänen <joni.pollanen@karelics.fi>
Signed-off-by: Joni Pöllänen <joni.pollanen@karelics.fi>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall! Left some minor cleanup-type comments and suggestions.
Signed-off-by: Joni Pöllänen <joni.pollanen@karelics.fi>
Signed-off-by: Joni Pöllänen <joni.pollanen@karelics.fi>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good from my side!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks nice! Great to have these cases also covered 👌
Adds check for the error flags present in the CancelGoal.Response message which we receive when canceling an action goal. Checking flags
ERROR_UNKNOWN_GOAL_ID
andERROR_GOAL_TERMINATED
to avoid falsely claiming a failure of cancellation if the goal is no longer valid (for example a case where the action server had restarted during the execution of a previous task and now a new task has been given).Also added a simple mypy config.
Next future improvement in this regard will be to monitor the connection to the server during task execution. This could give more flexibility to some of the hard coded timeout limits for example. Also should work nicely for the service tasks as well.