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.

Sign up for GitHub

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

dartsim: make gz-common geospatial component optional #516

Closed
wants to merge 2 commits into from
Closed

dartsim: make gz-common geospatial component optional #516

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cc9cced
Check that plugin exists before adding common test
scpeters Jun 23, 2023
8ba5edc
Make gz-common geospatial component optional
scpeters Jun 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,9 @@ message(STATUS "\n\n-- ====== Finding Dependencies ======")
#--------------------------------------
# Find gz-common
gz_find_package(gz-common5
COMPONENTS geospatial graphics profiler
REQUIRED_BY heightmap mesh dartsim tpe tpelib bullet)
COMPONENTS graphics profiler
OPTIONAL_COMPONENTS geospatial
REQUIRED_BY heightmap mesh dartsim tpe tpelib bullet bullet-featherstone)
set(GZ_COMMON_VER ${gz-common5_VERSION_MAJOR})

#--------------------------------------
Expand Down
16 changes: 12 additions & 4 deletions dartsim/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ install(
DESTINATION "${GZ_INCLUDE_INSTALL_DIR_FULL}")

gz_get_libsources_and_unittests(sources test_sources)
if(NOT gz-common${GZ_COMMON_VER}-geospatial_FOUND)
list(REMOVE_ITEM sources src/CustomHeightmapShape.cc)
endif()

# TODO(MXG): Think about a gz_add_plugin(~) macro for gz-cmake
set(engine_name dartsim-plugin)
Expand All @@ -28,15 +31,22 @@ target_link_libraries(${dartsim_plugin}
PUBLIC
${features}
${PROJECT_LIBRARY_TARGET_NAME}-sdf
${PROJECT_LIBRARY_TARGET_NAME}-heightmap
${PROJECT_LIBRARY_TARGET_NAME}-mesh
gz-common${GZ_COMMON_VER}::gz-common${GZ_COMMON_VER}
gz-common${GZ_COMMON_VER}::geospatial
gz-math${GZ_MATH_VER}::eigen3
PRIVATE
# We need to link this, even when the profiler isn't used to get headers.
gz-common${GZ_COMMON_VER}::profiler
)
if(gz-common${GZ_COMMON_VER}-geospatial_FOUND)
target_link_libraries(${dartsim_plugin}
PUBLIC
${PROJECT_LIBRARY_TARGET_NAME}-heightmap
gz-common${GZ_COMMON_VER}::geospatial)
target_compile_definitions(${dartsim_plugin}
PRIVATE
GZ_COMMON_GEOSPATIAL_FOUND)
endif()

# The Gazebo fork of DART contains additional code that allows customizing
# contact constraints. We check for the presence of "ContactSurface.hpp", which
Expand Down Expand Up @@ -86,9 +96,7 @@ gz_build_tests(
${dartsim_plugin}
gz-plugin${GZ_PLUGIN_VER}::loader
gz-common${GZ_COMMON_VER}::gz-common${GZ_COMMON_VER}
gz-common${GZ_COMMON_VER}::geospatial
${PROJECT_LIBRARY_TARGET_NAME}-sdf
${PROJECT_LIBRARY_TARGET_NAME}-heightmap
${PROJECT_LIBRARY_TARGET_NAME}-mesh
TEST_LIST tests)

Expand Down
4 changes: 4 additions & 0 deletions dartsim/src/EntityManagement_TEST.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@

#include <gz/plugin/Loader.hh>

#ifdef GZ_COMMON_GEOSPATIAL_FOUND
#include <gz/common/geospatial/Dem.hh>
#include <gz/common/geospatial/ImageHeightmap.hh>
#endif
#include <gz/common/MeshManager.hh>
#include <gz/common/Filesystem.hh>

Expand Down Expand Up @@ -208,6 +210,7 @@ TEST(EntityManagement_TEST, ConstructEmptyWorld)
EXPECT_NEAR(meshShapeScaledSize[1], 0.3831, 1e-4);
EXPECT_NEAR(meshShapeScaledSize[2], 0.0489, 1e-4);

