Merge pull request #35 from AndrewTapp/dev

v2.4.13

Author: AndrewTapp

README.md

@@ -54,7 +54,7 @@ Your solar system is organised in a simple hierarchy. Device and entity names in
 
 - **Voltage**, **Current**, **Optimizer voltage**, **Power** – Live values when the optimizer is reporting.
 - **Temperature** – Optimizer temperature from the SolarEdge One API (layout/energy by-inverter with `include-max-temperature`). The portal may report in °C or °F (`temperatureUnit`); the integration normalizes to °C for storage and Home Assistant displays in your preferred unit. Only available when using the One API; shown as “unknown” when missing or when using the legacy API. When the integration is not doing a full refresh (e.g. reusing data after a light check), it still refreshes temperatures when the temperature cache expires (30 minutes, TEMPERATURE_CACHE_TTL), so temperature stays up to date even when power/voltage are not updating.
-- **Lifetime energy** – Total energy produced (kWh); this only goes up over time. The integration uses the API’s raw energy value (unscaledEnergy, in Wh) so it updates correctly regardless of how the portal displays units (Wh/kWh/MWh). When optimizer-level lifetime data is reliable, site lifetime is the sum of optimizers; when it is not (e.g. mixed or missing data), the site uses the portal’s total directly.
+- **Lifetime energy** – Total energy produced (kWh); this only goes up over time. The integration uses the API’s raw energy value (unscaledEnergy, in Wh) so it updates correctly regardless of how the portal displays units (Wh/kWh/MWh). Site lifetime is the sum of inverters when aggregated data is reliable, or the portal total when unreliable (e.g. below `RELIABLE_THRESHOLD_KWH`). String totals are derived by summing that string's optimizer entries, not from API string-level keys that can be site totals.
 - **Last measurement** – When the portal last had a reading for this optimizer.
 - **Status** – The optimizer's status from the API. Blank (empty) status is treated as active and displayed as **blank** with the active icon. Values are shown in proper case: "Active", "Inactive", or the raw value for any other status. The icon changes based on status: check-circle for Active or blank, alert-circle for Inactive, help-circle for unknown (any other) status.
 - **Azimuth** – The panel's compass direction in degrees (0–360°), converted from radians. Only available when the API provides module orientation data. Icon: compass.
@@ -80,7 +80,7 @@ Names are kept short (e.g. “Current (average)”, “Power”) because the dev
 - The **coordinator** runs every **5 minutes** (`UPDATE_DELAY`). It does a lightweight check (one or a few optimizers) to see if the portal has new readings. When data is **fresh**, the desired interval between light checks is about **5 minutes**; when data is **stale or missing**, about **30 minutes**. When the light check detects new data, a full refresh runs so all sensors update; a full refresh is not triggered again within **5 minutes** of the last one (`LIGHT_CHECK_MIN_INTERVAL`). When using **SolarEdge One**, up to `LIGHT_CHECK_BATCH_SIZE` (5) optimizers are chosen at random for each check so different orientations and shade don't block updates; when falling back to the legacy API, a single representative optimizer is used for the light check. When data is currently from the **legacy** API, the integration also forces a full refresh **every 30 minutes** so it re-tries the SolarEdge One API and can switch back to One when it becomes available again.
 - **Lifetime energy** is only refreshed from the portal about **once per hour** (`LIFETIME_ENERGY_CACHE_TTL`), because that value changes slowly. It is derived from the API’s unscaled energy (Wh), not the display units, so values update correctly. Totals for strings, inverters, and the site are calculated from that data.
 
-- **Layout (panels)** is cached for **2 hours** when using SolarEdge One (`PANELS_CACHE_TTL_ONE`) and **1 hour** when using the legacy API (`PANELS_CACHE_TTL_LEGACY`).
+- **Layout (panels)** is cached for **2 hours** when using SolarEdge One (`PANELS_CACHE_TTL_ONE`) or the legacy API (`PANELS_CACHE_TTL_LEGACY`).
 
 - **Temperature** (SolarEdge One only): When the integration does not perform a full refresh (e.g. it reuses existing data after a light check), it still refreshes optimizer temperatures when the temperature cache expires (**30 minutes**, `TEMPERATURE_CACHE_TTL`). So temperature sensors stay updated even when power, voltage, and current are not being refreshed.
 
