Skip to content

Conversation

@unitALG
Copy link
Contributor

@unitALG unitALG commented Dec 2, 2025

Description

Added documentation for the InputSystem.TrackedDevice class, particularly for the 4 properties. Several users had requested more information about these properties and their values, specifically the IntegerControl value in the trackingState property.

Testing status & QA

No code was altered; documentation changes only. However, I did run the formatting tool and regenerated the documentation locally to test it.

Overall Product Risks

Very low risk (documentation only).

Comments to reviewers

I've requested a technical review from Chris Massie, who kindly provided me with some background on this class.

Checklist

  • Commit message for squash-merge is prefixed with DOCS: ___.

@u-pr-agent
Copy link
Contributor

u-pr-agent bot commented Dec 2, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪

The PR consists entirely of documentation updates within a single file, requiring only a check for clarity, accuracy, and formatting.
🏅 Score: 92

The documentation is detailed and provides helpful context, but there is a likely copy-paste error in the rotation property description and a potentially confusing technical explanation for the tracked status property.
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Copy-paste Error

In the <summary> for the deviceRotation property, the text states: "this means the 'center' eye position." This appears to be copied from devicePosition and should likely refer to rotation or orientation instead of position.

/// Represents the rotation portion of the input device's primary
/// [pose](xref:input-system-tracked-input-devices#tracked-pose-driver). For an HMD
/// device, this means the "center" eye position. For XR controllers, it means the "grip" pose.
/// </summary>
Documentation Accuracy

The remarks for isTracked state that it "might contain a float that includes bits". Since isTracked is a ButtonControl (typically representing a normalized 0 or 1 value) and trackingState is the IntegerControl specifically designed for bitwise flags (as noted in lines 26-28), verify if this description correctly applies to isTracked or if it conflates the two properties.

/// This property can contain a simple bit (1 or 0), but for some Open XR devices, this might
/// contain a float that includes bits that can indicate whether the device's position is actual
/// or inferred from its last-known [pose](xref:openxr-input#pose-data) (OpenXR Plugin package).
  • Update review

🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr-agent

@u-pr-agent
Copy link
Contributor

u-pr-agent bot commented Dec 2, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Correct copy-paste error in documentation summary

The summary for deviceRotation incorrectly refers to the HMD's primary pose as
"center eye position", which likely resulted from a copy-paste error from
devicePosition. It should refer to the "center eye pose" to accurately define the
source of the rotation data.

Packages/com.unity.inputsystem/InputSystem/Devices/TrackedDevice.cs [74-76]

 /// Represents the rotation portion of the input device's primary
 /// [pose](xref:input-system-tracked-input-devices#tracked-pose-driver). For an HMD
-/// device, this means the "center" eye position. For XR controllers, it means the "grip" pose.
+/// device, this means the "center" eye pose. For XR controllers, it means the "grip" pose.
Suggestion importance[1-10]: 6

__

Why: The documentation for deviceRotation incorrectly refers to "center eye position" due to a likely copy-paste error from devicePosition. Changing it to "pose" accurately reflects that the rotation is derived from the tracked pose.

Low
  • More suggestions

🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr-agent

@codecov-github-com
Copy link

codecov-github-com bot commented Dec 2, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

@@           Coverage Diff            @@
##           develop    #2298   +/-   ##
========================================
  Coverage    77.95%   77.95%           
========================================
  Files          477      477           
  Lines        97416    97422    +6     
========================================
+ Hits         75943    75949    +6     
  Misses       21473    21473           
