Only log xiaomi_miio update exceptions once (#41226)

* xiaomi_miio: only log update exceptions once

Replaces #37695

* add som more missed exception logger cases + do not change the control flow as pointed out by @cgtobi

* Use patch&MagickMock from tests.async_mock

* Fix linting for alarm_control_panel

* update the test to verify that the warning on update is only logged when the device was previously available
This commit is contained in:
Teemu R 2020-10-25 01:53:36 +02:00 committed by Paulus Schoutsen
parent 9c60195780
commit 0c1c7d797c
9 changed files with 127 additions and 87 deletions

View file

@ -195,8 +195,9 @@ class AirMonitorS1(AirMonitorB1):
self._humidity = state.humidity self._humidity = state.humidity
self._available = True self._available = True
except DeviceException as ex: except DeviceException as ex:
self._available = False if self._available:
_LOGGER.error("Got exception while fetching the state: %s", ex) self._available = False
_LOGGER.error("Got exception while fetching the state: %s", ex)
class AirMonitorV1(AirMonitorB1): class AirMonitorV1(AirMonitorB1):
@ -210,8 +211,9 @@ class AirMonitorV1(AirMonitorB1):
self._air_quality_index = state.aqi self._air_quality_index = state.aqi
self._available = True self._available = True
except DeviceException as ex: except DeviceException as ex:
self._available = False if self._available:
_LOGGER.error("Got exception while fetching the state: %s", ex) self._available = False
_LOGGER.error("Got exception while fetching the state: %s", ex)
@property @property
def unit_of_measurement(self): def unit_of_measurement(self):

View file

@ -3,7 +3,7 @@
from functools import partial from functools import partial
import logging import logging
from miio.gateway import GatewayException from miio import DeviceException
from homeassistant.components.alarm_control_panel import ( from homeassistant.components.alarm_control_panel import (
SUPPORT_ALARM_ARM_AWAY, SUPPORT_ALARM_ARM_AWAY,
@ -103,7 +103,7 @@ class XiaomiGatewayAlarm(AlarmControlPanelEntity):
partial(func, *args, **kwargs) partial(func, *args, **kwargs)
) )
_LOGGER.debug("Response received from miio device: %s", result) _LOGGER.debug("Response received from miio device: %s", result)
except GatewayException as exc: except DeviceException as exc:
_LOGGER.error(mask_error, exc) _LOGGER.error(mask_error, exc)
async def async_alarm_arm_away(self, code=None): async def async_alarm_arm_away(self, code=None):
@ -122,9 +122,11 @@ class XiaomiGatewayAlarm(AlarmControlPanelEntity):
"""Fetch state from the device.""" """Fetch state from the device."""
try: try:
state = await self.hass.async_add_executor_job(self._gateway.alarm.status) state = await self.hass.async_add_executor_job(self._gateway.alarm.status)
except GatewayException as ex: except DeviceException as ex:
self._available = False if self._available:
_LOGGER.error("Got exception while fetching the state: %s", ex) self._available = False
_LOGGER.error("Got exception while fetching the state: %s", ex)
return return
_LOGGER.debug("Got new state: %s", state) _LOGGER.debug("Got new state: %s", state)

View file

@ -655,8 +655,10 @@ class XiaomiGenericDevice(FanEntity):
return result == SUCCESS return result == SUCCESS
except DeviceException as exc: except DeviceException as exc:
_LOGGER.error(mask_error, exc) if self._available:
self._available = False _LOGGER.error(mask_error, exc)
self._available = False
return False return False
async def async_turn_on(self, speed: str = None, **kwargs) -> None: async def async_turn_on(self, speed: str = None, **kwargs) -> None:
@ -785,8 +787,9 @@ class XiaomiAirPurifier(XiaomiGenericDevice):
) )
except DeviceException as ex: except DeviceException as ex:
self._available = False if self._available:
_LOGGER.error("Got exception while fetching the state: %s", ex) self._available = False
_LOGGER.error("Got exception while fetching the state: %s", ex)
@property @property
def speed_list(self) -> list: def speed_list(self) -> list:
@ -1029,8 +1032,9 @@ class XiaomiAirHumidifier(XiaomiGenericDevice):
) )
except DeviceException as ex: except DeviceException as ex:
self._available = False if self._available:
_LOGGER.error("Got exception while fetching the state: %s", ex) self._available = False
_LOGGER.error("Got exception while fetching the state: %s", ex)
@property @property
def speed_list(self) -> list: def speed_list(self) -> list:
@ -1138,8 +1142,9 @@ class XiaomiAirFresh(XiaomiGenericDevice):
) )
except DeviceException as ex: except DeviceException as ex:
self._available = False if self._available:
_LOGGER.error("Got exception while fetching the state: %s", ex) self._available = False
_LOGGER.error("Got exception while fetching the state: %s", ex)
@property @property
def speed_list(self) -> list: def speed_list(self) -> list:

