Fix Timer change service (#93469)

homeassistant/components/timer/__init__.py

@@ -336,11 +336,17 @@ class Timer(collection.CollectionEntity, RestoreEntity):
             raise HomeAssistantError(
                 f"Timer {self.entity_id} is not running, only active timers can be changed"
             )
+        if self._remaining and (self._remaining + duration) > self._duration:
+            raise HomeAssistantError(
+                f"Not possible to change timer {self.entity_id} beyond configured duration"
+            )
+        if self._remaining and (self._remaining + duration) < timedelta():
+            raise HomeAssistantError(
+                f"Not possible to change timer {self.entity_id} to negative time remaining"
+            )
 
         self._listener()
-        self._listener = None
         self._end += duration
-        self._duration += duration
         self._remaining = self._end - dt_util.utcnow().replace(microsecond=0)
         self.hass.bus.async_fire(EVENT_TIMER_CHANGED, {ATTR_ENTITY_ID: self.entity_id})
         self._listener = async_track_point_in_utc_time(

tests/components/timer/test_init.py

@@ -233,7 +233,7 @@ async def test_methods_and_events(hass: HomeAssistant) -> None:
             "call": SERVICE_CHANGE,
             "state": STATUS_ACTIVE,
             "event": EVENT_TIMER_CHANGED,
-            "data": {CONF_DURATION: 15},
+            "data": {CONF_DURATION: -5},
         },
         {
             "call": SERVICE_START,
@@ -316,31 +316,51 @@ async def test_start_service(hass: HomeAssistant) -> None:
     assert state.attributes[ATTR_DURATION] == "0:00:15"
     assert state.attributes[ATTR_REMAINING] == "0:00:15"
 
+    with pytest.raises(
+        HomeAssistantError,
+        match="Not possible to change timer timer.test1 beyond configured duration",
+    ):
         await hass.services.async_call(
             DOMAIN,
             SERVICE_CHANGE,
-        {CONF_ENTITY_ID: "timer.test1", CONF_DURATION: 15},
+            {CONF_ENTITY_ID: "timer.test1", CONF_DURATION: 20},
+            blocking=True,
+        )
+
+    with pytest.raises(
+        HomeAssistantError,
+        match="Not possible to change timer timer.test1 to negative time remaining",
+    ):
+        await hass.services.async_call(
+            DOMAIN,
+            SERVICE_CHANGE,
+            {CONF_ENTITY_ID: "timer.test1", CONF_DURATION: -20},
             blocking=True,
         )
-    await hass.async_block_till_done()
-    state = hass.states.get("timer.test1")
-    assert state
-    assert state.state == STATUS_ACTIVE
-    assert state.attributes[ATTR_DURATION] == "0:00:30"
-    assert state.attributes[ATTR_REMAINING] == "0:00:30"
 
     await hass.services.async_call(
         DOMAIN,
         SERVICE_CHANGE,
-        {CONF_ENTITY_ID: "timer.test1", CONF_DURATION: -10},
+        {CONF_ENTITY_ID: "timer.test1", CONF_DURATION: -3},
         blocking=True,
     )
-    await hass.async_block_till_done()
     state = hass.states.get("timer.test1")
     assert state
     assert state.state == STATUS_ACTIVE
-    assert state.attributes[ATTR_DURATION] == "0:00:20"
+    assert state.attributes[ATTR_DURATION] == "0:00:15"
-    assert state.attributes[ATTR_REMAINING] == "0:00:20"
+    assert state.attributes[ATTR_REMAINING] == "0:00:12"
+
+    await hass.services.async_call(
+        DOMAIN,
+        SERVICE_CHANGE,
+        {CONF_ENTITY_ID: "timer.test1", CONF_DURATION: 2},
+        blocking=True,
+    )
+    state = hass.states.get("timer.test1")
+    assert state
+    assert state.state == STATUS_ACTIVE
+    assert state.attributes[ATTR_DURATION] == "0:00:15"
+    assert state.attributes[ATTR_REMAINING] == "0:00:14"
 
     await hass.services.async_call(
         DOMAIN, SERVICE_CANCEL, {CONF_ENTITY_ID: "timer.test1"}, blocking=True
@@ -349,22 +369,24 @@ async def test_start_service(hass: HomeAssistant) -> None:
     state = hass.states.get("timer.test1")
     assert state
     assert state.state == STATUS_IDLE
-    assert state.attributes[ATTR_DURATION] == "0:00:20"
+    assert state.attributes[ATTR_DURATION] == "0:00:15"
     assert ATTR_REMAINING not in state.attributes
 
-    with pytest.raises(HomeAssistantError):
+    with pytest.raises(
+        HomeAssistantError,
+        match="Timer timer.test1 is not running, only active timers can be changed",
+    ):
         await hass.services.async_call(
             DOMAIN,
             SERVICE_CHANGE,
-            {CONF_ENTITY_ID: "timer.test1", CONF_DURATION: 10},
+            {CONF_ENTITY_ID: "timer.test1", CONF_DURATION: 2},
             blocking=True,
         )
-        await hass.async_block_till_done()
 
     state = hass.states.get("timer.test1")
     assert state
     assert state.state == STATUS_IDLE
-    assert state.attributes[ATTR_DURATION] == "0:00:20"
+    assert state.attributes[ATTR_DURATION] == "0:00:15"
     assert ATTR_REMAINING not in state.attributes
 
 
@@ -372,7 +394,7 @@ async def test_wait_till_timer_expires(hass: HomeAssistant) -> None:
     """Test for a timer to end."""
     hass.state = CoreState.starting
 
-    await async_setup_component(hass, DOMAIN, {DOMAIN: {"test1": {CONF_DURATION: 10}}})
+    await async_setup_component(hass, DOMAIN, {DOMAIN: {"test1": {CONF_DURATION: 20}}})
 
     state = hass.states.get("timer.test1")
     assert state
@@ -405,7 +427,7 @@ async def test_wait_till_timer_expires(hass: HomeAssistant) -> None:
     await hass.services.async_call(
         DOMAIN,
         SERVICE_CHANGE,
-        {CONF_ENTITY_ID: "timer.test1", CONF_DURATION: 10},
+        {CONF_ENTITY_ID: "timer.test1", CONF_DURATION: -5},
         blocking=True,
     )
     await hass.async_block_till_done()