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

Stabilize commissioning feature #338

Merged
merged 14 commits into from
Apr 8, 2024
Merged

Stabilize commissioning feature #338

merged 14 commits into from
Apr 8, 2024

Conversation

mxgrey
Copy link
Contributor

@mxgrey mxgrey commented Mar 28, 2024

We have had a decommission feature in the unstable API for a while now, but it left some loose ends, e.g. what should be done with pending tasks that were already assigned to the decommissioned robot?

This PR adds a stable API for changing the commissioning of a robot. It provides a number of settings that allow the commission to be customized based on the needs of the deployment. Acceptance of dispatched tasks, direct tasks, and performing idle behavior can all be toggled separately. Any of these three can be toggled on or off at any time.

When requesting a change in commission it is also possible to specify what should happen with pending tasks for the robot. For dispatched tasks, there are three options, with reassign being the default:

  • reassign: The robot's tasks will be reassigned to other robots within the fleet. If the robot is being decommissioned for dispatched tasks then its pending tasks will be distributed across all the remaining robots in the fleet that are currently commissioned for dispatch requests. If there are no more robots in the fleet that are commissioned for dispatch requests, then the pending tasks will be canceled. If reassign is used while recommissioning a robot (toggling its dispatch_tasks commission to true) then the recommissioned robot may immediately draw in pending tasks from other robots in the fleet.
  • complete: There will be no change to the pending dispatched tasks of the robot. Even if the dispatch_tasks commission is being toggled off, the robot will continue to complete its pending task queue but will not accept any new dispatched tasks.
  • cancel: All pending dispatched tasks for the robot will be canceled. If set, this will take effect whether the robot's dispatch_tasks commission is being toggled on or off or remaining unchanged, so avoid using this setting recklessly.

The only options provided for handling the pending direct tasks are complete and cancel with complete being the default. We currently do not provide a way to reassign pending direct tasks from one robot to another.

Any changes to commissioning do not impact any tasks that are currently running. RMF does not currently provide a reliable way to hand off an ongoing task from one robot to another. If a robot that is currently running a task has been decommissioned, then the operator or system integrator must decide whether to allow the ongoing task to finish or to additionally issue a request to cancel the ongoing task. This choice will likely depend on operational and situational details, which is why this API does not take any automatic action on it.

This PR also fixes some previously existing issues:

  • If a robot in the fleet happens to begin one of its pending tasks in the very brief window between when a new task bid began and when that bid gets awarded, then that new task would never actually be run. This was a bug that has been overlooked because this race condition has such an extremely rare window that we have never noticed it happen before.
  • In some rare cases a newly awarded task bid would require the fleet adapter to replan all the tasks for its fleet, and that replanning was taking place on the main event loop worker, which would potentially delay all other main event loop callbacks that need to be processed. This has been fixed so that all task planning happens on a separate worker.

Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
…formation

Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
aaronchongth
aaronchongth previously approved these changes Apr 6, 2024
Copy link
Member

@aaronchongth aaronchongth left a comment

Choose a reason for hiding this comment

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

Very small nitpicks/questions, but everything else looks good on the first pass, thanks!

@@ -1043,7 +1147,11 @@ nlohmann::json TaskManager::submit_direct_request(
}
catch (const std::exception&)
{
json_errors.push_back(e);
nlohmann::json error;
error["code"] = 42;
Copy link
Member

Choose a reason for hiding this comment

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

any chance this is supposed to be 422?

ss << "The following planner errors occurred:";
for (const auto& e : errors)
std::stringstream ss;
ss << "Unabled to replan assignments when cancelling task ["
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ss << "Unabled to replan assignments when cancelling task ["
ss << "Unable to replan assignments when cancelling task ["

mxgrey and others added 2 commits April 8, 2024 13:31
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
@mxgrey mxgrey merged commit f849e97 into main Apr 8, 2024
3 of 4 checks passed
@mxgrey mxgrey deleted the commission_api branch April 8, 2024 07:13
arjo129 pushed a commit that referenced this pull request Jun 7, 2024
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Arjo Chakravarty <arjoc@google.com>
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.

None yet

2 participants