From f5cf28fc369f5c88b4d8f74add51bfe48672fe07 Mon Sep 17 00:00:00 2001
From: GermanHydrogen <admin@germanhydrogen.org>
Date: Wed, 20 Mar 2024 12:10:35 +0100
Subject: [PATCH 01/25] Added unit tests to mastermind

---
 pyobs/modules/robotic/mastermind.py      | 126 ++++++------
 tests/modules/robotic/test_mastermind.py | 237 +++++++++++++++++++++++
 2 files changed, 301 insertions(+), 62 deletions(-)
 create mode 100644 tests/modules/robotic/test_mastermind.py

diff --git a/pyobs/modules/robotic/mastermind.py b/pyobs/modules/robotic/mastermind.py
index 21a4d6015..a415377f8 100644
--- a/pyobs/modules/robotic/mastermind.py
+++ b/pyobs/modules/robotic/mastermind.py
@@ -53,6 +53,8 @@ def __init__(
         self._obs = None
         self._exp = None
 
+        self._first_late_start_warning = True
+
     async def open(self) -> None:
         """Open module."""
         await Module.open(self)
@@ -83,72 +85,72 @@ async def _run_thread(self) -> None:
         # wait a little
         await asyncio.sleep(1)
 
-        # flags
-        first_late_start_warning = True
-
         # run until closed
         while True:
-            # not running?
-            if not self._running:
-                # sleep a little and continue
-                await asyncio.sleep(1)
-                continue
-
-            # get now
-            now = Time.now()
-
-            # find task that we want to run now
-            task: Optional[Task] = await self._task_schedule.get_task(now)
-            if task is None or not await self._task_runner.can_run(task):
-                # no task found
+            await self._loop()
+
+    async def _loop(self):
+        # not running?
+        if not self._running:
+            # sleep a little and continue
+            await asyncio.sleep(1)
+            return
+
+        # get now
+        now = Time.now()
+
+        # find task that we want to run now
+        task: Optional[Task] = await self._task_schedule.get_task(now)
+        if task is None or not await self._task_runner.can_run(task):
+            # no task found
+            await asyncio.sleep(10)
+            return
+
+        # starting too late?
+        if not task.can_start_late:
+            late_start = now - task.start
+            if late_start > self._allowed_late_start * u.second:
+                # only warn once
+                if self._first_late_start_warning:
+                    log.warning(
+                        "Time since start of window (%.1f) too long (>%.1f), skipping task...",
+                        late_start.to_value("second"),
+                        self._allowed_late_start,
+                    )
+                self._first_late_start_warning = False
+
+                # sleep a little and skip
                 await asyncio.sleep(10)
-                continue
-
-            # starting too late?
-            if not task.can_start_late:
-                late_start = now - task.start
-                if late_start > self._allowed_late_start * u.second:
-                    # only warn once
-                    if first_late_start_warning:
-                        log.warning(
-                            "Time since start of window (%.1f) too long (>%.1f), skipping task...",
-                            late_start.to_value("second"),
-                            self._allowed_late_start,
-                        )
-                    first_late_start_warning = False
-
-                    # sleep a little and skip
-                    await asyncio.sleep(10)
-                    continue
-
-            # reset warning
-            first_late_start_warning = True
-
-            # task is definitely not None here
-            self._task = cast(Task, task)
-
-            # ETA
-            eta = now + self._task.duration * u.second
-
-            # send event
-            await self.comm.send_event(TaskStartedEvent(name=self._task.name, id=self._task.id, eta=eta))
-
-            # run task in thread
-            log.info("Running task %s...", self._task.name)
-            try:
-                await self._task_runner.run_task(self._task, task_schedule=self._task_schedule)
-            except:
-                # something went wrong
-                log.warning("Task %s failed.", self._task.name)
-                self._task = None
-                continue
-
-            # send event
-            await self.comm.send_event(TaskFinishedEvent(name=self._task.name, id=self._task.id))
-
-            # finish
-            log.info("Finished task %s.", self._task.name)
+                return
+
+        # reset warning
+        self._first_late_start_warning = True
+
+        # task is definitely not None here
+        self._task = cast(Task, task)
+
+        # ETA
+        eta = now + self._task.duration * u.second
+
+        # send event
+        await self.comm.send_event(TaskStartedEvent(name=self._task.name, id=self._task.id, eta=eta))
+
+        # run task in thread
+        log.info("Running task %s...", self._task.name)
+        try:
+            await self._task_runner.run_task(self._task, task_schedule=self._task_schedule)
+        except:
+            # something went wrong
+            log.warning("Task %s failed.", self._task.name)
             self._task = None
+            return
+
+        # send event
+        await self.comm.send_event(TaskFinishedEvent(name=self._task.name, id=self._task.id))
+
+        # finish
+        log.info("Finished task %s.", self._task.name)
+        self._task = None
 
     async def get_fits_header_before(
         self, namespaces: Optional[List[str]] = None, **kwargs: Any
diff --git a/tests/modules/robotic/test_mastermind.py b/tests/modules/robotic/test_mastermind.py
new file mode 100644
index 000000000..e7e098dec
--- /dev/null
+++ b/tests/modules/robotic/test_mastermind.py
@@ -0,0 +1,237 @@
+import asyncio
+import logging
+from datetime import timedelta
+from typing import Optional, Dict, List, Any, Tuple
+from unittest.mock import AsyncMock
+
+import pytest
+from astroplan import ObservingBlock
+
+import pyobs
+from pyobs.events import TaskStartedEvent, TaskFinishedEvent
+from pyobs.modules.robotic import Mastermind
+from pyobs.robotic import TaskSchedule, TaskRunner, Task, TaskArchive
+from pyobs.robotic.scripts import Script
+from pyobs.utils.time import Time
+
+
+class TestTaskScheduler(TaskSchedule):
+    async def set_schedule(self, blocks: List[ObservingBlock], start_time: Time) -> None:
+        pass
+
+    async def last_scheduled(self) -> Optional[Time]:
+        pass
+
+    async def get_schedule(self) -> Dict[str, Task]:
+        pass
+
+    async def get_task(self, time: Time) -> Optional[Task]:
+        pass
+
+
+class TestTask(Task):
+    def __init__(self, start: Time = None, can_start_late: bool = False, **kwargs: Any):
+        super().__init__(**kwargs)
+
+        if start is None:
+            self._start = Time.now()
+        else:
+            self._start = start
+
+        self._can_start_late = can_start_late
+
+    @property
+    def id(self) -> Any:
+        return 0
+
+    @property
+    def name(self) -> str:
+        return "Task"
+
+    @property
+    def duration(self) -> float:
+        return 0
+
+    @property
+    def start(self) -> Time:
+        return self._start
+
+    @property
+    def end(self) -> Time:
+        pass
+
+    async def can_run(self, scripts: Optional[Dict[str, Script]] = None) -> bool:
+        pass
+
+    @property
+    def can_start_late(self) -> bool:
+        return self._can_start_late
+
+    async def run(self, task_runner: TaskRunner, task_schedule: Optional[TaskSchedule] = None,
+                  task_archive: Optional[TaskArchive] = None, scripts: Optional[Dict[str, Script]] = None) -> None:
+        pass
+
+    def is_finished(self) -> bool:
+        pass
+
+    def get_fits_headers(self, namespaces: Optional[List[str]] = None) -> Dict[str, Tuple[Any, str]]:
+        return {"TASK-HDR": (0, "")}
+
+
+@pytest.mark.asyncio
+async def test_open(mocker):
+    mocker.patch("pyobs.modules.Module.open")
+    master = Mastermind(TestTaskScheduler(), TaskRunner())
+
+    master.comm.register_event = AsyncMock()
+
+    await master.open()
+
+    pyobs.modules.Module.open.assert_called_once()
+    assert master.comm.register_event.call_args_list[0][0][0] == TaskStartedEvent
+    assert master.comm.register_event.call_args_list[1][0][0] == TaskFinishedEvent
+
+
+@pytest.mark.asyncio
+async def test_start():
+    master = Mastermind(TestTaskScheduler(), TaskRunner())
+    await master.start()
+    assert await master.is_running() is True
+
+
+@pytest.mark.asyncio
+async def test_stop():
+    master = Mastermind(TestTaskScheduler(), TaskRunner())
+    await master.stop()
+    assert await master.is_running() is False
+
+
+@pytest.mark.asyncio
+async def test_loop_not_running(mocker):
+    mocker.patch("asyncio.sleep")
+    master = Mastermind(TestTaskScheduler(), TaskRunner())
+
+    await master._loop()
+    asyncio.sleep.assert_called_once_with(1)
+
+
+@pytest.mark.asyncio
+async def test_loop_not_task(mocker):
+    mocker.patch("asyncio.sleep")
+    scheduler = TestTaskScheduler()
+    scheduler.get_task = AsyncMock(return_value=None)
+    master = Mastermind(scheduler, TaskRunner())
+    await master.start()
+
+    await master._loop()
+    asyncio.sleep.assert_called_once_with(10)
+
+
+@pytest.mark.asyncio
+async def test_loop_not_runnable_task(mocker):
+    mocker.patch("asyncio.sleep")
+    scheduler = TestTaskScheduler()
+    scheduler.get_task = AsyncMock(return_value=TestTask())
+
+    runner = TaskRunner()
+    runner.can_run = AsyncMock(return_value=False)
+
+    master = Mastermind(scheduler, runner)
+    await master.start()
+
+    await master._loop()
+    asyncio.sleep.assert_called_once_with(10)
+
+
+@pytest.mark.asyncio
+async def test_loop_late_start(mocker, caplog):
+    mocker.patch("asyncio.sleep")
+    task = TestTask(Time.now() - timedelta(seconds=400), False)
+
+    scheduler = TestTaskScheduler()
+    scheduler.get_task = AsyncMock(return_value=task)
+
+    runner = TaskRunner()
+    runner.can_run = AsyncMock(return_value=True)
+
+    master = Mastermind(scheduler, runner)
+    await master.start()
+
+    with caplog.at_level(logging.WARNING):
+        await master._loop()
+        await master._loop()
+
+    assert caplog.messages[0] == "Time since start of window (400.0) too long (>300.0), skipping task..."
+
+    assert len(caplog.messages) == 1    # Test that only the first task throws error msg
+
+    asyncio.sleep.assert_called_with(10)
+
+
+@pytest.mark.asyncio
+async def test_loop_valid(mocker):
+    mocker.patch("asyncio.sleep")
+    task = TestTask(can_start_late=False)
+
+    scheduler = TestTaskScheduler()
+    scheduler.get_task = AsyncMock(return_value=task)
+
+    runner = TaskRunner()
+    runner.can_run = AsyncMock(return_value=True)
+    runner.run_task = AsyncMock()
+
+    master = Mastermind(scheduler, runner)
+    await master.start()
+
+    await master._loop()
+
+    asyncio.sleep.assert_not_called()
+    runner.run_task.assert_awaited_with(task, task_schedule=scheduler)
+
+    assert master._task is None
+
+
+@pytest.mark.asyncio
+async def test_loop_failed_task(mocker, caplog):
+    mocker.patch("asyncio.sleep")
+    task = TestTask(can_start_late=False)
+
+    scheduler = TestTaskScheduler()
+    scheduler.get_task = AsyncMock(return_value=task)
+
+    runner = TaskRunner()
+    runner.can_run = AsyncMock(return_value=True)
+    runner.run_task = AsyncMock(side_effect=Exception(""))
+
+    master = Mastermind(scheduler, runner)
+    await master.start()
+
+    with caplog.at_level(logging.WARNING):
+        await master._loop()
+
+    asyncio.sleep.assert_not_called()
+
+    assert caplog.messages[0] == "Task Task failed."
+    assert master._task is None
+
+
+@pytest.mark.asyncio
+async def test_get_fits_header_before_without_task():
+    master = Mastermind(TestTaskScheduler(), TaskRunner())
+
+    header = await master.get_fits_header_before()
+
+    assert header == {}
+
+
+@pytest.mark.asyncio
+async def test_get_fits_header_before_with_task():
+    master = Mastermind(TestTaskScheduler(), TaskRunner())
+
+    master._task = TestTask()
+
+    header = await master.get_fits_header_before()
+
+    assert header["TASK"] == ("Task", "Name of task")
+    assert header["REQNUM"] == ("0", "Unique ID of task")
+    assert header["TASK-HDR"] == (0, "")

From 45a771fd4572957d0d9495a0d956f413e74ce6f6 Mon Sep 17 00:00:00 2001
From: GermanHydrogen <admin@germanhydrogen.org>
Date: Wed, 20 Mar 2024 14:29:16 +0100
Subject: [PATCH 02/25] Cleaned up main loopmastermind

---
 pyobs/modules/robotic/mastermind.py | 78 +++++++++++++----------------
 1 file changed, 36 insertions(+), 42 deletions(-)

diff --git a/pyobs/modules/robotic/mastermind.py b/pyobs/modules/robotic/mastermind.py
index a415377f8..b171da75b 100644
--- a/pyobs/modules/robotic/mastermind.py
+++ b/pyobs/modules/robotic/mastermind.py
@@ -20,12 +20,12 @@ class Mastermind(Module, IAutonomous, IFitsHeaderBefore):
     __module__ = "pyobs.modules.robotic"
 
     def __init__(
-        self,
-        schedule: Union[TaskSchedule, Dict[str, Any]],
-        runner: Union[TaskRunner, Dict[str, Any]],
-        allowed_late_start: int = 300,
-        allowed_overrun: int = 300,
-        **kwargs: Any,
+            self,
+            schedule: Union[TaskSchedule, Dict[str, Any]],
+            runner: Union[TaskRunner, Dict[str, Any]],
+            allowed_late_start: int = 300,
+            allowed_overrun: int = 300,
+            **kwargs: Any,
     ):
         """Initialize a new auto focus system.
 
@@ -49,7 +49,7 @@ def __init__(
         self._task_runner = self.add_child_object(runner, TaskRunner)
 
         # observation name and exposure number
-        self._task = None
+        self._task: Optional[Task] = None
         self._obs = None
         self._exp = None
 
@@ -96,64 +96,58 @@ async def _loop(self):
             await asyncio.sleep(1)
             return
 
-        # get now
         now = Time.now()
 
-        # find task that we want to run now
-        task: Optional[Task] = await self._task_schedule.get_task(now)
-        if task is None or not await self._task_runner.can_run(task):
-            # no task found
+        self._task = await self._task_schedule.get_task(now)
+
+        if self._task is None or not await self._task_runner.can_run(self._task):
             await asyncio.sleep(10)
             return
 
-        # starting too late?
-        if not task.can_start_late:
-            late_start = now - task.start
-            if late_start > self._allowed_late_start * u.second:
-                # only warn once
-                if self._first_late_start_warning:
-                    log.warning(
-                        "Time since start of window (%.1f) too long (>%.1f), skipping task...",
-                        late_start.to_value("second"),
-                        self._allowed_late_start,
-                    )
-                self._first_late_start_warning = False
-
-                # sleep a little and skip
-                await asyncio.sleep(10)
-                return
-
-        # reset warning
-        self._first_late_start_warning = True
+        if not self._task.can_start_late and self._check_is_task_late(now):
+            await asyncio.sleep(10)
+            return
 
-        # task is definitely not None here
-        self._task = cast(Task, task)
+        await self._execute_task(now)
 
-        # ETA
-        eta = now + self._task.duration * u.second
+        self._remove_task()
 
-        # send event
+    def _check_is_task_late(self, now: Time) -> bool:
+        time_since_planned_start = now - self._task.start
+        is_late_start = time_since_planned_start > self._allowed_late_start * u.second
+
+        if is_late_start and self._first_late_start_warning:
+            log.warning(
+                "Time since start of window (%.1f) too long (>%.1f), skipping task...",
+                time_since_planned_start.to_value("second"),
+                self._allowed_late_start,
+            )
+            self._first_late_start_warning = False
+        else:
+            self._first_late_start_warning = True
+
+        return is_late_start
+
+    async def _execute_task(self, now: Time) -> None:
+        eta = now + self._task.duration * u.second
         await self.comm.send_event(TaskStartedEvent(name=self._task.name, id=self._task.id, eta=eta))
 
-        # run task in thread
         log.info("Running task %s...", self._task.name)
         try:
             await self._task_runner.run_task(self._task, task_schedule=self._task_schedule)
         except:
-            # something went wrong
             log.warning("Task %s failed.", self._task.name)
-            self._task = None
             return
 
-        # send event
         await self.comm.send_event(TaskFinishedEvent(name=self._task.name, id=self._task.id))
 
-        # finish
         log.info("Finished task %s.", self._task.name)
+
+    def _remove_task(self) -> None:
         self._task = None
 
     async def get_fits_header_before(
-        self, namespaces: Optional[List[str]] = None, **kwargs: Any
+            self, namespaces: Optional[List[str]] = None, **kwargs: Any
     ) -> Dict[str, Tuple[Any, str]]:
         """Returns FITS header for the current status of this module.
 

From ccee05dcb32382eb6ebf17211010605348241391 Mon Sep 17 00:00:00 2001
From: GermanHydrogen <admin@germanhydrogen.org>
Date: Wed, 20 Mar 2024 14:41:36 +0100
Subject: [PATCH 03/25] Removed depricated code from mastermind

---
 pyobs/modules/robotic/mastermind.py | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/pyobs/modules/robotic/mastermind.py b/pyobs/modules/robotic/mastermind.py
index b171da75b..207b86931 100644
--- a/pyobs/modules/robotic/mastermind.py
+++ b/pyobs/modules/robotic/mastermind.py
@@ -36,22 +36,16 @@ def __init__(
         """
         Module.__init__(self, **kwargs)
 
-        # store
         self._allowed_late_start = allowed_late_start
         self._allowed_overrun = allowed_overrun
-        self._running = False
 
-        # add thread func
+        self._running = False
         self.add_background_task(self._run_thread, True)
 
-        # get schedule and runner
         self._task_schedule = self.add_child_object(schedule, TaskSchedule)
         self._task_runner = self.add_child_object(runner, TaskRunner)
 
-        # observation name and exposure number
         self._task: Optional[Task] = None
-        self._obs = None
-        self._exp = None
 
         self._first_late_start_warning = True
 
@@ -59,12 +53,9 @@ async def open(self) -> None:
         """Open module."""
         await Module.open(self)
 
-        # subscribe to events
-        if self.comm:
-            await self.comm.register_event(TaskStartedEvent)
-            await self.comm.register_event(TaskFinishedEvent)
+        await self.comm.register_event(TaskStartedEvent)
+        await self.comm.register_event(TaskFinishedEvent)
 
-        # start
         self._running = True
 
     async def start(self, **kwargs: Any) -> None:

From f86557d1648864e4b4bd64d119d6b1fd85175ec8 Mon Sep 17 00:00:00 2001
From: GermanHydrogen <admin@germanhydrogen.org>
Date: Wed, 27 Mar 2024 09:55:30 +0100
Subject: [PATCH 04/25] Added is running method to background task

---
 pyobs/background_task.py      | 8 +++++++-
 tests/test_background_task.py | 4 ++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/pyobs/background_task.py b/pyobs/background_task.py
index 0ae82f119..dbf153c46 100644
--- a/pyobs/background_task.py
+++ b/pyobs/background_task.py
@@ -35,5 +35,11 @@ def _callback_function(self, args=None) -> None:
             log.error("Background task for %s has died, quitting...", self._func.__name__)
 
     def stop(self) -> None:
-        if self._task is not None:
+        if self.is_running():
             self._task.cancel()
+
+    def is_running(self) -> bool:
+        if self._task is None:
+            return False
+
+        return self._task.done()
diff --git a/tests/test_background_task.py b/tests/test_background_task.py
index f218a762f..bdf67bfa9 100644
--- a/tests/test_background_task.py
+++ b/tests/test_background_task.py
@@ -74,3 +74,7 @@ async def test_callback_restart(caplog):
     assert caplog.messages[0] == "Background task for test_function has died, restarting..."
     bg_task.start.assert_called_once()
 
+
+def test_is_running_not_started():
+    bg_task = BackgroundTask(AsyncMock(), True)
+    assert bg_task.is_running() is False

From 29a29739db11bca30fbaebbee72ac61b3bf8097e Mon Sep 17 00:00:00 2001
From: GermanHydrogen <admin@germanhydrogen.org>
Date: Wed, 27 Mar 2024 10:03:07 +0100
Subject: [PATCH 05/25] Updated background task in mastermind

---
 pyobs/modules/robotic/mastermind.py      | 19 ++++---------------
 tests/modules/robotic/test_mastermind.py | 23 ++++++++++-------------
 2 files changed, 14 insertions(+), 28 deletions(-)

diff --git a/pyobs/modules/robotic/mastermind.py b/pyobs/modules/robotic/mastermind.py
index 207b86931..b03c5425a 100644
--- a/pyobs/modules/robotic/mastermind.py
+++ b/pyobs/modules/robotic/mastermind.py
@@ -39,8 +39,7 @@ def __init__(
         self._allowed_late_start = allowed_late_start
         self._allowed_overrun = allowed_overrun
 
-        self._running = False
-        self.add_background_task(self._run_thread, True)
+        self._mastermind_loop = self.add_background_task(self._run_thread, True, True)
 
         self._task_schedule = self.add_child_object(schedule, TaskSchedule)
         self._task_runner = self.add_child_object(runner, TaskRunner)
@@ -56,37 +55,27 @@ async def open(self) -> None:
         await self.comm.register_event(TaskStartedEvent)
         await self.comm.register_event(TaskFinishedEvent)
 
-        self._running = True
-
     async def start(self, **kwargs: Any) -> None:
         """Starts a service."""
         log.info("Starting robotic system...")
-        self._running = True
+        self._mastermind_loop.start()
 
     async def stop(self, **kwargs: Any) -> None:
         """Stops a service."""
         log.info("Stopping robotic system...")
-        self._running = False
+        self._mastermind_loop.stop()
 
     async def is_running(self, **kwargs: Any) -> bool:
         """Whether a service is running."""
-        return self._running
+        return self._mastermind_loop.is_running()
 
     async def _run_thread(self) -> None:
-        # wait a little
         await asyncio.sleep(1)
 
-        # run until closed
         while True:
             await self._loop()
 
     async def _loop(self):
-        # not running?
-        if not self._running:
-            # sleep a little and continue
-            await asyncio.sleep(1)
-            return
-
         now = Time.now()
 
         self._task = await self._task_schedule.get_task(now)
diff --git a/tests/modules/robotic/test_mastermind.py b/tests/modules/robotic/test_mastermind.py
index e7e098dec..ed139b821 100644
--- a/tests/modules/robotic/test_mastermind.py
+++ b/tests/modules/robotic/test_mastermind.py
@@ -2,7 +2,7 @@
 import logging
 from datetime import timedelta
 from typing import Optional, Dict, List, Any, Tuple
-from unittest.mock import AsyncMock
+from unittest.mock import AsyncMock, Mock
 
 import pytest
 from astroplan import ObservingBlock
@@ -95,24 +95,26 @@ async def test_open(mocker):
 @pytest.mark.asyncio
 async def test_start():
     master = Mastermind(TestTaskScheduler(), TaskRunner())
+    master._mastermind_loop.start = Mock()
     await master.start()
-    assert await master.is_running() is True
+
+    master._mastermind_loop.start.assert_called_once()
 
 
 @pytest.mark.asyncio
 async def test_stop():
     master = Mastermind(TestTaskScheduler(), TaskRunner())
+    master._mastermind_loop.stop = Mock()
     await master.stop()
-    assert await master.is_running() is False
+
+    master._mastermind_loop.stop.assert_called_once()
 
 
 @pytest.mark.asyncio
-async def test_loop_not_running(mocker):
-    mocker.patch("asyncio.sleep")
+async def test_is_running():
     master = Mastermind(TestTaskScheduler(), TaskRunner())
-
-    await master._loop()
-    asyncio.sleep.assert_called_once_with(1)
+    master._mastermind_loop.is_running = Mock(return_value=True)
+    assert await master.is_running() is True
 
 
 @pytest.mark.asyncio
@@ -121,7 +123,6 @@ async def test_loop_not_task(mocker):
     scheduler = TestTaskScheduler()
     scheduler.get_task = AsyncMock(return_value=None)
     master = Mastermind(scheduler, TaskRunner())
-    await master.start()
 
     await master._loop()
     asyncio.sleep.assert_called_once_with(10)
@@ -137,7 +138,6 @@ async def test_loop_not_runnable_task(mocker):
     runner.can_run = AsyncMock(return_value=False)
 
     master = Mastermind(scheduler, runner)
-    await master.start()
 
     await master._loop()
     asyncio.sleep.assert_called_once_with(10)
@@ -155,7 +155,6 @@ async def test_loop_late_start(mocker, caplog):
     runner.can_run = AsyncMock(return_value=True)
 
     master = Mastermind(scheduler, runner)
-    await master.start()
 
     with caplog.at_level(logging.WARNING):
         await master._loop()
@@ -181,7 +180,6 @@ async def test_loop_valid(mocker):
     runner.run_task = AsyncMock()
 
     master = Mastermind(scheduler, runner)
-    await master.start()
 
     await master._loop()
 
@@ -204,7 +202,6 @@ async def test_loop_failed_task(mocker, caplog):
     runner.run_task = AsyncMock(side_effect=Exception(""))
 
     master = Mastermind(scheduler, runner)
-    await master.start()
 
     with caplog.at_level(logging.WARNING):
         await master._loop()

From f0a3f5ffe455a42b64a193be9b23e35e625712bb Mon Sep 17 00:00:00 2001
From: GermanHydrogen <admin@germanhydrogen.org>
Date: Wed, 27 Mar 2024 11:23:59 +0100
Subject: [PATCH 06/25] Refactoreesisting g schedular unit test

---
 tests/modules/robotic/test_scheduler.py | 82 +++++++++++--------------
 1 file changed, 37 insertions(+), 45 deletions(-)

diff --git a/tests/modules/robotic/test_scheduler.py b/tests/modules/robotic/test_scheduler.py
index edc6e24bf..5ff2afa23 100644
--- a/tests/modules/robotic/test_scheduler.py
+++ b/tests/modules/robotic/test_scheduler.py
@@ -1,67 +1,59 @@
-import time
+from typing import List
 
-from astroplan import ObservingBlock, FixedTarget
 import astropy.units as u
+import pytest
+from astroplan import ObservingBlock, FixedTarget
 from astropy.coordinates import SkyCoord
 
 from pyobs.modules.robotic import Scheduler
 
 
-def test_compare_block_lists():
-    # create lists of blocks
-    blocks = []
-    for i in range(10):
-        blocks.append(
-            ObservingBlock(
-                FixedTarget(SkyCoord(0.0 * u.deg, 0.0 * u.deg, frame="icrs"), name=str(i)), 10 * u.minute, 10
-            )
+@pytest.fixture
+def schedule_blocks() -> List[ObservingBlock]:
+    blocks = [
+        ObservingBlock(
+            FixedTarget(SkyCoord(0.0 * u.deg, 0.0 * u.deg, frame="icrs"), name=str(i)), 10 * u.minute, 10
         )
+        for i in range(10)
+    ]
+
+    return blocks
+
 
-    # create two lists from these with some overlap
-    blocks1 = blocks[:7]
-    blocks2 = blocks[5:]
+def test_compare_block_lists_with_overlap(schedule_blocks):
+    old_blocks = schedule_blocks[:7]
+    new_blocks = schedule_blocks[5:]
 
-    # compare
-    unique1, unique2 = Scheduler._compare_block_lists(blocks1, blocks2)
+    removed, added = Scheduler._compare_block_lists(old_blocks, new_blocks)
 
-    # get names
-    names1 = [int(b.target.name) for b in unique1]
-    names2 = [int(b.target.name) for b in unique2]
+    removed_names = [int(b.target.name) for b in removed]
+    new_names = [int(b.target.name) for b in added]
 
-    # names1 should contain 0, 1, 2, 3, 4
-    assert set(names1) == {0, 1, 2, 3, 4}
+    assert set(removed_names) == {0, 1, 2, 3, 4}
+    assert set(new_names) == {7, 8, 9}
 
-    # names2 should contain 7, 8, 9
-    assert set(names2) == {7, 8, 9}
 
-    # create two lists from these with no overlap
-    blocks1 = blocks[:5]
-    blocks2 = blocks[5:]
+def test_compare_block_lists_without_overlap(schedule_blocks):
+    old_blocks = schedule_blocks[:5]
+    new_blocks = schedule_blocks[5:]
 
-    # compare
-    unique1, unique2 = Scheduler._compare_block_lists(blocks1, blocks2)
+    removed, added = Scheduler._compare_block_lists(old_blocks, new_blocks)
 
-    # get names
-    names1 = [int(b.target.name) for b in unique1]
-    names2 = [int(b.target.name) for b in unique2]
+    removed_names = [int(b.target.name) for b in removed]
+    new_names = [int(b.target.name) for b in added]
 
-    # names1 should contain 0, 1, 2, 3, 4
-    assert set(names1) == {0, 1, 2, 3, 4}
+    assert set(removed_names) == {0, 1, 2, 3, 4}
+    assert set(new_names) == {5, 6, 7, 8, 9}
 
-    # names2 should contain 5, 6, 7, 8, 9
-    assert set(names2) == {5, 6, 7, 8, 9}
 
-    # create two identical lists
-    blocks1 = blocks
-    blocks2 = blocks
+def test_compare_block_lists_identical(schedule_blocks):
+    old_blocks = schedule_blocks
+    new_blocks = schedule_blocks
 
-    # compare
-    unique1, unique2 = Scheduler._compare_block_lists(blocks1, blocks2)
+    removed, added = Scheduler._compare_block_lists(old_blocks, new_blocks)
 
-    # get names
-    names1 = [int(b.target.name) for b in unique1]
-    names2 = [int(b.target.name) for b in unique2]
+    removed_names = [int(b.target.name) for b in removed]
+    new_names = [int(b.target.name) for b in added]
 
-    # both lists should be empty
-    assert len(names1) == 0
-    assert len(names2) == 0
+    assert len(removed_names) == 0
+    assert len(new_names) == 0

From 09958cf38f7c8c4b9dad3cc2ceea6bd3a51a9b0a Mon Sep 17 00:00:00 2001
From: GermanHydrogen <admin@germanhydrogen.org>
Date: Wed, 27 Mar 2024 12:11:53 +0100
Subject: [PATCH 07/25] Added unit tests to worker lof the scheduler

---
 pyobs/modules/robotic/scheduler.py      | 109 ++++++++++++------------
 tests/modules/robotic/test_scheduler.py | 104 +++++++++++++++++++++-
 2 files changed, 157 insertions(+), 56 deletions(-)

diff --git a/pyobs/modules/robotic/scheduler.py b/pyobs/modules/robotic/scheduler.py
index 2807d5d8f..d46f2c20c 100644
--- a/pyobs/modules/robotic/scheduler.py
+++ b/pyobs/modules/robotic/scheduler.py
@@ -79,6 +79,8 @@ def __init__(
         self.add_background_task(self._schedule_worker)
         self.add_background_task(self._update_worker)
 
+        self._last_change = None
+
     async def open(self) -> None:
         """Open module."""
         await Module.open(self)
@@ -102,74 +104,71 @@ async def is_running(self, **kwargs: Any) -> bool:
         return self._running
 
     async def _update_worker(self) -> None:
-        # time of last change in blocks
-        last_change = None
-
-        # run forever
         while True:
-            # not running?
             if self._running is False:
                 await asyncio.sleep(1)
                 continue
 
-            # got new time of last change?
-            t = await self._task_archive.last_changed()
-            if last_change is None or last_change < t:
-                # get schedulable blocks and sort them
-                log.info("Found update in schedulable block, downloading them...")
-                blocks = sorted(
-                    await self._task_archive.get_schedulable_blocks(),
-                    key=lambda x: json.dumps(x.configuration, sort_keys=True),
-                )
-                log.info("Downloaded %d schedulable block(s).", len(blocks))
-
-                # compare new and old lists
-                removed, added = self._compare_block_lists(self._blocks, blocks)
+            await self._worker_loop()
 
-                # schedule update
-                self._need_update = True
+            await asyncio.sleep(5)
 
-                # no changes?
-                if len(removed) == 0 and len(added) == 0:
-                    # no need to re-schedule
-                    log.info("No change in list of blocks detected.")
-                    self._need_update = False
+    async def _worker_loop(self):
+        # got new time of last change?
+        t = await self._task_archive.last_changed()
+        if self._last_change is None or self._last_change < t:
+            # get schedulable blocks and sort them
+            log.info("Found update in schedulable block, downloading them...")
+            blocks = sorted(
+                await self._task_archive.get_schedulable_blocks(),
+                key=lambda x: json.dumps(x.configuration, sort_keys=True),
+            )
+            log.info("Downloaded %d schedulable block(s).", len(blocks))
+
+            # compare new and old lists
+            removed, added = self._compare_block_lists(self._blocks, blocks)
+
+            # schedule update
+            self._need_update = True
 
-                # has only the current block been removed?
-                log.info("Removed: %d, added: %d", len(removed), len(added))
-                if len(removed) == 1:
-                    log.info(
-                        "Found 1 removed block with ID %d. Last task ID was %s, current is %s.",
-                        removed[0].target.name,
-                        str(self._last_task_id),
-                        str(self._current_task_id),
-                    )
-                if len(removed) == 1 and len(added) == 0 and removed[0].target.name == self._last_task_id:
-                    # no need to re-schedule
-                    log.info("Only one removed block detected, which is the one currently running.")
-                    self._need_update = False
+            # no changes?
+            if len(removed) == 0 and len(added) == 0:
+                # no need to re-schedule
+                log.info("No change in list of blocks detected.")
+                self._need_update = False
 
-                # check, if one of the removed blocks was actually in schedule
-                if len(removed) > 0 and self._need_update:
-                    schedule = await self._schedule.get_schedule()
-                    removed_from_schedule = [r for r in removed if r in schedule]
-                    if len(removed_from_schedule) == 0:
-                        log.info(f"Found {len(removed)} blocks, but none of them was scheduled.")
-                        self._need_update = False
+            # has only the current block been removed?
+            log.info("Removed: %d, added: %d", len(removed), len(added))
+            if len(removed) == 1:
+                log.info(
+                    "Found 1 removed block with ID %d. Last task ID was %s, current is %s.",
+                    removed[0].target.name,
+                    str(self._last_task_id),
+                    str(self._current_task_id),
+                )
+            if len(removed) == 1 and len(added) == 0 and removed[0].target.name == self._last_task_id:
+                # no need to re-schedule
+                log.info("Only one removed block detected, which is the one currently running.")
+                self._need_update = False
 
-                # store blocks
-                self._blocks = blocks
+            # check, if one of the removed blocks was actually in schedule
+            if len(removed) > 0 and self._need_update:
+                schedule = await self._schedule.get_schedule()
+                removed_from_schedule = [r for r in removed if r in schedule]
+                if len(removed_from_schedule) == 0:
+                    log.info(f"Found {len(removed)} blocks, but none of them was scheduled.")
+                    self._need_update = False
 
-                # schedule update
-                if self._need_update:
-                    log.info("Triggering scheduler run...")
+            # store blocks
+            self._blocks = blocks
 
-                # remember now
-                last_change = Time.now()
-                self._initial_update_done = True
+            # schedule update
+            if self._need_update:
+                log.info("Triggering scheduler run...")
 
-            # sleep a little
-            await asyncio.sleep(5)
+            # remember now
+            self._last_change = Time.now()
+            self._initial_update_done = True
 
     @staticmethod
     def _compare_block_lists(
diff --git a/tests/modules/robotic/test_scheduler.py b/tests/modules/robotic/test_scheduler.py
index 5ff2afa23..a21aa45d6 100644
--- a/tests/modules/robotic/test_scheduler.py
+++ b/tests/modules/robotic/test_scheduler.py
@@ -1,4 +1,5 @@
-from typing import List
+from typing import List, Optional, Dict
+from unittest.mock import Mock, AsyncMock
 
 import astropy.units as u
 import pytest
@@ -6,6 +7,8 @@
 from astropy.coordinates import SkyCoord
 
 from pyobs.modules.robotic import Scheduler
+from pyobs.robotic import TaskArchive, TaskSchedule, Task
+from pyobs.utils.time import Time
 
 
 @pytest.fixture
@@ -57,3 +60,102 @@ def test_compare_block_lists_identical(schedule_blocks):
 
     assert len(removed_names) == 0
     assert len(new_names) == 0
+
+
+class TestTaskArchive(TaskArchive):
+
+    async def last_changed(self) -> Optional[Time]:
+        pass
+
+    async def get_schedulable_blocks(self) -> List[ObservingBlock]:
+        pass
+
+
+class TestTaskSchedule(TaskSchedule):
+
+    async def set_schedule(self, blocks: List[ObservingBlock], start_time: Time) -> None:
+        pass
+
+    async def last_scheduled(self) -> Optional[Time]:
+        pass
+
+    async def get_schedule(self) -> Dict[str, Task]:
+        pass
+
+    async def get_task(self, time: Time) -> Optional[Task]:
+        pass
+
+
+@pytest.mark.asyncio
+async def test_worker_loop_not_changed():
+    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
+    scheduler._need_update = False
+
+    scheduler._task_archive.last_changed = AsyncMock(return_value=Time.now())
+    scheduler._last_change = Time.now()
+
+    await scheduler._worker_loop()
+
+    assert scheduler._need_update is False
+
+
+@pytest.mark.asyncio
+async def test_worker_loop_no_changes(schedule_blocks):
+    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
+    scheduler._task_archive.get_schedulable_blocks = AsyncMock(return_value=schedule_blocks)
+    scheduler._blocks = schedule_blocks
+
+    scheduler._need_update = False
+
+    await scheduler._worker_loop()
+
+    assert scheduler._need_update is False
+
+
+@pytest.mark.asyncio
+async def test_worker_loop_removed_current(schedule_blocks):
+    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
+    scheduler._task_archive.get_schedulable_blocks = AsyncMock(return_value=schedule_blocks)
+    scheduler._blocks = schedule_blocks
+    scheduler._last_task_id = "0"
+
+    scheduler._compare_block_lists = Mock(return_value=([schedule_blocks[0]], []))
+
+    scheduler._need_update = False
+
+    await scheduler._worker_loop()
+
+    assert scheduler._need_update is False
+
+
+@pytest.mark.asyncio
+async def test_worker_loop_removed_not_in_schedule(schedule_blocks):
+    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
+    scheduler._task_archive.get_schedulable_blocks = AsyncMock(return_value=schedule_blocks)
+    scheduler._schedule.get_schedule = AsyncMock(return_value=[])
+    scheduler._blocks = schedule_blocks
+
+    scheduler._compare_block_lists = Mock(return_value=([schedule_blocks[0]], []))
+
+    scheduler._need_update = False
+
+    await scheduler._worker_loop()
+
+    assert scheduler._need_update is False
+
+
+@pytest.mark.asyncio
+async def test_worker_loop_need_to_update(schedule_blocks):
+    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
+    scheduler._task_archive.get_schedulable_blocks = AsyncMock(return_value=schedule_blocks)
+    scheduler._schedule.get_schedule = AsyncMock(return_value=[])
+    scheduler._blocks = []
+
+    scheduler._compare_block_lists = Mock(return_value=([], [schedule_blocks[0]]))
+
+    scheduler._need_update = False
+
+    await scheduler._worker_loop()
+
+    assert scheduler._need_update is True
+    assert scheduler._blocks == schedule_blocks

From 768c903819d180c8e0ebd5bd6249cad24d909327 Mon Sep 17 00:00:00 2001
From: GermanHydrogen <admin@germanhydrogen.org>
Date: Tue, 9 Apr 2024 16:27:31 +0200
Subject: [PATCH 08/25] Added basic tests to prepare schuedule

---
 tests/modules/robotic/test_mastermind.py |   2 +-
 tests/modules/robotic/test_scheduler.py  | 122 ++++++++++++++++++++++-
 2 files changed, 122 insertions(+), 2 deletions(-)

diff --git a/tests/modules/robotic/test_mastermind.py b/tests/modules/robotic/test_mastermind.py
index ed139b821..5c67ec152 100644
--- a/tests/modules/robotic/test_mastermind.py
+++ b/tests/modules/robotic/test_mastermind.py
@@ -58,7 +58,7 @@ def start(self) -> Time:
 
     @property
     def end(self) -> Time:
-        pass
+        return self._start
 
     async def can_run(self, scripts: Optional[Dict[str, Script]] = None) -> bool:
         pass
diff --git a/tests/modules/robotic/test_scheduler.py b/tests/modules/robotic/test_scheduler.py
index a21aa45d6..cb708cd7b 100644
--- a/tests/modules/robotic/test_scheduler.py
+++ b/tests/modules/robotic/test_scheduler.py
@@ -1,21 +1,26 @@
+import datetime
 from typing import List, Optional, Dict
 from unittest.mock import Mock, AsyncMock
 
+import astroplan
 import astropy.units as u
 import pytest
 from astroplan import ObservingBlock, FixedTarget
 from astropy.coordinates import SkyCoord
 
+import pyobs
 from pyobs.modules.robotic import Scheduler
 from pyobs.robotic import TaskArchive, TaskSchedule, Task
 from pyobs.utils.time import Time
+from tests.modules.robotic.test_mastermind import TestTask
 
 
 @pytest.fixture
 def schedule_blocks() -> List[ObservingBlock]:
     blocks = [
         ObservingBlock(
-            FixedTarget(SkyCoord(0.0 * u.deg, 0.0 * u.deg, frame="icrs"), name=str(i)), 10 * u.minute, 10
+            FixedTarget(SkyCoord(0.0 * u.deg, 0.0 * u.deg, frame="icrs"), name=str(i)), 10 * u.minute, 10,
+            constraints=[], configuration={"request": {"id": str(i)}}
         )
         for i in range(10)
     ]
@@ -159,3 +164,118 @@ async def test_worker_loop_need_to_update(schedule_blocks):
 
     assert scheduler._need_update is True
     assert scheduler._blocks == schedule_blocks
+
+
+@pytest.mark.asyncio
+async def test_prepare_schedule_invalid_twilight():
+    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), twilight="invalid")
+
+    with pytest.raises(ValueError):
+        await scheduler._prepare_schedule()
+
+
+@pytest.mark.asyncio
+async def test_prepare_schedule_astronomical_twilight(schedule_blocks):
+    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), twilight="astronomical")
+    scheduler._blocks = schedule_blocks
+
+    _, _, _, constraints = await scheduler._prepare_schedule()
+
+    assert constraints[0].max_solar_altitude == -18 * u.deg
+
+
+@pytest.mark.asyncio
+async def test_prepare_schedule_nautical_twilight(schedule_blocks):
+    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), twilight="nautical")
+    scheduler._blocks = schedule_blocks
+
+    _, _, _, constraints = await scheduler._prepare_schedule()
+
+    assert constraints[0].max_solar_altitude == -12 * u.deg
+
+
+@pytest.mark.asyncio
+async def test_prepare_schedule_no_blocks():
+    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), twilight="nautical")
+
+    with pytest.raises(ValueError):
+        await scheduler._prepare_schedule()
+
+
+@pytest.mark.asyncio
+async def test_prepare_schedule_abort(schedule_blocks):
+    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), twilight="nautical")
+    scheduler._blocks = schedule_blocks
+    scheduler._need_update = True
+
+    with pytest.raises(ValueError):
+        await scheduler._prepare_schedule()
+
+
+@pytest.mark.asyncio
+async def test_prepare_schedule_no_start(schedule_blocks, mocker):
+    current_time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
+    mocker.patch("pyobs.utils.time.Time.now", return_value=current_time)
+
+    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
+    scheduler._blocks = schedule_blocks
+
+    _, start, _, _ = await scheduler._prepare_schedule()
+
+    assert start.to_datetime() == datetime.datetime(2024, 4, 1, 20, 1, 0)
+
+
+@pytest.mark.asyncio
+async def test_prepare_schedule_start(schedule_blocks, mocker):
+    current_time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
+    mocker.patch("pyobs.utils.time.Time.now", return_value=current_time)
+
+    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
+    scheduler._blocks = schedule_blocks
+    scheduler._schedule_start = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 1, 0))
+
+    _, start, _, _ = await scheduler._prepare_schedule()
+
+    assert start.to_datetime() == datetime.datetime(2024, 4, 1, 20, 1, 0)
+
+
+@pytest.mark.asyncio
+async def test_prepare_schedule_end(schedule_blocks, mocker):
+    current_time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
+    mocker.patch("pyobs.utils.time.Time.now", return_value=current_time)
+
+    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
+    scheduler._blocks = schedule_blocks
+    scheduler._schedule_start = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 1, 0))
+
+    _, _, end, _ = await scheduler._prepare_schedule()
+
+    assert end.to_datetime() == datetime.datetime(2024, 4, 2, 20, 1, 0)
+
+
+@pytest.mark.asyncio
+async def test_prepare_schedule_block_filtering(schedule_blocks, mocker):
+    current_time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
+    mocker.patch("pyobs.utils.time.Time.now", return_value=current_time)
+
+    over_time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 3, 20, 0, 0))
+    in_time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 2, 10, 0, 0))
+
+    schedule_blocks[1].constraints.append(astroplan.TimeConstraint(min=over_time, max=over_time))
+    schedule_blocks[2].constraints.append(astroplan.TimeConstraint(min=in_time, max=over_time))
+
+    blocks = [
+        schedule_blocks[0], schedule_blocks[1], schedule_blocks[2], schedule_blocks[3]
+    ]
+
+    task_scheduler = TestTaskSchedule()
+    task_scheduler.get_schedule = AsyncMock(return_value={"0": TestTask()})
+
+    scheduler = Scheduler(TestTaskArchive(), task_scheduler)
+    scheduler._schedule_start = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 1, 0))
+    scheduler._current_task_id = "0"
+    scheduler._blocks = blocks
+
+    res_blocks, _, _, _ = await scheduler._prepare_schedule()
+
+    assert [block.configuration["request"]["id"] for block in res_blocks] == ["2", "3"]

From 94ad64b3bff99f184ad2dc59a8a4d38049042393 Mon Sep 17 00:00:00 2001
From: GermanHydrogen <admin@germanhydrogen.org>
Date: Tue, 9 Apr 2024 17:02:03 +0200
Subject: [PATCH 09/25] Added unit tests to the event handlers of scheduler

---
 tests/modules/robotic/test_scheduler.py | 43 +++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/tests/modules/robotic/test_scheduler.py b/tests/modules/robotic/test_scheduler.py
index cb708cd7b..f43abc904 100644
--- a/tests/modules/robotic/test_scheduler.py
+++ b/tests/modules/robotic/test_scheduler.py
@@ -9,6 +9,7 @@
 from astropy.coordinates import SkyCoord
 
 import pyobs
+from pyobs.events import GoodWeatherEvent, TaskStartedEvent, TaskFinishedEvent
 from pyobs.modules.robotic import Scheduler
 from pyobs.robotic import TaskArchive, TaskSchedule, Task
 from pyobs.utils.time import Time
@@ -279,3 +280,45 @@ async def test_prepare_schedule_block_filtering(schedule_blocks, mocker):
     res_blocks, _, _, _ = await scheduler._prepare_schedule()
 
     assert [block.configuration["request"]["id"] for block in res_blocks] == ["2", "3"]
+
+
+@pytest.mark.asyncio
+async def test_on_task_started():
+    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), trigger_on_task_started=True)
+    time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
+    event = TaskStartedEvent(id=0, eta=time, name="")
+
+    await scheduler._on_task_started(event, "")
+
+    assert scheduler._current_task_id == 0
+    assert scheduler._last_task_id == 0
+    assert scheduler._need_update is True
+    assert scheduler._schedule_start == time
+
+
+@pytest.mark.asyncio
+async def test_on_task_finished(mocker):
+    current_time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
+    mocker.patch("pyobs.utils.time.Time.now", return_value=current_time)
+
+    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), trigger_on_task_finished=True)
+    event = TaskFinishedEvent(id=0, name="")
+
+    await scheduler._on_task_finished(event, "")
+
+    assert scheduler._current_task_id is None
+    assert scheduler._need_update is True
+
+    assert scheduler._schedule_start == current_time
+
+
+@pytest.mark.asyncio
+async def test_on_good_weather():
+    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), trigger_on_task_started=True)
+    time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
+    event = GoodWeatherEvent(id=0, eta=time, name="")
+
+    await scheduler._on_good_weather(event, "")
+
+    assert scheduler._need_update is True
+    assert scheduler._schedule_start == time

From a8e654922ae15a219e5c666d2ae1400546b58f91 Mon Sep 17 00:00:00 2001
From: GermanHydrogen <admin@germanhydrogen.org>
Date: Wed, 10 Apr 2024 16:51:23 +0200
Subject: [PATCH 10/25] Refactored prepare sechdule of schedule

---
 pyobs/modules/robotic/scheduler.py      | 171 +++++++++++++-----------
 tests/modules/robotic/test_scheduler.py |  17 +++
 2 files changed, 111 insertions(+), 77 deletions(-)

diff --git a/pyobs/modules/robotic/scheduler.py b/pyobs/modules/robotic/scheduler.py
index d46f2c20c..b738024db 100644
--- a/pyobs/modules/robotic/scheduler.py
+++ b/pyobs/modules/robotic/scheduler.py
@@ -5,6 +5,7 @@
 import multiprocessing as mp
 from typing import Union, List, Tuple, Any, Optional, Dict
 import astroplan
+import astropy
 from astroplan import ObservingBlock
 from astropy.time import TimeDelta
 import astropy.units as u
@@ -15,8 +16,7 @@
 from pyobs.utils.time import Time
 from pyobs.interfaces import IStartStop, IRunnable
 from pyobs.modules import Module
-from pyobs.robotic import TaskArchive, TaskSchedule
-
+from pyobs.robotic import TaskArchive, TaskSchedule, Task
 
 log = logging.getLogger(__name__)
 
@@ -201,55 +201,90 @@ def _compare_block_lists(
         return unique1, unique2
 
     async def _schedule_worker(self) -> None:
-        # run forever
+
         while True:
-            # need update?
             if self._need_update and self._initial_update_done:
-                # reset need for update
-                self._need_update = False
+                await self._schedule_worker_loop()
 
-                try:
-                    # prepare scheduler
-                    blocks, start, end, constraints = await self._prepare_schedule()
+            await asyncio.sleep(1)
 
-                    # schedule
-                    scheduled_blocks = await self._schedule_blocks(blocks, start, end, constraints)
+    async def _schedule_worker_loop(self) -> None:
+        # reset need for update
+        self._need_update = False
 
-                    # finish schedule
-                    await self._finish_schedule(scheduled_blocks, start)
+        try:
+            # prepare scheduler
+            blocks, start, end, constraints = await self._prepare_schedule()
 
-                except ValueError as e:
-                    log.warning(str(e))
+            # schedule
+            scheduled_blocks = await self._schedule_blocks(blocks, start, end, constraints)
 
-            # sleep a little
-            await asyncio.sleep(1)
+            # finish schedule
+            await self._finish_schedule(scheduled_blocks, start)
+
+        except ValueError as e:
+            log.warning(str(e))
 
     async def _prepare_schedule(self) -> Tuple[List[ObservingBlock], Time, Time, List[Any]]:
         """TaskSchedule blocks."""
 
-        # only global constraint is the night
+        converted_blocks = await self._convert_blocks_to_astroplan()
+
+        start, end = await self._get_time_range()
+
+        blocks = self._filter_blocks(converted_blocks, end)
+
+        # if need new update, skip here
+        if self._need_update:
+            raise ValueError("Not running scheduler, since update was requested.")
+
+        # no blocks found?
+        if len(blocks) == 0:
+            await self._schedule.set_schedule([], start)
+            raise ValueError("No blocks left for scheduling.")
+
+        constraints = await self._get_twilight_constraint()
+
+        # return all
+        return blocks, start, end, constraints
+
+    async def _get_twilight_constraint(self) -> List[astroplan.Constraint]:
         if self._twilight == "astronomical":
-            constraints = [astroplan.AtNightConstraint.twilight_astronomical()]
+            return [astroplan.AtNightConstraint.twilight_astronomical()]
         elif self._twilight == "nautical":
-            constraints = [astroplan.AtNightConstraint.twilight_nautical()]
+            return [astroplan.AtNightConstraint.twilight_nautical()]
         else:
             raise ValueError("Unknown twilight type.")
 
-        # make shallow copies of all blocks and loop them
+    async def _convert_blocks_to_astroplan(self) -> List[astroplan.ObservingBlock]:
         copied_blocks = [copy.copy(block) for block in self._blocks]
+
         for block in copied_blocks:
-            # astroplan's PriorityScheduler expects lower priorities to be more important, so calculate
-            # 1000 - priority
-            block.priority = 1000.0 - block.priority
-            if block.priority < 0:
-                block.priority = 0
-
-            # it also doesn't match the requested observing windows exactly, so we make them a little smaller.
-            for constraint in block.constraints:
-                if isinstance(constraint, astroplan.TimeConstraint):
-                    constraint.min += 30 * u.second
-                    constraint.max -= 30 * u.second
+            self._invert_block_priority(block)
+            self._tighten_block_time_constraints(block)
 
+        return copied_blocks
+
+    @staticmethod
+    def _invert_block_priority(block: astroplan.ObservingBlock):
+        """
+        astroplan's PriorityScheduler expects lower priorities to be more important, so calculate
+        1000 - priority
+        """
+        block.priority = max(1000.0 - block.priority, 0.0)
+
+    @staticmethod
+    def _tighten_block_time_constraints(block: astroplan.ObservingBlock):
+        """
+        astroplan's PriorityScheduler doesn't match the requested observing windows exactly,
+        so we make them a little smaller.
+        """
+        time_constraints = filter(lambda c: isinstance(c, astroplan.TimeConstraint), block.constraints)
+        for constraint in time_constraints:
+            constraint.min += 30 * u.second
+            constraint.max -= 30 * u.second
+
+    async def _get_time_range(self) -> Tuple[astropy.time.Time, astropy.time.Time]:
         # get start time for scheduler
         start = self._schedule_start
         now_plus_safety = Time.now() + self._safety_time * u.second
@@ -257,22 +292,7 @@ async def _prepare_schedule(self) -> Tuple[List[ObservingBlock], Time, Time, Lis
             # if no ETA exists or is in the past, use safety time
             start = now_plus_safety
 
-        # get running scheduled block, if any
-        if self._current_task_id is None:
-            log.info("No running block found.")
-            running_task = None
-        else:
-            # get running task from archive
-            log.info("Trying to find running block in current schedule...")
-            tasks = await self._schedule.get_schedule()
-            if self._current_task_id in tasks:
-                running_task = tasks[self._current_task_id]
-            else:
-                log.info("Running block not found in last schedule.")
-                running_task = None
-
-        # if start is before end time of currently running block, change that
-        if running_task is not None:
+        if (running_task := await self._get_current_task()) is not None:
             log.info("Found running block that ends at %s.", running_task.end)
 
             # get block end plus some safety
@@ -284,38 +304,35 @@ async def _prepare_schedule(self) -> Tuple[List[ObservingBlock], Time, Time, Lis
         # calculate end time
         end = start + TimeDelta(self._schedule_range * u.hour)
 
-        # remove currently running block and filter by start time
-        blocks: List[ObservingBlock] = []
-        for b in filter(lambda x: x.configuration["request"]["id"] != self._current_task_id, copied_blocks):
-            time_constraint_found = False
-            # loop all constraints
-            for c in b.constraints:
-                if isinstance(c, astroplan.TimeConstraint):
-                    # we found a time constraint
-                    time_constraint_found = True
-
-                    # does the window start before the end of the scheduling range?
-                    if c.min < end:
-                        # yes, store block and break loop
-                        blocks.append(b)
-                        break
-            else:
-                # loop has finished without breaking
-                # if no time constraint has been found, we still take the block
-                if time_constraint_found is False:
-                    blocks.append(b)
+        return start, end
 
-        # if need new update, skip here
-        if self._need_update:
-            raise ValueError("Not running scheduler, since update was requested.")
+    async def _get_current_task(self) -> Optional[Task]:
+        if self._current_task_id is None:
+            log.info("No running block found.")
+            return None
 
-        # no blocks found?
-        if len(blocks) == 0:
-            await self._schedule.set_schedule([], start)
-            raise ValueError("No blocks left for scheduling.")
+        log.info("Trying to find running block in current schedule...")
+        tasks = await self._schedule.get_schedule()
+        if self._current_task_id in tasks:
+            return tasks[self._current_task_id]
+        else:
+            log.info("Running block not found in last schedule.")
+            return None
 
-        # return all
-        return blocks, start, end, constraints
+    def _filter_blocks(self, blocks: List[astroplan.ObservingBlock], end: astropy.time.Time) -> List[astroplan.ObservingBlock]:
+        blocks_without_current = filter(lambda x: x.configuration["request"]["id"] != self._current_task_id, blocks)
+        blocks_in_schedule_range = filter(lambda b: self._is_block_starting_in_schedule(b, end), blocks_without_current)
+
+        return list(blocks_in_schedule_range)
+
+    @staticmethod
+    def _is_block_starting_in_schedule(block: astroplan.ObservingBlock, end: astropy.time.Time) -> bool:
+        time_constraints = [c for c in block.constraints if isinstance(c, astroplan.TimeConstraint)]
+
+        # does constraint start before the end of the scheduling range?
+        before_end = [c for c in time_constraints if c.min < end]
+
+        return len(time_constraints) == 0 or len(before_end) > 0
 
     async def _schedule_blocks(
         self, blocks: List[ObservingBlock], start: Time, end: Time, constraints: List[Any]
diff --git a/tests/modules/robotic/test_scheduler.py b/tests/modules/robotic/test_scheduler.py
index f43abc904..424314ec0 100644
--- a/tests/modules/robotic/test_scheduler.py
+++ b/tests/modules/robotic/test_scheduler.py
@@ -322,3 +322,20 @@ async def test_on_good_weather():
 
     assert scheduler._need_update is True
     assert scheduler._schedule_start == time
+
+
+@pytest.mark.asyncio
+async def test_convert_blocks_to_astroplan():
+    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
+    time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
+    block = ObservingBlock(
+            FixedTarget(SkyCoord(0.0 * u.deg, 0.0 * u.deg, frame="icrs"), name=0), 10 * u.minute, 2000.0,
+            constraints=[astroplan.TimeConstraint(min=time, max=time)]
+        )
+    scheduler._blocks = [block]
+
+    converted_blocks = await scheduler._convert_blocks_to_astroplan()
+
+    assert converted_blocks[0].priority == 0
+    assert converted_blocks[0].constraints[0].min.to_datetime() == datetime.datetime(2024, 4, 1, 20, 0, 30)
+    assert converted_blocks[0].constraints[0].max.to_datetime() == datetime.datetime(2024, 4, 1, 19, 59, 30)
\ No newline at end of file

From f7d93d9ebcf88dffdd940282b14414fc51255b70 Mon Sep 17 00:00:00 2001
From: GermanHydrogen <admin@germanhydrogen.org>
Date: Sat, 27 Apr 2024 10:29:05 +0200
Subject: [PATCH 11/25] Fixed typing

---
 pyobs/modules/robotic/scheduler.py | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/pyobs/modules/robotic/scheduler.py b/pyobs/modules/robotic/scheduler.py
index b738024db..b7fb01f50 100644
--- a/pyobs/modules/robotic/scheduler.py
+++ b/pyobs/modules/robotic/scheduler.py
@@ -3,7 +3,7 @@
 import json
 import logging
 import multiprocessing as mp
-from typing import Union, List, Tuple, Any, Optional, Dict
+from typing import Union, List, Tuple, Any, Optional, Dict, cast
 import astroplan
 import astropy
 from astroplan import ObservingBlock
@@ -52,8 +52,8 @@ def __init__(
         Module.__init__(self, **kwargs)
 
         # get scheduler
-        self._task_archive = self.add_child_object(tasks, TaskArchive)
-        self._schedule = self.add_child_object(schedule, TaskSchedule)
+        self._task_archive = self.add_child_object(tasks, TaskArchive)  # type: ignore
+        self._schedule = self.add_child_object(schedule, TaskSchedule)  # type: ignore
 
         # store
         self._schedule_range = schedule_range
@@ -79,7 +79,7 @@ def __init__(
         self.add_background_task(self._schedule_worker)
         self.add_background_task(self._update_worker)
 
-        self._last_change = None
+        self._last_change: Optional[Time] = None
 
     async def open(self) -> None:
         """Open module."""
@@ -113,7 +113,7 @@ async def _update_worker(self) -> None:
 
             await asyncio.sleep(5)
 
-    async def _worker_loop(self):
+    async def _worker_loop(self) -> None:
         # got new time of last change?
         t = await self._task_archive.last_changed()
         if self._last_change is None or self._last_change < t:
@@ -266,7 +266,7 @@ async def _convert_blocks_to_astroplan(self) -> List[astroplan.ObservingBlock]:
         return copied_blocks
 
     @staticmethod
-    def _invert_block_priority(block: astroplan.ObservingBlock):
+    def _invert_block_priority(block: astroplan.ObservingBlock) -> None:
         """
         astroplan's PriorityScheduler expects lower priorities to be more important, so calculate
         1000 - priority
@@ -274,7 +274,7 @@ def _invert_block_priority(block: astroplan.ObservingBlock):
         block.priority = max(1000.0 - block.priority, 0.0)
 
     @staticmethod
-    def _tighten_block_time_constraints(block: astroplan.ObservingBlock):
+    def _tighten_block_time_constraints(block: astroplan.ObservingBlock) -> None:
         """
         astroplan's PriorityScheduler doesn't match the requested observing windows exactly,
         so we make them a little smaller.
@@ -299,7 +299,7 @@ async def _get_time_range(self) -> Tuple[astropy.time.Time, astropy.time.Time]:
             block_end = running_task.end + 10.0 * u.second
             if start < block_end:
                 start = block_end
-                log.info("Start time would be within currently running block, shifting to %s.", start.isot)
+                log.info("Start time would be within currently running block, shifting to %s.", cast(Time, start).isot)
 
         # calculate end time
         end = start + TimeDelta(self._schedule_range * u.hour)
@@ -339,7 +339,7 @@ async def _schedule_blocks(
     ) -> List[ObservingBlock]:
 
         # run actual scheduler in separate process and wait for it
-        qout: mp.Queue = mp.Queue()
+        qout: mp.Queue[List[ObservingBlock]] = mp.Queue()
         p = mp.Process(target=self._schedule_process, args=(blocks, start, end, constraints, qout))
         p.start()
 
@@ -378,7 +378,7 @@ def _schedule_process(
         start: Time,
         end: Time,
         constraints: List[Any],
-        scheduled_blocks: mp.Queue,
+        scheduled_blocks: mp.Queue[List[ObservingBlock]],
     ) -> None:
         """Actually do the scheduling, usually run in a separate process."""
 

From 89f25f9944950b4f50605423b9e38f5bd851b585 Mon Sep 17 00:00:00 2001
From: GermanHydrogen <admin@germanhydrogen.org>
Date: Mon, 29 Apr 2024 16:34:12 +0200
Subject: [PATCH 12/25] Added missing test cases

---
 pyobs/modules/robotic/scheduler.py      |  2 +-
 tests/modules/robotic/test_scheduler.py | 94 +++++++++++++++++++++++--
 2 files changed, 88 insertions(+), 8 deletions(-)

diff --git a/pyobs/modules/robotic/scheduler.py b/pyobs/modules/robotic/scheduler.py
index b7fb01f50..2fff0fd6c 100644
--- a/pyobs/modules/robotic/scheduler.py
+++ b/pyobs/modules/robotic/scheduler.py
@@ -378,7 +378,7 @@ def _schedule_process(
         start: Time,
         end: Time,
         constraints: List[Any],
-        scheduled_blocks: mp.Queue[List[ObservingBlock]],
+        scheduled_blocks: mp.Queue   # type: ignore
     ) -> None:
         """Actually do the scheduling, usually run in a separate process."""
 
diff --git a/tests/modules/robotic/test_scheduler.py b/tests/modules/robotic/test_scheduler.py
index 424314ec0..bc84aacef 100644
--- a/tests/modules/robotic/test_scheduler.py
+++ b/tests/modules/robotic/test_scheduler.py
@@ -1,15 +1,16 @@
 import datetime
-from typing import List, Optional, Dict
+import multiprocessing
+from typing import List, Optional, Dict, Any
 from unittest.mock import Mock, AsyncMock
 
 import astroplan
 import astropy.units as u
 import pytest
-from astroplan import ObservingBlock, FixedTarget
+from astroplan import ObservingBlock, FixedTarget, Observer
 from astropy.coordinates import SkyCoord
 
 import pyobs
-from pyobs.events import GoodWeatherEvent, TaskStartedEvent, TaskFinishedEvent
+from pyobs.events import GoodWeatherEvent, TaskStartedEvent, TaskFinishedEvent, Event
 from pyobs.modules.robotic import Scheduler
 from pyobs.robotic import TaskArchive, TaskSchedule, Task
 from pyobs.utils.time import Time
@@ -282,6 +283,61 @@ async def test_prepare_schedule_block_filtering(schedule_blocks, mocker):
     assert [block.configuration["request"]["id"] for block in res_blocks] == ["2", "3"]
 
 
+def mock_schedule_process(blocks: List[ObservingBlock], start: Time, end: Time, constraints: List[Any],
+                          scheduled_blocks: multiprocessing.Queue) -> None:  # type: ignore
+    scheduled_blocks.put(blocks)
+
+
+@pytest.mark.asyncio
+async def test_schedule_blocks() -> None:
+    observer = Observer(longitude=20.8108 * u.deg, latitude=-32.375823 * u.deg,
+                        elevation=1798.0 * u.m, timezone="UTC")
+    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), observer=observer)
+
+    scheduler._schedule_process = mock_schedule_process  # type: ignore
+
+    time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
+    block = ObservingBlock(
+        FixedTarget(SkyCoord(0.0 * u.deg, 0.0 * u.deg, frame="icrs"), name=0), 10 * u.minute, 2000.0,
+        configuration={"request": {"id": "0"}}
+    )
+    blocks = [block]
+    scheduled_blocks = await scheduler._schedule_blocks(blocks, time, time, [])
+    assert scheduled_blocks[0].configuration["request"]["id"] == block.configuration["request"]["id"]
+
+
+@pytest.mark.asyncio
+async def test_finish_schedule_no_update() -> None:
+    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
+    scheduler._schedule.set_schedule = AsyncMock()  # type: ignore
+
+    scheduler._need_update = True
+
+    await scheduler._finish_schedule([], pyobs.utils.time.Time.now())
+    scheduler._schedule.set_schedule.assert_not_called()
+
+
+@pytest.mark.asyncio
+async def test_finish_schedule() -> None:
+    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
+    scheduler._schedule.set_schedule = AsyncMock()  # type: ignore
+
+    time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
+    block = ObservingBlock(
+        FixedTarget(SkyCoord(0.0 * u.deg, 0.0 * u.deg, frame="icrs"), name=0), 10 * u.minute, 2000.0,
+        configuration={"request": {"id": "0"}},
+        constraints=[astroplan.TimeConstraint(min=time, max=time)]
+    )
+    block.start_time = time
+    block.end_time = time
+    blocks = [block]
+
+    scheduler._need_update = False
+
+    await scheduler._finish_schedule(blocks, time)
+    scheduler._schedule.set_schedule.assert_called_with(blocks, time)
+
+
 @pytest.mark.asyncio
 async def test_on_task_started():
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), trigger_on_task_started=True)
@@ -296,6 +352,14 @@ async def test_on_task_started():
     assert scheduler._schedule_start == time
 
 
+@pytest.mark.asyncio
+async def test_on_task_started_wrong_event():
+    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), trigger_on_task_started=True)
+    event = Event()
+
+    assert await scheduler._on_task_started(event, "") is False
+
+
 @pytest.mark.asyncio
 async def test_on_task_finished(mocker):
     current_time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
@@ -312,6 +376,14 @@ async def test_on_task_finished(mocker):
     assert scheduler._schedule_start == current_time
 
 
+@pytest.mark.asyncio
+async def test_on_task_finished_wrong_event():
+    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), trigger_on_task_started=True)
+    event = Event()
+
+    assert await scheduler._on_task_finished(event, "") is False
+
+
 @pytest.mark.asyncio
 async def test_on_good_weather():
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), trigger_on_task_started=True)
@@ -324,18 +396,26 @@ async def test_on_good_weather():
     assert scheduler._schedule_start == time
 
 
+@pytest.mark.asyncio
+async def test_on_good_weather_not_weather_event():
+    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), trigger_on_task_started=True)
+    event = Event()
+
+    assert await scheduler._on_good_weather(event, "") is False
+
+
 @pytest.mark.asyncio
 async def test_convert_blocks_to_astroplan():
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
     time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
     block = ObservingBlock(
-            FixedTarget(SkyCoord(0.0 * u.deg, 0.0 * u.deg, frame="icrs"), name=0), 10 * u.minute, 2000.0,
-            constraints=[astroplan.TimeConstraint(min=time, max=time)]
-        )
+        FixedTarget(SkyCoord(0.0 * u.deg, 0.0 * u.deg, frame="icrs"), name=0), 10 * u.minute, 2000.0,
+        constraints=[astroplan.TimeConstraint(min=time, max=time)]
+    )
     scheduler._blocks = [block]
 
     converted_blocks = await scheduler._convert_blocks_to_astroplan()
 
     assert converted_blocks[0].priority == 0
     assert converted_blocks[0].constraints[0].min.to_datetime() == datetime.datetime(2024, 4, 1, 20, 0, 30)
-    assert converted_blocks[0].constraints[0].max.to_datetime() == datetime.datetime(2024, 4, 1, 19, 59, 30)
\ No newline at end of file
+    assert converted_blocks[0].constraints[0].max.to_datetime() == datetime.datetime(2024, 4, 1, 19, 59, 30)

From 1b87cecbed76630a080f9a900cd727bb0ac71ab4 Mon Sep 17 00:00:00 2001
From: GermanHydrogen <admin@germanhydrogen.org>
Date: Mon, 29 Apr 2024 16:37:21 +0200
Subject: [PATCH 13/25] Added type annotations to tescases

---
 tests/modules/robotic/test_scheduler.py | 70 ++++++++++++-------------
 1 file changed, 35 insertions(+), 35 deletions(-)

diff --git a/tests/modules/robotic/test_scheduler.py b/tests/modules/robotic/test_scheduler.py
index bc84aacef..d0aeec965 100644
--- a/tests/modules/robotic/test_scheduler.py
+++ b/tests/modules/robotic/test_scheduler.py
@@ -30,7 +30,7 @@ def schedule_blocks() -> List[ObservingBlock]:
     return blocks
 
 
-def test_compare_block_lists_with_overlap(schedule_blocks):
+def test_compare_block_lists_with_overlap(schedule_blocks) -> None:
     old_blocks = schedule_blocks[:7]
     new_blocks = schedule_blocks[5:]
 
@@ -43,7 +43,7 @@ def test_compare_block_lists_with_overlap(schedule_blocks):
     assert set(new_names) == {7, 8, 9}
 
 
-def test_compare_block_lists_without_overlap(schedule_blocks):
+def test_compare_block_lists_without_overlap(schedule_blocks) -> None:
     old_blocks = schedule_blocks[:5]
     new_blocks = schedule_blocks[5:]
 
@@ -56,7 +56,7 @@ def test_compare_block_lists_without_overlap(schedule_blocks):
     assert set(new_names) == {5, 6, 7, 8, 9}
 
 
-def test_compare_block_lists_identical(schedule_blocks):
+def test_compare_block_lists_identical(schedule_blocks) -> None:
     old_blocks = schedule_blocks
     new_blocks = schedule_blocks
 
@@ -94,11 +94,11 @@ async def get_task(self, time: Time) -> Optional[Task]:
 
 
 @pytest.mark.asyncio
-async def test_worker_loop_not_changed():
+async def test_worker_loop_not_changed() -> None:
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
     scheduler._need_update = False
 
-    scheduler._task_archive.last_changed = AsyncMock(return_value=Time.now())
+    scheduler._task_archive.last_changed = AsyncMock(return_value=Time.now())  # type: ignore
     scheduler._last_change = Time.now()
 
     await scheduler._worker_loop()
@@ -107,9 +107,9 @@ async def test_worker_loop_not_changed():
 
 
 @pytest.mark.asyncio
-async def test_worker_loop_no_changes(schedule_blocks):
+async def test_worker_loop_no_changes(schedule_blocks) -> None:
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
-    scheduler._task_archive.get_schedulable_blocks = AsyncMock(return_value=schedule_blocks)
+    scheduler._task_archive.get_schedulable_blocks = AsyncMock(return_value=schedule_blocks)  # type: ignore
     scheduler._blocks = schedule_blocks
 
     scheduler._need_update = False
@@ -120,13 +120,13 @@ async def test_worker_loop_no_changes(schedule_blocks):
 
 
 @pytest.mark.asyncio
-async def test_worker_loop_removed_current(schedule_blocks):
+async def test_worker_loop_removed_current(schedule_blocks) -> None:
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
-    scheduler._task_archive.get_schedulable_blocks = AsyncMock(return_value=schedule_blocks)
+    scheduler._task_archive.get_schedulable_blocks = AsyncMock(return_value=schedule_blocks)  # type: ignore
     scheduler._blocks = schedule_blocks
     scheduler._last_task_id = "0"
 
-    scheduler._compare_block_lists = Mock(return_value=([schedule_blocks[0]], []))
+    scheduler._compare_block_lists = Mock(return_value=([schedule_blocks[0]], []))  # type: ignore
 
     scheduler._need_update = False
 
@@ -136,13 +136,13 @@ async def test_worker_loop_removed_current(schedule_blocks):
 
 
 @pytest.mark.asyncio
-async def test_worker_loop_removed_not_in_schedule(schedule_blocks):
+async def test_worker_loop_removed_not_in_schedule(schedule_blocks)  -> None:
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
-    scheduler._task_archive.get_schedulable_blocks = AsyncMock(return_value=schedule_blocks)
-    scheduler._schedule.get_schedule = AsyncMock(return_value=[])
+    scheduler._task_archive.get_schedulable_blocks = AsyncMock(return_value=schedule_blocks)  # type: ignore
+    scheduler._schedule.get_schedule = AsyncMock(return_value=[])  # type: ignore
     scheduler._blocks = schedule_blocks
 
-    scheduler._compare_block_lists = Mock(return_value=([schedule_blocks[0]], []))
+    scheduler._compare_block_lists = Mock(return_value=([schedule_blocks[0]], []))  # type: ignore
 
     scheduler._need_update = False
 
@@ -152,13 +152,13 @@ async def test_worker_loop_removed_not_in_schedule(schedule_blocks):
 
 
 @pytest.mark.asyncio
-async def test_worker_loop_need_to_update(schedule_blocks):
+async def test_worker_loop_need_to_update(schedule_blocks) -> None:
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
-    scheduler._task_archive.get_schedulable_blocks = AsyncMock(return_value=schedule_blocks)
-    scheduler._schedule.get_schedule = AsyncMock(return_value=[])
+    scheduler._task_archive.get_schedulable_blocks = AsyncMock(return_value=schedule_blocks)  # type: ignore
+    scheduler._schedule.get_schedule = AsyncMock(return_value=[])  # type: ignore
     scheduler._blocks = []
 
-    scheduler._compare_block_lists = Mock(return_value=([], [schedule_blocks[0]]))
+    scheduler._compare_block_lists = Mock(return_value=([], [schedule_blocks[0]]))  # type: ignore
 
     scheduler._need_update = False
 
@@ -169,7 +169,7 @@ async def test_worker_loop_need_to_update(schedule_blocks):
 
 
 @pytest.mark.asyncio
-async def test_prepare_schedule_invalid_twilight():
+async def test_prepare_schedule_invalid_twilight() -> None:
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), twilight="invalid")
 
     with pytest.raises(ValueError):
@@ -177,7 +177,7 @@ async def test_prepare_schedule_invalid_twilight():
 
 
 @pytest.mark.asyncio
-async def test_prepare_schedule_astronomical_twilight(schedule_blocks):
+async def test_prepare_schedule_astronomical_twilight(schedule_blocks) -> None:
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), twilight="astronomical")
     scheduler._blocks = schedule_blocks
 
@@ -187,7 +187,7 @@ async def test_prepare_schedule_astronomical_twilight(schedule_blocks):
 
 
 @pytest.mark.asyncio
-async def test_prepare_schedule_nautical_twilight(schedule_blocks):
+async def test_prepare_schedule_nautical_twilight(schedule_blocks) -> None:
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), twilight="nautical")
     scheduler._blocks = schedule_blocks
 
@@ -197,7 +197,7 @@ async def test_prepare_schedule_nautical_twilight(schedule_blocks):
 
 
 @pytest.mark.asyncio
-async def test_prepare_schedule_no_blocks():
+async def test_prepare_schedule_no_blocks() -> None:
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), twilight="nautical")
 
     with pytest.raises(ValueError):
@@ -205,7 +205,7 @@ async def test_prepare_schedule_no_blocks():
 
 
 @pytest.mark.asyncio
-async def test_prepare_schedule_abort(schedule_blocks):
+async def test_prepare_schedule_abort(schedule_blocks) -> None:
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), twilight="nautical")
     scheduler._blocks = schedule_blocks
     scheduler._need_update = True
@@ -215,7 +215,7 @@ async def test_prepare_schedule_abort(schedule_blocks):
 
 
 @pytest.mark.asyncio
-async def test_prepare_schedule_no_start(schedule_blocks, mocker):
+async def test_prepare_schedule_no_start(schedule_blocks, mocker) -> None:
     current_time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
     mocker.patch("pyobs.utils.time.Time.now", return_value=current_time)
 
@@ -228,7 +228,7 @@ async def test_prepare_schedule_no_start(schedule_blocks, mocker):
 
 
 @pytest.mark.asyncio
-async def test_prepare_schedule_start(schedule_blocks, mocker):
+async def test_prepare_schedule_start(schedule_blocks, mocker) -> None:
     current_time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
     mocker.patch("pyobs.utils.time.Time.now", return_value=current_time)
 
@@ -242,7 +242,7 @@ async def test_prepare_schedule_start(schedule_blocks, mocker):
 
 
 @pytest.mark.asyncio
-async def test_prepare_schedule_end(schedule_blocks, mocker):
+async def test_prepare_schedule_end(schedule_blocks, mocker) -> None:
     current_time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
     mocker.patch("pyobs.utils.time.Time.now", return_value=current_time)
 
@@ -256,7 +256,7 @@ async def test_prepare_schedule_end(schedule_blocks, mocker):
 
 
 @pytest.mark.asyncio
-async def test_prepare_schedule_block_filtering(schedule_blocks, mocker):
+async def test_prepare_schedule_block_filtering(schedule_blocks, mocker) -> None:
     current_time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
     mocker.patch("pyobs.utils.time.Time.now", return_value=current_time)
 
@@ -271,7 +271,7 @@ async def test_prepare_schedule_block_filtering(schedule_blocks, mocker):
     ]
 
     task_scheduler = TestTaskSchedule()
-    task_scheduler.get_schedule = AsyncMock(return_value={"0": TestTask()})
+    task_scheduler.get_schedule = AsyncMock(return_value={"0": TestTask()})  # type: ignore
 
     scheduler = Scheduler(TestTaskArchive(), task_scheduler)
     scheduler._schedule_start = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 1, 0))
@@ -339,7 +339,7 @@ async def test_finish_schedule() -> None:
 
 
 @pytest.mark.asyncio
-async def test_on_task_started():
+async def test_on_task_started() -> None:
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), trigger_on_task_started=True)
     time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
     event = TaskStartedEvent(id=0, eta=time, name="")
@@ -353,7 +353,7 @@ async def test_on_task_started():
 
 
 @pytest.mark.asyncio
-async def test_on_task_started_wrong_event():
+async def test_on_task_started_wrong_event() -> None:
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), trigger_on_task_started=True)
     event = Event()
 
@@ -361,7 +361,7 @@ async def test_on_task_started_wrong_event():
 
 
 @pytest.mark.asyncio
-async def test_on_task_finished(mocker):
+async def test_on_task_finished(mocker) -> None:
     current_time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
     mocker.patch("pyobs.utils.time.Time.now", return_value=current_time)
 
@@ -377,7 +377,7 @@ async def test_on_task_finished(mocker):
 
 
 @pytest.mark.asyncio
-async def test_on_task_finished_wrong_event():
+async def test_on_task_finished_wrong_event() -> None:
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), trigger_on_task_started=True)
     event = Event()
 
@@ -385,7 +385,7 @@ async def test_on_task_finished_wrong_event():
 
 
 @pytest.mark.asyncio
-async def test_on_good_weather():
+async def test_on_good_weather() -> None:
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), trigger_on_task_started=True)
     time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
     event = GoodWeatherEvent(id=0, eta=time, name="")
@@ -397,7 +397,7 @@ async def test_on_good_weather():
 
 
 @pytest.mark.asyncio
-async def test_on_good_weather_not_weather_event():
+async def test_on_good_weather_not_weather_event() -> None:
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), trigger_on_task_started=True)
     event = Event()
 
@@ -405,7 +405,7 @@ async def test_on_good_weather_not_weather_event():
 
 
 @pytest.mark.asyncio
-async def test_convert_blocks_to_astroplan():
+async def test_convert_blocks_to_astroplan() -> None:
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
     time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
     block = ObservingBlock(

From 7139acbc496584f618eab0434d813683d538e378 Mon Sep 17 00:00:00 2001
From: GermanHydrogen <admin@germanhydrogen.org>
Date: Mon, 29 Apr 2024 17:26:40 +0200
Subject: [PATCH 14/25] Utilized the new background task api

---
 pyobs/modules/robotic/scheduler.py      | 71 ++++++++-----------------
 tests/modules/robotic/test_scheduler.py | 41 ++++----------
 2 files changed, 34 insertions(+), 78 deletions(-)

diff --git a/pyobs/modules/robotic/scheduler.py b/pyobs/modules/robotic/scheduler.py
index 2fff0fd6c..ad4c7f038 100644
--- a/pyobs/modules/robotic/scheduler.py
+++ b/pyobs/modules/robotic/scheduler.py
@@ -59,9 +59,7 @@ def __init__(
         self._schedule_range = schedule_range
         self._safety_time = safety_time
         self._twilight = twilight
-        self._running = True
-        self._initial_update_done = False
-        self._need_update = False
+
         self._trigger_on_task_started = trigger_on_task_started
         self._trigger_on_task_finished = trigger_on_task_finished
 
@@ -76,8 +74,8 @@ def __init__(
         self._blocks: List[ObservingBlock] = []
 
         # update thread
-        self.add_background_task(self._schedule_worker)
-        self.add_background_task(self._update_worker)
+        self._scheduler_task = self.add_background_task(self._schedule_task, autostart=False, restart=False)
+        self._update_task = self.add_background_task(self._update_worker)
 
         self._last_change: Optional[Time] = None
 
@@ -93,24 +91,19 @@ async def open(self) -> None:
 
     async def start(self, **kwargs: Any) -> None:
         """Start scheduler."""
-        self._running = True
+        self._update_task.start()
 
     async def stop(self, **kwargs: Any) -> None:
         """Stop scheduler."""
-        self._running = False
+        self._update_task.stop()
 
     async def is_running(self, **kwargs: Any) -> bool:
         """Whether scheduler is running."""
-        return self._running
+        return self._update_task.is_running()
 
     async def _update_worker(self) -> None:
         while True:
-            if self._running is False:
-                await asyncio.sleep(1)
-                continue
-
             await self._worker_loop()
-
             await asyncio.sleep(5)
 
     async def _worker_loop(self) -> None:
@@ -129,13 +122,13 @@ async def _worker_loop(self) -> None:
             removed, added = self._compare_block_lists(self._blocks, blocks)
 
             # schedule update
-            self._need_update = True
+            need_update = True
 
             # no changes?
             if len(removed) == 0 and len(added) == 0:
                 # no need to re-schedule
                 log.info("No change in list of blocks detected.")
-                self._need_update = False
+                need_update = False
 
             # has only the current block been removed?
             log.info("Removed: %d, added: %d", len(removed), len(added))
@@ -149,26 +142,27 @@ async def _worker_loop(self) -> None:
             if len(removed) == 1 and len(added) == 0 and removed[0].target.name == self._last_task_id:
                 # no need to re-schedule
                 log.info("Only one removed block detected, which is the one currently running.")
-                self._need_update = False
+                need_update = False
 
             # check, if one of the removed blocks was actually in schedule
-            if len(removed) > 0 and self._need_update:
+            if len(removed) > 0 and need_update:
                 schedule = await self._schedule.get_schedule()
                 removed_from_schedule = [r for r in removed if r in schedule]
                 if len(removed_from_schedule) == 0:
                     log.info(f"Found {len(removed)} blocks, but none of them was scheduled.")
-                    self._need_update = False
+                    need_update = False
 
             # store blocks
             self._blocks = blocks
 
             # schedule update
-            if self._need_update:
+            if need_update:
                 log.info("Triggering scheduler run...")
+                self._scheduler_task.stop()
+                self._scheduler_task.start()
 
             # remember now
             self._last_change = Time.now()
-            self._initial_update_done = True
 
     @staticmethod
     def _compare_block_lists(
@@ -200,18 +194,7 @@ def _compare_block_lists(
         unique2 = [names2[n] for n in additional2]
         return unique1, unique2
 
-    async def _schedule_worker(self) -> None:
-
-        while True:
-            if self._need_update and self._initial_update_done:
-                await self._schedule_worker_loop()
-
-            await asyncio.sleep(1)
-
-    async def _schedule_worker_loop(self) -> None:
-        # reset need for update
-        self._need_update = False
-
+    async def _schedule_task(self) -> None:
         try:
             # prepare scheduler
             blocks, start, end, constraints = await self._prepare_schedule()
@@ -234,10 +217,6 @@ async def _prepare_schedule(self) -> Tuple[List[ObservingBlock], Time, Time, Lis
 
         blocks = self._filter_blocks(converted_blocks, end)
 
-        # if need new update, skip here
-        if self._need_update:
-            raise ValueError("Not running scheduler, since update was requested.")
-
         # no blocks found?
         if len(blocks) == 0:
             await self._schedule.set_schedule([], start)
@@ -352,11 +331,6 @@ async def _schedule_blocks(
         return scheduled_blocks
 
     async def _finish_schedule(self, scheduled_blocks: List[ObservingBlock], start: Time) -> None:
-        # if need new update, skip here
-        if self._need_update:
-            log.info("Not using scheduler results, since update was requested.")
-            return
-
         # update
         await self._schedule.set_schedule(scheduled_blocks, start)
         if len(scheduled_blocks) > 0:
@@ -400,7 +374,8 @@ def _schedule_process(
 
     async def run(self, **kwargs: Any) -> None:
         """Trigger a re-schedule."""
-        self._need_update = True
+        self._scheduler_task.stop()
+        self._scheduler_task.start()
 
     async def _on_task_started(self, event: Event, sender: str) -> bool:
         """Re-schedule when task has started and we can predict its end.
@@ -422,8 +397,8 @@ async def _on_task_started(self, event: Event, sender: str) -> bool:
             eta = (event.eta - Time.now()).sec / 60
             log.info("Received task started event with ETA of %.0f minutes, triggering new scheduler run...", eta)
 
-            # set it
-            self._need_update = True
+            self._scheduler_task.stop()
+            self._scheduler_task.start()
             self._schedule_start = event.eta
 
         return True
@@ -446,8 +421,8 @@ async def _on_task_finished(self, event: Event, sender: str) -> bool:
             # get ETA in minutes
             log.info("Received task finished event, triggering new scheduler run...")
 
-            # set it
-            self._need_update = True
+            self._scheduler_task.stop()
+            self._scheduler_task.start()
             self._schedule_start = Time.now()
 
         return True
@@ -466,8 +441,8 @@ async def _on_good_weather(self, event: Event, sender: str) -> bool:
         eta = (event.eta - Time.now()).sec / 60
         log.info("Received good weather event with ETA of %.0f minutes, triggering new scheduler run...", eta)
 
-        # set it
-        self._need_update = True
+        self._scheduler_task.stop()
+        self._scheduler_task.start()
         self._schedule_start = event.eta
         return True
 
diff --git a/tests/modules/robotic/test_scheduler.py b/tests/modules/robotic/test_scheduler.py
index d0aeec965..95da22f39 100644
--- a/tests/modules/robotic/test_scheduler.py
+++ b/tests/modules/robotic/test_scheduler.py
@@ -138,33 +138,31 @@ async def test_worker_loop_removed_current(schedule_blocks) -> None:
 @pytest.mark.asyncio
 async def test_worker_loop_removed_not_in_schedule(schedule_blocks)  -> None:
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
+    scheduler._scheduler_task.start = Mock()  # type: ignore
     scheduler._task_archive.get_schedulable_blocks = AsyncMock(return_value=schedule_blocks)  # type: ignore
     scheduler._schedule.get_schedule = AsyncMock(return_value=[])  # type: ignore
     scheduler._blocks = schedule_blocks
 
     scheduler._compare_block_lists = Mock(return_value=([schedule_blocks[0]], []))  # type: ignore
 
-    scheduler._need_update = False
-
     await scheduler._worker_loop()
 
-    assert scheduler._need_update is False
+    scheduler._scheduler_task.start.assert_not_called()
 
 
 @pytest.mark.asyncio
 async def test_worker_loop_need_to_update(schedule_blocks) -> None:
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
+    scheduler._scheduler_task.start = Mock()  # type: ignore
     scheduler._task_archive.get_schedulable_blocks = AsyncMock(return_value=schedule_blocks)  # type: ignore
     scheduler._schedule.get_schedule = AsyncMock(return_value=[])  # type: ignore
     scheduler._blocks = []
 
     scheduler._compare_block_lists = Mock(return_value=([], [schedule_blocks[0]]))  # type: ignore
 
-    scheduler._need_update = False
-
     await scheduler._worker_loop()
 
-    assert scheduler._need_update is True
+    scheduler._scheduler_task.start.assert_called_once()
     assert scheduler._blocks == schedule_blocks
 
 
@@ -204,16 +202,6 @@ async def test_prepare_schedule_no_blocks() -> None:
         await scheduler._prepare_schedule()
 
 
-@pytest.mark.asyncio
-async def test_prepare_schedule_abort(schedule_blocks) -> None:
-    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), twilight="nautical")
-    scheduler._blocks = schedule_blocks
-    scheduler._need_update = True
-
-    with pytest.raises(ValueError):
-        await scheduler._prepare_schedule()
-
-
 @pytest.mark.asyncio
 async def test_prepare_schedule_no_start(schedule_blocks, mocker) -> None:
     current_time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
@@ -306,17 +294,6 @@ async def test_schedule_blocks() -> None:
     assert scheduled_blocks[0].configuration["request"]["id"] == block.configuration["request"]["id"]
 
 
-@pytest.mark.asyncio
-async def test_finish_schedule_no_update() -> None:
-    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
-    scheduler._schedule.set_schedule = AsyncMock()  # type: ignore
-
-    scheduler._need_update = True
-
-    await scheduler._finish_schedule([], pyobs.utils.time.Time.now())
-    scheduler._schedule.set_schedule.assert_not_called()
-
-
 @pytest.mark.asyncio
 async def test_finish_schedule() -> None:
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
@@ -341,6 +318,7 @@ async def test_finish_schedule() -> None:
 @pytest.mark.asyncio
 async def test_on_task_started() -> None:
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), trigger_on_task_started=True)
+    scheduler._scheduler_task.start = Mock()  # type: ignore
     time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
     event = TaskStartedEvent(id=0, eta=time, name="")
 
