From 15dbca7eeabaa4512a2b33eee70fb1c45a9adb82 Mon Sep 17 00:00:00 2001 From: Brian Brondel Date: Tue, 16 Jun 2026 16:36:24 -0400 Subject: [PATCH] Revise daytime control louver rules. --- doc/news/OSW-2455.bugfix.rst | 1 + python/lsst/ts/eas/dome_model.py | 107 +++++++++++++++------ tests/test_dome.py | 158 ++++++++++++++++++++++++++++++- 3 files changed, 234 insertions(+), 32 deletions(-) create mode 100644 doc/news/OSW-2455.bugfix.rst diff --git a/doc/news/OSW-2455.bugfix.rst b/doc/news/OSW-2455.bugfix.rst new file mode 100644 index 0000000..20362c4 --- /dev/null +++ b/doc/news/OSW-2455.bugfix.rst @@ -0,0 +1 @@ +Revised daytime control louver rules. diff --git a/python/lsst/ts/eas/dome_model.py b/python/lsst/ts/eas/dome_model.py index 516778b..dd22a88 100644 --- a/python/lsst/ts/eas/dome_model.py +++ b/python/lsst/ts/eas/dome_model.py @@ -50,6 +50,11 @@ # from the azimuth callback. AZIMUTH_CHANGE_THRESHOLD = 0.1 +# Tolerance (percent-open) within which a reported positionCommanded is +# considered equal to the position EAS last commanded. A larger difference is +# treated as a freshly sent command rather than EAS's own command echoed back. +LOUVER_COMMAND_TOLERANCE = 0.1 + # Atmospheric pressure used in the sun-altitude refraction correction. SUN_ALTITUDE_PRESSURE = 700 * u.hPa @@ -76,10 +81,9 @@ class DomeModel: the sun's azimuth at which the louver is considered to be facing the sun. louver_exposed_command : `float` - Commanded percent-open position for a louver that faces the sun. - louver_shaded_command : `float` - Commanded percent-open position for a louver that does not face the - sun. + Maximum commanded percent-open position for a louver that faces the + sun. A sun-facing louver is never opened beyond this value, nor beyond + the position the observer commanded. sun_altitude_threshold : `float` Sun altitude (degrees) above which louver positions are adjusted based on sun azimuth. @@ -102,7 +106,6 @@ def __init__( dome_open_threshold: float, louver_sun_angle: float, louver_exposed_command: float, - louver_shaded_command: float, sun_altitude_threshold: float, dome_remote: salobj.Remote, features_to_disable: list[str], @@ -123,15 +126,26 @@ def __init__( # at which a louver is considered to be "facing" the sun. self.louver_sun_angle: float = louver_sun_angle - # Desired command position for a louver facing the sun. + # Maximum command position for a louver facing the sun. self.louver_exposed_command: float = louver_exposed_command - # Desired command position for a louver that is not facing the sun. - self.louver_shaded_command: float = louver_shaded_command - # Sun altitude (degrees) above which louvers are adjusted. self.sun_altitude_threshold: float = sun_altitude_threshold + # Observer-commanded position per louver. EAS caps a sun-facing louver + # at `louver_exposed_command` but never opens a louver beyond what the + # observer requested, so the observer's request must be remembered + # separately. This list stores observed louver commands issued by the + # operator so that they can be re-issued when the louver returns to a + # shaded position. If no command has been observed, the value is None. + self.louver_operator_command: list[float] | None = None + + # Position EAS last intended for each louver (with -1 "do not move" + # resolved to the value carried forward). Used to distinguish EAS's own + # commands echoed back in positionCommanded from fresh observer + # commands. None until the first daytime adjustment; reset at sundown. + self.louver_eas_command: list[float] | None = None + self.on_open: deque[asyncio.Event] = deque() self.was_closed: bool | None = None @@ -160,13 +174,12 @@ def get_config_schema(cls) -> str: type: number default: 60.0 louver_exposed_command: - description: Commanded percent-open position for a louver that faces the sun. + description: >- + Maximum commanded percent-open position for a louver that faces the sun. + A sun-facing louver is never opened beyond this value, nor beyond the + position the observer commanded. type: number default: 50.0 - louver_shaded_command: - description: Commanded percent-open position for a louver that does not face the sun. - type: number - default: 100.0 sun_altitude_threshold: description: >- Sun altitude (degrees) above which louver positions are adjusted based @@ -177,7 +190,6 @@ def get_config_schema(cls) -> str: - dome_open_threshold - louver_sun_angle - louver_exposed_command - - louver_shaded_command additionalProperties: false """ ) @@ -294,13 +306,19 @@ async def set_louvers(self, position: list[float]) -> dict[str, Any] | None: async def adjust_louvers(self, sun_azimuth: float) -> None: """Adjust positions of louvers. - If a louver is actively being commanded (`positionCommanded` >= 0) then - the commanded position of the louver should be adjusted based on the - azimuth of the sun: if the louvers face the sun (within - `louver_sun_angle`) its commanded position should be set to - `exposed_lover_command`. If the louver does not face the sun, its - commanded position should be set to `shaded_louver_command` If the - louver is not commanded, it should remain uncommanded. + EAS never opens a louver beyond the position the observer commanded. A + louver the observer has opened is capped at `louver_exposed_command` + while it faces the sun (within `louver_sun_angle`) and is otherwise + left at the observer's commanded position. A louver the observer has + not opened remains uncommanded. + + Because EAS commands louvers through the same path the observer uses, + the `positionCommanded` telemetry is overwritten by EAS's own caps and + no longer reflects the observer's intent. The standing observer command + is therefore tracked separately in `self.louver_operator_command`. + A change in `positionCommanded` that differs from what EAS last sent + (`self.louver_eas_command`) is taken to be a fresh observer command and + updates the baseline. Parameters ---------- @@ -310,20 +328,47 @@ async def adjust_louvers(self, sun_azimuth: float) -> None: if self.louvers_telemetry is None or self.dome_azimuth is None: return + position_commanded = list(self.louvers_telemetry.positionCommanded[: len(LouverTable)]) + + # Reconcile the observer baseline against the latest telemetry. + if self.louver_operator_command is None or self.louver_eas_command is None: + # First daytime adjustment: adopt the reported commands as the + # observer's standing request. + self.louver_operator_command = list(position_commanded) + else: + for i, commanded in enumerate(position_commanded): + if commanded <= 0 or abs(commanded - self.louver_eas_command[i]) > LOUVER_COMMAND_TOLERANCE: + # The observer moved this louver (closed it, or commanded a + # position EAS did not). Adopt it as the new baseline. + self.louver_operator_command[i] = commanded + louver_azimuth = [(self.dome_azimuth + louver.azimuth) % CIRCLE for louver in LouverTable] sun_distance = [ min((sun_azimuth - az) % CIRCLE, (az - sun_azimuth) % CIRCLE) for az in louver_azimuth ] + + # Settle on what commands to send: louver_command = [ ( -1.0 - if cmd <= 0 - else self.louver_exposed_command - if sd < self.louver_sun_angle - else self.louver_shaded_command + if base <= 0 # <-- No command if the louver is closed. + else min(base, self.louver_exposed_command) # Min of command or `louver_exposed_command`... + if sd < self.louver_sun_angle # ...if the louver is exposed to the sun... + else base # ... or the observer's commanded position otherwise. ) - for cmd, sd in zip(self.louvers_telemetry.positionCommanded[: len(LouverTable)], sun_distance) + for base, sd in zip(self.louver_operator_command, sun_distance) ] + + # Copy the non-negative values in `louver_command` to + # `self.louver_eas_command` for later reference. + self.louver_eas_command = [ + previous if command < 0 else command + for command, previous in zip( + louver_command, + self.louver_eas_command if self.louver_eas_command is not None else louver_command, + ) + ] + await self.set_louvers(position=louver_command) async def update_louvers_for_sun(self) -> None: @@ -332,7 +377,9 @@ async def update_louvers_for_sun(self) -> None: Computes the sun's current altitude and azimuth at the observatory location and, if the sun is above `sun_altitude_threshold` and the ``day_louvers`` feature is not disabled, calls `adjust_louvers` - with the sun azimuth. + with the sun azimuth. When the sun is below the threshold, the tracked + observer baseline is cleared so the next daytime adjustment re-adopts + fresh observer commands rather than re-applying the previous day's. """ if "day_louvers" in self.features_to_disable: return @@ -348,6 +395,10 @@ async def update_louvers_for_sun(self) -> None: ) if altaz.alt.deg > self.sun_altitude_threshold: await self.adjust_louvers(altaz.az.deg) + else: + # Sundown: discard the stale observer baseline. + self.louver_operator_command = None + self.louver_eas_command = None async def monitor(self) -> None: """Monitor the sun position and adjust louvers accordingly. diff --git a/tests/test_dome.py b/tests/test_dome.py index 34a8792..2ee213a 100644 --- a/tests/test_dome.py +++ b/tests/test_dome.py @@ -142,7 +142,6 @@ async def asyncSetUp(self) -> None: dome_open_threshold=50.0, louver_sun_angle=60.0, louver_exposed_command=50.0, - louver_shaded_command=100.0, sun_altitude_threshold=0.0, dome_remote=self.fake_remote, features_to_disable=[], @@ -281,12 +280,14 @@ async def test_monitor_calls_adjust_louvers_when_sun_is_up(self) -> None: """monitor() adjusts louvers based on sun azimuth. With dome at azimuth 0 (slit pointing north) and sun at azimuth 90 - (east), with louver_sun_angle=60: + (east), with louver_sun_angle=60 and an observer command of 100: - Louver A1 (index 0) is uncommanded (positionCommanded=-1) => -1 - Louvers A2-E3 (indices 1-13) face the sun (panel normals are - 53.1-120.75 deg, within 60 deg of az 90) => exposed (50.0) + 53.1-120.75 deg, within 60 deg of az 90) => capped at the exposed + command min(100, 50) => 50.0 - Louvers F1-N2 (indices 14-33) face away from the sun (panel - normals 180-306.9 deg, outside the 60 deg window) => shaded (100.0) + normals 180-306.9 deg, outside the 60 deg window) => left at the + observer command (shaded louvers are uncapped) => 100.0 """ mock_altaz = mock.MagicMock() mock_altaz.alt.deg = 45.0 @@ -340,6 +341,155 @@ async def test_adjust_louvers_dome_opposite_sun_exposes_f1(self) -> None: "F1 should be exposed when the dome slit points opposite the sun", ) + async def test_adjust_louvers_below_exposed_limit_never_moves(self) -> None: + """A louver opened to <= louver_exposed_command is left alone. + + With the observer commanding 10 (below the exposed cap of 50), every + commanded louver is left at 10 whether it faces the sun or not, because + ``min(10, 50) == 10`` and shaded louvers are uncapped. + """ + self.model.dome_azimuth = 0.0 + self.model.louvers_telemetry = SimpleNamespace( + positionCommanded=[10.0] * 34, + ) + self.fake_remote.evt_summaryState.set_state(salobj.State.ENABLED) + + await self.model.adjust_louvers(90.0) + await spin_until(lambda: bool(self.fake_remote.cmd_setLouvers.calls)) + + position = self.fake_remote.cmd_setLouvers.calls[-1]["position"] + self.assertTrue(all(p == 10.0 for p in position)) + + async def test_adjust_louvers_splits_at_exposed_limit(self) -> None: + """Commanded position is limited for exposed louvers. + + With the observer commanding 80 (above the exposed cap of 50), + sun-facing louvers are capped to 50 while shaded louvers stay at 80. + """ + self.model.dome_azimuth = 0.0 + self.model.louvers_telemetry = SimpleNamespace( + positionCommanded=[80.0] * 34, + ) + self.fake_remote.evt_summaryState.set_state(salobj.State.ENABLED) + + await self.model.adjust_louvers(90.0) + await spin_until(lambda: bool(self.fake_remote.cmd_setLouvers.calls)) + + # With dome az 0 and sun az 90, indices 0-13 face the sun and 14-33 are + # shaded (see test_monitor_calls_adjust_louvers_when_sun_is_up). + position = self.fake_remote.cmd_setLouvers.calls[-1]["position"] + self.assertTrue(all(p == 50.0 for p in position[:14])) + self.assertTrue(all(p == 80.0 for p in position[14:])) + + async def test_adjust_louvers_closed_stays_closed(self) -> None: + """Louvers the observer has not opened remain uncommanded (-1).""" + self.model.dome_azimuth = 0.0 + self.model.louvers_telemetry = SimpleNamespace( + positionCommanded=[0.0] * 34, + ) + self.fake_remote.evt_summaryState.set_state(salobj.State.ENABLED) + + await self.model.adjust_louvers(90.0) + await spin_until(lambda: bool(self.fake_remote.cmd_setLouvers.calls)) + + position = self.fake_remote.cmd_setLouvers.calls[-1]["position"] + self.assertTrue(all(p == -1.0 for p in position)) + + async def test_adjust_louvers_reopens_after_leaving_sun(self) -> None: + """A louver re-opens to the observer command after leaving the sun. + + This is the regression guard for the overwrite problem: EAS commands + through the same path the observer uses, so its own cap is echoed back + in ``positionCommanded``. The observer baseline must be remembered so a + louver that was capped while exposed returns to the full observer + command once it is shaded, rather than staying at the cap. + """ + sun_az = 73.0 + f1_index = find_louver("F1").index + + # Cycle 1: dome points opposite the sun, so F1 faces the sun and is + # capped from the observer command of 80 down to 50. + self.model.dome_azimuth = (sun_az + 180.0) % 360.0 + self.model.louvers_telemetry = SimpleNamespace( + positionCommanded=[80.0] * 34, + ) + self.fake_remote.evt_summaryState.set_state(salobj.State.ENABLED) + + await self.model.adjust_louvers(sun_az) + await spin_until(lambda: bool(self.fake_remote.cmd_setLouvers.calls)) + cycle1 = self.fake_remote.cmd_setLouvers.calls[-1]["position"] + self.assertEqual(cycle1[f1_index], 50.0) + + # Echo EAS's own command back as the new telemetry, exactly as MTDome + # would report it. + self.model.louvers_telemetry = SimpleNamespace( + positionCommanded=list(cycle1), + ) + + # Cycle 2: dome now points at the sun, so F1 is shaded and must return + # to the remembered observer command of 80, not stay at the 50 cap. + self.model.dome_azimuth = sun_az + await self.model.adjust_louvers(sun_az) + await spin_until(lambda: len(self.fake_remote.cmd_setLouvers.calls) >= 2) + + cycle2 = self.fake_remote.cmd_setLouvers.calls[-1]["position"] + self.assertEqual(cycle2[f1_index], 80.0) + + async def test_adjust_louvers_new_observer_command_overrides_cap(self) -> None: + """A fresh observer command replaces the remembered baseline. + + After EAS caps an exposed louver, the observer commanding a new + position (distinct from EAS's last command) must update the baseline. + """ + sun_az = 73.0 + f1_index = find_louver("F1").index + + # Cycle 1: F1 faces the sun, capped from 80 to 50. + self.model.dome_azimuth = (sun_az + 180.0) % 360.0 + self.model.louvers_telemetry = SimpleNamespace( + positionCommanded=[80.0] * 34, + ) + self.fake_remote.evt_summaryState.set_state(salobj.State.ENABLED) + + await self.model.adjust_louvers(sun_az) + await spin_until(lambda: bool(self.fake_remote.cmd_setLouvers.calls)) + cycle1 = self.fake_remote.cmd_setLouvers.calls[-1]["position"] + + # The observer now commands F1 to 30, which differs from EAS's last + # command of 50 and so is taken as a fresh observer request. + echoed = list(cycle1) + echoed[f1_index] = 30.0 + self.model.louvers_telemetry = SimpleNamespace( + positionCommanded=echoed, + ) + + # Cycle 2: F1 still faces the sun; min(30, 50) == 30 confirms the + # baseline was updated to 30. + await self.model.adjust_louvers(sun_az) + await spin_until(lambda: len(self.fake_remote.cmd_setLouvers.calls) >= 2) + + cycle2 = self.fake_remote.cmd_setLouvers.calls[-1]["position"] + self.assertEqual(cycle2[f1_index], 30.0) + + async def test_update_louvers_for_sun_resets_baseline_at_sundown(self) -> None: + """The observer baseline is cleared when the sun drops below threshold. + + The reset depends only on sun altitude, not on dome state. + """ + self.model.louver_operator_command = [80.0] * 34 + self.model.louver_eas_command = [50.0] * 34 + + mock_altaz = mock.MagicMock() + mock_altaz.alt.deg = -10.0 + mock_sun = mock.MagicMock() + mock_sun.transform_to.return_value = mock_altaz + + with mock.patch("lsst.ts.eas.dome_model.get_sun", return_value=mock_sun): + await self.model.update_louvers_for_sun() + + self.assertIsNone(self.model.louver_operator_command) + self.assertIsNone(self.model.louver_eas_command) + async def test_monitor_skips_adjust_louvers_when_sun_is_down(self) -> None: """monitor() should not call adjust_louvers after sundown.""" mock_altaz = mock.MagicMock()