Goodwe port502ftp support with PORT stored on config data. (home-assistant#148628)

Co-authored-by: starkillerOG <[email protected]>

Author: jlanchares

homeassistant/components/goodwe/__init__.py

@@ -1,42 +1,37 @@
 """The Goodwe inverter component."""
 
-from goodwe import InverterError, connect
-from goodwe.const import GOODWE_TCP_PORT, GOODWE_UDP_PORT
+from goodwe import Inverter, InverterError, connect
+from goodwe.const import GOODWE_UDP_PORT
 
-from homeassistant.const import CONF_HOST
+from homeassistant.const import CONF_HOST, CONF_PORT
 from homeassistant.core import HomeAssistant
 from homeassistant.exceptions import ConfigEntryNotReady
 from homeassistant.helpers.device_registry import DeviceInfo
 
+from .config_flow import GoodweFlowHandler
 from .const import CONF_MODEL_FAMILY, DOMAIN, PLATFORMS
 from .coordinator import GoodweConfigEntry, GoodweRuntimeData, GoodweUpdateCoordinator
 
 
 async def async_setup_entry(hass: HomeAssistant, entry: GoodweConfigEntry) -> bool:
     """Set up the Goodwe components from a config entry."""
     host = entry.data[CONF_HOST]
+    port = entry.data.get(CONF_PORT, GOODWE_UDP_PORT)
     model_family = entry.data[CONF_MODEL_FAMILY]
 
     # Connect to Goodwe inverter
     try:
         inverter = await connect(
             host=host,
-            port=GOODWE_UDP_PORT,
+            port=port,
             family=model_family,
             retries=10,
         )
-    except InverterError as err_udp:
-        # First try with UDP failed, trying with the TCP port
+    except InverterError as err:
         try:
-            inverter = await connect(
-                host=host,
-                port=GOODWE_TCP_PORT,
-                family=model_family,
-                retries=10,
-            )
+            inverter = await async_check_port(hass, entry, host)
         except InverterError:
-            # Both ports are unavailable
-            raise ConfigEntryNotReady from err_udp
+            raise ConfigEntryNotReady from err
 
     device_info = DeviceInfo(
         configuration_url="https://www.semsportal.com",
@@ -66,6 +61,23 @@ async def async_setup_entry(hass: HomeAssistant, entry: GoodweConfigEntry) -> bo
     return True
 
 
+async def async_check_port(
+    hass: HomeAssistant, entry: GoodweConfigEntry, host: str
+) -> Inverter:
+    """Check the communication port of the inverter, it may have changed after a firmware update."""
+    inverter, port = await GoodweFlowHandler.async_detect_inverter_port(host=host)
+    family = type(inverter).__name__
+    hass.config_entries.async_update_entry(
+        entry,
+        data={
+            CONF_HOST: host,
+            CONF_PORT: port,
+            CONF_MODEL_FAMILY: family,
+        },
+    )
+    return inverter
+
+
 async def async_unload_entry(
     hass: HomeAssistant, config_entry: GoodweConfigEntry
 ) -> bool:
@@ -76,3 +88,31 @@ async def async_unload_entry(
 async def update_listener(hass: HomeAssistant, config_entry: GoodweConfigEntry) -> None:
     """Handle options update."""
     await hass.config_entries.async_reload(config_entry.entry_id)
+
+
+async def async_migrate_entry(
+    hass: HomeAssistant, config_entry: GoodweConfigEntry
+) -> bool:
+    """Migrate old config entries."""
+
+    if config_entry.version > 2:
+        # This means the user has downgraded from a future version
+        return False
+
+    if config_entry.version == 1:
+        # Update from version 1 to version 2 adding the PROTOCOL to the config entry
+        host = config_entry.data[CONF_HOST]
+        try:
+            inverter, port = await GoodweFlowHandler.async_detect_inverter_port(
+                host=host
+            )
+        except InverterError as err:
+            raise ConfigEntryNotReady from err
+        new_data = {
+            CONF_HOST: host,
+            CONF_PORT: port,
+            CONF_MODEL_FAMILY: type(inverter).__name__,
+        }
+        hass.config_entries.async_update_entry(config_entry, data=new_data, version=2)
+
+    return True

homeassistant/components/goodwe/config_flow.py

@@ -5,12 +5,12 @@
 import logging
 from typing import Any
 
-from goodwe import InverterError, connect
+from goodwe import Inverter, InverterError, connect
 from goodwe.const import GOODWE_TCP_PORT, GOODWE_UDP_PORT
 import voluptuous as vol
 
 from homeassistant.config_entries import ConfigFlow, ConfigFlowResult
-from homeassistant.const import CONF_HOST
+from homeassistant.const import CONF_HOST, CONF_PORT
 
 from .const import CONF_MODEL_FAMILY, DEFAULT_NAME, DOMAIN
 
@@ -26,16 +26,23 @@
 class GoodweFlowHandler(ConfigFlow, domain=DOMAIN):
     """Handle a Goodwe config flow."""
 
-    VERSION = 1
+    MINOR_VERSION = 2
 
-    async def _handle_successful_connection(self, inverter, host):
+    async def async_handle_successful_connection(
+        self,
+        inverter: Inverter,
+        host: str,
+        port: int,
+    ) -> ConfigFlowResult:
+        """Handle a successful connection storing it's values on the entry data."""
         await self.async_set_unique_id(inverter.serial_number)
         self._abort_if_unique_id_configured()
 
         return self.async_create_entry(
             title=DEFAULT_NAME,
             data={
                 CONF_HOST: host,
+                CONF_PORT: port,
                 CONF_MODEL_FAMILY: type(inverter).__name__,
             },
         )
@@ -48,19 +55,26 @@ async def async_step_user(
         if user_input is not None:
             host = user_input[CONF_HOST]
             try:
-                inverter = await connect(host=host, port=GOODWE_UDP_PORT, retries=10)
+                inverter, port = await self.async_detect_inverter_port(host=host)
             except InverterError:
-                try:
-                    inverter = await connect(
-                        host=host, port=GOODWE_TCP_PORT, retries=10
-                    )
-                except InverterError:
-                    errors[CONF_HOST] = "connection_error"
-                else:
-                    return await self._handle_successful_connection(inverter, host)
+                errors[CONF_HOST] = "connection_error"
             else:
-                return await self._handle_successful_connection(inverter, host)
-
+                return await self.async_handle_successful_connection(
+                    inverter, host, port
+                )
         return self.async_show_form(
             step_id="user", data_schema=CONFIG_SCHEMA, errors=errors
         )
+
+    @staticmethod
+    async def async_detect_inverter_port(
+        host: str,
+    ) -> tuple[Inverter, int]:
+        """Detects the port of the Inverter."""
+        port = GOODWE_UDP_PORT
+        try:
+            inverter = await connect(host=host, port=port, retries=10)
+        except InverterError:
+            port = GOODWE_TCP_PORT
+            inverter = await connect(host=host, port=port, retries=10)
+        return inverter, port

tests/components/goodwe/conftest.py

@@ -1,16 +1,22 @@
 """Fixtures for the Aladdin Connect integration tests."""
 
-from unittest.mock import AsyncMock, MagicMock
+from collections.abc import Generator
+from unittest.mock import AsyncMock, MagicMock, patch
 
 from goodwe import Inverter
+from goodwe.const import GOODWE_UDP_PORT
 import pytest
 
+TEST_HOST = "1.2.3.4"
+TEST_PORT = GOODWE_UDP_PORT
+TEST_SERIAL = "123456789"
+
 
 @pytest.fixture(name="mock_inverter")
-def fixture_mock_inverter():
+def fixture_mock_inverter() -> Generator[MagicMock]:
     """Set up inverter fixture."""
     mock_inverter = MagicMock(spec=Inverter)
-    mock_inverter.serial_number = "dummy_serial_nr"
+    mock_inverter.serial_number = TEST_SERIAL
     mock_inverter.arm_version = 1
     mock_inverter.arm_svn_version = 2
     mock_inverter.arm_firmware = "dummy.arm.version"
@@ -23,4 +29,11 @@ def fixture_mock_inverter():
 
     mock_inverter.read_runtime_data = AsyncMock(return_value={})
 
-    return mock_inverter
+    with (
+        patch(
+            "homeassistant.components.goodwe.config_flow.connect",
+            return_value=mock_inverter,
+        ),
+        patch("homeassistant.components.goodwe.connect", return_value=mock_inverter),
+    ):
+        yield mock_inverter

tests/components/goodwe/snapshots/test_diagnostics.ambr

@@ -3,8 +3,9 @@
   dict({
     'config_entry': dict({
       'data': dict({
-        'host': 'localhost',
+        'host': '1.2.3.4',
         'model_family': 'ET',
+        'port': 8899,
       }),
       'disabled_by': None,
       'discovery_keys': dict({
@@ -21,7 +22,7 @@
       ]),
       'title': 'Mock Title',
       'unique_id': None,
-      'version': 1,
+      'version': 2,
     }),
     'inverter': dict({
       'arm_firmware': 'dummy.arm.version',

tests/components/goodwe/test_config_flow.py

@@ -1,33 +1,29 @@
 """Test the Goodwe config flow."""
 
-from unittest.mock import AsyncMock, patch
+from unittest.mock import MagicMock, patch
 
 from goodwe import InverterError
+from goodwe.const import GOODWE_UDP_PORT
 
 from homeassistant.components.goodwe.const import (
     CONF_MODEL_FAMILY,
     DEFAULT_NAME,
     DOMAIN,
 )
 from homeassistant.config_entries import SOURCE_USER
-from homeassistant.const import CONF_HOST
+from homeassistant.const import CONF_HOST, CONF_PORT
 from homeassistant.core import HomeAssistant
 from homeassistant.data_entry_flow import FlowResultType
 
+from .conftest import TEST_SERIAL
+
 from tests.common import MockConfigEntry
 
 TEST_HOST = "1.2.3.4"
-TEST_SERIAL = "123456789"
-
+TEST_PORT = GOODWE_UDP_PORT
 
-def mock_inverter():
-    """Get a mock object of the inverter."""
-    goodwe_inverter = AsyncMock()
-    goodwe_inverter.serial_number = TEST_SERIAL
-    return goodwe_inverter
 
-
-async def test_manual_setup(hass: HomeAssistant) -> None:
+async def test_manual_setup(hass: HomeAssistant, mock_inverter: MagicMock) -> None:
     """Test manually setting up."""
     result = await hass.config_entries.flow.async_init(
         DOMAIN, context={"source": SOURCE_USER}
@@ -37,10 +33,6 @@ async def test_manual_setup(hass: HomeAssistant) -> None:
     assert not result["errors"]
 
     with (
-        patch(
-            "homeassistant.components.goodwe.config_flow.connect",
-            return_value=mock_inverter(),
-        ),
         patch(
             "homeassistant.components.goodwe.async_setup_entry", return_value=True
         ) as mock_setup_entry,
@@ -54,15 +46,20 @@ async def test_manual_setup(hass: HomeAssistant) -> None:
     assert result["title"] == DEFAULT_NAME
     assert result["data"] == {
         CONF_HOST: TEST_HOST,
-        CONF_MODEL_FAMILY: "AsyncMock",
+        CONF_PORT: TEST_PORT,
+        CONF_MODEL_FAMILY: "MagicMock",
     }
     assert len(mock_setup_entry.mock_calls) == 1
 
 
-async def test_manual_setup_already_exists(hass: HomeAssistant) -> None:
+async def test_manual_setup_already_exists(
+    hass: HomeAssistant, mock_inverter: MagicMock
+) -> None:
     """Test manually setting up and the device already exists."""
     entry = MockConfigEntry(
-        domain=DOMAIN, data={CONF_HOST: TEST_HOST}, unique_id=TEST_SERIAL
+        domain=DOMAIN,
+        data={CONF_HOST: TEST_HOST},
+        unique_id=TEST_SERIAL,
     )
     entry.add_to_hass(hass)
     result = await hass.config_entries.flow.async_init(
@@ -72,13 +69,7 @@ async def test_manual_setup_already_exists(hass: HomeAssistant) -> None:
     assert result["step_id"] == "user"
     assert not result["errors"]
 
-    with (
-        patch(
-            "homeassistant.components.goodwe.config_flow.connect",
-            return_value=mock_inverter(),
-        ),
-        patch("homeassistant.components.goodwe.async_setup_entry", return_value=True),
-    ):
+    with patch("homeassistant.components.goodwe.async_setup_entry", return_value=True):
         result = await hass.config_entries.flow.async_configure(
             result["flow_id"], {CONF_HOST: TEST_HOST}
         )

tests/components/goodwe/test_diagnostics.py

@@ -1,15 +1,17 @@
-"""Test the CO2Signal diagnostics."""
+"""Test the GoodWe diagnostics."""
 
-from unittest.mock import MagicMock, patch
+from unittest.mock import MagicMock
 
 from syrupy.assertion import SnapshotAssertion
 from syrupy.filters import props
 
 from homeassistant.components.goodwe import CONF_MODEL_FAMILY, DOMAIN
-from homeassistant.const import CONF_HOST
+from homeassistant.const import CONF_HOST, CONF_PORT
 from homeassistant.core import HomeAssistant
 from homeassistant.setup import async_setup_component
 
+from .conftest import TEST_HOST, TEST_PORT
+
 from tests.common import MockConfigEntry
 from tests.components.diagnostics import get_diagnostics_for_config_entry
 from tests.typing import ClientSessionGenerator
@@ -24,13 +26,17 @@ async def test_entry_diagnostics(
     """Test config entry diagnostics."""
 
     config_entry = MockConfigEntry(
+        version=2,
         domain=DOMAIN,
-        data={CONF_HOST: "localhost", CONF_MODEL_FAMILY: "ET"},
+        data={
+            CONF_HOST: TEST_HOST,
+            CONF_PORT: TEST_PORT,
+            CONF_MODEL_FAMILY: "ET",
+        },
         entry_id="3bd2acb0e4f0476d40865546d0d91921",
     )
     config_entry.add_to_hass(hass)
-    with patch("homeassistant.components.goodwe.connect", return_value=mock_inverter):
-        assert await async_setup_component(hass, DOMAIN, {})
+    assert await async_setup_component(hass, DOMAIN, {})
 
     result = await get_diagnostics_for_config_entry(hass, hass_client, config_entry)
     assert result == snapshot(exclude=props("created_at", "modified_at"))