Rympro integration code fixes (#86734)

* Address review comments

* Add coordinator.py to coveragerc

* Apply suggestions from code review

Co-authored-by: Marc Mueller <30130371+cdce8p@users.noreply.github.com>

* Update homeassistant/components/rympro/coordinator.py

Co-authored-by: Marc Mueller <30130371+cdce8p@users.noreply.github.com>

* Move SCAN_INTERVAL to coordinator.py

---------

Co-authored-by: Marc Mueller <30130371+cdce8p@users.noreply.github.com>

.coveragerc

@@ -998,6 +998,7 @@ omit =
     homeassistant/components/russound_rio/media_player.py
     homeassistant/components/russound_rnet/media_player.py
     homeassistant/components/rympro/__init__.py
+    homeassistant/components/rympro/coordinator.py
     homeassistant/components/rympro/sensor.py
     homeassistant/components/sabnzbd/__init__.py
     homeassistant/components/sabnzbd/sensor.py

CODEOWNERS

@@ -1003,8 +1003,8 @@ build.json @home-assistant/supervisor
 /tests/components/ruuvi_gateway/ @akx
 /homeassistant/components/ruuvitag_ble/ @akx
 /tests/components/ruuvitag_ble/ @akx
-/homeassistant/components/rympro/ @OnFreund
-/tests/components/rympro/ @OnFreund
+/homeassistant/components/rympro/ @OnFreund @elad-bar @maorcc
+/tests/components/rympro/ @OnFreund @elad-bar @maorcc
 /homeassistant/components/sabnzbd/ @shaiu
 /tests/components/sabnzbd/ @shaiu
 /homeassistant/components/safe_mode/ @home-assistant/core

homeassistant/components/rympro/__init__.py

@@ -1,21 +1,19 @@
 """The Read Your Meter Pro integration."""
 from __future__ import annotations
 
-from datetime import timedelta
 import logging
 
-from pyrympro import CannotConnectError, OperationError, RymPro, UnauthorizedError
+from pyrympro import CannotConnectError, RymPro, UnauthorizedError
 
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.const import CONF_EMAIL, CONF_PASSWORD, CONF_TOKEN, Platform
 from homeassistant.core import HomeAssistant
 from homeassistant.exceptions import ConfigEntryAuthFailed, ConfigEntryNotReady
 from homeassistant.helpers.aiohttp_client import async_get_clientsession
-from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed
 
 from .const import DOMAIN
+from .coordinator import RymProDataUpdateCoordinator
 
-SCAN_INTERVAL = 60 * 60
 PLATFORMS: list[Platform] = [Platform.SENSOR]
 _LOGGER = logging.getLogger(__name__)
 
@@ -32,14 +30,14 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
     except UnauthorizedError:
         try:
             token = await rympro.login(data[CONF_EMAIL], data[CONF_PASSWORD], "ha")
-            hass.config_entries.async_update_entry(
-                entry,
-                data={**data, CONF_TOKEN: token},
-            )
         except UnauthorizedError as error:
             raise ConfigEntryAuthFailed from error
+        hass.config_entries.async_update_entry(
+            entry,
+            data={**data, CONF_TOKEN: token},
+        )
 
-    coordinator = RymProDataUpdateCoordinator(hass, rympro, SCAN_INTERVAL)
+    coordinator = RymProDataUpdateCoordinator(hass, rympro)
     await coordinator.async_config_entry_first_refresh()
 
     hass.data.setdefault(DOMAIN, {})
@@ -56,27 +54,3 @@ async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
         hass.data[DOMAIN].pop(entry.entry_id)
 
     return unload_ok
-
-
-class RymProDataUpdateCoordinator(DataUpdateCoordinator):
-    """Class to manage fetching RYM Pro data."""
-
-    def __init__(self, hass: HomeAssistant, rympro: RymPro, scan_interval: int) -> None:
-        """Initialize global RymPro data updater."""
-        self.rympro = rympro
-        interval = timedelta(seconds=scan_interval)
-        super().__init__(
-            hass,
-            _LOGGER,
-            name=DOMAIN,
-            update_interval=interval,
-        )
-
-    async def _async_update_data(self):
-        """Fetch data from Rym Pro."""
-        try:
-            return await self.rympro.last_read()
-        except UnauthorizedError:
-            await self.hass.config_entries.async_reload(self.config_entry.entry_id)
-        except (CannotConnectError, OperationError) as error:
-            raise UpdateFailed(error) from error

homeassistant/components/rympro/coordinator.py

@@ -0,0 +1,42 @@
+"""The Read Your Meter Pro integration."""
+from __future__ import annotations
+
+from datetime import timedelta
+import logging
+
+from pyrympro import CannotConnectError, OperationError, RymPro, UnauthorizedError
+
+from homeassistant.core import HomeAssistant
+from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed
+
+from .const import DOMAIN
+
+SCAN_INTERVAL = 60 * 60
+
+_LOGGER = logging.getLogger(__name__)
+
+
+class RymProDataUpdateCoordinator(DataUpdateCoordinator[dict[int, dict]]):
+    """Class to manage fetching RYM Pro data."""
+
+    def __init__(self, hass: HomeAssistant, rympro: RymPro) -> None:
+        """Initialize global RymPro data updater."""
+        self.rympro = rympro
+        interval = timedelta(seconds=SCAN_INTERVAL)
+        super().__init__(
+            hass,
+            _LOGGER,
+            name=DOMAIN,
+            update_interval=interval,
+        )
+
+    async def _async_update_data(self) -> dict[int, dict]:
+        """Fetch data from Rym Pro."""
+        try:
+            return await self.rympro.last_read()
+        except UnauthorizedError as error:
+            assert self.config_entry
+            await self.hass.config_entries.async_reload(self.config_entry.entry_id)
+            raise UpdateFailed(error) from error
+        except (CannotConnectError, OperationError) as error:
+            raise UpdateFailed(error) from error

homeassistant/components/rympro/manifest.json

@@ -4,6 +4,6 @@
   "config_flow": true,
   "documentation": "https://www.home-assistant.io/integrations/rympro",
   "requirements": ["pyrympro==0.0.4"],
-  "codeowners": ["@OnFreund"],
+  "codeowners": ["@OnFreund", "@elad-bar", "@maorcc"],
   "iot_class": "cloud_polling"
 }

