Provide detailed and coarse collision models #199
Conversation
rhaschke
force-pushed
the
modular-xacro
branch
from
f317dfa
to
7961f52
Compare
rhaschke
changed the title
…t-states-desired) and frankaemika#194 (tool-frame) into develop
|
@rhaschke Thanks a lot for Implementing this! Merging the gazebo and normal |
…t-states-desired) and frankaemika#194 (tool-frame) into develop
…t-states-desired) and frankaemika#194 (tool-frame) into develop
…t-states-desired) and frankaemika#194 (tool-frame) into develop
We are still working on the other open points in #193. We will give you feedback here shortly |
|
Friendly ping, @gollth. Did you already had a chance do look into this PR? This is crucial for a new Noetic release of |
|
Hi @rhaschke, glad to hear the new release of MoveIT. We are currently quite busy with other stuff but have this collision model topic on the radar and will have a look soonish! |
There was a problem hiding this comment.
Hi @rhaschke, I will test your PR together with the new MoveIT shortly. In the meantime I can already give you some comments on the code itself.
- Please revert the
prefixtoarm_idto avoid breaking changes. I get your point, that here it is used as a prefix to joint names, but in all our nomenclature we use the termarm_id. Especially for multi-arm situation it makes sense to distinguish multiple robots by ids. - Please revert the renaming the
panda_arm.xacrofile for backwards compatibility
|
When I run ▏ERROR▕ /franka_control ⟩ ControllerManager::loadController() ⟩ L313 ⟫ Could not load controller 'franka_gripper' because controller type 'GripperCommand' does not exist.Am I missing a dependency? |
Note that we can maintain backwards compatibility by other means. For example, #210 illustrates how to deprecate a property in favour of a more meaningful name.
The culprit is the definition of the |
They serve different purposes: - coarse collision geometries mimic the internal self-collision checking of the real-robot controller - mesh models should be used for collision checking with environmental objects
- Declare inertial properties in a yaml file - Use xacro:inertial macro
xacro requires macro names to be different from actual tag names.
This way you can specify a rotational offset between panda_link* and panda_link*_sc
This macro abstracts the creation of a "collision capsule" from two collision spheres and a collision cylinder. Since arbitraty RPY rotations are hard to calculate in XACRO (would require rotation matrix multiplication and probably reduce redability) for now we only allow the discrete 6 orientations which are aligned with X/Y/Z axes.
The capsule at link8 covers the Pilot which is attached to link7. Though rigidly connected to link7, link8 is more a "virtual" link without geometry indicating the flange of the robot. That's why it makes more sense to also have the collision capsule attached to link7_sc.
…capsule-macro to SRR-1201/self-collision-geometries * commit '2a4542cd619942667e59e4a477a9c4137ba3617d': HACK: Disable gripper_sim_tests_with_object because flaky FIX: URDF tests in Python3/Noetic ADD: urdfdom-py as CI dependency ADD: URDF test suite ADD: Comment explaining sx,sy,sz properties in utils.xacro CHANGE: Use `<xacro:collision_capsule>` also for hand.xacro CHANGE: Move panda_link8_sc -> panda_link7_sc CHANGE: Use new `<xacro:collision_capsule>` macro for self collision geometries ADD: `<xacro:collision_capsule>` macro ADD: `rpy` parameter to `<xacro:link_detailed_coarse>`
The name "detailed_coarse" is a bit contradicting. More suitable might be a name which indicates that existing `<link>`s are augmented with additional functionality such as self-collision geometries. This in mind it also should also be rather consise so I went with `link_with_sc`.
|
Hi @rhaschke sorry that this took so long. We had a bit of internal discussions about this PR, but finally found consensus. Thanks for the fix in panda_moveit_config with the self-collision checks. Now this works as expected:
I took the freedom to rebase your Regarding the weird capsule at the Pilot: this was actually wrongly placed and rotated -> should also be fixed now. In addition this capsule is now part of link7 like you suggested. |
|
Thanks for addressing the remaining TODOs. Looks good for me now as well. I just removed
I'm curious what was controversial about this PR. |
Mostly naming and if the |
…ription-to-support-fr3 to develop * commit '7e8be0dd968f6c4b80aa54d4f6bcc06bd0e7284f': fix cartesian pose example generalize over arm_id in franka_example_controllers.yaml CHANGE: Parameterize tests for both panda + fr3 FIX: Make <arg> in `franka_robot` macro property CHANGE: Extract <limits> & <safety_controller> into custom macro update CHANGELOG.md allow selecting fr3 urdf for launch files define joint limits for fr3 load joint limits directly in top-level URDf use correct urdfs in launch files refactor franka_description to support for fr3
As pointed out in #189 (comment), the coarse self-collision models not only pose an issue for the hand but also for the rest of the robot. However, the hand is most crucial for grasping.
This PR implements the ideas proposed in #189 (comment), namely
linkto reduce boilerplate codepanda_gazebo.xacrointopanda_arm.xacroandhand.xacroSomebody from Franka (@gollth, @marcbone) or @rickstaa? should pick this up, if we agree on the general approach.This PR replaces/generalizes #189.
Fixes #190 (mimic joint is part of hand.xacro).
TODOs
link8has some coarse collision models associated, which should better get associated tolink7. Effectively, of course, that doesn't make a difference as both links are rigidly connected. However, semantically, it would be more appealing.link8is composed of two very similar spheres and a cylinder that is - in my opinion - wrongly oriented. I guess, this geometry should reflect a capsule as well? Could it get replaced by a simple sphere?