Flag Coverage Δ
inputsystem_MacOS_2022.3 5.54% <ø> (-0.01%) ⬇️
inputsystem_MacOS_2022.3_project 75.49% <ø> (-0.01%) ⬇️
inputsystem_MacOS_6000.0 5.32% <ø> (-0.01%) ⬇️
inputsystem_MacOS_6000.0_project 77.41% <ø> (-0.01%) ⬇️
inputsystem_MacOS_6000.2 5.32% <ø> (-0.01%) ⬇️
inputsystem_MacOS_6000.2_project 77.40% <ø> (-0.01%) ⬇️
inputsystem_MacOS_6000.3 5.32% <ø> (-0.01%) ⬇️
inputsystem_MacOS_6000.3_project 77.40% <ø> (-0.01%) ⬇️
inputsystem_MacOS_6000.4 5.33% <ø> (-0.01%) ⬇️
inputsystem_MacOS_6000.4_project 77.41% <ø> (-0.01%) ⬇️
inputsystem_MacOS_6000.5 5.33% <ø> (-0.01%) ⬇️
inputsystem_MacOS_6000.5_project 77.36% <ø> (-0.05%) ⬇️
inputsystem_Ubuntu_2022.3 5.54% <ø> (-0.01%) ⬇️
inputsystem_Ubuntu_2022.3_project 75.29% <ø> (-0.01%) ⬇️
inputsystem_Ubuntu_6000.0 5.32% <ø> (-0.01%) ⬇️
inputsystem_Ubuntu_6000.0_project 77.21% <ø> (-0.01%) ⬇️
inputsystem_Ubuntu_6000.2 5.32% <ø> (-0.01%) ⬇️
inputsystem_Ubuntu_6000.2_project 77.21% <ø> (-0.01%) ⬇️
inputsystem_Ubuntu_6000.3 5.32% <ø> (-0.01%) ⬇️
inputsystem_Ubuntu_6000.3_project 77.21% <ø> (-0.01%) ⬇️
inputsystem_Ubuntu_6000.4 5.33% <ø> (-0.01%) ⬇️
inputsystem_Ubuntu_6000.4_project 77.23% <ø> (+<0.01%) ⬆️
inputsystem_Ubuntu_6000.5 5.33% <ø> (-0.01%) ⬇️
inputsystem_Ubuntu_6000.5_project 77.22% <ø> (-0.01%) ⬇️
inputsystem_Windows_2022.3 5.54% <ø> (-0.01%) ⬇️
inputsystem_Windows_2022.3_project 75.62% <ø> (-0.01%) ⬇️
inputsystem_Windows_6000.0 5.32% <ø> (-0.01%) ⬇️
inputsystem_Windows_6000.0_project 77.53% <ø> (-0.01%) ⬇️
inputsystem_Windows_6000.2 5.32% <ø> (-0.01%) ⬇️
inputsystem_Windows_6000.2_project 77.53% <ø> (-0.01%) ⬇️
inputsystem_Windows_6000.3 5.32% <ø> (-0.01%) ⬇️
inputsystem_Windows_6000.3_project 77.53% <ø> (-0.01%) ⬇️
inputsystem_Windows_6000.4 5.33% <ø> (-0.01%) ⬇️
inputsystem_Windows_6000.4_project 77.54% <ø> (-0.01%) ⬇️
inputsystem_Windows_6000.5 5.33% <ø> (-0.01%) ⬇️
inputsystem_Windows_6000.5_project 77.54% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...y.inputsystem/InputSystem/Devices/TrackedDevice.cs 100.00% <ø> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

/// [Tracked Pose Driver](xref:input-system-tracked-input-devices#tracked-pose-driver)
/// component rather than being read directly from this class.
///
/// Refer to the [Starter Assets](https://docs.unity3d.com/Packages/com.unity.xr.interaction.toolkit@latest/index.html?subfolder=/manual/samples-spatial-keyboard.html)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Refer to the [Starter Assets](https://docs.unity3d.com/Packages/com.unity.xr.interaction.toolkit@latest/index.html?subfolder=/manual/samples-spatial-keyboard.html)
/// Refer to the [Starter Assets](https://docs.unity3d.com/Packages/com.unity.xr.interaction.toolkit@latest/index.html?subfolder=/manual/samples-starter-assets.html)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the com.unity.xr.interaction.toolkit package to the projectMetadata.json file and used the xref id

/// <remarks>
/// This property can contain a simple bit (1 or 0), but for some Open XR devices, this might
/// contain a float that includes bits that can indicate whether the device's position is actual
/// or inferred from its last-known [pose](xref:openxr-input#pose-data) (OpenXR Plugin package).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

xref:openxr-input#pose-data doesn't seem to link correctly when I built the documentation. I think you'll have to edit the package's projectMetadata.json to add an xref tag for "com.unity.xr.openxr" for this to work.

/// </summary>
/// <remarks>
/// This property can contain a simple bit (1 or 0), but for some Open XR devices, this might
/// contain a float that includes bits that can indicate whether the device's position is actual
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we ever actually set this field to a float, I think it's always 0 or 1. The com.unity.xr.openxr package has existing documentation for isTracked and it doesn't indicate that we treat it as anything other than a bool.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I'll update this. It is a little confusing because the TrackedDevice.isTracked property returns a ButtonControl not a bool.

/// or inferred from its last-known [pose](xref:openxr-input#pose-data) (OpenXR Plugin package).
///
/// For more information about how the float represents inferred position vs. actual position in
/// OpenXR devices, refer to [Reference Spaces](https://registry.khronos.org/OpenXR/specs/1.0/html/xrspec.html#reference-spaces)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be better to use the newer 1.1 version of the OpenXR spec: https://registry.khronos.org/OpenXR/specs/1.1/html/xrspec.html#spaces-reference-spaces

/// Indicates whether the input device is actively tracked or not.
/// </summary>
/// <remarks>
/// This property can contain a simple bit (1 or 0), but for some Open XR devices, this might
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// This property can contain a simple bit (1 or 0), but for some Open XR devices, this might
/// This property can contain a simple bit (1 or 0), but for some OpenXR devices, this might

OpenXR doesn't have a space.

- Fixed some links, clarified isTracked return in TrackedDevice.cs
- Added related XR packages to `xref` list in projectMetadata.json
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants