Skip to content

Make SISL optional #121

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

Closed
wants to merge 3 commits into from
Closed

Make SISL optional #121

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cb55baa
sisl optional
Sep 27, 2018
b8f4a85
also exclude trajectory
Oct 9, 2018
44e6b60
add include directories for sisl and link against sisl
Oct 9, 2018
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
41 changes: 29 additions & 12 deletions src/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,22 @@

find_package(SISL)
Copy link
Member

@doudou doudou Oct 19, 2018

Choose a reason for hiding this comment

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

I would rather that this is controlled by an explicit option (i.e. WITH_SISL), to avoid having people getting an unexpectedly crippled base/types.

Copy link
Member

@chhtz chhtz Oct 19, 2018

Choose a reason for hiding this comment

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

Adding an option would be fine for me.

However, I don't understand how this should interact with <depend package="external/sisl" optional="1" /> from package.xml. If the dependency is not found, would it still be mandatory to manually set the option?

Perhaps we could always install Spline.hpp and Trajectory.hpp, but have it generate an #error, if SISL was not found?

Copy link
Author

@arneboe arneboe Oct 22, 2018

Choose a reason for hiding this comment

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

Perhaps we could always install Spline.hpp and Trajectory.hpp, but have it generate an #error, if SISL was not found?

This shifts the error to the compile phase instead of the configuration phase where it should occur.

In general I am fine with an option but it conflicts with having optional=1 in the manifest.

Copy link
Member

@doudou doudou Oct 22, 2018

Choose a reason for hiding this comment

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

In general I am fine with an option but it conflicts with having optional=1 in the manifest.

Not necessarily. Autoproj will have to set the flag if sisl is not excluded, which would be done with

unless Autoproj.manifest.excluded?('external/sisl')

end

Within the package definition block in rock.core's 02stable.autobuild

Note that the package_enabled? call is actually deprecated since Autoproj v2. To facilitate the transition between v1 and v2, I disabled the deprecation warnings by default and then forgot I did :(


set(sisl_hpp "")
set(sisl_cpp "")

#compile without Spline and Trajectory if SISL is not found
if(SISL_FOUND)
set(sisl_hpp Spline.hpp
Trajectory.hpp)
set(sisl_cpp Spline.cpp
Trajectory.cpp)
include_directories(${SISL_INCLUDE_DIRS})
else()
message(WARNING "SISL not found. Compiling without spline and trajectory support!")
endif()

rock_library(
base-types
base-types
Angle.cpp
JointLimitRange.cpp
JointLimits.cpp
Expand All @@ -8,12 +25,11 @@ rock_library(
JointTransform.cpp
Pose.cpp
Pressure.cpp
Spline.cpp
${sisl_cpp}
Temperature.cpp
Time.cpp
TimeMark.cpp
Timeout.cpp
Trajectory.cpp
TransformWithCovariance.cpp
TwistWithCovariance.cpp
Waypoint.cpp
Expand Down Expand Up @@ -48,13 +64,12 @@ rock_library(
Point.hpp
Pose.hpp
Pressure.hpp
Singleton.hpp
Spline.hpp
Singleton.hpp
Temperature.hpp
Time.hpp
${sisl_hpp}
TimeMark.hpp
Timeout.hpp
Trajectory.hpp
TransformWithCovariance.hpp
TwistWithCovariance.hpp
Waypoint.hpp
Expand All @@ -63,7 +78,7 @@ rock_library(
commands/Motion2D.hpp
commands/Speed6D.hpp
commands/LinearAngular6DCommand.hpp
logging/logging_iostream_style.h
logging/logging_iostream_style.h
logging/logging_printf_style.h
samples/BodyState.hpp
samples/CommandSamples.hpp
Expand All @@ -84,15 +99,17 @@ rock_library(
samples/Wrench.hpp
samples/Wrenches.hpp
templates/TimeStamped.hpp
DEPS_CMAKE
SISL
DEPS_PKGCONFIG
DEPS_PKGCONFIG
base-logging
eigen3
)

install(FILES ${CMAKE_SOURCE_DIR}/src/Spline.hpp
DESTINATION include/base/geometry)
if(SISL_FOUND)
target_link_libraries(base-types ${SISL_LIBRARIES})

install(FILES ${CMAKE_SOURCE_DIR}/src/Spline.hpp
DESTINATION include/base/geometry)
endif()

configure_file(base-lib.pc.in ${CMAKE_BINARY_DIR}/base-lib.pc @ONLY)
install(FILES ${CMAKE_BINARY_DIR}/base-lib.pc DESTINATION lib/pkgconfig)