Refactor Rinnai integration for improved error handling, maintenance timers, and service definitions

Author: explosivo22

.github/workflows/dev-local-validate.yml

@@ -0,0 +1,51 @@
+name: Validate Local Polling Integration (dev-local)
+
+on:
+  push:
+    branches:
+      - dev-local
+  pull_request:
+    branches:
+      - dev-local
+  workflow_dispatch:
+
+permissions:
+  contents: read
+
+jobs:
+  validate-hacs:
+    name: HACS Validation
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v4
+
+      - name: HACS validation
+        uses: hacs/action@main
+        with:
+          category: integration
+
+  validate-hassfest:
+    name: Hassfest Validation
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v4
+
+      - name: Rename directory to match domain
+        run: |
+          mv custom_components/rinnaicontrolr-ha custom_components/rinnai
+
+      - name: Set up Python
+        uses: actions/setup-python@v5
+        with:
+          python-version: "3.12"
+          cache: "pip"
+
+      - name: Install Home Assistant
+        run: |
+          pip install homeassistant
+
+      - name: Run Hassfest
+        run: |
+          python -m script.hassfest --integration-path custom_components/rinnai

custom_components/rinnaicontrolr-ha/__init__.py

@@ -11,7 +11,6 @@
 from homeassistant.core import HomeAssistant
 from homeassistant.exceptions import ConfigEntryNotReady
 from homeassistant.helpers.aiohttp_client import async_get_clientsession
-from homeassistant.components.water_heater import DOMAIN as WATER_HEATER_DOMAIN
 
 from .const import (
     CONF_MAINT_REFRESH_INTERVAL,
@@ -30,36 +29,85 @@
 PLATFORMS = ["water_heater","binary_sensor", "sensor"]
 
 async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
-    """Set up Rinnai from config entry"""
+    """Set up Rinnai from config entry."""
     session = async_get_clientsession(hass)
     hass.data.setdefault(DOMAIN, {})
     hass.data[DOMAIN][entry.entry_id] = {}
 
     waterHeater = WaterHeater(entry.data[CONF_HOST])
+    
+    # Attempt to fetch system information from the controller
+    # This is required during setup to obtain device metadata (serial number, model)
+    # needed for device registry and coordinator initialization
     sysinfo = await waterHeater.get_sysinfo()
 
-    # Gracefully handle controller offline or unreachable
-    if not sysinfo or "sysinfo" not in sysinfo or "serial-number" not in sysinfo["sysinfo"]:
-        _LOGGER.error("Could not connect to Rinnai controller or missing sysinfo; will retry later.")
-        raise ConfigEntryNotReady("Rinnai controller is offline or unreachable.")
+    # Validate sysinfo response structure following HA best practices
+    # Raising ConfigEntryNotReady will trigger automatic retry by Home Assistant
+    if sysinfo is None:
+        _LOGGER.warning(
+            "Unable to connect to Rinnai controller at %s; setup will be retried automatically",
+            entry.data[CONF_HOST],
+        )
+        raise ConfigEntryNotReady(
+            f"Unable to connect to Rinnai controller at {entry.data[CONF_HOST]}"
+        )
+    
+    if not isinstance(sysinfo, dict):
+        _LOGGER.warning(
+            "Invalid response from Rinnai controller at %s; setup will be retried automatically",
+            entry.data[CONF_HOST],
+        )
+        raise ConfigEntryNotReady(
+            f"Invalid response from Rinnai controller at {entry.data[CONF_HOST]}"
+        )
+    
+    # Safely check nested structure
+    sysinfo_data = sysinfo.get("sysinfo")
+    if not isinstance(sysinfo_data, dict):
+        _LOGGER.warning(
+            "Missing system information from Rinnai controller at %s; setup will be retried automatically",
+            entry.data[CONF_HOST],
+        )
+        raise ConfigEntryNotReady(
+            f"Missing system information from Rinnai controller at {entry.data[CONF_HOST]}"
+        )
+    
+    serial_number = sysinfo_data.get("serial-number")
+    if not serial_number:
+        _LOGGER.warning(
+            "Missing serial number from Rinnai controller at %s; setup will be retried automatically",
+            entry.data[CONF_HOST],
+        )
+        raise ConfigEntryNotReady(
+            f"Missing serial number from Rinnai controller at {entry.data[CONF_HOST]}"
+        )
+    
+    _LOGGER.info(
+        "Successfully connected to Rinnai controller at %s (Serial: %s)",
+        entry.data[CONF_HOST],
+        serial_number,
+    )
 
     update_interval = entry.options.get(CONF_REFRESH_INTERVAL, DEFAULT_REFRESH_INTERVAL)
 
     maint_refresh_interval = entry.options.get(CONF_MAINT_REFRESH_INTERVAL, DEFAULT_MAINT_REFRESH_INTERVAL)
 
+    # Extract safely validated values
+    ayla_dsn = sysinfo_data.get("ayla-dsn", "unknown")
+
     coordinator = RinnaiDeviceDataUpdateCoordinator(
             hass, 
             entry.data[CONF_HOST],
-            sysinfo["sysinfo"]["serial-number"],
-            sysinfo["sysinfo"]["serial-number"],
-            sysinfo["sysinfo"].get("ayla-dsn", "unknown"),  # Use get method with default value
+            serial_number,
+            serial_number,
+            ayla_dsn,
             update_interval, 
             entry.options
         )
 
     coordinator.maint_refresh_interval = timedelta(seconds=maint_refresh_interval)
 
-    await coordinator.async_refresh()
+    await coordinator.async_config_entry_first_refresh()
 
     if not entry.options:
         await _async_options_updated(hass, entry)
@@ -68,18 +116,29 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
         COORDINATOR: coordinator
     }
 
