feat(rosbag): add rosbag capture with ring buffer for fault snapshots #134
+3,284
−8
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
There was a problem hiding this comment.
Pull request overview
Implements Issue #120 by adding optional rosbag2-based “time-window” snapshot recording (ring buffer + post-fault recording), persists bag metadata, and exposes bag retrieval via ROS 2 service + gateway REST download endpoint.
Changes:
- Added
GetRosbagservice and gateway/client plumbing to query rosbag metadata for a fault. - Implemented
RosbagCapturein fault manager with SQLite/in-memory metadata storage, lifecycle hooks, and storage limit enforcement. - Added REST download route (
/api/v1/faults/{fault_code}/snapshots/bag) plus docs and new unit/integration tests in the fault manager package.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 19 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ros2_medkit_msgs/srv/GetRosbag.srv | New ROS 2 service definition for rosbag metadata retrieval |
| src/ros2_medkit_msgs/CMakeLists.txt | Registers the new service for interface generation |
| src/ros2_medkit_gateway/src/http/rest_server.cpp | Adds REST route for downloading a fault’s rosbag |
| src/ros2_medkit_gateway/src/http/handlers/fault_handlers.cpp | Implements HTTP handler that fetches metadata and serves bag content |
| src/ros2_medkit_gateway/src/fault_manager.cpp | Adds gateway-side service client call for GetRosbag |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/handlers/fault_handlers.hpp | Declares new REST handler entrypoint |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/fault_manager.hpp | Declares new FaultManager API + client member |
| src/ros2_medkit_gateway/README.md | Documents the new download endpoint and rosbag capture behavior/config |
| src/ros2_medkit_fault_manager/test/test_rosbag_integration.test.py | New launch/integration test for rosbag capture + cleanup |
| src/ros2_medkit_fault_manager/test/test_rosbag_capture.cpp | New unit tests for RosbagCapture behavior |
| src/ros2_medkit_fault_manager/src/sqlite_fault_storage.cpp | Adds rosbag_files table and CRUD helpers for bag metadata |
| src/ros2_medkit_fault_manager/src/rosbag_capture.cpp | Implements ring-buffer recording + flush-to-bag logic |
| src/ros2_medkit_fault_manager/src/fault_storage.cpp | Adds in-memory rosbag metadata handling |
| src/ros2_medkit_fault_manager/src/fault_manager_node.cpp | Integrates rosbag capture into fault lifecycle + adds GetRosbag service |
| src/ros2_medkit_fault_manager/package.xml | Adds rosbag2 dependencies and test message deps |
| src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/sqlite_fault_storage.hpp | Declares new storage API for rosbag metadata |
| src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/snapshot_capture.hpp | Extends snapshot config with RosbagConfig |
| src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/rosbag_capture.hpp | New public class for ring-buffer rosbag capture |
| src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/fault_storage.hpp | Extends storage interface with rosbag metadata methods/types |
| src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/fault_manager_node.hpp | Wires rosbag capture + GetRosbag service declaration |
| src/ros2_medkit_fault_manager/config/snapshots.yaml | Adds rosbag-related configuration documentation/examples |
| src/ros2_medkit_fault_manager/CMakeLists.txt | Adds rosbag2 deps, builds new source, registers new tests |
| docs/tutorials/snapshots.rst | Adds tutorial documentation for rosbag time-window capture |
mfaferek93
force-pushed
the
feat/rosbag-capture-integration-tests
branch
2 times, most recently
from
f655382 to
fa1e55f
Compare
bburda
reviewed
Jan 24, 2026
mfaferek93
force-pushed
the
feat/rosbag-capture-integration-tests
branch
from
fc5f7d3 to
803d9f4
Compare
Implement Issue #120 - Rosbag Recording for Snapshots Features: - Ring buffer recording with configurable duration (duration_sec) - Post-fault recording (duration_after_sec) - Topic selection: 'all', 'config', or explicit comma-separated list - Lazy start mode (start on PREFAILED) or immediate start - Auto-cleanup of bag files when faults are cleared - Storage limits (max_bag_size_mb, max_total_storage_mb) - GetRosbag service to retrieve bag file metadata - REST API endpoint: GET /api/v1/faults/{code}/rosbag Implementation details: - RosbagCapture class with GenericSubscription for any message type - Discovery retry mechanism for reliable cross-process topic discovery - SQLite storage for rosbag file metadata - Integration with FaultManager lifecycle (PREFAILED/CONFIRMED/CLEARED) Tests: - Tests cover: bag creation, auto-cleanup, multiple faults, duration
- Fix clang_tidy performance warning: pass shared_ptr by const reference in message_callback lambda and function signature - Fix convert double quotes to single quotes - Fix simplify multi-line docstrings to single-line - Apply clang-format to all modified C++ files
- Update config/snapshots.yaml with full rosbag configuration section
including all parameters with descriptions and example configurations
- Add GET /faults/{code}/snapshots/bag endpoint documentation to gateway
README with examples, response formats, and configuration reference
- Include comparison table between JSON snapshots and rosbag capture
- Add comprehensive rosbag capture documentation with: - Feature comparison table (JSON vs rosbag) - Configuration options reference - REST API and ROS 2 service examples for downloading bags - Production and debugging configuration examples - Add note that MCAP format requires rosbag2_storage_mcap package - Add link to config/snapshots.yaml as configuration reference
Validates storage format during RosbagCapture initialization and provides clear error message if MCAP format is requested but plugin is unavailable. - Add validate_storage_format() method to check format validity - For MCAP, verify plugin availability by attempting to create a test bag - Throw runtime_error with installation instructions if MCAP unavailable - Add unit tests for invalid format and sqlite3 format validation
Security fixes: - Add fault_code validation (alphanumeric, underscore, hyphen, dot only) - Sanitize filename in Content-Disposition header to prevent injection - Prevent path traversal with '..' pattern check Functional fixes: - Fix orphaned bag files on re-confirm (delete old before overwrite) - Fix 'explicit' topics mode not working (was parsed as topic name) - Fix pruning during post-fault recording (could lose messages) - Implement streaming response for large rosbag downloads (64KB chunks) - Add UTC timestamp to download filename for unique identification Documentation: - Add note that YAML rosbag section is documentation-only - Fix filename format in README to match implementation - Document that REST API returns storage file, not full bag directory Tests: - Add @verifies REQ_INTEROP_088 markers for traceability - Add GetRosbag service client unit tests - Add gateway integration tests for /snapshots/bag endpoint - Add rosbag endpoint to API discovery
- Fix topics=auto mode treating "auto" as literal topic name - Fix post-fault recording failing with "directory already exists" - Fix mutex held during disk IO by copying buffer before writing - Replace unsafe malloc/free with RAII rcutils_uint8_array_init/fini - Return tar.gz archive for bag download (includes all segments) - Add fault_code validation returning HTTP 400 for invalid codes - Add missing includes (<cctype>, <algorithm>, <vector>) - Update README with correct download format and playback instructions - Update manual test plan with correct parameter names
mfaferek93
force-pushed
the
feat/rosbag-capture-integration-tests
branch
2 times, most recently
from
f4bde49 to
96c4e8c
Compare
- Remove issue number from snapshots.yaml config comment - Expand rosbag configuration descriptions in snapshots.rst - Add ASCII diagram explaining lazy_start mode behavior - Update download docs to reflect tar.gz archive format - Add happy path integration test for rosbag download
mfaferek93
force-pushed
the
feat/rosbag-capture-integration-tests
branch
from
96c4e8c to
97fc60a
Compare
The test_72_get_rosbag_happy_path test had dead code attempting to POST to a non-existent REST endpoint. The actual fault reporting is done via ROS2 service call using subprocess, so this dead code was causing AttributeError when the test was run.
mfaferek93
force-pushed
the
feat/rosbag-capture-integration-tests
branch
from
27f045c to
b8a9691
Compare
The test_72_get_rosbag_happy_path was failing because: 1. ros2 topic pub process died in CI (exit code 2) 2. ROS2 service call to report fault failed silently Fix: Use LIDAR_RANGE_INVALID fault which is auto-reported by lidar_sensor demo node (configured with invalid params: min_range=10 > max_range=5). Configure rosbag to capture /perception/lidar/scan topic instead of the non-existent /rosbag_test_topic. This eliminates dependency on external ros2 topic pub process.
bburda
approved these changes
Jan 25, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request
Summary
Implement Issue #120 - Rosbag Recording for Snapshots
Features:
Implementation details:
Tests:
Issue
Link the related issue (required):
Type
Testing
How was this tested / how should reviewers verify it?
Checklist