diff --git a/docs/api/rest.rst b/docs/api/rest.rst index 00aceb0aa..16d3fbca9 100644 --- a/docs/api/rest.rst +++ b/docs/api/rest.rst @@ -1514,7 +1514,15 @@ Query and manage faults. **Snapshot Types:** - - ``freeze_frame``: Data captured at fault confirmation. Entity frames for + - ``freeze_frame``: Data captured at fault confirmation: one entry per + captured topic, plus one entry named ``freeze_frame`` whose ``data`` maps + each topic to its value. That entry can also hold **reporter evidence** + under the key ``x-medkit-reported``: the key-value pairs a reporter sent + with its FAILED reports (for example the ``DiagnosticStatus`` values from + the diagnostic bridge), as string values, last value per key. Topic keys + start with ``/``, so they cannot clash with it. Evidence is written on + every FAILED report, so a fault with no capture config can still have a + ``freeze_frame`` entry with only evidence in it. Entity frames for faults that were already confirmed when the gateway started are captured at gateway start instead and carry ``"capture_origin": "startup"`` in their ``x-medkit`` block. An entity frame also carries ``source`` in diff --git a/docs/config/diagnostic-bridge.rst b/docs/config/diagnostic-bridge.rst index c40545b3e..0165e3367 100644 --- a/docs/config/diagnostic-bridge.rst +++ b/docs/config/diagnostic-bridge.rst @@ -37,7 +37,31 @@ The diagnostic bridge: - SEVERITY_ERROR (2) * - STALE - FAILED - - SEVERITY_ERROR (2) + - ``stale_severity``, SEVERITY_CRITICAL (3) by default + +.. warning:: + + This table previously documented STALE as SEVERITY_ERROR. The bridge has always sent + SEVERITY_CRITICAL, and CRITICAL **bypasses debounce** in the fault manager, so a STALE + status confirms a CRITICAL fault on its first sample. + + That matters because STALE is the one level a node can reach by design: a GPS goes STALE + in every tunnel, an IMU reports covariance -1 while it settles. Left unconfigured, every + such outage is a confirmed CRITICAL fault. Use ``stale_severity`` to set the level, or + ``stale_severity_overrides`` to name just the diagnostic names that go STALE on purpose; + those statuses then debounce like any other. + +.. important:: + + A non-CRITICAL STALE is only debounced if the fault manager is set up for it. With the + default ``confirmation_threshold: -1``, the first FAILED report confirms the fault. The + bridge's local filter only waits for 3 reports within 10 s. So with both defaults, a + planned STALE still becomes a confirmed fault after about three cycles. Set + ``confirmation_threshold`` to ``-3`` or lower to debounce it. + + This threshold is picked by ``source_id``. All statuses from the bridge share one + ``source_id`` (``/diagnostic_bridge``, unless ``use_hardware_id_as_source_id`` is on), so + a threshold for the bridge applies to **every** diagnostic it forwards, not only STALE ones. Parameters ---------- @@ -49,6 +73,9 @@ Parameters diagnostics_topic: "/diagnostics" # Topic to subscribe to auto_generate_codes: true # Auto-generate fault codes from names keyvalue_codes: ["fault_code"] # Take the code from these key-value keys + stale_severity: "WARN" # What a STALE status reports at + # Per diagnostic name, longest matching prefix wins: + "stale_severity_overrides.gps": "WARN" .. list-table:: :header-rows: 1 @@ -71,6 +98,61 @@ Parameters as its value, so a publisher can name its own code instead of relying on a mapping. Checked after ``name_to_code`` and before auto-generation. Empty strings in the list are ignored. + * - ``stale_severity`` + - ``CRITICAL`` + - Severity a STALE status reports at, one of ``INFO``, ``WARN``, ``ERROR``, + ``CRITICAL`` (case-insensitive). Applies to STALE only: the other levels are + facts about the status, not deployment decisions. A name that does not parse + is reported and ``CRITICAL`` is used. + * - ``stale_severity_overrides.`` + - ``-`` + - Severity for STALE statuses whose name starts with ````. Diagnostic names + are conventionally ``: ``, so a component prefix covers every + check it publishes. The **longest matching prefix** wins. An override that does + not parse is reported and **ignored**, leaving ``stale_severity`` in force - + applying ``CRITICAL`` to a typo would restore the immediate-confirm behaviour the + operator was configuring their way out of. + +Evidence +-------- + +A FAILED report carries the ``DiagnosticStatus`` key-values to the fault manager, which keeps +them in the fault's freeze frame and serves them from +``GET /api/v1/apps/{app}/faults/{code}``. The fault is reported under the bridge's +``source_id``, so with the bridge defaults the app is ``diagnostic_bridge``: + +.. code-block:: console + + $ curl -s localhost:8080/api/v1/apps/diagnostic_bridge/faults/FUSION_DIVERGED | jq '.environment_data.snapshots[] | select(.type == "freeze_frame" and .name == "freeze_frame") | .data["x-medkit-reported"]' + { + "rejected_fixes": "37", + "nis": "0.03" + } + +If topics were also captured for the code, the list has one entry per topic before the +freeze frame. Those entries also have type ``freeze_frame``, so the filter checks the name +too. + +A node that publishes outlier counts and gate statistics has already computed why it is +unhappy; before this the bridge read those values only to pick a fault code and dropped the +rest, so the fault record said a node complained but not what it saw. + +Evidence is written on **every** FAILED report, not only the one that confirms the fault, so a +code that never confirms still has evidence. Only the **last** value of each key is kept, and +near-miss entries do not store evidence. So a code with several near misses shows only the +numbers from its latest report. + +The frame's other keys are topic names sampled by the fault manager, always fully qualified +and so always starting with ``/``. ``x-medkit-reported`` cannot collide with one, which is what lets +a reader tell a value the reporter asserted from one the fault manager sampled. + +.. note:: + + Evidence limits per fault code: at most 32 keys, 128 bytes per key and 512 bytes per value + (bytes, not characters). There is no limit on total size. Bytes that are not valid UTF-8 + are stored as U+FFFD, and the limits are checked after that. An entry over a limit is + dropped, not cut short, and a warning is logged. The fault is recorded either way. A key already stored can always be updated, so a steady reporter + at the bound can still refresh its own numbers. Custom Fault Code Mappings -------------------------- diff --git a/docs/requirements/specs/faults.rst b/docs/requirements/specs/faults.rst index f3434a3f6..6c816a3d8 100644 --- a/docs/requirements/specs/faults.rst +++ b/docs/requirements/specs/faults.rst @@ -48,6 +48,19 @@ Faults shall take precedence over global defaults. Unspecified fields shall inherit from global configuration. When no entity prefix matches, global defaults shall apply. +.. req:: Reporter-Supplied Fault Evidence + :id: REQ_INTEROP_110 + :status: verified + :tags: Faults + + A FAILED fault report shall be able to carry key-value measurements, which the fault + manager shall retain in the fault's freeze frame under a reserved key distinct from + captured topic values, and serve with the fault's environment data. Evidence shall be + retained across reports that carry none, preserved when a confirmation capture rebuilds + the frame, and bounded per fault code in entry count and in key and value length + (measured in bytes), with entries exceeding a bound dropped whole and reported rather + than truncated. + .. req:: Fault Snapshot and Rosbag Capture :id: REQ_INTEROP_088 :status: verified diff --git a/src/ros2_medkit_diagnostic_bridge/CMakeLists.txt b/src/ros2_medkit_diagnostic_bridge/CMakeLists.txt index f81366bc8..34162201d 100644 --- a/src/ros2_medkit_diagnostic_bridge/CMakeLists.txt +++ b/src/ros2_medkit_diagnostic_bridge/CMakeLists.txt @@ -117,6 +117,8 @@ if(BUILD_TESTING) # repo convention) so it runs only in the integration phase. medkit_add_launch_test(test_integration test/test_integration.test.py TIMEOUT 60 LABELS "integration") + medkit_add_launch_test(test_stale_and_evidence test/test_stale_and_evidence.test.py TIMEOUT 90 + LABELS "integration") ros2_medkit_relax_vendor_warnings() endif() diff --git a/src/ros2_medkit_diagnostic_bridge/README.md b/src/ros2_medkit_diagnostic_bridge/README.md index da42c3528..451000024 100644 --- a/src/ros2_medkit_diagnostic_bridge/README.md +++ b/src/ros2_medkit_diagnostic_bridge/README.md @@ -15,7 +15,64 @@ to the FaultManager as faults. | OK (0) | - | Sends PASSED event (healing) | | WARN (1) | WARN (1) | Reports fault | | ERROR (2) | ERROR (2) | Reports fault | -| STALE (3) | CRITICAL (3) | Reports fault | +| STALE (3) | `stale_severity` (CRITICAL by default) | Reports fault | + +STALE is the one level whose severity is a deployment decision rather than a fact. A GPS in +every tunnel and a dead sensor both publish STALE, and CRITICAL +[bypasses debounce](../ros2_medkit_fault_manager/README.md), so an expected outage confirms a +CRITICAL fault on its first sample. Set `stale_severity`, or name the noisy diagnostic names +with `stale_severity_overrides`, and those statuses go through debounce like any other. + +This only helps if the fault manager debounces. With the default `confirmation_threshold: -1` +the first FAILED report confirms, and the bridge's local filter only waits for 3 reports in +10 s, so a planned STALE still confirms after about three cycles. Set `confirmation_threshold` +to `-3` or lower. The threshold is picked by `source_id`, and all statuses from the bridge +share one (`/diagnostic_bridge`, unless `use_hardware_id_as_source_id` is on), so it applies +to every diagnostic the bridge forwards. + +## Evidence + +A FAILED report carries the status's key-values to the fault manager, which keeps them in the +fault's freeze frame under the reserved key `x-medkit-reported` and serves them back from +`GET /api/v1/apps/{app}/faults/{code}` (with the bridge defaults the app is +`diagnostic_bridge`): + +```jsonc +{ + "environment_data": { + "snapshots": [ + // ...one entry per captured topic, if the code has a capture configured... + { + "type": "freeze_frame", + "name": "freeze_frame", + "data": { + "x-medkit-reported": { "rejected_fixes": "37", "nis": "0.03" } + }, + "x-medkit": { + "topic": "", + "message_type": "", + "full_data": { + "x-medkit-reported": { "rejected_fixes": "37", "nis": "0.03" } + }, + "captured_at": "2026-10-08T09:15:02.123456789Z" + } + } + ] + } +} +``` + +The numbers a publisher already computed to decide something was wrong are usually the whole +explanation of the fault, and before this they were dropped. The frame's other keys are topic +names sampled by the fault manager, which are always fully qualified and start with `/`, so a +reader can always tell a value the reporter asserted from one the fault manager sampled. + +`keyvalue_codes` still reads the same values to pick a fault code; the two uses are +independent and either can be used without the other. Only the last value of each key is +kept, and near-miss entries do not store evidence. Limits per fault code: 32 keys, 128 bytes +per key, 512 bytes per value, no limit on total size. An entry over a limit is dropped with a +warning, not cut short, and the fault is recorded either way. Bytes that are not valid UTF-8 +are stored as U+FFFD. ## Quick Start @@ -39,6 +96,8 @@ ros2 run ros2_medkit_diagnostic_bridge diagnostic_bridge_node | `max_tracked_sources` | integer | `512` | Maximum number of per-source FaultReporter instances retained by the bridge | | `name_to_code.` | string | - | Custom mapping from diagnostic name to fault code | | `keyvalue_codes` | string[] | - | List of keys used to search the diagnostic values for the fault code | +| `stale_severity` | string | `CRITICAL` | Severity a STALE status reports at. One of `INFO`, `WARN`, `ERROR`, `CRITICAL` | +| `stale_severity_overrides.` | string | - | Severity for STALE statuses whose name starts with ``. Longest matching prefix wins | ### Example Configuration diff --git a/src/ros2_medkit_diagnostic_bridge/config/diagnostic_bridge.yaml b/src/ros2_medkit_diagnostic_bridge/config/diagnostic_bridge.yaml index af88d6aaf..3247e5bcc 100644 --- a/src/ros2_medkit_diagnostic_bridge/config/diagnostic_bridge.yaml +++ b/src/ros2_medkit_diagnostic_bridge/config/diagnostic_bridge.yaml @@ -14,6 +14,19 @@ diagnostic_bridge: # Maximum number of per-source FaultReporter instances retained by the bridge. max_tracked_sources: 512 + # Severity a STALE status reports at. STALE is the one level a node can reach by + # design - a GPS in every tunnel, an IMU reporting covariance -1 - and CRITICAL + # bypasses debounce, so leaving it CRITICAL confirms a fault on the first sample. + # One of INFO, WARN, ERROR, CRITICAL. + stale_severity: "CRITICAL" + + # Per-diagnostic STALE severity. Matches on the start of the diagnostic name, so a + # component prefix covers every check it publishes. Longest matching prefix wins. + # Format: "stale_severity_overrides.": "" + # Example: + # "stale_severity_overrides.gps": "WARN" + # "stale_severity_overrides.gps: antenna": "ERROR" + # Custom diagnostic name to fault code mappings # Format: "name_to_code.": "" # Example: diff --git a/src/ros2_medkit_diagnostic_bridge/design/index.rst b/src/ros2_medkit_diagnostic_bridge/design/index.rst index 97231ec1a..6a834d91e 100644 --- a/src/ros2_medkit_diagnostic_bridge/design/index.rst +++ b/src/ros2_medkit_diagnostic_bridge/design/index.rst @@ -48,12 +48,14 @@ The following diagram shows the data flow and component relationships. +name: string +message: string +hardware_id: string + +values: KeyValue[] } } package "ros2_medkit_fault_reporter" { class FaultReporter { +report(fault_code, severity, description) + +report(fault_code, severity, description, evidence) +report_passed(fault_code) } } @@ -62,8 +64,10 @@ The following diagram shows the data flow and component relationships. class DiagnosticBridgeNode { +map_to_fault_code(status): string - +map_to_severity(level): optional - +is_ok_level(level): bool + +map_to_severity(level, name): optional + +stale_severity_for(name): uint8 + +{static} parse_severity_name(name): optional + +{static} is_ok_level(level): bool -- -diagnostics_callback(msg) -process_diagnostic(status) @@ -102,9 +106,10 @@ Data Flow Reporter -> FM : ReportFault(PASSED) note right: Triggers healing else level == WARN/ERROR/STALE - Bridge -> Reporter : report(fault_code, severity, msg) - Reporter -> FM : ReportFault(FAILED) - note right: Creates/updates fault + Bridge -> Reporter : report(fault_code, severity, msg, values) + Reporter -> FM : ReportFault(FAILED, evidence) + note right: Creates/updates fault, +keeps evidence in freeze frame end @enduml @@ -131,8 +136,9 @@ The bridge maps DiagnosticStatus levels to FaultManager severities: - ERROR (2) - Error-level fault * - STALE (3) - - CRITICAL (3) - - Stale data treated as critical + - ``stale_severity`` (CRITICAL by default), per diagnostic name via + ``stale_severity_overrides`` + - See `Configurable STALE severity`_ Main Components --------------- @@ -148,7 +154,8 @@ Main Components 3. **FaultReporter Integration** - Bridges to FaultManager - OK status → ``report_passed()`` for healing - - WARN/ERROR/STALE → ``report()`` with mapped severity + - WARN/ERROR/STALE → ``report()`` with mapped severity and the status ``values`` as + evidence Configuration ------------- @@ -172,6 +179,13 @@ Parameters * - ``name_to_code.`` - (none) - Custom mapping from diagnostic name to fault code + * - ``stale_severity`` + - ``CRITICAL`` + - Severity a STALE status reports at (``INFO``, ``WARN``, ``ERROR``, ``CRITICAL``) + * - ``stale_severity_overrides.`` + - (none) + - Severity for STALE statuses whose diagnostic name starts with ````; longest + prefix wins Example Launch Configuration ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -208,8 +222,23 @@ Examples: - ``"/robot/arm"`` → ``"ROBOT_ARM"`` - ``"sensor: status"`` → ``"SENSOR_STATUS"`` -STALE as CRITICAL -~~~~~~~~~~~~~~~~~ +Configurable STALE severity +~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +STALE maps to ``stale_severity``, CRITICAL by default, because stale data often means a lost +connection or a crashed node. But some nodes go STALE on purpose (a GPS in a tunnel, an IMU +while it settles), and CRITICAL skips debounce in the fault manager. So the severity is +configurable. ``stale_severity_overrides.`` sets it per diagnostic name, and the longest +matching prefix wins. That is why ``map_to_severity`` takes the diagnostic name as well as the +level. An override that cannot be parsed is logged and ignored, and ``stale_severity`` is used. + +A non-CRITICAL STALE is only held back if the fault manager's ``confirmation_threshold`` needs +more than one report. That threshold is picked by ``source_id``, which all statuses from the +bridge share, so it applies to every diagnostic the bridge forwards. + +Evidence +~~~~~~~~ -STALE diagnostics are mapped to CRITICAL severity because stale data typically indicates -a communication failure or node crash, which is a serious system health issue. +The status ``values`` are sent as ``ReportFault.evidence`` with every FAILED report. The fault +manager keeps them in the fault's freeze frame under ``x-medkit-reported``. See +:doc:`/config/diagnostic-bridge`. diff --git a/src/ros2_medkit_diagnostic_bridge/include/ros2_medkit_diagnostic_bridge/diagnostic_bridge_node.hpp b/src/ros2_medkit_diagnostic_bridge/include/ros2_medkit_diagnostic_bridge/diagnostic_bridge_node.hpp index 302be9c90..03fd4d4d3 100644 --- a/src/ros2_medkit_diagnostic_bridge/include/ros2_medkit_diagnostic_bridge/diagnostic_bridge_node.hpp +++ b/src/ros2_medkit_diagnostic_bridge/include/ros2_medkit_diagnostic_bridge/diagnostic_bridge_node.hpp @@ -26,6 +26,7 @@ #include "rclcpp/rclcpp.hpp" #include "diagnostic_msgs/msg/diagnostic_array.hpp" #include "ros2_medkit_fault_reporter/fault_reporter.hpp" +#include "ros2_medkit_msgs/msg/fault.hpp" namespace ros2_medkit_diagnostic_bridge { @@ -38,7 +39,11 @@ namespace ros2_medkit_diagnostic_bridge { /// - OK (0) -> PASSED event (healing) /// - WARN (1) -> WARN severity (1) /// - ERROR (2) -> ERROR severity (2) -/// - STALE (3) -> CRITICAL severity (3) +/// - STALE (3) -> stale_severity (CRITICAL by default, overridable per diagnostic name) +/// +/// STALE is configurable because a node can be STALE by design - a GPS in every tunnel, an +/// IMU reporting covariance -1 - and CRITICAL bypasses debounce, so an expected outage would +/// confirm a CRITICAL fault on its first sample. /// /// Example launch configuration: /// @code{.yaml} @@ -61,9 +66,20 @@ class DiagnosticBridgeNode : public rclcpp::Node { /// 4. Return empty string if no mapping found std::string map_to_fault_code(const diagnostic_msgs::msg::DiagnosticStatus & status) const; - /// Map DiagnosticStatus level to Fault severity - /// Returns std::nullopt if level is OK (should send PASSED instead) - static std::optional map_to_severity(uint8_t diagnostic_level); + /// Map DiagnosticStatus level to Fault severity for a named diagnostic. + /// Returns std::nullopt if level is OK (should send PASSED instead). + /// STALE resolves through stale_severity and stale_severity_overrides; every other + /// level maps fixed, so only the level that can be a design decision is configurable. + std::optional map_to_severity(uint8_t diagnostic_level, const std::string & diagnostic_name) const; + + /// Severity a STALE status from @p diagnostic_name reports at. + /// Longest matching prefix among stale_severity_overrides wins; stale_severity otherwise. + uint8_t stale_severity_for(const std::string & diagnostic_name) const; + + /// Parse a severity name ("INFO", "WARN", "ERROR", "CRITICAL", case-insensitive). + /// Returns std::nullopt for anything else, so a caller can report the typo rather than + /// silently substituting a severity nobody configured. + static std::optional parse_severity_name(const std::string & name); /// Check if diagnostic level indicates OK status static bool is_ok_level(uint8_t diagnostic_level); @@ -110,6 +126,17 @@ class DiagnosticBridgeNode : public rclcpp::Node { int max_tracked_sources_{512}; std::map name_to_code_; std::vector keyvalue_codes_; + + /// Severity a STALE status reports at when no override matches. + uint8_t stale_severity_{ros2_medkit_msgs::msg::Fault::SEVERITY_CRITICAL}; + + /// Per-diagnostic STALE severity, sorted by prefix length descending so the first + /// match found is the longest one. + struct StaleSeverityOverride { + std::string prefix; + uint8_t severity; + }; + std::vector stale_severity_overrides_; }; } // namespace ros2_medkit_diagnostic_bridge diff --git a/src/ros2_medkit_diagnostic_bridge/src/diagnostic_bridge_node.cpp b/src/ros2_medkit_diagnostic_bridge/src/diagnostic_bridge_node.cpp index 9bc3d87d2..a0b58bd61 100644 --- a/src/ros2_medkit_diagnostic_bridge/src/diagnostic_bridge_node.cpp +++ b/src/ros2_medkit_diagnostic_bridge/src/diagnostic_bridge_node.cpp @@ -58,18 +58,56 @@ void DiagnosticBridgeNode::load_parameters() { return !s.empty(); }); + // STALE is the one level whose severity is a deployment decision rather than a fact: + // a GPS in a tunnel and a dead sensor both publish STALE, and only the operator knows + // which is which. Default CRITICAL keeps today's behaviour for anyone not configuring it. + const std::string stale_severity_name = declare_parameter("stale_severity", "CRITICAL"); + if (auto parsed = parse_severity_name(stale_severity_name)) { + stale_severity_ = *parsed; + } else { + RCLCPP_WARN(get_logger(), + "stale_severity '%s' is not a severity name; using CRITICAL. " + "Expected one of INFO, WARN, ERROR, CRITICAL.", + stale_severity_name.c_str()); + } + // Load custom name_to_code mappings from parameter overrides // Format: name_to_code. = // Example: --ros-args -p "name_to_code.motor_temp:=MOTOR_OVERHEAT" auto params = get_node_parameters_interface()->get_parameter_overrides(); const std::string prefix = "name_to_code."; + const std::string stale_prefix = "stale_severity_overrides."; for (const auto & [name, value] : params) { if (name.rfind(prefix, 0) == 0 && value.get_type() == rclcpp::ParameterType::PARAMETER_STRING) { std::string diag_name = name.substr(prefix.length()); name_to_code_[diag_name] = value.get(); RCLCPP_DEBUG(get_logger(), "Loaded mapping: '%s' -> '%s'", diag_name.c_str(), name_to_code_[diag_name].c_str()); + continue; + } + + // Format: stale_severity_overrides. = + if (name.rfind(stale_prefix, 0) == 0 && value.get_type() == rclcpp::ParameterType::PARAMETER_STRING) { + const std::string diag_prefix = name.substr(stale_prefix.length()); + const std::string severity_name = value.get(); + auto parsed = parse_severity_name(severity_name); + if (!parsed) { + // Skipped, not defaulted: an operator who wrote a typo asked for something specific, + // and quietly applying CRITICAL would confirm the fault they were trying to debounce. + RCLCPP_WARN(get_logger(), + "stale_severity_overrides.%s = '%s' is not a severity name; ignoring this override. " + "Expected one of INFO, WARN, ERROR, CRITICAL.", + diag_prefix.c_str(), severity_name.c_str()); + continue; + } + stale_severity_overrides_.push_back(StaleSeverityOverride{diag_prefix, *parsed}); } } + + // Longest prefix first, so the first match found is the most specific one. + std::sort(stale_severity_overrides_.begin(), stale_severity_overrides_.end(), + [](const StaleSeverityOverride & a, const StaleSeverityOverride & b) { + return a.prefix.size() > b.prefix.size(); + }); } void DiagnosticBridgeNode::diagnostics_callback(const diagnostic_msgs::msg::DiagnosticArray::ConstSharedPtr & msg) { @@ -147,9 +185,14 @@ void DiagnosticBridgeNode::process_diagnostic(const diagnostic_msgs::msg::Diagno RCLCPP_DEBUG(get_logger(), "Diagnostic OK: %s -> PASSED for %s", status.name.c_str(), fault_code.c_str()); } else { // WARN, ERROR, STALE -> send FAILED event - auto severity = map_to_severity(status.level); + auto severity = map_to_severity(status.level, status.name); // severity is guaranteed to have value here (not OK level) - reporter->report(fault_code, *severity, status.message); + // + // The key-values travel with the report as evidence. They are the numbers the publisher + // already computed to decide something was wrong - outlier counts, gate statistics - and + // without them the fault record says a node complained but not what it saw. keyvalue_codes + // still reads the same values to pick the code; the two uses are independent. + reporter->report(fault_code, *severity, status.message, status.values); RCLCPP_DEBUG(get_logger(), "Diagnostic %s: %s -> fault %s (severity=%d)", status.name.c_str(), status.message.c_str(), fault_code.c_str(), *severity); } @@ -187,7 +230,8 @@ std::string DiagnosticBridgeNode::map_to_fault_code(const diagnostic_msgs::msg:: return ""; } -std::optional DiagnosticBridgeNode::map_to_severity(uint8_t diagnostic_level) { +std::optional DiagnosticBridgeNode::map_to_severity(uint8_t diagnostic_level, + const std::string & diagnostic_name) const { using DiagStatus = diagnostic_msgs::msg::DiagnosticStatus; using Fault = ros2_medkit_msgs::msg::Fault; @@ -199,12 +243,46 @@ std::optional DiagnosticBridgeNode::map_to_severity(uint8_t diagnostic_ case DiagStatus::ERROR: return Fault::SEVERITY_ERROR; case DiagStatus::STALE: - return Fault::SEVERITY_CRITICAL; + return stale_severity_for(diagnostic_name); default: return Fault::SEVERITY_ERROR; // Unknown level -> ERROR } } +uint8_t DiagnosticBridgeNode::stale_severity_for(const std::string & diagnostic_name) const { + // Entries are sorted longest-prefix-first, so the first hit is the most specific. + for (const auto & entry : stale_severity_overrides_) { + if (diagnostic_name.rfind(entry.prefix, 0) == 0) { + return entry.severity; + } + } + return stale_severity_; +} + +std::optional DiagnosticBridgeNode::parse_severity_name(const std::string & name) { + using Fault = ros2_medkit_msgs::msg::Fault; + + std::string upper; + upper.reserve(name.size()); + for (char c : name) { + upper += static_cast(std::toupper(static_cast(c))); + } + + if (upper == "INFO") { + return Fault::SEVERITY_INFO; + } + if (upper == "WARN") { + return Fault::SEVERITY_WARN; + } + if (upper == "ERROR") { + return Fault::SEVERITY_ERROR; + } + if (upper == "CRITICAL") { + return Fault::SEVERITY_CRITICAL; + } + return std::nullopt; +} + bool DiagnosticBridgeNode::is_ok_level(uint8_t diagnostic_level) { return diagnostic_level == diagnostic_msgs::msg::DiagnosticStatus::OK; } diff --git a/src/ros2_medkit_diagnostic_bridge/test/test_diagnostic_bridge.cpp b/src/ros2_medkit_diagnostic_bridge/test/test_diagnostic_bridge.cpp index 8dc16554f..a8b4a473f 100644 --- a/src/ros2_medkit_diagnostic_bridge/test/test_diagnostic_bridge.cpp +++ b/src/ros2_medkit_diagnostic_bridge/test/test_diagnostic_bridge.cpp @@ -55,36 +55,53 @@ std::shared_ptr make_node_with_keyvalue_codes(std::vector< options.append_parameter_override("keyvalue_codes", keyvalue_codes); return std::make_shared(options); } + +/// A bridge with no configuration beyond the defaults. Severity mapping is a method +/// rather than a free function now that STALE is configurable, so the cases that used +/// to call it statically need an instance to call it on. +std::shared_ptr make_default_node() { + return std::make_shared(rclcpp::NodeOptions()); +} + +std::shared_ptr +make_node_with_params(const std::vector> & overrides) { + rclcpp::NodeOptions options; + for (const auto & [name, value] : overrides) { + options.append_parameter_override(name, value); + } + return std::make_shared(options); +} } // namespace // Test severity mapping TEST_F(DiagnosticBridgeTest, MapToSeverity_Warn) { - auto result = DiagnosticBridgeNode::map_to_severity(DiagStatus::WARN); + auto result = make_default_node()->map_to_severity(DiagStatus::WARN, "any"); ASSERT_TRUE(result.has_value()); EXPECT_EQ(*result, Fault::SEVERITY_WARN); } TEST_F(DiagnosticBridgeTest, MapToSeverity_Error) { - auto result = DiagnosticBridgeNode::map_to_severity(DiagStatus::ERROR); + auto result = make_default_node()->map_to_severity(DiagStatus::ERROR, "any"); ASSERT_TRUE(result.has_value()); EXPECT_EQ(*result, Fault::SEVERITY_ERROR); } TEST_F(DiagnosticBridgeTest, MapToSeverity_Stale) { - auto result = DiagnosticBridgeNode::map_to_severity(DiagStatus::STALE); + // Unconfigured, STALE still maps to CRITICAL: the change is opt-in. + auto result = make_default_node()->map_to_severity(DiagStatus::STALE, "any"); ASSERT_TRUE(result.has_value()); EXPECT_EQ(*result, Fault::SEVERITY_CRITICAL); } TEST_F(DiagnosticBridgeTest, MapToSeverity_Ok) { // OK should return nullopt (use is_ok_level and send PASSED instead) - auto result = DiagnosticBridgeNode::map_to_severity(DiagStatus::OK); + auto result = make_default_node()->map_to_severity(DiagStatus::OK, "any"); EXPECT_FALSE(result.has_value()); } TEST_F(DiagnosticBridgeTest, MapToSeverity_Unknown) { // Unknown level defaults to ERROR - auto result = DiagnosticBridgeNode::map_to_severity(99); + auto result = make_default_node()->map_to_severity(99, "any"); ASSERT_TRUE(result.has_value()); EXPECT_EQ(*result, Fault::SEVERITY_ERROR); } @@ -238,6 +255,89 @@ TEST_F(DiagnosticBridgeTest, MapToFaultCode_NameToCodeOverride_PrecedesKeyValueC EXPECT_EQ(node->map_to_fault_code(diagnostic_status("/motor", 1, "", {{"code", "FROM_ATTRIBUTE"}})), "OVERRIDE_CODE"); } +// --------------------------------------------------------------------------- +// STALE severity: the level a node can be by design +// --------------------------------------------------------------------------- + +TEST_F(DiagnosticBridgeTest, StaleSeverity_DefaultIsCriticalForEveryName) { + auto node = make_default_node(); + EXPECT_EQ(node->stale_severity_for("gps"), Fault::SEVERITY_CRITICAL); + EXPECT_EQ(node->stale_severity_for("anything else"), Fault::SEVERITY_CRITICAL); +} + +TEST_F(DiagnosticBridgeTest, StaleSeverity_GlobalParameterAppliesToAll) { + auto node = make_node_with_params({{"stale_severity", "WARN"}}); + EXPECT_EQ(node->map_to_severity(DiagStatus::STALE, "gps").value(), Fault::SEVERITY_WARN); + EXPECT_EQ(node->map_to_severity(DiagStatus::STALE, "imu").value(), Fault::SEVERITY_WARN); + // Only STALE moves; the levels that are facts rather than decisions stay put. + EXPECT_EQ(node->map_to_severity(DiagStatus::ERROR, "gps").value(), Fault::SEVERITY_ERROR); +} + +TEST_F(DiagnosticBridgeTest, StaleSeverity_OverrideBeatsGlobalDefault) { + auto node = make_node_with_params({{"stale_severity_overrides.gps", "WARN"}}); + EXPECT_EQ(node->map_to_severity(DiagStatus::STALE, "gps").value(), Fault::SEVERITY_WARN); + // The issue's own acceptance criterion: without an override it stays CRITICAL. + EXPECT_EQ(node->map_to_severity(DiagStatus::STALE, "lidar").value(), Fault::SEVERITY_CRITICAL); +} + +TEST_F(DiagnosticBridgeTest, StaleSeverity_OverrideMatchesOnPrefix) { + auto node = make_node_with_params({{"stale_severity_overrides.gps", "WARN"}}); + // Diagnostic names are conventionally ": ", so a prefix is how an + // operator names every check a component publishes without listing them. + EXPECT_EQ(node->stale_severity_for("gps: fix quality"), Fault::SEVERITY_WARN); + EXPECT_EQ(node->stale_severity_for("gpsd"), Fault::SEVERITY_WARN); + EXPECT_EQ(node->stale_severity_for("imu: covariance"), Fault::SEVERITY_CRITICAL); +} + +TEST_F(DiagnosticBridgeTest, StaleSeverity_LongestPrefixWins) { + auto node = make_node_with_params({ + {"stale_severity_overrides.gps", "WARN"}, + {"stale_severity_overrides.gps: antenna", "ERROR"}, + }); + EXPECT_EQ(node->stale_severity_for("gps: antenna shorted"), Fault::SEVERITY_ERROR); + EXPECT_EQ(node->stale_severity_for("gps: fix quality"), Fault::SEVERITY_WARN); +} + +TEST_F(DiagnosticBridgeTest, StaleSeverity_OverrideCombinesWithNonCriticalDefault) { + auto node = make_node_with_params({ + {"stale_severity", "WARN"}, + {"stale_severity_overrides.safety_chain", "CRITICAL"}, + }); + // A deployment that treats STALE as routine still needs the one sensor where it is not. + EXPECT_EQ(node->stale_severity_for("safety_chain: estop"), Fault::SEVERITY_CRITICAL); + EXPECT_EQ(node->stale_severity_for("gps"), Fault::SEVERITY_WARN); +} + +TEST_F(DiagnosticBridgeTest, StaleSeverity_UnparseableGlobalFallsBackToCritical) { + auto node = make_node_with_params({{"stale_severity", "not-a-severity"}}); + EXPECT_EQ(node->stale_severity_for("gps"), Fault::SEVERITY_CRITICAL); +} + +TEST_F(DiagnosticBridgeTest, StaleSeverity_UnparseableOverrideIsIgnoredNotDefaulted) { + auto node = make_node_with_params({ + {"stale_severity", "WARN"}, + {"stale_severity_overrides.gps", "WHOOPS"}, + }); + // The typo must not be read as CRITICAL: an operator debouncing a noisy GPS would get + // the immediate-confirm behaviour they were configuring their way out of. + EXPECT_EQ(node->stale_severity_for("gps"), Fault::SEVERITY_WARN); +} + +TEST_F(DiagnosticBridgeTest, StaleSeverity_NamesAreCaseInsensitive) { + auto node = make_node_with_params({{"stale_severity", "warn"}}); + EXPECT_EQ(node->stale_severity_for("gps"), Fault::SEVERITY_WARN); +} + +TEST_F(DiagnosticBridgeTest, ParseSeverityName_AcceptsTheFourNamesAndNothingElse) { + EXPECT_EQ(DiagnosticBridgeNode::parse_severity_name("INFO").value(), Fault::SEVERITY_INFO); + EXPECT_EQ(DiagnosticBridgeNode::parse_severity_name("WARN").value(), Fault::SEVERITY_WARN); + EXPECT_EQ(DiagnosticBridgeNode::parse_severity_name("ERROR").value(), Fault::SEVERITY_ERROR); + EXPECT_EQ(DiagnosticBridgeNode::parse_severity_name("CRITICAL").value(), Fault::SEVERITY_CRITICAL); + EXPECT_FALSE(DiagnosticBridgeNode::parse_severity_name("").has_value()); + EXPECT_FALSE(DiagnosticBridgeNode::parse_severity_name("2").has_value()); + EXPECT_FALSE(DiagnosticBridgeNode::parse_severity_name("FATAL").has_value()); +} + int main(int argc, char ** argv) { testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); diff --git a/src/ros2_medkit_diagnostic_bridge/test/test_stale_and_evidence.test.py b/src/ros2_medkit_diagnostic_bridge/test/test_stale_and_evidence.test.py new file mode 100644 index 000000000..1603bd2b4 --- /dev/null +++ b/src/ros2_medkit_diagnostic_bridge/test/test_stale_and_evidence.test.py @@ -0,0 +1,256 @@ +#!/usr/bin/env python3 +# Copyright 2026 selfpatch +# +# 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. + +""" +Bridge and fault manager together, with confirmation_threshold: -3. + +test_integration uses -1, where every report confirms at once, so it cannot +show that a STALE override is debounced. This file checks: + +- STALE with an override is debounced; STALE without one confirms CRITICAL. +- The values of an ERROR status come back from GetFault. +- A value that is not valid UTF-8 does not crash the fault manager. +""" + +import json +import time +import unittest + +from diagnostic_msgs.msg import DiagnosticArray, DiagnosticStatus, KeyValue +from launch import LaunchDescription +import launch_ros.actions +import launch_testing.actions +import launch_testing.asserts +import launch_testing.markers +import rclpy +from rclpy.node import Node +from rclpy.serialization import serialize_message +from ros2_medkit_msgs.msg import Fault, Snapshot +from ros2_medkit_msgs.srv import GetFault, ListFaults + +REPORTED_KEY = 'x-medkit-reported' + +# Replaced in the serialized message. Same length as the new bytes, so the +# string length in the message stays correct. +UTF8_PLACEHOLDER = b'80#C' +LATIN1_DEGREES = b'80\xb0C' + + +def generate_test_description(): + """Launch a debouncing fault_manager and a bridge with one STALE override.""" + fault_manager_node = launch_ros.actions.Node( + package='ros2_medkit_fault_manager', + executable='fault_manager_node', + name='fault_manager', + output='screen', + parameters=[{ + 'storage_type': 'memory', + 'confirmation_threshold': -3, + 'healing_enabled': False, + }], + # Give the node room to flush coverage data at shutdown before SIGKILL. + sigterm_timeout='30', + sigkill_timeout='15', + ) + + diagnostic_bridge_node = launch_ros.actions.Node( + package='ros2_medkit_diagnostic_bridge', + executable='diagnostic_bridge_node', + name='diagnostic_bridge', + output='screen', + parameters=[{ + 'diagnostics_topic': '/diagnostics', + 'auto_generate_codes': True, + 'stale_severity_overrides.planned_gps': 'WARN', + }], + # Give the node room to flush coverage data at shutdown before SIGKILL. + sigterm_timeout='30', + sigkill_timeout='15', + ) + + return ( + LaunchDescription([ + fault_manager_node, + diagnostic_bridge_node, + launch_testing.actions.ReadyToTest(), + ]), + { + 'fault_manager_node': fault_manager_node, + 'diagnostic_bridge_node': diagnostic_bridge_node, + }, + ) + + +class TestStaleAndEvidence(unittest.TestCase): + """STALE overrides debounce, and status values survive into the fault.""" + + @classmethod + def setUpClass(cls): + """Initialize ROS 2 context, publisher and service clients.""" + rclpy.init() + cls.node = Node('test_stale_and_evidence_client') + cls.diag_pub = cls.node.create_publisher(DiagnosticArray, '/diagnostics', 10) + cls.list_faults_client = cls.node.create_client( + ListFaults, '/fault_manager/list_faults') + cls.get_fault_client = cls.node.create_client( + GetFault, '/fault_manager/get_fault') + + assert cls.list_faults_client.wait_for_service(timeout_sec=10.0), \ + 'ListFaults service not available' + assert cls.get_fault_client.wait_for_service(timeout_sec=10.0), \ + 'GetFault service not available' + + # Messages sent before the bridge subscribes are lost, so wait for it. + deadline = time.monotonic() + 15.0 + while cls.diag_pub.get_subscription_count() < 1: + assert time.monotonic() < deadline, ( + 'diagnostic_bridge did not subscribe to /diagnostics') + rclpy.spin_once(cls.node, timeout_sec=0.1) + + @classmethod + def tearDownClass(cls): + """Shutdown ROS 2 context.""" + 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) + return future.result() + + def _find_fault(self, code): + request = ListFaults.Request() + request.filter_by_severity = False + request.statuses = [Fault.STATUS_PREFAILED, Fault.STATUS_CONFIRMED] + result = self._call(self.list_faults_client, request) + faults = result.faults if result is not None else [] + return next((f for f in faults if f.fault_code == code), None) + + def _reported_evidence(self, code): + """Return the evidence object served by GetFault, or None.""" + request = GetFault.Request() + request.fault_code = code + result = self._call(self.get_fault_client, request) + self.assertIsNotNone(result, 'GetFault timed out: is the fault manager still up?') + if not result.success: + return None + frames = [ + s for s in result.environment_data.snapshots + if s.type == Snapshot.TYPE_FREEZE_FRAME and s.name == 'freeze_frame' + ] + if not frames: + return None + return json.loads(frames[0].data).get(REPORTED_KEY) + + def _status(self, name, level, values=()): + return DiagnosticStatus( + level=level, name=name, message='test', hardware_id='', + values=[KeyValue(key=k, value=v) for k, v in values], + ) + + def _publish(self, status): + msg = DiagnosticArray() + msg.header.stamp = self.node.get_clock().now().to_msg() + msg.status = [status] + self.diag_pub.publish(msg) + time.sleep(0.3) + + def _publish_until(self, publish, check, *, timeout=25.0): + """ + Publish until *check* returns something truthy, and return it. + + The bridge drops reports until its client finds the fault manager, so + one publish is not enough. + """ + deadline = time.monotonic() + timeout + last = None + while time.monotonic() < deadline: + publish() + last = check() + if last: + return last + self.fail(f'Condition not met within {timeout}s (last: {last})') + + def test_01_stale_with_override_is_debounced(self): + """STALE overridden to WARN is debounced instead of confirming at once.""" + status = self._status('planned_gps', DiagnosticStatus.STALE) + + first = self._publish_until( + lambda: self._publish(status), lambda: self._find_fault('PLANNED_GPS')) + self.assertEqual(first.severity, Fault.SEVERITY_WARN) + self.assertEqual( + first.status, Fault.STATUS_PREFAILED, + 'a STALE status overridden to WARN confirmed on its first report') + + # If it keeps coming, it still confirms. + def confirmed_fault(): + fault = self._find_fault('PLANNED_GPS') + return fault if fault and fault.status == Fault.STATUS_CONFIRMED else None + + confirmed = self._publish_until(lambda: self._publish(status), confirmed_fault) + self.assertEqual(confirmed.severity, Fault.SEVERITY_WARN) + + def test_02_stale_without_override_still_confirms_critical(self): + """STALE without an override is still CRITICAL and confirms at once.""" + status = self._status('unplanned_lidar', DiagnosticStatus.STALE) + + first = self._publish_until( + lambda: self._publish(status), lambda: self._find_fault('UNPLANNED_LIDAR')) + self.assertEqual(first.severity, Fault.SEVERITY_CRITICAL) + self.assertEqual(first.status, Fault.STATUS_CONFIRMED) + + def test_03_error_values_are_served_by_get_fault(self): + """The values of an ERROR status come back from GetFault.""" + status = self._status( + 'fusion_monitor', DiagnosticStatus.ERROR, + values=[('rejected_fixes', '37'), ('nis', '0.03')]) + + reported = self._publish_until( + lambda: self._publish(status), lambda: self._reported_evidence('FUSION_MONITOR')) + self.assertEqual(reported, {'rejected_fixes': '37', 'nis': '0.03'}) + + def test_04_invalid_utf8_value_does_not_crash_the_fault_manager(self): + """A Latin-1 byte in a value is stored as U+FFFD and nothing crashes.""" + status = self._status( + 'thermal_probe', DiagnosticStatus.ERROR, + values=[('temp', UTF8_PLACEHOLDER.decode())]) + msg = DiagnosticArray() + msg.status = [status] + raw = serialize_message(msg) + self.assertEqual(raw.count(UTF8_PLACEHOLDER), 1) + # rclpy cannot put invalid UTF-8 in a str field, so patch the bytes. + raw = raw.replace(UTF8_PLACEHOLDER, LATIN1_DEGREES) + + def publish_raw(): + self.diag_pub.publish(raw) + time.sleep(0.3) + + reported = self._publish_until( + publish_raw, lambda: self._reported_evidence('THERMAL_PROBE')) + self.assertEqual(reported, {'temp': '80�C'}) + + # Send it a few more times, then check the fault manager still answers. + for _ in range(5): + publish_raw() + self.assertEqual(self._reported_evidence('THERMAL_PROBE'), {'temp': '80�C'}) + + +@launch_testing.post_shutdown_test() +class TestStaleAndEvidenceShutdown(unittest.TestCase): + """Check that no node crashed.""" + + def test_exit_code(self, proc_info): + """Verify nodes exit cleanly.""" + launch_testing.asserts.assertExitCodes(proc_info) diff --git a/src/ros2_medkit_fault_manager/CMakeLists.txt b/src/ros2_medkit_fault_manager/CMakeLists.txt index 6a2ef6c07..a51bc794b 100644 --- a/src/ros2_medkit_fault_manager/CMakeLists.txt +++ b/src/ros2_medkit_fault_manager/CMakeLists.txt @@ -265,6 +265,12 @@ endif() medkit_add_gtest(test_correlation_engine test/test_correlation_engine.cpp) target_link_libraries(test_correlation_engine fault_manager_lib) + # Reporter-supplied evidence: the merge, its bounds, and the capture path that + # must not erase it. + medkit_add_gtest(test_reported_evidence test/test_reported_evidence.cpp) + target_link_libraries(test_reported_evidence fault_manager_lib) + medkit_target_dependencies(test_reported_evidence rclcpp ros2_medkit_msgs) + # Entity threshold resolver tests medkit_add_gtest(test_entity_thresholds test/test_entity_thresholds.cpp) target_link_libraries(test_entity_thresholds fault_manager_lib) @@ -286,6 +292,9 @@ endif() medkit_add_launch_test(test_rosbag_default_format test/integration/test_rosbag_default_format.test.py TIMEOUT 90 LABELS "integration") + medkit_add_launch_test(test_reported_evidence_integration + test/integration/test_reported_evidence.test.py TIMEOUT 60 LABELS "integration") + medkit_add_launch_test(test_entity_thresholds_integration test/integration/test_entity_thresholds_integration.test.py TIMEOUT 60 LABELS "integration") diff --git a/src/ros2_medkit_fault_manager/README.md b/src/ros2_medkit_fault_manager/README.md index e6ce3af93..44e6fd63f 100644 --- a/src/ros2_medkit_fault_manager/README.md +++ b/src/ros2_medkit_fault_manager/README.md @@ -54,7 +54,7 @@ ros2 service call /fault_manager/clear_fault ros2_medkit_msgs/srv/ClearFault \ - **Debounce filtering** (optional): AUTOSAR DEM-style counter-based fault confirmation with per-entity threshold overrides - **Snapshot capture**: Captures topic data when faults are confirmed for debugging (the value snapshots are deleted when the fault is cleared, unless `snapshots.retain_on_clear` is set) - **Near-miss series**: Appends one entry per FAILED report that moved the debounce counter without confirming, bounded per fault code and retained when the fault is cleared -- **Freeze-frame retention**: One compact JSON freeze-frame per fault code, retained across `clear_fault` (see below) +- **Freeze-frame retention**: One compact JSON freeze-frame per fault code, retained across `clear_fault` (see below). It also holds the evidence sent with FAILED reports, under `x-medkit-reported` - **Fault correlation** (optional): Root cause analysis with symptom muting and auto-clear - **Tamper-evident audit log** (optional): Append-only, hash-chained record of fault state transitions for verifiable history @@ -76,7 +76,9 @@ ros2 service call /fault_manager/clear_fault ros2_medkit_msgs/srv/ClearFault \ Snapshots capture topic data when faults are confirmed for post-mortem debugging. -Each confirm also writes a **freeze-frame**: a single compact JSON object mapping every captured topic to its value at confirmation time, keyed by fault code. It differs from per-topic snapshots in two ways: snapshots are deleted when the fault is cleared, while the freeze-frame is retained across `clear_fault` (once the snapshots are gone, `~/get_fault` serves the retained frame so the confirmed-state record stays available after acknowledgement); and a re-confirm that captures nothing (e.g. source publishers down) never overwrites an existing non-empty frame. A fault code with no configured capture set gets no freeze-frame row; a configured capture that samples nothing on its first run records an empty `{}` frame. Freeze-frame storage is bounded by the number of distinct fault codes (one row per code, replaced in place) and rows are never evicted. +Each confirm also writes a **freeze-frame**: a single compact JSON object mapping every captured topic to its value at confirmation time, keyed by fault code. It differs from per-topic snapshots in two ways: snapshots are deleted when the fault is cleared, while the freeze-frame is retained across `clear_fault` (once the snapshots are gone, `~/get_fault` serves the retained frame so the confirmed-state record stays available after acknowledgement); and a re-confirm that captures nothing (e.g. source publishers down) never overwrites an existing non-empty frame. A configured capture that samples nothing on its first run records an empty `{}` frame. + +The frame also holds **reporter evidence**: the key-value pairs a reporter sends in the `evidence` field of a FAILED `ReportFault`. They are kept under the key `x-medkit-reported`. Topic keys always start with `/`, so the two cannot clash. Evidence is written on every FAILED report that has any, even if the code has no capture config, so such a code can have a frame that holds only evidence. Only the last value of each key is kept. Limits per code: 32 keys, 128 bytes per key, 512 bytes per value, no limit on total size. An entry over a limit is dropped with a warning. Bytes that are not valid UTF-8 are stored as U+FFFD. A capture at confirmation keeps the evidence already in the frame. `~/get_fault` returns a frame with evidence even when per-topic snapshots exist, because the snapshots do not contain the evidence. Freeze-frame storage is bounded by the number of distinct fault codes (one row per code, replaced in place) and rows are never evicted. Under a fault storm, captures are bounded by a worker pool (`capture_pool_size`) draining a bounded queue (`capture_queue_depth`); excess captures are dropped per `capture_queue_full_policy` and logged (throttled). The pool is shared and is created when snapshots **or** rosbag is enabled, so these parameters bound both. `capture_pool_size` parallelizes freeze-frame snapshot capture only - rosbag stays single-writer regardless of pool size, and correlated faults confirming inside one post-roll window share a single recording. @@ -188,7 +190,8 @@ default, because they belong to the one confirmed occurrence rather than to the With `snapshots.retain_on_clear` on, `~/get_fault` keeps serving the freeze-frame alongside the retained snapshots, because it records the most recent confirmation while the snapshots may belong -to earlier ones. `~/get_snapshots` returns one entry per topic and serves the newest capture of +to earlier ones. Without it, the frame is returned once the snapshots are gone, or at any time if it holds +evidence. `~/get_snapshots` returns one entry per topic and serves the newest capture of that topic, whichever storage backend is in use. The bound is **per fault code, not per database**. Fault codes are unbounded in cardinality, so a 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 5a4806163..5cce27eab 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 @@ -20,8 +20,11 @@ #include #include #include +#include +#include #include "rclcpp/rclcpp.hpp" +#include "diagnostic_msgs/msg/key_value.hpp" #include "ros2_medkit_fault_manager/capture_thread_pool.hpp" #include "ros2_medkit_fault_manager/correlation/correlation_engine.hpp" #include "ros2_medkit_fault_manager/entity_threshold_resolver.hpp" @@ -191,6 +194,19 @@ class FaultManagerNode : public rclcpp::Node { /// Falls back to global config if no entity-specific overrides match. DebounceConfig resolve_config(const std::string & source_id) const; + /// Merge reporter-supplied evidence into a fault's freeze frame. + /// + /// Called for every FAILED report that carries any. Bounded per fault code; entries the + /// bounds reject are dropped and logged rather than truncated. The frame is read, merged + /// and written back, which is safe because the node's single-threaded executor serialises + /// this with every other callback that touches storage. + /// @param fault_code The code just reported. + /// @param evidence Key-value measurements from the report. + /// @param event_time Wall-clock time of the report. + void store_reported_evidence(const std::string & fault_code, + const std::vector & evidence, + const rclcpp::Time & event_time); + /// Create the tamper-evident audit log from parameters (nullptr if disabled). std::unique_ptr create_audit_log(); diff --git a/src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/fault_storage.hpp b/src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/fault_storage.hpp index 645978b1b..167d984b4 100644 --- a/src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/fault_storage.hpp +++ b/src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/fault_storage.hpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include "rclcpp/rclcpp.hpp" @@ -126,18 +127,60 @@ struct SnapshotData { int64_t capture_id{0}; }; -/// Compact freeze-frame captured when a fault confirms: a single JSON object mapping -/// each captured topic to its latest value at confirmation time. Unlike per-topic -/// snapshots, a freeze-frame is keyed by fault_code (one row per code) and is RETAINED -/// across clear_fault, so the confirmed-state record persists after acknowledgement. -/// A row exists only for fault codes with a configured capture set; a fault code with -/// no capture configured gets no row at all (lookup returns nullopt, never an empty {}). +/// Compact freeze-frame for a fault: one JSON object with the value of each captured topic +/// at confirmation, plus the evidence sent with FAILED reports (under kReportedEvidenceKey). +/// Unlike per-topic snapshots, it is keyed by fault_code (one row per code) and is kept +/// across clear_fault. +/// The row is written by the capture at confirmation, or by a FAILED report with evidence. +/// So a code with no capture config can still have a row that holds only evidence. A code +/// with neither has no row (lookup returns nullopt). struct FreezeFrameData { std::string fault_code; std::string data; ///< Compact JSON object: {"": , ...} int64_t captured_at_ns{0}; }; +/// Key inside the freeze-frame JSON object under which reporter-supplied evidence is kept. +/// +/// The frame's other keys are ROS topic names, which are always fully qualified and so +/// always start with '/'. This name cannot collide with one, which is what lets a reader +/// tell a value the reporter asserted from one the fault manager sampled off a topic. +inline constexpr const char * kReportedEvidenceKey = "x-medkit-reported"; + +/// Most evidence entries kept for one fault code. A reporter publishing at 10 Hz must not +/// be able to grow the fault store without limit, and a fault explained by more than this +/// many numbers is not explained by them. +inline constexpr size_t kMaxEvidenceEntries = 32; + +/// Longest evidence key kept, in bytes. A longer key is dropped together with its value. +inline constexpr size_t kMaxEvidenceKeyBytes = 128; + +/// Longest evidence value kept, in bytes (not characters). A longer value is dropped, not +/// cut short, because half a number is worse than no number. +inline constexpr size_t kMaxEvidenceValueBytes = 512; + +/// Merge reporter-supplied evidence into a freeze-frame JSON document. +/// +/// @param frame_json Existing frame ("" or invalid JSON is treated as an empty object). +/// @param evidence Key-value pairs from the report, in arrival order. +/// @param dropped Set to the number of entries rejected by the bounds above. +/// @return The frame with the evidence merged under kReportedEvidenceKey. Later reports +/// update the keys they name and leave the rest, so a fault accumulates what its +/// sources said rather than keeping only the last report's view. +std::string merge_reported_evidence(const std::string & frame_json, + const std::vector> & evidence, + size_t & dropped); + +/// Carry reporter evidence from an existing frame into a newly built one. +/// +/// A confirmation capture rebuilds the frame from the topics it sampled and would otherwise +/// drop evidence reported before it. Ordering decides which of the two writes lands last, so +/// the capture path must preserve rather than replace. +std::string preserve_reported_evidence(const std::string & frame_json, const std::string & previous_json); + +/// Whether a freeze-frame document carries reporter evidence. +bool has_reported_evidence(const std::string & frame_json); + /// Derive a recording's public identity from its bag path: the directory basename, /// `fault__`. Faults of one burst share a recording and therefore share /// this id. @@ -341,19 +384,20 @@ class FaultStorage { return 0; } - /// Store the compact freeze-frame captured for a fault (JSON dict of topic values). - /// Keyed by fault_code: a later capture for the same code replaces the frame. The frame - /// is retained across clear_fault so the confirmed-state record survives acknowledgement. + /// Store the compact freeze-frame for a fault (topic values, plus evidence under + /// kReportedEvidenceKey). Written by the capture at confirmation and by FAILED reports + /// with evidence. Keyed by fault_code: a later write for the same code replaces it. + /// The frame is retained across clear_fault so the confirmed-state record survives + /// acknowledgement. /// Storage is bounded by the number of distinct fault codes (one row per code, replaced /// in place); rows are never evicted. Faults themselves are never deleted (clear_fault /// only flips status), so there is currently no delete hook to tie eviction to. /// @param frame The freeze-frame to store virtual void store_freeze_frame(const FreezeFrameData & frame) = 0; - /// Get the freeze-frame captured for a fault, if any. + /// Get the freeze-frame stored for a fault, if any. /// @param fault_code The fault code to look up - /// @return The freeze-frame if one was captured, nullopt otherwise (including fault - /// codes with no capture configured, which never get a row) + /// @return The freeze-frame if a capture ran or evidence was reported, nullopt otherwise virtual std::optional get_freeze_frame(const std::string & fault_code) const = 0; /// Set the maximum number of near-miss entries retained per fault code. diff --git a/src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/snapshot_capture.hpp b/src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/snapshot_capture.hpp index 07ad5203b..99f34c139 100644 --- a/src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/snapshot_capture.hpp +++ b/src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/snapshot_capture.hpp @@ -171,8 +171,9 @@ class SnapshotCapture { /// the reporting source node's own published topics instead. A code explicitly /// listed in fault_specific or matched by a pattern never falls through - a /// present-but-empty topic list is a per-fault opt-out. Only when nothing - /// resolves does capture return early: no freeze_frames row is written - /// (no empty {} row) and FaultStorage::get_freeze_frame() returns nullopt for it. + /// resolves does capture return early without writing a freeze_frames row (no empty + /// {} row). The code can still have a row with only evidence, written by a FAILED + /// report. A capture that runs keeps the evidence already in the row. /// @param fault_code The fault code that was confirmed void capture(const std::string & fault_code); 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 0da318ecc..148caab55 100644 --- a/src/ros2_medkit_fault_manager/src/fault_manager_node.cpp +++ b/src/ros2_medkit_fault_manager/src/fault_manager_node.cpp @@ -878,6 +878,21 @@ void FaultManagerNode::handle_report_fault( response->accepted = true; + // Keep the numbers the reporter had when it decided the condition held. Written on every + // FAILED report, not only on the one that confirms: a fault that never confirms still owes + // its near-miss series an explanation, and a reporter that stops publishing after the first + // report would otherwise leave nothing behind. + // + // The fault is already stored at this point. If the connection is lost, only the evidence + // is missing, so log it and still publish the event. + if (request->event_type == ros2_medkit_msgs::srv::ReportFault::Request::EVENT_FAILED && !request->evidence.empty()) { + try { + store_reported_evidence(request->fault_code, request->evidence, event_time); + } catch (const FaultStorage::IgnorableConnectionException & e) { + RCLCPP_WARN(get_logger(), "Failed to connect with the fault storage server: %s", e.what()); + } + } + // Get updated fault state to publish event std::optional fault_after; try { @@ -1251,16 +1266,22 @@ void FaultManagerNode::handle_get_fault(const std::shared_ptr