Fix multi-line strings in DeclareLaunchArgument #18
Conversation
|
@MatthijsBurgh If you could fix the merge conflicts introduced by #7 that would be awesome! |
|
@fmauch please have a look at https://github.com/UniversalRobots/Universal_Robots_ROS2_GZ_Simulation/pull/18/files#diff-07ff1e87117ca592c12e8dcd194c3721e5cd805507f87a755e66fb65fce40670L72 and https://github.com/UniversalRobots/Universal_Robots_ROS2_GZ_Simulation/pull/18/files#diff-07ff1e87117ca592c12e8dcd194c3721e5cd805507f87a755e66fb65fce40670L80. As IMO the URDF/XACRO file should be in the description package. The description package should be configurable, to make the launch file reusable and also because you use these arguments in But my changes create a conflict for the default values. So please let me know how you want to solve it. |
|
@MatthijsBurgh that's exactly one of the changes we did in the latest refactoring of the description. The description now only contains the actual kinematic description. The ros2_control part gets added in the places that actually start the control structure. Hence, every control package (driver, gazebo classic, gz sim) build their own description assembling the kinematic description and the control description. We made those changes on purpose in order to remove dependencies and move the level of concern to where it actually belongs. |
|
That is fine. It means the rviz config is not always in the description pkg. So do you want to create another argument for that? And maybe also for the rviz config filename? |
|
I would say, let's not worry about that in this PR, that is what we have #22 for. |
|
So any changes needed here or not? |
|
No changes needed here, just the docstrings. Thank you for your continuous effort! |
|
But I already included more changes than just the docstrings. So are you happy with the current status of this PR? |
|
Sorry, my statement was not clear. Please remove all the changes that are not related to the docstrings. This PR should only alter the docstrings that are present on the current |
|
@fmauch done |
|
Thank you @MatthijsBurgh It seems like the |
|
@fmauch sorry, I missed that last argument and your comment. Should be fixed now. |
There was a problem hiding this comment.
Thanks for coming back to this @MatthijsBurgh, this looks good!
| description_package = LaunchConfiguration("description_package") | ||
| description_file = LaunchConfiguration("description_file") |
There was a problem hiding this comment.
please revert that change, as we want to get rid of the "description package" structure.
| description_package = LaunchConfiguration("description_package") | |
| description_file = LaunchConfiguration("description_file") |
| @@ -69,15 +71,15 @@ def launch_setup(context, *args, **kwargs): | |||
| ) | |||
|
|
|||
| rviz_config_file = PathJoinSubstitution( | |||
| [FindPackageShare("ur_description"), "rviz", "view_robot.rviz"] | |||
| [FindPackageShare(description_package), "rviz", "view_robot.rviz"] | |||
There was a problem hiding this comment.
please revert that change, as we want to get rid of the "description package" structure.
| ) | ||
|
|
||
| robot_description_content = Command( | ||
| [ | ||
| PathJoinSubstitution([FindExecutable(name="xacro")]), | ||
| " ", | ||
| PathJoinSubstitution( | ||
| [FindPackageShare("ur_simulation_gz"), "urdf", "ur_gz.urdf.xacro"] | ||
| [FindPackageShare(description_package), "urdf", description_file] |
There was a problem hiding this comment.
That change is not required, it's supposed to be like that.
| "sim_ignition:=true", | ||
| " ", |
There was a problem hiding this comment.
That's not needed anymore.
| "sim_ignition:=true", | |
| " ", |
I deleted my fork. I forgot #16 was not merged yet. I restored the branch with my changes.