From 7b452208b6174762b04bfe5e3d57ead5fb9b1fd6 Mon Sep 17 00:00:00 2001 From: Teemu R Date: Fri, 1 Dec 2017 12:28:59 +0100 Subject: [PATCH] Xiaomi Vacuum: remove deprecated calls (#10839) * vacuum.xiaomi_miio: read dnd status properly instead of using imprecise dnd flag from vacuum_state * vacuum.xiaomi_miio: use miio package instead of mirobo * check only that wanted calls have taken place, ignore order of calls * Fix linting issues * Remove empty line after docstring --- .../components/vacuum/xiaomi_miio.py | 19 +- tests/components/vacuum/test_xiaomi_miio.py | 217 +++++++----------- 2 files changed, 101 insertions(+), 135 deletions(-) diff --git a/homeassistant/components/vacuum/xiaomi_miio.py b/homeassistant/components/vacuum/xiaomi_miio.py index 131f5d5a77f..a2265706d87 100644 --- a/homeassistant/components/vacuum/xiaomi_miio.py +++ b/homeassistant/components/vacuum/xiaomi_miio.py @@ -48,6 +48,8 @@ FAN_SPEEDS = { ATTR_CLEANING_TIME = 'cleaning_time' ATTR_DO_NOT_DISTURB = 'do_not_disturb' +ATTR_DO_NOT_DISTURB_START = 'do_not_disturb_start' +ATTR_DO_NOT_DISTURB_END = 'do_not_disturb_end' ATTR_MAIN_BRUSH_LEFT = 'main_brush_left' ATTR_SIDE_BRUSH_LEFT = 'side_brush_left' ATTR_FILTER_LEFT = 'filter_left' @@ -87,7 +89,7 @@ SUPPORT_XIAOMI = SUPPORT_TURN_ON | SUPPORT_TURN_OFF | SUPPORT_PAUSE | \ @asyncio.coroutine def async_setup_platform(hass, config, async_add_devices, discovery_info=None): """Set up the Xiaomi vacuum cleaner robot platform.""" - from mirobo import Vacuum + from miio import Vacuum if PLATFORM not in hass.data: hass.data[PLATFORM] = {} @@ -155,6 +157,7 @@ class MiroboVacuum(VacuumDevice): self.consumable_state = None self.clean_history = None + self.dnd_state = None @property def name(self): @@ -200,7 +203,9 @@ class MiroboVacuum(VacuumDevice): if self.vacuum_state is not None: attrs.update({ ATTR_DO_NOT_DISTURB: - STATE_ON if self.vacuum_state.dnd else STATE_OFF, + STATE_ON if self.dnd_state.enabled else STATE_OFF, + ATTR_DO_NOT_DISTURB_START: str(self.dnd_state.start), + ATTR_DO_NOT_DISTURB_END: str(self.dnd_state.end), # Not working --> 'Cleaning mode': # STATE_ON if self.vacuum_state.in_cleaning else STATE_OFF, ATTR_CLEANING_TIME: int( @@ -223,7 +228,6 @@ class MiroboVacuum(VacuumDevice): / 3600)}) if self.vacuum_state.got_error: attrs[ATTR_ERROR] = self.vacuum_state.error - return attrs @property @@ -244,11 +248,11 @@ class MiroboVacuum(VacuumDevice): @asyncio.coroutine def _try_command(self, mask_error, func, *args, **kwargs): """Call a vacuum command handling error messages.""" - from mirobo import DeviceException, VacuumException + from miio import DeviceException try: yield from self.hass.async_add_job(partial(func, *args, **kwargs)) return True - except (DeviceException, VacuumException) as exc: + except DeviceException as exc: _LOGGER.error(mask_error, exc) return False @@ -365,12 +369,15 @@ class MiroboVacuum(VacuumDevice): def update(self): """Fetch state from the device.""" - from mirobo import DeviceException + from miio import DeviceException try: state = self._vacuum.status() self.vacuum_state = state + self.consumable_state = self._vacuum.consumable_status() self.clean_history = self._vacuum.clean_history() + self.dnd_state = self._vacuum.dnd_status() + self._is_on = state.is_on self._available = True except OSError as exc: diff --git a/tests/components/vacuum/test_xiaomi_miio.py b/tests/components/vacuum/test_xiaomi_miio.py index bdb85abb057..62ad5fc4a2b 100644 --- a/tests/components/vacuum/test_xiaomi_miio.py +++ b/tests/components/vacuum/test_xiaomi_miio.py @@ -1,6 +1,6 @@ """The tests for the Xiaomi vacuum platform.""" import asyncio -from datetime import timedelta +from datetime import timedelta, time from unittest import mock import pytest @@ -12,7 +12,8 @@ from homeassistant.components.vacuum import ( SERVICE_SEND_COMMAND, SERVICE_SET_FAN_SPEED, SERVICE_START_PAUSE, SERVICE_STOP, SERVICE_TOGGLE, SERVICE_TURN_OFF, SERVICE_TURN_ON) from homeassistant.components.vacuum.xiaomi_miio import ( - ATTR_CLEANED_AREA, ATTR_CLEANING_TIME, ATTR_DO_NOT_DISTURB, ATTR_ERROR, + ATTR_CLEANED_AREA, ATTR_CLEANING_TIME, ATTR_DO_NOT_DISTURB, + ATTR_DO_NOT_DISTURB_START, ATTR_DO_NOT_DISTURB_END, ATTR_ERROR, ATTR_MAIN_BRUSH_LEFT, ATTR_SIDE_BRUSH_LEFT, ATTR_FILTER_LEFT, ATTR_CLEANING_COUNT, ATTR_CLEANED_TOTAL_AREA, ATTR_CLEANING_TOTAL_TIME, CONF_HOST, CONF_NAME, CONF_TOKEN, PLATFORM, @@ -23,6 +24,12 @@ from homeassistant.const import ( STATE_ON) from homeassistant.setup import async_setup_component +# calls made when device status is requested +status_calls = [mock.call.Vacuum().status(), + mock.call.Vacuum().consumable_status(), + mock.call.Vacuum().clean_history(), + mock.call.Vacuum().dnd_status()] + @pytest.fixture def mock_mirobo_is_off(): @@ -33,7 +40,6 @@ def mock_mirobo_is_off(): mock_vacuum.Vacuum().status().fanspeed = 38 mock_vacuum.Vacuum().status().got_error = True mock_vacuum.Vacuum().status().error = 'Error message' - mock_vacuum.Vacuum().status().dnd = True mock_vacuum.Vacuum().status().battery = 82 mock_vacuum.Vacuum().status().clean_area = 123.43218 mock_vacuum.Vacuum().status().clean_time = timedelta( @@ -49,9 +55,12 @@ def mock_mirobo_is_off(): mock_vacuum.Vacuum().clean_history().total_duration = timedelta( hours=11, minutes=35, seconds=34) mock_vacuum.Vacuum().status().state = 'Test Xiaomi Charging' + mock_vacuum.Vacuum().dnd_status().enabled = True + mock_vacuum.Vacuum().dnd_status().start = time(hour=22, minute=0) + mock_vacuum.Vacuum().dnd_status().end = time(hour=6, minute=0) with mock.patch.dict('sys.modules', { - 'mirobo': mock_vacuum, + 'miio': mock_vacuum, }): yield mock_vacuum @@ -64,7 +73,6 @@ def mock_mirobo_is_on(): mock_vacuum.Vacuum().status().is_on = True mock_vacuum.Vacuum().status().fanspeed = 99 mock_vacuum.Vacuum().status().got_error = False - mock_vacuum.Vacuum().status().dnd = False mock_vacuum.Vacuum().status().battery = 32 mock_vacuum.Vacuum().status().clean_area = 133.43218 mock_vacuum.Vacuum().status().clean_time = timedelta( @@ -80,9 +88,10 @@ def mock_mirobo_is_on(): mock_vacuum.Vacuum().clean_history().total_duration = timedelta( hours=11, minutes=15, seconds=34) mock_vacuum.Vacuum().status().state = 'Test Xiaomi Cleaning' + mock_vacuum.Vacuum().dnd_status().enabled = False with mock.patch.dict('sys.modules', { - 'mirobo': mock_vacuum, + 'miio': mock_vacuum, }): yield mock_vacuum @@ -93,7 +102,7 @@ def mock_mirobo_errors(): mock_vacuum = mock.MagicMock() mock_vacuum.Vacuum().status.side_effect = OSError() with mock.patch.dict('sys.modules', { - 'mirobo': mock_vacuum, + 'miio': mock_vacuum, }): yield mock_vacuum @@ -136,6 +145,8 @@ def test_xiaomi_vacuum_services(hass, caplog, mock_mirobo_is_off): assert state.state == STATE_OFF assert state.attributes.get(ATTR_SUPPORTED_FEATURES) == 2047 assert state.attributes.get(ATTR_DO_NOT_DISTURB) == STATE_ON + assert state.attributes.get(ATTR_DO_NOT_DISTURB_START) == '22:00:00' + assert state.attributes.get(ATTR_DO_NOT_DISTURB_END) == '06:00:00' assert state.attributes.get(ATTR_ERROR) == 'Error message' assert (state.attributes.get(ATTR_BATTERY_ICON) == 'mdi:battery-charging-80') @@ -154,96 +165,75 @@ def test_xiaomi_vacuum_services(hass, caplog, mock_mirobo_is_off): # Call services yield from hass.services.async_call( DOMAIN, SERVICE_TURN_ON, blocking=True) - assert str(mock_mirobo_is_off.mock_calls[-4]) == 'call.Vacuum().start()' - assert str(mock_mirobo_is_off.mock_calls[-3]) == 'call.Vacuum().status()' - assert (str(mock_mirobo_is_off.mock_calls[-2]) - == 'call.Vacuum().consumable_status()') - assert (str(mock_mirobo_is_off.mock_calls[-1]) - == 'call.Vacuum().clean_history()') + + mock_mirobo_is_off.assert_has_calls( + [mock.call.Vacuum.start()], any_order=True) + mock_mirobo_is_off.assert_has_calls(status_calls, any_order=True) + mock_mirobo_is_off.reset_mock() yield from hass.services.async_call( DOMAIN, SERVICE_TURN_OFF, blocking=True) - assert str(mock_mirobo_is_off.mock_calls[-4]) == 'call.Vacuum().home()' - assert str(mock_mirobo_is_off.mock_calls[-3]) == 'call.Vacuum().status()' - assert (str(mock_mirobo_is_off.mock_calls[-2]) - == 'call.Vacuum().consumable_status()') - assert (str(mock_mirobo_is_off.mock_calls[-1]) - == 'call.Vacuum().clean_history()') + mock_mirobo_is_off.assert_has_calls( + [mock.call.Vacuum().home()], any_order=True) + mock_mirobo_is_off.assert_has_calls(status_calls, any_order=True) + mock_mirobo_is_off.reset_mock() yield from hass.services.async_call( DOMAIN, SERVICE_TOGGLE, blocking=True) - assert str(mock_mirobo_is_off.mock_calls[-4]) == 'call.Vacuum().start()' - assert str(mock_mirobo_is_off.mock_calls[-3]) == 'call.Vacuum().status()' - assert (str(mock_mirobo_is_off.mock_calls[-2]) - == 'call.Vacuum().consumable_status()') - assert (str(mock_mirobo_is_off.mock_calls[-1]) - == 'call.Vacuum().clean_history()') + mock_mirobo_is_off.assert_has_calls( + [mock.call.Vacuum().start()], any_order=True) + mock_mirobo_is_off.assert_has_calls(status_calls, any_order=True) + mock_mirobo_is_off.reset_mock() yield from hass.services.async_call( DOMAIN, SERVICE_STOP, blocking=True) - assert str(mock_mirobo_is_off.mock_calls[-4]) == 'call.Vacuum().stop()' - assert str(mock_mirobo_is_off.mock_calls[-3]) == 'call.Vacuum().status()' - assert (str(mock_mirobo_is_off.mock_calls[-2]) - == 'call.Vacuum().consumable_status()') - assert (str(mock_mirobo_is_off.mock_calls[-1]) - == 'call.Vacuum().clean_history()') + mock_mirobo_is_off.assert_has_calls( + [mock.call.Vacuum().stop()], any_order=True) + mock_mirobo_is_off.assert_has_calls(status_calls, any_order=True) + mock_mirobo_is_off.reset_mock() yield from hass.services.async_call( DOMAIN, SERVICE_START_PAUSE, blocking=True) - assert str(mock_mirobo_is_off.mock_calls[-4]) == 'call.Vacuum().start()' - assert str(mock_mirobo_is_off.mock_calls[-3]) == 'call.Vacuum().status()' - assert (str(mock_mirobo_is_off.mock_calls[-2]) - == 'call.Vacuum().consumable_status()') - assert (str(mock_mirobo_is_off.mock_calls[-1]) - == 'call.Vacuum().clean_history()') + mock_mirobo_is_off.assert_has_calls( + [mock.call.Vacuum().pause()], any_order=True) + mock_mirobo_is_off.assert_has_calls(status_calls, any_order=True) + mock_mirobo_is_off.reset_mock() yield from hass.services.async_call( DOMAIN, SERVICE_RETURN_TO_BASE, blocking=True) - assert str(mock_mirobo_is_off.mock_calls[-4]) == 'call.Vacuum().home()' - assert str(mock_mirobo_is_off.mock_calls[-3]) == 'call.Vacuum().status()' - assert (str(mock_mirobo_is_off.mock_calls[-2]) - == 'call.Vacuum().consumable_status()') - assert (str(mock_mirobo_is_off.mock_calls[-1]) - == 'call.Vacuum().clean_history()') + mock_mirobo_is_off.assert_has_calls( + [mock.call.Vacuum().home()], any_order=True) + mock_mirobo_is_off.assert_has_calls(status_calls, any_order=True) + mock_mirobo_is_off.reset_mock() yield from hass.services.async_call( DOMAIN, SERVICE_LOCATE, blocking=True) - assert str(mock_mirobo_is_off.mock_calls[-4]) == 'call.Vacuum().find()' - assert str(mock_mirobo_is_off.mock_calls[-3]) == 'call.Vacuum().status()' - assert (str(mock_mirobo_is_off.mock_calls[-2]) - == 'call.Vacuum().consumable_status()') - assert (str(mock_mirobo_is_off.mock_calls[-1]) - == 'call.Vacuum().clean_history()') + mock_mirobo_is_off.assert_has_calls( + [mock.call.Vacuum().find()], any_order=True) + mock_mirobo_is_off.assert_has_calls(status_calls, any_order=True) + mock_mirobo_is_off.reset_mock() yield from hass.services.async_call( DOMAIN, SERVICE_CLEAN_SPOT, {}, blocking=True) - assert str(mock_mirobo_is_off.mock_calls[-4]) == 'call.Vacuum().spot()' - assert str(mock_mirobo_is_off.mock_calls[-3]) == 'call.Vacuum().status()' - assert (str(mock_mirobo_is_off.mock_calls[-2]) - == 'call.Vacuum().consumable_status()') - assert (str(mock_mirobo_is_off.mock_calls[-1]) - == 'call.Vacuum().clean_history()') + mock_mirobo_is_off.assert_has_calls( + [mock.call.Vacuum().spot()], any_order=True) + mock_mirobo_is_off.assert_has_calls(status_calls, any_order=True) + mock_mirobo_is_off.reset_mock() # Set speed service: yield from hass.services.async_call( DOMAIN, SERVICE_SET_FAN_SPEED, {"fan_speed": 60}, blocking=True) - assert (str(mock_mirobo_is_off.mock_calls[-4]) - == 'call.Vacuum().set_fan_speed(60)') - assert str(mock_mirobo_is_off.mock_calls[-3]) == 'call.Vacuum().status()' - assert (str(mock_mirobo_is_off.mock_calls[-2]) - == 'call.Vacuum().consumable_status()') - assert (str(mock_mirobo_is_off.mock_calls[-1]) - == 'call.Vacuum().clean_history()') + mock_mirobo_is_off.assert_has_calls( + [mock.call.Vacuum().set_fan_speed(60)], any_order=True) + mock_mirobo_is_off.assert_has_calls(status_calls, any_order=True) + mock_mirobo_is_off.reset_mock() yield from hass.services.async_call( DOMAIN, SERVICE_SET_FAN_SPEED, {"fan_speed": "turbo"}, blocking=True) - assert (str(mock_mirobo_is_off.mock_calls[-4]) - == 'call.Vacuum().set_fan_speed(77)') - assert str(mock_mirobo_is_off.mock_calls[-3]) == 'call.Vacuum().status()' - assert (str(mock_mirobo_is_off.mock_calls[-2]) - == 'call.Vacuum().consumable_status()') - assert (str(mock_mirobo_is_off.mock_calls[-1]) - == 'call.Vacuum().clean_history()') + mock_mirobo_is_off.assert_has_calls( + [mock.call.Vacuum().set_fan_speed(77)], any_order=True) + mock_mirobo_is_off.assert_has_calls(status_calls, any_order=True) + mock_mirobo_is_off.reset_mock() assert 'ERROR' not in caplog.text yield from hass.services.async_call( @@ -253,24 +243,18 @@ def test_xiaomi_vacuum_services(hass, caplog, mock_mirobo_is_off): yield from hass.services.async_call( DOMAIN, SERVICE_SEND_COMMAND, {"command": "raw"}, blocking=True) - assert (str(mock_mirobo_is_off.mock_calls[-4]) - == "call.Vacuum().raw_command('raw', None)") - assert str(mock_mirobo_is_off.mock_calls[-3]) == 'call.Vacuum().status()' - assert (str(mock_mirobo_is_off.mock_calls[-2]) - == 'call.Vacuum().consumable_status()') - assert (str(mock_mirobo_is_off.mock_calls[-1]) - == 'call.Vacuum().clean_history()') + mock_mirobo_is_off.assert_has_calls( + [mock.call.Vacuum().raw_command('raw', None)], any_order=True) + mock_mirobo_is_off.assert_has_calls(status_calls, any_order=True) + mock_mirobo_is_off.reset_mock() yield from hass.services.async_call( DOMAIN, SERVICE_SEND_COMMAND, {"command": "raw", "params": {"k1": 2}}, blocking=True) - assert (str(mock_mirobo_is_off.mock_calls[-4]) - == "call.Vacuum().raw_command('raw', {'k1': 2})") - assert str(mock_mirobo_is_off.mock_calls[-3]) == 'call.Vacuum().status()' - assert (str(mock_mirobo_is_off.mock_calls[-2]) - == 'call.Vacuum().consumable_status()') - assert (str(mock_mirobo_is_off.mock_calls[-1]) - == 'call.Vacuum().clean_history()') + mock_mirobo_is_off.assert_has_calls( + [mock.call.Vacuum().raw_command('raw', {'k1': 2})], any_order=True) + mock_mirobo_is_off.assert_has_calls(status_calls, any_order=True) + mock_mirobo_is_off.reset_mock() @asyncio.coroutine @@ -308,62 +292,37 @@ def test_xiaomi_specific_services(hass, caplog, mock_mirobo_is_on): assert state.attributes.get(ATTR_CLEANED_TOTAL_AREA) == 323 assert state.attributes.get(ATTR_CLEANING_TOTAL_TIME) == 675 - # Check setting pause - yield from hass.services.async_call( - DOMAIN, SERVICE_START_PAUSE, blocking=True) - assert str(mock_mirobo_is_on.mock_calls[-4]) == 'call.Vacuum().pause()' - assert str(mock_mirobo_is_on.mock_calls[-3]) == 'call.Vacuum().status()' - assert (str(mock_mirobo_is_on.mock_calls[-2]) - == 'call.Vacuum().consumable_status()') - assert (str(mock_mirobo_is_on.mock_calls[-1]) - == 'call.Vacuum().clean_history()') - # Xiaomi vacuum specific services: yield from hass.services.async_call( DOMAIN, SERVICE_START_REMOTE_CONTROL, {ATTR_ENTITY_ID: entity_id}, blocking=True) - assert (str(mock_mirobo_is_on.mock_calls[-4]) - == "call.Vacuum().manual_start()") - assert str(mock_mirobo_is_on.mock_calls[-3]) == 'call.Vacuum().status()' - assert (str(mock_mirobo_is_on.mock_calls[-2]) - == 'call.Vacuum().consumable_status()') - assert (str(mock_mirobo_is_on.mock_calls[-1]) - == 'call.Vacuum().clean_history()') + mock_mirobo_is_on.assert_has_calls( + [mock.call.Vacuum().manual_start()], any_order=True) + mock_mirobo_is_on.assert_has_calls(status_calls, any_order=True) + mock_mirobo_is_on.reset_mock() + + control = {"duration": 1000, "rotation": -40, "velocity": -0.1} yield from hass.services.async_call( DOMAIN, SERVICE_MOVE_REMOTE_CONTROL, - {"duration": 1000, "rotation": -40, "velocity": -0.1}, blocking=True) - assert ('call.Vacuum().manual_control(' - in str(mock_mirobo_is_on.mock_calls[-4])) - assert 'duration=1000' in str(mock_mirobo_is_on.mock_calls[-4]) - assert 'rotation=-40' in str(mock_mirobo_is_on.mock_calls[-4]) - assert 'velocity=-0.1' in str(mock_mirobo_is_on.mock_calls[-4]) - assert str(mock_mirobo_is_on.mock_calls[-3]) == 'call.Vacuum().status()' - assert (str(mock_mirobo_is_on.mock_calls[-2]) - == 'call.Vacuum().consumable_status()') - assert (str(mock_mirobo_is_on.mock_calls[-1]) - == 'call.Vacuum().clean_history()') + control, blocking=True) + mock_mirobo_is_on.assert_has_calls( + [mock.call.Vacuum().manual_control(control)], any_order=True) + mock_mirobo_is_on.assert_has_calls(status_calls, any_order=True) + mock_mirobo_is_on.reset_mock() yield from hass.services.async_call( DOMAIN, SERVICE_STOP_REMOTE_CONTROL, {}, blocking=True) - assert (str(mock_mirobo_is_on.mock_calls[-4]) - == "call.Vacuum().manual_stop()") - assert str(mock_mirobo_is_on.mock_calls[-3]) == 'call.Vacuum().status()' - assert (str(mock_mirobo_is_on.mock_calls[-2]) - == 'call.Vacuum().consumable_status()') - assert (str(mock_mirobo_is_on.mock_calls[-1]) - == 'call.Vacuum().clean_history()') + mock_mirobo_is_on.assert_has_calls( + [mock.call.Vacuum().manual_stop()], any_order=True) + mock_mirobo_is_on.assert_has_calls(status_calls, any_order=True) + mock_mirobo_is_on.reset_mock() + control_once = {"duration": 2000, "rotation": 120, "velocity": 0.1} yield from hass.services.async_call( DOMAIN, SERVICE_MOVE_REMOTE_CONTROL_STEP, - {"duration": 2000, "rotation": 120, "velocity": 0.1}, blocking=True) - assert ('call.Vacuum().manual_control_once(' - in str(mock_mirobo_is_on.mock_calls[-4])) - assert 'duration=2000' in str(mock_mirobo_is_on.mock_calls[-4]) - assert 'rotation=120' in str(mock_mirobo_is_on.mock_calls[-4]) - assert 'velocity=0.1' in str(mock_mirobo_is_on.mock_calls[-4]) - assert str(mock_mirobo_is_on.mock_calls[-3]) == 'call.Vacuum().status()' - assert (str(mock_mirobo_is_on.mock_calls[-2]) - == 'call.Vacuum().consumable_status()') - assert (str(mock_mirobo_is_on.mock_calls[-1]) - == 'call.Vacuum().clean_history()') + control_once, blocking=True) + mock_mirobo_is_on.assert_has_calls( + [mock.call.Vacuum().manual_control_once(control_once)], any_order=True) + mock_mirobo_is_on.assert_has_calls(status_calls, any_order=True) + mock_mirobo_is_on.reset_mock()