homeassistant/components/rympro/sensor.py

@@ -8,14 +8,13 @@
 )
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.const import UnitOfVolume
-from homeassistant.core import HomeAssistant, callback
-from homeassistant.helpers import entity_registry as er
+from homeassistant.core import HomeAssistant
 from homeassistant.helpers.entity import DeviceInfo
 from homeassistant.helpers.entity_platform import AddEntitiesCallback
 from homeassistant.helpers.update_coordinator import CoordinatorEntity
 
-from . import RymProDataUpdateCoordinator
 from .const import DOMAIN
+from .coordinator import RymProDataUpdateCoordinator
 
 
 async def async_setup_entry(
@@ -26,18 +25,16 @@ async def async_setup_entry(
     """Set up sensors for device."""
     coordinator: RymProDataUpdateCoordinator = hass.data[DOMAIN][config_entry.entry_id]
     async_add_entities(
-        [
-            RymProSensor(coordinator, meter_id, meter["read"], config_entry.entry_id)
-            for meter_id, meter in coordinator.data.items()
-        ]
+        RymProSensor(coordinator, meter_id, meter["read"], config_entry.entry_id)
+        for meter_id, meter in coordinator.data.items()
     )
 
 
 class RymProSensor(CoordinatorEntity[RymProDataUpdateCoordinator], SensorEntity):
     """Sensor for RymPro meters."""
 
     _attr_has_entity_name = True
-    _attr_name = "Last Read"
+    _attr_name = "Total consumption"
     _attr_device_class = SensorDeviceClass.WATER
     _attr_native_unit_of_measurement = UnitOfVolume.CUBIC_METERS
     _attr_state_class = SensorStateClass.TOTAL_INCREASING
@@ -52,9 +49,8 @@ def __init__(
         """Initialize sensor."""
         super().__init__(coordinator)
         self._meter_id = meter_id
-        self._entity_registry: er.EntityRegistry | None = None
         unique_id = f"{entry_id}_{meter_id}"
-        self._attr_unique_id = f"{unique_id}_last_read"
+        self._attr_unique_id = f"{unique_id}_total_consumption"
         self._attr_extra_state_attributes = {"meter_id": str(meter_id)}
         self._attr_device_info = DeviceInfo(
             identifiers={(DOMAIN, unique_id)},
@@ -63,8 +59,7 @@ def __init__(
         )
         self._attr_native_value = last_read
 
-    @callback
-    def _handle_coordinator_update(self) -> None:
-        """Handle updated data from the coordinator."""
-        self._attr_native_value = self.coordinator.data[self._meter_id]["read"]
-        self.async_write_ha_state()
+    @property
+    def native_value(self) -> float | None:
+        """Return the state of the sensor."""
+        return self.coordinator.data[self._meter_id]["read"]

tests/components/rympro/test_config_flow.py

@@ -96,6 +96,30 @@ async def test_login_error(hass, exception, error):
     assert result2["type"] == FlowResultType.FORM
     assert result2["errors"] == {"base": error}
 
+    with patch(
+        "homeassistant.components.rympro.config_flow.RymPro.login",
+        return_value="test-token",
+    ), patch(
+        "homeassistant.components.rympro.config_flow.RymPro.account_info",
+        return_value={"accountNumber": TEST_DATA[CONF_UNIQUE_ID]},
+    ), patch(
+        "homeassistant.components.rympro.async_setup_entry",
+        return_value=True,
+    ) as mock_setup_entry:
+        result3 = await hass.config_entries.flow.async_configure(
+            result2["flow_id"],
+            {
+                CONF_EMAIL: TEST_DATA[CONF_EMAIL],
+                CONF_PASSWORD: TEST_DATA[CONF_PASSWORD],
+            },
+        )
+        await hass.async_block_till_done()
+
+    assert result3["type"] == FlowResultType.CREATE_ENTRY
+    assert result3["title"] == TEST_DATA[CONF_EMAIL]
+    assert result3["data"] == TEST_DATA
+    assert len(mock_setup_entry.mock_calls) == 1
+
 
 async def test_form_already_exists(hass, _config_entry):
     """Test that a flow with an existing account aborts."""