@@ -176,6 +176,8 @@ logger:
 
 Restart Home Assistant for the change to take effect. Debug logging covers the full lifecycle: config flow (user form, validation, unique_id check, entry creation, reauth form, options/reconfigure form when showing or saving — including entity_id_prefix, include_site_id_in_entity_id, use_solaredge_one — removal), setup and unload (dual API with use_solaredge_one, coordinator, platform forward, API session close, registry cleanup), coordinator updates (inverter models fetch, device creation with model and suffix, adaptive polling with configurable batch size (`LIGHT_CHECK_BATCH_SIZE`), full refresh vs reuse, obtained_from source, revert-to-One retry every 30 min when data from legacy, representative optimizers or random batch, lifetime energy entries, inactive device skipping with status, duplicate position resolution with suffix assignment, string/inverter/site aggregated data creation with status and child counts, refresh strategy determination, update complete), sensor setup (base_name, include_site_id, per-optimizer serial/model/panel_type/status, duplicate optimizer position resolution with suffixes, inactive device sensor skipping, aggregated sensors with status, obtained_from sensor, entity count, device status summary when inactive devices exist, status value updates), and API requests (SolarEdge One: OAuth, token, GET/POST to /services/ with configurable timeouts (`API_TIMEOUT_SHORT`/`API_TIMEOUT_LONG`), requestAllData (single batch for all optimizers; per-optimizer fallback when batch fails), optimizer/inverter information including panel type (description), cache hit/miss, optimizer temperatures with unit and F→°C conversion when portal sends Fahrenheit; legacy: login, layout, system data, lifetime energy; dual API: use_solaredge_one/legacy-only, fallback to legacy when One has no valid measurements or fails). All debug output is guarded with `isEnabledFor(logging.DEBUG)`, so there is no performance cost when the log level is `info`. Debug messages use consistent prefixes (e.g. `SolarEdge Optimizers`, `SolarEdge Optimizers coordinator`, `SolarEdge Optimizers sensor`, `SolarEdge One`, `SolarEdge Optimizers (legacy)`, `SolarEdge Dual API`) so you can filter logs easily. Turn logging back to `info` when you are done.
 
