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

Add support for external axes on the ABB driver #150

Conversation

gonzalocasas
Copy link
Contributor

@gonzalocasas gonzalocasas commented May 8, 2018

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.

  • Motion server now takes into account all 4 additional joints present in the message and assigned them to eax_a, eax_b, eax_c and eax_d.
  • Status server sends all connected external axes. Non-connected are zero filled.

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 #145

* Motion server now takes into account all 4 additional joints present in the message and assigned them to `eax_a`, `eax_b`, `eax_c` and `eax_d`.
* Status server sends all connected external axes. Non-connected are zero filled.
@gavanderhoorn
Copy link
Member

gavanderhoorn commented May 8, 2018

Thanks for the PR, much appreciated 👍 🎉

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

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

@@ -54,6 +54,7 @@ PROC main()
IF (trajectory_size > 0) THEN
FOR current_index FROM 1 TO trajectory_size DO
target.robax := trajectory{current_index}.joint_pos;
target.extax := trajectory{current_index}.extax_pos;

skip_move := (current_index = 1) AND is_near(target.robax, 0.1);
Copy link
Member

@gavanderhoorn gavanderhoorn May 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this also not need to check the pose(s) of the external axes? Otherwise we could run the risk of skipping points that satisfy is_near(..) for the robot itself, but not for the ext axes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed it on 3b1d413

@gonzalocasas
Copy link
Contributor Author

gonzalocasas commented May 8, 2018

Fixed unit conversion on 289fc4b

@gonzalocasas
Copy link
Contributor Author

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

@gavanderhoorn
Copy link
Member

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

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

@gavanderhoorn
Copy link
Member

Using metres now, I hope? :)

@gonzalocasas
Copy link
Contributor Author

Yes, indeed. 😉

@gavanderhoorn
Copy link
Member

@gonzalocasas: my apologies for letting this slip.

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

Copy link
Member

@gavanderhoorn gavanderhoorn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to work as described.

Tested in RobotStudio with an IRB-1600 on a track.

@gavanderhoorn gavanderhoorn merged commit a0621d6 into ros-industrial:kinetic-devel May 23, 2018
@gavanderhoorn
Copy link
Member

gavanderhoorn commented May 23, 2018

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

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

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

@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.

gavanderhoorn pushed a commit to ros-industrial/abb_driver that referenced this pull request Apr 25, 2020
Squashed commits:

* Add support for external axes on the ABB driver
* Motion server now takes into account all 4 additional joints present in the message and assigned them to `eax_a`, `eax_b`, `eax_c` and `eax_d`.
* Status server sends all connected external axes. Non-connected are zero filled.
* Convert to and from ROS and ABB units (meters and millimeters respectively)
* Include eax in `is_near` check
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants