Edit cmake#183
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the project’s CMake build definitions to use target_sources() and to decompose previously monolithic libraries (notably camera and localization) into smaller component targets, alongside some CMake policy/build invocation updates.
Changes:
- Restructures
utilsandpathingCMake targets to useadd_library(<name>)+target_sources(). - Splits
cameraandlocalizationinto multiple component libraries and introduces an aggregatecamerainterface target. - Updates CMake policy handling and raises the CMake minimum required version in
src/.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
src/utils/CMakeLists.txt |
Converts utils to target_sources() style and reformats link dependencies. |
src/pathing/CMakeLists.txt |
Converts pathing to target_sources() style and changes its link dependencies. |
src/localization/CMakeLists.txt |
Splits localization into multiple component libraries and rewires linking. |
src/camera/CMakeLists.txt |
Splits camera into component libraries and introduces an aggregate camera interface target. |
src/calibration/CMakeLists.txt |
Refactors calibration executables’ CMake definitions (with some redundancy introduced). |
src/CMakeLists.txt |
Raises cmake_minimum_required for the src subdir and adjusts executable link lists. |
CMakeLists.txt |
Adds policy guards and FetchContent extract-timestamp setting. |
scripts/build.sh |
Adds -Wno-dev to the CMake configure invocation. |
Comments suppressed due to low confidence (4)
src/localization/CMakeLists.txt:18
OpencvApriltagDetectordirectly uses OpenCV APIs, but the target links onlyutils. This can produce link failures when OpenCV isn't already on the final link line. Link this target against OpenCV (prefer imported targets if available, otherwise${OpenCV_LIBS}) so its dependency is explicit.
add_library(OpencvApriltagDetector)
target_sources(OpencvApriltagDetector
PRIVATE
opencv_apriltag_detector.cc
)
target_link_libraries(OpencvApriltagDetector PRIVATE
utils
)
src/localization/CMakeLists.txt:27
NvidiaApriltagDetectorincludes and uses OpenCV types/functions (and OpenCV-VPI interop), but the target links onlyutils. To avoid unresolved OpenCV symbols, add the required OpenCV link dependency (prefer imported targets if available, otherwise${OpenCV_LIBS}) to this target.
add_library(NvidiaApriltagDetector)
target_sources(NvidiaApriltagDetector
PRIVATE
nvidia_apriltag_detector.cc
)
target_link_libraries(NvidiaApriltagDetector PRIVATE
utils
)
src/calibration/CMakeLists.txt:17
frame_showersources are specified twice (inadd_executable(frame_shower frame_shower.cc)and again intarget_sources(frame_shower ...)). This is redundant; keep the source list in one place to avoid confusion.
add_executable(frame_shower frame_shower.cc)
target_sources(frame_shower
PRIVATE
frame_shower.cc
)
src/calibration/CMakeLists.txt:28
focus_calibratesources are specified twice (inadd_executable(focus_calibrate focus_calibrate.cc)and again intarget_sources(focus_calibrate ...)). This is redundant; keep the source list in one place to avoid confusion.
add_executable(focus_calibrate focus_calibrate.cc)
target_sources(focus_calibrate
PRIVATE
focus_calibrate.cc
)
| PRIVATE | ||
| cv_camera.cc | ||
| write_frame.cc | ||
| write_frame.cc |
Comment on lines
+73
to
+76
| target_link_libraries(UVCCamera PRIVATE | ||
| uvc | ||
| absl::status | ||
| utils |
Comment on lines
+95
to
+114
| add_library(localization) | ||
| target_sources(localization | ||
| PRIVATE | ||
| run_localization.cc | ||
| multi_camera_detector.cc | ||
| ) | ||
| target_link_libraries(localization PRIVATE | ||
| GPUApriltagDetector | ||
| OpencvApriltagDetector | ||
| NvidiaApriltagDetector | ||
| SquareSolver | ||
| JointSolver | ||
| MultitagSolver | ||
| PositionReceiver | ||
| UnambiguousEstimator | ||
| SimulationSender | ||
| NetworktableSender | ||
| utils | ||
| camera | ||
| ) |
Comment on lines
+1
to
+9
| add_library(GPUApriltagDetector) | ||
| target_sources(GPUApriltagDetector | ||
| PRIVATE | ||
| gpu_apriltag_detector.cc | ||
| ) | ||
| target_link_libraries(GPUApriltagDetector PRIVATE | ||
| 971apriltag | ||
| utils | ||
| ) |
| ) | ||
| target_link_libraries(pathing PRIVATE | ||
| utils | ||
| localization |
Comment on lines
+1
to
+6
| add_executable(IntrinsicsCalibrate intrinsics_calibrate.cc intrinsics_calibrate_lib.cc) | ||
| target_sources(IntrinsicsCalibrate | ||
| PRIVATE | ||
| intrinsics_calibrate.cc | ||
| intrinsics_calibrate_lib.cc | ||
| ) |
| @@ -1,4 +1,5 @@ | |||
| cmake_minimum_required(VERSION 3.10) | |||
| cmake_minimum_required(VERSION 3.16...4.0) | |||
Comment on lines
+3
to
+5
| if(POLICY CMP0135) | ||
| cmake_policy(SET CMP0135 NEW) | ||
| endif() |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.