From bd8c88f51bcb09c488f872925a9c06cc31e4742a Mon Sep 17 00:00:00 2001 From: Raman Gupta <7243222+raman325@users.noreply.github.com> Date: Wed, 31 May 2023 11:09:01 -0400 Subject: [PATCH] Add error handling for all zwave_js service calls (#93846) * Add error handling for all service calls * Switch siren to use internal function * Remove failing checks * Revert change to poll service, add comments, and add additional error handling * Add error handling for ping and refresh + review comment + add tests * Add test for statistics entity refresh --- homeassistant/components/zwave_js/button.py | 5 ++- homeassistant/components/zwave_js/climate.py | 12 +++---- homeassistant/components/zwave_js/cover.py | 24 ++++++------- homeassistant/components/zwave_js/entity.py | 16 ++++++--- homeassistant/components/zwave_js/fan.py | 18 +++++----- .../components/zwave_js/humidifier.py | 6 ++-- homeassistant/components/zwave_js/light.py | 6 ++-- homeassistant/components/zwave_js/lock.py | 18 ++++++++-- homeassistant/components/zwave_js/number.py | 2 +- homeassistant/components/zwave_js/select.py | 6 ++-- homeassistant/components/zwave_js/sensor.py | 22 +++++++++--- homeassistant/components/zwave_js/services.py | 7 ++-- homeassistant/components/zwave_js/siren.py | 16 +++------ homeassistant/components/zwave_js/update.py | 3 ++ tests/components/zwave_js/test_climate.py | 22 +++++++++++- tests/components/zwave_js/test_lock.py | 30 ++++++++++++++++ tests/components/zwave_js/test_sensor.py | 35 +++++++++++++++++-- tests/components/zwave_js/test_services.py | 12 +++++++ 18 files changed, 191 insertions(+), 69 deletions(-) diff --git a/homeassistant/components/zwave_js/button.py b/homeassistant/components/zwave_js/button.py index 7a6ec4c276c..33d1e6dfa63 100644 --- a/homeassistant/components/zwave_js/button.py +++ b/homeassistant/components/zwave_js/button.py @@ -77,7 +77,7 @@ class ZwaveBooleanNodeButton(ZWaveBaseEntity, ButtonEntity): async def async_press(self) -> None: """Press the button.""" - await self.info.node.async_set_value(self.info.primary_value, True) + await self._async_set_value(self.info.primary_value, True) class ZWaveNodePingButton(ButtonEntity): @@ -100,6 +100,9 @@ class ZWaveNodePingButton(ButtonEntity): async def async_poll_value(self, _: bool) -> None: """Poll a value.""" + # We log an error instead of raising an exception because this service call occurs + # in a separate task since it is called via the dispatcher and we don't want to + # raise the exception in that separate task because it is confusing to the user. LOGGER.error( "There is no value to refresh for this entity so the zwave_js.refresh_value" " service won't work for it" diff --git a/homeassistant/components/zwave_js/climate.py b/homeassistant/components/zwave_js/climate.py index 5397ab3a65b..82c212a99a5 100644 --- a/homeassistant/components/zwave_js/climate.py +++ b/homeassistant/components/zwave_js/climate.py @@ -437,7 +437,7 @@ class ZWaveClimate(ZWaveBaseEntity, ClimateEntity): except StopIteration: raise ValueError(f"Received an invalid fan mode: {fan_mode}") from None - await self.info.node.async_set_value(self._fan_mode, new_state) + await self._async_set_value(self._fan_mode, new_state) async def async_set_temperature(self, **kwargs: Any) -> None: """Set new target temperature.""" @@ -451,7 +451,7 @@ class ZWaveClimate(ZWaveBaseEntity, ClimateEntity): ) target_temp: float | None = kwargs.get(ATTR_TEMPERATURE) if target_temp is not None: - await self.info.node.async_set_value(setpoint, target_temp) + await self._async_set_value(setpoint, target_temp) elif len(self._current_mode_setpoint_enums) == 2: setpoint_low: ZwaveValue = self._setpoint_value_or_raise( self._current_mode_setpoint_enums[0] @@ -462,9 +462,9 @@ class ZWaveClimate(ZWaveBaseEntity, ClimateEntity): target_temp_low: float | None = kwargs.get(ATTR_TARGET_TEMP_LOW) target_temp_high: float | None = kwargs.get(ATTR_TARGET_TEMP_HIGH) if target_temp_low is not None: - await self.info.node.async_set_value(setpoint_low, target_temp_low) + await self._async_set_value(setpoint_low, target_temp_low) if target_temp_high is not None: - await self.info.node.async_set_value(setpoint_high, target_temp_high) + await self._async_set_value(setpoint_high, target_temp_high) async def async_set_hvac_mode(self, hvac_mode: HVACMode) -> None: """Set new target hvac mode.""" @@ -475,7 +475,7 @@ class ZWaveClimate(ZWaveBaseEntity, ClimateEntity): # Thermostat(valve) has no support for setting a mode, so we make it a no-op return - await self.info.node.async_set_value(self._current_mode, hvac_mode_id) + await self._async_set_value(self._current_mode, hvac_mode_id) async def async_set_preset_mode(self, preset_mode: str) -> None: """Set new target preset mode.""" @@ -487,7 +487,7 @@ class ZWaveClimate(ZWaveBaseEntity, ClimateEntity): preset_mode_value = self._hvac_presets.get(preset_mode) if preset_mode_value is None: raise ValueError(f"Received an invalid preset mode: {preset_mode}") - await self.info.node.async_set_value(self._current_mode, preset_mode_value) + await self._async_set_value(self._current_mode, preset_mode_value) class DynamicCurrentTempClimate(ZWaveClimate): diff --git a/homeassistant/components/zwave_js/cover.py b/homeassistant/components/zwave_js/cover.py index 1521d6e567f..4b3113af202 100644 --- a/homeassistant/components/zwave_js/cover.py +++ b/homeassistant/components/zwave_js/cover.py @@ -163,7 +163,7 @@ class CoverPositionMixin(ZWaveBaseEntity, CoverEntity): async def async_set_cover_position(self, **kwargs: Any) -> None: """Move the cover to a specific position.""" assert self._target_position_value - await self.info.node.async_set_value( + await self._async_set_value( self._target_position_value, self.percent_to_zwave_position(kwargs[ATTR_POSITION]), ) @@ -171,14 +171,14 @@ class CoverPositionMixin(ZWaveBaseEntity, CoverEntity): async def async_open_cover(self, **kwargs: Any) -> None: """Open the cover.""" assert self._target_position_value - await self.info.node.async_set_value( + await self._async_set_value( self._target_position_value, self._fully_open_position ) async def async_close_cover(self, **kwargs: Any) -> None: """Close cover.""" assert self._target_position_value - await self.info.node.async_set_value( + await self._async_set_value( self._target_position_value, self._fully_closed_position ) @@ -186,7 +186,7 @@ class CoverPositionMixin(ZWaveBaseEntity, CoverEntity): """Stop cover.""" assert self._stop_position_value # Stop the cover, will stop regardless of the actual direction of travel. - await self.info.node.async_set_value(self._stop_position_value, False) + await self._async_set_value(self._stop_position_value, False) class CoverTiltMixin(ZWaveBaseEntity, CoverEntity): @@ -259,7 +259,7 @@ class CoverTiltMixin(ZWaveBaseEntity, CoverEntity): async def async_set_cover_tilt_position(self, **kwargs: Any) -> None: """Move the cover tilt to a specific position.""" assert self._target_tilt_value - await self.info.node.async_set_value( + await self._async_set_value( self._target_tilt_value, self.percent_to_zwave_tilt(kwargs[ATTR_TILT_POSITION]), ) @@ -267,22 +267,18 @@ class CoverTiltMixin(ZWaveBaseEntity, CoverEntity): async def async_open_cover_tilt(self, **kwargs: Any) -> None: """Open the cover tilt.""" assert self._target_tilt_value - await self.info.node.async_set_value( - self._target_tilt_value, self._fully_open_tilt - ) + await self._async_set_value(self._target_tilt_value, self._fully_open_tilt) async def async_close_cover_tilt(self, **kwargs: Any) -> None: """Close the cover tilt.""" assert self._target_tilt_value - await self.info.node.async_set_value( - self._target_tilt_value, self._fully_closed_tilt - ) + await self._async_set_value(self._target_tilt_value, self._fully_closed_tilt) async def async_stop_cover_tilt(self, **kwargs: Any) -> None: """Stop the cover tilt.""" assert self._stop_tilt_value # Stop the tilt, will stop regardless of the actual direction of travel. - await self.info.node.async_set_value(self._stop_tilt_value, False) + await self._async_set_value(self._stop_tilt_value, False) class ZWaveMultilevelSwitchCover(CoverPositionMixin): @@ -455,8 +451,8 @@ class ZwaveMotorizedBarrier(ZWaveBaseEntity, CoverEntity): async def async_open_cover(self, **kwargs: Any) -> None: """Open the garage door.""" - await self.info.node.async_set_value(self._target_state, BarrierState.OPEN) + await self._async_set_value(self._target_state, BarrierState.OPEN) async def async_close_cover(self, **kwargs: Any) -> None: """Close the garage door.""" - await self.info.node.async_set_value(self._target_state, BarrierState.CLOSED) + await self._async_set_value(self._target_state, BarrierState.CLOSED) diff --git a/homeassistant/components/zwave_js/entity.py b/homeassistant/components/zwave_js/entity.py index ba2d753112c..4d72ad85821 100644 --- a/homeassistant/components/zwave_js/entity.py +++ b/homeassistant/components/zwave_js/entity.py @@ -67,12 +67,20 @@ class ZWaveBaseEntity(Entity): To be overridden by platforms needing this event. """ + async def _async_poll_value(self, value_or_id: str | ZwaveValue) -> None: + """Poll a value.""" + # We log an error instead of raising an exception because this service call occurs + # in a separate task and we don't want to raise the exception in that separate task + # because it is confusing to the user. + try: + await self.info.node.async_poll_value(value_or_id) + except BaseZwaveJSServerError as err: + LOGGER.error("Error while refreshing value %s: %s", value_or_id, err) + async def async_poll_value(self, refresh_all_values: bool) -> None: """Poll a value.""" if not refresh_all_values: - self.hass.async_create_task( - self.info.node.async_poll_value(self.info.primary_value) - ) + self.hass.async_create_task(self._async_poll_value(self.info.primary_value)) LOGGER.info( ( "Refreshing primary value %s for %s, " @@ -84,7 +92,7 @@ class ZWaveBaseEntity(Entity): return for value_id in self.watched_value_ids: - self.hass.async_create_task(self.info.node.async_poll_value(value_id)) + self.hass.async_create_task(self._async_poll_value(value_id)) LOGGER.info( ( diff --git a/homeassistant/components/zwave_js/fan.py b/homeassistant/components/zwave_js/fan.py index ea00b70b949..d0630649765 100644 --- a/homeassistant/components/zwave_js/fan.py +++ b/homeassistant/components/zwave_js/fan.py @@ -100,7 +100,7 @@ class ZwaveFan(ZWaveBaseEntity, FanEntity): percentage_to_ranged_value(DEFAULT_SPEED_RANGE, percentage) ) - await self.info.node.async_set_value(self._target_value, zwave_speed) + await self._async_set_value(self._target_value, zwave_speed) async def async_turn_on( self, @@ -122,15 +122,13 @@ class ZwaveFan(ZWaveBaseEntity, FanEntity): # when setting to a previous value to avoid waiting for the value to be # updated from the device which is typically delayed and causes a confusing # UX. - await self.info.node.async_set_value( - self._target_value, SET_TO_PREVIOUS_VALUE - ) + await self._async_set_value(self._target_value, SET_TO_PREVIOUS_VALUE) self._use_optimistic_state = True self.async_write_ha_state() async def async_turn_off(self, **kwargs: Any) -> None: """Turn the device off.""" - await self.info.node.async_set_value(self._target_value, 0) + await self._async_set_value(self._target_value, 0) @property def is_on(self) -> bool | None: @@ -174,13 +172,13 @@ class ValueMappingZwaveFan(ZwaveFan): async def async_set_percentage(self, percentage: int) -> None: """Set the speed percentage of the fan.""" zwave_speed = self.percentage_to_zwave_speed(percentage) - await self.info.node.async_set_value(self._target_value, zwave_speed) + await self._async_set_value(self._target_value, zwave_speed) async def async_set_preset_mode(self, preset_mode: str) -> None: """Set new preset mode.""" for zwave_value, mapped_preset_mode in self.fan_value_mapping.presets.items(): if preset_mode == mapped_preset_mode: - await self.info.node.async_set_value(self._target_value, zwave_value) + await self._async_set_value(self._target_value, zwave_value) return raise NotValidPresetModeError( @@ -342,13 +340,13 @@ class ZwaveThermostatFan(ZWaveBaseEntity, FanEntity): """Turn the device on.""" if not self._fan_off: raise HomeAssistantError("Unhandled action turn_on") - await self.info.node.async_set_value(self._fan_off, False) + await self._async_set_value(self._fan_off, False) async def async_turn_off(self, **kwargs: Any) -> None: """Turn the device off.""" if not self._fan_off: raise HomeAssistantError("Unhandled action turn_off") - await self.info.node.async_set_value(self._fan_off, True) + await self._async_set_value(self._fan_off, True) @property def is_on(self) -> bool | None: @@ -377,7 +375,7 @@ class ZwaveThermostatFan(ZWaveBaseEntity, FanEntity): except StopIteration: raise ValueError(f"Received an invalid fan mode: {preset_mode}") from None - await self.info.node.async_set_value(self._fan_mode, new_state) + await self._async_set_value(self._fan_mode, new_state) @property def preset_modes(self) -> list[str] | None: diff --git a/homeassistant/components/zwave_js/humidifier.py b/homeassistant/components/zwave_js/humidifier.py index 80760930c2e..02c6abbc732 100644 --- a/homeassistant/components/zwave_js/humidifier.py +++ b/homeassistant/components/zwave_js/humidifier.py @@ -175,7 +175,7 @@ class ZWaveHumidifier(ZWaveBaseEntity, HumidifierEntity): else: return - await self.info.node.async_set_value(self._current_mode, new_mode) + await self._async_set_value(self._current_mode, new_mode) async def async_turn_off(self, **kwargs: Any) -> None: """Turn off device.""" @@ -192,7 +192,7 @@ class ZWaveHumidifier(ZWaveBaseEntity, HumidifierEntity): else: return - await self.info.node.async_set_value(self._current_mode, new_mode) + await self._async_set_value(self._current_mode, new_mode) @property def target_humidity(self) -> int | None: @@ -204,7 +204,7 @@ class ZWaveHumidifier(ZWaveBaseEntity, HumidifierEntity): async def async_set_humidity(self, humidity: int) -> None: """Set new target humidity.""" if self._setpoint: - await self.info.node.async_set_value(self._setpoint, humidity) + await self._async_set_value(self._setpoint, humidity) @property def min_humidity(self) -> int: diff --git a/homeassistant/components/zwave_js/light.py b/homeassistant/components/zwave_js/light.py index e7ef38e5c18..1a9abb9b0f8 100644 --- a/homeassistant/components/zwave_js/light.py +++ b/homeassistant/components/zwave_js/light.py @@ -324,9 +324,7 @@ class ZwaveLight(ZWaveBaseEntity, LightEntity): color_name = MULTI_COLOR_MAP[color] colors_dict[color_name] = value # set updated color object - await self.info.node.async_set_value( - combined_color_val, colors_dict, zwave_transition - ) + await self._async_set_value(combined_color_val, colors_dict, zwave_transition) async def _async_set_brightness( self, brightness: int | None, transition: float | None = None @@ -350,7 +348,7 @@ class ZwaveLight(ZWaveBaseEntity, LightEntity): zwave_transition = {TRANSITION_DURATION_OPTION: "default"} # setting a value requires setting targetValue - await self.info.node.async_set_value( + await self._async_set_value( self._target_brightness, zwave_brightness, zwave_transition ) # We do an optimistic state update when setting to a previous value diff --git a/homeassistant/components/zwave_js/lock.py b/homeassistant/components/zwave_js/lock.py index efeadb9b6b3..ff4cb84b47c 100644 --- a/homeassistant/components/zwave_js/lock.py +++ b/homeassistant/components/zwave_js/lock.py @@ -13,12 +13,14 @@ from zwave_js_server.const.command_class.lock import ( LOCK_CMD_CLASS_TO_PROPERTY_MAP, DoorLockMode, ) +from zwave_js_server.exceptions import BaseZwaveJSServerError from zwave_js_server.util.lock import clear_usercode, set_usercode from homeassistant.components.lock import DOMAIN as LOCK_DOMAIN, LockEntity from homeassistant.config_entries import ConfigEntry from homeassistant.const import STATE_LOCKED, STATE_UNLOCKED from homeassistant.core import HomeAssistant, callback +from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers import config_validation as cv, entity_platform from homeassistant.helpers.dispatcher import async_dispatcher_connect from homeassistant.helpers.entity_platform import AddEntitiesCallback @@ -114,7 +116,7 @@ class ZWaveLock(ZWaveBaseEntity, LockEntity): ] ) if target_value is not None: - await self.info.node.async_set_value( + await self._async_set_value( target_value, STATE_TO_ZWAVE_MAP[self.info.primary_value.command_class][target_state], ) @@ -129,10 +131,20 @@ class ZWaveLock(ZWaveBaseEntity, LockEntity): async def async_set_lock_usercode(self, code_slot: int, usercode: str) -> None: """Set the usercode to index X on the lock.""" - await set_usercode(self.info.node, code_slot, usercode) + try: + await set_usercode(self.info.node, code_slot, usercode) + except BaseZwaveJSServerError as err: + raise HomeAssistantError( + f"Unable to set lock usercode on code_slot {code_slot}: {err}" + ) from err LOGGER.debug("User code at slot %s set", code_slot) async def async_clear_lock_usercode(self, code_slot: int) -> None: """Clear the usercode at index X on the lock.""" - await clear_usercode(self.info.node, code_slot) + try: + await clear_usercode(self.info.node, code_slot) + except BaseZwaveJSServerError as err: + raise HomeAssistantError( + f"Unable to clear lock usercode on code_slot {code_slot}: {err}" + ) from err LOGGER.debug("User code at slot %s cleared", code_slot) diff --git a/homeassistant/components/zwave_js/number.py b/homeassistant/components/zwave_js/number.py index 4cffa7a8522..6aa4a57ea4c 100644 --- a/homeassistant/components/zwave_js/number.py +++ b/homeassistant/components/zwave_js/number.py @@ -164,6 +164,6 @@ class ZwaveVolumeNumberEntity(ZWaveBaseEntity, NumberEntity): async def async_set_native_value(self, value: float) -> None: """Set new value.""" - await self.info.node.async_set_value( + await self._async_set_value( self.info.primary_value, round(value * self.correction_factor) ) diff --git a/homeassistant/components/zwave_js/select.py b/homeassistant/components/zwave_js/select.py index 49d4fec4a20..3956004336a 100644 --- a/homeassistant/components/zwave_js/select.py +++ b/homeassistant/components/zwave_js/select.py @@ -92,7 +92,7 @@ class ZwaveSelectEntity(ZWaveBaseEntity, SelectEntity): for key, val in self.info.primary_value.metadata.states.items() if val == option ) - await self.info.node.async_set_value(self.info.primary_value, int(key)) + await self._async_set_value(self.info.primary_value, int(key)) class ZWaveConfigParameterSelectEntity(ZwaveSelectEntity): @@ -162,7 +162,7 @@ class ZwaveDefaultToneSelectEntity(ZWaveBaseEntity, SelectEntity): for key, val in self._tones_value.metadata.states.items() if val == option ) - await self.info.node.async_set_value(self.info.primary_value, int(key)) + await self._async_set_value(self.info.primary_value, int(key)) class ZwaveMultilevelSwitchSelectEntity(ZWaveBaseEntity, SelectEntity): @@ -197,4 +197,4 @@ class ZwaveMultilevelSwitchSelectEntity(ZWaveBaseEntity, SelectEntity): """Change the selected option.""" assert self._target_value is not None key = next(key for key, val in self._lookup_map.items() if val == option) - await self.info.node.async_set_value(self._target_value, int(key)) + await self._async_set_value(self._target_value, int(key)) diff --git a/homeassistant/components/zwave_js/sensor.py b/homeassistant/components/zwave_js/sensor.py index 3bb893c303a..316c0b81eeb 100644 --- a/homeassistant/components/zwave_js/sensor.py +++ b/homeassistant/components/zwave_js/sensor.py @@ -11,6 +11,7 @@ from zwave_js_server.const.command_class.meter import ( RESET_METER_OPTION_TARGET_VALUE, RESET_METER_OPTION_TYPE, ) +from zwave_js_server.exceptions import BaseZwaveJSServerError from zwave_js_server.model.controller import Controller from zwave_js_server.model.controller.statistics import ControllerStatisticsDataType from zwave_js_server.model.driver import Driver @@ -43,6 +44,7 @@ from homeassistant.const import ( UnitOfTime, ) from homeassistant.core import HomeAssistant, callback +from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers import entity_platform from homeassistant.helpers.dispatcher import async_dispatcher_connect from homeassistant.helpers.entity_platform import AddEntitiesCallback @@ -671,9 +673,15 @@ class ZWaveMeterSensor(ZWaveNumericSensor): if value is not None: options[RESET_METER_OPTION_TARGET_VALUE] = value args = [options] if options else [] - await node.endpoints[endpoint].async_invoke_cc_api( - CommandClass.METER, "reset", *args, wait_for_result=False - ) + try: + await node.endpoints[endpoint].async_invoke_cc_api( + CommandClass.METER, "reset", *args, wait_for_result=False + ) + except BaseZwaveJSServerError as err: + LOGGER.error( + "Failed to reset meters on node %s endpoint %s: %s", node, endpoint, err + ) + raise HomeAssistantError from err LOGGER.debug( "Meters on node %s endpoint %s reset with the following options: %s", node, @@ -802,6 +810,9 @@ class ZWaveNodeStatusSensor(SensorEntity): async def async_poll_value(self, _: bool) -> None: """Poll a value.""" + # We log an error instead of raising an exception because this service call occurs + # in a separate task since it is called via the dispatcher and we don't want to + # raise the exception in that separate task because it is confusing to the user. LOGGER.error( "There is no value to refresh for this entity so the zwave_js.refresh_value" " service won't work for it" @@ -878,7 +889,10 @@ class ZWaveStatisticsSensor(SensorEntity): async def async_poll_value(self, _: bool) -> None: """Poll a value.""" - raise ValueError( + # We log an error instead of raising an exception because this service call occurs + # in a separate task since it is called via the dispatcher and we don't want to + # raise the exception in that separate task because it is confusing to the user. + LOGGER.error( "There is no value to refresh for this entity so the zwave_js.refresh_value" " service won't work for it" ) diff --git a/homeassistant/components/zwave_js/services.py b/homeassistant/components/zwave_js/services.py index 6f4f9f8e330..133cb407405 100644 --- a/homeassistant/components/zwave_js/services.py +++ b/homeassistant/components/zwave_js/services.py @@ -633,8 +633,11 @@ class ZWaveServices: "calls will still work for now but the service will be removed in a " "future release" ) - nodes: set[ZwaveNode] = service.data[const.ATTR_NODES] - await asyncio.gather(*(node.async_ping() for node in nodes)) + nodes: list[ZwaveNode] = list(service.data[const.ATTR_NODES]) + results = await asyncio.gather( + *(node.async_ping() for node in nodes), return_exceptions=True + ) + raise_exceptions_from_results(nodes, results) async def async_invoke_cc_api(self, service: ServiceCall) -> None: """Invoke a command class API.""" diff --git a/homeassistant/components/zwave_js/siren.py b/homeassistant/components/zwave_js/siren.py index 5a53d115528..6de6b0f4e45 100644 --- a/homeassistant/components/zwave_js/siren.py +++ b/homeassistant/components/zwave_js/siren.py @@ -79,14 +79,6 @@ class ZwaveSirenEntity(ZWaveBaseEntity, SirenEntity): return None return bool(self.info.primary_value.value) - async def async_set_value( - self, new_value: int, options: dict[str, Any] | None = None - ) -> None: - """Set a value on a siren node.""" - await self.info.node.async_set_value( - self.info.primary_value, new_value, options=options - ) - async def async_turn_on(self, **kwargs: Any) -> None: """Turn the device on.""" tone_id: int | None = kwargs.get(ATTR_TONE) @@ -95,11 +87,13 @@ class ZwaveSirenEntity(ZWaveBaseEntity, SirenEntity): options["volume"] = round(volume * 100) # Play the default tone if a tone isn't provided if tone_id is None: - await self.async_set_value(ToneID.DEFAULT, options) + await self._async_set_value( + self.info.primary_value, ToneID.DEFAULT, options + ) return - await self.async_set_value(tone_id, options) + await self._async_set_value(self.info.primary_value, tone_id, options) async def async_turn_off(self, **kwargs: Any) -> None: """Turn the device off.""" - await self.async_set_value(ToneID.OFF) + await self._async_set_value(self.info.primary_value, ToneID.OFF) diff --git a/homeassistant/components/zwave_js/update.py b/homeassistant/components/zwave_js/update.py index 243642086d0..8403e28a68b 100644 --- a/homeassistant/components/zwave_js/update.py +++ b/homeassistant/components/zwave_js/update.py @@ -291,6 +291,9 @@ class ZWaveNodeFirmwareUpdate(UpdateEntity): async def async_poll_value(self, _: bool) -> None: """Poll a value.""" + # We log an error instead of raising an exception because this service call occurs + # in a separate task since it is called via the dispatcher and we don't want to + # raise the exception in that separate task because it is confusing to the user. LOGGER.error( "There is no value to refresh for this entity so the zwave_js.refresh_value" " service won't work for it" diff --git a/tests/components/zwave_js/test_climate.py b/tests/components/zwave_js/test_climate.py index f0c746f7f22..00006673785 100644 --- a/tests/components/zwave_js/test_climate.py +++ b/tests/components/zwave_js/test_climate.py @@ -5,6 +5,7 @@ from zwave_js_server.const.command_class.thermostat import ( THERMOSTAT_OPERATING_STATE_PROPERTY, ) from zwave_js_server.event import Event +from zwave_js_server.exceptions import FailedZWaveCommand from zwave_js_server.model.node import Node from homeassistant.components.climate import ( @@ -30,6 +31,7 @@ from homeassistant.components.climate import ( HVACMode, ) from homeassistant.components.zwave_js.climate import ATTR_FAN_STATE +from homeassistant.components.zwave_js.const import DOMAIN, SERVICE_REFRESH_VALUE from homeassistant.components.zwave_js.helpers import ZwaveValueMatcher from homeassistant.const import ( ATTR_ENTITY_ID, @@ -49,7 +51,11 @@ from .common import ( async def test_thermostat_v2( - hass: HomeAssistant, client, climate_radio_thermostat_ct100_plus, integration + hass: HomeAssistant, + client, + climate_radio_thermostat_ct100_plus, + integration, + caplog: pytest.LogCaptureFixture, ) -> None: """Test a thermostat v2 command class entity.""" node = climate_radio_thermostat_ct100_plus @@ -280,6 +286,20 @@ async def test_thermostat_v2( blocking=True, ) + # Refresh value should log an error when there is an issue + client.async_send_command.reset_mock() + client.async_send_command.side_effect = FailedZWaveCommand("test", 1, "test") + await hass.services.async_call( + DOMAIN, + SERVICE_REFRESH_VALUE, + { + ATTR_ENTITY_ID: CLIMATE_RADIO_THERMOSTAT_ENTITY, + }, + blocking=True, + ) + + assert "Error while refreshing value" in caplog.text + async def test_thermostat_different_endpoints( hass: HomeAssistant, diff --git a/tests/components/zwave_js/test_lock.py b/tests/components/zwave_js/test_lock.py index 42aee148aff..5a5711d9dad 100644 --- a/tests/components/zwave_js/test_lock.py +++ b/tests/components/zwave_js/test_lock.py @@ -1,4 +1,5 @@ """Test the Z-Wave JS lock platform.""" +import pytest from zwave_js_server.const import CommandClass from zwave_js_server.const.command_class.lock import ( ATTR_CODE_SLOT, @@ -6,6 +7,7 @@ from zwave_js_server.const.command_class.lock import ( CURRENT_MODE_PROPERTY, ) from zwave_js_server.event import Event +from zwave_js_server.exceptions import FailedZWaveCommand from zwave_js_server.model.node import Node, NodeStatus from homeassistant.components.lock import ( @@ -27,6 +29,7 @@ from homeassistant.const import ( STATE_UNLOCKED, ) from homeassistant.core import HomeAssistant +from homeassistant.exceptions import HomeAssistantError from .common import SCHLAGE_BE469_LOCK_ENTITY, replace_value_of_zwave_value @@ -153,6 +156,33 @@ async def test_door_lock( } assert args["value"] == 0 + client.async_send_command.reset_mock() + + client.async_send_command.side_effect = FailedZWaveCommand("test", 1, "test") + # Test set usercode service error handling + with pytest.raises(HomeAssistantError): + await hass.services.async_call( + ZWAVE_JS_DOMAIN, + SERVICE_SET_LOCK_USERCODE, + { + ATTR_ENTITY_ID: SCHLAGE_BE469_LOCK_ENTITY, + ATTR_CODE_SLOT: 1, + ATTR_USERCODE: "1234", + }, + blocking=True, + ) + + # Test clear usercode service error handling + with pytest.raises(HomeAssistantError): + await hass.services.async_call( + ZWAVE_JS_DOMAIN, + SERVICE_CLEAR_LOCK_USERCODE, + {ATTR_ENTITY_ID: SCHLAGE_BE469_LOCK_ENTITY, ATTR_CODE_SLOT: 1}, + blocking=True, + ) + + client.async_send_command.reset_mock() + event = Event( type="dead", data={ diff --git a/tests/components/zwave_js/test_sensor.py b/tests/components/zwave_js/test_sensor.py index bf6ff36d8f0..766d5684af5 100644 --- a/tests/components/zwave_js/test_sensor.py +++ b/tests/components/zwave_js/test_sensor.py @@ -4,6 +4,7 @@ import copy import pytest from zwave_js_server.const.command_class.meter import MeterType from zwave_js_server.event import Event +from zwave_js_server.exceptions import FailedZWaveCommand from zwave_js_server.model.node import Node from homeassistant.components.sensor import ( @@ -39,6 +40,7 @@ from homeassistant.const import ( UnitOfTime, ) from homeassistant.core import HomeAssistant +from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers import entity_registry as er from .common import ( @@ -420,6 +422,18 @@ async def test_reset_meter( client.async_send_command_no_wait.reset_mock() + client.async_send_command_no_wait.side_effect = FailedZWaveCommand( + "test", 1, "test" + ) + + with pytest.raises(HomeAssistantError): + await hass.services.async_call( + DOMAIN, + SERVICE_RESET_METER, + {ATTR_ENTITY_ID: METER_ENERGY_SENSOR}, + blocking=True, + ) + async def test_meter_attributes( hass: HomeAssistant, @@ -609,7 +623,7 @@ NODE_STATISTICS_SUFFIXES_UNKNOWN = { async def test_statistics_sensors( - hass: HomeAssistant, zp3111, client, integration + hass: HomeAssistant, zp3111, client, integration, caplog: pytest.LogCaptureFixture ) -> None: """Test statistics sensors.""" ent_reg = er.async_get(hass) @@ -730,10 +744,27 @@ async def test_statistics_sensors( (NODE_STATISTICS_ENTITY_PREFIX, NODE_STATISTICS_SUFFIXES_UNKNOWN), ): for suffix_key, val in suffixes.items(): - state = hass.states.get(f"{prefix}{suffix_key}") + entity_id = f"{prefix}{suffix_key}" + state = hass.states.get(entity_id) assert state assert state.state == str(val) + await hass.services.async_call( + DOMAIN, + SERVICE_REFRESH_VALUE, + {ATTR_ENTITY_ID: entity_id}, + blocking=True, + ) + + assert caplog.text.count("There is no value to refresh for this entity") == len( + [ + *CONTROLLER_STATISTICS_SUFFIXES, + *CONTROLLER_STATISTICS_SUFFIXES_UNKNOWN, + *NODE_STATISTICS_SUFFIXES, + *NODE_STATISTICS_SUFFIXES_UNKNOWN, + ] + ) + ENERGY_PRODUCTION_ENTITY_MAP = { "energy_production_power": { diff --git a/tests/components/zwave_js/test_services.py b/tests/components/zwave_js/test_services.py index d1edfa5040f..a92d34dd412 100644 --- a/tests/components/zwave_js/test_services.py +++ b/tests/components/zwave_js/test_services.py @@ -1539,6 +1539,18 @@ async def test_ping( blocking=True, ) + client.async_send_command.reset_mock() + client.async_send_command.side_effect = FailedZWaveCommand("test", 1, "test") + with pytest.raises(HomeAssistantError): + await hass.services.async_call( + DOMAIN, + SERVICE_PING, + { + ATTR_ENTITY_ID: CLIMATE_RADIO_THERMOSTAT_ENTITY, + }, + blocking=True, + ) + async def test_invoke_cc_api( hass: HomeAssistant,