ur_modern_driver refactor

[Zagitta]

Hello Thomas, here's the final code for the complete refactor as we've previously discussed.
During the 11/7 ROS Industrial meeting I asked about their preferred way to handle the code adoption and they greatly preferred if the changes could be merge into this repository to avoid having to redirect people elsewhere and cause more confusion than the old ur_driver already provides.

If you don't have time to handle the merging and issues that'll most likely crop up when people start using the new code you can add me as maintainer to the repository and I will do my best to handle it over the next month or two until the adoption ROS Industrial procedure has completed.

To get everyone else up to speed here's a rundown of everything that has happened in this rewrite:

• Implemented support for Kinetic (Indigo naturally still works, Jade is untested) Branch for kinetic? #58 Fails to compile under Jade due to changed controller_manager_msgs structure #82
• Adopted clang-format with ROS specific style file
• Fixed issues ur3_ros_control.launch wrong controller names #98 and Less copy paste  #88
• Implemented RT digital IO Expose digital IO from realtime interface #50
• Implemented joint temperature Publishing Joint Temperature and information #81
• Implemented parsing of tool analog values Are the tool analog inputs available?  #83, still needs publishing.
• Implemented an enable robot service at /ur_driver/robot_enable that must be called on startup before the driver will accept commands. It must also be called if the robot ever enters a protective or emergency stop mode to ensure controllers acknowledge the stop mode.
• Added unit testing for message parsing (more work to be done here)

Besides the general list of changes above the driver now has a more sensible software design in the form of 2 producer/consumer work queues called pipelines, one for real time messages and one for the rest.
Each pipeline consists of a producer that received data from the robot, parses it and dumps the parsed messages into the queue which a consumer then picks up and runs through the various sub-consumers that publishes the messages.

The code has so far only been tested on a live UR5 v1.8, but has been developed against URSim v3.3.4 and should therefore work but it remains to be seen.

Last but not least I believe most of the blockers for #18 to happen have been resolved, the primary thing lacking now is better documentation but I'll try to get that done after my thesis defense.

Best regards, Simon R.

[dniewinski]

Just an FYI: Just tested this on a UR5 with 3.5.4.10845 (Apr 13 2018) and Kinetic with MoveIt! and didn't notice any issues.

[shaun-edwards]

We are actively using the refactored branch and have exercised the following functionality without any issue:

• Joint states
• IO
• Action based motion

+1 for merging these changes.

[gavanderhoorn]

As an update: as part of the repository maintenance activities that will be initiated next month (September), this PR will be merged into this repository.

If anyone has any serious objections to this, now would be the time to make them heard.

We will not request additional changes from the submitter (@Zagitta), so enhancements or small fixups will be done after the initial merge.

[gavanderhoorn]

Finally got this merged. 🎆

Thanks @Zagitta for all the work and thanks to the many reviewers and users of this PR for their feedback.

👍

As I've written in earlier comments and on other PRs: there have been various reasons for why some PRs have been open for a rather long time and I do apologise for that. Even though it may seem that way, this is not indicative of disinterest in the feature or fix.

[v4hn]

Thank you so much for your time and commitment @gavanderhoorn !
Happy ROSCon 🥇

[gonzalocasas]

OMG! This is such an amazing milestone! Thanks so much, @Zagitta & @gavanderhoorn!

[ahundt]

This is awesome, thanks! 👍

[AndyZe]

I'm just noticing, the /joint_speed topic is gone from this refactored version. I very much liked that topic for streaming poses to the robot in real time. Would it be difficult to re-enable /joint_speed?

I know /ur_script can also handle streaming in real time, but it's not compatible with other robots.

[miguelprada]

I never used the /joint_speed topic myself, but I believe you should achieve similar functionality by properly configuring a velocity_controllers/JointGroupVelocityController. This should be even more compatible with other robots than the /joint_speed topic.

[gavanderhoorn]

@miguelprada: provided those "other robots" also have a ros_control compatible hardware_interface of course :)

@AndyZe: the omission of the joint_speed topic seems to have been unintentional. Adding it back in shouldn't be too difficult, we'd basically have to implement something similar to how the URScript topic is supported.

Something like this.

[AndyZe]

@miguelprada A ros_control question for you. (Maybe we can move the discussion to email or answers.ros.org? I can't do google groups).

Is there a fundamental reason why I can't stream JointTrajectory messages to the default ros_control type, velocity_controllers/JointTrajectoryController? I attempted this earlier today, and got the following error:

[ WARN] [1539981427.410135432]: Dropping first 1 trajectory point(s) out of 30, as they occur before the current time.
First valid point will be reached in 0.002s.
[ERROR] [1539981427.411941084]: Unexpected error: No trajectory defined at current time. Please contact the package maintainer.

I can understand why some points were skipped, and I can overcome that issue easily. But I don't understand the error related to "no trajectory defined at current time."

I would like to avoid switching back and forth between different ros_control types because I need to plan trajectories as well as stream commands. (And setting up a new control type has been hard.)

[gavanderhoorn]

@AndyZe: I'm not @miguelprada, but: the JointTrajectory action interface accepts JointTrajectory goals. Those are inherently position based, but do support velocity and acceleration constraints.

That interface is not meant for streaming velocities, and that is why you get that error (the action server sees a trajectory coming in with no values for the position field).

[gavanderhoorn]

I would like to avoid switching back and forth between different ros_control types because I need to plan trajectories as well as stream commands. (And setting up a new control type has been hard.)

I don't see a way around this: ros_control supports controller switching and that would seem to allow you to do precisely what you want: when you need to stream velocities, switch to the controller @miguelprada suggested. For trajectory execution, switch to the joint_trajectory_controller.

[miguelprada]

@gavanderhoorn beat me to it 😉

I believe you are probably trying to send joint trajectories following the pattern that the previous joint_speed topic was expecting, i.e. a single JointTrajectoryPoint with only velocity/acceleration setpoints. This will never work with the joint_trajectory_controller which, as stated, is inherently position based.

That's irrespective of which underlying joint interface is used by the controller to track your trajectory, e.g. in velocity_controllers/JointTrajectoryController joint velocity commands are used internally by the controller to track your position trajectory.

You can use joint_trajectory_controller to stream positions (single point trajectories), and that's precisely what rqt_joint_trajectory_controller does, but even this is kind of abusing the controller somehow.

[AndyZe]

Alright, thanks guys. Actually my JointTrajectoryPoint did include a position but I guess a velocity_controllers/JointTrajectoryController isn't suitable for single-point streaming, anyway (as miguelprada alluded to). More in this github issue.

[miguelprada]

Alright, thanks guys. Actually my JointTrajectoryPoint did include a position but I guess a velocity_controllers/JointTrajectoryController isn't suitable for single-point streaming, anyway (as miguelprada alluded to). More in this github issue.

I never tried to imply that the velocity_controllers/JointTrajectoryController is ill-suited for single-point streaming. I often use velocity_controllers/JointTrajectoryController together with rqt_joint_trajectory_controller to jog the robot around, and if I'm not wrong this is effectively streaming single trajectory points.

How are you exactly filling your trajectory message? In particular, how are you timestamping the message and each of the points (i.e. the time_from_start field)? I would suggest you play around with rqt_joint_trajectory_controller and try to figure out why this works and your code doesn't.

[gavanderhoorn]

@AndyZe @miguelprada: could we create a new issue for discussing the velocity streaming approaches with this driver? I welcome the discussion, but let's not have it in the comment section of an already merged PR.