HC20DTP and HC30PL Update #597
Conversation
The HC20 package was updated with the HC20DTP and HC30PL variant. HC20 visual meshes were changed from .stl to .dae format to improve rendering.
gavanderhoorn
force-pushed
the
HC20DTP_HC30PL_Update
branch
from
a8cee48 to
5ebcc03
Compare
|
High-level comment: could you please use multiple commits in future PRs when changing multiple things and adding support for new variants/series? |
|
@gavanderhoorn Yes, I will try do split them up in the future. |
| @@ -2,13 +2,13 @@ | |||
| <?xml-model href="http://download.ros.org/schema/package_format2.xsd" schematypens="http://www.w3.org/2001/XMLSchema"?> | |||
| <package format="2"> | |||
| <name>motoman_hc20_support</name> | |||
| <version>0.1.0</version> | |||
| <version>1.0.0</version> | |||
There was a problem hiding this comment.
We'll address this in a later PR, but we can't bump the version of this single package like this.
Catkin/Bloom limits things such that every package in a repository must have the same version.
Please revert this change and keep version at 0.1.0.
| which also supports the DT and XP variants. | ||
| which also supports the DT, XP, DTP and HC30PL variants. |
There was a problem hiding this comment.
we normally reserve this kind of phrasing for variants which don't get their own xacro:macro.
The DTP and 30PL do have their own xacro:macro, so I would recommend we change this to reflect that.
| <li>HC20DTP</li> | ||
| <li>HC30PL</li> |
There was a problem hiding this comment.
Seeing as we noticed there are actual variants of variants in other recently merged support packages: is there nothing we can state here as to which specific specification were used?
| <!-- ROS base_link to Robot Manufacturer World Coordinates transform --> | ||
| <link name="${prefix}base" /> | ||
| <joint name="${prefix}base_link-base" type="fixed"> | ||
| <origin xyz="0 0 0.380" rpy="0 0 0"/> | ||
| <parent link="${prefix}base_link"/> | ||
| <child link="${prefix}base"/> | ||
| </joint> |
There was a problem hiding this comment.
should we perhaps take this opportunity to add flange and base here?
Same comment for the 30PL below.
| <child link="${prefix}link_4_r"/> | ||
| <origin xyz="0.880 0 0" rpy="0 0 0" /> | ||
| <axis xyz="-1 0 0" /> | ||
| <limit lower="${radians(-15)}" upper="${radians(15)}" effort="368.48" velocity="${radians(112)}"/> |
There was a problem hiding this comment.
Is this correct? 15 degrees only?
| <child link="${prefix}link_5_b"/> | ||
| <origin xyz="0 0 0" rpy="0 0 0" /> | ||
| <axis xyz="0 -1 0" /> | ||
| <limit lower="${radians(-15)}" upper="${radians(15)}" effort="141.12" velocity="${radians(132)}"/> |
| <geometry> | ||
| <mesh filename="package://motoman_hc20_support/meshes/hc20dtp/visual/link_5_b.dae"/> | ||
| </geometry> | ||
| <origin rpy="0 1.57075 0"/> |
There was a problem hiding this comment.
why do these Collada meshes have a different orientation?
It would be nice to have the meshes aligned already, so as to simplify the xacro:macro.
Same comment for the collision of link_5_b and both meshes for link_6_t.
The HC20 package was updated with the HC20DTP and HC30PL variant. HC20 visual meshes were changed from .stl to .dae format to improve rendering.