Cleanup TODOs and collision coverage (#2286)

Author: jslee02

.clang-format

@@ -110,7 +110,7 @@ IncludeCategories:
     Priority: 50
 
   # C++ standard library headers, then C
-  # TODO(JS): Add when new C++ STL header begins with "c" is found
+  # C-prefix headers (including the full C++20 set) are matched by the "^<c.*>$" regex
   - Regex: "^<charconv>$"
     Priority: 100
   - Regex: "^<chrono>$"

.github/workflows/ci_windows.yml

@@ -45,7 +45,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        build_type: ["Release"] # TODO: Add Debug
+        build_type: ["Release"] # Windows CI keeps Release-only to keep runtimes acceptable
     steps:
       - name: Checkout
         uses: actions/checkout@v6

CMakeLists.txt

@@ -462,11 +462,8 @@ elseif(CMAKE_COMPILER_IS_GNUCXX)
     message(FATAL_ERROR "The installed g++ version is ${GCC_VERSION}. ${PROJECT_NAME} requires g++ ${gcc_required_version} or greater.")
   endif()
   if(GCC_VERSION VERSION_GREATER_EQUAL 13.2.0)
-    # TODO: These warnings should be properly addressed and these compiler options removed
-    add_compile_options(-Wno-overloaded-virtual -Wno-alloc-size-larger-than -Wno-dangling-pointer)
-  endif()
-  if(GCC_VERSION VERSION_GREATER_EQUAL 13.2.0)
-    # TODO: These warnings should be properly addressed and these compiler options removed
+    # GCC 13 currently reports noisy diagnostics in upstream dependencies; silence
+    # them until Eigen/FCL updates remove the false positives.
     add_compile_options(-Wno-overloaded-virtual -Wno-alloc-size-larger-than -Wno-dangling-pointer)
   endif()
   set(CMAKE_CXX_FLAGS_RELEASE "-O3 -DNDEBUG")

cmake/dart_defs.cmake

@@ -728,8 +728,7 @@ function(add_component package_name component)
     FILE "${package_name}_${component}Targets.cmake"
     DESTINATION "${CONFIG_INSTALL_DIR}"
   )
-  # TODO(JS): It would be nice if we could check if ${target} has at least one
-  # dependency target.
+  # Record dependency state so install_component_exports can guard against empty components.
 
   set_property(
     TARGET "${target}"
@@ -747,6 +746,11 @@ function(add_component package_name component)
     TARGET "${target}"
     PROPERTY "${component_prefix}LIBRARIES"
   )
+  set_property(
+    TARGET "${target}"
+    PROPERTY "${component_prefix}HAS_DEPENDENCY_TARGET"
+    FALSE
+  )
   set_property(
     GLOBAL
     APPEND
@@ -852,6 +856,9 @@ function(add_component_targets package_name component)
   endif()
 
   set(target "${component_prefix}${component}")
+  if(NOT dependency_targets)
+    message(FATAL_ERROR "Component '${component}' must have at least one dependency target.")
+  endif()
   add_dependencies("${target}" ${ARGN})
 
   foreach(dependency_target IN LISTS dependency_targets)
@@ -882,6 +889,11 @@ function(add_component_targets package_name component)
     )
   endforeach()
 
