-
Notifications
You must be signed in to change notification settings - Fork 32
Keep more than one rosbag per fault, and stop losing the evidence around it #623
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
3784e75
18067e3
5e6420f
3acd26b
98db636
6d86d5d
22922bb
31a1cad
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 |
|---|---|---|
|
|
@@ -250,6 +250,7 @@ Capture continuous rosbag recordings around fault events. | |
| max_buffer_mb: 256 # Ring-buffer RAM cap | ||
| max_bag_size_mb: 50 # Max size per bag file | ||
| max_total_storage_mb: 500 # Max total storage | ||
| max_bags_per_fault: 1 # Recordings kept per fault code | ||
| auto_cleanup: true # Auto-delete old bags | ||
|
|
||
| .. list-table:: | ||
|
|
@@ -324,10 +325,33 @@ Capture continuous rosbag recordings around fault events. | |
| - Maximum total storage for all rosbags (MB). A recording shared by a | ||
| burst of faults counts once towards the total, and eviction removes a | ||
| whole burst's bag at a time (oldest first). | ||
| * - ``rosbag.max_bags_per_fault`` | ||
|
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 add a row for |
||
| - ``1`` | ||
| - How many recordings one fault code keeps. Past the cap the oldest is | ||
| unlinked, so the default reproduces the historical behaviour exactly: a | ||
| new recording replaces the previous one. ``0`` means unlimited, bounded | ||
| only by ``max_total_storage_mb``. ``3`` is a reasonable value for a fault | ||
|
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 also mention |
||
| that flaps - see the note below before raising it. | ||
| * - ``rosbag.auto_cleanup`` | ||
| - ``true`` | ||
| - Delete a fault's bag when the fault is cleared. A recording shared by a | ||
| burst survives until the last fault referencing it clears. | ||
| - Delete a fault's bags when the fault is cleared. A recording shared by a | ||
| burst survives until the last fault referencing it clears. Leave this | ||
| ``false`` when raising ``max_bags_per_fault``, or acknowledging a fault | ||
| discards the history that was just kept. | ||
|
|
||
| .. note:: | ||
|
|
||
| ``max_bags_per_fault`` is a **fairness** knob, not a depth knob. | ||
| ``max_total_storage_mb`` is the real disk bound and eviction across it is | ||
| global and oldest-first, so a fault that flaps often enough will consume the | ||
| budget and push out every other fault's black box. Raise the per-fault cap | ||
| when you need the history of a specific intermittent fault; raise the total | ||
| budget with it if other faults still need theirs. | ||
|
|
||
| The cap keeps the newest recordings and evicts the oldest. It deliberately | ||
| does not match ``snapshots.max_per_fault``, which rejects new snapshots once | ||
|
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. This note says |
||
| full: refusing a new recording would mean a technician standing next to a | ||
| machine faulting right now downloads a bag from three days ago. | ||
|
|
||
| .. _rosbag-recording-lifecycle: | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -79,6 +79,13 @@ default_topics: | |
| # `default_topics` from this YAML file. Rosbag settings must be configured | ||
| # via ROS 2 parameters (--ros-args -p snapshots.rosbag.*) or launch files. | ||
|
|
||
| # Keep a fault's value snapshots when it is acknowledged (default: false) | ||
| # Off is the 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 - evidence that no longer | ||
| # lines up. Growth stays bounded by max_per_fault either way. | ||
| retain_on_clear: false | ||
|
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. Nothing reads this key. |
||
|
|
||
| rosbag: | ||
| # Enable/disable rosbag capture (default: false) | ||
| # When disabled, only JSON snapshots are captured | ||
|
|
@@ -145,17 +152,32 @@ rosbag: | |
|
|
||
| # Storage path for bag files (default: "" = system temp directory) | ||
| # Empty string uses /tmp/rosbag_snapshots/ | ||
| # Bag files are named: {fault_code}_{timestamp}/ | ||
| # Bag files are named: fault_{fault_code}_{timestamp}/ and that directory name is | ||
| # the recording's public id - the last segment of its bulk-data URL. | ||
| storage_path: "" | ||
|
|
||
| # Maximum size per bag file in MB (default: 50) | ||
| # If a bag exceeds this size, it will be closed even if still recording | ||
| max_bag_size_mb: 50 | ||
|
|
||
| # Maximum total storage for all bag files in MB (default: 500) | ||
| # Oldest bags are deleted when this limit is exceeded | ||
| # Oldest bags are deleted when this limit is exceeded. This is the real disk bound; | ||
| # max_bags_per_fault below only decides how the budget is shared out. | ||
| max_total_storage_mb: 500 | ||
|
|
||
| # Recordings kept per fault code (default: 1, 0 = unlimited) | ||
| # A fault that keeps re-confirming leaves a trail of black boxes instead of only | ||
| # the latest one. Past the cap the fault's OLDEST recording is dropped, and the bag | ||
| # is deleted once no fault still references it (a burst shares one recording). | ||
| # | ||
| # 1 is the historical behaviour: each re-confirmation replaces the previous bag. | ||
| # 3 is a good starting point for an intermittent fault you are chasing. | ||
| # | ||
| # Think of this as fairness rather than depth: the disk is bounded by | ||
| # max_total_storage_mb either way, and a high value lets one flapping fault consume | ||
| # the budget and evict every other fault's recording. | ||
| max_bags_per_fault: 1 | ||
|
|
||
| # Maximum in-memory ring buffer size in MB (default: 256) | ||
| # Oldest buffered messages are dropped once the buffer exceeds this, so a broad | ||
| # subscribe set on a busy robot cannot grow memory without bound. | ||
|
|
||
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 update the listing section above as well? Line 1337 shows a UUID as
id, but it is a recording id now. Line 1343 showsx-medkit.fault_code, which is now thefault_codesarray. The text at lines 1352-1355 says "each fault gets its own descriptor with the full bag size", and this PR changes that to one descriptor per recording. Line 1367 still saysContent-Disposition: attachment; filename="FAULT_CODE.mcap", but the filename is<recording_id>.<format>now. The tutorial page was updated, so the two pages now describe the same response in different ways.