-
Notifications
You must be signed in to change notification settings - Fork 2
feat(#79): add SQLite persistent storage for fault manager #93
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
Conversation
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.
Pull request overview
This pull request adds SQLite persistent storage to the fault manager, replacing the in-memory-only storage with a configurable backend that can use either SQLite for persistence or memory for testing/temporary use. The implementation adds a new SqliteFaultStorage class with WAL mode for performance, nanosecond timestamp precision, and JSON serialization for reporting sources.
Key changes:
- Implements SQLite-backed persistent storage with automatic database initialization and directory creation
- Adds ROS parameters (
storage_typeanddatabase_path) to configure storage backend selection - Updates integration and unit tests to use in-memory storage for test isolation
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ros2_medkit_fault_manager/src/sqlite_fault_storage.cpp | New SQLite storage implementation with RAII statement wrapper, JSON array serialization, and thread-safe operations |
| src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/sqlite_fault_storage.hpp | Header defining SqliteFaultStorage class interface implementing FaultStorage |
| src/ros2_medkit_fault_manager/src/fault_manager_node.cpp | Adds storage factory method and parameters for configurable backend selection with directory auto-creation |
| src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/fault_manager_node.hpp | Updates node interface to expose storage type and use unique_ptr for polymorphic storage |
| src/ros2_medkit_fault_manager/test/test_sqlite_storage.cpp | Comprehensive unit tests for SQLite storage including persistence, timestamp precision, and JSON handling |
| src/ros2_medkit_fault_manager/test/test_integration.test.py | Updates integration tests to use memory storage for isolation |
| src/ros2_medkit_fault_manager/test/test_fault_manager.cpp | Updates FaultManagerNode tests to explicitly use memory storage |
| src/ros2_medkit_fault_manager/CMakeLists.txt | Adds SQLite3 dependency and links library, adds test_sqlite_storage target |
| src/ros2_medkit_fault_manager/package.xml | Adds libsqlite3-dev dependency |
| src/ros2_medkit_fault_manager/README.md | Documents new parameters and storage backend options with usage examples |
4b2ed65 to
a595fa4
Compare
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.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Replace in-memory storage with SQLite backend so faults survive node restarts. Storage type is configurable via ROS parameters. Changes: - Add SqliteFaultStorage class implementing FaultStorage interface - Use WAL mode for optimal write performance - Store timestamps with nanosecond precision - Store reporting_sources as JSON array - Add ROS parameters: storage_type (sqlite|memory), database_path - Default: SQLite at /var/lib/ros2_medkit/faults.db - Auto-create database directory if needed - Add comprehensive unit tests for SQLite storage (14 tests) - Update integration tests to use in-memory storage for isolation - Update README with new parameters documentation
a595fa4 to
118dcd9
Compare
Pull Request
Summary
Replace in-memory storage with SQLite backend so faults survive node
restarts. Storage type is configurable via ROS parameters.
Changes:
Issue
Link the related issue (required):
Type
Testing
How was this tested / how should reviewers verify it?
Checklist