Retroactive review changes for #443 (#485)

* Retroactive review changes for #443

* Move up __init__

* Add test for test_change_switch_settings_service

* Add docs

* Add test for defaults

* Remove unused function

* Add skip var

* move pylint marker

* Refactor validate

* Update README.md

---------

Co-authored-by: Benjamin Auquite <halomastar@gmail.com>

README.md

@@ -110,7 +110,7 @@ adaptive_lighting:
 
 ### Services
 
-`adaptive_lighting.**apply**` applies Adaptive Lighting settings to lights on demand.
+`adaptive_lighting.apply` applies Adaptive Lighting settings to lights on demand.
 
 | Service data attribute | Optional | Description                                                                                  |
 | ---------------------- | -------- | -------------------------------------------------------------------------------------------- |
@@ -132,16 +132,23 @@ adaptive_lighting:
 
 `adaptive_lighting.change_switch_settings` (new in 1.7.0) Change any of the above configuration options of Adaptive Lighting (such as `sunrise_time` or `prefer_rgb_color`) with a service call directly from your script/automation.
 
-| DISALLOWED service data   | Description                                                                                         |
-| ------------------------- | --------------------------------------------------------------------------------------------------- |
-| `entity_id`               | You cannot change the switch's unique_id, it's already been registered                              |
-| `lights`                  | See above, you may call adaptive_lighting.apply() with your lights or create a new config instead   |
-| `name`                    | See above. You can already rename your switch's display name in Home Assistant's UI.                |
-| `interval`                | Nope. The interval is only used once when the config loads. A config change and restart is required |
+| Service data attribute                            | Description                                                                                       |
+| ------------------------------------------------- | ------------------------------------------------------------------------------------------------- |
+| `use_defaults`                                    | (default: 'current' for current settings) You can set this to 'factory', 'configuration', or 'current' to reset the variables not being set with this service call. 'current' leaves them as is, 'configuration' resets to whatever already initializes at startup, 'factory' resets to the default values listed in the documentation.                        |
+| all other keys except the ones in the table below | See above, you may call `adaptive_lighting.apply` with your lights or create a new config instead |
+
+
+| **DISALLOWED** service data | Description                                                                                       |
+| --------------------------- | ------------------------------------------------------------------------------------------------- |
+| `entity_id`                 | You cannot change the switch's `entity_id`, it's already been registered                          |
+| `lights`                    | See above, you may call `adaptive_lighting.apply` with your lights or create a new config instead |
+| `name`                      | See above. You can already rename your switch's display name in Home Assistant's UI.              |
+| `interval`                  | The interval is only used once when the config loads. A config change and restart is required     |
 
 ## Automation examples
 
 Reset the `manual_control` status of a light after an hour.
