Skip to content

Commit

Permalink
Use ServiceValidationError for invalid fan preset_mode and move check…
Browse files Browse the repository at this point in the history
… to fan entity component (#104560)

* Use ServiceValidationError for fan preset_mode

* Use _valid_preset_mode_or_raise to raise

* Move preset_mode validation to entity component

* Fix bond fan and comments

* Fixes baf, fjaraskupan and template

* More integration adjustments

* Add custom components mock and test code

* Make NotValidPresetModeError subclass

* Update homeassistant/components/fan/strings.json

Co-authored-by: Martin Hjelmare <marhje52@gmail.com>

* Keep bond has_action validation

* Move demo test asserts outside context block

* Follow up comment

* Update homeassistant/components/fan/strings.json

Co-authored-by: G Johansson <goran.johansson@shiftit.se>

* Fix demo tests

* Remove pylint disable

* Remove unreachable code

* Update homeassistant/components/fan/__init__.py

Co-authored-by: G Johansson <goran.johansson@shiftit.se>

* Use NotValidPresetModeError, Final methods

* Address comments

* Correct docst

* Follow up comments

* Update homeassistant/components/fan/__init__.py

Co-authored-by: Erik Montnemery <erik@montnemery.com>

---------

Co-authored-by: Martin Hjelmare <marhje52@gmail.com>
Co-authored-by: G Johansson <goran.johansson@shiftit.se>
Co-authored-by: Erik Montnemery <erik@montnemery.com>
  • Loading branch information
4 people committed Nov 29, 2023
1 parent 49381ce commit 953a212
Show file tree
Hide file tree
Showing 22 changed files with 260 additions and 118 deletions.
2 changes: 0 additions & 2 deletions homeassistant/components/baf/fan.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,6 @@ async def async_turn_off(self, **kwargs: Any) -> None:

async def async_set_preset_mode(self, preset_mode: str) -> None:
"""Set the preset mode of the fan."""
if preset_mode != PRESET_MODE_AUTO:
raise ValueError(f"Invalid preset mode: {preset_mode}")
self._device.fan_mode = OffOnAuto.AUTO

async def async_set_direction(self, direction: str) -> None:
Expand Down
4 changes: 0 additions & 4 deletions homeassistant/components/bond/fan.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,10 +199,6 @@ async def async_turn_on(

async def async_set_preset_mode(self, preset_mode: str) -> None:
"""Set the preset mode of the fan."""
if preset_mode != PRESET_MODE_BREEZE or not self._device.has_action(
Action.BREEZE_ON
):
raise ValueError(f"Invalid preset mode: {preset_mode}")
await self._hub.bond.action(self._device.device_id, Action(Action.BREEZE_ON))

async def async_turn_off(self, **kwargs: Any) -> None:
Expand Down
13 changes: 3 additions & 10 deletions homeassistant/components/demo/fan.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,12 +161,9 @@ def preset_modes(self) -> list[str] | None:

def set_preset_mode(self, preset_mode: str) -> None:
"""Set new preset mode."""
if self.preset_modes and preset_mode in self.preset_modes:
self._preset_mode = preset_mode
self._percentage = None
self.schedule_update_ha_state()
else:
raise ValueError(f"Invalid preset mode: {preset_mode}")
self._preset_mode = preset_mode
self._percentage = None
self.schedule_update_ha_state()

def turn_on(
self,
Expand Down Expand Up @@ -230,10 +227,6 @@ def preset_modes(self) -> list[str] | None:

async def async_set_preset_mode(self, preset_mode: str) -> None:
"""Set new preset mode."""
if self.preset_modes is None or preset_mode not in self.preset_modes:
raise ValueError(
f"{preset_mode} is not a valid preset_mode: {self.preset_modes}"
)
self._preset_mode = preset_mode
self._percentage = None
self.async_write_ha_state()
Expand Down
49 changes: 43 additions & 6 deletions homeassistant/components/fan/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@
SERVICE_TURN_ON,
STATE_ON,
)
from homeassistant.core import HomeAssistant
from homeassistant.core import HomeAssistant, callback
from homeassistant.exceptions import ServiceValidationError
import homeassistant.helpers.config_validation as cv
from homeassistant.helpers.config_validation import ( # noqa: F401
PLATFORM_SCHEMA,
Expand Down Expand Up @@ -77,8 +78,19 @@ class FanEntityFeature(IntFlag):
# mypy: disallow-any-generics


class NotValidPresetModeError(ValueError):
"""Exception class when the preset_mode in not in the preset_modes list."""
class NotValidPresetModeError(ServiceValidationError):
"""Raised when the preset_mode is not in the preset_modes list."""

def __init__(
self, *args: object, translation_placeholders: dict[str, str] | None = None
) -> None:
"""Initialize the exception."""
super().__init__(
*args,
translation_domain=DOMAIN,
translation_key="not_valid_preset_mode",
translation_placeholders=translation_placeholders,
)


@bind_hass
Expand Down Expand Up @@ -107,7 +119,7 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool:
),
vol.Optional(ATTR_PRESET_MODE): cv.string,
},
"async_turn_on",
"async_handle_turn_on_service",
)
component.async_register_entity_service(SERVICE_TURN_OFF, {}, "async_turn_off")
component.async_register_entity_service(SERVICE_TOGGLE, {}, "async_toggle")
Expand Down Expand Up @@ -156,7 +168,7 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool:
component.async_register_entity_service(
SERVICE_SET_PRESET_MODE,
{vol.Required(ATTR_PRESET_MODE): cv.string},
"async_set_preset_mode",
"async_handle_set_preset_mode_service",
[FanEntityFeature.SET_SPEED, FanEntityFeature.PRESET_MODE],
)

