Skip to content

Conversation

@mfaferek93
Copy link
Collaborator

@mfaferek93 mfaferek93 commented Dec 27, 2025

Pull Request

Summary

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

Issue

Link the related issue (required):


Type

  • Bug fix
  • New feature or tests
  • Breaking change
  • Documentation only

Testing

How was this tested / how should reviewers verify it?


Checklist

  • Breaking changes are clearly described (and announced in docs / changelog if needed)
  • Tests were added or updated if needed
  • Docs were updated if behavior or public API changed

Copilot AI review requested due to automatic review settings December 27, 2025 10:25
Copy link
Contributor

Copilot AI left a 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_type and database_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

@mfaferek93 mfaferek93 force-pushed the 79/SQLitePersistentStoragePluginImpl branch 3 times, most recently from 4b2ed65 to a595fa4 Compare December 27, 2025 11:45
@mfaferek93 mfaferek93 requested a review from Copilot December 27, 2025 11:59
Copy link
Contributor

Copilot AI left a 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
@mfaferek93 mfaferek93 force-pushed the 79/SQLitePersistentStoragePluginImpl branch from a595fa4 to 118dcd9 Compare December 27, 2025 12:12
@mfaferek93 mfaferek93 requested a review from bburda December 27, 2025 12:27
@mfaferek93 mfaferek93 merged commit c8c32b8 into main Dec 27, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Persistent Storage

3 participants