Branch for kinetic? - Githubissues

jonbinney commented 8 years ago

hardware_interface has some ABI breaking changes between indigo and kinetic. I've updated ur_modern_driver to work with them in a branch in our fork: https://github.com/iron-ox/ur_modern_driver/commit/883070d0b6c0c32b78bb1ca7155b8f3a1ead416c

Those changes aren't backwards compatible with indigo though. If a new kinetic-devel branch is created, I can submit a PR.

carlosjoserg commented 8 years ago

Hi @jonbinney, have you been able to actually move a robot with this change on Kinetic or you only suggest a compilation error fix?

I'm on 16.04/Kinetic, Polyscope 3.2.x, an UR3 robot that I used to move with this driver (with use_ros_control=True) on 14.04/Indigo same Polyscope. I found other things before being able to move the robot again by running roslaunch ur_modern_driver ur3_ros_control.launch robot_ip:=MY.REAL.IP.ADDRESS, and still half-luck:

In your commit, you didn't upgraded the deprecated canSwitch() for the prepareSwitch(), see https://github.com/ros-controls/ros_control/commit/276b478820f51b3d758a25233b7494506699d3cc.
Now, it seems to be the need to have an interface running by default here, otherwise both remain false always (not really sure, it might be due to https://github.com/ros-controls/ros_control/commit/ad68d85af8714a4309c10121c2601d0407402d20). However, with position_interface_running_=true by default, a weird thing happened, keep reading.
I don't remember seeing it on Inidgo, but now this produces a big red warning in Kinetic as the arg tag is not recognized within the node tag.
Warnings from URDFs, roslaunch, xacro, etc, like everything needs to be prepended with xacro, pi is already a global property so no need to add it, --inorder is to be added when loading the robot_description, but these are more related to the ros industrial repo that has the descriptions.

After all these changes, I still couldn't move the robot. Now the weird thing:

I'm using the position interface, so servoj() is used to write commands here. There is a check for the reverse port here which I never pass (the network config is the same as before). So I went to see where this flag is set to true and it seems to be only in the openServo() here. This function seems to be only called at the end of the uploadProg() here, which it is called at the beginning of doTraj() with an if-check here. So, after trying several things, I came with a crazy one: add a call of uploadProg() by the end of start() before the return here, so the reverse_connected_ can take the value of true at least once. Result: The robot makes a big weird aggressive jump initially, I guess going to the setpoint defined in the script here as the communication gets there before the ros control loop runs, but then... the position-based joint trajectory controller works like a charm :no_mouth:

I don't see what else can be happening to avoid the initial jump, and ran out of ideas... any clue, could it be something independent of the ROS-distro that I'm missing?

jonbinney commented 8 years ago

@carlosjoserg I have been using this on kinetic on ubuntu 16.04 server to control a UR10. As you point out, there are other changes that will need to be made for kinetic. My setup is a bit different than yours:

i have a UR10 instead of a UR3
i'm launching the driver node directly from my launchfile

Here are the settings I'm launching the node with:

  <arg name="robot_ip" default="192.168.42.100"/>
  <arg name="min_payload"  default="0.0"/>
  <arg name="max_payload"  default="10.0"/>
  <arg name="servoj_time" default="0.008" />
  <arg name="base_frame" default="base" />
  <arg name="tool_frame" default="tool0_controller" />

  <arg name="max_velocity" default="10.0"/> <!-- [rad/s] -->

  <node unless="$(arg sim)" name="ur_driver" pkg="ur_modern_driver" type="ur_driver" output="screen">
    <remap from="follow_joint_trajectory" to="arm_controller/follow_joint_trajectory" />
    <param name="prefix" type="str" value="" />
    <param name="robot_ip_address" type="str" value="$(arg robot_ip)" />
    <param name="min_payload" type="double" value="$(arg min_payload)" />
    <param name="max_payload" type="double" value="$(arg max_payload)" />
    <param name="max_velocity" type="double" value="$(arg max_velocity)" />
    <param name="servoj_time" type="double" value="$(arg servoj_time)" />
    <param name="base_frame" type="str" value="$(arg base_frame)"/>
    <param name="tool_frame" type="str" value="$(arg tool_frame)"/>
  </node>

It's also possible that there are new problems if other pacakages have been updated recently; i don't think i've updated the ROS packages on the robot's computer in the last week. I'm not sure why my setup works, but you have to add the uploadprog() call. Actually I didn't even look further into the code; after I fixed the few things in that one commit and got it to compile, the robot started working.

@ThomasTimm it looks like kinetic-specific code fixes are needed. Can you create a kinetic-devel branch so that we can start contributing fixes?

carlosjoserg commented 8 years ago

Thanks @jonbinney , however, I don't see too much differences in why it didn't work here w.r.t. to your setup either, and sincerely, I don't know how it is working for you, since I finally got to the issue...and please, forget all dirty stuff I mentioned above that resulted in a weird behavior.

Let me explain the issue using your commit as example. In your commit, you do the following diff:

-       if (controller_it->hardware_interface
+        
+        if (controller_it->type

But controller_it->type does not return the the hardware interface type of the resources, but the controller type, i.e. something like position_controllers/JointTrajectoryController, and that's the reason why the switch wasn't taking place at starting. So I was wrong, both position_interface_running_ and velocity_interface_running_ are ok to be false at init here.

To actually get to the resource hardware interface type, you now need to do something like: controller_it->claimed_resources.at(0).hardware_interface, but that zero there does not look good, but I don't see any other way to check that. However, like that works like before on Indigo. I'll be thinking how to check that properly with the new changes in kinetic, when I have something decent, I'll prepare a PR if wanted.

Best

v4hn commented 7 years ago

What is the current state for kinetic support in ur_modern_driver? Our lab slowly prepares to move to kinetic, but ur_modern_driver (among others) does not officially support kinetic yet...

jonbinney commented 7 years ago

I'm using it on kinetic with a UR10, and I'm able to execute trajectories without any noticeable problems using the fork i mentioned in my above comment. I don't think the fixes have been merged in (no kinetic branch to put them in as noted in this issue).

BenBlumer commented 7 years ago

@ThomasTimm could you please create a branch for Kinetic? No need to write the code yourself -- the rest of the community can contribute. We just need a place to contribute to!

(Or if you're comfortable giving privileges, I'm happy to create and maintain the branch myself.)

BenBlumer commented 7 years ago

Also -- I can confirm that @carlosjoserg solution of swapping every instance of ->hardware_interface for ->claimed_resources.at(0).hardware_interface in ur_modern_driver/src/ur_hardware_interface.cpp allows the package to build in Kinetic.

BrettHemes commented 7 years ago

+1, @ThomasTimm can we get a new branch?

jkwang1992 commented 7 years ago

@carlosjoserg I have been successfully using UR3 robot in Ubuntu 16.04/Kinetic, Polyscope 3.2.x according to the suggestion from @jonbinney . @ThomasTimm If there is a new branch, maybe I can contribute something useful.

carlosjoserg commented 7 years ago

@WangJIankun1992 Well, as far as I could see and test, https://github.com/iron-ox/ur_modern_driver/commit/883070d0b6c0c32b78bb1ca7155b8f3a1ead416c didn't work... have you tried switching controllers back and forth?

@ThomasTimm A way to deal with different versions, ROS-wise and temporarily, without creating new branches is to use ROS_VERSION macros to create a code branch, as it was done here for the Kuka LWR (and more details in the answer here ). Since the non-ROS side of the driver looks very standard to me, that might be work.

If you think that would be ok, I can prepare a PR this week with that approach. Any suggestion is welcome though.

ThomasTimm commented 7 years ago

We are currently working on re-structuring the entire code in a separate branch, so that is why I am not doing much maintenance at the moment.

If you still want a kinetic version, knowing that most of the code will soon be changed, then you are welcome to prepare a PR @carlosjoserg

v4hn commented 7 years ago

Thank you for responding! Great to hear you're working on improvements! Do you plan to include new features?

If you still want a kinetic version, knowing that most of the code will soon be changed, then you are welcome to prepare a PR

It would be great to see this as a short-time solution!

BenBlumer commented 7 years ago

Thanks for the heads up, Thomas.

It sounds like @carlosjoserg has a good temporary solution. The proposed change to make it Kinetic-compatible might even be helpful to you guys in your new code base.

jkwang1992 commented 7 years ago

@carlosjoserg I use the universal_robot-kinetic-devel and replace the ur_driver with ur_modern_driver, then I change the ur_hardware_interface.cpp, specifically change controller_it->hardware_interface to controller_it->type. Finally, it works well.

carlosjoserg commented 7 years ago

If you still want a kinetic version, knowing that most of the code will soon be changed, then you are welcome to prepare a PR @carlosjoserg

Ok @ThomasTimm ... Then I think we can wait for that to happen (at least me), thanks!

fbe555 commented 7 years ago

@jonbinney & @ThomasTimm I'm trying to get moveit to work with ur_modern_driver on ubuntu 16.04 and ROS kinetic. I have had the robot working with moveit under indigo.

My issue is that the controller fails right away (with no arm movement what so ever) when executing giving the following warnings and error:

[ WARN] [1488729387.911238138]: Dropping first 1 trajectory point(s) out of 17, as they occur before the current time.
First valid point will be reached in 0.245s.
[ERROR] [1488729388.383042428]: Path tolerances failed for joint: shoulder_pan_joint
[ERROR] [1488729388.383145943]: Path state tolerances failed:
[ERROR] [1488729388.383226899]: Position Error: 0.10378 Position Tolerance: 0.1
[ WARN] [1488729388.414463490]: Controller /vel_based_pos_traj_controller failed with error code PATH_TOLERANCE_VIOLATED
[ WARN] [1488729388.414889256]: Controller handle /vel_based_pos_traj_controller reports status ABORTED
[ INFO] [1488729388.527062063]: ABORTED: Solution found but controller failed during execution

If i increase the path position tolerances, the same message will occur with a value slighty above the increased tolerance. I might be misinterpreting, but it seems like the controller is failing because the driver takes the commands, doesn't move the arm, resulting in the posistion tolerance being violated.

Have you seen anything similar, and do you have any clues on how I can proceed?

jwhendy commented 7 years ago

As a ROS noob, I'm at the mercy of the experts who know what's going on here, but it would be awesome to know a ballpark on the path forward. This driver is the only thing forcing a dual indigo/kinetic setup across a number of machines.

I read "soon" re. restructuring and filled in "any day" in my mind; that was 2.5mos ago. If there are fixes from the community that work, my vote is a temporary branch to have something usable vs. waiting for the polished code release.

jonbinney commented 7 years ago

@jwhendy the current status seems to be that some people, including myself, have hacked ur_modern_driver to work on kinetic from source. I've been using the the iron-kinetic-devel branch from our fork successfully since july: https://github.com/iron-ox/ur_modern_driver/tree/iron-kinetic-devel

As others have pointed out, my fix doesn't work for everyone (reading comments above it sounds like others may have made better fixes). Since it has worked for me, I haven't touched this in quite a while.

Meanwhile I am also curious - @ThomasTimm any update on the restructuring that you are working on?

ThomasTimm commented 7 years ago

It is a student of mine that is working on the refactoring - you can follow the progress on https://github.com/Zagitta/ur_modern_driver/tree/refactor I was told he would have it working during the last weekend, but he stills have some issues (for instance, implementing the action server), but hopefully it will done in a week or two. Then I would really appreciate help testing it out.

zplizzi commented 7 years ago

@ThomasTimm is this still in progress?

ghost commented 7 years ago

@ThomasTimm I'm also running into the sam issue using ur_modern with kinetic and wondering if this is still in progress?

Geonhee-LEE commented 6 years ago

Hello. when I run on UR5 throughout below process, I additionally have a new problem "can't locate [ur_driver] in package [ur_modern_driver].. anybody is working well?

thanks.

jkwang1992 commented 6 years ago

@Geonee Following commands may help you: mkdir -p /tmp/ws/src
cd /tmp/ws/src
git clone https://github.com/ros-industrial/universal_robot.git
cd /tmp/ws
rosdep update
rosdep install --from-paths src --ignore-src
catkin_make
source devel/setup.bash
You need to replace the ur_driver with ur_modern_driver, and change the ur_hardware_interface.cpp, specifically change controller_it->hardware_interface to controller_it->type. Finally, it works well.

felixvd commented 6 years ago

Reconfirming the comment from @BenBlumer. Can we get a kinetic-dev branch to push that simple change to, @ThomasTimm ?

v4hn commented 6 years ago

Hey Felix, long time no see!

I believe this thread is very much outdated by now, but nobody added a comment about that here. https://github.com/ThomasTimm/ur_modern_driver/pull/120 by @Zagitta is the branch that works with ROS kinetic (multiple groups use/d it by now), but it is still not merged upstream. From my understanding, @ThomasTimm wants to see it merged back to ros-industrial/universal_robot so that the name "ur_modern_driver" is no longer needed, but the ROS-I crew, in particular @gavanderhoorn, disagrees with the politics of UR and could not be convinced to merge it yet. See here for a heated discussion... By now, @potiuk also wrote a very nice new "low bandwidth controller" on top of Zagitta's work. It's not perfect yet, but it can also work nicely for many use cases.

Back when kinetic was just released, the whole situation was a bit funny. With ROS melodic incoming though, it is simply ridiculous...

felixvd commented 6 years ago

Cheers, thanks for the summary :) I just found this thread via Google, but now that the PR is referenced, I guess it could be closed.

ilya0ics commented 6 years ago

Hello, any news about kinetic support yet?

gavanderhoorn commented 6 years ago

We now have a kinetic-devel, which also includes the work by @Zagitta.

gavanderhoorn commented 6 years ago

Thanks @jonbinney for starting the discussion and everyone else for contributing to it.

ros-industrial-attic / ur_modern_driver

Branch for kinetic? #58