diff --git a/src/ros2_medkit_fault_reporter/CHANGELOG.rst b/src/ros2_medkit_fault_reporter/CHANGELOG.rst index 51eb06477..4ee219e10 100644 --- a/src/ros2_medkit_fault_reporter/CHANGELOG.rst +++ b/src/ros2_medkit_fault_reporter/CHANGELOG.rst @@ -2,6 +2,11 @@ Changelog for package ros2_medkit_fault_reporter ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +Forthcoming +----------- +* ``FaultReporter`` can now be constructed from an ``rclcpp_lifecycle::LifecycleNode`` (and from a plain ``rclcpp::Node`` reference or explicit node interfaces), enabling use inside lifecycle nodes (`#555 `_) +* Contributors: @zeerekahmad + 0.6.0 (2026-06-22) ------------------ * No functional changes; version bump for the coordinated 0.6.0 release. diff --git a/src/ros2_medkit_fault_reporter/CMakeLists.txt b/src/ros2_medkit_fault_reporter/CMakeLists.txt index ca8d838fd..9868716f3 100644 --- a/src/ros2_medkit_fault_reporter/CMakeLists.txt +++ b/src/ros2_medkit_fault_reporter/CMakeLists.txt @@ -38,6 +38,7 @@ find_package(ament_cmake REQUIRED) include(ROS2MedkitCompat) find_package(rclcpp REQUIRED) +find_package(rclcpp_lifecycle REQUIRED) find_package(ros2_medkit_msgs REQUIRED) # Library target @@ -53,6 +54,7 @@ target_include_directories(fault_reporter_lib PUBLIC medkit_target_dependencies(fault_reporter_lib rclcpp + rclcpp_lifecycle ros2_medkit_msgs ) @@ -76,7 +78,7 @@ install(DIRECTORY include/ # Export for downstream packages ament_export_targets(export_${PROJECT_NAME} HAS_LIBRARY_TARGET) -ament_export_dependencies(rclcpp ros2_medkit_msgs) +ament_export_dependencies(rclcpp rclcpp_lifecycle ros2_medkit_msgs) # Testing if(BUILD_TESTING) @@ -96,13 +98,23 @@ if(BUILD_TESTING) # Unit tests ament_add_gtest(test_local_filter test/test_local_filter.cpp) target_link_libraries(test_local_filter fault_reporter_lib) - medkit_target_dependencies(test_local_filter rclcpp ros2_medkit_msgs) + medkit_target_dependencies(test_local_filter rclcpp rclcpp_lifecycle ros2_medkit_msgs) if(ENABLE_COVERAGE) target_compile_options(test_local_filter PRIVATE --coverage -O0 -g) target_link_options(test_local_filter PRIVATE --coverage) endif() + ament_add_gtest(test_fault_reporter_construction test/test_fault_reporter_construction.cpp) + target_link_libraries(test_fault_reporter_construction fault_reporter_lib) + medkit_target_dependencies(test_fault_reporter_construction rclcpp rclcpp_lifecycle ros2_medkit_msgs) + medkit_set_test_domain(test_fault_reporter_construction) + + if(ENABLE_COVERAGE) + target_compile_options(test_fault_reporter_construction PRIVATE --coverage -O0 -g) + target_link_options(test_fault_reporter_construction PRIVATE --coverage) + endif() + # Integration tests install(DIRECTORY test DESTINATION share/${PROJECT_NAME} diff --git a/src/ros2_medkit_fault_reporter/README.md b/src/ros2_medkit_fault_reporter/README.md index 69a08b74a..bb36182da 100644 --- a/src/ros2_medkit_fault_reporter/README.md +++ b/src/ros2_medkit_fault_reporter/README.md @@ -34,9 +34,47 @@ class MyNode : public rclcpp::Node { That's it! The fault will be immediately confirmed in FaultManager. +### Using with a lifecycle node + +`FaultReporter` also works inside an `rclcpp_lifecycle::LifecycleNode` — construct it from the node +directly (no `shared_from_this()` needed), typically in `on_configure()`: + +```cpp +#include "rclcpp_lifecycle/lifecycle_node.hpp" +#include "ros2_medkit_fault_reporter/fault_reporter.hpp" + +using CallbackReturn = rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn; + +class MyLifecycleNode : public rclcpp_lifecycle::LifecycleNode { + public: + MyLifecycleNode() : LifecycleNode("my_node") {} + + CallbackReturn on_configure(const rclcpp_lifecycle::State &) override { + reporter_ = std::make_unique( + *this, get_fully_qualified_name()); + return CallbackReturn::SUCCESS; + } + + private: + std::unique_ptr reporter_; +}; +``` + +### Constructors + +`FaultReporter` can be built from whichever handle you have: + +- `FaultReporter(rclcpp::Node & node, source_id[, service_name])` +- `FaultReporter(rclcpp::Node::SharedPtr node, source_id[, service_name])` +- `FaultReporter(rclcpp_lifecycle::LifecycleNode & node, source_id[, service_name])` +- `FaultReporter(rclcpp_lifecycle::LifecycleNode::SharedPtr node, source_id[, service_name])` +- `FaultReporter(node_base, node_graph, node_services, node_params, logger, source_id[, service_name])` + — the interface-based constructor the others delegate to, for custom wiring. + ## Features - **Simple API**: Just call `report()` to report a fault +- **Lifecycle node support**: Construct from a regular `Node` or an `rclcpp_lifecycle::LifecycleNode` - **Local Filtering** (optional): Suppress repeated faults until threshold is met - **Per-fault tracking**: Each fault_code has independent filtering - **Severity bypass**: High-severity faults bypass local filtering diff --git a/src/ros2_medkit_fault_reporter/include/ros2_medkit_fault_reporter/fault_reporter.hpp b/src/ros2_medkit_fault_reporter/include/ros2_medkit_fault_reporter/fault_reporter.hpp index 9e1696696..358e31708 100644 --- a/src/ros2_medkit_fault_reporter/include/ros2_medkit_fault_reporter/fault_reporter.hpp +++ b/src/ros2_medkit_fault_reporter/include/ros2_medkit_fault_reporter/fault_reporter.hpp @@ -18,6 +18,7 @@ #include #include "rclcpp/rclcpp.hpp" +#include "rclcpp_lifecycle/lifecycle_node.hpp" #include "ros2_medkit_fault_reporter/local_filter.hpp" #include "ros2_medkit_msgs/srv/report_fault.hpp" @@ -45,9 +46,39 @@ namespace ros2_medkit_fault_reporter { /// std::unique_ptr reporter_; /// }; /// @endcode +/// +/// Inside a lifecycle node, construct it from the node directly (no shared_from_this needed): +/// @code +/// class MyLifecycleNode : public rclcpp_lifecycle::LifecycleNode { +/// public: +/// MyLifecycleNode() : LifecycleNode("my_node") {} +/// +/// rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn +/// on_configure(const rclcpp_lifecycle::State &) override { +/// reporter_ = std::make_unique(*this, get_fully_qualified_name()); +/// return rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn::SUCCESS; +/// } +/// }; +/// @endcode class FaultReporter { public: - /// Construct a FaultReporter + /// Base Constructor for FaultReporter + /// + /// @param node_base ROS2 Node Base Interface for creating the service client + /// @param node_graph ROS2 Node Graph Interface for creating the service client + /// @param node_services ROS2 Node Services Interface for creating the service client + /// @param node_params ROS2 Node Parameters Interface for loading parameters + /// @param logger ROS2 Node Logger for reporting logs + /// @param source_id Identifier for this reporter (typically node's FQN) + /// @param service_name Name of the ReportFault service (default: /fault_manager/report_fault) + FaultReporter(const rclcpp::node_interfaces::NodeBaseInterface::SharedPtr & node_base, + const rclcpp::node_interfaces::NodeGraphInterface::SharedPtr & node_graph, + const rclcpp::node_interfaces::NodeServicesInterface::SharedPtr & node_services, + const rclcpp::node_interfaces::NodeParametersInterface::SharedPtr & node_params, + const rclcpp::Logger & logger, const std::string & source_id, + const std::string & service_name = "/fault_manager/report_fault"); + + /// Construct a FaultReporter via Node Shared Ptr (maintain backwards compatibility) /// /// @param node The ROS 2 node to use for service client and parameters /// @param source_id Identifier for this reporter (typically node's FQN) @@ -55,6 +86,33 @@ class FaultReporter { FaultReporter(const rclcpp::Node::SharedPtr & node, const std::string & source_id, const std::string & service_name = "/fault_manager/report_fault"); + /// Construct a FaultReporter via Node + /// + /// @param node The ROS 2 node to use for service client and parameters + /// @param source_id Identifier for this reporter (typically node's FQN) + /// @param service_name Name of the ReportFault service (default: /fault_manager/report_fault) + FaultReporter(rclcpp::Node & node, const std::string & source_id, + const std::string & service_name = "/fault_manager/report_fault"); + + /// Construct a FaultReporter via Lifecycle Node + /// + /// The node reference is non-const because the ROS 2 node interface getters + /// (get_node_base_interface() and friends) are non-const member functions. + /// + /// @param node The ROS 2 node to use for service client and parameters + /// @param source_id Identifier for this reporter (typically node's FQN) + /// @param service_name Name of the ReportFault service (default: /fault_manager/report_fault) + FaultReporter(rclcpp_lifecycle::LifecycleNode & node, const std::string & source_id, + const std::string & service_name = "/fault_manager/report_fault"); + + /// Construct a FaultReporter via Lifecycle Node Shared Ptr + /// + /// @param node The ROS 2 node to use for service client and parameters + /// @param source_id Identifier for this reporter (typically node's FQN) + /// @param service_name Name of the ReportFault service (default: /fault_manager/report_fault) + FaultReporter(const rclcpp_lifecycle::LifecycleNode::SharedPtr & node, const std::string & source_id, + const std::string & service_name = "/fault_manager/report_fault"); + /// Report a FAILED event (fault occurrence) /// /// The fault will be forwarded to the FaultManager if local filtering @@ -83,7 +141,7 @@ class FaultReporter { private: /// Load filter configuration from ROS parameters - void load_parameters(const rclcpp::Node::SharedPtr & node); + void load_parameters(const rclcpp::node_interfaces::NodeParametersInterface::SharedPtr & node_params); /// Send the fault report to FaultManager (async, fire-and-forget) void send_report(const std::string & fault_code, uint8_t event_type, uint8_t severity, diff --git a/src/ros2_medkit_fault_reporter/package.xml b/src/ros2_medkit_fault_reporter/package.xml index 6c929a9fd..cb2fd5a3a 100644 --- a/src/ros2_medkit_fault_reporter/package.xml +++ b/src/ros2_medkit_fault_reporter/package.xml @@ -12,6 +12,7 @@ ros2_medkit_cmake rclcpp + rclcpp_lifecycle ros2_medkit_msgs ament_lint_auto diff --git a/src/ros2_medkit_fault_reporter/src/fault_reporter.cpp b/src/ros2_medkit_fault_reporter/src/fault_reporter.cpp index b08725b6e..500607283 100644 --- a/src/ros2_medkit_fault_reporter/src/fault_reporter.cpp +++ b/src/ros2_medkit_fault_reporter/src/fault_reporter.cpp @@ -14,48 +14,92 @@ #include "ros2_medkit_fault_reporter/fault_reporter.hpp" +#include "rclcpp/version.h" +#include "rclcpp_lifecycle/lifecycle_node.hpp" + namespace ros2_medkit_fault_reporter { -FaultReporter::FaultReporter(const rclcpp::Node::SharedPtr & node, const std::string & source_id, +FaultReporter::FaultReporter(const rclcpp::node_interfaces::NodeBaseInterface::SharedPtr & node_base, + const rclcpp::node_interfaces::NodeGraphInterface::SharedPtr & node_graph, + const rclcpp::node_interfaces::NodeServicesInterface::SharedPtr & node_services, + const rclcpp::node_interfaces::NodeParametersInterface::SharedPtr & node_params, + const rclcpp::Logger & logger, const std::string & source_id, const std::string & service_name) - : source_id_(source_id), logger_(node->get_logger()) { + : source_id_(source_id), logger_(logger) { // Validate source_id if (source_id_.empty()) { RCLCPP_WARN(logger_, "FaultReporter created with empty source_id, fault origins will be difficult to trace"); } - // Create service client (stored as shared_ptr, independent of node lifetime) - client_ = node->create_client(service_name); + // Create service client. It outlives the node handle used to construct it, but still relies on the + // ROS context and the node interfaces staying valid - reporting after node teardown / rclcpp shutdown + // is not safe. +#if RCLCPP_VERSION_GTE(29, 0, 0) // Kilted and later + client_ = rclcpp::create_client(node_base, node_graph, node_services, + service_name, rclcpp::ServicesQoS()); +#else // Humble, Jazzy + client_ = rclcpp::create_client( + node_base, node_graph, node_services, service_name, rmw_qos_profile_services_default, nullptr); +#endif // Load configuration from parameters (node only needed during construction) - load_parameters(node); + load_parameters(node_params); RCLCPP_DEBUG(logger_, "FaultReporter initialized for source: %s", source_id_.c_str()); } -void FaultReporter::load_parameters(const rclcpp::Node::SharedPtr & node) { +FaultReporter::FaultReporter(const rclcpp::Node::SharedPtr & node, const std::string & source_id, + const std::string & service_name) + : FaultReporter(node->get_node_base_interface(), node->get_node_graph_interface(), + node->get_node_services_interface(), node->get_node_parameters_interface(), node->get_logger(), + source_id, service_name) { +} + +FaultReporter::FaultReporter(rclcpp::Node & node, const std::string & source_id, const std::string & service_name) + : FaultReporter(node.get_node_base_interface(), node.get_node_graph_interface(), node.get_node_services_interface(), + node.get_node_parameters_interface(), node.get_logger(), source_id, service_name) { +} + +FaultReporter::FaultReporter(rclcpp_lifecycle::LifecycleNode & node, const std::string & source_id, + const std::string & service_name) + : FaultReporter(node.get_node_base_interface(), node.get_node_graph_interface(), node.get_node_services_interface(), + node.get_node_parameters_interface(), node.get_logger(), source_id, service_name) { +} + +FaultReporter::FaultReporter(const rclcpp_lifecycle::LifecycleNode::SharedPtr & node, const std::string & source_id, + const std::string & service_name) + : FaultReporter(node->get_node_base_interface(), node->get_node_graph_interface(), + node->get_node_services_interface(), node->get_node_parameters_interface(), node->get_logger(), + source_id, service_name) { +} + +void FaultReporter::load_parameters(const rclcpp::node_interfaces::NodeParametersInterface::SharedPtr & node_params) { FilterConfig config; // Declare parameters with defaults if not already declared - if (!node->has_parameter("fault_reporter.local_filtering.enabled")) { - node->declare_parameter("fault_reporter.local_filtering.enabled", config.enabled); + if (!node_params->has_parameter("fault_reporter.local_filtering.enabled")) { + node_params->declare_parameter("fault_reporter.local_filtering.enabled", rclcpp::ParameterValue(config.enabled)); } - if (!node->has_parameter("fault_reporter.local_filtering.default_threshold")) { - node->declare_parameter("fault_reporter.local_filtering.default_threshold", config.default_threshold); + if (!node_params->has_parameter("fault_reporter.local_filtering.default_threshold")) { + node_params->declare_parameter("fault_reporter.local_filtering.default_threshold", + rclcpp::ParameterValue(config.default_threshold)); } - if (!node->has_parameter("fault_reporter.local_filtering.default_window_sec")) { - node->declare_parameter("fault_reporter.local_filtering.default_window_sec", config.default_window_sec); + if (!node_params->has_parameter("fault_reporter.local_filtering.default_window_sec")) { + node_params->declare_parameter("fault_reporter.local_filtering.default_window_sec", + rclcpp::ParameterValue(config.default_window_sec)); } - if (!node->has_parameter("fault_reporter.local_filtering.bypass_severity")) { - node->declare_parameter("fault_reporter.local_filtering.bypass_severity", static_cast(config.bypass_severity)); + if (!node_params->has_parameter("fault_reporter.local_filtering.bypass_severity")) { + node_params->declare_parameter("fault_reporter.local_filtering.bypass_severity", + rclcpp::ParameterValue(static_cast(config.bypass_severity))); } - config.enabled = node->get_parameter("fault_reporter.local_filtering.enabled").as_bool(); + config.enabled = node_params->get_parameter("fault_reporter.local_filtering.enabled").as_bool(); config.default_threshold = - static_cast(node->get_parameter("fault_reporter.local_filtering.default_threshold").as_int()); - config.default_window_sec = node->get_parameter("fault_reporter.local_filtering.default_window_sec").as_double(); + static_cast(node_params->get_parameter("fault_reporter.local_filtering.default_threshold").as_int()); + config.default_window_sec = + node_params->get_parameter("fault_reporter.local_filtering.default_window_sec").as_double(); config.bypass_severity = - static_cast(node->get_parameter("fault_reporter.local_filtering.bypass_severity").as_int()); + static_cast(node_params->get_parameter("fault_reporter.local_filtering.bypass_severity").as_int()); filter_.set_config(config); diff --git a/src/ros2_medkit_fault_reporter/test/test_fault_reporter_construction.cpp b/src/ros2_medkit_fault_reporter/test/test_fault_reporter_construction.cpp new file mode 100644 index 000000000..a15ac49c0 --- /dev/null +++ b/src/ros2_medkit_fault_reporter/test/test_fault_reporter_construction.cpp @@ -0,0 +1,93 @@ +// Copyright 2025 mfaferek93 +// +// 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 + +#include + +#include "rclcpp/rclcpp.hpp" +#include "rclcpp_lifecycle/lifecycle_node.hpp" +#include "ros2_medkit_fault_reporter/fault_reporter.hpp" + +using ros2_medkit_fault_reporter::FaultReporter; + +namespace { + +// The filter parameters the base constructor declares on the node during construction. +constexpr const char * kEnabledParam = "fault_reporter.local_filtering.enabled"; + +} // namespace + +class FaultReporterConstructionTest : public ::testing::Test { + protected: + void SetUp() override { + rclcpp::init(0, nullptr); + } + + void TearDown() override { + rclcpp::shutdown(); + } +}; + +TEST_F(FaultReporterConstructionTest, ConstructsFromNodeReference) { + auto node = std::make_shared("fault_reporter_node_ref"); + + FaultReporter reporter(*node, "test_source"); + + EXPECT_TRUE(node->has_parameter(kEnabledParam)); + EXPECT_NO_THROW((void)reporter.is_service_ready()); +} + +TEST_F(FaultReporterConstructionTest, ConstructsFromNodeSharedPtr) { + auto node = std::make_shared("fault_reporter_node_shared"); + + FaultReporter reporter(node, "test_source"); + + EXPECT_TRUE(node->has_parameter(kEnabledParam)); + EXPECT_NO_THROW((void)reporter.is_service_ready()); +} + +TEST_F(FaultReporterConstructionTest, ConstructsFromLifecycleNodeReference) { + auto node = std::make_shared("fault_reporter_lifecycle_ref"); + + FaultReporter reporter(*node, "test_source"); + + EXPECT_TRUE(node->has_parameter(kEnabledParam)); + EXPECT_NO_THROW((void)reporter.is_service_ready()); +} + +TEST_F(FaultReporterConstructionTest, ConstructsFromLifecycleNodeSharedPtr) { + auto node = std::make_shared("fault_reporter_lifecycle_shared"); + + FaultReporter reporter(node, "test_source"); + + EXPECT_TRUE(node->has_parameter(kEnabledParam)); + EXPECT_NO_THROW((void)reporter.is_service_ready()); +} + +TEST_F(FaultReporterConstructionTest, ConstructsFromInterfaces) { + auto node = std::make_shared("fault_reporter_interfaces"); + + FaultReporter reporter(node->get_node_base_interface(), node->get_node_graph_interface(), + node->get_node_services_interface(), node->get_node_parameters_interface(), node->get_logger(), + "test_source"); + + EXPECT_TRUE(node->has_parameter(kEnabledParam)); + EXPECT_NO_THROW((void)reporter.is_service_ready()); +} + +int main(int argc, char ** argv) { + testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +}