moveit / moveit_tutorials

A sphinx-based centralized documentation repo for MoveIt
https://moveit.github.io/moveit_tutorials/
BSD 3-Clause "New" or "Revised" License
463 stars 692 forks source link

Improve Low-Level Controllers Topic #810

Closed 01binary closed 4 months ago

01binary commented 5 months ago

Description

There is a lot of confusion around different controller managers supported by MoveIt and their configuration settings, integrating controllers with MoveIt by linking them with existing controller handle allocator plugins, and writing controller handles and allocators. Issues reported on GitHub, posts on Robotics Stack Exchange, ROS Answers, Google Groups attest that this part of documentation is both critical to integrating new robots with MoveIt and lacks sufficient depth and clarity.

This pull request addresses issues created in MoveIt 1 repository:

Questions for reviewers:

Checklist

welcome[bot] commented 5 months ago

Thanks for helping in improving MoveIt and open source robotics!

01binary commented 5 months ago

I don't know whether the ros_control tutorials explain that idea. Probably they do.

I checked ROS Control 1 documentation - it's just one page and links to presentation & paper. The presentation mentions "resources" but does not say that resources are joints. No specific mention of controllers claiming joints. Also checked ROS Control 2 documentation - it's smoother but still has large gaps.

I recommend leaving this in, otherwise I will have to copy-and-paste it into 100 different answers on Robotics Stack Exchange - it's a lot easier to link to relevant documentation.

Other than that, things looking good. Last remaining issue is we need to use an example project that has ROS Control configuration, and Panda does not. I see you replaced all the examples with Panda config.

The MSA-generated project must have the following to be a valid example here:

Panda only has RViz configuration to launch with fake controllers.

Would you like me to setup panda_moveit_config with ROS controls in a separate PR, get that merged, and then come back here?

rhaschke commented 5 months ago

Would you like me to setup panda_moveit_config with ROS controls in a separate PR, get that merged, and then come back here?

No, please don't do that. If you want to augment a robot config with such a config, consider using the Panda robot or Fanuc robot in moveit_resources.

Before merging this, I need to work through the code example sections. I didn't find time for this yet.

01binary commented 5 months ago

Sorry, I didn't realize I was not being clear. By "setup panda with moveit config" I meant augmenting panda_moveit_config by re-running the wizard to make sure all the files normally generated by MSA exist, are filled out, and work.

rhaschke commented 5 months ago

I think I understood you correctly. And I suggest not to update the standard panda_moveit_config which is very special and deviates in many regards from the MSA-generated files. However, the panda_moveit_config in moveit_resources is much closer to the MSA defaults. Hence, you should use that version.

01binary commented 4 months ago

My first few attempts to re-gen panda_moveit_config in moveit_resources (using panda_description) failed.

EDIT 1

Apparently MSA does not add filename attribute to <plugin name="gazebo_ros_control> element in URDF generated for simulation. I haven't seen this error before because my robot URDF already had the <gazebo><plugin> section in it, so MSA didn't have to add one. After I added the attribute Gazebo finally spawned the model, but then crashed after unpausing physics.

Continuing to troubleshoot. If this takes too long I'm fine with giving up on this aspect, and getting it patched up later in another PR. No reason to over-extend and keep PRs open for a long time when there's always another PR.

EDIT 2

If you are looking to test the code, see third_draft branch in trajectory_controller_example repo. If you clone that into a ROS1 workspace and install packages with rosdep I feel like it should build on the first attempt with no issues.

rhaschke commented 4 months ago

Regarding the issues you observed with Gazebo:

  1. I added the missing filename attribute in MSA via https://github.com/ros-planning/moveit/pull/3572
  2. The crash of Gazebo when unpausing physics is caused by a bug in joint_state_controller, fixed via https://github.com/ros-controls/ros_controllers/pull/630
01binary commented 4 months ago

I think we found a happy middle here: the topic starts with a survey of controller managers and their settings (split by type), then goes into integrating controllers (again split by type)

welcome[bot] commented 4 months ago

Congrats on getting your first MoveIt pull request merged and improving open source robotics!

01binary commented 4 months ago

Thanks for the merge! Sorry the PR had to bounce as both of us were busy at different times.

01binary commented 4 months ago

@simonschmeisser we can setup Zoom if you like, to make it a bit more interactive.

First question I have, is the current documentation after this pull request better than the previous version?

it is not clear to me when I would use which controller manager

The "Low-Level Controllers" topic follows the following macro-structure:

  1. Enumerate controller manager types
  2. Explain the use case for each controller manager and its configuration
  3. Explain how controllers interact with controller managers

Here's a high-level summary, let me know if this makes sense.

Currently my robot is not moving and I have no idea why. Is there a way to make ros_control acknowledge that it received a trajectory? Print the content? This kind of thing