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

Update Lutron in callback #108779

Merged
merged 5 commits into from
Jan 25, 2024
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
10 changes: 4 additions & 6 deletions homeassistant/components/lutron/binary_sensor.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,11 @@ class LutronOccupancySensor(LutronDevice, BinarySensorEntity):
_lutron_device: OccupancyGroup
_attr_device_class = BinarySensorDeviceClass.OCCUPANCY

@property
def is_on(self) -> bool:
"""Return true if the binary sensor is on."""
# Error cases will end up treated as unoccupied.
return self._lutron_device.state == OccupancyGroup.State.OCCUPIED

@property
def extra_state_attributes(self) -> Mapping[str, Any] | None:
"""Return the state attributes."""
return {"lutron_integration_id": self._lutron_device.id}

def _update_attrs(self) -> None:
"""Update the state attributes."""
self._attr_is_on = self._lutron_device.state == OccupancyGroup.State.OCCUPIED
MartinHjelmare marked this conversation as resolved.
Show resolved Hide resolved
23 changes: 9 additions & 14 deletions homeassistant/components/lutron/cover.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,16 +53,6 @@ class LutronCover(LutronDevice, CoverEntity):
_lutron_device: Output
_attr_name = None

@property
def is_closed(self) -> bool:
"""Return if the cover is closed."""
return self._lutron_device.last_level() < 1

@property
def current_cover_position(self) -> int:
"""Return the current position of cover."""
return self._lutron_device.last_level()

def close_cover(self, **kwargs: Any) -> None:
"""Close the cover."""
self._lutron_device.level = 0
Expand All @@ -77,10 +67,15 @@ def set_cover_position(self, **kwargs: Any) -> None:
position = kwargs[ATTR_POSITION]
self._lutron_device.level = position

def update(self) -> None:
"""Call when forcing a refresh of the device."""
# Reading the property (rather than last_level()) fetches value
level = self._lutron_device.level
def _request_state(self) -> None:
"""Request the state from the device."""
self._lutron_device.level # pylint: disable=pointless-statement

def _update_attrs(self) -> None:
"""Update the state attributes."""
level = self._lutron_device.last_level()
self._attr_is_closed = level < 1
self._attr_current_cover_position = level
_LOGGER.debug("Lutron ID: %d updated to %f", self._lutron_device.id, level)

@property
Expand Down
12 changes: 12 additions & 0 deletions homeassistant/components/lutron/entity.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,17 @@ async def async_added_to_hass(self) -> None:
"""Register callbacks."""
self._lutron_device.subscribe(self._update_callback, None)

def _request_state(self) -> None:
"""Request the state."""

def _update_attrs(self) -> None:
"""Update the entity's attributes."""

def _update_callback(
self, _device: LutronEntity, _context: None, _event: LutronEvent, _params: dict
) -> None:
"""Run when invoked by pylutron when the device state changes."""
self._update_attrs()
self.schedule_update_ha_state()
MartinHjelmare marked this conversation as resolved.
Show resolved Hide resolved

@property
Expand All @@ -41,6 +48,11 @@ def unique_id(self) -> str | None:
return None
return f"{self._controller.guid}_{self._lutron_device.uuid}"

def update(self) -> None:
"""Update the entity's state."""
self._request_state()
self._update_attrs()
MartinHjelmare marked this conversation as resolved.
Show resolved Hide resolved


class LutronDevice(LutronBaseEntity):
"""Representation of a Lutron device entity."""
Expand Down
29 changes: 12 additions & 17 deletions homeassistant/components/lutron/light.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,6 @@ class LutronLight(LutronDevice, LightEntity):
_prev_brightness: int | None = None
_attr_name = None

@property
def brightness(self) -> int:
"""Return the brightness of the light."""
new_brightness = to_hass_level(self._lutron_device.last_level())
Copy link
Contributor

Choose a reason for hiding this comment

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

If you don't call last_level you may not be getting the right value. State is easily lost in a lutron system due to how the updates flow related to scenes. I strongly discourage this approach as this has been stable for a long time now.

if new_brightness != 0:
self._prev_brightness = new_brightness
return new_brightness

def turn_on(self, **kwargs: Any) -> None:
"""Turn the light on."""
if ATTR_BRIGHTNESS in kwargs and self._lutron_device.is_dimmable:
Expand All @@ -82,12 +74,15 @@ def extra_state_attributes(self) -> Mapping[str, Any] | None:
"""Return the state attributes."""
return {"lutron_integration_id": self._lutron_device.id}

@property
def is_on(self) -> bool:
"""Return true if device is on."""
return self._lutron_device.last_level() > 0

def update(self) -> None:
"""Call when forcing a refresh of the device."""
if self._prev_brightness is None:
self._prev_brightness = to_hass_level(self._lutron_device.level)
def _request_state(self) -> None:
"""Request the state from the device."""
self._lutron_device.level # pylint: disable=pointless-statement

def _update_attrs(self) -> None:
"""Update the state attributes."""
level = self._lutron_device.last_level()
self._attr_is_on = level > 0
hass_level = to_hass_level(level)
self._attr_brightness = hass_level
if self._prev_brightness is None or hass_level != 0:
self._prev_brightness = hass_level
7 changes: 2 additions & 5 deletions homeassistant/components/lutron/scene.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,8 @@ async def async_setup_entry(
entry_data: LutronData = hass.data[DOMAIN][config_entry.entry_id]

async_add_entities(
[
LutronScene(area_name, keypad, device, entry_data.client)
for area_name, keypad, device, led in entry_data.scenes
],
True,
LutronScene(area_name, keypad, device, entry_data.client)
for area_name, keypad, device, led in entry_data.scenes
)


Expand Down
35 changes: 12 additions & 23 deletions homeassistant/components/lutron/switch.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,6 @@ class LutronSwitch(LutronDevice, SwitchEntity):

_lutron_device: Output

def __init__(
self, area_name: str, lutron_device: Output, controller: Lutron
) -> None:
"""Initialize the switch."""
self._prev_state = None
super().__init__(area_name, lutron_device, controller)

def turn_on(self, **kwargs: Any) -> None:
"""Turn the switch on."""
self._lutron_device.level = 100
Expand All @@ -64,15 +57,13 @@ def extra_state_attributes(self) -> Mapping[str, Any] | None:
"""Return the state attributes."""
return {"lutron_integration_id": self._lutron_device.id}

@property
def is_on(self) -> bool:
"""Return true if device is on."""
return self._lutron_device.last_level() > 0
def _request_state(self) -> None:
"""Request the state from the device."""
self._lutron_device.level # pylint: disable=pointless-statement

def update(self) -> None:
"""Call when forcing a refresh of the device."""
if self._prev_state is None:
self._prev_state = self._lutron_device.level > 0
def _update_attrs(self) -> None:
"""Update the state attributes."""
self._attr_is_on = self._lutron_device.last_level() > 0


class LutronLed(LutronKeypad, SwitchEntity):
Expand Down Expand Up @@ -110,12 +101,10 @@ def extra_state_attributes(self) -> Mapping[str, Any] | None:
"led": self._lutron_device.name,
}

@property
def is_on(self) -> bool:
"""Return true if device is on."""
return self._lutron_device.last_state

def update(self) -> None:
"""Call when forcing a refresh of the device."""
# The following property getter actually triggers an update in Lutron
def _request_state(self) -> None:
"""Request the state from the device."""
self._lutron_device.state # pylint: disable=pointless-statement

def _update_attrs(self) -> None:
"""Update the state attributes."""
self._attr_is_on = self._lutron_device.last_state