Skip to content

Conversation

@bergercookie
Copy link

Add a substitution class for loading a xacro file during evaluation.

P.S. Midway through writing this I discovered that I could use the Command substitution to call to the xacro executable (#366). However I still believe there's value in having a dedicated class for in that

  • it should be faster to call a pythn function than to invoke subprocess and
  • it should be more straightforward for the developer at hand to discover and also use this substitution type for manipulating xacro files instead of concatenating the individual command parts

Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

Note that in order to do this, you would have to make xacro a dependency of this package, and that would mean pulling it into the ROS 2 core. Overall, I don't think this is a good move; while I know xacro is popular, it isn't the only way to do things. So I don't think it should be "special" and I think use of the Command substitution could just be better documented.

@bergercookie
Copy link
Author

@clalancette, ok, thanks for the feedback, closing this.

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.

2 participants