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

Basic support for Yaskawa Motoman GP180-120 6DOF arm #277

Merged
merged 24 commits into from
Sep 19, 2019

Conversation

jmarsik
Copy link
Contributor

@jmarsik jmarsik commented Jun 3, 2019

Meshes are derived from official Yaskawa 3D model files. Collision meshes are convex hulls of visual meshes created in MeshLab. Visual meshes don't include the balancer.

Origin point of all meshes is shifted from the original origin point in URDF to keep the meshes creation from Yaskawa 3D model files as simple as possible (currently it means just loading the STEP file into Fusion 360 and exporting individual components as STL files).

This pull request is created instead of previous pull request to motoman_experimental repository (ros-industrial/motoman_experimental#46).

…e derived from official Yaskawa 3D model files; collision meshes are convex hulls; URDF does not contain correct effort limit values yet
@jmarsik
Copy link
Contributor Author

jmarsik commented Jun 3, 2019

Travis build for ROS Indigo is still failing with this error:

Traceback (most recent call last):
  File "/opt/ros/indigo/share/xacro/xacro.py", line 62, in <module>
    xacro.main()
  File "/opt/ros/indigo/lib/python2.7/dist-packages/xacro/__init__.py", line 696, in main
    eval_self_contained(doc)
  File "/opt/ros/indigo/lib/python2.7/dist-packages/xacro/__init__.py", line 626, in eval_self_contained
    eval_all(doc.documentElement, macros, symbols)
  File "/opt/ros/indigo/lib/python2.7/dist-packages/xacro/__init__.py", line 553, in eval_all
    eval_all(body, macros, scoped)
  File "/opt/ros/indigo/lib/python2.7/dist-packages/xacro/__init__.py", line 609, in eval_all
    result = eval_text(at[1], symbols)
  File "/opt/ros/indigo/lib/python2.7/dist-packages/xacro/__init__.py", line 483, in eval_text
    results.append(handle_expr(lex.next()[1][2:-1]))
  File "/opt/ros/indigo/lib/python2.7/dist-packages/xacro/__init__.py", line 470, in handle_expr
    return eval_expr(lex, symbols)
  File "/opt/ros/indigo/lib/python2.7/dist-packages/xacro/__init__.py", line 444, in eval_expr
    result = eval_term(lex, symbols)
  File "/opt/ros/indigo/lib/python2.7/dist-packages/xacro/__init__.py", line 418, in eval_term
    result = eval_factor(lex, symbols)
  File "/opt/ros/indigo/lib/python2.7/dist-packages/xacro/__init__.py", line 398, in eval_factor
    return neg * eval_lit(lex, symbols)
  File "/opt/ros/indigo/lib/python2.7/dist-packages/xacro/__init__.py", line 369, in eval_lit
    raise XacroException("Property wasn't defined: %s" % str(ex))
xacro.XacroException: Property wasn't defined: u'DEG2RAD'

The property is defined in xacro files like this:

<?xml version="1.0" ?>
<robot xmlns:xacro="http://ros.org/wiki/xacro">
<xacro:macro name="motoman_gp180_120" params="prefix">
      ...
     <property name="DEG2RAD" value="0.017453292519943295" />

What's wrong with that for ROS Indigo? I remember creating some URDFs for KUKA KR210 back in Indigo days and I also used this DEG2RAD property (declared in included file from kuka_resources package).

@gavanderhoorn
Copy link
Member

Again, thanks for the PR. Contributing support for new models like this is much appreciated 👍

Some high-level comments as a pre-review:

  • please rename the package to motoman_gp180_support. The 120 is a variant of the GP 180 and variant specs should not appear in the package name (only series names) as it would result in way too many packages
  • there is no need to use DEG2RAD. Since Jade, xacro supports Python expressions and other niceties. To convert between degrees and radians, simply use ${radians(the_nr_in_degrees)}.
  • please use lower-case filenames for everything, so please rename the mesh files. I would also suggest to change the filenames from S_AXIS.STL to something like link_s.stl as that would correspond to the name of the link in the urdf
  • please scale meshes appropriately, so don't use scale
  • please re-orient meshes appropriately so we don't need to rotate them in the visual elements

Additionally:

Origin point of all meshes is shifted from the original origin point in URDF to keep the meshes creation from Yaskawa 3D model files as simple as possible (currently it means just loading the STEP file into Fusion 360 and exporting individual components as STL files).

Unfortunately I'm going to have to ask you to please fix the origins of the meshes. I realise that is some work, but it makes using the meshes outside of this package much easier, would align with the rest of the models in this repository and would simplify the xacro macro significantly.

I also noticed that the origin elements of the joints tend to have translations in multiple dimensions. While this is supported, IK solvers such as IKFast tend to not like this very much so it is highly recommended to make sure there are only translation in single dimensions for all joints. See motomini_macro.xacro for an example.

… robot series, 120 is variant of this series); moved meshes to subdirectory for GP180-120 variant; renamed meshes to lowercase
…ocation; scaling, rotating and translating origin point in URDF no longer needed; added tools directory with script and README file describing automation of the processing of the source meshes (derived from Yaskawa 3D model STEP file)
@jmarsik
Copy link
Contributor Author

