-
Notifications
You must be signed in to change notification settings - Fork 49
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
Separate coarse autofocus from fine autofocus #659
Separate coarse autofocus from fine autofocus #659
Conversation
Remove automatic fine focus from coarse focus Enhance tests Fix bug preventing final FIT being kept if no plots
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 A few suggestions but otherwise LGTM
pocs/camera/camera.py
Outdated
default False | ||
coarse (bool, optional): Whether to perform a coarse focus, otherwise will perform | ||
a fine focus. Default False. | ||
plots (bool, optional: Whether to write focus plots to images folder, default False. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would prefer to see this called make_plots
but we could make a separate issue to change things like that all at once.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. Yes, there are probably quite a few of these sorts of terminology inconsistencies to be fixed.
if blocking: | ||
fine_event.wait() | ||
focus_event.wait() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should include some ultimate timeout in the wait
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Needs to be a rather conservative one though, as it's a bit involved to predict exactly how long it will take.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That might be a job for another PR actually. Want to add timeouts to the camera's threaded operations more generally to prevent the lock ups we sometimes get.
pocs/focuser/focuser.py
Outdated
@@ -490,14 +463,13 @@ def _autofocus(self, | |||
|
|||
final_focus = self.move_to(best_focus) | |||
|
|||
final_fn = "{}_{}.{}".format(final_focus, "final", self._camera.file_extension) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this include the focus_type
? I don't seem to see it set anywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good idea. It should. I'll put that in.
Codecov Report
@@ Coverage Diff @@
## develop #659 +/- ##
===========================================
+ Coverage 69.99% 70.54% +0.55%
===========================================
Files 64 64
Lines 5645 5633 -12
Branches 795 793 -2
===========================================
+ Hits 3951 3974 +23
+ Misses 1484 1447 -37
- Partials 210 212 +2
Continue to review full report at Codecov.
|
Change in behaviour for autofocus so that a coarse focus is not automatically followed followed by a fine focus. This is required for the changes planned for AstroHuntsman/huntsman-pocs#69
Also incorporates some improvements to the autofocus tests (checking for created files), and fixes a bug introduced by #621 that prevented the final focus position image being taken when plotting was disabled, which is now the default.