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

Fix ItemIdentityException #811

Merged
merged 1 commit into from Jan 13, 2022
Merged

Fix ItemIdentityException #811

merged 1 commit into from Jan 13, 2022

Conversation

corycrean
Copy link
Contributor

This PR fixes an ItemIdentityException I encountered while working through this tutorial (more details here).

I found that two different materials were being given the same name. It looks like the material name gets set in two places. Here, the name gets created by appending the word "Material" and an integer i to the path to the file containing the material. Then, here, that name is changed by appending another integer, material_count, and the word "Robot". The problem is that the digits in i and material_name run together, so some combinations of i and material_count result in exactly the same string.

In the issue I linked to, the file package://moveit_resources_panda_description/meshes/visual/link6.dae contained (at least) 15 different materials, so AssimpLoader::loadMaterials named them:

package://moveit_resources_panda_description/meshes/visual/link6.daeMaterial0
package://moveit_resources_panda_description/meshes/visual/link6.daeMaterial1
...
package://moveit_resources_panda_description/meshes/visual/link6.daeMaterial14

Material14 was added when material_count=1120, resulting in the name
package://moveit_resources_panda_description/meshes/visual/link6.daeMaterial141120Robot

Later, Material1 was added when material_count=41120, which resulted in the same name, hence the ItemIdentityException. To fix this, I added an underscore before appending material_count to the material name, so for the preceding example we get two different names:
package://moveit_resources_panda_description/meshes/visual/link6.daeMaterial14_1120Robot
package://moveit_resources_panda_description/meshes/visual/link6.daeMaterial1_41120Robot

@jacobperron jacobperron self-assigned this Jan 13, 2022
Copy link
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the detailed explanation of the problem!

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

cpplint failures are unrelated.

@jacobperron jacobperron added this to Proposed in Foxy Patch Release 7 via automation Jan 13, 2022
@jacobperron jacobperron added this to Needs backport in Galactic Patch Release 2 Jan 13, 2022
@jacobperron jacobperron moved this from Proposed to Needs backport in Foxy Patch Release 7 Jan 13, 2022
@jacobperron jacobperron merged commit ca59b6f into ros2:ros2 Jan 13, 2022
@corycrean corycrean deleted the material_name branch January 18, 2022 20:43
jacobperron pushed a commit that referenced this pull request Jan 31, 2022
jacobperron pushed a commit that referenced this pull request Jan 31, 2022
@jacobperron jacobperron removed this from Needs backport in Foxy Patch Release 7 Jan 31, 2022
@jacobperron jacobperron removed this from Needs backport in Galactic Patch Release 2 Jan 31, 2022
jacobperron added a commit that referenced this pull request Feb 3, 2022
Co-authored-by: Cory Crean <cory.crean@picknik.ai>
jacobperron added a commit that referenced this pull request Feb 19, 2022
Co-authored-by: Cory Crean <cory.crean@picknik.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants