From f691a6ac9e46d8b19e6f6310e37aba589cabbe3d Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Mon, 6 Apr 2026 16:21:08 +0200 Subject: [PATCH 01/10] feat(cmake): centralize compiler warnings with ROS2MedkitWarnings module Add ROS2MedkitWarnings.cmake with production-grade warning flags aligned with OpenSSF, AUTOSAR C++14, and MISRA C++ guidelines. Enable -Werror by default. Replace per-package add_compile_options blocks with a single include(ROS2MedkitWarnings) across all 12 packages. New flags: -Wswitch-enum, -Wfloat-equal, -Wcast-qual, -Wundef, -Wzero-as-null-pointer-constant, -Wextra-semi, -Wdouble-promotion, -Wsign-conversion, -Wnon-virtual-dtor, -Wold-style-cast, -Woverloaded-virtual, -Wcast-align, -Wnull-dereference, -Wimplicit-fallthrough, -Wformat=2, -Wuseless-cast (GCC), -Wduplicated-cond, -Wduplicated-branches, -Wlogical-op (GCC). Fix all warnings surfaced by the new flags: implicit conversions, useless casts, incomplete switch-enum coverage, float-to-double in printf, char sign conversions. Exclude vendored dynmsg and gtest/gmock from strict warnings via set_source_files_properties and ros2_medkit_relax_vendor_warnings(). Closes #356 --- src/ros2_medkit_cmake/CMakeLists.txt | 1 + .../cmake/ROS2MedkitWarnings.cmake | 108 ++++++++++++++++++ .../cmake/ros2_medkit_cmake-extras.cmake | 3 +- .../CMakeLists.txt | 6 +- .../ros2_medkit_beacon_common/CMakeLists.txt | 6 +- .../CMakeLists.txt | 6 +- .../ros2_medkit_param_beacon/CMakeLists.txt | 6 +- .../ros2_medkit_topic_beacon/CMakeLists.txt | 6 +- src/ros2_medkit_fault_manager/CMakeLists.txt | 6 +- .../src/fault_manager_node.cpp | 4 +- .../src/sqlite_fault_storage.cpp | 2 +- src/ros2_medkit_fault_reporter/CMakeLists.txt | 6 +- src/ros2_medkit_gateway/CMakeLists.txt | 7 +- .../src/aggregation/peer_client.cpp | 3 +- .../src/configuration_manager.cpp | 2 + .../src/discovery/discovery_enums.cpp | 1 + .../src/discovery/discovery_manager.cpp | 1 + .../discovery/manifest/manifest_parser.cpp | 1 + .../src/discovery/merge_pipeline.cpp | 5 + src/ros2_medkit_gateway/src/gateway_node.cpp | 11 +- .../src/http/entity_path_utils.cpp | 6 + .../src/http/handlers/bulkdata_handlers.cpp | 2 +- .../src/http/handlers/config_handlers.cpp | 2 + .../handlers/cyclic_subscription_handlers.cpp | 3 + .../src/http/handlers/data_handlers.cpp | 2 +- .../src/http/handlers/handler_context.cpp | 1 + .../src/http/handlers/operation_handlers.cpp | 7 ++ .../src/http/handlers/sse_fault_handler.cpp | 2 +- .../src/http/handlers/trigger_handlers.cpp | 2 + .../src/http/handlers/update_handlers.cpp | 31 +++++ src/ros2_medkit_gateway/src/lock_manager.cpp | 3 + src/ros2_medkit_gateway/src/log_manager.cpp | 2 +- .../src/models/aggregation_service.cpp | 1 + .../src/models/thread_safe_entity_cache.cpp | 4 + .../src/native_topic_sampler.cpp | 4 + .../src/openapi/capability_generator.cpp | 13 +++ .../src/updates/update_manager.cpp | 5 + .../test/test_resource_sampler_registry.cpp | 2 +- .../CMakeLists.txt | 5 +- .../demo_nodes/brake_actuator.cpp | 2 +- .../demo_nodes/long_calibration_action.cpp | 3 +- src/ros2_medkit_msgs/CMakeLists.txt | 4 - .../ros2_medkit_graph_provider/CMakeLists.txt | 6 +- src/ros2_medkit_serialization/CMakeLists.txt | 17 ++- .../src/json_serializer.cpp | 3 +- 45 files changed, 256 insertions(+), 67 deletions(-) create mode 100644 src/ros2_medkit_cmake/cmake/ROS2MedkitWarnings.cmake diff --git a/src/ros2_medkit_cmake/CMakeLists.txt b/src/ros2_medkit_cmake/CMakeLists.txt index eeec9c6d8..185254dec 100644 --- a/src/ros2_medkit_cmake/CMakeLists.txt +++ b/src/ros2_medkit_cmake/CMakeLists.txt @@ -26,6 +26,7 @@ install( cmake/ROS2MedkitLinting.cmake cmake/ROS2MedkitSanitizers.cmake cmake/ROS2MedkitTestDomain.cmake + cmake/ROS2MedkitWarnings.cmake DESTINATION share/${PROJECT_NAME}/cmake ) diff --git a/src/ros2_medkit_cmake/cmake/ROS2MedkitWarnings.cmake b/src/ros2_medkit_cmake/cmake/ROS2MedkitWarnings.cmake new file mode 100644 index 000000000..d6ed03088 --- /dev/null +++ b/src/ros2_medkit_cmake/cmake/ROS2MedkitWarnings.cmake @@ -0,0 +1,108 @@ +# Copyright 2026 bburda +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +include_guard(GLOBAL) + +# Production-grade compiler warning configuration for ros2_medkit packages. +# Aligned with AUTOSAR C++14, MISRA C++, OpenSSF, and Airbus SecLab guidelines +# for ASIL QM/B level code. +# +# Provides: +# option ENABLE_WERROR (default ON) - treat warnings as errors +# function ros2_medkit_relax_vendor_warnings() - call after ament_add_gtest/gmock +# to suppress strict warnings on gtest/gmock vendored targets +# +# Usage: +# find_package(ros2_medkit_cmake REQUIRED) +# include(ROS2MedkitWarnings) +# ... +# if(BUILD_TESTING) +# ament_add_gtest(...) +# ros2_medkit_relax_vendor_warnings() +# endif() + +option(ENABLE_WERROR "Treat compiler warnings as errors" ON) + +if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") + add_compile_options( + # Core warnings + -Wall + -Wextra + -Wpedantic + + # Type safety (MISRA, OpenSSF) + -Wconversion + -Wsign-conversion + -Wdouble-promotion + -Wfloat-equal # MISRA M6-2-2, AUTOSAR M6-2-2 + + # Shadow / hiding + -Wshadow + -Woverloaded-virtual + + # OOP correctness + -Wnon-virtual-dtor + -Wold-style-cast + + # Const correctness (MISRA, Airbus SecLab) + -Wcast-qual + + # Memory / alignment + -Wcast-align + -Wnull-dereference + + # Control flow + -Wimplicit-fallthrough + -Wswitch-enum # AUTOSAR A6-4-6 + + # Preprocessor (MISRA) + -Wundef + + # Format strings (OpenSSF) + -Wformat=2 + + # Modern C++ (AUTOSAR A4-10-1) + -Wzero-as-null-pointer-constant + + # Code cleanliness (AUTOSAR) + -Wextra-semi + ) + + if(ENABLE_WERROR) + add_compile_options(-Werror) + endif() + + # GCC-only warnings + if(CMAKE_COMPILER_IS_GNUCXX) + add_compile_options( + -Wduplicated-cond + -Wduplicated-branches + -Wlogical-op + -Wuseless-cast + ) + endif() +endif() + +# Suppress strict warnings on gtest/gmock vendor targets. +# ament_add_gtest/ament_add_gmock build gtest/gmock sources as subdirectory +# targets, which inherit add_compile_options flags. The vendored code triggers +# warnings from our strict flag set that we cannot fix. +# Call this once at the end of the BUILD_TESTING block. +function(ros2_medkit_relax_vendor_warnings) + foreach(_target gmock gmock_main gtest gtest_main) + if(TARGET ${_target}) + target_compile_options(${_target} PRIVATE -Wno-error) + endif() + endforeach() +endfunction() diff --git a/src/ros2_medkit_cmake/cmake/ros2_medkit_cmake-extras.cmake b/src/ros2_medkit_cmake/cmake/ros2_medkit_cmake-extras.cmake index 16e659753..a2f5fa418 100644 --- a/src/ros2_medkit_cmake/cmake/ros2_medkit_cmake-extras.cmake +++ b/src/ros2_medkit_cmake/cmake/ros2_medkit_cmake-extras.cmake @@ -16,6 +16,7 @@ # Adds the installed cmake module directory to CMAKE_MODULE_PATH so that # include(ROS2MedkitCompat), include(ROS2MedkitCcache), # include(ROS2MedkitLinting), include(ROS2MedkitSanitizers), -# and include(ROS2MedkitTestDomain) work transparently. +# include(ROS2MedkitTestDomain), and include(ROS2MedkitWarnings) +# work transparently. list(APPEND CMAKE_MODULE_PATH "${ros2_medkit_cmake_DIR}") diff --git a/src/ros2_medkit_diagnostic_bridge/CMakeLists.txt b/src/ros2_medkit_diagnostic_bridge/CMakeLists.txt index 3033ca0f9..b886c180b 100644 --- a/src/ros2_medkit_diagnostic_bridge/CMakeLists.txt +++ b/src/ros2_medkit_diagnostic_bridge/CMakeLists.txt @@ -23,10 +23,7 @@ find_package(ros2_medkit_cmake REQUIRED) include(ROS2MedkitCcache) include(ROS2MedkitSanitizers) include(ROS2MedkitLinting) - -if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") - add_compile_options(-Wall -Wextra -Wpedantic -Wshadow -Wconversion) -endif() +include(ROS2MedkitWarnings) # Code coverage option option(ENABLE_COVERAGE "Enable code coverage reporting" OFF) @@ -130,6 +127,7 @@ if(BUILD_TESTING) TARGET test_integration TIMEOUT 60 ) + ros2_medkit_relax_vendor_warnings() endif() ament_package() diff --git a/src/ros2_medkit_discovery_plugins/ros2_medkit_beacon_common/CMakeLists.txt b/src/ros2_medkit_discovery_plugins/ros2_medkit_beacon_common/CMakeLists.txt index 651111a27..2668e741f 100644 --- a/src/ros2_medkit_discovery_plugins/ros2_medkit_beacon_common/CMakeLists.txt +++ b/src/ros2_medkit_discovery_plugins/ros2_medkit_beacon_common/CMakeLists.txt @@ -15,10 +15,6 @@ cmake_minimum_required(VERSION 3.8) project(ros2_medkit_beacon_common) -if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") - add_compile_options(-Wall -Wextra -Wpedantic -Wshadow -Wconversion) -endif() - set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD_REQUIRED ON) @@ -27,6 +23,7 @@ find_package(ros2_medkit_cmake REQUIRED) include(ROS2MedkitCompat) include(ROS2MedkitCcache) include(ROS2MedkitSanitizers) +include(ROS2MedkitWarnings) find_package(ament_cmake REQUIRED) find_package(ros2_medkit_msgs REQUIRED) @@ -102,6 +99,7 @@ if(BUILD_TESTING) ament_add_gtest(test_beacon_entity_mapper test/test_beacon_entity_mapper.cpp) target_link_libraries(test_beacon_entity_mapper beacon_common_lib) medkit_target_dependencies(test_beacon_entity_mapper ros2_medkit_gateway) + ros2_medkit_relax_vendor_warnings() endif() ament_package() diff --git a/src/ros2_medkit_discovery_plugins/ros2_medkit_linux_introspection/CMakeLists.txt b/src/ros2_medkit_discovery_plugins/ros2_medkit_linux_introspection/CMakeLists.txt index db36d0474..d713bdea6 100644 --- a/src/ros2_medkit_discovery_plugins/ros2_medkit_linux_introspection/CMakeLists.txt +++ b/src/ros2_medkit_discovery_plugins/ros2_medkit_linux_introspection/CMakeLists.txt @@ -11,10 +11,7 @@ include(ROS2MedkitCompat) include(ROS2MedkitCcache) include(ROS2MedkitSanitizers) include(ROS2MedkitLinting) - -if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") - add_compile_options(-Wall -Wextra -Wpedantic -Wshadow -Wconversion) -endif() +include(ROS2MedkitWarnings) find_package(ament_cmake REQUIRED) find_package(ros2_medkit_gateway REQUIRED) @@ -95,6 +92,7 @@ if(BUILD_TESTING) target_link_libraries(test_systemd_plugin medkit_linux_utils) target_include_directories(test_systemd_plugin PRIVATE ${ros2_medkit_gateway_INCLUDE_DIRS}) medkit_target_dependencies(test_systemd_plugin nlohmann_json) + ros2_medkit_relax_vendor_warnings() endif() ament_package() diff --git a/src/ros2_medkit_discovery_plugins/ros2_medkit_param_beacon/CMakeLists.txt b/src/ros2_medkit_discovery_plugins/ros2_medkit_param_beacon/CMakeLists.txt index d78fd634d..e0c8a8b4a 100644 --- a/src/ros2_medkit_discovery_plugins/ros2_medkit_param_beacon/CMakeLists.txt +++ b/src/ros2_medkit_discovery_plugins/ros2_medkit_param_beacon/CMakeLists.txt @@ -15,10 +15,6 @@ cmake_minimum_required(VERSION 3.8) project(ros2_medkit_param_beacon) -if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") - add_compile_options(-Wall -Wextra -Wpedantic -Wshadow -Wconversion) -endif() - set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD_REQUIRED ON) @@ -27,6 +23,7 @@ find_package(ros2_medkit_cmake REQUIRED) include(ROS2MedkitCompat) include(ROS2MedkitCcache) include(ROS2MedkitSanitizers) +include(ROS2MedkitWarnings) find_package(ament_cmake REQUIRED) find_package(ros2_medkit_beacon_common REQUIRED) @@ -93,6 +90,7 @@ if(BUILD_TESTING) nlohmann_json::nlohmann_json ros2_medkit_beacon_common::beacon_common_lib) medkit_set_test_domain(test_param_beacon_plugin) + ros2_medkit_relax_vendor_warnings() endif() ament_package() diff --git a/src/ros2_medkit_discovery_plugins/ros2_medkit_topic_beacon/CMakeLists.txt b/src/ros2_medkit_discovery_plugins/ros2_medkit_topic_beacon/CMakeLists.txt index 35e21bf5b..a08de3876 100644 --- a/src/ros2_medkit_discovery_plugins/ros2_medkit_topic_beacon/CMakeLists.txt +++ b/src/ros2_medkit_discovery_plugins/ros2_medkit_topic_beacon/CMakeLists.txt @@ -15,10 +15,6 @@ cmake_minimum_required(VERSION 3.8) project(ros2_medkit_topic_beacon) -if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") - add_compile_options(-Wall -Wextra -Wpedantic -Wshadow -Wconversion) -endif() - set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD_REQUIRED ON) @@ -27,6 +23,7 @@ find_package(ros2_medkit_cmake REQUIRED) include(ROS2MedkitCompat) include(ROS2MedkitCcache) include(ROS2MedkitSanitizers) +include(ROS2MedkitWarnings) find_package(ament_cmake REQUIRED) find_package(ros2_medkit_beacon_common REQUIRED) @@ -98,6 +95,7 @@ if(BUILD_TESTING) nlohmann_json::nlohmann_json ros2_medkit_beacon_common::beacon_common_lib) medkit_set_test_domain(test_topic_beacon_plugin) + ros2_medkit_relax_vendor_warnings() endif() ament_package() diff --git a/src/ros2_medkit_fault_manager/CMakeLists.txt b/src/ros2_medkit_fault_manager/CMakeLists.txt index a7e397a72..cfaeb3407 100644 --- a/src/ros2_medkit_fault_manager/CMakeLists.txt +++ b/src/ros2_medkit_fault_manager/CMakeLists.txt @@ -25,10 +25,7 @@ include(ROS2MedkitCompat) include(ROS2MedkitCcache) include(ROS2MedkitSanitizers) include(ROS2MedkitLinting) - -if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") - add_compile_options(-Wall -Wextra -Wpedantic -Wshadow -Wconversion) -endif() +include(ROS2MedkitWarnings) # Code coverage option option(ENABLE_COVERAGE "Enable code coverage reporting" OFF) @@ -220,6 +217,7 @@ if(BUILD_TESTING) TIMEOUT 60 ) set_tests_properties(test_entity_thresholds_integration PROPERTIES LABELS "integration") + ros2_medkit_relax_vendor_warnings() endif() ament_package() diff --git a/src/ros2_medkit_fault_manager/src/fault_manager_node.cpp b/src/ros2_medkit_fault_manager/src/fault_manager_node.cpp index 95c44fc40..e88809927 100644 --- a/src/ros2_medkit_fault_manager/src/fault_manager_node.cpp +++ b/src/ros2_medkit_fault_manager/src/fault_manager_node.cpp @@ -108,9 +108,9 @@ FaultManagerNode::FaultManagerNode(const rclcpp::NodeOptions & options) : Node(" snapshot_recapture_cooldown_sec_); snapshot_recapture_cooldown_sec_ = 0.0; } - int max_snapshots = declare_parameter("snapshots.max_per_fault", 10); + auto max_snapshots = declare_parameter("snapshots.max_per_fault", 10); if (max_snapshots < 0) { - RCLCPP_WARN(get_logger(), "snapshots.max_per_fault should be >= 0, got %d. Disabling limit.", max_snapshots); + RCLCPP_WARN(get_logger(), "snapshots.max_per_fault should be >= 0, got %ld. Disabling limit.", max_snapshots); max_snapshots = 0; } diff --git a/src/ros2_medkit_fault_manager/src/sqlite_fault_storage.cpp b/src/ros2_medkit_fault_manager/src/sqlite_fault_storage.cpp index 4befa6415..5295f0280 100644 --- a/src/ros2_medkit_fault_manager/src/sqlite_fault_storage.cpp +++ b/src/ros2_medkit_fault_manager/src/sqlite_fault_storage.cpp @@ -363,7 +363,7 @@ bool SqliteFaultStorage::report_fault_event(const std::string & fault_code, uint int64_t existing_count = check_stmt.column_int64(1); std::string sources_json = check_stmt.column_text(2); std::string current_status = check_stmt.column_text(3); - int32_t debounce_counter = static_cast(check_stmt.column_int(4)); + int32_t debounce_counter = check_stmt.column_int(4); // CLEARED faults can be reactivated by FAILED events bool is_reactivation = false; diff --git a/src/ros2_medkit_fault_reporter/CMakeLists.txt b/src/ros2_medkit_fault_reporter/CMakeLists.txt index cd37096c2..f7aa10730 100644 --- a/src/ros2_medkit_fault_reporter/CMakeLists.txt +++ b/src/ros2_medkit_fault_reporter/CMakeLists.txt @@ -23,10 +23,7 @@ find_package(ros2_medkit_cmake REQUIRED) include(ROS2MedkitCcache) include(ROS2MedkitSanitizers) include(ROS2MedkitLinting) - -if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") - add_compile_options(-Wall -Wextra -Wpedantic -Wshadow -Wconversion) -endif() +include(ROS2MedkitWarnings) # Code coverage option option(ENABLE_COVERAGE "Enable code coverage reporting" OFF) @@ -113,6 +110,7 @@ if(BUILD_TESTING) TARGET test_integration TIMEOUT 60 ) + ros2_medkit_relax_vendor_warnings() endif() ament_package() diff --git a/src/ros2_medkit_gateway/CMakeLists.txt b/src/ros2_medkit_gateway/CMakeLists.txt index 5a84a8ac0..bc15c183c 100644 --- a/src/ros2_medkit_gateway/CMakeLists.txt +++ b/src/ros2_medkit_gateway/CMakeLists.txt @@ -11,10 +11,7 @@ include(ROS2MedkitCompat) include(ROS2MedkitCcache) include(ROS2MedkitSanitizers) include(ROS2MedkitLinting) - -if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") - add_compile_options(-Wall -Wextra -Wpedantic -Wshadow -Wconversion) -endif() +include(ROS2MedkitWarnings) # Code coverage option option(ENABLE_COVERAGE "Enable code coverage reporting" OFF) @@ -778,7 +775,7 @@ if(BUILD_TESTING) target_link_options(${_target} PRIVATE --coverage) endforeach() endif() - + ros2_medkit_relax_vendor_warnings() endif() # Export include directories for downstream packages (plugins) diff --git a/src/ros2_medkit_gateway/src/aggregation/peer_client.cpp b/src/ros2_medkit_gateway/src/aggregation/peer_client.cpp index e965a5233..ce73d3888 100644 --- a/src/ros2_medkit_gateway/src/aggregation/peer_client.cpp +++ b/src/ros2_medkit_gateway/src/aggregation/peer_client.cpp @@ -50,7 +50,8 @@ constexpr size_t MAX_ENTITIES_PER_COLLECTION = 1000; std::string encode_query_param(const std::string & value) { std::string result; result.reserve(value.size()); - for (unsigned char c : value) { + for (char ch : value) { + auto c = static_cast(ch); if (std::isalnum(c) || c == '-' || c == '.' || c == '_' || c == '~') { result += static_cast(c); } else { diff --git a/src/ros2_medkit_gateway/src/configuration_manager.cpp b/src/ros2_medkit_gateway/src/configuration_manager.cpp index c8e489afa..913e97444 100644 --- a/src/ros2_medkit_gateway/src/configuration_manager.cpp +++ b/src/ros2_medkit_gateway/src/configuration_manager.cpp @@ -607,6 +607,8 @@ rclcpp::ParameterValue ConfigurationManager::json_to_parameter_value(const json return rclcpp::ParameterValue(value.get>()); } break; + case rclcpp::ParameterType::PARAMETER_NOT_SET: + case rclcpp::ParameterType::PARAMETER_BYTE_ARRAY: default: break; } diff --git a/src/ros2_medkit_gateway/src/discovery/discovery_enums.cpp b/src/ros2_medkit_gateway/src/discovery/discovery_enums.cpp index 5d9e6118c..d7591b9a3 100644 --- a/src/ros2_medkit_gateway/src/discovery/discovery_enums.cpp +++ b/src/ros2_medkit_gateway/src/discovery/discovery_enums.cpp @@ -32,6 +32,7 @@ std::string discovery_mode_to_string(DiscoveryMode mode) { return "manifest_only"; case DiscoveryMode::HYBRID: return "hybrid"; + case DiscoveryMode::RUNTIME_ONLY: default: return "runtime_only"; } diff --git a/src/ros2_medkit_gateway/src/discovery/discovery_manager.cpp b/src/ros2_medkit_gateway/src/discovery/discovery_manager.cpp index 9015db98f..0d62c02c4 100644 --- a/src/ros2_medkit_gateway/src/discovery/discovery_manager.cpp +++ b/src/ros2_medkit_gateway/src/discovery/discovery_manager.cpp @@ -161,6 +161,7 @@ void DiscoveryManager::create_strategy() { break; } + case DiscoveryMode::RUNTIME_ONLY: default: active_strategy_ = runtime_strategy_.get(); RCLCPP_INFO(node_->get_logger(), "Discovery mode: runtime_only (default_component=%s)", diff --git a/src/ros2_medkit_gateway/src/discovery/manifest/manifest_parser.cpp b/src/ros2_medkit_gateway/src/discovery/manifest/manifest_parser.cpp index 1ae55c431..aaad67908 100644 --- a/src/ros2_medkit_gateway/src/discovery/manifest/manifest_parser.cpp +++ b/src/ros2_medkit_gateway/src/discovery/manifest/manifest_parser.cpp @@ -278,6 +278,7 @@ std::string ManifestConfig::policy_to_string(UnmanifestedNodePolicy policy) { return "error"; case UnmanifestedNodePolicy::INCLUDE_AS_ORPHAN: return "include_as_orphan"; + case UnmanifestedNodePolicy::WARN: default: return "warn"; } diff --git a/src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp b/src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp index 64830570a..45d4c7219 100644 --- a/src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp +++ b/src/ros2_medkit_gateway/src/discovery/merge_pipeline.cpp @@ -186,6 +186,8 @@ void apply_field_group_merge(Entity & target, const Entity & source, FieldGroup case FieldGroup::METADATA: merge_scalar(target.source, source.source, res.scalar); break; + case FieldGroup::LIVE_DATA: + case FieldGroup::STATUS: default: break; } @@ -223,6 +225,7 @@ void apply_field_group_merge(Entity & target, const Entity & source, FieldGroup merge_scalar(target.source, source.source, res.scalar); merge_scalar(target.variant, source.variant, res.scalar); break; + case FieldGroup::STATUS: default: break; } @@ -282,6 +285,8 @@ void apply_field_group_merge(Entity & target, const Entity & source, FieldGroup case FieldGroup::METADATA: merge_scalar(target.source, source.source, res.scalar); break; + case FieldGroup::LIVE_DATA: + case FieldGroup::STATUS: default: break; } diff --git a/src/ros2_medkit_gateway/src/gateway_node.cpp b/src/ros2_medkit_gateway/src/gateway_node.cpp index 823764185..53b042491 100644 --- a/src/ros2_medkit_gateway/src/gateway_node.cpp +++ b/src/ros2_medkit_gateway/src/gateway_node.cpp @@ -55,6 +55,7 @@ nlohmann::json parameter_to_json(const rclcpp::Parameter & param) { return param.as_double_array(); case rclcpp::ParameterType::PARAMETER_STRING_ARRAY: return param.as_string_array(); + case rclcpp::ParameterType::PARAMETER_NOT_SET: default: return nullptr; } @@ -1073,7 +1074,7 @@ GatewayNode::GatewayNode(const rclcpp::NodeOptions & options) : Node("ros2_medki payload["data"] = *sample.data; return payload; } - return tl::make_unexpected(std::string("Topic data not available: " + resource_path)); + return tl::make_unexpected("Topic data not available: " + resource_path); }, true); @@ -1089,7 +1090,7 @@ GatewayNode::GatewayNode(const rclcpp::NodeOptions & options) : Node("ros2_medki const auto & cache = get_thread_safe_cache(); auto entity_ref = cache.find_entity(entity_id); if (!entity_ref) { - return tl::make_unexpected(std::string("Entity not found: " + entity_id)); + return tl::make_unexpected("Entity not found: " + entity_id); } if (entity_ref->type == SovdEntityType::FUNCTION) { @@ -1157,7 +1158,7 @@ GatewayNode::GatewayNode(const rclcpp::NodeOptions & options) : Node("ros2_medki } } if (source_id.empty()) { - return tl::make_unexpected(std::string("Entity no longer available: " + entity_id)); + return tl::make_unexpected("Entity no longer available: " + entity_id); } auto result = fault_mgr->list_faults(source_id); if (!result.success) { @@ -1207,7 +1208,7 @@ GatewayNode::GatewayNode(const rclcpp::NodeOptions & options) : Node("ros2_medki const auto & cache = get_thread_safe_cache(); auto entity_ref = cache.find_entity(entity_id); if (!entity_ref) { - return tl::make_unexpected(std::string("Entity not found: " + entity_id)); + return tl::make_unexpected("Entity not found: " + entity_id); } if (entity_ref->type == SovdEntityType::FUNCTION) { @@ -1266,7 +1267,7 @@ GatewayNode::GatewayNode(const rclcpp::NodeOptions & options) : Node("ros2_medki } if (fqn.empty()) { - return tl::make_unexpected(std::string("Entity no longer available: " + entity_id)); + return tl::make_unexpected("Entity no longer available: " + entity_id); } auto result = log_mgr->get_logs({fqn}, prefix_match, "", "", entity_id); if (!result.has_value()) { diff --git a/src/ros2_medkit_gateway/src/http/entity_path_utils.cpp b/src/ros2_medkit_gateway/src/http/entity_path_utils.cpp index 8076accb2..204c74d88 100644 --- a/src/ros2_medkit_gateway/src/http/entity_path_utils.cpp +++ b/src/ros2_medkit_gateway/src/http/entity_path_utils.cpp @@ -53,6 +53,10 @@ std::string get_type_segment(SovdEntityType type, bool is_nested) { return "subareas"; case SovdEntityType::COMPONENT: return "subcomponents"; + case SovdEntityType::SERVER: + case SovdEntityType::APP: + case SovdEntityType::FUNCTION: + case SovdEntityType::UNKNOWN: default: break; } @@ -66,6 +70,8 @@ std::string get_type_segment(SovdEntityType type, bool is_nested) { return "areas"; case SovdEntityType::FUNCTION: return "functions"; + case SovdEntityType::SERVER: + case SovdEntityType::UNKNOWN: default: return ""; } diff --git a/src/ros2_medkit_gateway/src/http/handlers/bulkdata_handlers.cpp b/src/ros2_medkit_gateway/src/http/handlers/bulkdata_handlers.cpp index 8c145afaa..a85446aff 100644 --- a/src/ros2_medkit_gateway/src/http/handlers/bulkdata_handlers.cpp +++ b/src/ros2_medkit_gateway/src/http/handlers/bulkdata_handlers.cpp @@ -133,7 +133,7 @@ void BulkDataHandlers::handle_list_descriptors(const httplib::Request & req, htt for (const auto & rosbag : all_rosbags) { std::string fault_code = rosbag.value("fault_code", ""); std::string format = rosbag.value("format", "mcap"); - uint64_t size_bytes = rosbag.value("size_bytes", 0); + uint64_t size_bytes = rosbag.value("size_bytes", uint64_t{0}); double duration_sec = rosbag.value("duration_sec", 0.0); // Use fault_code as bulk_data_id diff --git a/src/ros2_medkit_gateway/src/http/handlers/config_handlers.cpp b/src/ros2_medkit_gateway/src/http/handlers/config_handlers.cpp index 564062d46..9bf387d9c 100644 --- a/src/ros2_medkit_gateway/src/http/handlers/config_handlers.cpp +++ b/src/ros2_medkit_gateway/src/http/handlers/config_handlers.cpp @@ -133,6 +133,8 @@ static ErrorClassification classify_error_code(ParameterErrorCode error_code) { result.status_code = 400; result.error_code = ERR_INVALID_PARAMETER; break; + case ParameterErrorCode::NONE: + case ParameterErrorCode::SHUT_DOWN: case ParameterErrorCode::INTERNAL_ERROR: default: result.status_code = 500; diff --git a/src/ros2_medkit_gateway/src/http/handlers/cyclic_subscription_handlers.cpp b/src/ros2_medkit_gateway/src/http/handlers/cyclic_subscription_handlers.cpp index 4149fda16..3e77750ae 100644 --- a/src/ros2_medkit_gateway/src/http/handlers/cyclic_subscription_handlers.cpp +++ b/src/ros2_medkit_gateway/src/http/handlers/cyclic_subscription_handlers.cpp @@ -409,6 +409,9 @@ std::string CyclicSubscriptionHandlers::extract_entity_type(const httplib::Reque return "components"; case SovdEntityType::FUNCTION: return "functions"; + case SovdEntityType::SERVER: + case SovdEntityType::AREA: + case SovdEntityType::UNKNOWN: default: RCLCPP_WARN(HandlerContext::logger(), "Unexpected entity type in cyclic subscription path: %s", req.path.c_str()); return "apps"; diff --git a/src/ros2_medkit_gateway/src/http/handlers/data_handlers.cpp b/src/ros2_medkit_gateway/src/http/handlers/data_handlers.cpp index 1949f9de0..781c0d86b 100644 --- a/src/ros2_medkit_gateway/src/http/handlers/data_handlers.cpp +++ b/src/ros2_medkit_gateway/src/http/handlers/data_handlers.cpp @@ -240,7 +240,7 @@ void DataHandlers::handle_put_data_item(const httplib::Request & req, httplib::R json data = body["data"]; // Validate message type format (e.g., std_msgs/msg/Float32) - size_t slash_count = std::count(msg_type.begin(), msg_type.end(), '/'); + auto slash_count = static_cast(std::count(msg_type.begin(), msg_type.end(), '/')); size_t msg_pos = msg_type.find("/msg/"); bool valid_format = (slash_count == 2) && (msg_pos != std::string::npos) && (msg_pos > 0) && (msg_pos + 5 < msg_type.length()); diff --git a/src/ros2_medkit_gateway/src/http/handlers/handler_context.cpp b/src/ros2_medkit_gateway/src/http/handlers/handler_context.cpp index c5c272ad0..e8acaceeb 100644 --- a/src/ros2_medkit_gateway/src/http/handlers/handler_context.cpp +++ b/src/ros2_medkit_gateway/src/http/handlers/handler_context.cpp @@ -145,6 +145,7 @@ EntityInfo HandlerContext::get_entity_info(const std::string & entity_id, SovdEn break; case SovdEntityType::SERVER: + case SovdEntityType::UNKNOWN: default: break; } diff --git a/src/ros2_medkit_gateway/src/http/handlers/operation_handlers.cpp b/src/ros2_medkit_gateway/src/http/handlers/operation_handlers.cpp index 6f08c2b26..f0a4db78a 100644 --- a/src/ros2_medkit_gateway/src/http/handlers/operation_handlers.cpp +++ b/src/ros2_medkit_gateway/src/http/handlers/operation_handlers.cpp @@ -68,6 +68,8 @@ void OperationHandlers::handle_list_operations(const httplib::Request & req, htt ops = cache.get_function_operations(entity_id); entity_type = "function"; break; + case SovdEntityType::SERVER: + case SovdEntityType::UNKNOWN: default: HandlerContext::send_error(res, 404, ERR_ENTITY_NOT_FOUND, "Entity not found", {{"entity_id", entity_id}}); return; @@ -201,6 +203,8 @@ void OperationHandlers::handle_get_operation(const httplib::Request & req, httpl ops = cache.get_function_operations(entity_id); entity_type = "function"; break; + case SovdEntityType::SERVER: + case SovdEntityType::UNKNOWN: default: HandlerContext::send_error(res, 404, ERR_ENTITY_NOT_FOUND, "Entity type does not support operations", {{"entity_id", entity_id}}); @@ -320,6 +324,7 @@ static std::string sovd_status_from_ros2(ActionGoalStatus status) { case ActionGoalStatus::CANCELED: case ActionGoalStatus::ABORTED: return "failed"; + case ActionGoalStatus::UNKNOWN: default: return "running"; } @@ -384,6 +389,8 @@ void OperationHandlers::handle_create_execution(const httplib::Request & req, ht ops = cache.get_function_operations(entity_id); entity_type = "function"; break; + case SovdEntityType::SERVER: + case SovdEntityType::UNKNOWN: default: HandlerContext::send_error(res, 404, ERR_ENTITY_NOT_FOUND, "Entity type does not support operations", {{"entity_id", entity_id}}); diff --git a/src/ros2_medkit_gateway/src/http/handlers/sse_fault_handler.cpp b/src/ros2_medkit_gateway/src/http/handlers/sse_fault_handler.cpp index 1da86cb4f..21289d81b 100644 --- a/src/ros2_medkit_gateway/src/http/handlers/sse_fault_handler.cpp +++ b/src/ros2_medkit_gateway/src/http/handlers/sse_fault_handler.cpp @@ -52,7 +52,7 @@ SSEFaultHandler::SSEFaultHandler(HandlerContext & ctx, std::shared_ptr(keepalive_interval.count()), kKeepaliveIntervalSec); + keepalive_interval.count(), kKeepaliveIntervalSec); } const auto fault_events_topic = build_fault_manager_events_topic(ctx_.node()); diff --git a/src/ros2_medkit_gateway/src/http/handlers/trigger_handlers.cpp b/src/ros2_medkit_gateway/src/http/handlers/trigger_handlers.cpp index b33cd8d38..ac416ff4a 100644 --- a/src/ros2_medkit_gateway/src/http/handlers/trigger_handlers.cpp +++ b/src/ros2_medkit_gateway/src/http/handlers/trigger_handlers.cpp @@ -498,6 +498,8 @@ std::string TriggerHandlers::extract_entity_type(const httplib::Request & req) { return "areas"; case SovdEntityType::FUNCTION: return "functions"; + case SovdEntityType::SERVER: + case SovdEntityType::UNKNOWN: default: RCLCPP_WARN(HandlerContext::logger(), "Unexpected entity type in trigger path: %s", req.path.c_str()); return "apps"; diff --git a/src/ros2_medkit_gateway/src/http/handlers/update_handlers.cpp b/src/ros2_medkit_gateway/src/http/handlers/update_handlers.cpp index 731629f91..0c2b72ec4 100644 --- a/src/ros2_medkit_gateway/src/http/handlers/update_handlers.cpp +++ b/src/ros2_medkit_gateway/src/http/handlers/update_handlers.cpp @@ -124,6 +124,14 @@ void UpdateHandlers::handle_register_update(const httplib::Request & req, httpli case UpdateErrorCode::AlreadyExists: HandlerContext::send_error(res, 400, ERR_X_MEDKIT_UPDATE_ALREADY_EXISTS, result.error().message); break; + case UpdateErrorCode::NotFound: + case UpdateErrorCode::InProgress: + case UpdateErrorCode::NotPrepared: + case UpdateErrorCode::NotAutomated: + case UpdateErrorCode::InvalidRequest: + case UpdateErrorCode::Deleting: + case UpdateErrorCode::NoBackend: + case UpdateErrorCode::Internal: default: HandlerContext::send_error(res, 400, ERR_INVALID_REQUEST, result.error().message); break; @@ -162,6 +170,13 @@ void UpdateHandlers::handle_delete_update(const httplib::Request & req, httplib: case UpdateErrorCode::NotFound: HandlerContext::send_error(res, 404, ERR_X_MEDKIT_UPDATE_NOT_FOUND, result.error().message); break; + case UpdateErrorCode::AlreadyExists: + case UpdateErrorCode::NotPrepared: + case UpdateErrorCode::NotAutomated: + case UpdateErrorCode::InvalidRequest: + case UpdateErrorCode::Deleting: + case UpdateErrorCode::NoBackend: + case UpdateErrorCode::Internal: default: HandlerContext::send_error(res, 500, ERR_INTERNAL_ERROR, result.error().message); break; @@ -197,6 +212,12 @@ void UpdateHandlers::handle_prepare(const httplib::Request & req, httplib::Respo case UpdateErrorCode::Deleting: HandlerContext::send_error(res, 409, ERR_X_MEDKIT_UPDATE_IN_PROGRESS, result.error().message); break; + case UpdateErrorCode::AlreadyExists: + case UpdateErrorCode::NotPrepared: + case UpdateErrorCode::NotAutomated: + case UpdateErrorCode::InvalidRequest: + case UpdateErrorCode::NoBackend: + case UpdateErrorCode::Internal: default: HandlerContext::send_error(res, 400, ERR_INVALID_REQUEST, result.error().message); break; @@ -236,6 +257,11 @@ void UpdateHandlers::handle_execute(const httplib::Request & req, httplib::Respo case UpdateErrorCode::Deleting: HandlerContext::send_error(res, 409, ERR_X_MEDKIT_UPDATE_IN_PROGRESS, result.error().message); break; + case UpdateErrorCode::AlreadyExists: + case UpdateErrorCode::NotAutomated: + case UpdateErrorCode::InvalidRequest: + case UpdateErrorCode::NoBackend: + case UpdateErrorCode::Internal: default: HandlerContext::send_error(res, 400, ERR_INVALID_REQUEST, result.error().message); break; @@ -275,6 +301,11 @@ void UpdateHandlers::handle_automated(const httplib::Request & req, httplib::Res case UpdateErrorCode::Deleting: HandlerContext::send_error(res, 409, ERR_X_MEDKIT_UPDATE_IN_PROGRESS, result.error().message); break; + case UpdateErrorCode::AlreadyExists: + case UpdateErrorCode::NotPrepared: + case UpdateErrorCode::InvalidRequest: + case UpdateErrorCode::NoBackend: + case UpdateErrorCode::Internal: default: HandlerContext::send_error(res, 400, ERR_INVALID_REQUEST, result.error().message); break; diff --git a/src/ros2_medkit_gateway/src/lock_manager.cpp b/src/ros2_medkit_gateway/src/lock_manager.cpp index 07a5caf8f..fe7a9b9a1 100644 --- a/src/ros2_medkit_gateway/src/lock_manager.cpp +++ b/src/ros2_medkit_gateway/src/lock_manager.cpp @@ -54,6 +54,9 @@ std::string LockManager::get_entity_type_string(const std::string & entity_id) c return "component"; case SovdEntityType::APP: return "app"; + case SovdEntityType::SERVER: + case SovdEntityType::FUNCTION: + case SovdEntityType::UNKNOWN: default: return "unknown"; } diff --git a/src/ros2_medkit_gateway/src/log_manager.cpp b/src/ros2_medkit_gateway/src/log_manager.cpp index 5d4403723..779c77c22 100644 --- a/src/ros2_medkit_gateway/src/log_manager.cpp +++ b/src/ros2_medkit_gateway/src/log_manager.cpp @@ -247,7 +247,7 @@ void LogManager::add_log_entry(const std::string & entity_id, const std::string auto secs = std::chrono::duration_cast(epoch); auto nsecs = std::chrono::duration_cast(epoch - secs); - entry.stamp_sec = static_cast(secs.count()); + entry.stamp_sec = secs.count(); entry.stamp_nanosec = static_cast(nsecs.count()); entry.level = severity_to_level(severity); if (entry.level == 0) { diff --git a/src/ros2_medkit_gateway/src/models/aggregation_service.cpp b/src/ros2_medkit_gateway/src/models/aggregation_service.cpp index 96a5b03f2..c0b503e2c 100644 --- a/src/ros2_medkit_gateway/src/models/aggregation_service.cpp +++ b/src/ros2_medkit_gateway/src/models/aggregation_service.cpp @@ -155,6 +155,7 @@ nlohmann::json AggregationService::build_collection_x_medkit(SovdEntityType type case SovdEntityType::SERVER: x_medkit["aggregation_level"] = "server"; break; + case SovdEntityType::UNKNOWN: default: break; } diff --git a/src/ros2_medkit_gateway/src/models/thread_safe_entity_cache.cpp b/src/ros2_medkit_gateway/src/models/thread_safe_entity_cache.cpp index c744f7b1f..4478cfa6a 100644 --- a/src/ros2_medkit_gateway/src/models/thread_safe_entity_cache.cpp +++ b/src/ros2_medkit_gateway/src/models/thread_safe_entity_cache.cpp @@ -419,6 +419,8 @@ AggregatedConfigurations ThreadSafeEntityCache::get_entity_configurations(const return get_area_configurations(entity_id); case SovdEntityType::FUNCTION: return get_function_configurations(entity_id); + case SovdEntityType::SERVER: + case SovdEntityType::UNKNOWN: default: return {}; } @@ -865,6 +867,8 @@ AggregatedData ThreadSafeEntityCache::get_entity_data(const std::string & entity return get_area_data(entity_id); case SovdEntityType::FUNCTION: return get_function_data(entity_id); + case SovdEntityType::SERVER: + case SovdEntityType::UNKNOWN: default: return {}; } diff --git a/src/ros2_medkit_gateway/src/native_topic_sampler.cpp b/src/ros2_medkit_gateway/src/native_topic_sampler.cpp index 6941e1e2e..0bb070b67 100644 --- a/src/ros2_medkit_gateway/src/native_topic_sampler.cpp +++ b/src/ros2_medkit_gateway/src/native_topic_sampler.cpp @@ -49,6 +49,7 @@ std::string reliability_to_string(rclcpp::ReliabilityPolicy policy) { case rclcpp::ReliabilityPolicy::BestAvailable: return "best_available"; #endif + case rclcpp::ReliabilityPolicy::Unknown: default: return "unknown"; } @@ -67,6 +68,7 @@ std::string durability_to_string(rclcpp::DurabilityPolicy policy) { case rclcpp::DurabilityPolicy::BestAvailable: return "best_available"; #endif + case rclcpp::DurabilityPolicy::Unknown: default: return "unknown"; } @@ -81,6 +83,7 @@ std::string history_to_string(rclcpp::HistoryPolicy policy) { return "keep_all"; case rclcpp::HistoryPolicy::SystemDefault: return "system_default"; + case rclcpp::HistoryPolicy::Unknown: default: return "unknown"; } @@ -99,6 +102,7 @@ std::string liveliness_to_string(rclcpp::LivelinessPolicy policy) { case rclcpp::LivelinessPolicy::BestAvailable: return "best_available"; #endif + case rclcpp::LivelinessPolicy::Unknown: default: return "unknown"; } diff --git a/src/ros2_medkit_gateway/src/openapi/capability_generator.cpp b/src/ros2_medkit_gateway/src/openapi/capability_generator.cpp index ecf66ba3b..ecb9ea500 100644 --- a/src/ros2_medkit_gateway/src/openapi/capability_generator.cpp +++ b/src/ros2_medkit_gateway/src/openapi/capability_generator.cpp @@ -458,6 +458,8 @@ bool CapabilityGenerator::validate_entity_hierarchy(const ResolvedPath & resolve return false; } break; + case SovdEntityType::SERVER: + case SovdEntityType::UNKNOWN: default: return false; } @@ -487,6 +489,8 @@ bool CapabilityGenerator::validate_entity_hierarchy(const ResolvedPath & resolve return false; } break; + case SovdEntityType::SERVER: + case SovdEntityType::UNKNOWN: default: return false; } @@ -680,6 +684,8 @@ void CapabilityGenerator::add_resource_collection_paths(nlohmann::json & paths, case SovdEntityType::FUNCTION: ops = cache.get_function_operations(entity_id); break; + case SovdEntityType::SERVER: + case SovdEntityType::UNKNOWN: default: break; } @@ -702,6 +708,13 @@ void CapabilityGenerator::add_resource_collection_paths(nlohmann::json & paths, paths[col_path] = path_builder.build_logs_collection(entity_path); add_log_configuration_path(paths, col_path, entity_path); break; + case ResourceCollection::DATA_LISTS: + case ResourceCollection::LOCKS: + case ResourceCollection::MODES: + case ResourceCollection::COMMUNICATION_LOGS: + case ResourceCollection::TRIGGERS: + case ResourceCollection::SCRIPTS: + case ResourceCollection::UPDATES: default: // For other collections we don't have specific builders, add generic listing { diff --git a/src/ros2_medkit_gateway/src/updates/update_manager.cpp b/src/ros2_medkit_gateway/src/updates/update_manager.cpp index d30920e1d..ccecdf09f 100644 --- a/src/ros2_medkit_gateway/src/updates/update_manager.cpp +++ b/src/ros2_medkit_gateway/src/updates/update_manager.cpp @@ -93,6 +93,8 @@ tl::expected UpdateManager::register_update(const nlohmann::j return tl::make_unexpected(UpdateError{UpdateErrorCode::AlreadyExists, err.message}); case UpdateBackendError::InvalidInput: return tl::make_unexpected(UpdateError{UpdateErrorCode::InvalidRequest, err.message}); + case UpdateBackendError::NotFound: + case UpdateBackendError::Internal: default: return tl::make_unexpected(UpdateError{UpdateErrorCode::Internal, err.message}); } @@ -148,6 +150,9 @@ tl::expected UpdateManager::delete_update(const std::string & switch (err.code) { case UpdateBackendError::NotFound: return tl::make_unexpected(UpdateError{UpdateErrorCode::NotFound, err.message}); + case UpdateBackendError::AlreadyExists: + case UpdateBackendError::InvalidInput: + case UpdateBackendError::Internal: default: return tl::make_unexpected(UpdateError{UpdateErrorCode::Internal, err.message}); } diff --git a/src/ros2_medkit_gateway/test/test_resource_sampler_registry.cpp b/src/ros2_medkit_gateway/test/test_resource_sampler_registry.cpp index d99c75ea1..ff9c2d1b5 100644 --- a/src/ros2_medkit_gateway/test/test_resource_sampler_registry.cpp +++ b/src/ros2_medkit_gateway/test/test_resource_sampler_registry.cpp @@ -134,7 +134,7 @@ TEST(ResourceSamplerRegistryTest, UpdatesSamplerRegisteredAsBuiltin) { if (resource_path == "known-pkg") { return nlohmann::json{{"status", "inProgress"}, {"progress", 42}}; } - return tl::make_unexpected(std::string("Update not found: " + resource_path)); + return tl::make_unexpected("Update not found: " + resource_path); }, true); diff --git a/src/ros2_medkit_integration_tests/CMakeLists.txt b/src/ros2_medkit_integration_tests/CMakeLists.txt index f9ea1d867..eef2db415 100644 --- a/src/ros2_medkit_integration_tests/CMakeLists.txt +++ b/src/ros2_medkit_integration_tests/CMakeLists.txt @@ -4,15 +4,12 @@ project(ros2_medkit_integration_tests) set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD_REQUIRED ON) -if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") - add_compile_options(-Wall -Wextra -Wpedantic) -endif() - find_package(ament_cmake REQUIRED) # Shared cmake modules (multi-distro compat, ccache, linting) find_package(ros2_medkit_cmake REQUIRED) include(ROS2MedkitCompat) +include(ROS2MedkitWarnings) find_package(ament_cmake_python REQUIRED) find_package(rclcpp REQUIRED) diff --git a/src/ros2_medkit_integration_tests/demo_nodes/brake_actuator.cpp b/src/ros2_medkit_integration_tests/demo_nodes/brake_actuator.cpp index 879d5c551..4e0e4781f 100644 --- a/src/ros2_medkit_integration_tests/demo_nodes/brake_actuator.cpp +++ b/src/ros2_medkit_integration_tests/demo_nodes/brake_actuator.cpp @@ -65,7 +65,7 @@ class BrakeActuator : public rclcpp::Node { target_pressure_ = 100.0f; } - RCLCPP_INFO(this->get_logger(), "Received command: %.2f bar", target_pressure_); + RCLCPP_INFO(this->get_logger(), "Received command: %.2f bar", static_cast(target_pressure_)); } void timer_callback() { diff --git a/src/ros2_medkit_integration_tests/demo_nodes/long_calibration_action.cpp b/src/ros2_medkit_integration_tests/demo_nodes/long_calibration_action.cpp index 1ae5f864e..b0eae69b5 100644 --- a/src/ros2_medkit_integration_tests/demo_nodes/long_calibration_action.cpp +++ b/src/ros2_medkit_integration_tests/demo_nodes/long_calibration_action.cpp @@ -122,7 +122,8 @@ class LongCalibrationAction : public rclcpp::Node { } // Compute next Fibonacci number (simulating calibration step) - sequence.push_back(sequence[i] + sequence[i - 1]); + auto idx = static_cast(i); + sequence.push_back(sequence[idx] + sequence[idx - 1]); // Publish feedback goal_handle->publish_feedback(feedback); diff --git a/src/ros2_medkit_msgs/CMakeLists.txt b/src/ros2_medkit_msgs/CMakeLists.txt index 550c7b1aa..29498b2bb 100644 --- a/src/ros2_medkit_msgs/CMakeLists.txt +++ b/src/ros2_medkit_msgs/CMakeLists.txt @@ -15,10 +15,6 @@ cmake_minimum_required(VERSION 3.8) project(ros2_medkit_msgs) -if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") - add_compile_options(-Wall -Wextra -Wpedantic) -endif() - find_package(ament_cmake REQUIRED) find_package(rosidl_default_generators REQUIRED) find_package(builtin_interfaces REQUIRED) diff --git a/src/ros2_medkit_plugins/ros2_medkit_graph_provider/CMakeLists.txt b/src/ros2_medkit_plugins/ros2_medkit_graph_provider/CMakeLists.txt index 08670b473..7129048e5 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_graph_provider/CMakeLists.txt +++ b/src/ros2_medkit_plugins/ros2_medkit_graph_provider/CMakeLists.txt @@ -15,10 +15,6 @@ cmake_minimum_required(VERSION 3.8) project(ros2_medkit_graph_provider) -if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") - add_compile_options(-Wall -Wextra -Wpedantic -Wshadow -Wconversion) -endif() - set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD_REQUIRED ON) @@ -27,6 +23,7 @@ find_package(ros2_medkit_cmake REQUIRED) include(ROS2MedkitCompat) include(ROS2MedkitCcache) include(ROS2MedkitSanitizers) +include(ROS2MedkitWarnings) find_package(ament_cmake REQUIRED) find_package(ros2_medkit_gateway REQUIRED) @@ -116,6 +113,7 @@ if(BUILD_TESTING) OpenSSL::SSL OpenSSL::Crypto ) medkit_set_test_domain(test_graph_provider_plugin) + ros2_medkit_relax_vendor_warnings() endif() ament_package() diff --git a/src/ros2_medkit_serialization/CMakeLists.txt b/src/ros2_medkit_serialization/CMakeLists.txt index fe4cacfde..7ca928ac9 100644 --- a/src/ros2_medkit_serialization/CMakeLists.txt +++ b/src/ros2_medkit_serialization/CMakeLists.txt @@ -11,10 +11,7 @@ include(ROS2MedkitCompat) include(ROS2MedkitCcache) include(ROS2MedkitSanitizers) include(ROS2MedkitLinting) - -if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") - add_compile_options(-Wall -Wextra -Wpedantic) -endif() +include(ROS2MedkitWarnings) # Code coverage option option(ENABLE_COVERAGE "Enable code coverage reporting" OFF) @@ -52,6 +49,17 @@ add_library(${PROJECT_NAME} src/vendored/dynmsg/yaml_utils.cpp ) +# Vendored dynmsg is third-party code - suppress our strict warnings +set_source_files_properties( + src/vendored/dynmsg/typesupport.cpp + src/vendored/dynmsg/message_reading_cpp.cpp + src/vendored/dynmsg/msg_parser_cpp.cpp + src/vendored/dynmsg/string_utils.cpp + src/vendored/dynmsg/vector_utils.cpp + src/vendored/dynmsg/yaml_utils.cpp + PROPERTIES COMPILE_OPTIONS "-Wno-error;-Wno-old-style-cast;-Wno-sign-conversion" +) + # Enable PIC so this static library can be linked into shared objects # (e.g. test_gateway_plugin.so via gateway_lib.a) set_target_properties(${PROJECT_NAME} PROPERTIES POSITION_INDEPENDENT_CODE ON) @@ -173,6 +181,7 @@ if(BUILD_TESTING) target_compile_options(test_service_action_types PRIVATE --coverage -O0 -g) target_link_options(test_service_action_types PRIVATE --coverage) endif() + ros2_medkit_relax_vendor_warnings() endif() ament_package() diff --git a/src/ros2_medkit_serialization/src/json_serializer.cpp b/src/ros2_medkit_serialization/src/json_serializer.cpp index 19d5772f4..c611b18d6 100644 --- a/src/ros2_medkit_serialization/src/json_serializer.cpp +++ b/src/ros2_medkit_serialization/src/json_serializer.cpp @@ -270,9 +270,10 @@ nlohmann::json JsonSerializer::yaml_to_json(const YAML::Node & yaml) { return obj; } - default: + case YAML::NodeType::Undefined: return nullptr; } + return nullptr; // Unreachable - all enum values handled above } YAML::Node JsonSerializer::json_to_yaml(const nlohmann::json & json) { From 5e15467ed69de17131a7fde0b783ba5d49f16c03 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Mon, 6 Apr 2026 16:55:48 +0200 Subject: [PATCH 02/10] fix(cmake): remove -Wnull-dereference due to GCC 13 STL false positives GCC 13 with optimization triggers false -Wnull-dereference warnings on standard library headers (streambuf inlining), breaking CI builds. This is a known GCC issue. Null dereference checks are covered by clang-tidy instead. --- src/ros2_medkit_cmake/cmake/ROS2MedkitWarnings.cmake | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ros2_medkit_cmake/cmake/ROS2MedkitWarnings.cmake b/src/ros2_medkit_cmake/cmake/ROS2MedkitWarnings.cmake index d6ed03088..8d48cafc0 100644 --- a/src/ros2_medkit_cmake/cmake/ROS2MedkitWarnings.cmake +++ b/src/ros2_medkit_cmake/cmake/ROS2MedkitWarnings.cmake @@ -60,7 +60,8 @@ if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") # Memory / alignment -Wcast-align - -Wnull-dereference + # Note: -Wnull-dereference omitted - GCC 13 false positives on STL + # (streambuf inlining). Covered by clang-tidy's null checks instead. # Control flow -Wimplicit-fallthrough From 451b6b219754d00566455fa701befd64506eb2a7 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Mon, 6 Apr 2026 17:09:51 +0200 Subject: [PATCH 03/10] fix(cmake): restore -Wnull-dereference, default ENABLE_WERROR to OFF Restore -Wnull-dereference which was removed due to GCC 13 false positives on STL headers. With ENABLE_WERROR defaulting to OFF, all warning flags remain active for visibility without breaking the build on different GCC versions. ENABLE_WERROR can be toggled ON once external includes are treated as system headers. --- src/ros2_medkit_cmake/cmake/ROS2MedkitWarnings.cmake | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/ros2_medkit_cmake/cmake/ROS2MedkitWarnings.cmake b/src/ros2_medkit_cmake/cmake/ROS2MedkitWarnings.cmake index 8d48cafc0..4097353ba 100644 --- a/src/ros2_medkit_cmake/cmake/ROS2MedkitWarnings.cmake +++ b/src/ros2_medkit_cmake/cmake/ROS2MedkitWarnings.cmake @@ -32,7 +32,7 @@ include_guard(GLOBAL) # ros2_medkit_relax_vendor_warnings() # endif() -option(ENABLE_WERROR "Treat compiler warnings as errors" ON) +option(ENABLE_WERROR "Treat compiler warnings as errors" OFF) if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") add_compile_options( @@ -60,8 +60,7 @@ if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") # Memory / alignment -Wcast-align - # Note: -Wnull-dereference omitted - GCC 13 false positives on STL - # (streambuf inlining). Covered by clang-tidy's null checks instead. + -Wnull-dereference # Control flow -Wimplicit-fallthrough From 4028a532aa020bd5ab983ff080801b4efd2fcb3e Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Mon, 6 Apr 2026 17:21:46 +0200 Subject: [PATCH 04/10] feat(cmake): use selective -Werror for flags safe from external headers Replace blanket ENABLE_WERROR=OFF with selective -Werror= for warnings that only fire on our code: shadow, switch-enum, old-style-cast, float-equal, cast-qual, undef, zero-as-null-pointer-constant, extra-semi, overloaded-virtual, non-virtual-dtor, implicit-fallthrough, format, duplicated-cond/branches, logical-op, useless-cast (GCC). Flags that false-positive on external headers (STL, ROS 2, nlohmann, gtest) remain as warnings only: conversion, sign-conversion, double-promotion, null-dereference, cast-align. The relax function now suppresses specific promoted flags on gtest/gmock targets instead of blanket -Wno-error. --- .../cmake/ROS2MedkitWarnings.cmake | 60 ++++++++++++++++--- 1 file changed, 51 insertions(+), 9 deletions(-) diff --git a/src/ros2_medkit_cmake/cmake/ROS2MedkitWarnings.cmake b/src/ros2_medkit_cmake/cmake/ROS2MedkitWarnings.cmake index 4097353ba..2c2d7e821 100644 --- a/src/ros2_medkit_cmake/cmake/ROS2MedkitWarnings.cmake +++ b/src/ros2_medkit_cmake/cmake/ROS2MedkitWarnings.cmake @@ -15,13 +15,15 @@ include_guard(GLOBAL) # Production-grade compiler warning configuration for ros2_medkit packages. -# Aligned with AUTOSAR C++14, MISRA C++, OpenSSF, and Airbus SecLab guidelines -# for ASIL QM/B level code. +# Aligned with AUTOSAR C++14, MISRA C++, OpenSSF, and Airbus SecLab guidelines. +# +# Strategy: all warnings enabled, selective -Werror= only for warnings +# that don't false-positive on external headers (STL, ROS 2, gtest, nlohmann). +# Flags that DO fire on external code remain as warnings for visibility. # # Provides: -# option ENABLE_WERROR (default ON) - treat warnings as errors +# option ENABLE_WERROR (default ON) - selective warnings-as-errors # function ros2_medkit_relax_vendor_warnings() - call after ament_add_gtest/gmock -# to suppress strict warnings on gtest/gmock vendored targets # # Usage: # find_package(ros2_medkit_cmake REQUIRED) @@ -32,9 +34,10 @@ include_guard(GLOBAL) # ros2_medkit_relax_vendor_warnings() # endif() -option(ENABLE_WERROR "Treat compiler warnings as errors" OFF) +option(ENABLE_WERROR "Treat select compiler warnings as errors" ON) if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") + # -- All warnings (report everything, some as errors, rest as warnings) ------ add_compile_options( # Core warnings -Wall @@ -79,8 +82,24 @@ if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") -Wextra-semi ) + # -- Selective -Werror for flags safe from external header false positives ---- + # NOT promoted: -Wconversion, -Wsign-conversion, -Wdouble-promotion, + # -Wnull-dereference, -Wcast-align (false positives on STL/ROS 2/nlohmann) if(ENABLE_WERROR) - add_compile_options(-Werror) + add_compile_options( + -Werror=shadow + -Werror=switch-enum + -Werror=old-style-cast + -Werror=float-equal + -Werror=cast-qual + -Werror=undef + -Werror=zero-as-null-pointer-constant + -Werror=extra-semi + -Werror=overloaded-virtual + -Werror=non-virtual-dtor + -Werror=implicit-fallthrough + -Werror=format=2 + ) endif() # GCC-only warnings @@ -91,18 +110,41 @@ if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") -Wlogical-op -Wuseless-cast ) + if(ENABLE_WERROR) + add_compile_options( + -Werror=duplicated-cond + -Werror=duplicated-branches + -Werror=logical-op + -Werror=useless-cast + ) + endif() endif() endif() # Suppress strict warnings on gtest/gmock vendor targets. # ament_add_gtest/ament_add_gmock build gtest/gmock sources as subdirectory -# targets, which inherit add_compile_options flags. The vendored code triggers -# warnings from our strict flag set that we cannot fix. +# targets, which inherit add_compile_options flags. Vendored code may trigger +# promoted warnings (-Werror=switch-enum, -Werror=useless-cast, etc.). # Call this once at the end of the BUILD_TESTING block. function(ros2_medkit_relax_vendor_warnings) foreach(_target gmock gmock_main gtest gtest_main) if(TARGET ${_target}) - target_compile_options(${_target} PRIVATE -Wno-error) + target_compile_options(${_target} PRIVATE + -Wno-error=switch-enum + -Wno-error=old-style-cast + -Wno-error=useless-cast + -Wno-error=cast-qual + -Wno-error=zero-as-null-pointer-constant + -Wno-error=undef + -Wno-error=extra-semi + -Wno-error=float-equal + -Wno-error=shadow + -Wno-error=format + -Wno-error=implicit-fallthrough + -Wno-error=duplicated-cond + -Wno-error=duplicated-branches + -Wno-error=logical-op + ) endif() endforeach() endfunction() From 2d75f676415e1e6eb48854fa2d85028c98d0bb66 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Mon, 6 Apr 2026 17:41:34 +0200 Subject: [PATCH 05/10] fix(cmake): suppress all warnings on vendored and gtest code with -w Selective -Werror= overrides blanket -Wno-error, so per-flag suppression lists are fragile and break across GCC versions. Use -w (suppress all warnings) on vendored dynmsg sources and gtest/gmock targets instead - these are third-party code we don't modify. --- .../cmake/ROS2MedkitWarnings.cmake | 17 +---------------- src/ros2_medkit_serialization/CMakeLists.txt | 2 +- 2 files changed, 2 insertions(+), 17 deletions(-) diff --git a/src/ros2_medkit_cmake/cmake/ROS2MedkitWarnings.cmake b/src/ros2_medkit_cmake/cmake/ROS2MedkitWarnings.cmake index 2c2d7e821..43d5b4018 100644 --- a/src/ros2_medkit_cmake/cmake/ROS2MedkitWarnings.cmake +++ b/src/ros2_medkit_cmake/cmake/ROS2MedkitWarnings.cmake @@ -129,22 +129,7 @@ endif() function(ros2_medkit_relax_vendor_warnings) foreach(_target gmock gmock_main gtest gtest_main) if(TARGET ${_target}) - target_compile_options(${_target} PRIVATE - -Wno-error=switch-enum - -Wno-error=old-style-cast - -Wno-error=useless-cast - -Wno-error=cast-qual - -Wno-error=zero-as-null-pointer-constant - -Wno-error=undef - -Wno-error=extra-semi - -Wno-error=float-equal - -Wno-error=shadow - -Wno-error=format - -Wno-error=implicit-fallthrough - -Wno-error=duplicated-cond - -Wno-error=duplicated-branches - -Wno-error=logical-op - ) + target_compile_options(${_target} PRIVATE -w) endif() endforeach() endfunction() diff --git a/src/ros2_medkit_serialization/CMakeLists.txt b/src/ros2_medkit_serialization/CMakeLists.txt index 7ca928ac9..133267077 100644 --- a/src/ros2_medkit_serialization/CMakeLists.txt +++ b/src/ros2_medkit_serialization/CMakeLists.txt @@ -57,7 +57,7 @@ set_source_files_properties( src/vendored/dynmsg/string_utils.cpp src/vendored/dynmsg/vector_utils.cpp src/vendored/dynmsg/yaml_utils.cpp - PROPERTIES COMPILE_OPTIONS "-Wno-error;-Wno-old-style-cast;-Wno-sign-conversion" + PROPERTIES COMPILE_OPTIONS "-w" ) # Enable PIC so this static library can be linked into shared objects From a6ec484347f6a0963cd6823aa48d176498a23a04 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Mon, 6 Apr 2026 17:52:17 +0200 Subject: [PATCH 06/10] fix(cmake): demote -Wuseless-cast from error to warning Type aliases (time_t, int64_t, size_t) resolve to different underlying types across distros. A static_cast needed for portability on Jazzy (GCC 13) becomes useless on Humble (GCC 11) where the types match. Keep the warning for visibility but don't fail the build. --- src/ros2_medkit_cmake/cmake/ROS2MedkitWarnings.cmake | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ros2_medkit_cmake/cmake/ROS2MedkitWarnings.cmake b/src/ros2_medkit_cmake/cmake/ROS2MedkitWarnings.cmake index 43d5b4018..ad8a141cc 100644 --- a/src/ros2_medkit_cmake/cmake/ROS2MedkitWarnings.cmake +++ b/src/ros2_medkit_cmake/cmake/ROS2MedkitWarnings.cmake @@ -110,12 +110,14 @@ if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") -Wlogical-op -Wuseless-cast ) + # Note: -Wuseless-cast NOT promoted - type aliases (time_t, int64_t, size_t) + # resolve differently across distros, making casts useless on one but needed + # on another. if(ENABLE_WERROR) add_compile_options( -Werror=duplicated-cond -Werror=duplicated-branches -Werror=logical-op - -Werror=useless-cast ) endif() endif() From eadf4ae3c102dd08b638e6e9d6c6c9c376a69f36 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Mon, 6 Apr 2026 18:14:12 +0200 Subject: [PATCH 07/10] fix(cmake): mark vendored cpp-httplib include as SYSTEM The vendored httplib.h header triggers strict warnings (switch-enum, old-style-cast, format-nonliteral) which become errors with our selective -Werror flags. Mark the vendored include directory as SYSTEM so compiler warnings from httplib.h are suppressed. On Jazzy the system package is used via pkg-config which already treats includes as system headers. --- src/ros2_medkit_cmake/cmake/ROS2MedkitCompat.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ros2_medkit_cmake/cmake/ROS2MedkitCompat.cmake b/src/ros2_medkit_cmake/cmake/ROS2MedkitCompat.cmake index d28794a33..bcb7f5c8a 100644 --- a/src/ros2_medkit_cmake/cmake/ROS2MedkitCompat.cmake +++ b/src/ros2_medkit_cmake/cmake/ROS2MedkitCompat.cmake @@ -98,7 +98,7 @@ macro(medkit_find_cpp_httplib) message(STATUS "[MedkitCompat] cpp-httplib: using cmake config (source build)") elseif(_mfch_VENDORED_DIR AND EXISTS "${_mfch_VENDORED_DIR}/httplib.h") add_library(cpp_httplib_vendored INTERFACE) - target_include_directories(cpp_httplib_vendored INTERFACE "${_mfch_VENDORED_DIR}") + target_include_directories(cpp_httplib_vendored SYSTEM INTERFACE "${_mfch_VENDORED_DIR}") add_library(cpp_httplib_target ALIAS cpp_httplib_vendored) message(STATUS "[MedkitCompat] cpp-httplib: using vendored header (${_mfch_VENDORED_DIR}/httplib.h)") else() From 385fe13cec1f17364a1eee1e14c36eda7823c852 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Mon, 6 Apr 2026 19:29:05 +0200 Subject: [PATCH 08/10] fix(gateway,fault_manager): fix format specifier portability issues Use PRId64 with static_cast for portable printf formatting of parameter values and chrono durations. The underlying type of declare_parameter and chrono::milliseconds::rep varies across platforms (long vs long long), so explicit casts ensure format specifier agreement. --- src/ros2_medkit_fault_manager/src/fault_manager_node.cpp | 4 +++- .../src/http/handlers/sse_fault_handler.cpp | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/ros2_medkit_fault_manager/src/fault_manager_node.cpp b/src/ros2_medkit_fault_manager/src/fault_manager_node.cpp index e88809927..758f7df8c 100644 --- a/src/ros2_medkit_fault_manager/src/fault_manager_node.cpp +++ b/src/ros2_medkit_fault_manager/src/fault_manager_node.cpp @@ -17,6 +17,7 @@ #include #include +#include #include #include #include @@ -110,7 +111,8 @@ FaultManagerNode::FaultManagerNode(const rclcpp::NodeOptions & options) : Node(" } auto max_snapshots = declare_parameter("snapshots.max_per_fault", 10); if (max_snapshots < 0) { - RCLCPP_WARN(get_logger(), "snapshots.max_per_fault should be >= 0, got %ld. Disabling limit.", max_snapshots); + RCLCPP_WARN(get_logger(), "snapshots.max_per_fault should be >= 0, got %" PRId64 ". Disabling limit.", + static_cast(max_snapshots)); max_snapshots = 0; } diff --git a/src/ros2_medkit_gateway/src/http/handlers/sse_fault_handler.cpp b/src/ros2_medkit_gateway/src/http/handlers/sse_fault_handler.cpp index 21289d81b..1da86cb4f 100644 --- a/src/ros2_medkit_gateway/src/http/handlers/sse_fault_handler.cpp +++ b/src/ros2_medkit_gateway/src/http/handlers/sse_fault_handler.cpp @@ -52,7 +52,7 @@ SSEFaultHandler::SSEFaultHandler(HandlerContext & ctx, std::shared_ptr(keepalive_interval.count()), kKeepaliveIntervalSec); } const auto fault_events_topic = build_fault_manager_events_topic(ctx_.node()); From a006a9660654294f38634493935b4db9c40aee0c Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Mon, 6 Apr 2026 21:18:17 +0200 Subject: [PATCH 09/10] fix(cmake): namespace ENABLE_WERROR option as MEDKIT_ENABLE_WERROR Avoid potential collision with other packages in colcon workspace when passing cmake args via colcon build --cmake-args. --- src/ros2_medkit_cmake/cmake/ROS2MedkitWarnings.cmake | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ros2_medkit_cmake/cmake/ROS2MedkitWarnings.cmake b/src/ros2_medkit_cmake/cmake/ROS2MedkitWarnings.cmake index ad8a141cc..e52bb3cd9 100644 --- a/src/ros2_medkit_cmake/cmake/ROS2MedkitWarnings.cmake +++ b/src/ros2_medkit_cmake/cmake/ROS2MedkitWarnings.cmake @@ -22,7 +22,7 @@ include_guard(GLOBAL) # Flags that DO fire on external code remain as warnings for visibility. # # Provides: -# option ENABLE_WERROR (default ON) - selective warnings-as-errors +# option MEDKIT_ENABLE_WERROR (default ON) - selective warnings-as-errors # function ros2_medkit_relax_vendor_warnings() - call after ament_add_gtest/gmock # # Usage: @@ -34,7 +34,7 @@ include_guard(GLOBAL) # ros2_medkit_relax_vendor_warnings() # endif() -option(ENABLE_WERROR "Treat select compiler warnings as errors" ON) +option(MEDKIT_ENABLE_WERROR "Treat select compiler warnings as errors" ON) if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") # -- All warnings (report everything, some as errors, rest as warnings) ------ @@ -85,7 +85,7 @@ if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") # -- Selective -Werror for flags safe from external header false positives ---- # NOT promoted: -Wconversion, -Wsign-conversion, -Wdouble-promotion, # -Wnull-dereference, -Wcast-align (false positives on STL/ROS 2/nlohmann) - if(ENABLE_WERROR) + if(MEDKIT_ENABLE_WERROR) add_compile_options( -Werror=shadow -Werror=switch-enum @@ -113,7 +113,7 @@ if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") # Note: -Wuseless-cast NOT promoted - type aliases (time_t, int64_t, size_t) # resolve differently across distros, making casts useless on one but needed # on another. - if(ENABLE_WERROR) + if(MEDKIT_ENABLE_WERROR) add_compile_options( -Werror=duplicated-cond -Werror=duplicated-branches From a2dc0ece9a519bd307c1206f053de25f32c91438 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Mon, 6 Apr 2026 21:29:38 +0200 Subject: [PATCH 10/10] fix: address review feedback on warning infrastructure - Separate ParameterErrorCode::NONE from error cases in classify_error_code with comment explaining it is unreachable - Add comment to ros2_medkit_msgs explaining why ROS2MedkitWarnings is not included (rosidl-generated code) - Simplify format specifier in fault_manager_node: use %ld directly instead of PRId64 + static_cast (declare_parameter returns long) --- src/ros2_medkit_fault_manager/src/fault_manager_node.cpp | 4 +--- .../src/http/handlers/config_handlers.cpp | 5 +++++ src/ros2_medkit_msgs/CMakeLists.txt | 2 ++ 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/ros2_medkit_fault_manager/src/fault_manager_node.cpp b/src/ros2_medkit_fault_manager/src/fault_manager_node.cpp index 758f7df8c..e88809927 100644 --- a/src/ros2_medkit_fault_manager/src/fault_manager_node.cpp +++ b/src/ros2_medkit_fault_manager/src/fault_manager_node.cpp @@ -17,7 +17,6 @@ #include #include -#include #include #include #include @@ -111,8 +110,7 @@ FaultManagerNode::FaultManagerNode(const rclcpp::NodeOptions & options) : Node(" } auto max_snapshots = declare_parameter("snapshots.max_per_fault", 10); if (max_snapshots < 0) { - RCLCPP_WARN(get_logger(), "snapshots.max_per_fault should be >= 0, got %" PRId64 ". Disabling limit.", - static_cast(max_snapshots)); + RCLCPP_WARN(get_logger(), "snapshots.max_per_fault should be >= 0, got %ld. Disabling limit.", max_snapshots); max_snapshots = 0; } diff --git a/src/ros2_medkit_gateway/src/http/handlers/config_handlers.cpp b/src/ros2_medkit_gateway/src/http/handlers/config_handlers.cpp index 9bf387d9c..d65cdfce4 100644 --- a/src/ros2_medkit_gateway/src/http/handlers/config_handlers.cpp +++ b/src/ros2_medkit_gateway/src/http/handlers/config_handlers.cpp @@ -134,6 +134,11 @@ static ErrorClassification classify_error_code(ParameterErrorCode error_code) { result.error_code = ERR_INVALID_PARAMETER; break; case ParameterErrorCode::NONE: + // NONE means "no error" - caller (classify_parameter_error) pre-filters + // this case, so reaching here indicates a programming error. + result.status_code = 500; + result.error_code = ERR_INTERNAL_ERROR; + break; case ParameterErrorCode::SHUT_DOWN: case ParameterErrorCode::INTERNAL_ERROR: default: diff --git a/src/ros2_medkit_msgs/CMakeLists.txt b/src/ros2_medkit_msgs/CMakeLists.txt index 29498b2bb..71ab7039c 100644 --- a/src/ros2_medkit_msgs/CMakeLists.txt +++ b/src/ros2_medkit_msgs/CMakeLists.txt @@ -15,6 +15,8 @@ cmake_minimum_required(VERSION 3.8) project(ros2_medkit_msgs) +# Note: ROS2MedkitWarnings is intentionally NOT included here. +# rosidl-generated code triggers strict warnings we cannot fix. find_package(ament_cmake REQUIRED) find_package(rosidl_default_generators REQUIRED) find_package(builtin_interfaces REQUIRED)