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

EasyFullControl fleet_adapter_mir and cart delivery PerformAction plugin #36

Merged
merged 50 commits into from
Nov 13, 2024

Conversation

xiyuoh
Copy link
Member

@xiyuoh xiyuoh commented Jan 12, 2024

This PR:

  • Added note that marks MiRFleet adapter as inactive
  • Switches MiR fleet adapter to use the EasyFullControl API
  • Creates standard RMF missions from the fleet adapter via MiR's REST API calls so that users don't have to manually add them on the hardware. This also allows us to automatically create more missions on the MiR required by RMF in the future. Mission definitions can be found in rmf_missions.json.
  • Various improvements on the fleet adapter, such as using mission queue IDs to monitor mission/task completion, better localizing process when switching maps to a new level, and an expanded list of RMF missions that help robots get to a MiR position more accurately instead of only relying on coordinates
  • Adds a plugin system for MiR custom actions. Plugins can be defined and configured inside mir_config.yaml
  • Adds a CartDelivery perform action plugin that helps users to integrate delivery tasks with RMF. Users will need to create their own pickup and dropoff missions and specify the mission names inside mir_config.yaml. This plugin includes a task script for users to submit dispatch_pickup tasks. Remaining cart delivery related missions required by RMF can be automatically created on hardware during runtime by parsing rmf_cart_missions.json.

One major flaw in this fleet adapter is the is_charging function inside robot_adapter_mir.py. As far as I know and after checking with vendors, there doesn't seem to be a MiR API that we can use to check whether the robot has begun charging. The MiR status provides a mode_id and state_id, currently there's no known state/mode for charging. Will experiment with hardware to verify this.

Tested on hardware with MiR200 and MiR250:

  • Standard RMF mission creation (rmf_missions.json)
  • Fleet adapter able to launch and run
  • MiR able to perform patrol tasks and map switch
  • RMF cart delivery mission creation (rmf_cart_missions.json)
  • MiR able to perform delivery tasks

@xiyuoh xiyuoh requested a review from mxgrey January 12, 2024 10:23
@xiyuoh xiyuoh force-pushed the xiyu/efc_fleet_adapter_mir branch from 62b1f03 to 9adf286 Compare January 15, 2024 08:51
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
…supported

Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
@xiyuoh xiyuoh force-pushed the xiyu/efc_fleet_adapter_mir branch from 9adf286 to 946d98c Compare January 15, 2024 09:11
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
@xiyuoh
Copy link
Member Author

xiyuoh commented Feb 5, 2024

Update from MiR vendor: no available indication of when the robot has started charging. There is no existing state_id or mode_id meant for that data. Using mission status text is currently the only way to verify whether charging has started.

Copy link
Collaborator

@mxgrey mxgrey left a comment

Choose a reason for hiding this comment

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

There are a lot of excellent designs in this PR. Using the json files to define the MiR missions, and then loading and configuring them at startup is fantastic.

This PR presents a huge improvement over the previous state of the fleet adapter. There's only one problem: The current approach to plugin support does not actually support plugins. Instead the only way a downstream user can integrate or customize their own actions is to fork the source code of this repository and edit it.

At first I considered accepting this as a temporary limitation, merging the PR as-is, and then improving the situation later. However, I'm concerned that early adopters of these changes will end up being punished with a diverging code base after forking as needed. That makes it difficult for them to take in improvements that we're going to make in the future and it also discourages downstream users from contributing anything upwards.

I also feel that the gap to support plugins from where this PR is right now is actually not very large. The key thing will be understanding how to use importlib to load a module and then getattr to instantiate a class from that module. I left a few tips in my inline comments, but let's talk about this more in person when we have a chance.

configs/mir_config.yaml Outdated Show resolved Hide resolved
fleet_adapter_mir/fleet_adapter_mir/robot_adapter_mir.py Outdated Show resolved Hide resolved
fleet_adapter_mir/README.md Outdated Show resolved Hide resolved
fleet_adapter_mir/README.md Outdated Show resolved Hide resolved
fleet_adapter_mir/README.md Outdated Show resolved Hide resolved
fleet_adapter_mir/fleet_adapter_mir/fleet_adapter_mir.py Outdated Show resolved Hide resolved
fleet_adapter_mir/README.md Outdated Show resolved Hide resolved
configs/mir_config.yaml Show resolved Hide resolved
fleet_adapter_mir/fleet_adapter_mir/robot_adapter_mir.py Outdated Show resolved Hide resolved
…in pickup/dropoff, and move action-related code into its own package

Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
xiyuoh added 4 commits March 6, 2024 16:06
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
@xiyuoh
Copy link
Member Author

