Fix #19: Updated abb_irb4400_support package

[culletom]

Corrected joint limits and velocities, and reformatted the urdf files. Also added additional files so that it matches other support packages.

[culletom]

Just wanted to confirm that the joint transforms need to be modified before I went ahead and changed them, particularly between joint 1 and joint 2.

[Levi-Armstrong]

They are acceptable, don't go through the trouble.

[Levi-Armstrong]

@culletom, There is an existing error with the urdf that affects the kinematics, joint_4 x-axis translation should be 1.38 and joint_6-tool0 x-axis tranlation should be 0.14. This would also require a correction to the meshes.

[gavanderhoorn]

Make this a private param of the joint_state_publisher node.

[gavanderhoorn]

Changes look good, but some minor things need to be addressed.

Please also check the comment by @Levi-Armstrong about link lengths for link_4 and the tool0 transform.

[Levi-Armstrong]

I have verified with RobotStudio that J23 is coupled.

Also, I would prefer the naming convention to be model_payload_reach. This will cover all possible variants for the abb robots, so for this variant it would be irb4400l_30_243. @shaun-edwards & @gavanderhoorn what are your thoughts?

[gavanderhoorn]

The naming should definitely include all information needed to uniquely identify a manipulator variant. For the ABB that appears to be model nr, payload and reach.

Should the L be grouped with the model nr though, or with the 30? Looking at abb-irb4400-data it looks like it should be grouped with the 30 ('IRB 4400-L10', on that page). It would then become abb_irb4400_l30_243?

[gavanderhoorn]

For other models (irb 1600 fi), we would get: abb_irb1600_10_120 (for the '1600 - 10 / 1.2') and abb_irb1600_6_145 (for the '1600 - 6 / 1.45').

The 'rule' would then be to append the payload in kgs, and the reach in meters, separated by underscores. And always use at least two decimal places after the dot for the reach (so 120, instead of 12).

[Levi-Armstrong]

In RobotStudio they group the nr with the model, I think this would be better so not to confuse people. The confusion being they may think the L has some significant related to the payload when it actual stands for long reach.

[Levi-Armstrong]

I agree.

The 'rule' would then be to append the payload in kgs, and the reach in meters, separated by underscores. And always use at least two decimal places after the dot for the reach (so 120, instead of 12).

[gavanderhoorn]

In RobotStudio they group the nr with the model, I think this would be better so not to confuse people. The confusion being they may think the L has some significant related to the payload when it actual stands for long reach.

Ok. Agreed. I was just going with what was listed on the ABB site.

[Levi-Armstrong]

Yea, that is what I original found too.

On Fri, Mar 6, 2015 at 2:59 AM, G.A. vd. Hoorn notifications@github.com
wrote:

In RobotStudio they group the nr with the model, I think this would be
better so not to confuse people. The confusion being they may think the L
has some significant related to the payload when it actual stands for long
reach.

Ok. Agreed. I was just going with what was listed on the ABB site.

—
Reply to this email directly or view it on GitHub
#21 (comment)
.

Levi Armstrong

[culletom]

Ok I'll update the PR to include this naming convention, and the other comments mentioned. Should file names also be updated, or is it just within the urdf / xacros?

[gavanderhoorn]

Everything needs to be consistent.

As this is a package in an experimental repository, that is allowed without consideration for any bw-compatibility.

[gavanderhoorn]

We should clarify that for support pkg naming, no variant names should be used. With the grouping of the L with the model nr, contributors could get that idea, so we might want to make sure we document this somewhere.

[Levi-Armstrong]

@gavanderhoorn & @shaun-edwards, would this be the best place for documenting the naming convention for the abb robots?

[gavanderhoorn]

I'd like to keep that page about working with the pkgs. We could update File and directory layout for robot support repositories (or I should finish the rep version some time and add it there, as an appendix or something).

[gavanderhoorn]

Possible text for ABB specific naming rules for the linked wiki page/rep:

---

Appendix A: Vendor specific naming

ABB

Variant names for ABB manipulators may include a modifier (indicating support for a particular feature, or a physical characteristic of the variant), a payload specification (one or more digits, indicating maximum supported payload in kg) and finally a specification of the reach of the arm (in meters). Examples: IRB 1600 - 10 / 1.2 and IRB 4400L - 10.

All ABB support packages shall be named using the following template:

  abb_irbMODEL_support

Note that this template omits the modifier, payload and reach components included in the actual product name.

All artefacts within ABB support, MoveIt configuration and plugin packages shall be named using the following template:

  abb_irbMODEL[MODIFIER]_PAYLOAD_REACH

Where MODEL is a numeric string, MODIFIER is a string, PAYLOAD is a numeric string (max payload in kg) and REACH is a numeric string (reach converted to centimeters, avoiding fractional numbers).

If a particular model does not have variants based on reach, the REACH component should be omitted from the ROS-Industrial name as well.

Examples: abb_irb1600_10_120 and abb_irb4400l_10.

---

@Levi-Armstrong: does that make sense?

[culletom]

@gavanderhoorn I've renamed all files to follow your outlined convention, but I will wait for confirmation from @Levi-Armstrong before committing.

