Conversation
| allowed_execution_duration_scaling: 1.2 | ||
| # Allow more than the expected execution time before triggering a trajectory cancel (applied after scaling) | ||
| allowed_goal_duration_margin: 0.5 | ||
| # Allowed joint-value tolerance for validation that trajectory's first point matches current robot state |
There was a problem hiding this comment.
It's nice to see some comments! Much needed
| @@ -1,3 +1,15 @@ | |||
| # MoveIt uses this configuration for controller management | |||
There was a problem hiding this comment.
In ROS1, we needed fake_controllers.yaml and ros_controllers.yaml.
In ROS2, fake_controllers.yaml is not needed. There is a flag that gets passed into ros2_control that determines whether it spoofs controllers or not. Here are some links that can help you track down how this use_fake_hardware flag usually works:
From the launch file, it goes to an xacro file:
So I suggest deleting fake_controllers.yaml unless you and Vatan are really changing up the way things work.
| moveit_manage_controllers: True | ||
| moveit_controller_manager: moveit_simple_controller_manager/MoveItSimpleControllerManager |
There was a problem hiding this comment.
What would you think about deleting these 2 lines? They should have no effect on the Fanuc robot. They are VERY rarely needed (never in my career so far).
An example where they are needed would be something like,
- A humanoid robot has 2 arm controllers and 2 leg controllers
- All of the controllers are in an inactive state for some reason
- You send a command to the right leg
- MoveItSimpleControllerManager wakes up the right leg controller
So you can see, it's a pretty contrived case. I'm advocating to delete MoveItSimpleControllerManager completely
|
You're giving me credit for understanding more than I actually do. Mostly, in this PR I'm just moving the parameters I see in the launch files to the yaml. |
| allowed_start_tolerance: 0.01 | ||
|
|
||
| # Simulation settings for using moveit_sim_controllers | ||
| moveit_sim_hw_interface: |
There was a problem hiding this comment.
I'm not sure what these parameters do either.
There was a problem hiding this comment.
I'm not sure. Prob could delete them, I think. Please try it, then I'll approve the PR either way
There was a problem hiding this comment.
I tend to try to leave as much of the code alone if I don't understand it. I'd rather they be left here than to remove them and then have to put them back in. Maybe someone with more ros control knowledge can weigh in.
There was a problem hiding this comment.
That would probably be @davetcoleman himself. I think this is for the SimpleControllerManager, not ros2_control itself.
I'm of the opposite mindset. If there is zero effect on the robot, remove the parameters. Otherwise you are going to have lots of people copy/paste these parameters in their own setups when it only adds complexity.
There was a problem hiding this comment.
@JafarAbdi didn't you port this stuff to ROS2? Would you agree that these moveit_sim_controllers are not needed in ROS2 since we have the ros2_control fake controllers?
|
Oops, I did not see #120 when I made this. CC: @stephanie-eng @stephanie-eng @AndyZe @JafarAbdi Do you have a strong reason for naming the controller files? I like keeping it simple with |
|
So I'd be cool with dropping the robot name, for sure |
|
It makes sense that they would be different, but that's news to me. Which should be loaded to get |
|
(specifically this method) |
|
That would be moveit_controllers.yaml |
|
Closing in favor of #120 |
controller_listparadigm to thecontroller_namesversion.Partial block for moveit/moveit2#1131