+**Code quality:** The project uses Pylint and pycodestyle (e.g. via CodeFactor). See `pyproject.toml` for tool config and `CODEQUALITY.md` for intentional patterns and suppressions.
+
 ## Translations
 
 The integration is localized for multiple languages: config flow (labels, errors, entry title), sensor and device names, and API locale follow the user’s Home Assistant language where supported. See [Internationalization (i18n)](https://github.com/AndrewTapp/solaredgeoptimizers/blob/main/docs/internationalization.md) for details.

custom_components/solaredgeoptimizers/__init__.py

@@ -276,27 +276,34 @@ async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
     if LOGGER.isEnabledFor(logging.DEBUG):
         LOGGER.debug("SolarEdge Optimizers: Unloading config entry %s", entry.entry_id)
     if unload_ok := await hass.config_entries.async_unload_platforms(entry, PLATFORMS):
-        # Remove from entity and device registries so no leftovers after delete
-        try:
-            remove_entities_and_devices_for_entry(hass, entry)
-        except Exception as e:  # pylint: disable=broad-except
-            LOGGER.warning(
-                "SolarEdge Optimizers: Error cleaning registries during unload: %s",
-                e,
-            )
-        # Added cleanup of coordinator resources
+        # Pop coordinator first so we always close its API (release file descriptors) even if cleanup fails
         coordinator = hass.data[DOMAIN].pop(entry.entry_id, None)
-        # Close API sessions to prevent file descriptor leaks
-        if coordinator and hasattr(coordinator, "my_api"):
+        try:
+            # Remove from entity and device registries so no leftovers after delete
             try:
-                await hass.async_add_executor_job(coordinator.my_api.close)
+                remove_entities_and_devices_for_entry(hass, entry)
+            except Exception as e:  # pylint: disable=broad-except
+                LOGGER.warning(
+                    "SolarEdge Optimizers: Error cleaning registries during unload: %s",
+                    e,
+                )
+        finally:
+            # Always close API sessions to release all file descriptors (legacy Session pool, etc.)
+            if coordinator is not None and hasattr(coordinator, "my_api"):
                 if LOGGER.isEnabledFor(logging.DEBUG):
                     LOGGER.debug(
-                        "SolarEdge Optimizers: Closed API session for entry %s during unload",
+                        "SolarEdge Optimizers: Unload finally: closing API sessions for entry %s",
                         entry.entry_id,
                     )
-            except Exception as e:  # pylint: disable=broad-except
-                LOGGER.warning("SolarEdge Optimizers: Error closing API sessions: %s", e)
+                try:
+                    await hass.async_add_executor_job(coordinator.my_api.close)
+                    if LOGGER.isEnabledFor(logging.DEBUG):
+                        LOGGER.debug(
+                            "SolarEdge Optimizers: Closed API session for entry %s during unload",
+                            entry.entry_id,
+                        )
+                except Exception as e:  # pylint: disable=broad-except
+                    LOGGER.warning("SolarEdge Optimizers: Error closing API sessions: %s", e)
     else:
         if LOGGER.isEnabledFor(logging.DEBUG):
             LOGGER.debug(

custom_components/solaredgeoptimizers/api_dual.py

@@ -170,6 +170,8 @@ def close(self) -> None:
         """Close both API clients and release all file descriptors (sessions, connection pools).
         Idempotent; safe to call multiple times. Both clients are always closed even if one raises.
         """
+        if _LOGGER.isEnabledFor(logging.DEBUG):
+            _LOGGER.debug("SolarEdge Dual API: Closing both API clients (One and Legacy)")
         for name, client in [("One", self._one), ("Legacy", self._legacy)]:
             try:
                 client.close()

custom_components/solaredgeoptimizers/config_flow.py

@@ -288,11 +288,16 @@ async def async_remove_entry(
         # Close API so all sessions/connection pools are released (in case unload did not run or did not close)
         coordinator = hass.data.get(DOMAIN, {}).pop(entry.entry_id, None)
         if coordinator is not None and hasattr(coordinator, "my_api"):
+            if _LOGGER.isEnabledFor(logging.DEBUG):
+                _LOGGER.debug(
+                    "SolarEdge Optimizers config: Closing API on config entry removal for entry %s",
+                    entry.entry_id,
+                )
             try:
                 await hass.async_add_executor_job(coordinator.my_api.close)
                 if _LOGGER.isEnabledFor(logging.DEBUG):
                     _LOGGER.debug(
-                        "SolarEdge Optimizers: Closed API on config entry removal for entry %s",
+                        "SolarEdge Optimizers config: Closed API on config entry removal for entry %s",
                         entry.entry_id,
                     )
             except Exception as e:  # pylint: disable=broad-except
@@ -317,7 +322,9 @@ def __init__(self, entry: ConfigEntry) -> None:
         """Initialize options flow."""
         self._entry = entry
 
-    def async_show_form(self, *, step_id=None, data_schema=None, errors=None, description_placeholders=None, last_step=None, preview=None):
+    def async_show_form(  # pylint: disable=too-many-arguments
+        self, *, step_id=None, data_schema=None, errors=None, description_placeholders=None, last_step=None, preview=None
+    ):
         """Show form and ensure frontend uses this integration's translations (options.step.init.data.*)."""
         result = super().async_show_form(
             step_id=step_id,

custom_components/solaredgeoptimizers/coordinator.py

@@ -270,21 +270,27 @@ async def _fetch_inverter_models(self, site) -> None:
             _LOGGER.warning("SolarEdge Optimizers: Could not fetch inverter models: %s", e)
 
     def _build_lifetime_energy_lookup(self, lifetime_energy_data):
-        """Build stringId -> energy_data lookup; derive string total from optimizer sum when needed."""
+        """Build stringId -> energy_data lookup; derive string total from optimizer sum when needed.
+
+        Prefer the sum of this string's optimizer entries over any API string-level key. The legacy
+        layout/energy response can contain a key that matches a stringId but holds a site- or
+        inverter-level total; using that would inflate one string and (with multiple inverters or
+        duplicate layout entries) produce a grossly inflated site total and double-counting.
+        """
         lifetime_energy_lookup = {}
         for inv in self._site_structure.inverters:
             for s in inv.strings:
-                key = str(s.stringId)
-                if key in lifetime_energy_data:
-                    lifetime_energy_lookup[s.stringId] = lifetime_energy_data[key]
+                total_wh = 0.0
+                for opt in s.optimizers:
+                    ent = lifetime_energy_data.get(str(opt.optimizerId)) or lifetime_energy_data.get(opt.optimizerId)
+                    if ent and isinstance(ent.get("unscaledEnergy"), (int, float)):
+                        total_wh += float(ent["unscaledEnergy"])
+                if total_wh > 0:
+                    lifetime_energy_lookup[s.stringId] = {"unscaledEnergy": total_wh}
                 else:
-                    total_wh = 0.0
-                    for opt in s.optimizers:
-                        ent = lifetime_energy_data.get(str(opt.optimizerId)) or lifetime_energy_data.get(opt.optimizerId)
-                        if ent and isinstance(ent.get("unscaledEnergy"), (int, float)):
-                            total_wh += float(ent["unscaledEnergy"])
-                    if total_wh > 0:
-                        lifetime_energy_lookup[s.stringId] = {"unscaledEnergy": total_wh}
+                    key = str(s.stringId)
+                    if key in lifetime_energy_data:
+                        lifetime_energy_lookup[s.stringId] = lifetime_energy_data[key]
         return lifetime_energy_lookup
 
     def _aggregate_optimizers_in_string(self, string, data_dict, timetocheck):
@@ -516,7 +522,11 @@ def _register_inverter_and_string_devices(
                 via_device=(DOMAIN, inv_device_id),
             )
             if _LOGGER.isEnabledFor(logging.DEBUG):
-                _LOGGER.debug("Created device for string: %s (suffix=%s)", string_name, str_suffix or "(none)")
+                _LOGGER.debug(
+                    "SolarEdge Optimizers: Created device for string: %s (suffix=%s)",
+                    string_name,
+                    str_suffix or "(none)",
+                )
 
     def ensure_devices_registered(self) -> None:
         """Ensure site, inverter, and string devices exist in the device registry.
@@ -842,6 +852,12 @@ def _calculate_aggregated_data(
         Child counts (optimizer count, string count, inverter count) count only active (blank or ACTIVE) devices.
         """
         lifetime_energy_lookup = self._build_lifetime_energy_lookup(lifetime_energy_data)
+        if _LOGGER.isEnabledFor(logging.DEBUG):
+            _LOGGER.debug(
+                "SolarEdge Optimizers coordinator: Lifetime energy lookup built with %d string(s) for site %s",
+                len(lifetime_energy_lookup),
+                site_id,
+            )
         site_id_str = str(site_id)
         site = SiteRollupState(
             current=0.0, power=0.0, voltage_sum=0.0, voltage_count=0,
@@ -864,6 +880,13 @@ def _calculate_aggregated_data(
             and portal_site_lifetime_kwh >= RELIABLE_THRESHOLD_KWH
             and site.lifetime_energy < RELIABLE_THRESHOLD_KWH
         ):
+            if _LOGGER.isEnabledFor(logging.DEBUG):
+                _LOGGER.debug(
+                    "SolarEdge Optimizers coordinator: Site %s using portal lifetime (aggregated=%.3f kWh < threshold, portal=%.3f kWh)",
+                    site_id,
+                    site.lifetime_energy,
+                    portal_site_lifetime_kwh,
+                )
             site = site._replace(lifetime_energy=portal_site_lifetime_kwh)
 
         site_aggregated = self._create_site_aggregated(site_id, site, current_utc)
@@ -977,7 +1000,7 @@ async def _run_light_check(self, now_utc, latest_measurement) -> bool:
             return self._light_check_should_trigger_full_refresh(rep_list, latest_measurement, now_utc)
         except Exception as e:  # pylint: disable=broad-except
             if _LOGGER.isEnabledFor(logging.DEBUG):
-                _LOGGER.debug("Lightweight update check failed: %s", e)
+                _LOGGER.debug("SolarEdge Optimizers coordinator: Lightweight update check failed: %s", e)
             return False
 
     def _index_optimizers_by_position(self, data_dict: dict) -> None:

custom_components/solaredgeoptimizers/docs/SolarEdge-One-API-Summary.md

@@ -36,7 +36,7 @@ A site-level sensor **Obtained from** shows whether current data came from **"On
 - **Auth**: Single OAuth flow and Bearer token instead of mixing Basic Auth and session cookies.
 - **Data**: Structured JSON (e.g. `power_W`, `voltage_V`, `optimizerVoltage_V`) instead of locale-dependent labels; layout in a single v2 structure.
 - **Devices**: Optimizer and inverter **model** (and serial) come from the API, so HA devices can show real model names (e.g. P405-4RM4MRM-NA25, SE5000H-RW000BNN4). When the API provides a **panel type** (description, e.g. SunPower SPR-MAX3-400), it is included in the optimizer device model and exposed as a **panel_type** attribute on optimizer sensors.
-- **Polling**: Lightweight “any new data?” check can use a batch request (up to `LIGHT_CHECK_BATCH_SIZE` = 5 optimizers) instead of one panel; 1-hour stale threshold for live values (legacy used 2 hours). **Full refresh** fetches all optimizer live data in **one batch POST** (`requestSystemDataBatch` with all serials), reducing portal load; if that batch fails (e.g. 5xx), the integration falls back to per-optimizer requests.
+- **Polling**: Lightweight “any new data?” check can use a batch request (up to `LIGHT_CHECK_BATCH_SIZE` = 5 optimizers) instead of one panel; 1-hour stale threshold for live values (legacy uses 2 hours). Site layout (panels) is cached for **2 hours** (`PANELS_CACHE_TTL_ONE`) when using SolarEdge One. **Full refresh** fetches all optimizer live data in **one batch POST** (`requestSystemDataBatch` with all serials), reducing portal load; if that batch fails (e.g. 5xx), the integration falls back to per-optimizer requests.
 
 ## In this integration