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

Complete calibration choreography #1803

Draft
wants to merge 31 commits into
base: restructured-gaze-calibration-and-mapping
from

Conversation

@romanroibu
Copy link
Member

romanroibu commented Feb 4, 2020

TODO
  • Implement persistence for screen marker, single marker, natural feature choreographies, but not hmd.
  • Remove old calibration plugins from calibration_routines
  • Perform basic tests for hmd choreography

Low priority

  • Perform extensive tests for all choreographies
  • Show choreography plugins in the list of core plugins
  • Hide choreography plugins from the Plugin Manager

Waiting for gazer rework

  • Reenable validation
@romanroibu

This comment has been minimized.

Copy link
Member Author

romanroibu commented Feb 4, 2020

@papr the code at 936f63d8ce49ad395aca6a21d95fe8a88fcf70ab has the screen marker calibration working with the 2d gazer, if you need to test.

romanroibu added 16 commits Feb 5, 2020
…g the calibration markers
This would happen if the observer is a private method (starting with '__'). This patch will make adding such an observer raise a ValueError imediately on calling add_observer.
@romanroibu romanroibu requested a review from pfaion Feb 7, 2020

def _perform_start(self):
# TODO: Is this check duplicated for all calibration choreographies?
if not self.g_pool.capture.online:

This comment has been minimized.

Copy link
@romanroibu

romanroibu Feb 10, 2020

Author Member

@papr Could you please clarify why this check exists and if it makes sense for all choreography plugins, or only screen marker and single marker?

This comment has been minimized.

Copy link
@papr

papr Feb 10, 2020

Member

Necessary for all choreos that rely on having a world video, i.e. screen marker, single marker, and natural features.

# TODO: Why the subject is always "calibration.started", and not "accuracy_test.started" for accuracy test mode?
# mode=self.__current_mode,
# mode=mode,

This comment has been minimized.

Copy link
@romanroibu

romanroibu Feb 10, 2020

Author Member

@papr Could you please clarify why the previous implementation was sending calibration.started notifications, even when the user was triggering an accuracy test? Is it just a copy/paste bug or was it intentional? Also, what parts could depend on the old behavior?

This comment has been minimized.

Copy link
@papr

papr Feb 10, 2020

Member

Can be renamed to validation.started. Other instances of accuracy_test should be renamed to validation.

@romanroibu romanroibu requested a review from papr Feb 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.