From 1c1312f1c425deb3cd4978623e40fc8c8b10dbe3 Mon Sep 17 00:00:00 2001 From: Igor Suarez-Sola Date: Thu, 10 Oct 2024 17:39:15 -0700 Subject: [PATCH 1/2] Extend TCS readiness check to other image types beyond OBJECT .- Added image types ENGTEST, ACQ and CWFS to take_imgtype method in the BaseCamera class for tcs readiness checks. This requires the camera attribute: `tcs_ready_to_take_data` to be set. .- enums: in mtcs_async_mock.py replace MTRotator.EnabledSubstate from INITIALIZING to STATIONARY and point enums from idl package to the new xml one. .- Added unit tests methods for this logic. --- .../ts/observatory/control/base_camera.py | 5 +- .../control/mock/base_camera_async_mock.py | 55 +++++++++ .../control/mock/mtcs_async_mock.py | 61 +++++----- tests/test_generic_camera.py | 110 ++++++++++++++++++ 4 files changed, 198 insertions(+), 33 deletions(-) diff --git a/python/lsst/ts/observatory/control/base_camera.py b/python/lsst/ts/observatory/control/base_camera.py index e63b4c82..227d1c9b 100644 --- a/python/lsst/ts/observatory/control/base_camera.py +++ b/python/lsst/ts/observatory/control/base_camera.py @@ -965,7 +965,8 @@ async def take_imgtype( if imgtype not in ["BIAS", "DARK"]: await self.setup_instrument(**kwargs) - if imgtype == "OBJECT" and self.ready_to_take_data is not None: + tcs_ready_imgtypes = ["OBJECT", "ENGTEST", "ACQ", "CWFS"] + if imgtype in tcs_ready_imgtypes and self.ready_to_take_data is not None: self.log.debug(f"imagetype: {imgtype}, wait for TCS to be ready.") try: await asyncio.wait_for( @@ -977,7 +978,7 @@ async def take_imgtype( "Timeout waiting for TCS to report as ready to take data " f"(timeout={self.max_tcs_wait_time})." ) - elif imgtype == "OBJECT" and self.ready_to_take_data is None: + elif imgtype in tcs_ready_imgtypes and self.ready_to_take_data is None: self.log.debug(f"imagetype: {imgtype}, TCS synchronization not configured.") else: self.log.debug(f"imagetype: {imgtype}, skip TCS synchronization.") diff --git a/python/lsst/ts/observatory/control/mock/base_camera_async_mock.py b/python/lsst/ts/observatory/control/mock/base_camera_async_mock.py index 86d1123a..56c87d4e 100644 --- a/python/lsst/ts/observatory/control/mock/base_camera_async_mock.py +++ b/python/lsst/ts/observatory/control/mock/base_camera_async_mock.py @@ -26,6 +26,7 @@ import typing import astropy +import pytest from lsst.ts import utils from lsst.ts.observatory.control import CameraExposure from lsst.ts.observatory.control.mock import RemoteGroupAsyncMock @@ -785,6 +786,60 @@ def assert_take_calibration(self, expected_camera_exposure: CameraExposure) -> N "await counts for cmd_discardRows.set_start." ) + async def assert_take_image_tcs_sync( + self, + image_type: str, + exptime: float = 1.0, + n: int = 1, + should_fail: bool = False, + **kwargs: typing.Any, + ) -> None: + """Test that taking an image of a given type waits for TCS readiness + and asserts the camera commands. + + Parameters + ---------- + image_type : str + The image type to test (e.g., "OBJECT", "ENGTEST", "ACQ","CWFS"). + exptime : float + Exposure time. + n : int + Number of images to take. + should_fail : bool + If True, simulate TCS failure and expect a RuntimeError. + **kwargs + Additional arguments to pass to the `assert_take_` method. + """ + image_type_lower = image_type.lower() + + assert_method_name = f"assert_take_{image_type_lower}" + + assert_take_method = getattr(self, assert_method_name, None) + + if assert_take_method is None: + raise AttributeError( + f"No method found for image type '{image_type}'. " + f"Ensure that '{assert_method_name}' exist." + ) + + with self.get_fake_tcs() as fake_tcs: + fake_tcs.fail = should_fail + + if should_fail: + with pytest.raises(RuntimeError): + await assert_take_method( + exptime=exptime, + n=n, + **kwargs, + ) + else: + await assert_take_method( + exptime=exptime, + n=n, + **kwargs, + ) + assert fake_tcs.called == 1 + @contextlib.contextmanager def get_fake_tcs(self) -> typing.Generator[FakeTCS, None, None]: fake_tcs = FakeTCS(self.log) diff --git a/python/lsst/ts/observatory/control/mock/mtcs_async_mock.py b/python/lsst/ts/observatory/control/mock/mtcs_async_mock.py index 8540f5d5..af3abb4e 100644 --- a/python/lsst/ts/observatory/control/mock/mtcs_async_mock.py +++ b/python/lsst/ts/observatory/control/mock/mtcs_async_mock.py @@ -25,11 +25,10 @@ import typing import numpy as np -from lsst.ts import idl, utils -from lsst.ts.idl.enums import MTM1M3 +from lsst.ts import utils, xml from lsst.ts.observatory.control.maintel.mtcs import MTCS, MTCSUsages from lsst.ts.observatory.control.mock import RemoteGroupAsyncMock -from lsst.ts.xml.enums import MTDome +from lsst.ts.xml.enums import MTM1M3, MTDome class MTCSAsyncMock(RemoteGroupAsyncMock): @@ -106,7 +105,7 @@ async def setup_types(self) -> None: ) self._mtrotator_evt_in_position = types.SimpleNamespace(inPosition=True) self._mtrotator_evt_controller_state = types.SimpleNamespace( - enabledSubstate=idl.enums.MTRotator.EnabledSubstate.INITIALIZING + enabledSubstate=xml.enums.MTRotator.EnabledSubstate.STATIONARY ) # MTDome data @@ -127,19 +126,19 @@ async def setup_types(self) -> None: # MTM1M3 data self._mtm1m3_evt_detailed_state = types.SimpleNamespace( - detailedState=idl.enums.MTM1M3.DetailedState.PARKED + detailedState=xml.enums.MTM1M3.DetailedState.PARKED ) self._mtm1m3_evt_applied_balance_forces = types.SimpleNamespace( forceMagnitude=0.0 ) self._mtm1m3_evt_hp_test_status = types.SimpleNamespace( - testState=[idl.enums.MTM1M3.HardpointTest.NOTTESTED] * 6 + testState=[xml.enums.MTM1M3.HardpointTest.NOTTESTED] * 6 ) self._mtm1m3_evt_force_actuator_bump_test_status = types.SimpleNamespace( actuatorId=0, - primaryTest=[idl.enums.MTM1M3.BumpTest.NOTTESTED] + primaryTest=[xml.enums.MTM1M3.BumpTest.NOTTESTED] * len(self.mtcs.get_m1m3_actuator_ids()), - secondaryTest=[idl.enums.MTM1M3.BumpTest.NOTTESTED] + secondaryTest=[xml.enums.MTM1M3.BumpTest.NOTTESTED] * len(self.mtcs.get_m1m3_actuator_secondary_ids()), primaryTestTimestamps=[0.0] * len(self.mtcs.get_m1m3_actuator_ids()), secondaryTestTimestamps=[0.0] @@ -149,8 +148,8 @@ async def setup_types(self) -> None: slewFlag=False, balanceForcesApplied=False, ) - self.desired_hp_test_final_status = idl.enums.MTM1M3.HardpointTest.PASSED - self.desired_bump_test_final_status = idl.enums.MTM1M3.BumpTest.PASSED + self.desired_hp_test_final_status = xml.enums.MTM1M3.HardpointTest.PASSED + self.desired_bump_test_final_status = xml.enums.MTM1M3.BumpTest.PASSED self.m1m3_actuator_offset = 101 @@ -437,11 +436,11 @@ async def mtrotator_cmd_stop(self, *args: typing.Any, **kwargs: typing.Any) -> N async def _mtrotator_stop(self) -> None: self._mtrotator_evt_controller_state.enabledSubstate = ( - idl.enums.MTRotator.EnabledSubstate.CONSTANT_VELOCITY + xml.enums.MTRotator.EnabledSubstate.CONSTANT_VELOCITY ) await asyncio.sleep(1.0) self._mtrotator_evt_controller_state.enabledSubstate = ( - idl.enums.MTRotator.EnabledSubstate.STATIONARY + xml.enums.MTRotator.EnabledSubstate.STATIONARY ) async def mtrotator_tel_rotation_next( @@ -587,7 +586,7 @@ async def mtm1m3_cmd_raise_m1m3( if ( raise_m1m3 and self._mtm1m3_evt_detailed_state.detailedState - == idl.enums.MTM1M3.DetailedState.PARKED + == xml.enums.MTM1M3.DetailedState.PARKED ): self._mtm1m3_raise_task = asyncio.create_task(self.execute_raise_m1m3()) else: @@ -604,7 +603,7 @@ async def mtm1m3_cmd_enter_engineering( # since it could change considerably from what the m1m3 actually does. asyncio.create_task( self._set_m1m3_detailed_state( - idl.enums.MTM1M3.DetailedState.PARKEDENGINEERING + xml.enums.MTM1M3.DetailedState.PARKEDENGINEERING ) ) @@ -626,7 +625,7 @@ async def mtm1m3_cmd_exit_engineering( # simply put m1m3 in PARKED state, regardless of which engineering # state it was before. asyncio.create_task( - self._set_m1m3_detailed_state(idl.enums.MTM1M3.DetailedState.PARKED) + self._set_m1m3_detailed_state(xml.enums.MTM1M3.DetailedState.PARKED) ) async def mtm1m3_cmd_test_hardpoint( @@ -658,8 +657,8 @@ async def mtm1m3_cmd_abort_raise_m1m3( self, *args: typing.Any, **kwargs: typing.Any ) -> None: if self._mtm1m3_evt_detailed_state.detailedState in { - idl.enums.MTM1M3.DetailedState.RAISINGENGINEERING, - idl.enums.MTM1M3.DetailedState.RAISING, + xml.enums.MTM1M3.DetailedState.RAISINGENGINEERING, + xml.enums.MTM1M3.DetailedState.RAISING, }: self._mtm1m3_abort_raise_task = asyncio.create_task( self.execute_abort_raise_m1m3() @@ -670,12 +669,12 @@ async def mtm1m3_cmd_abort_raise_m1m3( async def execute_raise_m1m3(self) -> None: self.log.debug("Start raising M1M3...") self._mtm1m3_evt_detailed_state.detailedState = ( - idl.enums.MTM1M3.DetailedState.RAISING + xml.enums.MTM1M3.DetailedState.RAISING ) await asyncio.sleep(self.execute_raise_lower_m1m3_time) self.log.debug("Done raising M1M3...") self._mtm1m3_evt_detailed_state.detailedState = ( - idl.enums.MTM1M3.DetailedState.ACTIVE + xml.enums.MTM1M3.DetailedState.ACTIVE ) async def mtm1m3_cmd_lower_m1m3( @@ -686,7 +685,7 @@ async def mtm1m3_cmd_lower_m1m3( if ( lower_m1m3 and self._mtm1m3_evt_detailed_state.detailedState - == idl.enums.MTM1M3.DetailedState.ACTIVE + == xml.enums.MTM1M3.DetailedState.ACTIVE ): self._mtm1m3_lower_task = asyncio.create_task(self.execute_lower_m1m3()) else: @@ -697,12 +696,12 @@ async def mtm1m3_cmd_lower_m1m3( async def execute_lower_m1m3(self) -> None: self.log.debug("Start lowering M1M3...") self._mtm1m3_evt_detailed_state.detailedState = ( - idl.enums.MTM1M3.DetailedState.LOWERING + xml.enums.MTM1M3.DetailedState.LOWERING ) await asyncio.sleep(self.execute_raise_lower_m1m3_time) self.log.debug("Done lowering M1M3...") self._mtm1m3_evt_detailed_state.detailedState = ( - idl.enums.MTM1M3.DetailedState.PARKED + xml.enums.MTM1M3.DetailedState.PARKED ) async def execute_abort_raise_m1m3(self) -> None: @@ -712,12 +711,12 @@ async def execute_abort_raise_m1m3(self) -> None: self.log.debug("Set m1m3 detailed state to lowering...") self._mtm1m3_evt_detailed_state.detailedState = ( - idl.enums.MTM1M3.DetailedState.LOWERING + xml.enums.MTM1M3.DetailedState.LOWERING ) await asyncio.sleep(self.execute_raise_lower_m1m3_time) self.log.debug("M1M3 raise task done, set detailed state to parked...") self._mtm1m3_evt_detailed_state.detailedState = ( - idl.enums.MTM1M3.DetailedState.PARKED + xml.enums.MTM1M3.DetailedState.PARKED ) async def mtm1m3_cmd_enable_hardpoint_corrections( @@ -741,7 +740,7 @@ async def mtm1m3_cmd_disable_hp_corrections( ) async def _set_m1m3_detailed_state( - self, detailed_state: idl.enums.MTM1M3.DetailedState + self, detailed_state: xml.enums.MTM1M3.DetailedState ) -> None: self.log.debug( f"M1M3 detailed state: {self._mtm1m3_evt_detailed_state.detailedState!r} -> {detailed_state!r}" @@ -751,8 +750,8 @@ async def _set_m1m3_detailed_state( async def _mtm1m3_cmd_test_hardpoint(self, hp: int) -> None: hp_test_status = [ - idl.enums.MTM1M3.HardpointTest.TESTINGPOSITIVE, - idl.enums.MTM1M3.HardpointTest.TESTINGNEGATIVE, + xml.enums.MTM1M3.HardpointTest.TESTINGPOSITIVE, + xml.enums.MTM1M3.HardpointTest.TESTINGNEGATIVE, self.desired_hp_test_final_status, ] @@ -765,10 +764,10 @@ async def _mtm1m3_cmd_force_actuator_bump_test( self, actuator_id: int, test_primary: bool, test_secondary: bool ) -> None: bump_test_states = [ - idl.enums.MTM1M3.BumpTest.TESTINGPOSITIVE, - idl.enums.MTM1M3.BumpTest.TESTINGPOSITIVEWAIT, - idl.enums.MTM1M3.BumpTest.TESTINGNEGATIVE, - idl.enums.MTM1M3.BumpTest.TESTINGNEGATIVEWAIT, + xml.enums.MTM1M3.BumpTest.TESTINGPOSITIVE, + xml.enums.MTM1M3.BumpTest.TESTINGPOSITIVEWAIT, + xml.enums.MTM1M3.BumpTest.TESTINGNEGATIVE, + xml.enums.MTM1M3.BumpTest.TESTINGNEGATIVEWAIT, ] if test_primary: diff --git a/tests/test_generic_camera.py b/tests/test_generic_camera.py index f413e003..740960bb 100644 --- a/tests/test_generic_camera.py +++ b/tests/test_generic_camera.py @@ -21,6 +21,9 @@ import logging +# from typing import Any, Dict, NoReturn, Optional +from typing import Any, Dict + import pytest from lsst.ts import utils from lsst.ts.observatory.control import Usages @@ -316,3 +319,110 @@ async def test_take_cwfs(self) -> None: async def test_take_acq(self) -> None: await self.assert_take_acq() + + image_types: Dict[str, Dict[str, Any]] = { + "BIAS": { + "assert_method": "assert_take_bias", + "params": {"nbias": 1}, + }, + "DARK": { + "assert_method": "assert_take_darks", + "params": {"ndarks": 1, "exptime": 1.0}, + }, + "FLAT": { + "assert_method": "assert_take_flats", + "params": {"nflats": 1, "exptime": 1.0}, + }, + "FOCUS": { + "assert_method": "assert_take_focus", + "params": {"n": 1, "exptime": 1.0}, + }, + "OBJECT": { + "assert_method": "assert_take_object", + "params": {"n": 1, "exptime": 1.0}, + }, + "ENGTEST": { + "assert_method": "assert_take_engtest", + "params": {"n": 1, "exptime": 1.0}, + }, + "ACQ": { + "assert_method": "assert_take_acq", + "params": {"n": 1, "exptime": 1.0}, + }, + "CWFS": { + "assert_method": "assert_take_cwfs", + "params": {"n": 1, "exptime": 1.0}, + }, + } + + def reset_mocks(self) -> None: + self.remote_group.camera.cmd_takeImages.set.reset_mock() + self.remote_group.camera.cmd_takeImages.start.reset_mock() + self.remote_group.camera.evt_endReadout.flush.reset_mock() + + async def test_take_image_no_tcs_sync(self) -> None: + image_types_no_tcs = ["BIAS", "DARK", "FLAT", "FOCUS"] + + for img_type in image_types_no_tcs: + with self.subTest(image_type=img_type): + with self.get_fake_tcs() as fake_tcs: + fake_tcs.fail = False + methods = self.image_types.get(img_type) + assert ( + methods is not None + ), f"Image type '{img_type}' not found in image_types mapping." + + assert_method_name = methods["assert_method"] + params = methods["params"] + + self.reset_mocks() + + assert_take_method = getattr(self, assert_method_name, None) + assert ( + assert_take_method is not None + ), f"Image type '{img_type}' not found in image_types mapping." + + await assert_take_method(**params) + + # Ensure that ready_to_take_data was not called + assert fake_tcs.called == 0 + + async def test_take_image_tcs_sync(self) -> None: + image_types_tcs = ["OBJECT", "ENGTEST", "ACQ", "CWFS"] + + for img_type in image_types_tcs: + with self.subTest(image_type=img_type): + methods = self.image_types.get(img_type) + assert ( + methods is not None + ), f"Image type '{img_type}' not found in image_types mapping." + + params = methods["params"] + + self.reset_mocks() + + await self.assert_take_image_tcs_sync( + image_type=img_type, + should_fail=False, + **params, + ) + + async def test_take_image_tcs_sync_fail(self) -> None: + image_types_tcs = ["OBJECT", "ENGTEST", "ACQ", "CWFS"] + + for img_type in image_types_tcs: + with self.subTest(image_type=img_type): + methods = self.image_types.get(img_type) + assert ( + methods is not None + ), f"Image type '{img_type}' not found in image_types mapping." + + params = methods["params"] + + self.reset_mocks() + + await self.assert_take_image_tcs_sync( + image_type=img_type, + should_fail=True, + **params, + ) From f3cf295c46ded905f43028a05483abd34361faab Mon Sep 17 00:00:00 2001 From: Igor Suarez-Sola Date: Thu, 10 Oct 2024 17:46:32 -0700 Subject: [PATCH 2/2] News fragment --- doc/news/DM-46179.bugfix.rst | 1 + doc/news/DM-46179.feature.rst | 2 ++ doc/news/DM-46179.removal.rst | 1 + 3 files changed, 4 insertions(+) create mode 100644 doc/news/DM-46179.bugfix.rst create mode 100644 doc/news/DM-46179.feature.rst create mode 100644 doc/news/DM-46179.removal.rst diff --git a/doc/news/DM-46179.bugfix.rst b/doc/news/DM-46179.bugfix.rst new file mode 100644 index 00000000..2a53e940 --- /dev/null +++ b/doc/news/DM-46179.bugfix.rst @@ -0,0 +1 @@ +Fix MTRotator enumeration from INITIALIZING to STATIONARY diff --git a/doc/news/DM-46179.feature.rst b/doc/news/DM-46179.feature.rst new file mode 100644 index 00000000..6d199d7d --- /dev/null +++ b/doc/news/DM-46179.feature.rst @@ -0,0 +1,2 @@ +Extend TCS readiness check to other image types beyond OBJECT, such as: +ENGTEST, CWFS and ACQ. \ No newline at end of file diff --git a/doc/news/DM-46179.removal.rst b/doc/news/DM-46179.removal.rst new file mode 100644 index 00000000..b4979d91 --- /dev/null +++ b/doc/news/DM-46179.removal.rst @@ -0,0 +1 @@ +In MTCSAsyncMock remove old idl.enums import in favor of new xml.enums \ No newline at end of file