+    # Start maintenance timer after coordinator is fully initialized
+    coordinator.start_maintenance_timer()
+
     await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS)
 
     entry.async_on_unload(entry.add_update_listener(_async_options_updated))
 
+    # Ensure maintenance timer is stopped when entry is unloaded
+    entry.async_on_unload(coordinator.stop_maintenance_timer)
+    
     return True
 
 async def _async_options_updated(hass: HomeAssistant, entry: ConfigEntry):
     """Update options."""
+    # Reload the integration to apply new settings including maintenance interval changes
     await hass.config_entries.async_reload(entry.entry_id)
 
 async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry):
     """Unload a config entry."""
+    # Stop maintenance timer before unloading
+    coordinator = hass.data[DOMAIN][entry.entry_id][COORDINATOR]
+    coordinator.stop_maintenance_timer()
+    
     unload_ok = await hass.config_entries.async_unload_platforms(entry, PLATFORMS)
     if unload_ok:
         hass.data[DOMAIN].pop(entry.entry_id)

custom_components/rinnaicontrolr-ha/device.py

@@ -4,10 +4,10 @@
 
 import async_timeout
 
-from homeassistant.core import HomeAssistant
+from homeassistant.core import HomeAssistant, callback
 from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed
 from homeassistant.util import Throttle
-from homeassistant.helpers.event import async_track_time_interval
+from homeassistant.helpers.event import async_call_later, async_track_time_interval
 
 from .rinnai import WaterHeater
 
@@ -35,7 +35,9 @@ def __init__(
 		self._device_info: Optional[Dict[str, Any]] | None = None
 		self.options = options
 		self.maint_refresh_interval = timedelta(seconds=self.options.get(CONF_MAINT_REFRESH_INTERVAL, DEFAULT_MAINT_REFRESH_INTERVAL))
-		self._unsub_maintenance_timer = None  # Track the maintenance timer
+		self._unsub_maintenance_timer = None  # Track the recurring maintenance timer
+		self._unsub_maintenance_delay = None  # Track the initial delay timer
+		self._maintenance_enabled = self.options.get(CONF_MAINT_INTERVAL_ENABLED, False)
 		super().__init__(
 			hass,
 			LOGGER,
@@ -397,33 +399,79 @@ async def async_do_maintenance_retrieval(self, _event=None):
 	async def _update_device(self, *_) -> None:
 		"""Update the device information from the API"""
 		self._device_info = await self.waterHeater.get_status()
+		LOGGER.debug("Rinnai device data: %s", self._device_info)
+
+	def start_maintenance_timer(self):
+		"""Start the maintenance retrieval timer if enabled.
+		
+		Note: The first maintenance retrieval will occur after the configured interval,
+		not immediately. This prevents unnecessary API calls during initialization.
+		"""
+		# Cancel any existing timers first
+		if self._unsub_maintenance_delay:
+			self._unsub_maintenance_delay()
+			self._unsub_maintenance_delay = None
+		if self._unsub_maintenance_timer:
+			self._unsub_maintenance_timer()
+			self._unsub_maintenance_timer = None
 
-		# Handle dynamic maintenance update interval
-		if self.options[CONF_MAINT_INTERVAL_ENABLED]:
-			# Cancel previous maintenance update timer if it exists
-			if self._unsub_maintenance_timer:
-				self._unsub_maintenance_timer()
+		# Only set up timer if maintenance is enabled
+		if self.options.get(CONF_MAINT_INTERVAL_ENABLED, False):
+			from datetime import datetime
+			next_run = datetime.now() + self.maint_refresh_interval
+			LOGGER.info(
+				"Maintenance retrieval scheduled. First run will occur at approximately %s (in %s seconds)",
+				next_run.strftime("%H:%M:%S"),
+				self.maint_refresh_interval.total_seconds(),
+			)
 
-			# Set new maintenance update interval
-			self._unsub_maintenance_timer = async_track_time_interval(
-				self.hass, self.async_do_maintenance_retrieval, self.maint_refresh_interval
+			# Schedule the first call after the interval delay
+			# This prevents immediate execution on startup
+			self._unsub_maintenance_delay = async_call_later(
+				self.hass,
+				self.maint_refresh_interval.total_seconds(),
+				self._start_maintenance_interval,
 			)
 		else:
-			LOGGER.debug("Skipping Maintenance retrieval since disabled inside of configuration")
+			LOGGER.debug("Maintenance retrieval disabled in configuration")
+	
+	@callback
+	def _start_maintenance_interval(self, _now=None):
+		"""Start the recurring maintenance interval timer.
 		
-		LOGGER.debug("Rinnai device data: %s", self._device_info)
-
-	async def async_added_to_hass(self):
-		"""Called when the device is added to Home Assistant"""
-		# If maintenance retrieval is enabled, initialize the timer
-		if self.options[CONF_MAINT_INTERVAL_ENABLED]:
-			self._unsub_maintenance_timer = async_track_time_interval(
-				self.hass, self.async_do_maintenance_retrieval, self.maint_refresh_interval
-            )
-
-	async def async_will_remove_from_hass(self):
-		"""Called when the device is removed from Home Assistant"""
-		# Unsubscribe from any existing maintenance update timer
+		This is called after the initial delay to set up the recurring timer.
+		"""
+		# Clear the delay timer reference since it's now complete
+		self._unsub_maintenance_delay = None
+		
+		LOGGER.info(
+			"Starting maintenance retrieval cycle. Will run every %s",
+			self.maint_refresh_interval,
+		)
+		
+		# Perform the first maintenance retrieval
+		self.hass.async_create_task(self.async_do_maintenance_retrieval())
+		
+		# Set up recurring timer for subsequent calls
+		self._unsub_maintenance_timer = async_track_time_interval(
+			self.hass, self.async_do_maintenance_retrieval, self.maint_refresh_interval
+		)
+	
+	def stop_maintenance_timer(self):
+		"""Stop the maintenance retrieval timer and any pending delayed calls"""
+		# Cancel the delayed initial call if it hasn't fired yet
+		if self._unsub_maintenance_delay:
+			self._unsub_maintenance_delay()
+			self._unsub_maintenance_delay = None
+			LOGGER.debug("Cancelled pending maintenance retrieval delay")
+		
+		# Cancel the recurring timer if it exists
 		if self._unsub_maintenance_timer:
 			self._unsub_maintenance_timer()
-			self._unsub_maintenance_timer = None
+			self._unsub_maintenance_timer = None
+			LOGGER.debug("Maintenance retrieval timer stopped")
+
+	async def async_shutdown(self):
+		"""Called when the coordinator is being shut down"""
+		# Clean up the maintenance timer
+		self.stop_maintenance_timer()

custom_components/rinnaicontrolr-ha/entity.py

@@ -1,10 +1,10 @@
-"""Base entity class for Flo entities."""
+"""Base entity class for Rinnai entities."""
 from __future__ import annotations
 
 from typing import Any
 
-from homeassistant.helpers.device_registry import CONNECTION_NETWORK_MAC
-from homeassistant.helpers.entity import DeviceInfo, Entity
+from homeassistant.helpers.device_registry import DeviceInfo
+from homeassistant.helpers.entity import Entity
 
 from .const import DOMAIN as RINNAI_DOMAIN
 from .device import RinnaiDeviceDataUpdateCoordinator
@@ -19,7 +19,7 @@ def __init__(
         self,
         entity_type: str,
         name: str,
-        device: RinnaiDeviceUpdateCoordinator,
+        device: RinnaiDeviceDataUpdateCoordinator,
         **kwargs,
     ) -> None:
         """Init Rinnai entity."""
@@ -28,11 +28,8 @@ def __init__(
 
         self._device: RinnaiDeviceDataUpdateCoordinator = device
         self._state: Any = None
-
-    @property
-    def device_info(self) -> DeviceInfo:
-        """Return a device description for device registry."""
-        return DeviceInfo(
+        
+        self._attr_device_info = DeviceInfo(
             identifiers={(RINNAI_DOMAIN, self._device.serial_number)},
             manufacturer=self._device.manufacturer,
             model=self._device.model,

custom_components/rinnaicontrolr-ha/manifest.json

@@ -4,7 +4,7 @@
     "codeowners": [ "@explosivo22" ],
     "config_flow": true,
     "documentation": "https://github.com/explosivo22/rinnaicontrolr-ha/",
-    "iot_class": "cloud_polling",
+    "iot_class": "local_polling",
     "issue_tracker": "https://github.com/explosivo22/rinnaicontrolr-ha/issues",
     "version": "2.1.8"
 }