+  set_property(
+    TARGET "${target}"
+    PROPERTY "${component_prefix}HAS_DEPENDENCY_TARGET"
+    TRUE
+  )
   set_property(
     TARGET "${target}"
     APPEND
@@ -899,11 +911,24 @@ function(install_component_exports package_name)
   get_property(components GLOBAL PROPERTY "${package_name}_COMPONENTS")
 
   set(output_prefix "${CMAKE_CURRENT_BINARY_DIR}/${CONFIG_INSTALL_DIR}")
+  file(MAKE_DIRECTORY "${output_prefix}")
 
   foreach(component IN LISTS components)
     set(target "${component_prefix}${component}")
 
-    # TODO: Replace this manual generation with a configure_file.
+    get_property(
+      has_dependency_targets
+      TARGET "${target}"
+      PROPERTY "${component_prefix}HAS_DEPENDENCY_TARGET"
+    )
+    if(NOT has_dependency_targets)
+      message(
+        FATAL_ERROR
+        "Component '${component}' has no dependency targets. "
+        "Call add_component_targets(${package_name} ${component} <targets>)."
+      )
+    endif()
+
     set(output_path
       "${output_prefix}/${package_name}_${component}Component.cmake")
 

dart/CMakeLists.txt

@@ -171,6 +171,20 @@ endif()
 # C++ standard settings
 target_compile_features(dart PUBLIC cxx_std_20)
 
+string(TOLOWER "${CMAKE_SYSTEM_PROCESSOR}" dart_host_processor)
+set(dart_host_architecture "unknown")
+if(dart_host_processor MATCHES "amd64|x86_64")
+  set(dart_host_architecture "x86_64")
+elseif(dart_host_processor MATCHES "i[3-6]86")
+  set(dart_host_architecture "x86")
+elseif(dart_host_processor MATCHES "aarch64|arm64")
+  set(dart_host_architecture "arm64")
+elseif(dart_host_processor MATCHES "^arm")
+  set(dart_host_architecture "arm")
+elseif(dart_host_processor MATCHES "riscv64")
+  set(dart_host_architecture "riscv64")
+endif()
+
 # Build DART with all available SIMD instructions
 if(DART_ENABLE_SIMD)
   if(MSVC)
@@ -189,14 +203,13 @@ if(DART_ENABLE_SIMD)
     target_compile_options(dart PUBLIC -march=native -faligned-new)
   endif()
 else()
-  # TODO: Improve this part to detect the system's architecture and apply the
-  # appropriate SIMD instructions.
-  if(CMAKE_SYSTEM_PROCESSOR MATCHES "arm64")
-    # Remove SSE flags if set elsewhere
-    string(REPLACE "-msse" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
-    string(REPLACE "-msse2" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
-    string(REPLACE "-msse3" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
-    string(REPLACE "-mssse3" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
+  # Remove x86-specific flags when building on non-x86 architectures to keep
+  # builds portable without enabling the SIMD option.
+  if(NOT dart_host_architecture STREQUAL "x86"
+     AND NOT dart_host_architecture STREQUAL "x86_64")
+    foreach(flag "-msse" "-msse2" "-msse3" "-mssse3")
+      string(REPLACE "${flag}" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
+    endforeach()
   endif()
 endif()
 

examples/CMakeLists.txt

@@ -31,8 +31,6 @@ add_subdirectory(capsule_ground_contact)
 if(DART_HAVE_OCTOMAP)
   add_subdirectory(point_cloud)
 endif()
-# TODO: Re-enable when rerun example has source files
-# add_subdirectory(rerun)
 add_subdirectory(rigid_chain)
 add_subdirectory(rigid_cubes)
 add_subdirectory(rigid_loop)

python/tests/unit/dynamics/test_meta_skeleton.py renamed to python/tests/integration/test_meta_skeleton.py

@@ -1,11 +1,6 @@
-import platform
-
 import dartpy as dart
 import pytest
 
-# TODO(JS): Move this to integration category once created
-
-
 def test_basic():
     urdfParser = dart.io.DartLoader()
     kr5 = urdfParser.parse_skeleton("dart://sample/urdf/KR5/KR5 sixx R650.urdf")

python/tests/unit/collision/test_collision.py

@@ -1,15 +1,29 @@
-import platform
-
 import dartpy as dart
 import numpy as np
 import pytest
 
 
-def collision_groups_tester(cd):
-    size = [1, 1, 1]
-    pos1 = [0, 0, 0]
-    pos2 = [0.5, 0, 0]
+def _collision_detector_factories():
+    detectors = [("fcl", dart.FCLCollisionDetector)]
+
+    if hasattr(dart, "DARTCollisionDetector"):
+        detectors.append(("dart", dart.DARTCollisionDetector))
+
+    if hasattr(dart, "BulletCollisionDetector"):
+        detectors.append(("bullet", dart.BulletCollisionDetector))
+
+    if hasattr(dart, "OdeCollisionDetector"):
+        detectors.append(("ode", dart.OdeCollisionDetector))
+
+    return detectors
 
+
+_COLLISION_DETECTORS = _collision_detector_factories()
+if not _COLLISION_DETECTORS:
+    pytest.skip("No collision detectors available", allow_module_level=True)
+
+
+def collision_groups_tester(cd):
     simple_frame1 = dart.SimpleFrame()
     simple_frame2 = dart.SimpleFrame()
 
@@ -102,25 +116,16 @@ def collision_groups_tester(cd):
     assert not group.collide(group2)
 
 
-def test_collision_groups():
-    cd = dart.FCLCollisionDetector()
-    collision_groups_tester(cd)
+_COLLISION_IDS = [name for name, _ in _COLLISION_DETECTORS]
 
-    cd = dart.DARTCollisionDetector()
-    collision_groups_tester(cd)
 
-    if hasattr(dart, "BulletCollisionDetector"):
-        cd = dart.BulletCollisionDetector()
-        collision_groups_tester(cd)
-
-    if hasattr(dart, "OdeCollisionDetector"):
-        cd = dart.OdeCollisionDetector()
-        collision_groups_tester(cd)
+@pytest.mark.parametrize("name, cd_factory", _COLLISION_DETECTORS, ids=_COLLISION_IDS)
+def test_collision_groups(name, cd_factory):
+    collision_groups_tester(cd_factory())
 
 
-# TODO: Add more collision detectors
-@pytest.mark.parametrize("cd", [dart.FCLCollisionDetector()])
-def test_filter(cd):
+@pytest.mark.parametrize("name, cd_factory", _COLLISION_DETECTORS, ids=_COLLISION_IDS)
+def test_filter(name, cd_factory):
     # Create two bodies skeleton. The two bodies are placed at the same position
     # with the same size shape so that they collide by default.
     skel = dart.Skeleton()
@@ -143,7 +148,7 @@ def test_filter(cd):
 
     # Set a new collision detector
     constraint_solver = world.get_constraint_solver()
-    constraint_solver.set_collision_detector(cd)
+    constraint_solver.set_collision_detector(cd_factory())
 
     # Get the collision group from the constraint solver
     group = constraint_solver.get_collision_group()