Expand Down Expand Up @@ -237,17 +249,30 @@ def set_preset_mode(self, preset_mode: str) -> None:
"""Set new preset mode."""
raise NotImplementedError()

@final
async def async_handle_set_preset_mode_service(self, preset_mode: str) -> None:
"""Validate and set new preset mode."""
self._valid_preset_mode_or_raise(preset_mode)
await self.async_set_preset_mode(preset_mode)

async def async_set_preset_mode(self, preset_mode: str) -> None:
"""Set new preset mode."""
await self.hass.async_add_executor_job(self.set_preset_mode, preset_mode)

@final
@callback
def _valid_preset_mode_or_raise(self, preset_mode: str) -> None:
"""Raise NotValidPresetModeError on invalid preset_mode."""
preset_modes = self.preset_modes
if not preset_modes or preset_mode not in preset_modes:
preset_modes_str: str = ", ".join(preset_modes or [])
raise NotValidPresetModeError(
f"The preset_mode {preset_mode} is not a valid preset_mode:"
f" {preset_modes}"
f" {preset_modes}",
translation_placeholders={
"preset_mode": preset_mode,
"preset_modes": preset_modes_str,
},
)

def set_direction(self, direction: str) -> None:
Expand All @@ -267,6 +292,18 @@ def turn_on(
"""Turn on the fan."""
raise NotImplementedError()

@final
async def async_handle_turn_on_service(
self,
percentage: int | None = None,
preset_mode: str | None = None,
**kwargs: Any,
) -> None:
"""Validate and turn on the fan."""
if preset_mode is not None:
self._valid_preset_mode_or_raise(preset_mode)
await self.async_turn_on(percentage, preset_mode, **kwargs)

async def async_turn_on(
self,
percentage: int | None = None,
Expand Down
5 changes: 5 additions & 0 deletions homeassistant/components/fan/strings.json
Original file line number Diff line number Diff line change
Expand Up @@ -144,5 +144,10 @@
"reverse": "Reverse"
}
}
},
"exceptions": {
"not_valid_preset_mode": {
"message": "Preset mode {preset_mode} is not valid, valid preset modes are: {preset_modes}."
}
}
}
8 changes: 3 additions & 5 deletions homeassistant/components/fjaraskupan/fan.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,9 @@ async def async_turn_on(

async def async_set_preset_mode(self, preset_mode: str) -> None:
"""Set new preset mode."""
if command := PRESET_TO_COMMAND.get(preset_mode):
async with self.coordinator.async_connect_and_update() as device:
await device.send_command(command)
else:
raise UnsupportedPreset(f"The preset {preset_mode} is unsupported")
command = PRESET_TO_COMMAND[preset_mode]
async with self.coordinator.async_connect_and_update() as device:
await device.send_command(command)

async def async_turn_off(self, **kwargs: Any) -> None:
"""Turn the entity off."""
Expand Down
2 changes: 0 additions & 2 deletions homeassistant/components/mqtt/fan.py
Original file line number Diff line number Diff line change
Expand Up @@ -553,8 +553,6 @@ async def async_set_preset_mode(self, preset_mode: str) -> None:
This method is a coroutine.
"""
self._valid_preset_mode_or_raise(preset_mode)

mqtt_payload = self._command_templates[ATTR_PRESET_MODE](preset_mode)

await self.async_publish(
Expand Down
9 changes: 0 additions & 9 deletions homeassistant/components/template/fan.py
Original file line number Diff line number Diff line change
Expand Up @@ -282,15 +282,6 @@ async def async_set_percentage(self, percentage: int) -> None:

async def async_set_preset_mode(self, preset_mode: str) -> None:
"""Set the preset_mode of the fan."""
if self.preset_modes and preset_mode not in self.preset_modes:
_LOGGER.error(
"Received invalid preset_mode: %s for entity %s. Expected: %s",
preset_mode,
self.entity_id,
self.preset_modes,
)
return

self._preset_mode = preset_mode

if self._set_preset_mode_script:
Expand Down
3 changes: 1 addition & 2 deletions homeassistant/components/tradfri/fan.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,7 @@ async def async_set_preset_mode(self, preset_mode: str) -> None:
if not self._device_control:
return

if not preset_mode == ATTR_AUTO:
raise ValueError("Preset must be 'Auto'.")
# Preset must be 'Auto'

await self._api(self._device_control.turn_on_auto_mode())

Expand Down
12 changes: 1 addition & 11 deletions homeassistant/components/vallox/fan.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,7 @@
ValloxInvalidInputException,
)

from homeassistant.components.fan import (
FanEntity,
FanEntityFeature,
NotValidPresetModeError,
)
from homeassistant.components.fan import FanEntity, FanEntityFeature
from homeassistant.config_entries import ConfigEntry
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import HomeAssistantError
Expand Down Expand Up @@ -200,12 +196,6 @@ async def _async_set_preset_mode_internal(self, preset_mode: str) -> bool:
Returns true if the mode has been changed, false otherwise.
"""
try:
self._valid_preset_mode_or_raise(preset_mode)

except NotValidPresetModeError as err:
raise ValueError(f"Not valid preset mode: {preset_mode}") from err

if preset_mode == self.preset_mode:
return False

Expand Down
22 changes: 0 additions & 22 deletions homeassistant/components/xiaomi_miio/fan.py
Original file line number Diff line number Diff line change
Expand Up @@ -530,9 +530,6 @@ async def async_set_preset_mode(self, preset_mode: str) -> None:
This method is a coroutine.
"""
if preset_mode not in self.preset_modes:
_LOGGER.warning("'%s'is not a valid preset mode", preset_mode)
return
if await self._try_command(
"Setting operation mode of the miio device failed.",
self._device.set_mode,
Expand Down Expand Up @@ -623,9 +620,6 @@ def operation_mode_class(self):

async def async_set_preset_mode(self, preset_mode: str) -> None:
"""Set the preset mode of the fan."""
if preset_mode not in self.preset_modes:
_LOGGER.warning("'%s'is not a valid preset mode", preset_mode)
return
if await self._try_command(
"Setting operation mode of the miio device failed.",
self._device.set_mode,
Expand Down Expand Up @@ -721,9 +715,6 @@ async def async_set_preset_mode(self, preset_mode: str) -> None:
This method is a coroutine.
"""
if preset_mode not in self.preset_modes:
_LOGGER.warning("'%s'is not a valid preset mode", preset_mode)
return
if await self._try_command(
"Setting operation mode of the miio device failed.",
self._device.set_mode,
Expand Down Expand Up @@ -809,9 +800,6 @@ async def async_set_percentage(self, percentage: int) -> None:

async def async_set_preset_mode(self, preset_mode: str) -> None:
"""Set the preset mode of the fan. This method is a coroutine."""
if preset_mode not in self.preset_modes:
_LOGGER.warning("'%s'is not a valid preset mode", preset_mode)
return
if await self._try_command(
"Setting operation mode of the miio device failed.",
self._device.set_mode,
Expand Down Expand Up @@ -958,10 +946,6 @@ def _handle_coordinator_update(self):

async def async_set_preset_mode(self, preset_mode: str) -> None:
"""Set the preset mode of the fan."""
if preset_mode not in self.preset_modes:
_LOGGER.warning("'%s'is not a valid preset mode", preset_mode)
return

if preset_mode == ATTR_MODE_NATURE:
await self._try_command(
"Setting natural fan speed percentage of the miio device failed.",
Expand Down Expand Up @@ -1034,9 +1018,6 @@ def _handle_coordinator_update(self):

async def async_set_preset_mode(self, preset_mode: str) -> None:
"""Set the preset mode of the fan."""
if preset_mode not in self.preset_modes:
_LOGGER.warning("'%s'is not a valid preset mode", preset_mode)
return
await self._try_command(
"Setting operation mode of the miio device failed.",
self._device.set_mode,
Expand Down Expand Up @@ -1093,9 +1074,6 @@ def _handle_coordinator_update(self):

async def async_set_preset_mode(self, preset_mode: str) -> None:
"""Set the preset mode of the fan."""
if preset_mode not in self.preset_modes:
_LOGGER.warning("'%s'is not a valid preset mode", preset_mode)
return
await self._try_command(
"Setting operation mode of the miio device failed.",
self._device.set_mode,
Expand Down
6 changes: 0 additions & 6 deletions homeassistant/components/zha/fan.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
ATTR_PRESET_MODE,
FanEntity,
FanEntityFeature,
NotValidPresetModeError,
)
from homeassistant.config_entries import ConfigEntry
from homeassistant.const import STATE_UNAVAILABLE, Platform
Expand Down Expand Up @@ -131,11 +130,6 @@ async def async_set_percentage(self, percentage: int) -> None:

async def async_set_preset_mode(self, preset_mode: str) -> None:
"""Set the preset mode for the fan."""
if preset_mode not in self.preset_modes:
raise NotValidPresetModeError(
f"The preset_mode {preset_mode} is not a valid preset_mode:"
f" {self.preset_modes}"
)
await self._async_set_fan_mode(self.preset_name_to_mode[preset_mode])

@abstractmethod
Expand Down
6 changes: 0 additions & 6 deletions homeassistant/components/zwave_js/fan.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
DOMAIN as FAN_DOMAIN,
FanEntity,
FanEntityFeature,
NotValidPresetModeError,
)
from homeassistant.config_entries import ConfigEntry
from homeassistant.core import HomeAssistant, callback
Expand Down Expand Up @@ -181,11 +180,6 @@ async def async_set_preset_mode(self, preset_mode: str) -> None:
await self._async_set_value(self._target_value, zwave_value)
return

raise NotValidPresetModeError(
f"The preset_mode {preset_mode} is not a valid preset_mode:"
f" {self.preset_modes}"
)

@property
def available(self) -> bool:
"""Return whether the entity is available."""
Expand Down
9 changes: 7 additions & 2 deletions tests/components/bond/test_fan.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
SERVICE_SET_PERCENTAGE,
SERVICE_SET_PRESET_MODE,
FanEntityFeature,
NotValidPresetModeError,
)
from homeassistant.const import (
ATTR_ENTITY_ID,
Expand Down Expand Up @@ -251,10 +252,14 @@ async def test_turn_on_fan_preset_mode_not_supported(hass: HomeAssistant) -> Non
props={"max_speed": 6},
)

with patch_bond_action(), patch_bond_device_state(), pytest.raises(ValueError):
with patch_bond_action(), patch_bond_device_state(), pytest.raises(
NotValidPresetModeError
):
await turn_fan_on(hass, "fan.name_1", preset_mode=PRESET_MODE_BREEZE)

with patch_bond_action(), patch_bond_device_state(), pytest.raises(ValueError):
with patch_bond_action(), patch_bond_device_state(), pytest.raises(
NotValidPresetModeError
):
await hass.services.async_call(
FAN_DOMAIN,
SERVICE_SET_PRESET_MODE,
Expand Down

0 comments on commit 953a212

Please sign in to comment.