Fritz code quality improvements from #48287 and #50055 (#50479)

Co-authored-by: J. Nick Koston <nick@koston.org>

homeassistant/components/fritz/binary_sensor.py

@@ -21,7 +21,7 @@ async def async_setup_entry(
 ) -> None:
     """Set up entry."""
     _LOGGER.debug("Setting up FRITZ!Box binary sensors")
-    fritzbox_tools = hass.data[DOMAIN][entry.entry_id]
+    fritzbox_tools: FritzBoxTools = hass.data[DOMAIN][entry.entry_id]
 
     if "WANIPConn1" in fritzbox_tools.connection.services:
         # Only routers are supported at the moment
@@ -33,13 +33,15 @@ async def async_setup_entry(
 class FritzBoxConnectivitySensor(FritzBoxBaseEntity, BinarySensorEntity):
     """Define FRITZ!Box connectivity class."""
 
-    def __init__(self, fritzbox_tools: FritzBoxTools, device_friendlyname: str) -> None:
+    def __init__(
+        self, fritzbox_tools: FritzBoxTools, device_friendly_name: str
+    ) -> None:
         """Init FRITZ!Box connectivity class."""
         self._unique_id = f"{fritzbox_tools.unique_id}-connectivity"
-        self._name = f"{device_friendlyname} Connectivity"
+        self._name = f"{device_friendly_name} Connectivity"
         self._is_on = True
         self._is_available = True
-        super().__init__(fritzbox_tools, device_friendlyname)
+        super().__init__(fritzbox_tools, device_friendly_name)
 
     @property
     def name(self):
@@ -78,7 +80,7 @@ class FritzBoxConnectivitySensor(FritzBoxBaseEntity, BinarySensorEntity):
                 is_up = link_props["NewPhysicalLinkStatus"]
                 self._is_on = is_up == "Up"
             else:
-                self._is_on = self._fritzbox_tools.fritzstatus.is_connected
+                self._is_on = self._fritzbox_tools.fritz_status.is_connected
 
             self._is_available = True
 

homeassistant/components/fritz/common.py

@@ -61,8 +61,8 @@ class FritzBoxTools:
         self._devices: dict[str, Any] = {}
         self._unique_id = None
         self.connection = None
-        self.fritzhosts = None
+        self.fritz_hosts = None
-        self.fritzstatus = None
+        self.fritz_status = None
         self.hass = hass
         self.host = host
         self.password = password
@@ -86,7 +86,7 @@ class FritzBoxTools:
             timeout=60.0,
         )
 
-        self.fritzstatus = FritzStatus(fc=self.connection)
+        self.fritz_status = FritzStatus(fc=self.connection)
         info = self.connection.call_action("DeviceInfo:1", "GetInfo")
         if self._unique_id is None:
             self._unique_id = info["NewSerialNumber"]
@@ -97,7 +97,7 @@ class FritzBoxTools:
 
     async def async_start(self):
         """Start FritzHosts connection."""
-        self.fritzhosts = FritzHosts(fc=self.connection)
+        self.fritz_hosts = FritzHosts(fc=self.connection)
 
         await self.hass.async_add_executor_job(self.scan_devices)
 
@@ -135,7 +135,7 @@ class FritzBoxTools:
 
     def _update_info(self):
         """Retrieve latest information from the FRITZ!Box."""
-        return self.fritzhosts.get_hosts_info()
+        return self.fritz_hosts.get_hosts_info()
 
     def scan_devices(self, now: datetime | None = None) -> None:
         """Scan for new devices and return a list of found device ids."""

homeassistant/components/fritz/config_flow.py

@@ -22,7 +22,7 @@ from .const import (
     DEFAULT_PORT,
     DOMAIN,
     ERROR_AUTH_INVALID,
-    ERROR_CONNECTION_ERROR,
+    ERROR_CANNOT_CONNECT,
     ERROR_UNKNOWN,
 )
 
@@ -60,7 +60,7 @@ class FritzBoxToolsFlowHandler(ConfigFlow, domain=DOMAIN):
         except FritzSecurityError:
             return ERROR_AUTH_INVALID
         except FritzConnectionException:
-            return ERROR_CONNECTION_ERROR
+            return ERROR_CANNOT_CONNECT
         except Exception:  # pylint: disable=broad-except
             _LOGGER.exception("Unexpected exception")
             return ERROR_UNKNOWN