xiyuoh commented Mar 28, 2024

After our previous discussion, I've refactored the way we are creating MirAction objects in this new plugin system. They are now

  • Only created when a task comes in with the performable action, and destroyed when the task is completed
  • Created via an ActionFactory, and is tracked in the RobotAdapter under self.current_action
  • More accurate plugin system with the use of importlib to import the relevant modules

xiyuoh added 6 commits April 16, 2024 20:38
pickup pos is not found

Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
xiyuoh added 10 commits October 4, 2024 08:32
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
…vior in dispatch_delivery task

Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
@xiyuoh
Copy link
Member Author

xiyuoh commented Oct 8, 2024

Thanks for the meticulous review @mxgrey ! I've gone through another round of refactor, with the following changes:

  • Import action plugins and any supporting modules only once at startup. Each plugin's ActionFactory is instantiated once, and the adapter stores these factories in a dictionary. The dictionary key is the plugin_name as opposed to the suggested action name, as each plugin/ActionFactory could support multiple actions.
  • Using an ActionContext to store important handles needed throughout the plugin.
    After some thought, I decided to create the ActionContext at startup and exclude execution. This is because the ActionFactory may need access to some of the ActionContext members (e.g. access to MirAPI for creating missions) before a perform_action() is called.
  • At startup, the ActionFactory ensures that the imported config file contains a list of supported actions, otherwise it raises an error (55b8877). Depending on whether an action missions JSON filepath is provided, it will create the plugin-related mission on the robot. This mission creation step was previously inside the MirAction.init(), which meant that the adapter had to check for existing missions at every perform_action() call.
  • Raise errors if plugin-specific keys are not provided in the rmf_cart_delivery config: 55b8877
  • Added ActionFactory.supports_action() and implemented in rmf_cart_delivery
  • Splitting CartDelivery into CartPickup and CartDropoff. I think it would be a better demonstration of what you mentioned about a single plugin supporting multiple actions.
  • Adding cancellation behavior to dispatch_delivery. This ensures that if the delivery task is cancelled in the middle of a cart pickup, the robot will dropoff the cart before ending the task.
  • Some fixes to the existing CartPickup and CartDropoff logic
  • Minor fixes and improvements to address review comments: a153c8d, cecab1e, 9610e97
  • Better documentation (30d58ff). The links provided to the new docs in README are pointing to the main branch, which doesn't exist right now. Here are the newly added mir_missions.md and mir_actions.md.

Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
@xiyuoh xiyuoh requested a review from mxgrey October 8, 2024 09:49
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
Copy link
Collaborator

@mxgrey mxgrey left a comment

Choose a reason for hiding this comment

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

This PR is looking very good and getting super super close.

I left just a few more items of feedback which I think should be pretty straightforward to address. After those are done we should be fully ready to merge.

Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
@xiyuoh xiyuoh requested a review from mxgrey November 13, 2024 06:07
Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
Copy link
Collaborator

@mxgrey mxgrey left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for all the massive iterating.

This is without question the most professional and modular RMF fleet adapter known to exist. As we work on the next generation of Open-RMF's SDK we'll definitely want to take advantage of lessons learned in this effort to figure out how to lift as much of this upstream as we can.

Great job bringing this to completion 🎉

@xiyuoh
Copy link
Member Author

xiyuoh commented Nov 13, 2024

Thanks Grey, really appreciate the detailed review! I've updated with the changes mentioned since they're small and quick to add in. I'll have to trouble you to look at 45ac09a that I just pushed, it happens to remove the exception message with the grammar mistake.

Edit: missed out 3045bcc

Signed-off-by: Xiyu Oh <xiyu@openrobotics.org>
@mxgrey mxgrey merged commit a9cef7d into main Nov 13, 2024
1 check passed
@mxgrey mxgrey deleted the xiyu/efc_fleet_adapter_mir branch November 13, 2024 08:34
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