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

Use perform_action instead of dock() for cleaning tasks #162

Closed
wants to merge 18 commits into from

Conversation

xiyuoh
Copy link
Member

@xiyuoh xiyuoh commented Sep 30, 2022

This PR follows the changes made for rmf-web in open-rmf/rmf-web#643. It also updates how performable actions are added to the rmf demo fleet adapter.

This PR:

  • Adds a check_perform_action() to RobotCommandHandle to process performable actions such as cleaning tasks
  • Removes dock_name from cleaning zones in hotel and airport building.yaml
  • Adds an action field to the relevant worlds' config.yaml to specify clean as a performable action, and updates fleet_adapter accordingly to add them to the fleet if the field is populated
  • Updates dispatch_clean to publish an ApiRequest with a compose task json

@Yadunund
Copy link
Member

Yadunund commented Sep 30, 2022

Suggest to refactor as below so that you can also update the schedule itinerary of the robot while cleaning. This is a feature that is currently supported in the fleet adapters we have implemented for other cleaning robots.

  1. Add YAML nodes containing waypoints for processes to Fleet config
process_waypoints:
  zone_1: [[x1,y1], ... [xn, yn]]
  zone_2: [[x1,y1], ...[xn, yn]]
  1. Add an API to the fleet manager to retrieve the list of waypoints for a given provess str. (the fleet_manager will return the list populated as described in Step 1)

  2. Update start_process() in fleet_manager to publish path request with waypoint locations from the process_waypoints yaml elements in the config.

  3. Update the action_executor such that when it parses an action, it
    a. Calls the API from 2) to get the waypoints for the process.
    b. Calls the start_process() api to trigger the fleet manager to send the path request to slotcar
    c. Use the unstable participant API to update RMF traffic schedule with the cleaning path of the robot. ie take the result of a) and call the participant.set_itinerary() API. This step is important for real world deployments where we want other robots in the facility to be aware of the cleaning action. This has already been implemented within perform_action for the Ecobot fleet adapter which you can reference here

This way, there is no explicit mention of clean_ in any of the APIs here. We keep things process and vendor neutral. It also does not require a cleaning server.

@xiyuoh xiyuoh marked this pull request as ready for review October 5, 2022 14:49
@xiyuoh xiyuoh requested a review from Yadunund October 5, 2022 14:49
@@ -37,6 +37,40 @@ rmf_fleet:
delivery: False
clean: True
finishing_request: "park" # [park, charge, nothing]
action: ["clean"]
Copy link
Member

Choose a reason for hiding this comment

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

Nit, since this is a list of actions it should probably be plural? Imho it's also a bit confusing to see the clean: True above, is it still used / should we remove or deprecate it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed to actions in 96fff51. clean: True is not used anymore, removed it from all the configs and fleet_adapter.py in fdb2d28


self._action_thread = threading.Thread(target=_perform_action)
self._action_thread.start()
return
Copy link
Member

Choose a reason for hiding this comment

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

Nit return is redundant here

Copy link
Member Author

Choose a reason for hiding this comment

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

@@ -543,6 +608,7 @@ def update_state(self):
if not self.started_action:
self.started_action = True
self.api.toggle_action(self.name, self.started_action)
self.check_perform_action()
Copy link
Member

Choose a reason for hiding this comment

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

I find the name of the function a bit misleading, check_perform_action implies that it is a function called periodically to check the status of the action, while here it is really only used once to start the action itself. By contrast in the ecobot fleet adapter the function is called periodically. I guess we can rename it to something more accurate, i.e. start_perform_action?

Copy link
Member Author

Choose a reason for hiding this comment

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

agreed, updated in 96fff51

@luca-della-vedova
Copy link
Member

I think there might be an issue with the update of the RMF schedule, i.e. below:

schedule_not_updated.mp4

I think it is due to the fact that the schedule submitted in every iteration is always the same (initial schedule fetched through the retrieve_process_waypoints API), we should probably keep track of the process waypoints, the position of the robot and update the schedule accordingly over time, to make sure traffic deconfliction works as intended

mxgrey added a commit that referenced this pull request Jun 30, 2023
Signed-off-by: Michael X. Grey <[email protected]>
@mxgrey
Copy link
Collaborator

mxgrey commented Jul 12, 2023

The relevant parts of this PR have been merged into #188 and the cleaning activity should be working nicely when combined with open-rmf/rmf_ros2#286

I'll close this PR now since it's no longer needed.

@mxgrey mxgrey closed this Jul 12, 2023
@mxgrey mxgrey deleted the feature/perform_action_clean branch July 12, 2023 09:39
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.

4 participants