One thing I have yet to fix is the error with the x-axis translations, as I can't find the CAD model for the L30 on the ABB website and so I'm not sure how to modify the meshes. Should I translate the mesh origins using something like blender, or is there an easier approach?

[Levi-Armstrong]

@gavanderhoorn, in reference to the vendor specific naming, I would suggest that the naming always include the reach. In the case of the irb 4400l there is two payloads variants 10kg and 30kg and they actual have different reaches; the 10kg is 2.55m and the 30kg is 2.43m. The suggest naming for the variants would be abb_irb4400l_10_255 and 4400l_30_243. Do you agree?

[Levi-Armstrong]

@culletom; I would use meshlab to translate the mesh; the command is under Filters>Normals, Curvatures and Orientation>Transform: Move, Translate, Center. Make sure the freeze matrix is checked, if not when you save the mesh the changes will not be applied.

[gavanderhoorn]

@Levi-Armstrong wrote:

@gavanderhoorn, in reference to the vendor specific naming, I would suggest that the naming always include the reach. In the case of the irb 4400l there is two payloads variants 10kg and 30kg and they actual have different reaches; the 10kg is 2.55m and the 30kg is 2.43m. The suggest naming for the variants would be abb_irb4400l_10_255 and 4400l_30_243. Do you agree?

Hm, not sure. I've always approached these kind of issues such that the minimum amount of information should be used to discriminate between two variants. In the case you describe, the payload already uniquely identifies the variant, so including reach would seem to be unnecessary. Also, the ABB model/variant name doesn't include the reach for those models, which would make the ros-i name different.

But perhaps we should split this discussion of into a separate thread/issue, as it's getting a bit off-topic for this PR.

[gavanderhoorn]

Let's continue on ros-industrial/abb#75 with the naming discussion.

[culletom]

@Levi-Armstrong thanks for the advice, made the translations much easier.

The last commit brings in all the suggested changes. I also renamed the files, but depending on the outcome of the naming discussion these might need to be renamed again. However if this ends up being the case it can be done later in a sperate PR, as other packages will have to be renamed too.

[gavanderhoorn]

@culletom: yeah, I'm sorry about the naming 'mess'. Thanks for sticking with us though, really appreciated.

---

As to your last commit: as shown here, the meshes/VARIANT/ dir should not include the mfg prefix.

This is probably not too apparent from the linked text, but the rationale is that only entities that are referenced externally / included in other files without the package name included in the URI need the mfg prefix, otherwise it is redundant.

As an example: a xacro macro file can only be included in another xacro by referring to it like package://mfg_pkg/path/to/the_macro.xacro or $(find mfg_pkg)/path/to/the_macro.xacro. This already includes the mfg_pkg in the URI, so adding it again to the_macro.xacro is unneeded (as there is no chance of clashes).

But, the actual robot macro that is defined by the macro should get the prefix, as it will be invoked as <xacro:robot_macro />, which does not include the mfg prefix, so if there are two mfgs that have a robot model/variant, we can get clashes, hence the requirement for the prefix.

I hope it now makes sense that the meshes/VARIANT dir does not need the prefix. It can simply be named irb4400l_30. If in the future more variants are added, and those can share meshes with the L-30, we should probably create a meshes/irb4400, move common meshes there and reference them from the variant specific urdfs/xacros.

---

I guess I should spend some time updating the documentation on support pkg layout.

We do also have a support pkg generator, although it has not been widely 'advertised'.

[culletom]

@gavanderhoorn no problem, naming has come up in a number of different PRs, so its important to settle once and for all!

Yes what you have suggested makes perfect sense - I've updated the files to reflect this.

[Levi-Armstrong]

@culletom, The package.xml file is missing the <build_depend>roslaunch</build_depend>, other than that it looks pretty good. Thank you for contributing.

[gavanderhoorn]

Please add a DEPENDENCIES argument to the roslaunch_add_file_check(..) invocation. See here for an example.

[gavanderhoorn]

@culletom: pending acceptance of the new naming guidelines for ABB pkgs, I'd say this is pretty complete now. If you address the two minor points related to the roslaunch dependency and use, I think we can merge this as it is right now.

If you could squash all fixups into the first commit that would be even nicer (if you want you can extend the commit msg to keep a record of what was changed).

[culletom]

@gavanderhoorn I've just included the roslaunch dependencies and consolidated the PR into one commit. With regards to the roslaunch dependencies, these currently aren't included in other abb support packages, so perhaps this needs to be done?

[gavanderhoorn]

We'll do the other pkgs when they receive other updates. I'll leave it to @Levi-Armstrong to determine whether he wants to roslaunch test all support pkgs before releasing into Indigo or not.

[Levi-Armstrong]

@culletom: Which support packages are missing the roslaunch dependencies?

[Levi-Armstrong]

+1

[culletom]

@Levi-Armstrong I was referring to the points brought up by @gavanderhoorn (see the last two 'outdated' line notes above), which highlight the need to include a DEPENDENCIES argument in the roslaunch_add_file_check(..) invocation in the CMakeList.txt file. This is currently not done in any of the other abb support packages.