Skip to content

Add GoToZone task-sequence event - #136

Open
chart-singapore wants to merge 6 commits into
open-rmf:mainfrom
chart-sg:feat/post_arrival_goal
Open

chart-singapore wants to merge 6 commits into
open-rmf:mainfrom
chart-sg:feat/post_arrival_goal

Conversation

@chart-singapore

Copy link
Copy Markdown

New feature implementation

Implemented feature

This is part of an 8-repo Simple GoToZone feature. Tracked in Stage A of open-rmf/rmf#726.

Adds GoToZone to rmf_task_sequence::events — a task-level description that constructs a GoToPlace activity model over all zone internal vertices, wrapped in a ZoneGuardModel for bidding protection (prevent bidding from the robot already in the target zone). This is the task-planner-side entry point for zone tasks.

Implementation description

GoToZone::Description

Factory: GoToZone::Description::make(zone_name, modifiers), where modifiers is std::optional<Modifiers> (default std::nullopt). The Modifiers struct carries group_hint, orientation_hint, and preferred_waypoints.

  • make_model() — looks up the zone via Graph::find_known_zone, collects waypoint indices for every internal vertex resolvable via graph.find_waypoint(name), builds a GoToPlace::Description::make_for_one_of(goals) activity model, and wraps it in ZoneGuardModel. Returns nullptr if the zone is unknown, if every internal-vertex name fails to resolve to a waypoint, or if the inner GoToPlace model fails to build.
  • generate_header() — produces the task header (category, description, duration).

ZoneGuardModel

Wraps an inner Activity::ConstModelPtr, purpose is to override estimate_finish(). For each candidate robot: if the robot's current waypoint is already one of the zone's internal-vertex waypoints, returns std::nullopt (purpose: excluding that robot from bidding on this zone task while keeping it available for every other task type).

This guard works at the estimate_finish level (per-robot) rather than at make_model (per-task) because the per-candidate robot state is only available on each estimate_finish call.

GenAI Use

We follow OSRA's policy on GenAI tools

  • I used a GenAI tool in this PR.
  • I did not use GenAI

Signed-off-by: kjchee <keai_jiang_chee@cgh.com.sg>
class GoToZone::Description : public Event::Description
{
public:
struct Modifiers

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The policy for all the C++ rmf_ libraries is that all public-facing APIs must use the pimpl pattern.

This should be changed to

class Modifiers
{
public:
  const std::string& group_hint() const;
  Modifiers& set_group_hint(std::string hint);

  std::optional<double> orientation_hint() const;
  Modifiers& set_orientation_hint(std::optional<double> hint);

  const std::vector<std::string>& preferred_waypoints() const;
  Modifiers& set_preferred_waypoints(std::vector<std::string> waypoints);

  class Implementation;
private:
  rmf_utils::impl_ptr<Implementation> _pimpl;
};

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Fixed in commit ad40936

namespace {
// Wraps a GoToPlace Activity::Model and rejects robots whose current
// waypoint is inside the target zone. Prevents robots already in the
// zone from winning bids for zone tasks targeting the same zone.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe we discussed this and concluded that we should not block robots who are already in a zone from being used for a GoToZone task.

If the entire purpose of a task is to have a robot in a zone, the task should be allowed to use a robot that's already in the zone.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Fixed in commit 81613a2.

Initially I just removed the ZoneGuardModel, but that alone wasn't enough. Eg, a robot already parked on a zone waypoint still lost the next GoToZone to the one at its charger.

After investigation, it turns out the GoToPlace one-of model picks its goal in make_model, from the invariant state. For the first phase of a task that's an empty State() (Task.cpp:387), so the nearest-goal loop never runs and every robot gets costed to goals[0].

So to fix that issue, now GoToZone gets its own small model, one GoToPlace per zone vertex, and estimate_finish takes whichever finishes first for that robot. A robot already on a zone vertex bids zero. Tested on chart_lab map, the 2nd GoToZone now goes to the robot already inside.

Signed-off-by: kjchee <keai_jiang_chee@cgh.com.sg>
Signed-off-by: kjchee <keai_jiang_chee@cgh.com.sg>
Signed-off-by: kjchee <keai_jiang_chee@cgh.com.sg>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Review

Development

Successfully merging this pull request may close these issues.

4 participants