From 23ae34483589cd09573660830a71ca3895af6f6a Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Thu, 27 Aug 2026 15:21:59 +0200 Subject: [PATCH 1/5] fault_manager: document which debounce lever fits which reporter, and test it The debounce counter only moves when an event arrives, so confirmation_threshold and healing_threshold only work for a reporter that keeps sending FAILED while a condition is still there. The docs recommended confirmation_threshold: -3 with healing_threshold: 3 to everyone. For a reporter that sends one FAILED per raise and one clear per de-assert the second event never comes. The fault stops at PREFAILED and never confirms, and ListFaults with an empty status filter returns CONFIRMED only, so nobody sees it. Healing breaks the same way: it needs healing_threshold minus confirmation_threshold consecutive PASSED events and only one is sent, so a confirmed fault stays CONFIRMED until someone calls ~/clear_fault. auto_confirm_after_sec is the lever for that kind of reporter and already works. It holds the first FAILED in PREFAILED and confirms it only if it is still there when the window closes, so a condition that recovers never reaches an operator. The docs listed the parameter but never said what it is for. Add an integration test driving the real node over the real services with the event counts such a reporter sends: one FAILED per raise, one PASSED per clear. It also pins the case where a condition clears inside the window, which is what stops a config from passing by only delaying a false alarm. No production code change. --- docs/config/fault-manager.rst | 26 ++ src/ros2_medkit_fault_manager/CMakeLists.txt | 6 + src/ros2_medkit_fault_manager/README.md | 30 +++ .../test_debounce_and_healing.test.py | 249 ++++++++++++++++++ 4 files changed, 311 insertions(+) create mode 100644 src/ros2_medkit_fault_manager/test/integration/test_debounce_and_healing.test.py diff --git a/docs/config/fault-manager.rst b/docs/config/fault-manager.rst index 6c9e6cdb3..a2428647d 100644 --- a/docs/config/fault-manager.rst +++ b/docs/config/fault-manager.rst @@ -75,6 +75,32 @@ The fault manager uses AUTOSAR DEM-style debounce filtering to prevent fault fla For immediate fault confirmation (no debounce), set ``confirmation_threshold: 0``. Faults with ``SEVERITY_CRITICAL`` always bypass debounce regardless of this setting. +.. important:: + + The counter only moves when an event arrives, so ``confirmation_threshold`` and + ``healing_threshold`` work only for a reporter that keeps sending FAILED while the condition + is still there. + + A reporter that sends one FAILED when a condition appears and one clear when it goes away + never sends the second event. ``confirmation_threshold: -3`` then leaves the fault in + PREFAILED forever, and the default fault list returns CONFIRMED only, so the fault is never + seen. ``healing_threshold: 3`` has the same problem: healing needs + ``healing_threshold - confirmation_threshold`` consecutive PASSED events, and only one is + sent, so the fault stays CONFIRMED until someone calls ``~/clear_fault``. + + For such a reporter, filter by time instead of by count: + + .. code-block:: yaml + + confirmation_threshold: -2 # first FAILED stays PREFAILED + auto_confirm_after_sec: 3.0 # confirm it if it is still there after 3 s + healing_enabled: true + healing_threshold: 0 # heal on the single PASSED + + Choose ``auto_confirm_after_sec`` from how often the reporter samples, so a condition has to + survive a few sampling cycles before it confirms. A glitch that clears in time never reaches + CONFIRMED, because the clear takes the fault out of PREFAILED before the timer fires. + Near-Miss Retention ~~~~~~~~~~~~~~~~~~~ diff --git a/src/ros2_medkit_fault_manager/CMakeLists.txt b/src/ros2_medkit_fault_manager/CMakeLists.txt index 624b4d4c2..98090512e 100644 --- a/src/ros2_medkit_fault_manager/CMakeLists.txt +++ b/src/ros2_medkit_fault_manager/CMakeLists.txt @@ -192,6 +192,12 @@ if(BUILD_TESTING) medkit_add_launch_test(test_entity_thresholds_integration test/integration/test_entity_thresholds_integration.test.py TIMEOUT 60 LABELS "integration") + # Drives the debounce and healing pair with the event counts an edge-triggered + # reporter actually sends: one FAILED per raise, one PASSED per clear. Two + # cases wait out the auto-confirm window, hence the timeout. + medkit_add_launch_test(test_debounce_and_healing test/integration/test_debounce_and_healing.test.py + TIMEOUT 120 LABELS "integration") + medkit_add_launch_test(test_rosbag_entity_scope test/integration/test_rosbag_entity_scope.test.py TIMEOUT 120 LABELS "integration") diff --git a/src/ros2_medkit_fault_manager/README.md b/src/ros2_medkit_fault_manager/README.md index 98ac04883..b68daea97 100644 --- a/src/ros2_medkit_fault_manager/README.md +++ b/src/ros2_medkit_fault_manager/README.md @@ -263,6 +263,36 @@ events to return to the default (CONFIRMED-only) list. During that window `last_ reflects the activity; `occurrence_count` does not, because it counts the edge that started the occurrence, not every report within it. +### Choosing the right lever for your reporter + +The counter only moves when an event arrives, so the count-based settings above work only for a +reporter that keeps sending FAILED while the condition is still there. A reporter that samples a +value on a timer and reports on every sample is of that kind. + +Many reporters do not work that way. They send one FAILED when the condition appears and one clear +when it goes away, and nothing in between. For such a reporter the second FAILED never arrives, so +`confirmation_threshold: -3` means the fault stays PREFAILED and never confirms. The default fault +list returns CONFIRMED only, so the fault is invisible. Healing has the same problem in reverse: +`healing_threshold: 3` needs four consecutive PASSED events after a fault confirmed at `-1`, and +only one PASSED is ever sent, so the fault stays CONFIRMED until someone calls `~/clear_fault`. + +Pick by how your reporter behaves: + +| Reporter repeats FAILED while the condition holds | Reporter sends one event per transition | +|---|---| +| `confirmation_threshold: -N` filters N noisy samples | `confirmation_threshold: -2` plus `auto_confirm_after_sec` | +| `healing_threshold: N` needs N clean samples | `healing_threshold: 0` heals on the single PASSED | + +For the second column the filtering is done by time, not by count. `confirmation_threshold: -2` +keeps the first FAILED in PREFAILED, and `auto_confirm_after_sec` confirms it if it is still there +when the timeout expires. Choose the timeout from how often the reporter samples, so a condition +has to survive a few sampling cycles before it confirms. A glitch that clears in time never +reaches CONFIRMED, because a clear takes the fault out of PREFAILED before the timer fires. + +Two settings ignore the counter completely. `SEVERITY_CRITICAL` confirms at once while +`critical_immediate_confirm` is true, which is the default. `auto_confirm_after_sec` promotes a +PREFAILED fault without changing its counter. + ### Fault Lifecycle with Debounce ``` diff --git a/src/ros2_medkit_fault_manager/test/integration/test_debounce_and_healing.test.py b/src/ros2_medkit_fault_manager/test/integration/test_debounce_and_healing.test.py new file mode 100644 index 000000000..c7770ecf6 --- /dev/null +++ b/src/ros2_medkit_fault_manager/test/integration/test_debounce_and_healing.test.py @@ -0,0 +1,249 @@ +#!/usr/bin/env python3 +# 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. + +""" +Debounce and healing contract for an edge-triggered fault reporter. + +A reporter is edge-triggered when it sends one FAILED event as a condition +appears and one clear as it goes away, instead of repeating FAILED on every +sample. The count-based debounce cannot filter a noisy sample for such a +reporter: the second FAILED that would move the counter never arrives. The +time-based lever does the filtering instead, so these tests pin the pair. + +Every case sends the number of events an edge-triggered reporter really sends +(one), never the number the counter would need. +""" + +import os +import tempfile +import time +import unittest + +from launch import LaunchDescription +import launch_ros.actions +import launch_testing.actions +import launch_testing.markers +import rclpy +from rclpy.node import Node +from ros2_medkit_msgs.msg import Fault +from ros2_medkit_msgs.srv import ListFaults, ReportFault + +DATABASE_PATH = os.path.join(tempfile.mkdtemp(prefix='debounce_healing_'), 'faults.db') + +# Seconds a fault stays PREFAILED before the timer confirms it. The node runs +# that timer once a second, so a confirmation lands within AUTO_CONFIRM_SEC + 1. +AUTO_CONFIRM_SEC = 3.0 + +# Every status, so a test can see a fault the default CONFIRMED-only filter hides. +ALL_STATUSES = ['PREFAILED', 'PREPASSED', 'CONFIRMED', 'HEALED', 'CLEARED'] + + +def generate_test_description(): + """Launch fault_manager with the appliance debounce and healing settings.""" + fault_manager_node = launch_ros.actions.Node( + package='ros2_medkit_fault_manager', + executable='fault_manager_node', + name='fault_manager', + output='screen', + parameters=[{ + 'storage_type': 'sqlite', + 'database_path': DATABASE_PATH, + # Below -1, so the first FAILED lands in PREFAILED instead of + # confirming. An edge-triggered reporter never sends the second + # event, so the counter stays here and the timer below decides. + 'confirmation_threshold': -2, + 'auto_confirm_after_sec': AUTO_CONFIRM_SEC, + # A clear arrives as one PASSED event, so healing has to finish on + # that one event. Threshold 0 is what makes it reachable. + 'healing_enabled': True, + 'healing_threshold': 0, + }], + sigterm_timeout='30', + sigkill_timeout='15', + ) + + return ( + LaunchDescription([ + fault_manager_node, + launch_testing.actions.ReadyToTest(), + ]), + { + 'fault_manager_node': fault_manager_node, + }, + ) + + +class TestDebounceAndHealing(unittest.TestCase): + """One failed read must not confirm, and a de-assert must heal unattended.""" + + @classmethod + def setUpClass(cls): + rclpy.init() + cls.node = Node('test_debounce_healing_client') + cls.report_client = cls.node.create_client(ReportFault, '/fault_manager/report_fault') + cls.list_client = cls.node.create_client(ListFaults, '/fault_manager/list_faults') + + assert cls.report_client.wait_for_service(timeout_sec=10.0), \ + 'report_fault service not available' + assert cls.list_client.wait_for_service(timeout_sec=10.0), \ + 'list_faults service not available' + + @classmethod + def tearDownClass(cls): + cls.node.destroy_node() + rclpy.shutdown() + + def _call(self, client, request): + future = client.call_async(request) + rclpy.spin_until_future_complete(self.node, future, timeout_sec=5.0) + self.assertIsNotNone(future.result(), 'Service call timed out') + return future.result() + + def _report(self, fault_code, event_type, severity=Fault.SEVERITY_ERROR): + """Send one ReportFault event, the way an edge-triggered reporter does.""" + request = ReportFault.Request() + request.fault_code = fault_code + request.event_type = event_type + request.severity = severity + request.description = 'debounce and healing contract test' + request.source_id = '/test_plc' + response = self._call(self.report_client, request) + self.assertTrue(response.accepted, f'ReportFault rejected for {fault_code}') + + def _status_of(self, fault_code): + """Return the status of one fault, or None when the store has no such fault.""" + request = ListFaults.Request() + request.statuses = ALL_STATUSES + response = self._call(self.list_client, request) + for fault in response.faults: + if fault.fault_code == fault_code: + return fault.status + return None + + def _default_filter_codes(self): + """Fault codes an operator sees with no status filter (CONFIRMED only).""" + response = self._call(self.list_client, ListFaults.Request()) + return [fault.fault_code for fault in response.faults] + + def _wait_for_status(self, fault_code, expected, timeout_sec): + """Poll until the fault reaches expected, returning the last status seen.""" + deadline = time.time() + timeout_sec + status = self._status_of(fault_code) + while time.time() < deadline and status != expected: + time.sleep(0.25) + status = self._status_of(fault_code) + return status + + def test_01_single_failed_read_does_not_confirm(self): + """One bad sample must not raise a confirmed fault.""" + code = 'PLC_SINGLE_READ' + self._report(code, ReportFault.Request.EVENT_FAILED) + + status = self._status_of(code) + self.assertIsNotNone(status, 'fault was not recorded at all') + self.assertNotEqual( + status, Fault.STATUS_CONFIRMED, + 'a single failed read confirmed the fault immediately' + ) + self.assertEqual(status, Fault.STATUS_PREFAILED) + + def test_02_prefailed_fault_is_hidden_from_the_default_list(self): + """A not-yet-confirmed fault must not reach an operator listing.""" + code = 'PLC_HIDDEN_WHILE_PENDING' + self._report(code, ReportFault.Request.EVENT_FAILED) + + self.assertNotIn( + code, self._default_filter_codes(), + 'an unconfirmed fault is already visible in the default fault list' + ) + + def test_03_sustained_condition_confirms_with_nobody_acting(self): + """ + A real fault must still surface. + + The reporter sends its one FAILED and never repeats it, so only the + time-based lever can promote this. Without it the fault would stay + PREFAILED forever and the appliance would go quiet. + """ + code = 'PLC_SUSTAINED' + self._report(code, ReportFault.Request.EVENT_FAILED) + + status = self._wait_for_status( + code, Fault.STATUS_CONFIRMED, AUTO_CONFIRM_SEC + 5.0 + ) + self.assertEqual( + status, Fault.STATUS_CONFIRMED, + f'a sustained fault never confirmed, it is still {status}' + ) + self.assertIn(code, self._default_filter_codes()) + + def test_04_transient_that_clears_in_time_never_confirms(self): + """ + The falsifying case for the whole setting. + + A glitch that goes away before the window closes must never confirm. + If it does, the configuration only delays a false alarm rather than + filtering it. + """ + code = 'PLC_TRANSIENT' + self._report(code, ReportFault.Request.EVENT_FAILED) + self._report(code, ReportFault.Request.EVENT_PASSED) + + # Sit past the auto-confirm window and the timer tick behind it. + time.sleep(AUTO_CONFIRM_SEC + 3.0) + + status = self._status_of(code) + self.assertNotEqual( + status, Fault.STATUS_CONFIRMED, + 'a transient that already cleared was confirmed by the timer' + ) + self.assertNotIn(code, self._default_filter_codes()) + + def test_05_deasserted_alarm_heals_with_nobody_acting(self): + """ + A confirmed fault must return to healed on the reporter's single clear. + + The reporter sends exactly one PASSED, so healing has to complete on + that one event. With healing off, or with a threshold above zero, the + fault stays CONFIRMED until a human clears it. + """ + code = 'PLC_DEASSERTED' + self._report(code, ReportFault.Request.EVENT_FAILED) + + status = self._wait_for_status( + code, Fault.STATUS_CONFIRMED, AUTO_CONFIRM_SEC + 5.0 + ) + self.assertEqual( + status, Fault.STATUS_CONFIRMED, 'fault never confirmed, cannot test healing' + ) + + self._report(code, ReportFault.Request.EVENT_PASSED) + + status = self._status_of(code) + self.assertEqual( + status, Fault.STATUS_HEALED, + f'a de-asserted alarm did not heal on its single clear, it is {status}' + ) + self.assertNotIn(code, self._default_filter_codes()) + + +@launch_testing.post_shutdown_test() +class TestDebounceAndHealingShutdown(unittest.TestCase): + """Check the node exited cleanly.""" + + def test_exit_code(self, proc_info, fault_manager_node): + launch_testing.asserts.assertExitCodes( + proc_info, allowable_exit_codes=[0, -2, -15], process=fault_manager_node + ) From 141a3b145d7abee867c72b10bc2fc841002011a6 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Sun, 6 Sep 2026 10:10:35 +0200 Subject: [PATCH 2/5] fault_manager: make a time-based confirmation as visible as a reported one A confirmation raised by the auto-confirm timer only reached the database and the audit log. The report path also publishes EVENT_CONFIRMED and enqueues snapshot + rosbag capture, so a fault confirmed by the timer produced no event for subscribers and no black-box recording, while the same fault confirmed by a report produced both. The capture block is now one helper shared by both paths, so they cannot drift apart again. Three smaller fixes on the same surface: auto_confirm_after_sec accepted NaN. Every comparison against NaN is false, so the `< 0.0` guard passed it through and the `> 0.0` timer guard then rejected it, disabling time-based confirmation with nothing logged. A negative value at least warned. Test the positive form and negate it. The startup line reported the requested thresholds rather than the ones in force, so a sanitized value was logged as if it had been accepted. It now reads from the sanitized config. The configuration guide recommended confirmation_threshold: 0 for immediate confirmation, which the node rejects and replaces with -1; it now says -1. bringup_params.yaml paired healing_enabled with healing_threshold: 3 under a comment promising a fault heals when its action recovers. The action bridge emits one PASSED per recovery and healing costs healing_threshold minus the counter the fault confirmed at, so that preset never healed. Threshold 0. Tests: a new integration test asserts the timer publishes EVENT_CONFIRMED, failing without the fix with the event list empty. The healing suite is parametrized over healing_threshold so 0 is shown to be what makes healing reachable, with the run at 3 asserting the latch rather than skipping. --- docs/config/fault-manager.rst | 36 +-- src/ros2_medkit_fault_manager/CMakeLists.txt | 5 + src/ros2_medkit_fault_manager/README.md | 30 ++- .../fault_manager_node.hpp | 5 + .../src/fault_manager_node.cpp | 172 +++++++------ .../test_auto_confirm_visibility.test.py | 179 ++++++++++++++ .../test_debounce_and_healing.test.py | 225 +++++++++--------- .../config/bringup_params.yaml | 7 +- 8 files changed, 444 insertions(+), 215 deletions(-) create mode 100644 src/ros2_medkit_fault_manager/test/integration/test_auto_confirm_visibility.test.py diff --git a/docs/config/fault-manager.rst b/docs/config/fault-manager.rst index a2428647d..c6f1eb58d 100644 --- a/docs/config/fault-manager.rst +++ b/docs/config/fault-manager.rst @@ -72,34 +72,38 @@ The fault manager uses AUTOSAR DEM-style debounce filtering to prevent fault fla .. tip:: - For immediate fault confirmation (no debounce), set ``confirmation_threshold: 0``. + For immediate fault confirmation (no debounce), set ``confirmation_threshold: -1``, + which is also the default. ``0`` is rejected: the threshold must be strictly + negative, and the node falls back to ``-1`` with a warning. Faults with ``SEVERITY_CRITICAL`` always bypass debounce regardless of this setting. .. important:: The counter only moves when an event arrives, so ``confirmation_threshold`` and - ``healing_threshold`` work only for a reporter that keeps sending FAILED while the condition - is still there. + ``healing_threshold`` are tunable only for a reporter that keeps sending events while a + condition holds. Confirmation needs repeated FAILED; healing needs repeated PASSED. A reporter that sends one FAILED when a condition appears and one clear when it goes away never sends the second event. ``confirmation_threshold: -3`` then leaves the fault in - PREFAILED forever, and the default fault list returns CONFIRMED only, so the fault is never - seen. ``healing_threshold: 3`` has the same problem: healing needs - ``healing_threshold - confirmation_threshold`` consecutive PASSED events, and only one is - sent, so the fault stays CONFIRMED until someone calls ``~/clear_fault``. + PREFAILED, and the default fault list returns CONFIRMED only, so the fault is never seen. + Healing has the same shape: it needs ``healing_threshold - confirmation_threshold`` + consecutive PASSED events counted from where the fault confirmed, and only one is sent. - For such a reporter, filter by time instead of by count: + For such a reporter, leave the confirmation threshold alone and make healing reachable: .. code-block:: yaml - confirmation_threshold: -2 # first FAILED stays PREFAILED - auto_confirm_after_sec: 3.0 # confirm it if it is still there after 3 s - healing_enabled: true - healing_threshold: 0 # heal on the single PASSED - - Choose ``auto_confirm_after_sec`` from how often the reporter samples, so a condition has to - survive a few sampling cycles before it confirms. A glitch that clears in time never reaches - CONFIRMED, because the clear takes the fault out of PREFAILED before the timer fires. + fault_manager: + ros__parameters: + confirmation_threshold: -1 # the default; the single FAILED confirms + healing_enabled: true + healing_threshold: 0 # heal on the single PASSED + + ``auto_confirm_after_sec`` promotes a fault that stayed PREFAILED for that long and looks + like it would allow a deeper threshold. It does not: HEALED is latched, leaving that latch + costs ``healing_threshold - confirmation_threshold`` FAILED events, and a one-event reporter + sends one. The second occurrence of a fault code would then never confirm again. Filter + noisy samples in the reporter instead, where the samples are. Near-Miss Retention ~~~~~~~~~~~~~~~~~~~ diff --git a/src/ros2_medkit_fault_manager/CMakeLists.txt b/src/ros2_medkit_fault_manager/CMakeLists.txt index 98090512e..3e933467d 100644 --- a/src/ros2_medkit_fault_manager/CMakeLists.txt +++ b/src/ros2_medkit_fault_manager/CMakeLists.txt @@ -198,6 +198,11 @@ if(BUILD_TESTING) medkit_add_launch_test(test_debounce_and_healing test/integration/test_debounce_and_healing.test.py TIMEOUT 120 LABELS "integration") + # Time-based confirmation has to reach the event stream, not just the store: + # the SSE feed and every capture path key off the published event. + medkit_add_launch_test(test_auto_confirm_visibility test/integration/test_auto_confirm_visibility.test.py + TIMEOUT 120 LABELS "integration") + medkit_add_launch_test(test_rosbag_entity_scope test/integration/test_rosbag_entity_scope.test.py TIMEOUT 120 LABELS "integration") diff --git a/src/ros2_medkit_fault_manager/README.md b/src/ros2_medkit_fault_manager/README.md index b68daea97..42ecff604 100644 --- a/src/ros2_medkit_fault_manager/README.md +++ b/src/ros2_medkit_fault_manager/README.md @@ -236,6 +236,8 @@ For systems that need to filter transient faults, enable debounce filtering by s ### Configuration ```bash +# For a reporter that repeats its events while a condition holds. +# See "Choosing the right lever for your reporter" below before copying this. ros2 run ros2_medkit_fault_manager fault_manager_node --ros-args \ -p confirmation_threshold:=-3 \ -p healing_enabled:=true \ @@ -280,18 +282,22 @@ Pick by how your reporter behaves: | Reporter repeats FAILED while the condition holds | Reporter sends one event per transition | |---|---| -| `confirmation_threshold: -N` filters N noisy samples | `confirmation_threshold: -2` plus `auto_confirm_after_sec` | -| `healing_threshold: N` needs N clean samples | `healing_threshold: 0` heals on the single PASSED | - -For the second column the filtering is done by time, not by count. `confirmation_threshold: -2` -keeps the first FAILED in PREFAILED, and `auto_confirm_after_sec` confirms it if it is still there -when the timeout expires. Choose the timeout from how often the reporter samples, so a condition -has to survive a few sampling cycles before it confirms. A glitch that clears in time never -reaches CONFIRMED, because a clear takes the fault out of PREFAILED before the timer fires. - -Two settings ignore the counter completely. `SEVERITY_CRITICAL` confirms at once while -`critical_immediate_confirm` is true, which is the default. `auto_confirm_after_sec` promotes a -PREFAILED fault without changing its counter. +| `confirmation_threshold: -N` confirms on the Nth FAILED, so it rides out N-1 noisy samples | `confirmation_threshold` cannot filter here; see below | +| `healing_threshold: N` needs `N - confirmation_threshold` clean samples | `healing_threshold: 0` heals on the single PASSED | + +For the second column, `auto_confirm_after_sec` looks like it fills the gap: it promotes a fault +that has stayed PREFAILED for that long, without changing the counter. Pairing it with +`confirmation_threshold: -2` does keep a single FAILED out of CONFIRMED. It also has a trap. HEALED +is latched, and leaving that latch costs `healing_threshold - confirmation_threshold` FAILED +events, which at `-2` is two. A reporter that sends one means the SECOND occurrence of a fault code +never confirms again, and `occurrence_count` does not move either. Prefer leaving +`confirmation_threshold` at `-1` for such a reporter, and filter noisy samples in the reporter +itself, where the samples are. + +Two things ignore the counter. `SEVERITY_CRITICAL` confirms at once unless +`critical_immediate_confirm` is turned off in the debounce config, and that field is not exposed as +a ROS parameter. `auto_confirm_after_sec` promotes on elapsed time since the last FAILED, which is +not the same as observing that the condition is still there. ### Fault Lifecycle with Debounce diff --git a/src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/fault_manager_node.hpp b/src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/fault_manager_node.hpp index 6d202e68c..85183f755 100644 --- a/src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/fault_manager_node.hpp +++ b/src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/fault_manager_node.hpp @@ -172,6 +172,11 @@ class FaultManagerNode : public rclcpp::Node { /// @param event_type One of FaultEvent::EVENT_CONFIRMED, EVENT_CLEARED, EVENT_UPDATED /// @param fault The fault data associated with this event /// @param auto_cleared_codes Optional list of auto-cleared symptom fault codes (for EVENT_CLEARED) + /// Enqueue snapshot + rosbag capture for a fault that has just confirmed. + /// Shared by the report path and the time-based confirmation timer so a + /// confirmation produces the same evidence whichever one produced it. + void capture_on_confirm(const std::string & fault_code); + void publish_fault_event(const std::string & event_type, const ros2_medkit_msgs::msg::Fault & fault, const std::vector & auto_cleared_codes = {}); 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 de8c731a2..c7e42c6e2 100644 --- a/src/ros2_medkit_fault_manager/src/fault_manager_node.cpp +++ b/src/ros2_medkit_fault_manager/src/fault_manager_node.cpp @@ -140,8 +140,14 @@ FaultManagerNode::FaultManagerNode(const rclcpp::NodeOptions & options) : Node(" // Time-based auto-confirmation parameter auto_confirm_after_sec_ = declare_parameter("auto_confirm_after_sec", 0.0); - if (auto_confirm_after_sec_ < 0.0) { - RCLCPP_WARN(get_logger(), "auto_confirm_after_sec should be >= 0, got %.2f. Disabling.", auto_confirm_after_sec_); + // Positive test, then negated: every comparison against NaN is false, so a + // plain `< 0.0` accepts NaN and the timer is then never created either, which + // disables time-based confirmation with nothing logged. clang-tidy's + // readability-simplify-boolean-expr suggests the DeMorgan rewrite that puts + // that back - leave this form alone. + if (!(std::isfinite(auto_confirm_after_sec_) && auto_confirm_after_sec_ >= 0.0)) { + RCLCPP_WARN(get_logger(), "auto_confirm_after_sec must be a finite value >= 0, got %.2f. Disabling.", + auto_confirm_after_sec_); auto_confirm_after_sec_ = 0.0; } @@ -445,6 +451,11 @@ FaultManagerNode::FaultManagerNode(const rclcpp::NodeOptions & options) : Node(" auto fault = storage_->get_fault(fault_code); if (fault) { audit_transition(kTransitionConfirmed, *fault, "auto_confirm_timer", confirmed_at_ns); + // A timer-driven confirmation is a confirmation: it has to reach the + // event stream and the black box exactly like one raised by a report, + // or subscribers see no alarm and no recording is ever made for it. + publish_fault_event(ros2_medkit_msgs::msg::FaultEvent::EVENT_CONFIRMED, *fault); + capture_on_confirm(fault_code); } } RCLCPP_INFO(get_logger(), "Auto-confirmed %zu PREFAILED fault(s) due to time threshold", confirmed.size()); @@ -452,11 +463,12 @@ FaultManagerNode::FaultManagerNode(const rclcpp::NodeOptions & options) : Node(" RCLCPP_INFO(get_logger(), "FaultManager node started (storage=%s, confirmation_threshold=%d, " "healing=%s, auto_confirm_after=%.1fs)", - storage_type_.c_str(), confirmation_threshold_, healing_enabled_ ? "enabled" : "disabled", - auto_confirm_after_sec_); + storage_type_.c_str(), global_config_.confirmation_threshold, + global_config_.healing_enabled ? "enabled" : "disabled", auto_confirm_after_sec_); } else { RCLCPP_INFO(get_logger(), "FaultManager node started (storage=%s, confirmation_threshold=%d, healing=%s)", - storage_type_.c_str(), confirmation_threshold_, healing_enabled_ ? "enabled" : "disabled"); + storage_type_.c_str(), global_config_.confirmation_threshold, + global_config_.healing_enabled ? "enabled" : "disabled"); } } @@ -657,6 +669,82 @@ void FaultManagerNode::audit_transition(const char * transition, const ros2_medk } } +void FaultManagerNode::capture_on_confirm(const std::string & fault_code) { + if (!capture_pool_) { + return; + } + const bool cooldown_enabled = snapshot_recapture_cooldown_sec_ > 0.0; + std::unique_lock cd_lock(last_capture_mutex_, std::defer_lock); + if (cooldown_enabled) { + cd_lock.lock(); + } + + bool on_cooldown = false; + if (cooldown_enabled) { + const auto cooldown = std::chrono::duration(snapshot_recapture_cooldown_sec_); + const auto sweep_now = std::chrono::steady_clock::now(); + // Bound the map (issue #441): a storm of distinct fault codes would otherwise + // leave one permanent entry per code. Entries older than the cooldown can never + // gate a capture again, so drop them while we hold the lock. + for (auto it = last_capture_times_.begin(); it != last_capture_times_.end();) { + if (sweep_now - it->second >= cooldown) { + it = last_capture_times_.erase(it); + } else { + ++it; + } + } + auto it = last_capture_times_.find(fault_code); + if (it != last_capture_times_.end()) { + on_cooldown = (sweep_now - it->second) < cooldown; + } + } + + if (on_cooldown) { + RCLCPP_DEBUG(get_logger(), "Skipping capture for '%s' - cooldown active", fault_code.c_str()); + } else { + const EnqueueOutcome outcome = capture_pool_->enqueue(fault_code); + const auto now = std::chrono::steady_clock::now(); + // RCLCPP_WARN_THROTTLE needs a non-const Clock lvalue (Humble/Lyrical + // compat); mirror rosbag_capture.cpp's local-copy pattern. Cast the + // uint64_t counter to unsigned long long + %llu to avoid -Wuseless-cast + // (uint64_t == unsigned long on LP64). + rclcpp::Clock throttle_clock(*get_clock()); + switch (outcome.result) { + case EnqueueResult::kAccepted: + if (cooldown_enabled) { + last_capture_times_[fault_code] = now; + } + break; + case EnqueueResult::kEvictedOldest: + if (cooldown_enabled) { + last_capture_times_[fault_code] = now; + if (outcome.evicted_code) { + last_capture_times_.erase(*outcome.evicted_code); // keep evicted fault retriable + } + } + RCLCPP_WARN_THROTTLE(get_logger(), throttle_clock, 2000, + "Capture queue full (drop_oldest): evicted pending '%s' for '%s' " + "(pool=%d, queue=%d, total_dropped=%llu)", + outcome.evicted_code ? outcome.evicted_code->c_str() : "?", fault_code.c_str(), + capture_pool_size_, capture_queue_depth_, + static_cast(capture_pool_->dropped_captures())); + break; + case EnqueueResult::kDroppedNewest: + // Cooldown NOT recorded: capture still possible if the fault later + // heals/clears and re-confirms. + RCLCPP_WARN_THROTTLE(get_logger(), throttle_clock, 2000, + "Capture queue full (reject_newest): dropped capture for '%s' " + "(pool=%d, queue=%d, total_dropped=%llu)", + fault_code.c_str(), capture_pool_size_, capture_queue_depth_, + static_cast(capture_pool_->dropped_captures())); + break; + case EnqueueResult::kRejectedShuttingDown: + RCLCPP_DEBUG(get_logger(), "Capture pool shutting down; skipped capture for '%s'", fault_code.c_str()); + break; + } + } +} + void FaultManagerNode::handle_report_fault( const std::shared_ptr & request, const std::shared_ptr & response) { @@ -792,78 +880,8 @@ void FaultManagerNode::handle_report_fault( // already serialized; last_capture_mutex_ only guards last_capture_times_ itself // (the cooldown check, the update below, and the expired-entry sweep). It is taken // solely when the cooldown is enabled, since the map is otherwise never touched. - if (just_confirmed && capture_pool_) { - const std::string fault_code = request->fault_code; - const bool cooldown_enabled = snapshot_recapture_cooldown_sec_ > 0.0; - std::unique_lock cd_lock(last_capture_mutex_, std::defer_lock); - if (cooldown_enabled) { - cd_lock.lock(); - } - - bool on_cooldown = false; - if (cooldown_enabled) { - const auto cooldown = std::chrono::duration(snapshot_recapture_cooldown_sec_); - const auto sweep_now = std::chrono::steady_clock::now(); - // Bound the map (issue #441): a storm of distinct fault codes would otherwise - // leave one permanent entry per code. Entries older than the cooldown can never - // gate a capture again, so drop them while we hold the lock. - for (auto it = last_capture_times_.begin(); it != last_capture_times_.end();) { - if (sweep_now - it->second >= cooldown) { - it = last_capture_times_.erase(it); - } else { - ++it; - } - } - auto it = last_capture_times_.find(fault_code); - if (it != last_capture_times_.end()) { - on_cooldown = (sweep_now - it->second) < cooldown; - } - } - - if (on_cooldown) { - RCLCPP_DEBUG(get_logger(), "Skipping capture for '%s' - cooldown active", fault_code.c_str()); - } else { - const EnqueueOutcome outcome = capture_pool_->enqueue(fault_code); - const auto now = std::chrono::steady_clock::now(); - // RCLCPP_WARN_THROTTLE needs a non-const Clock lvalue (Humble/Lyrical - // compat); mirror rosbag_capture.cpp's local-copy pattern. Cast the - // uint64_t counter to unsigned long long + %llu to avoid -Wuseless-cast - // (uint64_t == unsigned long on LP64). - rclcpp::Clock throttle_clock(*get_clock()); - switch (outcome.result) { - case EnqueueResult::kAccepted: - if (cooldown_enabled) { - last_capture_times_[fault_code] = now; - } - break; - case EnqueueResult::kEvictedOldest: - if (cooldown_enabled) { - last_capture_times_[fault_code] = now; - if (outcome.evicted_code) { - last_capture_times_.erase(*outcome.evicted_code); // keep evicted fault retriable - } - } - RCLCPP_WARN_THROTTLE(get_logger(), throttle_clock, 2000, - "Capture queue full (drop_oldest): evicted pending '%s' for '%s' " - "(pool=%d, queue=%d, total_dropped=%llu)", - outcome.evicted_code ? outcome.evicted_code->c_str() : "?", fault_code.c_str(), - capture_pool_size_, capture_queue_depth_, - static_cast(capture_pool_->dropped_captures())); - break; - case EnqueueResult::kDroppedNewest: - // Cooldown NOT recorded: capture still possible if the fault later - // heals/clears and re-confirms. - RCLCPP_WARN_THROTTLE(get_logger(), throttle_clock, 2000, - "Capture queue full (reject_newest): dropped capture for '%s' " - "(pool=%d, queue=%d, total_dropped=%llu)", - fault_code.c_str(), capture_pool_size_, capture_queue_depth_, - static_cast(capture_pool_->dropped_captures())); - break; - case EnqueueResult::kRejectedShuttingDown: - RCLCPP_DEBUG(get_logger(), "Capture pool shutting down; skipped capture for '%s'", fault_code.c_str()); - break; - } - } + if (just_confirmed) { + capture_on_confirm(request->fault_code); } // Handle PREFAILED state for lazy_start rosbag capture diff --git a/src/ros2_medkit_fault_manager/test/integration/test_auto_confirm_visibility.test.py b/src/ros2_medkit_fault_manager/test/integration/test_auto_confirm_visibility.test.py new file mode 100644 index 000000000..0e615efe6 --- /dev/null +++ b/src/ros2_medkit_fault_manager/test/integration/test_auto_confirm_visibility.test.py @@ -0,0 +1,179 @@ +#!/usr/bin/env python3 +# 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. + +""" +A time-based confirmation must be as visible as a reported one. + +auto_confirm_after_sec promotes a fault that has sat in PREFAILED past its +window. Everything downstream of a confirmation keys off the event stream: the +SSE fault feed the gateway serves, black-box capture, per-entity freeze frames. +A promotion that only reaches the database is an alarm nobody is told about, +so this pins the event rather than the stored status. +""" + +import os +import shutil +import tempfile +import threading +import time +import unittest + +from launch import LaunchDescription +import launch_ros.actions +import launch_testing.actions +import launch_testing.markers +import rclpy +from rclpy.node import Node +from ros2_medkit_msgs.msg import Fault, FaultEvent +from ros2_medkit_msgs.srv import ReportFault + +# Below -1 so the first FAILED lands in PREFAILED and only the timer can promote it. +CONFIRMATION_THRESHOLD = -2 +AUTO_CONFIRM_SEC = 3.0 + +_temp_dirs = [] + + +@launch_testing.markers.keep_alive +def generate_test_description(): + """Launch fault_manager with time-based confirmation enabled.""" + temp_dir = tempfile.mkdtemp(prefix='auto_confirm_visibility_') + _temp_dirs.append(temp_dir) + + fault_manager_node = launch_ros.actions.Node( + package='ros2_medkit_fault_manager', + executable='fault_manager_node', + name='fault_manager', + output='screen', + parameters=[{ + 'storage_type': 'sqlite', + 'database_path': os.path.join(temp_dir, 'faults.db'), + 'confirmation_threshold': CONFIRMATION_THRESHOLD, + 'auto_confirm_after_sec': AUTO_CONFIRM_SEC, + }], + sigterm_timeout='30', + sigkill_timeout='15', + ) + + return ( + LaunchDescription([ + fault_manager_node, + launch_testing.actions.ReadyToTest(), + ]), + { + 'fault_manager_node': fault_manager_node, + }, + ) + + +class TestAutoConfirmVisibility(unittest.TestCase): + """A timer-driven confirmation must publish the same event a report does.""" + + @classmethod + def setUpClass(cls): + rclpy.init() + cls.node = Node('test_auto_confirm_client') + cls.report_client = cls.node.create_client(ReportFault, '/fault_manager/report_fault') + + cls.events = [] + cls.events_lock = threading.Lock() + cls.node.create_subscription( + FaultEvent, '/fault_manager/events', cls._on_event, 100 + ) + + assert cls.report_client.wait_for_service(timeout_sec=20.0), \ + 'report_fault service not available' + + @classmethod + def _on_event(cls, msg): + with cls.events_lock: + cls.events.append((msg.event_type, msg.fault.fault_code)) + + @classmethod + def tearDownClass(cls): + cls.node.destroy_node() + rclpy.shutdown() + + def _events_for(self, fault_code): + with self.events_lock: + return [e for e in self.events if e[1] == fault_code] + + def _spin_for(self, seconds): + deadline = time.time() + seconds + while time.time() < deadline: + rclpy.spin_once(self.node, timeout_sec=0.1) + + def _report_failed(self, fault_code): + request = ReportFault.Request() + request.fault_code = fault_code + request.event_type = ReportFault.Request.EVENT_FAILED + request.severity = Fault.SEVERITY_ERROR + request.description = 'auto-confirm visibility test' + request.source_id = '/test_plc' + future = self.report_client.call_async(request) + rclpy.spin_until_future_complete(self.node, future, timeout_sec=20.0) + self.assertIsNotNone(future.result(), 'ReportFault timed out') + self.assertTrue(future.result().accepted) + + def test_01_timer_confirmation_publishes_a_confirmed_event(self): + """ + The one event a subscriber has to see. + + A single FAILED leaves the fault in PREFAILED, and the reporter never + sends a second one, so the timer is what confirms it. If that promotion + does not publish, the SSE feed and every capture path downstream stay + silent for a fault that is now confirmed in the database. + """ + code = 'PLC_TIMER_CONFIRMED' + self._report_failed(code) + + # Nothing confirmed yet: the raise alone must not produce a confirmation. + self._spin_for(0.5) + self.assertNotIn( + FaultEvent.EVENT_CONFIRMED, [e[0] for e in self._events_for(code)], + 'the fault confirmed on its first failed report, so the timer is not ' + 'what is under test' + ) + + self._spin_for(AUTO_CONFIRM_SEC + 4.0) + + self.assertIn( + FaultEvent.EVENT_CONFIRMED, [e[0] for e in self._events_for(code)], + 'a time-based confirmation published no event, so nothing downstream of it is told' + ) + + def test_02_the_event_carries_the_confirmed_fault(self): + """The published event must name the fault, not just fire.""" + code = 'PLC_TIMER_PAYLOAD' + self._report_failed(code) + self._spin_for(AUTO_CONFIRM_SEC + 4.0) + + confirmed = [e for e in self._events_for(code) if e[0] == FaultEvent.EVENT_CONFIRMED] + self.assertEqual( + len(confirmed), 1, f'expected exactly one confirmation event, got {confirmed}' + ) + self.assertEqual(confirmed[0][1], code) + + +@launch_testing.post_shutdown_test() +class TestAutoConfirmVisibilityShutdown(unittest.TestCase): + """Check the node exited cleanly and clean up the database.""" + + def test_exit_code(self, proc_info, fault_manager_node): + launch_testing.asserts.assertExitCodes(proc_info, process=fault_manager_node) + + def test_temp_dirs_removed(self): + for path in _temp_dirs: + shutil.rmtree(path, ignore_errors=True) diff --git a/src/ros2_medkit_fault_manager/test/integration/test_debounce_and_healing.test.py b/src/ros2_medkit_fault_manager/test/integration/test_debounce_and_healing.test.py index c7770ecf6..f40faaed7 100644 --- a/src/ros2_medkit_fault_manager/test/integration/test_debounce_and_healing.test.py +++ b/src/ros2_medkit_fault_manager/test/integration/test_debounce_and_healing.test.py @@ -14,19 +14,25 @@ # limitations under the License. """ -Debounce and healing contract for an edge-triggered fault reporter. +Healing contract for a reporter that sends one event per transition. -A reporter is edge-triggered when it sends one FAILED event as a condition -appears and one clear as it goes away, instead of repeating FAILED on every -sample. The count-based debounce cannot filter a noisy sample for such a -reporter: the second FAILED that would move the counter never arrives. The -time-based lever does the filtering instead, so these tests pin the pair. +Such a reporter raises a fault with a single FAILED and de-asserts it with a +single PASSED, never repeating either while the condition holds. Both debounce +directions count events, so for this reporter only the counts reachable in one +event are usable. -Every case sends the number of events an edge-triggered reporter really sends -(one), never the number the counter would need. +The suite runs twice, once per healing_threshold, because the whole contract +turns on that value: at 0 the single PASSED heals, at the default 3 it does not +and the fault stays CONFIRMED with nobody able to clear it but a human. Each +test states which outcome it expects for the threshold it runs under, so the +run at 3 is a falsifying control rather than a skipped case. + +Every case sends the number of events such a reporter really sends, never the +number the counter would need. """ import os +import shutil import tempfile import time import unittest @@ -40,35 +46,39 @@ from ros2_medkit_msgs.msg import Fault from ros2_medkit_msgs.srv import ListFaults, ReportFault -DATABASE_PATH = os.path.join(tempfile.mkdtemp(prefix='debounce_healing_'), 'faults.db') +# Only 0 lets a single PASSED reach the healing threshold. 3 is the parameter +# default and is exercised to show it leaves the fault confirmed. +HEALING_THRESHOLDS = [0, 3] -# Seconds a fault stays PREFAILED before the timer confirms it. The node runs -# that timer once a second, so a confirmation lands within AUTO_CONFIRM_SEC + 1. -AUTO_CONFIRM_SEC = 3.0 +# Long enough that any timer-driven promotion would have fired. No test sets +# auto_confirm_after_sec, so a status observed after this period is stable. +QUIET_PERIOD_SEC = 5.0 # Every status, so a test can see a fault the default CONFIRMED-only filter hides. ALL_STATUSES = ['PREFAILED', 'PREPASSED', 'CONFIRMED', 'HEALED', 'CLEARED'] +_temp_dirs = [] + + +@launch_testing.markers.keep_alive +@launch_testing.parametrize('healing_threshold', HEALING_THRESHOLDS) +def generate_test_description(healing_threshold): + """Launch fault_manager with healing on and the threshold under test.""" + temp_dir = tempfile.mkdtemp(prefix='debounce_healing_') + _temp_dirs.append(temp_dir) -def generate_test_description(): - """Launch fault_manager with the appliance debounce and healing settings.""" fault_manager_node = launch_ros.actions.Node( package='ros2_medkit_fault_manager', executable='fault_manager_node', name='fault_manager', output='screen', parameters=[{ + # SQLite rather than the in-memory store: the counter that decides + # healing is persisted, and both backends have to agree on it. 'storage_type': 'sqlite', - 'database_path': DATABASE_PATH, - # Below -1, so the first FAILED lands in PREFAILED instead of - # confirming. An edge-triggered reporter never sends the second - # event, so the counter stays here and the timer below decides. - 'confirmation_threshold': -2, - 'auto_confirm_after_sec': AUTO_CONFIRM_SEC, - # A clear arrives as one PASSED event, so healing has to finish on - # that one event. Threshold 0 is what makes it reachable. + 'database_path': os.path.join(temp_dir, 'faults.db'), 'healing_enabled': True, - 'healing_threshold': 0, + 'healing_threshold': healing_threshold, }], sigterm_timeout='30', sigkill_timeout='15', @@ -81,12 +91,13 @@ def generate_test_description(): ]), { 'fault_manager_node': fault_manager_node, + 'healing_threshold': healing_threshold, }, ) -class TestDebounceAndHealing(unittest.TestCase): - """One failed read must not confirm, and a de-assert must heal unattended.""" +class TestHealingOnASingleClear(unittest.TestCase): + """A de-asserted alarm must reach healed on the one PASSED it gets.""" @classmethod def setUpClass(cls): @@ -95,9 +106,9 @@ def setUpClass(cls): cls.report_client = cls.node.create_client(ReportFault, '/fault_manager/report_fault') cls.list_client = cls.node.create_client(ListFaults, '/fault_manager/list_faults') - assert cls.report_client.wait_for_service(timeout_sec=10.0), \ + assert cls.report_client.wait_for_service(timeout_sec=20.0), \ 'report_fault service not available' - assert cls.list_client.wait_for_service(timeout_sec=10.0), \ + assert cls.list_client.wait_for_service(timeout_sec=20.0), \ 'list_faults service not available' @classmethod @@ -107,17 +118,17 @@ def tearDownClass(cls): def _call(self, client, request): future = client.call_async(request) - rclpy.spin_until_future_complete(self.node, future, timeout_sec=5.0) + rclpy.spin_until_future_complete(self.node, future, timeout_sec=20.0) self.assertIsNotNone(future.result(), 'Service call timed out') return future.result() def _report(self, fault_code, event_type, severity=Fault.SEVERITY_ERROR): - """Send one ReportFault event, the way an edge-triggered reporter does.""" + """Send one ReportFault event, the way a one-event reporter does.""" request = ReportFault.Request() request.fault_code = fault_code request.event_type = event_type request.severity = severity - request.description = 'debounce and healing contract test' + request.description = 'healing contract test' request.source_id = '/test_plc' response = self._call(self.report_client, request) self.assertTrue(response.accepted, f'ReportFault rejected for {fault_code}') @@ -137,113 +148,109 @@ def _default_filter_codes(self): response = self._call(self.list_client, ListFaults.Request()) return [fault.fault_code for fault in response.faults] - def _wait_for_status(self, fault_code, expected, timeout_sec): - """Poll until the fault reaches expected, returning the last status seen.""" - deadline = time.time() + timeout_sec - status = self._status_of(fault_code) - while time.time() < deadline and status != expected: - time.sleep(0.25) - status = self._status_of(fault_code) - return status - - def test_01_single_failed_read_does_not_confirm(self): - """One bad sample must not raise a confirmed fault.""" - code = 'PLC_SINGLE_READ' - self._report(code, ReportFault.Request.EVENT_FAILED) - - status = self._status_of(code) - self.assertIsNotNone(status, 'fault was not recorded at all') - self.assertNotEqual( - status, Fault.STATUS_CONFIRMED, - 'a single failed read confirmed the fault immediately' - ) - self.assertEqual(status, Fault.STATUS_PREFAILED) - - def test_02_prefailed_fault_is_hidden_from_the_default_list(self): - """A not-yet-confirmed fault must not reach an operator listing.""" - code = 'PLC_HIDDEN_WHILE_PENDING' - self._report(code, ReportFault.Request.EVENT_FAILED) - - self.assertNotIn( - code, self._default_filter_codes(), - 'an unconfirmed fault is already visible in the default fault list' - ) - - def test_03_sustained_condition_confirms_with_nobody_acting(self): + def test_01_a_de_asserted_alarm_heals_on_its_single_clear(self, healing_threshold): """ - A real fault must still surface. + The fix this suite exists for. - The reporter sends its one FAILED and never repeats it, so only the - time-based lever can promote this. Without it the fault would stay - PREFAILED forever and the appliance would go quiet. + One FAILED raises the fault, one PASSED de-asserts it, and nobody + clears anything by hand. At threshold 0 the counter reaches the + threshold on that one event; at 3 it cannot, and the fault latches. """ - code = 'PLC_SUSTAINED' + code = 'PLC_DEASSERTED' self._report(code, ReportFault.Request.EVENT_FAILED) - - status = self._wait_for_status( - code, Fault.STATUS_CONFIRMED, AUTO_CONFIRM_SEC + 5.0 - ) self.assertEqual( - status, Fault.STATUS_CONFIRMED, - f'a sustained fault never confirmed, it is still {status}' + self._status_of(code), Fault.STATUS_CONFIRMED, + 'the raise did not confirm, so healing cannot be under test' ) - self.assertIn(code, self._default_filter_codes()) - def test_04_transient_that_clears_in_time_never_confirms(self): + self._report(code, ReportFault.Request.EVENT_PASSED) + status = self._status_of(code) + + if healing_threshold == 0: + self.assertEqual( + status, Fault.STATUS_HEALED, + f'a de-asserted alarm did not heal on its single clear, it is {status}' + ) + self.assertNotIn(code, self._default_filter_codes()) + else: + self.assertEqual( + status, Fault.STATUS_CONFIRMED, + 'threshold 3 unexpectedly healed on one PASSED, so threshold 0 ' + 'is not what makes healing reachable' + ) + self.assertIn(code, self._default_filter_codes()) + + def test_02_a_healed_fault_confirms_again_when_the_condition_returns(self, healing_threshold): """ - The falsifying case for the whole setting. + The guard that a healed fault is not a dead fault. - A glitch that goes away before the window closes must never confirm. - If it does, the configuration only delays a false alarm rather than - filtering it. + HEALED is latched, and escaping the latch costs + healing_threshold - confirmation_threshold FAILED events. A one-event + reporter sends one, so any healing_threshold above 0 leaves the second + occurrence of a fault code permanently invisible. """ - code = 'PLC_TRANSIENT' + code = 'PLC_RERAISE' self._report(code, ReportFault.Request.EVENT_FAILED) self._report(code, ReportFault.Request.EVENT_PASSED) - # Sit past the auto-confirm window and the timer tick behind it. - time.sleep(AUTO_CONFIRM_SEC + 3.0) + expected_after_clear = ( + Fault.STATUS_HEALED if healing_threshold == 0 else Fault.STATUS_CONFIRMED + ) + self.assertEqual(self._status_of(code), expected_after_clear) + self._report(code, ReportFault.Request.EVENT_FAILED) status = self._status_of(code) - self.assertNotEqual( + self.assertEqual( status, Fault.STATUS_CONFIRMED, - 'a transient that already cleared was confirmed by the timer' + f'a returning condition did not leave the fault confirmed, it is {status}' ) - self.assertNotIn(code, self._default_filter_codes()) - - def test_05_deasserted_alarm_heals_with_nobody_acting(self): - """ - A confirmed fault must return to healed on the reporter's single clear. + self.assertIn(code, self._default_filter_codes()) - The reporter sends exactly one PASSED, so healing has to complete on - that one event. With healing off, or with a threshold above zero, the - fault stays CONFIRMED until a human clears it. - """ - code = 'PLC_DEASSERTED' + def test_03_a_healed_fault_stays_healed_while_the_condition_is_gone(self, healing_threshold): + """A settled fault must not change status without an event.""" + code = 'PLC_STAYS_HEALED' self._report(code, ReportFault.Request.EVENT_FAILED) + self._report(code, ReportFault.Request.EVENT_PASSED) + settled = self._status_of(code) - status = self._wait_for_status( - code, Fault.STATUS_CONFIRMED, AUTO_CONFIRM_SEC + 5.0 - ) + time.sleep(QUIET_PERIOD_SEC) self.assertEqual( - status, Fault.STATUS_CONFIRMED, 'fault never confirmed, cannot test healing' + self._status_of(code), settled, + 'the fault changed status with no event to cause it' ) + if healing_threshold == 0: + self.assertEqual(settled, Fault.STATUS_HEALED) + self.assertNotIn(code, self._default_filter_codes()) + else: + self.assertEqual(settled, Fault.STATUS_CONFIRMED) + self.assertIn(code, self._default_filter_codes()) + + def test_04_one_failed_read_confirms_immediately(self, healing_threshold): + """ + The limitation, pinned so nobody assumes otherwise. - self._report(code, ReportFault.Request.EVENT_PASSED) + confirmation_threshold defaults to -1, so the first FAILED confirms. + Filtering a single noisy read is not reachable from this node's + configuration for a one-event reporter: raising the threshold means the + second event that would confirm never arrives. + """ + code = 'PLC_SINGLE_READ' + self._report(code, ReportFault.Request.EVENT_FAILED) - status = self._status_of(code) self.assertEqual( - status, Fault.STATUS_HEALED, - f'a de-asserted alarm did not heal on its single clear, it is {status}' + self._status_of(code), Fault.STATUS_CONFIRMED, + 'the documented immediate-confirmation behaviour changed' ) - self.assertNotIn(code, self._default_filter_codes()) + self.assertIn(code, self._default_filter_codes()) @launch_testing.post_shutdown_test() -class TestDebounceAndHealingShutdown(unittest.TestCase): - """Check the node exited cleanly.""" +class TestHealingShutdown(unittest.TestCase): + """Check the node exited cleanly and clean up the databases.""" def test_exit_code(self, proc_info, fault_manager_node): - launch_testing.asserts.assertExitCodes( - proc_info, allowable_exit_codes=[0, -2, -15], process=fault_manager_node - ) + launch_testing.asserts.assertExitCodes(proc_info, process=fault_manager_node) + + def test_temp_dirs_removed(self): + for path in _temp_dirs: + shutil.rmtree(path, ignore_errors=True) diff --git a/src/ros2_medkit_gateway/config/bringup_params.yaml b/src/ros2_medkit_gateway/config/bringup_params.yaml index b9c0de51d..da736fbff 100644 --- a/src/ros2_medkit_gateway/config/bringup_params.yaml +++ b/src/ros2_medkit_gateway/config/bringup_params.yaml @@ -15,8 +15,13 @@ fault_manager: # Heal on recovery so an action SUCCEEDED clears its fault (CONFIRMED -> HEALED). # Note: log faults have no recovery signal, so LOG_* faults do not auto-clear. + # + # 0, not the parameter default of 3: healing costs healing_threshold minus the + # counter the fault confirmed at, and the action bridge sends exactly one PASSED + # per recovery. Any higher value leaves the fault CONFIRMED after the action + # recovers, which is the opposite of what this preset is for. healing_enabled: true - healing_threshold: 3 + healing_threshold: 0 # Black-box rosbag capture: a crash-safe, entity-scoped ring buffer flushed on # fault confirmation. Produces a non-empty bag on the happy path. From 231b9782a2c29f3d6c1c75c5d5c011b9673d4bb6 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Sun, 6 Sep 2026 14:42:35 +0200 Subject: [PATCH 3/5] fault_manager: honour muting on the timer path, and prove the capture half Review of the previous commit found the timer path publishing a confirmation that the report path would have suppressed. The report path wraps every EVENT_CONFIRMED publish in `if (!should_mute)`; the timer published unconditionally, so a symptom muted by a root cause reached SSE, the trigger subscribers and the entity freeze frames while the fault list hid it. The timer now asks the correlation engine, through a new `is_muted`. Capture stays ungated, matching the report path, where `just_confirmed` is set regardless of muting. `std::isfinite` was added without ``, so it compiled only through a transitive include. The same NaN-through-a-range-check the last commit fixed on `auto_confirm_after_sec` was still present on five other double parameters in this file, two of which feed `create_wall_timer` through a cast; they now share one guard shape and one explanation. `auto_confirm_after_sec` also gains an upper bound: the SQLite backend evaluates the window as `static_cast(value * 1e9)`, undefined once the product leaves int64. `capture_on_confirm` was declared between `publish_fault_event`'s doc comment and its signature, so three `@param` tags bound to the wrong function. The locking comment the extraction left behind moved to the function that takes the lock, and now names both callers. The startup line reports `healing_threshold`, the value this whole area turns on. The warning for a positive `confirmation_threshold` no longer advertises 0, which the sanitizer rejects. `config/fault_manager.yaml` had the same unreachable pairing the bringup preset just lost, under a comment inviting the reader to enable it. Tests: the timer's capture half had no coverage - deleting the call left both suites green. A snapshot case closes that. Its first version did not discriminate either: `GetSnapshots` answers success with an empty topics map when nothing was captured, so the assertion now inspects the payload. The visibility suite waits for the event publisher to match before asserting an absence, measures elapsed time instead of trusting fixed windows, and asserts the event's fault rather than the code it filtered on. `keep_alive` moved below `parametrize`, where the marker survives. Documentation: the complete example no longer ships the pairing the same page warns about, the healing threshold is described as the counter target it is, and five statements that were wrong before this branch are corrected - occurrence counting, the namespaced-deployment example's node nesting, what `audit_log.transitions: all` covers, what happens to out-of-range thresholds, and the claim that PREFAILED implies a negative counter. --- docs/config/fault-manager.rst | 12 +- src/ros2_medkit_fault_manager/CMakeLists.txt | 10 +- src/ros2_medkit_fault_manager/README.md | 16 +- .../config/fault_manager.yaml | 8 +- .../correlation/correlation_engine.hpp | 5 + .../fault_manager_node.hpp | 7 +- .../src/correlation/correlation_engine.cpp | 5 + .../src/fault_manager_node.cpp | 67 +++--- .../test_auto_confirm_visibility.test.py | 202 ++++++++++++++---- .../test_debounce_and_healing.test.py | 29 ++- 10 files changed, 275 insertions(+), 86 deletions(-) diff --git a/docs/config/fault-manager.rst b/docs/config/fault-manager.rst index c6f1eb58d..400de56c0 100644 --- a/docs/config/fault-manager.rst +++ b/docs/config/fault-manager.rst @@ -65,7 +65,9 @@ The fault manager uses AUTOSAR DEM-style debounce filtering to prevent fault fla - When true, PASSED events can heal confirmed faults. * - ``healing_threshold`` - ``3`` - - Number of PASSED events to transition from CONFIRMED to HEALED. + - Counter value at which a fault heals, not a number of events. Healing costs + ``healing_threshold`` minus the counter the fault confirmed at, so with the + default ``-1`` it takes four PASSED events. * - ``auto_confirm_after_sec`` - ``0.0`` - Auto-confirm prefailed faults after this duration. Set to 0 to disable. @@ -560,7 +562,7 @@ by default: with it off there is no table, no file and no write cost. - Turn the audit log on. * - ``audit_log.transitions`` - ``"all"`` - - Which transitions are recorded: ``all`` (occurred, confirmed, cleared) or + - Which transitions are recorded: ``all`` (occurred, confirmed, healed, cleared) or ``confirmed_only``. Any other value falls back to ``all`` with a warning. * - ``audit_log.retention_max_records`` - ``0`` @@ -623,11 +625,13 @@ Complete Example storage_type: "sqlite" database_path: "/var/lib/ros2_medkit/faults.db" - # Debounce (require 3 FAILED events to confirm) + # Debounce for a reporter that repeats its events while a condition holds: + # three FAILED events confirm, and four PASSED events heal from there. + # For a reporter that sends one event per transition, use -1 with + # healing_threshold 0 instead - see the note under Debounce Settings. confirmation_threshold: -3 healing_enabled: true healing_threshold: 3 - auto_confirm_after_sec: 30.0 # Per-entity debounce overrides entity_thresholds: diff --git a/src/ros2_medkit_fault_manager/CMakeLists.txt b/src/ros2_medkit_fault_manager/CMakeLists.txt index 3e933467d..064cf44f1 100644 --- a/src/ros2_medkit_fault_manager/CMakeLists.txt +++ b/src/ros2_medkit_fault_manager/CMakeLists.txt @@ -192,14 +192,16 @@ if(BUILD_TESTING) medkit_add_launch_test(test_entity_thresholds_integration test/integration/test_entity_thresholds_integration.test.py TIMEOUT 60 LABELS "integration") - # Drives the debounce and healing pair with the event counts an edge-triggered - # reporter actually sends: one FAILED per raise, one PASSED per clear. Two - # cases wait out the auto-confirm window, hence the timeout. + # Drives healing with the event counts a one-event-per-transition reporter + # actually sends: one FAILED per raise, one PASSED per clear. Parametrized over + # healing_threshold, so the node launches twice, and one case holds a settled + # fault for a quiet period - hence the timeout. medkit_add_launch_test(test_debounce_and_healing test/integration/test_debounce_and_healing.test.py TIMEOUT 120 LABELS "integration") # Time-based confirmation has to reach the event stream, not just the store: - # the SSE feed and every capture path key off the published event. + # the SSE feed and the trigger subscribers key off the published event, and + # black-box capture is enqueued alongside it. medkit_add_launch_test(test_auto_confirm_visibility test/integration/test_auto_confirm_visibility.test.py TIMEOUT 120 LABELS "integration") diff --git a/src/ros2_medkit_fault_manager/README.md b/src/ros2_medkit_fault_manager/README.md index 42ecff604..c12e54c8a 100644 --- a/src/ros2_medkit_fault_manager/README.md +++ b/src/ros2_medkit_fault_manager/README.md @@ -47,7 +47,8 @@ ros2 service call /fault_manager/clear_fault ros2_medkit_msgs/srv/ClearFault \ ## Features - **Multi-source aggregation**: Same `fault_code` from different sources creates a single fault -- **Occurrence tracking**: Counts total reports and tracks all reporting sources +- **Occurrence tracking**: Counts outages, not reports - the count starts at one and rises only + when a cleared fault is raised again - and tracks all reporting sources - **Severity escalation**: Fault severity is updated if a higher severity is reported - **Persistent storage**: SQLite backend ensures faults survive node restarts - **Debounce filtering** (optional): AUTOSAR DEM-style counter-based fault confirmation with per-entity threshold overrides @@ -256,7 +257,9 @@ The fault manager uses an AUTOSAR DEM-style debounce model: The counter is always clamped to `[confirmation_threshold, healing_threshold]`, so a long run of one-sided events cannot push it out to the integer limits and delay the opposite transition. `confirmation_threshold < 0 <= healing_threshold` is required (`healing_threshold = 0` heals on a -single PASSED event); invalid thresholds fall back to safe defaults with a warning. +single PASSED event). A positive confirmation threshold or a negative healing threshold is +sign-flipped with a warning, so `5` becomes `-5`; a confirmation threshold of `0` is then +rejected and falls back to `-1`. `CONFIRMED` and `HEALED` are **latched** (hysteresis): once reached, the status holds until the counter reaches the opposite threshold, so a single opposite-direction event cannot flip it. As a @@ -320,7 +323,7 @@ PREFAILED -----> CONFIRMED -----> HEALED (retained) | Status | Description | |--------|-------------| -| `PREFAILED` | Debounce counter < 0, not yet confirmed | +| `PREFAILED` | Not yet confirmed. Usually a negative counter, but a fault that returns to 0 keeps the status it had | | `CONFIRMED` | Fault is active and verified | | `HEALED` | Resolved via PASSED events (if healing enabled) | | `CLEARED` | Manually acknowledged via `~/clear_fault` | @@ -504,9 +507,10 @@ names automatically via the `fault_manager.namespace` parameter: ```yaml # gateway_params.yaml -fault_manager: - namespace: "robot1" # -> /robot1/fault_manager/list_faults - service_timeout_sec: 5.0 +ros2_medkit_gateway: + ros__parameters: + fault_manager.namespace: "robot1" # -> /robot1/fault_manager/list_faults + fault_manager.service_timeout_sec: 5.0 ``` Launch the fault manager in a namespace: diff --git a/src/ros2_medkit_fault_manager/config/fault_manager.yaml b/src/ros2_medkit_fault_manager/config/fault_manager.yaml index 4dd549a67..9817ec293 100644 --- a/src/ros2_medkit_fault_manager/config/fault_manager.yaml +++ b/src/ros2_medkit_fault_manager/config/fault_manager.yaml @@ -18,8 +18,14 @@ fault_manager: # Healing OFF by default: a recovery signal (e.g. action SUCCEEDED) does not # auto-clear the fault until this is enabled. + # + # Threshold 0, so that turning healing on here works. Healing costs + # healing_threshold minus the counter the fault confirmed at, and a reporter + # that signals recovery once - the action bridge sends one PASSED - cannot + # move the counter further than that. Any higher value leaves the fault + # CONFIRMED after it has recovered. healing_enabled: false - healing_threshold: 3 + healing_threshold: 0 # Black-box rosbag capture OFF by default (opt-in). When enabled it defaults # to entity-scoped capture and is crash-safe (falls back / self-disables if no diff --git a/src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/correlation/correlation_engine.hpp b/src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/correlation/correlation_engine.hpp index c5d8fa8e1..f1092138a 100644 --- a/src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/correlation/correlation_engine.hpp +++ b/src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/correlation/correlation_engine.hpp @@ -115,6 +115,11 @@ class CorrelationEngine { /// Get count of muted faults uint32_t get_muted_count() const; + /// Whether a fault code is currently muted as a symptom. + /// @param fault_code Code to test + /// @return True while the code is suppressed by a root cause + bool is_muted(const std::string & fault_code) const; + /// Get all active clusters /// @return List of cluster data std::vector get_clusters() const; diff --git a/src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/fault_manager_node.hpp b/src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/fault_manager_node.hpp index 85183f755..9157102d9 100644 --- a/src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/fault_manager_node.hpp +++ b/src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/fault_manager_node.hpp @@ -172,14 +172,15 @@ class FaultManagerNode : public rclcpp::Node { /// @param event_type One of FaultEvent::EVENT_CONFIRMED, EVENT_CLEARED, EVENT_UPDATED /// @param fault The fault data associated with this event /// @param auto_cleared_codes Optional list of auto-cleared symptom fault codes (for EVENT_CLEARED) + void publish_fault_event(const std::string & event_type, const ros2_medkit_msgs::msg::Fault & fault, + const std::vector & auto_cleared_codes = {}); + /// Enqueue snapshot + rosbag capture for a fault that has just confirmed. /// Shared by the report path and the time-based confirmation timer so a /// confirmation produces the same evidence whichever one produced it. + /// @param fault_code Code of the fault that reached CONFIRMED void capture_on_confirm(const std::string & fault_code); - void publish_fault_event(const std::string & event_type, const ros2_medkit_msgs::msg::Fault & fault, - const std::vector & auto_cleared_codes = {}); - /// Validate severity value static bool is_valid_severity(uint8_t severity); diff --git a/src/ros2_medkit_fault_manager/src/correlation/correlation_engine.cpp b/src/ros2_medkit_fault_manager/src/correlation/correlation_engine.cpp index 25b326a36..c7cd12728 100644 --- a/src/ros2_medkit_fault_manager/src/correlation/correlation_engine.cpp +++ b/src/ros2_medkit_fault_manager/src/correlation/correlation_engine.cpp @@ -251,6 +251,11 @@ uint32_t CorrelationEngine::get_muted_count() const { return static_cast(muted_faults_.size()); } +bool CorrelationEngine::is_muted(const std::string & fault_code) const { + std::lock_guard lock(mutex_); + return muted_faults_.find(fault_code) != muted_faults_.end(); +} + std::vector CorrelationEngine::get_clusters() const { std::lock_guard lock(mutex_); 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 c7e42c6e2..146f0e3b4 100644 --- a/src/ros2_medkit_fault_manager/src/fault_manager_node.cpp +++ b/src/ros2_medkit_fault_manager/src/fault_manager_node.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -121,8 +122,7 @@ FaultManagerNode::FaultManagerNode(const rclcpp::NodeOptions & options) : Node(" auto confirmation_threshold_param = declare_parameter("confirmation_threshold", -1); if (confirmation_threshold_param > 0) { - RCLCPP_WARN(get_logger(), - "confirmation_threshold should be <= 0 (0 or -1 = immediate confirmation), got %d. Using %d.", + RCLCPP_WARN(get_logger(), "confirmation_threshold should be < 0 (-1 = immediate confirmation), got %d. Using %d.", static_cast(confirmation_threshold_param), static_cast(-confirmation_threshold_param)); confirmation_threshold_param = -confirmation_threshold_param; } @@ -140,20 +140,27 @@ FaultManagerNode::FaultManagerNode(const rclcpp::NodeOptions & options) : Node(" // Time-based auto-confirmation parameter auto_confirm_after_sec_ = declare_parameter("auto_confirm_after_sec", 0.0); - // Positive test, then negated: every comparison against NaN is false, so a - // plain `< 0.0` accepts NaN and the timer is then never created either, which - // disables time-based confirmation with nothing logged. clang-tidy's + // Every double parameter in this node is range-checked in this shape, tested + // positively and then negated, because every comparison against NaN is false: + // a plain `< 0.0` accepts NaN, and the guard that would have caught it later + // (`> 0.0` before creating the timer) is false for NaN too, so the feature + // switches itself off with nothing logged. clang-tidy's // readability-simplify-boolean-expr suggests the DeMorgan rewrite that puts // that back - leave this form alone. - if (!(std::isfinite(auto_confirm_after_sec_) && auto_confirm_after_sec_ >= 0.0)) { - RCLCPP_WARN(get_logger(), "auto_confirm_after_sec must be a finite value >= 0, got %.2f. Disabling.", - auto_confirm_after_sec_); + // Upper bound as well as lower: the SQLite backend evaluates the window as + // static_cast(auto_confirm_after_sec * 1e9), which is undefined once + // the product leaves the int64 range. + constexpr double kMaxAutoConfirmSec = 9.0e9; + if (!(std::isfinite(auto_confirm_after_sec_) && auto_confirm_after_sec_ >= 0.0 && + auto_confirm_after_sec_ <= kMaxAutoConfirmSec)) { + RCLCPP_WARN(get_logger(), "auto_confirm_after_sec must be a finite value in [0, %.1e], got %.2f. Disabling.", + kMaxAutoConfirmSec, auto_confirm_after_sec_); auto_confirm_after_sec_ = 0.0; } // Capture cooldown parameters (gates both snapshot and rosbag capture) snapshot_recapture_cooldown_sec_ = declare_parameter("snapshots.recapture_cooldown_sec", 60.0); - if (snapshot_recapture_cooldown_sec_ < 0.0) { + if (!(std::isfinite(snapshot_recapture_cooldown_sec_) && snapshot_recapture_cooldown_sec_ >= 0.0)) { RCLCPP_WARN(get_logger(), "snapshots.recapture_cooldown_sec should be >= 0, got %.2f. Disabling.", snapshot_recapture_cooldown_sec_); snapshot_recapture_cooldown_sec_ = 0.0; @@ -426,7 +433,7 @@ FaultManagerNode::FaultManagerNode(const rclcpp::NodeOptions & options) : Node(" // Create correlation cleanup timer if correlation is enabled if (correlation_engine_) { auto cleanup_interval_sec = declare_parameter("correlation.cleanup_interval_sec", 5.0); - if (cleanup_interval_sec <= 0.0) { + if (!(std::isfinite(cleanup_interval_sec) && cleanup_interval_sec > 0.0)) { RCLCPP_WARN(get_logger(), "correlation.cleanup_interval_sec must be positive, got %.2f. Using default 5.0s", cleanup_interval_sec); cleanup_interval_sec = 5.0; @@ -454,7 +461,15 @@ FaultManagerNode::FaultManagerNode(const rclcpp::NodeOptions & options) : Node(" // A timer-driven confirmation is a confirmation: it has to reach the // event stream and the black box exactly like one raised by a report, // or subscribers see no alarm and no recording is ever made for it. - publish_fault_event(ros2_medkit_msgs::msg::FaultEvent::EVENT_CONFIRMED, *fault); + // + // Muting is checked for the same reason the report path checks it: a + // symptom suppressed by a root cause must not be announced, or SSE and + // the trigger subscribers see an alarm that the fault list hides. + // Capture is deliberately not gated, matching the report path, where + // just_confirmed is set regardless of muting. + if (!correlation_engine_ || !correlation_engine_->is_muted(fault_code)) { + publish_fault_event(ros2_medkit_msgs::msg::FaultEvent::EVENT_CONFIRMED, *fault); + } capture_on_confirm(fault_code); } } @@ -462,13 +477,16 @@ FaultManagerNode::FaultManagerNode(const rclcpp::NodeOptions & options) : Node(" }); RCLCPP_INFO(get_logger(), "FaultManager node started (storage=%s, confirmation_threshold=%d, " - "healing=%s, auto_confirm_after=%.1fs)", + "healing=%s, healing_threshold=%d, auto_confirm_after=%.1fs)", storage_type_.c_str(), global_config_.confirmation_threshold, - global_config_.healing_enabled ? "enabled" : "disabled", auto_confirm_after_sec_); + global_config_.healing_enabled ? "enabled" : "disabled", global_config_.healing_threshold, + auto_confirm_after_sec_); } else { - RCLCPP_INFO(get_logger(), "FaultManager node started (storage=%s, confirmation_threshold=%d, healing=%s)", + RCLCPP_INFO(get_logger(), + "FaultManager node started (storage=%s, confirmation_threshold=%d, healing=%s, " + "healing_threshold=%d)", storage_type_.c_str(), global_config_.confirmation_threshold, - global_config_.healing_enabled ? "enabled" : "disabled"); + global_config_.healing_enabled ? "enabled" : "disabled", global_config_.healing_threshold); } } @@ -538,7 +556,7 @@ std::unique_ptr FaultManagerNode::create_storage() { std::unique_ptr FaultManagerNode::create_audit_log() { const bool enabled = declare_parameter("audit_log.enabled", false); - // Which transitions to record: "all" (occurred/confirmed/cleared) or + // Which transitions to record: "all" (occurred/confirmed/healed/cleared) or // "confirmed_only". const std::string transitions = declare_parameter("audit_log.transitions", "all"); audit_confirmed_only_ = (transitions == "confirmed_only"); @@ -670,6 +688,12 @@ void FaultManagerNode::audit_transition(const char * transition, const ros2_medk } void FaultManagerNode::capture_on_confirm(const std::string & fault_code) { + // Capture snapshots/rosbag when a fault confirms, via the bounded pool. + // Both callers - the report handler and the auto-confirm timer - run on the + // node's single-threaded executor, so confirmations are already serialized; + // last_capture_mutex_ only guards last_capture_times_ itself (the cooldown + // check, the update below, and the expired-entry sweep). It is taken solely + // when the cooldown is enabled, since the map is otherwise never touched. if (!capture_pool_) { return; } @@ -875,11 +899,6 @@ void FaultManagerNode::handle_report_fault( audit_transition(kTransitionHealed, *fault_after, "auto_heal", event_time.nanoseconds()); } - // Capture snapshots/rosbag when a fault confirms via the bounded pool (issue #441). - // handle_report_fault runs on the single-threaded executor, so confirmations are - // already serialized; last_capture_mutex_ only guards last_capture_times_ itself - // (the cooldown check, the update below, and the expired-entry sweep). It is taken - // solely when the cooldown is enabled, since the map is otherwise never touched. if (just_confirmed) { capture_on_confirm(request->fault_code); } @@ -1174,7 +1193,7 @@ SnapshotConfig FaultManagerNode::create_snapshot_config() { // Validate timeout_sec (must be positive) config.timeout_sec = declare_parameter("snapshots.timeout_sec", 1.0); - if (config.timeout_sec <= 0.0) { + if (!(std::isfinite(config.timeout_sec) && config.timeout_sec > 0.0)) { RCLCPP_WARN(get_logger(), "snapshots.timeout_sec must be positive, got %.2f. Using default 1.0s", config.timeout_sec); config.timeout_sec = 1.0; @@ -1211,14 +1230,14 @@ SnapshotConfig FaultManagerNode::create_snapshot_config() { config.rosbag.enabled = declare_parameter("snapshots.rosbag.enabled", false); if (config.rosbag.enabled) { config.rosbag.duration_sec = declare_parameter("snapshots.rosbag.duration_sec", 5.0); - if (config.rosbag.duration_sec <= 0.0) { + if (!(std::isfinite(config.rosbag.duration_sec) && config.rosbag.duration_sec > 0.0)) { RCLCPP_WARN(get_logger(), "snapshots.rosbag.duration_sec must be positive, got %.2f. Using default 5.0s", config.rosbag.duration_sec); config.rosbag.duration_sec = 5.0; } config.rosbag.duration_after_sec = declare_parameter("snapshots.rosbag.duration_after_sec", 1.0); - if (config.rosbag.duration_after_sec < 0.0) { + if (!(std::isfinite(config.rosbag.duration_after_sec) && config.rosbag.duration_after_sec >= 0.0)) { RCLCPP_WARN(get_logger(), "snapshots.rosbag.duration_after_sec must be non-negative, got %.2f. Using 0.0s", config.rosbag.duration_after_sec); config.rosbag.duration_after_sec = 0.0; diff --git a/src/ros2_medkit_fault_manager/test/integration/test_auto_confirm_visibility.test.py b/src/ros2_medkit_fault_manager/test/integration/test_auto_confirm_visibility.test.py index 0e615efe6..7df624788 100644 --- a/src/ros2_medkit_fault_manager/test/integration/test_auto_confirm_visibility.test.py +++ b/src/ros2_medkit_fault_manager/test/integration/test_auto_confirm_visibility.test.py @@ -17,12 +17,13 @@ A time-based confirmation must be as visible as a reported one. auto_confirm_after_sec promotes a fault that has sat in PREFAILED past its -window. Everything downstream of a confirmation keys off the event stream: the -SSE fault feed the gateway serves, black-box capture, per-entity freeze frames. -A promotion that only reaches the database is an alarm nobody is told about, -so this pins the event rather than the stored status. +window. The SSE fault feed the gateway serves, the trigger subscribers and the +per-entity freeze frames all key off the published event, and black-box capture +is enqueued alongside it. A promotion that only reaches the database is an alarm +nobody is told about and no recording is made for, so this pins both halves. """ +import json import os import shutil import tempfile @@ -30,6 +31,7 @@ import time import unittest +from ament_index_python.packages import get_package_share_directory from launch import LaunchDescription import launch_ros.actions import launch_testing.actions @@ -37,21 +39,35 @@ import rclpy from rclpy.node import Node from ros2_medkit_msgs.msg import Fault, FaultEvent -from ros2_medkit_msgs.srv import ReportFault +from ros2_medkit_msgs.srv import GetSnapshots, ReportFault +from sensor_msgs.msg import Temperature # Below -1 so the first FAILED lands in PREFAILED and only the timer can promote it. CONFIRMATION_THRESHOLD = -2 AUTO_CONFIRM_SEC = 3.0 +# The node runs the promotion timer once a second, so a confirmation lands +# within AUTO_CONFIRM_SEC + 1 of the last FAILED under no load. The waits below +# are generous on top of that because CI runs this under ASan and TSan; they +# bound the wait, they do not define the contract. +CONFIRM_WAIT_SEC = AUTO_CONFIRM_SEC + 12.0 + +# Mapped to /test/temperature by test_snapshots.yaml, so a confirmation on this +# code is expected to capture that topic. +SNAPSHOT_FAULT_CODE = 'TEST_SNAPSHOT_FAULT' + _temp_dirs = [] @launch_testing.markers.keep_alive def generate_test_description(): - """Launch fault_manager with time-based confirmation enabled.""" + """Launch fault_manager with time-based confirmation and snapshots on.""" temp_dir = tempfile.mkdtemp(prefix='auto_confirm_visibility_') _temp_dirs.append(temp_dir) + pkg_share = get_package_share_directory('ros2_medkit_fault_manager') + snapshot_config = os.path.join(pkg_share, 'test', 'test_snapshots.yaml') + fault_manager_node = launch_ros.actions.Node( package='ros2_medkit_fault_manager', executable='fault_manager_node', @@ -62,6 +78,10 @@ def generate_test_description(): 'database_path': os.path.join(temp_dir, 'faults.db'), 'confirmation_threshold': CONFIRMATION_THRESHOLD, 'auto_confirm_after_sec': AUTO_CONFIRM_SEC, + 'snapshots.enabled': True, + 'snapshots.config_file': snapshot_config, + 'snapshots.timeout_sec': 3.0, + 'snapshots.background_capture': False, }], sigterm_timeout='30', sigkill_timeout='15', @@ -79,27 +99,46 @@ def generate_test_description(): class TestAutoConfirmVisibility(unittest.TestCase): - """A timer-driven confirmation must publish the same event a report does.""" + """A timer-driven confirmation must publish an event and capture evidence.""" @classmethod def setUpClass(cls): rclpy.init() - cls.node = Node('test_auto_confirm_client') - cls.report_client = cls.node.create_client(ReportFault, '/fault_manager/report_fault') + try: + cls.node = Node('test_auto_confirm_client') + cls.report_client = cls.node.create_client(ReportFault, '/fault_manager/report_fault') + cls.snapshot_client = cls.node.create_client( + GetSnapshots, '/fault_manager/get_snapshots' + ) + cls.temp_publisher = cls.node.create_publisher(Temperature, '/test/temperature', 10) - cls.events = [] - cls.events_lock = threading.Lock() - cls.node.create_subscription( - FaultEvent, '/fault_manager/events', cls._on_event, 100 - ) + cls.events = [] + cls.events_lock = threading.Lock() + cls.event_sub = cls.node.create_subscription( + FaultEvent, '/fault_manager/events', cls._on_event, 100 + ) + + assert cls.report_client.wait_for_service(timeout_sec=30.0), \ + 'report_fault service not available' + assert cls.snapshot_client.wait_for_service(timeout_sec=30.0), \ + 'get_snapshots service not available' - assert cls.report_client.wait_for_service(timeout_sec=20.0), \ - 'report_fault service not available' + # The publisher uses volatile durability, so an event sent before the + # subscription matches is lost. Without this wait an assertion that + # no event arrived could pass because nothing was listening yet. + deadline = time.time() + 30.0 + while time.time() < deadline and cls.event_sub.get_publisher_count() == 0: + rclpy.spin_once(cls.node, timeout_sec=0.1) + assert cls.event_sub.get_publisher_count() > 0, \ + 'fault event publisher never matched; absence assertions would be meaningless' + except Exception: + rclpy.shutdown() + raise @classmethod def _on_event(cls, msg): with cls.events_lock: - cls.events.append((msg.event_type, msg.fault.fault_code)) + cls.events.append(msg) @classmethod def tearDownClass(cls): @@ -108,22 +147,32 @@ def tearDownClass(cls): def _events_for(self, fault_code): with self.events_lock: - return [e for e in self.events if e[1] == fault_code] + return [e for e in self.events if e.fault.fault_code == fault_code] def _spin_for(self, seconds): - deadline = time.time() + seconds - while time.time() < deadline: + deadline = time.monotonic() + seconds + while time.monotonic() < deadline: + rclpy.spin_once(self.node, timeout_sec=0.1) + + def _wait_for_confirmed_event(self, fault_code, timeout_sec): + """Spin until a confirmation for this code arrives, returning it or None.""" + deadline = time.monotonic() + timeout_sec + while time.monotonic() < deadline: + for event in self._events_for(fault_code): + if event.event_type == FaultEvent.EVENT_CONFIRMED: + return event rclpy.spin_once(self.node, timeout_sec=0.1) + return None - def _report_failed(self, fault_code): + def _report_failed(self, fault_code, severity=Fault.SEVERITY_ERROR): request = ReportFault.Request() request.fault_code = fault_code request.event_type = ReportFault.Request.EVENT_FAILED - request.severity = Fault.SEVERITY_ERROR + request.severity = severity request.description = 'auto-confirm visibility test' request.source_id = '/test_plc' future = self.report_client.call_async(request) - rclpy.spin_until_future_complete(self.node, future, timeout_sec=20.0) + rclpy.spin_until_future_complete(self.node, future, timeout_sec=30.0) self.assertIsNotNone(future.result(), 'ReportFault timed out') self.assertTrue(future.result().accepted) @@ -133,47 +182,120 @@ def test_01_timer_confirmation_publishes_a_confirmed_event(self): A single FAILED leaves the fault in PREFAILED, and the reporter never sends a second one, so the timer is what confirms it. If that promotion - does not publish, the SSE feed and every capture path downstream stay - silent for a fault that is now confirmed in the database. + does not publish, the SSE feed and the trigger subscribers stay silent + for a fault that is now confirmed in the database. """ code = 'PLC_TIMER_CONFIRMED' + raised_at = time.monotonic() self._report_failed(code) - # Nothing confirmed yet: the raise alone must not produce a confirmation. + # The raise alone must not confirm, or the timer is not what is measured. self._spin_for(0.5) self.assertNotIn( - FaultEvent.EVENT_CONFIRMED, [e[0] for e in self._events_for(code)], - 'the fault confirmed on its first failed report, so the timer is not ' - 'what is under test' + FaultEvent.EVENT_CONFIRMED, [e.event_type for e in self._events_for(code)], + 'the fault confirmed on its first failed report' ) - self._spin_for(AUTO_CONFIRM_SEC + 4.0) - - self.assertIn( - FaultEvent.EVENT_CONFIRMED, [e[0] for e in self._events_for(code)], - 'a time-based confirmation published no event, so nothing downstream of it is told' + event = self._wait_for_confirmed_event(code, CONFIRM_WAIT_SEC) + self.assertIsNotNone( + event, 'a time-based confirmation published no event, so nothing downstream is told' + ) + self.assertGreaterEqual( + time.monotonic() - raised_at, AUTO_CONFIRM_SEC, + 'the confirmation arrived before the configured window had elapsed' ) def test_02_the_event_carries_the_confirmed_fault(self): - """The published event must name the fault, not just fire.""" + """The event must carry the fault, not just its code.""" code = 'PLC_TIMER_PAYLOAD' self._report_failed(code) - self._spin_for(AUTO_CONFIRM_SEC + 4.0) - confirmed = [e for e in self._events_for(code) if e[0] == FaultEvent.EVENT_CONFIRMED] + event = self._wait_for_confirmed_event(code, CONFIRM_WAIT_SEC) + self.assertIsNotNone(event, 'no confirmation event arrived') + self.assertEqual(event.fault.fault_code, code) self.assertEqual( - len(confirmed), 1, f'expected exactly one confirmation event, got {confirmed}' + event.fault.status, Fault.STATUS_CONFIRMED, + 'the event carries a fault that is not confirmed' + ) + self.assertEqual(event.fault.severity, Fault.SEVERITY_ERROR) + self.assertIn('/test_plc', event.fault.reporting_sources) + + def test_03_timer_confirmation_captures_a_snapshot(self): + """ + The other half of the fix, and the one an event assertion cannot reach. + + Confirmation is what triggers black-box capture. A promotion that + publishes but never enqueues capture leaves an alarm with no evidence + behind it, which is what the operator opens the fault to look for. + + @verifies REQ_INTEROP_088 + """ + temp_msg = Temperature() + temp_msg.temperature = 85.5 + temp_msg.variance = 0.1 + + stop_publishing = threading.Event() + + def keep_publishing(): + while not stop_publishing.is_set(): + self.temp_publisher.publish(temp_msg) + time.sleep(0.05) + + pub_thread = threading.Thread(target=keep_publishing) + pub_thread.start() + try: + self._report_failed(SNAPSHOT_FAULT_CODE) + event = self._wait_for_confirmed_event(SNAPSHOT_FAULT_CODE, CONFIRM_WAIT_SEC) + self.assertIsNotNone(event, 'no confirmation event arrived') + + snapshot = self._wait_for_snapshot(SNAPSHOT_FAULT_CODE, timeout_sec=20.0) + finally: + stop_publishing.set() + pub_thread.join() + + self.assertIsNotNone( + snapshot, + 'a time-based confirmation captured no snapshot, so the alarm has no evidence' ) - self.assertEqual(confirmed[0][1], code) + parsed = json.loads(snapshot) + self.assertEqual(parsed['fault_code'], SNAPSHOT_FAULT_CODE) + self.assertIn( + '/test/temperature', parsed['topics'], + f'the configured topic was not captured, only {list(parsed["topics"])}' + ) + + def _wait_for_snapshot(self, fault_code, timeout_sec): + """Poll GetSnapshots until one exists for this fault, or give up.""" + deadline = time.monotonic() + timeout_sec + while time.monotonic() < deadline: + request = GetSnapshots.Request() + request.fault_code = fault_code + request.topic = '' + future = self.snapshot_client.call_async(request) + rclpy.spin_until_future_complete(self.node, future, timeout_sec=10.0) + result = future.result() + if result is not None and result.success and result.data: + # success with an empty topics map is what the service answers + # when the fault exists but nothing was ever captured for it, so + # the payload has to be inspected rather than merely present. + parsed = json.loads(result.data) + if parsed.get('topics'): + return result.data + rclpy.spin_once(self.node, timeout_sec=0.2) + return None @launch_testing.post_shutdown_test() class TestAutoConfirmVisibilityShutdown(unittest.TestCase): - """Check the node exited cleanly and clean up the database.""" + """Check the node exited cleanly and the databases are gone.""" def test_exit_code(self, proc_info, fault_manager_node): launch_testing.asserts.assertExitCodes(proc_info, process=fault_manager_node) def test_temp_dirs_removed(self): + # Idempotent: the parametrized suite runs this once per launch and the + # list is module-level, so a directory may already be gone. The contract + # is the end state, not that this call did the removing. for path in _temp_dirs: shutil.rmtree(path, ignore_errors=True) + self.assertFalse(os.path.exists(path), f'{path} survived cleanup') diff --git a/src/ros2_medkit_fault_manager/test/integration/test_debounce_and_healing.test.py b/src/ros2_medkit_fault_manager/test/integration/test_debounce_and_healing.test.py index f40faaed7..2785db445 100644 --- a/src/ros2_medkit_fault_manager/test/integration/test_debounce_and_healing.test.py +++ b/src/ros2_medkit_fault_manager/test/integration/test_debounce_and_healing.test.py @@ -60,8 +60,11 @@ _temp_dirs = [] -@launch_testing.markers.keep_alive +# keep_alive below parametrize: parametrize rebuilds the description with +# functools.update_wrapper against the undecorated function, so a marker set on +# the outer wrapper never reaches the per-parameter run. @launch_testing.parametrize('healing_threshold', HEALING_THRESHOLDS) +@launch_testing.markers.keep_alive def generate_test_description(healing_threshold): """Launch fault_manager with healing on and the threshold under test.""" temp_dir = tempfile.mkdtemp(prefix='debounce_healing_') @@ -102,13 +105,21 @@ class TestHealingOnASingleClear(unittest.TestCase): @classmethod def setUpClass(cls): rclpy.init() + try: + cls._setup_clients() + except Exception: + rclpy.shutdown() + raise + + @classmethod + def _setup_clients(cls): cls.node = Node('test_debounce_healing_client') cls.report_client = cls.node.create_client(ReportFault, '/fault_manager/report_fault') cls.list_client = cls.node.create_client(ListFaults, '/fault_manager/list_faults') - assert cls.report_client.wait_for_service(timeout_sec=20.0), \ + assert cls.report_client.wait_for_service(timeout_sec=30.0), \ 'report_fault service not available' - assert cls.list_client.wait_for_service(timeout_sec=20.0), \ + assert cls.list_client.wait_for_service(timeout_sec=30.0), \ 'list_faults service not available' @classmethod @@ -118,7 +129,7 @@ def tearDownClass(cls): def _call(self, client, request): future = client.call_async(request) - rclpy.spin_until_future_complete(self.node, future, timeout_sec=20.0) + rclpy.spin_until_future_complete(self.node, future, timeout_sec=10.0) self.assertIsNotNone(future.result(), 'Service call timed out') return future.result() @@ -206,6 +217,12 @@ def test_02_a_healed_fault_confirms_again_when_the_condition_returns(self, heali ) self.assertIn(code, self._default_filter_codes()) + # At 0 the fault genuinely left CONFIRMED and came back, which is the + # case the latch could swallow. At 3 it never left, so this run only + # shows that a repeat FAILED does not disturb a confirmed fault. + if healing_threshold == 0: + self.assertEqual(expected_after_clear, Fault.STATUS_HEALED) + def test_03_a_healed_fault_stays_healed_while_the_condition_is_gone(self, healing_threshold): """A settled fault must not change status without an event.""" code = 'PLC_STAYS_HEALED' @@ -252,5 +269,9 @@ def test_exit_code(self, proc_info, fault_manager_node): launch_testing.asserts.assertExitCodes(proc_info, process=fault_manager_node) def test_temp_dirs_removed(self): + # Idempotent: the parametrized suite runs this once per launch and the + # list is module-level, so a directory may already be gone. The contract + # is the end state, not that this call did the removing. for path in _temp_dirs: shutil.rmtree(path, ignore_errors=True) + self.assertFalse(os.path.exists(path), f'{path} survived cleanup') From 21010642029beacd0978e57b371ade8c56e2c9e8 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Sun, 6 Sep 2026 15:45:41 +0200 Subject: [PATCH 4/5] fault_manager: count publishers through the node, not the subscription Subscription.get_publisher_count does not exist in rclpy on every supported distro, so the wait for the event publisher to match raised AttributeError on Humble before any test ran. Node.count_publishers answers the same question and is present across all of them. The topic name is now a constant, so the subscription and the count cannot drift apart. --- .../integration/test_auto_confirm_visibility.test.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/ros2_medkit_fault_manager/test/integration/test_auto_confirm_visibility.test.py b/src/ros2_medkit_fault_manager/test/integration/test_auto_confirm_visibility.test.py index 7df624788..e13f4014e 100644 --- a/src/ros2_medkit_fault_manager/test/integration/test_auto_confirm_visibility.test.py +++ b/src/ros2_medkit_fault_manager/test/integration/test_auto_confirm_visibility.test.py @@ -56,6 +56,8 @@ # code is expected to capture that topic. SNAPSHOT_FAULT_CODE = 'TEST_SNAPSHOT_FAULT' +EVENT_TOPIC = '/fault_manager/events' + _temp_dirs = [] @@ -115,7 +117,7 @@ def setUpClass(cls): cls.events = [] cls.events_lock = threading.Lock() cls.event_sub = cls.node.create_subscription( - FaultEvent, '/fault_manager/events', cls._on_event, 100 + FaultEvent, EVENT_TOPIC, cls._on_event, 100 ) assert cls.report_client.wait_for_service(timeout_sec=30.0), \ @@ -126,10 +128,12 @@ def setUpClass(cls): # The publisher uses volatile durability, so an event sent before the # subscription matches is lost. Without this wait an assertion that # no event arrived could pass because nothing was listening yet. - deadline = time.time() + 30.0 - while time.time() < deadline and cls.event_sub.get_publisher_count() == 0: + # Node.count_publishers rather than Subscription.get_publisher_count: + # the latter does not exist in rclpy on every supported distro. + deadline = time.monotonic() + 30.0 + while time.monotonic() < deadline and cls.node.count_publishers(EVENT_TOPIC) == 0: rclpy.spin_once(cls.node, timeout_sec=0.1) - assert cls.event_sub.get_publisher_count() > 0, \ + assert cls.node.count_publishers(EVENT_TOPIC) > 0, \ 'fault event publisher never matched; absence assertions would be meaningless' except Exception: rclpy.shutdown() From 0b1955e65e58faef4cccf8c550ed5a78fe6b0dc1 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Sun, 6 Sep 2026 17:21:55 +0200 Subject: [PATCH 5/5] fault_manager: cover the muting gate on the timer path The gate that stops a timer confirmation announcing a muted symptom had no test. No integration test set a correlation config together with auto_confirm_after_sec, so the path was unproven either way. The new case reports a root cause and then a symptom inside the rule window, and asserts three things: the root cause is announced, the symptom is not, and the symptom is still CONFIRMED in the store when muted faults are included. The third assertion is what makes the other two mean something. Without it the case would also pass if the timer had never confirmed the symptom at all, which is a different behaviour with the same visible result. Reuses the hierarchical rule already in test_correlation.yaml, so no new configuration file. Removing the gate fails the case and prints the symptom's own fault_confirmed event. --- .../test_auto_confirm_visibility.test.py | 55 ++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/src/ros2_medkit_fault_manager/test/integration/test_auto_confirm_visibility.test.py b/src/ros2_medkit_fault_manager/test/integration/test_auto_confirm_visibility.test.py index e13f4014e..6522e8794 100644 --- a/src/ros2_medkit_fault_manager/test/integration/test_auto_confirm_visibility.test.py +++ b/src/ros2_medkit_fault_manager/test/integration/test_auto_confirm_visibility.test.py @@ -39,7 +39,7 @@ import rclpy from rclpy.node import Node from ros2_medkit_msgs.msg import Fault, FaultEvent -from ros2_medkit_msgs.srv import GetSnapshots, ReportFault +from ros2_medkit_msgs.srv import GetSnapshots, ListFaults, ReportFault from sensor_msgs.msg import Temperature # Below -1 so the first FAILED lands in PREFAILED and only the timer can promote it. @@ -69,6 +69,7 @@ def generate_test_description(): pkg_share = get_package_share_directory('ros2_medkit_fault_manager') snapshot_config = os.path.join(pkg_share, 'test', 'test_snapshots.yaml') + correlation_config = os.path.join(pkg_share, 'test', 'test_correlation.yaml') fault_manager_node = launch_ros.actions.Node( package='ros2_medkit_fault_manager', @@ -84,6 +85,9 @@ def generate_test_description(): 'snapshots.config_file': snapshot_config, 'snapshots.timeout_sec': 3.0, 'snapshots.background_capture': False, + # Carries a hierarchical rule with mute_symptoms, so a symptom of + # ESTOP_001 is muted while the timer still confirms it. + 'correlation.config_file': correlation_config, }], sigterm_timeout='30', sigkill_timeout='15', @@ -112,6 +116,7 @@ def setUpClass(cls): cls.snapshot_client = cls.node.create_client( GetSnapshots, '/fault_manager/get_snapshots' ) + cls.list_client = cls.node.create_client(ListFaults, '/fault_manager/list_faults') cls.temp_publisher = cls.node.create_publisher(Temperature, '/test/temperature', 10) cls.events = [] @@ -124,6 +129,8 @@ def setUpClass(cls): 'report_fault service not available' assert cls.snapshot_client.wait_for_service(timeout_sec=30.0), \ 'get_snapshots service not available' + assert cls.list_client.wait_for_service(timeout_sec=30.0), \ + 'list_faults service not available' # The publisher uses volatile durability, so an event sent before the # subscription matches is lost. Without this wait an assertion that @@ -268,6 +275,52 @@ def keep_publishing(): f'the configured topic was not captured, only {list(parsed["topics"])}' ) + def test_04_a_muted_symptom_is_confirmed_but_not_announced(self): + """ + The gate the report path has and the timer used to be missing. + + Correlation mutes a symptom so that only its root cause is announced. + The report path wraps every confirmation publish in that check; the + timer did not, so a symptom promoted by the timer reached subscribers + while the default fault list still hid it. The symptom must still + confirm in the store: it is the announcement that is suppressed, not + the confirmation. + """ + root = 'ESTOP_001' + symptom = 'MOTOR_COMM_1' + + # Root first, then the symptom inside the rule's window, so correlation + # sees the second as caused by the first. + self._report_failed(root) + self._report_failed(symptom) + + root_event = self._wait_for_confirmed_event(root, CONFIRM_WAIT_SEC) + self.assertIsNotNone(root_event, 'the root cause was never announced') + + # Give the timer a full further window: if the symptom were going to be + # announced, it would have been by now. + self._spin_for(AUTO_CONFIRM_SEC + 2.0) + announced = [ + e for e in self._events_for(symptom) if e.event_type == FaultEvent.EVENT_CONFIRMED + ] + self.assertEqual( + [], announced, + 'a muted symptom was announced as confirmed, which is what muting exists to prevent' + ) + + request = ListFaults.Request() + request.statuses = ['CONFIRMED'] + request.include_muted = True + future = self.list_client.call_async(request) + rclpy.spin_until_future_complete(self.node, future, timeout_sec=30.0) + self.assertIsNotNone(future.result(), 'ListFaults timed out') + confirmed = [f.fault_code for f in future.result().faults] + self.assertIn( + symptom, confirmed, + 'the symptom never confirmed, so this run does not show that muting is what ' + 'suppressed the event' + ) + def _wait_for_snapshot(self, fault_code, timeout_sec): """Poll GetSnapshots until one exists for this fault, or give up.""" deadline = time.monotonic() + timeout_sec