Repository navigation
diagnostic_bridge: keep the numbers behind a fault, and stop confirming every planned STALE #691
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With the fault manager default
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a note here and in the README in 237d92a. With |
||
| ``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.<name>`` | ||
| - ``-`` | ||
| - Severity for STALE statuses whose name starts with ``<name>``. Diagnostic names | ||
| are conventionally ``<component>: <check>``, 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 | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you remove this paragraph? It describes what the bridge did before this PR. The docs should describe only the current behaviour, and the PR description already has this. |
||
| 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 | ||
| -------------------------- | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After thinking about it again, 110 does not map to the SOVD spec either. It describes how the fault manager stores evidence. We keep the |
||
| :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 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you remove "and before this they were dropped"? It describes the behaviour before this PR, and the README should describe only the current one. |
||
| 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.<name>` | 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.<name>` | string | - | Severity for STALE statuses whose name starts with `<name>`. Longest matching prefix wins | | ||
|
|
||
| ### Example Configuration | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you start this warning with the current behaviour? For example: "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." After the release, the old table means nothing to a reader. The PR description already explains the doc fix.