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

DM-40956 Twilight Flats Script: Allow immediate tracking and ignoring tcs components #154

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

elanaku
Copy link
Contributor

@elanaku elanaku commented Oct 29, 2024

No description provided.

@elanaku elanaku requested a review from edennihy October 29, 2024 16:52
"""Abstract method to set the instrument. Change the filter
and start tracking.
"""
# slew to desired field
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment should be removed, no slew is happening here.

@@ -242,14 +249,18 @@ def get_schema(cls):
description: If True, track sky. If False, keep az and el constant.
type: boolean
default: True
position_telescope:
description: If True, position telescope relative to the sun. \
If False, assume the telescope is in correct position.
Copy link
Contributor

Choose a reason for hiding this comment

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

I really, really don't like this idea. We can't rely on the users to avoid a dangerous scenario. When this flag is False, could the users start taking images when the telescope is pointed due West at an elevation of 20 degrees? We need some protection against that.

If we are not going to slew the telescope, we should still assume that we have telescope telemetry and that we can check that the telescope position is not pointed at the sun and raise an exception if it is too close.

@@ -209,3 +209,15 @@ async def slew_azel_and_setup_instrument(self, az, el):
filter=self.config.filter,
grating=self.config.grating,
)

async def setup_instrument(self, az, el):
"""Abstract method to set the instrument. Change the filter
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you want to put the start_tracking in the setup_instrument method...it's misleading. You could either rename this method and it's purpose or (an even better option) split the two.

@@ -282,10 +293,13 @@ async def configure(self, config: types.SimpleNamespace):
if comp in self.camera.components_attr:
self.log.debug(f"Ignoring Camera component {comp}.")
setattr(self.camera.check, comp, False)
elif comp in self.tcs.components_attr:
Copy link
Contributor

Choose a reason for hiding this comment

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

This is also kind of scary, because the use case is that we really should only ignore the MTDome and MTDomeTrajectory. We should never ignore the MTCS for example, but it is allowed with the way this is configured.

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