SMA code quality improvement and bugfix (#49346)

* Minor code quality improvements
Thanks to @MartinHjelmare

* Convert legacy dict config to list

* Improved test

* Typo

* Test improvements

* Create fixtures in conftest.py

homeassistant/components/sma/__init__.py

@@ -2,6 +2,7 @@
 import asyncio
 from datetime import timedelta
 import logging
+from typing import List
 
 import pysma
 
@@ -39,7 +40,7 @@ from .const import (
 _LOGGER = logging.getLogger(__name__)
 
 
-async def _parse_legacy_options(entry: ConfigEntry, sensor_def: pysma.Sensors) -> None:
+def _parse_legacy_options(entry: ConfigEntry, sensor_def: pysma.Sensors) -> List[str]:
     """Parse legacy configuration options.
 
     This will parse the legacy CONF_SENSORS and CONF_CUSTOM configuration options
@@ -57,7 +58,18 @@ async def _parse_legacy_options(entry: ConfigEntry, sensor_def: pysma.Sensors) -
     # Parsing of sensors configuration
     config_sensors = entry.data.get(CONF_SENSORS)
     if not config_sensors:
-        return
+        return []
+
+    # Support import of legacy config that should have been removed from 0.99, but was still functional
+    # See also #25880 and #26306. Functional support was dropped in #48003
+    if isinstance(config_sensors, dict):
+        config_sensors_list = []
+
+        for name, attr in config_sensors.items():
+            config_sensors_list.append(name)
+            config_sensors_list.extend(attr)
+
+        config_sensors = config_sensors_list
 
     # Find and replace sensors removed from pysma
     # This only alters the config, the actual sensor migration takes place in _migrate_old_unique_ids
@@ -70,20 +82,21 @@ async def _parse_legacy_options(entry: ConfigEntry, sensor_def: pysma.Sensors) -
     for sensor in sensor_def:
         sensor.enabled = sensor.name in config_sensors
 
+    return config_sensors
 
-async def _migrate_old_unique_ids(
+
-    hass: HomeAssistant, entry: ConfigEntry, sensor_def: pysma.Sensors
+def _migrate_old_unique_ids(
+    hass: HomeAssistant,
+    entry: ConfigEntry,
+    sensor_def: pysma.Sensors,
+    config_sensors: List[str],
 ) -> None:
     """Migrate legacy sensor entity_id format to new format."""
     entity_registry = er.async_get(hass)
 
     # Create list of all possible sensor names
-    possible_sensors = list(
+    possible_sensors = set(
-        set(
+        config_sensors + [s.name for s in sensor_def] + list(pysma.LEGACY_MAP)
-            entry.data.get(CONF_SENSORS)
-            + [s.name for s in sensor_def]
-            + list(pysma.LEGACY_MAP)
-        )
     )
 
     for sensor in possible_sensors:
@@ -107,7 +120,7 @@ async def _migrate_old_unique_ids(
         if not entity_id:
             continue
 
-        # Change entity_id to new format using the device serial in entry.unique_id
+        # Change unique_id to new format using the device serial in entry.unique_id
         new_unique_id = f"{entry.unique_id}-{pysma_sensor.key}_{pysma_sensor.key_idx}"
         entity_registry.async_update_entity(entity_id, new_unique_id=new_unique_id)
 
@@ -118,15 +131,15 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
     sensor_def = pysma.Sensors()
 
     if entry.source == SOURCE_IMPORT:
-        await _parse_legacy_options(entry, sensor_def)
+        config_sensors = _parse_legacy_options(entry, sensor_def)
-        await _migrate_old_unique_ids(hass, entry, sensor_def)
+        _migrate_old_unique_ids(hass, entry, sensor_def, config_sensors)
 
     # Init the SMA interface
-    protocol = "https" if entry.data.get(CONF_SSL) else "http"
+    protocol = "https" if entry.data[CONF_SSL] else "http"
-    url = f"{protocol}://{entry.data.get(CONF_HOST)}"
+    url = f"{protocol}://{entry.data[CONF_HOST]}"
-    verify_ssl = entry.data.get(CONF_VERIFY_SSL)
+    verify_ssl = entry.data[CONF_VERIFY_SSL]
-    group = entry.data.get(CONF_GROUP)
+    group = entry.data[CONF_GROUP]
-    password = entry.data.get(CONF_PASSWORD)
+    password = entry.data[CONF_PASSWORD]
 
     session = async_get_clientsession(hass, verify_ssl=verify_ssl)
     sma = pysma.SMA(session, url, password, group)

tests/components/sma/__init__.py

@@ -1,11 +1,6 @@
 """Tests for the sma integration."""
 from unittest.mock import patch
 
-from homeassistant.components.sma.const import DOMAIN
-from homeassistant.helpers import entity_registry as er
-
-from tests.common import MockConfigEntry
-
 MOCK_DEVICE = {
     "manufacturer": "SMA",
     "name": "SMA Device Name",
@@ -38,6 +33,25 @@ MOCK_IMPORT = {
     },
 }
 
+MOCK_IMPORT_DICT = {
+    "platform": "sma",
+    "host": "1.1.1.1",
+    "ssl": True,
+    "verify_ssl": False,
+    "group": "user",
+    "password": "password",
+    "sensors": {
+        "pv_power": [],
+        "pv_gen_meter": [],
+        "solar_daily": ["daily_yield", "total_yield"],
+        "status": ["grid_power", "frequency", "voltage_l1", "operating_time"],
+    },
+    "custom": {
+        "operating_time": {"key": "6400_00462E00", "unit": "uur", "factor": 3600},
+        "solar_daily": {"key": "6400_00262200", "unit": "kWh", "factor": 1000},
+    },
+}
+
 MOCK_CUSTOM_SENSOR = {
     "name": "yesterday_consumption",
     "key": "6400_00543A01",
@@ -83,41 +97,6 @@ MOCK_CUSTOM_SETUP_DATA = dict(
     **MOCK_USER_INPUT,
 )
 
-MOCK_LEGACY_ENTRY = er.RegistryEntry(
-    entity_id="sensor.pv_power",
-    unique_id="sma-6100_0046C200-pv_power",
-    platform="sma",
-    unit_of_measurement="W",
-    original_name="pv_power",
-)
-
-
-async def init_integration(hass):
-    """Create a fake SMA Config Entry."""
-    entry = MockConfigEntry(
-        domain=DOMAIN,
-        title=MOCK_DEVICE["name"],
-        unique_id=MOCK_DEVICE["serial"],
-        data=MOCK_CUSTOM_SETUP_DATA,
-        source="import",
-    )
-    entry.add_to_hass(hass)
-
-    with patch(
-        "homeassistant.helpers.update_coordinator.DataUpdateCoordinator.async_config_entry_first_refresh"
-    ):
-        await hass.config_entries.async_setup(entry.entry_id)
-    await hass.async_block_till_done()
-    return entry
-
-
-def _patch_validate_input(return_value=MOCK_DEVICE, side_effect=None):
-    return patch(
-        "homeassistant.components.sma.config_flow.validate_input",
-        return_value=return_value,
-        side_effect=side_effect,
-    )
-
 
 def _patch_async_setup_entry(return_value=True):
     return patch(

tests/components/sma/conftest.py

@@ -0,0 +1,33 @@
+"""Fixtures for sma tests."""
+from unittest.mock import patch
+
+import pytest
+
+from homeassistant.components.sma.const import DOMAIN
+
+from . import MOCK_CUSTOM_SETUP_DATA, MOCK_DEVICE
+
+from tests.common import MockConfigEntry
+
+
+@pytest.fixture
+def mock_config_entry():
+    """Return the default mocked config entry."""
+    return MockConfigEntry(
+        domain=DOMAIN,
+        title=MOCK_DEVICE["name"],
+        unique_id=MOCK_DEVICE["serial"],
+        data=MOCK_CUSTOM_SETUP_DATA,
+        source="import",
+    )
+
+
+@pytest.fixture
+async def init_integration(hass, mock_config_entry):
+    """Create a fake SMA Config Entry."""
+    mock_config_entry.add_to_hass(hass)
+
+    with patch("pysma.SMA.read"):
+        await hass.config_entries.async_setup(mock_config_entry.entry_id)
+    await hass.async_block_till_done()
+    return mock_config_entry

tests/components/sma/test_config_flow.py

@@ -11,20 +11,18 @@ from homeassistant.data_entry_flow import (
     RESULT_TYPE_CREATE_ENTRY,
     RESULT_TYPE_FORM,
 )
-from homeassistant.helpers import entity_registry as er
 
 from . import (
     MOCK_DEVICE,
     MOCK_IMPORT,
-    MOCK_LEGACY_ENTRY,
+    MOCK_IMPORT_DICT,
     MOCK_SETUP_DATA,
     MOCK_USER_INPUT,
     _patch_async_setup_entry,
-    _patch_validate_input,
 )
 
 
-async def test_form(hass, aioclient_mock):
+async def test_form(hass):
     """Test we get the form."""
     await setup.async_setup_component(hass, "persistent_notification", {})
     result = await hass.config_entries.flow.async_init(
@@ -49,30 +47,34 @@ async def test_form(hass, aioclient_mock):
     assert len(mock_setup_entry.mock_calls) == 1
 
 
-async def test_form_cannot_connect(hass, aioclient_mock):
+async def test_form_cannot_connect(hass):
     """Test we handle cannot connect error."""
-    aioclient_mock.get("https://1.1.1.1/data/l10n/en-US.json", exc=aiohttp.ClientError)
-
     result = await hass.config_entries.flow.async_init(
         DOMAIN, context={"source": SOURCE_USER}
     )
 
-    result = await hass.config_entries.flow.async_configure(
+    with patch(
-        result["flow_id"],
+        "pysma.SMA.new_session", side_effect=aiohttp.ClientError
-        MOCK_USER_INPUT,
+    ), _patch_async_setup_entry() as mock_setup_entry:
-    )
+        result = await hass.config_entries.flow.async_configure(
+            result["flow_id"],
+            MOCK_USER_INPUT,
+        )
 
     assert result["type"] == RESULT_TYPE_FORM
     assert result["errors"] == {"base": "cannot_connect"}
+    assert len(mock_setup_entry.mock_calls) == 0
 
 
-async def test_form_invalid_auth(hass, aioclient_mock):
+async def test_form_invalid_auth(hass):
     """Test we handle invalid auth error."""
     result = await hass.config_entries.flow.async_init(
         DOMAIN, context={"source": SOURCE_USER}
     )
 
-    with patch("pysma.SMA.new_session", return_value=False):
+    with patch(
+        "pysma.SMA.new_session", return_value=False
+    ), _patch_async_setup_entry() as mock_setup_entry:
         result = await hass.config_entries.flow.async_configure(
             result["flow_id"],
             MOCK_USER_INPUT,
@@ -80,9 +82,10 @@ async def test_form_invalid_auth(hass, aioclient_mock):
 
     assert result["type"] == RESULT_TYPE_FORM
     assert result["errors"] == {"base": "invalid_auth"}
+    assert len(mock_setup_entry.mock_calls) == 0
 
 
-async def test_form_cannot_retrieve_device_info(hass, aioclient_mock):
+async def test_form_cannot_retrieve_device_info(hass):
     """Test we handle cannot retrieve device info error."""
     result = await hass.config_entries.flow.async_init(
         DOMAIN, context={"source": SOURCE_USER}
@@ -90,7 +93,7 @@ async def test_form_cannot_retrieve_device_info(hass, aioclient_mock):
 
     with patch("pysma.SMA.new_session", return_value=True), patch(
         "pysma.SMA.read", return_value=False
-    ):
+    ), _patch_async_setup_entry() as mock_setup_entry:
         result = await hass.config_entries.flow.async_configure(
             result["flow_id"],
             MOCK_USER_INPUT,
@@ -98,6 +101,7 @@ async def test_form_cannot_retrieve_device_info(hass, aioclient_mock):
 
     assert result["type"] == RESULT_TYPE_FORM
     assert result["errors"] == {"base": "cannot_retrieve_device_info"}
+    assert len(mock_setup_entry.mock_calls) == 0
 
 
 async def test_form_unexpected_exception(hass):
@@ -106,7 +110,9 @@ async def test_form_unexpected_exception(hass):
         DOMAIN, context={"source": SOURCE_USER}
     )
 
-    with _patch_validate_input(side_effect=Exception):
+    with patch(
+        "pysma.SMA.new_session", side_effect=Exception
+    ), _patch_async_setup_entry() as mock_setup_entry:
         result = await hass.config_entries.flow.async_configure(
             result["flow_id"],
             MOCK_USER_INPUT,
@@ -114,28 +120,23 @@ async def test_form_unexpected_exception(hass):
 
     assert result["type"] == RESULT_TYPE_FORM
     assert result["errors"] == {"base": "unknown"}
+    assert len(mock_setup_entry.mock_calls) == 0
 
 
-async def test_form_already_configured(hass):
+async def test_form_already_configured(hass, mock_config_entry):
     """Test starting a flow by user when already configured."""
-    result = await hass.config_entries.flow.async_init(
+    mock_config_entry.add_to_hass(hass)
-        DOMAIN, context={"source": SOURCE_USER}
-    )
-
-    with _patch_validate_input():
-        result = await hass.config_entries.flow.async_configure(
-            result["flow_id"],
-            MOCK_USER_INPUT,
-        )
-
-    assert result["type"] == RESULT_TYPE_CREATE_ENTRY
-    assert result["result"].unique_id == MOCK_DEVICE["serial"]
 
     result = await hass.config_entries.flow.async_init(
         DOMAIN, context={"source": SOURCE_USER}
     )
 
-    with _patch_validate_input():
+    with patch("pysma.SMA.new_session", return_value=True), patch(
+        "pysma.SMA.device_info", return_value=MOCK_DEVICE
+    ), patch(
+        "pysma.SMA.close_session", return_value=True
+    ), _patch_async_setup_entry() as mock_setup_entry:
+
         result = await hass.config_entries.flow.async_configure(
             result["flow_id"],
             MOCK_USER_INPUT,
@@ -143,16 +144,18 @@ async def test_form_already_configured(hass):
 
     assert result["type"] == RESULT_TYPE_ABORT
     assert result["reason"] == "already_configured"
+    assert len(mock_setup_entry.mock_calls) == 0
 
 
 async def test_import(hass):
     """Test we can import."""
-    entity_registry = er.async_get(hass)
-    entity_registry._register_entry(MOCK_LEGACY_ENTRY)
-
     await setup.async_setup_component(hass, "persistent_notification", {})
 
-    with _patch_validate_input():
+    with patch("pysma.SMA.new_session", return_value=True), patch(
+        "pysma.SMA.device_info", return_value=MOCK_DEVICE
+    ), patch(
+        "pysma.SMA.close_session", return_value=True
+    ), _patch_async_setup_entry() as mock_setup_entry:
         result = await hass.config_entries.flow.async_init(
             DOMAIN,
             context={"source": SOURCE_IMPORT},
@@ -162,9 +165,25 @@ async def test_import(hass):
     assert result["type"] == RESULT_TYPE_CREATE_ENTRY
     assert result["title"] == MOCK_USER_INPUT["host"]
     assert result["data"] == MOCK_IMPORT
+    assert len(mock_setup_entry.mock_calls) == 1
 
-    assert MOCK_LEGACY_ENTRY.original_name not in result["data"]["sensors"]
-    assert "pv_power_a" in result["data"]["sensors"]
 
-    entity = entity_registry.async_get(MOCK_LEGACY_ENTRY.entity_id)
+async def test_import_sensor_dict(hass):
-    assert entity.unique_id == f"{MOCK_DEVICE['serial']}-6380_40251E00_0"
+    """Test we can import."""
+    await setup.async_setup_component(hass, "persistent_notification", {})
+
+    with patch("pysma.SMA.new_session", return_value=True), patch(
+        "pysma.SMA.device_info", return_value=MOCK_DEVICE
+    ), patch(
+        "pysma.SMA.close_session", return_value=True
+    ), _patch_async_setup_entry() as mock_setup_entry:
+        result = await hass.config_entries.flow.async_init(
+            DOMAIN,
+            context={"source": SOURCE_IMPORT},
+            data=MOCK_IMPORT_DICT,
+        )
+
+    assert result["type"] == RESULT_TYPE_CREATE_ENTRY
+    assert result["title"] == MOCK_USER_INPUT["host"]
+    assert result["data"] == MOCK_IMPORT_DICT
+    assert len(mock_setup_entry.mock_calls) == 1

tests/components/sma/test_sensor.py

@@ -5,13 +5,11 @@ from homeassistant.const import (
     POWER_WATT,
 )
 
-from . import MOCK_CUSTOM_SENSOR, init_integration
+from . import MOCK_CUSTOM_SENSOR
 
 
-async def test_sensors(hass):
+async def test_sensors(hass, init_integration):
     """Test states of the sensors."""
-    await init_integration(hass)
-
     state = hass.states.get("sensor.current_consumption")
     assert state
     assert state.attributes.get(ATTR_UNIT_OF_MEASUREMENT) == POWER_WATT