Skip to content
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

Support VRTrackers for protocol rotations #566

Merged
merged 1 commit into from
Feb 9, 2023

Conversation

Erimelowo
Copy link
Member

  • Use getRawRotation for all trackers since they all support it.
  • Use VRTracker getRotation for getRotationReferenceAdjusted (that's adjusted to the HMD)
  • use VRTracker getRawRotation for addRotationIdentityAdjusted (that's normally adjusted to the identity Quaternion, which I believe is already how we get rotations from OpenVR)

@0forks
Copy link
Contributor

0forks commented Feb 9, 2023

The rotation field should be always passing values from getRawRotation, I agree.

I believe this PR is to fix rotation being (0, 0, 0) for OpenVR trackers when the raw rotation toggle is off in the GUI?
Reference/identity-adjusted rotations are supposed to be sent only for IMU trackers, sending them also for playspace-aligned trackers and HMD, which come through the feeder app, feels wrong. The protocol docs state that these fields are SlimeVR-specific, and affected by resets.

I intended that the protocol consumer (right now only the GUI?) would fall back to raw rotation if those quats are not sent. But as of now this is not the case, this will be corrected by #569.

@Erimelowo
Copy link
Member Author

sending them also for playspace-aligned trackers and HMD, which come through the feeder app, feels wrong.

VRTrackers are affected by resets now, so they should have their adjusted rotations sent just like IMU trackers.

@Eirenliel Eirenliel merged commit 5e6aefe into main Feb 9, 2023
@Eirenliel Eirenliel deleted the vrtracker-protocol-rotation branch February 9, 2023 15:35
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.

3 participants