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

Refactor zwave_js.fan and add tests #93256

Merged
merged 2 commits into from
May 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions homeassistant/components/zwave_js/discovery.py
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this related to the fan changes?

Copy link
Contributor Author

@raman325 raman325 May 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. Per discussion here: #92275 (comment)

We can now assume that the target value exists instead of guarding for the value being None since we require it during discovery. The target value is required to be included as part of the Z-Wave spec so this should be a no-op from a user perspective.

Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,12 @@ def get_config_parameter_discovery_schema(
type={ValueType.NUMBER},
)

SWITCH_MULTILEVEL_TARGET_VALUE_SCHEMA = ZWaveValueDiscoverySchema(
command_class={CommandClass.SWITCH_MULTILEVEL},
property={TARGET_VALUE_PROPERTY},
type={ValueType.NUMBER},
)

SWITCH_BINARY_CURRENT_VALUE_SCHEMA = ZWaveValueDiscoverySchema(
command_class={CommandClass.SWITCH_BINARY}, property={CURRENT_VALUE_PROPERTY}
)
Expand All @@ -261,6 +267,7 @@ def get_config_parameter_discovery_schema(
product_id={0x3131},
product_type={0x4944},
primary_value=SWITCH_MULTILEVEL_CURRENT_VALUE_SCHEMA,
required_values=[SWITCH_MULTILEVEL_TARGET_VALUE_SCHEMA],
),
# GE/Jasco - In-Wall Smart Fan Control - 12730 / ZW4002
ZWaveDiscoverySchema(
Expand Down Expand Up @@ -842,6 +849,7 @@ def get_config_parameter_discovery_schema(
device_class_generic={"Multilevel Switch"},
device_class_specific={"Fan Switch"},
primary_value=SWITCH_MULTILEVEL_CURRENT_VALUE_SCHEMA,
required_values=[SWITCH_MULTILEVEL_TARGET_VALUE_SCHEMA],
),
# number platform
# valve control for thermostats
Expand Down
37 changes: 10 additions & 27 deletions homeassistant/components/zwave_js/fan.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
from homeassistant.helpers.dispatcher import async_dispatcher_connect
from homeassistant.helpers.entity_platform import AddEntitiesCallback
from homeassistant.util.percentage import (
int_states_in_range,
percentage_to_ranged_value,
ranged_value_to_percentage,
)
Expand Down Expand Up @@ -85,7 +84,9 @@ def __init__(
) -> None:
"""Initialize the fan."""
super().__init__(config_entry, driver, info)
self._target_value = self.get_zwave_value(TARGET_VALUE_PROPERTY)
target_value = self.get_zwave_value(TARGET_VALUE_PROPERTY)
assert target_value
self._target_value = target_value

async def async_set_percentage(self, percentage: int) -> None:
"""Set the speed percentage of the fan."""
Expand All @@ -96,9 +97,7 @@ async def async_set_percentage(self, percentage: int) -> None:
percentage_to_ranged_value(DEFAULT_SPEED_RANGE, percentage)
)

if (target_value := self._target_value) is None:
raise HomeAssistantError("Missing target value on device.")
await self.info.node.async_set_value(target_value, zwave_speed)
await self.info.node.async_set_value(self._target_value, zwave_speed)

