From 558d9ddfea4bb2dc9e36359362a9410b05ca7b05 Mon Sep 17 00:00:00 2001 From: mfaferek93 Date: Thu, 2 Jul 2026 15:42:29 +0200 Subject: [PATCH 1/4] fix(ci): suppress clang-tidy in vendored headers matched by the header filter The quality job's -header-filter='.*ros2_medkit.*' matches vendored headers because their install paths contain the package name, so any TU including them (e.g. via lock_manager.hpp) fails with third-party warnings. Wrap them in NOLINTBEGIN/NOLINTEND. --- .../src/vendored/tl_expected/include/tl/expected.hpp | 3 +++ .../ros2_medkit_serialization/vendored/dynmsg/config.hpp | 3 +++ .../vendored/dynmsg/message_reading.hpp | 3 +++ .../ros2_medkit_serialization/vendored/dynmsg/msg_parser.hpp | 3 +++ .../ros2_medkit_serialization/vendored/dynmsg/string_utils.hpp | 3 +++ .../include/ros2_medkit_serialization/vendored/dynmsg/types.h | 3 +++ .../ros2_medkit_serialization/vendored/dynmsg/typesupport.hpp | 3 +++ .../ros2_medkit_serialization/vendored/dynmsg/vector_utils.hpp | 3 +++ .../ros2_medkit_serialization/vendored/dynmsg/yaml_utils.hpp | 3 +++ 9 files changed, 27 insertions(+) diff --git a/src/ros2_medkit_gateway/src/vendored/tl_expected/include/tl/expected.hpp b/src/ros2_medkit_gateway/src/vendored/tl_expected/include/tl/expected.hpp index 59e59aa1b..244e63b20 100644 --- a/src/ros2_medkit_gateway/src/vendored/tl_expected/include/tl/expected.hpp +++ b/src/ros2_medkit_gateway/src/vendored/tl_expected/include/tl/expected.hpp @@ -1,3 +1,5 @@ +// Vendored third-party header, excluded from clang-tidy lint (installed path matches the CI header filter). +// NOLINTBEGIN /// // expected - An implementation of std::expected with extensions // Written in 2017 by Sy Brand (tartanllama@gmail.com, @TartanLlama) @@ -2473,3 +2475,4 @@ void swap(expected &lhs, } // namespace tl #endif +// NOLINTEND diff --git a/src/ros2_medkit_serialization/include/ros2_medkit_serialization/vendored/dynmsg/config.hpp b/src/ros2_medkit_serialization/include/ros2_medkit_serialization/vendored/dynmsg/config.hpp index 56170e243..2bca4e043 100644 --- a/src/ros2_medkit_serialization/include/ros2_medkit_serialization/vendored/dynmsg/config.hpp +++ b/src/ros2_medkit_serialization/include/ros2_medkit_serialization/vendored/dynmsg/config.hpp @@ -1,3 +1,5 @@ +// Vendored third-party header, excluded from clang-tidy lint (installed path matches the CI header filter). +// NOLINTBEGIN // Copyright 2021 Christophe Bedard // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -43,3 +45,4 @@ #endif // DYNMSG_PARSER_DEBUG #endif // ROS2_MEDKIT_SERIALIZATION__VENDORED__DYNMSG__CONFIG_HPP_ +// NOLINTEND diff --git a/src/ros2_medkit_serialization/include/ros2_medkit_serialization/vendored/dynmsg/message_reading.hpp b/src/ros2_medkit_serialization/include/ros2_medkit_serialization/vendored/dynmsg/message_reading.hpp index 0682bec84..dcd7a66da 100644 --- a/src/ros2_medkit_serialization/include/ros2_medkit_serialization/vendored/dynmsg/message_reading.hpp +++ b/src/ros2_medkit_serialization/include/ros2_medkit_serialization/vendored/dynmsg/message_reading.hpp @@ -1,3 +1,5 @@ +// Vendored third-party header, excluded from clang-tidy lint (installed path matches the CI header filter). +// NOLINTBEGIN // Copyright 2020 Open Source Robotics Foundation, Inc. // Copyright 2021 Christophe Bedard // @@ -57,3 +59,4 @@ YAML::Node message_to_yaml(const RosMessage_Cpp & message); } // namespace dynmsg #endif // ROS2_MEDKIT_SERIALIZATION__VENDORED__DYNMSG__MESSAGE_READING_HPP_ +// NOLINTEND diff --git a/src/ros2_medkit_serialization/include/ros2_medkit_serialization/vendored/dynmsg/msg_parser.hpp b/src/ros2_medkit_serialization/include/ros2_medkit_serialization/vendored/dynmsg/msg_parser.hpp index ef29ab68a..912d44ed5 100644 --- a/src/ros2_medkit_serialization/include/ros2_medkit_serialization/vendored/dynmsg/msg_parser.hpp +++ b/src/ros2_medkit_serialization/include/ros2_medkit_serialization/vendored/dynmsg/msg_parser.hpp @@ -1,3 +1,5 @@ +// Vendored third-party header, excluded from clang-tidy lint (installed path matches the CI header filter). +// NOLINTBEGIN // Copyright 2020 Open Source Robotics Foundation, Inc. // Copyright 2021 Christophe Bedard // @@ -86,3 +88,4 @@ void yaml_and_typeinfo_to_rosmsg( } // namespace dynmsg #endif // ROS2_MEDKIT_SERIALIZATION__VENDORED__DYNMSG__MSG_PARSER_HPP_ +// NOLINTEND diff --git a/src/ros2_medkit_serialization/include/ros2_medkit_serialization/vendored/dynmsg/string_utils.hpp b/src/ros2_medkit_serialization/include/ros2_medkit_serialization/vendored/dynmsg/string_utils.hpp index d242ddf0f..487c3db70 100644 --- a/src/ros2_medkit_serialization/include/ros2_medkit_serialization/vendored/dynmsg/string_utils.hpp +++ b/src/ros2_medkit_serialization/include/ros2_medkit_serialization/vendored/dynmsg/string_utils.hpp @@ -1,3 +1,5 @@ +// Vendored third-party header, excluded from clang-tidy lint (installed path matches the CI header filter). +// NOLINTBEGIN // Copyright 2020 Open Source Robotics Foundation, Inc. // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -27,3 +29,4 @@ std::string u16string_to_string(const std::u16string & input); } // extern "C" #endif // ROS2_MEDKIT_SERIALIZATION__VENDORED__DYNMSG__STRING_UTILS_HPP_ +// NOLINTEND diff --git a/src/ros2_medkit_serialization/include/ros2_medkit_serialization/vendored/dynmsg/types.h b/src/ros2_medkit_serialization/include/ros2_medkit_serialization/vendored/dynmsg/types.h index 49d45078d..283e2117e 100644 --- a/src/ros2_medkit_serialization/include/ros2_medkit_serialization/vendored/dynmsg/types.h +++ b/src/ros2_medkit_serialization/include/ros2_medkit_serialization/vendored/dynmsg/types.h @@ -1,3 +1,5 @@ +// Vendored third-party header, excluded from clang-tidy lint (installed path matches the CI header filter). +// NOLINTBEGIN // Copyright 2020 Open Source Robotics Foundation, Inc. // Copyright 2021 Christophe Bedard // @@ -27,3 +29,4 @@ typedef rcutils_ret_t dynmsg_ret_t; #define DYNMSG_RET_ERROR RCUTILS_RET_ERROR #endif // ROS2_MEDKIT_SERIALIZATION__VENDORED__DYNMSG__TYPES_H_ +// NOLINTEND diff --git a/src/ros2_medkit_serialization/include/ros2_medkit_serialization/vendored/dynmsg/typesupport.hpp b/src/ros2_medkit_serialization/include/ros2_medkit_serialization/vendored/dynmsg/typesupport.hpp index 43aad97be..03584facb 100644 --- a/src/ros2_medkit_serialization/include/ros2_medkit_serialization/vendored/dynmsg/typesupport.hpp +++ b/src/ros2_medkit_serialization/include/ros2_medkit_serialization/vendored/dynmsg/typesupport.hpp @@ -1,3 +1,5 @@ +// Vendored third-party header, excluded from clang-tidy lint (installed path matches the CI header filter). +// NOLINTBEGIN // Copyright 2020 Open Source Robotics Foundation, Inc. // Copyright 2021 Christophe Bedard // @@ -161,3 +163,4 @@ void ros_message_destroy_with_allocator(RosMessage_Cpp * ros_msg, rcutils_alloca } // namespace dynmsg #endif // ROS2_MEDKIT_SERIALIZATION__VENDORED__DYNMSG__TYPESUPPORT_HPP_ +// NOLINTEND diff --git a/src/ros2_medkit_serialization/include/ros2_medkit_serialization/vendored/dynmsg/vector_utils.hpp b/src/ros2_medkit_serialization/include/ros2_medkit_serialization/vendored/dynmsg/vector_utils.hpp index c60896061..0c1778837 100644 --- a/src/ros2_medkit_serialization/include/ros2_medkit_serialization/vendored/dynmsg/vector_utils.hpp +++ b/src/ros2_medkit_serialization/include/ros2_medkit_serialization/vendored/dynmsg/vector_utils.hpp @@ -1,3 +1,5 @@ +// Vendored third-party header, excluded from clang-tidy lint (installed path matches the CI header filter). +// NOLINTBEGIN // Copyright 2021 Christophe Bedard // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -34,3 +36,4 @@ size_t get_vector_size(const uint8_t * vector, size_t element_size); } // namespace dynmsg #endif // ROS2_MEDKIT_SERIALIZATION__VENDORED__DYNMSG__VECTOR_UTILS_HPP_ +// NOLINTEND diff --git a/src/ros2_medkit_serialization/include/ros2_medkit_serialization/vendored/dynmsg/yaml_utils.hpp b/src/ros2_medkit_serialization/include/ros2_medkit_serialization/vendored/dynmsg/yaml_utils.hpp index a5f8e92b7..e523eae6c 100644 --- a/src/ros2_medkit_serialization/include/ros2_medkit_serialization/vendored/dynmsg/yaml_utils.hpp +++ b/src/ros2_medkit_serialization/include/ros2_medkit_serialization/vendored/dynmsg/yaml_utils.hpp @@ -1,3 +1,5 @@ +// Vendored third-party header, excluded from clang-tidy lint (installed path matches the CI header filter). +// NOLINTBEGIN // Copyright 2021 Christophe Bedard // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -40,3 +42,4 @@ std::string yaml_to_string( } // namespace dynmsg #endif // ROS2_MEDKIT_SERIALIZATION__VENDORED__DYNMSG__YAML_UTILS_HPP_ +// NOLINTEND From 60a6f05ff53f1c9eb2f7d9fd08d1fe6098ce9368 Mon Sep 17 00:00:00 2001 From: mfaferek93 Date: Thu, 2 Jul 2026 15:42:37 +0200 Subject: [PATCH 2/4] fix(test): raise flaky fault-surface deadline in secured OPC-UA test On slow CI runners the fired alarm does not always propagate through report -> fault_manager -> REST within 40s. Raise the wait to 120s and the CTest timeout to 420s so a failing run still prints diagnostics. Passing runs exit the poll early and are unaffected. --- src/ros2_medkit_plugins/ros2_medkit_opcua/CMakeLists.txt | 5 ++++- .../test/integration/test_opcua_secured.test.py | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/CMakeLists.txt b/src/ros2_medkit_plugins/ros2_medkit_opcua/CMakeLists.txt index 563d257a2..5f60f3432 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/CMakeLists.txt +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/CMakeLists.txt @@ -351,10 +351,13 @@ if(BUILD_TESTING) "${CMAKE_CURRENT_SOURCE_DIR}/test/integration/test_opcua_secured.test.py" "${CMAKE_BINARY_DIR}/test_alarm_server" "${CMAKE_CURRENT_SOURCE_DIR}/test/integration/gen_test_certs.sh") + # TIMEOUT must exceed the sum of the script's internal wait deadlines so a + # slow failing run still reaches its own FAIL diagnostics instead of being + # killed by CTest. set_tests_properties(test_opcua_secured PROPERTIES LABELS "integration" SKIP_RETURN_CODE 77 - TIMEOUT 300) + TIMEOUT 420) endif() ros2_medkit_relax_vendor_warnings() diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/test/integration/test_opcua_secured.test.py b/src/ros2_medkit_plugins/ros2_medkit_opcua/test/integration/test_opcua_secured.test.py index fa6b889c2..3498ea552 100755 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/test/integration/test_opcua_secured.test.py +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/test/integration/test_opcua_secured.test.py @@ -358,10 +358,13 @@ def main(): # Secured A&C: fire an alarm over the server stdin -> CONFIRMED fault. server.stdin.write('fire Overpressure 750\n') server.stdin.flush() + # Generous deadline for slow CI runners: the alarm has already fired on + # the server, but report -> fault_manager -> REST propagation can take + # a while under load. A longer deadline only slows the failure path. faults = wait_json( f'{base}/faults', lambda j: any(i.get('fault_code') == ALARM_CODE and i.get('status') == 'CONFIRMED' - for i in j.get('items', [])), deadline=40) + for i in j.get('items', [])), deadline=120) if not (faults and any(i.get('fault_code') == ALARM_CODE and i.get('status') == 'CONFIRMED' for i in faults.get('items', []))): print('FAIL: alarm did not surface as a CONFIRMED fault', file=sys.stderr) From 8d1a9f1e1b91cc8a06e02cc1e26e6013cb3f49f0 Mon Sep 17 00:00:00 2001 From: mfaferek93 Date: Thu, 2 Jul 2026 16:39:51 +0200 Subject: [PATCH 3/4] fix(opcua): mark bundled open62541 include dirs as SYSTEM Only open62541pp's own interface dirs were marked SYSTEM; the bundled open62541 target's dirs stayed plain -I, so including a header like client_highlevel.h turns its old-style casts into hard clang-tidy errors via -Werror=old-style-cast. --- .../ros2_medkit_opcua/CMakeLists.txt | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/CMakeLists.txt b/src/ros2_medkit_plugins/ros2_medkit_opcua/CMakeLists.txt index 5f60f3432..54481e1eb 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/CMakeLists.txt +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/CMakeLists.txt @@ -74,19 +74,23 @@ fetchcontent_makeavailable(open62541pp) set_directory_properties(PROPERTIES COMPILE_OPTIONS "${_medkit_saved_compile_options}") unset(_medkit_saved_compile_options) -# Mark open62541pp's interface include directories as SYSTEM so consumers -# (our plugin and tests) do not propagate strict warnings into third-party -# header code. CMake propagates SYSTEM-ness only when the interface property -# is explicitly marked; we do that here on the open62541pp target so downstream -# target_link_libraries picks it up automatically. -if(TARGET open62541pp) - get_target_property(_op62_includes open62541pp INTERFACE_INCLUDE_DIRECTORIES) - if(_op62_includes) - set_target_properties(open62541pp PROPERTIES - INTERFACE_SYSTEM_INCLUDE_DIRECTORIES "${_op62_includes}") +# Mark the interface include directories of open62541pp AND the bundled +# open62541 target as SYSTEM so consumers (our plugin and tests) do not +# propagate strict warnings into third-party header code. CMake propagates +# SYSTEM-ness only when the interface property is explicitly marked. The +# open62541 target matters for clang-tidy: its headers (e.g. +# client_highlevel.h) contain old-style casts that -Werror=old-style-cast +# turns into hard errors when reached through a plain -I path. +foreach(_op62_target open62541pp open62541) + if(TARGET ${_op62_target}) + get_target_property(_op62_includes ${_op62_target} INTERFACE_INCLUDE_DIRECTORIES) + if(_op62_includes) + set_target_properties(${_op62_target} PROPERTIES + INTERFACE_SYSTEM_INCLUDE_DIRECTORIES "${_op62_includes}") + endif() + unset(_op62_includes) endif() - unset(_op62_includes) -endif() +endforeach() # ---- MODULE target: loaded via dlopen at runtime by PluginManager ---- # Symbols from ros2_medkit_gateway are resolved from the host process at runtime. From bd4df05a7ef4904733ed0c320434c2d03e37e72e Mon Sep 17 00:00:00 2001 From: mfaferek93 Date: Thu, 2 Jul 2026 16:40:00 +0200 Subject: [PATCH 4/4] fix(opcua): resolve clang-tidy findings in plugin sources Complete the special-member sets of OpcuaClient, OpcuaPlugin and OpcuaPoller with deleted moves, drop a no-effect std::move on the trivially copyable Subscription handle, build event-field Variants via the deep-copy constructor, and convert an index loop to range-for. --- .../include/ros2_medkit_opcua/opcua_client.hpp | 2 ++ .../include/ros2_medkit_opcua/opcua_plugin.hpp | 5 +++++ .../include/ros2_medkit_opcua/opcua_poller.hpp | 2 ++ .../ros2_medkit_opcua/src/opcua_client.cpp | 17 +++++++---------- 4 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/opcua_client.hpp b/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/opcua_client.hpp index 3c46f69c2..04b0a5a7d 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/opcua_client.hpp +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/opcua_client.hpp @@ -110,6 +110,8 @@ class OpcuaClient { OpcuaClient(const OpcuaClient &) = delete; OpcuaClient & operator=(const OpcuaClient &) = delete; + OpcuaClient(OpcuaClient &&) = delete; + OpcuaClient & operator=(OpcuaClient &&) = delete; /// Connect to OPC-UA server /// @return true if connected successfully diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/opcua_plugin.hpp b/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/opcua_plugin.hpp index 3fda44993..9c62123aa 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/opcua_plugin.hpp +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/opcua_plugin.hpp @@ -71,6 +71,11 @@ class OpcuaPlugin : public ros2_medkit_gateway::GatewayPlugin, OpcuaPlugin(); ~OpcuaPlugin() override; + OpcuaPlugin(const OpcuaPlugin &) = delete; + OpcuaPlugin & operator=(const OpcuaPlugin &) = delete; + OpcuaPlugin(OpcuaPlugin &&) = delete; + OpcuaPlugin & operator=(OpcuaPlugin &&) = delete; + // -- GatewayPlugin interface -- std::string name() const override { return "opcua"; diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/opcua_poller.hpp b/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/opcua_poller.hpp index eb3f9712e..83b502a0e 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/opcua_poller.hpp +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/opcua_poller.hpp @@ -130,6 +130,8 @@ class OpcuaPoller { OpcuaPoller(const OpcuaPoller &) = delete; OpcuaPoller & operator=(const OpcuaPoller &) = delete; + OpcuaPoller(OpcuaPoller &&) = delete; + OpcuaPoller & operator=(OpcuaPoller &&) = delete; /// Start the poller thread void start(const PollerConfig & config); diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_client.cpp b/src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_client.cpp index 9d7df4628..0ff03414f 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_client.cpp +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_client.cpp @@ -764,7 +764,7 @@ uint32_t OpcuaClient::create_subscription(double publish_interval_ms, DataChange uint32_t sub_id = sub.subscriptionId(); std::lock_guard sub_lock(impl_->sub_mutex); - impl_->subscriptions.push_back({std::move(sub), std::move(callback)}); + impl_->subscriptions.push_back({sub, std::move(callback)}); return sub_id; } catch (const opcua::BadStatus &) { @@ -1118,16 +1118,13 @@ static void on_event_trampoline_c(UA_Client * /*client*/, UA_UInt32 sub_id, void return; } - // Copy the UA_Variant fields into open62541pp wrappers. UA_Variant_copy - // duplicates the underlying buffer, which the opcua::Variant destructor - // will free. + // Copy the UA_Variant fields into open62541pp wrappers. The Variant + // lvalue constructor deep-copies the underlying buffer, which the + // opcua::Variant destructor will free. std::vector values; values.reserve(n_fields); for (size_t i = 0; i < n_fields; ++i) { - UA_Variant copy; - UA_Variant_init(©); - UA_Variant_copy(&fields[i], ©); - values.emplace_back(opcua::Variant{std::move(copy)}); + values.emplace_back(opcua::Variant{fields[i]}); } // Auto-prepended positions (matching make_event_filter): @@ -1349,8 +1346,8 @@ OpcuaClient::call_method(const opcua::NodeId & object_id, const opcua::NodeId & auto arg_results = result.getInputArgumentResults(); std::vector arg_codes; arg_codes.reserve(arg_results.size()); - for (size_t i = 0; i < arg_results.size(); ++i) { - arg_codes.push_back(arg_results[i].get()); + for (const auto & arg_result : arg_results) { + arg_codes.push_back(arg_result.get()); } if (client_debug_enabled()) { // RCLCPP_DEBUG_STREAM constructs its std::stringstream unconditionally,