Migrate XrTime to monotonic time in devicetracker API#352
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (25)
💤 Files with no reviewable changes (2)
📝 WalkthroughWalkthroughThis PR refactors the tracker timing abstraction to decouple from OpenXR-specific time representation. The Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Session as DeviceIOSession
participant Tracker as ITrackerImpl
participant OXR as OpenXR Layer
rect rgba(200, 150, 100, 0.5)
Note over App,OXR: Before (XrTime flow)
App->>Session: update()
Session->>Session: time_converter_.os_monotonic_now()
Session->>Session: XrTime current_time
Session->>Tracker: update(current_time)
Tracker->>OXR: xrLocateSpace(current_time)
end
rect rgba(100, 150, 200, 0.5)
Note over App,OXR: After (int64_t monotonic ns flow)
App->>Session: update()
Session->>Session: os_monotonic_now_ns()
Session->>Session: int64_t monotonic_ns
Session->>Tracker: update(monotonic_ns)
Tracker->>Tracker: convert_monotonic_ns_to_xrtime()
Tracker->>OXR: xrLocateSpace(xr_time)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
67967db to
1dc79d5
Compare
1dc79d5 to
01a35c9
Compare
There was a problem hiding this comment.
Pull request overview
This PR migrates the tracker implementation update loop from OpenXR XrTime to a runtime-agnostic monotonic nanosecond timestamp, keeping OpenXR-specific time conversion inside live tracker implementations. It also formalizes hand-joint indexing in the schema to match OpenXR’s XrHandJointEXT, and removes unintended OpenXR dependencies from higher-level/public tracker APIs.
Changes:
- Update
ITrackerImpl::updateto acceptint64_t monotonic_time_ns, and updateDeviceIOSession+ all live tracker impls to use a single monotonic clock read per frame with per-impl conversion toXrTime. - Add
HandJointenum to the FlatBuffers schema + Python bindings, and update C++/Python schema tests to referenceNUM_JOINTSrather than hard-coded26. - Enforce layering:
deviceio_base/deviceio_trackersno longer include/link OpenXR; OpenXR dependencies live indeviceio_session/live_trackers/ tests.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/core/schema/python/schema_init.py | Exports HandJoint through the Python schema package. |
| src/core/schema/python/hand_bindings.h | Adds pybind11 binding for HandJoint; updates bounds checking to use HandJoint_NUM_JOINTS. |
| src/core/schema/fbs/hand.fbs | Introduces HandJoint enum aligned with OpenXR joint ordinals; documents fixed-size semantics. |
| src/core/schema_tests/python/test_hand.py | Updates tests to use HandJoint.NUM_JOINTS and adds enum sentinel assertions. |
| src/core/schema_tests/cpp/test_hand.cpp | Adds OpenXR parity static_asserts and replaces hard-coded 26 with HandJoint_NUM_JOINTS. |
| src/core/schema_tests/cpp/CMakeLists.txt | Links OpenXR headers for parity checks in schema tests. |
| src/core/oxr/cpp/inc/oxr/oxr_session.hpp | Switches OpenXR handle deleter types to PFN_* to tolerate XR_NO_PROTOTYPES include order. |
| src/core/live_trackers/cpp/live_head_tracker_impl.hpp | Changes update signature to monotonic ns and stores last_update_time_ as int64_t. |
| src/core/live_trackers/cpp/live_head_tracker_impl.cpp | Converts monotonic ns → XrTime once per frame; writes MCAP timestamps with monotonic + xr_time. |
| src/core/live_trackers/cpp/live_hand_tracker_impl.hpp | Changes update signature/storage to monotonic ns; keeps helper using XrTime. |
| src/core/live_trackers/cpp/live_hand_tracker_impl.cpp | Converts monotonic ns → XrTime once; uses xr_time for OpenXR + MCAP. |
| src/core/live_trackers/cpp/live_generic_3axis_pedal_tracker_impl.hpp | Updates update signature to monotonic ns. |
| src/core/live_trackers/cpp/live_generic_3axis_pedal_tracker_impl.cpp | Renames unused update parameter to monotonic ns. |
| src/core/live_trackers/cpp/live_full_body_tracker_pico_impl.hpp | Updates update signature/storage to monotonic ns. |
| src/core/live_trackers/cpp/live_full_body_tracker_pico_impl.cpp | Converts monotonic ns → XrTime only after null-handle early return; uses xr_time for MCAP. |
| src/core/live_trackers/cpp/live_frame_metadata_tracker_oak_impl.hpp | Updates update signature to monotonic ns. |
| src/core/live_trackers/cpp/live_frame_metadata_tracker_oak_impl.cpp | Renames unused update parameter to monotonic ns. |
| src/core/live_trackers/cpp/live_controller_tracker_impl.hpp | Updates update signature/storage to monotonic ns. |
| src/core/live_trackers/cpp/live_controller_tracker_impl.cpp | Uses per-frame xr_time for xrLocateSpace and MCAP timestamps. |
| src/core/live_trackers/cpp/CMakeLists.txt | Ensures live_trackers links oxr::oxr_utils publicly (headers/types). |
| src/core/live_trackers/AGENTS.md | Adds package-specific rules for time conversion, MCAP timestamps, includes, and CMake linkage. |
| src/core/deviceio_trackers/python/tracker_bindings.cpp | Removes OpenXR include; sources joint constants from schema HandJoint_* instead of OpenXR. |
| src/core/deviceio_trackers/cpp/inc/deviceio_trackers/hand_tracker.hpp | Removes OpenXR-derived debug API (get_joint_name) from public tracker API. |
| src/core/deviceio_trackers/cpp/hand_tracker.cpp | Removes get_joint_name implementation and OpenXR-based joint name table. |
| src/core/deviceio_trackers/cpp/CMakeLists.txt | Drops OpenXR extension linking from deviceio_trackers to keep it OpenXR-free. |
| src/core/deviceio_trackers/AGENTS.md | Documents “no OpenXR dependency” boundary for deviceio_trackers. |
| src/core/deviceio_session/cpp/inc/deviceio_session/deviceio_session.hpp | Removes session-owned XrTimeConverter and OpenXR time include. |
| src/core/deviceio_session/cpp/deviceio_session.cpp | Uses os_monotonic_now_ns() and passes monotonic ns to impls. |
| src/core/deviceio_session/cpp/CMakeLists.txt | Propagates oxr::oxr_utils + extension headers to consumers of deviceio_session public headers. |
| src/core/deviceio_session/AGENTS.md | Documents the new update-loop contract and include requirements. |
| src/core/deviceio_base/cpp/inc/deviceio_base/tracker.hpp | Makes ITrackerImpl::update monotonic ns and removes OpenXR dependency from base interface. |
| src/core/deviceio_base/cpp/CMakeLists.txt | Removes OpenXR/oxr link dependencies from the INTERFACE base library. |
| src/core/deviceio_base/AGENTS.md | Documents runtime-agnostic API boundary for deviceio_base. |
| src/core/AGENTS.md | Adds src/core AGENTS index and preflight reminder. |
| AGENTS.md | Adds repo-wide agent preflight, pre-commit, and learning-loop requirements. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary by CodeRabbit
Documentation
Refactor