Skip to content

Conversation

@mfaferek93
Copy link
Collaborator

Pull Request

Summary

When running with use_sim_time=true, fault timestamps were recorded in simulation time (seconds since sim start) instead of wall clock time. This caused incorrect timestamps like "1970-01-01" when UI interpreted simulation time as Unix epoch.

Changes:

  • Add get_wall_clock_time() helper using std::chrono::system_clock
  • Replace node->now() with wall clock for fault timestamps in:
    • FaultManagerNode: report_fault_event, publish_fault_event, auto-confirm
    • SnapshotCapture: on-demand and background cache timestamps
    • RosbagCapture: message timestamps and bag metadata
  • Add unit test to verify timestamps are valid wall clock values

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

@mfaferek93 mfaferek93 marked this pull request as ready for review February 1, 2026 18:02
Copilot AI review requested due to automatic review settings February 1, 2026 18:02
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 PR fixes a bug where the FaultManager was using simulation time (when use_sim_time=true) instead of wall clock time for fault timestamps, causing incorrect timestamps like "1970-01-01" when UI interpreted simulation time as Unix epoch.

Changes:

  • Added get_wall_clock_time() helper method to return wall clock time using std::chrono::system_clock
  • Replaced all fault-related timestamp recording from now() to wall clock time in FaultManagerNode, SnapshotCapture, and RosbagCapture
  • Added comprehensive unit test to verify timestamps are valid wall clock values

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/ros2_medkit_fault_manager/include/ros2_medkit_fault_manager/fault_manager_node.hpp Added declaration for get_wall_clock_time() helper method
src/ros2_medkit_fault_manager/src/fault_manager_node.cpp Added chrono include, implemented get_wall_clock_time(), and replaced now() calls with wall clock time for fault events and auto-confirmation
src/ros2_medkit_fault_manager/src/snapshot_capture.cpp Replaced node_->now() with direct wall clock time calculations for snapshot timestamps
src/ros2_medkit_fault_manager/src/rosbag_capture.cpp Replaced node_->now() with direct wall clock time calculations for rosbag metadata and message timestamps
src/ros2_medkit_fault_manager/test/test_fault_manager.cpp Added new test TimestampUsesWallClockNotSimTime to verify fault event timestamps use wall clock time

When running with use_sim_time=true, fault timestamps were recorded
in simulation time (seconds since sim start) instead of wall clock time.
This caused incorrect timestamps like "1970-01-01" when UI interpreted
simulation time as Unix epoch.

Changes:
- Add time_utils.hpp with get_wall_clock_ns() and get_wall_clock_time()
- Replace node->now() with wall clock utilities for fault timestamps in:
  - FaultManagerNode: report_fault_event, publish_fault_event, auto-confirm
  - SnapshotCapture: on-demand and background cache timestamps
  - RosbagCapture: message timestamps and bag metadata
- Add unit test to verify timestamps are valid wall clock values
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 5 out of 5 changed files in this pull request and generated no new comments.

@mfaferek93 mfaferek93 self-assigned this Feb 1, 2026
@mfaferek93 mfaferek93 requested a review from bburda February 1, 2026 18:32
@mfaferek93 mfaferek93 added the bug Something isn't working label Feb 1, 2026
@mfaferek93 mfaferek93 merged commit 0e521fb into main Feb 1, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] FaultManager uses simulation time instead of wall clock for fault timestamps

3 participants