@@ -348,7 +326,7 @@ async def test_on_task_started() -> None:
 
     assert scheduler._current_task_id == 0
     assert scheduler._last_task_id == 0
-    assert scheduler._need_update is True
+    scheduler._scheduler_task.start.assert_called_once()
     assert scheduler._schedule_start == time
 
 
@@ -366,12 +344,13 @@ async def test_on_task_finished(mocker) -> None:
     mocker.patch("pyobs.utils.time.Time.now", return_value=current_time)
 
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), trigger_on_task_finished=True)
+    scheduler._scheduler_task.start = Mock()  # type: ignore
     event = TaskFinishedEvent(id=0, name="")
 
     await scheduler._on_task_finished(event, "")
 
     assert scheduler._current_task_id is None
-    assert scheduler._need_update is True
+    scheduler._scheduler_task.start.assert_called_once()
 
     assert scheduler._schedule_start == current_time
 
@@ -387,12 +366,14 @@ async def test_on_task_finished_wrong_event() -> None:
 @pytest.mark.asyncio
 async def test_on_good_weather() -> None:
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), trigger_on_task_started=True)
+    scheduler._scheduler_task.start = Mock()  # type: ignore
+
     time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
     event = GoodWeatherEvent(id=0, eta=time, name="")
 
     await scheduler._on_good_weather(event, "")
 
