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

MoveIt Config Redesign - Migrate MSA to ROS 2 #313

Closed
henningkayser opened this issue Dec 4, 2020 · 18 comments
Closed

MoveIt Config Redesign - Migrate MSA to ROS 2 #313

henningkayser opened this issue Dec 4, 2020 · 18 comments
Assignees
Labels
enhancement New feature or request Epic roadmap Current and upcoming roadmap features, listed on the MoveIt website

Comments

@henningkayser
Copy link
Member

henningkayser commented Dec 4, 2020

Porting the MSA requires more effort than simply applying the migration steps. With the switch to Python launch files, the new rclcpp::Parameter interface and related config changes (dropped XmlRpc support) require non-trivial re-implementation of some parts, including:

  1. Initial Migration (Initial port of MSA gui #740)
  2. Standardized yaml configs and parameter parsing (Standardize parameter parsing #431)
  3. New generic launch scripts, available as templates (Standardize usage of ROS Parameters #335)
  4. Adaption of changed config/launch setups to MSA layout and implementation (Rethinking the layout of MoveIt Setup Assistant #449)

Design Ideas

Config files
All nodes/classes declare parameters in a standardized way so that it's possible to generate valid config (yaml) files automatically (e.g. ros2 param dump), ideally including descriptions and comments. This would allow MSA to generate config files from the actual node implementation. All exceptions should be caught and handled accordingly, parsing and conversion of abstract data types should be handled with proper parameter documentation and runtime messages/warnings. Optionally, node configs include version tags, loading of outdated configs would then result in a deprecation warning.

Launch files
MoveIt configs should be static by default and only contain the config (yaml,srdf) files needed to describe the setup configuration. The default launch scripts are not copied into each MoveIt config package, they are only used to initialize a MoveIt setup instance (MoveGroup, MoveItCpp) by reading the config files from a config package. Customization of launch descriptions can be enabled by supporting launch file overrides or exporting/importing of factory functions from the launch scripts. This would allow to keep a standard and reduce redundancy (and maintenance overhead) in launch files. Of course it's up to the devs to copy the whole launch architecture and modify it as needed.

@henningkayser henningkayser added the enhancement New feature or request label Dec 4, 2020
@henningkayser henningkayser added this to the MoveIt Config Redesign milestone May 4, 2021
@henningkayser henningkayser changed the title Migrate MoveItSetupAssistant to ROS 2 MoveIt Config Redesign - Migrate MSA to ROS 2 May 31, 2021
@henningkayser henningkayser added the roadmap Current and upcoming roadmap features, listed on the MoveIt website label May 31, 2021
@henningkayser henningkayser self-assigned this May 31, 2021
@DLu
Copy link
Contributor

DLu commented Oct 11, 2021

MoveIt Setup Assistant Refactor Proposal

I propose that MSA be reimplemented using pluginlib. Most of the "widgets" would be moved to separate packages and implemented as plugins. The core moveit_setup_assistant package would only contain a few key elements.

  • List of available plugins
  • Set of shared data files
  • Functionality for loading some subset of the plugins
  • Framework to populate the GUI (including RViz display)
  • Intro and Outro widgets

The data files need to be shared at the high level because there are multiple configuration steps that operate on the same data file (i.e. the SRDF) and some configuration steps will need to load data from other configuration steps.

The overall control flow might be something like:

  • Show the Initial SetupScreenWidget to select the URDF to operate on OR load an existing moveit_config package (same as before)
  • With an eye toward Rethinking the layout of MoveIt Setup Assistant #449, ask the user what they want to do.
    • Each of the available plugins could report a hierarchical string describing what that configuration step does. For example, the VirtualJointsWidget could report something like ["Configure the SRDF", "Update the Virtual Joints"] and multiple operations would be grouped under the section "Configure the SRDF"
    • User would then select a subset of the options which would dictate which plugins get loaded.
  • The user would then walk through each of the chosen configuration steps.
    • There needs to be a mechanism for specifying dependencies, so that, for example, you can't specify robot poses until you configure the planning groups.
  • At the end, you are presented with the ConfigurationFilesWidget which presents the different shared files that need to be written to.

As much as possible, the GUI logic should be kept separately from the core data-management.

@v4hn
Copy link
Contributor

v4hn commented Oct 17, 2021

I propose that MSA be reimplemented using pluginlib.

Reading this from you reminded me of slide 30 from your 2017 ROSCon talk. 😂

I would love plugin support for MSA for users to add their own configuration options.
I don't see the merit in "outsourcing" all standard tabs though and would keep them local classes. But, granted, the difference between local class and plugin in the local package is minor.

As you said the defining data structures need to be defined outside the plugins anyway.

Asking what steps to perform later on would be just another technical hurdle for the user.
It's equally possible to always add all plugins/widgets to the list. Maybe invoke them lazily if the user clicks on them.

As much as possible, the GUI logic should be kept separately from the core data-management.

👍 It would be great to have cli programs for all functionality, if just to finally write reasonable CI tests for MSA!

@gavanderhoorn
Copy link
Member

As I don't see it mentioned yet: moveit/moveit#1375.

@mayman99
Copy link
Contributor

mayman99 commented Oct 29, 2021

What we were trying to implement here ros-planning/moveit#1375 agrees with most of what @DLu and @v4hn are proposing.

As explained in issue#1527, these are the issues that were present in that implementation.

- Data modularization: Each widget should store its data locally. Currently, all the data is stored centrally in MoveItConfigData.
- Add loading of existing config files to allow proper editing of files. This will render the ConfigurationFilesWidget obsolete as  there is no danger to overwrite manual changes.
- Use Qt's UI files to layout widgets - instead of manually creating and layouting items. 

I couldn't alocate enough time back them to tackle those further requirements. I can help at the moment.

@henningkayser
Copy link
Member Author

Hey @DLu I’m using ZenHub in GitHub, click this link to join my workspace and see other features available in GitHub or download the ZenHub extension and sign up with your GitHub account.
Posted using ZenHub

@cgarry-vs
Copy link

Are you aiming to have this go out with the Humble Hawksbill release?

@JafarAbdi
Copy link
Member

Are you aiming to have this go out with the Humble Hawksbill release?

Yes, that's the plan

@vatanaksoytezer
Copy link
Contributor

We plan to attempt to merge feature MSA branch very soon after #1240, and the plan is after some testing next week we will hopefully release Humble with MSA.

@nlgilbert
Copy link

Any updates on getting MSA in ROS 2? Will we see it in any versions prior to Humble (Foxy or Galactic)?

@DLu
Copy link
Contributor

DLu commented Jun 16, 2022

It is in the process of being merged into main: #1254 and will be backported to old releases.

@jure-no
Copy link

jure-no commented Jun 20, 2022

Is there any new information, when will we get MSA for humble? It would really help me a lot

@vatanaksoytezer
Copy link
Contributor

It's waiting for some more reviews and maybe a few bug fixes. You can try to use the feature/msa branch meanwhile.

@jure-no
Copy link

jure-no commented Jun 20, 2022

Is this expected to be published in the next 14 days to one month?

@vatanaksoytezer
Copy link
Contributor

Don't want to make any promises but, yes hopefully it should be ready in that time interval

@mikeferguson
Copy link
Contributor

mikeferguson commented Jun 27, 2022

Branch is merged - kudos to @DLu and @vatanaksoytezer

It looks like this ticket is complete and can be closed?

@jure-no
Copy link

jure-no commented Jun 28, 2022

Really? that is awesome. Where can i get MSA for moveit2 now?

@vatanaksoytezer
Copy link
Contributor

@jure-no You can use the main branch to compile on Humble or Rolling by following the instructions on our webpage, please let us know if you see any issues.

@mikeferguson we can close the ticket, thanks for the heads up!

@mikeferguson
Copy link
Contributor

@jure-no I will note, MOST things work with the MSA - but you'll see a few remaining bug tickets for certain more advanced features (and some active PRs) if you search "MSA" in our issue tracker. If you run into any issues, please post an issue (or comment on an existing one if that particular bug is blocking you).

@mikeferguson mikeferguson unpinned this issue Jun 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Epic roadmap Current and upcoming roadmap features, listed on the MoveIt website
Projects
None yet
Development

No branches or pull requests