homeassistant/components/fritz/const.py

@@ -12,7 +12,7 @@ DEFAULT_PORT = 49000
 DEFAULT_USERNAME = ""
 
 ERROR_AUTH_INVALID = "invalid_auth"
-ERROR_CONNECTION_ERROR = "connection_error"
+ERROR_CANNOT_CONNECT = "cannot_connect"
 ERROR_UNKNOWN = "unknown_error"
 
 FRITZ_SERVICES = "fritz_services"

homeassistant/components/fritz/device_tracker.py

@@ -19,7 +19,7 @@ from homeassistant.helpers.device_registry import CONNECTION_NETWORK_MAC
 from homeassistant.helpers.dispatcher import async_dispatcher_connect
 from homeassistant.helpers.typing import ConfigType
 
-from .common import FritzBoxTools
+from .common import FritzBoxTools, FritzDevice
 from .const import DATA_FRITZ, DEFAULT_DEVICE_NAME, DOMAIN
 
 _LOGGER = logging.getLogger(__name__)
@@ -75,7 +75,9 @@ async def async_setup_entry(
         """Update the values of the router."""
         _async_add_entities(router, async_add_entities, data_fritz)
 
-    async_dispatcher_connect(hass, router.signal_device_new, update_router)
+    entry.async_on_unload(
+        async_dispatcher_connect(hass, router.signal_device_new, update_router)
+    )
 
     update_router()
 
@@ -109,7 +111,7 @@ def _async_add_entities(router, async_add_entities, data_fritz):
 class FritzBoxTracker(ScannerEntity):
     """This class queries a FRITZ!Box router."""
 
-    def __init__(self, router: FritzBoxTools, device):
+    def __init__(self, router: FritzBoxTools, device: FritzDevice) -> None:
         """Initialize a FRITZ!Box device."""
         self._router = router
         self._mac = device.mac_address
@@ -158,9 +160,9 @@ class FritzBoxTracker(ScannerEntity):
         return {
             "connections": {(CONNECTION_NETWORK_MAC, self._mac)},
             "identifiers": {(DOMAIN, self.unique_id)},
-            "name": self.name,
+            "default_name": self.name,
-            "manufacturer": "AVM",
+            "default_manufacturer": "AVM",
-            "model": "FRITZ!Box Tracked device",
+            "default_model": "FRITZ!Box Tracked device",
             "via_device": (
                 DOMAIN,
                 self._router.unique_id,

homeassistant/components/fritz/manifest.json

@@ -3,8 +3,7 @@
   "name": "AVM FRITZ!Box Tools",
   "documentation": "https://www.home-assistant.io/integrations/fritz",
   "requirements": [
-    "fritzconnection==1.4.2",
+    "fritzconnection==1.4.2"
-    "xmltodict==0.12.0"
   ],
   "codeowners": [
     "@mammuth",

homeassistant/components/fritz/sensor.py

@@ -8,7 +8,7 @@ from typing import Callable, TypedDict
 from fritzconnection.core.exceptions import FritzConnectionException
 from fritzconnection.lib.fritzstatus import FritzStatus
 
-from homeassistant.components.binary_sensor import BinarySensorEntity
+from homeassistant.components.sensor import SensorEntity
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.const import DEVICE_CLASS_TIMESTAMP
 from homeassistant.core import HomeAssistant
@@ -72,33 +72,34 @@ async def async_setup_entry(
 ) -> None:
     """Set up entry."""
     _LOGGER.debug("Setting up FRITZ!Box sensors")
-    fritzbox_tools = hass.data[DOMAIN][entry.entry_id]
+    fritzbox_tools: FritzBoxTools = hass.data[DOMAIN][entry.entry_id]
 
     if "WANIPConn1" not in fritzbox_tools.connection.services:
         # Only routers are supported at the moment
         return
 
+    entities = []
     for sensor_type in SENSOR_DATA:
-        async_add_entities(
+        entities.append(FritzBoxSensor(fritzbox_tools, entry.title, sensor_type))
-            [FritzBoxSensor(fritzbox_tools, entry.title, sensor_type)],
+
-            True,
+    if entities:
-        )
+        async_add_entities(entities, True)
 
 
-class FritzBoxSensor(FritzBoxBaseEntity, BinarySensorEntity):
+class FritzBoxSensor(FritzBoxBaseEntity, SensorEntity):
     """Define FRITZ!Box connectivity class."""
 
     def __init__(
-        self, fritzbox_tools: FritzBoxTools, device_friendlyname: str, sensor_type: str
+        self, fritzbox_tools: FritzBoxTools, device_friendly_name: str, sensor_type: str
     ) -> None:
         """Init FRITZ!Box connectivity class."""
         self._sensor_data: SensorData = SENSOR_DATA[sensor_type]
         self._unique_id = f"{fritzbox_tools.unique_id}-{sensor_type}"
-        self._name = f"{device_friendlyname} {self._sensor_data['name']}"
+        self._name = f"{device_friendly_name} {self._sensor_data['name']}"
         self._is_available = True
         self._last_value: str | None = None
         self._state: str | None = None
-        super().__init__(fritzbox_tools, device_friendlyname)
+        super().__init__(fritzbox_tools, device_friendly_name)
 
     @property
     def _state_provider(self) -> Callable:
@@ -140,7 +141,7 @@ class FritzBoxSensor(FritzBoxBaseEntity, BinarySensorEntity):
         _LOGGER.debug("Updating FRITZ!Box sensors")
 
         try:
-            status: FritzStatus = self._fritzbox_tools.fritzstatus
+            status: FritzStatus = self._fritzbox_tools.fritz_status
             self._is_available = True
         except FritzConnectionException:
             _LOGGER.error("Error getting the state from the FRITZ!Box", exc_info=True)

homeassistant/components/fritz/services.py

@@ -12,10 +12,10 @@ _LOGGER = logging.getLogger(__name__)
 
 async def async_setup_services(hass: HomeAssistant):
     """Set up services for Fritz integration."""
-    if hass.data.get(FRITZ_SERVICES, False):
-        return
 
-    hass.data[FRITZ_SERVICES] = True
+    for service in [SERVICE_REBOOT, SERVICE_RECONNECT]:
+        if hass.services.has_service(DOMAIN, service):
+            return
 
     async def async_call_fritz_service(service_call):
         """Call correct Fritz service."""
@@ -31,7 +31,7 @@ async def async_setup_services(hass: HomeAssistant):
 
         for entry in fritzbox_entry_ids:
             _LOGGER.debug("Executing service %s", service_call.service)
-            fritz_tools = hass.data[DOMAIN].get(entry)
+            fritz_tools = hass.data[DOMAIN][entry]
             await fritz_tools.service_fritzbox(service_call.service)
 
     for service in [SERVICE_REBOOT, SERVICE_RECONNECT]:

homeassistant/components/fritz/strings.json

@@ -10,16 +10,6 @@
           "password": "[%key:common::config_flow::data::password%]"
         }
       },
-      "start_config": {
-        "title": "Setup FRITZ!Box Tools - mandatory",
-        "description": "Setup FRITZ!Box Tools to control your FRITZ!Box.\nMinimum needed: username, password.",
-        "data": {
-          "host": "[%key:common::config_flow::data::host%]",
-          "port": "[%key:common::config_flow::data::port%]",
-          "username": "[%key:common::config_flow::data::username%]",
-          "password": "[%key:common::config_flow::data::password%]"
-        }
-      },
       "reauth_confirm": {
         "title": "Updating FRITZ!Box Tools - credentials",
         "description": "Update FRITZ!Box Tools credentials for: {host}.\n\nFRITZ!Box Tools is unable to log in to your FRITZ!Box.",
@@ -35,7 +25,7 @@
       "reauth_successful": "[%key:common::config_flow::abort::reauth_successful%]"
     },
     "error": {
-      "connection_error": "[%key:common::config_flow::error::cannot_connect%]",
+      "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]",
       "already_in_progress": "[%key:common::config_flow::abort::already_in_progress%]",
       "already_configured": "[%key:common::config_flow::abort::already_configured_device%]",
       "invalid_auth": "[%key:common::config_flow::error::invalid_auth%]"

homeassistant/components/fritz/translations/en.json

@@ -8,7 +8,7 @@
         "error": {
             "already_configured": "Device is already configured",
             "already_in_progress": "Configuration flow is already in progress",
-            "connection_error": "Failed to connect",
+            "cannot_connect": "Failed to connect",
             "invalid_auth": "Invalid authentication"
         },
         "flow_title": "{name}",
@@ -28,16 +28,6 @@
                 },
                 "description": "Update FRITZ!Box Tools credentials for: {host}.\n\nFRITZ!Box Tools is unable to log in to your FRITZ!Box.",
                 "title": "Updating FRITZ!Box Tools - credentials"
-            },
-            "start_config": {
-                "data": {
-                    "host": "Host",
-                    "password": "Password",
-                    "port": "Port",
-                    "username": "Username"
-                },
-                "description": "Setup FRITZ!Box Tools to control your FRITZ!Box.\nMinimum needed: username, password.",
-                "title": "Setup FRITZ!Box Tools - mandatory"
             }
         }
     }