-    assert scheduler._need_update is True
+    scheduler._scheduler_task.start.assert_called_once()
     assert scheduler._schedule_start == time
 
 

From 534570068b9b08a1749407973e5adb8b9f6597d0 Mon Sep 17 00:00:00 2001
From: GermanHydrogen <admin@germanhydrogen.org>
Date: Tue, 30 Apr 2024 08:30:16 +0200
Subject: [PATCH 15/25] Seperated update worker loop

---
 pyobs/modules/robotic/scheduler.py      | 103 ++++++++++++------------
 tests/modules/robotic/test_scheduler.py |  23 +++---
 2 files changed, 62 insertions(+), 64 deletions(-)

diff --git a/pyobs/modules/robotic/scheduler.py b/pyobs/modules/robotic/scheduler.py
index ad4c7f038..b8351f749 100644
--- a/pyobs/modules/robotic/scheduler.py
+++ b/pyobs/modules/robotic/scheduler.py
@@ -103,66 +103,67 @@ async def is_running(self, **kwargs: Any) -> bool:
 
     async def _update_worker(self) -> None:
         while True:
-            await self._worker_loop()
+            await self._update_worker_loop()
             await asyncio.sleep(5)
 
-    async def _worker_loop(self) -> None:
+    async def _update_worker_loop(self) -> None:
         # got new time of last change?
         t = await self._task_archive.last_changed()
         if self._last_change is None or self._last_change < t:
