ubi-agni / human_hand

modular, articulated human hand model
Other
31 stars 6 forks source link

Duplicate taxels with different indices #4

Closed guihomework closed 3 years ago

guihomework commented 3 years ago

I think I found a bug in the taxel generation within the xacro, but would like to provide the solution myself. I just document it here.

example with P3l using L2 layout

      <taxel idx="4" rpy="-0.0 1.4543 -1.6852" xyz="-0.0835 -0.0198 -0.0287">
        <geometry>
          <mesh filename="package://human_hand_description/model/meshes/taxels/L2/ptmp.stl" scale="-0.00101 0.00101 0.00101"/>
        </geometry>
      </taxel>
      <taxel idx="11" rpy="-0.0 1.4543 -1.6852" xyz="-0.0835 -0.0198 -0.0287">
        <geometry>
          <mesh filename="package://human_hand_description/model/meshes/taxels/L2/ptmp.stl" scale="-0.00101 0.00101 0.00101"/>
        </geometry>
      </taxel>

to reproproduce roslaunch human_hand_description upload.launch tactile_mapping:=P3l

then look at the produced urdf ('rosparam get /robot_description -p')

the problem occurs for layout L2 and L3 and seems to stem from a merging of previously separated [L1/L2/L3].urdf.xacro

A PR will be provided to recover correct values but keeping the merged single tactile_markers.urdf.xacro

guihomework commented 3 years ago

in L1, there was a case were the same index (ptop) was assigned to ptop taxel and ptmp taxels https://github.com/ubi-agni/human_hand/commit/68bd941a3fbf986dd0d379c6f4933291b3219d02#diff-478f489f677f5e846ed0480f3b5efcb6R79 https://github.com/ubi-agni/human_hand/commit/68bd941a3fbf986dd0d379c6f4933291b3219d02#diff-478f489f677f5e846ed0480f3b5efcb6R81 This is acceptable. L2 and L3 had different index for those 2 taxels at the time.

all Layouts had the same index (ptmp/ptmd and ptip/ptid) associated to the ptmp+ptmp2/ptmd+ptmd2 and ptip+ptip2/ptid+ptid2 like here https://github.com/ubi-agni/human_hand/commit/68bd941a3fbf986dd0d379c6f4933291b3219d02#diff-78cace2a3d10d4f552a78edd8e322f4dR55 and https://github.com/ubi-agni/human_hand/commit/68bd941a3fbf986dd0d379c6f4933291b3219d02#diff-78cace2a3d10d4f552a78edd8e322f4dR83 This is acceptable

but in a merging commit, the L1 and L2/L3 were merged, however the source index (from the map) is different for the same destination taxel example here: https://github.com/ubi-agni/human_hand/commit/72a5c18895e1c67d8580c2d320bbc32c0f112234#diff-65c216c2e442308041c877f502120165R69 and https://github.com/ubi-agni/human_hand/commit/72a5c18895e1c67d8580c2d320bbc32c0f112234#diff-65c216c2e442308041c877f502120165R70

They should not co-exist. one line is for L1 the other is for L2/L3 there are other places where this occurs. There must be a distinction between L1/L2/L3 for those special taxels.

guihomework commented 3 years ago

One thing that seems to have been used is that non-existent indices in the yaml of certain layouts will just fail to be found and produce a default value of -1 which should then not generate a taxel. This mechanism is fine, but there seem to be an issue with layouts having all those indices existing, hence producing duplicates.

rhaschke commented 3 years ago

Non-existent indices in the yaml of certain layouts will fallback to a default value of -1, which then does not generate a taxel. But there seems to be an issue with layouts having all those indices existing, hence producing duplicates.

I agree. The duplication of taxel meshes with different indices: https://github.com/ubi-agni/human_hand/blob/577d93177b0e4fad104679b05b3b91a265201959/model/tactile_markers.urdf.xacro#L102-L103 was intended (for L1 layout) to activate both taxel meshes from the same sensor, because we didn't have generated specific (larger) meshes for L1. However, for L2 and L3 this poses issues, because those sensors get two conflicting definitions in the urdf. I agree that these duplicates should be guarded by a L1 layout check. Removing them completely and just referring to duplicate mappings in the yaml is not an option, because the calibration approach in GloveViz cannot handle unknown taxels. So, the yaml mapping is constrained to the taxels known from the layout .svgs.

Here is an image comparing the three layouts (L1 to L3 from left to right): image

guihomework commented 3 years ago

I had done a similar comparison in the tactile_marker.urdf.xacro by looking at the original L1/L2/L3.urdf.xacro and the existing taxels indices in taxel.yaml

The PR with 2 small xacro:if is ready, I am testing.