#ifdef GZ_COMMON_GEOSPATIAL_FOUND
// image heightmap
auto heightmapLink = model->ConstructEmptyLink("heightmap_link");
heightmapLink->AttachFixedJoint(child, "heightmap_joint");
Expand Down Expand Up @@ -266,6 +269,7 @@ TEST(EntityManagement_TEST, ConstructEmptyWorld)
EXPECT_NEAR(sizeDem.X(), demShapeRecast->GetSize()[0], 1e-3);
EXPECT_NEAR(sizeDem.Y(), demShapeRecast->GetSize()[1], 1e-3);
EXPECT_NEAR(sizeDem.Z(), demShapeRecast->GetSize()[2], 1e-6);
#endif
}

TEST(EntityManagement_TEST, RemoveEntities)
Expand Down
4 changes: 4 additions & 0 deletions dartsim/src/ShapeFeatures.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@
#include <gz/common/Mesh.hh>
#include <gz/common/MeshManager.hh>

#ifdef GZ_COMMON_GEOSPATIAL_FOUND
#include "CustomHeightmapShape.hh"
#endif
#include "CustomMeshShape.hh"

namespace gz {
Expand Down Expand Up @@ -325,6 +327,7 @@ Identity ShapeFeatures::AttachSphereShape(
return this->GenerateIdentity(shapeID, this->shapes.at(shapeID));
}

#ifdef GZ_COMMON_GEOSPATIAL_FOUND
//////////////////////////////////////////////////
Identity ShapeFeatures::CastToHeightmapShape(
const Identity &_shapeID) const
Expand Down Expand Up @@ -376,6 +379,7 @@ Identity ShapeFeatures::AttachHeightmapShape(
const std::size_t shapeID = this->AddShape({sn, _name});
return this->GenerateIdentity(shapeID, this->shapes.at(shapeID));
}
#endif

/////////////////////////////////////////////////
Identity ShapeFeatures::CastToMeshShape(
Expand Down
6 changes: 6 additions & 0 deletions dartsim/src/ShapeFeatures.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@
#include <gz/physics/CapsuleShape.hh>
#include <gz/physics/CylinderShape.hh>
#include <gz/physics/EllipsoidShape.hh>
#ifdef GZ_COMMON_GEOSPATIAL_FOUND
#include <gz/physics/heightmap/HeightmapShape.hh>
#endif
#include <gz/physics/mesh/MeshShape.hh>
#include <gz/physics/PlaneShape.hh>
#include <gz/physics/SphereShape.hh>
Expand Down Expand Up @@ -67,9 +69,11 @@ struct ShapeFeatureList : FeatureList<
// SetSphereShapeProperties,
AttachSphereShapeFeature,

#ifdef GZ_COMMON_GEOSPATIAL_FOUND
heightmap::GetHeightmapShapeProperties,
// heightmap::SetHeightmapShapeProperties,
heightmap::AttachHeightmapShapeFeature,
#endif

mesh::GetMeshShapeProperties,
// mesh::SetMeshShapeProperties,
Expand Down Expand Up @@ -165,6 +169,7 @@ class ShapeFeatures :
const Pose3d &_pose) override;


#ifdef GZ_COMMON_GEOSPATIAL_FOUND
// ----- Heightmap Features -----
public: Identity CastToHeightmapShape(
const Identity &_shapeID) const override;
Expand All @@ -179,6 +184,7 @@ class ShapeFeatures :
const Pose3d &_pose,
const LinearVector3d &_size,
int _subSampling) override;
#endif

