From 3fcd7866ccea1d4f1223a32529188cb26905de2a Mon Sep 17 00:00:00 2001 From: On Freund Date: Tue, 12 May 2020 23:30:59 +0300 Subject: [PATCH] Try to automatically detect zones on first run of Monoprice integration (#35127) --- .../components/monoprice/__init__.py | 17 ++++- homeassistant/components/monoprice/const.py | 3 + .../components/monoprice/media_player.py | 10 ++- .../components/monoprice/test_media_player.py | 63 +++++++++++++++++-- 4 files changed, 84 insertions(+), 9 deletions(-) diff --git a/homeassistant/components/monoprice/__init__.py b/homeassistant/components/monoprice/__init__.py index d5173d40d89..9bceff1531c 100644 --- a/homeassistant/components/monoprice/__init__.py +++ b/homeassistant/components/monoprice/__init__.py @@ -10,7 +10,13 @@ from homeassistant.const import CONF_PORT from homeassistant.core import HomeAssistant from homeassistant.exceptions import ConfigEntryNotReady -from .const import DOMAIN, MONOPRICE_OBJECT, UNDO_UPDATE_LISTENER +from .const import ( + CONF_NOT_FIRST_RUN, + DOMAIN, + FIRST_RUN, + MONOPRICE_OBJECT, + UNDO_UPDATE_LISTENER, +) PLATFORMS = ["media_player"] @@ -32,11 +38,20 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry): _LOGGER.error("Error connecting to Monoprice controller at %s", port) raise ConfigEntryNotReady + # double negative to handle absence of value + first_run = not bool(entry.data.get(CONF_NOT_FIRST_RUN)) + + if first_run: + hass.config_entries.async_update_entry( + entry, data={**entry.data, CONF_NOT_FIRST_RUN: True} + ) + undo_listener = entry.add_update_listener(_update_listener) hass.data.setdefault(DOMAIN, {})[entry.entry_id] = { MONOPRICE_OBJECT: monoprice, UNDO_UPDATE_LISTENER: undo_listener, + FIRST_RUN: first_run, } for component in PLATFORMS: diff --git a/homeassistant/components/monoprice/const.py b/homeassistant/components/monoprice/const.py index 180ec452831..576e4aa0e69 100644 --- a/homeassistant/components/monoprice/const.py +++ b/homeassistant/components/monoprice/const.py @@ -11,8 +11,11 @@ CONF_SOURCE_4 = "source_4" CONF_SOURCE_5 = "source_5" CONF_SOURCE_6 = "source_6" +CONF_NOT_FIRST_RUN = "not_first_run" + SERVICE_SNAPSHOT = "snapshot" SERVICE_RESTORE = "restore" +FIRST_RUN = "first_run" MONOPRICE_OBJECT = "monoprice_object" UNDO_UPDATE_LISTENER = "update_update_listener" diff --git a/homeassistant/components/monoprice/media_player.py b/homeassistant/components/monoprice/media_player.py index 985cd88e47f..4d6d337667e 100644 --- a/homeassistant/components/monoprice/media_player.py +++ b/homeassistant/components/monoprice/media_player.py @@ -19,6 +19,7 @@ from homeassistant.helpers import config_validation as cv, entity_platform, serv from .const import ( CONF_SOURCES, DOMAIN, + FIRST_RUN, MONOPRICE_OBJECT, SERVICE_RESTORE, SERVICE_SNAPSHOT, @@ -77,7 +78,9 @@ async def async_setup_entry(hass, config_entry, async_add_entities): MonopriceZone(monoprice, sources, config_entry.entry_id, zone_id) ) - async_add_entities(entities, True) + # only call update before add if it's the first run so we can try to detect zones + first_run = hass.data[DOMAIN][config_entry.entry_id][FIRST_RUN] + async_add_entities(entities, first_run) platform = entity_platform.current_platform.get() @@ -134,16 +137,19 @@ class MonopriceZone(MediaPlayerEntity): self._volume = None self._source = None self._mute = None + self._update_success = True def update(self): """Retrieve latest state.""" try: state = self._monoprice.zone_status(self._zone_id) except SerialException: + self._update_success = False _LOGGER.warning("Could not update zone %d", self._zone_id) return if not state: + self._update_success = False return self._state = STATE_ON if state.power else STATE_OFF @@ -158,7 +164,7 @@ class MonopriceZone(MediaPlayerEntity): @property def entity_registry_enabled_default(self): """Return if the entity should be enabled when first added to the entity registry.""" - return self._zone_id < 20 + return self._zone_id < 20 or self._update_success @property def device_info(self): diff --git a/tests/components/monoprice/test_media_player.py b/tests/components/monoprice/test_media_player.py index f70a19f51fc..ccd70c628e2 100644 --- a/tests/components/monoprice/test_media_player.py +++ b/tests/components/monoprice/test_media_player.py @@ -17,6 +17,7 @@ from homeassistant.components.media_player.const import ( SUPPORT_VOLUME_STEP, ) from homeassistant.components.monoprice.const import ( + CONF_NOT_FIRST_RUN, CONF_SOURCES, DOMAIN, SERVICE_RESTORE, @@ -41,6 +42,7 @@ MOCK_OPTIONS = {CONF_SOURCES: {"2": "two", "4": "four"}} ZONE_1_ID = "media_player.zone_11" ZONE_2_ID = "media_player.zone_12" +ZONE_7_ID = "media_player.zone_21" class AttrDict(dict): @@ -100,8 +102,6 @@ async def test_cannot_connect(hass): config_entry = MockConfigEntry(domain=DOMAIN, data=MOCK_CONFIG) config_entry.add_to_hass(hass) await hass.config_entries.async_setup(config_entry.entry_id) - # setup_component(self.hass, DOMAIN, MOCK_CONFIG) - # self.hass.async_block_till_done() await hass.async_block_till_done() assert hass.states.get(ZONE_1_ID) is None @@ -113,8 +113,6 @@ async def _setup_monoprice(hass, monoprice): config_entry = MockConfigEntry(domain=DOMAIN, data=MOCK_CONFIG) config_entry.add_to_hass(hass) await hass.config_entries.async_setup(config_entry.entry_id) - # setup_component(self.hass, DOMAIN, MOCK_CONFIG) - # self.hass.async_block_till_done() await hass.async_block_till_done() @@ -127,8 +125,17 @@ async def _setup_monoprice_with_options(hass, monoprice): ) config_entry.add_to_hass(hass) await hass.config_entries.async_setup(config_entry.entry_id) - # setup_component(self.hass, DOMAIN, MOCK_CONFIG) - # self.hass.async_block_till_done() + await hass.async_block_till_done() + + +async def _setup_monoprice_not_first_run(hass, monoprice): + with patch( + "homeassistant.components.monoprice.get_monoprice", new=lambda *a: monoprice, + ): + data = {**MOCK_CONFIG, CONF_NOT_FIRST_RUN: True} + config_entry = MockConfigEntry(domain=DOMAIN, data=data) + config_entry.add_to_hass(hass) + await hass.config_entries.async_setup(config_entry.entry_id) await hass.async_block_till_done() @@ -479,3 +486,47 @@ async def test_volume_up_down(hass): hass, SERVICE_VOLUME_DOWN, {"entity_id": ZONE_1_ID} ) assert monoprice.zones[11].volume == 37 + + +async def test_first_run_with_available_zones(hass): + """Test first run with all zones available.""" + monoprice = MockMonoprice() + await _setup_monoprice(hass, monoprice) + + registry = await hass.helpers.entity_registry.async_get_registry() + + entry = registry.async_get(ZONE_7_ID) + assert not entry.disabled + + +async def test_first_run_with_failing_zones(hass): + """Test first run with failed zones.""" + monoprice = MockMonoprice() + + with patch.object(MockMonoprice, "zone_status", side_effect=SerialException): + await _setup_monoprice(hass, monoprice) + + registry = await hass.helpers.entity_registry.async_get_registry() + + entry = registry.async_get(ZONE_1_ID) + assert not entry.disabled + + entry = registry.async_get(ZONE_7_ID) + assert entry.disabled + assert entry.disabled_by == "integration" + + +async def test_not_first_run_with_failing_zone(hass): + """Test first run with failed zones.""" + monoprice = MockMonoprice() + + with patch.object(MockMonoprice, "zone_status", side_effect=SerialException): + await _setup_monoprice_not_first_run(hass, monoprice) + + registry = await hass.helpers.entity_registry.async_get_registry() + + entry = registry.async_get(ZONE_1_ID) + assert not entry.disabled + + entry = registry.async_get(ZONE_7_ID) + assert not entry.disabled