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

Store path to mesh in MeshShape #397

Merged
merged 4 commits into from
May 27, 2015
Merged

Store path to mesh in MeshShape #397

merged 4 commits into from
May 27, 2015

Conversation

mkoval
Copy link
Collaborator

@mkoval mkoval commented May 27, 2015

This pull request adds a getMeshPath method to MeshShape. This returns the path to mesh on disk if it is available. If not, it returns an empty string (the default behavior).

This information is necessary to render meshes with non-embedded textures. Assimp references external textures with a path relative to the directory containing the mesh.

@mkoval
Copy link
Collaborator Author

mkoval commented May 27, 2015

I noticed that "testDynamics" is failing on Travis. I didn't make any changes to the dynamics code, so I suspect that this is caused by previous changes in the memory_management branch.

@mxgrey Is that possible?

@mxgrey
Copy link
Member

mxgrey commented May 27, 2015

testDynamics was failing temporarily while I was merging the master branch (with the new finite differencing feature) into memory_management. But I believe I fixed the cause the of the failure before I even finalized the commit for the merge. Strangely, the test only failed in one configuration (Mac / clang / release) and passed in all the others. Unfortunately, there's no way (that I know of) to obtain more detailed information about what failed.

Considering how finicky clang has been on Travis lately, all I can think of doing is telling Travis to rerun it. If it fails again, then we'll need to find someone who can test that configuration on their own machine to get us more details about what's failing.

@mxgrey
Copy link
Member

mxgrey commented May 27, 2015

I've now rerun it, and the same failure pops up. It's definitely a problem that originated in memory_management.

It only fails on Mac / Clang / release, so if anyone has a Mac that they can test it on, that would be very helpful. My best guess is that there are some differences in the implementation of rand() between platforms, and one of the randomized tests that gets produced by the Mac / Clang combination is just a little bit outside of the tolerance that we have set.

@scpeters
Copy link
Collaborator

/Users/scpeters/ws/gazebo_dart/src/dart/unittests/testDynamics.cpp:1660: Failure
Value of: equals(deltaVel1, deltaVel2, 1e-5)
  Actual: false
Expected: true

@scpeters
Copy link
Collaborator

Also, use make test "ARGS=-VV" to see all the console out from test files.

@mxgrey
Copy link
Member

mxgrey commented May 27, 2015

Thanks, @scpeters ! Did it also print out the values of detlaVel1 and deltaVel2? Are they far off, or just barely shy of a tolerance of 1e-5?

Next time this happens, I'll push a commit with make test "ARGS=-VV".

@mkoval
Copy link
Collaborator Author

mkoval commented May 27, 2015

@mxgrey Besides the testDynamics issues, do you have any comments on this pull request? 😄

@mxgrey
Copy link
Member

mxgrey commented May 27, 2015

Looks good. Does the path name only matter for URDF? I could add it to the Skel and SDF parsers as well.

@mkoval
Copy link
Collaborator Author

mkoval commented May 27, 2015

We should add it anywhere that we load meshes from disk (which includes the skel and sdf parsers). I just forgot about them because I only use the URDF loader. 😆

mxgrey added a commit that referenced this pull request May 27, 2015
@mxgrey mxgrey merged commit c127038 into dartsim:memory_management May 27, 2015
@mxgrey
Copy link
Member

mxgrey commented May 27, 2015

I can merge it in and add it to the other parsers.

@scpeters
Copy link
Collaborator

@mxgrey sorry I didn't paste the whole output. I just looked for the failure and pasted that. I also deleted the build already. IMO, the make test "ARGS=-VV" is just a good idea anyway to maximize the usefulness of travis.

@jslee02 jslee02 added this to the Release DART 5.0 milestone Jun 16, 2015
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.

4 participants