// ----- Mesh Features -----
public: Identity CastToMeshShape(
Expand Down
4 changes: 4 additions & 0 deletions heightmap/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
if(NOT gz-common${GZ_COMMON_VER}-geospatial_FOUND)
# Skip adding this component
return()
endif()
gz_add_component(heightmap INTERFACE
GET_TARGET_NAME heightmap)

Expand Down
38 changes: 18 additions & 20 deletions test/common_test/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,21 +24,23 @@ set(TEST_INSTALL_DIR ${CMAKE_INSTALL_LIBEXECDIR}/gz/${GZ_DESIGNATION}${PROJECT_V
include(GzPython)

function(configure_common_test PHYSICS_ENGINE_NAME test_name)
set(target_name ${test_name}_${PHYSICS_ENGINE_NAME})
add_test(NAME ${target_name}
COMMAND
${test_name}
$<TARGET_FILE:${PROJECT_LIBRARY_TARGET_NAME}-${PHYSICS_ENGINE_NAME}-plugin>
--gtest_output=xml:${CMAKE_BINARY_DIR}/test_results/${target_name}.xml
)
if(NOT SKIP_${PHYSICS_ENGINE_NAME} AND NOT INTERNAL_SKIP_${PHYSICS_ENGINE_NAME})
Copy link
Contributor

Choose a reason for hiding this comment

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

Is INTERNAL_SKIP_* part of the public API of gz-cmake? The prefix INTERNAL_ made me think it wasn't, but I'm not sure.

Copy link
Member Author

Choose a reason for hiding this comment

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

The INTERNAL_SKIP_* variables are set when dependencies that are REQUIRED_BY a particular component are not found (see GzFindPackage.cmake). It's not documented behavior, so it's brittle to depend on, but I couldn't figure out a better way, other than reproducing all the REQUIRED_BY logic within this file, which would be a pain. I initially tried if(TARGET ${PROJECT_LIBRARY_TARGET_NAME}-${PHYSICS_ENGINE_NAME}-plugin), but that didn't work because the test folder is added by GzConfigureBuild.cmake before the component folders are added and the corresponding targets created. Reversing this order in gz-cmake is possible, but it may cause other problems, and would require a gz-cmake release. Using INTERNAL_SKIP_* seems least bad to me, though it's partly moot if we can't apply a similar change to gz-rendering

set(target_name ${test_name}_${PHYSICS_ENGINE_NAME})
add_test(NAME ${target_name}
COMMAND
${test_name}
$<TARGET_FILE:${PROJECT_LIBRARY_TARGET_NAME}-${PHYSICS_ENGINE_NAME}-plugin>
--gtest_output=xml:${CMAKE_BINARY_DIR}/test_results/${target_name}.xml
)

set_tests_properties(${target_name} PROPERTIES TIMEOUT 240)
set_tests_properties(${target_name} PROPERTIES TIMEOUT 240)

if(Python3_Interpreter_FOUND)
# Check that the test produced a result and create a failure if it didn't.
# Guards against crashed and timed out tests.
add_test(check_${target_name} ${Python3_EXECUTABLE} ${GZ_CMAKE_TOOLS_DIR}/check_test_ran.py
${CMAKE_BINARY_DIR}/test_results/${target_name}.xml)
if(Python3_Interpreter_FOUND)
# Check that the test produced a result and create a failure if it didn't.
# Guards against crashed and timed out tests.
add_test(check_${target_name} ${Python3_EXECUTABLE} ${GZ_CMAKE_TOOLS_DIR}/check_test_ran.py
${CMAKE_BINARY_DIR}/test_results/${target_name}.xml)
endif()
endif()
endfunction()

Expand Down Expand Up @@ -66,12 +68,8 @@ foreach(test ${tests})

install(TARGETS ${test_executable} DESTINATION ${TEST_INSTALL_DIR})

if (${BULLET_FOUND})
configure_common_test("bullet" ${test_executable})
configure_common_test("bullet-featherstone" ${test_executable})
endif()
if (${DART_FOUND})
configure_common_test("dartsim" ${test_executable})
endif()
configure_common_test("bullet" ${test_executable})
configure_common_test("bullet-featherstone" ${test_executable})
configure_common_test("dartsim" ${test_executable})
configure_common_test("tpe" ${test_executable})
endforeach()