Skip to content

Commit

Permalink
Refactor auto-pilot macro (#19)
Browse files Browse the repository at this point in the history
* Rename the auto mode to auto-pilot mode

* Add SpectrographHelper

* Refactor auto pilot macro

* Split testing into linting and pytest

* Change workflow name

* Update deps

* Fix leftover merge conflict markers in goto_field.py

* Unwind FPS if reverse fails

* Preserve the count in auto-pilot

* If exposure starts, mark configuration as goto'd

* Fix tests

* Actually commit jaeger helper changes

* Fix auto-pilot message

* Additional improvements and handling of corner cases

* Fix tests

* Update changelog
  • Loading branch information
albireox authored May 26, 2024
1 parent 145aac6 commit c9b1c87
Show file tree
Hide file tree
Showing 27 changed files with 1,018 additions and 636 deletions.
47 changes: 47 additions & 0 deletions .github/workflows/linting.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
name: Linting

on:
push:
paths-ignore:
- 'docs/**'
pull_request:
paths-ignore:
- 'docs/**'

jobs:
build:
runs-on: ubuntu-latest

strategy:
fail-fast: false

steps:
- uses: actions/checkout@v4

- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: '3.12'
cache: 'pip'

- name: Clone actorkeys
uses: actions/checkout@v4
with:
repository: sdss/actorkeys
ref: sdss5
path: actorkeys

- name: Install dependencies
run: |
pip install --upgrade wheel pip setuptools
pip install .
- name: Lint with ruff
run: |
pip install ruff
ruff check src/hal
- name: Lint with black
run: |
pip install black
black --check src/hal
10 changes: 0 additions & 10 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,16 +42,6 @@ jobs:
pip install --upgrade wheel pip setuptools
pip install .
- name: Lint with ruff
run: |
pip install ruff
ruff src/hal
- name: Lint with black
run: |
pip install black
black --check src/hal
- name: Test with pytest
run: |
pip install pytest pytest-mock pytest-asyncio pytest-cov
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Next version

### 🚀 New

* [#19](https://github.com/sdss/HAL/pull/19) Fully rewritten version of the auto-pilot. The behaviour is generally unchanged but this rewrite fixes several bugs and should allow the auto-pilot to be started and stopped at any point. The command has been changed to `hal auto-pilot` although `hal auto` is still being aliased.

### ✨ Improved

* Fail `expose` and `goto-field` macros if another macro is running.
Expand Down
664 changes: 336 additions & 328 deletions poetry.lock

Large diffs are not rendered by default.

9 changes: 5 additions & 4 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,12 @@ hal = "hal.__main__:hal"
[tool.poetry.dependencies]
python = "^3.10,<4.0"
sdsstools = "^1.0.0"
sdss-clu = "^2.0.0"
sdss-clu = "^2.2.7"
click-default-group = "^1.2.2"
numpy = "^1.22.1"
sdssdb = ">=0.8.3"
peewee = "^3.17.0"
typing-extensions = "^4.11.0"

[tool.poetry.group.dev.dependencies]
ipython = ">=8.0.0"
Expand All @@ -59,12 +60,12 @@ ruff = ">=0.0.291"

[tool.black]
line-length = 88
target-version = ['py311']
target-version = ['py312']
fast = true

[tool.ruff]
line-length = 88
target-version = 'py311'
target-version = 'py312'
exclude = ["typings/"]

[ruff.lint]
Expand All @@ -91,7 +92,7 @@ asyncio_mode = "auto"
branch = true
include = ["src/hal/*"]
omit = [
"*/__init__.py",
"src/hal/__init__.py",
"src/hal/__main__.py",
"src/hal/exceptions.py",
"src/hal/macros/test_macro.py",
Expand Down
5 changes: 1 addition & 4 deletions src/hal/actor/actor.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

import os

from typing import ClassVar, TypeVar
from typing import ClassVar

from clu.legacy import LegacyActor

Expand All @@ -21,9 +21,6 @@
__all__ = ["HALActor", "ActorHelpers"]


T = TypeVar("T", bound="HALActor")


class HALActor(LegacyActor):
"""HAL actor."""

Expand Down
2 changes: 1 addition & 1 deletion src/hal/actor/commands/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ def fail_if_running_macro(command: HALCommandType):
return True


from .auto import *
from .auto_pilot import *
from .bypass import *
from .calibrations import *
from .expose import *
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@
from .. import HALCommandType


__all__ = ["auto"]
__all__ = ["auto_pilot"]


@hal_command_parser.command(name="auto")
@hal_command_parser.command(name="auto-pilot", aliases=["auto"])
@click.option(
"--stop",
is_flag=True,
Expand Down Expand Up @@ -65,7 +65,7 @@
default=None,
help="Preload the next design this many seconds before the exposure completes.",
)
async def auto(
async def auto_pilot(
command: HALCommandType,
stop: bool = False,
now: bool = False,
Expand All @@ -75,18 +75,18 @@ async def auto(
count: int = 1,
preload_ahead: float | None = None,
):
"""Starts the auto mode."""
"""Starts the auto-pilot mode."""

assert command.actor

expose_macro = command.actor.helpers.macros["expose"]
assert isinstance(expose_macro, ExposeMacro)

macro = command.actor.helpers.macros["auto"]
macro = command.actor.helpers.macros["auto_pilot"]

if (stop or modify or pause or resume) and not macro.running:
return command.fail(
"I'm afraid I cannot do that Dave. The auto mode is not running."
"I'm afraid I cannot do that Dave. The auto pilot mode is not running."
)

if pause and resume:
Expand All @@ -102,10 +102,10 @@ async def auto(

if stop is True:
if now is True:
command.warning(auto_mode_message="Cancelling auto mode NOW.")
command.warning(auto_pilot_message="Stopping auto-pilot mode NOW.")
macro.cancel(now=True)
else:
command.warning(auto_mode_message="Cancelling auto after stage completes.")
command.warning(auto_pilot_message="Stopping auto-pilot after this stage.")
macro.cancel(now=False)

return command.finish()
Expand All @@ -130,10 +130,14 @@ async def auto(
"I'm afraid I cannot do that Dave. The auto mode is already running."
)

macro.reset(command, count=count, preload_ahead_time=preload_ahead)

result: bool = True
while True:
# Reset the macro (stages and such) but keep the current config.
macro.reset(command, reset_config=False)

# Run the auto loop until the command is cancelled.
macro.reset(command, count=count, preload_ahead_time=preload_ahead)
if not await macro.run():
result = False
break
Expand Down
25 changes: 3 additions & 22 deletions src/hal/actor/commands/goto_field.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@

import click

from hal import config

from . import fail_if_running_macro, hal_command_parser, stages


Expand Down Expand Up @@ -99,32 +97,15 @@ async def goto_field(
if not fail_if_running_macro(command):
return

observatory = command.actor.observatory
jaeger_helper = command.actor.helpers.jaeger

if stages is not None and auto is True:
return command.fail("--auto cannot be used with custom stages.")
elif auto is True:
configuration = jaeger_helper.configuration

# The jaeger helper marks if a configuration is a new field or not but
# only if the previous configuration has been observed, which only works
# in full HAL auto mode. For goto-field auto mode, we can only check if the
# previous configuration has the same field_id, regardless of whether it
# was observed or not.
previous = jaeger_helper._previous[-1] if jaeger_helper._previous else None
auto_mode_stages = config["macros"]["goto_field"]["auto_mode"]
configuration = command.actor.helpers.jaeger.configuration

if configuration is None:
return command.fail("No configuration loaded. Auto mode cannot be used.")
elif configuration.cloned is True:
stages = auto_mode_stages["cloned_stages"][observatory]
elif previous and previous.field_id == configuration.field_id:
stages = auto_mode_stages["repeat_field_stages"][observatory]
elif configuration.is_rm_field is True:
stages = auto_mode_stages["rm_field_stages"][observatory]
else:
stages = auto_mode_stages["new_field_stages"][observatory]

stages = configuration.get_goto_field_stages()

if stages is not None and len(stages) == 0:
return command.finish("No stages to run.")
Expand Down
2 changes: 1 addition & 1 deletion src/hal/etc/hal.yml
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ macros:
APO: 730
LCO: 900

auto:
auto_pilot:
guider_time: 15
min_rms: 3.0
count: 1
Expand Down
2 changes: 1 addition & 1 deletion src/hal/etc/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
]
},
"expose_is_paused": { "type": "boolean" },
"auto_mode_message": { "type": "string" },
"auto_pilot_message": { "type": "string" },
"additionalProperties": false
}
}
39 changes: 39 additions & 0 deletions src/hal/helpers/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

from __future__ import annotations

import asyncio

from typing import TYPE_CHECKING, cast

from clu import Command, CommandStatus
Expand Down Expand Up @@ -60,6 +62,43 @@ async def _send_command(
return cast(Command, cmd)


class SpectrographHelper(HALHelper):
"""A helper class to control a spectrograph."""

def __init__(self, actor: HALActor):
super().__init__(actor)

self._exposure_time_remaining_timer: asyncio.Task | None = None
self._exposure_time_remaining: float = 0

def is_exposing(self) -> bool:
"""Returns ``True`` if the spectrograph is exposing."""

raise NotImplementedError()

@property
def exposure_time_remaining(self) -> float:
"""Returns the remaining exposure time in seconds."""

if not self.is_exposing() or self._exposure_time_remaining <= 0.0:
return 0.0

return self._exposure_time_remaining

async def _timer(self):
"""Updates the exposure time remaining."""

try:
while self._exposure_time_remaining > 0.0:
await asyncio.sleep(0.1)
self._exposure_time_remaining -= 0.1
except asyncio.CancelledError:
pass
finally:
self._exposure_time_remaining_timer = None
self._exposure_time_remaining = 0.0


def get_default_exposure_time(observatory: str, design_mode: str | None = None):
"""Returns the default exposure time for the current design mode."""

Expand Down
Loading

0 comments on commit c9b1c87

Please sign in to comment.