+
 ```yaml
 - alias: "Adaptive lighting: reset manual_control after 1 hour"
   mode: parallel
@@ -186,42 +193,46 @@ Set your sunrise and sunset time based on your alarm. The below script sets suns
 iphone_carly_wakeup:
   alias: iPhone Carly Wakeup
   sequence:
-  - condition: state
-    entity_id: input_boolean.carly_iphone_wakeup
-    state: 'off'
-  - service: input_datetime.set_datetime
-    target:
-      entity_id: input_datetime.carly_iphone_wakeup
-    data:
-      time: '{{ now().strftime("%H:%M:%S") }}'
-  - service: input_boolean.turn_on
-    target:
+    - condition: state
       entity_id: input_boolean.carly_iphone_wakeup
-  - repeat:
-      count: '{{ (states.switch    | map(attribute=''entity_id'')    | select(">","switch.adaptive_lighting_al_")    |
-        select("<", "switch.adaptive_lighting_al_z")    | join(",")).split(",")|length
-        }}'
-      sequence:
-      - service: adaptive_lighting.change_switch_settings
-        data:
-          entity_id: switch.adaptive_lighting_al_den_ceilingfan_lights
-          sunrise_time: '{{ now().strftime("%H:%M:%S") }}'
-          sunset_time: '{{ (as_timestamp(now()) + 12*60*60) | timestamp_custom("%H:%M:%S")
-            }}'
-  - service: script.turn_on
-    target:
-      entity_id: script.run_wakeup_routine
-  - service: input_boolean.turn_off
-    target:
-      entity_id:
-      - input_boolean.carly_iphone_winddown
-      - input_boolean.carly_iphone_bedtime
-  - service: input_datetime.set_datetime
-    target:
-      entity_id: input_datetime.wakeup_time
-    data:
-      time: '{{ now().strftime("%H:%M:%S") }}'
-  - service: script.adaptive_lighting_disable_sleep_mode
+      state: "off"
+    - service: input_datetime.set_datetime
+      target:
+        entity_id: input_datetime.carly_iphone_wakeup
+      data:
+        time: '{{ now().strftime("%H:%M:%S") }}'
+    - service: input_boolean.turn_on
+      target:
+        entity_id: input_boolean.carly_iphone_wakeup
+    - repeat:
+        count: >
+          {{ (states.switch
+              | map(attribute="entity_id")
+              | select(">","switch.adaptive_lighting_al_")
+              | select("<", "switch.adaptive_lighting_al_z")
+              | join(",")
+             ).split(",") | length }}
+        sequence:
+          - service: adaptive_lighting.change_switch_settings
+            data:
+              entity_id: switch.adaptive_lighting_al_den_ceilingfan_lights
+              sunrise_time: '{{ now().strftime("%H:%M:%S") }}'
+              sunset_time: >
+                {{ (as_timestamp(now()) + 12*60*60) | timestamp_custom("%H:%M:%S") }}
+    - service: script.turn_on
+      target:
+        entity_id: script.run_wakeup_routine
+    - service: input_boolean.turn_off
+      target:
+        entity_id:
+          - input_boolean.carly_iphone_winddown
+          - input_boolean.carly_iphone_bedtime
+    - service: input_datetime.set_datetime
+      target:
+        entity_id: input_datetime.wakeup_time
+      data:
+        time: '{{ now().strftime("%H:%M:%S") }}'
+    - service: script.adaptive_lighting_disable_sleep_mode
   mode: queued
   icon: mdi:weather-sunset
   max: 10

custom_components/adaptive_lighting/__init__.py

@@ -41,9 +41,6 @@ async def async_setup(hass: HomeAssistant, config: dict[str, Any]):
     # This will reload any changes the user made to any YAML configurations.
     await async_setup_reload_service(hass, DOMAIN, PLATFORMS)
 
-    # quickly populate data for change_switch_settings before actually loading integration.
-    # update_services_yaml()
-
     if DOMAIN in config:
         for entry in config[DOMAIN]:
             hass.async_create_task(

custom_components/adaptive_lighting/manifest.json

@@ -8,5 +8,5 @@
   "iot_class": "calculated",
   "issue_tracker": "https://github.com/basnijholt/adaptive-lighting/issues",
   "requirements": [],
-  "version": "1.6.0"
+  "version": "1.7.0"
 }

custom_components/adaptive_lighting/switch.py

@@ -189,11 +189,6 @@ def _int_to_bytes(i: int, signed: bool = False) -> bytes:
     return i.to_bytes((bits + 7) // 8, "little", signed=signed)
 
 
-def int_between(min_int, max_int):
-    """Return an integer between 'min_int' and 'max_int'."""
-    return vol.All(vol.Coerce(int), vol.Range(min=min_int, max=max_int))
-
-
 def _short_hash(string: str, length: int = 4) -> str:
     """Create a hash of 'string' with length 'length'."""
     str_hash_bytes = _int_to_bytes(hash(string), signed=True)
@@ -365,12 +360,10 @@ async def handle_change_switch_settings(
     which = data.get(CONF_USE_DEFAULTS, "current")
     if which == "current":  # use whatever we're already using.
         defaults = switch._current_settings  # pylint: disable=protected-access
-    # not needed since validate() does this part for us
     elif which == "factory":  # use actual defaults listed in the documentation
         defaults = {key: default for key, default, _ in VALIDATION_TUPLES}
-    elif (
-        which == "configuration"
-    ):  # use whatever's in the config flow or configuration.yaml
+    elif which == "configuration":
+        # use whatever's in the config flow or configuration.yaml
         defaults = switch._config_backup  # pylint: disable=protected-access
     else:
         defaults = None
@@ -387,15 +380,13 @@ async def handle_change_switch_settings(
 
     all_lights = switch._lights  # pylint: disable=protected-access
     switch.turn_on_off_listener.reset(*all_lights, reset_manual_control=False)
-    # pylint: disable=protected-access
     if switch.is_on:
-        await switch._update_attrs_and_maybe_adapt_lights(
+        await switch._update_attrs_and_maybe_adapt_lights(  # pylint: disable=protected-access
             all_lights,
             transition=switch._initial_transition,
             force=True,
             context=switch.create_context("service", parent=service_call.context),
         )
-    # pylint: enable=protected-access
 
 
 @callback
@@ -509,8 +500,8 @@ async def async_setup_entry(
                     _fire_manual_control_event(this_switch, light, service_call.context)
             else:
                 this_switch.turn_on_off_listener.reset(*all_lights)
-                # pylint: disable=protected-access
                 if this_switch.is_on:
+                    # pylint: disable=protected-access
                     await this_switch._update_attrs_and_maybe_adapt_lights(
                         all_lights,
                         transition=this_switch._initial_transition,
@@ -556,9 +547,10 @@ async def async_setup_entry(
     )
 
     args = {vol.Optional(CONF_USE_DEFAULTS, default="current"): cv.string}
+    # Modifying these after init isn't possible
+    skip = (CONF_INTERVAL, CONF_NAME, CONF_LIGHTS)
     for k, _, valid in VALIDATION_TUPLES:
-        # Modifying these after initialization isn't possible (yet)
-        if k != CONF_INTERVAL and k != CONF_NAME and k != CONF_LIGHTS:
+        if k not in skip:
             args[vol.Optional(k)] = valid
     platform = entity_platform.current_platform.get()
     platform.async_register_entity_service(
@@ -568,24 +560,24 @@ async def async_setup_entry(
     )
 
 
-def validate(config_entry: ConfigEntry, **kwargs):
+def validate(
+    config_entry: ConfigEntry,
+    service_data: dict[str, Any] | None = None,
+    defaults: dict[str, Any] | None = None,
+):
     """Get the options and data from the config_entry and add defaults."""
-    # defaults and data will exist only if this is called from change_switch_settings
-    defaults = kwargs.get("defaults")
-    service_data = kwargs.get("data")
     if defaults is None:
-        defaults = {key: default for key, default, _ in VALIDATION_TUPLES}
+        data = {key: default for key, default, _ in VALIDATION_TUPLES}
+    else:
+        data = defaults
+
     if config_entry is not None:
-        # Is this deepcopy necessary?
-        # We're already creating a new array for the defaults variable...
-        # afterwards defaults is never used again.
-        data = deepcopy(defaults)
+        assert service_data is None
+        assert defaults is None
         data.update(config_entry.options)  # come from options flow
         data.update(config_entry.data)  # all yaml settings come from data
     else:
-        # no idea how to clear original data from memory
-        # hopefully it does it automatically, otherwise TODO.
-        data = deepcopy(defaults)
+        assert service_data is not None
         data.update(service_data)
     data = {key: replace_none_str(value) for key, value in data.items()}
     for key, (validate_value, _) in EXTRA_VALIDATION.items():
@@ -756,67 +748,6 @@ def _attributes_have_changed(
 class AdaptiveSwitch(SwitchEntity, RestoreEntity):
     """Representation of a Adaptive Lighting switch."""
 
-    def _set_changeable_settings(
-        self,
-        data: dict,
-        defaults: dict,
-    ):
-        # Should only contain the settings we want the users to be able to change during runtime.
-        data = validate(
-            config_entry=None,
-            data=data,
-            defaults=defaults,
-        )
-
-        # backup data for use in change_switch_settings "current" CONF_USE_DEFAULTS
-        self._current_settings = data
-
-        self._detect_non_ha_changes = data[CONF_DETECT_NON_HA_CHANGES]
-        self._include_config_in_attributes = data[CONF_INCLUDE_CONFIG_IN_ATTRIBUTES]
-        self._initial_transition = data[CONF_INITIAL_TRANSITION]
-        self._sleep_transition = data[CONF_SLEEP_TRANSITION]
-        self._only_once = data[CONF_ONLY_ONCE]
-        self._prefer_rgb_color = data[CONF_PREFER_RGB_COLOR]
-        self._separate_turn_on_commands = data[CONF_SEPARATE_TURN_ON_COMMANDS]
-        self._take_over_control = data[CONF_TAKE_OVER_CONTROL]
-        self._transition = data[CONF_TRANSITION]
-        self._adapt_delay = data[CONF_ADAPT_DELAY]
-        self._send_split_delay = data[CONF_SEND_SPLIT_DELAY]
-        _loc = get_astral_location(self.hass)
-        if isinstance(_loc, tuple):
-            # Astral v2.2
-            location, _ = _loc
-        else:
-            # Astral v1
-            location = _loc
-
-        self._sun_light_settings = SunLightSettings(
-            name=self._name,
-            astral_location=location,
-            max_brightness=data[CONF_MAX_BRIGHTNESS],
-            max_color_temp=data[CONF_MAX_COLOR_TEMP],
-            min_brightness=data[CONF_MIN_BRIGHTNESS],
-            min_color_temp=data[CONF_MIN_COLOR_TEMP],
-            sleep_brightness=data[CONF_SLEEP_BRIGHTNESS],
-            sleep_color_temp=data[CONF_SLEEP_COLOR_TEMP],
-            sleep_rgb_color=data[CONF_SLEEP_RGB_COLOR],
-            sleep_rgb_or_color_temp=data[CONF_SLEEP_RGB_OR_COLOR_TEMP],
-            sunrise_offset=data[CONF_SUNRISE_OFFSET],
-            sunrise_time=data[CONF_SUNRISE_TIME],
-            max_sunrise_time=data[CONF_MAX_SUNRISE_TIME],
-            sunset_offset=data[CONF_SUNSET_OFFSET],
-            sunset_time=data[CONF_SUNSET_TIME],
-            min_sunset_time=data[CONF_MIN_SUNSET_TIME],
-            time_zone=self.hass.config.time_zone,
-            transition=data[CONF_TRANSITION],
-        )
-        _LOGGER.debug(
-            "%s: Set switch settings for lights '%s'. now using data: '%s'",
-            self._name,
-            self._lights,
-            data,
-        )
-
     def __init__(
         self,
         hass,
@@ -827,8 +758,7 @@ class AdaptiveSwitch(SwitchEntity, RestoreEntity):
         adapt_brightness_switch: SimpleSwitch,
     ):
         """Initialize the Adaptive Lighting switch."""
-        # Set attributes we DON'T want users modifying
-        # during runtime here.
+        # Set attributes that can't be modified during runtime
         self.hass = hass
         self.turn_on_off_listener = turn_on_off_listener
         self.sleep_mode_switch = sleep_mode_switch
@@ -887,6 +817,67 @@ class AdaptiveSwitch(SwitchEntity, RestoreEntity):
             data,
         )
 
+    def _set_changeable_settings(
+        self,
+        data: dict,
+        defaults: dict,
+    ):
+        # Only pass settings users can change during runtime
+        data = validate(
+            config_entry=None,
+            service_data=data,
+            defaults=defaults,
+        )
+
+        # backup data for use in change_switch_settings "current" CONF_USE_DEFAULTS
+        self._current_settings = data
+
+        self._detect_non_ha_changes = data[CONF_DETECT_NON_HA_CHANGES]
+        self._include_config_in_attributes = data[CONF_INCLUDE_CONFIG_IN_ATTRIBUTES]
+        self._initial_transition = data[CONF_INITIAL_TRANSITION]
+        self._sleep_transition = data[CONF_SLEEP_TRANSITION]
+        self._only_once = data[CONF_ONLY_ONCE]
+        self._prefer_rgb_color = data[CONF_PREFER_RGB_COLOR]
+        self._separate_turn_on_commands = data[CONF_SEPARATE_TURN_ON_COMMANDS]
+        self._take_over_control = data[CONF_TAKE_OVER_CONTROL]
+        self._transition = data[CONF_TRANSITION]
+        self._adapt_delay = data[CONF_ADAPT_DELAY]
+        self._send_split_delay = data[CONF_SEND_SPLIT_DELAY]
+        _loc = get_astral_location(self.hass)
+        if isinstance(_loc, tuple):
+            # Astral v2.2
+            location, _ = _loc
+        else:
+            # Astral v1
+            location = _loc
+
+        self._sun_light_settings = SunLightSettings(
+            name=self._name,
+            astral_location=location,
+            max_brightness=data[CONF_MAX_BRIGHTNESS],
+            max_color_temp=data[CONF_MAX_COLOR_TEMP],
+            min_brightness=data[CONF_MIN_BRIGHTNESS],
+            min_color_temp=data[CONF_MIN_COLOR_TEMP],
+            sleep_brightness=data[CONF_SLEEP_BRIGHTNESS],
+            sleep_color_temp=data[CONF_SLEEP_COLOR_TEMP],
+            sleep_rgb_color=data[CONF_SLEEP_RGB_COLOR],
+            sleep_rgb_or_color_temp=data[CONF_SLEEP_RGB_OR_COLOR_TEMP],
+            sunrise_offset=data[CONF_SUNRISE_OFFSET],
+            sunrise_time=data[CONF_SUNRISE_TIME],
+            max_sunrise_time=data[CONF_MAX_SUNRISE_TIME],
+            sunset_offset=data[CONF_SUNSET_OFFSET],
+            sunset_time=data[CONF_SUNSET_TIME],
+            min_sunset_time=data[CONF_MIN_SUNSET_TIME],
+            time_zone=self.hass.config.time_zone,
+            transition=data[CONF_TRANSITION],
+        )
+        _LOGGER.debug(
+            "%s: Set switch settings for lights '%s'. now using data: '%s'",
+            self._name,
+            self._lights,
+            data,
+        )
+
     @property
     def name(self):
         """Return the name of the device if any."""

tests/test_switch.py

@@ -13,6 +13,7 @@ from homeassistant.components.adaptive_lighting.const import (
     CONF_DETECT_NON_HA_CHANGES,
     CONF_INITIAL_TRANSITION,
     CONF_MANUAL_CONTROL,
+    CONF_MAX_BRIGHTNESS,
     CONF_MIN_COLOR_TEMP,
     CONF_PREFER_RGB_COLOR,
     CONF_SEPARATE_TURN_ON_COMMANDS,
@@ -21,12 +22,14 @@ from homeassistant.components.adaptive_lighting.const import (
     CONF_SUNSET_TIME,
     CONF_TRANSITION,
     CONF_TURN_ON_LIGHTS,
+    CONF_USE_DEFAULTS,
     DEFAULT_MAX_BRIGHTNESS,
     DEFAULT_NAME,
     DEFAULT_SLEEP_BRIGHTNESS,
     DEFAULT_SLEEP_COLOR_TEMP,
     DOMAIN,
     SERVICE_APPLY,
+    SERVICE_CHANGE_SWITCH_SETTINGS,
     SERVICE_SET_MANUAL_CONTROL,
     SLEEP_MODE_SWITCH,
     UNDO_UPDATE_LISTENER,
@@ -65,6 +68,7 @@ from homeassistant.setup import async_setup_component
 from homeassistant.util.color import color_temperature_mired_to_kelvin
 import homeassistant.util.dt as dt_util
 import pytest
+import voluptuous.error
 
 from tests.common import MockConfigEntry, mock_area_registry
 from tests.components.demo.test_light import ENTITY_LIGHT
@@ -928,3 +932,55 @@ async def test_area(hass):
         switch.turn_on_off_listener.last_service_data,
     )
     assert light.entity_id not in switch.turn_on_off_listener.last_service_data
+
+
+async def test_change_switch_settings_service(hass):
+    """Test adaptive_lighting.change_switch_settings service."""
+    switch, (_, _, light) = await setup_lights_and_switch(hass)
+    entity_id = light.entity_id
+    assert entity_id not in switch._lights
+
+    async def change_switch_settings(**kwargs):
+        await hass.services.async_call(
+            DOMAIN,
+            SERVICE_CHANGE_SWITCH_SETTINGS,
+            {
+                ATTR_ENTITY_ID: ENTITY_SWITCH,
+                **kwargs,
+            },
+            blocking=True,
+        )
+        await hass.async_block_till_done()
+
+    # Test changing sunrise offset
+    assert switch._sun_light_settings.sunrise_offset.total_seconds() == 0
+    await change_switch_settings(**{CONF_SUNRISE_OFFSET: 10})
+    assert switch._sun_light_settings.sunrise_offset.total_seconds() == 10
+
+    # Test changing max brightness
+    assert switch._sun_light_settings.max_brightness == 100
+    await change_switch_settings(**{CONF_MAX_BRIGHTNESS: 50})
+    assert switch._sun_light_settings.max_brightness == 50
+
+    # Test changing to illegal max brightness
+    with pytest.raises(
+        voluptuous.error.MultipleInvalid,
+        match="value must be at most 100 for dictionary",
+    ):
+        await change_switch_settings(**{CONF_MAX_BRIGHTNESS: 5000})
+
+    # Change CONF_MIN_COLOR_TEMP, the factory default is 2000, but setup_lights_and_switch
+    # sets it to 2500
+    assert switch._sun_light_settings.min_color_temp == 2500
+
+    # testing with "factory" should change it to 2000
+    await change_switch_settings(**{CONF_USE_DEFAULTS: "factory"})
+    assert switch._sun_light_settings.min_color_temp == 2000
+
+    # testing with "current" should not change things
+    await change_switch_settings(**{CONF_USE_DEFAULTS: "current"})
+    assert switch._sun_light_settings.min_color_temp == 2000
+
+    # testing with "configuration" should revert back to 2500
+    await change_switch_settings(**{CONF_USE_DEFAULTS: "configuration"})
+    assert switch._sun_light_settings.min_color_temp == 2500