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

Compass is Moving in Opposite Direction of Phone #42

Merged
merged 4 commits into from
Aug 31, 2016
Merged

Conversation

ghost
Copy link

@ghost ghost commented Aug 31, 2016

@ghost ghost assigned cdzombak Aug 31, 2016
@cdzombak
Copy link
Contributor

assigning @jaredsinclair as a second reviewer.

@jaredsinclair
Copy link
Contributor

Okay, I've made a few minor modifications. The changes @JulietaFernandez made were spot-on. The changes I made:

  1. Change the default reference angle to 0. I didn't realize until now that the compass view transform in NYTVideo was adding M_PI to the received value to compensate for NYT360Video's non-zero reference angle. My change makes the implementation for the host app much simpler. Note: this means NYTVideo will need to be updated after picking up this change.
  2. Updated the unit tests to be in reference to one rotation rather than pi which can make reading the tests more difficult. I think it's easier to grasp "one rotation" than "pi" rotations.
  3. I added documentation with notes about all this stuff.

@jaredsinclair
Copy link
Contributor

cc @cdzombak

@jaredsinclair
Copy link
Contributor

👍🏻

@cdzombak
Copy link
Contributor

Thank you both, @JulietaFernandez and @jaredsinclair.

@cdzombak cdzombak merged commit 86f1d96 into develop Aug 31, 2016
@cdzombak cdzombak deleted the feature/MO-7115 branch August 31, 2016 19:27
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.

2 participants