Skip to content

Commit

Permalink
⚡️ Implement delayed refresh for SignalRGB light
Browse files Browse the repository at this point in the history
Add _schedule_delayed_refresh and _delayed_refresh methods to
SignalRGBLight class. These methods ensure effect application
is verified after a short delay, with retry logic for failed
applications. Update tests to accommodate new functionality.
  • Loading branch information
hyperb1iss committed Aug 14, 2024
1 parent 48f2ee9 commit d21aa7a
Show file tree
Hide file tree
Showing 3 changed files with 123 additions and 15 deletions.
86 changes: 76 additions & 10 deletions custom_components/signalrgb/light.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

from __future__ import annotations

import asyncio
from datetime import timedelta
from typing import Any

Expand Down Expand Up @@ -112,7 +113,10 @@ def __init__(
self._current_effect: Effect | None = None
self._is_on: bool = False
self._brightness: int = 0 # This is now 0-100
self._pending_effect: str | None = None
self._requested_effect: str | None = None
self._retry_count: int = 0
self._max_retries: int = 3
self._refresh_task: asyncio.Task | None = None
LOGGER.debug("SignalRGBLight initialized: %s", self.entity_id)

async def async_added_to_hass(self) -> None:
Expand Down Expand Up @@ -179,6 +183,7 @@ def extra_state_attributes(self) -> dict[str, Any]:
async def async_turn_on(self, **kwargs: Any) -> None:
"""Instruct the light to turn on."""
LOGGER.debug("Turning on %s with kwargs: %s", self.entity_id, kwargs)

if not self.is_on:
LOGGER.debug("Light was off, turning on")
await self.hass.async_add_executor_job(
Expand All @@ -201,11 +206,12 @@ async def async_turn_on(self, **kwargs: Any) -> None:

if ATTR_EFFECT in kwargs:
effect = kwargs[ATTR_EFFECT]
LOGGER.debug("Applying effect: %s", effect)
LOGGER.debug("Requesting effect: %s", effect)
self._requested_effect = effect
self._retry_count = 0
await self._apply_effect(effect)

# Schedule a delayed refresh
await self.coordinator.async_request_refresh()
self._schedule_delayed_refresh()

async def async_turn_off(self, **kwargs: Any) -> None:
"""Instruct the light to turn off."""
Expand All @@ -230,15 +236,50 @@ async def _apply_effect(self, effect: str) -> None:

# Update state immediately
self._current_effect = effect_obj
self._pending_effect = effect
self.async_write_ha_state()
LOGGER.debug("Effect applied and state updated immediately: %s", effect)

# await self.coordinator.async_request_refresh()
except SignalRGBException as err:
LOGGER.error("Failed to apply effect %s: %s", effect, err)
raise HomeAssistantError(f"Failed to apply effect: {err}") from err

def _schedule_delayed_refresh(self) -> None:
"""Schedule a delayed refresh to verify the effect was applied correctly."""
if self._refresh_task:
self._refresh_task.cancel()

self._refresh_task = asyncio.create_task(self._delayed_refresh())

async def _delayed_refresh(self) -> None:
"""Perform a delayed refresh and retry if necessary."""
await asyncio.sleep(2) # Wait for 2 seconds before refreshing
await self.coordinator.async_request_refresh()

if self._requested_effect and self.effect != self._requested_effect:
LOGGER.warning(
"Applied effect doesn't match requested effect. "
"Requested: %s, Applied: %s",
self._requested_effect,
self.effect,
)
if self._retry_count < self._max_retries:
self._retry_count += 1
LOGGER.debug(
"Retrying effect application (Attempt %s of %s)",
self._retry_count,
self._max_retries,
)
await self._apply_effect(self._requested_effect)
self._schedule_delayed_refresh()
else:
LOGGER.error(
"Failed to apply effect %s after %s attempts",
self._requested_effect,
self._max_retries,
)
self._requested_effect = None
self._retry_count = 0

async def async_update_effect_list(self) -> None:
"""Update the list of available effects."""
LOGGER.debug("Updating effect list for %s", self.entity_id)
Expand Down Expand Up @@ -266,16 +307,25 @@ def _handle_coordinator_update(self) -> None:
):
self._current_effect = new_effect
if (
self._pending_effect
and new_effect.attributes.name != self._pending_effect
self._requested_effect
and new_effect.attributes.name != self._requested_effect
):
LOGGER.warning(
"Applied effect doesn't match requested effect. "
"Requested: %s, Applied: %s",
self._pending_effect,
self._requested_effect,
new_effect.attributes.name,
)
self._pending_effect = None
elif (
self._requested_effect
and new_effect.attributes.name == self._requested_effect
):
LOGGER.info(
"Requested effect %s successfully applied",
self._requested_effect,
)
self._requested_effect = None
self._retry_count = 0

LOGGER.debug(
"Updated state - Effect: %s, Is On: %s, Brightness: %s",
Expand All @@ -289,3 +339,19 @@ def _handle_coordinator_update(self) -> None:
LOGGER.warning("No data received from coordinator for %s", self.entity_id)
self.async_write_ha_state()
LOGGER.debug("State updated after coordinator update")

async def async_will_remove_from_hass(self) -> None:
"""Clean up resources when entity is removed."""
if self._refresh_task and not self._refresh_task.done():
self._refresh_task.cancel()
try:
# Wait for the task to be cancelled, but don't wait indefinitely
await asyncio.wait([self._refresh_task], timeout=1)
except asyncio.TimeoutError:
pass # The task didn't finish cancelling in time, but that's okay
await super().async_will_remove_from_hass()

# For testing delayed refresh
def _cancel_refresh_task(self):
if self._refresh_task:
self._refresh_task.cancel()
2 changes: 2 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
"""Pytest configuration file for the tests directory."""

# pylint: disable=unused-argument

import os
import sys
from pathlib import Path
Expand Down
50 changes: 45 additions & 5 deletions tests/test_light.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

# pylint: disable=protected-access, redefined-outer-name

import asyncio
from unittest.mock import AsyncMock, MagicMock, patch

import pytest
Expand Down Expand Up @@ -64,6 +65,7 @@ def mock_coordinator():
"""Mock DataUpdateCoordinator."""
coordinator = MagicMock(spec=DataUpdateCoordinator)
coordinator.data = None
coordinator.async_request_refresh = AsyncMock()
return coordinator


Expand Down Expand Up @@ -103,17 +105,27 @@ async def test_brightness(self, mock_light):

async def test_turn_on(self, mock_light, mock_coordinator):
"""Test turning on the light."""
await mock_light.async_turn_on()
with patch("asyncio.sleep", new_callable=AsyncMock):
await mock_light.async_turn_on()

mock_light.hass.async_add_executor_job.assert_any_call(
setattr, mock_light._client, "enabled", True
)
assert mock_light._is_on is True
assert mock_light.async_write_ha_state.call_count == 1 # Once for turning on

# Run the delayed refresh
await mock_light._delayed_refresh()
mock_coordinator.async_request_refresh.assert_called_once()

# Clean up
mock_light._cancel_refresh_task()

async def test_turn_on_with_brightness(self, mock_light, mock_coordinator):
"""Test turning on the light with brightness."""
await mock_light.async_turn_on(**{ATTR_BRIGHTNESS: 128})
with patch("asyncio.sleep", new_callable=AsyncMock):
await mock_light.async_turn_on(**{ATTR_BRIGHTNESS: 128})

mock_light.hass.async_add_executor_job.assert_any_call(
setattr, mock_light._client, "enabled", True
)
Expand All @@ -125,8 +137,14 @@ async def test_turn_on_with_brightness(self, mock_light, mock_coordinator):
assert (
mock_light.async_write_ha_state.call_count == 2
) # Once for on, once for brightness

# Run the delayed refresh
await mock_light._delayed_refresh()
mock_coordinator.async_request_refresh.assert_called_once()

# Clean up
mock_light._cancel_refresh_task()

async def test_turn_on_with_effect(self, mock_light, mock_coordinator):
"""Test turning on the light with an effect."""
mock_effect = "Rainbow Wave"
Expand All @@ -140,18 +158,25 @@ async def test_turn_on_with_effect(self, mock_light, mock_coordinator):
None, # For apply_effect
]

await mock_light.async_turn_on(**{ATTR_EFFECT: mock_effect})
with patch("asyncio.sleep", new_callable=AsyncMock):
await mock_light.async_turn_on(**{ATTR_EFFECT: mock_effect})

assert mock_light._is_on is True
assert mock_light._current_effect == mock_effect_obj
assert (
mock_light.async_write_ha_state.call_count == 2
) # Once for on, once for effect

# Run the delayed refresh
await mock_light._delayed_refresh()
mock_coordinator.async_request_refresh.assert_called_once()

# Clean up
mock_light._cancel_refresh_task()

async def test_turn_off(self, mock_light, mock_coordinator):
"""Test turning off the light."""
with patch("asyncio.sleep", new_callable=AsyncMock):
await mock_light.async_turn_off()
await mock_light.async_turn_off()
mock_light.hass.async_add_executor_job.assert_called_with(
setattr, mock_light._client, "enabled", False
)
Expand Down Expand Up @@ -241,3 +266,18 @@ async def test_extra_state_attributes(self, mock_light):
# Test when light is off
mock_light._is_on = False
assert mock_light.extra_state_attributes == {}

async def test_async_will_remove_from_hass(self, mock_light):
"""Test the async_will_remove_from_hass method."""

# Create a real asyncio.Task for _refresh_task
async def mock_refresh():
await asyncio.sleep(10) # Simulate a long-running task

mock_light._refresh_task = asyncio.create_task(mock_refresh())

# Call the method we're testing
await mock_light.async_will_remove_from_hass()

# Assert that the task was cancelled
assert mock_light._refresh_task.cancelled()

0 comments on commit d21aa7a

Please sign in to comment.