async def async_turn_on(
self,
Expand All @@ -112,16 +111,12 @@ async def async_turn_on(
elif preset_mode is not None:
await self.async_set_preset_mode(preset_mode)
else:
if (target_value := self._target_value) is None:
raise HomeAssistantError("Missing target value on device.")
# Value 255 tells device to return to previous value
await self.info.node.async_set_value(target_value, 255)
await self.info.node.async_set_value(self._target_value, 255)

async def async_turn_off(self, **kwargs: Any) -> None:
"""Turn the device off."""
if (target_value := self._target_value) is None:
raise HomeAssistantError("Missing target value on device.")
await self.info.node.async_set_value(target_value, 0)
await self.info.node.async_set_value(self._target_value, 0)

@property
def is_on(self) -> bool | None:
Expand All @@ -146,11 +141,6 @@ def percentage_step(self) -> float:
"""Return the step size for percentage."""
return 1

@property
def speed_count(self) -> int:
"""Return the number of speeds the fan supports."""
return int_states_in_range(DEFAULT_SPEED_RANGE)


class ValueMappingZwaveFan(ZwaveFan):
"""A Zwave fan with a value mapping data (e.g., 1-24 is low)."""
Expand All @@ -166,18 +156,14 @@ def __init__(

async def async_set_percentage(self, percentage: int) -> None:
"""Set the speed percentage of the fan."""
if (target_value := self._target_value) is None:
raise HomeAssistantError("Missing target value on device.")
zwave_speed = self.percentage_to_zwave_speed(percentage)
await self.info.node.async_set_value(target_value, zwave_speed)
await self.info.node.async_set_value(self._target_value, zwave_speed)

async def async_set_preset_mode(self, preset_mode: str) -> None:
"""Set new preset mode."""
if (target_value := self._target_value) is None:
raise HomeAssistantError("Missing target value on device.")
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(target_value, zwave_value)
await self.info.node.async_set_value(self._target_value, zwave_value)
return

raise NotValidPresetModeError(
Expand Down Expand Up @@ -277,12 +263,9 @@ def percentage_to_zwave_speed(self, percentage: int) -> int:
assert step_percentage

if percentage <= step_percentage:
return max_speed
break

# This shouldn't actually happen; the last entry in
# `self.fan_value_mapping.speeds` should map to 100%.
(_, last_max_speed) = self.fan_value_mapping.speeds[-1]
return last_max_speed
return max_speed

def zwave_speed_to_percentage(self, zwave_speed: int) -> int | None:
"""Convert a Zwave speed to a percentage.
Expand Down
68 changes: 68 additions & 0 deletions tests/components/zwave_js/test_fan.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
ATTR_PRESET_MODE,
ATTR_PRESET_MODES,
DOMAIN as FAN_DOMAIN,
SERVICE_SET_PERCENTAGE,
SERVICE_SET_PRESET_MODE,
FanEntityFeature,
NotValidPresetModeError,
Expand Down Expand Up @@ -167,6 +168,50 @@ async def test_generic_fan(
assert state.state == "off"
assert state.attributes[ATTR_PERCENTAGE] == 0

client.async_send_command.reset_mock()

# Test setting percentage to 0
await hass.services.async_call(
"fan",
SERVICE_SET_PERCENTAGE,
{"entity_id": entity_id, "percentage": 0},
blocking=True,
)

assert len(client.async_send_command.call_args_list) == 1
args = client.async_send_command.call_args[0][0]
assert args["command"] == "node.set_value"
assert args["nodeId"] == 17
assert args["valueId"] == {
"commandClass": 38,
"endpoint": 0,
"property": "targetValue",
}
assert args["value"] == 0

# Test value is None
event = Event(
type="value updated",
data={
"source": "node",
"event": "value updated",
"nodeId": 17,
"args": {
"commandClassName": "Multilevel Switch",
"commandClass": 38,
"endpoint": 0,
"property": "currentValue",
"newValue": None,
"prevValue": 0,
"propertyName": "currentValue",
},
},
)
node.receive_event(event)

state = hass.states.get(entity_id)
assert state.state == STATE_UNKNOWN


async def test_configurable_speeds_fan(
hass: HomeAssistant, client, hs_fc200, integration
Expand Down Expand Up @@ -361,6 +406,29 @@ async def get_percentage_from_zwave_speed(zwave_speed):
assert state.attributes[ATTR_PERCENTAGE_STEP] == pytest.approx(33.3333, rel=1e-3)
assert state.attributes[ATTR_PRESET_MODES] == []

# Test value is None
event = Event(
type="value updated",
data={
"source": "node",
"event": "value updated",
"nodeId": node_id,
"args": {
"commandClassName": "Multilevel Switch",
"commandClass": 38,
"endpoint": 0,
"property": "currentValue",
"newValue": None,
"prevValue": 0,
"propertyName": "currentValue",
},
},
)
node.receive_event(event)

state = hass.states.get(entity_id)
assert state.state == STATE_UNKNOWN


async def test_inovelli_lzw36(
hass: HomeAssistant, client, inovelli_lzw36, integration
Expand Down