Cleanup of code reviews from initial modern forms (#51794)

homeassistant/components/modern_forms/__init__.py

@@ -15,7 +15,6 @@ from homeassistant.components.fan import DOMAIN as FAN_DOMAIN
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.const import ATTR_MODEL, ATTR_NAME, ATTR_SW_VERSION, CONF_HOST
 from homeassistant.core import HomeAssistant
-from homeassistant.exceptions import ConfigEntryNotReady
 from homeassistant.helpers.aiohttp_client import async_get_clientsession
 from homeassistant.helpers.entity import DeviceInfo
 from homeassistant.helpers.update_coordinator import (
@@ -38,24 +37,13 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
 
     # Create Modern Forms instance for this entry
     coordinator = ModernFormsDataUpdateCoordinator(hass, host=entry.data[CONF_HOST])
-    await coordinator.async_refresh()
+    await coordinator.async_config_entry_first_refresh()
-
-    if not coordinator.last_update_success:
-        raise ConfigEntryNotReady
 
     hass.data.setdefault(DOMAIN, {})
     hass.data[DOMAIN][entry.entry_id] = coordinator
 
-    if entry.unique_id is None:
-        hass.config_entries.async_update_entry(
-            entry, unique_id=coordinator.data.info.mac_address
-        )
-
     # Set up all platforms for this device/entry.
-    for platform in PLATFORMS:
+    hass.config_entries.async_setup_platforms(entry, PLATFORMS)
-        hass.async_create_task(
-            hass.config_entries.async_forward_entry_setup(entry, platform)
-        )
 
     return True
 
@@ -106,7 +94,7 @@ class ModernFormsDataUpdateCoordinator(DataUpdateCoordinator[ModernFormsDeviceSt
         host: str,
     ) -> None:
         """Initialize global Modern Forms data updater."""
-        self.modernforms = ModernFormsDevice(
+        self.modern_forms = ModernFormsDevice(
             host, session=async_get_clientsession(hass)
         )
 
@@ -125,7 +113,7 @@ class ModernFormsDataUpdateCoordinator(DataUpdateCoordinator[ModernFormsDeviceSt
     async def _async_update_data(self) -> ModernFormsDevice:
         """Fetch data from Modern Forms."""
         try:
-            return await self.modernforms.update(
+            return await self.modern_forms.update(
                 full_update=not self.last_update_success
             )
         except ModernFormsError as error:
@@ -152,7 +140,6 @@ class ModernFormsDeviceEntity(CoordinatorEntity[ModernFormsDataUpdateCoordinator
         self._entry_id = entry_id
         self._attr_icon = icon
         self._attr_name = name
-        self._unsub_dispatcher = None
 
     @property
     def device_info(self) -> DeviceInfo:

homeassistant/components/modern_forms/const.py

@@ -4,25 +4,11 @@ DOMAIN = "modern_forms"
 
 ATTR_IDENTIFIERS = "identifiers"
 ATTR_MANUFACTURER = "manufacturer"
-ATTR_MODEL = "model"
-ATTR_OWNER = "owner"
-ATTR_IDENTITY = "identity"
-ATTR_MCU_FIRMWARE_VERSION = "mcu_firmware_version"
-ATTR_FIRMWARE_VERSION = "firmware_version"
 
-SIGNAL_INSTANCE_ADD = f"{DOMAIN}_instance_add_signal." "{}"
-SIGNAL_INSTANCE_REMOVE = f"{DOMAIN}_instance_remove_signal." "{}"
-SIGNAL_ENTITY_REMOVE = f"{DOMAIN}_entity_remove_signal." "{}"
-
-CONF_ON_UNLOAD = "ON_UNLOAD"
-
-OPT_BRIGHTNESS = "brightness"
 OPT_ON = "on"
 OPT_SPEED = "speed"
 
 # Services
-SERVICE_SET_LIGHT_SLEEP_TIMER = "set_light_sleep_timer"
-SERVICE_CLEAR_LIGHT_SLEEP_TIMER = "clear_light_sleep_timer"
 SERVICE_SET_FAN_SLEEP_TIMER = "set_fan_sleep_timer"
 SERVICE_CLEAR_FAN_SLEEP_TIMER = "clear_fan_sleep_timer"
 

homeassistant/components/modern_forms/fan.py

@@ -1,16 +1,15 @@
 """Support for Modern Forms Fan Fans."""
 from __future__ import annotations
 
-from functools import partial
+from typing import Any
-from typing import Any, Callable
 
 from aiomodernforms.const import FAN_POWER_OFF, FAN_POWER_ON
 import voluptuous as vol
 
 from homeassistant.components.fan import SUPPORT_DIRECTION, SUPPORT_SET_SPEED, FanEntity
 from homeassistant.config_entries import ConfigEntry
-from homeassistant.core import callback
 import homeassistant.helpers.entity_platform as entity_platform
+from homeassistant.helpers.entity_platform import AddEntitiesCallback
 from homeassistant.helpers.typing import HomeAssistantType
 from homeassistant.util.percentage import (
     int_states_in_range,
@@ -35,7 +34,9 @@ from .const import (
 
 
 async def async_setup_entry(
-    hass: HomeAssistantType, config_entry: ConfigEntry, async_add_entities: Callable
+    hass: HomeAssistantType,
+    config_entry: ConfigEntry,
+    async_add_entities: AddEntitiesCallback,
 ) -> None:
     """Set up a Modern Forms platform from config entry."""
 
@@ -61,11 +62,9 @@ async def async_setup_entry(
         "async_clear_fan_sleep_timer",
     )
 
-    update_func = partial(
+    async_add_entities(
-        async_update_fan, config_entry, coordinator, {}, async_add_entities
+        [ModernFormsFanEntity(entry_id=config_entry.entry_id, coordinator=coordinator)]
     )
-    coordinator.async_add_listener(update_func)
-    update_func()
 
 
 class ModernFormsFanEntity(FanEntity, ModernFormsDeviceEntity):
@@ -82,7 +81,7 @@ class ModernFormsFanEntity(FanEntity, ModernFormsDeviceEntity):
             coordinator=coordinator,
             name=f"{coordinator.data.info.device_name} Fan",
         )
-        self._attr_unique_id = f"{self.coordinator.data.info.mac_address}_fan"
+        self._attr_unique_id = f"{self.coordinator.data.info.mac_address}"
 
     @property
     def supported_features(self) -> int:
@@ -117,7 +116,7 @@ class ModernFormsFanEntity(FanEntity, ModernFormsDeviceEntity):
     @modernforms_exception_handler
     async def async_set_direction(self, direction: str) -> None:
         """Set the direction of the fan."""
-        await self.coordinator.modernforms.fan(direction=direction)
+        await self.coordinator.modern_forms.fan(direction=direction)
 
     @modernforms_exception_handler
     async def async_set_percentage(self, percentage: int) -> None:
@@ -142,12 +141,12 @@ class ModernFormsFanEntity(FanEntity, ModernFormsDeviceEntity):
             data[OPT_SPEED] = round(
                 percentage_to_ranged_value(self.SPEED_RANGE, percentage)
             )
-        await self.coordinator.modernforms.fan(**data)
+        await self.coordinator.modern_forms.fan(**data)
 
     @modernforms_exception_handler
     async def async_turn_off(self, **kwargs: Any) -> None:
         """Turn the fan off."""
-        await self.coordinator.modernforms.fan(on=FAN_POWER_OFF)
+        await self.coordinator.modern_forms.fan(on=FAN_POWER_OFF)
 
     @modernforms_exception_handler
     async def async_set_fan_sleep_timer(
@@ -155,26 +154,11 @@ class ModernFormsFanEntity(FanEntity, ModernFormsDeviceEntity):
         sleep_time: int,
     ) -> None:
         """Set a Modern Forms light sleep timer."""
-        await self.coordinator.modernforms.fan(sleep=sleep_time * 60)
+        await self.coordinator.modern_forms.fan(sleep=sleep_time * 60)
 
     @modernforms_exception_handler
     async def async_clear_fan_sleep_timer(
         self,
     ) -> None:
         """Clear a Modern Forms fan sleep timer."""
-        await self.coordinator.modernforms.fan(sleep=CLEAR_TIMER)
+        await self.coordinator.modern_forms.fan(sleep=CLEAR_TIMER)
-
-
-@callback
-def async_update_fan(
-    entry: ConfigEntry,
-    coordinator: ModernFormsDataUpdateCoordinator,
-    current: dict[str, ModernFormsFanEntity],
-    async_add_entities,
-) -> None:
-    """Update Modern Forms Fan info."""
-    if not current:
-        current[entry.entry_id] = ModernFormsFanEntity(
-            entry_id=entry.entry_id, coordinator=coordinator
-        )
-        async_add_entities([current[entry.entry_id]])

homeassistant/components/modern_forms/manifest.json

@@ -9,7 +9,6 @@
   "zeroconf": [
     {"type":"_easylink._tcp.local.", "name":"wac*"}
   ],
-  "dependencies": [],
   "codeowners": [
     "@wonderslug"
   ],

homeassistant/components/modern_forms/services.yaml

@@ -15,10 +15,7 @@ set_fan_sleep_timer:
         number:
           min: 1
           max: 1440
-          step: 1
           unit_of_measurement: minutes
-          mode: slider
-
 clear_fan_sleep_timer:
   name: Clear fan sleep timer
   description: Clear the sleep timer on a Modern Forms fan.

homeassistant/components/modern_forms/strings.json

@@ -1,5 +1,4 @@
 {
-  "title": "Modern Forms",
   "config": {
     "flow_title": "{name}",
     "step": {
@@ -9,9 +8,6 @@
           "host": "[%key:common::config_flow::data::host%]"
         }
       },
-      "confirm": {
-        "description": "[%key:common::config_flow::description::confirm_setup%]"
-      },
       "zeroconf_confirm": {
         "description": "Do you want to add the Modern Forms fan named `{name}` to Home Assistant?",
         "title": "Discovered Modern Forms fan device"

tests/components/modern_forms/test_config_flow.py

@@ -39,15 +39,20 @@ async def test_full_user_flow_implementation(
     assert result.get("type") == RESULT_TYPE_FORM
     assert "flow_id" in result
 
-    result = await hass.config_entries.flow.async_configure(
+    with patch(
+        "homeassistant.components.modern_forms.async_setup_entry",
+        return_value=True,
+    ) as mock_setup_entry:
+        result2 = await hass.config_entries.flow.async_configure(
             result["flow_id"], user_input={CONF_HOST: "192.168.1.123"}
         )
 
-    assert result.get("title") == "ModernFormsFan"
+    assert result2.get("title") == "ModernFormsFan"
-    assert "data" in result
+    assert "data" in result2
-    assert result.get("type") == RESULT_TYPE_CREATE_ENTRY
+    assert result2.get("type") == RESULT_TYPE_CREATE_ENTRY
-    assert result["data"][CONF_HOST] == "192.168.1.123"
+    assert result2["data"][CONF_HOST] == "192.168.1.123"
-    assert result["data"][CONF_MAC] == "AA:BB:CC:DD:EE:FF"
+    assert result2["data"][CONF_MAC] == "AA:BB:CC:DD:EE:FF"
+    assert len(mock_setup_entry.mock_calls) == 1
 
 
 async def test_full_zeroconf_flow_implementation(
@@ -166,12 +171,26 @@ async def test_user_device_exists_abort(
         headers={"Content-Type": CONTENT_TYPE_JSON},
     )
 
-    await init_integration(hass, aioclient_mock)
+    await init_integration(hass, aioclient_mock, skip_setup=True)
+
+    await hass.config_entries.flow.async_init(
+        DOMAIN,
+        context={"source": SOURCE_USER},
+        data={
+            "host": "192.168.1.123",
+            "hostname": "example.local.",
+            "properties": {CONF_MAC: "AA:BB:CC:DD:EE:FF"},
+        },
+    )
 
     result = await hass.config_entries.flow.async_init(
         DOMAIN,
         context={"source": SOURCE_USER},
-        data={CONF_HOST: "192.168.1.123"},
+        data={
+            "host": "192.168.1.123",
+            "hostname": "example.local.",
+            "properties": {CONF_MAC: "AA:BB:CC:DD:EE:FF"},
+        },
     )
 
     assert result.get("type") == RESULT_TYPE_ABORT
@@ -182,7 +201,17 @@ async def test_zeroconf_with_mac_device_exists_abort(
     hass: HomeAssistant, aioclient_mock: AiohttpClientMocker
 ) -> None:
     """Test we abort zeroconf flow if a Modern Forms device already configured."""
-    await init_integration(hass, aioclient_mock)
+    await init_integration(hass, aioclient_mock, skip_setup=True)
+
+    await hass.config_entries.flow.async_init(
+        DOMAIN,
+        context={"source": SOURCE_USER},
+        data={
+            "host": "192.168.1.123",
+            "hostname": "example.local.",
+            "properties": {CONF_MAC: "AA:BB:CC:DD:EE:FF"},
+        },
+    )
 
     result = await hass.config_entries.flow.async_init(
         DOMAIN,

tests/components/modern_forms/test_fan.py

@@ -48,7 +48,7 @@ async def test_fan_state(
 
     entry = entity_registry.async_get("fan.modernformsfan_fan")
     assert entry
-    assert entry.unique_id == "AA:BB:CC:DD:EE:FF_fan"
+    assert entry.unique_id == "AA:BB:CC:DD:EE:FF"
 
 
 async def test_change_state(

tests/components/modern_forms/test_init.py

@@ -39,14 +39,6 @@ async def test_unload_config_entry(
     assert not hass.data.get(DOMAIN)
 
 
-async def test_setting_unique_id(hass, aioclient_mock):
-    """Test we set unique ID if not set yet."""
-    entry = await init_integration(hass, aioclient_mock)
-
-    assert hass.data[DOMAIN]
-    assert entry.unique_id == "AA:BB:CC:DD:EE:FF"
-
-
 async def test_fan_only_device(hass, aioclient_mock):
     """Test we set unique ID if not set yet."""
     await init_integration(