Skip to content

Update panda configs for ros2#51

Merged
henningkayser merged 9 commits intomoveit:ros2from
JafarAbdi:pr-update_configs
Mar 10, 2021
Merged

Update panda configs for ros2#51
henningkayser merged 9 commits intomoveit:ros2from
JafarAbdi:pr-update_configs

Conversation

@JafarAbdi
Copy link
Copy Markdown
Member

Move the common config files for MoveIt2 demos to panda_moveit_config package


<xacro:macro name="panda_ros2_control" params="name initial_positions_file">
<xacro:if value="${initial_positions_file == ''}">
<xacro:property name="initial_positions" value="${dict(panda_joint1=0.0,
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This's to prevent duplicating the whole config file just to change the start positions, see collision_start_positions.yaml singularity_start_positions.yaml servo_launch_test_common.py for examples

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

IMO, it would actually be much cleaner to provide a default config file which would also be set as the default property value in the top-level xacro.

Copy link
Copy Markdown

@destogl destogl left a comment

Choose a reason for hiding this comment

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

Just a few comments.

Copy link
Copy Markdown
Member

@henningkayser henningkayser left a comment

Choose a reason for hiding this comment

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

Looks good to me, please consider adding a default initial positions config instead of specifying the constants inline.

@henningkayser henningkayser merged commit b4797b7 into moveit:ros2 Mar 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants