Skip to content
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

Add response code to action_msgs/CancelGoal service #63

Closed
jacobperron opened this issue Feb 13, 2019 · 5 comments · Fixed by #76
Closed

Add response code to action_msgs/CancelGoal service #63

jacobperron opened this issue Feb 13, 2019 · 5 comments · Fixed by #76
Assignees
Labels
enhancement New feature or request

Comments

@jacobperron
Copy link
Member

Feature request

Feature description

It was discussed as part of the Actions design doc that a response code be added to the CancelGoal service to better communicate to an action client the reason for a declined cancel request.
Currently, in the event of a declined request (whether it is due to invalid data or is flat-out rejected by the server) the only information the action client receives is an empty list of goal IDs.

Implementation considerations

To avoid confusion with the action Result service, I propose we avoid the word "result" as proposed in ros2/design#193 (comment) and instead use "response":

int8 RESPONSE_OK = 0
int8 RESPONSE_REJECTED = 1
# A specific goal was attempted to be canceled, but no such goal exists
int8 RESPONSE_INVALID_GOAL_ID = 2

# Goal info containing an ID and timestamp
GoalInfo goal_info
---
# Goals that accepted the cancel request
GoalInfo[] goals_canceling
# Response code
int8 response

Open to ideas on changes to the possible error codes.

@jacobperron jacobperron added the enhancement New feature or request label Feb 13, 2019
@vmayoral
Copy link
Member

@ahcorde and @carlossv, I think you'll be interested on this.

@jacobperron jacobperron added the ready Work is about to start (Kanban column) label Apr 25, 2019
@jacobperron jacobperron self-assigned this Apr 25, 2019
@jacobperron jacobperron added in progress Actively being worked on (Kanban column) and removed ready Work is about to start (Kanban column) labels Apr 26, 2019
@jacobperron jacobperron added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Apr 27, 2019
@jacobperron
Copy link
Member Author

PR with proposed API: #76
Moved the constants into the response section of the interface definition.

@mkhansenbot
Copy link

@mjeronimo, please take a look

@mjeronimo
Copy link

What happens if there is a request to cancel multiple goals? Are the only options that all or canceled or none are canceled? If so, if there is a failure to cancel all of them, would the reason for the failed cancellation be the same for all of them?

If a client can send a request to cancel many goals (per the spec, "If the goal ID is empty and timestamp is zero, cancel all goals"), but only some are canceled then we can't just have a single reason/response code. Or, if they can fail to cancel for different reasons, it's also not enough information.

@jacobperron
Copy link
Member Author

That's a good point. Currently, the implementation will only return ERROR_UNKNOWN_GOAL_ID or ERROR_GOAL_TERMINATED if the cancel request was for a single goal (I don't think they make sense in the case where the request if for more than one goal). ERROR_REJECTED will be returned if the cancel request for each goal is rejected. I.e. if the request is accepted for at least one goal, then ERROR_NONE is returned.

It's possible that during a "cancel all goals" request that the user's callback only accepts the request for a subset of active goals. This is because the user-defined callback is called separately for each goal. In retrospect this might be misleading since ERROR_NONE is returned when only some goals are canceled during a "cancel all goals" request. We might consider being more explicit and constrain the users choice to either accept the cancel request for all goals or none, leaving no room rejecting the request for a subset of goals.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants