Skip to content

Commit

Permalink
Do optimistic state update for Z-Wave multilevel switch entities (#90490
Browse files Browse the repository at this point in the history
)

* Do optimistic state update for Z-Wave multilevel switch entities

* simplify

* define constant for setting value to previous value

* Rework to only consider value of 255 and only places where we know that the previous state is known by the integration due to the service being called

* missed commit

* better code

* Add tests and use constant from lib

* fix logic

* fix bug

* Add comments with more details
  • Loading branch information
raman325 authored May 24, 2023
1 parent 3e93dd6 commit 66f7218
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 25 deletions.
21 changes: 19 additions & 2 deletions homeassistant/components/zwave_js/fan.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

from zwave_js_server.client import Client as ZwaveClient
from zwave_js_server.const import TARGET_VALUE_PROPERTY, CommandClass
from zwave_js_server.const.command_class.multilevel_switch import SET_TO_PREVIOUS_VALUE
from zwave_js_server.const.command_class.thermostat import (
THERMOSTAT_FAN_OFF_PROPERTY,
THERMOSTAT_FAN_STATE_PROPERTY,
Expand Down Expand Up @@ -88,6 +89,8 @@ def __init__(
assert target_value
self._target_value = target_value

self._use_optimistic_state: bool = False

async def async_set_percentage(self, percentage: int) -> None:
"""Set the speed percentage of the fan."""
if percentage == 0:
Expand All @@ -111,8 +114,19 @@ async def async_turn_on(
elif preset_mode is not None:
await self.async_set_preset_mode(preset_mode)
else:
# Value 255 tells device to return to previous value
await self.info.node.async_set_value(self._target_value, 255)
if self.info.primary_value.command_class != CommandClass.SWITCH_MULTILEVEL:
raise HomeAssistantError(
"`percentage` or `preset_mode` must be provided"
)
# If this is a Multilevel Switch CC value, we do an optimistic state update
# 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
)
self._use_optimistic_state = True
self.async_write_ha_state()

async def async_turn_off(self, **kwargs: Any) -> None:
"""Turn the device off."""
Expand All @@ -121,6 +135,9 @@ async def async_turn_off(self, **kwargs: Any) -> None:
@property
def is_on(self) -> bool | None:
"""Return true if device is on (speed above 0)."""
if self._use_optimistic_state:
self._use_optimistic_state = False
return True
if self.info.primary_value.value is None:
# guard missing value
return None
Expand Down
22 changes: 17 additions & 5 deletions homeassistant/components/zwave_js/light.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
TARGET_COLOR_PROPERTY,
ColorComponent,
)
from zwave_js_server.const.command_class.multilevel_switch import SET_TO_PREVIOUS_VALUE
from zwave_js_server.model.driver import Driver
from zwave_js_server.model.value import Value

Expand Down Expand Up @@ -164,6 +165,8 @@ def __init__(
if self.supports_brightness_transition or self.supports_color_transition:
self._attr_supported_features |= LightEntityFeature.TRANSITION

self._set_optimistic_state: bool = False

@callback
def on_value_update(self) -> None:
"""Call when a watched value is added or updated."""
Expand All @@ -187,10 +190,11 @@ def color_mode(self) -> str | None:
@property
def is_on(self) -> bool | None:
"""Return true if device is on (brightness above 0)."""
if self._set_optimistic_state:
self._set_optimistic_state = False
return True
brightness = self.brightness
if brightness is None:
return None
return brightness > 0
return brightness > 0 if brightness is not None else None

@property
def hs_color(self) -> tuple[float, float] | None:
Expand Down Expand Up @@ -332,8 +336,7 @@ async def _async_set_brightness(
if not self._target_brightness:
return
if brightness is None:
# Level 255 means to set it to previous value.
zwave_brightness = 255
zwave_brightness = SET_TO_PREVIOUS_VALUE
else:
# Zwave multilevel switches use a range of [0, 99] to control brightness.
zwave_brightness = byte_to_zwave_brightness(brightness)
Expand All @@ -350,6 +353,15 @@ async def _async_set_brightness(
await self.info.node.async_set_value(
self._target_brightness, zwave_brightness, zwave_transition
)
# We do an optimistic state update 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.
if (
zwave_brightness == SET_TO_PREVIOUS_VALUE
and self.info.primary_value.command_class == CommandClass.SWITCH_MULTILEVEL
):
self._set_optimistic_state = True
self.async_write_ha_state()

@callback
def _calculate_color_values(self) -> None:
Expand Down
43 changes: 25 additions & 18 deletions tests/components/zwave_js/test_fan.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,13 @@ async def test_generic_fan(
state = hass.states.get(entity_id)

assert state
assert state.state == "off"
assert state.state == STATE_OFF

# Test turn on setting speed
# Test turn on no speed
await hass.services.async_call(
"fan",
"turn_on",
{"entity_id": entity_id, "percentage": 66},
{"entity_id": entity_id},
blocking=True,
)

Expand All @@ -62,26 +62,22 @@ async def test_generic_fan(
"endpoint": 0,
"property": "targetValue",
}
assert args["value"] == 66
assert args["value"] == 255

client.async_send_command.reset_mock()

# Test setting unknown speed
with pytest.raises(MultipleInvalid):
await hass.services.async_call(
"fan",
"set_percentage",
{"entity_id": entity_id, "percentage": "bad"},
blocking=True,
)
# Due to optimistic updates, the state should be on even though the Z-Wave state
# hasn't been updated yet
state = hass.states.get(entity_id)

client.async_send_command.reset_mock()
assert state
assert state.state == STATE_ON

# Test turn on no speed
# Test turn on setting speed
await hass.services.async_call(
"fan",
"turn_on",
{"entity_id": entity_id},
{"entity_id": entity_id, "percentage": 66},
blocking=True,
)

Expand All @@ -94,7 +90,18 @@ async def test_generic_fan(
"endpoint": 0,
"property": "targetValue",
}
assert args["value"] == 255
assert args["value"] == 66

client.async_send_command.reset_mock()

# Test setting unknown speed
with pytest.raises(MultipleInvalid):
await hass.services.async_call(
"fan",
"set_percentage",
{"entity_id": entity_id, "percentage": "bad"},
blocking=True,
)

client.async_send_command.reset_mock()

Expand Down Expand Up @@ -140,7 +147,7 @@ async def test_generic_fan(
node.receive_event(event)

state = hass.states.get(entity_id)
assert state.state == "on"
assert state.state == STATE_ON
assert state.attributes[ATTR_PERCENTAGE] == 100

client.async_send_command.reset_mock()
Expand All @@ -165,7 +172,7 @@ async def test_generic_fan(
node.receive_event(event)

state = hass.states.get(entity_id)
assert state.state == "off"
assert state.state == STATE_OFF
assert state.attributes[ATTR_PERCENTAGE] == 0

client.async_send_command.reset_mock()
Expand Down
7 changes: 7 additions & 0 deletions tests/components/zwave_js/test_light.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,13 @@ async def test_light(
}
assert args["value"] == 255

# Due to optimistic updates, the state should be on even though the Z-Wave state
# hasn't been updated yet
state = hass.states.get(BULB_6_MULTI_COLOR_LIGHT_ENTITY)

assert state
assert state.state == STATE_ON

client.async_send_command.reset_mock()

# Test turning on with transition
Expand Down

0 comments on commit 66f7218

Please sign in to comment.