Enhance collision detection and intervention triggers in AutonomyMetricsLogger#8
Enhance collision detection and intervention triggers in AutonomyMetricsLogger#8ibrahimhroob wants to merge 1 commit intomainfrom
Conversation
…icsLogger - Added parameters for collision moving threshold and required zero count. - Improved collision detection logic with hysteresis and debounce. - Updated intervention triggers to require autonomous mode and added activity field checks. - Enhanced YAML configuration for intervention on message with new parameters.
There was a problem hiding this comment.
Pull request overview
This PR refines AutonomyMetricsLogger’s collision detection and YAML-driven intervention triggers to reduce noise and improve robustness during autonomous operation.
Changes:
- Added new collision tuning parameters (
collision_moving_threshold,collision_zero_required_count) and implemented hysteresis + debounced falling-edge collision detection. - Enhanced
intervention_on_messagetriggers with autonomous-mode gating, rising-edge detection, activity-field evaluation, and cooldown support. - Updated configuration/docs to expose new parameters and recommended presets.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/autonomy_metrics/config/metrics_full.yaml |
Adds new intervention_on_message trigger parameters (mode requirement, rising-edge, cooldown, activity field, min threshold). |
src/autonomy_metrics/autonomy_metrics/metric_logger.py |
Implements collision hysteresis/debounce, intervention trigger activity logic + cooldowns, and manual-mode suppression controls. |
README.md |
Documents new parameters, updated collision logic, and refined YAML intervention trigger semantics/presets. |
Comments suppressed due to low confidence (1)
src/autonomy_metrics/autonomy_metrics/metric_logger.py:379
trigger_interventiondocstring says the event is always logged even in Manual mode, but the new logic returns early (suppresses logging) unlesslog_when_not_counted=Trueorforce_count=True. Please update the docstring to match the actual behavior (and/or rename the flag/adjust logic if the intent is still to always log).
"""
Log an intervention-type event.
- Only increments self.incidents when the robot is in Autonomous mode
(self.details['operation_mode'] == self.AUTO), unless force_count=True.
- Always logs the event (even in Manual) so history is preserved.
"""
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -561,7 +653,7 @@ def estop_role_callback(self, topic_name, msg): | |||
| self.get_logger().info(f"E-Stop state changed: {v}") | |||
| if v: | |||
| # E-stop counts as intervention | |||
There was a problem hiding this comment.
This comment says “E-stop counts as intervention”, but with the new trigger_intervention(..., log_when_not_counted=True) call it will not increment incidents in Manual mode. Please adjust the comment to reflect the updated counting/logging behavior (or change the call if E-stop should always count).
| # E-stop counts as intervention | |
| # E-stop triggers an intervention event; in Manual mode it will be logged but not counted as an incident |
| - intervention_on_message: triggers on any message | ||
| - intervention_on_change[field]: triggers when selected field changes |
There was a problem hiding this comment.
The handle_intervention_triggers docstring still states intervention_on_message triggers on “any message”, but the new implementation triggers based on message activity (activity_field, rising-edge, cooldown, etc.). Please update this docstring so it reflects the new semantics and avoids misleading future readers.
| - intervention_on_message: triggers on any message | |
| - intervention_on_change[field]: triggers when selected field changes | |
| - intervention_on_message: triggers on message *activity* derived from a | |
| configurable activity_field (with optional min_abs_value), using | |
| rising-edge and cooldown semantics; if activity cannot be evaluated, | |
| any received message is treated as activity. | |
| - intervention_on_change[field]: triggers when the selected field changes | |
| value. |
| return | ||
|
|
||
| if not collision_is_zero_now: | ||
| # In hysteresis band; keep previous state and wait for clearer signal. |
There was a problem hiding this comment.
In check_collision_condition, the hysteresis-band path (not collision_is_zero_now) returns without resetting collision_zero_streak. That allows zeros separated by “band” samples to be counted as consecutive near-zero samples, which defeats the intended debounce (collision_zero_required_count). Consider resetting collision_zero_streak (or otherwise breaking the streak) when entering the hysteresis band.
| # In hysteresis band; keep previous state and wait for clearer signal. | |
| # In hysteresis band; keep previous state and wait for clearer signal. | |
| self.collision_zero_streak = 0 |
| We only increment again if the cycle repeats: | ||
| velocity on /cmd_vel/collision -> zero -> velocity -> zero -> ... | ||
| """ |
There was a problem hiding this comment.
The function-level docstring above this block still describes the old falling-edge rule (velocity determined solely by collision_zero_threshold) and doesn’t mention the new hysteresis (collision_moving_threshold) + debounce (collision_zero_required_count) behavior. Please update the docstring so it matches the updated implementation.
What type of PR is this? (check all applicable)
Description
Related Tickets & Documents
QA Instructions, Screenshots, Recordings
Please replace this line with instructions on how to test your changes.
[optional] Are there any post deployment tasks we need to perform?