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

Cancel Mission on STOP #18

Merged

Conversation

lowellausen
Copy link
Contributor

After some recent re-evaluations we have thought that it might make more sense for the Mission task to actually be cancelled on Stop.

For example, you have a robot doing a mission of visiting many different locations, and then you press a big red Stop button: you expect the robot to stop, not to just skip the current location where it was going to and proceed to the next one. If you want to cancel only the current location, you do have the option to cancel that one task specifically, as long as you have allow_skipping enabled

Changes:

  • Changed the Mission specification to cancel_on_stop=True
  • Removed some comments that didn't seem to make too much sense to me (I can add them again or modify if requested)
  • Will skip a unsuccessful if the whole mission wasn't cancelled
  • Will cancel a mission if cancel was requested
  • Will abort a mission if a subtask was unsuccessful and skipping is not allowed

Copy link
Contributor

@jonipol jonipol left a comment

Choose a reason for hiding this comment

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

LGTM

@Jannkar
Copy link
Collaborator

Jannkar commented Jun 30, 2024

Sounds good. I also agree that Missions in general should be cancelled on STOP command.

Copy link
Collaborator

@Jannkar Jannkar left a comment

Choose a reason for hiding this comment

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

The tests needed an approval to be ran automatically, so did that now. It seems like two of them are currently failing.

task_manager/task_manager/tasks/mission.py Show resolved Hide resolved
task_manager/task_manager/tasks/mission.py Outdated Show resolved Hide resolved
task_manager/task_manager/tasks/mission.py Show resolved Hide resolved
@Jannkar
Copy link
Collaborator

Jannkar commented Jul 5, 2024

The changes look great now! I've emailed you about one more thing to discuss. Once we resolve that one, this one is good to go.

@lowellausen
Copy link
Contributor Author

The changes look great now! I've emailed you about one more thing to discuss. Once we resolve that one, this one is good to go.

Hey! Not sure if I got your email 🤔

@lowellausen-karelics lowellausen-karelics merged commit 3052660 into Karelics:main Jul 17, 2024
1 check passed
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.

4 participants