-
Notifications
You must be signed in to change notification settings - Fork 486
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 tests related to shininess #3223
Conversation
The logic in World::LoadModel currently creates an empty link in models without links by calling GetElement("link") without checking if such an element exists. This checks HasElement("link") before calling GetElement, which fixes a nested model test. Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
The pr2 test was failing due to timeouts in the shininess service call. Move the call from Visual::Load to Visual::Init to allow the service advertisement in World::Load some time to take effect. Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@traversaro what does it take to use libsdformat 9.8.0 in condo-forge? |
the tests are all fixed in the jenkins ubuntu and homebrew builds, but condo-forge is failing due to a lack of libsdformat 9.8 |
We need to manually bump the v9 branch as unfortunately we do not have the autotick bot for non-default branches. I can do it in ~1 hour. |
Strange, apparently conda-forge should already contain 9.8, see https://anaconda.org/conda-forge/libsdformat and https://github.com/conda-forge/libsdformat-feedstock/blob/9bda5c89b3bf47bab6169bc7add488f4c710e336/recipe/meta.yaml#L2 . I need to check what is happening. |
I tried to play removing the pin from qt and to forge the installation of latest sdformat, but this happens:
Probably there is some dependency that is not properly tracked in the pinned version in conda-forge, I will need to check a bit more. |
Ok, I was able to reproduce the problem locally. If I try to force the installation of a recent sdformat this happens:
If I remove
So the latest sdformat 9.8 was not build with urdfdom 3.1 . I can trigger a build to force this, but in the mean time you can just remove dartsim (that in any case is an optional dependency) from the dependencies installed. |
Workaround for an issue with libsdformat 9.8 Signed-off-by: Steve Peters <scpeters@openrobotics.org>
I've removed |
Thanks. I opened two PRs to hopefully avoid this problem in the future:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Follow-up to gazebosim/gazebo-classic#3213 and gazebosim/gazebo-classic#3223 Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Follow-up to gazebosim/gazebo-classic#3213 and gazebosim/gazebo-classic#3223 Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Reverts a portion of #3223 that broke shininess functionality. Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Reverts a portion of #3223 that broke shininess functionality. Signed-off-by: Steve Peters <scpeters@openrobotics.org>
* Add Visual::Shininess accessor and corresponding visual_shininess integration test that confirms shininess values are properly added to Visuals in the shapes_shininess example world. * Fix the test by reverting part of #3223, moving the shininess service call back to Visual::Load. A different fix for the PR2 tests is made by checking for existence of shininess service before calling. Signed-off-by: Steve Peters <scpeters@openrobotics.org>
In #3223 (comment) we added some workarounds for the conda-forge CI, namely pin the version of some dependencies and removed dartsim. Now that the long term fixes to that problems (linked in #3223 (comment)) has been fixed, we can remove the workarounds.
The material shininess features added in #3213 introduced a few test regressions that we difficult to notice due to the large number of existing test failures in our CI. Having been able to observe several sets of builds since then, I've noticed the following test failures that I believe we introduced:
The
PhysicsEngines/NestedModelTest.SpawnNestedModel/*
tests (and possiblyModelListWidget_TEST.NestedLinkProperties
) should be fixed by b29d38c (diff without whitespace). The remaining tests should be fixed by 6c4c188.