Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use ServiceValidationError for invalid fan preset_mode and move check to fan entity component #104560

Merged
merged 23 commits into from
Nov 29, 2023
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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(
jbouwh marked this conversation as resolved.
Show resolved Hide resolved
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
40 changes: 34 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,8 @@ 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):
"""Exception class when the preset_mode is not in the preset_modes list."""
jbouwh marked this conversation as resolved.
Show resolved Hide resolved


@bind_hass
Expand Down Expand Up @@ -107,7 +108,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 +157,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_on_service",
jbouwh marked this conversation as resolved.
Show resolved Hide resolved
[FanEntityFeature.SET_SPEED, FanEntityFeature.PRESET_MODE],
)

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

@final
async def async_handle_set_preset_mode_on_service(self, preset_mode: str) -> None:
jbouwh marked this conversation as resolved.
Show resolved Hide resolved
"""Validate and set new preset mode and set it."""
jbouwh marked this conversation as resolved.
Show resolved Hide resolved
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:
gjohansson-ST marked this conversation as resolved.
Show resolved Hide resolved
"""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_domain=DOMAIN,
translation_key="not_valid_preset_mode",
jbouwh marked this conversation as resolved.
Show resolved Hide resolved
translation_placeholders={
"preset_mode": preset_mode,
"preset_modes": preset_modes_str,
},
)

def set_direction(self, direction: str) -> None:
Expand All @@ -267,6 +283,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