Skip to content

Conversation

@kjchee
Copy link
Contributor

@kjchee kjchee commented Dec 23, 2025

Bug fix

Fixed bug

When using 'one_of' feature in GoToPlace, a reachable goal followed by an unreachable goal (eg. lane closed) combination will trigger a runtime failure. eg:

Screenshot from 2025-12-24 06-45-16

The root cause is a nullopt dereference during goal selection, where the estimate time comparison unconditionally accesses invariant_duration_opt for a subsequent unreachable goal while generating the GoToPlace model.

Fix applied

Applied a nullopt check before else if (shortest_travel_time.value() > invariant_duration_opt.value()).
If invariant_duration_opt is nullopt then we skip this goal candidate.

To test

a. Before fix:

  1. run office demo
  2. i turn off finishing request (set to 'nothing') to ensure at start, tinyRobot1 is at waypoint tinyRobot1_charger, and tintRobot2 is at waypoint tinyRobot2_charger.
  3. close lanes between waypoint presupplies & waypoint patrol_D2
    ros2 topic pub /lane_closure_requests rmf_fleet_msgs/msg/LaneRequest "{fleet_name: 'tinyRobot', open_lanes: [], close_lanes: [0,1]}" --once
Screenshot from 2025-12-24 07-00-24
  1. send gotoplace task with 'one_of' (coe & supplies)
    ros2 run rmf_demos_tasks dispatch_go_to_place -p coe supplies --use_sim_time
  2. we will see the runtime failure.

b. After fix:

  1. apply the fix and repeat the above step, and there will be no runtime failure

GenAI Use

We follow OSRA's policy on GenAI tools

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

Generated-by:

@mxgrey mxgrey added this to PMC Board Dec 23, 2025
@github-project-automation github-project-automation bot moved this to Inbox in PMC Board Dec 23, 2025
@arjo129 arjo129 self-requested a review December 24, 2025 01:32
Copy link
Member

@arjo129 arjo129 left a comment

Choose a reason for hiding this comment

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

Thanks for catching this! Tested and confirmed that the fix solves the bug,

@arjo129 arjo129 merged commit 1ab3a1d into open-rmf:main Dec 24, 2025
4 checks passed
@github-project-automation github-project-automation bot moved this from Inbox to Done in PMC Board Dec 24, 2025
@codecov
Copy link

codecov bot commented Dec 24, 2025

Codecov Report

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

Additional details and impacted files
@@            Coverage Diff            @@
##             main   #130       +/-   ##
=========================================
- 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants