OSW-2455: Revise daytime control louver rules.

doc/news/OSW-2455.bugfix.rst

@@ -0,0 +1 @@
+Revised daytime control louver rules.

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.

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()