jmarsik commented Jun 4, 2019

Thank you for your comments and also for help with using Jade+ xacro on Indigo!

I have implemented most of your suggestions. The only thing remaining is changing the location of joints so that they are "in line" (with translation only in single dimension). Will try to do that tomorrow.
I have also noticed that the collision meshes are wrong, I will re-do them.

What about effort limit values? Is there some best practice about what values should I use if I don't know them?

…he joints are "in line" with only one axis translation between two neighboring joints (exception is link_3_u for upper arm, there is 2-axis translation)
…ic collision meshes generation from visual meshes
@jmarsik
Copy link
Contributor Author

jmarsik commented Jun 5, 2019

It's done, can you please check it? Thanks.

There is still one link/joint that uses translation in 2 axes, see picture.
I don't know how to do it for this one. I have to use translation in 2 axes or translation in 1 axis plus rotation.

image

The URDF was changed to rotate the whole model so that the X-axis point forward and kinematic model adjusted to match the standard Yaskawa model and dimensions.  The mesh were then adjusted to realign properly with the new kinematic model.  The direction of motion for each axis was also corrected to match the real robot motion.  Finally, the visual meshes were reduce by 50% to reduce the display processing.
@EricMarcil
Copy link
Contributor

I hope you don't mind. I just pushed some commits on your branch.
The whole model was build with the X-axis pointing backward. When I was looking a the Tool location, the X values was negative when it should be positive. The motion direction of some axis were also reverse (which might have been a side effect of the robot being build in the wrong X direction). Finally, your kinematic model worked but was slightly different from the standard Yaskawa model. The standard model has the R, B, T frame axes origins all overlapping in the center of the wrist joint.

So, the URDF was changed to rotate the whole model so that the X-axis point forward and kinematic model adjusted to match the standard Yaskawa model and dimensions. The mesh were then adjusted to realign properly with the new kinematic model. The direction of motion for each axis was also corrected to match the real robot motion. Finally, the visual meshes were reduce by 50% to reduce the display processing.

Finally, the yellow line linking the U axis to the R-axis is correct. It's just the mathematically representation and doesn't need to go thought the physical arm.

With these changes, I don't have a problem releasing the model. Please review it and let me know if you have any questions.

@EricMarcil
Copy link
Contributor

Thanks for your work on this model. I usually create these models from scratch when requested, so this did saved me time.

…s made by Eric Marcil in last commit; added simplification step of visual meshes to 50% faces
@jmarsik
Copy link
Contributor Author

jmarsik commented Jun 7, 2019

Thank you Eric for your corrections. I have updated my scripts to produce meshes from original Yaskawa STEP file to be usable with your corrected URDF. I did not update the meshes themselves because your meshes are perfectly ok. But I tested the URDF with meshes produced by my scripts and the result looks the same.

One question - what about the base "link"? Should it be in this position (see picture)? Is Yaskawa controller robot coordinate system origin placed in this position?

image

What about the effort limits in URDF file? I understand that those are probably not used when running with the real robot through MotoROS, right?

I plan to further improve the URDF so that it can be used in Gazebo simulator (in another pull request, in a few days or weeks), is it ok to include Gazebo stuff in the URDF itself or should I create separate SDF file? I see that in motoman_sia10d_support package there are some Gazebo and simulation related properties directly in the URDF, so that probably means that I can do it in a similar way?

@gavanderhoorn
Copy link
Member

Thanks @EricMarcil for the first look and correcting some issues already.

And thanks @jmarsik for the PR and iterating on it.

As to your questions:

One question - what about the base "link"? Should it be in this position (see picture)? Is Yaskawa controller robot coordinate system origin placed in this position?

yes, that is exactly where it should be. The purpose of base is to provide a mapping between the Yaskawa world frame and the ROS TF tree of the manipulator model.

What about the effort limits in URDF file? I understand that those are probably not used when running with the real robot through MotoROS, right?

they are not used by MotoROS, no. But it would be nice if we can get them correct. @EricMarcil would this be something you could contribute like you've done for the other models you've contributed?

I plan to further improve the URDF so that it can be used in Gazebo simulator (in another pull request, in a few days or weeks), is it ok to include Gazebo stuff in the URDF itself or should I create separate SDF file?

you don't necessarily need to create an SDF, but please don't "pollute" the manipulator xacro with Gazebo-specific tags. It isn't needed, as we can achieve the same result in a different way: by "wrapping" the plain xacro with one that adds the Gazebo extensions (using link references and other mechanisms).

It's not perfect, but take a look at how this is done in abb_irb120_gazebo.

@jmarsik
Copy link
Contributor Author

jmarsik commented Jun 11, 2019

I thought that the origin of Yaskawa controller robot coordinate system is placed in a physical joint position between base and S link or on the bottom of the robot base (same as base_link "link"). See picture for Robot coordinate system on https://www.motoman.com/en-us/about/company/robotics-glossary. But I may be wrong because my experience with Yaskawa robots is still limited. We don't have any real robot available at the moment, still deciding which variant we should use for our application.

When I try to visualize GP12 from motoman_gp12_support in RViz, I see that the base "link" is placed exactly like the one in my package after Eric's correction.

Regarding the effort limit values, it would be nice to know how (from what documentation) will Eric determine them.

If that's ok, then I think that we can proceed and merge the pull request (maybe after correction of effort limit values, that's up to you),

I will then probably contribute another variant into the same package (for GP180 variant of the manipulator with shorter U link) and then I will start working on Gazebo simulation support with your suggestions in mind. Should I place the wrapping / extending URDF in the same package (motoman_gp180_support) or create a new package dedicated to simulation support of GP180 and its variants (like abb_irb120_gazebo)?

@EricMarcil
Copy link
Contributor

It's the 1st time I see that glossary and that picture is schematic and misleading. The robot frame is always at the intersection of the S-axis and the horizontal plane going through the L-axis. It is clearly identified in the operator's manuals.

For the effort, I usually use the Max Acc/Dec Torque [Nm]. See the chart below:
image

For the standard GP180, here is the chart:
image

@jmarsik
Copy link
Contributor Author

jmarsik commented Jun 12, 2019

Thank you for the values Eric! I have updated the URDF. Also, thank you for the explanation of Yaskawa controller robot coordinate system origin position, I will keep this in mind.

Can we merge the pull request now?

@gavanderhoorn
Copy link
Member

Can we merge the pull request now?

I will do a final review. If everything is ok, then we can merge.

@jmarsik
Copy link
Contributor Author

jmarsik commented Jun 15, 2019

One more question about future motoman_gp180_gazebo and motoman_gp180_120_moveit_config packages to support simulation of GP180-120 in Gazebo with readily available support for MoveIt motion planning ... should I include them in this motoman repository or would motoman_experimental be a better choice?

@EricMarcil
Copy link
Contributor

I know there are some old move_it_config in the motoman branch but we decided to stop including them because already with the increasing number of models the repository is growing. Plus, in most cases, people would have to modify it anyways to make it match their specific environment. So motoman_experimental would be a better choice.

@jmarsik
Copy link
Contributor Author

jmarsik commented Jun 20, 2019

I know there are some old move_it_config in the motoman branch but we decided to stop including them because already with the increasing number of models the repository is growing. Plus, in most cases, people would have to modify it anyways to make it match their specific environment. So motoman_experimental would be a better choice.

I thought so, will do another pull request to motoman_experimental repository after successful merge of this one.

@jmarsik
Copy link
Contributor Author

jmarsik commented Jul 9, 2019

@gavanderhoorn could you please do the final review so that we can close this PR?

@jmarsik
Copy link
Contributor Author

jmarsik commented Aug 9, 2019

Can we merge this PR so that I can continue with my contributions?

gavanderhoorn
gavanderhoorn previously approved these changes Aug 28, 2019
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 is OK to merge.

I've pushed a few commits to fix some minor things and address some of the issues that remained after the last time this was discussed (multi-dim transforms, etc).

I'm not entirely sure I'm comfortable with the tools sub directory, but it is specific to the GP180 120, so I can't come up with a better location for those files right now.

@gavanderhoorn
Copy link
Member

gavanderhoorn commented Aug 28, 2019

@jmarsik: thanks for your PR, this is a nice contribution. 👍

And thanks for iterating on it.

Apologies for taking so long: your PR was submitted right before/in the holiday season.

We'll wait for Travis to give the final OK and then merge.

@gavanderhoorn
Copy link
Member

Note: I'm going to squash-merge this PR. All of the commits that followed the initial one are fixups, and don't need to end up in the repository history.

Especially the many changes to binary meshes are undesirable, as they bloat the repository considerably.

@cjue
Copy link
Contributor

cjue commented Sep 4, 2019

For the effort, I usually use the Max Acc/Dec Torque [Nm]. See the chart below:
image

For the standard GP180, here is the chart:
image

@EricMarcil: I am currently working on basic GP88 support and would like to add the correct efforts there as well. Are these YRC1000 parameter charts available for download anywhere, or could you post the values for the GP88?

@jmarsik: Your stl processing scripts are very helpful, thanks for contributing them!

I will create a PR for my motoman_gp88_support as soon as possible.

@gavanderhoorn
Copy link
Member

Not sure what is going on with Travis. It seems to be having some problems.

Just restarted the build.

@EricMarcil
Copy link
Contributor

@cjue : I don't want to post GP88 here because it is not related to this pull. I've open issue #288 for it. Please refer to that issue for further discussion about the GP88 support.

@jmarsik
Copy link
Contributor Author

jmarsik commented Sep 9, 2019

Travis appears to still have some problem, could you try to restart it again?

An additional approx 2MB reduction.
@gavanderhoorn gavanderhoorn merged commit 197d2f8 into ros-industrial:kinetic-devel Sep 19, 2019
@gavanderhoorn
Copy link
Member

Thanks @jmarsik for the PR 👍

And thank you for iterating on it.

I've squash-merged everything as almost all commits were fixups based on review feedback, and we don't want those to end-up in the history of the repository. Especially not the many intermediate versions of the meshes.

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.

4 participants