Introduce start_at_departure option for go_to_place event

[kjchee]

Bug fix

Fixed bug

Summary

This PR addresses a semantic mismatch in how earliest_start_time from a task booking is propagated and interpreted downstream in the task planning pipeline.

Current Behavior
• The dispatch layer (eg. dispatch_patrol.py) sets [unix_millis_earliest_start_time].
• This value flows into FleetUpdateHandle.cpp then to PendingTask::make(), where it is stored as earliest_start_time.
• However, in GoToPlace.cpp, the argument is received and treated as earliest_arrival_time (in estimate_finish())).

As a result, the planner interprets the constraint as “do not arrive before this time”, while the booking contract intends “do not start before this time.” The "wait_until" return from estimate_finish() is sometime earlier than the [unix_millis_earliest_start_time].

This creates a semantic mismatch between task request and task execution.

Example
• Booking specifies earliest_start_time = 10:00am.
• Current behavior: Robot may plan so that it arrives at 10:00am (treating it as arrival gating).
• Intended behavior: Robot should only begin traveling no earlier than 10:00am (start gating).

Fix applied

• Update the function signature and usage in GoToPlace to consistently interpret the value as earliest_start_time (departure gate).

• Aligns planner behavior with the booking contract, ensuring the robot does not begin before the allowed start time.

Notes for Reviewers

From tracing the variable through the task dispatch json, it appears [unix_millis_earliest_start_time] is currently misinterpreted as earliest_arrival_time in GoToPlace.

This PR corrects the semantics and logic in GoToPlace, but I would appreciate confirmation from reviewers on whether the original design intent was indeed “start gating” rather than “arrival gating.”

If the original intent was truly to gate arrival, then further discussion may be needed (coz user thought it is earliest start time of the task). Otherwise, this fix resolves both the semantic mismatch and the resulting logic discrepancy.

[mxgrey]

Following up on the discussion during the PMC session today, I think what we should do is add a field to GoToPlace::Description called bool start_at_departure which is false by default. When start_at_departure is true, we will consider the departure of the robot to be the beginning of the GoToPlace event instead of the arrival.

We will also need to update these schemas to include an optional start_at_departure field. That way users who want the robot activity to begin at a specific time (rather than having the robot reach a destination at a specific time) can just set "start_at_departure": true inside their go_to_place objects in their task descriptions.

I believe this should resolve the problem without negatively impacting users who want to keep the current behavior. Let me know what you think.

[kjchee]

@mxgrey yea, I fully agree with adding the start_at_departure field, it makes the behavior much clearer without disrupting existing users.

I’m okay to proceed with the changes. Just to confirm, should I continue implementing them within this same PR, or would you prefer a separate one?

[mxgrey]

You can keep working off of this PR to keep the conversation all in one place. I can edit the PR title later so it makes sense with the new changes.

[kjchee]

Done, and ready for review.

The schema changes are in another PR in rmf_ros2 open-rmf/rmf_ros2#467, thanks.

[cwrx777]

@mxgrey @kjchee
Is the definition of unix_millis_earliest_state_time and how it is used consistent across all kind of tasks?

[kjchee]

Is the definition of unix_millis_earliest_state_time

@cwrx777 after Grey explained in the PMC session, I kind of agree:
1. Some users only care about the main task. For example, if they say “start delivery at 4 pm,” they mean the robot should arrive at the pickup spot and begin the pickup at 4 pm.
2. While other users care about the entire operation. For them, “earliest start time = 4 pm” means the robot should depart from its current location at 4 pm to head to the pickup spot.

So the start_at_departure flag helps cover both scenarios.

how it is used consistent across all kind of tasks?

@mxgrey yesterday I did a thorough read of the other tasks/events. So far, perform_action and WaitFor are not affected. I’m not sure if we also need to implement the same flag for Clean and Delivery. For Clean, if the user follows the rmf_demos example (dispatch_clean = go_to_place + perform_action), then it already falls back to using go_to_place with the start_at_departure flag.

[mxgrey]

Sorry for how delayed this review is, I've been busy travelling the last few weeks.

I'm quite happy with where we've settled on this. I think the implementation here looks perfect, I just left one minor request about the documentation for the new method.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (99ec3bf) to head (a3acea3).
⚠️ Report is 49 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##             main   #128       +/-   ##
=========================================
- Coverage   36.44%      0   -36.45%     
=========================================
  Files          65      0       -65     
  Lines        2557      0     -2557     
  Branches     1416      0     -1416     
=========================================
- Hits          932      0      -932     
+ Misses        550      0      -550     
+ Partials     1075      0     -1075     

Flag	Coverage Δ
tests	?

Flags with carried forward coverage won't be shown. Click here to find out more.
see 64 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.