View file

@ -65,7 +65,7 @@ class XiaomiGatewayDevice(Entity):
self._entry = entry self._entry = entry
self._unique_id = sub_device.sid self._unique_id = sub_device.sid
self._name = f"{sub_device.name} ({sub_device.sid})" self._name = f"{sub_device.name} ({sub_device.sid})"
self._available = None self._available = False
@property @property
def unique_id(self): def unique_id(self):
@ -100,5 +100,6 @@ class XiaomiGatewayDevice(Entity):
await self.hass.async_add_executor_job(self._sub_device.update) await self.hass.async_add_executor_job(self._sub_device.update)
self._available = True self._available = True
except gateway.GatewayException as ex: except gateway.GatewayException as ex:
self._available = False if self._available:
_LOGGER.error("Got exception while fetching the state: %s", ex) self._available = False
_LOGGER.error("Got exception while fetching the state: %s", ex)

View file

@ -324,8 +324,10 @@ class XiaomiPhilipsAbstractLight(LightEntity):
return result == SUCCESS return result == SUCCESS
except DeviceException as exc: except DeviceException as exc:
_LOGGER.error(mask_error, exc) if self._available:
self._available = False _LOGGER.error(mask_error, exc)
self._available = False
return False return False
async def async_turn_on(self, **kwargs): async def async_turn_on(self, **kwargs):
@ -356,8 +358,10 @@ class XiaomiPhilipsAbstractLight(LightEntity):
try: try:
state = await self.hass.async_add_executor_job(self._light.status) state = await self.hass.async_add_executor_job(self._light.status)
except DeviceException as ex: except DeviceException as ex:
self._available = False if self._available:
_LOGGER.error("Got exception while fetching the state: %s", ex) self._available = False
_LOGGER.error("Got exception while fetching the state: %s", ex)
return return
_LOGGER.debug("Got new state: %s", state) _LOGGER.debug("Got new state: %s", state)
@ -380,8 +384,10 @@ class XiaomiPhilipsGenericLight(XiaomiPhilipsAbstractLight):
try: try:
state = await self.hass.async_add_executor_job(self._light.status) state = await self.hass.async_add_executor_job(self._light.status)
except DeviceException as ex: except DeviceException as ex:
self._available = False if self._available:
_LOGGER.error("Got exception while fetching the state: %s", ex) self._available = False
_LOGGER.error("Got exception while fetching the state: %s", ex)
return return
_LOGGER.debug("Got new state: %s", state) _LOGGER.debug("Got new state: %s", state)
@ -536,8 +542,10 @@ class XiaomiPhilipsBulb(XiaomiPhilipsGenericLight):
try: try:
state = await self.hass.async_add_executor_job(self._light.status) state = await self.hass.async_add_executor_job(self._light.status)
except DeviceException as ex: except DeviceException as ex:
self._available = False if self._available:
_LOGGER.error("Got exception while fetching the state: %s", ex) self._available = False
_LOGGER.error("Got exception while fetching the state: %s", ex)
return return
_LOGGER.debug("Got new state: %s", state) _LOGGER.debug("Got new state: %s", state)
@ -593,8 +601,10 @@ class XiaomiPhilipsCeilingLamp(XiaomiPhilipsBulb):
try: try:
state = await self.hass.async_add_executor_job(self._light.status) state = await self.hass.async_add_executor_job(self._light.status)
except DeviceException as ex: except DeviceException as ex:
self._available = False if self._available:
_LOGGER.error("Got exception while fetching the state: %s", ex) self._available = False
_LOGGER.error("Got exception while fetching the state: %s", ex)
return return
_LOGGER.debug("Got new state: %s", state) _LOGGER.debug("Got new state: %s", state)
@ -637,8 +647,10 @@ class XiaomiPhilipsEyecareLamp(XiaomiPhilipsGenericLight):
try: try:
state = await self.hass.async_add_executor_job(self._light.status) state = await self.hass.async_add_executor_job(self._light.status)
except DeviceException as ex: except DeviceException as ex:
self._available = False if self._available:
_LOGGER.error("Got exception while fetching the state: %s", ex) self._available = False
_LOGGER.error("Got exception while fetching the state: %s", ex)
return return
_LOGGER.debug("Got new state: %s", state) _LOGGER.debug("Got new state: %s", state)
@ -778,8 +790,10 @@ class XiaomiPhilipsEyecareLampAmbientLight(XiaomiPhilipsAbstractLight):
try: try:
state = await self.hass.async_add_executor_job(self._light.status) state = await self.hass.async_add_executor_job(self._light.status)
except DeviceException as ex: except DeviceException as ex:
self._available = False if self._available:
_LOGGER.error("Got exception while fetching the state: %s", ex) self._available = False
_LOGGER.error("Got exception while fetching the state: %s", ex)
return return
_LOGGER.debug("Got new state: %s", state) _LOGGER.debug("Got new state: %s", state)
@ -932,8 +946,10 @@ class XiaomiPhilipsMoonlightLamp(XiaomiPhilipsBulb):
try: try:
state = await self.hass.async_add_executor_job(self._light.status) state = await self.hass.async_add_executor_job(self._light.status)
except DeviceException as ex: except DeviceException as ex:
self._available = False if self._available:
_LOGGER.error("Got exception while fetching the state: %s", ex) self._available = False
_LOGGER.error("Got exception while fetching the state: %s", ex)
return return
_LOGGER.debug("Got new state: %s", state) _LOGGER.debug("Got new state: %s", state)

