From 0dac7aaad89fd4cfbcf6d1b0538234cba9b020d3 Mon Sep 17 00:00:00 2001 From: GermanHydrogen Date: Tue, 19 Mar 2024 17:55:07 +0100 Subject: [PATCH 1/5] Added unit tests to base roof --- tests/modules/roof/__init__.py | 0 tests/modules/roof/test_baseroof.py | 69 +++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+) create mode 100644 tests/modules/roof/__init__.py create mode 100644 tests/modules/roof/test_baseroof.py diff --git a/tests/modules/roof/__init__.py b/tests/modules/roof/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/modules/roof/test_baseroof.py b/tests/modules/roof/test_baseroof.py new file mode 100644 index 00000000..59470dda --- /dev/null +++ b/tests/modules/roof/test_baseroof.py @@ -0,0 +1,69 @@ +from typing import Optional, Any +from unittest.mock import AsyncMock + +import pytest + +import pyobs +from pyobs.modules.roof import BaseRoof +from pyobs.utils.enums import MotionStatus + + +class TestBaseRoof(BaseRoof): + async def init(self, **kwargs: Any) -> None: + pass + + async def park(self, **kwargs: Any) -> None: + pass + + async def stop_motion(self, device: Optional[str] = None, **kwargs: Any) -> None: + pass + + +@pytest.mark.asyncio +async def test_open(mocker): + mocker.patch("pyobs.mixins.WeatherAwareMixin.open") + mocker.patch("pyobs.mixins.MotionStatusMixin.open") + mocker.patch("pyobs.modules.Module.open") + + telescope = TestBaseRoof() + await telescope.open() + + pyobs.mixins.WeatherAwareMixin.open.assert_called_once_with(telescope) + pyobs.mixins.MotionStatusMixin.open.assert_called_once_with(telescope) + pyobs.modules.Module.open.assert_called_once_with(telescope) + + +@pytest.mark.asyncio +async def test_get_fits_header_before_open(): + telescope = TestBaseRoof() + + telescope.get_motion_status = AsyncMock(return_value=MotionStatus.POSITIONED) + header = await telescope.get_fits_header_before() + + assert header["ROOF-OPN"] == (True, "True for open, false for closed roof") + + +@pytest.mark.asyncio +async def test_get_fits_header_before_closed(): + telescope = TestBaseRoof() + + telescope.get_motion_status = AsyncMock(return_value=MotionStatus.PARKED) + header = await telescope.get_fits_header_before() + + assert header["ROOF-OPN"] == (False, "True for open, false for closed roof") + + +@pytest.mark.asyncio +async def test_ready(): + telescope = TestBaseRoof() + + telescope.get_motion_status = AsyncMock(return_value=MotionStatus.TRACKING) + assert await telescope.is_ready() is True + + +@pytest.mark.asyncio +async def test_not_ready(): + telescope = TestBaseRoof() + + telescope.get_motion_status = AsyncMock(return_value=MotionStatus.PARKING) + assert await telescope.is_ready() is False From 7c4a274161d197b073125201e8133219d8fb14c9 Mon Sep 17 00:00:00 2001 From: GermanHydrogen Date: Tue, 19 Mar 2024 18:05:41 +0100 Subject: [PATCH 2/5] Added unit tests to b dome --- tests/modules/roof/test_basedome.py | 35 +++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 tests/modules/roof/test_basedome.py diff --git a/tests/modules/roof/test_basedome.py b/tests/modules/roof/test_basedome.py new file mode 100644 index 00000000..81eb9407 --- /dev/null +++ b/tests/modules/roof/test_basedome.py @@ -0,0 +1,35 @@ +from typing import Any, Tuple, Optional + +import pytest + +from pyobs.modules.roof import BaseDome + + +class TestBaseDome(BaseDome): + + async def init(self, **kwargs: Any) -> None: + pass + + async def park(self, **kwargs: Any) -> None: + pass + + async def stop_motion(self, device: Optional[str] = None, **kwargs: Any) -> None: + pass + + async def move_altaz(self, alt: float, az: float, **kwargs: Any) -> None: + pass + + async def get_altaz(self, **kwargs: Any) -> Tuple[float, float]: + return 60.0, 0.0 + + +@pytest.mark.asyncio +async def test_get_fits_header_before(mocker): + dome = TestBaseDome() + + mocker.patch("pyobs.modules.roof.BaseRoof.get_fits_header_before", return_value={"ROOF-OPN": (True, "")}) + + header = await dome.get_fits_header_before() + + assert "ROOF-OPN" in header + assert header["ROOF-AZ"] == (0.0, "Azimuth of roof slit, deg E of N") From 8705dd8bc7e63eba3366036508867c71b5204582 Mon Sep 17 00:00:00 2001 From: GermanHydrogen Date: Tue, 19 Mar 2024 18:33:33 +0100 Subject: [PATCH 3/5] Added unit tests to dummy roof --- pyobs/modules/roof/dummyroof.py | 6 ++-- tests/modules/roof/test_dummyroof.py | 47 ++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 3 deletions(-) create mode 100644 tests/modules/roof/test_dummyroof.py diff --git a/pyobs/modules/roof/dummyroof.py b/pyobs/modules/roof/dummyroof.py index 8b6eb29b..ffdefbf8 100644 --- a/pyobs/modules/roof/dummyroof.py +++ b/pyobs/modules/roof/dummyroof.py @@ -47,7 +47,7 @@ async def init(self, **kwargs: Any) -> None: # already open? if self.open_percentage != 100: # acquire lock - with LockWithAbort(self._lock_motion, self._abort_motion): + async with LockWithAbort(self._lock_motion, self._abort_motion): # change status await self._change_motion_status(MotionStatus.INITIALIZING) @@ -84,7 +84,7 @@ async def park(self, **kwargs: Any) -> None: # already closed? if self.open_percentage != 0: # acquire lock - with LockWithAbort(self._lock_motion, self._abort_motion): + async with LockWithAbort(self._lock_motion, self._abort_motion): # change status await self._change_motion_status(MotionStatus.PARKING) @@ -126,7 +126,7 @@ async def stop_motion(self, device: Optional[str] = None, **kwargs: Any) -> None # abort # acquire lock - with LockWithAbort(self._lock_motion, self._abort_motion): + async with LockWithAbort(self._lock_motion, self._abort_motion): # change status await self._change_motion_status(MotionStatus.IDLE) diff --git a/tests/modules/roof/test_dummyroof.py b/tests/modules/roof/test_dummyroof.py new file mode 100644 index 00000000..16be5465 --- /dev/null +++ b/tests/modules/roof/test_dummyroof.py @@ -0,0 +1,47 @@ +from unittest.mock import AsyncMock, Mock + +import pytest + +from pyobs.events import RoofOpenedEvent +from pyobs.modules.roof import DummyRoof +from pyobs.utils.enums import MotionStatus + + +@pytest.mark.asyncio +async def test_init(mocker): + mocker.patch("asyncio.sleep") + + roof = DummyRoof() + roof._change_motion_status = AsyncMock() + roof.comm.send_event = Mock() + + await roof.init() + + assert roof.open_percentage == 100 + roof._change_motion_status.assert_awaited_with(MotionStatus.IDLE) + roof.comm.send_event(RoofOpenedEvent()) + + +@pytest.mark.asyncio +async def test_park(mocker): + mocker.patch("asyncio.sleep") + + roof = DummyRoof() + roof.open_percentage = 100 + + roof._change_motion_status = AsyncMock() + roof.comm.send_event = Mock() + + await roof.park() + + assert roof.open_percentage == 0 + roof._change_motion_status.assert_awaited_with(MotionStatus.PARKED) + + +@pytest.mark.asyncio +async def test_stop_motion(): + roof = DummyRoof() + roof._change_motion_status = AsyncMock() + await roof.stop_motion() + + roof._change_motion_status.assert_awaited_with(MotionStatus.IDLE) \ No newline at end of file From 3967cb6c792b12a24cd1bc78ba07b685a753084f Mon Sep 17 00:00:00 2001 From: GermanHydrogen Date: Wed, 20 Mar 2024 10:09:12 +0100 Subject: [PATCH 4/5] Refactored dummy roof --- pyobs/modules/roof/dummyroof.py | 75 ++++++++++++---------------- tests/modules/roof/test_dummyroof.py | 74 ++++++++++++++++++++++++--- 2 files changed, 97 insertions(+), 52 deletions(-) diff --git a/pyobs/modules/roof/dummyroof.py b/pyobs/modules/roof/dummyroof.py index ffdefbf8..58b894ac 100644 --- a/pyobs/modules/roof/dummyroof.py +++ b/pyobs/modules/roof/dummyroof.py @@ -17,12 +17,15 @@ class DummyRoof(BaseRoof, IRoof): __module__ = "pyobs.modules.roof" + _ROOF_CLOSED_PERCENTAGE = 0 + _ROOF_OPEN_PERCENTAGE = 100 + def __init__(self, **kwargs: Any): """Creates a new dummy root.""" BaseRoof.__init__(self, **kwargs) # dummy state - self.open_percentage = 0 + self._open_percentage: int = self._ROOF_CLOSED_PERCENTAGE # allow to abort motion self._lock_motion = asyncio.Lock() @@ -44,34 +47,30 @@ async def init(self, **kwargs: Any) -> None: AcquireLockFailed: If current motion could not be aborted. """ - # already open? - if self.open_percentage != 100: - # acquire lock - async with LockWithAbort(self._lock_motion, self._abort_motion): - # change status - await self._change_motion_status(MotionStatus.INITIALIZING) + if self._is_open(): + return + + async with LockWithAbort(self._lock_motion, self._abort_motion): + await self._change_motion_status(MotionStatus.INITIALIZING) - # open roof - while self.open_percentage < 100: - # open more - self.open_percentage += 1 + await self._move_roof(self._ROOF_OPEN_PERCENTAGE) - # abort? - if self._abort_motion.is_set(): - await self._change_motion_status(MotionStatus.IDLE) - return + await self._change_motion_status(MotionStatus.IDLE) + self.comm.send_event(RoofOpenedEvent()) - # wait a little - await asyncio.sleep(0.1) + def _is_open(self): + return self._open_percentage == self._ROOF_OPEN_PERCENTAGE - # open fully - self.open_percentage = 100 + async def _move_roof(self, target_pos: int) -> None: + step = 1 if target_pos > self._open_percentage else -1 - # change status + while self._open_percentage != target_pos: + if self._abort_motion.is_set(): await self._change_motion_status(MotionStatus.IDLE) + return - # send event - self.comm.send_event(RoofOpenedEvent()) + self._open_percentage += step + await asyncio.sleep(0.1) @timeout(15) async def park(self, **kwargs: Any) -> None: @@ -81,35 +80,23 @@ async def park(self, **kwargs: Any) -> None: AcquireLockFailed: If current motion could not be aborted. """ - # already closed? - if self.open_percentage != 0: - # acquire lock - async with LockWithAbort(self._lock_motion, self._abort_motion): - # change status - await self._change_motion_status(MotionStatus.PARKING) - - # send event - self.comm.send_event(RoofClosingEvent()) + if self._is_closed(): + return - # close roof - while self.open_percentage > 0: - # close more - self.open_percentage -= 1 + async with LockWithAbort(self._lock_motion, self._abort_motion): + await self._change_motion_status(MotionStatus.PARKING) + self.comm.send_event(RoofClosingEvent()) - # abort? - if self._abort_motion.is_set(): - await self._change_motion_status(MotionStatus.IDLE) - return + await self._move_roof(self._ROOF_CLOSED_PERCENTAGE) - # wait a little - await asyncio.sleep(0.1) + await self._change_motion_status(MotionStatus.PARKED) - # change status - await self._change_motion_status(MotionStatus.PARKED) + def _is_closed(self): + return self._open_percentage == self._ROOF_CLOSED_PERCENTAGE def get_percent_open(self) -> float: """Get the percentage the roof is open.""" - return self.open_percentage + return self._open_percentage async def stop_motion(self, device: Optional[str] = None, **kwargs: Any) -> None: """Stop the motion. diff --git a/tests/modules/roof/test_dummyroof.py b/tests/modules/roof/test_dummyroof.py index 16be5465..62dd30ae 100644 --- a/tests/modules/roof/test_dummyroof.py +++ b/tests/modules/roof/test_dummyroof.py @@ -2,13 +2,28 @@ import pytest -from pyobs.events import RoofOpenedEvent +import pyobs +from pyobs.events import RoofOpenedEvent, RoofClosingEvent from pyobs.modules.roof import DummyRoof from pyobs.utils.enums import MotionStatus @pytest.mark.asyncio -async def test_init(mocker): +async def test_open(mocker) -> None: + mocker.patch("pyobs.modules.roof.BaseRoof.open") + roof = DummyRoof() + roof.comm.register_event = AsyncMock() + + await roof.open() + + pyobs.modules.roof.BaseRoof.open.assert_called_once() + + assert roof.comm.register_event.call_args_list[0][0][0] == RoofOpenedEvent + assert roof.comm.register_event.call_args_list[1][0][0] == RoofClosingEvent + + +@pytest.mark.asyncio +async def test_init(mocker) -> None: mocker.patch("asyncio.sleep") roof = DummyRoof() @@ -17,31 +32,74 @@ async def test_init(mocker): await roof.init() - assert roof.open_percentage == 100 roof._change_motion_status.assert_awaited_with(MotionStatus.IDLE) roof.comm.send_event(RoofOpenedEvent()) @pytest.mark.asyncio -async def test_park(mocker): +async def test_park(mocker) -> None: mocker.patch("asyncio.sleep") roof = DummyRoof() - roof.open_percentage = 100 + roof._open_percentage = 100 roof._change_motion_status = AsyncMock() roof.comm.send_event = Mock() await roof.park() - assert roof.open_percentage == 0 roof._change_motion_status.assert_awaited_with(MotionStatus.PARKED) @pytest.mark.asyncio -async def test_stop_motion(): +async def test_move_roof_open(mocker) -> None: + mocker.patch("asyncio.sleep") + + roof = DummyRoof() + + await roof._move_roof(roof._ROOF_OPEN_PERCENTAGE) + + assert roof._open_percentage == 100 + + +@pytest.mark.asyncio +async def test_move_roof_closed(mocker) -> None: + mocker.patch("asyncio.sleep") + + roof = DummyRoof() + + await roof._move_roof(roof._ROOF_CLOSED_PERCENTAGE) + + assert roof._open_percentage == 0 + + +@pytest.mark.asyncio +async def test_move_roof_abort(mocker) -> None: + mocker.patch("asyncio.sleep") + + roof = DummyRoof() + + roof._abort_motion.set() + await roof._move_roof(roof._ROOF_OPEN_PERCENTAGE) + + assert roof._open_percentage == 0 + + +@pytest.mark.asyncio +async def test_move_roof_open(mocker) -> None: + mocker.patch("asyncio.sleep") + + roof = DummyRoof() + + await roof._move_roof(roof._ROOF_OPEN_PERCENTAGE) + + assert roof._open_percentage == 100 + + +@pytest.mark.asyncio +async def test_stop_motion() -> None: roof = DummyRoof() roof._change_motion_status = AsyncMock() await roof.stop_motion() - roof._change_motion_status.assert_awaited_with(MotionStatus.IDLE) \ No newline at end of file + roof._change_motion_status.assert_awaited_with(MotionStatus.IDLE) From 1381eb5986d7f5fa0017bb6420ae1c188debb3ff Mon Sep 17 00:00:00 2001 From: GermanHydrogen Date: Wed, 20 Mar 2024 10:12:31 +0100 Subject: [PATCH 5/5] Fixed comments --- pyobs/modules/roof/basedome.py | 3 --- pyobs/modules/roof/baseroof.py | 3 +-- pyobs/modules/roof/dummyroof.py | 28 +++++++++++----------------- 3 files changed, 12 insertions(+), 22 deletions(-) diff --git a/pyobs/modules/roof/basedome.py b/pyobs/modules/roof/basedome.py index f5e20c7d..086c5c04 100644 --- a/pyobs/modules/roof/basedome.py +++ b/pyobs/modules/roof/basedome.py @@ -19,7 +19,6 @@ def __init__(self, **kwargs: Any): """Initialize a new base dome.""" BaseRoof.__init__(self, **kwargs) - # register exception exc.register_exception(exc.MotionError, 3, timespan=600, callback=self._default_remote_error_callback) async def get_fits_header_before( @@ -34,10 +33,8 @@ async def get_fits_header_before( Dictionary containing FITS headers. """ - # get from parent hdr = await BaseRoof.get_fits_header_before(self, namespaces, **kwargs) - # add azimuth and return it _, az = await self.get_altaz() hdr["ROOF-AZ"] = (az, "Azimuth of roof slit, deg E of N") return hdr diff --git a/pyobs/modules/roof/baseroof.py b/pyobs/modules/roof/baseroof.py index 9c5f4582..0d188022 100644 --- a/pyobs/modules/roof/baseroof.py +++ b/pyobs/modules/roof/baseroof.py @@ -19,7 +19,6 @@ def __init__(self, **kwargs: Any): """Initialize a new base roof.""" Module.__init__(self, **kwargs) - # init mixins WeatherAwareMixin.__init__(self, **kwargs) MotionStatusMixin.__init__(self, **kwargs) @@ -50,7 +49,7 @@ async def get_fits_header_before( } async def is_ready(self, **kwargs: Any) -> bool: - """Returns the device is "ready", whatever that means for the specific device. + """The roof is ready, if it is open. Returns: True, if roof is open. diff --git a/pyobs/modules/roof/dummyroof.py b/pyobs/modules/roof/dummyroof.py index 58b894ac..1a3aca43 100644 --- a/pyobs/modules/roof/dummyroof.py +++ b/pyobs/modules/roof/dummyroof.py @@ -27,7 +27,6 @@ def __init__(self, **kwargs: Any): # dummy state self._open_percentage: int = self._ROOF_CLOSED_PERCENTAGE - # allow to abort motion self._lock_motion = asyncio.Lock() self._abort_motion = asyncio.Event() @@ -35,7 +34,6 @@ async def open(self) -> None: """Open module.""" await BaseRoof.open(self) - # register event await self.comm.register_event(RoofOpenedEvent) await self.comm.register_event(RoofClosingEvent) @@ -61,17 +59,6 @@ async def init(self, **kwargs: Any) -> None: def _is_open(self): return self._open_percentage == self._ROOF_OPEN_PERCENTAGE - async def _move_roof(self, target_pos: int) -> None: - step = 1 if target_pos > self._open_percentage else -1 - - while self._open_percentage != target_pos: - if self._abort_motion.is_set(): - await self._change_motion_status(MotionStatus.IDLE) - return - - self._open_percentage += step - await asyncio.sleep(0.1) - @timeout(15) async def park(self, **kwargs: Any) -> None: """Close the roof. @@ -94,6 +81,17 @@ async def park(self, **kwargs: Any) -> None: def _is_closed(self): return self._open_percentage == self._ROOF_CLOSED_PERCENTAGE + async def _move_roof(self, target_pos: int) -> None: + step = 1 if target_pos > self._open_percentage else -1 + + while self._open_percentage != target_pos: + if self._abort_motion.is_set(): + await self._change_motion_status(MotionStatus.IDLE) + return + + self._open_percentage += step + await asyncio.sleep(0.1) + def get_percent_open(self) -> float: """Get the percentage the roof is open.""" return self._open_percentage @@ -108,13 +106,9 @@ async def stop_motion(self, device: Optional[str] = None, **kwargs: Any) -> None AcquireLockFailed: If current motion could not be aborted. """ - # change status await self._change_motion_status(MotionStatus.ABORTING) - # abort - # acquire lock async with LockWithAbort(self._lock_motion, self._abort_motion): - # change status await self._change_motion_status(MotionStatus.IDLE)