diff --git a/custom_components/adaptive_lighting/switch.py b/custom_components/adaptive_lighting/switch.py index bc918eca..e8b3a8cd 100644 --- a/custom_components/adaptive_lighting/switch.py +++ b/custom_components/adaptive_lighting/switch.py @@ -475,14 +475,12 @@ async def handle_apply_service(hass: HomeAssistant, service_call: ServiceCall) - transition = data.get(CONF_TRANSITION) if transition is None: transition = switch.initial_transition - previous = switch.manager.get_manual_control_attributes(light) - if manual_attributes: - # Mark before adapting, so a regular adaptation cannot slip in - # between and overwrite the values with those of the current time. - switch.manager.add_manual_control_attributes( - light, - manual_attributes, - ) + # Hold before adapting, so a regular adaptation cannot slip in + # between and overwrite the values with those of the current time. + previous = switch.manager.hold_manual_control_attributes( + light, + manual_attributes, + ) applied = False try: applied = ( @@ -498,13 +496,19 @@ async def handle_apply_service(hass: HomeAssistant, service_call: ServiceCall) - ) ) finally: - if manual_attributes and not applied: + if manual_attributes and applied: + # Like a manual change, which (re)starts the auto reset. + switch.manager.add_manual_control_attributes( + light, + manual_attributes, + ) + switch.fire_manual_control_event(light, context) + else: switch.manager.restore_manual_control_attributes( light, previous, + manual_attributes, ) - if manual_attributes and applied: - switch.fire_manual_control_event(light, context) async def handle_set_manual_control_service( @@ -1494,7 +1498,7 @@ class AdaptiveSwitch(SwitchEntity, RestoreEntity): force: bool = False, at_time: datetime.datetime | None = None, ) -> bool: - """Adapt a light, returning whether any adaptation was sent to it.""" + """Adapt a light, returning whether it was adapted (see `_execute_adaptation_calls`).""" if (lock := self.manager.turn_off_locks.get(light)) and lock.locked(): _LOGGER.debug("%s: '%s' is locked", self._name, light) return False @@ -1512,11 +1516,14 @@ class AdaptiveSwitch(SwitchEntity, RestoreEntity): if data is None: return False # nothing to adapt - await self.execute_cancellable_adaptation_calls(data) - return True + return await self.execute_cancellable_adaptation_calls(data) - async def _execute_adaptation_calls(self, data: AdaptationData) -> None: - """Executes a sequence of adaptation service calls for the given service datas.""" + async def _execute_adaptation_calls(self, data: AdaptationData) -> bool: + """Executes a sequence of adaptation service calls for the given service datas. + + Returns whether the whole sequence was carried out, where service calls that + are redundant because the light is already in that state count as carried out. + """ for index in range(data.max_length): is_first_call = index == 0 @@ -1525,7 +1532,7 @@ class AdaptiveSwitch(SwitchEntity, RestoreEntity): await asyncio.sleep(data.sleep_time) if self._removed: - return + return False # Instead of directly iterating the generator in the while-loop, we get # the next item here after the sleep to make sure it incorporates state @@ -1548,7 +1555,7 @@ class AdaptiveSwitch(SwitchEntity, RestoreEntity): self._name, data.entity_id, ) - return + return False _LOGGER.debug( "%s: Scheduling 'light.turn_on' with the following 'service_data': %s" @@ -1572,18 +1579,20 @@ class AdaptiveSwitch(SwitchEntity, RestoreEntity): service_data, context=data.context, ) + return True async def execute_cancellable_adaptation_calls( self, data: AdaptationData, - ) -> None: + ) -> bool: """Executes a cancellable sequence of adaptation service calls for the given service datas. Wraps the sequence of service calls in a task that can be cancelled from elsewhere, e.g., - to cancel an ongoing adaptation when a light is turned off. + to cancel an ongoing adaptation when a light is turned off. Returns whether the whole + sequence was carried out, i.e., not stopped or cancelled. """ if self._removed: - return + return False # Prevent overlap of multiple adaptation sequences self.manager.cancel_ongoing_adaptation_calls(data.entity_id) @@ -1599,7 +1608,7 @@ class AdaptiveSwitch(SwitchEntity, RestoreEntity): self.manager.adaptation_tasks_brightness[data.entity_id] = task if LightControlAttributes.COLOR in data.attributes: self.manager.adaptation_tasks_color[data.entity_id] = task - await task + return await task except asyncio.CancelledError: _LOGGER.debug( "%s: Ongoing adaptation of %s cancelled, with AdaptationData: %s", @@ -1607,6 +1616,7 @@ class AdaptiveSwitch(SwitchEntity, RestoreEntity): data.entity_id, data, ) + return False async def _update_attrs_and_maybe_adapt_lights( self, @@ -2557,18 +2567,36 @@ class AdaptiveLightingManager: new = current | attributes self.set_manual_control_attributes(light, new) - def restore_manual_control_attributes( + def hold_manual_control_attributes( self, light: str, attributes: LightControlAttributes, - ) -> None: - """Restore the manual control of a light after marking it had no effect.""" + ) -> LightControlAttributes: + """Stop adapting attributes of a light, without (re)starting its auto reset. + + Returns the previous manual control, for `restore_manual_control_attributes`. + """ + previous = self.get_manual_control_attributes(light) if attributes: - self.set_manual_control_attributes(light, attributes) + self.manual_control[light] = previous | attributes + return previous + + def restore_manual_control_attributes( + self, + light: str, + previous: LightControlAttributes, + held: LightControlAttributes, + ) -> None: + """Undo `hold_manual_control_attributes`, keeping a running auto reset. + + Only the attributes that were added by holding them are released, so a reset + in the meantime isn't undone. + """ + current = self.get_manual_control_attributes(light) + restored = current & ~(held & ~previous) + if restored == current: return - self.manual_control[light] = LightControlAttributes.NONE - if timer := self.auto_reset_manual_control_timers.pop(light, None): - timer.cancel() + self.manual_control[light] = restored self._schedule_manual_control_state_update(light) def invalidate_manual_control_state( diff --git a/tests/test_switch.py b/tests/test_switch.py index e07c8656..cc14c360 100644 --- a/tests/test_switch.py +++ b/tests/test_switch.py @@ -2338,7 +2338,7 @@ async def test_apply_service_at_time_marks_manual_before_adapting(hass): manual_during_adaptation.append( switch.manager.get_manual_control_attributes(light), ) - await adapt_light(light, *args, **kwargs) + return await adapt_light(light, *args, **kwargs) with patch.object(switch, "_adapt_light", side_effect=record_and_adapt): await hass.services.async_call( @@ -2352,6 +2352,10 @@ async def test_apply_service_at_time_marks_manual_before_adapting(hass): blocking=True, ) assert manual_during_adaptation == [LightControlAttributes.ALL] + assert ( + switch.manager.get_manual_control_attributes(ENTITY_LIGHT_1) + == LightControlAttributes.ALL + ) @pytest.mark.parametrize("failure", ["locked", "error"]) @@ -2407,6 +2411,121 @@ async def test_apply_service_at_time_not_applied_restores_manual_control( assert events == [] +async def test_apply_service_at_time_cancelled_restores_manual_control(hass): + """A time whose adaptation is cancelled before it's sent isn't held.""" + switch, _ = await setup_lights_and_switch(hass) + events = [] + hass.bus.async_listen(f"{DOMAIN}.manual_control", events.append) + switch.manager.reset(ENTITY_LIGHT_1) + started = asyncio.Event() + + async def never_finishes(data): + started.set() + await asyncio.Event().wait() + + with patch.object(switch, "_execute_adaptation_calls", side_effect=never_finishes): + call = hass.async_create_task( + hass.services.async_call( + DOMAIN, + SERVICE_APPLY, + { + ATTR_ENTITY_ID: switch.entity_id, + CONF_LIGHTS: [ENTITY_LIGHT_1], + CONF_APPLY_TIME: "23:30:00", + }, + blocking=True, + ), + ) + await started.wait() + # E.g., the light becomes unavailable + switch.manager.cancel_ongoing_adaptation_calls(ENTITY_LIGHT_1) + await call + await hass.async_block_till_done() + assert not switch.manager.get_manual_control_attributes(ENTITY_LIGHT_1) + assert events == [] + + +async def test_apply_service_at_time_not_applied_keeps_auto_reset(hass): + """A time that isn't applied doesn't extend the auto reset of manual control.""" + switch, _ = await setup_lights_and_switch(hass, {CONF_AUTORESET_CONTROL: 600}) + switch.manager.reset(ENTITY_LIGHT_1) + switch.manager.set_manual_control_attributes( + ENTITY_LIGHT_1, + LightControlAttributes.BRIGHTNESS, + ) + timer = switch.manager.auto_reset_manual_control_timers[ENTITY_LIGHT_1] + start_time = timer.start_time + + async def apply(): + await hass.services.async_call( + DOMAIN, + SERVICE_APPLY, + { + ATTR_ENTITY_ID: switch.entity_id, + CONF_LIGHTS: [ENTITY_LIGHT_1], + CONF_APPLY_TIME: "23:30:00", + }, + blocking=True, + ) + await hass.async_block_till_done() + + lock = switch.manager.turn_off_locks.setdefault(ENTITY_LIGHT_1, asyncio.Lock()) + with patch( + "homeassistant.components.adaptive_lighting.switch.dt_util.utcnow", + return_value=start_time + datetime.timedelta(minutes=5), + ): + async with lock: + await apply() + assert switch.manager.auto_reset_manual_control_timers[ENTITY_LIGHT_1] is timer + assert timer.start_time == start_time + assert ( + switch.manager.get_manual_control_attributes(ENTITY_LIGHT_1) + == LightControlAttributes.BRIGHTNESS + ) + + # When applied, it restarts like any manual change. + await apply() + assert timer.start_time > start_time + assert ( + switch.manager.get_manual_control_attributes(ENTITY_LIGHT_1) + == LightControlAttributes.ALL + ) + + +async def test_apply_service_at_time_already_at_values(hass): + """A light that already has the values for the time is held there too.""" + switch, _ = await setup_lights_and_switch( + hass, + {CONF_SKIP_REDUNDANT_COMMANDS: True}, + ) + events = [] + hass.bus.async_listen(f"{DOMAIN}.manual_control", events.append) + + async def apply(): + await hass.services.async_call( + DOMAIN, + SERVICE_APPLY, + { + ATTR_ENTITY_ID: switch.entity_id, + CONF_LIGHTS: [ENTITY_LIGHT_1], + CONF_APPLY_TIME: "23:30:00", + }, + blocking=True, + ) + await hass.async_block_till_done() + + await apply() + switch.manager.reset(ENTITY_LIGHT_1) + calls = _track_adaptive_light_calls(hass) + await apply() # nothing left to send + assert calls == [] + assert ( + switch.manager.get_manual_control_attributes(ENTITY_LIGHT_1) + == LightControlAttributes.ALL + ) + assert len(events) == 2 + + async def test_apply_service_at_time_ignores_transition_reports(hass): """A report from during the transition doesn't count as a manual change.""" switch, _ = await setup_lights_and_switch(hass)