From 84c5d568e73c40e766be43391957bfb90de0924c Mon Sep 17 00:00:00 2001 From: Michal Faferek Date: Sun, 21 Jun 2026 19:00:34 +0200 Subject: [PATCH 1/2] fix(log_bridge): skip the medkit stack's own nodes by default log_bridge only self-excluded its own node, so fault_manager/gateway/ bridge logs on /rosout were promoted into faults about medkit itself (a feedback loop). Skip the medkit stack by default via a new exclude_medkit_stack param (default true, disable to debug medkit's logs). Addresses #458 --- .../config/log_bridge.yaml | 4 ++++ .../ros2_medkit_log_bridge/log_bridge_node.hpp | 4 ++++ .../src/log_bridge_node.cpp | 11 +++++++++++ .../test/test_log_bridge.cpp | 17 +++++++++++++++++ 4 files changed, 36 insertions(+) diff --git a/src/ros2_medkit_log_bridge/config/log_bridge.yaml b/src/ros2_medkit_log_bridge/config/log_bridge.yaml index 951265c7..991d6600 100644 --- a/src/ros2_medkit_log_bridge/config/log_bridge.yaml +++ b/src/ros2_medkit_log_bridge/config/log_bridge.yaml @@ -21,3 +21,7 @@ log_bridge: # forwarded immediately; the same code within the window is suppressed. # 0.0 disables. Tames ERROR/FATAL floods, which bypass the per-node filter. report_cooldown_sec: 5.0 + # Skip the medkit stack's own infrastructure nodes (fault_manager, gateway, + # the other bridges). Their /rosout lines would otherwise feed back as + # faults about medkit itself. Set false only to debug medkit's own logs. + exclude_medkit_stack: true diff --git a/src/ros2_medkit_log_bridge/include/ros2_medkit_log_bridge/log_bridge_node.hpp b/src/ros2_medkit_log_bridge/include/ros2_medkit_log_bridge/log_bridge_node.hpp index a8f9e850..ebde41b8 100644 --- a/src/ros2_medkit_log_bridge/include/ros2_medkit_log_bridge/log_bridge_node.hpp +++ b/src/ros2_medkit_log_bridge/include/ros2_medkit_log_bridge/log_bridge_node.hpp @@ -121,6 +121,10 @@ class LogBridgeNode : public rclcpp::Node { int max_tracked_nodes_; double report_cooldown_sec_; std::string own_node_name_; + // When true (default), never promote logs from the medkit stack's own + // infrastructure nodes (fault_manager, gateway, the other bridges) - else + // their /rosout lines feed back into faults about medkit itself. + bool exclude_medkit_stack_; }; } // namespace ros2_medkit_log_bridge diff --git a/src/ros2_medkit_log_bridge/src/log_bridge_node.cpp b/src/ros2_medkit_log_bridge/src/log_bridge_node.cpp index e9bcff08..41361468 100644 --- a/src/ros2_medkit_log_bridge/src/log_bridge_node.cpp +++ b/src/ros2_medkit_log_bridge/src/log_bridge_node.cpp @@ -88,6 +88,7 @@ void LogBridgeNode::load_parameters() { if (report_cooldown_sec_ < 0.0) { report_cooldown_sec_ = 0.0; } + exclude_medkit_stack_ = declare_parameter("exclude_medkit_stack", true); } void LogBridgeNode::log_callback(const rcl_interfaces::msg::Log::ConstSharedPtr & msg) { @@ -148,6 +149,16 @@ std::string LogBridgeNode::node_source_id(const std::string & log_name) { } bool LogBridgeNode::node_is_eligible(const std::string & source_id) const { + // Skip the medkit stack's own infrastructure nodes by default: promoting + // their /rosout lines would report faults about medkit itself (e.g. the + // fault_manager logging a confirmed fault feeds back as a new fault). + if (exclude_medkit_stack_) { + static const std::vector kMedkitStack = {"fault_manager", "ros2_medkit_gateway", "diagnostic_bridge", + "action_status_bridge"}; + if (contains_substr(kMedkitStack, source_id)) { + return false; + } + } if (!include_only_nodes_.empty() && !contains_substr(include_only_nodes_, source_id)) { return false; } diff --git a/src/ros2_medkit_log_bridge/test/test_log_bridge.cpp b/src/ros2_medkit_log_bridge/test/test_log_bridge.cpp index 37aafabe..da96eb4c 100644 --- a/src/ros2_medkit_log_bridge/test/test_log_bridge.cpp +++ b/src/ros2_medkit_log_bridge/test/test_log_bridge.cpp @@ -219,6 +219,23 @@ TEST_F(LogBridgeTest, NodeEligibility_IncludeOnlySubstring) { EXPECT_FALSE(node->node_is_eligible("/amcl")); } +TEST_F(LogBridgeTest, NodeEligibility_ExcludesMedkitStackByDefault) { + auto node = make_node_with({}); + // medkit's own infrastructure must not feed its own logs back as faults + EXPECT_FALSE(node->node_is_eligible("/fault_manager")); + EXPECT_FALSE(node->node_is_eligible("/robot1/fault_manager")); + EXPECT_FALSE(node->node_is_eligible("/ros2_medkit_gateway")); + EXPECT_FALSE(node->node_is_eligible("/diagnostic_bridge")); + EXPECT_FALSE(node->node_is_eligible("/action_status_bridge")); + // ordinary application nodes are still promoted + EXPECT_TRUE(node->node_is_eligible("/bt_navigator")); +} + +TEST_F(LogBridgeTest, NodeEligibility_MedkitStackExclusionDisablable) { + auto node = make_node_with({rclcpp::Parameter("exclude_medkit_stack", false)}); + EXPECT_TRUE(node->node_is_eligible("/fault_manager")); +} + // --- source_id normalization to node FQN (entity association) --- TEST_F(LogBridgeTest, NodeSourceId_PrependsLeadingSlash) { From f384e187e713efacfb8cf7b201cfdbfa674a309e Mon Sep 17 00:00:00 2001 From: Michal Faferek Date: Sun, 21 Jun 2026 19:12:01 +0200 Subject: [PATCH 2/2] fix(log_bridge): match medkit stack on the raw logger name Address review: node_source_id collapses a namespaced logger ("robot1.fault_manager") to its namespace ("/robot1"), so matching the medkit stack against source_id missed namespaced nodes. Match the raw logger name (msg->name) instead via a static is_medkit_stack_logger, and test the real namespaced logger-name form. Document the param in the README. --- src/ros2_medkit_log_bridge/README.md | 1 + .../log_bridge_node.hpp | 5 ++++ .../src/log_bridge_node.cpp | 25 +++++++++++------- .../test/test_log_bridge.cpp | 26 +++++++++---------- 4 files changed, 34 insertions(+), 23 deletions(-) diff --git a/src/ros2_medkit_log_bridge/README.md b/src/ros2_medkit_log_bridge/README.md index 1c2098eb..6557a906 100644 --- a/src/ros2_medkit_log_bridge/README.md +++ b/src/ros2_medkit_log_bridge/README.md @@ -94,6 +94,7 @@ ros2 launch ros2_medkit_log_bridge log_bridge.launch.py | `include_only_nodes` | `[]` | if set, only promote nodes whose FQN matches | | `max_tracked_nodes` | `512` | cap on per-node reporters; least-recently-used nodes evicted past this | | `report_cooldown_sec` | `5.0` | per-fault_code forward debounce; `0.0` disables | +| `exclude_medkit_stack` | `true` | skip medkit's own infrastructure nodes (`fault_manager`, gateway, the other bridges) so their logs do not feed back as faults; set `false` to debug medkit's own logs | `exclude_nodes` / `include_only_nodes` match as **unanchored substrings** against the node FQN: `planner` matches `/planner_server` and diff --git a/src/ros2_medkit_log_bridge/include/ros2_medkit_log_bridge/log_bridge_node.hpp b/src/ros2_medkit_log_bridge/include/ros2_medkit_log_bridge/log_bridge_node.hpp index ebde41b8..be8d18e7 100644 --- a/src/ros2_medkit_log_bridge/include/ros2_medkit_log_bridge/log_bridge_node.hpp +++ b/src/ros2_medkit_log_bridge/include/ros2_medkit_log_bridge/log_bridge_node.hpp @@ -63,6 +63,11 @@ class LogBridgeNode : public rclcpp::Node { /// include/exclude lists. Exposed for unit testing. bool node_is_eligible(const std::string & source_id) const; + /// Whether a logger name belongs to the medkit stack's own infrastructure + /// (fault_manager, gateway, the other bridges). Matched on the raw logger + /// name so namespaced nodes are caught. Exposed for unit testing. + static bool is_medkit_stack_logger(const std::string & logger_name); + /// Map an rcl_interfaces/msg/Log.name (a logger name, e.g. "bt_navigator" or /// "controller_manager.resource_manager") to the originating node's /// fully-qualified name ("/bt_navigator", "/controller_manager"). The gateway diff --git a/src/ros2_medkit_log_bridge/src/log_bridge_node.cpp b/src/ros2_medkit_log_bridge/src/log_bridge_node.cpp index 41361468..4ac2100e 100644 --- a/src/ros2_medkit_log_bridge/src/log_bridge_node.cpp +++ b/src/ros2_medkit_log_bridge/src/log_bridge_node.cpp @@ -100,6 +100,12 @@ void LogBridgeNode::log_callback(const rcl_interfaces::msg::Log::ConstSharedPtr if (source_id == own_node_name_ || source_id == "/log_bridge") { return; } + // Skip the medkit stack's own infrastructure nodes (else fault_manager's own + // /rosout lines feed back as faults about medkit). Matched on the raw logger + // name, which keeps the namespace (node_source_id collapses it to the ns). + if (exclude_medkit_stack_ && is_medkit_stack_logger(msg->name)) { + return; + } if (!node_is_eligible(source_id)) { return; } @@ -148,17 +154,16 @@ std::string LogBridgeNode::node_source_id(const std::string & log_name) { return node; } +bool LogBridgeNode::is_medkit_stack_logger(const std::string & logger_name) { + // Match medkit's own infrastructure against the raw logger name so a + // namespaced node (e.g. "robot1.fault_manager") is still caught, even though + // node_source_id would collapse its FQN to the namespace. + static const std::vector kMedkitStack = {"fault_manager", "ros2_medkit_gateway", "diagnostic_bridge", + "action_status_bridge"}; + return contains_substr(kMedkitStack, logger_name); +} + bool LogBridgeNode::node_is_eligible(const std::string & source_id) const { - // Skip the medkit stack's own infrastructure nodes by default: promoting - // their /rosout lines would report faults about medkit itself (e.g. the - // fault_manager logging a confirmed fault feeds back as a new fault). - if (exclude_medkit_stack_) { - static const std::vector kMedkitStack = {"fault_manager", "ros2_medkit_gateway", "diagnostic_bridge", - "action_status_bridge"}; - if (contains_substr(kMedkitStack, source_id)) { - return false; - } - } if (!include_only_nodes_.empty() && !contains_substr(include_only_nodes_, source_id)) { return false; } diff --git a/src/ros2_medkit_log_bridge/test/test_log_bridge.cpp b/src/ros2_medkit_log_bridge/test/test_log_bridge.cpp index da96eb4c..173d2a58 100644 --- a/src/ros2_medkit_log_bridge/test/test_log_bridge.cpp +++ b/src/ros2_medkit_log_bridge/test/test_log_bridge.cpp @@ -219,21 +219,21 @@ TEST_F(LogBridgeTest, NodeEligibility_IncludeOnlySubstring) { EXPECT_FALSE(node->node_is_eligible("/amcl")); } -TEST_F(LogBridgeTest, NodeEligibility_ExcludesMedkitStackByDefault) { - auto node = make_node_with({}); - // medkit's own infrastructure must not feed its own logs back as faults - EXPECT_FALSE(node->node_is_eligible("/fault_manager")); - EXPECT_FALSE(node->node_is_eligible("/robot1/fault_manager")); - EXPECT_FALSE(node->node_is_eligible("/ros2_medkit_gateway")); - EXPECT_FALSE(node->node_is_eligible("/diagnostic_bridge")); - EXPECT_FALSE(node->node_is_eligible("/action_status_bridge")); - // ordinary application nodes are still promoted - EXPECT_TRUE(node->node_is_eligible("/bt_navigator")); +// Matched on the raw logger name (msg->name), which is what log_callback feeds +// it - including the namespaced form "robot1.fault_manager" that node_source_id +// would otherwise collapse to "/robot1". +TEST_F(LogBridgeTest, MedkitStackLogger_MatchesOwnInfra) { + EXPECT_TRUE(LogBridgeNode::is_medkit_stack_logger("fault_manager")); + EXPECT_TRUE(LogBridgeNode::is_medkit_stack_logger("robot1.fault_manager")); + EXPECT_TRUE(LogBridgeNode::is_medkit_stack_logger("ros2_medkit_gateway")); + EXPECT_TRUE(LogBridgeNode::is_medkit_stack_logger("diagnostic_bridge")); + EXPECT_TRUE(LogBridgeNode::is_medkit_stack_logger("action_status_bridge")); } -TEST_F(LogBridgeTest, NodeEligibility_MedkitStackExclusionDisablable) { - auto node = make_node_with({rclcpp::Parameter("exclude_medkit_stack", false)}); - EXPECT_TRUE(node->node_is_eligible("/fault_manager")); +TEST_F(LogBridgeTest, MedkitStackLogger_AllowsApplicationNodes) { + EXPECT_FALSE(LogBridgeNode::is_medkit_stack_logger("bt_navigator")); + EXPECT_FALSE(LogBridgeNode::is_medkit_stack_logger("robot1.controller_server")); + EXPECT_FALSE(LogBridgeNode::is_medkit_stack_logger("amcl")); } // --- source_id normalization to node FQN (entity association) ---