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

Update MH5: Add Short & Long Variants #380

Open
wants to merge 6 commits into
base: kinetic-devel
Choose a base branch
from

Conversation

acbuynak
Copy link
Contributor

@acbuynak acbuynak commented Jan 5, 2021

Overhaul of MH5 Support package.

Defined two physically and parametrically unique variants (short & long) of the MH5 and stored within a single MH5 package. Kept support files for each variant separate excepting the joint_names_mh5.yaml. Some of the mesh files are the same/similar, but I kept the meshes grouped by variant for overall clarity.

Initial work tracked in Issue #377

Prior to merging a release date needs assigned in the drafted changelog.rst

@EricMarcil
Copy link
Contributor

@acbuynak: Thanks for the contribution. Here are a few things to review:

  • I don't think we need to repeat all the meshes that are identical between the MH5 and MH5L. You can probably just keep the link_3_u.stl file in the MH5L and make reference to the MH5 for the other models.
  • Some of the visual models seems a bit large. I normally try to get them down below 1 MB but we don't want to loose to much detail. So it's a question of balancing the two.
  • For the collision models, we usually do a convex hull around the model to reduce the size as much as possible. You models seem more detailled but when I checked the model size, they were not that bad. So they are probably fine.

@EricMarcil
Copy link
Contributor

I've complete checking the motion direction and range by comparing with our simulator (MotoSim). It's all matching up.

@acbuynak
Copy link
Contributor Author

@EricMarcil - Thanks for checking the PR.

Responses to your above bullet points...
1 - Repeated Meshes. I understand the need to reduce overall package size, but wanted to emphasize that this package contained two models by making a distinction in the meshes folder.
2 - Mesh Size. Agreed, I missed that. Thanks. I will simplify (quadratic edge collapse decimation in MeshLab) to something more reasonable that retains sufficient fidelity.
3 - Collision Mesh Detail. When testing the models, I discovered that a convex hull caused a false collision of links L & R at the extreme joint limit of U. The correction was 22f11d6 to allow the "V" cutout in the L link to be slotted with the R link.

@gavanderhoorn
Copy link
Member

gavanderhoorn commented Jan 18, 2021

Thanks for the PR.

I'll try to take a look at it today.

At a high-level and as a first comment: I do agree with @EricMarcil that we'd rather reuse meshes as much as possible. This reduces total size of the package and maintenance as well (as we don't have to keep two sets of meshes in sync).

The fact the package supports multiple variants should already be clear from the _macro.xacro files, the package manifest and the various .launch files.

@acbuynak
Copy link
Contributor Author

acbuynak commented Jan 18, 2021

Sounds good. I've drafted a commit to consolidate meshes. I'm planning to keep the 'unique' MH5L mesh files in their own folder, the rest will reference to the MH5.

Just a minor note...
I did a side-by-side comparison of each short & long variant mesh. I found the file I have for the U-joints to be different. Long variant meshes were sourced from Motoman Tech Support.

I'm leaning to use the short variant (right image) for both URDF's. They're close enough. Collision mesh varies by ~1cm at abs max. I note this because my actual robot (long variant) matches the short mesh file shown. (edit. error, I have the left shown u-joint) This may be a feature option to the robot, rather than short/long variant.
Thoughts?

Summary btwn Short/Long meshes:
-Unique: L, U, R
-Common: base, S, B, T

mh5_u_link

@EricMarcil
Copy link
Contributor

EricMarcil commented Jan 18, 2021

That's interesting. All the models (long and short) that I have in MotoSim matches your left picture (MH5L). But I know that the MH5 and MH5L have a lot of variations. So it is quite possible that you have the right variant.
But, I believe that the left model is the most common.
I would recommend putting the right model, in a folder MH5_other or something similar with the name: link3_u_old. Then maybe in the urdf, have a commented reference to it that people can easily customize depending on their needs.

Out of curiousity, can you send me the exact model type (ex: YR-MH0005S-C00) that you have on the name plate on the robot arm. And confirm that your robot is a DX200. Maybe also send me your all.prm file, I'll double check that is is matching the standard values that I had sent you before. You can use my work e-mail: [email protected]

@acbuynak
Copy link
Contributor Author

@EricMarcil - Emailed you my parameters file.

For transparency, I caught a mistake above. My physical MH5L U-Link matches the long variant shown on the left as EricMarcil noted is common.

I like the suggestion of retaining a link_3_u_old files w/ commented option in URDF. I will add this in next commit.

Meshes were simplified using "Quadric Edge Collapse Decimation" method 
to improve collision planning between L and R links. Attention was made 
to retain a similar number of faces & verticies as the convex hull had 
for comparable performance.
Previous collision meshes were created using the "Convex Hull" method.
Reduced MH5L "L" and "R" link mesh file sizes from ~2MB to suggested < 1 MB.
Mesh files were reduced using "Quadric Edge Collapse Decimation" method
with priority to maintaining boundary location and quality of polygons.
Link meshes for the MH5L matching those of the MH5 were removed.
Effected URDF references redirected to MH5 meshes.
Moved unique "link-s" mesh to dedicated mh5_old_variants directory.
Updated changelog to reflect shared 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.

3 participants