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

Lane closure #15

Merged
merged 7 commits into from
Mar 25, 2021
Merged

Lane closure #15

merged 7 commits into from
Mar 25, 2021

Conversation

mxgrey
Copy link
Contributor

@mxgrey mxgrey commented Mar 24, 2021

New feature implementation

Implemented feature

Allow fleet adapters to close navigation graph lanes.

Implementation description

The rmf_fleet_adapter::agv::FleetUpdateHandle API can now be told to close or open navigation graph lanes during runtime.

For the legacy fleet adapter (aka the full_control adapter) you can send a LaneRequest message to open or close lanes at any time. If the full_control adapter detects that the robot is currently on a lane that was just closed, then it will have the robot retreat back to the start of the lane and find a new plan from there. NOTE: We do not offer very strong guarantees about the effectiveness or reliability of this behavior for the legacy fleet adapter, because the rmf_fleet_msgs API does not always convey as much information as we need to make the behavior reliable.

Dependencies

Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
@mxgrey mxgrey added this to In Review in Research & Development Mar 24, 2021
@mxgrey mxgrey requested a review from Yadunund March 25, 2021 03:05
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
@mxgrey
Copy link
Contributor Author

mxgrey commented Mar 25, 2021

I've added some CLI tools for opening and closing lanes.

Using open-rmf/rmf_demos#13 it's easy to test the lane closure feature with the following commands (using a separate terminal for each):

$ ros2 launch rmf_demos office.launch.xml
$ ros2 run rmf_demos_tasks dispatch_loop -s tinyRobot1_charger -f pantry -n 20 --use_sim_time

Once that is running, you can use

$ ros2 run rmf_fleet_adapter close_lanes tinyRobot <lanes>

to close <lanes> (replace with the indices of the lanes you want to close) and then use

$ ros2 run rmf_fleet_adapter open_lanes tinyRobot <lanes>

to reopen <lanes>.

For this demo, you can use the illustration below to pick which lanes to open and close and watch the result. Note that each "lane" is unidirectional, so closing lane 30 will block the robot from going east while closing lane 31 will block the robot from going west.
office_lanes_annotate

If you close the lane that the robot is currently on, the fleet adapter should instruct it to return to the start of the lane and find a new path from there.

Yadunund
Yadunund previously approved these changes Mar 25, 2021
Copy link
Member

@Yadunund Yadunund left a comment

Choose a reason for hiding this comment

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

Tested and works well! I know this is expected behaviour but when a robot is moving towards a goal and all lanes to that goal are closed, the Planner will fail to find a solution and full_control will crash:
[full_control-15] [SearchForPath] CRITICAL ERROR: Impossible plan requested! Participant [tinyRobot_1] owned by [tinyRobot] Requested path (4) --> (2)

Just wondering if the fleet adapter should do some kind of check to see if the requested lane closure will lead to this type of error and hence not close the lane?

I was also planning to open a ticket in rmf_demos to add scripts to open/close lanes but I see that they have been added here directly 👍🏼

@mxgrey
Copy link
Contributor Author

mxgrey commented Mar 25, 2021

Just wondering if the fleet adapter should do some kind of check to see if the requested lane closure will lead to this type of error and hence not close the lane?

I think the fleet adapter needs to respect lane closing commands, because it may be the case that sending the robot down a closed lane would be harmful to the robot or others.

Instead we need to improve the error handling pipeline for critical task failures. I've added a ticket #16 for this.

@Yadunund
Copy link
Member

Agreed, that is a better approach.

Also, maybe in a future PR we can have FleetUpdateHandle publish to /closed_lanes (akin to how it publishes to /fleet_states)? Assuming we have fleets integrated with the new fleet adapter APIs, this information will not be broadcasted.

@mxgrey
Copy link
Contributor Author

mxgrey commented Mar 25, 2021

Also, maybe in a future PR we can have FleetUpdateHandle publish to /closed_lanes (akin to how it publishes to /fleet_states)? Assuming we have fleets integrated with the new fleet adapter APIs, this information will not be broadcasted.

Yeah, I wanted to save that for later to give us time to decide for sure how we want to approach that before committing to that particular topic/message long-term.

Signed-off-by: Michael X. Grey <grey@openrobotics.org>
@mxgrey mxgrey merged commit d18ee1b into main Mar 25, 2021
Research & Development automation moved this from In Review to Done Mar 25, 2021
@mxgrey mxgrey deleted the feature/lane_closure branch March 25, 2021 09:32
luca-della-vedova pushed a commit that referenced this pull request Jan 10, 2023
* Added recharge_soc to Constraints

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Updated ChargeBatter factory

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Constraints go into Configuration

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Add test for recharge_soc constraint

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* drain_battery separated from requests. Added to Configuration

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Uncrustify

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Moved drain_battery into Constraints

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Added Request::Model

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Uncrustify

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Removed max_charge_soc from ChargeBattery::make()

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Regroup arguments in make()

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Add model to PendingTask

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Optimizations

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Pass mutable reference of EstimateCache into estimate_finish

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Disable test printout

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Uncrustify

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Removed unused num_loops and cleaning_path

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Removed unused variables

Signed-off-by: Yadunund <yadunund@openrobotics.org>

* Update ChargeBattery.cpp

Signed-off-by: Yadunund <yadunund@openrobotics.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants