From 0dac7aaad89fd4cfbcf6d1b0538234cba9b020d3 Mon Sep 17 00:00:00 2001 From: GermanHydrogen Date: Tue, 19 Mar 2024 17:55:07 +0100 Subject: [PATCH 01/15] 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 02/15] 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 03/15] 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 04/15] 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 05/15] 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) From 3539746af4de81fbd1ff96e96af4eb1d16ff33fd Mon Sep 17 00:00:00 2001 From: germanhydrogen Date: Thu, 21 Mar 2024 13:07:05 +0100 Subject: [PATCH 06/15] Added background task class to simplify the functionallity of object --- pyobs/background_task.py | 38 ++++++++++++++++++++ pyobs/object.py | 76 +++++++++------------------------------- 2 files changed, 55 insertions(+), 59 deletions(-) create mode 100644 pyobs/background_task.py diff --git a/pyobs/background_task.py b/pyobs/background_task.py new file mode 100644 index 00000000..cabe0d47 --- /dev/null +++ b/pyobs/background_task.py @@ -0,0 +1,38 @@ +import asyncio +import logging +from typing import Optional, Coroutine, Any, Callable + +log = logging.getLogger(__name__) + + +class BackgroundTask: + def __init__(self, func: Callable[..., Coroutine[Any, Any, None]], restart: bool) -> None: + self._func: Callable[..., Coroutine[Any, Any, None]] = func + self._restart: bool = restart + self._task: Optional[asyncio.Future] = None + + def start(self) -> None: + self._start_task() + + def _callback_function(self, args=None) -> None: + try: + exception = self._task.exception() + except asyncio.CancelledError: + return + + if exception is not None: + log.exception("Exception in thread method %s." % self._func.__name__) + + if self._restart: + log.error("Background task for %s has died, restarting...", self._func.__name__) + self._start_task() + else: + log.error("Background task for %s has died, quitting...", self._func.__name__) + + def _start_task(self) -> None: + self._task = asyncio.create_task(self._func()) + self._task.add_done_callback(self._callback_function) + + def stop(self) -> None: + if self._task is not None: + self._task.cancel() diff --git a/pyobs/object.py b/pyobs/object.py index e887c4ff..b9f011df 100644 --- a/pyobs/object.py +++ b/pyobs/object.py @@ -23,6 +23,7 @@ from astroplan import Observer from astropy.coordinates import EarthLocation +from pyobs.background_task import BackgroundTask from pyobs.comm import Comm from pyobs.comm.dummy import DummyComm @@ -270,12 +271,10 @@ def __init__( self._opened = False # background tasks - self._background_tasks: Dict[ - Callable[..., Coroutine[Any, Any, None]], Tuple[Optional[asyncio.Task[bool]], bool] - ] = {} - self._watchdog_task: Optional[asyncio.Task[None]] = None + self._background_tasks: List[Tuple[BackgroundTask, bool]] = [] - def add_background_task(self, func: Callable[..., Coroutine[Any, Any, None]], restart: bool = True) -> None: + def add_background_task(self, func: Callable[..., Coroutine[Any, Any, None]], + restart: bool = True, autostart: bool = True) -> None: """Add a new function that should be run in the background. MUST be called in constructor of derived class or at least before calling open() on the object. @@ -283,20 +282,16 @@ def add_background_task(self, func: Callable[..., Coroutine[Any, Any, None]], re Args: func: Func to add. restart: Whether to restart this function. + autostart: Whether to start this function when the module is opened """ - # create thread - self._background_tasks[func] = (None, restart) + background_task = BackgroundTask(func, restart) + self._background_tasks = (background_task, autostart) async def open(self) -> None: """Open module.""" - # start background tasks - for func, (task, restart) in self._background_tasks.items(): - log.info("Starting background task for %s...", func.__name__) - self._background_tasks[func] = (asyncio.create_task(self._background_func(func)), restart) - if len(self._background_tasks) > 0: - self._watchdog_task = asyncio.create_task(self._watchdog()) + self._perform_background_task_autostart() # open child objects for obj in self._child_objects: @@ -309,6 +304,11 @@ async def open(self) -> None: # success self._opened = True + def _perform_background_task_autostart(self): + todo = filter(lambda b: b[1] is True, self._background_tasks) + for task in todo: + task.start() + @property def opened(self) -> bool: """Whether object has been opened.""" @@ -322,58 +322,16 @@ async def close(self) -> None: if hasattr(obj, "close"): await obj.close() - # join watchdog and then all threads - if self._watchdog_task and not self._watchdog_task.done(): - self._watchdog_task.cancel() - for func, (task, restart) in self._background_tasks.items(): - if task and not task.done(): - task.cancel() - - @staticmethod - async def _background_func(target: Callable[..., Coroutine[Any, Any, None]]) -> None: - """Run given function. - - Args: - target: Function to run. - """ - try: - await target() - - except asyncio.CancelledError: - # task was canceled - return + self._stop_background_tasks() - except: - log.exception("Exception in thread method %s." % target.__name__) + def _stop_background_tasks(self) -> None: + for task, _ in self._background_tasks: + task.stop() def quit(self) -> None: """Can be overloaded to quit program.""" pass - async def _watchdog(self) -> None: - """Watchdog thread that tries to restart threads if they quit.""" - - while True: - # get dead taks that need to be restarted - dead = {} - for func, (task, restart) in self._background_tasks.items(): - if task.done(): - dead[func] = restart - - # restart dead tasks or quit - for func, restart in dead.items(): - if restart: - log.error("Background task for %s has died, restarting...", func.__name__) - del self._background_tasks[func] - self._background_tasks[func] = (asyncio.create_task(self._background_func(func)), restart) - else: - log.error("Background task for %s has died, quitting...", func.__name__) - self.quit() - return - - # sleep a little - await asyncio.sleep(1) - @overload def get_object( self, From b2fd98b31fbab5bb19d1e1b9c6b85fbfa6df883c Mon Sep 17 00:00:00 2001 From: GermanHydrogen Date: Thu, 21 Mar 2024 15:37:43 +0100 Subject: [PATCH 07/15] Added unit tests to background task --- pyobs/background_task.py | 11 +++---- tests/test_background_task.py | 61 +++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 7 deletions(-) create mode 100644 tests/test_background_task.py diff --git a/pyobs/background_task.py b/pyobs/background_task.py index cabe0d47..f2e6d809 100644 --- a/pyobs/background_task.py +++ b/pyobs/background_task.py @@ -12,7 +12,8 @@ def __init__(self, func: Callable[..., Coroutine[Any, Any, None]], restart: bool self._task: Optional[asyncio.Future] = None def start(self) -> None: - self._start_task() + self._task = asyncio.create_task(self._func()) + self._task.add_done_callback(self._callback_function) def _callback_function(self, args=None) -> None: try: @@ -21,18 +22,14 @@ def _callback_function(self, args=None) -> None: return if exception is not None: - log.exception("Exception in thread method %s." % self._func.__name__) + log.exception("Exception in task %s." % self._func.__name__) if self._restart: log.error("Background task for %s has died, restarting...", self._func.__name__) - self._start_task() + self.start() else: log.error("Background task for %s has died, quitting...", self._func.__name__) - def _start_task(self) -> None: - self._task = asyncio.create_task(self._func()) - self._task.add_done_callback(self._callback_function) - def stop(self) -> None: if self._task is not None: self._task.cancel() diff --git a/tests/test_background_task.py b/tests/test_background_task.py new file mode 100644 index 00000000..b42e87d2 --- /dev/null +++ b/tests/test_background_task.py @@ -0,0 +1,61 @@ +import asyncio +import logging +from unittest.mock import AsyncMock, Mock + +import pytest + +from pyobs.background_task import BackgroundTask + + +@pytest.mark.asyncio +async def test_callback_canceled(caplog): + test_function = AsyncMock() + task = asyncio.create_task(test_function()) + task.exception = Mock(side_effect=asyncio.CancelledError()) + + bg_task = BackgroundTask(test_function, False) + bg_task._task = task + + with caplog.at_level(logging.ERROR): + bg_task._callback_function() + + assert len(caplog.messages) == 0 + + +@pytest.mark.asyncio +async def test_callback_exception(caplog): + test_function = AsyncMock() + test_function.__name__ = "test_function" + + task = asyncio.create_task(test_function()) + task.exception = Mock(return_value=Exception("")) + + bg_task = BackgroundTask(test_function, False) + bg_task._task = task + + with caplog.at_level(logging.ERROR): + bg_task._callback_function() + + assert caplog.messages[0] == "Exception in task test_function." + assert caplog.messages[1] == "Background task for test_function has died, quitting..." + + +@pytest.mark.asyncio +async def test_callback_restart(caplog): + test_function = AsyncMock() + test_function.__name__ = "test_function" + + task = asyncio.create_task(test_function()) + task.exception = Mock(return_value=None) + + bg_task = BackgroundTask(test_function, True) + bg_task._task = task + + bg_task.start = Mock() + + with caplog.at_level(logging.ERROR): + bg_task._callback_function() + + assert caplog.messages[0] == "Background task for test_function has died, restarting..." + bg_task.start.assert_called_once() + From ff9ab2399cdaa13b1beeee14637b789f490a49b3 Mon Sep 17 00:00:00 2001 From: Tim-Oliver Husser Date: Thu, 21 Mar 2024 21:00:55 +0100 Subject: [PATCH 08/15] new auto-docs --- docs/source/api/interfaces.rst | 16 ++++++++++++++++ docs/source/api/vfs.rst | 5 +++++ docs/source/modules/pyobs.modules.robotic.rst | 7 +++++++ 3 files changed, 28 insertions(+) diff --git a/docs/source/api/interfaces.rst b/docs/source/api/interfaces.rst index 33c069b4..84778c53 100644 --- a/docs/source/api/interfaces.rst +++ b/docs/source/api/interfaces.rst @@ -195,6 +195,14 @@ ILatLon :show-inheritance: :undoc-members: +IMode +~~~~~ + +.. autoclass:: pyobs.interfaces.IMode + :members: + :show-inheritance: + :undoc-members: + IModule ~~~~~~~ @@ -243,6 +251,14 @@ IPointingHGS :show-inheritance: :undoc-members: +IPointingHelioprojective +~~~~~~~~~~~~~~~~~~~~~~~~ + +.. autoclass:: pyobs.interfaces.IPointingHelioprojective + :members: + :show-inheritance: + :undoc-members: + IPointingRaDec ~~~~~~~~~~~~~~ diff --git a/docs/source/api/vfs.rst b/docs/source/api/vfs.rst index 09503810..c1005982 100644 --- a/docs/source/api/vfs.rst +++ b/docs/source/api/vfs.rst @@ -31,6 +31,11 @@ MemoryFile .. autoclass:: pyobs.vfs.MemoryFile +SFTPFile +^^^^^^^^ + +.. autoclass:: pyobs.vfs.SFTPFile + SMBFile ^^^^^^^ diff --git a/docs/source/modules/pyobs.modules.robotic.rst b/docs/source/modules/pyobs.modules.robotic.rst index 29807b6b..57d54a81 100644 --- a/docs/source/modules/pyobs.modules.robotic.rst +++ b/docs/source/modules/pyobs.modules.robotic.rst @@ -24,3 +24,10 @@ Scheduler :members: :show-inheritance: +ScriptRunner +~~~~~~~~~~~~ + +.. autoclass:: pyobs.modules.robotic.ScriptRunner + :members: + :show-inheritance: + From e5c82013cd20cfd87b05e616657641f3ed8d3ed6 Mon Sep 17 00:00:00 2001 From: Tim-Oliver Husser Date: Thu, 21 Mar 2024 21:13:02 +0100 Subject: [PATCH 09/15] added theme to requirements for readthedocs --- .readthedocs.yml | 1 + docs/requirements.txt | 1 + 2 files changed, 2 insertions(+) create mode 100644 docs/requirements.txt diff --git a/.readthedocs.yml b/.readthedocs.yml index 2dee87bd..e9cddd78 100644 --- a/.readthedocs.yml +++ b/.readthedocs.yml @@ -12,3 +12,4 @@ python: install: - method: pip path: . + requirements: docs/requirements.txt \ No newline at end of file diff --git a/docs/requirements.txt b/docs/requirements.txt new file mode 100644 index 00000000..4170c03e --- /dev/null +++ b/docs/requirements.txt @@ -0,0 +1 @@ +sphinx-rtd-theme \ No newline at end of file From 937e0611070f1df354d6a58e6a2465ab29d6561f Mon Sep 17 00:00:00 2001 From: Tim-Oliver Husser Date: Thu, 21 Mar 2024 21:15:18 +0100 Subject: [PATCH 10/15] v1.12.7 --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 26da42f3..f0956c86 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,7 +1,7 @@ [tool.poetry] name = "pyobs-core" packages = [{ include = "pyobs" }] -version = "1.12.6" +version = "1.12.7" description = "robotic telescope software" authors = ["Tim-Oliver Husser "] license = "MIT" From 8c6744401720f7f3cdb9d7ee914f70323e8fe26c Mon Sep 17 00:00:00 2001 From: Tim-Oliver Husser Date: Thu, 21 Mar 2024 21:17:03 +0100 Subject: [PATCH 11/15] fixed yaml --- .readthedocs.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.readthedocs.yml b/.readthedocs.yml index e9cddd78..ce1d6a8a 100644 --- a/.readthedocs.yml +++ b/.readthedocs.yml @@ -12,4 +12,4 @@ python: install: - method: pip path: . - requirements: docs/requirements.txt \ No newline at end of file + - requirements: docs/requirements.txt \ No newline at end of file From 8b57d23171a15a3850b9a3e5db8a590ddbd2513f Mon Sep 17 00:00:00 2001 From: Tim-Oliver Husser Date: Thu, 21 Mar 2024 21:17:08 +0100 Subject: [PATCH 12/15] v1.12.8 --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index f0956c86..791e7b7c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,7 +1,7 @@ [tool.poetry] name = "pyobs-core" packages = [{ include = "pyobs" }] -version = "1.12.7" +version = "1.12.8" description = "robotic telescope software" authors = ["Tim-Oliver Husser "] license = "MIT" From 9cbe7d026d58dce9234d77276076f16774943290 Mon Sep 17 00:00:00 2001 From: GermanHydrogen Date: Fri, 22 Mar 2024 09:39:58 +0100 Subject: [PATCH 13/15] Added unit tests to object for background tasks --- pyobs/object.py | 12 ++++++---- tests/test_object.py | 53 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 4 deletions(-) create mode 100644 tests/test_object.py diff --git a/pyobs/object.py b/pyobs/object.py index b9f011df..c076d03d 100644 --- a/pyobs/object.py +++ b/pyobs/object.py @@ -274,7 +274,7 @@ def __init__( self._background_tasks: List[Tuple[BackgroundTask, bool]] = [] def add_background_task(self, func: Callable[..., Coroutine[Any, Any, None]], - restart: bool = True, autostart: bool = True) -> None: + restart: bool = True, autostart: bool = True) -> BackgroundTask: """Add a new function that should be run in the background. MUST be called in constructor of derived class or at least before calling open() on the object. @@ -283,10 +283,14 @@ def add_background_task(self, func: Callable[..., Coroutine[Any, Any, None]], func: Func to add. restart: Whether to restart this function. autostart: Whether to start this function when the module is opened + Returns: + Background task """ background_task = BackgroundTask(func, restart) - self._background_tasks = (background_task, autostart) + self._background_tasks.append((background_task, autostart)) + + return background_task async def open(self) -> None: """Open module.""" @@ -304,9 +308,9 @@ async def open(self) -> None: # success self._opened = True - def _perform_background_task_autostart(self): + def _perform_background_task_autostart(self) -> None: todo = filter(lambda b: b[1] is True, self._background_tasks) - for task in todo: + for task, _ in todo: task.start() @property diff --git a/tests/test_object.py b/tests/test_object.py new file mode 100644 index 00000000..ec2916eb --- /dev/null +++ b/tests/test_object.py @@ -0,0 +1,53 @@ +from unittest.mock import AsyncMock + +import pyobs +from pyobs.background_task import BackgroundTask +from pyobs.object import Object + + +def test_add_background_task(): + obj = Object() + test_function = AsyncMock() + + task = obj.add_background_task(test_function, False, False) + + assert task._func == test_function + assert task._restart is False + + assert obj._background_tasks[0] == (task, False) + + +def test_perform_background_task_autostart(mocker): + mocker.patch("pyobs.background_task.BackgroundTask.start") + + obj = Object() + test_function = AsyncMock() + + obj.add_background_task(test_function, False, True) + obj._perform_background_task_autostart() + + pyobs.background_task.BackgroundTask.start.assert_called_once() + + +def test_perform_background_task_no_autostart(mocker): + mocker.patch("pyobs.background_task.BackgroundTask.start") + + obj = Object() + test_function = AsyncMock() + + obj.add_background_task(test_function, False, False) + obj._perform_background_task_autostart() + + pyobs.background_task.BackgroundTask.start.assert_not_called() + + +def test_stop_background_task(mocker): + mocker.patch("pyobs.background_task.BackgroundTask.stop") + + obj = Object() + test_function = AsyncMock() + + obj.add_background_task(test_function, False, False) + obj._stop_background_tasks() + + pyobs.background_task.BackgroundTask.stop.assert_called_once() \ No newline at end of file From e3beb32cc7dec329f134bc2f5b36906b333f656f Mon Sep 17 00:00:00 2001 From: GermanHydrogen Date: Fri, 22 Mar 2024 10:22:30 +0100 Subject: [PATCH 14/15] Added handling of pyobs errors to background task --- pyobs/background_task.py | 8 ++++++-- tests/test_background_task.py | 21 ++++++++++++++++++--- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/pyobs/background_task.py b/pyobs/background_task.py index f2e6d809..6bcc23a6 100644 --- a/pyobs/background_task.py +++ b/pyobs/background_task.py @@ -2,6 +2,8 @@ import logging from typing import Optional, Coroutine, Any, Callable +from pyobs.utils.exceptions import PyObsError + log = logging.getLogger(__name__) @@ -21,8 +23,10 @@ def _callback_function(self, args=None) -> None: except asyncio.CancelledError: return - if exception is not None: - log.exception("Exception in task %s." % self._func.__name__) + if isinstance(exception, PyObsError): + raise exception + elif exception is not None: + log.error("Exception %s in task %s.", exception, self._func.__name__) if self._restart: log.error("Background task for %s has died, restarting...", self._func.__name__) diff --git a/tests/test_background_task.py b/tests/test_background_task.py index b42e87d2..8c4f6d03 100644 --- a/tests/test_background_task.py +++ b/tests/test_background_task.py @@ -3,7 +3,7 @@ from unittest.mock import AsyncMock, Mock import pytest - +import pyobs.utils.exceptions as exc from pyobs.background_task import BackgroundTask @@ -28,7 +28,7 @@ async def test_callback_exception(caplog): test_function.__name__ = "test_function" task = asyncio.create_task(test_function()) - task.exception = Mock(return_value=Exception("")) + task.exception = Mock(return_value=Exception("TestError")) bg_task = BackgroundTask(test_function, False) bg_task._task = task @@ -36,10 +36,25 @@ async def test_callback_exception(caplog): with caplog.at_level(logging.ERROR): bg_task._callback_function() - assert caplog.messages[0] == "Exception in task test_function." + assert caplog.messages[0] == "Exception TestError in task test_function." assert caplog.messages[1] == "Background task for test_function has died, quitting..." +@pytest.mark.asyncio +async def test_callback_pyobs_error(): + test_function = AsyncMock() + test_function.__name__ = "test_function" + + task = asyncio.create_task(test_function()) + task.exception = Mock(return_value=exc.ImageError("TestError")) + + bg_task = BackgroundTask(test_function, False) + bg_task._task = task + + with pytest.raises(exc.ImageError): + bg_task._callback_function() + + @pytest.mark.asyncio async def test_callback_restart(caplog): test_function = AsyncMock() From 0b37f2eb6b25e0fc66e2088e7d9a564898be6d63 Mon Sep 17 00:00:00 2001 From: GermanHydrogen Date: Fri, 22 Mar 2024 15:42:59 +0100 Subject: [PATCH 15/15] Changed excn filter in background task to severe error --- pyobs/background_task.py | 4 ++-- tests/test_background_task.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pyobs/background_task.py b/pyobs/background_task.py index 6bcc23a6..0ae82f11 100644 --- a/pyobs/background_task.py +++ b/pyobs/background_task.py @@ -2,7 +2,7 @@ import logging from typing import Optional, Coroutine, Any, Callable -from pyobs.utils.exceptions import PyObsError +from pyobs.utils.exceptions import SevereError log = logging.getLogger(__name__) @@ -23,7 +23,7 @@ def _callback_function(self, args=None) -> None: except asyncio.CancelledError: return - if isinstance(exception, PyObsError): + if isinstance(exception, SevereError): raise exception elif exception is not None: log.error("Exception %s in task %s.", exception, self._func.__name__) diff --git a/tests/test_background_task.py b/tests/test_background_task.py index 8c4f6d03..f218a762 100644 --- a/tests/test_background_task.py +++ b/tests/test_background_task.py @@ -46,12 +46,12 @@ async def test_callback_pyobs_error(): test_function.__name__ = "test_function" task = asyncio.create_task(test_function()) - task.exception = Mock(return_value=exc.ImageError("TestError")) + task.exception = Mock(return_value=exc.SevereError(exc.ImageError("TestError"))) bg_task = BackgroundTask(test_function, False) bg_task._task = task - with pytest.raises(exc.ImageError): + with pytest.raises(exc.SevereError): bg_task._callback_function()