Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Branch for kinetic? #58

Closed
jonbinney opened this issue Jul 6, 2016 · 29 comments
Closed

Branch for kinetic? #58

jonbinney opened this issue Jul 6, 2016 · 29 comments

Comments

@jonbinney
Copy link

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: iron-ox@883070d

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

@carlosjoserg
Copy link

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 ros-controls/ros_control@276b478.
  • 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 ros-controls/ros_control@ad68d85). 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 😶

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
Copy link
Author

@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
Copy link

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
Copy link

v4hn commented Dec 16, 2016

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
Copy link
Author

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
Copy link

BenBlumer commented Jan 27, 2017

@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
Copy link

BenBlumer commented Jan 27, 2017

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
Copy link
Member

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

@jkwang1992
Copy link

@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
Copy link

@WangJIankun1992 Well, as far as I could see and test, iron-ox@883070d 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
Copy link
Collaborator

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
Copy link

v4hn commented Feb 14, 2017 via email

@BenBlumer
Copy link

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
Copy link

@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
Copy link

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
Copy link

fbe555 commented Mar 5, 2017

@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
Copy link

jwhendy commented Apr 25, 2017

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
Copy link
Author

@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
Copy link
Collaborator

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
Copy link

zplizzi commented Jun 27, 2017

@ThomasTimm is this still in progress?

@ghost
Copy link

ghost commented Jun 28, 2017

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

mrjogo pushed a commit to mrjogo/ur_modern_driver that referenced this issue Nov 8, 2017
@Geonhee-LEE
Copy link

Geonhee-LEE commented Jan 10, 2018

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
Copy link

@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
Copy link

felixvd commented Apr 23, 2018

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

@v4hn
Copy link

v4hn commented Apr 23, 2018

Hey Felix, long time no see!

I believe this thread is very much outdated by now, but nobody added a comment about that here.
#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
Copy link

felixvd commented Apr 23, 2018

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
Copy link

ilya0ics commented Jul 3, 2018

Hello, any news about kinetic support yet?

@gavanderhoorn
Copy link
Member

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

@gavanderhoorn
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests