search

Kawasaki-Robotics / khi_robot

ROS KHI robot meta-package
http://wiki.ros.org/khi_robot
BSD 3-Clause "New" or "Revised" License
54 stars 28 forks source link

Potential race condition in khi_robot_control? #69

Closed Yuki-cpp closed 2 years ago

Yuki-cpp commented 2 years ago

Hello,

When using the khi_robot driver we noticed that sometimes running rs013n_bringup.launch would fail requesting some controllers to be registered in the hardware_interface::RobotHW class.

After a bit of digging, I found the potential cause for that behavior here: https://github.com/Kawasaki-Robotics/khi_robot/blob/9d916ea93cea35c511a18aa22dd85d31a8be8746/khi_robot_control/src/main.cpp#L329

It appears that when the controller manager is created, some services are advertised. Those services require the controllers to be loaded in the hardware interface. However, some controllers are only loaded here, a couple of lines later: https://github.com/Kawasaki-Robotics/khi_robot/blob/9d916ea93cea35c511a18aa22dd85d31a8be8746/khi_robot_control/src/main.cpp#L331

If one of the mentioned services is called before we call robot.open I would expect the behavior we're encountering. A fix for that would be to create the controller manager after calling robot.open which would ensure that the controllers are always loaded when needed. I implemented that locally and it seems to work fine.

That being said, I am not very familiar with this driver code-base and might have missed something important. Is my assertion of the situation accurate? Am I missing anything here? If so, I'm more than happy to provide a PR to fix that.

Thank you

matsui-hiro commented 2 years ago

Hi, @Yuki-cpp Thank you for investigating the problem.

That being said, I am not very familiar with this driver code-base and might have missed something important. Is my assertion of the >situation accurate? Am I missing anything here?

I think there is no problem even if the call order is changed. If you don't mind, could you make PR of your local fix?

Yuki-cpp commented 2 years ago

Thank you @matsui-hiro. I openned the PR, let me know if you have any change/request about it.