From 8e30aa13ce415006de447ab2a51cf1577cd84040 Mon Sep 17 00:00:00 2001 From: Bram Date: Sun, 31 Jul 2022 08:34:21 +0200 Subject: [PATCH 1/4] Better error handling in LUT flow --- custom_components/powercalc/config_flow.py | 8 +++++--- tests/test_config_flow.py | 8 ++++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/custom_components/powercalc/config_flow.py b/custom_components/powercalc/config_flow.py index 8690d45fc..3d11c79ee 100644 --- a/custom_components/powercalc/config_flow.py +++ b/custom_components/powercalc/config_flow.py @@ -344,9 +344,11 @@ async def async_step_lut(self, user_input: dict[str, str] = None) -> FlowResult: return await self.async_step_lut_manufacturer() - model_info = await autodiscover_model( - self.hass, self.source_entity.entity_entry - ) + model_info = None + if self.source_entity.entity_entry: + model_info = await autodiscover_model( + self.hass, self.source_entity.entity_entry + ) if model_info: return self.async_show_form( step_id="lut", diff --git a/tests/test_config_flow.py b/tests/test_config_flow.py index 9e4928ecb..28b6f4a7b 100644 --- a/tests/test_config_flow.py +++ b/tests/test_config_flow.py @@ -248,6 +248,14 @@ async def test_lut_autodiscover_flow(hass: HomeAssistant): assert hass.states.get("sensor.test_power") assert hass.states.get("sensor.test_energy") +async def test_lut_not_autodiscovered(hass: HomeAssistant): + light_entity = MockLight("test", STATE_ON) + light_entity._attr_unique_id = None + await create_mock_light_entity(hass, light_entity) + + result = await _goto_virtual_power_strategy_step(hass, CalculationStrategy.LUT) + assert result["type"] == data_entry_flow.FlowResultType.FORM + assert result["step_id"] == "lut_manufacturer" async def test_lut_autodiscover_flow_not_confirmed(hass: HomeAssistant): """ From 4cbac7eaf87d8a305f9d1f4d1ea00caf7481283d Mon Sep 17 00:00:00 2001 From: Bram Date: Sun, 31 Jul 2022 08:50:09 +0200 Subject: [PATCH 2/4] Fix error message for supported color mode --- custom_components/powercalc/strategy/lut.py | 14 +------------- tests/strategy/test_lut.py | 11 +++++++++++ 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/custom_components/powercalc/strategy/lut.py b/custom_components/powercalc/strategy/lut.py index be80044ad..258c951f3 100644 --- a/custom_components/powercalc/strategy/lut.py +++ b/custom_components/powercalc/strategy/lut.py @@ -232,18 +232,6 @@ async def validate_config(self): if self._source_entity.domain != light.DOMAIN: raise StrategyConfigurationError("Only light entities can use the LUT mode") - if self._model.manufacturer is None: - _LOGGER.error( - "Manufacturer not supplied for entity: %s", - self._source_entity.entity_id, - ) - - if self._model.model is None: - _LOGGER.error( - "Model not supplied for entity: %s", self._source_entity.entity_id - ) - return - for color_mode in self._source_entity.supported_color_modes: if color_mode in LUT_COLOR_MODES: try: @@ -251,7 +239,7 @@ async def validate_config(self): self._model, color_mode ) except LutFileNotFound: - raise ModelNotSupported("No lookup file found for mode", color_mode) + raise ModelNotSupported(f"No lookup file found for mode: {color_mode}") @dataclass diff --git a/tests/strategy/test_lut.py b/tests/strategy/test_lut.py index d948fa590..f74b063e0 100644 --- a/tests/strategy/test_lut.py +++ b/tests/strategy/test_lut.py @@ -115,6 +115,17 @@ async def test_validation_fails_for_non_light_entities(hass: HomeAssistant): ) await strategy.validate_config() +async def test_validation_fails_unsupported_color_mode(hass: HomeAssistant): + with pytest.raises(StrategyConfigurationError): + source_entity = create_source_entity("light", [ColorMode.COLOR_TEMP]) + strategy_factory = PowerCalculatorStrategyFactory(hass) + strategy = strategy_factory.create( + config={}, + strategy=CalculationStrategy.LUT, + light_model=LightModel(hass, "signify", "LWA017", None), # This model only supports brightness + source_entity=source_entity, + ) + await strategy.validate_config() def _create_lut_strategy( hass: HomeAssistant, From 36067477e3bd30d6f0dc4957054df033d64a9754 Mon Sep 17 00:00:00 2001 From: Bram Date: Sun, 31 Jul 2022 09:18:07 +0200 Subject: [PATCH 3/4] Improve error handling in config flow --- custom_components/powercalc/config_flow.py | 14 +++++++++++--- custom_components/powercalc/errors.py | 2 +- custom_components/powercalc/strategy/lut.py | 4 ++-- custom_components/powercalc/strings.json | 8 ++++++-- custom_components/powercalc/translations/en.json | 8 ++++++-- 5 files changed, 26 insertions(+), 10 deletions(-) diff --git a/custom_components/powercalc/config_flow.py b/custom_components/powercalc/config_flow.py index 3d11c79ee..6b7e1a150 100644 --- a/custom_components/powercalc/config_flow.py +++ b/custom_components/powercalc/config_flow.py @@ -5,6 +5,7 @@ import copy import logging from typing import Any +from black import err import voluptuous as vol from homeassistant import config_entries @@ -382,16 +383,19 @@ async def async_step_lut_manufacturer( async def async_step_lut_model( self, user_input: dict[str, str] = None ) -> FlowResult: + errors = {} if user_input is not None: self.sensor_config.update({CONF_MODEL: user_input.get(CONF_MODEL)}) - return self.create_config_entry() + errors = await self.validate_strategy_config() + if not errors: + return self.create_config_entry() return self.async_show_form( step_id="lut_model", data_schema=_create_lut_schema_model( self.hass, self.sensor_config.get(CONF_MANUFACTURER) ), - errors={}, + errors=errors, ) async def validate_strategy_config(self) -> dict: @@ -402,7 +406,11 @@ async def validate_strategy_config(self) -> dict: try: await strategy.validate_config() except StrategyConfigurationError as error: - return {"base": error.get_config_flow_translate_key()} + translation = error.get_config_flow_translate_key() + if translation is None: + translation = "unknown" + _LOGGER.error(str(error)) + return {"base": translation} return {} @callback diff --git a/custom_components/powercalc/errors.py b/custom_components/powercalc/errors.py index 2eb50bb8a..48cde5d35 100644 --- a/custom_components/powercalc/errors.py +++ b/custom_components/powercalc/errors.py @@ -35,7 +35,7 @@ def __init__(self, message: str, config_flow_trans_key: str = None): super().__init__(message) self._config_flow_trans_key = config_flow_trans_key - def get_config_flow_translate_key(self) -> str: + def get_config_flow_translate_key(self) -> str | None: return self._config_flow_trans_key diff --git a/custom_components/powercalc/strategy/lut.py b/custom_components/powercalc/strategy/lut.py index 258c951f3..ef677cd88 100644 --- a/custom_components/powercalc/strategy/lut.py +++ b/custom_components/powercalc/strategy/lut.py @@ -230,7 +230,7 @@ def get_nearest_higher_brightness(self, dict: dict, search_key: int) -> int: async def validate_config(self): if self._source_entity.domain != light.DOMAIN: - raise StrategyConfigurationError("Only light entities can use the LUT mode") + raise StrategyConfigurationError("Only light entities can use the LUT mode", "lut_unsupported_color_mode") for color_mode in self._source_entity.supported_color_modes: if color_mode in LUT_COLOR_MODES: @@ -239,7 +239,7 @@ async def validate_config(self): self._model, color_mode ) except LutFileNotFound: - raise ModelNotSupported(f"No lookup file found for mode: {color_mode}") + raise ModelNotSupported(f"No lookup file found for mode: {color_mode}", "lut_unsupported_color_mode") @dataclass diff --git a/custom_components/powercalc/strings.json b/custom_components/powercalc/strings.json index 1fcf4c668..c43b48ebe 100644 --- a/custom_components/powercalc/strings.json +++ b/custom_components/powercalc/strings.json @@ -129,7 +129,10 @@ "daily_energy_mandatory": "You must supply at least one of Value or Value template", "fixed_mandatory": "You must supply at least one of Power, Power template or States power", "fixed_states_power_only": "This entity can only work with 'states_power' not 'power'", - "group_mandatory": "You must define at least subgroups or power and energy-entities" + "group_mandatory": "You must define at least subgroups or power and energy-entities", + "lut_unsupported_color_mode": "The LUT profile does not support one of the color modes of your light. See the logs for more info", + "lut_wrong_domain": "Only light entities can use the LUT mode", + "unknown": "Unknown error occured, please see the logs for additional information" } }, "options": { @@ -172,7 +175,8 @@ "linear_min_higher_as_max": "[%key:component::powercalc::config::error::linear_min_higher_as_max%]", "fixed_mandatory": "[%key:component::powercalc::config::error::fixed_mandatory%]", "fixed_states_power_only": "[%key:component::powercalc::config::error::fixed_states_power_only%]", - "group_mandatory": "[%key:component::powercalc::config::error::group_mandatory%]" + "group_mandatory": "[%key:component::powercalc::config::error::group_mandatory%]", + "unknown": "[%key:component::powercalc::config::error::unknown%]" } } } diff --git a/custom_components/powercalc/translations/en.json b/custom_components/powercalc/translations/en.json index c325580f7..9e963b9dd 100644 --- a/custom_components/powercalc/translations/en.json +++ b/custom_components/powercalc/translations/en.json @@ -9,7 +9,10 @@ "fixed_states_power_only": "This entity can only work with 'states_power' not 'power'", "group_mandatory": "You must define at least subgroups or power and energy-entities", "linear_mandatory": "You must supply at least one of max_power or calibrate", - "linear_min_higher_as_max": "Max power cannot be lower than min power" + "linear_min_higher_as_max": "Max power cannot be lower than min power", + "lut_unsupported_color_mode": "The LUT profile does not support one of the color modes of your light. See the logs for more info", + "lut_wrong_domain": "Only light entities can use the LUT mode", + "unknown": "Unknown error occured, please see the logs for additional information" }, "step": { "daily_energy": { @@ -138,7 +141,8 @@ "fixed_states_power_only": "This entity can only work with 'states_power' not 'power'", "group_mandatory": "You must define at least subgroups or power and energy-entities", "linear_mandatory": "You must supply at least one of max_power or calibrate", - "linear_min_higher_as_max": "Max power cannot be lower than min power" + "linear_min_higher_as_max": "Max power cannot be lower than min power", + "unknown": "Unknown error occured, please see the logs for additional information" }, "step": { "init": { From d71d616bb7481fcc3ca222b1927c8c934acb685f Mon Sep 17 00:00:00 2001 From: Bram Date: Sun, 31 Jul 2022 09:19:40 +0200 Subject: [PATCH 4/4] Remove unused import --- custom_components/powercalc/config_flow.py | 1 - 1 file changed, 1 deletion(-) diff --git a/custom_components/powercalc/config_flow.py b/custom_components/powercalc/config_flow.py index 6b7e1a150..8d57bdba7 100644 --- a/custom_components/powercalc/config_flow.py +++ b/custom_components/powercalc/config_flow.py @@ -5,7 +5,6 @@ import copy import logging from typing import Any -from black import err import voluptuous as vol from homeassistant import config_entries