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-46276: modified wavelength change and setup laser #164

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

Conversation

parfa30
Copy link
Contributor

@parfa30 parfa30 commented Sep 12, 2024

Changing the laser wavelength and setup laser functions in mtcalsys so are more usable in a variety of sal scripts.

@parfa30 parfa30 marked this pull request as draft September 13, 2024 15:21
@parfa30 parfa30 marked this pull request as ready for review September 13, 2024 16:52
Copy link
Contributor

@edennihy edennihy left a comment

Choose a reason for hiding this comment

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

Hi Parker, I added a handful of recommended changes and suggestions to start here. @tribeiro actually looked at most of them together to make sure we were in agreement, so we should be pretty close to finished once these changes are in.

One other missing piece is the unit tests for these methods. The nice part about that though is since they are being unit tested here, you will not need to test them again in the scripts when they are used.

python/lsst/ts/observatory/control/data/mtcalsys.yaml Outdated Show resolved Hide resolved
python/lsst/ts/observatory/control/maintel/mtcalsys.py Outdated Show resolved Hide resolved
python/lsst/ts/observatory/control/maintel/mtcalsys.py Outdated Show resolved Hide resolved
python/lsst/ts/observatory/control/maintel/mtcalsys.py Outdated Show resolved Hide resolved
python/lsst/ts/observatory/control/maintel/mtcalsys.py Outdated Show resolved Hide resolved
"""Start the propagation of the Tunable Laser"""
# get laser detailed state

await self.rem.tunablelaser.cmd_startPropagateLaser.start(
Copy link
Contributor

Choose a reason for hiding this comment

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

How does the propagation work? This might be a good situation to add a check on a evt to check that the propagation has actually started. You can check against the detailedstate evt that the laser is propagating with the right mode before this method completes.

Maybe using something like this

async def wait_for_lamp_to_warm_up(self) -> None:

await self.rem.tunablelaser.cmd_stopPropagateLaser.start(
timeout=self.long_timeout
)

Copy link
Contributor

Choose a reason for hiding this comment

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

Same thing here, you should add a check that the detailed state is nonpropagating before this method completes.


async def laser_start_propagate(self) -> None:
"""Start the propagation of the Tunable Laser"""
# get laser detailed state
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can remove this comment

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