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

Initial commit of abb_irb2600 support #24

Merged

Conversation

Jmeyer1292
Copy link
Contributor

This PR adds an implementation of the ABB IRB2600. I'm not entirely sure which arm length, I need to confirm.

Addressing points from @gavanderhoorn's previous critique:

  1. The urdf and meshes were created with the Solidworks exporter, though I didn't do it. I'll look into the local joint transformation issues, but the over all robot works.
  2. I don't know about the 'base' transformation, or even why it exists. Seems like it should be at 0,0,0. @Levi-Armstrong Do you know why this is where it is?
  3. I agree on the precise model comment.

@gavanderhoorn
Copy link
Member

Moved here from ros-industrial/abb#87.

@gavanderhoorn
Copy link
Member

@Jmeyer1292: will you update the PR after you've confirmed the specific variant this is? In light of ros-industrial/abb#47, I feel we shouldn't accept any more support pkgs without the proper naming in place. Otherwise we'd have to update 'all' packages to use the correct names later.

@gavanderhoorn
Copy link
Member

Regarding the base transform: it should most likely be a 'zero transform', just like all other ABB pkgs.

This pkg was probably based on the M-10iA support pkg, which has its base transform at 0.45m over Z (here).

@Levi-Armstrong
Copy link
Member

@Jmeyer1292:

I don't know about the 'base' transformation, or even why it exists. Seems like it should be at 0,0,0.

My understanding is that the base transformation is the location of the actual base/zero location for the robot controllers, since not all robot have there origin at the mounting plate. This was added to allow for users to query between the base transformation and the end effector transformation where the position and orientation should now match between ROS and teach pendant readout.

@gavanderhoorn
Copy link
Member

See Create a URDF for an Industrial Robot - Additional/Standard Frames for more info about this.

@Jmeyer1292 Jmeyer1292 force-pushed the abb_irb2600 branch 2 times, most recently from dd42509 to 9e4c971 Compare July 30, 2015 22:38
@Jmeyer1292
Copy link
Contributor Author

I've updated the package to follow the new naming conventions. New move-it configs for the 20kg and 12kg variants of the 165cm model.

@gavanderhoorn
Copy link
Member

Some comments:

  • almost all launch files are missing an empty last line
  • the joint_names_ yaml file in config does not have a variant specific suffix. I know the contents will be the same (as we use joint_N for all ABB robots), but this is for (future) consistency with other files in the package
  • the meshes/irb2600_165 is a bit confusing (see also my comment on Define ABB specific naming rules/guidelines abb#75). I know those are the current rules, so this is not really a critique, more of an observation
  • I like how you avoided duplication of the xacro macros in urdf, but apart from payload these variants are completely identical (which made your setup possible), so I would just keep only one macro and add a note to the package manifest telling users which variants are supported by that one macro
  • As the variants are identical, I'd suggest keeping only a single MoveIt configuration pkg as well. A note similar to the one in the support package could be added to the manifest of the config pkg.

It could be that maximum accelerations are affected by payload, but we don't know (afaik), and the MoveIt config pkgs also don't reflect that.

@gavanderhoorn
Copy link
Member

@Jmeyer1292: I've opened a PR against your abb_irb2600 branch with a few minor tweaks to some files I think would make sense.

The only 'issue' then is the fact that some joint transforms in the xacro macro have translations in more than a single dimension (ie: x and z fi). This is almost always an artefact of using the Solidworks urdf exporter and relying on it to derive the joint origins. In my own pkgs I've tried to avoid that by adding coordinate origins (reference geometry) and making the exporter use those.

I'll leave it to @Levi-Armstrong to decide whether that should be fixed.

@Jmeyer1292
Copy link
Contributor Author

@Levi-Armstrong I have merged in Gijs fixes and rebased on the latest version of groovy-devel. Please advise regarding the xacro macro translations mentioned by Gijs above.

@Levi-Armstrong
Copy link
Member

@Jmeyer1292

I would prefer that the joint position be changed, I briefly check the dimensions and some appear to be off a few millimeters. The joint per the product specification should be as below.

joint_1 (0, 0, 0.445)
joint_2 (0.15, 0, 0)
joint_3 (0, 0, 0.7)
joint_4 (0, 0, 0.115)
joint_5 (0.795, 0, 0)
joint_6 (0.085, 0, 0)
tool_0 (0, 0, 0)

@simonschmeisser
Copy link
Contributor

any news on this?

@gavanderhoorn
Copy link
Member

gavanderhoorn commented Mar 6, 2017

@Levi-Armstrong's last comment about the joint offsets still needs to be addressed and the CMakeLists.txt of the support pkg has a DEPENDENCIES argument in the roslaunch_add_file_check(..) macro that needs to be removed.

Other than that I think this could be merged (although a regen of the MoveIt pkg with the latest MSA might be also be nice).

@Jmeyer1292
Copy link
Contributor Author

@simonschmeisser I went to the ABB website to pull down the CAD models and start over on this package, but all of the solidworks assemblies are gone. Will ask this week about my CAD options.

@Jmeyer1292 Jmeyer1292 changed the base branch from groovy-devel to kinetic-devel May 2, 2017 23:26
@Jmeyer1292
Copy link
Contributor Author

Alrighty. Two years later, here I am. I have updated the xacro file to match the spec sheet and our conventions. I re-exported all of the cad models and regenerated the convex hulls using blender.

I removed the dependency stuff @gavanderhoorn mentioned. And I entirely dropped the moveit config package as I'd rather not maintain it.

@@ -0,0 +1,3 @@
<launch>
<param name="robot_description" command="$(find xacro)/xacro.py '$(find abb_irb2600_support)/urdf/irb2600_12_165.xacro'" />
Copy link
Contributor

Choose a reason for hiding this comment

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

depreciated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It hasn't lost that much value to wear and tear.

@Levi-Armstrong Levi-Armstrong merged commit a6f32a9 into ros-industrial:kinetic-devel May 3, 2017
@Jmeyer1292 Jmeyer1292 deleted the abb_irb2600 branch May 4, 2017 16:03
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.

5 participants