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

Migrate XYOrbitViewController #282

Merged
merged 20 commits into from
Jun 12, 2018

Conversation

Martin-Idel-SI
Copy link
Contributor

@Martin-Idel-SI Martin-Idel-SI commented Jun 6, 2018

Closes #219

This PR migrates the XYOrbitViewController and also updates the OrbitViewController.
In particular, we worked on consistency in "mimic" again, working with the originally intended behaviour.

We tried to enable "mimic" behaviour also when the camera was below the xy-plane, but didn't manage to have it working as above the xy-plane (sadly, it's not a symmetric problem).

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@Martin-Idel-SI Martin-Idel-SI force-pushed the feature/migrate_xy_orbit_camera branch from fa3e7f9 to d731a5b Compare June 11, 2018 08:04
@Martin-Idel-SI
Copy link
Contributor Author

We added two small fixes and rebased onto current master. So a new CI is in order:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@clalancette
Copy link
Contributor

clalancette commented Jun 11, 2018

@Martin-Idel-SI FYI, your macOS build failed because of a Jenkins disconnect, and then the restart of it happened to get onto a node that is having trouble right now. I've killed off the new job, so it would probably be best if you kick off a new macOS job on the build farm by hand. Sorry for the disruption.

@Martin-Idel-SI
Copy link
Contributor Author

@clalancette Thanks for the explanation. I'll restart:

  • macOS Build Status

@wjwwood wjwwood added in review Waiting for review (Kanban column) 🔥 labels Jun 11, 2018
@wjwwood wjwwood self-assigned this Jun 12, 2018
@wjwwood wjwwood force-pushed the feature/migrate_xy_orbit_camera branch from d731a5b to 8901ee6 Compare June 12, 2018 07:21
Copy link
Member

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

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

code changes lgtm, will merge with CI

@wjwwood
Copy link
Member

wjwwood commented Jun 12, 2018

I built and tested the controller out locally (on my macbook), looks good there too.

CI (using --packages-up-to rviz2 rviz_rendering_tests rviz_visual_testing_framework):

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@wjwwood
Copy link
Member

wjwwood commented Jun 12, 2018

Missed a cpplint issue:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@wjwwood wjwwood merged commit 8595e16 into ros2:ros2 Jun 12, 2018
@rohbotics rohbotics removed the in review Waiting for review (Kanban column) label Jun 12, 2018
@Martin-Idel-SI Martin-Idel-SI deleted the feature/migrate_xy_orbit_camera branch June 13, 2018 06:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants