Skip to content

Add nested src/CMakeLists.txt with sas_core_pure library + fix Docker compose - #10

Merged
mmmarinho merged 8 commits into
jazzyfrom
refactor/nested-src-cmake
Aug 15, 2026
Merged

Add nested src/CMakeLists.txt with sas_core_pure library + fix Docker compose#10
mmmarinho merged 8 commits into
jazzyfrom
refactor/nested-src-cmake

Conversation

@mmmarinho

Copy link
Copy Markdown
Member

This PR introduces two improvements:

1. Nested src/CMakeLists.txt with sas_core_pure library

All C++ sources in src/ are now built via add_subdirectory(src) into a static library target sas_core_pure. This library contains all non-ROS2 C++ code:

  • sas_clock.cpp, sas_core.cpp, sas_object.cpp, sas_shutdown_signaler.cpp
  • sas_robot_driver.cpp, examples/sas_robot_driver_example.cpp
  • eigen3_std_conversions.cpp

The parent CMakeLists.txt creates an INTERFACE alias sas_core that transitively links to sas_core_pure, so external ROS packages that depend on sas_core continue to work unchanged.

2. Fix docker/compose.yml for container environments

The original bind mount (../../sas_core:/root/sas_core_devel/src/sas_core) fails when Docker Desktop cannot access the container's internal filesystem. Replaced with COPY in the Dockerfile so the source is baked into the image at build time. All 6 ROS2 examples (C++ and Python) build and run successfully.


This PR was created by an AI agent (OpenHands) on behalf of the user.

… compose for container environments

- Create src/CMakeLists.txt that builds all non-ROS2 C++ sources into a
  static library target `sas_core_pure` with -fPIC.

- Refactor parent CMakeLists.txt: delegate source compilation to
  add_subdirectory(src) and keep an INTERFACE alias `sas_core` so external
  ROS packages can still link against the original target name.

- Remove the bind-mount from docker/compose.yml and COPY the source into
  the Dockerfile instead, since Docker Desktop cannot access container
  internal paths.

- All 6 ROS2 examples (C++ and Python) build and run successfully.
… parent CMakeLists.txt

- Relocate src/CMakeLists.txt to cmake/sas_core_pure.cmake
- Switch from add_subdirectory(src) to include(cmake/sas_core_pure.cmake)
- Use CMAKE_CURRENT_LIST_DIR for correct path resolution in included file
- Update test_consumer to link against sas_core::sas_core_pure target
- Fix Docker COPY to include full build context
@mmmarinho

Copy link
Copy Markdown
Member Author

CMake refactoring: sas_core_pure moved to cmake/sas_core_pure.cmake

What changed:

  • Relocated the nested src/CMakeLists.txt to cmake/sas_core_pure.cmake in the repository root
  • Parent CMakeLists.txt now uses include(cmake/sas_core_pure.cmake) instead of add_subdirectory(src)
  • All paths in the .cmake file use CMAKE_CURRENT_LIST_DIR to correctly resolve relative paths when included (rather than CMAKE_CURRENT_SOURCE_DIR)

Additional fixes in this commit:

  • Fixed test_consumer to link against sas_core::sas_core_pure (the exported namespaced target)
  • Fixed Docker COPY command to include full build context

Docker Compose builds both packages and runs all examples + test_consumer successfully.

Commented by an AI agent (OpenHands) on behalf of the user.

- Rename cmake/sas_core_pure.cmake to cmake/cpplib.cmake
- Extract Python pybind11 wrapper to cmake/pythonlib.cmake
- Add ROS2_BUILD option (ON by default) in main CMakeLists.txt
- When ROS2_BUILD=OFF: only builds cpplib + pythonlib targets
- When ROS2_BUILD=ON: full ament build with examples and exports
- Python module install path adapts to ROS2 (ament) or plain CMake context
@mmmarinho

Copy link
Copy Markdown
Member Author

CMake restructuring: cpplib.cmake, pythonlib.cmake, and ROS2_BUILD option

What changed:

1. Renamed cmake/sas_core_pure.cmake to cmake/cpplib.cmake

  • Cleaner name reflecting it is the platform-independent C++ library.

2. Extracted Python wrapper into cmake/pythonlib.cmake

  • Moved pybind11 setup and _sas_core module build from the parent CMakeLists.txt into its own included module.
  • Install path adapts automatically: uses ament PYTHON_INSTALL_DIR when ROS2_BUILD=ON, falls back to standard Python site-packages otherwise.

3. Added ROS2_BUILD CMake option (default: ON)

  • Parent CMakeLists.txt now has option(ROS2_BUILD ...) — when disabled, only the pure C++ library (cpplib.cmake) and Python bindings (pythonlib.cmake) are built.
  • The ROS2-specific section (ament exports, example executables, ament_package()) is gated behind if(ROS2_BUILD).
  • This enables consuming sas_core_pure from non-ROS2 CMake projects via include(cmake/cpplib.cmake).

Docker Compose continues to build both packages (sas_core + test_consumer) and run all examples successfully.

Commented by an AI agent (OpenHands) on behalf of the user.

…ackage

- Add test_consumer/test/test_python_import.py: verifies sas_core Python
  bindings (Clock, Statistics, RobotDriver, ShutdownSignaler) import
  correctly from a downstream ROS2 package
- Create standalone_consumer/: temporary plain-CMake package (no ROS2/ament)
  that includes cmake/cpplib.cmake and cmake/pythonlib.cmake to validate
  sas_core can be consumed outside of ROS2
- standalone_consumer/src/standalone_consumer.cpp: links sas_core_pure
  and exercises sas::concatenate, sas::incremental_mean, ShutdownSignaler
- standalone_consumer/test/test_python_import.py: validates Python
  bindings from the non-ROS2 build
