Port towards jrl-v2 - #2923
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
👋 Hi,
This is a reminder message to assign an extra build label to this Pull Request if needed.
By default, this PR will be build with minimal build options (URDF support and Python bindings)
The possible extra labels are:
- build_collision (build Pinocchio with coal support)
- build_casadi (build Pinocchio with CasADi support)
- build_autodiff (build Pinocchio with CppAD support)
- build_codegen (build Pinocchio with CppADCodeGen support)
- build_extra (build Pinocchio with extra algorithms)
- build_mpfr (build Pinocchio with Boost.Multiprecision support)
- build_sdf (build Pinocchio with SDF parser)
- build_accelerate (build Pinocchio with APPLE Accelerate framework support)
- build_all (build Pinocchio with ALL the options stated above)
Thanks.
The Pinocchio development team.
| # TODO why not neccessary in Simple | ||
| set(Boost_USE_DEBUG_RUNTIME OFF) |
There was a problem hiding this comment.
We should investigate this issue before merging
aeb3d24 to
a11965f
Compare
| list(APPEND SDF_VERSIONS "") | ||
| foreach(version IN LISTS SDF_VERSIONS) | ||
| find_package(SDFormat${version} QUIET) | ||
| if(SDFormat${version}_FOUND) | ||
| set(SDFormat_FOUND True) | ||
| jrl_find_package(SDFormat${version} CONFIG) | ||
| message(STATUS "SDFormat${version} Found") | ||
| add_library(pinocchio_sdformat ALIAS sdformat${version}::sdformat${version}) | ||
| break() | ||
| endif() | ||
| endforeach(version) | ||
| if(NOT SDFormat_FOUND) | ||
| message( | ||
| FATAL_ERROR | ||
| "SDFormat not found. Accepted versions: ${SDF_VERSIONS}" | ||
| ) | ||
| endif() |
There was a problem hiding this comment.
This should be done in the find-package part of the cmake.
There was a problem hiding this comment.
no, now it is pushed
| ) | ||
|
|
||
| add_pinocchio_unit_test(value) | ||
| if(BUILD_WITH_COLLISION_SUPPORT) |
There was a problem hiding this comment.
Instead of doing:
if(BUILD_WITH_URDF_SUPPORT)
...
if(BUILD_WITH_COLLISION_SUPPORT)
...
endif()
endif()We can try to do this:
if(BUILD_WITH_URDF_SUPPORT)
...
endif()
if(BUILD_WITH_URDF_SUPPORT and BUILD_WITH_COLLISION_SUPPORT)
...
endif()I think this improve readability.
| add_pinocchio_unit_test(compute-all-terms) | ||
| add_pinocchio_unit_test(energy) | ||
| add_pinocchio_unit_test(frames) | ||
| if(NOT MSVC AND NOT MSVC_VERSION) |
There was a problem hiding this comment.
TODO: try to remove this condition before merging
| endif() | ||
|
|
||
| if(BUILD_WITH_AUTODIFF_SUPPORT) | ||
| macro(ADD_CPPAD_UNIT_TEST name) |
There was a problem hiding this comment.
TODO: I think the macro is useless. Linking againts pinocchio should do the job
| add_dependencies(pinocchio-test-cpp-cppad pinocchio-test-cpp-${name}) | ||
| target_link_libraries( | ||
| pinocchio-test-cpp-${name} | ||
| PRIVATE pinocchio_cppad_headers |
There was a problem hiding this comment.
| PRIVATE pinocchio_cppad_headers | |
| PRIVATE pinocchio_cppad |
If we link against pinocchio_cppad_headers, we will not benefit from explicit template instantiation.
There was a problem hiding this comment.
does not link otherwise
| ) | ||
| target_link_libraries( | ||
| pinocchio-test-cpp-${name} | ||
| PRIVATE pinocchio_cppadcg_headers |
There was a problem hiding this comment.
| PRIVATE pinocchio_cppadcg_headers | |
| PRIVATE pinocchio_cppadcg |
There was a problem hiding this comment.
does not link otherwise
| ) | ||
| target_link_libraries( | ||
| pinocchio-test-cpp-${name} | ||
| PRIVATE pinocchio_casadi_headers |
There was a problem hiding this comment.
| PRIVATE pinocchio_casadi_headers | |
| PRIVATE pinocchio_casadi |
There was a problem hiding this comment.
does not link otherwise
| # | ||
| include(${JRL_CMAKE_MODULES}/python-helpers.cmake) | ||
|
|
||
| set(${PROJECT_NAME}_PYTHON_TESTS |
There was a problem hiding this comment.
Instead of doing a list and calling pinocchio_python_add_test in a foreach loop we can directly call pinocchio_python_add_test on each entry.
There was a problem hiding this comment.
Like in C++ no need to put that in a separated directory.
| PATTERN "*.so" | ||
| PATTERN "*.pyd" |
There was a problem hiding this comment.
Guilhem commented on another repo that we need to install the bindings as TARGETS for RPATH to work properly (which does not when installed like this via directory install).
So we need to add a install(TARGETS .... DESTINATION ${python_install_dir}/pinocchio)
And remove the PATTERN that installs *.so and *.pyd
d1a766a to
eccdf5a
Compare
eccdf5a to
f268452
Compare
3 participants
Description
Port pinocchio towards jrl-v2
Checklist
pre-commit run --all-filesorpixi run lintTODO
development/release.mdto use jrl-release.pycmake/get-example-robot-data.cmakescript to be able to fetch content example-robot-data (back compatibility)-[ ] support static library -> closes #2918
closes #2906