Automatically retry lost/timed out LIFX requests (#91157)

homeassistant/components/lifx/config_flow.py

@@ -18,12 +18,19 @@
 from homeassistant.helpers import device_registry as dr
 from homeassistant.helpers.typing import DiscoveryInfoType
 
-from .const import _LOGGER, CONF_SERIAL, DOMAIN, TARGET_ANY
+from .const import (
+ _LOGGER,
+ CONF_SERIAL,
+ DEFAULT_ATTEMPTS,
+ DOMAIN,
+ OVERALL_TIMEOUT,
+ TARGET_ANY,
+)
 from .discovery import async_discover_devices
 from .util import (
  async_entry_is_legacy,
- async_execute_lifx,
  async_get_legacy_entry,
+ async_multi_execute_lifx_with_retries,
  formatted_serial,
  lifx_features,
  mac_matches_serial_number,
@@ -225,13 +232,15 @@ async def _async_try_connect(
  # get_version required for lifx_features()
  # get_label required to log the name of the device
  # get_group required to populate suggested areas
- messages = await asyncio.gather(
- *[
- async_execute_lifx(device.get_hostfirmware),
- async_execute_lifx(device.get_version),
- async_execute_lifx(device.get_label),
- async_execute_lifx(device.get_group),
- ]
+ messages = await async_multi_execute_lifx_with_retries(
+ [
+ device.get_hostfirmware,
+ device.get_version,
+ device.get_label,
+ device.get_group,
+ ],
+ DEFAULT_ATTEMPTS,
+ OVERALL_TIMEOUT,
  )
  except asyncio.TimeoutError:
  return None

homeassistant/components/lifx/const.py

@@ -7,11 +7,22 @@
 TARGET_ANY = "00:00:00:00:00:00"
 
 DISCOVERY_INTERVAL = 10
-MESSAGE_TIMEOUT = 1.65
-MESSAGE_RETRIES = 5
-OVERALL_TIMEOUT = 9
+# The number of seconds before we will no longer accept a response
+# to a message and consider it invalid
+MESSAGE_TIMEOUT = 18
+# Disable the retries in the library since they are not spaced out
+# enough to account for WiFi and UDP dropouts
+MESSAGE_RETRIES = 1
+OVERALL_TIMEOUT = 15
 UNAVAILABLE_GRACE = 90
 
+# The number of times to retry a request message
+DEFAULT_ATTEMPTS = 5
+# The maximum time to wait for a bulb to respond to an update
+MAX_UPDATE_TIME = 90
+# The number of tries to send each request message to a bulb during an update
+MAX_ATTEMPTS_PER_UPDATE_REQUEST_MESSAGE = 5
+
 CONF_LABEL = "label"
 CONF_SERIAL = "serial"
 
@@ -50,4 +61,5 @@
 }
 DATA_LIFX_MANAGER = "lifx_manager"
 
+
 _LOGGER = logging.getLogger(__package__)

homeassistant/components/lifx/coordinator.py

@@ -28,28 +28,32 @@
 from homeassistant.exceptions import HomeAssistantError
 from homeassistant.helpers import entity_registry as er
 from homeassistant.helpers.debounce import Debouncer
-from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed
+from homeassistant.helpers.update_coordinator import DataUpdateCoordinator
 
 from .const import (
  _LOGGER,
  ATTR_REMAINING,
+ DEFAULT_ATTEMPTS,
  DOMAIN,
  IDENTIFY_WAVEFORM,
+ MAX_ATTEMPTS_PER_UPDATE_REQUEST_MESSAGE,
+ MAX_UPDATE_TIME,
  MESSAGE_RETRIES,
  MESSAGE_TIMEOUT,
+ OVERALL_TIMEOUT,
  TARGET_ANY,
  UNAVAILABLE_GRACE,
 )
 from .util import (
  async_execute_lifx,
+ async_multi_execute_lifx_with_retries,
  get_real_mac_addr,
  infrared_brightness_option_to_value,
  infrared_brightness_value_to_option,
  lifx_features,
 )
 
 LIGHT_UPDATE_INTERVAL = 10
-SENSOR_UPDATE_INTERVAL = 30
 REQUEST_REFRESH_DELAY = 0.35
 LIFX_IDENTIFY_DELAY = 3.0
 RSSI_DBM_FW = AwesomeVersion("2.77")
@@ -186,60 +190,84 @@ def async_get_entity_id(self, platform: Platform, key: str) -> str | None:
  platform, DOMAIN, f"{self.serial_number}_{key}"
  )
 
- async def _async_update_data(self) -> None:
- """Fetch all device data from the api."""
- async with self.lock:
- if self.device.host_firmware_version is None:
- self.device.get_hostfirmware()
- if self.device.product is None:
- self.device.get_version()
- if self.device.group is None:
- self.device.get_group()
-
- response = await async_execute_lifx(self.device.get_color)
-
- if self.device.product is None:
- raise UpdateFailed(
- f"Failed to fetch get version from device: {self.device.ip_addr}"
- )
-
- # device.mac_addr is not the mac_address, its the serial number
- if self.device.mac_addr == TARGET_ANY:
- self.device.mac_addr = response.target_addr
+ async def _async_populate_device_info(self) -> None:
+ """Populate device info."""
+ methods: list[Callable] = []
+ device = self.device
+ if self.device.host_firmware_version is None:
+ methods.append(device.get_hostfirmware)
+ if self.device.product is None:
+ methods.append(device.get_version)
+ if self.device.group is None:
+ methods.append(device.get_group)
+ assert methods, "Device info already populated"
+ await async_multi_execute_lifx_with_retries(
+ methods, DEFAULT_ATTEMPTS, OVERALL_TIMEOUT
+ )
 
- if self._update_rssi is True:
- await self.async_update_rssi()
+ @callback
+ def _async_build_color_zones_update_requests(self) -> list[Callable]:
+ """Build a color zones update request."""
+ device = self.device
+ return [
+ partial(device.get_color_zones, start_index=zone)
+ for zone in range(0, len(device.color_zones), 8)
+ ]
 
- # Update extended multizone devices
- if lifx_features(self.device)["extended_multizone"]:
- await self.async_get_extended_color_zones()
- await self.async_get_multizone_effect()
- # use legacy methods for older devices
- elif lifx_features(self.device)["multizone"]:
- await self.async_get_color_zones()
- await self.async_get_multizone_effect()
+ async def _async_update_data(self) -> None:
+ """Fetch all device data from the api."""
+ device = self.device
+ if (
+ device.host_firmware_version is None
+ or device.product is None
+ or device.group is None
+ ):
+ await self._async_populate_device_info()
+
+ num_zones = len(device.color_zones) if device.color_zones is not None else 0
+ features = lifx_features(self.device)
+ is_extended_multizone = features["extended_multizone"]
+ is_legacy_multizone = not is_extended_multizone and features["multizone"]
+ update_rssi = self._update_rssi
+ methods: list[Callable] = [self.device.get_color]
+ if update_rssi:
+ methods.append(self.device.get_wifiinfo)
+ if is_extended_multizone:
+ methods.append(self.device.get_extended_color_zones)
+ elif is_legacy_multizone:
+ methods.extend(self._async_build_color_zones_update_requests())
+ if is_extended_multizone or is_legacy_multizone:
+ methods.append(self.device.get_multizone_effect)
+ if features["hev"]:
+ methods.append(self.device.get_hev_cycle)
+ if features["infrared"]:
+ methods.append(self.device.get_infrared)
+
+ responses = await async_multi_execute_lifx_with_retries(
+ methods, MAX_ATTEMPTS_PER_UPDATE_REQUEST_MESSAGE, MAX_UPDATE_TIME
+ )
+ # device.mac_addr is not the mac_address, its the serial number
+ if device.mac_addr == TARGET_ANY:
+ device.mac_addr = responses[0].target_addr
 
- if lifx_features(self.device)["hev"]:
- await self.async_get_hev_cycle()
+ if update_rssi:
+ # We always send the rssi request second
+ self._rssi = int(floor(10 * log10(responses[1].signal) + 0.5))
 
- if lifx_features(self.device)["infrared"]:
- await async_execute_lifx(self.device.get_infrared)
+ if is_extended_multizone or is_legacy_multizone:
+ self.active_effect = FirmwareEffect[self.device.effect.get("effect", "OFF")]
+ if is_legacy_multizone and num_zones != len(device.color_zones):
+ # The number of zones has changed so we need
+ # to update the zones again. This happens rarely.
+ await self.async_get_color_zones()
 
  async def async_get_color_zones(self) -> None:
  """Get updated color information for each zone."""
- zone = 0
- top = 1
- while zone < top:
- # Each get_color_zones can update 8 zones at once
- resp = await async_execute_lifx(
- partial(self.device.get_color_zones, start_index=zone)
- )
- zone += 8
- top = resp.count
-
- # We only await multizone responses so don't ask for just one
- if zone == top - 1:
- zone -= 1
+ await async_multi_execute_lifx_with_retries(
+ self._async_build_color_zones_update_requests(),
+ DEFAULT_ATTEMPTS,
+ OVERALL_TIMEOUT,
+ )
 
  async def async_get_extended_color_zones(self) -> None:
  """Get updated color information for all zones."""
@@ -323,11 +351,6 @@ async def async_set_extended_color_zones(
  )
  )
 
- async def async_get_multizone_effect(self) -> None:
- """Update the device firmware effect running state."""
- await async_execute_lifx(self.device.get_multizone_effect)
- self.active_effect = FirmwareEffect[self.device.effect.get("effect", "OFF")]
-
  async def async_set_multizone_effect(
  self,
  effect: str,
@@ -415,22 +438,12 @@ def _async_disable_rssi_updates() -> None:
  self._update_rssi = True
  return _async_disable_rssi_updates
 
- async def async_update_rssi(self) -> None:
- """Update RSSI value."""
- resp = await async_execute_lifx(self.device.get_wifiinfo)
- self._rssi = int(floor(10 * log10(resp.signal) + 0.5))
-
  def async_get_hev_cycle_state(self) -> bool | None:
  """Return the current HEV cycle state."""
  if self.device.hev_cycle is None:
  return None
  return bool(self.device.hev_cycle.get(ATTR_REMAINING, 0) > 0)
 
- async def async_get_hev_cycle(self) -> None:
- """Update the HEV cycle status from a LIFX Clean bulb."""
- if lifx_features(self.device)["hev"]:
- await async_execute_lifx(self.device.get_hev_cycle)
-
  async def async_set_hev_cycle_state(self, enable: bool, duration: int = 0) -> None:
  """Start or stop an HEV cycle on a LIFX Clean bulb."""
  if lifx_features(self.device)["hev"]:

homeassistant/components/lifx/light.py

@@ -206,61 +206,60 @@ async def async_turn_off(self, **kwargs: Any) -> None:
  async def set_state(self, **kwargs: Any) -> None:
  """Set a color on the light and turn it on/off."""
  self.coordinator.async_set_updated_data(None)
- async with self.coordinator.lock:
- # Cancel any pending refreshes
- bulb = self.bulb
+ # Cancel any pending refreshes
+ bulb = self.bulb
 
-  await self.effects_conductor.stop([bulb])
+ await self.effects_conductor.stop([bulb])
 
-  if ATTR_EFFECT in kwargs:
-  await self.default_effect(**kwargs)
-  return
+ if ATTR_EFFECT in kwargs:
+ await self.default_effect(**kwargs)
+ return
 
-  if ATTR_INFRARED in kwargs:
-  infrared_entity_id = self.coordinator.async_get_entity_id(
-  Platform.SELECT, INFRARED_BRIGHTNESS
-  )
-  _LOGGER.warning(
-  (
-  "The 'infrared' attribute of 'lifx.set_state' is deprecated:"
-  " call 'select.select_option' targeting '%s' instead"
-  ),
-  infrared_entity_id,
-  )
-  bulb.set_infrared(convert_8_to_16(kwargs[ATTR_INFRARED]))
-
-  if ATTR_TRANSITION in kwargs:
-  fade = int(kwargs[ATTR_TRANSITION] * 1000)
-  else:
-  fade = 0
-
-  # These are both False if ATTR_POWER is not set
-  power_on = kwargs.get(ATTR_POWER, False)
-  power_off = not kwargs.get(ATTR_POWER, True)
-
-  hsbk = find_hsbk(self.hass, **kwargs)
-
-  if not self.is_on:
-  if power_off:
-  await self.set_power(False)
-  # If fading on with color, set color immediately
-  if hsbk and power_on:
-  await self.set_color(hsbk, kwargs)
-  await self.set_power(True, duration=fade)
-  elif hsbk:
-  await self.set_color(hsbk, kwargs, duration=fade)
-  elif power_on:
-  await self.set_power(True, duration=fade)
-  else:
-  if power_on:
-  await self.set_power(True)
-  if hsbk:
-  await self.set_color(hsbk, kwargs, duration=fade)
-  if power_off:
-  await self.set_power(False, duration=fade)
-
-  # Avoid state ping-pong by holding off updates as the state settles
-  await asyncio.sleep(LIFX_STATE_SETTLE_DELAY)
+ if ATTR_INFRARED in kwargs:
+ infrared_entity_id = self.coordinator.async_get_entity_id(
+ Platform.SELECT, INFRARED_BRIGHTNESS
+ )
+ _LOGGER.warning(
+ (
+ "The 'infrared' attribute of 'lifx.set_state' is deprecated:"
+ " call 'select.select_option' targeting '%s' instead"
+ ),
+ infrared_entity_id,
+ )
+ bulb.set_infrared(convert_8_to_16(kwargs[ATTR_INFRARED]))
+
+ if ATTR_TRANSITION in kwargs:
+ fade = int(kwargs[ATTR_TRANSITION] * 1000)
+ else:
+ fade = 0
+
+ # These are both False if ATTR_POWER is not set
+ power_on = kwargs.get(ATTR_POWER, False)
+ power_off = not kwargs.get(ATTR_POWER, True)
+
+ hsbk = find_hsbk(self.hass, **kwargs)
+
+ if not self.is_on:
+ if power_off:
+ await self.set_power(False)
+ # If fading on with color, set color immediately
+ if hsbk and power_on:
+ await self.set_color(hsbk, kwargs)
+ await self.set_power(True, duration=fade)
+ elif hsbk:
+ await self.set_color(hsbk, kwargs, duration=fade)
+ elif power_on:
+ await self.set_power(True, duration=fade)
+ else:
+ if power_on:
+ await self.set_power(True)
+ if hsbk:
+ await self.set_color(hsbk, kwargs, duration=fade)
+ if power_off:
+ await self.set_power(False, duration=fade)
+
+ # Avoid state ping-pong by holding off updates as the state settles
+ await asyncio.sleep(LIFX_STATE_SETTLE_DELAY)
 
  # Update when the transition starts and ends
  await self.update_during_transition(fade)