- Fix pythonlib.cmake to always emit _sas_core module (not project-name
  dependent) so the import works regardless of consuming project
- Fix pythonlib.cmake add_subdirectory to provide binary dir for
  out-of-tree includes
- Update Dockerfile to copy standalone_consumer outside colcon workspace
- Update compose.yml to build standalone_consumer with plain cmake and
  run both Python import tests
@mmmarinho

Copy link
Copy Markdown
Member Author

Python import tests and standalone non-ROS2 CMake test package

test_consumer Python import test

  • Added test_consumer/test/test_python_import.py that imports Clock, Statistics, RobotDriver, and ShutdownSignaler from sas_core to verify the Python bindings work from a downstream ROS2 package.
  • Updated test_consumer/CMakeLists.txt to register the test via add_test().

standalone_consumer -- non-ROS2 CMake test package

  • New standalone_consumer/ directory: a plain CMake project (no package.xml, no ament) that validates cpplib.cmake and pythonlib.cmake can be included and used outside of ROS2.
  • CMakeLists.txt sets SAS_CORE_SOURCE_DIR to locate the sas_core cmake modules, then includes cpplib.cmake and pythonlib.cmake.
  • src/standalone_consumer.cpp links against sas_core_pure and exercises sas::concatenate, sas::incremental_mean, and ShutdownSignaler.
  • test/test_python_import.py verifies the Python bindings import from the non-ROS2 build.
  • Docker Compose builds this with plain cmake/make (outside the colcon workspace) and runs both the C++ binary and the Python test.

Fixes in pythonlib.cmake

  • Module name is now always _sas_core (not _PROJECT_NAME) so the import works regardless of which project includes pythonlib.cmake.
  • add_subdirectory() now provides an explicit binary directory to support out-of-tree includes.

All four test stages pass in Docker Compose:

  1. ROS2 examples (C++ + Python) from sas_core
  2. test_consumer C++ binary
  3. test_consumer Python import test
  4. standalone_consumer C++ binary + Python import test (no ROS2)

Commented by an AI agent (OpenHands) on behalf of the user.

@mmmarinho mmmarinho left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Originally intended files are ok, but the tests need to be better organized if added in this PR. Maybe better to leave it for another one.

- Remove test_consumer/ and standalone_consumer/: temporary validation
  packages no longer needed in the PR
- Simplify cpplib.cmake: use target_sources() instead of inline source
  list (CMake modern convention), remove redundant temporary variables
- Simplify pythonlib.cmake: hardcode module name _sas_core directly,
  clean up install path logic, remove verbose comment blocks
- Simplify root CMakeLists.txt: use foreach(IN ITEMS ...) loop for
  example executables, remove duplicate find_package(Eigen3) (kept in
  cpplib.cmake), remove redundant ament_export_include_directories()
  (target exports already propagate include dirs), clean up section
  headers
- Restore Dockerfile and compose.yml: removed test package references
@mmmarinho

Copy link
Copy Markdown
Member Author

Simplified CMake per best practices and removed temporary test packages

Removed

  • test_consumer/ and standalone_consumer/ -- these were temporary validation packages used to verify the refactored CMake structure works. They are no longer needed in the PR.
  • Dockerfile and compose.yml references to those packages.

CMake simplifications

cpplib.cmake

  • Switched to target_sources() instead of inline source list in add_library() (modern CMake convention).
  • Removed redundant temporary variables.

pythonlib.cmake

  • Hardcoded module name as _sas_core directly (removed indirection variable).
  • Cleaned up install path logic and removed verbose comment blocks.

root CMakeLists.txt

  • Added foreach(IN ITEMS ...) loop for example executables to reduce boilerplate.
  • Removed duplicate find\_package(Eigen3) from root (only needed in cpplib.cmake where it is a dependency).
  • Removed redundant ament\_export\_include\_directories() -- target exports already propagate include directories.
  • Cleaned up section headers.

All 6 targets build and run correctly in Docker Compose (sas_core_pure, _sas_core, sas_core_example, sas_clock_example, sas_clock_sched_fifo_example, sas_robot_driver_example).

_Commented by an AI agent (OpenHands) on behalf of the user._

@mmmarinho
mmmarinho marked this pull request as ready for review August 15, 2026 15:49
@mmmarinho

Copy link
Copy Markdown
Member Author

Added a Using as a non-ROS2 dependency section to README.md with a FetchContent example, the -DROS2_BUILD=OFF flag, and the Eigen3/dqrobotics dependency note.

Commented by an AI agent (OpenHands) on behalf of the user.

@mmmarinho mmmarinho self-assigned this Aug 15, 2026
openhands-agent and others added 2 commits August 15, 2026 15:56
- Set GIT_TAG to jazzy (the stable branch)
- Move ROS2_BUILD=OFF into the CMake example using set(... CACHE BOOL FORCE)
  so the option is disabled before FetchContent_MakeAvailable, avoiding the
  need for a separate cmake command-line flag
@mmmarinho

mmmarinho commented Aug 15, 2026

Copy link
Copy Markdown
Member Author

Updated the FetchContent example:

  • Set GIT_TAG to jazzy (the stable branch) instead of the placeholder
  • Added set(ROS2_BUILD OFF CACHE BOOL FORCE) before FetchContent_MakeAvailable so the non-ROS2 build works from within the CMake file itself (verified against the official CMake FetchContent docs -- pre-set cache values are respected by option()).

Commented by an AI agent (OpenHands) on behalf of the user.

@mmmarinho
mmmarinho merged commit a57c651 into jazzy Aug 15, 2026
2 checks passed
@mmmarinho
mmmarinho deleted the refactor/nested-src-cmake branch August 15, 2026 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants