Skip to content

Add MoveIt Servo Tutorial - #688

Open
tahsinkose wants to merge 10 commits into
ros-industrial:noetic-develfrom
tahsinkose:tahsin/add-moveit-servo-tutorial
Open

Add MoveIt Servo Tutorial#688
tahsinkose wants to merge 10 commits into
ros-industrial:noetic-develfrom
tahsinkose:tahsin/add-moveit-servo-tutorial

Conversation

@tahsinkose

Copy link
Copy Markdown

This PR adds two usage examples of MoveIt Servo with UR5 robot in the new ur5_moveit_tutorials package

@tahsinkose

Copy link
Copy Markdown
Author

@RobertWilbrandt @fmauch could you please have a look at this PR?

@AndyZe AndyZe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It would have been very nice to have this a few years ago. Personally I'd still like to see it merged after the changes get applied. I guess most of the community's efforts go towards ROS2 now, for better or worse...

Comment thread ur5_moveit_tutorials/CMakeLists.txt Outdated
Comment on lines +6 to +9
catkin_package(
)
include_directories(
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
catkin_package(
)
include_directories(
)
catkin_package()

<!-- Load and start the controllers listed in the 'controllers' arg. -->
<node name="ros_control_controller_spawner" pkg="controller_manager" type="spawner"
args="$(arg controllers)" output="screen" respawn="false" />
<group unless="$(eval arg('controllers') == '')">

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think there's any reason for group if there's just one node in the group? Suggest reverting.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I should actually have used if clause instead of this 😄

@tahsinkose
tahsinkose force-pushed the tahsin/add-moveit-servo-tutorial branch from d0913a8 to 1e65d66 Compare November 2, 2024 13:54
@tahsinkose

tahsinkose commented Nov 2, 2024

Copy link
Copy Markdown
Author

@AndyZe addressed your review, PTAL 🙏🏼

It would have been very nice to have this a few years ago. Personally I'd still like to see it merged after the changes get applied. I guess most of the community's efforts go towards ROS2 now, for better or worse...

I can migrate them to ROS 2 when I finally switch to ROS 2 🙈

@tahsinkose

Copy link
Copy Markdown
Author

@AndyZe friendly ping 🙂

@AndyZe

AndyZe commented Nov 8, 2024

Copy link
Copy Markdown
Contributor

I don't have merge power here

@tahsinkose

Copy link
Copy Markdown
Author

I don't have merge power here

Who should we reach then? Do you know that @AndyZe?

@tahsinkose

Copy link
Copy Markdown
Author

@urfeex could you review this PR and possibly merge?

@urfeex

urfeex commented Nov 3, 2025

Copy link
Copy Markdown
Collaborator

I'm afraid, our resources towards ROS 1 are rather limited at the moment.

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