requirements_all.txt

@@ -2368,7 +2368,6 @@ xboxapi==2.0.1
 xknx==0.18.1
 
 # homeassistant.components.bluesound
-# homeassistant.components.fritz
 # homeassistant.components.rest
 # homeassistant.components.startca
 # homeassistant.components.ted5000

requirements_test_all.txt

@@ -1271,7 +1271,6 @@ xbox-webapi==2.0.11
 xknx==0.18.1
 
 # homeassistant.components.bluesound
-# homeassistant.components.fritz
 # homeassistant.components.rest
 # homeassistant.components.startca
 # homeassistant.components.ted5000

tests/components/fritz/__init__.py

@@ -106,23 +106,22 @@ class FritzConnectionMock:  # pylint: disable=too-few-public-methods
     def __init__(self):
         """Inint Mocking class."""
         type(self).modelname = mock.PropertyMock(return_value=self.MODELNAME)
-        self.call_action = mock.Mock(side_effect=self._side_effect_callaction)
+        self.call_action = mock.Mock(side_effect=self._side_effect_call_action)
-        type(self).actionnames = mock.PropertyMock(
+        type(self).action_names = mock.PropertyMock(
-            side_effect=self._side_effect_actionnames
+            side_effect=self._side_effect_action_names
         )
         services = {
             srv: None
-            for srv, _ in list(self.FRITZBOX_DATA.keys())
+            for srv, _ in list(self.FRITZBOX_DATA) + list(self.FRITZBOX_DATA_INDEXED)
-            + list(self.FRITZBOX_DATA_INDEXED.keys())
         }
         type(self).services = mock.PropertyMock(side_effect=[services])
 
-    def _side_effect_callaction(self, service, action, **kwargs):
+    def _side_effect_call_action(self, service, action, **kwargs):
         if kwargs:
             index = next(iter(kwargs.values()))
             return self.FRITZBOX_DATA_INDEXED[(service, action)][index]
 
         return self.FRITZBOX_DATA[(service, action)]
 
-    def _side_effect_actionnames(self):
+    def _side_effect_action_names(self):
-        return list(self.FRITZBOX_DATA.keys()) + list(self.FRITZBOX_DATA_INDEXED.keys())
+        return list(self.FRITZBOX_DATA) + list(self.FRITZBOX_DATA_INDEXED)

tests/components/fritz/test_config_flow.py

@@ -7,7 +7,7 @@ import pytest
 from homeassistant.components.fritz.const import (
     DOMAIN,
     ERROR_AUTH_INVALID,
-    ERROR_CONNECTION_ERROR,
+    ERROR_CANNOT_CONNECT,
     ERROR_UNKNOWN,
 )
 from homeassistant.components.ssdp import (
@@ -111,6 +111,7 @@ async def test_user_already_configured(hass: HomeAssistant, fc_class_mock):
         )
         assert result["type"] == RESULT_TYPE_FORM
         assert result["step_id"] == "user"
+        assert result["errors"]["base"] == "already_configured"
 
 
 async def test_exception_security(hass: HomeAssistant):
@@ -156,7 +157,7 @@ async def test_exception_connection(hass: HomeAssistant):
 
         assert result["type"] == RESULT_TYPE_FORM
         assert result["step_id"] == "user"
-        assert result["errors"]["base"] == ERROR_CONNECTION_ERROR
+        assert result["errors"]["base"] == ERROR_CANNOT_CONNECT
 
 
 async def test_exception_unknown(hass: HomeAssistant):
@@ -248,6 +249,7 @@ async def test_reauth_not_successful(hass: HomeAssistant, fc_class_mock):
 
         assert result["type"] == RESULT_TYPE_FORM
         assert result["step_id"] == "reauth_confirm"
+        assert result["errors"]["base"] == "cannot_connect"
 
 
 async def test_ssdp_already_configured(hass: HomeAssistant, fc_class_mock):