diff --git a/docs/api/rest.rst b/docs/api/rest.rst index 00aceb0aa..d52b019ba 100644 --- a/docs/api/rest.rst +++ b/docs/api/rest.rst @@ -1514,7 +1514,19 @@ 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 ``"x-medkit": {"reported": {"": {"": {"value": + "37", "reported_at": ""}}}}``: the key-value pairs reporters + sent with the FAILED reports of the current occurrence (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. + When per-topic entries are listed, the ``freeze_frame`` entry holds only + the evidence, since the topic values are already in those entries. Its + ``captured_at`` is then the time of the newest evidence. 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..7fb8368a5 100644 --- a/docs/config/diagnostic-bridge.rst +++ b/docs/config/diagnostic-bridge.rst @@ -37,7 +37,30 @@ The diagnostic bridge: - SEVERITY_ERROR (2) * - STALE - FAILED - - SEVERITY_ERROR (2) + - ``stale_severity``, SEVERITY_CRITICAL (3) by default + +.. warning:: + + STALE reports SEVERITY_CRITICAL by default, 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 +72,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 +97,67 @@ 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 per occurrence of the fault and serves them in the fault's freeze frame 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' + { + "/diagnostic_bridge": { + "nis": { "value": "0.03", "reported_at": "2026-10-19T07:54:03.120Z" }, + "rejected_fixes": { "value": "37", "reported_at": "2026-10-19T07:54:03.120Z" } + } + } + +The evidence is grouped by ``source_id``, then by key. Each key has its value and the time of +the last report that set it. 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. The freeze frame then holds only the evidence, because the topic values +are already in those entries. + +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. + +A new occurrence of the fault (a FAILED report after it was cleared) starts with no evidence. +With ``snapshots.retain_on_clear`` on, the fault manager keeps the evidence of older +occurrences too, up to ``snapshots.max_evidence_occurrences``, but serves only the current one. + +The frame's other keys are topic names sampled by the fault manager, always fully qualified +and so always starting with ``/``. ``x-medkit`` 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 occurrence of a fault code: at most 32 keys over all sources together, + 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. If + the dropped entry updates a key already stored, that key is removed, so an old value is not + shown as current. 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/config/fault-manager.rst b/docs/config/fault-manager.rst index e222b6aca..22520c11a 100644 --- a/docs/config/fault-manager.rst +++ b/docs/config/fault-manager.rst @@ -428,6 +428,7 @@ Basic Snapshot Settings config_file: "" # Path to YAML config file recapture_cooldown_sec: 60.0 # Min seconds between snapshot captures per fault max_per_fault: 10 # Max snapshots stored per fault code (0 = unlimited) + max_evidence_occurrences: 10 # With retain_on_clear, occurrences whose evidence is kept capture_pool_size: 2 # Max concurrent capture threads (>= 1) capture_queue_depth: 16 # Max pending captures before policy applies (>= 1) capture_queue_full_policy: reject_newest # reject_newest | drop_oldest @@ -479,7 +480,16 @@ Basic Snapshot Settings historical behaviour: clearing a fault deletes them. Turn it on together with ``rosbag.max_bags_per_fault``, or acknowledging leaves the fault holding recordings whose matching readings are gone. Independent of - ``max_per_fault``, which still bounds growth either way. + ``max_per_fault``, which still bounds growth either way. Also keeps the + reporter evidence of older occurrences of a fault (see + ``max_evidence_occurrences``); without it a new occurrence deletes the + evidence of the older ones. + * - ``snapshots.max_evidence_occurrences`` + - ``10`` + - With ``retain_on_clear`` on, how many occurrences per fault code keep + their reporter evidence. The oldest are dropped first. ``GetFault`` serves + only the current occurrence. Set to 0 for unlimited. Has no effect + without ``retain_on_clear``. * - ``snapshots.capture_pool_size`` - ``2`` - Max concurrent capture threads under a fault storm (>= 1). The capture pool is 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..44ecf4687 100644 --- a/src/ros2_medkit_diagnostic_bridge/README.md +++ b/src/ros2_medkit_diagnostic_bridge/README.md @@ -15,7 +15,72 @@ 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 per +occurrence of the fault and serves them in the fault's freeze frame under `x-medkit.reported`, +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": { + "/diagnostic_bridge": { + "nis": { "value": "0.03", "reported_at": "2026-10-08T09:15:02.123Z" }, + "rejected_fixes": { "value": "37", "reported_at": "2026-10-08T09:15:02.123Z" } + } + } + } + }, + "x-medkit": { + "topic": "", + "message_type": "", + "full_data": { "x-medkit": { "reported": { "...": "same as data" } } }, + "captured_at": "2026-10-08T09:15:02.123Z" + } + } + ] + } +} +``` + +The evidence is grouped by `source_id`, then by key, and each key has the time of the last +report that set it. When the code has captured topics, the frame holds only the evidence, +because the topic values are already in the per-topic entries. 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. A new occurrence (a FAILED report after a +clear) starts with no evidence. Limits per occurrence: 32 keys over all sources, 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. If it updates a stored key, that +key is removed. Bytes that are not valid UTF-8 are stored as U+FFFD. ## Quick Start @@ -39,6 +104,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..7af8b3542 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,9 @@ 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 and stores the evidence end @enduml @@ -131,8 +135,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 +153,7 @@ 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 ``values`` as evidence Configuration ------------- @@ -172,6 +177,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 +220,24 @@ 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 per occurrence and serves them in the fault's freeze frame under +``"x-medkit": {"reported": ...}``, grouped by ``source_id``. 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..b944a6426 --- /dev/null +++ b/src/ros2_medkit_diagnostic_bridge/test/test_stale_and_evidence.test.py @@ -0,0 +1,260 @@ +#!/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 + +# 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 served by GetFault as {key: value}, 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 + by_source = json.loads(frames[0].data).get('x-medkit', {}).get('reported') + if not by_source: + return None + # Every status comes from the bridge, so there is one source. + self.assertEqual(len(by_source), 1, by_source) + entries = next(iter(by_source.values())) + return {key: entry['value'] for key, entry in entries.items()} + + 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 5f886f77b..ea4a0623e 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) @@ -291,6 +297,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 35da4d358..8af5d0830 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 and per-fault_code 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). `~/get_fault` adds the evidence sent with FAILED reports to it, 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 @@ -79,6 +79,10 @@ 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. +**Reporter evidence** is the key-value pairs a reporter sends in the `evidence` field of a FAILED `ReportFault`. It is stored in its own table, not in the freeze-frame row, keyed by fault code, occurrence (`occurrence_count`), `source_id` and key. Only the report handler writes it, so it never races with a capture. `~/get_fault` serves the evidence of the current occurrence inside the served freeze-frame, as `"x-medkit": {"reported": {"": {"": {"value": "37", "reported_at": "..."}}}}`. Topic keys always start with `/`, so they cannot clash with `x-medkit`. A code with no capture config gets a served frame that holds only evidence; its `captured_at_ns` is then the time of the newest evidence. When per-topic snapshots are served, the frame holds only the evidence, since the topic values are already in the snapshots. + +Evidence is written on every FAILED report that has any. Only the last value of each key is kept, with the time of the report that set it. A new occurrence (a FAILED report after the fault was cleared) starts with no evidence, and the evidence of older occurrences is deleted, unless `snapshots.retain_on_clear` is on: then the evidence of the last `snapshots.max_evidence_occurrences` occurrences is kept, oldest dropped first. Limits per occurrence: 32 keys over all sources together, 128 bytes per key, 512 bytes per value, no limit on total size. An entry over a limit is dropped with a warning; if it updates a stored key, that key is removed, so an old value is not shown as current. Bytes that are not valid UTF-8 are stored as U+FFFD. + 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. That single-writer property also shapes what each fault of a burst gets. Nothing is buffered while a post-fault window is open (messages go straight into the open bag), and the flush that opened that bag already emptied the ring buffer, so a fault confirming right _after_ the window closes has no pre-fault history available. What a confirmation gets is decided by the buffer, so a fault arriving before any captured topic has published lands the same way. It gets a **post-fault-only bag**: its own recording holding just its `duration_after_sec` window, entered through the same post-roll state machine, so later faults of the burst attach to it normally. With `duration_after_sec: 0` there is no window to record into and such a fault gets no bag; if the bag cannot be written at all, no recording is opened and no metadata row is stored. The `duration_sec` on a stored bag is the span the recording was open rather than the configured windows, so a post-fault-only bag usually reports roughly `duration_after_sec` where a full one reports its buffered history too. It is a recording span, not a content span: a window during which nothing was published still reports the seconds it covered. It can also exceed `duration_sec + duration_after_sec`, because the ring buffer is pruned only when a message arrives - a topic that stops publishing keeps its last window buffered until the next confirmation flushes it, which is deliberate for a black box. See `docs/config/fault-manager.rst` for the full lifecycle. @@ -93,6 +97,7 @@ That single-writer property also shapes what each fault of a burst gets. Nothing | `snapshots.config_file` | string | `""` | Path to YAML config for `fault_specific` and `patterns` | | `snapshots.recapture_cooldown_sec` | double | `60.0` | Min seconds between captures for the same fault code. | | `snapshots.max_per_fault` | int | `10` | Max snapshots retained per fault. | +| `snapshots.max_evidence_occurrences` | int | `10` | With `retain_on_clear`, occurrences per fault code whose reporter evidence is kept, oldest dropped first (0 = unlimited). | | `snapshots.capture_pool_size` | int | `2` | Max concurrent capture threads under a fault storm (>= 1). Parallelizes snapshot capture only; rosbag stays single-writer. | | `snapshots.capture_queue_depth` | int | `16` | Max pending captures before the full-queue policy applies (>= 1). | | `snapshots.capture_queue_full_policy` | string | `reject_newest` | Policy when the queue is full: `reject_newest` or `drop_oldest`. | @@ -193,7 +198,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's topic values are returned once the snapshots are gone. +`~/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 f9f9871a2..3759674cb 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/fault_audit_log.hpp" @@ -203,6 +206,18 @@ class FaultManagerNode : public rclcpp::Node { void warn_on_conflicting_debounce_policy(const std::string & fault_code, const std::string & source_id, const DebounceConfig & resolved); + /// Store reporter-supplied evidence for the fault's current occurrence. + /// + /// Called for every FAILED report that carries any. Bounded per occurrence; entries the + /// bounds reject are dropped and logged rather than truncated. + /// @param fault The fault after the report, for its code and occurrence_count. + /// @param source_id Source of the report. + /// @param evidence Key-value measurements from the report. + /// @param event_time Wall-clock time of the report. + void store_reported_evidence(const ros2_medkit_msgs::msg::Fault & fault, const std::string & source_id, + 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 97ce6f8f1..c3cb5379d 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 @@ -22,6 +22,8 @@ #include #include #include +#include +#include #include #include "rclcpp/rclcpp.hpp" @@ -133,12 +135,85 @@ struct SnapshotData { /// 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 {}). +/// Reporter evidence is not stored here but in its own table (ReportedEvidence); GetFault +/// adds it to the frame it serves. struct FreezeFrameData { std::string fault_code; std::string data; ///< Compact JSON object: {"": , ...} int64_t captured_at_ns{0}; }; +/// One evidence entry: a key-value pair a reporter sent in the `evidence` field of a +/// FAILED report. +/// +/// Stored in its own table, keyed by (fault_code, occurrence, source_id, key). Only the +/// report handler writes it, so it never races with the capture thread, which writes the +/// freeze frame. The occurrence is the fault's occurrence_count after the report, so the +/// evidence of one occurrence stays apart from the next one, like the per-topic snapshots. +struct ReportedEvidence { + std::string fault_code; + uint32_t occurrence{0}; + std::string source_id; + std::string key; + std::string value; + int64_t reported_at_ns{0}; ///< Time of the last report that set this key +}; + +/// Vendor key in the served freeze-frame object. Evidence is served under +/// `"x-medkit": {"reported": {...}}`. The frame's other keys are ROS topic names, which +/// always start with '/', so the two cannot clash. +inline constexpr const char * kVendorExtensionKey = "x-medkit"; +inline constexpr const char * kReportedEvidenceKey = "reported"; + +/// Most evidence keys kept for one occurrence of a fault code, summed over all its +/// sources. 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; + +/// What one report does to the stored evidence of its (fault code, occurrence, source). +struct EvidenceUpdate { + std::vector> upserts; ///< key -> value to store + std::vector deletes; ///< keys to remove + size_t dropped{0}; ///< entries rejected by the bounds +}; + +/// Apply the evidence bounds to one report. Shared by every backend so they agree. +/// +/// Invalid UTF-8 in keys and values is replaced with U+FFFD first, and the bounds are +/// checked on the result, because that is what gets stored. An entry with an empty or +/// oversized key is dropped. An entry with an oversized value is dropped and its stored key +/// is deleted, so an old value is not shown as current. A new key is dropped once the +/// occurrence holds kMaxEvidenceEntries keys; a key already stored is always updated. +/// @param stored_keys Keys stored for this occurrence, as (source_id, key) +/// @param source_id Source of this report +/// @param evidence Key-value pairs from the report, in arrival order +EvidenceUpdate plan_evidence_update(const std::set> & stored_keys, + const std::string & source_id, + const std::vector> & evidence); + +/// Lowest occurrence whose evidence is kept when @p occurrence is the current one. Shared by +/// every backend. Occurrence numbers go up by one per occurrence, so keeping the last N +/// occurrences is a range. An occurrence without evidence still counts as one of the N. +/// @param retain Whether snapshots.retain_on_clear is on +/// @param max_occurrences Most occurrences kept with retention on (0 = unlimited) +uint32_t oldest_kept_evidence_occurrence(uint32_t occurrence, bool retain, size_t max_occurrences); + +/// Build the freeze-frame document GetFault serves. +/// +/// @param frame_json Stored topic values ("" when they are not served or there are none) +/// @param evidence Evidence of the current occurrence +/// @return The topic values plus `"x-medkit": {"reported": {"": {"": +/// {"value": "37", "reported_at": ""}}}}` when there is evidence +std::string build_served_freeze_frame(const std::string & frame_json, const std::vector & evidence); + /// 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. @@ -357,6 +432,32 @@ class FaultStorage { /// codes with no capture configured, which never get a row) virtual std::optional get_freeze_frame(const std::string & fault_code) const = 0; + /// Most occurrences per fault code whose evidence is kept while + /// set_retain_snapshots_on_clear is on (0 = unlimited). The oldest go first. Without + /// retention only the current occurrence is kept, whatever this says. + virtual void set_max_evidence_occurrences_per_fault(size_t /*max_count*/) { + } + + /// Store the evidence of one FAILED report, in one transaction. + /// + /// First removes the evidence of older occurrences of the code: all of them, or, with + /// set_retain_snapshots_on_clear on, those past set_max_evidence_occurrences_per_fault. + /// Then applies plan_evidence_update to the rows of (fault_code, occurrence). + /// @param fault_code The code just reported + /// @param occurrence The fault's occurrence_count after the report + /// @param source_id Source of the report + /// @param evidence Key-value pairs from the report, in arrival order + /// @param reported_at_ns Time of the report + /// @return How many entries the bounds dropped + virtual size_t store_reported_evidence(const std::string & fault_code, uint32_t occurrence, + const std::string & source_id, + const std::vector> & evidence, + int64_t reported_at_ns) = 0; + + /// Get the evidence of one occurrence of a fault, ordered by source_id, then key. + virtual std::vector get_reported_evidence(const std::string & fault_code, + uint32_t occurrence) const = 0; + /// Set the maximum number of near-miss entries retained per fault code. /// /// Entries beyond the bound are evicted oldest-first, including entries already stored when the @@ -524,6 +625,13 @@ class InMemoryFaultStorage : public FaultStorage { void store_freeze_frame(const FreezeFrameData & frame) override; std::optional get_freeze_frame(const std::string & fault_code) const override; + void set_max_evidence_occurrences_per_fault(size_t max_count) override; + size_t store_reported_evidence(const std::string & fault_code, uint32_t occurrence, const std::string & source_id, + const std::vector> & evidence, + int64_t reported_at_ns) override; + std::vector get_reported_evidence(const std::string & fault_code, + uint32_t occurrence) const override; + void set_max_rosbags_per_fault(size_t max_count) override; size_t set_max_near_misses_per_fault(size_t max_count) override; @@ -564,6 +672,9 @@ class InMemoryFaultStorage : public FaultStorage { std::map faults_; std::vector snapshots_; std::map freeze_frames_; ///< fault_code -> freeze-frame (retained across clear) + /// (fault_code, occurrence, source_id, key) -> entry, the same key as the SQL tables + std::map, ReportedEvidence> reported_evidence_; + size_t max_evidence_occurrences_per_fault_{0}; ///< 0 = unlimited /// One entry per LINK, mirroring the flat SQLite table rather than a map keyed by /// fault code - a fault holds several recordings now, and a recording several /// faults. diff --git a/src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/postgres_fault_storage.hpp b/src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/postgres_fault_storage.hpp index d27a256b0..ec1bdf9b6 100644 --- a/src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/postgres_fault_storage.hpp +++ b/src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/postgres_fault_storage.hpp @@ -90,6 +90,14 @@ class PgFaultStorage : public FaultStorage { void store_freeze_frame(const FreezeFrameData & frame) override; std::optional get_freeze_frame(const std::string & fault_code) const override; + + void set_max_evidence_occurrences_per_fault(size_t max_count) override; + size_t store_reported_evidence(const std::string & fault_code, uint32_t occurrence, const std::string & source_id, + const std::vector> & evidence, + int64_t reported_at_ns) override; + std::vector get_reported_evidence(const std::string & fault_code, + uint32_t occurrence) const override; + size_t set_max_near_misses_per_fault(size_t max_count) override; std::vector get_near_misses(const std::string & fault_code) const override; @@ -193,6 +201,7 @@ class PgFaultStorage : public FaultStorage { size_t max_snapshots_per_fault_{0}; ///< 0 = unlimited size_t max_near_misses_per_fault_{0}; ///< 0 = unlimited bool retain_snapshots_on_clear_{false}; + size_t max_evidence_occurrences_per_fault_{0}; ///< 0 = unlimited /// Defaults to 1, the pre-#620 behaviour: a new recording replaces the old one. /// 0 = unlimited, bounded only by max_total_storage_mb. size_t max_rosbags_per_fault_{1}; diff --git a/src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/sqlite_fault_storage.hpp b/src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/sqlite_fault_storage.hpp index a5a1fab9f..c9d3fd8e9 100644 --- a/src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/sqlite_fault_storage.hpp +++ b/src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/sqlite_fault_storage.hpp @@ -78,6 +78,13 @@ class SqliteFaultStorage : public FaultStorage { void store_freeze_frame(const FreezeFrameData & frame) override; std::optional get_freeze_frame(const std::string & fault_code) const override; + void set_max_evidence_occurrences_per_fault(size_t max_count) override; + size_t store_reported_evidence(const std::string & fault_code, uint32_t occurrence, const std::string & source_id, + const std::vector> & evidence, + int64_t reported_at_ns) override; + std::vector get_reported_evidence(const std::string & fault_code, + uint32_t occurrence) const override; + size_t set_max_near_misses_per_fault(size_t max_count) override; std::vector get_near_misses(const std::string & fault_code) const override; @@ -169,6 +176,7 @@ class SqliteFaultStorage : public FaultStorage { size_t max_snapshots_per_fault_{0}; ///< 0 = unlimited size_t max_near_misses_per_fault_{0}; ///< 0 = unlimited bool retain_snapshots_on_clear_{false}; + size_t max_evidence_occurrences_per_fault_{0}; ///< 0 = unlimited /// Defaults to 1, the pre-#620 behaviour: a new recording replaces the old one. /// 0 = unlimited, bounded only by max_total_storage_mb. size_t max_rosbags_per_fault_{1}; 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 d9393090a..c90d333a5 100644 --- a/src/ros2_medkit_fault_manager/src/fault_manager_node.cpp +++ b/src/ros2_medkit_fault_manager/src/fault_manager_node.cpp @@ -223,6 +223,16 @@ FaultManagerNode::FaultManagerNode(const rclcpp::NodeOptions & options) : Node(" max_snapshots = 0; } + // With retain_on_clear on, reporter evidence of older occurrences is kept too. This bounds + // how many occurrences per fault code keep it, oldest dropped first. Without retention only + // the current occurrence keeps evidence. + auto max_evidence_occurrences = declare_parameter("snapshots.max_evidence_occurrences", 10); + if (max_evidence_occurrences < 0) { + RCLCPP_WARN(get_logger(), "snapshots.max_evidence_occurrences should be >= 0, got %ld. Disabling limit.", + max_evidence_occurrences); + max_evidence_occurrences = 0; + } + // Near-miss retention. The series is what any later look at "how often did this code approach // confirmation" reads, so it is bounded rather than unlimited by default: an appliance runs for // weeks and the rows are cheap but not free. 0 means unlimited and lets the file grow with the @@ -249,6 +259,7 @@ FaultManagerNode::FaultManagerNode(const rclcpp::NodeOptions & options) : Node(" // Independent of the cap: max_per_fault 0 means unlimited, and retention across // acknowledgement is a separate question from how many sets are kept. storage_->set_retain_snapshots_on_clear(retain_snapshots_on_clear); + storage_->set_max_evidence_occurrences_per_fault(static_cast(max_evidence_occurrences)); // Apply near-miss retention bound to storage (0 = unlimited). Applying it also trims a series // that a previous run left over the bound, which deletes history for good, so say when it does. @@ -935,6 +946,23 @@ void FaultManagerNode::handle_report_fault( RCLCPP_WARN(get_logger(), "Failed to connect with the fault storage server: %s", e.what()); return; } + + // 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 (fault_after && request->event_type == ros2_medkit_msgs::srv::ReportFault::Request::EVENT_FAILED && + !request->evidence.empty()) { + try { + store_reported_evidence(*fault_after, request->source_id, request->evidence, event_time); + } catch (const FaultStorage::IgnorableConnectionException & e) { + RCLCPP_WARN(get_logger(), "Failed to connect with the fault storage server: %s", e.what()); + } + } + if (fault_after) { // Only for codes the store holds: a PASSED for an unknown code writes nothing, so it must // not add an entry here either (the diagnostic bridge sends PASSED for every OK status). @@ -1304,25 +1332,39 @@ void FaultManagerNode::handle_get_fault(const std::shared_ptr