View file

@ -233,8 +233,9 @@ class XiaomiAirQualityMonitor(Entity):
) )
except DeviceException as ex: except DeviceException as ex:
self._available = False if self._available:
_LOGGER.error("Got exception while fetching the state: %s", ex) self._available = False
_LOGGER.error("Got exception while fetching the state: %s", ex)
class XiaomiGatewaySensor(XiaomiGatewayDevice): class XiaomiGatewaySensor(XiaomiGatewayDevice):

View file

@ -280,8 +280,10 @@ class XiaomiPlugGenericSwitch(SwitchEntity):
return result == SUCCESS return result == SUCCESS
except DeviceException as exc: except DeviceException as exc:
_LOGGER.error(mask_error, exc) if self._available:
self._available = False _LOGGER.error(mask_error, exc)
self._available = False
return False return False
async def async_turn_on(self, **kwargs): async def async_turn_on(self, **kwargs):
@ -316,8 +318,9 @@ class XiaomiPlugGenericSwitch(SwitchEntity):
self._state_attrs[ATTR_TEMPERATURE] = state.temperature self._state_attrs[ATTR_TEMPERATURE] = state.temperature
except DeviceException as ex: except DeviceException as ex:
self._available = False if self._available:
_LOGGER.error("Got exception while fetching the state: %s", ex) self._available = False
_LOGGER.error("Got exception while fetching the state: %s", ex)
async def async_set_wifi_led_on(self): async def async_set_wifi_led_on(self):
"""Turn the wifi led on.""" """Turn the wifi led on."""
@ -402,8 +405,9 @@ class XiaomiPowerStripSwitch(XiaomiPlugGenericSwitch):
self._state_attrs[ATTR_POWER_PRICE] = state.power_price self._state_attrs[ATTR_POWER_PRICE] = state.power_price
except DeviceException as ex: except DeviceException as ex:
self._available = False if self._available:
_LOGGER.error("Got exception while fetching the state: %s", ex) self._available = False
_LOGGER.error("Got exception while fetching the state: %s", ex)
async def async_set_power_mode(self, mode: str): async def async_set_power_mode(self, mode: str):
"""Set the power mode.""" """Set the power mode."""
@ -492,8 +496,9 @@ class ChuangMiPlugSwitch(XiaomiPlugGenericSwitch):
self._state_attrs[ATTR_LOAD_POWER] = state.load_power self._state_attrs[ATTR_LOAD_POWER] = state.load_power
except DeviceException as ex: except DeviceException as ex:
self._available = False if self._available:
_LOGGER.error("Got exception while fetching the state: %s", ex) self._available = False
_LOGGER.error("Got exception while fetching the state: %s", ex)
class XiaomiAirConditioningCompanionSwitch(XiaomiPlugGenericSwitch): class XiaomiAirConditioningCompanionSwitch(XiaomiPlugGenericSwitch):
@ -541,5 +546,6 @@ class XiaomiAirConditioningCompanionSwitch(XiaomiPlugGenericSwitch):
self._state_attrs[ATTR_LOAD_POWER] = state.load_power self._state_attrs[ATTR_LOAD_POWER] = state.load_power
except DeviceException as ex: except DeviceException as ex:
self._available = False if self._available:
_LOGGER.error("Got exception while fetching the state: %s", ex) self._available = False
_LOGGER.error("Got exception while fetching the state: %s", ex)