-            # get schedulable blocks and sort them
-            log.info("Found update in schedulable block, downloading them...")
-            blocks = sorted(
-                await self._task_archive.get_schedulable_blocks(),
-                key=lambda x: json.dumps(x.configuration, sort_keys=True),
-            )
-            log.info("Downloaded %d schedulable block(s).", len(blocks))
+            await self._update_blocks()
 
-            # compare new and old lists
-            removed, added = self._compare_block_lists(self._blocks, blocks)
+            self._last_change = Time.now()
 
-            # schedule update
-            need_update = True
+    async def _update_blocks(self) -> None:
+        # get schedulable blocks and sort them
+        log.info("Found update in schedulable block, downloading them...")
+        blocks = sorted(
+            await self._task_archive.get_schedulable_blocks(),
+            key=lambda x: json.dumps(x.configuration, sort_keys=True),
+        )
+        log.info("Downloaded %d schedulable block(s).", len(blocks))
+
+        # compare new and old lists
+        removed, added = self._compare_block_lists(self._blocks, blocks)
+
+        # store blocks
+        self._blocks = blocks
+
+        # schedule update
+        if await self._need_update(removed, added):
+            log.info("Triggering scheduler run...")
+            self._scheduler_task.stop()     # Stop current run
+            self._scheduler_task.start()
 
-            # no changes?
-            if len(removed) == 0 and len(added) == 0:
-                # no need to re-schedule
-                log.info("No change in list of blocks detected.")
-                need_update = False
+    async def _need_update(self, removed: List[ObservingBlock], added: List[ObservingBlock]) -> bool:
+        if len(removed) == 0 and len(added) == 0:
+            # no need to re-schedule
+            log.info("No change in list of blocks detected.")
+            return False
 
-            # has only the current block been removed?
-            log.info("Removed: %d, added: %d", len(removed), len(added))
-            if len(removed) == 1:
-                log.info(
-                    "Found 1 removed block with ID %d. Last task ID was %s, current is %s.",
-                    removed[0].target.name,
-                    str(self._last_task_id),
-                    str(self._current_task_id),
-                )
-            if len(removed) == 1 and len(added) == 0 and removed[0].target.name == self._last_task_id:
-                # no need to re-schedule
-                log.info("Only one removed block detected, which is the one currently running.")
-                need_update = False
-
-            # check, if one of the removed blocks was actually in schedule
-            if len(removed) > 0 and need_update:
-                schedule = await self._schedule.get_schedule()
-                removed_from_schedule = [r for r in removed if r in schedule]
-                if len(removed_from_schedule) == 0:
-                    log.info(f"Found {len(removed)} blocks, but none of them was scheduled.")
-                    need_update = False
-
-            # store blocks
-            self._blocks = blocks
-
-            # schedule update
-            if need_update:
-                log.info("Triggering scheduler run...")
-                self._scheduler_task.stop()
-                self._scheduler_task.start()
-
-            # remember now
-            self._last_change = Time.now()
+        # has only the current block been removed?
+        log.info("Removed: %d, added: %d", len(removed), len(added))
+        if len(removed) == 1:
+            log.info(
+                "Found 1 removed block with ID %d. Last task ID was %s, current is %s.",
+                removed[0].target.name,
+                str(self._last_task_id),
+                str(self._current_task_id),
+            )
+        if len(removed) == 1 and len(added) == 0 and removed[0].target.name == self._last_task_id:
+            # no need to re-schedule
+            log.info("Only one removed block detected, which is the one currently running.")
+            return False
+
+        # check, if one of the removed blocks was actually in schedule
+        if len(removed) > 0:
+            schedule = await self._schedule.get_schedule()
+            removed_from_schedule = [r for r in removed if r in schedule]
+            if len(removed_from_schedule) == 0:
+                log.info(f"Found {len(removed)} blocks, but none of them was scheduled.")
+                return False
+
+        return True
 
     @staticmethod
     def _compare_block_lists(
diff --git a/tests/modules/robotic/test_scheduler.py b/tests/modules/robotic/test_scheduler.py
index 95da22f39..b9f27de76 100644
--- a/tests/modules/robotic/test_scheduler.py
+++ b/tests/modules/robotic/test_scheduler.py
@@ -96,43 +96,40 @@ async def get_task(self, time: Time) -> Optional[Task]:
 @pytest.mark.asyncio
 async def test_worker_loop_not_changed() -> None:
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
-    scheduler._need_update = False
 
     scheduler._task_archive.last_changed = AsyncMock(return_value=Time.now())  # type: ignore
     scheduler._last_change = Time.now()
 
-    await scheduler._worker_loop()
+    await scheduler._update_worker_loop()
 
-    assert scheduler._need_update is False
 
 
 @pytest.mark.asyncio
 async def test_worker_loop_no_changes(schedule_blocks) -> None:
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
+    scheduler._scheduler_task.start = Mock()  # type: ignore
     scheduler._task_archive.get_schedulable_blocks = AsyncMock(return_value=schedule_blocks)  # type: ignore
     scheduler._blocks = schedule_blocks
 
-    scheduler._need_update = False
+    await scheduler._update_worker_loop()
 
-    await scheduler._worker_loop()
-
-    assert scheduler._need_update is False
+    scheduler._scheduler_task.start.assert_not_called()
 
 
 @pytest.mark.asyncio
 async def test_worker_loop_removed_current(schedule_blocks) -> None:
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
+    scheduler._scheduler_task.start = Mock()  # type: ignore
+
     scheduler._task_archive.get_schedulable_blocks = AsyncMock(return_value=schedule_blocks)  # type: ignore
     scheduler._blocks = schedule_blocks
     scheduler._last_task_id = "0"
 
     scheduler._compare_block_lists = Mock(return_value=([schedule_blocks[0]], []))  # type: ignore
 
-    scheduler._need_update = False
+    await scheduler._update_worker_loop()
 
-    await scheduler._worker_loop()
-
-    assert scheduler._need_update is False
+    scheduler._scheduler_task.start.assert_not_called()
 
 
 @pytest.mark.asyncio
@@ -145,7 +142,7 @@ async def test_worker_loop_removed_not_in_schedule(schedule_blocks)  -> None:
 
     scheduler._compare_block_lists = Mock(return_value=([schedule_blocks[0]], []))  # type: ignore
 
-    await scheduler._worker_loop()
+    await scheduler._update_worker_loop()
 
     scheduler._scheduler_task.start.assert_not_called()
 
@@ -160,7 +157,7 @@ async def test_worker_loop_need_to_update(schedule_blocks) -> None:
 
     scheduler._compare_block_lists = Mock(return_value=([], [schedule_blocks[0]]))  # type: ignore
 
-    await scheduler._worker_loop()
+    await scheduler._update_worker_loop()
 
     scheduler._scheduler_task.start.assert_called_once()
     assert scheduler._blocks == schedule_blocks

From 9b3b55519f4477283c1b932df35caa3e28f582fd Mon Sep 17 00:00:00 2001
From: germanhydrogen <admin@germanhydrogen.org>
Date: Tue, 30 Apr 2024 12:22:11 +0200
Subject: [PATCH 16/25] Fixed worker loop tests

---
 tests/modules/robotic/test_scheduler.py | 39 ++++++++++++++++++-------
 1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/tests/modules/robotic/test_scheduler.py b/tests/modules/robotic/test_scheduler.py
index b9f27de76..f38578410 100644
--- a/tests/modules/robotic/test_scheduler.py
+++ b/tests/modules/robotic/test_scheduler.py
@@ -96,28 +96,47 @@ async def get_task(self, time: Time) -> Optional[Task]:
 @pytest.mark.asyncio
 async def test_worker_loop_not_changed() -> None:
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
+    scheduler._update_blocks = AsyncMock()
 
-    scheduler._task_archive.last_changed = AsyncMock(return_value=Time.now())  # type: ignore
-    scheduler._last_change = Time.now()
+    time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
+    scheduler._task_archive.last_changed = AsyncMock(return_value=time)  # type: ignore
+    scheduler._last_change = time
+
+    await scheduler._update_worker_loop()
+
+    scheduler._update_blocks.assert_not_called()
+
+
+@pytest.mark.asyncio
+async def test_worker_loop_new_changes(mocker) -> None:
+    time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
+    mocker.patch("pyobs.utils.time.Time.now", return_value=time)
+    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
+    scheduler._update_blocks = AsyncMock()
+
+    scheduler._task_archive.last_changed = AsyncMock(return_value=time)  # type: ignore
+    scheduler._last_change = time - datetime.timedelta(minutes=1)
 
     await scheduler._update_worker_loop()
 
+    scheduler._update_blocks.assert_called_once()
+    assert scheduler._last_change == time
 
 
 @pytest.mark.asyncio
-async def test_worker_loop_no_changes(schedule_blocks) -> None:
+async def test_update_blocks_no_changes(schedule_blocks) -> None:
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
     scheduler._scheduler_task.start = Mock()  # type: ignore
     scheduler._task_archive.get_schedulable_blocks = AsyncMock(return_value=schedule_blocks)  # type: ignore
     scheduler._blocks = schedule_blocks
 
-    await scheduler._update_worker_loop()
+    await scheduler._update_blocks()
 
     scheduler._scheduler_task.start.assert_not_called()
 
 
 @pytest.mark.asyncio
-async def test_worker_loop_removed_current(schedule_blocks) -> None:
+async def test_update_blocks_removed_current(schedule_blocks) -> None:
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
     scheduler._scheduler_task.start = Mock()  # type: ignore
 
@@ -127,13 +146,13 @@ async def test_worker_loop_removed_current(schedule_blocks) -> None:
 
     scheduler._compare_block_lists = Mock(return_value=([schedule_blocks[0]], []))  # type: ignore
 
-    await scheduler._update_worker_loop()
+    await scheduler._update_blocks()
 
     scheduler._scheduler_task.start.assert_not_called()
 
 
 @pytest.mark.asyncio
-async def test_worker_loop_removed_not_in_schedule(schedule_blocks)  -> None:
+async def test_update_blocks_removed_not_in_schedule(schedule_blocks)  -> None:
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
     scheduler._scheduler_task.start = Mock()  # type: ignore
     scheduler._task_archive.get_schedulable_blocks = AsyncMock(return_value=schedule_blocks)  # type: ignore
@@ -142,13 +161,13 @@ async def test_worker_loop_removed_not_in_schedule(schedule_blocks)  -> None:
 
     scheduler._compare_block_lists = Mock(return_value=([schedule_blocks[0]], []))  # type: ignore
 
-    await scheduler._update_worker_loop()
+    await scheduler._update_blocks()
 
     scheduler._scheduler_task.start.assert_not_called()
 
 
 @pytest.mark.asyncio
-async def test_worker_loop_need_to_update(schedule_blocks) -> None:
+async def test_update_blocks_need_to_update(schedule_blocks) -> None:
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
     scheduler._scheduler_task.start = Mock()  # type: ignore
     scheduler._task_archive.get_schedulable_blocks = AsyncMock(return_value=schedule_blocks)  # type: ignore
@@ -157,7 +176,7 @@ async def test_worker_loop_need_to_update(schedule_blocks) -> None:
 
     scheduler._compare_block_lists = Mock(return_value=([], [schedule_blocks[0]]))  # type: ignore
 
-    await scheduler._update_worker_loop()
+    await scheduler._update_blocks()
 
     scheduler._scheduler_task.start.assert_called_once()
     assert scheduler._blocks == schedule_blocks

From ad20208b21bda16ebb5ec5dc510f6e321c51760d Mon Sep 17 00:00:00 2001
From: germanhydrogen <admin@germanhydrogen.org>
Date: Tue, 30 Apr 2024 18:58:33 +0200
Subject: [PATCH 17/25] Added broad test for pointing series

---
 tests/modules/robotic/conftest.py            |  8 ++++
 tests/modules/robotic/test_pointingseries.py | 47 ++++++++++++++++++++
 tests/modules/robotic/test_scheduler.py      |  5 +--
 3 files changed, 57 insertions(+), 3 deletions(-)
 create mode 100644 tests/modules/robotic/conftest.py
 create mode 100644 tests/modules/robotic/test_pointingseries.py

diff --git a/tests/modules/robotic/conftest.py b/tests/modules/robotic/conftest.py
new file mode 100644
index 000000000..10425e389
--- /dev/null
+++ b/tests/modules/robotic/conftest.py
@@ -0,0 +1,8 @@
+import pytest
+from astroplan import Observer
+import astropy.units as u
+
+@pytest.fixture(scope='module')
+def observer():
+    return Observer(longitude=20.8108 * u.deg, latitude=-32.375823 * u.deg,
+                        elevation=1798.0 * u.m, timezone="UTC")
\ No newline at end of file
diff --git a/tests/modules/robotic/test_pointingseries.py b/tests/modules/robotic/test_pointingseries.py
new file mode 100644
index 000000000..c86a73cac
--- /dev/null
+++ b/tests/modules/robotic/test_pointingseries.py
@@ -0,0 +1,47 @@
+import datetime
+from typing import Any, Dict
+from unittest.mock import AsyncMock
+
+import pytest
+
+import pyobs
+from pyobs.modules.robotic import PointingSeries
+
+
+class MockAcquisition:
+    async def acquire_target(self, **kwargs: Any) -> Dict[str, Any]:
+        return None
+
+
+class MockTelescope:
+    async def move_radec(*args, **kwargs):
+        pass
+
+
+class MockProxy:
+    def __init__(self, **kwargs):
+        self.acquisition = MockAcquisition()
+        self.telescope = MockTelescope()
+
+    async def __call__(self, name, *args, **kwargs):
+        if name == "acquisition":
+            return self.acquisition
+        if name == "telescope":
+            return self.telescope
+
+        raise ValueError
+
+
+@pytest.mark.asyncio
+async def test_run_thread(observer, mocker):
+    mock_proxy = MockProxy()
+    mock_proxy.telescope.move_radec = AsyncMock()
+
+    current_time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
+
+    mocker.patch("pyobs.utils.time.Time.now", return_value=current_time)
+    series = PointingSeries(num_alt=1, num_az=1, observer=observer, finish=1)
+    series.comm.proxy = mock_proxy
+    await series._run_thread()
+
+    mock_proxy.telescope.move_radec.assert_called_once_with(151.12839530803322, 27.74121078725986)
diff --git a/tests/modules/robotic/test_scheduler.py b/tests/modules/robotic/test_scheduler.py
index f38578410..1cc5f4b68 100644
--- a/tests/modules/robotic/test_scheduler.py
+++ b/tests/modules/robotic/test_scheduler.py
@@ -293,9 +293,8 @@ def mock_schedule_process(blocks: List[ObservingBlock], start: Time, end: Time,
 
 
 @pytest.mark.asyncio
-async def test_schedule_blocks() -> None:
-    observer = Observer(longitude=20.8108 * u.deg, latitude=-32.375823 * u.deg,
-                        elevation=1798.0 * u.m, timezone="UTC")
+async def test_schedule_blocks(observer) -> None:
+
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), observer=observer)
 
     scheduler._schedule_process = mock_schedule_process  # type: ignore

From f91156680a76210882e21846cb938cf21466574f Mon Sep 17 00:00:00 2001
From: GermanHydrogen <admin@germanhydrogen.org>
Date: Sun, 5 May 2024 12:20:38 +0200
Subject: [PATCH 18/25] Seperated pointing series running into a seperate class

---
 .../robotic/_pointingseriesiterator.py        | 104 ++++++++++++++++++
 pyobs/modules/robotic/pointing.py             |  98 +++++------------
 tests/modules/robotic/test_pointingseries.py  |   4 +-
 3 files changed, 133 insertions(+), 73 deletions(-)
 create mode 100644 pyobs/modules/robotic/_pointingseriesiterator.py

diff --git a/pyobs/modules/robotic/_pointingseriesiterator.py b/pyobs/modules/robotic/_pointingseriesiterator.py
new file mode 100644
index 000000000..7b6957b05
--- /dev/null
+++ b/pyobs/modules/robotic/_pointingseriesiterator.py
@@ -0,0 +1,104 @@
+from __future__ import annotations
+
+import logging
+import random
+from typing import Tuple, Any, Optional, List, Dict
+
+import astropy
+import astropy.units as u
+import pandas as pd
+from astroplan import Observer
+from astropy.coordinates import SkyCoord
+
+from pyobs.interfaces import IAcquisition, ITelescope
+from pyobs.utils import exceptions as exc
+from pyobs.utils.time import Time
+
+log = logging.getLogger(__name__)
+
+
+class _PointingSeriesIterator:
+    def __init__(self,
+                 observer: Observer,
+                 telescope: ITelescope,
+                 acquisition: IAcquisition,
+                 dec_range: Tuple[float, float],
+                 finish_percentage: float,
+                 min_moon_dist: float,
+                 grid_points: pd.DataFrame) -> None:
+
+        self._observer = observer
+        self._telescope = telescope
+        self._acquisition = acquisition
+
+        self._dec_range = dec_range
+        self._finish_fraction = 1.0 - finish_percentage / 100.0
+        self._min_moon_dist = min_moon_dist
+
+        self._grid_points = grid_points
+
+    def __aiter__(self) -> _PointingSeriesIterator:
+        return self
+
+    async def __anext__(self) -> Optional[Dict[str, Any]]:
+        if self._is_finished():
+            raise StopAsyncIteration()
+
+        todo_coords = self._get_todo_coords()
+        log.info("Grid points left to do: %d", len(todo_coords))
+
+        alt, az, radec = self._find_next_point(todo_coords)
+
+        try:
+            acquisition_result = await self._acquire_target(radec)
+        except (ValueError, exc.RemoteError):
+            log.info("Could not acquire target.")
+            return None
+
+        self._grid_points.loc[alt, az] = True
+        return acquisition_result
+
+    def _is_finished(self) -> bool:
+        num_finished_coords: int = sum(self._grid_points["done"].values)
+        total_num_coords: int = len(self._grid_points)
+
+        return num_finished_coords/total_num_coords >= self._finish_fraction
+
+    def _get_todo_coords(self) -> List[Tuple[float, float]]:
+        return list(self._grid_points[~self._grid_points["done"]].index)
+
+    def _find_next_point(self, todo_coords: List[Tuple[float, float]]) -> Tuple[float, float, astropy.coordinates.SkyCoord]:
+        moon = self._observer.moon_altaz(Time.now())
+        # try to find a good point
+        while True:
+            # pick a random index and remove from list
+            alt, az = random.sample(todo_coords, 1)[0]
+            todo_coords.remove((alt, az))
+            altaz = SkyCoord(
+                alt=alt * u.deg, az=az * u.deg, frame="altaz", obstime=Time.now(), location=self._observer.location
+            )
+
+            # get RA/Dec
+            radec = altaz.icrs
+
+            # moon far enough away?
+            if altaz.separation(moon).degree >= self._min_moon_dist:
+                # yep, are we in declination range?
+                if self._dec_range[0] <= radec.dec.degree < self._dec_range[1]:
+                    # yep, break here, we found our target
+                    break
+
+            # to do list empty?
+            if len(todo_coords) == 0:
+                # could not find a grid point
+                log.info("Could not find a suitable grid point, resetting todo list for next entry...")
+                todo_coords = list(self._grid_points.pd_grid.index)
+                continue
+
+        log.info("Picked grid point at Alt=%.2f, Az=%.2f (%s).", alt, az, radec.to_string("hmsdms"))
+        return alt, az, radec
+
+    async def _acquire_target(self, target: astropy.coordinates.SkyCoord) -> dict[str, Any]:
+        await self._telescope.move_radec(float(target.ra.degree), float(target.dec.degree))
+
+        return await self._acquisition.acquire_target()
diff --git a/pyobs/modules/robotic/pointing.py b/pyobs/modules/robotic/pointing.py
index 7c8f47100..d6428b3d2 100644
--- a/pyobs/modules/robotic/pointing.py
+++ b/pyobs/modules/robotic/pointing.py
@@ -2,13 +2,16 @@
 import random
 from typing import Tuple, Any, Optional, List, Dict
 
+import astropy
 import numpy as np
 import pandas as pd
+from astropy import units as u
 from astropy.coordinates import SkyCoord
 import astropy.units as u
 
 from pyobs.interfaces import IAcquisition, ITelescope
 from pyobs.modules import Module
+from pyobs.modules.robotic._pointingseriesiterator import _PointingSeriesIterator
 from pyobs.utils import exceptions as exc
 from pyobs.interfaces import IAutonomous
 from pyobs.utils.time import Time
