chore: cleanup

Author: bdraco

src/uiprotect/data/public_devices.py

@@ -169,37 +169,20 @@ def is_active(self) -> bool:
 
         The server does not emit a stop event when the siren finishes its
         timed run, so ``sirenStatus.isActive`` in the WS payload stays
-        ``True`` until the next update. We therefore derive the real state
-        from ``turn_off_at`` when it is available: if the calculated end time
-        is still in the future the siren is still active; once it has passed
-        the siren has stopped regardless of what the server field says.
+        ``True`` until the next update. We therefore AND the server flag
+        with a clock check against ``turn_off_at``: a manual stop clears
+        ``isActive`` immediately, and a timed expiry is caught by the
+        clock check even though the server flag is still ``True``.
         If no timing information is present we fall back to the raw server flag.
         """
         turn_off_at = self.siren_status.turn_off_at
         if turn_off_at is not None:
-            return datetime.now(UTC) < turn_off_at
+            return self.siren_status.is_active and datetime.now(UTC) < turn_off_at
         return self.siren_status.is_active
 
-    def _normalize_siren_duration(
-        self, duration: int | SirenDuration | None
-    ) -> SirenDuration:
-        if duration is None:
-            return SirenDuration.FIVE
-        if isinstance(duration, SirenDuration):
-            return duration
-        try:
-            return SirenDuration(duration)
-        except ValueError as err:
-            raise BadRequest(
-                "duration must be one of the supported siren durations "
-                f"{', '.join(str(item.value) for item in SirenDuration)} seconds"
-            ) from err
-
     async def play(self, duration: int | SirenDuration | None = None) -> None:
         """Play the siren. ``duration`` may be a supported integer or :class:`SirenDuration`; defaults to 5 seconds."""
-        await self._api.play_siren_public(
-            self.id, duration=self._normalize_siren_duration(duration)
-        )
+        await self._api.play_siren_public(self.id, duration=duration)
 
     async def stop(self) -> None:
         """Stop an active siren."""

tests/test_api_public.py

@@ -453,6 +453,28 @@ def test_siren_model_from_unifi_dict() -> None:
     )
     assert siren_expired.is_active is False
 
+    # Manual stop before the timer elapsed: server clears isActive but may
+    # leave activatedAt/duration populated. The clock check would still say
+    # "active"; the server flag must win.
+    future_at_ms = int((time.time() + 10) * 1000)
+    siren_stopped = Siren.from_unifi_dict(
+        id=SIREN_ID,
+        modelKey="siren",
+        state="CONNECTED",
+        name="Front Siren",
+        mac="AA:BB:CC:DD:EE:FF",
+        volume=80,
+        ledSettings={"isEnabled": True},
+        sirenStatus={"isActive": False, "activatedAt": future_at_ms, "duration": 5},
+        connectionType="lora",
+        wirelessConnectionState={
+            "signalState": {"signalQuality": 85, "signalStrength": -45},
+            "batteryStatus": {"percentage": 90, "isLow": False},
+            "bridge": None,
+        },
+    )
+    assert siren_stopped.is_active is False
+
 
 def test_relay_model_from_unifi_dict() -> None:
     relay = Relay.from_unifi_dict(
@@ -1682,22 +1704,16 @@ async def test_siren_device_action_helpers(
     assert siren.siren_status.turn_off_at is None
     assert siren.is_active is False
 
-    # play() with no args: _normalize_siren_duration(None) → SirenDuration.FIVE
+    # play() forwards duration unchanged; play_siren_public is the single
+    # validation/normalization site (covered by test_play_siren_public_*).
     await siren.play()
-    protect_client.play_siren_public.assert_awaited_with(
-        SIREN_ID, duration=SirenDuration.FIVE
-    )
+    protect_client.play_siren_public.assert_awaited_with(SIREN_ID, duration=None)
 
-    # play() with a SirenDuration enum: returned as-is
     await siren.play(duration=SirenDuration.TEN)
     protect_client.play_siren_public.assert_awaited_with(
         SIREN_ID, duration=SirenDuration.TEN
     )
 
-    # play() with an invalid int: _normalize_siren_duration raises BadRequest
-    with pytest.raises(BadRequest):
-        await siren.play(duration=99)
-
     protect_client.play_siren_public.reset_mock()
     await siren.play(duration=5)
     protect_client.play_siren_public.assert_awaited_once_with(SIREN_ID, duration=5)