ros-industrial / abb

ROS-Industrial ABB support (http://wiki.ros.org/abb)
145 stars 152 forks source link

Add support for external axes on the ABB driver #150

Closed gonzalocasas closed 6 years ago

gonzalocasas commented 6 years ago

As discussed on https://github.com/ros-industrial/abb/issues/135, here's a proposal for integrated external axes support on the ABB driver. I tested on IRB4600 real hardware on a linear track.

I explicitly did not change tabs to spaces in places where it would have been logical to do so (e.g. ROS_common.sys) to avoid merge conflicts with https://github.com/ros-industrial/abb/pull/145

gavanderhoorn commented 6 years ago

Thanks for the PR, much appreciated :+1: :tada:

Quick question: is there a conversion from mm to m missing (in both directions)? ROS requires linear joints to report their state in metres. I believe extjoint stores positions in millimeters.

gonzalocasas commented 6 years ago

@gavanderhoorn you're totally right, one sec...

gonzalocasas commented 6 years ago

Fixed unit conversion on https://github.com/ros-industrial/abb/pull/150/commits/289fc4b7b658180d57cdc06e19bd45902e5b671a

gonzalocasas commented 6 years ago

@gavanderhoorn thanks for checking this! hope it's better now 😉

gavanderhoorn commented 6 years ago

@gonzalocasas: thanks for iterating.

You wrote you've tested this on your hw. Did you only look at the joint_state topic, or did you command motion as well?

gonzalocasas commented 6 years ago

Both. I was sending trajectories using this little roslibpy library that we released recently so I was testing the full loop.

gavanderhoorn commented 6 years ago

Using metres now, I hope? :)

gonzalocasas commented 6 years ago

Yes, indeed. 😉

gavanderhoorn commented 6 years ago

@gonzalocasas: my apologies for letting this slip.

I'll confirm this implementation tomorrow in RobotStudio and merge afterwards.

gavanderhoorn commented 6 years ago

Thanks again for contributing this @gonzalocasas 👍

much appreciated.

Could I ask you to add a Features section (after the Overview section) to wiki/abb_driver describing this new support? I'll add other things we might want to list (such as basic 6 axis robot support, agnostic of actual robot model configured, etc).

gavanderhoorn commented 6 years ago

One thing I forgot to ask/mention: this PR assumes that external axes are all linear units. Afaik, that is not necessarily true. I'm not sure how to determine at runtime whether an axis is a linear unit or a rotational one, but that would seem to be required here.

gonzalocasas commented 6 years ago

Could I ask you to add a Features section (after the Overview section) to wiki/abb_driver describing this new support?

@gavanderhoorn thx for adding that. I thought I edited already, but I think I left the tab open in preview, silly me. Anyway, I added one bit to that. And maybe a triviality, but instead of saying a basic 6 axis robot maybe we can go for robots with up to 6 revolute joints, since I think this will work without changes on a SCARA 4-axis robot as well (I will give that a try in the short term).

gavanderhoorn commented 6 years ago

@gonzalocasas wrote:

And maybe a triviality, but instead of saying a basic 6 axis robot maybe we can go for robots with up to 6 revolute joints, since I think this will work without changes on a SCARA 4-axis robot as well (I will give that a try in the short term).

Yes, that may be too limited a description. I just wanted to add a statement about what is at least definitely supported.

If you can verify support for 4-axis robots (and I actually expect that to work right away), please update that bullet point.

Anyway, I added one bit to that.

Thanks.

I've changed the text around a bit more again. Mostly order of things.