@@ -56,13 +59,14 @@ def __init__(
         self._num_alt = num_alt
         self._az_range = tuple(az_range)
         self._num_az = num_az
-        self._dec_range = dec_range
-        self._min_moon_dist = min_moon_dist
-        self._finish = 1.0 - finish / 100.0
         self._exp_time = exp_time
         self._acquisition = acquisition
         self._telescope = telescope
 
+        self._dec_range = dec_range
+        self._min_moon_dist = min_moon_dist
+        self._finish = finish
+
         # if Az range is [0, 360], we got north double, so remove one step
         if self._az_range == (0.0, 360.0):
             self._az_range = (0.0, 360.0 - 360.0 / self._num_az)
@@ -85,16 +89,7 @@ async def is_running(self, **kwargs: Any) -> bool:
     async def _run_thread(self) -> None:
         """Run a pointing series."""
 
-        # create grid
-        grid: Dict[str, List[Any]] = {"alt": [], "az": [], "done": []}
-        for az in np.linspace(self._az_range[0], self._az_range[1], self._num_az):
-            for alt in np.linspace(self._alt_range[0], self._alt_range[1], self._num_alt):
-                grid["alt"] += [alt]
-                grid["az"] += [az]
-                grid["done"] += [False]
-
-        # to dataframe
-        pd_grid = pd.DataFrame(grid).set_index(["alt", "az"])
+        pd_grid = self._generate_grid()
 
         # get acquisition and telescope units
         acquisition = await self.proxy(self._acquisition, IAcquisition)
@@ -104,69 +99,28 @@ async def _run_thread(self) -> None:
         if self.observer is None:
             raise ValueError("No observer given.")
 
-        # loop until finished
-        while True:
-            # get all entries without offset measurements
-            todo = list(pd_grid[~pd_grid["done"]].index)
-            if len(todo) / len(pd_grid) < self._finish:
-                log.info("Finished.")
-                break
-            log.info("Grid points left to do: %d", len(todo))
-
-            # get moon
-            moon = self.observer.moon_altaz(Time.now())
-
-            # try to find a good point
-            while True:
-                # pick a random index and remove from list
-                alt, az = random.sample(todo, 1)[0]
-                todo.remove((alt, az))
-                altaz = SkyCoord(
-                    alt=alt * u.deg, az=az * u.deg, frame="altaz", obstime=Time.now(), location=self.observer.location
-                )
-
-                # get RA/Dec
-                radec = altaz.icrs
-
-                # moon far enough away?
-                if altaz.separation(moon).degree >= self._min_moon_dist:
-                    # yep, are we in declination range?
-                    if self._dec_range[0] <= radec.dec.degree < self._dec_range[1]:
-                        # yep, break here, we found our target
-                        break
-
-                # to do list empty?
-                if len(todo) == 0:
-                    # could not find a grid point
-                    log.info("Could not find a suitable grid point, resetting todo list for next entry...")
-                    todo = list(pd_grid.index)
-                    continue
-
-            # log finding
-            log.info("Picked grid point at Alt=%.2f, Az=%.2f (%s).", alt, az, radec.to_string("hmsdms"))
-
-            # acquire target and process result
-            try:
-                # move telescope
-                await telescope.move_radec(float(radec.ra.degree), float(radec.dec.degree))
-
-                # acquire target
-                acq = await acquisition.acquire_target()
-
-                #  process result
-                if acq is not None:
-                    await self._process_acquisition(**acq)
-
-            except (ValueError, exc.RemoteError):
-                log.info("Could not acquire target.")
-                continue
-
-            # finished
-            pd_grid.loc[alt, az] = True
+        pointing_series = _PointingSeriesIterator(self.observer, telescope, acquisition, self._dec_range, self._min_moon_dist, self._finish, pd_grid)
+
+        async for acquisition_result in pointing_series:
+            if acquisition_result is not None:
+                await self._process_acquisition(**acquisition_result)
 
         # finished
         log.info("Pointing series finished.")
 
+    def _generate_grid(self) -> pd.DataFrame:
+        # create grid
+        grid: Dict[str, List[Any]] = {"alt": [], "az": [], "done": []}
+        for az in np.linspace(self._az_range[0], self._az_range[1], self._num_az):
+            for alt in np.linspace(self._alt_range[0], self._alt_range[1], self._num_alt):
+                grid["alt"] += [alt]
+                grid["az"] += [az]
+                grid["done"] += [False]
+        # to dataframe
+        pd_grid = pd.DataFrame(grid).set_index(["alt", "az"])
+
+        return pd_grid
+
     async def _process_acquisition(
         self,
         datetime: str,
diff --git a/tests/modules/robotic/test_pointingseries.py b/tests/modules/robotic/test_pointingseries.py
index c86a73cac..a68ea9a10 100644
--- a/tests/modules/robotic/test_pointingseries.py
+++ b/tests/modules/robotic/test_pointingseries.py
@@ -10,7 +10,7 @@
 
 class MockAcquisition:
     async def acquire_target(self, **kwargs: Any) -> Dict[str, Any]:
-        return None
+        return {"datetime": "", "ra": 0.0, "dec": 0.0, "az": 0.0, "alt": 0.0}
 
 
 class MockTelescope:
@@ -42,6 +42,8 @@ async def test_run_thread(observer, mocker):
     mocker.patch("pyobs.utils.time.Time.now", return_value=current_time)
     series = PointingSeries(num_alt=1, num_az=1, observer=observer, finish=1)
     series.comm.proxy = mock_proxy
+    series._process_acquisition = AsyncMock()
     await series._run_thread()
 
     mock_proxy.telescope.move_radec.assert_called_once_with(151.12839530803322, 27.74121078725986)
+    series._process_acquisition.assert_called_once_with(**{"datetime": "", "ra": 0.0, "dec": 0.0, "az": 0.0, "alt": 0.0})

From e04f18ce8e78dff93b8e7de17825187e5d661367 Mon Sep 17 00:00:00 2001
From: GermanHydrogen <admin@germanhydrogen.org>
Date: Sun, 5 May 2024 13:13:13 +0200
Subject: [PATCH 19/25] Added looped random iterator

---
 .../robotic/_pointingseriesiterator.py        | 54 +++++++++++--------
 .../robotic/test_loopedrandomiterator.py      | 10 ++++
 2 files changed, 41 insertions(+), 23 deletions(-)
 create mode 100644 tests/modules/robotic/test_loopedrandomiterator.py

diff --git a/pyobs/modules/robotic/_pointingseriesiterator.py b/pyobs/modules/robotic/_pointingseriesiterator.py
index 7b6957b05..429c1628f 100644
--- a/pyobs/modules/robotic/_pointingseriesiterator.py
+++ b/pyobs/modules/robotic/_pointingseriesiterator.py
@@ -2,6 +2,8 @@
 
 import logging
 import random
+from collections.abc import Iterator
+from copy import copy
 from typing import Tuple, Any, Optional, List, Dict
 
 import astropy
@@ -17,6 +19,24 @@
 log = logging.getLogger(__name__)
 
 
+class _LoopedRandomIterator(Iterator[Any]):
+    def __init__(self, data: List[Any]) -> None:
+        self._data = copy(data)
+        self._todo = copy(data)
+
+    def __iter__(self) -> _LoopedRandomIterator:
+        return self
+
+    def __next__(self) -> Any:
+        if len(self._todo) == 0:
+            self._todo = copy(self._data)
+
+        item = random.sample(self._todo, 1)[0]
+        self._todo.remove(item)
+
+        return item
+
+
 class _PointingSeriesIterator:
     def __init__(self,
                  observer: Observer,
@@ -67,38 +87,26 @@ def _is_finished(self) -> bool:
     def _get_todo_coords(self) -> List[Tuple[float, float]]:
         return list(self._grid_points[~self._grid_points["done"]].index)
 
-    def _find_next_point(self, todo_coords: List[Tuple[float, float]]) -> Tuple[float, float, astropy.coordinates.SkyCoord]:
+    def _find_next_point(self, todo_coords: List[Tuple[float, float]]) -> Tuple[float, float, astropy.coordinates.SkyCoord]:  # type: ignore
         moon = self._observer.moon_altaz(Time.now())
-        # try to find a good point
-        while True:
-            # pick a random index and remove from list
-            alt, az = random.sample(todo_coords, 1)[0]
-            todo_coords.remove((alt, az))
+
+        for alt, az in _LoopedRandomIterator(todo_coords):
             altaz = SkyCoord(
                 alt=alt * u.deg, az=az * u.deg, frame="altaz", obstime=Time.now(), location=self._observer.location
             )
-
-            # get RA/Dec
             radec = altaz.icrs
 
-            # moon far enough away?
-            if altaz.separation(moon).degree >= self._min_moon_dist:
-                # yep, are we in declination range?
-                if self._dec_range[0] <= radec.dec.degree < self._dec_range[1]:
-                    # yep, break here, we found our target
-                    break
+            if self._is_valid_target(altaz, radec, moon):
+                log.info("Picked grid point at Alt=%.2f, Az=%.2f (%s).", alt, az, radec.to_string("hmsdms"))
+                return alt, az, radec
 
-            # to do list empty?
-            if len(todo_coords) == 0:
-                # could not find a grid point
-                log.info("Could not find a suitable grid point, resetting todo list for next entry...")
-                todo_coords = list(self._grid_points.pd_grid.index)
-                continue
+    def _is_valid_target(self, altaz_coords: SkyCoord, radec_coords: SkyCoord, moon: SkyCoord) -> bool:
+        moon_separation_condition: bool = altaz_coords.separation(moon).degree >= self._min_moon_dist
+        dec_range_condition: bool = self._dec_range[0] <= radec_coords.dec.degree < self._dec_range[1]
 
-        log.info("Picked grid point at Alt=%.2f, Az=%.2f (%s).", alt, az, radec.to_string("hmsdms"))
-        return alt, az, radec
+        return moon_separation_condition and dec_range_condition
 
-    async def _acquire_target(self, target: astropy.coordinates.SkyCoord) -> dict[str, Any]:
+    async def _acquire_target(self, target: SkyCoord) -> dict[str, Any]:
         await self._telescope.move_radec(float(target.ra.degree), float(target.dec.degree))
 
         return await self._acquisition.acquire_target()
diff --git a/tests/modules/robotic/test_loopedrandomiterator.py b/tests/modules/robotic/test_loopedrandomiterator.py
new file mode 100644
index 000000000..a11e4c607
--- /dev/null
+++ b/tests/modules/robotic/test_loopedrandomiterator.py
@@ -0,0 +1,10 @@
+from pyobs.modules.robotic._pointingseriesiterator import _LoopedRandomIterator
+
+
+def test_iter() -> None:
+    data = [1, 2]
+
+    iterator = _LoopedRandomIterator(data)
+
+    assert data == [next(iterator), next(iterator)]  # Check first cycle
+    assert data == [next(iterator), next(iterator)]  # Check second cycle

From 79158dd9a6f883be8736734ea9e90d790cd8d14b Mon Sep 17 00:00:00 2001
From: GermanHydrogen <admin@germanhydrogen.org>
Date: Mon, 6 May 2024 08:43:18 +0200
Subject: [PATCH 20/25] Added tests to pointing series iterator

---
 .../robotic/_pointingseriesiterator.py        |  4 +-
 tests/modules/robotic/conftest.py             | 13 ++++
 .../robotic/test_loopedrandomiterator.py      |  4 +-
 tests/modules/robotic/test_pointingseries.py  | 23 ++----
 .../robotic/test_pointingseriesiterator.py    | 73 +++++++++++++++++++
 5 files changed, 97 insertions(+), 20 deletions(-)
 create mode 100644 tests/modules/robotic/test_pointingseriesiterator.py

diff --git a/pyobs/modules/robotic/_pointingseriesiterator.py b/pyobs/modules/robotic/_pointingseriesiterator.py
index 429c1628f..fd053a461 100644
--- a/pyobs/modules/robotic/_pointingseriesiterator.py
+++ b/pyobs/modules/robotic/_pointingseriesiterator.py
@@ -82,7 +82,7 @@ def _is_finished(self) -> bool:
         num_finished_coords: int = sum(self._grid_points["done"].values)
         total_num_coords: int = len(self._grid_points)
 
-        return num_finished_coords/total_num_coords >= self._finish_fraction
+        return num_finished_coords >= self._finish_fraction * total_num_coords
 
     def _get_todo_coords(self) -> List[Tuple[float, float]]:
         return list(self._grid_points[~self._grid_points["done"]].index)
@@ -94,7 +94,7 @@ def _find_next_point(self, todo_coords: List[Tuple[float, float]]) -> Tuple[floa
             altaz = SkyCoord(
                 alt=alt * u.deg, az=az * u.deg, frame="altaz", obstime=Time.now(), location=self._observer.location
             )
-            radec = altaz.icrs
+            radec = altaz.transform_to("icrs")
 
             if self._is_valid_target(altaz, radec, moon):
                 log.info("Picked grid point at Alt=%.2f, Az=%.2f (%s).", alt, az, radec.to_string("hmsdms"))
diff --git a/tests/modules/robotic/conftest.py b/tests/modules/robotic/conftest.py
index 10425e389..91e59b6e2 100644
--- a/tests/modules/robotic/conftest.py
+++ b/tests/modules/robotic/conftest.py
@@ -1,7 +1,20 @@
+from typing import Any, Dict
+
 import pytest
 from astroplan import Observer
 import astropy.units as u
 
+
+class MockAcquisition:
+    async def acquire_target(self, **kwargs: Any) -> Dict[str, Any]:
+        return {"datetime": "", "ra": 0.0, "dec": 0.0, "az": 0.0, "alt": 0.0}
+
+
+class MockTelescope:
+    async def move_radec(*args: Any, **kwargs: Any) -> None:
+        pass
+
+
 @pytest.fixture(scope='module')
 def observer():
     return Observer(longitude=20.8108 * u.deg, latitude=-32.375823 * u.deg,
diff --git a/tests/modules/robotic/test_loopedrandomiterator.py b/tests/modules/robotic/test_loopedrandomiterator.py
index a11e4c607..87903bd33 100644
--- a/tests/modules/robotic/test_loopedrandomiterator.py
+++ b/tests/modules/robotic/test_loopedrandomiterator.py
@@ -6,5 +6,5 @@ def test_iter() -> None:
 
     iterator = _LoopedRandomIterator(data)
 
-    assert data == [next(iterator), next(iterator)]  # Check first cycle
-    assert data == [next(iterator), next(iterator)]  # Check second cycle
+    assert set(data) == {next(iterator), next(iterator)}  # Check first cycle
+    assert set(data) == {next(iterator), next(iterator)}  # Check second cycle
diff --git a/tests/modules/robotic/test_pointingseries.py b/tests/modules/robotic/test_pointingseries.py
index a68ea9a10..29221e948 100644
--- a/tests/modules/robotic/test_pointingseries.py
+++ b/tests/modules/robotic/test_pointingseries.py
@@ -1,29 +1,20 @@
 import datetime
-from typing import Any, Dict
+from typing import Any, Union
 from unittest.mock import AsyncMock
 
 import pytest
 
 import pyobs
 from pyobs.modules.robotic import PointingSeries
-
-
-class MockAcquisition:
-    async def acquire_target(self, **kwargs: Any) -> Dict[str, Any]:
-        return {"datetime": "", "ra": 0.0, "dec": 0.0, "az": 0.0, "alt": 0.0}
-
-
-class MockTelescope:
-    async def move_radec(*args, **kwargs):
-        pass
+from tests.modules.robotic.conftest import MockAcquisition, MockTelescope
 
 
 class MockProxy:
-    def __init__(self, **kwargs):
+    def __init__(self, **kwargs: Any) -> None:
         self.acquisition = MockAcquisition()
         self.telescope = MockTelescope()
 
-    async def __call__(self, name, *args, **kwargs):
+    async def __call__(self, name: str, *args: Any, **kwargs: Any) -> Union[MockTelescope, MockAcquisition]:
         if name == "acquisition":
             return self.acquisition
         if name == "telescope":
@@ -35,14 +26,14 @@ async def __call__(self, name, *args, **kwargs):
 @pytest.mark.asyncio
 async def test_run_thread(observer, mocker):
     mock_proxy = MockProxy()
-    mock_proxy.telescope.move_radec = AsyncMock()
+    mock_proxy.telescope.move_radec = AsyncMock()  # type: ignore
 
     current_time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
 
     mocker.patch("pyobs.utils.time.Time.now", return_value=current_time)
     series = PointingSeries(num_alt=1, num_az=1, observer=observer, finish=1)
-    series.comm.proxy = mock_proxy
-    series._process_acquisition = AsyncMock()
+    series.comm.proxy = mock_proxy  # type: ignore
+    series._process_acquisition = AsyncMock()  # type: ignore
     await series._run_thread()
 
     mock_proxy.telescope.move_radec.assert_called_once_with(151.12839530803322, 27.74121078725986)
diff --git a/tests/modules/robotic/test_pointingseriesiterator.py b/tests/modules/robotic/test_pointingseriesiterator.py
new file mode 100644
index 000000000..128e8b6ea
--- /dev/null
+++ b/tests/modules/robotic/test_pointingseriesiterator.py
@@ -0,0 +1,73 @@
+import datetime
+from unittest.mock import AsyncMock
+
+import astroplan
+import pandas as pd
+import pytest
+import pytest_mock
+from astropy.coordinates import SkyCoord
+
+import pyobs
+from pyobs.modules.robotic._pointingseriesiterator import _PointingSeriesIterator
+from tests.modules.robotic.conftest import MockTelescope, MockAcquisition
+
+
+@pytest.mark.asyncio
+async def test_empty(observer: astroplan.Observer) -> None:
+    coords = pd.DataFrame({"alt": [], "az": [], "done": []})
+    coords.set_index(["alt", "az"])
+
+    iterator = _PointingSeriesIterator(observer,
+                                       MockTelescope(),  # type: ignore
+                                       MockAcquisition(),  # type: ignore
+                                       (-80.0, 80.0), 1, 0, coords)
+
+    with pytest.raises(StopAsyncIteration):
+        await anext(iterator)  # type: ignore
+
+
+@pytest.mark.asyncio
+async def test_next(observer: astroplan.Observer, mocker: pytest_mock.MockerFixture) -> None:
+    current_time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
+
+    mocker.patch("astropy.coordinates.SkyCoord.transform_to", return_value=SkyCoord(0, 0, unit='deg', frame='icrs'))
+    mocker.patch("astroplan.Observer.moon_altaz", return_value=SkyCoord(-90, 0, unit='deg', frame='altaz', obstime=current_time, location=observer.location))
+
+    coords = pd.DataFrame({"alt": [60], "az": [0], "done": [False]})
+    coords = coords.set_index(["alt", "az"])
+
+    mocker.patch("pyobs.utils.time.Time.now", return_value=current_time)
+
+    iterator = _PointingSeriesIterator(observer,
+                                       MockTelescope(),     # type: ignore
+                                       MockAcquisition(),   # type: ignore
+                                       (-80.0, 80.0), 1, 0, coords)
+
+    iterator._telescope.move_radec = AsyncMock()  # type: ignore
+
+    assert await anext(iterator) == {"datetime": "", "ra": 0.0, "dec": 0.0, "az": 0.0, "alt": 0.0}  # type: ignore
+
+    iterator._telescope.move_radec.assert_called_once_with(0, 0)
+
+
+@pytest.mark.asyncio
+async def test_acquire_error(observer: astroplan.Observer, mocker: pytest_mock.MockerFixture) -> None:
+    current_time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
+
+    mocker.patch("astropy.coordinates.SkyCoord.transform_to", return_value=SkyCoord(0, 0, unit='deg', frame='icrs'))
+    mocker.patch("astroplan.Observer.moon_altaz", return_value=SkyCoord(-90, 0, unit='deg', frame='altaz', obstime=current_time, location=observer.location))
+
+    coords = pd.DataFrame({"alt": [60], "az": [0], "done": [False]})
+    coords = coords.set_index(["alt", "az"])
+
+    mocker.patch("pyobs.utils.time.Time.now", return_value=current_time)
+
+    iterator = _PointingSeriesIterator(observer,
+                                       MockTelescope(),     # type: ignore
+                                       MockAcquisition(),   # type: ignore
+                                       (-80.0, 80.0), 1, 0, coords)
+
+    iterator._acquisition.acquire_target = AsyncMock(side_effect=ValueError)  # type: ignore
+
+    assert await anext(iterator) is None  # type: ignore
+

From 9e733f29af9813d28f9b71276ae6fa21b03e4a59 Mon Sep 17 00:00:00 2001
From: GermanHydrogen <admin@germanhydrogen.org>
Date: Mon, 6 May 2024 09:29:41 +0200
Subject: [PATCH 21/25] Changed api of pointing series iterator to facilitate
 better testing of pointing series

---
 .../robotic/_pointingseriesiterator.py        | 32 +++++--
 pyobs/modules/robotic/pointing.py             | 83 +++++++++----------
 tests/modules/robotic/test_pointingseries.py  |  4 +-
 .../robotic/test_pointingseriesiterator.py    | 45 +++++++---
 4 files changed, 98 insertions(+), 66 deletions(-)

diff --git a/pyobs/modules/robotic/_pointingseriesiterator.py b/pyobs/modules/robotic/_pointingseriesiterator.py
index fd053a461..e1feeb08a 100644
--- a/pyobs/modules/robotic/_pointingseriesiterator.py
+++ b/pyobs/modules/robotic/_pointingseriesiterator.py
@@ -4,7 +4,7 @@
 import random
 from collections.abc import Iterator
 from copy import copy
-from typing import Tuple, Any, Optional, List, Dict
+from typing import Tuple, Any, Optional, List, Dict, cast
 
 import astropy
 import astropy.units as u
@@ -40,24 +40,36 @@ def __next__(self) -> Any:
 class _PointingSeriesIterator:
     def __init__(self,
                  observer: Observer,
-                 telescope: ITelescope,
-                 acquisition: IAcquisition,
                  dec_range: Tuple[float, float],
                  finish_percentage: float,
-                 min_moon_dist: float,
-                 grid_points: pd.DataFrame) -> None:
+                 min_moon_dist: float) -> None:
 
         self._observer = observer
-        self._telescope = telescope
-        self._acquisition = acquisition
+        self._telescope: Optional[ITelescope] = None
+        self._acquisition: Optional[IAcquisition] = None
 
         self._dec_range = dec_range
         self._finish_fraction = 1.0 - finish_percentage / 100.0
         self._min_moon_dist = min_moon_dist
 
+        self._grid_points: pd.DataFrame = pd.DataFrame({"alt": [], "az": [], "done": []})
+
+    def set_grid_points(self, grid_points: pd.DataFrame) -> None:
         self._grid_points = grid_points
 
+    def set_telescope(self, telescope: ITelescope) -> None:
+        self._telescope = telescope
+
+    def set_acquisition(self, acquisition: IAcquisition) -> None:
+        self._acquisition = acquisition
+
     def __aiter__(self) -> _PointingSeriesIterator:
+        if self._acquisition is None:
+            raise ValueError("Acquisition is not set.")
+
+        if self._telescope is None:
+            raise ValueError("Telescope is not set.")
+
         return self
 
     async def __anext__(self) -> Optional[Dict[str, Any]]:
@@ -107,6 +119,8 @@ def _is_valid_target(self, altaz_coords: SkyCoord, radec_coords: SkyCoord, moon:
         return moon_separation_condition and dec_range_condition
 
     async def _acquire_target(self, target: SkyCoord) -> dict[str, Any]:
-        await self._telescope.move_radec(float(target.ra.degree), float(target.dec.degree))
+        telescope = cast(ITelescope, self._telescope)   # Telescope has to be set in order to enter the iteration
+        await telescope.move_radec(float(target.ra.degree), float(target.dec.degree))
 
-        return await self._acquisition.acquire_target()
+        acquisition = cast(IAcquisition, self._acquisition)
+        return await acquisition.acquire_target()   # Acquisition has to be set in order to enter the iteration
diff --git a/pyobs/modules/robotic/pointing.py b/pyobs/modules/robotic/pointing.py
index d6428b3d2..0e799e740 100644
--- a/pyobs/modules/robotic/pointing.py
+++ b/pyobs/modules/robotic/pointing.py
@@ -1,20 +1,13 @@
 import logging
-import random
 from typing import Tuple, Any, Optional, List, Dict
 
-import astropy
 import numpy as np
 import pandas as pd
-from astropy import units as u
-from astropy.coordinates import SkyCoord
-import astropy.units as u
 
 from pyobs.interfaces import IAcquisition, ITelescope
+from pyobs.interfaces import IAutonomous
 from pyobs.modules import Module
 from pyobs.modules.robotic._pointingseriesiterator import _PointingSeriesIterator
-from pyobs.utils import exceptions as exc
-from pyobs.interfaces import IAutonomous
-from pyobs.utils.time import Time
 
 log = logging.getLogger(__name__)
 
@@ -25,18 +18,18 @@ class PointingSeries(Module, IAutonomous):
     __module__ = "pyobs.modules.robotic"
 
     def __init__(
-        self,
-        alt_range: Tuple[float, float] = (30.0, 85.0),
-        num_alt: int = 8,
-        az_range: Tuple[float, float] = (0.0, 360.0),
-        num_az: int = 24,
-        dec_range: Tuple[float, float] = (-80.0, 80.0),
-        min_moon_dist: float = 15.0,
-        finish: int = 90,
-        exp_time: float = 1.0,
-        acquisition: str = "acquisition",
-        telescope: str = "telescope",
-        **kwargs: Any,
+            self,
+            alt_range: Tuple[float, float] = (30.0, 85.0),
+            num_alt: int = 8,
+            az_range: Tuple[float, float] = (0.0, 360.0),
+            num_az: int = 24,
+            dec_range: Tuple[float, float] = (-80.0, 80.0),
+            min_moon_dist: float = 15.0,
+            finish: int = 90,
+            exp_time: float = 1.0,
+            acquisition: str = "acquisition",
+            telescope: str = "telescope",
+            **kwargs: Any,
     ):
         """Initialize a new auto focus system.
 
@@ -59,13 +52,15 @@ def __init__(
         self._num_alt = num_alt
         self._az_range = tuple(az_range)
         self._num_az = num_az
-        self._exp_time = exp_time
+
         self._acquisition = acquisition
         self._telescope = telescope
 
-        self._dec_range = dec_range
-        self._min_moon_dist = min_moon_dist
-        self._finish = finish
+        self._pointing_series_iterator = _PointingSeriesIterator(
+            self.observer, dec_range,
+            finish_percentage=finish,
+            min_moon_dist=min_moon_dist
+        )
 
         # if Az range is [0, 360], we got north double, so remove one step
         if self._az_range == (0.0, 360.0):
@@ -86,22 +81,22 @@ async def is_running(self, **kwargs: Any) -> bool:
         """Whether a service is running."""
         return True
 
-    async def _run_thread(self) -> None:
-        """Run a pointing series."""
+    async def open(self, **kwargs: Any) -> None:
+        await Module.open(self)
 
-        pd_grid = self._generate_grid()
-
-        # get acquisition and telescope units
         acquisition = await self.proxy(self._acquisition, IAcquisition)
         telescope = await self.proxy(self._telescope, ITelescope)
 
-        # check observer
-        if self.observer is None:
-            raise ValueError("No observer given.")
+        self._pointing_series_iterator.set_acquisition(acquisition)
+        self._pointing_series_iterator.set_telescope(telescope)
 
-        pointing_series = _PointingSeriesIterator(self.observer, telescope, acquisition, self._dec_range, self._min_moon_dist, self._finish, pd_grid)
+    async def _run_thread(self) -> None:
+        """Run a pointing series."""
+
+        pd_grid = self._generate_grid()
+        self._pointing_series_iterator.set_grid_points(pd_grid)
 
-        async for acquisition_result in pointing_series:
+        async for acquisition_result in self._pointing_series_iterator:
             if acquisition_result is not None:
                 await self._process_acquisition(**acquisition_result)
 
@@ -122,16 +117,16 @@ def _generate_grid(self) -> pd.DataFrame:
         return pd_grid
 
     async def _process_acquisition(
-        self,
-        datetime: str,
-        ra: float,
-        dec: float,
-        alt: float,
-        az: float,
-        off_ra: Optional[float] = None,
-        off_dec: Optional[float] = None,
-        off_alt: Optional[float] = None,
-        off_az: Optional[float] = None,
+            self,
+            datetime: str,
+            ra: float,
+            dec: float,
+            alt: float,
+            az: float,
+            off_ra: Optional[float] = None,
+            off_dec: Optional[float] = None,
+            off_alt: Optional[float] = None,
+            off_az: Optional[float] = None,
     ) -> None:
         """Process the result of the acquisition. Either ra_off/dec_off or alt_off/az_off must be given.
 
diff --git a/tests/modules/robotic/test_pointingseries.py b/tests/modules/robotic/test_pointingseries.py
index 29221e948..f0f964197 100644
--- a/tests/modules/robotic/test_pointingseries.py
+++ b/tests/modules/robotic/test_pointingseries.py
@@ -31,9 +31,11 @@ async def test_run_thread(observer, mocker):
     current_time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
 
     mocker.patch("pyobs.utils.time.Time.now", return_value=current_time)
-    series = PointingSeries(num_alt=1, num_az=1, observer=observer, finish=1)
+    series = PointingSeries(num_alt=1, num_az=1, observer=observer, finish=0)
     series.comm.proxy = mock_proxy  # type: ignore
     series._process_acquisition = AsyncMock()  # type: ignore
+
+    await series.open()
     await series._run_thread()
 
     mock_proxy.telescope.move_radec.assert_called_once_with(151.12839530803322, 27.74121078725986)
diff --git a/tests/modules/robotic/test_pointingseriesiterator.py b/tests/modules/robotic/test_pointingseriesiterator.py
index 128e8b6ea..a8146ea01 100644
--- a/tests/modules/robotic/test_pointingseriesiterator.py
+++ b/tests/modules/robotic/test_pointingseriesiterator.py
@@ -12,15 +12,36 @@
 from tests.modules.robotic.conftest import MockTelescope, MockAcquisition
 
 
+@pytest.mark.asyncio
+async def test_telescope_not_set(observer: astroplan.Observer) -> None:
+    iterator = _PointingSeriesIterator(observer, (-80.0, 80.0), 0, 0)
+    iterator.set_acquisition(MockAcquisition())  # type: ignore
+
+    with pytest.raises(ValueError):
+        async for _ in iterator:
+            ...
+
+
+@pytest.mark.asyncio
+async def test_acquisition_not_set(observer: astroplan.Observer) -> None:
+    iterator = _PointingSeriesIterator(observer, (-80.0, 80.0), 0, 0)
+    iterator.set_telescope(MockTelescope())  # type: ignore
+
+    with pytest.raises(ValueError):
+        async for _ in iterator:
+            ...
+
+
 @pytest.mark.asyncio
 async def test_empty(observer: astroplan.Observer) -> None:
     coords = pd.DataFrame({"alt": [], "az": [], "done": []})
     coords.set_index(["alt", "az"])
 
-    iterator = _PointingSeriesIterator(observer,
-                                       MockTelescope(),  # type: ignore
-                                       MockAcquisition(),  # type: ignore
-                                       (-80.0, 80.0), 1, 0, coords)
+    iterator = _PointingSeriesIterator(observer, (-80.0, 80.0), 0, 0)
+
+    iterator.set_telescope(MockTelescope())  # type: ignore
+    iterator.set_acquisition(MockAcquisition())  # type: ignore
+    iterator.set_grid_points(coords)
 
     with pytest.raises(StopAsyncIteration):
         await anext(iterator)  # type: ignore
@@ -38,10 +59,10 @@ async def test_next(observer: astroplan.Observer, mocker: pytest_mock.MockerFixt
 
     mocker.patch("pyobs.utils.time.Time.now", return_value=current_time)
 
-    iterator = _PointingSeriesIterator(observer,
-                                       MockTelescope(),     # type: ignore
-                                       MockAcquisition(),   # type: ignore
-                                       (-80.0, 80.0), 1, 0, coords)
+    iterator = _PointingSeriesIterator(observer,(-80.0, 80.0), 0, 0)
+    iterator.set_telescope(MockTelescope())  # type: ignore
+    iterator.set_acquisition(MockAcquisition())  # type: ignore
+    iterator.set_grid_points(coords)
 
     iterator._telescope.move_radec = AsyncMock()  # type: ignore
 
@@ -62,10 +83,10 @@ async def test_acquire_error(observer: astroplan.Observer, mocker: pytest_mock.M
 
     mocker.patch("pyobs.utils.time.Time.now", return_value=current_time)
 
-    iterator = _PointingSeriesIterator(observer,
-                                       MockTelescope(),     # type: ignore
-                                       MockAcquisition(),   # type: ignore
-                                       (-80.0, 80.0), 1, 0, coords)
+    iterator = _PointingSeriesIterator(observer, (-80.0, 80.0), 0, 0)
+    iterator.set_telescope(MockTelescope())  # type: ignore
+    iterator.set_acquisition(MockAcquisition())  # type: ignore
+    iterator.set_grid_points(coords)
 
     iterator._acquisition.acquire_target = AsyncMock(side_effect=ValueError)  # type: ignore
 

From 7da04b03f1f0076744185eda747091165c67e982 Mon Sep 17 00:00:00 2001
From: GermanHydrogen <admin@germanhydrogen.org>
Date: Mon, 6 May 2024 10:51:43 +0200
Subject: [PATCH 22/25] Added missing tests to pointing series

---
 pyobs/modules/robotic/pointing.py             |  6 +-
 tests/modules/robotic/test_pointingseries.py  | 55 +++++++++++++++++--
 .../robotic/test_pointingseriesiterator.py    |  3 +-
 3 files changed, 54 insertions(+), 10 deletions(-)

diff --git a/pyobs/modules/robotic/pointing.py b/pyobs/modules/robotic/pointing.py
index 0e799e740..09eb5381f 100644
--- a/pyobs/modules/robotic/pointing.py
+++ b/pyobs/modules/robotic/pointing.py
@@ -56,6 +56,9 @@ def __init__(
         self._acquisition = acquisition
         self._telescope = telescope
 
+        if self.observer is None:
+            raise ValueError("No observer given.")
+
         self._pointing_series_iterator = _PointingSeriesIterator(
             self.observer, dec_range,
             finish_percentage=finish,
@@ -71,11 +74,9 @@ def __init__(
 
     async def start(self, **kwargs: Any) -> None:
         """Starts a service."""
-        pass
 
     async def stop(self, **kwargs: Any) -> None:
         """Stops a service."""
-        pass
 
     async def is_running(self, **kwargs: Any) -> bool:
         """Whether a service is running."""
@@ -141,7 +142,6 @@ async def _process_acquisition(
             off_alt: Found Alt offset.
             off_az: Found Az offset.
         """
-        pass
 
 
 __all__ = ["PointingSeries"]
diff --git a/tests/modules/robotic/test_pointingseries.py b/tests/modules/robotic/test_pointingseries.py
index f0f964197..6b49682de 100644
--- a/tests/modules/robotic/test_pointingseries.py
+++ b/tests/modules/robotic/test_pointingseries.py
@@ -1,8 +1,12 @@
+from __future__ import annotations
 import datetime
-from typing import Any, Union
+from typing import Any, Union, Optional, Dict
 from unittest.mock import AsyncMock
 
+import pandas as pd
 import pytest
+import pytest_mock
+from astroplan import Observer
 
 import pyobs
 from pyobs.modules.robotic import PointingSeries
@@ -23,20 +27,61 @@ async def __call__(self, name: str, *args: Any, **kwargs: Any) -> Union[MockTele
         raise ValueError
 
 
+class MockPointingSeriesIterator:
+    def __init__(self) -> None:
+        self.grid_points = pd.DataFrame()
+        self.counter = None
+
+    def set_grid_points(self, grid_points: pd.DataFrame) -> None:
+        self.grid_points = grid_points
+
+    def __aiter__(self) -> MockPointingSeriesIterator:
+        self.counter = len(self.grid_points)
+        return self
+
+    async def __anext__(self) -> Optional[Dict[str, Any]]:
+        self.counter -= 1
+        if self.counter >= 0:
+            return {"datetime": "", "ra": 0.0, "dec": 0.0, "az": 0.0, "alt": 0.0}
+        raise StopAsyncIteration
+
+
 @pytest.mark.asyncio
-async def test_run_thread(observer, mocker):
+async def test_open(observer: Observer, mocker: pytest_mock.MockerFixture) -> None:
+    mocker.patch("pyobs.modules.Module.open")
     mock_proxy = MockProxy()
     mock_proxy.telescope.move_radec = AsyncMock()  # type: ignore
 
+    series = PointingSeries(num_alt=1, num_az=1, observer=observer, finish=0)
+    series.comm.proxy = mock_proxy  # type: ignore
+
+    await series.open()
+
+    assert series._pointing_series_iterator._acquisition == mock_proxy.acquisition  # type: ignore
+    assert series._pointing_series_iterator._telescope == mock_proxy.telescope  # type: ignore
+
+
+@pytest.mark.asyncio
+async def test_run_thread(observer: Observer, mocker: pytest_mock.MockFixture) -> None:
     current_time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
 
     mocker.patch("pyobs.utils.time.Time.now", return_value=current_time)
     series = PointingSeries(num_alt=1, num_az=1, observer=observer, finish=0)
-    series.comm.proxy = mock_proxy  # type: ignore
+    series._pointing_series_iterator = MockPointingSeriesIterator()  # type: ignore
     series._process_acquisition = AsyncMock()  # type: ignore
 
-    await series.open()
     await series._run_thread()
 
-    mock_proxy.telescope.move_radec.assert_called_once_with(151.12839530803322, 27.74121078725986)
     series._process_acquisition.assert_called_once_with(**{"datetime": "", "ra": 0.0, "dec": 0.0, "az": 0.0, "alt": 0.0})
+
+
+@pytest.mark.asyncio
+async def test_no_observer() -> None:
+    with pytest.raises(ValueError):
+        PointingSeries(num_alt=1, num_az=1, finish=0)
+
+
+@pytest.mark.asyncio
+async def test_is_running(observer: Observer) -> None:
+    series = PointingSeries(num_alt=1, num_az=1, observer=observer, finish=0)
+    assert await series.is_running() is True
diff --git a/tests/modules/robotic/test_pointingseriesiterator.py b/tests/modules/robotic/test_pointingseriesiterator.py
index a8146ea01..c91906417 100644
--- a/tests/modules/robotic/test_pointingseriesiterator.py
+++ b/tests/modules/robotic/test_pointingseriesiterator.py
@@ -68,7 +68,7 @@ async def test_next(observer: astroplan.Observer, mocker: pytest_mock.MockerFixt
 
     assert await anext(iterator) == {"datetime": "", "ra": 0.0, "dec": 0.0, "az": 0.0, "alt": 0.0}  # type: ignore
 
-    iterator._telescope.move_radec.assert_called_once_with(0, 0)
+    iterator._telescope.move_radec.assert_called_once_with(0, 0)  # type: ignore
 
 
 @pytest.mark.asyncio
@@ -91,4 +91,3 @@ async def test_acquire_error(observer: astroplan.Observer, mocker: pytest_mock.M
     iterator._acquisition.acquire_target = AsyncMock(side_effect=ValueError)  # type: ignore
 
     assert await anext(iterator) is None  # type: ignore
-

From 14c24b2ad5975e2ad5198b3febc2d6cf002b1cf2 Mon Sep 17 00:00:00 2001
From: GermanHydrogen <admin@germanhydrogen.org>
Date: Mon, 6 May 2024 15:37:43 +0200
Subject: [PATCH 23/25] Extracted task scheduler methods

---
 pyobs/modules/robotic/_taskscheduler.py     | 218 ++++++++++++++++++++
 pyobs/modules/robotic/scheduler.py          | 196 ++----------------
 tests/modules/robotic/conftest.py           |  22 +-
 tests/modules/robotic/test_scheduler.py     | 176 ----------------
 tests/modules/robotic/test_taskscheduler.py | 179 ++++++++++++++++
 5 files changed, 428 insertions(+), 363 deletions(-)
 create mode 100644 pyobs/modules/robotic/_taskscheduler.py
 create mode 100644 tests/modules/robotic/test_taskscheduler.py

diff --git a/pyobs/modules/robotic/_taskscheduler.py b/pyobs/modules/robotic/_taskscheduler.py
new file mode 100644
index 000000000..946101757
--- /dev/null
+++ b/pyobs/modules/robotic/_taskscheduler.py
@@ -0,0 +1,218 @@
+import asyncio
+import copy
+import logging
+import multiprocessing as mp
+from typing import List, Tuple, Any, Optional, cast
+
+import astroplan
+import astropy
+import astropy.units as u
+from astroplan import ObservingBlock, Observer
+from astropy.time import TimeDelta
+
+from pyobs.robotic import TaskSchedule, Task
+from pyobs.utils.time import Time
+
+log = logging.getLogger(__name__)
+
+
+class _TaskScheduler:
+    def __init__(self, schedule: TaskSchedule, observer: Observer, schedule_range: int, safety_time: int, twilight: str) -> None:
+        self._schedule = schedule
+        self._observer = observer
+
+        self._schedule_range = schedule_range
+        self._safety_time = safety_time
+        self._twilight = twilight
+
+        self._blocks: List[ObservingBlock] = []
+
+        self._current_task_id: Optional[str] = None
+        self._schedule_start: Optional[Time] = None
+
+    def set_current_task_id(self, task_id: str) -> None:
+        self._current_task_id = task_id
+
+    def set_schedule_start(self, time: Optional[Time]) -> None:
+        self._schedule_start = time
+
+    def set_blocks(self, blocks: List[ObservingBlock]) -> None:
+        self._blocks = blocks
+
+    async def schedule_task(self) -> None:
+        try:
+            # prepare scheduler
+            blocks, start, end, constraints = await self._prepare_schedule()
+
+            # schedule
+            scheduled_blocks = await self._schedule_blocks(blocks, start, end, constraints)
+
+            # finish schedule
+            await self._finish_schedule(scheduled_blocks, start)
+
+        except ValueError as e:
+            log.warning(str(e))
+
+    async def _prepare_schedule(self) -> Tuple[List[ObservingBlock], Time, Time, List[Any]]:
+        """TaskSchedule blocks."""
+
+        converted_blocks = await self._convert_blocks_to_astroplan()
+
+        start, end = await self._get_time_range()
+
+        blocks = self._filter_blocks(converted_blocks, end)
+
+        # no blocks found?
+        if len(blocks) == 0:
+            await self._schedule.set_schedule([], start)
+            raise ValueError("No blocks left for scheduling.")
+
+        constraints = await self._get_twilight_constraint()
+
+        # return all
+        return blocks, start, end, constraints
+
+    async def _get_twilight_constraint(self) -> List[astroplan.Constraint]:
+        if self._twilight == "astronomical":
+            return [astroplan.AtNightConstraint.twilight_astronomical()]
+        elif self._twilight == "nautical":
+            return [astroplan.AtNightConstraint.twilight_nautical()]
+        else:
+            raise ValueError("Unknown twilight type.")
+
+    async def _convert_blocks_to_astroplan(self) -> List[astroplan.ObservingBlock]:
+        copied_blocks = [copy.copy(block) for block in self._blocks]
+
+        for block in copied_blocks:
+            self._invert_block_priority(block)
+            self._tighten_block_time_constraints(block)
+
+        return copied_blocks
+
+    @staticmethod
+    def _invert_block_priority(block: astroplan.ObservingBlock) -> None:
+        """
+        astroplan's PriorityScheduler expects lower priorities to be more important, so calculate
+        1000 - priority
+        """
+        block.priority = max(1000.0 - block.priority, 0.0)
+
+    @staticmethod
+    def _tighten_block_time_constraints(block: astroplan.ObservingBlock) -> None:
+        """
+        astroplan's PriorityScheduler doesn't match the requested observing windows exactly,
+        so we make them a little smaller.
+        """
+        time_constraints = filter(lambda c: isinstance(c, astroplan.TimeConstraint), block.constraints)
+        for constraint in time_constraints:
+            constraint.min += 30 * u.second
+            constraint.max -= 30 * u.second
+
+    async def _get_time_range(self) -> Tuple[astropy.time.Time, astropy.time.Time]:
+        # get start time for scheduler
+        start = self._schedule_start
+        now_plus_safety = Time.now() + self._safety_time * u.second
+        if start is None or start < now_plus_safety:
+            # if no ETA exists or is in the past, use safety time
+            start = now_plus_safety
+
+        if (running_task := await self._get_current_task()) is not None:
+            log.info("Found running block that ends at %s.", running_task.end)
+
+            # get block end plus some safety
+            block_end = running_task.end + 10.0 * u.second
+            if start < block_end:
+                start = block_end
+                log.info("Start time would be within currently running block, shifting to %s.", cast(Time, start).isot)
+
+        # calculate end time
+        end = start + TimeDelta(self._schedule_range * u.hour)
+
+        return start, end
+
+    async def _get_current_task(self) -> Optional[Task]:
+        if self._current_task_id is None:
+            log.info("No running block found.")
+            return None
+
+        log.info("Trying to find running block in current schedule...")
+        tasks = await self._schedule.get_schedule()
+        if self._current_task_id in tasks:
+            return tasks[self._current_task_id]
+        else:
+            log.info("Running block not found in last schedule.")
+            return None
+
+    def _filter_blocks(self, blocks: List[astroplan.ObservingBlock], end: astropy.time.Time) -> List[astroplan.ObservingBlock]:
+        blocks_without_current = filter(lambda x: x.configuration["request"]["id"] != self._current_task_id, blocks)
+        blocks_in_schedule_range = filter(lambda b: self._is_block_starting_in_schedule(b, end), blocks_without_current)
+
+        return list(blocks_in_schedule_range)
+
+    @staticmethod
+    def _is_block_starting_in_schedule(block: astroplan.ObservingBlock, end: astropy.time.Time) -> bool:
+        time_constraints = [c for c in block.constraints if isinstance(c, astroplan.TimeConstraint)]
+
+        # does constraint start before the end of the scheduling range?
+        before_end = [c for c in time_constraints if c.min < end]
+
+        return len(time_constraints) == 0 or len(before_end) > 0
+
+    async def _schedule_blocks(
+        self, blocks: List[ObservingBlock], start: Time, end: Time, constraints: List[Any]
+    ) -> List[ObservingBlock]:
+
+        # run actual scheduler in separate process and wait for it
+        qout: mp.Queue[List[ObservingBlock]] = mp.Queue()
+        p = mp.Process(target=self._schedule_process, args=(blocks, start, end, constraints, qout))
+        p.start()
+
+        # wait for process to finish
+        # note that the process only finishes, when the queue is empty! so we have to poll the queue first
+        # and then the process.
+        loop = asyncio.get_running_loop()
+        scheduled_blocks: List[ObservingBlock] = await loop.run_in_executor(None, qout.get, True)
+        await loop.run_in_executor(None, p.join)
+        return scheduled_blocks
+
+    async def _finish_schedule(self, scheduled_blocks: List[ObservingBlock], start: Time) -> None:
+        # update
+        await self._schedule.set_schedule(scheduled_blocks, start)
+        if len(scheduled_blocks) > 0:
+            log.info("Finished calculating schedule for %d block(s):", len(scheduled_blocks))
+            for i, block in enumerate(scheduled_blocks, 1):
+                log.info(
+                    "  #%d: %s to %s (%.1f)",
+                    block.configuration["request"]["id"],
+                    block.start_time.strftime("%H:%M:%S"),
+                    block.end_time.strftime("%H:%M:%S"),
+                    block.priority,
+                )
+        else:
+            log.info("Finished calculating schedule for 0 blocks.")
+
+    def _schedule_process(
+        self,
+        blocks: List[ObservingBlock],
+        start: Time,
+        end: Time,
+        constraints: List[Any],
+        scheduled_blocks: mp.Queue   # type: ignore
+    ) -> None:
+        """Actually do the scheduling, usually run in a separate process."""
+
+        # log it
+        log.info("Calculating schedule for %d schedulable block(s) starting at %s...", len(blocks), start)
+
+        # we don't need any transitions
+        transitioner = astroplan.Transitioner()
+
+        # create scheduler
+        scheduler = astroplan.PriorityScheduler(constraints, self._observer, transitioner=transitioner)
+
+        # run scheduler
+        time_range = astroplan.Schedule(start, end)
+        schedule = scheduler(blocks, time_range)
+
+        # put scheduled blocks in queue
+        scheduled_blocks.put(schedule.scheduled_blocks)
\ No newline at end of file
diff --git a/pyobs/modules/robotic/scheduler.py b/pyobs/modules/robotic/scheduler.py
index b8351f749..eb635a691 100644
--- a/pyobs/modules/robotic/scheduler.py
+++ b/pyobs/modules/robotic/scheduler.py
@@ -13,6 +13,7 @@
 from pyobs.events.taskfinished import TaskFinishedEvent
 from pyobs.events.taskstarted import TaskStartedEvent
 from pyobs.events import GoodWeatherEvent, Event
+from pyobs.modules.robotic._taskscheduler import _TaskScheduler
 from pyobs.utils.time import Time
 from pyobs.interfaces import IStartStop, IRunnable
 from pyobs.modules import Module
@@ -56,15 +57,15 @@ def __init__(
         self._schedule = self.add_child_object(schedule, TaskSchedule)  # type: ignore
 
         # store
-        self._schedule_range = schedule_range
-        self._safety_time = safety_time
-        self._twilight = twilight
+        #self._schedule_range = schedule_range
+        #self._safety_time = safety_time
+        #self._twilight = twilight
 
         self._trigger_on_task_started = trigger_on_task_started
         self._trigger_on_task_finished = trigger_on_task_finished
 
         # time to start next schedule from
-        self._schedule_start: Optional[Time] = None
+        #self._schedule_start: Optional[Time] = None
 
         # ID of currently running task, and current (or last if finished) block
         self._current_task_id = None
@@ -73,8 +74,10 @@ def __init__(
         # blocks
         self._blocks: List[ObservingBlock] = []
 
+        self._task_scheduler = _TaskScheduler(self._schedule, self.observer, schedule_range, safety_time, twilight)
+
         # update thread
-        self._scheduler_task = self.add_background_task(self._schedule_task, autostart=False, restart=False)
+        self._scheduler_task = self.add_background_task(self._task_scheduler.schedule_task, autostart=False, restart=False)
         self._update_task = self.add_background_task(self._update_worker)
 
         self._last_change: Optional[Time] = None
@@ -128,6 +131,7 @@ async def _update_blocks(self) -> None:
 
         # store blocks
         self._blocks = blocks
+        self._task_scheduler.set_blocks(blocks)
 
         # schedule update
         if await self._need_update(removed, added):
@@ -195,184 +199,6 @@ def _compare_block_lists(
         unique2 = [names2[n] for n in additional2]
         return unique1, unique2
 
-    async def _schedule_task(self) -> None:
-        try:
-            # prepare scheduler
-            blocks, start, end, constraints = await self._prepare_schedule()
-
-            # schedule
-            scheduled_blocks = await self._schedule_blocks(blocks, start, end, constraints)
-
-            # finish schedule
-            await self._finish_schedule(scheduled_blocks, start)
-
-        except ValueError as e:
-            log.warning(str(e))
-
-    async def _prepare_schedule(self) -> Tuple[List[ObservingBlock], Time, Time, List[Any]]:
-        """TaskSchedule blocks."""
-
-        converted_blocks = await self._convert_blocks_to_astroplan()
-
-        start, end = await self._get_time_range()
-
-        blocks = self._filter_blocks(converted_blocks, end)
-
-        # no blocks found?
-        if len(blocks) == 0:
-            await self._schedule.set_schedule([], start)
-            raise ValueError("No blocks left for scheduling.")
-
-        constraints = await self._get_twilight_constraint()
-
-        # return all
-        return blocks, start, end, constraints
-
-    async def _get_twilight_constraint(self) -> List[astroplan.Constraint]:
-        if self._twilight == "astronomical":
-            return [astroplan.AtNightConstraint.twilight_astronomical()]
-        elif self._twilight == "nautical":
-            return [astroplan.AtNightConstraint.twilight_nautical()]
-        else:
-            raise ValueError("Unknown twilight type.")
-
-    async def _convert_blocks_to_astroplan(self) -> List[astroplan.ObservingBlock]:
-        copied_blocks = [copy.copy(block) for block in self._blocks]
-
-        for block in copied_blocks:
-            self._invert_block_priority(block)
-            self._tighten_block_time_constraints(block)
-
-        return copied_blocks
-
-    @staticmethod
-    def _invert_block_priority(block: astroplan.ObservingBlock) -> None:
-        """
-        astroplan's PriorityScheduler expects lower priorities to be more important, so calculate
-        1000 - priority
-        """
-        block.priority = max(1000.0 - block.priority, 0.0)
-
-    @staticmethod
-    def _tighten_block_time_constraints(block: astroplan.ObservingBlock) -> None:
-        """
-        astroplan's PriorityScheduler doesn't match the requested observing windows exactly,
-        so we make them a little smaller.
-        """
-        time_constraints = filter(lambda c: isinstance(c, astroplan.TimeConstraint), block.constraints)
-        for constraint in time_constraints:
-            constraint.min += 30 * u.second
-            constraint.max -= 30 * u.second
-
-    async def _get_time_range(self) -> Tuple[astropy.time.Time, astropy.time.Time]:
-        # get start time for scheduler
-        start = self._schedule_start
-        now_plus_safety = Time.now() + self._safety_time * u.second
-        if start is None or start < now_plus_safety:
-            # if no ETA exists or is in the past, use safety time
-            start = now_plus_safety
-
-        if (running_task := await self._get_current_task()) is not None:
-            log.info("Found running block that ends at %s.", running_task.end)
-
-            # get block end plus some safety
-            block_end = running_task.end + 10.0 * u.second
-            if start < block_end:
-                start = block_end
-                log.info("Start time would be within currently running block, shifting to %s.", cast(Time, start).isot)
-
-        # calculate end time
-        end = start + TimeDelta(self._schedule_range * u.hour)
-
-        return start, end
-
-    async def _get_current_task(self) -> Optional[Task]:
-        if self._current_task_id is None:
-            log.info("No running block found.")
-            return None
-
-        log.info("Trying to find running block in current schedule...")
-        tasks = await self._schedule.get_schedule()
-        if self._current_task_id in tasks:
-            return tasks[self._current_task_id]
-        else:
-            log.info("Running block not found in last schedule.")
-            return None
-
-    def _filter_blocks(self, blocks: List[astroplan.ObservingBlock], end: astropy.time.Time) -> List[astroplan.ObservingBlock]:
-        blocks_without_current = filter(lambda x: x.configuration["request"]["id"] != self._current_task_id, blocks)
-        blocks_in_schedule_range = filter(lambda b: self._is_block_starting_in_schedule(b, end), blocks_without_current)
-
-        return list(blocks_in_schedule_range)
-
-    @staticmethod
-    def _is_block_starting_in_schedule(block: astroplan.ObservingBlock, end: astropy.time.Time) -> bool:
-        time_constraints = [c for c in block.constraints if isinstance(c, astroplan.TimeConstraint)]
-
-        # does constraint start before the end of the scheduling range?
-        before_end = [c for c in time_constraints if c.min < end]
-
-        return len(time_constraints) == 0 or len(before_end) > 0
-
-    async def _schedule_blocks(
-        self, blocks: List[ObservingBlock], start: Time, end: Time, constraints: List[Any]
-    ) -> List[ObservingBlock]:
-
-        # run actual scheduler in separate process and wait for it
-        qout: mp.Queue[List[ObservingBlock]] = mp.Queue()
-        p = mp.Process(target=self._schedule_process, args=(blocks, start, end, constraints, qout))
-        p.start()
-
-        # wait for process to finish
-        # note that the process only finishes, when the queue is empty! so we have to poll the queue first
-        # and then the process.
-        loop = asyncio.get_running_loop()
-        scheduled_blocks: List[ObservingBlock] = await loop.run_in_executor(None, qout.get, True)
-        await loop.run_in_executor(None, p.join)
-        return scheduled_blocks
-
-    async def _finish_schedule(self, scheduled_blocks: List[ObservingBlock], start: Time) -> None:
-        # update
-        await self._schedule.set_schedule(scheduled_blocks, start)
-        if len(scheduled_blocks) > 0:
-            log.info("Finished calculating schedule for %d block(s):", len(scheduled_blocks))
-            for i, block in enumerate(scheduled_blocks, 1):
-                log.info(
-                    "  #%d: %s to %s (%.1f)",
-                    block.configuration["request"]["id"],
-                    block.start_time.strftime("%H:%M:%S"),
-                    block.end_time.strftime("%H:%M:%S"),
-                    block.priority,
-                )
-        else:
-            log.info("Finished calculating schedule for 0 blocks.")
-
-    def _schedule_process(
-        self,
-        blocks: List[ObservingBlock],
-        start: Time,
-        end: Time,
-        constraints: List[Any],
-        scheduled_blocks: mp.Queue   # type: ignore
-    ) -> None:
-        """Actually do the scheduling, usually run in a separate process."""
-
-        # log it
-        log.info("Calculating schedule for %d schedulable block(s) starting at %s...", len(blocks), start)
-
-        # we don't need any transitions
-        transitioner = astroplan.Transitioner()
-
-        # create scheduler
-        scheduler = astroplan.PriorityScheduler(constraints, self.observer, transitioner=transitioner)
-
-        # run scheduler
-        time_range = astroplan.Schedule(start, end)
-        schedule = scheduler(blocks, time_range)
-
-        # put scheduled blocks in queue
-        scheduled_blocks.put(schedule.scheduled_blocks)
-
     async def run(self, **kwargs: Any) -> None:
         """Trigger a re-schedule."""
         self._scheduler_task.stop()
@@ -391,6 +217,7 @@ async def _on_task_started(self, event: Event, sender: str) -> bool:
         # store it
         self._current_task_id = event.id
         self._last_task_id = event.id
+        self._task_scheduler.set_current_task_id(event.id)
 
         # trigger?
         if self._trigger_on_task_started:
@@ -401,6 +228,7 @@ async def _on_task_started(self, event: Event, sender: str) -> bool:
             self._scheduler_task.stop()
             self._scheduler_task.start()
             self._schedule_start = event.eta
+            self._task_scheduler.set_schedule_start(event.eta)
 
         return True
 
@@ -425,6 +253,7 @@ async def _on_task_finished(self, event: Event, sender: str) -> bool:
             self._scheduler_task.stop()
             self._scheduler_task.start()
             self._schedule_start = Time.now()
+            self._task_scheduler.set_schedule_start(Time.now())
 
         return True
 
@@ -445,6 +274,7 @@ async def _on_good_weather(self, event: Event, sender: str) -> bool:
         self._scheduler_task.stop()
         self._scheduler_task.start()
         self._schedule_start = event.eta
+        self._task_scheduler.set_schedule_start(event.eta)
         return True
 
     async def abort(self, **kwargs: Any) -> None:
diff --git a/tests/modules/robotic/conftest.py b/tests/modules/robotic/conftest.py
index 91e59b6e2..d8ce28d9a 100644
--- a/tests/modules/robotic/conftest.py
+++ b/tests/modules/robotic/conftest.py
@@ -1,8 +1,9 @@
-from typing import Any, Dict
+from typing import Any, Dict, List
 
 import pytest
-from astroplan import Observer
+from astroplan import Observer, ObservingBlock, FixedTarget
 import astropy.units as u
+from astropy.coordinates import SkyCoord
 
 
 class MockAcquisition:
@@ -16,6 +17,19 @@ async def move_radec(*args: Any, **kwargs: Any) -> None:
 
 
 @pytest.fixture(scope='module')
-def observer():
+def observer() -> Observer:
     return Observer(longitude=20.8108 * u.deg, latitude=-32.375823 * u.deg,
-                        elevation=1798.0 * u.m, timezone="UTC")
\ No newline at end of file
+                        elevation=1798.0 * u.m, timezone="UTC")
+
+
+@pytest.fixture(scope='module')
+def schedule_blocks() -> List[ObservingBlock]:
+    blocks = [
+        ObservingBlock(
+            FixedTarget(SkyCoord(0.0 * u.deg, 0.0 * u.deg, frame="icrs"), name=str(i)), 10 * u.minute, 10,
+            constraints=[], configuration={"request": {"id": str(i)}}
+        )
+        for i in range(10)
+    ]
+
+    return blocks
diff --git a/tests/modules/robotic/test_scheduler.py b/tests/modules/robotic/test_scheduler.py
index 1cc5f4b68..b4771b248 100644
--- a/tests/modules/robotic/test_scheduler.py
+++ b/tests/modules/robotic/test_scheduler.py
@@ -17,18 +17,6 @@
 from tests.modules.robotic.test_mastermind import TestTask
 
 
-@pytest.fixture
-def schedule_blocks() -> List[ObservingBlock]:
-    blocks = [
-        ObservingBlock(
-            FixedTarget(SkyCoord(0.0 * u.deg, 0.0 * u.deg, frame="icrs"), name=str(i)), 10 * u.minute, 10,
-            constraints=[], configuration={"request": {"id": str(i)}}
-        )
-        for i in range(10)
-    ]
-
-    return blocks
-
 
 def test_compare_block_lists_with_overlap(schedule_blocks) -> None:
     old_blocks = schedule_blocks[:7]
@@ -182,154 +170,6 @@ async def test_update_blocks_need_to_update(schedule_blocks) -> None:
     assert scheduler._blocks == schedule_blocks
 
 
-@pytest.mark.asyncio
-async def test_prepare_schedule_invalid_twilight() -> None:
-    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), twilight="invalid")
-
-    with pytest.raises(ValueError):
-        await scheduler._prepare_schedule()
-
-
-@pytest.mark.asyncio
-async def test_prepare_schedule_astronomical_twilight(schedule_blocks) -> None:
-    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), twilight="astronomical")
-    scheduler._blocks = schedule_blocks
-
-    _, _, _, constraints = await scheduler._prepare_schedule()
-
-    assert constraints[0].max_solar_altitude == -18 * u.deg
-
-
-@pytest.mark.asyncio
-async def test_prepare_schedule_nautical_twilight(schedule_blocks) -> None:
-    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), twilight="nautical")
-    scheduler._blocks = schedule_blocks
-
-    _, _, _, constraints = await scheduler._prepare_schedule()
-
-    assert constraints[0].max_solar_altitude == -12 * u.deg
-
-
-@pytest.mark.asyncio
-async def test_prepare_schedule_no_blocks() -> None:
-    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), twilight="nautical")
-
-    with pytest.raises(ValueError):
-        await scheduler._prepare_schedule()
-
-
-@pytest.mark.asyncio
-async def test_prepare_schedule_no_start(schedule_blocks, mocker) -> None:
-    current_time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
-    mocker.patch("pyobs.utils.time.Time.now", return_value=current_time)
-
-    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
-    scheduler._blocks = schedule_blocks
-
-    _, start, _, _ = await scheduler._prepare_schedule()
-
-    assert start.to_datetime() == datetime.datetime(2024, 4, 1, 20, 1, 0)
-
-
-@pytest.mark.asyncio
-async def test_prepare_schedule_start(schedule_blocks, mocker) -> None:
-    current_time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
-    mocker.patch("pyobs.utils.time.Time.now", return_value=current_time)
-
-    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
-    scheduler._blocks = schedule_blocks
-    scheduler._schedule_start = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 1, 0))
-
-    _, start, _, _ = await scheduler._prepare_schedule()
-
-    assert start.to_datetime() == datetime.datetime(2024, 4, 1, 20, 1, 0)
-
-
-@pytest.mark.asyncio
-async def test_prepare_schedule_end(schedule_blocks, mocker) -> None:
-    current_time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
-    mocker.patch("pyobs.utils.time.Time.now", return_value=current_time)
-
-    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
-    scheduler._blocks = schedule_blocks
-    scheduler._schedule_start = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 1, 0))
-
-    _, _, end, _ = await scheduler._prepare_schedule()
-
-    assert end.to_datetime() == datetime.datetime(2024, 4, 2, 20, 1, 0)
-
-
-@pytest.mark.asyncio
-async def test_prepare_schedule_block_filtering(schedule_blocks, mocker) -> None:
-    current_time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
-    mocker.patch("pyobs.utils.time.Time.now", return_value=current_time)
-
-    over_time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 3, 20, 0, 0))
-    in_time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 2, 10, 0, 0))
-
-    schedule_blocks[1].constraints.append(astroplan.TimeConstraint(min=over_time, max=over_time))
-    schedule_blocks[2].constraints.append(astroplan.TimeConstraint(min=in_time, max=over_time))
-
-    blocks = [
-        schedule_blocks[0], schedule_blocks[1], schedule_blocks[2], schedule_blocks[3]
-    ]
-
-    task_scheduler = TestTaskSchedule()
-    task_scheduler.get_schedule = AsyncMock(return_value={"0": TestTask()})  # type: ignore
-
-    scheduler = Scheduler(TestTaskArchive(), task_scheduler)
-    scheduler._schedule_start = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 1, 0))
-    scheduler._current_task_id = "0"
-    scheduler._blocks = blocks
-
-    res_blocks, _, _, _ = await scheduler._prepare_schedule()
-
-    assert [block.configuration["request"]["id"] for block in res_blocks] == ["2", "3"]
-
-
-def mock_schedule_process(blocks: List[ObservingBlock], start: Time, end: Time, constraints: List[Any],
-                          scheduled_blocks: multiprocessing.Queue) -> None:  # type: ignore
-    scheduled_blocks.put(blocks)
-
-
-@pytest.mark.asyncio
-async def test_schedule_blocks(observer) -> None:
-
-    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), observer=observer)
-
-    scheduler._schedule_process = mock_schedule_process  # type: ignore
-
-    time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
-    block = ObservingBlock(
-        FixedTarget(SkyCoord(0.0 * u.deg, 0.0 * u.deg, frame="icrs"), name=0), 10 * u.minute, 2000.0,
-        configuration={"request": {"id": "0"}}
-    )
-    blocks = [block]
-    scheduled_blocks = await scheduler._schedule_blocks(blocks, time, time, [])
-    assert scheduled_blocks[0].configuration["request"]["id"] == block.configuration["request"]["id"]
-
-
-@pytest.mark.asyncio
-async def test_finish_schedule() -> None:
-    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
-    scheduler._schedule.set_schedule = AsyncMock()  # type: ignore
-
-    time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
-    block = ObservingBlock(
-        FixedTarget(SkyCoord(0.0 * u.deg, 0.0 * u.deg, frame="icrs"), name=0), 10 * u.minute, 2000.0,
-        configuration={"request": {"id": "0"}},
-        constraints=[astroplan.TimeConstraint(min=time, max=time)]
-    )
-    block.start_time = time
-    block.end_time = time
-    blocks = [block]
-
-    scheduler._need_update = False
-
-    await scheduler._finish_schedule(blocks, time)
-    scheduler._schedule.set_schedule.assert_called_with(blocks, time)
-
-
 @pytest.mark.asyncio
 async def test_on_task_started() -> None:
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), trigger_on_task_started=True)
@@ -399,19 +239,3 @@ async def test_on_good_weather_not_weather_event() -> None:
 
     assert await scheduler._on_good_weather(event, "") is False
 
-
-@pytest.mark.asyncio
-async def test_convert_blocks_to_astroplan() -> None:
-    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
-    time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
-    block = ObservingBlock(
-        FixedTarget(SkyCoord(0.0 * u.deg, 0.0 * u.deg, frame="icrs"), name=0), 10 * u.minute, 2000.0,
-        constraints=[astroplan.TimeConstraint(min=time, max=time)]
-    )
-    scheduler._blocks = [block]
-
-    converted_blocks = await scheduler._convert_blocks_to_astroplan()
-
-    assert converted_blocks[0].priority == 0
-    assert converted_blocks[0].constraints[0].min.to_datetime() == datetime.datetime(2024, 4, 1, 20, 0, 30)
-    assert converted_blocks[0].constraints[0].max.to_datetime() == datetime.datetime(2024, 4, 1, 19, 59, 30)
diff --git a/tests/modules/robotic/test_taskscheduler.py b/tests/modules/robotic/test_taskscheduler.py
new file mode 100644
index 000000000..9c8bf9e97
--- /dev/null
+++ b/tests/modules/robotic/test_taskscheduler.py
@@ -0,0 +1,179 @@
+import datetime
+import multiprocessing
+from typing import List, Any
+from unittest.mock import AsyncMock
+
+import astroplan
+import astropy.units as u
+import pytest
+import pytest_mock
+from astroplan import ObservingBlock, FixedTarget, Observer
+from astropy.coordinates import SkyCoord
+
+from pyobs.modules.robotic._taskscheduler import _TaskScheduler
+from pyobs.utils.time import Time
+from tests.modules.robotic.test_mastermind import TestTask
+from tests.modules.robotic.test_scheduler import TestTaskSchedule
+
+
+@pytest.mark.asyncio
+async def test_prepare_schedule_invalid_twilight(observer: Observer) -> None:
+    scheduler = _TaskScheduler(TestTaskSchedule(), observer, 24, 60, "invalid")
+
+    with pytest.raises(ValueError):
+        await scheduler._prepare_schedule()
+
+
+@pytest.mark.asyncio
+async def test_prepare_schedule_astronomical_twilight(observer: Observer, schedule_blocks: List[ObservingBlock]) -> None:
+    scheduler = _TaskScheduler(TestTaskSchedule(), observer, 24, 60, "astronomical")
+    scheduler._blocks = schedule_blocks
+
+    _, _, _, constraints = await scheduler._prepare_schedule()
+
+    assert constraints[0].max_solar_altitude == -18 * u.deg
+
+
+@pytest.mark.asyncio
+async def test_prepare_schedule_nautical_twilight(observer: Observer, schedule_blocks: List[ObservingBlock]) -> None:
+    scheduler = _TaskScheduler(TestTaskSchedule(), observer, 24, 60, "nautical")
+    scheduler._blocks = schedule_blocks
+
+    _, _, _, constraints = await scheduler._prepare_schedule()
+
+    assert constraints[0].max_solar_altitude == -12 * u.deg
+
+
+@pytest.mark.asyncio
+async def test_prepare_schedule_no_blocks(observer: Observer) -> None:
+    scheduler = _TaskScheduler(TestTaskSchedule(), observer, 24, 60, "nautical")
+
+    with pytest.raises(ValueError):
+        await scheduler._prepare_schedule()
+
+
+@pytest.mark.asyncio
+async def test_prepare_schedule_no_start(observer: Observer, schedule_blocks: List[ObservingBlock], mocker: pytest_mock.MockFixture) -> None:
+    current_time = Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
+    mocker.patch("pyobs.utils.time.Time.now", return_value=current_time)
+
+    scheduler = _TaskScheduler(TestTaskSchedule(), observer, 24, 60, "nautical")
+    scheduler._blocks = schedule_blocks
+
+    _, start, _, _ = await scheduler._prepare_schedule()
+
+    assert start.to_datetime() == datetime.datetime(2024, 4, 1, 20, 1, 0)
+
+
+@pytest.mark.asyncio
+async def test_prepare_schedule_start(observer: Observer, schedule_blocks: List[ObservingBlock], mocker: pytest_mock.MockFixture) -> None:
+    current_time = Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
+    mocker.patch("pyobs.utils.time.Time.now", return_value=current_time)
+
+    scheduler = _TaskScheduler(TestTaskSchedule(), observer, 24, 60, "nautical")
+    scheduler._blocks = schedule_blocks
+    scheduler._schedule_start = Time(datetime.datetime(2024, 4, 1, 20, 1, 0))
+
+    _, start, _, _ = await scheduler._prepare_schedule()
+
+    assert start.to_datetime() == datetime.datetime(2024, 4, 1, 20, 1, 0)
+
+
+@pytest.mark.asyncio
+async def test_prepare_schedule_end(observer: Observer, schedule_blocks: List[ObservingBlock], mocker: pytest_mock.MockFixture) -> None:
+    current_time = Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
+    mocker.patch("pyobs.utils.time.Time.now", return_value=current_time)
+
+    scheduler = _TaskScheduler(TestTaskSchedule(), observer, 24, 60, "nautical")
+    scheduler._blocks = schedule_blocks
+    scheduler._schedule_start = Time(datetime.datetime(2024, 4, 1, 20, 1, 0))
+
+    _, _, end, _ = await scheduler._prepare_schedule()
+
+    assert end.to_datetime() == datetime.datetime(2024, 4, 2, 20, 1, 0)
+
+
+@pytest.mark.asyncio
+async def test_prepare_schedule_block_filtering(observer: Observer, schedule_blocks: List[ObservingBlock], mocker: pytest_mock.MockFixture) -> None:
+    current_time = Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
+    mocker.patch("pyobs.utils.time.Time.now", return_value=current_time)
+
+    over_time = Time(datetime.datetime(2024, 4, 3, 20, 0, 0))
+    in_time = Time(datetime.datetime(2024, 4, 2, 10, 0, 0))
+
+    schedule_blocks[1].constraints.append(astroplan.TimeConstraint(min=over_time, max=over_time))
+    schedule_blocks[2].constraints.append(astroplan.TimeConstraint(min=in_time, max=over_time))
+
+    blocks = [
+        schedule_blocks[0], schedule_blocks[1], schedule_blocks[2], schedule_blocks[3]
+    ]
+
+    task_scheduler = TestTaskSchedule()
+    task_scheduler.get_schedule = AsyncMock(return_value={"0": TestTask()})  # type: ignore
+
+    scheduler = _TaskScheduler(task_scheduler, observer, 24, 60, "nautical")
+    scheduler._schedule_start = Time(datetime.datetime(2024, 4, 1, 20, 1, 0))
+    scheduler._current_task_id = "0"
+    scheduler._blocks = blocks
+
+    res_blocks, _, _, _ = await scheduler._prepare_schedule()
+
+    assert [block.configuration["request"]["id"] for block in res_blocks] == ["2", "3"]
+
+
+def mock_schedule_process(blocks: List[ObservingBlock], start: Time, end: Time, constraints: List[Any],
+                          scheduled_blocks: multiprocessing.Queue) -> None:  # type: ignore
+    scheduled_blocks.put(blocks)
+
+
+@pytest.mark.asyncio
+async def test_schedule_blocks(observer: Observer) -> None:
+
+    scheduler = _TaskScheduler(TestTaskSchedule(), observer, 24, 60, "nautical")
+
+    scheduler._schedule_process = mock_schedule_process  # type: ignore
+
+    time = Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
+    block = ObservingBlock(
+        FixedTarget(SkyCoord(0.0 * u.deg, 0.0 * u.deg, frame="icrs"), name=0), 10 * u.minute, 2000.0,
+        configuration={"request": {"id": "0"}}
+    )
+    blocks = [block]
+    scheduled_blocks = await scheduler._schedule_blocks(blocks, time, time, [])
+    assert scheduled_blocks[0].configuration["request"]["id"] == block.configuration["request"]["id"]
+
+
+@pytest.mark.asyncio
+async def test_finish_schedule(observer: Observer) -> None:
+    scheduler = _TaskScheduler(TestTaskSchedule(), observer, 24, 60, "nautical")
+    scheduler._schedule.set_schedule = AsyncMock()  # type: ignore
+
+    time = Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
+    block = ObservingBlock(
+        FixedTarget(SkyCoord(0.0 * u.deg, 0.0 * u.deg, frame="icrs"), name=0), 10 * u.minute, 2000.0,
+        configuration={"request": {"id": "0"}},
+        constraints=[astroplan.TimeConstraint(min=time, max=time)]
+    )
+    block.start_time = time
+    block.end_time = time
+    blocks = [block]
+
+    await scheduler._finish_schedule(blocks, time)
+    scheduler._schedule.set_schedule.assert_called_with(blocks, time)
+
+
+@pytest.mark.asyncio
+async def test_convert_blocks_to_astroplan(observer: Observer) -> None:
+    scheduler = _TaskScheduler(TestTaskSchedule(), observer, 24, 60, "nautical")
+    time = Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
+    block = ObservingBlock(
+        FixedTarget(SkyCoord(0.0 * u.deg, 0.0 * u.deg, frame="icrs"), name=0), 10 * u.minute, 2000.0,
+        constraints=[astroplan.TimeConstraint(min=time, max=time)]
+    )
+    scheduler._blocks = [block]
+
+    converted_blocks = await scheduler._convert_blocks_to_astroplan()
+
+    assert converted_blocks[0].priority == 0
+    assert converted_blocks[0].constraints[0].min.to_datetime() == datetime.datetime(2024, 4, 1, 20, 0, 30)
+    assert converted_blocks[0].constraints[0].max.to_datetime() == datetime.datetime(2024, 4, 1, 19, 59, 30)

From 6fb41a9e979f5b3dfebff62d53e4d23e3dc39b01 Mon Sep 17 00:00:00 2001
From: GermanHydrogen <admin@germanhydrogen.org>
Date: Mon, 6 May 2024 16:30:09 +0200
Subject: [PATCH 24/25] Seperated task update from scheduler

---
 pyobs/modules/robotic/_taskscheduler.py   |   2 +-
 pyobs/modules/robotic/_taskupdater.py     | 126 +++++++++++++++++++
 pyobs/modules/robotic/scheduler.py        | 105 +++-------------
 tests/modules/robotic/test_scheduler.py   | 147 +++-------------------
 tests/modules/robotic/test_taskupdater.py | 128 +++++++++++++++++++
 5 files changed, 289 insertions(+), 219 deletions(-)
 create mode 100644 pyobs/modules/robotic/_taskupdater.py
 create mode 100644 tests/modules/robotic/test_taskupdater.py

diff --git a/pyobs/modules/robotic/_taskscheduler.py b/pyobs/modules/robotic/_taskscheduler.py
index 946101757..7c3480995 100644
--- a/pyobs/modules/robotic/_taskscheduler.py
+++ b/pyobs/modules/robotic/_taskscheduler.py
@@ -30,7 +30,7 @@ def __init__(self, schedule: TaskSchedule, observer: Observer, schedule_range: i
         self._current_task_id: Optional[str] = None
         self._schedule_start: Optional[Time] = None
 
-    def set_current_task_id(self, task_id: str) -> None:
+    def set_current_task_id(self, task_id: Optional[str]) -> None:
         self._current_task_id = task_id
 
     def set_schedule_start(self, time: Optional[Time]) -> None:
diff --git a/pyobs/modules/robotic/_taskupdater.py b/pyobs/modules/robotic/_taskupdater.py
new file mode 100644
index 000000000..206282c24
--- /dev/null
+++ b/pyobs/modules/robotic/_taskupdater.py
@@ -0,0 +1,126 @@
+import json
+import json
+import logging
+from typing import List, Tuple, Optional
+
+from astroplan import ObservingBlock
+
+from pyobs.robotic import TaskArchive, TaskSchedule
+from pyobs.utils.time import Time
+
+log = logging.getLogger(__name__)
+
+
+class _TaskUpdater:
+    def __init__(self, task_archive: TaskArchive, task_schedule: TaskSchedule):
+        self._task_archive = task_archive
+        self._schedule = task_schedule
+
+        self._blocks: List[ObservingBlock] = []
+
+        self._current_task_id: Optional[str] = None
+        self._last_task_id: Optional[str] = None
+
+        self._last_change: Optional[Time] = None
+
+    def set_current_task_id(self, task_id: Optional[str]) -> None:
+        self._current_task_id = task_id
+
+    def set_last_task_id(self, task_id: str) -> None:
+        self._last_task_id = task_id
+
+    async def update(self) -> Optional[List[ObservingBlock]]:
+        # got new time of last change?
+        t = await self._task_archive.last_changed()
+        if self._last_change is None or self._last_change < t:
+            blocks = await self._update_blocks()
+
+            self._last_change = Time.now()
+            return blocks
+
+        return None
+
+    async def _update_blocks(self) -> Optional[List[ObservingBlock]]:
+        # get schedulable blocks and sort them
+        log.info("Found update in schedulable block, downloading them...")
+        blocks = sorted(
+            await self._task_archive.get_schedulable_blocks(),
+            key=lambda x: json.dumps(x.configuration, sort_keys=True),
+        )
+        log.info("Downloaded %d schedulable block(s).", len(blocks))
+
+        # compare new and old lists
+        removed, added = self._compare_block_lists(self._blocks, blocks)
+
+        # store blocks
+        self._blocks = blocks
+
+
+        # schedule update
+        if await self._need_update(removed, added):
+            log.info("Triggering scheduler run...")
+            return blocks
+            #self._scheduler_task.stop()  # Stop current run
+            #self._scheduler_task.start()
+
+        return None
+
+    async def _need_update(self, removed: List[ObservingBlock], added: List[ObservingBlock]) -> bool:
+        if len(removed) == 0 and len(added) == 0:
+            # no need to re-schedule
+            log.info("No change in list of blocks detected.")
+            return False
+
+        # has only the current block been removed?
+        log.info("Removed: %d, added: %d", len(removed), len(added))
+        if len(removed) == 1:
+            log.info(
+                "Found 1 removed block with ID %d. Last task ID was %s, current is %s.",
+                removed[0].target.name,
+                str(self._last_task_id),
+                str(self._current_task_id),
+            )
+        if len(removed) == 1 and len(added) == 0 and removed[0].target.name == self._last_task_id:
+            # no need to re-schedule
+            log.info("Only one removed block detected, which is the one currently running.")
+            return False
+
+        # check, if one of the removed blocks was actually in schedule
+        if len(removed) > 0:
+            schedule = await self._schedule.get_schedule()
+            removed_from_schedule = [r for r in removed if r in schedule]
+            if len(removed_from_schedule) == 0:
+                log.info(f"Found {len(removed)} blocks, but none of them was scheduled.")
+                return False
+
+        return True
+
+    @staticmethod
+    def _compare_block_lists(
+            blocks1: List[ObservingBlock], blocks2: List[ObservingBlock]
+    ) -> Tuple[List[ObservingBlock], List[ObservingBlock]]:
+        """Compares two lists of ObservingBlocks and returns two lists, containing those that are missing in list 1
+        and list 2, respectively.
+
+        Args:
+            blocks1: First list of blocks.
+            blocks2: Second list of blocks.
+
+        Returns:
+            (tuple): Tuple containing:
+                unique1:  Blocks that exist in blocks1, but not in blocks2.
+                unique2:  Blocks that exist in blocks2, but not in blocks1.
+        """
+
+        # get dictionaries with block names
+        names1 = {b.target.name: b for b in blocks1}
+        names2 = {b.target.name: b for b in blocks2}
+
+        # find elements in names1 that are missing in names2 and vice versa
+        additional1 = set(names1.keys()).difference(names2.keys())
+        additional2 = set(names2.keys()).difference(names1.keys())
+
+        # get blocks for names and return them
+        unique1 = [names1[n] for n in additional1]
+        unique2 = [names2[n] for n in additional2]
+        return unique1, unique2
\ No newline at end of file
diff --git a/pyobs/modules/robotic/scheduler.py b/pyobs/modules/robotic/scheduler.py
index eb635a691..9f23f84b9 100644
--- a/pyobs/modules/robotic/scheduler.py
+++ b/pyobs/modules/robotic/scheduler.py
@@ -14,6 +14,7 @@
 from pyobs.events.taskstarted import TaskStartedEvent
 from pyobs.events import GoodWeatherEvent, Event
 from pyobs.modules.robotic._taskscheduler import _TaskScheduler
+from pyobs.modules.robotic._taskupdater import _TaskUpdater
 from pyobs.utils.time import Time
 from pyobs.interfaces import IStartStop, IRunnable
 from pyobs.modules import Module
@@ -68,12 +69,13 @@ def __init__(
         #self._schedule_start: Optional[Time] = None
 
         # ID of currently running task, and current (or last if finished) block
-        self._current_task_id = None
-        self._last_task_id = None
+        #self._current_task_id = None
+        #self._last_task_id = None
 
         # blocks
         self._blocks: List[ObservingBlock] = []
 
+        self._task_updater = _TaskUpdater(self._task_archive, self._schedule)
         self._task_scheduler = _TaskScheduler(self._schedule, self.observer, schedule_range, safety_time, twilight)
 
         # update thread
@@ -110,94 +112,14 @@ async def _update_worker(self) -> None:
             await asyncio.sleep(5)
 
     async def _update_worker_loop(self) -> None:
-        # got new time of last change?
-        t = await self._task_archive.last_changed()
-        if self._last_change is None or self._last_change < t:
-            await self._update_blocks()
-
-            self._last_change = Time.now()
-
-    async def _update_blocks(self) -> None:
-        # get schedulable blocks and sort them
-        log.info("Found update in schedulable block, downloading them...")
-        blocks = sorted(
-            await self._task_archive.get_schedulable_blocks(),
-            key=lambda x: json.dumps(x.configuration, sort_keys=True),
-        )
-        log.info("Downloaded %d schedulable block(s).", len(blocks))
-
-        # compare new and old lists
-        removed, added = self._compare_block_lists(self._blocks, blocks)
-
-        # store blocks
-        self._blocks = blocks
-        self._task_scheduler.set_blocks(blocks)
-
-        # schedule update
-        if await self._need_update(removed, added):
-            log.info("Triggering scheduler run...")
-            self._scheduler_task.stop()     # Stop current run
-            self._scheduler_task.start()
-
-    async def _need_update(self, removed: List[ObservingBlock], added: List[ObservingBlock]) -> bool:
-        if len(removed) == 0 and len(added) == 0:
-            # no need to re-schedule
-            log.info("No change in list of blocks detected.")
-            return False
-
-        # has only the current block been removed?
-        log.info("Removed: %d, added: %d", len(removed), len(added))
-        if len(removed) == 1:
-            log.info(
-                "Found 1 removed block with ID %d. Last task ID was %s, current is %s.",
-                removed[0].target.name,
-                str(self._last_task_id),
-                str(self._current_task_id),
-            )
-        if len(removed) == 1 and len(added) == 0 and removed[0].target.name == self._last_task_id:
-            # no need to re-schedule
-            log.info("Only one removed block detected, which is the one currently running.")
-            return False
-
-        # check, if one of the removed blocks was actually in schedule
-        if len(removed) > 0:
-            schedule = await self._schedule.get_schedule()
-            removed_from_schedule = [r for r in removed if r in schedule]
-            if len(removed_from_schedule) == 0:
-                log.info(f"Found {len(removed)} blocks, but none of them was scheduled.")
-                return False
-
-        return True
+        blocks = await self._task_updater.update()
 
-    @staticmethod
-    def _compare_block_lists(
-        blocks1: List[ObservingBlock], blocks2: List[ObservingBlock]
-    ) -> Tuple[List[ObservingBlock], List[ObservingBlock]]:
-        """Compares two lists of ObservingBlocks and returns two lists, containing those that are missing in list 1
-        and list 2, respectively.
+        if blocks is None:
+            return
 
-        Args:
-            blocks1: First list of blocks.
-            blocks2: Second list of blocks.
-
-        Returns:
-            (tuple): Tuple containing:
-                unique1:  Blocks that exist in blocks1, but not in blocks2.
-                unique2:  Blocks that exist in blocks2, but not in blocks1.
-        """
-
-        # get dictionaries with block names
-        names1 = {b.target.name: b for b in blocks1}
-        names2 = {b.target.name: b for b in blocks2}
-
-        # find elements in names1 that are missing in names2 and vice versa
-        additional1 = set(names1.keys()).difference(names2.keys())
-        additional2 = set(names2.keys()).difference(names1.keys())
-
-        # get blocks for names and return them
-        unique1 = [names1[n] for n in additional1]
-        unique2 = [names2[n] for n in additional2]
-        return unique1, unique2
+        self._scheduler_task.stop()
+        self._task_scheduler.set_blocks(blocks)
+        self._scheduler_task.start()
 
     async def run(self, **kwargs: Any) -> None:
         """Trigger a re-schedule."""
@@ -215,9 +137,9 @@ async def _on_task_started(self, event: Event, sender: str) -> bool:
             return False
 
         # store it
-        self._current_task_id = event.id
-        self._last_task_id = event.id
         self._task_scheduler.set_current_task_id(event.id)
+        self._task_updater.set_current_task_id(event.id)
+        self._task_updater.set_last_task_id(event.id)
 
         # trigger?
         if self._trigger_on_task_started:
@@ -243,7 +165,8 @@ async def _on_task_finished(self, event: Event, sender: str) -> bool:
             return False
 
         # reset current task
-        self._current_task_id = None
+        self._task_scheduler.set_current_task_id(None)
+        self._task_updater.set_current_task_id(None)
 
         # trigger?
         if self._trigger_on_task_finished:
diff --git a/tests/modules/robotic/test_scheduler.py b/tests/modules/robotic/test_scheduler.py
index b4771b248..6d7bcd33e 100644
--- a/tests/modules/robotic/test_scheduler.py
+++ b/tests/modules/robotic/test_scheduler.py
@@ -1,60 +1,16 @@
 import datetime
-import multiprocessing
-from typing import List, Optional, Dict, Any
+from typing import List, Optional, Dict
 from unittest.mock import Mock, AsyncMock
 
-import astroplan
-import astropy.units as u
 import pytest
-from astroplan import ObservingBlock, FixedTarget, Observer
-from astropy.coordinates import SkyCoord
+import pytest_mock
+from astroplan import ObservingBlock
 
 import pyobs
 from pyobs.events import GoodWeatherEvent, TaskStartedEvent, TaskFinishedEvent, Event
 from pyobs.modules.robotic import Scheduler
 from pyobs.robotic import TaskArchive, TaskSchedule, Task
 from pyobs.utils.time import Time
-from tests.modules.robotic.test_mastermind import TestTask
-
-
-
-def test_compare_block_lists_with_overlap(schedule_blocks) -> None:
-    old_blocks = schedule_blocks[:7]
-    new_blocks = schedule_blocks[5:]
-
-    removed, added = Scheduler._compare_block_lists(old_blocks, new_blocks)
-
-    removed_names = [int(b.target.name) for b in removed]
-    new_names = [int(b.target.name) for b in added]
-
-    assert set(removed_names) == {0, 1, 2, 3, 4}
-    assert set(new_names) == {7, 8, 9}
-
-
-def test_compare_block_lists_without_overlap(schedule_blocks) -> None:
-    old_blocks = schedule_blocks[:5]
-    new_blocks = schedule_blocks[5:]
-
-    removed, added = Scheduler._compare_block_lists(old_blocks, new_blocks)
-
-    removed_names = [int(b.target.name) for b in removed]
-    new_names = [int(b.target.name) for b in added]
-
-    assert set(removed_names) == {0, 1, 2, 3, 4}
-    assert set(new_names) == {5, 6, 7, 8, 9}
-
-
-def test_compare_block_lists_identical(schedule_blocks) -> None:
-    old_blocks = schedule_blocks
-    new_blocks = schedule_blocks
-
-    removed, added = Scheduler._compare_block_lists(old_blocks, new_blocks)
-
-    removed_names = [int(b.target.name) for b in removed]
-    new_names = [int(b.target.name) for b in added]
-
-    assert len(removed_names) == 0
-    assert len(new_names) == 0
 
 
 class TestTaskArchive(TaskArchive):
@@ -82,105 +38,43 @@ async def get_task(self, time: Time) -> Optional[Task]:
 
 
 @pytest.mark.asyncio
-async def test_worker_loop_not_changed() -> None:
-    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
-    scheduler._update_blocks = AsyncMock()
-
-    time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
-    scheduler._task_archive.last_changed = AsyncMock(return_value=time)  # type: ignore
-    scheduler._last_change = time
-
-    await scheduler._update_worker_loop()
-
-    scheduler._update_blocks.assert_not_called()
-
-
-@pytest.mark.asyncio
-async def test_worker_loop_new_changes(mocker) -> None:
-    time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
-    mocker.patch("pyobs.utils.time.Time.now", return_value=time)
-    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
-    scheduler._update_blocks = AsyncMock()
-
-    scheduler._task_archive.last_changed = AsyncMock(return_value=time)  # type: ignore
-    scheduler._last_change = time - datetime.timedelta(minutes=1)
-
-    await scheduler._update_worker_loop()
-
-    scheduler._update_blocks.assert_called_once()
-    assert scheduler._last_change == time
-
-
-@pytest.mark.asyncio
-async def test_update_blocks_no_changes(schedule_blocks) -> None:
-    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
-    scheduler._scheduler_task.start = Mock()  # type: ignore
-    scheduler._task_archive.get_schedulable_blocks = AsyncMock(return_value=schedule_blocks)  # type: ignore
-    scheduler._blocks = schedule_blocks
-
-    await scheduler._update_blocks()
-
-    scheduler._scheduler_task.start.assert_not_called()
-
-
-@pytest.mark.asyncio
-async def test_update_blocks_removed_current(schedule_blocks) -> None:
-    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
-    scheduler._scheduler_task.start = Mock()  # type: ignore
-
-    scheduler._task_archive.get_schedulable_blocks = AsyncMock(return_value=schedule_blocks)  # type: ignore
-    scheduler._blocks = schedule_blocks
-    scheduler._last_task_id = "0"
-
-    scheduler._compare_block_lists = Mock(return_value=([schedule_blocks[0]], []))  # type: ignore
-
-    await scheduler._update_blocks()
-
-    scheduler._scheduler_task.start.assert_not_called()
-
-
-@pytest.mark.asyncio
-async def test_update_blocks_removed_not_in_schedule(schedule_blocks)  -> None:
-    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
+async def test_update_worker_loop_no_update() -> None:
+    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), trigger_on_task_started=True)
+    scheduler._task_updater.update = AsyncMock(return_value=None)  # type: ignore
     scheduler._scheduler_task.start = Mock()  # type: ignore
-    scheduler._task_archive.get_schedulable_blocks = AsyncMock(return_value=schedule_blocks)  # type: ignore
-    scheduler._schedule.get_schedule = AsyncMock(return_value=[])  # type: ignore
-    scheduler._blocks = schedule_blocks
-
-    scheduler._compare_block_lists = Mock(return_value=([schedule_blocks[0]], []))  # type: ignore
 
-    await scheduler._update_blocks()
+    await scheduler._update_worker_loop()
 
     scheduler._scheduler_task.start.assert_not_called()
 
 
 @pytest.mark.asyncio
-async def test_update_blocks_need_to_update(schedule_blocks) -> None:
-    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule())
+async def test_update_worker_loop_with_update() -> None:
+    blocks: List[ObservingBlock] = []
+    scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), trigger_on_task_started=True)
+    scheduler._task_updater.update = AsyncMock(return_value=blocks)  # type: ignore
     scheduler._scheduler_task.start = Mock()  # type: ignore
-    scheduler._task_archive.get_schedulable_blocks = AsyncMock(return_value=schedule_blocks)  # type: ignore
-    scheduler._schedule.get_schedule = AsyncMock(return_value=[])  # type: ignore
-    scheduler._blocks = []
 
-    scheduler._compare_block_lists = Mock(return_value=([], [schedule_blocks[0]]))  # type: ignore
-
-    await scheduler._update_blocks()
+    await scheduler._update_worker_loop()
 
+    assert scheduler._task_scheduler._blocks == blocks
     scheduler._scheduler_task.start.assert_called_once()
-    assert scheduler._blocks == schedule_blocks
 
 
 @pytest.mark.asyncio
 async def test_on_task_started() -> None:
     scheduler = Scheduler(TestTaskArchive(), TestTaskSchedule(), trigger_on_task_started=True)
     scheduler._scheduler_task.start = Mock()  # type: ignore
+
     time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
     event = TaskStartedEvent(id=0, eta=time, name="")
 
     await scheduler._on_task_started(event, "")
 
-    assert scheduler._current_task_id == 0
-    assert scheduler._last_task_id == 0
+    assert scheduler._task_scheduler._current_task_id == 0  # type: ignore
+    assert scheduler._task_updater._current_task_id == 0  # type: ignore
+    assert scheduler._task_updater._last_task_id == 0  # type: ignore
+
     scheduler._scheduler_task.start.assert_called_once()
     assert scheduler._schedule_start == time
 
@@ -194,7 +88,7 @@ async def test_on_task_started_wrong_event() -> None:
 
 
 @pytest.mark.asyncio
-async def test_on_task_finished(mocker) -> None:
+async def test_on_task_finished(mocker: pytest_mock.MockFixture) -> None:
     current_time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
     mocker.patch("pyobs.utils.time.Time.now", return_value=current_time)
 
@@ -204,7 +98,7 @@ async def test_on_task_finished(mocker) -> None:
 
     await scheduler._on_task_finished(event, "")
 
-    assert scheduler._current_task_id is None
+    assert scheduler._task_updater._current_task_id is None
     scheduler._scheduler_task.start.assert_called_once()
 
     assert scheduler._schedule_start == current_time
@@ -238,4 +132,3 @@ async def test_on_good_weather_not_weather_event() -> None:
     event = Event()
 
     assert await scheduler._on_good_weather(event, "") is False
-
diff --git a/tests/modules/robotic/test_taskupdater.py b/tests/modules/robotic/test_taskupdater.py
new file mode 100644
index 000000000..0cadbf85e
--- /dev/null
+++ b/tests/modules/robotic/test_taskupdater.py
@@ -0,0 +1,128 @@
+import datetime
+from typing import List
+from unittest.mock import Mock, AsyncMock
+
+import pytest
+import pytest_mock
+from astroplan import ObservingBlock
+
+import pyobs
+from pyobs.modules.robotic._taskupdater import _TaskUpdater
+from pyobs.utils.time import Time
+from tests.modules.robotic.test_scheduler import TestTaskArchive, TestTaskSchedule
+
+
+def test_compare_block_lists_with_overlap(schedule_blocks: List[ObservingBlock]) -> None:
+    old_blocks = schedule_blocks[:7]
+    new_blocks = schedule_blocks[5:]
+
+    removed, added = _TaskUpdater._compare_block_lists(old_blocks, new_blocks)
+
+    removed_names = [int(b.target.name) for b in removed]
+    new_names = [int(b.target.name) for b in added]
+
+    assert set(removed_names) == {0, 1, 2, 3, 4}
+    assert set(new_names) == {7, 8, 9}
+
+
+def test_compare_block_lists_without_overlap(schedule_blocks: List[ObservingBlock]) -> None:
+    old_blocks = schedule_blocks[:5]
+    new_blocks = schedule_blocks[5:]
+
+    removed, added = _TaskUpdater._compare_block_lists(old_blocks, new_blocks)
+
+    removed_names = [int(b.target.name) for b in removed]
+    new_names = [int(b.target.name) for b in added]
+
+    assert set(removed_names) == {0, 1, 2, 3, 4}
+    assert set(new_names) == {5, 6, 7, 8, 9}
+
+
+def test_compare_block_lists_identical(schedule_blocks: List[ObservingBlock]) -> None:
+    old_blocks = schedule_blocks
+    new_blocks = schedule_blocks
+
+    removed, added = _TaskUpdater._compare_block_lists(old_blocks, new_blocks)
+
+    removed_names = [int(b.target.name) for b in removed]
+    new_names = [int(b.target.name) for b in added]
+
+    assert len(removed_names) == 0
+    assert len(new_names) == 0
+
+
+@pytest.mark.asyncio
+async def test_worker_loop_not_changed() -> None:
+    scheduler = _TaskUpdater(TestTaskArchive(), TestTaskSchedule())
+    scheduler._update_blocks = AsyncMock()  # type: ignore
+
+    time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
+    scheduler._task_archive.last_changed = AsyncMock(return_value=time)  # type: ignore
+    scheduler._last_change = time
+
+    await scheduler.update()
+
+    scheduler._update_blocks.assert_not_called()
+
+
+@pytest.mark.asyncio
+async def test_worker_loop_new_changes(mocker: pytest_mock.MockFixture) -> None:
+    time = pyobs.utils.time.Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
+    mocker.patch("pyobs.utils.time.Time.now", return_value=time)
+    scheduler = _TaskUpdater(TestTaskArchive(), TestTaskSchedule())
+    scheduler._update_blocks = AsyncMock()  # type: ignore
+
+    scheduler._task_archive.last_changed = AsyncMock(return_value=time)  # type: ignore
+    scheduler._last_change = time - datetime.timedelta(minutes=1)  # type: ignore
+
+    await scheduler.update()
+
+    scheduler._update_blocks.assert_called_once()
+    assert scheduler._last_change == time
+
+
+@pytest.mark.asyncio
+async def test_update_blocks_no_changes(schedule_blocks: List[ObservingBlock]) -> None:
+    scheduler = _TaskUpdater(TestTaskArchive(), TestTaskSchedule())
+    scheduler._task_archive.get_schedulable_blocks = AsyncMock(return_value=schedule_blocks)  # type: ignore
+    scheduler._blocks = schedule_blocks
+
+    assert await scheduler._update_blocks() is None
+
+
+@pytest.mark.asyncio
+async def test_update_blocks_removed_current(schedule_blocks: List[ObservingBlock]) -> None:
+    scheduler = _TaskUpdater(TestTaskArchive(), TestTaskSchedule())
+
+    scheduler._task_archive.get_schedulable_blocks = AsyncMock(return_value=schedule_blocks)  # type: ignore
+    scheduler._blocks = schedule_blocks
+    scheduler._last_task_id = "0"
+
+    scheduler._compare_block_lists = Mock(return_value=([schedule_blocks[0]], []))  # type: ignore
+
+    assert await scheduler._update_blocks() is None
+
+
+@pytest.mark.asyncio
+async def test_update_blocks_removed_not_in_schedule(schedule_blocks: List[ObservingBlock]) -> None:
+    scheduler = _TaskUpdater(TestTaskArchive(), TestTaskSchedule())
+    scheduler._task_archive.get_schedulable_blocks = AsyncMock(return_value=schedule_blocks)  # type: ignore
+    scheduler._schedule.get_schedule = AsyncMock(return_value=[])  # type: ignore
+    scheduler._blocks = schedule_blocks
+
+    scheduler._compare_block_lists = Mock(return_value=([schedule_blocks[0]], []))  # type: ignore
+
+    assert await scheduler._update_blocks() is None
+
+
+@pytest.mark.asyncio
+async def test_update_blocks_need_to_update(schedule_blocks: List[ObservingBlock]) -> None:
+    scheduler = _TaskUpdater(TestTaskArchive(), TestTaskSchedule())
+
+    scheduler._task_archive.get_schedulable_blocks = AsyncMock(return_value=schedule_blocks)  # type: ignore
+    scheduler._schedule.get_schedule = AsyncMock(return_value=[])  # type: ignore
+    scheduler._blocks = []
+
+    scheduler._compare_block_lists = Mock(return_value=([], [schedule_blocks[0]]))  # type: ignore
+
+    assert await scheduler._update_blocks() == schedule_blocks
\ No newline at end of file

From 578cf0585a7c472cdcf1c3a66f1a19aaee3a1f7a Mon Sep 17 00:00:00 2001
From: GermanHydrogen <admin@germanhydrogen.org>
Date: Mon, 6 May 2024 17:15:58 +0200
Subject: [PATCH 25/25] Added missing unit test to task scheduler

---
 pyobs/modules/robotic/_taskscheduler.py     | 46 +++++++----------
 tests/modules/robotic/test_taskscheduler.py | 56 +++++++++++++--------
 2 files changed, 55 insertions(+), 47 deletions(-)

diff --git a/pyobs/modules/robotic/_taskscheduler.py b/pyobs/modules/robotic/_taskscheduler.py
index 7c3480995..593260bc6 100644
--- a/pyobs/modules/robotic/_taskscheduler.py
+++ b/pyobs/modules/robotic/_taskscheduler.py
@@ -23,13 +23,24 @@ def __init__(self, schedule: TaskSchedule, observer: Observer, schedule_range: i
 
         self._schedule_range = schedule_range
         self._safety_time = safety_time
-        self._twilight = twilight
+
+        twilight_constraint = self._get_twilight_constraint(twilight)
+        self._scheduler = astroplan.PriorityScheduler(twilight_constraint, self._observer, transitioner=astroplan.Transitioner())
 
         self._blocks: List[ObservingBlock] = []
 
         self._current_task_id: Optional[str] = None
         self._schedule_start: Optional[Time] = None
 
+    @staticmethod
+    def _get_twilight_constraint(twilight: str) -> List[astroplan.AtNightConstraint]:
+        if twilight == "astronomical":
+            return [astroplan.AtNightConstraint.twilight_astronomical()]
+        elif twilight == "nautical":
+            return [astroplan.AtNightConstraint.twilight_nautical()]
+        else:
+            raise ValueError("Unknown twilight type.")
+
     def set_current_task_id(self, task_id: Optional[str]) -> None:
         self._current_task_id = task_id
 
@@ -42,10 +53,10 @@ def set_blocks(self, blocks: List[ObservingBlock]) -> None:
     async def schedule_task(self) -> None:
         try:
             # prepare scheduler
-            blocks, start, end, constraints = await self._prepare_schedule()
+            blocks, start, end = await self._prepare_schedule()
 
             # schedule
-            scheduled_blocks = await self._schedule_blocks(blocks, start, end, constraints)
+            scheduled_blocks = await self._schedule_blocks(blocks, start, end)
 
             # finish schedule
             await self._finish_schedule(scheduled_blocks, start)
@@ -53,7 +64,7 @@ async def schedule_task(self) -> None:
         except ValueError as e:
             log.warning(str(e))
 
-    async def _prepare_schedule(self) -> Tuple[List[ObservingBlock], Time, Time, List[Any]]:
+    async def _prepare_schedule(self) -> Tuple[List[ObservingBlock], Time, Time]:
         """TaskSchedule blocks."""
 
         converted_blocks = await self._convert_blocks_to_astroplan()
@@ -67,18 +78,8 @@ async def _prepare_schedule(self) -> Tuple[List[ObservingBlock], Time, Time, Lis
             await self._schedule.set_schedule([], start)
             raise ValueError("No blocks left for scheduling.")
 
-        constraints = await self._get_twilight_constraint()
-
         # return all
-        return blocks, start, end, constraints
-
-    async def _get_twilight_constraint(self) -> List[astroplan.Constraint]:
-        if self._twilight == "astronomical":
-            return [astroplan.AtNightConstraint.twilight_astronomical()]
-        elif self._twilight == "nautical":
-            return [astroplan.AtNightConstraint.twilight_nautical()]
-        else:
-            raise ValueError("Unknown twilight type.")
+        return blocks, start, end
 
     async def _convert_blocks_to_astroplan(self) -> List[astroplan.ObservingBlock]:
         copied_blocks = [copy.copy(block) for block in self._blocks]
@@ -158,13 +159,11 @@ def _is_block_starting_in_schedule(block: astroplan.ObservingBlock, end: astropy
 
         return len(time_constraints) == 0 or len(before_end) > 0
 
-    async def _schedule_blocks(
-        self, blocks: List[ObservingBlock], start: Time, end: Time, constraints: List[Any]
-    ) -> List[ObservingBlock]:
+    async def _schedule_blocks(self, blocks: List[ObservingBlock], start: Time, end: Time) -> List[ObservingBlock]:
 
         # run actual scheduler in separate process and wait for it
         qout: mp.Queue[List[ObservingBlock]] = mp.Queue()
-        p = mp.Process(target=self._schedule_process, args=(blocks, start, end, constraints, qout))
+        p = mp.Process(target=self._schedule_process, args=(blocks, start, end, qout))
         p.start()
 
         # wait for process to finish
@@ -196,7 +195,6 @@ def _schedule_process(
         blocks: List[ObservingBlock],
         start: Time,
         end: Time,
-        constraints: List[Any],
         scheduled_blocks: mp.Queue   # type: ignore
     ) -> None:
         """Actually do the scheduling, usually run in a separate process."""
@@ -204,15 +202,9 @@ def _schedule_process(
         # log it
         log.info("Calculating schedule for %d schedulable block(s) starting at %s...", len(blocks), start)
 
-        # we don't need any transitions
-        transitioner = astroplan.Transitioner()
-
-        # create scheduler
-        scheduler = astroplan.PriorityScheduler(constraints, self._observer, transitioner=transitioner)
-
         # run scheduler
         time_range = astroplan.Schedule(start, end)
-        schedule = scheduler(blocks, time_range)
+        schedule = self._scheduler(blocks, time_range)
 
         # put scheduled blocks in queue
         scheduled_blocks.put(schedule.scheduled_blocks)
\ No newline at end of file
diff --git a/tests/modules/robotic/test_taskscheduler.py b/tests/modules/robotic/test_taskscheduler.py
index 9c8bf9e97..1407e6ab5 100644
--- a/tests/modules/robotic/test_taskscheduler.py
+++ b/tests/modules/robotic/test_taskscheduler.py
@@ -1,7 +1,7 @@
 import datetime
 import multiprocessing
 from typing import List, Any
-from unittest.mock import AsyncMock
+from unittest.mock import AsyncMock, Mock
 
 import astroplan
 import astropy.units as u
@@ -18,30 +18,21 @@
 
 @pytest.mark.asyncio
 async def test_prepare_schedule_invalid_twilight(observer: Observer) -> None:
-    scheduler = _TaskScheduler(TestTaskSchedule(), observer, 24, 60, "invalid")
-
     with pytest.raises(ValueError):
-        await scheduler._prepare_schedule()
+        _TaskScheduler(TestTaskSchedule(), observer, 24, 60, "invalid")
 
 
 @pytest.mark.asyncio
 async def test_prepare_schedule_astronomical_twilight(observer: Observer, schedule_blocks: List[ObservingBlock]) -> None:
     scheduler = _TaskScheduler(TestTaskSchedule(), observer, 24, 60, "astronomical")
-    scheduler._blocks = schedule_blocks
 
-    _, _, _, constraints = await scheduler._prepare_schedule()
-
-    assert constraints[0].max_solar_altitude == -18 * u.deg
+    assert scheduler._scheduler.constraints[0].max_solar_altitude == -18 * u.deg
 
 
 @pytest.mark.asyncio
 async def test_prepare_schedule_nautical_twilight(observer: Observer, schedule_blocks: List[ObservingBlock]) -> None:
     scheduler = _TaskScheduler(TestTaskSchedule(), observer, 24, 60, "nautical")
-    scheduler._blocks = schedule_blocks
-
-    _, _, _, constraints = await scheduler._prepare_schedule()
-
-    assert constraints[0].max_solar_altitude == -12 * u.deg
+    assert scheduler._scheduler.constraints[0].max_solar_altitude == -12 * u.deg
 
 
 @pytest.mark.asyncio
@@ -60,7 +51,7 @@ async def test_prepare_schedule_no_start(observer: Observer, schedule_blocks: Li
     scheduler = _TaskScheduler(TestTaskSchedule(), observer, 24, 60, "nautical")
     scheduler._blocks = schedule_blocks
 
-    _, start, _, _ = await scheduler._prepare_schedule()
+    _, start, _ = await scheduler._prepare_schedule()
 
     assert start.to_datetime() == datetime.datetime(2024, 4, 1, 20, 1, 0)
 
@@ -74,7 +65,7 @@ async def test_prepare_schedule_start(observer: Observer, schedule_blocks: List[
     scheduler._blocks = schedule_blocks
     scheduler._schedule_start = Time(datetime.datetime(2024, 4, 1, 20, 1, 0))
 
-    _, start, _, _ = await scheduler._prepare_schedule()
+    _, start, _ = await scheduler._prepare_schedule()
 
     assert start.to_datetime() == datetime.datetime(2024, 4, 1, 20, 1, 0)
 
@@ -88,7 +79,7 @@ async def test_prepare_schedule_end(observer: Observer, schedule_blocks: List[Ob
     scheduler._blocks = schedule_blocks
     scheduler._schedule_start = Time(datetime.datetime(2024, 4, 1, 20, 1, 0))
 
-    _, _, end, _ = await scheduler._prepare_schedule()
+    _, _, end = await scheduler._prepare_schedule()
 
     assert end.to_datetime() == datetime.datetime(2024, 4, 2, 20, 1, 0)
 
@@ -116,12 +107,12 @@ async def test_prepare_schedule_block_filtering(observer: Observer, schedule_blo
     scheduler._current_task_id = "0"
     scheduler._blocks = blocks
 
-    res_blocks, _, _, _ = await scheduler._prepare_schedule()
+    res_blocks, _, _ = await scheduler._prepare_schedule()
 
     assert [block.configuration["request"]["id"] for block in res_blocks] == ["2", "3"]
 
 
-def mock_schedule_process(blocks: List[ObservingBlock], start: Time, end: Time, constraints: List[Any],
+def mock_schedule_process(blocks: List[ObservingBlock], start: Time, end: Time,
                           scheduled_blocks: multiprocessing.Queue) -> None:  # type: ignore
     scheduled_blocks.put(blocks)
 
@@ -130,7 +121,6 @@ def mock_schedule_process(blocks: List[ObservingBlock], start: Time, end: Time,
 async def test_schedule_blocks(observer: Observer) -> None:
 
     scheduler = _TaskScheduler(TestTaskSchedule(), observer, 24, 60, "nautical")
-
     scheduler._schedule_process = mock_schedule_process  # type: ignore
 
     time = Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
@@ -139,7 +129,7 @@ async def test_schedule_blocks(observer: Observer) -> None:
         configuration={"request": {"id": "0"}}
     )
     blocks = [block]
-    scheduled_blocks = await scheduler._schedule_blocks(blocks, time, time, [])
+    scheduled_blocks = await scheduler._schedule_blocks(blocks, time, time)
     assert scheduled_blocks[0].configuration["request"]["id"] == block.configuration["request"]["id"]
 
 
@@ -177,3 +167,29 @@ async def test_convert_blocks_to_astroplan(observer: Observer) -> None:
     assert converted_blocks[0].priority == 0
     assert converted_blocks[0].constraints[0].min.to_datetime() == datetime.datetime(2024, 4, 1, 20, 0, 30)
     assert converted_blocks[0].constraints[0].max.to_datetime() == datetime.datetime(2024, 4, 1, 19, 59, 30)
+
+
+class MockSchedule:
+    def __init__(self, blocks: List[ObservingBlock]) -> None:
+        self.scheduled_blocks = blocks
+
+
+@pytest.mark.asyncio
+async def test_schedule_process(observer: Observer) -> None:
+    scheduler = _TaskScheduler(TestTaskSchedule(), observer, 24, 60, "nautical")
+
+    time = Time(datetime.datetime(2024, 4, 1, 20, 0, 0))
+    block = ObservingBlock(
+        FixedTarget(SkyCoord(0.0 * u.deg, 0.0 * u.deg, frame="icrs"), name=0), 10 * u.minute, 2000.0,
+        constraints=[astroplan.TimeConstraint(min=time, max=time)]
+    )
+    blocks = [block]
+    scheduler._scheduler = Mock(return_value=MockSchedule(blocks))
+
+    queue = multiprocessing.Queue()  # type: ignore
+    scheduler._schedule_process(blocks, time, time, queue)
+    converted_blocks = queue.get()
+
+    assert converted_blocks[0].priority == 2000.0
+    assert converted_blocks[0].constraints[0].min.to_datetime() == datetime.datetime(2024, 4, 1, 20, 0)
+    assert converted_blocks[0].constraints[0].max.to_datetime() == datetime.datetime(2024, 4, 1, 20, 0)