diff --git a/CODEOWNERS b/CODEOWNERS index a021a4a28ed17..ade3ce4dea754 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -240,7 +240,7 @@ homeassistant/components/minecraft_server/* @elmurato homeassistant/components/minio/* @tkislan homeassistant/components/mobile_app/* @robbiet480 homeassistant/components/modbus/* @adamchengtkc @janiversen -homeassistant/components/monoprice/* @etsinko +homeassistant/components/monoprice/* @etsinko @OnFreund homeassistant/components/moon/* @fabaff homeassistant/components/mpd/* @fabaff homeassistant/components/mqtt/* @home-assistant/core @emontnemery diff --git a/homeassistant/components/monoprice/__init__.py b/homeassistant/components/monoprice/__init__.py index 37593f6828e32..9bceff1531c14 100644 --- a/homeassistant/components/monoprice/__init__.py +++ b/homeassistant/components/monoprice/__init__.py @@ -10,7 +10,13 @@ from homeassistant.core import HomeAssistant from homeassistant.exceptions import ConfigEntryNotReady -from .const import DOMAIN +from .const import ( + CONF_NOT_FIRST_RUN, + DOMAIN, + FIRST_RUN, + MONOPRICE_OBJECT, + UNDO_UPDATE_LISTENER, +) PLATFORMS = ["media_player"] @@ -28,12 +34,25 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry): try: monoprice = await hass.async_add_executor_job(get_monoprice, port) - hass.data.setdefault(DOMAIN, {})[entry.entry_id] = monoprice except SerialException: _LOGGER.error("Error connecting to Monoprice controller at %s", port) raise ConfigEntryNotReady - entry.add_update_listener(_update_listener) + # 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: hass.async_create_task( @@ -54,6 +73,10 @@ async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry): ) ) + if unload_ok: + hass.data[DOMAIN][entry.entry_id][UNDO_UPDATE_LISTENER]() + hass.data[DOMAIN].pop(entry.entry_id) + return unload_ok diff --git a/homeassistant/components/monoprice/config_flow.py b/homeassistant/components/monoprice/config_flow.py index cbabc65a54b01..6c6bc87bf28df 100644 --- a/homeassistant/components/monoprice/config_flow.py +++ b/homeassistant/components/monoprice/config_flow.py @@ -99,7 +99,9 @@ def async_get_options_flow(config_entry): @core.callback def _key_for_source(index, source, previous_sources): if str(index) in previous_sources: - key = vol.Optional(source, default=previous_sources[str(index)]) + key = vol.Optional( + source, description={"suggested_value": previous_sources[str(index)]} + ) else: key = vol.Optional(source) diff --git a/homeassistant/components/monoprice/const.py b/homeassistant/components/monoprice/const.py index ea4667a77ffe6..576e4aa0e6909 100644 --- a/homeassistant/components/monoprice/const.py +++ b/homeassistant/components/monoprice/const.py @@ -11,5 +11,11 @@ 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/manifest.json b/homeassistant/components/monoprice/manifest.json index c88673b285537..93cebc9d88546 100644 --- a/homeassistant/components/monoprice/manifest.json +++ b/homeassistant/components/monoprice/manifest.json @@ -3,6 +3,6 @@ "name": "Monoprice 6-Zone Amplifier", "documentation": "https://www.home-assistant.io/integrations/monoprice", "requirements": ["pymonoprice==0.3"], - "codeowners": ["@etsinko"], + "codeowners": ["@etsinko", "@OnFreund"], "config_flow": true } diff --git a/homeassistant/components/monoprice/media_player.py b/homeassistant/components/monoprice/media_player.py index 855d1b41f9886..4d6d337667e62 100644 --- a/homeassistant/components/monoprice/media_player.py +++ b/homeassistant/components/monoprice/media_player.py @@ -16,7 +16,14 @@ from homeassistant.const import CONF_PORT, STATE_OFF, STATE_ON from homeassistant.helpers import config_validation as cv, entity_platform, service -from .const import CONF_SOURCES, DOMAIN, SERVICE_RESTORE, SERVICE_SNAPSHOT +from .const import ( + CONF_SOURCES, + DOMAIN, + FIRST_RUN, + MONOPRICE_OBJECT, + SERVICE_RESTORE, + SERVICE_SNAPSHOT, +) _LOGGER = logging.getLogger(__name__) @@ -58,7 +65,7 @@ async def async_setup_entry(hass, config_entry, async_add_entities): """Set up the Monoprice 6-zone amplifier platform.""" port = config_entry.data[CONF_PORT] - monoprice = hass.data[DOMAIN][config_entry.entry_id] + monoprice = hass.data[DOMAIN][config_entry.entry_id][MONOPRICE_OBJECT] sources = _get_sources(config_entry) @@ -71,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() @@ -128,16 +137,19 @@ def __init__(self, monoprice, sources, namespace, zone_id): 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 @@ -152,7 +164,7 @@ def update(self): @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 f70a19f51fc1e..ccd70c628e282 100644 --- a/tests/components/monoprice/test_media_player.py +++ b/tests/components/monoprice/test_media_player.py @@ -17,6 +17,7 @@ SUPPORT_VOLUME_STEP, ) from homeassistant.components.monoprice.const import ( + CONF_NOT_FIRST_RUN, CONF_SOURCES, DOMAIN, SERVICE_RESTORE, @@ -41,6 +42,7 @@ 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