View file

@ -485,10 +485,10 @@ class MiroboVacuum(StateVacuumEntity):
self.dnd_state = self._vacuum.dnd_status() self.dnd_state = self._vacuum.dnd_status()
self._available = True self._available = True
except OSError as exc: except (OSError, DeviceException) as exc:
_LOGGER.error("Got OSError while fetching the state: %s", exc) if self._available:
except DeviceException as exc: self._available = False
_LOGGER.warning("Got exception while fetching the state: %s", exc) _LOGGER.warning("Got exception while fetching the state: %s", exc)
# Fetch timers separately, see #38285 # Fetch timers separately, see #38285
try: try:

View file

@ -2,6 +2,7 @@
from datetime import datetime, time, timedelta from datetime import datetime, time, timedelta
from unittest import mock from unittest import mock
from miio import DeviceException
import pytest import pytest
from pytz import utc from pytz import utc
@ -52,9 +53,12 @@ from homeassistant.const import (
CONF_PLATFORM, CONF_PLATFORM,
STATE_OFF, STATE_OFF,
STATE_ON, STATE_ON,
STATE_UNAVAILABLE,
) )
from homeassistant.setup import async_setup_component from homeassistant.setup import async_setup_component
from tests.async_mock import MagicMock, patch
PLATFORM = "xiaomi_miio" PLATFORM = "xiaomi_miio"
# calls made when device status is requested # calls made when device status is requested
@ -70,7 +74,7 @@ STATUS_CALLS = [
@pytest.fixture(name="mock_mirobo_is_got_error") @pytest.fixture(name="mock_mirobo_is_got_error")
def mirobo_is_got_error_fixture(): def mirobo_is_got_error_fixture():
"""Mock mock_mirobo.""" """Mock mock_mirobo."""
mock_vacuum = mock.MagicMock() mock_vacuum = MagicMock()
mock_vacuum.status().data = {"test": "raw"} mock_vacuum.status().data = {"test": "raw"}
mock_vacuum.status().is_on = False mock_vacuum.status().is_on = False
mock_vacuum.status().fanspeed = 38 mock_vacuum.status().fanspeed = 38
@ -98,21 +102,19 @@ def mirobo_is_got_error_fixture():
mock_vacuum.dnd_status().start = time(hour=22, minute=0) mock_vacuum.dnd_status().start = time(hour=22, minute=0)
mock_vacuum.dnd_status().end = time(hour=6, minute=0) mock_vacuum.dnd_status().end = time(hour=6, minute=0)
mock_timer_1 = mock.MagicMock() mock_timer_1 = MagicMock()
mock_timer_1.enabled = True mock_timer_1.enabled = True
mock_timer_1.cron = "5 5 1 8 1" mock_timer_1.cron = "5 5 1 8 1"
mock_timer_1.next_schedule = datetime(2020, 5, 23, 13, 21, 10, tzinfo=utc) mock_timer_1.next_schedule = datetime(2020, 5, 23, 13, 21, 10, tzinfo=utc)
mock_timer_2 = mock.MagicMock() mock_timer_2 = MagicMock()
mock_timer_2.enabled = False mock_timer_2.enabled = False
mock_timer_2.cron = "5 5 1 8 2" mock_timer_2.cron = "5 5 1 8 2"
mock_timer_2.next_schedule = datetime(2020, 5, 23, 13, 21, 10, tzinfo=utc) mock_timer_2.next_schedule = datetime(2020, 5, 23, 13, 21, 10, tzinfo=utc)
mock_vacuum.timer.return_value = [mock_timer_1, mock_timer_2] mock_vacuum.timer.return_value = [mock_timer_1, mock_timer_2]
with mock.patch( with patch("homeassistant.components.xiaomi_miio.vacuum.Vacuum") as mock_vaccum_cls:
"homeassistant.components.xiaomi_miio.vacuum.Vacuum"
) as mock_vaccum_cls:
mock_vaccum_cls.return_value = mock_vacuum mock_vaccum_cls.return_value = mock_vacuum
yield mock_vacuum yield mock_vacuum
@ -135,14 +137,12 @@ new_fanspeeds = {
@pytest.fixture(name="mock_mirobo_fanspeeds", params=[old_fanspeeds, new_fanspeeds]) @pytest.fixture(name="mock_mirobo_fanspeeds", params=[old_fanspeeds, new_fanspeeds])
def mirobo_old_speeds_fixture(request): def mirobo_old_speeds_fixture(request):
"""Fixture for testing both types of fanspeeds.""" """Fixture for testing both types of fanspeeds."""
mock_vacuum = mock.MagicMock() mock_vacuum = MagicMock()
mock_vacuum.status().battery = 32 mock_vacuum.status().battery = 32
mock_vacuum.fan_speed_presets.return_value = request.param mock_vacuum.fan_speed_presets.return_value = request.param
mock_vacuum.status().fanspeed = list(request.param.values())[0] mock_vacuum.status().fanspeed = list(request.param.values())[0]
with mock.patch( with patch("homeassistant.components.xiaomi_miio.vacuum.Vacuum") as mock_vaccum_cls:
"homeassistant.components.xiaomi_miio.vacuum.Vacuum"
) as mock_vaccum_cls:
mock_vaccum_cls.return_value = mock_vacuum mock_vaccum_cls.return_value = mock_vacuum
yield mock_vacuum yield mock_vacuum
@ -150,7 +150,7 @@ def mirobo_old_speeds_fixture(request):
@pytest.fixture(name="mock_mirobo_is_on") @pytest.fixture(name="mock_mirobo_is_on")
def mirobo_is_on_fixture(): def mirobo_is_on_fixture():
"""Mock mock_mirobo.""" """Mock mock_mirobo."""
mock_vacuum = mock.MagicMock() mock_vacuum = MagicMock()
mock_vacuum.status().data = {"test": "raw"} mock_vacuum.status().data = {"test": "raw"}
mock_vacuum.status().is_on = True mock_vacuum.status().is_on = True
mock_vacuum.status().fanspeed = 99 mock_vacuum.status().fanspeed = 99
@ -176,46 +176,53 @@ def mirobo_is_on_fixture():
mock_vacuum.status().state_code = 5 mock_vacuum.status().state_code = 5
mock_vacuum.dnd_status().enabled = False mock_vacuum.dnd_status().enabled = False
mock_timer_1 = mock.MagicMock() mock_timer_1 = MagicMock()
mock_timer_1.enabled = True mock_timer_1.enabled = True
mock_timer_1.cron = "5 5 1 8 1" mock_timer_1.cron = "5 5 1 8 1"
mock_timer_1.next_schedule = datetime(2020, 5, 23, 13, 21, 10, tzinfo=utc) mock_timer_1.next_schedule = datetime(2020, 5, 23, 13, 21, 10, tzinfo=utc)
mock_timer_2 = mock.MagicMock() mock_timer_2 = MagicMock()
mock_timer_2.enabled = False mock_timer_2.enabled = False
mock_timer_2.cron = "5 5 1 8 2" mock_timer_2.cron = "5 5 1 8 2"
mock_timer_2.next_schedule = datetime(2020, 5, 23, 13, 21, 10, tzinfo=utc) mock_timer_2.next_schedule = datetime(2020, 5, 23, 13, 21, 10, tzinfo=utc)
mock_vacuum.timer.return_value = [mock_timer_1, mock_timer_2] mock_vacuum.timer.return_value = [mock_timer_1, mock_timer_2]
with mock.patch( with patch("homeassistant.components.xiaomi_miio.vacuum.Vacuum") as mock_vaccum_cls:
"homeassistant.components.xiaomi_miio.vacuum.Vacuum"
) as mock_vaccum_cls:
mock_vaccum_cls.return_value = mock_vacuum mock_vaccum_cls.return_value = mock_vacuum
yield mock_vacuum yield mock_vacuum
@pytest.fixture(name="mock_mirobo_errors") async def test_xiaomi_exceptions(hass, caplog, mock_mirobo_is_on):
def mirobo_errors_fixture(): """Test error logging on exceptions."""
"""Mock mock_mirobo_errors to simulate a bad vacuum status request."""
mock_vacuum = mock.MagicMock()
mock_vacuum.status.side_effect = OSError()
with mock.patch(
"homeassistant.components.xiaomi_miio.vacuum.Vacuum"
) as mock_vaccum_cls:
mock_vaccum_cls.return_value = mock_vacuum
yield mock_vacuum
async def test_xiaomi_exceptions(hass, caplog, mock_mirobo_errors):
"""Test vacuum supported features."""
entity_name = "test_vacuum_cleaner_error" entity_name = "test_vacuum_cleaner_error"
await setup_component(hass, entity_name) entity_id = await setup_component(hass, entity_name)
def is_available():
state = hass.states.get(entity_id)
return state.state != STATE_UNAVAILABLE
# The initial setup has to be done successfully
assert "Initializing with host 192.168.1.100 (token 12345...)" in caplog.text assert "Initializing with host 192.168.1.100 (token 12345...)" in caplog.text
assert mock_mirobo_errors.status.call_count == 1 assert "WARNING" not in caplog.text
assert "ERROR" in caplog.text assert is_available()
assert "Got OSError while fetching the state" in caplog.text
# Second update causes an exception, which should be logged
mock_mirobo_is_on.status.side_effect = DeviceException("dummy exception")
await hass.helpers.entity_component.async_update_entity(entity_id)
assert "WARNING" in caplog.text
assert "Got exception while fetching the state" in caplog.text
assert not is_available()
# Third update does not get logged as the device is already unavailable,
# so we clear the log and reset the status to test that
caplog.clear()
mock_mirobo_is_on.status.reset_mock()
await hass.helpers.entity_component.async_update_entity(entity_id)
assert "Got exception while fetching the state" not in caplog.text
assert not is_available()
assert mock_mirobo_is_on.status.call_count == 1
async def test_xiaomi_vacuum_services(hass, caplog, mock_mirobo_is_got_error): async def test_xiaomi_vacuum_services(hass, caplog, mock_mirobo_is_got_error):
@ -463,7 +470,7 @@ async def test_xiaomi_vacuum_fanspeeds(hass, caplog, mock_mirobo_fanspeeds):
{"entity_id": entity_id, "fan_speed": "invent"}, {"entity_id": entity_id, "fan_speed": "invent"},
blocking=True, blocking=True,
) )
assert "ERROR" in caplog.text assert "Fan speed step not recognized" in caplog.text
async def test_xiaomi_vacuum_goto_service(hass, caplog, mock_mirobo_is_on): async def test_xiaomi_vacuum_goto_service(hass, caplog, mock_mirobo_is_on):