xArm-Developer / xarm_ros

ROS packages for robotic products from UFACTORY
https://www.ufactory.cc/
BSD 3-Clause "New" or "Revised" License
196 stars 147 forks source link

xarm_driver publishes joint_states even though joint state controller should instead #185

Closed BryanStuurman closed 1 year ago

BryanStuurman commented 1 year ago

xarm_api/src/xarm_driver.cpp creates a publisher for the topic "joint_states" on line 236: https://github.com/xArm-Developer/xarm_ros/blob/master/xarm_api/src/xarm_driver.cpp#L236 However this functionality is preferentially provided by the ros joint_state_controller, and solutions that use that (I note xarm_ros no longer starts it) end up with colliding messages, which resulted in awful velocity data due to some sample rate mismatches ( some of that can be seen here: https://github.com/xArm-Developer/xarm_ros/issues/96)

Additionally I've recently updated to firmware 2.01 on one robot and i see that the velocity data from xarm_hw.read() is now discretized at 10Hz instead of 100Hz regardless of the read back rate. Can the old faster rate be had somehow? seems like i spoke too soon, I can see smooth data.

BryanStuurman commented 1 year ago

Here's an example of the difference in functionality when the joint_state_controller is reactivated, patched (https://github.com/ros-controls/ros_controllers/commit/d14d9023e899d7103953d275e207829e60fec8e4) and the built in joint_states publisher is moved to "joint_states2":

Trajectory velocity

Screenshot from 2023-05-29 21-54-55

Slow manual mode motion

Screenshot from 2023-05-29 21-58-09

Fast jerks in manual mode

Screenshot from 2023-05-29 21-58-58

penglongxiang commented 1 year ago

Hi @BryanStuurman, thanks for the feedback. To make it clear, you are suggesting on re-activating the joint_state_controller to publish "joint_states" topic and remove the same topic publisher in xarm_driver node because of its low resolution (5Hz by default setting) and delay, is that right? We will take a look into this issue and reply to you later.

BryanStuurman commented 1 year ago

Yes, that is my suggestion. Low resolution and delay are contributing factors, and because this driver is for use with ros_control, and the joint_state_controller is the expected interface that is used for it. Like other controllers it means that all of the expected features are available, like proper namespaces, management with the controller manager interface, and the ability to load custom variants.

penglongxiang commented 1 year ago

Hi @BryanStuurman we have made the changes as you suggested, please update the repository and see if it behaves as expected.

BryanStuurman commented 1 year ago

This is looking good.