Skip to content

Migrate to ROS2#10

Open
chuoru wants to merge 58 commits intoisri-aist:masterfrom
chuoru:master
Open

Migrate to ROS2#10
chuoru wants to merge 58 commits intoisri-aist:masterfrom
chuoru:master

Conversation

@chuoru
Copy link
Copy Markdown

@chuoru chuoru commented Mar 25, 2025

@ThomasDuvinage @mmurooka
With @nqtabokado, we migrated this repo to ROS2. The main modification is in the way of using subproject since ament_cmake (colcon) is unable to add_subproject like catkin anymore.

All actions are confirmed with correct number of test case (previously in https://github.com/isri-aist/NMPC/actions only 3 test cases are executed).
https://github.com/chuoru/NMPC/actions/runs/14055514181
https://github.com/chuoru/NMPC/actions/runs/14055514146
https://github.com/chuoru/NMPC/actions/runs/14055514128

Comment thread nmpc_cgmres/CMakeLists.txt Outdated
endif()

if(NOT NMPC_STANDALONE)
if(NOT EXISTS "${CMAKE_CURRENT_BINARY_DIR}/install_manifest.txt")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why is this needed ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@ThomasDuvinage We have problem with warning about missing install_manifest.txt. This is our solution. However, I saw that you have fixed this problem in QpSolverCollection, can you share with us?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As mentioned in ament documentation, ament_package should be called at the end of the CMakeLists.txt. Sorry about that I missed that last time.

In order for other ament package to found the generated library, you need to export it using ament cmake functions.

Please have a look to :

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@chuoru the problem here is caused by add_project_dependency(...)

I didn't look deep into it but removing it helped to solve the issue.

Comment thread standalone/CMakeLists.txt
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

CMakeLists.txt must be kept in place. Otherwise, compiling NMPC as a standalone from mc-rtc-superbuild for example will not be possible. Furthermore this doesn't respect Modern Cmake standard.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@ThomasDuvinage This will be the most problematic thing to modify. We found out that ament_cmake (colcon) doesn't consider add_subproject in case the CMakeLists.txt in NMPC folder. This makes ROS2 only recognize NMPC as sole packages not set of packages. I will investigate a bit more about this problem and mc-rtc-superbuild

Comment thread nmpc_fmpc/CMakeLists.txt Outdated
find_package(Eigen3 REQUIRED)
include_directories(${EIGEN3_INCLUDE_DIR})

if(NOT EXISTS "${CMAKE_CURRENT_BINARY_DIR}/install_manifest.txt")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same question

Comment thread nmpc_fmpc/CMakeLists.txt Outdated
Comment thread nmpc_ddp/CMakeLists.txt Outdated
Comment thread .github/workflows/ci-colcon.yaml Outdated
Comment thread nmpc_cgmres/doc/Doxyfile.extra.in
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same comment as before

Comment thread nmpc_ddp/package.xml
option(INSTALL_DOCUMENTATION "Generate and install the documentation" OFF)

include(../cmake/base.cmake)
project(nmpc_cgmres LANGUAGES CXX)
Copy link
Copy Markdown

@nqtabokado nqtabokado Apr 14, 2025

Choose a reason for hiding this comment

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

@ThomasDuvinage why don't you want to use include(../cmake/base.cmake) (submodule) anymore?
After you change install(DIRECTORY include/nmpc_ddp DESTINATION "${INCLUDE_INSTALL_DIR}") to install(DIRECTORY include/nmpc_ddp DESTINATION include), ctest can't found test.
image

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@ThomasDuvinage I fixed error Ctest No tests were found after you remove submodule and implement CI colcon standalone in PR: chuoru#4
Please take a look when @chuoru have time. Thanks !

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.

3 participants