Conversation
eb41a1e to
541538d
Compare
Hmm. Reading the existing local panda_moveit_config repo (I see .setup_assistant present) fails.I'm doing this in Docker container via
?? Adding UPDATE: I ended up running MSA 2.5.4 against |
Runtime error while verifying demo.launch.pyUPDATE: Resolved by stashing away the files generated in ros1 era. |
a496a64 to
838613a
Compare
rhaschke
left a comment
There was a problem hiding this comment.
Please stick to franka_description for the URDF! Only this one has the coarse collision models, which are also considered by the real robot's controller.
| <xacro:arg name="initial_positions_file" default="initial_positions.yaml" /> | ||
|
|
||
| <!-- Import panda urdf file --> | ||
| <xacro:include filename="$(find moveit_resources_panda_description)/urdf/panda.urdf" /> |
There was a problem hiding this comment.
panda_moveit_config should rely on franka_description, not on moveit_resources_panda_description.
| package: franka_description | ||
| relative_path: robots/panda/panda.urdf.xacro | ||
| xacro_args: hand:=true |
There was a problem hiding this comment.
panda_moveit_config should rely on franka_description, not on moveit_resources_panda_description.
There was a problem hiding this comment.
Makes sense especially having seen your comment moveit/moveit2_tutorials#61 (comment), which tells me the switch from panda_moveit_config to moveit_resources_panda_moveit_config in moveit2_tutorial might be unlikely a widely agreed move. Then, the entire PR will have to be re-done because the PR at the time of writing is made referencing to the model files in moveit_resources_panda_description package. Made the PR back to draft state.
| <package format="3"> | ||
| <name>panda_moveit_config</name> | ||
| <version>0.8.1</version> | ||
| <version>0.3.0</version> |
There was a problem hiding this comment.
I suggest switching to version 2.x
There was a problem hiding this comment.
Why not 1.x, when the current version is 0.x?
I admit I do not remember why I was downgrading to 0.3.
8980ecd to
f7efe7f
Compare
130s
left a comment
There was a problem hiding this comment.
This PR should still remain as draft before moveit/moveit2_tutorials#704 is closed. I updated other irrelevant review feedbacks.
| <xacro:arg name="initial_positions_file" default="initial_positions.yaml" /> | ||
|
|
||
| <!-- Import panda urdf file --> | ||
| <xacro:include filename="$(find moveit_resources_panda_description)/urdf/panda.urdf" /> |
| <package format="3"> | ||
| <name>panda_moveit_config</name> | ||
| <version>0.8.1</version> | ||
| <version>0.3.0</version> |
There was a problem hiding this comment.
Why not 1.x, when the current version is 0.x?
I admit I do not remember why I was downgrading to 0.3.
…e during ROS1 era
…rent to ros version.
… user/editors' convenience
f7efe7f to
17561bf
Compare
|
Dear Isaac, |
|
@rhaschke Sorry, (aside from the decision moveit/moveit2_tutorials#704 (comment)) I fully agree to keep this package away from moveit_resource. The push I made yesterday was just a sync, didn't mean to request re-review. I'll update this PR accordingly. |
|
Aside from the franka_description stuff, I primarily wanted to point out that there is already a moveit_config package. |
Issue
CoS
Confirmed on RViz2 planning and execution went as expected.
Impl approach
moveit_resources_panda_descriptioninstalled from deb, then merge the generated files into this repo.ros1folder, for easier access in case of need. IINM, this package is used widely for MoveIt tutorial so such a special setup might be hopefully acceptable.