From 6ba52b4b845cefe3a34b188e91fdab8633d4de63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20S=C3=A1nchez-Gallego?= Date: Wed, 25 Dec 2024 10:20:56 -0800 Subject: [PATCH] Add anti-flap mechanism (#31) * Update changelog * Add anti-flap mechanism * Add one more test * Update changelog --- CHANGELOG.md | 5 +-- python/lvmecp/actor/commands/dome.py | 4 +-- python/lvmecp/dome.py | 47 ++++++++++++++++------------ python/lvmecp/etc/lvmecp.yml | 1 + tests/test_command_dome.py | 40 ++++++++++++++++++++--- 5 files changed, 69 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a3b1294..69f1876 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,9 @@ ### 🚀 New -* [#29](https://github.com/sdss/lvmecp/pull/29) Add a new engineering mode that can be used to bypass the heartbeat and to allow the dome to open during daytime. -* [#30](https://github.com/sdss/lvmecp/pull/30) Prevent the dome from opening during daytime. Close if daytime is detected. +* [#29](https://github.com/sdss/lvmecp/pull/29) Add a new engineering mode that can be used to bypass the heartbeat and to allow the dome to open during daytime. Part of RORR RID-025. +* [#30](https://github.com/sdss/lvmecp/pull/30) Prevent the dome from opening during daytime. Close if daytime is detected. Part of RORR RID-025. +* [#31](https://github.com/sdss/lvmecp/pull/31) Prevent the dome from opening multiple times in a short period of time. RORR RID-019. ### ✨ Improved diff --git a/python/lvmecp/actor/commands/dome.py b/python/lvmecp/actor/commands/dome.py index 965c482..63242d3 100644 --- a/python/lvmecp/actor/commands/dome.py +++ b/python/lvmecp/actor/commands/dome.py @@ -42,7 +42,7 @@ async def open(command: ECPCommand, force=False): try: await command.actor.plc.dome.open(force=force) except DomeError as err: - return command.fail(err) + return command.fail(f"Dome failed to open with error: {err}") status = command.actor.plc.dome.status if status and status & DomeStatus.OPEN: @@ -61,7 +61,7 @@ async def close(command: ECPCommand, force=False): try: await command.actor.plc.dome.close(force=force) except DomeError as err: - return command.fail(err) + return command.fail(f"Dome failed to close with error: {err}") status = command.actor.plc.dome.status if status and status & DomeStatus.CLOSED: diff --git a/python/lvmecp/dome.py b/python/lvmecp/dome.py index 5c7cde1..29c006b 100644 --- a/python/lvmecp/dome.py +++ b/python/lvmecp/dome.py @@ -31,6 +31,9 @@ class DomeController(PLCModule[DomeStatus]): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) + # Timestamps when we have opened the dome. For the anti-flap mechanism. + self._open_attempt_times: list[float] = [] + async def _update_internal(self, use_cache: bool = True, **kwargs): dome_registers = await self.plc.modbus.read_group("dome", use_cache=use_cache) @@ -148,8 +151,10 @@ async def _move(self, open: bool, force: bool = False): async def open(self, force: bool = False): """Open the dome.""" + self._open_attempt_times.append(time()) + if not self.is_allowed(): - raise DomeError("Dome cannot be opened during daytime.") + raise DomeError("Dome is not allowed to open.") await self._move(True, force=force) @@ -180,32 +185,34 @@ async def reset(self): await asyncio.sleep(1) def is_allowed(self): - """Returns whether the dome is allowed to move. - - Currently the only check performed is to confirm that it is not daytime, - but this method could be expanded in the future. - - """ - - is_daytime: bool | None - if not config["dome.daytime_allowed"]: - is_daytime = self.is_daytime() - else: - is_daytime = None - - if not is_daytime: - return True + """Returns whether the dome is allowed to move.""" if self.plc._actor and self.plc._actor._engineering_mode: self.plc._actor.write( "w", - text="Daytime detected but engineering mode is active. " - "Allowing to open the dome.", + text="Skipping dome tests due to engineering mode.", ) - return True - return False + if not config["dome.daytime_allowed"] and self.is_daytime(): + raise DomeError("Dome is not allowed to open during daytime.") + + anti_flap_n, anti_flap_interval = config["dome.anti_flap_tolerance"] or [3, 600] + + attempts_in_interval: list[float] = [] + for tt in self._open_attempt_times[::-1]: + if time() - tt < anti_flap_interval: + attempts_in_interval.append(tt) + else: + break + + if len(attempts_in_interval) >= anti_flap_n: + raise DomeError( + "Too many open attempts in a short interval. " + f"Wait {anti_flap_interval} seconds before trying again." + ) + + return True def is_daytime(self): # pragma: no cover """Returns whether it is daytime.""" diff --git a/python/lvmecp/etc/lvmecp.yml b/python/lvmecp/etc/lvmecp.yml index a51f596..580ebe0 100644 --- a/python/lvmecp/etc/lvmecp.yml +++ b/python/lvmecp/etc/lvmecp.yml @@ -175,6 +175,7 @@ safety: dome: daytime_allowed: false daytime_tolerance: 600 + anti_flap_tolerance: [3, 600] hvac: host: 10.8.38.49 diff --git a/tests/test_command_dome.py b/tests/test_command_dome.py index fcba82e..2a785f7 100644 --- a/tests/test_command_dome.py +++ b/tests/test_command_dome.py @@ -12,8 +12,11 @@ from typing import TYPE_CHECKING +import pytest + import lvmecp.actor.actor import lvmecp.dome +from lvmecp.exceptions import DomeError from lvmecp.maskbits import DomeStatus @@ -57,11 +60,16 @@ async def test_command_dome_daytime(actor: ECPActor, mocker: MockerFixture): async def test_command_dome_daytime_allowed(actor: ECPActor, mocker: MockerFixture): - mocker.patch.object( - lvmecp.dome, - "config", - return_value={"dome": {"daytime_allowed": True}}, + mocker.patch.dict( + "lvmecp.dome.config", + { + "dome": { + "daytime_allowed": True, + "anti_flap_tolerance": [3, 600], + } + }, ) + mocker.patch.object(actor.plc.dome, "is_daytime", return_value=True) mocker.patch.object(actor.plc.dome, "_move", return_value=True) @@ -113,3 +121,27 @@ async def test_actor_daytime_task_eng_mode(actor: ECPActor, mocker: MockerFixtur dome_close_mock.assert_not_called() task.cancel() + + +async def test_dome_anti_flap(actor: ECPActor, mocker: MockerFixture): + mocker.patch.dict("lvmecp.dome.config", {"dome": {"anti_flap_tolerance": [3, 1]}}) + + mocker.patch.object(actor.plc.dome, "is_daytime", return_value=False) + mocker.patch.object(actor.plc.dome, "_move", return_value=True) + + await actor.plc.dome.open() + await asyncio.sleep(0.1) + + await actor.plc.dome.open() + await asyncio.sleep(0.1) + + with pytest.raises(DomeError): + await actor.plc.dome.open() + await asyncio.sleep(0.1) + + +async def test_dome_not_allowed(actor: ECPActor, mocker: MockerFixture): + mocker.patch.object(actor.plc.dome, "is_allowed", return_value=False) + + with pytest.raises(DomeError, match="Dome is not allowed to open."): + await actor.plc.dome.open()