From 726cf0bda75fe6e6749f526cda8fbd47e1381c2a Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Mon, 7 Sep 2026 14:19:23 +0200 Subject: [PATCH 1/5] turtlebot3: stop the debounce profile from hiding the goal-status faults The debounce profile sets confirmation_threshold -3, which suits the no-progress check: it repeats every five seconds while the robot is stuck, so three reports arrive and the profile filters roughly fifteen seconds of it. That is the contrast with the storm profile. The other two faults do not repeat. NAVIGATION_GOAL_ABORTED and NAVIGATION_GOAL_CANCELED are raised from a status change, guarded so they fire once, and cleared by one PASSED when a later goal succeeds. Under -3 the counter reaches -1 and stops, so both stayed PREFAILED for good and the default CONFIRMED-only fault list never showed them. Measured against a running fault_manager: one FAILED leaves PREFAILED at -1, silence changes nothing, and the PASSED only moves the counter to 0. They now report under their own source_id and take their thresholds from a per-source file, -1 to confirm on the single event and 0 to heal on the single clear. The same measurement with that file in place gives CONFIRMED on the raise and HEALED on the recovery, while the no-progress fault still needs its three reports. healing_threshold was 3 with a comment saying three PASSED events heal. Healing costs healing_threshold minus the counter at recovery, so from -3 it would have taken six, and the detector sends one. It is 0. The storm profile set confirmation_threshold 0, which the fault manager rejects and replaces with -1 while logging a warning. It now says -1. --- .../config/entity_thresholds_debounce.yaml | 26 +++++++++++++++++++ .../config/medkit_params.yaml | 2 +- .../config/medkit_params_debounce.yaml | 18 ++++++++++--- .../docker-compose.debounce.yml | 2 ++ .../scripts/anomaly_detector.py | 21 ++++++++++++--- 5 files changed, 62 insertions(+), 7 deletions(-) create mode 100644 demos/turtlebot3_integration/config/entity_thresholds_debounce.yaml diff --git a/demos/turtlebot3_integration/config/entity_thresholds_debounce.yaml b/demos/turtlebot3_integration/config/entity_thresholds_debounce.yaml new file mode 100644 index 0000000..490432f --- /dev/null +++ b/demos/turtlebot3_integration/config/entity_thresholds_debounce.yaml @@ -0,0 +1,26 @@ +# Copyright 2026 bburda +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# Per-source debounce for the debounce demo. Keys match a source_id by longest +# prefix, so the entry below applies only to the goal-status reporter and the +# global thresholds still cover the no-progress one. + +/anomaly_detector/goal_status: + # A goal aborting or being cancelled is a single status change, so exactly one + # FAILED arrives and exactly one PASSED on the next success. -1 confirms on + # that one event and 0 heals on it. The global -3 would leave both codes in + # PREFAILED for good, which the default CONFIRMED-only fault list never shows. + confirmation_threshold: -1 + healing_enabled: true + healing_threshold: 0 diff --git a/demos/turtlebot3_integration/config/medkit_params.yaml b/demos/turtlebot3_integration/config/medkit_params.yaml index 975678d..10820a6 100644 --- a/demos/turtlebot3_integration/config/medkit_params.yaml +++ b/demos/turtlebot3_integration/config/medkit_params.yaml @@ -50,7 +50,7 @@ fault_manager: database_path: "/var/lib/ros2_medkit/faults.db" # Debounce configuration - confirmation_threshold: 0 # Immediate confirmation + confirmation_threshold: -1 # Immediate confirmation; 0 is rejected and falls back to this healing_enabled: false healing_threshold: 3 auto_confirm_after_sec: 0.0 diff --git a/demos/turtlebot3_integration/config/medkit_params_debounce.yaml b/demos/turtlebot3_integration/config/medkit_params_debounce.yaml index d75eab6..a6130a6 100644 --- a/demos/turtlebot3_integration/config/medkit_params_debounce.yaml +++ b/demos/turtlebot3_integration/config/medkit_params_debounce.yaml @@ -38,11 +38,23 @@ fault_manager: database_path: "/var/lib/ros2_medkit/faults.db" # === DEBOUNCE CONFIGURATION (Article 3) === - confirmation_threshold: -3 # Need 3 sustained FAILED events to confirm - healing_enabled: true # Auto-heal when problem resolves - healing_threshold: 3 # Need 3 PASSED events to heal + # -3 filters the no-progress reporter, which repeats every 5 s while the + # robot is stuck: three reports, so roughly 15 s of it, before an operator + # sees anything. That is the contrast with the storm profile. + confirmation_threshold: -3 + healing_enabled: true + # Healing costs healing_threshold minus the counter at recovery, so from -3 + # a value of 3 would need six PASSED events. The detector sends one per + # recovery, so anything above 0 never heals. + healing_threshold: 0 auto_confirm_after_sec: 0.0 # Disabled + # The goal-status faults fire once per status change, so the -3 above would + # hide them permanently. They report under their own source and take their + # own thresholds from here. + entity_thresholds: + config_file: "/root/demo_ws/src/turtlebot3_medkit_demo/config/entity_thresholds.yaml" + # Snapshot configuration (freeze frames) snapshots: enabled: true diff --git a/demos/turtlebot3_integration/docker-compose.debounce.yml b/demos/turtlebot3_integration/docker-compose.debounce.yml index 9585713..170f704 100644 --- a/demos/turtlebot3_integration/docker-compose.debounce.yml +++ b/demos/turtlebot3_integration/docker-compose.debounce.yml @@ -15,7 +15,9 @@ services: turtlebot3-demo: volumes: - ./config/medkit_params_debounce.yaml:/root/demo_ws/src/turtlebot3_medkit_demo/config/medkit_params.yaml:ro + - ./config/entity_thresholds_debounce.yaml:/root/demo_ws/src/turtlebot3_medkit_demo/config/entity_thresholds.yaml:ro turtlebot3-demo-nvidia: volumes: - ./config/medkit_params_debounce.yaml:/root/demo_ws/src/turtlebot3_medkit_demo/config/medkit_params.yaml:ro + - ./config/entity_thresholds_debounce.yaml:/root/demo_ws/src/turtlebot3_medkit_demo/config/entity_thresholds.yaml:ro diff --git a/demos/turtlebot3_integration/scripts/anomaly_detector.py b/demos/turtlebot3_integration/scripts/anomaly_detector.py index 13f848d..d436abf 100755 --- a/demos/turtlebot3_integration/scripts/anomaly_detector.py +++ b/demos/turtlebot3_integration/scripts/anomaly_detector.py @@ -46,6 +46,8 @@ SEVERITY_CRITICAL = 3 # Event types +GOAL_STATUS_SOURCE = '/goal_status' + EVENT_FAILED = 0 EVENT_PASSED = 1 @@ -153,6 +155,7 @@ def goal_status_callback(self, msg: GoalStatusArray): if status.status == GoalStatus.STATUS_ABORTED: self.report_fault( fault_code='NAVIGATION_GOAL_ABORTED', + source_suffix=GOAL_STATUS_SOURCE, severity=SEVERITY_ERROR, description=f'Navigation goal ABORTED - path planning or execution failed (goal: {goal_id[:8]})', event_type=EVENT_FAILED @@ -162,6 +165,7 @@ def goal_status_callback(self, msg: GoalStatusArray): elif status.status == GoalStatus.STATUS_CANCELED: self.report_fault( fault_code='NAVIGATION_GOAL_CANCELED', + source_suffix=GOAL_STATUS_SOURCE, severity=SEVERITY_WARN, description=f'Navigation goal CANCELED (goal: {goal_id[:8]})', event_type=EVENT_FAILED @@ -172,12 +176,14 @@ def goal_status_callback(self, msg: GoalStatusArray): # Clear navigation faults self.report_fault( fault_code='NAVIGATION_GOAL_ABORTED', + source_suffix=GOAL_STATUS_SOURCE, severity=SEVERITY_INFO, description='Navigation goal succeeded', event_type=EVENT_PASSED ) self.report_fault( fault_code='NAVIGATION_GOAL_CANCELED', + source_suffix=GOAL_STATUS_SOURCE, severity=SEVERITY_INFO, description='Navigation goal succeeded', event_type=EVENT_PASSED @@ -274,14 +280,23 @@ def check_timer_callback(self): self.last_no_progress_report_time = now self.get_logger().warn(f'No navigation progress for {time_since_progress:.1f}s') - def report_fault(self, fault_code: str, severity: int, description: str, event_type: int): - """Report a fault to FaultManager via service call.""" + def report_fault(self, fault_code: str, severity: int, description: str, event_type: int, + source_suffix: str = ''): + """Report a fault to FaultManager via service call. + + source_suffix separates reporters that behave differently. The + no-progress check repeats while the condition holds, so a count-based + debounce can filter it. The goal-status faults fire once on a status + change, so any threshold past the first event would hide them for good. + Reporting them under their own source lets the debounce configuration + give each the threshold that suits it. + """ request = ReportFault.Request() request.fault_code = fault_code request.event_type = event_type request.severity = severity request.description = description - request.source_id = self.get_fully_qualified_name() + request.source_id = self.get_fully_qualified_name() + source_suffix # Track active faults if event_type == EVENT_FAILED: From 0c619b78d376a2a547b0801afd78814a3013aafc Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Mon, 7 Sep 2026 15:22:32 +0200 Subject: [PATCH 2/5] ota_demo: poll for the environment_data snapshots Per-topic snapshots are written on the fault manager capture thread pool. They land a bit after the fault reports CONFIRMED, and CONFIRMED is the status the previous step waits for. The check read the fault detail once at that moment, so it sometimes saw an empty snapshot list while the capture was still running. In one run the check ran 124 ms before the fault manager logged "Captured 3/3 snapshots" for the same fault code. The rosbag check below already polls for the same reason. This makes the snapshot check poll too, with a 30s budget. --- tests/smoke_test_demo_narrative.sh | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/tests/smoke_test_demo_narrative.sh b/tests/smoke_test_demo_narrative.sh index 08d5491..a76a203 100755 --- a/tests/smoke_test_demo_narrative.sh +++ b/tests/smoke_test_demo_narrative.sh @@ -300,16 +300,23 @@ section "Fault detail: environment_data snapshot + MCAP rosbag capture" if api_get "/${NAV_ENTITY}/faults/${NAV_CODE}"; then pass "GET /${NAV_ENTITY}/faults/${NAV_CODE} returns 200" - if echo "$RESPONSE" | jq -e '(.environment_data.snapshots // []) | length >= 1' > /dev/null 2>&1; then - pass "fault detail has >=1 environment_data snapshot" - else - fail "fault detail has >=1 environment_data snapshot" \ - "got $(echo "$RESPONSE" | jq -c '.environment_data.snapshots // []' 2>/dev/null)" - fi else fail "GET /${NAV_ENTITY}/faults/${NAV_CODE} returns 200" "unexpected status code" fi +# Per-topic snapshots are captured on the fault manager's capture thread pool, +# so they are written slightly AFTER the fault reports CONFIRMED - the status +# the previous step polled for. Checking once races that write and reads an +# empty snapshot list from a fault whose capture is still in flight. +if poll_until "/${NAV_ENTITY}/faults/${NAV_CODE}" \ + '(.environment_data.snapshots // []) | length >= 1' \ + 30; then + pass "fault detail has >=1 environment_data snapshot" +else + fail "fault detail has >=1 environment_data snapshot" \ + "no snapshot after ~30s: $(echo "$RESPONSE" | jq -c '.environment_data.snapshots // []' 2>/dev/null)" +fi + # The MCAP rosbag is written ASYNCHRONOUSLY - the ring buffer is flushed on # confirm, then rosbag.duration_after_sec more seconds are recorded and the bag # is finalized + registered a few seconds AFTER the fault confirms. So poll for From 584c30bfe13f0dcd982c28e456691fa4773c9a0c Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Tue, 8 Sep 2026 11:22:25 +0200 Subject: [PATCH 3/5] turtlebot3: key the debounce override on the detector's namespaced node The per-source threshold key is matched as an anchored prefix of the reported source_id, and the detector reports its fully qualified node name. The launch file puts it in the "bridge" namespace, so the key needs /bridge in front of it; without that the resolver falls back to the global -3 and the goal-status faults stay PREFAILED, where the default fault list never shows them. Healing a confirmed fault costs healing_threshold minus the counter, and the counter is clamped at the confirmation threshold, so a confirmed fault needs a burst of PASSED events rather than one. The detector sent exactly one and then stopped, so NAVIGATION_NO_PROGRESS and LOCALIZATION_UNCERTAINTY could confirm and never heal. A recovery now answers with three, throttled on the same window as the FAILED side. Add a smoke test for the debounce profile and a CI job that runs it, plus the compose override entry for the CI service that job starts. Without that entry the demo comes up on the default profile while the test believes it is debouncing. --- .github/workflows/ci.yml | 35 +++ .../config/entity_thresholds_debounce.yaml | 9 +- .../config/medkit_params_debounce.yaml | 8 +- .../docker-compose.debounce.yml | 12 + .../scripts/anomaly_detector.py | 58 +++- tests/smoke_test_debounce.sh | 264 ++++++++++++++++++ 6 files changed, 367 insertions(+), 19 deletions(-) create mode 100755 tests/smoke_test_debounce.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e39e9ac..ad58227 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -94,6 +94,41 @@ jobs: working-directory: demos/turtlebot3_integration run: docker compose --profile ci down + # The debounce profile is a second configuration of the same demo, so it needs + # its own run: the default profile never loads the per-source threshold file, + # and a key that stops matching it fails silently by falling back to the + # global thresholds. + build-and-test-turtlebot-debounce: + runs-on: ubuntu-24.04 + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Build and start turtlebot3 demo in debounce mode + working-directory: demos/turtlebot3_integration + run: | + docker compose --profile ci \ + -f docker-compose.yml -f docker-compose.debounce.yml \ + up -d --build turtlebot3-demo-ci + + - name: Run debounce smoke tests + run: ./tests/smoke_test_debounce.sh + + - name: Show container logs on failure + if: failure() + working-directory: demos/turtlebot3_integration + run: | + docker compose --profile ci \ + -f docker-compose.yml -f docker-compose.debounce.yml \ + logs turtlebot3-demo-ci --tail=200 + + - name: Teardown + if: always() + working-directory: demos/turtlebot3_integration + run: | + docker compose --profile ci \ + -f docker-compose.yml -f docker-compose.debounce.yml down + build-and-test-moveit: runs-on: ubuntu-24.04 steps: diff --git a/demos/turtlebot3_integration/config/entity_thresholds_debounce.yaml b/demos/turtlebot3_integration/config/entity_thresholds_debounce.yaml index 490432f..149fc6c 100644 --- a/demos/turtlebot3_integration/config/entity_thresholds_debounce.yaml +++ b/demos/turtlebot3_integration/config/entity_thresholds_debounce.yaml @@ -15,8 +15,15 @@ # Per-source debounce for the debounce demo. Keys match a source_id by longest # prefix, so the entry below applies only to the goal-status reporter and the # global thresholds still cover the no-progress one. +# +# The prefix is anchored at the start of the source_id, which the detector builds +# as its fully qualified node name plus a suffix. demo.launch.py puts the node in +# the "bridge" namespace, so the name is /bridge/anomaly_detector and the leading +# /bridge is part of the key. Renaming the node or its namespace in the launch +# file breaks this match silently: the resolver falls back to the global +# thresholds and the goal-status faults go back to hiding in PREFAILED. -/anomaly_detector/goal_status: +/bridge/anomaly_detector/goal_status: # A goal aborting or being cancelled is a single status change, so exactly one # FAILED arrives and exactly one PASSED on the next success. -1 confirms on # that one event and 0 heals on it. The global -3 would leave both codes in diff --git a/demos/turtlebot3_integration/config/medkit_params_debounce.yaml b/demos/turtlebot3_integration/config/medkit_params_debounce.yaml index a6130a6..99337ac 100644 --- a/demos/turtlebot3_integration/config/medkit_params_debounce.yaml +++ b/demos/turtlebot3_integration/config/medkit_params_debounce.yaml @@ -43,9 +43,11 @@ fault_manager: # sees anything. That is the contrast with the storm profile. confirmation_threshold: -3 healing_enabled: true - # Healing costs healing_threshold minus the counter at recovery, so from -3 - # a value of 3 would need six PASSED events. The detector sends one per - # recovery, so anything above 0 never heals. + # A confirmed fault sits at the confirmation threshold, because the counter is + # clamped to the band, so healing costs healing_threshold minus -3 PASSED + # events: three at 0, six at 3. The detector answers a recovery with a burst + # of three, which is what makes 0 the value that heals and 3 the value that + # would not. healing_threshold: 0 auto_confirm_after_sec: 0.0 # Disabled diff --git a/demos/turtlebot3_integration/docker-compose.debounce.yml b/demos/turtlebot3_integration/docker-compose.debounce.yml index 170f704..ab96013 100644 --- a/demos/turtlebot3_integration/docker-compose.debounce.yml +++ b/demos/turtlebot3_integration/docker-compose.debounce.yml @@ -7,6 +7,13 @@ # DEBOUNCE (with filtering): # docker compose --profile cpu -f docker-compose.yml -f docker-compose.debounce.yml up -d # +# DEBOUNCE in CI (headless, driven by tests/smoke_test_debounce.sh): +# docker compose --profile ci -f docker-compose.yml -f docker-compose.debounce.yml up -d +# +# Every service that can run the demo needs an entry here. A service left out +# starts with the default profile while the caller believes it is debouncing, +# and every assertion about thresholds then measures the wrong configuration. +# # The override mounts the debounce config over the default one inside the container. # colcon build --symlink-install means the installed config points to the source, # so mounting over the source path works. @@ -21,3 +28,8 @@ services: volumes: - ./config/medkit_params_debounce.yaml:/root/demo_ws/src/turtlebot3_medkit_demo/config/medkit_params.yaml:ro - ./config/entity_thresholds_debounce.yaml:/root/demo_ws/src/turtlebot3_medkit_demo/config/entity_thresholds.yaml:ro + + turtlebot3-demo-ci: + volumes: + - ./config/medkit_params_debounce.yaml:/root/demo_ws/src/turtlebot3_medkit_demo/config/medkit_params.yaml:ro + - ./config/entity_thresholds_debounce.yaml:/root/demo_ws/src/turtlebot3_medkit_demo/config/entity_thresholds.yaml:ro diff --git a/demos/turtlebot3_integration/scripts/anomaly_detector.py b/demos/turtlebot3_integration/scripts/anomaly_detector.py index d436abf..08c9d3c 100755 --- a/demos/turtlebot3_integration/scripts/anomaly_detector.py +++ b/demos/turtlebot3_integration/scripts/anomaly_detector.py @@ -51,6 +51,15 @@ EVENT_FAILED = 0 EVENT_PASSED = 1 +# How many PASSED events one recovery sends. The fault manager heals a fault when +# its debounce counter climbs from confirmation_threshold back to +# healing_threshold, so the burst has to be at least the span between them: 3 for +# the debounce profile (-3 to 0), 1 for the goal-status source (-1 to 0). Sending +# a fixed 3 covers both. It is deliberately not unbounded - a fault manager with +# healing disabled keeps a fault CONFIRMED forever, and every further PASSED would +# publish another EVENT_UPDATED onto the fault event stream. +HEAL_PASSED_REPEATS = 3 + class AnomalyDetectorNode(Node): """Monitors navigation metrics and reports faults to FaultManager.""" @@ -123,8 +132,12 @@ def __init__(self): self.last_no_progress_report_time: Optional[Time] = None self.report_throttle_sec = 5.0 - # Track active faults for PASSED events - self.active_faults: set = set() + # Codes with PASSED reports still owed to the fault manager. + # A debounced fault heals only when its counter climbs from the + # confirmation threshold back to the healing one, which takes as many + # PASSED events as the span between them. One PASSED per recovery leaves + # a confirmed fault stuck, so each recovery owes a burst instead. + self.pending_heal_reports: dict = {} self.get_logger().info( f'AnomalyDetector started (cov_warn={self.covariance_warn_threshold}, ' @@ -222,7 +235,7 @@ def amcl_pose_callback(self, msg: PoseWithCovarianceStamped): self.last_covariance_report_time = now else: # Send PASSED to clear previous warnings - if 'LOCALIZATION_UNCERTAINTY' in self.active_faults: + if self.pending_heal_reports.get('LOCALIZATION_UNCERTAINTY'): self.report_fault( fault_code='LOCALIZATION_UNCERTAINTY', severity=SEVERITY_INFO, @@ -241,18 +254,34 @@ def odom_callback(self, msg: Odometry): distance = math.sqrt((x - last_x)**2 + (y - last_y)**2) if distance > self.min_progress_distance: - self.last_progress_time = self.get_clock().now() - # Clear no-progress fault if robot is moving - if 'NAVIGATION_NO_PROGRESS' in self.active_faults: + now = self.get_clock().now() + self.last_progress_time = now + # Clear no-progress fault if robot is moving. Odometry arrives far + # faster than the fault manager needs, and the burst has to be + # spread out, so this shares the FAILED side's throttle: one report + # per code per report_throttle_sec, whichever direction it goes. + if self.pending_heal_reports.get('NAVIGATION_NO_PROGRESS') and self._may_report_no_progress(now): self.report_fault( fault_code='NAVIGATION_NO_PROGRESS', severity=SEVERITY_INFO, description='Robot making progress', event_type=EVENT_PASSED ) + self.last_no_progress_report_time = now self.last_position = (x, y) + def _may_report_no_progress(self, now: Time) -> bool: + """True when the no-progress throttle window has elapsed. + + Shared by the FAILED and PASSED sides so the code is reported at most + once per report_throttle_sec regardless of direction. + """ + if self.last_no_progress_report_time is None: + return True + elapsed = (now - self.last_no_progress_report_time).nanoseconds / 1e9 + return elapsed > self.report_throttle_sec + def check_timer_callback(self): """Periodic check for no-progress condition.""" if not self.has_active_goal: @@ -265,12 +294,7 @@ def check_timer_callback(self): time_since_progress = (now - self.last_progress_time).nanoseconds / 1e9 if time_since_progress > self.no_progress_timeout_sec: - can_report = True - if self.last_no_progress_report_time is not None: - elapsed = (now - self.last_no_progress_report_time).nanoseconds / 1e9 - can_report = elapsed > self.report_throttle_sec - - if can_report: + if self._may_report_no_progress(now): self.report_fault( fault_code='NAVIGATION_NO_PROGRESS', severity=SEVERITY_WARN, @@ -298,11 +322,15 @@ def report_fault(self, fault_code: str, severity: int, description: str, event_t request.description = description request.source_id = self.get_fully_qualified_name() + source_suffix - # Track active faults + # A FAILED event (re-)arms the healing burst; each PASSED spends one of it. if event_type == EVENT_FAILED: - self.active_faults.add(fault_code) + self.pending_heal_reports[fault_code] = HEAL_PASSED_REPEATS else: - self.active_faults.discard(fault_code) + remaining = self.pending_heal_reports.get(fault_code, 0) - 1 + if remaining > 0: + self.pending_heal_reports[fault_code] = remaining + else: + self.pending_heal_reports.pop(fault_code, None) # Async service call future = self.fault_client.call_async(request) diff --git a/tests/smoke_test_debounce.sh b/tests/smoke_test_debounce.sh new file mode 100755 index 0000000..6ff49c7 --- /dev/null +++ b/tests/smoke_test_debounce.sh @@ -0,0 +1,264 @@ +#!/bin/bash +# Smoke tests for the turtlebot3_integration debounce profile +# +# Runs from the host against the containerized gateway, with the demo started +# under the debounce overlay: +# +# cd demos/turtlebot3_integration +# docker compose --profile ci -f docker-compose.yml -f docker-compose.debounce.yml up -d --build +# ./tests/smoke_test_debounce.sh +# +# What this pins: +# 1. The node name the per-source threshold file keys on is the name the +# detector actually registers. The key is an anchored prefix of the +# reported source_id, so a namespace change in the launch file makes it +# stop matching silently: the resolver falls back to the global thresholds +# and the goal-status faults sink into PREFAILED, where the default fault +# list never shows them. +# 2. The thresholds themselves. The goal-status source confirms on one FAILED +# and heals on one PASSED, while the base source still needs three FAILED. +# That contrast is the whole point of the debounce profile. +# 3. That a confirmed fault on the base source can still heal. Its counter is +# clamped at the confirmation threshold, so healing costs a burst of PASSED +# events, not one - which is why the detector answers a recovery with a +# burst and why healing_threshold is 0. +# +# Faults are injected through the gateway's own SOVD operation endpoint for +# /fault_manager/report_fault rather than by driving Gazebo. Navigation-driven +# injection is too timing-dependent for CI (see the header of +# smoke_test_turtlebot3.sh), and the contract under test is how the fault +# manager resolves thresholds per source, which a report exercises exactly as +# the detector does. + +GATEWAY_URL="${1:-http://localhost:8080}" +API_BASE="${GATEWAY_URL}/api/v1" + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=tests/smoke_lib.sh +source "${SCRIPT_DIR}/smoke_lib.sh" + +trap print_summary EXIT + +DEMO_CONTAINER="${DEMO_CONTAINER:-turtlebot3_medkit_demo_ci}" + +# Manifest entity ids for the two nodes this test talks about. +DETECTOR_APP="anomaly-detector" +FAULT_MANAGER_APP="medkit-fault-manager" + +# The launch file puts the detector in the "bridge" namespace. This name is +# duplicated in config/entity_thresholds_debounce.yaml (as the key prefix) and +# in scripts/anomaly_detector.py (as the reported source_id); this test is what +# keeps the three in step. +DETECTOR_NODE="/bridge/anomaly_detector" +GOAL_SOURCE="${DETECTOR_NODE}/goal_status" +BASE_SOURCE="${DETECTOR_NODE}" + +# Codes are unique per run so a re-run against a live container starts from no +# stored state, and so nothing collides with the codes the real detector reports +# while the test is in flight. +RUN_TAG="$(date +%s)" +GOAL_CODE="DEBOUNCE_SMOKE_GOAL_${RUN_TAG}" +BASE_CODE="DEBOUNCE_SMOKE_BASE_${RUN_TAG}" + +EVENT_FAILED=0 +EVENT_PASSED=1 +SEVERITY_INFO=0 +SEVERITY_WARN=1 + +# --- Helpers --- +# +# smoke_lib.sh sets `pipefail`, and `grep -q` closes the pipe on its first +# match. A producer with more output than a pipe buffer then dies of SIGPIPE and +# the whole pipeline reports failure even though the pattern WAS found - so a +# match on a long `docker logs` reads as a miss while identical code passes on +# short output. Every match below therefore reads a captured string with a +# here-string, never a pipeline. + +# Report one fault event the way the detector does: through the SOVD operation +# the gateway exposes for the fault manager's report_fault service. +# Usage: report_fault CODE EVENT_TYPE SEVERITY SOURCE_ID +report_fault() { + local code="$1" event="$2" severity="$3" source_id="$4" + local body http_code payload + payload=$(jq -nc --arg c "$code" --argjson e "$event" --argjson s "$severity" --arg src "$source_id" \ + '{parameters: {fault_code: $c, event_type: $e, severity: $s, description: "debounce smoke", source_id: $src}}') + body=$(curl -s -w "\n%{http_code}" -X POST \ + "${API_BASE}/apps/${FAULT_MANAGER_APP}/operations/report_fault/executions" \ + -H 'Content-Type: application/json' -d "$payload" 2>/dev/null) || true + http_code=$(tail -1 <<< "$body") + body=$(sed '$d' <<< "$body") + if [ "$http_code" != "200" ]; then + echo " report_fault ${code} event=${event} src=${source_id}: HTTP ${http_code}" >&2 + return 1 + fi + # A 200 that did not accept the report would leave every later assertion + # measuring a fault that was never recorded. + if ! jq -e '.parameters.accepted == true' <<< "$body" > /dev/null 2>&1; then + echo " report_fault ${code} not accepted: $(head -c 200 <<< "$body")" >&2 + return 1 + fi + return 0 +} + +# Report an event and register a pass/fail for the reporting itself. +# Usage: report_or_fail CODE EVENT_TYPE SEVERITY SOURCE_ID DESCRIPTION +report_or_fail() { + if report_fault "$1" "$2" "$3" "$4"; then + pass "$5" + else + fail "$5" "report_fault call did not succeed" + fi +} + +# Assert a fault reaches a raw debounce status within max_wait seconds. +# Usage: assert_status CODE EXPECTED_STATUS DESCRIPTION [max_wait] +assert_status() { + local code="$1" expected="$2" description="$3" max_wait="${4:-15}" + if poll_until "/faults?status=all" \ + ".items[] | select(.fault_code == \"${code}\" and .status == \"${expected}\")" \ + "$max_wait"; then + pass "$description" + else + local got + got=$(jq -r ".items[] | select(.fault_code == \"${code}\") | .status" <<< "$RESPONSE" 2>/dev/null) + fail "$description" "status is '${got:-}', expected '${expected}'" + fi +} + +# Assert a fault does NOT hold a status, giving it time to prove it would. +# Usage: refute_status CODE FORBIDDEN_STATUS DESCRIPTION [settle_seconds] +refute_status() { + local code="$1" forbidden="$2" description="$3" settle="${4:-5}" + sleep "$settle" + # An unreachable endpoint must not read as "the status was not reached" - + # that turns every outage into a silent pass for this whole class of check. + if ! api_get "/faults?status=all"; then + fail "$description" "GET /faults?status=all did not return 200" + return + fi + if jq -e ".items[] | select(.fault_code == \"${code}\" and .status == \"${forbidden}\")" <<< "$RESPONSE" > /dev/null 2>&1; then + fail "$description" "status reached '${forbidden}'" + elif ! jq -e ".items[] | select(.fault_code == \"${code}\")" <<< "$RESPONSE" > /dev/null 2>&1; then + # The fault vanishing entirely is not the same as it holding a different + # status, and would make the refutation vacuous. + fail "$description" "fault ${code} is absent from the list" + else + pass "$description" + fi +} + +# --- Preconditions --- + +wait_for_gateway 180 + +section "Debounce profile is the one that loaded" + +# A compose overlay that skips this service starts the demo on the default +# profile, and every threshold assertion below would then measure the wrong +# configuration while still looking plausible. The resolver announces what it +# read at startup, so check for that line rather than for the absence of an +# error - an absent error is also what a container that never got that far +# produces. +LOG_WAIT=0 +THRESHOLD_LOADED="" +while [ "$LOG_WAIT" -lt 60 ]; do + CONTAINER_LOGS=$(docker logs "$DEMO_CONTAINER" 2>&1 || true) + if grep -q "Loaded 1 entity threshold entries" <<< "$CONTAINER_LOGS"; then + THRESHOLD_LOADED=yes + break + fi + sleep 3 + LOG_WAIT=$((LOG_WAIT + 3)) +done + +if [ -n "$THRESHOLD_LOADED" ]; then + pass "fault manager loaded the per-source threshold file" +else + fail "fault manager loaded the per-source threshold file" \ + "no 'Loaded 1 entity threshold entries' in logs of ${DEMO_CONTAINER} after ${LOG_WAIT}s" +fi + +section "Detector node name matches the threshold key" + +# The threshold key is an anchored prefix of the source_id the detector reports, +# and the source_id starts with the node's fully qualified name. The gateway +# publishes that name, so this catches a launch-file rename before the +# behavioural assertions below turn into a confusing threshold failure. +if poll_until "/apps/${DETECTOR_APP}" \ + ".[\"x-medkit\"].ros2.node == \"${DETECTOR_NODE}\"" 120; then + pass "detector app reports ROS node ${DETECTOR_NODE}" +else + fail "detector app reports ROS node ${DETECTOR_NODE}" \ + "got $(jq -c '.["x-medkit"].ros2 // "no x-medkit.ros2"' <<< "$RESPONSE" 2>/dev/null)" +fi + +section "Fault reporting operation is available" + +if poll_until "/apps/${FAULT_MANAGER_APP}/operations" \ + ".items[] | select(.id == \"report_fault\")" 120; then + pass "report_fault is exposed as an operation on ${FAULT_MANAGER_APP}" +else + fail "report_fault is exposed as an operation on ${FAULT_MANAGER_APP}" \ + "operation not found; faults cannot be injected" + exit 1 +fi + +# --- Goal-status source: confirms on one event, heals on one --- + +section "Goal-status source (confirmation_threshold -1, healing_threshold 0)" + +report_or_fail "$GOAL_CODE" "$EVENT_FAILED" "$SEVERITY_WARN" "$GOAL_SOURCE" \ + "reported one FAILED as ${GOAL_SOURCE}" + +assert_status "$GOAL_CODE" "CONFIRMED" "one FAILED confirms the goal-status fault" + +if api_get "/faults?status=all" && \ + jq -e --arg src "$GOAL_SOURCE" \ + ".items[] | select(.fault_code == \"${GOAL_CODE}\") | .reporting_sources | index(\$src)" <<< "$RESPONSE" > /dev/null 2>&1; then + pass "fault records ${GOAL_SOURCE} as its reporting source" +else + fail "fault records ${GOAL_SOURCE} as its reporting source" \ + "got $(jq -c ".items[] | select(.fault_code == \"${GOAL_CODE}\") | .reporting_sources" <<< "$RESPONSE" 2>/dev/null)" +fi + +report_or_fail "$GOAL_CODE" "$EVENT_PASSED" "$SEVERITY_INFO" "$GOAL_SOURCE" \ + "reported one PASSED as ${GOAL_SOURCE}" + +assert_status "$GOAL_CODE" "HEALED" "one PASSED heals the goal-status fault" + +# --- Base source: still filtered by the global -3 --- + +section "Base source (global confirmation_threshold -3)" + +report_or_fail "$BASE_CODE" "$EVENT_FAILED" "$SEVERITY_WARN" "$BASE_SOURCE" \ + "reported one FAILED as ${BASE_SOURCE}" + +assert_status "$BASE_CODE" "PREFAILED" "one FAILED leaves the base fault PREFAILED" +refute_status "$BASE_CODE" "CONFIRMED" "one FAILED does not confirm the base fault" + +for _ in 1 2; do + report_or_fail "$BASE_CODE" "$EVENT_FAILED" "$SEVERITY_WARN" "$BASE_SOURCE" \ + "reported a further FAILED as ${BASE_SOURCE}" +done + +assert_status "$BASE_CODE" "CONFIRMED" "three FAILED confirm the base fault" + +# --- Base source: a confirmed fault still heals, but costs a burst --- + +section "Healing a confirmed base fault" + +report_or_fail "$BASE_CODE" "$EVENT_PASSED" "$SEVERITY_INFO" "$BASE_SOURCE" \ + "reported one PASSED as ${BASE_SOURCE}" +refute_status "$BASE_CODE" "HEALED" "one PASSED does not heal a confirmed base fault" + +for _ in 1 2; do + report_or_fail "$BASE_CODE" "$EVENT_PASSED" "$SEVERITY_INFO" "$BASE_SOURCE" \ + "reported a further PASSED as ${BASE_SOURCE}" +done + +assert_status "$BASE_CODE" "HEALED" "three PASSED heal the confirmed base fault" + +# --- Summary --- + +# print_summary runs via EXIT trap; exit code reflects test results +[ "$FAIL_COUNT" -eq 0 ] From 104ee956c511407ee2d255c829c20c8381ea5f4f Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Tue, 8 Sep 2026 19:47:40 +0200 Subject: [PATCH 4/5] turtlebot3: finish the healing burst from the periodic check The burst was sent from the odometry callback, so it only advanced while the robot kept reporting movement. A recovery that ended before the burst was spent, because the goal finished or the robot stopped, left the remaining PASSED events owed and the fault confirmed for good. The periodic check now sends them whenever the no-progress condition does not hold, which also covers the case where no goal is active and the fault cannot apply. Pin the configured -3 in the smoke test. The old sequence passed against -2 as well, because nothing was asserted between the second and third event in either direction. The test now asserts the intermediate states instead of only refuting one forbidden value, which also passed when the fault moved to some other unexpected status or left the list. Add assertions on the confirmed-only listing, and correct the comment claiming a PREFAILED fault is missing from the default fault list. The default filter includes PREFAILED alongside CONFIRMED, so such a fault is listed; what it never does is count as confirmed. --- .../config/entity_thresholds_debounce.yaml | 9 ++- .../config/medkit_params_debounce.yaml | 7 +- .../scripts/anomaly_detector.py | 54 +++++++------ tests/smoke_test_debounce.sh | 78 +++++++++++++------ 4 files changed, 98 insertions(+), 50 deletions(-) diff --git a/demos/turtlebot3_integration/config/entity_thresholds_debounce.yaml b/demos/turtlebot3_integration/config/entity_thresholds_debounce.yaml index 149fc6c..4f88916 100644 --- a/demos/turtlebot3_integration/config/entity_thresholds_debounce.yaml +++ b/demos/turtlebot3_integration/config/entity_thresholds_debounce.yaml @@ -21,13 +21,16 @@ # the "bridge" namespace, so the name is /bridge/anomaly_detector and the leading # /bridge is part of the key. Renaming the node or its namespace in the launch # file breaks this match silently: the resolver falls back to the global -# thresholds and the goal-status faults go back to hiding in PREFAILED. +# thresholds and the goal-status faults never leave PREFAILED. /bridge/anomaly_detector/goal_status: # A goal aborting or being cancelled is a single status change, so exactly one # FAILED arrives and exactly one PASSED on the next success. -1 confirms on - # that one event and 0 heals on it. The global -3 would leave both codes in - # PREFAILED for good, which the default CONFIRMED-only fault list never shows. + # that one event and 0 heals on it. Under the global -3 the counter stops at -1 + # and both codes stay PREFAILED for good. They are still listed, because the + # default fault filter includes PREFAILED alongside CONFIRMED, but they never + # count as confirmed: confirmedDTC stays 0, a confirmed-only query does not + # return them, and nothing that keys off confirmation runs. confirmation_threshold: -1 healing_enabled: true healing_threshold: 0 diff --git a/demos/turtlebot3_integration/config/medkit_params_debounce.yaml b/demos/turtlebot3_integration/config/medkit_params_debounce.yaml index 99337ac..c7028c2 100644 --- a/demos/turtlebot3_integration/config/medkit_params_debounce.yaml +++ b/demos/turtlebot3_integration/config/medkit_params_debounce.yaml @@ -1,8 +1,9 @@ # ros2_medkit gateway configuration for TurtleBot3 demo # ARTICLE 3 - DEBOUNCE VERSION -# Differences from default: -# confirmation_threshold: -3 (was 0) — requires 3 sustained FAILED events -# healing_enabled: true (was false) — auto-heal after PASSED events +# Differences from the default profile in medkit_params.yaml: +# confirmation_threshold: -3, so three sustained FAILED events are needed +# before an operator sees the fault, instead of the default -1 +# healing_enabled: true, so a fault clears itself once the condition is gone # # Node runs under /diagnostics namespace, so we need to match that here diagnostics: diff --git a/demos/turtlebot3_integration/scripts/anomaly_detector.py b/demos/turtlebot3_integration/scripts/anomaly_detector.py index 08c9d3c..919cd32 100755 --- a/demos/turtlebot3_integration/scripts/anomaly_detector.py +++ b/demos/turtlebot3_integration/scripts/anomaly_detector.py @@ -254,20 +254,10 @@ def odom_callback(self, msg: Odometry): distance = math.sqrt((x - last_x)**2 + (y - last_y)**2) if distance > self.min_progress_distance: - now = self.get_clock().now() - self.last_progress_time = now - # Clear no-progress fault if robot is moving. Odometry arrives far - # faster than the fault manager needs, and the burst has to be - # spread out, so this shares the FAILED side's throttle: one report - # per code per report_throttle_sec, whichever direction it goes. - if self.pending_heal_reports.get('NAVIGATION_NO_PROGRESS') and self._may_report_no_progress(now): - self.report_fault( - fault_code='NAVIGATION_NO_PROGRESS', - severity=SEVERITY_INFO, - description='Robot making progress', - event_type=EVENT_PASSED - ) - self.last_no_progress_report_time = now + # Movement only records progress. The PASSED events that clear the + # fault are sent from the periodic check, which keeps sending them + # after the robot stops. + self.last_progress_time = self.get_clock().now() self.last_position = (x, y) @@ -283,17 +273,20 @@ def _may_report_no_progress(self, now: Time) -> bool: return elapsed > self.report_throttle_sec def check_timer_callback(self): - """Periodic check for no-progress condition.""" - if not self.has_active_goal: - return + """Periodic check for the no-progress condition, in both directions.""" + now = self.get_clock().now() - if self.last_progress_time is None: - return + time_since_progress = None + if self.last_progress_time is not None: + time_since_progress = (now - self.last_progress_time).nanoseconds / 1e9 - now = self.get_clock().now() - time_since_progress = (now - self.last_progress_time).nanoseconds / 1e9 + stuck = ( + self.has_active_goal + and time_since_progress is not None + and time_since_progress > self.no_progress_timeout_sec + ) - if time_since_progress > self.no_progress_timeout_sec: + if stuck: if self._may_report_no_progress(now): self.report_fault( fault_code='NAVIGATION_NO_PROGRESS', @@ -303,6 +296,23 @@ def check_timer_callback(self): ) self.last_no_progress_report_time = now self.get_logger().warn(f'No navigation progress for {time_since_progress:.1f}s') + return + + # The condition does not hold: the robot is moving again, or no goal is + # active so the fault cannot apply. Spend whatever is left of the healing + # burst. Driving this from the timer rather than from odometry matters, + # because a recovery that ends before the burst is spent - the goal + # finishes, or the robot simply stops - would otherwise leave the fault + # confirmed for good, with the remaining PASSED events owed and nothing + # left to send them. + if self.pending_heal_reports.get('NAVIGATION_NO_PROGRESS') and self._may_report_no_progress(now): + self.report_fault( + fault_code='NAVIGATION_NO_PROGRESS', + severity=SEVERITY_INFO, + description='Robot making progress', + event_type=EVENT_PASSED + ) + self.last_no_progress_report_time = now def report_fault(self, fault_code: str, severity: int, description: str, event_type: int, source_suffix: str = ''): diff --git a/tests/smoke_test_debounce.sh b/tests/smoke_test_debounce.sh index 6ff49c7..39505bc 100755 --- a/tests/smoke_test_debounce.sh +++ b/tests/smoke_test_debounce.sh @@ -125,25 +125,47 @@ assert_status() { fi } -# Assert a fault does NOT hold a status, giving it time to prove it would. -# Usage: refute_status CODE FORBIDDEN_STATUS DESCRIPTION [settle_seconds] -refute_status() { - local code="$1" forbidden="$2" description="$3" settle="${4:-5}" +# Assert a fault still holds a status after a settle period. +# Stronger than refuting one forbidden value, which also passes when the fault +# moved to some other unexpected status, or vanished from the list entirely. +# Usage: assert_stable_status CODE EXPECTED_STATUS DESCRIPTION [settle_seconds] +assert_stable_status() { + local code="$1" expected="$2" description="$3" settle="${4:-5}" sleep "$settle" - # An unreachable endpoint must not read as "the status was not reached" - - # that turns every outage into a silent pass for this whole class of check. + # An unreachable endpoint must not read as a satisfied assertion - that turns + # every outage into a silent pass for this whole class of check. if ! api_get "/faults?status=all"; then fail "$description" "GET /faults?status=all did not return 200" return fi - if jq -e ".items[] | select(.fault_code == \"${code}\" and .status == \"${forbidden}\")" <<< "$RESPONSE" > /dev/null 2>&1; then - fail "$description" "status reached '${forbidden}'" - elif ! jq -e ".items[] | select(.fault_code == \"${code}\")" <<< "$RESPONSE" > /dev/null 2>&1; then - # The fault vanishing entirely is not the same as it holding a different - # status, and would make the refutation vacuous. - fail "$description" "fault ${code} is absent from the list" + local got + got=$(jq -r ".items[] | select(.fault_code == \"${code}\") | .status" <<< "$RESPONSE" 2>/dev/null) + if [ "$got" = "$expected" ]; then + pass "$description" else + fail "$description" "status is '${got:-}', expected '${expected}'" + fi +} + +# Assert whether a fault is in the confirmed-only listing, which is what an +# operator watching for active faults queries. The plain /faults list is not the +# discriminator here: its default filter includes PREFAILED as well as CONFIRMED, +# so a fault stuck in PREFAILED still appears there. +# Usage: assert_confirmed_listing CODE present|absent DESCRIPTION +assert_confirmed_listing() { + local code="$1" expected="$2" description="$3" + if ! api_get "/faults?status=confirmed"; then + fail "$description" "GET /faults?status=confirmed did not return 200" + return + fi + local found=absent + if jq -e ".items[] | select(.fault_code == \"${code}\")" <<< "$RESPONSE" > /dev/null 2>&1; then + found=present + fi + if [ "$found" = "$expected" ]; then pass "$description" + else + fail "$description" "fault is ${found} in the confirmed listing, expected ${expected}" fi } @@ -211,6 +233,8 @@ report_or_fail "$GOAL_CODE" "$EVENT_FAILED" "$SEVERITY_WARN" "$GOAL_SOURCE" \ "reported one FAILED as ${GOAL_SOURCE}" assert_status "$GOAL_CODE" "CONFIRMED" "one FAILED confirms the goal-status fault" +assert_confirmed_listing "$GOAL_CODE" present \ + "the goal-status fault shows up in the confirmed listing" if api_get "/faults?status=all" && \ jq -e --arg src "$GOAL_SOURCE" \ @@ -234,14 +258,22 @@ report_or_fail "$BASE_CODE" "$EVENT_FAILED" "$SEVERITY_WARN" "$BASE_SOURCE" \ "reported one FAILED as ${BASE_SOURCE}" assert_status "$BASE_CODE" "PREFAILED" "one FAILED leaves the base fault PREFAILED" -refute_status "$BASE_CODE" "CONFIRMED" "one FAILED does not confirm the base fault" +assert_confirmed_listing "$BASE_CODE" absent \ + "the base fault stays out of the confirmed listing after one FAILED" -for _ in 1 2; do - report_or_fail "$BASE_CODE" "$EVENT_FAILED" "$SEVERITY_WARN" "$BASE_SOURCE" \ - "reported a further FAILED as ${BASE_SOURCE}" -done +report_or_fail "$BASE_CODE" "$EVENT_FAILED" "$SEVERITY_WARN" "$BASE_SOURCE" \ + "reported a second FAILED as ${BASE_SOURCE}" + +# Without this the sequence would also pass against a threshold of -2, so it is +# what pins the configured -3. +assert_stable_status "$BASE_CODE" "PREFAILED" "two FAILED still do not confirm the base fault" + +report_or_fail "$BASE_CODE" "$EVENT_FAILED" "$SEVERITY_WARN" "$BASE_SOURCE" \ + "reported a third FAILED as ${BASE_SOURCE}" assert_status "$BASE_CODE" "CONFIRMED" "three FAILED confirm the base fault" +assert_confirmed_listing "$BASE_CODE" present \ + "the base fault reaches the confirmed listing on the third FAILED" # --- Base source: a confirmed fault still heals, but costs a burst --- @@ -249,12 +281,14 @@ section "Healing a confirmed base fault" report_or_fail "$BASE_CODE" "$EVENT_PASSED" "$SEVERITY_INFO" "$BASE_SOURCE" \ "reported one PASSED as ${BASE_SOURCE}" -refute_status "$BASE_CODE" "HEALED" "one PASSED does not heal a confirmed base fault" +assert_stable_status "$BASE_CODE" "CONFIRMED" "one PASSED leaves the confirmed base fault CONFIRMED" -for _ in 1 2; do - report_or_fail "$BASE_CODE" "$EVENT_PASSED" "$SEVERITY_INFO" "$BASE_SOURCE" \ - "reported a further PASSED as ${BASE_SOURCE}" -done +report_or_fail "$BASE_CODE" "$EVENT_PASSED" "$SEVERITY_INFO" "$BASE_SOURCE" \ + "reported a second PASSED as ${BASE_SOURCE}" +assert_stable_status "$BASE_CODE" "CONFIRMED" "two PASSED still do not heal the confirmed base fault" + +report_or_fail "$BASE_CODE" "$EVENT_PASSED" "$SEVERITY_INFO" "$BASE_SOURCE" \ + "reported a third PASSED as ${BASE_SOURCE}" assert_status "$BASE_CODE" "HEALED" "three PASSED heal the confirmed base fault" From c78f676c22891976e8ad798e849d52d98891f9d8 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Tue, 8 Sep 2026 21:32:18 +0200 Subject: [PATCH 5/5] turtlebot3: bound the goal-success clears and keep undelivered PASSED owed A successful goal reported PASSED for both goal-status codes whether or not either had ever been raised, so every successful goal cost two service calls for the life of the process. Under a profile with healing disabled a confirmed fault stays confirmed, so each of those also published a further update about a fault that was already over. The clears now go only to codes that still owe PASSED events. The healing burst is counted down when a PASSED is sent, not when it lands, so a report that never reached the fault manager spent part of the budget and left the counter short with nothing left to send. An undelivered or rejected PASSED now puts its share back. Add a static check that the debounce override covers every service that can run the demo, and run it in the debounce job. It also covers the cpu and nvidia services, which CI never starts. Say in the per-source threshold file that its healing fields repeat the global values on purpose, so that source keeps its behaviour if the global profile is retuned. --- .github/workflows/ci.yml | 6 ++ .../config/entity_thresholds_debounce.yaml | 3 + .../scripts/anomaly_detector.py | 42 ++++++++----- tests/check_debounce_overlay.sh | 61 +++++++++++++++++++ 4 files changed, 96 insertions(+), 16 deletions(-) create mode 100755 tests/check_debounce_overlay.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ad58227..a41ebaf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -104,6 +104,12 @@ jobs: - name: Checkout repository uses: actions/checkout@v4 + # Static check, so it also covers the cpu and nvidia services that CI + # never starts. A service missing from the override runs the default + # profile while the caller believes it is debouncing. + - name: Check the debounce override covers every demo service + run: ./tests/check_debounce_overlay.sh + - name: Build and start turtlebot3 demo in debounce mode working-directory: demos/turtlebot3_integration run: | diff --git a/demos/turtlebot3_integration/config/entity_thresholds_debounce.yaml b/demos/turtlebot3_integration/config/entity_thresholds_debounce.yaml index 4f88916..27adc0e 100644 --- a/demos/turtlebot3_integration/config/entity_thresholds_debounce.yaml +++ b/demos/turtlebot3_integration/config/entity_thresholds_debounce.yaml @@ -32,5 +32,8 @@ # count as confirmed: confirmedDTC stays 0, a confirmed-only query does not # return them, and nothing that keys off confirmation runs. confirmation_threshold: -1 + # These two repeat the global values rather than changing them. They are stated + # so this source keeps confirming and healing on a single event even if the + # global profile is retuned later; only confirmation_threshold differs today. healing_enabled: true healing_threshold: 0 diff --git a/demos/turtlebot3_integration/scripts/anomaly_detector.py b/demos/turtlebot3_integration/scripts/anomaly_detector.py index 919cd32..2142529 100755 --- a/demos/turtlebot3_integration/scripts/anomaly_detector.py +++ b/demos/turtlebot3_integration/scripts/anomaly_detector.py @@ -186,21 +186,21 @@ def goal_status_callback(self, msg: GoalStatusArray): self.get_logger().info(f'Navigation goal {goal_id[:8]} CANCELED') elif status.status == GoalStatus.STATUS_SUCCEEDED: - # Clear navigation faults - self.report_fault( - fault_code='NAVIGATION_GOAL_ABORTED', - source_suffix=GOAL_STATUS_SOURCE, - severity=SEVERITY_INFO, - description='Navigation goal succeeded', - event_type=EVENT_PASSED - ) - self.report_fault( - fault_code='NAVIGATION_GOAL_CANCELED', - source_suffix=GOAL_STATUS_SOURCE, - severity=SEVERITY_INFO, - description='Navigation goal succeeded', - event_type=EVENT_PASSED - ) + # Clear only the codes this detector still owes PASSED events + # for. Reporting both unconditionally sends two events per + # successful goal for the life of the process, and under a + # profile with healing disabled a confirmed fault stays + # confirmed, so each one publishes a further update about a + # fault that is already over. + for cleared_code in ('NAVIGATION_GOAL_ABORTED', 'NAVIGATION_GOAL_CANCELED'): + if self.pending_heal_reports.get(cleared_code): + self.report_fault( + fault_code=cleared_code, + source_suffix=GOAL_STATUS_SOURCE, + severity=SEVERITY_INFO, + description='Navigation goal succeeded', + event_type=EVENT_PASSED + ) def amcl_pose_callback(self, msg: PoseWithCovarianceStamped): """Monitor AMCL localization covariance.""" @@ -350,13 +350,23 @@ def report_fault(self, fault_code: str, severity: int, description: str, event_t def _handle_fault_response(self, future, fault_code: str, event_type: int): """Handle response from fault_manager service.""" + delivered = False try: response = future.result() - if not response.accepted: + delivered = response.accepted + if not delivered: self.get_logger().warn(f'Fault report rejected: {fault_code}') except Exception as e: self.get_logger().error(f'Failed to report fault {fault_code}: {e}') + # The burst is counted down when a PASSED is sent, not when it lands. One + # that never landed would otherwise spend part of the budget, leaving the + # fault manager short of its healing threshold with nothing left to send. + # Callbacks all run on the single spin thread, so this needs no lock. + if not delivered and event_type == EVENT_PASSED: + owed = min(self.pending_heal_reports.get(fault_code, 0) + 1, HEAL_PASSED_REPEATS) + self.pending_heal_reports[fault_code] = owed + def main(args=None): rclpy.init(args=args) diff --git a/tests/check_debounce_overlay.sh b/tests/check_debounce_overlay.sh new file mode 100755 index 0000000..f4b3c22 --- /dev/null +++ b/tests/check_debounce_overlay.sh @@ -0,0 +1,61 @@ +#!/bin/bash +# Check that the debounce override covers every service that can run the demo. +# +# A service missing from docker-compose.debounce.yml starts on the default +# profile while the caller believes it is debouncing. Nothing fails at that +# point: the demo comes up, the gateway answers, and every later assertion about +# thresholds quietly measures the wrong configuration. This is a static check on +# the merged compose file, so it needs no running container. +# +# Usage: ./tests/check_debounce_overlay.sh [demo_directory] + +set -euo pipefail + +DEMO_DIR="${1:-demos/turtlebot3_integration}" + +for cmd in docker jq; do + if ! command -v "$cmd" > /dev/null 2>&1; then + echo "Error: required command '$cmd' not found in PATH" >&2 + exit 1 + fi +done + +# Every service that launches demo.launch.py. Add a service here when you add one +# to docker-compose.yml. +DEMO_SERVICES=(turtlebot3-demo turtlebot3-demo-nvidia turtlebot3-demo-ci) + +REQUIRED_TARGETS=( + /root/demo_ws/src/turtlebot3_medkit_demo/config/medkit_params.yaml + /root/demo_ws/src/turtlebot3_medkit_demo/config/entity_thresholds.yaml +) + +MERGED=$(cd "$DEMO_DIR" && docker compose \ + --profile cpu --profile nvidia --profile ci \ + -f docker-compose.yml -f docker-compose.debounce.yml \ + config --format json) + +failures=0 + +for service in "${DEMO_SERVICES[@]}"; do + if ! jq -e --arg s "$service" '.services[$s]' > /dev/null 2>&1 <<< "$MERGED"; then + echo "FAIL ${service}: not present in the merged compose file" >&2 + failures=$((failures + 1)) + continue + fi + for target in "${REQUIRED_TARGETS[@]}"; do + if jq -e --arg s "$service" --arg t "$target" \ + '.services[$s].volumes // [] | map(.target) | index($t)' > /dev/null 2>&1 <<< "$MERGED"; then + echo "PASS ${service} mounts ${target##*/}" + else + echo "FAIL ${service} does not mount ${target}" >&2 + failures=$((failures + 1)) + fi + done +done + +if [ "$failures" -ne 0 ]; then + echo "${failures} missing debounce override mount(s)" >&2 + exit 1 +fi + +echo "All demo services carry the debounce override."