Improve robotic_manipulation_node stability #8
Conversation
Signed-off-by: Kacper Dąbrowski <[email protected]>
Signed-off-by: Kacper Dąbrowski <[email protected]>
Signed-off-by: Kacper Dąbrowski <[email protected]>
|
Tested on Ubuntu 22 + humble, branch -> https://github.com/RobotecAI/rai/tree/jm/feat/o3de-bench-more-tasks I didn't see weird, ineffective movements of the arm, which was the case in some scenarios earlier. It could be the infinite loop you mentioned above. Service Overall works fine for me! |
There was a problem hiding this comment.
I do not like the blocking call in the constructor, but the code does the job. Since it is a minimal codebase, I could live with it.
The whole workspace should get some attention: I would suggest at least adding a license note in every file and clang-format the files.
ros2_ws/src/robotic_manipulation/include/robotic_manipulation/arm_controller.h
Show resolved
Hide resolved
| @@ -19,10 +19,12 @@ class ArmController { | |||
| bool GetGripper(); | |||
|
|
|||
| std::vector<double> CaptureJointValues(); | |||
| void SetJointValues(std::vector<double> const &jointValues); | |||
| bool SetJointValues(std::vector<double> const &jointValues); | |||
There was a problem hiding this comment.
Out of the scope of this change: some consistency with const T & vs. T const & would be an added value.
There was a problem hiding this comment.
Please see my latest comment
| ArmController::ArmController() { | ||
| m_node = rclcpp::Node::make_shared("arm_controller"); | ||
| m_node->set_parameter(rclcpp::Parameter("use_sim_time", true)); | ||
|
|
||
| WaitForClockMessage(); |
There was a problem hiding this comment.
Not sure if a constructor is the best place to put a blocking call.
Signed-off-by: Kacper Dąbrowski <[email protected]>
|
Thanks for the review @jhanca-robotecai! I created a separate PR to this branch containing the suggested changes and a few more to generally improve the code quality of the package: #11. If you have any other suggestions, please leave comments in that PR. |
This PR:
improves the stability of the robotic_manipulation_node by waiting for the simulation to open before making any requests to moveit actions, in case the node is launched before the simulation.
updates the MoveThroughWaypoints method not to fall into an infinite loop if moveit decides not to work correctly, but rather abandon making a move after a chosen number of tries (currently 10).
adds a
/reset_manipulatorservice of typestd_srvs/srv/Triggerthat allows to reset all the manipulator joints back into the starting positions (useful when after making a few moves the manipulators ends in an unnatural positions and the chance of moveit failing rises)