diff --git a/CMakeLists.txt b/CMakeLists.txt index f639afbf8..37badd5c6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -133,6 +133,13 @@ endif() # Build dependencies (OpenXR SDK, etc.) add_subdirectory(deps) +# Project warning set. Deliberately after add_subdirectory(deps): directory-scope +# compile options are inherited only by subdirectories added *after* this call, so +# third-party trees keep their own flags while everything below (src/, examples/, +# plugins) is held to ours. See cmake/CompilerWarnings.cmake. +include(cmake/CompilerWarnings.cmake) +isaac_teleop_enable_compiler_warnings() + # Enable CTest at top level so tests from subdirectories are discoverable if(BUILD_TESTING) # Make sure to call this after `deps` is added so that Catch2 is available diff --git a/cmake/CompilerWarnings.cmake b/cmake/CompilerWarnings.cmake new file mode 100644 index 000000000..02c05d3f3 --- /dev/null +++ b/cmake/CompilerWarnings.cmake @@ -0,0 +1,120 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +# ============================================================================== +# Compiler warnings for first-party code +# ============================================================================== +# isaac_teleop_enable_compiler_warnings() applies the project's warning set to the +# CALLING directory scope, which CMake then inherits into every subdirectory added +# after the call. The top-level CMakeLists.txt therefore calls it *after* +# add_subdirectory(deps) so third-party trees (OpenXR SDK, yaml-cpp, pybind11, +# mcap, flatbuffers, Catch2, ...) keep building with their own flags and are never +# held to warning levels we do not control. +# +# Ordering alone does not cover code fetched from inside src/plugins/, which is added +# after the call and would otherwise inherit these flags. See the third-party +# containment scope at the bottom of this file for how those trees opt out. + +option(ISAAC_TELEOP_ENABLE_WARNINGS "Enable the project warning set on first-party C++ targets" ON) +option(ISAAC_TELEOP_WARNINGS_AS_ERRORS "Promote the project warning set to errors (-Werror / /WX)" OFF) + +function(isaac_teleop_enable_compiler_warnings) + if(NOT ISAAC_TELEOP_ENABLE_WARNINGS) + message(STATUS "Compiler warnings: disabled (ISAAC_TELEOP_ENABLE_WARNINGS=OFF)") + return() + endif() + + set(_gnu_like + -Wall + -Wextra + # Deliberately off: C-style aggregate init of OpenXR/Vulkan structs (which + # zero-fill the tail on purpose) trips this on essentially every call site. + # The native_openxr example already suppressed it for the same reason. + -Wno-missing-field-initializers + # Bug classes worth failing a build over. + -Wnon-virtual-dtor # deleting through a base pointer without a virtual dtor + -Woverloaded-virtual # a derived overload silently hiding a base virtual + -Wimplicit-fallthrough # unannotated switch fallthrough + -Wextra-semi # stray ';' after a member function definition + ) + + set(_msvc + /W4 + /permissive- + ) + + if(ISAAC_TELEOP_WARNINGS_AS_ERRORS) + list(APPEND _gnu_like -Werror) + list(APPEND _msvc /WX) + endif() + + add_compile_options( + "$<$,$>:${_gnu_like}>" + "$<$,$>:${_msvc}>" + ) + + message(STATUS "Compiler warnings: enabled (warnings as errors: ${ISAAC_TELEOP_WARNINGS_AS_ERRORS})") +endfunction() + +# ============================================================================== +# Third-party containment +# ============================================================================== +# Ordering keeps deps/ clean, but it cannot help a tree fetched from *inside* an +# already-flagged directory: a subdirectory snapshots the COMPILE_OPTIONS directory +# property at the point it is added, so anything the plugins pull in via FetchContent +# would inherit our flags. That is not hypothetical — the OAK plugin fetches DepthAI, +# which fetches XLink, and XLink does not build at -Wall -Wextra -Werror. +# +# Wrap any such add_subdirectory() or FetchContent_MakeAvailable() in this pair. It +# clears the calling directory's COMPILE_OPTIONS for the duration and restores them +# afterwards, so the fetched tree builds with its own flags while first-party targets +# declared later in the same file still get ours: +# +# isaac_teleop_third_party_scope_begin() +# FetchContent_MakeAvailable(some_dependency) +# isaac_teleop_third_party_scope_end() +# +# These are macros rather than functions on purpose: add_subdirectory() and the +# property writes have to happen in the caller's directory scope, not a nested one. + +macro(isaac_teleop_third_party_scope_begin) + if(DEFINED _isaac_teleop_saved_compile_options) + message(FATAL_ERROR + "isaac_teleop_third_party_scope_begin(): a scope is already open in this " + "directory. The scopes do not nest; close the first one before opening another.") + endif() + get_property(_isaac_teleop_saved_compile_options DIRECTORY PROPERTY COMPILE_OPTIONS) + set_property(DIRECTORY PROPERTY COMPILE_OPTIONS "") +endmacro() + +# Keeping our flags out of a third-party tree stops its own sources from being held to +# them, but our sources still #include its headers, and a warning raised inside a header +# is attributed to the first-party TU that pulled it in. Mark the dependency's interface +# includes as SYSTEM so consumers get -isystem and stay quiet about code we do not own. +# (add_subdirectory(... SYSTEM) does this in one step, but it needs CMake 3.25 and the +# project floor is 3.20.) +function(isaac_teleop_mark_include_dirs_system target) + if(NOT TARGET ${target}) + message(FATAL_ERROR "isaac_teleop_mark_include_dirs_system(): no such target '${target}'") + endif() + # Property writes are rejected on ALIAS targets, so resolve to the real one first. + get_target_property(_aliased ${target} ALIASED_TARGET) + if(_aliased) + set(target ${_aliased}) + endif() + get_target_property(_includes ${target} INTERFACE_INCLUDE_DIRECTORIES) + if(_includes) + set_target_properties(${target} PROPERTIES + INTERFACE_SYSTEM_INCLUDE_DIRECTORIES "${_includes}") + endif() +endfunction() + +macro(isaac_teleop_third_party_scope_end) + if(NOT DEFINED _isaac_teleop_saved_compile_options) + message(FATAL_ERROR + "isaac_teleop_third_party_scope_end(): no matching " + "isaac_teleop_third_party_scope_begin() in this directory.") + endif() + set_property(DIRECTORY PROPERTY COMPILE_OPTIONS "${_isaac_teleop_saved_compile_options}") + unset(_isaac_teleop_saved_compile_options) +endmacro() diff --git a/examples/native_openxr/xdev_list/main.cpp b/examples/native_openxr/xdev_list/main.cpp index c5f84346a..c2777d2cb 100644 --- a/examples/native_openxr/xdev_list/main.cpp +++ b/examples/native_openxr/xdev_list/main.cpp @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -92,13 +93,6 @@ std::vector enumerate_xdevs(const OpenXRBundle& openxr_bundle, XrX return xdevIds; } -/*! - * Print information about available XDevs using XR_MNDX_xdev_space extension - */ -static void print_xdev_info(const OpenXRBundle& openxr_bundle) -{ -} - /*! * XDev List Application - Prints information about available XDevs */ @@ -156,7 +150,10 @@ class XDevListApp : public HeadlessApp throw std::runtime_error("Failed to get properties for XDev " + std::to_string(xdevId)); } - std::string serial_str = properties.serial ? properties.serial : ""; + // serial is a fixed char[256], never a pointer, so a null check would always be true. + // Bound the length so a runtime that fills the array without a terminator cannot + // walk past it. + std::string serial_str(properties.serial, ::strnlen(properties.serial, sizeof(properties.serial))); if (serial_str == "Head Device (0)" || serial_str == "Head Device (1)") { std::cout << "[CREATE HAND] XDev ID=" << xdevId << " Name=\"" << properties.name << "\"" @@ -168,7 +165,7 @@ class XDevListApp : public HeadlessApp else { std::cout << "[SKIP] XDev ID=" << xdevId << " Name=\"" << properties.name << "\"" - << " Serial=\"" << (properties.serial ? properties.serial : "") << "\"" << std::endl; + << " Serial=\"" << serial_str << "\"" << std::endl; } } @@ -179,7 +176,7 @@ class XDevListApp : public HeadlessApp } }; -int main(int argc, char* argv[]) +int main(int /*argc*/, char* argv[]) try { XDevListApp app; diff --git a/examples/oxr/cpp/oxr_session_sharing.cpp b/examples/oxr/cpp/oxr_session_sharing.cpp index 91d586378..7736810b8 100644 --- a/examples/oxr/cpp/oxr_session_sharing.cpp +++ b/examples/oxr/cpp/oxr_session_sharing.cpp @@ -11,7 +11,7 @@ #include #include -int main(int argc, char** argv) +int main(int /*argc*/, char** argv) try { std::cout << "OpenXR Session Sharing Example" << std::endl; diff --git a/examples/oxr/cpp/oxr_simple_api_demo.cpp b/examples/oxr/cpp/oxr_simple_api_demo.cpp index 15d0176bb..fdd9c298f 100644 --- a/examples/oxr/cpp/oxr_simple_api_demo.cpp +++ b/examples/oxr/cpp/oxr_simple_api_demo.cpp @@ -20,7 +20,7 @@ * Internal lifecycle methods (initialize, update, cleanup) are hidden! */ -int main(int argc, char** argv) +int main(int /*argc*/, char** argv) try { std::cout << "OpenXR Simple API Demo" << std::endl; diff --git a/examples/schemaio/full_body_printer.cpp b/examples/schemaio/full_body_printer.cpp index 42f4a5db5..f7e585229 100644 --- a/examples/schemaio/full_body_printer.cpp +++ b/examples/schemaio/full_body_printer.cpp @@ -73,7 +73,7 @@ void print_body_pose(const core::FullBodyPoseT& data, size_t sample_count) } // namespace -int main(int argc, char** argv) +int main(int /*argc*/, char** argv) try { std::cout << "Full Body Printer (XR_BD_body_tracking)" << std::endl; diff --git a/examples/schemaio/pedal_printer.cpp b/examples/schemaio/pedal_printer.cpp index e00740d0a..4a8f1c74c 100644 --- a/examples/schemaio/pedal_printer.cpp +++ b/examples/schemaio/pedal_printer.cpp @@ -37,7 +37,7 @@ void print_pedal_data(const core::Generic3AxisPedalOutputT& data, size_t sample_ std::cout << std::endl; } -int main(int argc, char** argv) +int main(int /*argc*/, char** argv) try { std::cout << "Pedal Printer (collection: " << COLLECTION_ID << ")" << std::endl; diff --git a/examples/schemaio/pedal_pusher.cpp b/examples/schemaio/pedal_pusher.cpp index b129dae2e..cfb9207c2 100644 --- a/examples/schemaio/pedal_pusher.cpp +++ b/examples/schemaio/pedal_pusher.cpp @@ -67,7 +67,7 @@ class Generic3AxisPedalPusher core::SchemaPusher m_pusher; }; -int main(int argc, char** argv) +int main(int /*argc*/, char** argv) try { std::cout << "Schema Pusher (collection: " << COLLECTION_ID << ")" << std::endl; diff --git a/src/plugins/oak/CMakeLists.txt b/src/plugins/oak/CMakeLists.txt index 8e43feaa9..f2df161b6 100644 --- a/src/plugins/oak/CMakeLists.txt +++ b/src/plugins/oak/CMakeLists.txt @@ -182,7 +182,14 @@ if(NOT depthai_POPULATED) function(export) endfunction() + # DepthAI and its own FetchContent'd deps (XLink, ...) are third-party: keep the + # project warning set out of them, or -Werror builds fail in code we do not own. + isaac_teleop_third_party_scope_begin() add_subdirectory(${depthai_SOURCE_DIR} ${depthai_BINARY_DIR} EXCLUDE_FROM_ALL) + isaac_teleop_third_party_scope_end() + + # ...and our own sources include DepthAI's headers, so those need -isystem too. + isaac_teleop_mark_include_dirs_system(depthai::core) endif() message(STATUS "DepthAI v${DEPTHAI_VERSION} ready") @@ -201,7 +208,9 @@ FetchContent_Declare( URL "https://github.com/libsdl-org/SDL/releases/download/release-${SDL2_VERSION}/SDL2-${SDL2_VERSION}.tar.gz" URL_HASH "SHA256=5f5993c530f084535c65a6879e9b26ad441169b3e25d789d83287040a9ca5165" ) +isaac_teleop_third_party_scope_begin() FetchContent_MakeAvailable(sdl2) +isaac_teleop_third_party_scope_end() message(STATUS "SDL2 ${SDL2_VERSION} — live preview support enabled") # ============================================================================== diff --git a/src/plugins/oak/core/oak_camera.cpp b/src/plugins/oak/core/oak_camera.cpp index 5cf3eb2d6..f84713df8 100644 --- a/src/plugins/oak/core/oak_camera.cpp +++ b/src/plugins/oak/core/oak_camera.cpp @@ -63,7 +63,7 @@ dai::DeviceInfo OakCamera::find_device(const std::string& device_id) if (device_id.empty()) { - std::cout << "Found " << devices.size() << " OAK device(s), using: " << devices[0].getMxId() << std::endl; + std::cout << "Found " << devices.size() << " OAK device(s), using: " << devices[0].getDeviceId() << std::endl; return devices[0]; } diff --git a/src/plugins/oglo_tactile/CMakeLists.txt b/src/plugins/oglo_tactile/CMakeLists.txt index db08d7b93..8d5639c87 100644 --- a/src/plugins/oglo_tactile/CMakeLists.txt +++ b/src/plugins/oglo_tactile/CMakeLists.txt @@ -35,7 +35,9 @@ FetchContent_Declare( GIT_SHALLOW TRUE ) set(JSON_BuildTests OFF CACHE INTERNAL "") +isaac_teleop_third_party_scope_begin() FetchContent_MakeAvailable(nlohmann_json) +isaac_teleop_third_party_scope_end() # ------------------------------------------------------------------------------ # BLE backend: BlueZ over the system libdbus (AFL-2.1, permissive). diff --git a/src/plugins/plugin_utils/wrist_pose_source.cpp b/src/plugins/plugin_utils/wrist_pose_source.cpp index 38e14ef2f..ec67515b0 100644 --- a/src/plugins/plugin_utils/wrist_pose_source.cpp +++ b/src/plugins/plugin_utils/wrist_pose_source.cpp @@ -7,6 +7,7 @@ #include #include +#include #include #include @@ -197,7 +198,10 @@ void WristPoseSource::initialize_xdev_hand_trackers() continue; } - std::string serial_str = properties.serial ? properties.serial : ""; + // serial is a fixed char[256], never a pointer, so a null check would always be true. + // Bound the length so a runtime that fills the array without a terminator cannot + // walk past it. + std::string serial_str(properties.serial, ::strnlen(properties.serial, sizeof(properties.serial))); seen_serials.push_back(serial_str); if (serial_str == "Head Device (0)") diff --git a/src/plugins/rebot_devarm_leader/CMakeLists.txt b/src/plugins/rebot_devarm_leader/CMakeLists.txt index 16ae76127..df8203552 100644 --- a/src/plugins/rebot_devarm_leader/CMakeLists.txt +++ b/src/plugins/rebot_devarm_leader/CMakeLists.txt @@ -14,5 +14,16 @@ target_link_libraries(rebot_devarm_leader_plugin PRIVATE isaacteleop_schema ) +# The CAN bus implementations are SocketCAN, so their bodies live behind +# #ifdef __linux__ and compile to throwing stubs elsewhere. Clang then correctly +# reports the (still-declared) private members as unused. The warning is real but +# unactionable off Linux, so suppress it only there and only for this target; +# on Linux the fields are used and the warning stays live. GCC has no such warning. +if(NOT CMAKE_SYSTEM_NAME STREQUAL "Linux") + target_compile_options(rebot_devarm_leader_plugin PRIVATE + $<$:-Wno-unused-private-field> + ) +endif() + install(TARGETS rebot_devarm_leader_plugin RUNTIME DESTINATION plugins/rebot_devarm_leader) install(FILES plugin.yaml README.md DESTINATION plugins/rebot_devarm_leader)